Skip to content

Commit 0def4af

Browse files
authored
fix: add nil pointer check for active cluster selection policy mappers (#7398)
<!-- Describe what has changed in this PR --> **What changed?** - add nil pointer check for active cluster selection policy mappers <!-- Tell your future self why have you made these changes --> **Why?** We want to remove those fields from ActiveClusterSelectionPolicy. However, the change is not safe to rollback because of the nil-pointer dereference of the new data that doesn't have those fields serialized into thrift. This change is to create a commit that's safe to be rolled back. <!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? --> **How did you test it?** unit tests <!-- Assuming the worst case, what can be broken when deploying this change to production? --> **Potential risks** <!-- Is it notable for release? e.g. schema updates, configuration or data migration required? If so, please mention it, and also update CHANGELOG.md --> **Release notes** <!-- Is there any documentation updates should be made for config, https://cadenceworkflow.io/docs/operation-guide/setup/ ? If so, please open an PR in https://github.com/cadence-workflow/cadence-docs --> **Documentation Changes**
1 parent 53d9b92 commit 0def4af

File tree

2 files changed

+5
-3
lines changed

2 files changed

+5
-3
lines changed

common/types/mapper/thrift/shared.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7059,9 +7059,9 @@ func ToActiveClusterSelectionPolicy(t *shared.ActiveClusterSelectionPolicy) *typ
70597059
}
70607060
return &types.ActiveClusterSelectionPolicy{
70617061
ActiveClusterSelectionStrategy: ToActiveClusterSelectionStrategy(t.Strategy),
7062-
ExternalEntityType: *t.ExternalEntityType,
7063-
ExternalEntityKey: *t.ExternalEntityKey,
7064-
StickyRegion: *t.StickyRegion,
7062+
ExternalEntityType: t.GetExternalEntityType(),
7063+
ExternalEntityKey: t.GetExternalEntityKey(),
7064+
StickyRegion: t.GetStickyRegion(),
70657065
ClusterAttribute: ToClusterAttribute(t.ClusterAttribute),
70667066
}
70677067
}

common/types/mapper/thrift/shared_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3591,6 +3591,8 @@ func TestQueueStateConversion(t *testing.T) {
35913591
}
35923592
}
35933593

3594+
// TODO: The way we're testing mappers doesn't have good coverage. The round trip tests don't cover serialization and deserialization of thrift.
3595+
// We should also generate test data in thrift types and do round trip tests from thrift to internal and back to thrift.
35943596
func TestActiveClusterSelectionPolicyConversion(t *testing.T) {
35953597
testCases := []*types.ActiveClusterSelectionPolicy{
35963598
nil,

0 commit comments

Comments
 (0)