Skip to content

Commit

Permalink
Merge pull request hyperledger#93 from hyperledger/uuid-or-name
Browse files Browse the repository at this point in the history 
Add optional name/get-first semantics to CRUD
  • Loading branch information
@nguyer
nguyer authored Jul 27, 2023
2 parents 51b218b + 5d2fcf6 commit f52df06
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 6 deletions.
91 changes: 87 additions & 4 deletions pkg/dbsql/crud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ type CRUD[T Resource] interface {
Insert(ctx context.Context, inst T, hooks ...PostCompletionHook) (err error)
Replace(ctx context.Context, inst T, hooks ...PostCompletionHook) (err error)
GetByID(ctx context.Context, id string, getOpts ...GetOption) (inst T, err error)
GetByUUIDOrName(ctx context.Context, uuidOrName string, getOpts ...GetOption) (result T, err error)
GetByName(ctx context.Context, name string, getOpts ...GetOption) (instance T, err error)
GetFirst(ctx context.Context, filter ffapi.Filter, getOpts ...GetOption) (instance T, err error)
GetSequenceForID(ctx context.Context, id string) (seq int64, err error)
GetMany(ctx context.Context, filter ffapi.Filter) (instances []T, fr *ffapi.FilterResult, err error)
Count(ctx context.Context, filter ffapi.Filter) (count int64, err error)
Expand All @@ -116,6 +119,9 @@ type CrudBase[T Resource] struct {
TimesDisabled bool // no management of the time columns
PatchDisabled bool // allows non-pointer fields, but prevents UpdateSparse function
ImmutableColumns []string
NameField string // If supporting name semantics
QueryFactory ffapi.QueryFactory // Must be set when name is set
IDValidator func(ctx context.Context, idStr string) error // if IDs must conform to a pattern, such as a UUID (prebuilt UUIDValidator provided for that)

NilValue func() T // nil value typed to T
NewInstance func() T
Expand All @@ -129,6 +135,11 @@ type CrudBase[T Resource] struct {
ReadQueryModifier func(sq.SelectBuilder) sq.SelectBuilder
}

func UUIDValidator(ctx context.Context, idStr string) error {
_, err := fftypes.ParseUUID(ctx, idStr)
return err
}

// Validate checks things that must be true about a CRUD collection using this framework.
// Intended for use in the unit tests of microservices (will exercise all the functions of the CrudBase):
// - the mandatory columns exist - id/created/updated
Expand Down Expand Up @@ -179,6 +190,9 @@ func (c *CrudBase[T]) Validate() {
if !isNil(c.GetFieldPtr(inst, fftypes.NewUUID().String())) {
panic("GetFieldPtr() must return nil for unknown column")
}
if c.NameField != "" && c.QueryFactory == nil {
panic("QueryFactory must be set when name semantics are enabled")
}
}

func (c *CrudBase[T]) idFilter(id string) sq.Eq {
Expand Down Expand Up @@ -251,6 +265,12 @@ func (c *CrudBase[T]) attemptSetSequence(inst interface{}, seq int64) {
}

func (c *CrudBase[T]) attemptInsert(ctx context.Context, tx *TXWrapper, inst T, requestConflictEmptyResult bool) (err error) {
if c.IDValidator != nil {
if err := c.IDValidator(ctx, inst.GetID()); err != nil {
return err
}
}

c.setInsertTimestamps(inst)
insert := sq.Insert(c.Table).Columns(c.Columns...)
values := make([]interface{}, len(c.Columns))
Expand Down Expand Up @@ -486,17 +506,24 @@ func (c *CrudBase[T]) GetSequenceForID(ctx context.Context, id string) (seq int6
return seq, nil
}

func (c *CrudBase[T]) GetByID(ctx context.Context, id string, getOpts ...GetOption) (inst T, err error) {

failNotFound := false
func processGetOpts(ctx context.Context, getOpts []GetOption) (failNotFound bool, err error) {
for _, o := range getOpts {
switch o {
case FailIfNotFound:
failNotFound = true
default:
return c.NilValue(), i18n.NewError(ctx, i18n.MsgDBUnknownGetOption, o)
return false, i18n.NewError(ctx, i18n.MsgDBUnknownGetOption, o)
}
}
return failNotFound, nil
}

func (c *CrudBase[T]) GetByID(ctx context.Context, id string, getOpts ...GetOption) (inst T, err error) {

failNotFound, err := processGetOpts(ctx, getOpts)
if err != nil {
return c.NilValue(), err
}

tableFrom, cols, readCols := c.getReadCols(nil)
query := sq.Select(readCols...).
Expand Down Expand Up @@ -540,6 +567,62 @@ func (c *CrudBase[T]) GetMany(ctx context.Context, filter ffapi.Filter) (instanc
return c.getManyScoped(ctx, tableFrom, fi, cols, readCols, preconditions)
}

// GetFirst returns a single match (like GetByID), but using a generic filter
func (c *CrudBase[T]) GetFirst(ctx context.Context, filter ffapi.Filter, getOpts ...GetOption) (instance T, err error) {
failNotFound, err := processGetOpts(ctx, getOpts)
if err != nil {
return c.NilValue(), err
}

results, _, err := c.GetMany(ctx, filter.Limit(1))
if err != nil {
return c.NilValue(), err
}
if len(results) == 0 {
if failNotFound {
return c.NilValue(), i18n.NewError(ctx, i18n.Msg404NoResult)
}
return c.NilValue(), nil
}

return results[0], nil
}

// GetByName is a special case of GetFirst, for the designated name column
func (c *CrudBase[T]) GetByName(ctx context.Context, name string, getOpts ...GetOption) (instance T, err error) {
if c.NameField == "" {
return c.NilValue(), i18n.NewError(ctx, i18n.MsgCollectionNotConfiguredWithName, c.Table)
}
return c.GetFirst(ctx, c.QueryFactory.NewFilter(ctx).Eq(c.NameField, name), getOpts...)
}

// GetByUUIDOrName provides the following semantic, for resolving strings in a URL path to a resource that
// for convenience are able to be a UUID or a name of an object:
// - If the string is valid according to the IDValidator, we attempt a lookup by ID first
// - If not found by ID, or not a valid ID, then continue to find by name
//
// Note: The ID wins in the above logic. If resource1 has a name that matches the ID of resource2, then
//
// resource2 will be returned (not resource1).
func (c *CrudBase[T]) GetByUUIDOrName(ctx context.Context, uuidOrName string, getOpts ...GetOption) (result T, err error) {
validID := true
if c.IDValidator != nil {
idErr := c.IDValidator(ctx, uuidOrName)
validID = idErr == nil
}
if validID {
// Do a get with failure on not found - so that if this works,
// we return the value that was retrieved (and don't need a
// complex generic-friendly nil check).
// Regardless of the error type, we continue to do a lookup by name
result, err = c.GetByID(ctx, uuidOrName, FailIfNotFound)
if err == nil {
return result, nil
}
}
return c.GetByName(ctx, uuidOrName, getOpts...)
}

func (c *CrudBase[T]) getManyScoped(ctx context.Context, tableFrom string, fi *ffapi.FilterInfo, cols, readCols []string, preconditions []sq.Sqlizer) (instances []T, fr *ffapi.FilterResult, err error) {
query, fop, fi, err := c.DB.filterSelectFinalized(ctx, c.ReadTableAlias, sq.Select(readCols...).From(tableFrom), fi, c.FilterFieldMap,
[]interface{}{
Expand Down
114 changes: 112 additions & 2 deletions pkg/dbsql/crud_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,15 @@ import (
type TestCRUDable struct {
ResourceBase
NS *string `json:"namespace"`
Name *string `json:"name"`
Field1 *string `json:"f1"`
Field2 *fftypes.FFBigInt `json:"f2"`
Field3 *fftypes.JSONAny `json:"f3"`
}

var CRUDableQueryFactory = &ffapi.QueryFields{
"ns": &ffapi.StringField{},
"name": &ffapi.StringField{},
"id": &ffapi.UUIDField{},
"created": &ffapi.TimeField{},
"updated": &ffapi.TimeField{},
Expand Down Expand Up @@ -127,6 +129,7 @@ func newCRUDCollection(db *Database, ns string) *TestCRUD {
ColumnCreated,
ColumnUpdated,
"ns",
"name",
"field1",
"field2",
"field3",
Expand All @@ -140,6 +143,9 @@ func newCRUDCollection(db *Database, ns string) *TestCRUD {
NewInstance: func() *TestCRUDable { return &TestCRUDable{} },
ScopedFilter: func() squirrel.Eq { return sq.Eq{"ns": ns} },
EventHandler: nil, // set below
NameField: "name",
QueryFactory: CRUDableQueryFactory,
IDValidator: UUIDValidator,
GetFieldPtr: func(inst *TestCRUDable, col string) interface{} {
switch col {
case ColumnID:
Expand All @@ -150,6 +156,8 @@ func newCRUDCollection(db *Database, ns string) *TestCRUD {
return &inst.Updated
case "ns":
return &inst.NS
case "name":
return &inst.Name
case "field1":
return &inst.Field1
case "field2":
Expand Down Expand Up @@ -293,6 +301,7 @@ func TestCRUDWithDBEnd2End(t *testing.T) {
ResourceBase: ResourceBase{
ID: fftypes.NewUUID(),
},
Name: strPtr("bob"),
NS: strPtr("ns1"),
Field1: strPtr("hello1"),
Field2: fftypes.NewFFBigInt(12345),
Expand All @@ -315,6 +324,21 @@ func TestCRUDWithDBEnd2End(t *testing.T) {
assert.NoError(t, err)
checkEqualExceptTimes(t, *c1, *c1copy)

// Check we get it back by name
c1copy, err = iCrud.GetByName(ctx, *c1.Name)
assert.NoError(t, err)
checkEqualExceptTimes(t, *c1, *c1copy)

// Check we get it back by name, in name or UUID
c1copy, err = iCrud.GetByUUIDOrName(ctx, *c1.Name)
assert.NoError(t, err)
checkEqualExceptTimes(t, *c1, *c1copy)

// Check we get it back by UUID, in name or UUID
c1copy, err = iCrud.GetByUUIDOrName(ctx, c1.GetID())
assert.NoError(t, err)
checkEqualExceptTimes(t, *c1, *c1copy)

// Upsert the existing row optimized
c1copy.Field1 = strPtr("hello again - 1")
created, err := iCrud.Upsert(ctx, c1copy, UpsertOptimizationExisting)
Expand Down Expand Up @@ -628,7 +652,11 @@ func TestUpsertFailInsert(t *testing.T) {
mock.ExpectBegin()
mock.ExpectQuery("SELECT.*").WillReturnRows(mock.NewRows([]string{db.sequenceColumn}))
mock.ExpectExec("INSERT.*").WillReturnError(fmt.Errorf("pop"))
_, err := tc.Upsert(context.Background(), &TestCRUDable{}, UpsertOptimizationSkip)
_, err := tc.Upsert(context.Background(), &TestCRUDable{
ResourceBase: ResourceBase{
ID: fftypes.NewUUID(),
},
}, UpsertOptimizationSkip)
assert.Regexp(t, "FF00177", err)
assert.NoError(t, mock.ExpectationsWereMet())
}
Expand Down Expand Up @@ -715,11 +743,24 @@ func TestInsertInsertFail(t *testing.T) {
tc := newCRUDCollection(&db.Database, "ns1")
mock.ExpectBegin()
mock.ExpectExec("INSERT.*").WillReturnError(fmt.Errorf("pop"))
err := tc.Insert(context.Background(), &TestCRUDable{})
err := tc.Insert(context.Background(), &TestCRUDable{
ResourceBase: ResourceBase{
ID: fftypes.NewUUID(),
},
})
assert.Regexp(t, "FF00177", err)
assert.NoError(t, mock.ExpectationsWereMet())
}

func TestInsertMissingUUID(t *testing.T) {
db, mock := NewMockProvider().UTInit()
tc := newCRUDCollection(&db.Database, "ns1")
mock.ExpectBegin()
err := tc.Insert(context.Background(), &TestCRUDable{})
assert.Regexp(t, "FF00138", err)
assert.NoError(t, mock.ExpectationsWereMet())
}

func TestReplaceBeginFail(t *testing.T) {
db, mock := NewMockProvider().UTInit()
tc := newCRUDCollection(&db.Database, "ns1")
Expand Down Expand Up @@ -773,6 +814,52 @@ func TestGetByIDScanFail(t *testing.T) {
assert.NoError(t, mock.ExpectationsWereMet())
}

func TestGetByNameNoNameSemantics(t *testing.T) {
db, _ := NewMockProvider().UTInit()
tc := newLinkableCollection(&db.Database, "ns1")
_, err := tc.GetByName(context.Background(), "any")
assert.Regexp(t, "FF00214", err)
}

func TestGetByUUIDOrNameNotUUIDNilResult(t *testing.T) {
db, mock := NewMockProvider().UTInit()
tc := newCRUDCollection(&db.Database, "ns1")
mock.ExpectQuery("SELECT.*name =.*").WillReturnRows(sqlmock.NewRows([]string{}))
res, err := tc.GetByUUIDOrName(context.Background(), "something")
assert.NoError(t, err)
assert.Nil(t, res)
}

func TestGetByUUIDOrNameErrNotFoundUUIDParsableString(t *testing.T) {
db, mock := NewMockProvider().UTInit()
tc := newCRUDCollection(&db.Database, "ns1")
mock.ExpectQuery("SELECT.*id =.*").WillReturnRows(sqlmock.NewRows([]string{}))
mock.ExpectQuery("SELECT.*name =.*").WillReturnRows(sqlmock.NewRows([]string{}))
res, err := tc.GetByUUIDOrName(context.Background(), fftypes.NewUUID().String(), FailIfNotFound)
assert.Regexp(t, "FF00164", err)
assert.Nil(t, res)
assert.NoError(t, mock.ExpectationsWereMet())
}

func TestGetFirstBadGetOpts(t *testing.T) {
db, mock := NewMockProvider().UTInit()
tc := newCRUDCollection(&db.Database, "ns1")
res, err := tc.GetFirst(context.Background(), CRUDableQueryFactory.NewFilter(context.Background()).And(), GetOption(-99))
assert.Regexp(t, "FF00212", err)
assert.Nil(t, res)
assert.NoError(t, mock.ExpectationsWereMet())
}

func TestGetFirstQueryFail(t *testing.T) {
db, mock := NewMockProvider().UTInit()
tc := newCRUDCollection(&db.Database, "ns1")
mock.ExpectQuery("SELECT.*").WillReturnError(fmt.Errorf("pop"))
res, err := tc.GetFirst(context.Background(), CRUDableQueryFactory.NewFilter(context.Background()).And())
assert.Regexp(t, "FF00176", err)
assert.Nil(t, res)
assert.NoError(t, mock.ExpectationsWereMet())
}

func TestGetManyInvalidOp(t *testing.T) {
db, mock := NewMockProvider().UTInit()
tc := newCRUDCollection(&db.Database, "ns1")
Expand Down Expand Up @@ -1075,3 +1162,26 @@ func TestValidateGetFieldPtrNotNilForUnknown(t *testing.T) {
tc.Validate()
})
}

func TestValidateNameSemanticsWithoutQueryFactory(t *testing.T) {
db, _ := NewMockProvider().UTInit()
tc := &CrudBase[*TestCRUDable]{
DB: &db.Database,
NewInstance: func() *TestCRUDable { return &TestCRUDable{} },
Columns: []string{ColumnID},
TimesDisabled: true,
GetFieldPtr: func(inst *TestCRUDable, col string) interface{} {
if col == "id" {
var t *string
return &t
}
return nil
},
NilValue: func() *TestCRUDable { return nil },
ScopedFilter: func() sq.Eq { return sq.Eq{} },
NameField: "name",
}
assert.PanicsWithValue(t, "QueryFactory must be set when name semantics are enabled", func() {
tc.Validate()
})
}
1 change: 1 addition & 0 deletions pkg/i18n/en_base_error_messages.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,5 @@ var (
MsgInvalidTLSDnMismatch = ffe("FF00211", "Certificate subject does not meet requirements")
MsgDBUnknownGetOption = ffe("FF00212", "Unknown get option (%d)", 400)
MsgDBPatchNotSupportedForCollection = ffe("FF00213", "Patch not supported for collection '%s'", 405)
MsgCollectionNotConfiguredWithName = ffe("FF00214", "Name based queries not supported for collection '%s'", 405)
)
1 change: 1 addition & 0 deletions test/dbmigrations/000001_create_crudables_table.up.sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ CREATE TABLE crudables (
created BIGINT NOT NULL,
updated BIGINT NOT NULL,
ns VARCHAR(64) NOT NULL,
name TEXT,
field1 TEXT,
field2 VARCHAR(65),
field3 TEXT
Expand Down

0 comments on commit f52df06

Please sign in to comment.