Fix issue with search attribute keyword list type (#529)

Fixes #526

Author: cretz

src/Temporalio/Common/SearchAttributeCollection.cs

@@ -25,7 +25,7 @@ public class SearchAttributeCollection : IReadOnlyCollection<SearchAttributeKey>
         /// <summary>
         /// An empty search attribute collection.
         /// </summary>
-        public static readonly SearchAttributeCollection Empty = new(new());
+        public static readonly SearchAttributeCollection Empty = new();
 
         private static readonly Dictionary<ByteString, IndexedValueType> NameToIndexType =
             Enum.GetValues(typeof(IndexedValueType)).Cast<IndexedValueType>().ToDictionary(
@@ -41,9 +41,12 @@ public class SearchAttributeCollection : IReadOnlyCollection<SearchAttributeKey>
         /// <summary>
         /// Initializes a new instance of the <see cref="SearchAttributeCollection"/> class.
         /// </summary>
-        /// <param name="values">The values that are actually used. Warning, this is not copied.
-        /// </param>
-        internal SearchAttributeCollection(SortedDictionary<SearchAttributeKey, object> values) =>
+        internal SearchAttributeCollection()
+            : this(new())
+        {
+        }
+
+        private SearchAttributeCollection(SortedDictionary<SearchAttributeKey, object> values) =>
             this.values = values;
 
         /// <summary>
@@ -200,7 +203,7 @@ internal static (object Value, IndexedValueType Type)? PayloadToObject(Payload p
             static object JsonElementToObject(JsonElement elem, IndexedValueType valueType) => elem.ValueKind switch
             {
                 JsonValueKind.Array =>
-                    elem.EnumerateArray().Select(j => JsonElementToObject(j, valueType)).ToList(),
+                    elem.EnumerateArray().Select(j => JsonElementToObject(j, valueType)).OfType<string>().ToList(),
                 JsonValueKind.False or JsonValueKind.True =>
                     elem.GetBoolean(),
                 JsonValueKind.Number when valueType == IndexedValueType.Int =>
@@ -369,7 +372,8 @@ private Builder InternalSet(SearchAttributeKey key, object value)
                 {
                     values.Remove(existingKey);
                 }
-                values[key] = value ?? throw new ArgumentException("Null value", nameof(value));
+                values[key] = key.NormalizeValue(
+                    value ?? throw new ArgumentException("Null value", nameof(value)));
                 return this;
             }
         }

src/Temporalio/Common/SearchAttributeKey.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using Temporalio.Api.Enums.V1;
 
 namespace Temporalio.Common
@@ -166,6 +167,24 @@ public int CompareTo(SearchAttributeKey? other)
         /// <returns>Whether equal.</returns>
         public bool Equals(SearchAttributeKey? other) =>
             other != null && other.Name == Name && other.ValueType == ValueType;
+
+        /// <summary>
+        /// Normalize value into a canonical representation based on its value type.
+        /// </summary>
+        /// <typeparam name="T">Expected return type.</typeparam>
+        /// <param name="value">Value to normalize.</param>
+        /// <returns>Normalized value.</returns>
+        internal T NormalizeValue<T>(T value)
+            where T : notnull
+        {
+            // For keyword list, it will always be IReadOnlyCollection<string>, but we need to
+            // normalize as a list of strings
+            if (ValueType == IndexedValueType.KeywordList && value is IReadOnlyCollection<string> strColl)
+            {
+                return (T)(object)strColl.ToList();
+            }
+            return value;
+        }
     }
 
     /// <summary>

src/Temporalio/Worker/WorkflowInstance.cs

@@ -143,7 +143,7 @@ public WorkflowInstance(WorkflowInstanceDetails details)
                 false);
             var initialSearchAttributes = details.Init.SearchAttributes;
             typedSearchAttributes = new(
-                () => initialSearchAttributes == null ? new(new()) :
+                () => initialSearchAttributes == null ? new() :
                     SearchAttributeCollection.FromProto(initialSearchAttributes),
                 false);
             var act = details.InitialActivation;

src/Temporalio/Workflows/SearchAttributeUpdate.cs

