refactor lookups so we can easily check for not found and do the "right" thing in the repos (#42)

internal/db/read_writer.go

@@ -14,6 +14,13 @@ import (
 	"google.golang.org/protobuf/proto"
 )
 
+var (
+	// ErrRecordNotFound returns a "record not found" error and it only occurs
+	// when attempting to read from the database into struct.
+	// When reading into a slice it won't return this error.
+	ErrRecordNotFound = errors.New("record not found")
+)
+
 // Reader interface defines lookups/searching for resources
 type Reader interface {
 	// LookupByName will lookup resource by its friendly name which must be unique
@@ -396,7 +403,13 @@ func (rw *GormReadWriter) LookupByName(ctx context.Context, resource ResourceNam
 	if resource.GetName() == "" {
 		return errors.New("error name empty string for lookup by name")
 	}
-	return rw.Tx.Where("name = ?", resource.GetName()).First(resource).Error
+	if err := rw.Tx.Where("name = ?", resource.GetName()).First(resource).Error; err != nil {
+		if err == gorm.ErrRecordNotFound {
+			return ErrRecordNotFound
+		}
+		return err
+	}
+	return nil
 }
 
 // LookupByPublicId will lookup resource my its public_id which must be unique
@@ -416,7 +429,13 @@ func (rw *GormReadWriter) LookupByPublicId(ctx context.Context, resource Resourc
 	if resource.GetPublicId() == "" {
 		return errors.New("error public id empty string for lookup by public id")
 	}
-	return rw.Tx.Where("public_id = ?", resource.GetPublicId()).First(resource).Error
+	if err := rw.Tx.Where("public_id = ?", resource.GetPublicId()).First(resource).Error; err != nil {
+		if err == gorm.ErrRecordNotFound {
+			return ErrRecordNotFound
+		}
+		return err
+	}
+	return nil
 }
 
 // LookupWhere will lookup the first resource using a where clause with parameters (it only returns the first one)

internal/db/read_writer_test.go

@@ -9,7 +9,6 @@ import (
 	"github.com/hashicorp/watchtower/internal/db/db_test"
 	"github.com/hashicorp/watchtower/internal/oplog"
 	"github.com/hashicorp/watchtower/internal/oplog/store"
-	"github.com/jinzhu/gorm"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -320,7 +319,7 @@ func TestGormReadWriter_LookupByName(t *testing.T) {
 		foundUser.Name = "fn-" + id
 		err = w.LookupByName(context.Background(), foundUser)
 		assert.True(err != nil)
-		assert.Equal(gorm.ErrRecordNotFound, err)
+		assert.Equal(ErrRecordNotFound, err)
 	})
 }
 
@@ -375,7 +374,7 @@ func TestGormReadWriter_LookupByPublicId(t *testing.T) {
 		foundUser.PublicId = id
 		err = w.LookupByPublicId(context.Background(), foundUser)
 		assert.True(err != nil)
-		assert.Equal(gorm.ErrRecordNotFound, err)
+		assert.Equal(ErrRecordNotFound, err)
 	})
 }
 
