Simplified FindFirstVersionHistoryByItem (cadence-workflow#4882)

MartinGomes · Jun 29, 2022 · aff5ecf · aff5ecf
1 parent 4cfb741
commit aff5ecf
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 30 deletions.
diff --git a/common/persistence/versionHistory.go b/common/persistence/versionHistory.go
@@ -529,18 +529,18 @@ func (h *VersionHistories) FindLCAVersionHistoryIndexAndItem(
 	return versionHistoryIndex, versionHistoryItem, nil
 }
 
-// FindFirstVersionHistoryIndexByItem find the first version history index which
+// FindFirstVersionHistoryByItem find the first version history index and history which
 // contains the given version history item
-func (h *VersionHistories) FindFirstVersionHistoryIndexByItem(
+func (h *VersionHistories) FindFirstVersionHistoryByItem(
 	item *VersionHistoryItem,
-) (int, error) {
+) (index int, history *VersionHistory, err error) {
 
 	for index, localHistory := range h.Histories {
 		if localHistory.ContainsItem(item) {
-			return index, nil
+			return index, localHistory, nil
 		}
 	}
-	return 0, &types.BadRequestError{Message: "version histories does not contains given item."}
+	return 0, nil, &types.BadRequestError{Message: "version histories does not contains given item."}
 }
 
 // IsRebuilt returns true if the current branch index's last write version is not the largest

diff --git a/common/persistence/versionHistory_test.go b/common/persistence/versionHistory_test.go
@@ -633,7 +633,7 @@ func (s *versionHistoriesSuite) TestFindLCAVersionHistoryIndexAndItem_SameEventI
 	s.Equal(NewVersionHistoryItem(7, 6), item)
 }
 
-func (s *versionHistoriesSuite) TestFindFirstVersionHistoryIndexByItem() {
+func (s *versionHistoriesSuite) TestFindFirstVersionHistoryByItem() {
 	versionHistory1 := NewVersionHistory([]byte("branch token 1"), []*VersionHistoryItem{
 		{EventID: 3, Version: 0},
 		{EventID: 5, Version: 4},
@@ -650,15 +650,17 @@ func (s *versionHistoriesSuite) TestFindFirstVersionHistoryIndexByItem() {
 	_, _, err := histories.AddVersionHistory(versionHistory2)
 	s.Nil(err)
 
-	index, err := histories.FindFirstVersionHistoryIndexByItem(NewVersionHistoryItem(8, 10))
+	index, history, err := histories.FindFirstVersionHistoryByItem(NewVersionHistoryItem(8, 10))
 	s.NoError(err)
 	s.Equal(1, index)
+	s.Equal(versionHistory2, history)
 
-	index, err = histories.FindFirstVersionHistoryIndexByItem(NewVersionHistoryItem(4, 4))
+	index, history, err = histories.FindFirstVersionHistoryByItem(NewVersionHistoryItem(4, 4))
 	s.NoError(err)
 	s.Equal(0, index)
+	s.Equal(versionHistory1, history)
 
-	_, err = histories.FindFirstVersionHistoryIndexByItem(NewVersionHistoryItem(41, 4))
+	_, _, err = histories.FindFirstVersionHistoryByItem(NewVersionHistoryItem(41, 4))
 	s.Error(err)
 }
 

diff --git a/service/frontend/adminHandler.go b/service/frontend/adminHandler.go
@@ -1460,12 +1460,7 @@ func (adh *adminHandlerImpl) setRequestDefaultValueAndGetTargetVersionHistory(
 		// this is a special case, target branch remains the same
 	} else {
 		endItem := persistence.NewVersionHistoryItem(request.GetEndEventID(), request.GetEndEventVersion())
-		idx, err := versionHistories.FindFirstVersionHistoryIndexByItem(endItem)
-		if err != nil {
-			return nil, err
-		}
-
-		targetBranch, err = versionHistories.GetVersionHistory(idx)
+		_, targetBranch, err = versionHistories.FindFirstVersionHistoryByItem(endItem)
 		if err != nil {
 			return nil, err
 		}
@@ -1479,11 +1474,7 @@ func (adh *adminHandlerImpl) setRequestDefaultValueAndGetTargetVersionHistory(
 		// this is a special case, start event is on the same branch as target branch
 	} else {
 		if !targetBranch.ContainsItem(startItem) {
-			idx, err := versionHistories.FindFirstVersionHistoryIndexByItem(startItem)
-			if err != nil {
-				return nil, err
-			}
-			startBranch, err := versionHistories.GetVersionHistory(idx)
+			_, startBranch, err := versionHistories.FindFirstVersionHistoryByItem(startItem)
 			if err != nil {
 				return nil, err
 			}

diff --git a/service/history/ndc/workflow_resetter.go b/service/history/ndc/workflow_resetter.go
@@ -182,7 +182,7 @@ func (r *workflowResetterImpl) getBaseBranchToken(
 	baseVersionHistories := mutableState.GetVersionHistories()
 	if baseVersionHistories != nil {
 
-		index, err := baseVersionHistories.FindFirstVersionHistoryIndexByItem(
+		_, baseVersionHistory, err := baseVersionHistories.FindFirstVersionHistoryByItem(
 			persistence.NewVersionHistoryItem(baseLastEventID, baseLastEventVersion),
 		)
 		if err != nil {
@@ -201,10 +201,6 @@ func (r *workflowResetterImpl) getBaseBranchToken(
 			)
 		}
 
-		baseVersionHistory, err := baseVersionHistories.GetVersionHistory(index)
-		if err != nil {
-			return nil, err
-		}
 		branchToken = baseVersionHistory.GetBranchToken()
 	}
 	return branchToken, nil

diff --git a/service/history/replication/task_ack_manager.go b/service/history/replication/task_ack_manager.go
@@ -584,7 +584,7 @@ func getVersionHistoryItems(
 		}
 	}
 
-	versionHistoryIndex, err := versionHistories.FindFirstVersionHistoryIndexByItem(
+	_, versionHistory, err := versionHistories.FindFirstVersionHistoryByItem(
 		persistence.NewVersionHistoryItem(
 			eventID,
 			version,
@@ -594,9 +594,5 @@ func getVersionHistoryItems(
 		return nil, nil, err
 	}
 
-	versionHistory, err := versionHistories.GetVersionHistory(versionHistoryIndex)
-	if err != nil {
-		return nil, nil, err
-	}
 	return versionHistory.ToInternalType().Items, versionHistory.GetBranchToken(), nil
 }