-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement MultiUpsert method in client and expose it via the Client interface #304
base: master
Are you sure you want to change the base?
Changes from 7 commits
bca2507
7581046
dff6d44
b7a2c2b
1ad1a8c
ad11b4a
117acb0
db6c89e
dd53450
b74e439
536404b
255dd3b
9018de7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,10 +138,12 @@ type Client interface { | |
// to update in fieldsToUpdate (or all the fields if you use dosa.All()) | ||
Upsert(ctx context.Context, fieldsToUpdate []string, objectToUpdate DomainObject) error | ||
|
||
// TODO: Coming in v2.1 | ||
// MultiUpsert creates or updates multiple rows. A list of fields to | ||
// update can be specified. Use All() or nil for all fields. | ||
// MultiUpsert(context.Context, []string, ...DomainObject) (MultiResult, error) | ||
// update can be specified. Use All() or nil for all fields. Partial | ||
// successes are possible, so it is critical to inspect the MultiResult response | ||
// to check for failures. | ||
// NOTE: This API only upserts objects of same entity type from same scope. | ||
MultiUpsert(context.Context, []string, ...DomainObject) (MultiResult, error) | ||
|
||
// Remove removes a row by primary key. The passed-in entity should contain | ||
// the primary key field values, all other fields are ignored. | ||
|
@@ -151,7 +153,7 @@ type Client interface { | |
// given RemoveRangeOp. | ||
RemoveRange(ctx context.Context, removeRangeOp *RemoveRangeOp) error | ||
|
||
// TODO: Coming in v2.1 | ||
// TODO: Coming in v2.7 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 3.0 |
||
// MultiRemove removes multiple rows by primary key. The passed-in entity should | ||
// contain the primary key field values. | ||
// MultiRemove(context.Context, ...DomainObject) (MultiResult, error) | ||
|
@@ -410,12 +412,69 @@ func (c *client) createOrUpsert(ctx context.Context, fieldsToUpdate []string, en | |
return fn(ctx, re.EntityInfo(), fieldValues) | ||
} | ||
|
||
// MultiUpsert updates several entities by primary key, The entities provided | ||
// must contain values for all components of its primary key for the operation | ||
// to succeed. If `fieldsToUpdate` is provided, only a subset of fields will be | ||
// updated. | ||
func (c *client) MultiUpsert(context.Context, []string, ...DomainObject) (MultiResult, error) { | ||
panic("not implemented") | ||
// MultiUpsert updates several entities of the same type by primary key, The | ||
// entities provided must contain values for all components of its primary key | ||
// for the operation to succeed. If `fieldsToUpdate` is provided, only a subset | ||
// of fields will be updated. Moreover, all entities being upserted must be part | ||
// of the same partition otherwise the request will be rejected. | ||
// NOTE: This endpoint is not officially released. No guarantees about correctness | ||
// or performance of this API will be guaranteed until v2.6 is released. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remember to update the comment when release |
||
func (c *client) MultiUpsert(ctx context.Context, fieldsToUpdate []string, entities ...DomainObject) (MultiResult, error) { | ||
if !c.initialized { | ||
return nil, &ErrNotInitialized{} | ||
} | ||
|
||
if len(entities) == 0 { | ||
return nil, fmt.Errorf("the number of entities to upsert is zero") | ||
} | ||
|
||
// lookup registered entity, registry will return error if registration | ||
// is not found | ||
var re *RegisteredEntity | ||
var listMultiValues []map[string]FieldValue | ||
for _, entity := range entities { | ||
ere, err := c.registrar.Find(entity) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
if re == nil { | ||
re = ere | ||
} else if re != ere { | ||
return nil, fmt.Errorf("inconsistent entity type for multi upsert: %v vs %v", re, ere) | ||
} | ||
|
||
// translate entity field values to a map of primary key name/values pairs | ||
keyFieldValues := re.KeyFieldValues(entity) | ||
|
||
// translate remaining entity fields values to map of column name/value pairs | ||
fieldValues, err := re.OnlyFieldValues(entity, fieldsToUpdate) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// merge key and remaining values | ||
for k, v := range keyFieldValues { | ||
fieldValues[k] = v | ||
} | ||
|
||
listMultiValues = append(listMultiValues, fieldValues) | ||
} | ||
|
||
results, err := c.connector.MultiUpsert(ctx, re.EntityInfo(), listMultiValues) | ||
if err != nil { | ||
return nil, err | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add errors.wrap |
||
} | ||
|
||
multiResult := MultiResult{} | ||
// map results to entity fields | ||
for i, entity := range entities { | ||
if results[i] != nil { | ||
multiResult[entity] = results[i] | ||
} | ||
} | ||
|
||
return multiResult, nil | ||
} | ||
|
||
// Remove deletes an entity by primary key, The entity provided must contain | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -400,6 +400,70 @@ func TestClient_Upsert(t *testing.T) { | |
assert.NoError(t, c3.Upsert(ctx, fieldsToUpdate, cte1)) | ||
assert.Equal(t, cte1.Email, updatedEmail) | ||
} | ||
|
||
func TestClient_MultiUpsert(t *testing.T) { | ||
reg1, _ := dosaRenamed.NewRegistrar(scope, namePrefix, cte1) | ||
reg2, _ := dosaRenamed.NewRegistrar(scope, namePrefix, cte1, cte2) | ||
fieldsToUpsert := []string{"Email"} | ||
|
||
e1 := &ClientTestEntity1{ID: int64(1), Email: "[email protected]"} | ||
e2 := &ClientTestEntity1{ID: int64(2), Email: "[email protected]"} | ||
|
||
// uninitialized | ||
c1 := dosaRenamed.NewClient(reg1, nullConnector) | ||
_, err := c1.MultiUpsert(ctx, dosaRenamed.All(), cte1) | ||
assert.Error(t, err) | ||
|
||
// empty upsert | ||
c1.Initialize(ctx) | ||
_, err = c1.MultiUpsert(ctx, dosaRenamed.All()) | ||
assert.Error(t, err) | ||
assert.Contains(t, err.Error(), "zero") | ||
|
||
// unregistered object | ||
c1.Initialize(ctx) | ||
_, err = c1.MultiUpsert(ctx, dosaRenamed.All(), cte2) | ||
assert.Error(t, err) | ||
assert.Contains(t, err.Error(), "ClientTestEntity2") | ||
|
||
// multi read different types of object | ||
c1.Initialize(ctx) | ||
_, err = c1.MultiUpsert(ctx, dosaRenamed.All(), cte2, cte1) | ||
assert.Error(t, err) | ||
assert.Contains(t, err.Error(), "ClientTestEntity2") | ||
|
||
// happy path, mock connector | ||
ctrl := gomock.NewController(t) | ||
defer ctrl.Finish() | ||
mockConn := mocks.NewMockConnector(ctrl) | ||
mockConn.EXPECT().CheckSchema(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(int32(1), nil).AnyTimes() | ||
mockConn.EXPECT().MultiUpsert(ctx, gomock.Any(), gomock.Any()). | ||
Do(func(_ context.Context, _ *dosaRenamed.EntityInfo, allValues []map[string]dosaRenamed.FieldValue) { | ||
assert.Equal(t, allValues[0]["id"], e1.ID) | ||
assert.Equal(t, allValues[0]["email"], e1.Email) | ||
assert.Equal(t, allValues[1]["id"], e2.ID) | ||
assert.Equal(t, allValues[1]["email"], e2.Email) | ||
|
||
}).Return([]error{nil, nil}, nil).Times(1) | ||
c2 := dosaRenamed.NewClient(reg2, mockConn) | ||
assert.NoError(t, c2.Initialize(ctx)) | ||
rs, err := c2.MultiUpsert(ctx, fieldsToUpsert, e1, e2) | ||
assert.NoError(t, err) | ||
assert.Empty(t, rs) | ||
|
||
// system error, mock connector | ||
mockConn.EXPECT().MultiUpsert(ctx, gomock.Any(), gomock.Any()).Return([]error{nil, nil}, errors.New("connector error")) | ||
rs, err = c2.MultiUpsert(ctx, fieldsToUpsert, e1, e2) | ||
assert.Error(t, err) | ||
assert.Empty(t, rs) | ||
|
||
// single entity error, mock connector | ||
mockConn.EXPECT().MultiUpsert(ctx, gomock.Any(), gomock.Any()).Return([]error{errors.New("single error"), nil}, nil) | ||
rs, err = c2.MultiUpsert(ctx, fieldsToUpsert, e1, e2) | ||
assert.NoError(t, err) | ||
assert.NotNil(t, rs[e1]) | ||
} | ||
|
||
func TestClient_CreateIfNotExists(t *testing.T) { | ||
reg1, _ := dosaRenamed.NewRegistrar("test", "team.service", cte1) | ||
reg2, _ := dosaRenamed.NewRegistrar("test", "team.service", cte1, cte2) | ||
|
@@ -726,11 +790,12 @@ func TestClient_MultiRead(t *testing.T) { | |
|
||
// uninitialized | ||
c1 := dosaRenamed.NewClient(reg1, nullConnector) | ||
assert.Error(t, c1.Read(ctx, fieldsToRead, cte1)) | ||
_, err := c1.MultiRead(ctx, fieldsToRead, cte1) | ||
assert.Error(t, err) | ||
|
||
// unregistered object | ||
c1.Initialize(ctx) | ||
_, err := c1.MultiRead(ctx, dosaRenamed.All(), cte2) | ||
_, err = c1.MultiRead(ctx, dosaRenamed.All(), cte2) | ||
assert.Error(t, err) | ||
assert.Contains(t, err.Error(), "ClientTestEntity2") | ||
|
||
|
@@ -762,20 +827,6 @@ func TestClient_MultiRead(t *testing.T) { | |
assert.Equal(t, rs[e2].Error(), "not fonud") | ||
} | ||
|
||
/* TODO: Coming in v2.1 | ||
func TestClient_Unimplemented(t *testing.T) { | ||
reg1, _ := dosaRenamed.NewRegistrar(scope, namePrefix, cte1) | ||
|
||
c := dosaRenamed.NewClient(reg1, nullConnector) | ||
assert.Panics(t, func() { | ||
c.MultiUpsert(ctx, dosaRenamed.All(), &ClientTestEntity1{}) | ||
}) | ||
assert.Panics(t, func() { | ||
c.MultiRemove(ctx, &ClientTestEntity1{}) | ||
}) | ||
} | ||
*/ | ||
|
||
func TestAdminClient_CreateScope(t *testing.T) { | ||
c := dosaRenamed.NewAdminClient(nullConnector) | ||
assert.NotNil(t, c) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any breaking changes here, what's the reason for wanting to call this 3.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i guess no reason. should i add a 2.7.0?