@@ -416,7 +415,7 @@ func TestGormReadWriter_LookupWhere(t *testing.T) {
 		var foundUser db_test.TestUser
 		err = w.LookupWhere(context.Background(), &foundUser, "public_id = ?", id)
 		assert.True(err != nil)
-		assert.Equal(gorm.ErrRecordNotFound, err)
+		assert.Equal(ErrRecordNotFound, err)
 	})
 	t.Run("bad-where", func(t *testing.T) {
 		w := GormReadWriter{Tx: db}
@@ -633,7 +632,7 @@ func TestGormReadWriter_Delete(t *testing.T) {
 
 		err = w.LookupByPublicId(context.Background(), foundUser)
 		assert.True(err != nil)
-		assert.Equal(gorm.ErrRecordNotFound, err)
+		assert.Equal(ErrRecordNotFound, err)
 	})
 	t.Run("valid-WithOplog", func(t *testing.T) {
 		w := GormReadWriter{Tx: db}
@@ -680,7 +679,7 @@ func TestGormReadWriter_Delete(t *testing.T) {
 
 		err = w.LookupByPublicId(context.Background(), foundUser)
 		assert.True(err != nil)
-		assert.Equal(gorm.ErrRecordNotFound, err)
+		assert.Equal(ErrRecordNotFound, err)
 	})
 	t.Run("no-wrapper-WithOplog", func(t *testing.T) {
 		w := GormReadWriter{Tx: db}

internal/iam/repository_scope.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
+
+	"github.com/hashicorp/watchtower/internal/db"
 )
 
 // CreateScope will create a scope in the repository and return the written scope
@@ -30,7 +32,8 @@ func (r *Repository) UpdateScope(ctx context.Context, scope *Scope, fieldMaskPat
 	return resource.(*Scope), err
 }
 
-// LookupScope will look up a scope in the respository
+// LookupScope will look up a scope in the repository.  If the scope is not
+// found, it will return nil, nil.
 func (r *Repository) LookupScope(ctx context.Context, opt ...Option) (*Scope, error) {
 	opts := getOpts(opt...)
 	withPublicId := opts.withPublicId
@@ -40,6 +43,9 @@ func (r *Repository) LookupScope(ctx context.Context, opt ...Option) (*Scope, er
 		scope := allocScope()
 		scope.PublicId = withPublicId
 		if err := r.reader.LookupByPublicId(ctx, &scope); err != nil {
+			if err == db.ErrRecordNotFound {
+				return nil, nil
+			}
 			return nil, err
 		}
 		return &scope, nil
@@ -48,6 +54,9 @@ func (r *Repository) LookupScope(ctx context.Context, opt ...Option) (*Scope, er
 		scope := allocScope()
 		scope.Name = withName
 		if err := r.reader.LookupByName(ctx, &scope); err != nil {
+			if err == db.ErrRecordNotFound {
+				return nil, nil
+			}
 			return nil, err
 		}
 		return &scope, nil

internal/iam/repository_scope_test.go

@@ -133,3 +133,44 @@ func Test_Repository_UpdateScope(t *testing.T) {
 		assert.Equal("failed to update scope: error on update you cannot change a scope's parent", err.Error())
 	})
 }
+
+func Test_Repository_LookupScope(t *testing.T) {
+	t.Parallel()
+	cleanup, conn := db.TestSetup(t, "postgres")
+	defer cleanup()
+	assert := assert.New(t)
+	defer conn.Close()
+
+	t.Run("found-and-not-found", func(t *testing.T) {
+		rw := &db.GormReadWriter{Tx: conn}
+		wrapper := db.TestWrapper(t)
+		repo, err := NewRepository(rw, rw, wrapper)
+		id, err := uuid.GenerateUUID()
+		assert.Nil(err)
+
+		s, err := NewOrganization(WithName("fname-" + id))
+		s, err = repo.CreateScope(context.Background(), s)
+		assert.Nil(err)
+		assert.True(s != nil)
+		assert.True(s.GetPublicId() != "")
+		assert.Equal(s.GetName(), "fname-"+id)
+
+		foundScope, err := repo.LookupScope(context.Background(), WithPublicId(s.PublicId))
+		assert.Nil(err)
+		assert.Equal(foundScope.GetPublicId(), s.GetPublicId())
+
+		foundScope, err = repo.LookupScope(context.Background(), WithName("fname-"+id))
+		assert.Nil(err)
+		assert.Equal(foundScope.GetPublicId(), s.GetPublicId())
+
+		invalidId, err := uuid.GenerateUUID()
+		assert.Nil(err)
+		notFoundById, err := repo.LookupScope(context.Background(), WithPublicId(invalidId))
+		assert.Nil(err)
+		assert.Nil(notFoundById)
+
+		notFoundByName, err := repo.LookupScope(context.Background(), WithName(invalidId))
+		assert.Nil(err)
+		assert.Nil(notFoundByName)
+	})
+}