diff --git a/common/persistence/visibility/defs.go b/common/persistence/visibility/defs.go index ad7fa559110..71d839cc1cc 100644 --- a/common/persistence/visibility/defs.go +++ b/common/persistence/visibility/defs.go @@ -24,6 +24,12 @@ package visibility +import ( + "go.temporal.io/server/common/persistence/sql/sqlplugin/mysql" + "go.temporal.io/server/common/persistence/sql/sqlplugin/postgresql" + "go.temporal.io/server/common/persistence/sql/sqlplugin/sqlite" +) + const ( // AdvancedVisibilityWritingModeOff means do not write to advanced visibility store AdvancedVisibilityWritingModeOff = "off" @@ -40,3 +46,14 @@ func DefaultAdvancedVisibilityWritingMode(advancedVisibilityConfigExist bool) st } return AdvancedVisibilityWritingModeOff } + +func AllowListForValidation(pluginName string) bool { + switch pluginName { + case mysql.PluginNameV8, postgresql.PluginNameV12, sqlite.PluginName: + // Advanced visibility with SQL DB don't support list of values + return false + default: + // Otherwise, enable for backward compatibility. + return true + } +} diff --git a/common/persistence/visibility/store/elasticsearch/visibility_store.go b/common/persistence/visibility/store/elasticsearch/visibility_store.go index 43b836b9e6e..5076ba2a0f9 100644 --- a/common/persistence/visibility/store/elasticsearch/visibility_store.go +++ b/common/persistence/visibility/store/elasticsearch/visibility_store.go @@ -874,7 +874,7 @@ func (s *visibilityStore) generateESDoc(request *store.InternalVisibilityRequest return nil, serviceerror.NewUnavailable(fmt.Sprintf("Unable to read search attribute types: %v", err)) } - searchAttributes, err := searchattribute.Decode(request.SearchAttributes, &typeMap) + searchAttributes, err := searchattribute.Decode(request.SearchAttributes, &typeMap, true) if err != nil { s.metricsHandler.Counter(metrics.ElasticsearchDocumentGenerateFailuresCount.GetMetricName()).Record(1) return nil, serviceerror.NewInternal(fmt.Sprintf("Unable to decode search attributes: %v", err)) diff --git a/common/persistence/visibility/store/elasticsearch/visibility_store_read_test.go b/common/persistence/visibility/store/elasticsearch/visibility_store_read_test.go index 1fb8c1bdd9e..8217b425a8e 100644 --- a/common/persistence/visibility/store/elasticsearch/visibility_store_read_test.go +++ b/common/persistence/visibility/store/elasticsearch/visibility_store_read_test.go @@ -920,7 +920,7 @@ func (s *ESVisibilitySuite) TestParseESDoc_SearchAttributes() { info, err := s.visibilityStore.parseESDoc("", docSource, searchattribute.TestNameTypeMap, testNamespace) s.NoError(err) s.NotNil(info) - customSearchAttributes, err := searchattribute.Decode(info.SearchAttributes, &searchattribute.TestNameTypeMap) + customSearchAttributes, err := searchattribute.Decode(info.SearchAttributes, &searchattribute.TestNameTypeMap, true) s.NoError(err) s.Len(customSearchAttributes, 7) diff --git a/common/persistence/visibility/store/sql/visibility_store.go b/common/persistence/visibility/store/sql/visibility_store.go index 8a54ebfffc4..6c4f5cfa306 100644 --- a/common/persistence/visibility/store/sql/visibility_store.go +++ b/common/persistence/visibility/store/sql/visibility_store.go @@ -482,7 +482,7 @@ func (s *VisibilityStore) prepareSearchAttributesForDb( } var searchAttributes sqlplugin.VisibilitySearchAttributes - searchAttributes, err = searchattribute.Decode(request.SearchAttributes, &saTypeMap) + searchAttributes, err = searchattribute.Decode(request.SearchAttributes, &saTypeMap, false) if err != nil { return nil, err } diff --git a/common/searchattribute/encode.go b/common/searchattribute/encode.go index 0d415e50469..59ff784d621 100644 --- a/common/searchattribute/encode.go +++ b/common/searchattribute/encode.go @@ -67,7 +67,11 @@ func Encode(searchAttributes map[string]interface{}, typeMap *NameTypeMap) (*com // 1. type from typeMap, // 2. if typeMap is nil, type from MetadataType field is used. // In case of error, it will continue to next search attribute and return last error. -func Decode(searchAttributes *commonpb.SearchAttributes, typeMap *NameTypeMap) (map[string]interface{}, error) { +func Decode( + searchAttributes *commonpb.SearchAttributes, + typeMap *NameTypeMap, + allowList bool, +) (map[string]interface{}, error) { if len(searchAttributes.GetIndexedFields()) == 0 { return nil, nil } @@ -84,7 +88,7 @@ func Decode(searchAttributes *commonpb.SearchAttributes, typeMap *NameTypeMap) ( } } - searchAttributeValue, err := DecodeValue(saPayload, saType) + searchAttributeValue, err := DecodeValue(saPayload, saType, allowList) if err != nil { lastErr = err result[saName] = nil diff --git a/common/searchattribute/encode_test.go b/common/searchattribute/encode_test.go index a7d07bf02bf..083a652f2ec 100644 --- a/common/searchattribute/encode_test.go +++ b/common/searchattribute/encode_test.go @@ -137,7 +137,7 @@ func Test_Decode_Success(t *testing.T) { }, typeMap) assert.NoError(err) - vals, err := Decode(sa, typeMap) + vals, err := Decode(sa, typeMap, true) assert.NoError(err) assert.Len(vals, 6) assert.Equal("val1", vals["key1"]) @@ -154,7 +154,7 @@ func Test_Decode_Success(t *testing.T) { delete(sa.IndexedFields["key5"].Metadata, "type") delete(sa.IndexedFields["key6"].Metadata, "type") - vals, err = Decode(sa, typeMap) + vals, err = Decode(sa, typeMap, true) assert.NoError(err) assert.Len(vals, 6) assert.Equal("val1", vals["key1"]) @@ -185,7 +185,7 @@ func Test_Decode_NilMap(t *testing.T) { }, typeMap) assert.NoError(err) - vals, err := Decode(sa, nil) + vals, err := Decode(sa, nil, true) assert.NoError(err) assert.Len(sa.IndexedFields, 6) assert.Equal("val1", vals["key1"]) @@ -211,11 +211,15 @@ func Test_Decode_Error(t *testing.T) { }, typeMap) assert.NoError(err) - vals, err := Decode(sa, &NameTypeMap{customSearchAttributes: map[string]enumspb.IndexedValueType{ - "key1": enumspb.INDEXED_VALUE_TYPE_TEXT, - "key4": enumspb.INDEXED_VALUE_TYPE_INT, - "key3": enumspb.INDEXED_VALUE_TYPE_BOOL, - }}) + vals, err := Decode( + sa, + &NameTypeMap{customSearchAttributes: map[string]enumspb.IndexedValueType{ + "key1": enumspb.INDEXED_VALUE_TYPE_TEXT, + "key4": enumspb.INDEXED_VALUE_TYPE_INT, + "key3": enumspb.INDEXED_VALUE_TYPE_BOOL, + }}, + true, + ) assert.Error(err) assert.True(errors.Is(err, ErrInvalidName)) assert.Len(sa.IndexedFields, 3) @@ -227,7 +231,7 @@ func Test_Decode_Error(t *testing.T) { delete(sa.IndexedFields["key2"].Metadata, "type") delete(sa.IndexedFields["key3"].Metadata, "type") - vals, err = Decode(sa, nil) + vals, err = Decode(sa, nil, true) assert.Error(err) assert.True(errors.Is(err, ErrInvalidType)) assert.Len(vals, 3) diff --git a/common/searchattribute/encode_value.go b/common/searchattribute/encode_value.go index e72a0a3daa9..51e839b1cae 100644 --- a/common/searchattribute/encode_value.go +++ b/common/searchattribute/encode_value.go @@ -48,12 +48,44 @@ func EncodeValue(val interface{}, t enumspb.IndexedValueType) (*commonpb.Payload // DecodeValue decodes search attribute value from Payload using (in order): // 1. passed type t. // 2. type from MetadataType field, if t is not specified. -func DecodeValue(value *commonpb.Payload, t enumspb.IndexedValueType) (interface{}, error) { +// allowList allows list of values when it's not keyword list type. +func DecodeValue( + value *commonpb.Payload, + t enumspb.IndexedValueType, + allowList bool, +) (any, error) { if t == enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED { - t = enumspb.IndexedValueType(enumspb.IndexedValueType_value[string(value.Metadata[MetadataType])]) + t = enumspb.IndexedValueType( + enumspb.IndexedValueType_value[string(value.Metadata[MetadataType])], + ) } - // Here are similar code sections for all types. + switch t { + case enumspb.INDEXED_VALUE_TYPE_BOOL: + return decodeValueTyped[bool](value, allowList) + case enumspb.INDEXED_VALUE_TYPE_DATETIME: + return decodeValueTyped[time.Time](value, allowList) + case enumspb.INDEXED_VALUE_TYPE_DOUBLE: + return decodeValueTyped[float64](value, allowList) + case enumspb.INDEXED_VALUE_TYPE_INT: + return decodeValueTyped[int64](value, allowList) + case enumspb.INDEXED_VALUE_TYPE_KEYWORD: + return decodeValueTyped[string](value, allowList) + case enumspb.INDEXED_VALUE_TYPE_TEXT: + return decodeValueTyped[string](value, allowList) + case enumspb.INDEXED_VALUE_TYPE_KEYWORD_LIST: + return decodeValueTyped[[]string](value, false) + default: + return nil, fmt.Errorf("%w: %v", ErrInvalidType, t) + } +} + +// decodeValueTyped tries to decode to the given type. +// If the input is a list and allowList is false, then it will return only the first element. +// If the input is a list and allowList is true, then it will return the decoded list. +// +//nolint:revive // allowList is a control flag +func decodeValueTyped[T any](value *commonpb.Payload, allowList bool) (any, error) { // At first, it tries to decode to pointer of actual type (i.e. `*string` for `string`). // This is to ensure that `nil` values are decoded back as `nil` using `NilPayloadConverter`. // If value is not `nil` but some value of expected type, the code relies on the fact that @@ -62,82 +94,28 @@ func DecodeValue(value *commonpb.Payload, t enumspb.IndexedValueType) (interface // If decoding to pointer type fails, it tries to decode to array of the same type because // search attributes support polymorphism: field of specific type may also have an array of that type. // If resulting slice has zero length, it gets substitute with `nil` to treat nils and empty slices equally. + // If allowList is true, it returns the list as it is. If allowList is false and the list has + // only one element, then return it. Otherwise, return an error. // If search attribute value is `nil`, it means that search attribute needs to be removed from the document. - - switch t { - case enumspb.INDEXED_VALUE_TYPE_TEXT, - enumspb.INDEXED_VALUE_TYPE_KEYWORD, - enumspb.INDEXED_VALUE_TYPE_KEYWORD_LIST: - var val *string - if err := payload.Decode(value, &val); err != nil { - var listVal []string - err = payload.Decode(value, &listVal) - if len(listVal) == 0 { - return nil, err - } - return listVal, err - } - if val == nil { - return nil, nil + var val *T + if err := payload.Decode(value, &val); err != nil { + var listVal []T + if err := payload.Decode(value, &listVal); err != nil { + return nil, err } - return *val, nil - case enumspb.INDEXED_VALUE_TYPE_INT: - var val *int64 - if err := payload.Decode(value, &val); err != nil { - var listVal []int64 - err = payload.Decode(value, &listVal) - if len(listVal) == 0 { - return nil, err - } - return listVal, err - } - if val == nil { - return nil, nil - } - return *val, nil - case enumspb.INDEXED_VALUE_TYPE_DOUBLE: - var val *float64 - if err := payload.Decode(value, &val); err != nil { - var listVal []float64 - err = payload.Decode(value, &listVal) - if len(listVal) == 0 { - return nil, err - } - return listVal, err - } - if val == nil { + if len(listVal) == 0 { return nil, nil } - return *val, nil - case enumspb.INDEXED_VALUE_TYPE_BOOL: - var val *bool - if err := payload.Decode(value, &val); err != nil { - var listVal []bool - err = payload.Decode(value, &listVal) - if len(listVal) == 0 { - return nil, err - } - return listVal, err + if allowList { + return listVal, nil } - if val == nil { - return nil, nil + if len(listVal) == 1 { + return listVal[0], nil } - return *val, nil - case enumspb.INDEXED_VALUE_TYPE_DATETIME: - var val *time.Time - if err := payload.Decode(value, &val); err != nil { - var listVal []time.Time - err = payload.Decode(value, &listVal) - if len(listVal) == 0 { - return nil, err - } - return listVal, err - } - if val == nil { - return nil, nil - } - return *val, nil - default: - return nil, fmt.Errorf("%w: %v", ErrInvalidType, t) + return nil, fmt.Errorf("list of values not allowed for type %T", listVal[0]) + } + if val == nil { + return nil, nil } + return *val, nil } diff --git a/common/searchattribute/encode_value_test.go b/common/searchattribute/encode_value_test.go index 6ca7652f6b7..a3416983171 100644 --- a/common/searchattribute/encode_value_test.go +++ b/common/searchattribute/encode_value_test.go @@ -35,179 +35,340 @@ import ( "go.temporal.io/server/common/payload" ) -func Test_DecodeValue_FromMetadata_Success(t *testing.T) { - assert := assert.New(t) +func Test_DecodeValue_AllowList_FromMetadata_Success(t *testing.T) { + s := assert.New(t) + allowList := true payloadStr := payload.EncodeString("qwe") payloadStr.Metadata["type"] = []byte("Text") - decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) // MetadataType is used. - assert.NoError(err) - assert.Equal("qwe", decodedStr) + decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) // MetadataType is used. + s.NoError(err) + s.Equal("qwe", decodedStr) payloadBool, err := payload.Encode(true) - assert.NoError(err) + s.NoError(err) payloadBool.Metadata["type"] = []byte("Bool") - decodedBool, err := DecodeValue(payloadBool, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) // MetadataType is used. - assert.NoError(err) - assert.Equal(true, decodedBool) + decodedBool, err := DecodeValue(payloadBool, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) // MetadataType is used. + s.NoError(err) + s.Equal(true, decodedBool) payloadNil, err := payload.Encode(nil) - assert.NoError(err) + s.NoError(err) payloadNil.Metadata["type"] = []byte("Double") - decodedNil, err := DecodeValue(payloadNil, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) - assert.NoError(err) - assert.Nil(decodedNil) + decodedNil, err := DecodeValue(payloadNil, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Nil(decodedNil) payloadSlice, err := payload.Encode([]string{"val1", "val2"}) - assert.NoError(err) + s.NoError(err) payloadSlice.Metadata["type"] = []byte("Keyword") - decodedSlice, err := DecodeValue(payloadSlice, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) - assert.NoError(err) - assert.Equal([]string{"val1", "val2"}, decodedSlice) + decodedSlice, err := DecodeValue(payloadSlice, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Equal([]string{"val1", "val2"}, decodedSlice) payloadEmptySlice, err := payload.Encode([]string{}) - assert.NoError(err) + s.NoError(err) payloadEmptySlice.Metadata["type"] = []byte("Keyword") - decodedNil, err = DecodeValue(payloadEmptySlice, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) - assert.NoError(err) - assert.Nil(decodedNil) + decodedNil, err = DecodeValue(payloadEmptySlice, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Nil(decodedNil) var expectedEncodedRepresentation = "2022-03-07T21:27:35.986848-05:00" timeValue, err := time.Parse(time.RFC3339, expectedEncodedRepresentation) - assert.NoError(err) + s.NoError(err) payloadDatetime, err := payload.Encode(timeValue) - assert.NoError(err) + s.NoError(err) payloadDatetime.Metadata["type"] = []byte("Datetime") - decodedDatetime, err := DecodeValue(payloadDatetime, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) - assert.NoError(err) - assert.Equal(timeValue, decodedDatetime) + decodedDatetime, err := DecodeValue(payloadDatetime, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Equal(timeValue, decodedDatetime) } -func Test_DecodeValue_FromParameter_Success(t *testing.T) { - assert := assert.New(t) +func Test_DecodeValue_AllowList_FromParameter_Success(t *testing.T) { + s := assert.New(t) + allowList := true payloadStr := payload.EncodeString("qwe") - decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_TEXT) - assert.NoError(err) - assert.Equal("qwe", decodedStr) + decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_TEXT, allowList) + s.NoError(err) + s.Equal("qwe", decodedStr) payloadInt, err := payload.Encode(123) - assert.NoError(err) - decodedInt, err := DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT) - assert.NoError(err) - assert.Equal(int64(123), decodedInt) + s.NoError(err) + decodedInt, err := DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT, allowList) + s.NoError(err) + s.Equal(int64(123), decodedInt) payloadNil, err := payload.Encode(nil) - assert.NoError(err) - decodedNil, err := DecodeValue(payloadNil, enumspb.INDEXED_VALUE_TYPE_DOUBLE) - assert.NoError(err) - assert.Nil(decodedNil) + s.NoError(err) + decodedNil, err := DecodeValue(payloadNil, enumspb.INDEXED_VALUE_TYPE_DOUBLE, allowList) + s.NoError(err) + s.Nil(decodedNil) payloadSlice, err := payload.Encode([]string{"val1", "val2"}) - assert.NoError(err) - decodedSlice, err := DecodeValue(payloadSlice, enumspb.INDEXED_VALUE_TYPE_KEYWORD) - assert.NoError(err) - assert.Equal([]string{"val1", "val2"}, decodedSlice) + s.NoError(err) + decodedSlice, err := DecodeValue(payloadSlice, enumspb.INDEXED_VALUE_TYPE_KEYWORD, allowList) + s.NoError(err) + s.Equal([]string{"val1", "val2"}, decodedSlice) payloadEmptySlice, err := payload.Encode([]string{}) - assert.NoError(err) - decodedNil, err = DecodeValue(payloadEmptySlice, enumspb.INDEXED_VALUE_TYPE_KEYWORD) - assert.NoError(err) - assert.Nil(decodedNil) + s.NoError(err) + decodedNil, err = DecodeValue(payloadEmptySlice, enumspb.INDEXED_VALUE_TYPE_KEYWORD, allowList) + s.NoError(err) + s.Nil(decodedNil) var expectedEncodedRepresentation = "2022-03-07T21:27:35.986848-05:00" timeValue, err := time.Parse(time.RFC3339, expectedEncodedRepresentation) - assert.NoError(err) + s.NoError(err) payloadDatetime, err := payload.Encode(timeValue) - assert.NoError(err) - decodedDatetime, err := DecodeValue(payloadDatetime, enumspb.INDEXED_VALUE_TYPE_DATETIME) - assert.NoError(err) - assert.Equal(timeValue, decodedDatetime) + s.NoError(err) + decodedDatetime, err := DecodeValue(payloadDatetime, enumspb.INDEXED_VALUE_TYPE_DATETIME, allowList) + s.NoError(err) + s.Equal(timeValue, decodedDatetime) payloadInt, err = payload.Encode(123) - assert.NoError(err) + s.NoError(err) payloadInt.Metadata["type"] = []byte("String") // MetadataType is not used. - decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT) - assert.NoError(err) - assert.Equal(int64(123), decodedInt) + decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT, allowList) + s.NoError(err) + s.Equal(int64(123), decodedInt) payloadInt, err = payload.Encode(123) - assert.NoError(err) + s.NoError(err) payloadInt.Metadata["type"] = []byte("UnknownType") // MetadataType is not used. - decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT) - assert.NoError(err) - assert.Equal(int64(123), decodedInt) + decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT, allowList) + s.NoError(err) + s.Equal(int64(123), decodedInt) payloadBool, err := payload.Encode(true) - assert.NoError(err) - decodedBool, err := DecodeValue(payloadBool, enumspb.INDEXED_VALUE_TYPE_BOOL) - assert.NoError(err) - assert.Equal(true, decodedBool) + s.NoError(err) + decodedBool, err := DecodeValue(payloadBool, enumspb.INDEXED_VALUE_TYPE_BOOL, allowList) + s.NoError(err) + s.Equal(true, decodedBool) } -func Test_DecodeValue_Error(t *testing.T) { - assert := assert.New(t) +func Test_DecodeValue_AllowList_Error(t *testing.T) { + s := assert.New(t) + allowList := true payloadStr := payload.EncodeString("qwe") - decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) - assert.Error(err) - assert.ErrorIs(err, ErrInvalidType) - assert.Nil(decodedStr) + decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.Error(err) + s.ErrorIs(err, ErrInvalidType) + s.Nil(decodedStr) payloadInt, err := payload.Encode(123) - assert.NoError(err) + s.NoError(err) payloadInt.Metadata["type"] = []byte("UnknownType") - decodedInt, err := DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) - assert.Error(err) - assert.ErrorIs(err, ErrInvalidType) - assert.Nil(decodedInt) + decodedInt, err := DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.Error(err) + s.ErrorIs(err, ErrInvalidType) + s.Nil(decodedInt) payloadInt, err = payload.Encode(123) - assert.NoError(err) + s.NoError(err) payloadInt.Metadata["type"] = []byte("Text") - decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED) - assert.Error(err) - assert.ErrorIs(err, converter.ErrUnableToDecode, err.Error()) - assert.Nil(decodedInt) + decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.Error(err) + s.ErrorIs(err, converter.ErrUnableToDecode, err.Error()) + s.Nil(decodedInt) +} + +func Test_DecodeValue_NotAllowList_FromMetadata_Success(t *testing.T) { + s := assert.New(t) + allowList := false + + payloadStr := payload.EncodeString("qwe") + payloadStr.Metadata["type"] = []byte("Text") + decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Equal("qwe", decodedStr) + + payloadBool, err := payload.Encode(true) + s.NoError(err) + payloadBool.Metadata["type"] = []byte("Bool") + decodedBool, err := DecodeValue(payloadBool, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Equal(true, decodedBool) + + payloadNil, err := payload.Encode(nil) + s.NoError(err) + payloadNil.Metadata["type"] = []byte("Double") + decodedNil, err := DecodeValue(payloadNil, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Nil(decodedNil) + + payloadKeyword, err := payload.Encode([]string{"Keyword"}) + s.NoError(err) + payloadKeyword.Metadata["type"] = []byte("Keyword") + decodedKeyword, err := DecodeValue(payloadKeyword, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Equal("Keyword", decodedKeyword) + + payloadSlice, err := payload.Encode([]string{"val1", "val2"}) + s.NoError(err) + payloadSlice.Metadata["type"] = []byte("KeywordList") + decodedSlice, err := DecodeValue(payloadSlice, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Equal([]string{"val1", "val2"}, decodedSlice) + + payloadEmptySlice, err := payload.Encode([]string{}) + s.NoError(err) + payloadEmptySlice.Metadata["type"] = []byte("Keyword") + decodedNil, err = DecodeValue(payloadEmptySlice, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Nil(decodedNil) + + var expectedEncodedRepresentation = "2022-03-07T21:27:35.986848-05:00" + timeValue, err := time.Parse(time.RFC3339, expectedEncodedRepresentation) + s.NoError(err) + payloadDatetime, err := payload.Encode(timeValue) + s.NoError(err) + payloadDatetime.Metadata["type"] = []byte("Datetime") + decodedDatetime, err := DecodeValue(payloadDatetime, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.NoError(err) + s.Equal(timeValue, decodedDatetime) +} + +func Test_DecodeValue_NotAllowList_FromParameter_Success(t *testing.T) { + s := assert.New(t) + allowList := false + + payloadStr := payload.EncodeString("qwe") + decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_TEXT, allowList) + s.NoError(err) + s.Equal("qwe", decodedStr) + + payloadInt, err := payload.Encode(123) + s.NoError(err) + decodedInt, err := DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT, allowList) + s.NoError(err) + s.Equal(int64(123), decodedInt) + + payloadNil, err := payload.Encode(nil) + s.NoError(err) + decodedNil, err := DecodeValue(payloadNil, enumspb.INDEXED_VALUE_TYPE_DOUBLE, allowList) + s.NoError(err) + s.Nil(decodedNil) + + payloadKeyword, err := payload.Encode([]string{"Keyword"}) + s.NoError(err) + decodedKeyword, err := DecodeValue(payloadKeyword, enumspb.INDEXED_VALUE_TYPE_KEYWORD, allowList) + s.NoError(err) + s.Equal("Keyword", decodedKeyword) + + payloadSlice, err := payload.Encode([]string{"val1", "val2"}) + s.NoError(err) + decodedSlice, err := DecodeValue(payloadSlice, enumspb.INDEXED_VALUE_TYPE_KEYWORD_LIST, allowList) + s.NoError(err) + s.Equal([]string{"val1", "val2"}, decodedSlice) + + payloadEmptySlice, err := payload.Encode([]string{}) + s.NoError(err) + decodedNil, err = DecodeValue(payloadEmptySlice, enumspb.INDEXED_VALUE_TYPE_KEYWORD, allowList) + s.NoError(err) + s.Nil(decodedNil) + + var expectedEncodedRepresentation = "2022-03-07T21:27:35.986848-05:00" + timeValue, err := time.Parse(time.RFC3339, expectedEncodedRepresentation) + s.NoError(err) + payloadDatetime, err := payload.Encode(timeValue) + s.NoError(err) + decodedDatetime, err := DecodeValue(payloadDatetime, enumspb.INDEXED_VALUE_TYPE_DATETIME, allowList) + s.NoError(err) + s.Equal(timeValue, decodedDatetime) + + payloadInt, err = payload.Encode(123) + s.NoError(err) + payloadInt.Metadata["type"] = []byte("String") // MetadataType is not used. + decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT, allowList) + s.NoError(err) + s.Equal(int64(123), decodedInt) + + payloadInt, err = payload.Encode(123) + s.NoError(err) + payloadInt.Metadata["type"] = []byte("UnknownType") // MetadataType is not used. + decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_INT, allowList) + s.NoError(err) + s.Equal(int64(123), decodedInt) + + payloadBool, err := payload.Encode(true) + s.NoError(err) + decodedBool, err := DecodeValue(payloadBool, enumspb.INDEXED_VALUE_TYPE_BOOL, allowList) + s.NoError(err) + s.Equal(true, decodedBool) +} + +func Test_DecodeValue_NotAllowList_Error(t *testing.T) { + s := assert.New(t) + allowList := false + + payloadStr := payload.EncodeString("qwe") + decodedStr, err := DecodeValue(payloadStr, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.Error(err) + s.ErrorIs(err, ErrInvalidType) + s.Nil(decodedStr) + + payloadInt, err := payload.Encode(123) + s.NoError(err) + payloadInt.Metadata["type"] = []byte("UnknownType") + decodedInt, err := DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.Error(err) + s.ErrorIs(err, ErrInvalidType) + s.Nil(decodedInt) + + payloadInt, err = payload.Encode(123) + s.NoError(err) + payloadInt.Metadata["type"] = []byte("Text") + decodedInt, err = DecodeValue(payloadInt, enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED, allowList) + s.Error(err) + s.ErrorIs(err, converter.ErrUnableToDecode, err.Error()) + s.Nil(decodedInt) + + payloadSlice, err := payload.Encode([]string{"val1", "val2"}) + s.NoError(err) + decodedSlice, err := DecodeValue(payloadSlice, enumspb.INDEXED_VALUE_TYPE_KEYWORD, allowList) + s.Error(err) + s.Nil(decodedSlice) } func Test_EncodeValue(t *testing.T) { - assert := assert.New(t) + s := assert.New(t) encodedPayload, err := EncodeValue(123, enumspb.INDEXED_VALUE_TYPE_INT) - assert.NoError(err) - assert.Equal("123", string(encodedPayload.GetData())) - assert.Equal("Int", string(encodedPayload.Metadata["type"])) + s.NoError(err) + s.Equal("123", string(encodedPayload.GetData())) + s.Equal("Int", string(encodedPayload.Metadata["type"])) encodedPayload, err = EncodeValue("qwe", enumspb.INDEXED_VALUE_TYPE_TEXT) - assert.NoError(err) - assert.Equal(`"qwe"`, string(encodedPayload.GetData())) - assert.Equal("Text", string(encodedPayload.Metadata["type"])) + s.NoError(err) + s.Equal(`"qwe"`, string(encodedPayload.GetData())) + s.Equal("Text", string(encodedPayload.Metadata["type"])) encodedPayload, err = EncodeValue(nil, enumspb.INDEXED_VALUE_TYPE_DOUBLE) - assert.NoError(err) - assert.Equal("", string(encodedPayload.GetData())) - assert.Equal("Double", string(encodedPayload.Metadata["type"])) - assert.Equal("binary/null", string(encodedPayload.Metadata["encoding"])) + s.NoError(err) + s.Equal("", string(encodedPayload.GetData())) + s.Equal("Double", string(encodedPayload.Metadata["type"])) + s.Equal("binary/null", string(encodedPayload.Metadata["encoding"])) encodedPayload, err = EncodeValue([]string{"val1", "val2"}, enumspb.INDEXED_VALUE_TYPE_KEYWORD) - assert.NoError(err) - assert.Equal(`["val1","val2"]`, string(encodedPayload.GetData())) - assert.Equal("Keyword", string(encodedPayload.Metadata["type"])) - assert.Equal("json/plain", string(encodedPayload.Metadata["encoding"])) + s.NoError(err) + s.Equal(`["val1","val2"]`, string(encodedPayload.GetData())) + s.Equal("Keyword", string(encodedPayload.Metadata["type"])) + s.Equal("json/plain", string(encodedPayload.Metadata["encoding"])) encodedPayload, err = EncodeValue([]string{}, enumspb.INDEXED_VALUE_TYPE_KEYWORD) - assert.NoError(err) - assert.Equal("[]", string(encodedPayload.GetData())) - assert.Equal("Keyword", string(encodedPayload.Metadata["type"])) - assert.Equal("json/plain", string(encodedPayload.Metadata["encoding"])) + s.NoError(err) + s.Equal("[]", string(encodedPayload.GetData())) + s.Equal("Keyword", string(encodedPayload.Metadata["type"])) + s.Equal("json/plain", string(encodedPayload.Metadata["encoding"])) var expectedEncodedRepresentation = "2022-03-07T21:27:35.986848-05:00" timeValue, err := time.Parse(time.RFC3339, expectedEncodedRepresentation) - assert.NoError(err) + s.NoError(err) encodedPayload, err = EncodeValue(timeValue, enumspb.INDEXED_VALUE_TYPE_DATETIME) - assert.NoError(err) - assert.Equal(`"`+expectedEncodedRepresentation+`"`, string(encodedPayload.GetData()), + s.NoError(err) + s.Equal(`"`+expectedEncodedRepresentation+`"`, string(encodedPayload.GetData()), "Datetime Search Attribute is expected to be encoded in RFC 3339 format") - assert.Equal("Datetime", string(encodedPayload.Metadata["type"])) + s.Equal("Datetime", string(encodedPayload.Metadata["type"])) } diff --git a/common/searchattribute/stringify.go b/common/searchattribute/stringify.go index de15636d152..a7191a6ebac 100644 --- a/common/searchattribute/stringify.go +++ b/common/searchattribute/stringify.go @@ -57,7 +57,7 @@ func Stringify(searchAttributes *commonpb.SearchAttributes, typeMap *NameTypeMap if typeMap != nil { saType, _ = typeMap.getType(saName, customCategory|predefinedCategory) } - saValue, err := DecodeValue(saPayload, saType) + saValue, err := DecodeValue(saPayload, saType, true) if err != nil { // If DecodeValue failed, save error and use raw JSON from Data field. result[saName] = string(saPayload.GetData()) diff --git a/common/searchattribute/validator.go b/common/searchattribute/validator.go index 89141a39405..c3a297a19cd 100644 --- a/common/searchattribute/validator.go +++ b/common/searchattribute/validator.go @@ -45,6 +45,9 @@ type ( searchAttributesSizeOfValueLimit dynamicconfig.IntPropertyFnWithNamespaceFilter searchAttributesTotalSizeLimit dynamicconfig.IntPropertyFnWithNamespaceFilter indexName string + + // allowList allows list of values when it's not keyword list type. + allowList bool } ) @@ -56,6 +59,7 @@ func NewValidator( searchAttributesSizeOfValueLimit dynamicconfig.IntPropertyFnWithNamespaceFilter, searchAttributesTotalSizeLimit dynamicconfig.IntPropertyFnWithNamespaceFilter, indexName string, + allowList bool, ) *Validator { return &Validator{ searchAttributesProvider: searchAttributesProvider, @@ -64,6 +68,7 @@ func NewValidator( searchAttributesSizeOfValueLimit: searchAttributesSizeOfValueLimit, searchAttributesTotalSizeLimit: searchAttributesTotalSizeLimit, indexName: indexName, + allowList: allowList, } } @@ -116,7 +121,8 @@ func (v *Validator) Validate(searchAttributes *commonpb.SearchAttributes, namesp ) } - if _, err = DecodeValue(saPayload, saType); err != nil { + _, err = DecodeValue(saPayload, saType, v.allowList) + if err != nil { var invalidValue interface{} if err = payload.Decode(saPayload, &invalidValue); err != nil { invalidValue = fmt.Sprintf("value from <%s>", saPayload.String()) diff --git a/common/searchattribute/validator_test.go b/common/searchattribute/validator_test.go index 9df4d1b82ee..71ad45546b7 100644 --- a/common/searchattribute/validator_test.go +++ b/common/searchattribute/validator_test.go @@ -55,6 +55,7 @@ func (s *searchAttributesValidatorSuite) TestSearchAttributesValidate() { dynamicconfig.GetIntPropertyFilteredByNamespace(sizeOfValueLimit), dynamicconfig.GetIntPropertyFilteredByNamespace(sizeOfTotalLimit), "", + true, ) namespace := "namespace" @@ -131,6 +132,7 @@ func (s *searchAttributesValidatorSuite) TestSearchAttributesValidate_Mapper() { dynamicconfig.GetIntPropertyFilteredByNamespace(sizeOfValueLimit), dynamicconfig.GetIntPropertyFilteredByNamespace(sizeOfTotalLimit), "", + false, ) namespace := "test-namespace" @@ -193,6 +195,7 @@ func (s *searchAttributesValidatorSuite) TestSearchAttributesValidateSize() { dynamicconfig.GetIntPropertyFilteredByNamespace(sizeOfValueLimit), dynamicconfig.GetIntPropertyFilteredByNamespace(sizeOfTotalLimit), "", + false, ) namespace := "namespace" @@ -231,6 +234,7 @@ func (s *searchAttributesValidatorSuite) TestSearchAttributesValidateSize_Mapper dynamicconfig.GetIntPropertyFilteredByNamespace(sizeOfValueLimit), dynamicconfig.GetIntPropertyFilteredByNamespace(sizeOfTotalLimit), "", + false, ) namespace := "test-namespace" diff --git a/service/frontend/workflow_handler.go b/service/frontend/workflow_handler.go index 626c820dd80..2400966b112 100644 --- a/service/frontend/workflow_handler.go +++ b/service/frontend/workflow_handler.go @@ -73,6 +73,7 @@ import ( "go.temporal.io/server/common/payloads" "go.temporal.io/server/common/persistence" "go.temporal.io/server/common/persistence/serialization" + "go.temporal.io/server/common/persistence/visibility" "go.temporal.io/server/common/persistence/visibility/manager" "go.temporal.io/server/common/primitives" "go.temporal.io/server/common/primitives/timestamp" @@ -184,6 +185,7 @@ func NewWorkflowHandler( config.SearchAttributesSizeOfValueLimit, config.SearchAttributesTotalSizeLimit, visibilityMrg.GetIndexName(), + visibility.AllowListForValidation(visibilityMrg.GetName()), ), archivalMetadata: archivalMetadata, healthServer: healthServer, diff --git a/service/frontend/workflow_handler_test.go b/service/frontend/workflow_handler_test.go index 28484efb39f..a7ecb25efe2 100644 --- a/service/frontend/workflow_handler_test.go +++ b/service/frontend/workflow_handler_test.go @@ -160,6 +160,8 @@ func (s *workflowHandlerSuite) SetupTest() { mockMonitor := s.mockResource.MembershipMonitor mockMonitor.EXPECT().GetMemberCount(primitives.FrontendService).Return(5, nil).AnyTimes() + + s.mockVisibilityMgr.EXPECT().GetName().Return("").AnyTimes() } func (s *workflowHandlerSuite) TearDownTest() { diff --git a/service/history/commandChecker_test.go b/service/history/commandChecker_test.go index 289f827cec2..8d42642c563 100644 --- a/service/history/commandChecker_test.go +++ b/service/history/commandChecker_test.go @@ -131,6 +131,7 @@ func (s *commandAttrValidatorSuite) SetupTest() { config.SearchAttributesSizeOfValueLimit, config.SearchAttributesTotalSizeLimit, "index-name", + false, )) } diff --git a/service/history/historyEngine.go b/service/history/historyEngine.go index 3cffd7dab11..c6051d42a58 100644 --- a/service/history/historyEngine.go +++ b/service/history/historyEngine.go @@ -47,6 +47,7 @@ import ( "go.temporal.io/server/common/namespace" "go.temporal.io/server/common/persistence" "go.temporal.io/server/common/persistence/serialization" + "go.temporal.io/server/common/persistence/visibility" "go.temporal.io/server/common/persistence/visibility/manager" "go.temporal.io/server/common/primitives/timestamp" "go.temporal.io/server/common/sdk" @@ -230,6 +231,7 @@ func NewEngineWithShardContext( config.SearchAttributesSizeOfValueLimit, config.SearchAttributesTotalSizeLimit, config.DefaultVisibilityIndexName, + visibility.AllowListForValidation(persistenceVisibilityMgr.GetName()), ) historyEngImpl.workflowTaskHandler = newWorkflowTaskHandlerCallback(historyEngImpl) diff --git a/service/history/historyEngine2_test.go b/service/history/historyEngine2_test.go index 5f3e7fb7b46..2bf20fe1aa8 100644 --- a/service/history/historyEngine2_test.go +++ b/service/history/historyEngine2_test.go @@ -194,6 +194,7 @@ func (s *engine2Suite) SetupTest() { s.config.SearchAttributesSizeOfValueLimit, s.config.SearchAttributesTotalSizeLimit, s.config.DefaultVisibilityIndexName, + false, ), workflowConsistencyChecker: api.NewWorkflowConsistencyChecker(mockShard, s.workflowCache), } diff --git a/service/history/workflowTaskHandlerCallbacks_test.go b/service/history/workflowTaskHandlerCallbacks_test.go index a32f114cfca..0093c406897 100644 --- a/service/history/workflowTaskHandlerCallbacks_test.go +++ b/service/history/workflowTaskHandlerCallbacks_test.go @@ -125,6 +125,7 @@ func (s *WorkflowTaskHandlerCallbackSuite) SetupTest() { config.SearchAttributesSizeOfValueLimit, config.SearchAttributesTotalSizeLimit, config.DefaultVisibilityIndexName, + false, ), workflowConsistencyChecker: api.NewWorkflowConsistencyChecker(mockShard, workflowCache), }