-
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
Open
mattgode
wants to merge
13
commits into
master
Choose a base branch
from
mgode/implement-multi-endpoints-client
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
bca2507
Add implementation for multi upsert in the dosa client
mattgode 7581046
Improve test coverage of multi upsert
mattgode dff6d44
Update changelog to refect changes in diff
mattgode b7a2c2b
Remove nonsensical comment
mattgode 1ad1a8c
Add warning to multi upsert method
mattgode ad11b4a
Revert excess changes to client mock
mattgode 117acb0
Merge branch 'master' into mgode/implement-multi-endpoints-client
tiffanycheng db6c89e
Update comments and wrap error
dd53450
Update CHANGELOG.md
tiffanycheng b74e439
Update client.go
tiffanycheng 536404b
Update CHANGELOG.md
tiffanycheng 255dd3b
Update client.go
tiffanycheng 9018de7
Update client.go
tiffanycheng File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
# Changelog | ||
|
||
## v3.0.0 (unreleased) | ||
## v2.7.0 (unreleased) | ||
- Implement MultiUpsert method in client and expose it via the Client interface (#304) | ||
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. 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 commentThe reason will be displayed to describe this comment to others. Learn more. i guess no reason. should i add a 2.7.0? |
||
|
||
## v2.6.0 (2018-04-16) | ||
- Fix bug in invalidating fallback cache on upsert (#292) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
we intend to release 3.0 as next release