Skip to content

Commit

Permalink
Fix remaining stuff
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sushisource
Sushisource committed Feb 23, 2023
1 parent 6b9b8af commit f264d93
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 68 deletions.
39 changes: 23 additions & 16 deletions service/frontend/workflow_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3613,7 +3613,7 @@ func (wh *WorkflowHandler) UpdateWorkerBuildIdCompatability(ctx context.Context,
return nil, errRequestNotSet
}

if err := wh.validateBuildIdOrderingUpdate(request); err != nil {
if err := wh.validateBuildIdCompatabilityUpdate(request); err != nil {
return nil, err
}

Expand Down Expand Up @@ -4351,10 +4351,10 @@ func (wh *WorkflowHandler) validateTaskQueue(t *taskqueuepb.TaskQueue) error {
return nil
}

func (wh *WorkflowHandler) validateBuildIdOrderingUpdate(
func (wh *WorkflowHandler) validateBuildIdCompatabilityUpdate(
req *workflowservice.UpdateWorkerBuildIdCompatabilityRequest,
) error {
errstr := "request to update worker build id ordering requires:"
errstr := "request to update worker build id compatability requires:"
hadErr := false

checkIdLen := func(id string) {
Expand All @@ -4377,30 +4377,37 @@ func (wh *WorkflowHandler) validateBuildIdOrderingUpdate(
errstr += " an operation to be specified."
hadErr = true
}
if x, ok := req.GetOperation().(*workflowservice.UpdateWorkerBuildIdCompatabilityRequest_NewCompatibleVersion_); ok {
if x.NewCompatibleVersion.GetNewVersionId() == "" {
errstr += " `new_version_id` to be set."
if x, ok := req.GetOperation().(*workflowservice.UpdateWorkerBuildIdCompatabilityRequest_AddNewCompatibleVersion_); ok {
if x.AddNewCompatibleVersion.GetNewVersionId() == "" {
errstr += " `add_new_compatible_version` to be set."
hadErr = true
} else {
checkIdLen(x.NewCompatibleVersion.GetNewVersionId())
checkIdLen(x.AddNewCompatibleVersion.GetNewVersionId())
}
if x.NewCompatibleVersion.GetExistingCompatibleVersion() == "" {
if x.AddNewCompatibleVersion.GetExistingCompatibleVersion() == "" {
errstr += " `existing_compatible_version` to be set."
hadErr = true
}
} else if x, ok := req.GetOperation().(*workflowservice.UpdateWorkerBuildIdCompatabilityRequest_NewDefaultVersionId); ok {
if x.NewDefaultVersionId == "" {
errstr += " `new_default_major_version_id` to be set."
} else if x, ok := req.GetOperation().(*workflowservice.UpdateWorkerBuildIdCompatabilityRequest_AddNewVersionIdInNewDefaultSet); ok {
if x.AddNewVersionIdInNewDefaultSet == "" {
errstr += " `add_new_version_id_in_new_default_set` to be set."
hadErr = true
} else {
checkIdLen(x.NewDefaultVersionId)
checkIdLen(x.AddNewVersionIdInNewDefaultSet)
}
} else if x, ok := req.GetOperation().(*workflowservice.UpdateWorkerBuildIdCompatabilityRequest_ExistingVersionIdInSetToPromote); ok {
if x.ExistingVersionIdInSetToPromote == "" {
errstr += " `existing_version_id_in_set_to_promote` to be set."
} else if x, ok := req.GetOperation().(*workflowservice.UpdateWorkerBuildIdCompatabilityRequest_PromoteSetByVersionId); ok {
if x.PromoteSetByVersionId == "" {
errstr += " `promote_set_by_version_id` to be set."
hadErr = true
} else {
checkIdLen(x.ExistingVersionIdInSetToPromote)
checkIdLen(x.PromoteSetByVersionId)
}
} else if x, ok := req.GetOperation().(*workflowservice.UpdateWorkerBuildIdCompatabilityRequest_PromoteVersionIdWithinSet); ok {
if x.PromoteVersionIdWithinSet == "" {
errstr += " `promote_version_id_within_set` to be set."
hadErr = true
} else {
checkIdLen(x.PromoteVersionIdWithinSet)
}
}
if hadErr {
Expand Down
2 changes: 1 addition & 1 deletion service/matching/matchingEngine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ func (e *matchingEngineImpl) UpdateWorkerBuildIdCompatability(
return nil, err
}
err = tqMgr.MutateVersioningData(hCtx.Context, func(data *persistencespb.VersioningData) error {
return UpdateVersionsGraph(data, req.GetRequest(), e.config.MaxVersionGraphSize())
return UpdateVersionSets(data, req.GetRequest(), e.config.MaxVersionGraphSize())
})
if err != nil {
return nil, err
Expand Down
43 changes: 15 additions & 28 deletions service/matching/matchingEngine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1978,14 +1978,13 @@ func (s *matchingEngineSuite) TestGetVersioningData() {
},
})
s.NoError(err)
//s.NotNil(res.GetResponse().GetCurrentDefault())
//lastNode := res.GetResponse().GetCurrentDefault()
//s.Equal(mkVerId("99"), lastNode.GetVersion())
//for lastNode.GetPreviousIncompatible() != nil {
// lastNode = lastNode.GetPreviousIncompatible()
//}
//s.Equal(mkVerId("0"), lastNode.GetVersion())
//s.Equal(mkVerId("99.9"), res.GetResponse().GetCompatibleLeaves()[0].GetVersion())
majorSets := res.GetResponse().GetMajorVersionSets()
curDefault := majorSets[len(majorSets)-1]
s.NotNil(curDefault)
s.Equal("99", curDefault.GetVersions()[0])
lastNode := curDefault.GetVersions()[len(curDefault.GetVersions())-1]
s.Equal("99.9", lastNode)
s.Equal("0", majorSets[0].GetVersions()[0])

// Ensure depth limiting works
res, err = s.matchingEngine.GetWorkerBuildIdCompatability(s.handlerContext, &matchingservice.GetWorkerBuildIdCompatabilityRequest{
Expand All @@ -1997,9 +1996,12 @@ func (s *matchingEngineSuite) TestGetVersioningData() {
},
})
s.NoError(err)
//s.NotNil(res.GetResponse().GetCurrentDefault())
//s.Nil(res.GetResponse().GetCurrentDefault().GetPreviousIncompatible())
//s.Nil(res.GetResponse().GetCompatibleLeaves()[0].GetPreviousCompatible())
majorSets = res.GetResponse().GetMajorVersionSets()
curDefault = majorSets[len(majorSets)-1]
s.Equal("99", curDefault.GetVersions()[0])
lastNode = curDefault.GetVersions()[len(curDefault.GetVersions())-1]
s.Equal("99.9", lastNode)
s.Equal(1, len(majorSets))

res, err = s.matchingEngine.GetWorkerBuildIdCompatability(s.handlerContext, &matchingservice.GetWorkerBuildIdCompatabilityRequest{
NamespaceId: namespaceID.String(),
Expand All @@ -2010,23 +2012,8 @@ func (s *matchingEngineSuite) TestGetVersioningData() {
},
})
s.NoError(err)
//s.NotNil(res.GetResponse().GetCurrentDefault())
//lastNode = res.GetResponse().GetCurrentDefault()
//for {
// if lastNode.GetPreviousIncompatible() == nil {
// break
// }
// lastNode = lastNode.GetPreviousIncompatible()
//}
//s.Equal(mkVerId("95"), lastNode.GetVersion())
//lastNode = res.GetResponse().GetCompatibleLeaves()[0]
//for {
// if lastNode.GetPreviousCompatible() == nil {
// break
// }
// lastNode = lastNode.GetPreviousCompatible()
//}
//s.Equal(mkVerId("99.5"), lastNode.GetVersion())
majorSets = res.GetResponse().GetMajorVersionSets()
s.Equal("95", majorSets[0].GetVersions()[0])
}

func (s *matchingEngineSuite) TestActivityQueueMetadataInvalidate() {
Expand Down
9 changes: 5 additions & 4 deletions service/matching/version_sets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
)

func ToBuildIdOrderingResponse(g *persistence.VersioningData, maxDepth int) *workflowservice.GetWorkerBuildIdCompatabilityResponse {
// TODO: Current default pointer not represented in response. Can either shuffle it to front or change API.
return depthLimiter(g, maxDepth, false)
}

Expand All @@ -56,9 +55,11 @@ func HashVersioningData(data *persistence.VersioningData) []byte {

func depthLimiter(g *persistence.VersioningData, maxDepth int, mutate bool) *workflowservice.GetWorkerBuildIdCompatabilityResponse {
if maxDepth <= 0 || maxDepth >= len(g.GetVersionSets()) {
return &workflowservice.GetWorkerBuildIdCompatabilityResponse{MajorVersionSets: g.VersionSets}
return &workflowservice.GetWorkerBuildIdCompatabilityResponse{MajorVersionSets: g.GetVersionSets()}
}
shortened := g.GetVersionSets()[maxDepth:]
sets := g.GetVersionSets()
startIndex := len(sets) - maxDepth
shortened := g.GetVersionSets()[startIndex:]
if mutate {
g.VersionSets = shortened
}
Expand Down Expand Up @@ -92,7 +93,7 @@ func depthLimiter(g *persistence.VersioningData, maxDepth int, mutate bool) *wor
// Deletions are not permitted, as inserting new versions can accomplish the same goals with less complexity. However,
// sets may be dropped when the number of sets limit is reached. They are dropped oldest first - the current default set
// is never dropped, instead dropping the next oldest set.
func UpdateVersionsGraph(existingData *persistence.VersioningData, req *workflowservice.UpdateWorkerBuildIdCompatabilityRequest, maxSize int) error {
func UpdateVersionSets(existingData *persistence.VersioningData, req *workflowservice.UpdateWorkerBuildIdCompatabilityRequest, maxSize int) error {
err := updateImpl(existingData, req)
if err != nil {
return err
Expand Down
47 changes: 28 additions & 19 deletions service/matching/version_sets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestNewDefaultUpdate(t *testing.T) {
data := mkInitialData(2)

req := mkNewDefReq("2")
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

curd := data.VersionSets[len(data.VersionSets)-1]
Expand All @@ -105,7 +105,7 @@ func TestNewDefaultGraphUpdateOfEmptyGraph(t *testing.T) {
data := &persistencepb.VersioningData{}

req := mkNewDefReq("1")
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

curd := data.VersionSets[len(data.VersionSets)-1]
Expand All @@ -117,7 +117,7 @@ func TestNewDefaultGraphUpdateCompatWithCurDefault(t *testing.T) {
data := mkInitialData(2)

req := mkNewCompatReq("1.1", "1", true)
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

curd := data.VersionSets[len(data.VersionSets)-1]
Expand All @@ -130,7 +130,7 @@ func TestNewDefaultGraphUpdateCompatWithNonDefaultSet(t *testing.T) {
data := mkInitialData(2)

req := mkNewCompatReq("0.1", "0", true)
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

curd := data.VersionSets[len(data.VersionSets)-1]
Expand All @@ -143,7 +143,7 @@ func TestNewCompatibleWithVerInOlderSet(t *testing.T) {
data := mkInitialData(3)

req := mkNewCompatReq("0.1", "0", false)
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

curd := data.VersionSets[len(data.VersionSets)-1]
Expand All @@ -159,11 +159,11 @@ func TestNewCompatibleWithNonDefaultGraphUpdate(t *testing.T) {
data := mkInitialData(2)

req := mkNewCompatReq("0.1", "0", false)
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

req = mkNewCompatReq("0.2", "0.1", false)
err = UpdateVersionsGraph(data, req, 0)
err = UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

curd := data.VersionSets[len(data.VersionSets)-1]
Expand All @@ -174,7 +174,7 @@ func TestNewCompatibleWithNonDefaultGraphUpdate(t *testing.T) {

// Ensure setting a compatible version which targets a non-leaf compat version ends up without a branch
req = mkNewCompatReq("0.3", "0.1", false)
err = UpdateVersionsGraph(data, req, 0)
err = UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

assert.Equal(t, "1", curd.Versions[0])
Expand All @@ -188,7 +188,7 @@ func TestCompatibleTargetsNotFound(t *testing.T) {
data := mkInitialData(1)

req := mkNewCompatReq("1.1", "1", false)
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.Error(t, err)
assert.IsType(t, &serviceerror.NotFound{}, err)
}
Expand All @@ -197,7 +197,7 @@ func TestMakeExistingSetDefault(t *testing.T) {
data := mkInitialData(4)

req := mkExistingDefault("2")
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)

assert.NoError(t, err)
assert.Equal(t, "0", data.VersionSets[0].Versions[0])
Expand All @@ -208,7 +208,7 @@ func TestMakeExistingSetDefault(t *testing.T) {
// Add a compatible version to a set and then make that set the default via the compatible version
req = mkNewCompatReq("1.1", "1", true)

err = UpdateVersionsGraph(data, req, 0)
err = UpdateVersionSets(data, req, 0)
assert.NoError(t, err)
assert.Equal(t, "0", data.VersionSets[0].Versions[0])
assert.Equal(t, "3", data.VersionSets[1].Versions[0])
Expand All @@ -220,11 +220,11 @@ func TestSayVersionIsCompatWithDifferentSetThanItsAlreadyCompatWithNotAllowed(t
data := mkInitialData(3)

req := mkNewCompatReq("0.1", "0", false)
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

req = mkNewCompatReq("0.1", "1", false)
err = UpdateVersionsGraph(data, req, 0)
err = UpdateVersionSets(data, req, 0)
assert.Error(t, err)
assert.IsType(t, &serviceerror.InvalidArgument{}, err)
}
Expand All @@ -236,7 +236,7 @@ func TestLimitsMaxSize(t *testing.T) {
for i := 0; i < 20; i++ {
id := fmt.Sprintf("%d", i)
req := mkNewDefReq(id)
err := UpdateVersionsGraph(data, req, maxSize)
err := UpdateVersionSets(data, req, maxSize)
assert.NoError(t, err)
}

Expand All @@ -250,12 +250,12 @@ func TestPromoteWithinVersion(t *testing.T) {

for i := 1; i <= 5; i++ {
req := mkNewCompatReq(fmt.Sprintf("0.%d", i), "0", false)
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)
}

req := mkPromoteInSet("0.1")
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.NoError(t, err)

curd := data.VersionSets[0].Versions[len(data.VersionSets[0].Versions)-1]
Expand All @@ -266,7 +266,16 @@ func TestAddAlreadyExtantVersionAsDefaultErrors(t *testing.T) {
data := mkInitialData(3)

req := mkNewDefReq("0")
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.Error(t, err)
assert.IsType(t, &serviceerror.InvalidArgument{}, err)
}

func TestAddAlreadyExtantVersionToAnotherSetErrors(t *testing.T) {
data := mkInitialData(3)

req := mkNewCompatReq("0", "1", false)
err := UpdateVersionSets(data, req, 0)
assert.Error(t, err)
assert.IsType(t, &serviceerror.InvalidArgument{}, err)
}
Expand All @@ -275,7 +284,7 @@ func TestMakeSetDefaultTargetingNonexistentVersionErrors(t *testing.T) {
data := mkInitialData(3)

req := mkExistingDefault("crab boi")
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.Error(t, err)
assert.IsType(t, &serviceerror.NotFound{}, err)
}
Expand All @@ -284,7 +293,7 @@ func TestPromoteWithinSetTargetingNonexistentVersionErrors(t *testing.T) {
data := mkInitialData(3)

req := mkPromoteInSet("i'd rather be writing rust ;)")
err := UpdateVersionsGraph(data, req, 0)
err := UpdateVersionSets(data, req, 0)
assert.Error(t, err)
assert.IsType(t, &serviceerror.NotFound{}, err)
}

0 comments on commit f264d93

Please sign in to comment.