Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Convert version history bad request to internal error #4690

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Convert version history bad request to internal error #4690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions common/persistence/versionhistory/version_histories.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func CopyVersionHistories(h *historyspb.VersionHistories) *historyspb.VersionHis
// GetVersionHistory gets the VersionHistory according to index provided.
func GetVersionHistory(h *historyspb.VersionHistories, index int32) (*historyspb.VersionHistory, error) {
if index < 0 || index >= int32(len(h.Histories)) {
return nil, serviceerror.NewInvalidArgument("version histories index is out of range.")
return nil, serviceerror.NewInternal("version histories index is out of range.")
}

return h.Histories[index], nil
Expand All @@ -67,7 +67,7 @@ func GetVersionHistory(h *historyspb.VersionHistories, index int32) (*historyspb
// AddVersionHistory adds a VersionHistory and return the whether current branch is changed.
func AddVersionHistory(h *historyspb.VersionHistories, v *historyspb.VersionHistory) (bool, int32, error) {
if v == nil {
return false, 0, serviceerror.NewInvalidArgument("version histories is null.")
return false, 0, serviceerror.NewInternal("version histories is null.")
}

// assuming existing version histories inside are valid
Expand All @@ -86,7 +86,7 @@ func AddVersionHistory(h *historyspb.VersionHistories, v *historyspb.VersionHist
}

if incomingFirstItem.Version != currentFirstItem.Version {
return false, 0, serviceerror.NewInvalidArgument("version history first item does not match.")
return false, 0, serviceerror.NewInternal("version history first item does not match.")
}

// TODO maybe we need more strict validation
Expand Down Expand Up @@ -147,7 +147,7 @@ func FindFirstVersionHistoryIndexByVersionHistoryItem(h *historyspb.VersionHisto
return int32(versionHistoryIndex), nil
}
}
return 0, serviceerror.NewInvalidArgument("version histories does not contains given item.")
return 0, serviceerror.NewInternal("version histories does not contains given item.")
}

// IsVersionHistoriesRebuilt returns true if the current branch index's last write version is not the largest among all branches' last write version.
Expand Down Expand Up @@ -178,7 +178,7 @@ func IsVersionHistoriesRebuilt(h *historyspb.VersionHistories) (bool, error) {
// SetCurrentVersionHistoryIndex set the current VersionHistory index.
func SetCurrentVersionHistoryIndex(h *historyspb.VersionHistories, currentVersionHistoryIndex int32) error {
if currentVersionHistoryIndex < 0 || currentVersionHistoryIndex >= int32(len(h.Histories)) {
return serviceerror.NewInvalidArgument("invalid current version history index.")
return serviceerror.NewInternal("invalid current version history index.")
}

h.CurrentVersionHistoryIndex = currentVersionHistoryIndex
Expand Down
16 changes: 8 additions & 8 deletions common/persistence/versionhistory/version_history.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func CopyVersionHistory(v *historyspb.VersionHistory) *historyspb.VersionHistory
// CopyVersionHistoryUntilLCAVersionHistoryItem returns copy of VersionHistory up until LCA item.
func CopyVersionHistoryUntilLCAVersionHistoryItem(v *historyspb.VersionHistory, lcaItem *historyspb.VersionHistoryItem) (*historyspb.VersionHistory, error) {
versionHistory := &historyspb.VersionHistory{}
notFoundErr := serviceerror.NewInvalidArgument("version history does not contains the LCA item.")
notFoundErr := serviceerror.NewInternal("version history does not contains the LCA item.")
for _, item := range v.Items {
if item.Version < lcaItem.Version {
if err := AddOrUpdateVersionHistoryItem(versionHistory, item); err != nil {
Expand Down Expand Up @@ -93,11 +93,11 @@ func AddOrUpdateVersionHistoryItem(v *historyspb.VersionHistory, item *historysp

lastItem := v.Items[len(v.Items)-1]
if item.Version < lastItem.Version {
return serviceerror.NewInvalidArgument(fmt.Sprintf("cannot update version history with a lower version %v. Last version: %v", item.Version, lastItem.Version))
return serviceerror.NewInternal(fmt.Sprintf("cannot update version history with a lower version %v. Last version: %v", item.Version, lastItem.Version))
}

if item.GetEventId() <= lastItem.GetEventId() {
return serviceerror.NewInvalidArgument(fmt.Sprintf("cannot add version history with a lower event id %v. Last event id: %v", item.GetEventId(), lastItem.GetEventId()))
return serviceerror.NewInternal(fmt.Sprintf("cannot add version history with a lower event id %v. Last event id: %v", item.GetEventId(), lastItem.GetEventId()))
}

if item.Version > lastItem.Version {
Expand Down Expand Up @@ -149,7 +149,7 @@ func FindLCAVersionHistoryItem(v *historyspb.VersionHistory, remote *historyspb.
}
}

return nil, serviceerror.NewInvalidArgument("version history is malformed. No joint point found.")
return nil, serviceerror.NewInternal("version history is malformed. No joint point found.")
}

// IsLCAVersionHistoryItemAppendable checks if a LCA VersionHistoryItem is appendable.
Expand All @@ -167,15 +167,15 @@ func IsLCAVersionHistoryItemAppendable(v *historyspb.VersionHistory, lcaItem *hi
// GetFirstVersionHistoryItem return the first VersionHistoryItem.
func GetFirstVersionHistoryItem(v *historyspb.VersionHistory) (*historyspb.VersionHistoryItem, error) {
if len(v.Items) == 0 {
return nil, serviceerror.NewInvalidArgument("version history is empty.")
return nil, serviceerror.NewInternal("version history is empty.")
}
return CopyVersionHistoryItem(v.Items[0]), nil
}

// GetLastVersionHistoryItem return the last VersionHistoryItem.
func GetLastVersionHistoryItem(v *historyspb.VersionHistory) (*historyspb.VersionHistoryItem, error) {
if len(v.Items) == 0 {
return nil, serviceerror.NewInvalidArgument("version history is empty.")
return nil, serviceerror.NewInternal("version history is empty.")
}
return CopyVersionHistoryItem(v.Items[len(v.Items)-1]), nil
}
Expand All @@ -187,7 +187,7 @@ func GetVersionHistoryEventVersion(v *historyspb.VersionHistory, eventID int64)
return 0, err
}
if eventID < common.FirstEventID || eventID > lastItem.GetEventId() {
return 0, serviceerror.NewInvalidArgument("input event ID is not in range.")
return 0, serviceerror.NewInternal("input event ID is not in range.")
}

// items are sorted by eventID & version
Expand All @@ -198,7 +198,7 @@ func GetVersionHistoryEventVersion(v *historyspb.VersionHistory, eventID int64)
return currentItem.GetVersion(), nil
}
}
return 0, serviceerror.NewInvalidArgument("input event ID is not in range.")
return 0, serviceerror.NewInternal("input event ID is not in range.")
}

// IsEmptyVersionHistory indicate whether version history is empty
Expand Down
16 changes: 8 additions & 8 deletions common/persistence/versionhistory/version_history_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,22 +110,22 @@ func (s *versionHistorySuite) TestDuplicateUntilLCAItem_Failure() {
history := NewVersionHistory(BranchToken, Items)

_, err := CopyVersionHistoryUntilLCAVersionHistoryItem(history, NewVersionHistoryItem(4, 0))
s.IsType(&serviceerror.InvalidArgument{}, err)
s.IsType(&serviceerror.Internal{}, err)

_, err = CopyVersionHistoryUntilLCAVersionHistoryItem(history, NewVersionHistoryItem(2, 1))
s.IsType(&serviceerror.InvalidArgument{}, err)
s.IsType(&serviceerror.Internal{}, err)

_, err = CopyVersionHistoryUntilLCAVersionHistoryItem(history, NewVersionHistoryItem(5, 3))
s.IsType(&serviceerror.InvalidArgument{}, err)
s.IsType(&serviceerror.Internal{}, err)

_, err = CopyVersionHistoryUntilLCAVersionHistoryItem(history, NewVersionHistoryItem(7, 5))
s.IsType(&serviceerror.InvalidArgument{}, err)
s.IsType(&serviceerror.Internal{}, err)

_, err = CopyVersionHistoryUntilLCAVersionHistoryItem(history, NewVersionHistoryItem(4, 0))
s.IsType(&serviceerror.InvalidArgument{}, err)
s.IsType(&serviceerror.Internal{}, err)

_, err = CopyVersionHistoryUntilLCAVersionHistoryItem(history, NewVersionHistoryItem(7, 4))
s.IsType(&serviceerror.InvalidArgument{}, err)
s.IsType(&serviceerror.Internal{}, err)
}

func (s *versionHistorySuite) TestSetBranchToken() {
Expand Down Expand Up @@ -398,7 +398,7 @@ func (s *versionHistorySuite) TestGetFirstItem_Failure() {
history := NewVersionHistory(BranchToken, []*historyspb.VersionHistoryItem{})

_, err := GetFirstVersionHistoryItem(history)
s.IsType(&serviceerror.InvalidArgument{}, err)
s.IsType(&serviceerror.Internal{}, err)
}

func (s *versionHistorySuite) TestGetLastItem_Success() {
Expand Down Expand Up @@ -432,7 +432,7 @@ func (s *versionHistorySuite) TestGetLastItem_Failure() {
history := NewVersionHistory(BranchToken, []*historyspb.VersionHistoryItem{})

_, err := GetLastVersionHistoryItem(history)
s.IsType(&serviceerror.InvalidArgument{}, err)
s.IsType(&serviceerror.Internal{}, err)
}

func (s *versionHistoriesSuite) TestGetVersion_Success() {
Expand Down