@@ -90,7 +90,7 @@ public class SearchAttributeUpdate<T> : SearchAttributeUpdate
         internal SearchAttributeUpdate(SearchAttributeKey<T> key, T value)
         {
             Key = key;
-            this.value = value;
+            this.value = key.NormalizeValue(value);
             HasValue = true;
         }
 

tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs

@@ -1404,8 +1404,7 @@ public class SearchAttributesWorkflow
             Set(AttrDateTime, new DateTimeOffset(2001, 1, 1, 0, 0, 0, TimeSpan.Zero)).
             Set(AttrDouble, 123.45).
             Set(AttrKeyword, "SomeKeyword").
-            // TODO(cretz): Fix after Temporal dev server upgraded
-            // Set(AttrKeywordList, new[] { "SomeKeyword1", "SomeKeyword2" }).
+            Set(AttrKeywordList, new[] { "SomeKeyword1", "SomeKeyword2" }).
             Set(AttrLong, 678).
             Set(AttrText, "SomeText").
             ToSearchAttributeCollection();
@@ -1424,6 +1423,7 @@ public class SearchAttributesWorkflow
         public static readonly SearchAttributeCollection AttributesFirstUpdated = new SearchAttributeCollection.Builder().
             Set(AttrBool, false).
             Set(AttrDateTime, new DateTimeOffset(2002, 1, 1, 0, 0, 0, TimeSpan.Zero)).
+            Set(AttrKeywordList, new[] { "SomeKeyword1", "SomeKeyword2" }).
             Set(AttrDouble, 234.56).
             ToSearchAttributeCollection();
 
@@ -1434,21 +1434,27 @@ public class SearchAttributesWorkflow
             AttrDateTime.ValueUnset(),
             AttrDouble.ValueUnset(),
             AttrKeyword.ValueSet("AnotherKeyword"),
+            AttrKeywordList.ValueSet(new[] { "SomeKeyword3", "SomeKeyword4" }),
             AttrLong.ValueSet(789),
             AttrText.ValueSet("SomeOtherText"),
         };
 
         public static readonly SearchAttributeCollection AttributesSecondUpdated = new SearchAttributeCollection.Builder().
             Set(AttrKeyword, "AnotherKeyword").
+            Set(AttrKeywordList, new[] { "SomeKeyword3", "SomeKeyword4" }).
             Set(AttrLong, 789).
             Set(AttrText, "SomeOtherText").
             ToSearchAttributeCollection();
 
         public static void AssertAttributesEqual(
             SearchAttributeCollection expected, SearchAttributeCollection actual) =>
+            // xUnit compares dictionaries using Equals on key and value properly even if they have
+            // differing subtypes
             Assert.Equal(
-                expected.UntypedValues.Where(kvp => kvp.Key.Name.StartsWith("DotNet")),
-                actual.UntypedValues.Where(kvp => kvp.Key.Name.StartsWith("DotNet")));
+                expected.UntypedValues.Where(kvp => kvp.Key.Name.StartsWith("DotNet")).
+                    ToDictionary(kvp => kvp.Key, kvp => kvp.Value),
+                actual.UntypedValues.Where(kvp => kvp.Key.Name.StartsWith("DotNet")).
+                    ToDictionary(kvp => kvp.Key, kvp => kvp.Value));
 
         private bool proceed;
 

tests/Temporalio.Tests/WorkflowEnvironmentTestBase.cs

@@ -84,8 +84,7 @@ await Client.OperatorService.AddSearchAttributesAsync(new()
                     [AttrDateTime.Name] = AttrDateTime.ValueType,
                     [AttrDouble.Name] = AttrDouble.ValueType,
                     [AttrKeyword.Name] = AttrKeyword.ValueType,
-                    // TODO(cretz): Fix after Temporal dev server upgraded
-                    // [AttrKeywordList.Name] = AttrKeywordList.ValueType,
+                    [AttrKeywordList.Name] = AttrKeywordList.ValueType,
                     [AttrLong.Name] = AttrLong.ValueType,
                     [AttrText.Name] = AttrText.ValueType,
                 },