[AppConfiguration] Fix: GetConfigurationSettings does not set the Con…

…tentType property (#38917)
Azure · Sep 25, 2023 · 6da0421 · 6da0421
1 parent 209a340
commit 6da0421
Show file tree

Hide file tree

Showing 8 changed files with 169 additions and 143 deletions.
diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/CHANGELOG.md b/sdk/appconfiguration/Azure.Data.AppConfiguration/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 ### Bugs Fixed
 
+- Fixed `GetConfigurationSettings(SettingSelector)` not setting `ContentType` and `LastModified` properties [(#38524)](https://github.com/Azure/azure-sdk-for-net/issues/38524).
+
 ### Other Changes
 
 ## 1.2.1 (2023-09-13)

diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/assets.json b/sdk/appconfiguration/Azure.Data.AppConfiguration/assets.json
@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "net",
   "TagPrefix": "net/appconfiguration/Azure.Data.AppConfiguration",
-  "Tag": "net/appconfiguration/Azure.Data.AppConfiguration_c13e90b6f4"
+  "Tag": "net/appconfiguration/Azure.Data.AppConfiguration_bebb69bca2"
 }
diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs b/sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient.cs
@@ -4,7 +4,6 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
-using System.Text.Json;
 using System.Threading;
 using System.Threading.Tasks;
 using Azure.Core;
@@ -653,7 +652,7 @@ public virtual AsyncPageable<ConfigurationSetting> GetConfigurationSettingsAsync
             var label = selector.LabelFilter;
 
             RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken);
-            IEnumerable<string> fieldsString = selector.Fields == SettingFields.All ? null : selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(',');
+            IEnumerable<string> fieldsString = selector.Fields.Split();
 
             HttpMessage FirstPageRequest(int? pageSizeHint) => CreateGetConfigurationSettingsRequest(key, label, null, dateTime, fieldsString, null, null, context);
             HttpMessage NextPageRequest(int? pageSizeHint, string nextLink) => CreateGetConfigurationSettingsNextPageRequest(nextLink, key, label, null, dateTime, fieldsString, null, null, context);
@@ -673,7 +672,7 @@ public virtual Pageable<ConfigurationSetting> GetConfigurationSettings(SettingSe
             var dateTime = selector.AcceptDateTime?.UtcDateTime.ToString(AcceptDateTimeFormat, CultureInfo.InvariantCulture);
 
             RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken);
-            IEnumerable<string> fieldsString = selector.Fields == SettingFields.All ? null : selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(',');
+            IEnumerable<string> fieldsString = selector.Fields.Split();
 
             HttpMessage FirstPageRequest(int? pageSizeHint) => CreateGetConfigurationSettingsRequest(key, label, null, dateTime, fieldsString, null, null, context);
             HttpMessage NextPageRequest(int? pageSizeHint, string nextLink) => CreateGetConfigurationSettingsNextPageRequest(nextLink, key, label, null, dateTime, fieldsString, null, null, context);
@@ -725,7 +724,7 @@ public virtual AsyncPageable<ConfigurationSetting> GetConfigurationSettingsForSn
             Argument.AssertNotNullOrEmpty(snapshotName, nameof(snapshotName));
 
             RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken);
-            IEnumerable<string> fieldsString = fields == SettingFields.All ? null : fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(',');
+            IEnumerable<string> fieldsString = fields.Split();
 
             HttpMessage FirstPageRequest(int? pageSizeHint) => CreateGetConfigurationSettingsRequest(null, null, null, null, fieldsString, snapshotName, null, context);
             HttpMessage NextPageRequest(int? pageSizeHint, string nextLink) => CreateGetConfigurationSettingsNextPageRequest(nextLink, null, null, null, null, fieldsString, snapshotName, null, context);
@@ -743,7 +742,7 @@ public virtual Pageable<ConfigurationSetting> GetConfigurationSettingsForSnapsho
             Argument.AssertNotNullOrEmpty(snapshotName, nameof(snapshotName));
 
             RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken);
-            IEnumerable<string> fieldsString = fields == SettingFields.All ? null : fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(',');
+            IEnumerable<string> fieldsString = fields.Split();
 
             HttpMessage FirstPageRequest(int? pageSizeHint) => CreateGetConfigurationSettingsRequest(null, null, null, null, fieldsString, snapshotName, null, context);
             HttpMessage NextPageRequest(int? pageSizeHint, string nextLink) => CreateGetConfigurationSettingsNextPageRequest(nextLink, null, null, null, null, fieldsString, snapshotName, null, context);
@@ -1244,7 +1243,7 @@ public virtual AsyncPageable<ConfigurationSetting> GetRevisionsAsync(SettingSele
             var label = selector.LabelFilter;
             var dateTime = selector.AcceptDateTime?.UtcDateTime.ToString(AcceptDateTimeFormat, CultureInfo.InvariantCulture);
             RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken);
-            IEnumerable<string> fieldsString = selector.Fields == SettingFields.All ? null : selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(',');
+            IEnumerable<string> fieldsString = selector.Fields.Split();
 
             HttpMessage FirstPageRequest(int? pageSizeHint) => CreateGetRevisionsRequest(key, label, null, dateTime, fieldsString, context);
             HttpMessage NextPageRequest(int? pageSizeHint, string nextLink) => CreateGetRevisionsNextPageRequest(nextLink, key, label, null, dateTime, fieldsString, context);
@@ -1263,7 +1262,7 @@ public virtual Pageable<ConfigurationSetting> GetRevisions(SettingSelector selec
             var label = selector.LabelFilter;
             var dateTime = selector.AcceptDateTime?.UtcDateTime.ToString(AcceptDateTimeFormat, CultureInfo.InvariantCulture);
             RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken);
-            IEnumerable<string> fieldsString = selector.Fields == SettingFields.All ? null : selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(',');
+            IEnumerable<string> fieldsString = selector.Fields.Split();
 
             HttpMessage FirstPageRequest(int? pageSizeHint) => CreateGetRevisionsRequest(key, label, null, dateTime, fieldsString, context);
             HttpMessage NextPageRequest(int? pageSizeHint, string nextLink) => CreateGetRevisionsNextPageRequest(nextLink, key, label, null, dateTime, fieldsString, context);

diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient_private.cs b/sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient_private.cs
@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Collections.Generic;
 using System.ComponentModel;
 using System.Diagnostics;
 using System.Threading;
@@ -51,30 +52,6 @@ private static void ParseConnectionString(string connectionString, out Uri uri,
             }
         }
 
-        internal static void BuildBatchQuery(RequestUriBuilder builder, SettingSelector selector, string pageLink)
-        {
-            if (!string.IsNullOrEmpty(selector.KeyFilter))
-            {
-                builder.AppendQuery(KeyQueryFilter, selector.KeyFilter);
-            }
-
-            if (!string.IsNullOrEmpty(selector.LabelFilter))
-            {
-                builder.AppendQuery(LabelQueryFilter, selector.LabelFilter);
-            }
-
-            if (selector.Fields != SettingFields.All)
-            {
-                var filter = selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked");
-                builder.AppendQuery(FieldsQueryFilter, filter);
-            }
-
-            if (!string.IsNullOrEmpty(pageLink))
-            {
-                builder.AppendQuery("after", pageLink, escapeValue: false);
-            }
-        }
-
         #region nobody wants to see these
         /// <summary>
         /// Check if two ConfigurationSetting instances are equal.

diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/src/Models/SettingFieldsExtensions.cs b/sdk/appconfiguration/Azure.Data.AppConfiguration/src/Models/SettingFieldsExtensions.cs
@@ -0,0 +1,50 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Collections.Generic;
+
+namespace Azure.Data.AppConfiguration
+{
+    internal static class SettingFieldsExtensions
+    {
+        /// <summary>
+        /// Maps a SettingFields member to its corresponding service name in accordance with the REST API specification.
+        /// </summary>
+        private static readonly IReadOnlyDictionary<SettingFields, string> s_serviceNameMap = new Dictionary<SettingFields, string>()
+        {
+            { SettingFields.Key         , "key"           },
+            { SettingFields.Label       , "label"         },
+            { SettingFields.Value       , "value"         },
+            { SettingFields.ContentType , "content_type"  },
+            { SettingFields.ETag        , "etag"          },
+            { SettingFields.LastModified, "last_modified" },
+            { SettingFields.IsReadOnly  , "locked"        },
+            { SettingFields.Tags        , "tags"          }
+        };
+
+        /// <summary>
+        /// Splits <see cref="SettingFields"/> flags into their corresponding service names.
+        /// </summary>
+        /// <param name="fields">The flags to split.</param>
+        /// <returns>An enumerable containing the names of the flags. The method returns <c>null</c> for <see cref="SettingFields.All"/>.</returns>
+        public static IEnumerable<string> Split(this SettingFields fields)
+        {
+            if (fields == SettingFields.All)
+            {
+                return null;
+            }
+
+            var splitFields = new List<string>();
+
+            foreach (SettingFields currentField in s_serviceNameMap.Keys)
+            {
+                if ((fields & currentField) == currentField)
+                {
+                    splitFields.Add(s_serviceNameMap[currentField]);
+                }
+            }
+
+            return splitFields;
+        }
+    }
+}
diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationLiveTests.cs b/sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationLiveTests.cs
@@ -1046,7 +1046,7 @@ public async Task GetBatchSettingWithFields()
 
                 Assert.IsNotNull(batch[0].Key);
                 Assert.IsNotNull(batch[0].Label);
-                Assert.IsNotNull(batch[0].ETag);
+                Assert.AreNotEqual(batch[0].ETag, default(ETag));
                 Assert.IsNull(batch[0].Value);
                 Assert.IsNull(batch[0].ContentType);
                 Assert.IsNull(batch[0].LastModified);
@@ -1118,7 +1118,7 @@ public async Task GetBatchSettingWithAllFields()
                 Assert.IsNotNull(batch[0].Label);
                 Assert.IsNotNull(batch[0].Value);
                 Assert.IsNotNull(batch[0].ContentType);
-                Assert.IsNotNull(batch[0].ETag);
+                Assert.AreNotEqual(batch[0].ETag, default(ETag));
                 Assert.IsNotNull(batch[0].LastModified);
                 Assert.IsNotNull(batch[0].IsReadOnly);
             }
@@ -1128,6 +1128,46 @@ public async Task GetBatchSettingWithAllFields()
             }
         }
 
+        [RecordedTest]
+        public async Task GetBatchSettingWithAllFieldsSetExplicitly()
+        {
+            ConfigurationClient service = GetClient();
+            string key = GenerateKeyId("keyFields-");
+            ConfigurationSetting setting = await service.AddConfigurationSettingAsync(new ConfigurationSetting(key, "my_value", "my_label")
+            {
+                ContentType = "content-type",
+                Tags = { { "my_tag", "my_tag_value" } }
+            });
+
+            try
+            {
+                SettingSelector selector = new SettingSelector
+                {
+                    KeyFilter = key,
+                    Fields = SettingFields.Key | SettingFields.Label | SettingFields.Value | SettingFields.ContentType
+                        | SettingFields.ETag | SettingFields.LastModified | SettingFields.IsReadOnly | SettingFields.Tags
+                };
+
+                ConfigurationSetting[] batch = (await service.GetConfigurationSettingsAsync(selector, CancellationToken.None).ToEnumerableAsync())
+                    .ToArray();
+
+                Assert.AreEqual(1, batch.Length);
+
+                Assert.IsNotNull(batch[0].Key);
+                Assert.IsNotNull(batch[0].Label);
+                Assert.IsNotNull(batch[0].Value);
+                Assert.IsNotNull(batch[0].ContentType);
+                Assert.AreNotEqual(batch[0].ETag, default(ETag));
+                Assert.IsNotNull(batch[0].LastModified);
+                Assert.IsNotNull(batch[0].IsReadOnly);
+                Assert.IsNotEmpty(batch[0].Tags);
+            }
+            finally
+            {
+                AssertStatus200(await service.DeleteConfigurationSettingAsync(setting.Key, setting.Label));
+            }
+        }
+
         [RecordedTest]
         public async Task GetBatchSettingSpecialCharacters()
         {

diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationSettingTests.cs b/sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationSettingTests.cs
@@ -5,8 +5,6 @@
 using System;
 using System.Collections.Generic;
 using System.Text.Json;
-using System.Text.Json.Serialization;
-using Azure.Core;
 
 namespace Azure.Data.AppConfiguration.Tests
 {
@@ -29,113 +27,6 @@ public class ConfigurationSettingTests
             }
         };
 
-        [Test]
-        public void FilterReservedCharacter()
-        {
-            var selector = new SettingSelector
-            {
-                KeyFilter = @"my_key,key\,key",
-                LabelFilter = @"my_label,label\,label"
-            };
-
-            var builder = new RequestUriBuilder();
-            builder.Reset(new Uri("http://localhost/"));
-
-            ConfigurationClient.BuildBatchQuery(builder, selector, null);
-
-            Assert.AreEqual(@"http://localhost/?key=my_key%2Ckey%5C%2Ckey&label=my_label%2Clabel%5C%2Clabel", builder.ToUri().AbsoluteUri);
-        }
-
-        [Test]
-        public void FilterContains()
-        {
-            var selector = new SettingSelector{ KeyFilter = "*key*", LabelFilter = "*label*" };
-            var builder = new RequestUriBuilder();
-            builder.Reset(new Uri("http://localhost/"));
-
-            ConfigurationClient.BuildBatchQuery(builder, selector, null);
-
-            Assert.AreEqual("http://localhost/?key=%2Akey%2A&label=%2Alabel%2A", builder.ToUri().AbsoluteUri);
-        }
-
-        [Test]
-        public void FilterNullLabel()
-        {
-            var selector = new SettingSelector { LabelFilter = "\0" };
-
-            var builder = new RequestUriBuilder();
-            builder.Reset(new Uri("http://localhost/"));
-
-            ConfigurationClient.BuildBatchQuery(builder, selector, null);
-
-            Assert.AreEqual("http://localhost/?label=%00", builder.ToUri().AbsoluteUri);
-        }
-
-        [Test]
-        public void FilterOnlyKey()
-        {
-            var key = "my-key";
-            var selector = new SettingSelector { KeyFilter = key };
-
-            var builder = new RequestUriBuilder();
-            builder.Reset(new Uri("http://localhost/"));
-
-            ConfigurationClient.BuildBatchQuery(builder, selector, null);
-
-            Assert.AreEqual($"http://localhost/?key={key}", builder.ToUri().AbsoluteUri);
-        }
-
-        [Test]
-        public void FilterOnlyLabel()
-        {
-            var label = "my-label";
-            var selector = new SettingSelector
-            {
-                LabelFilter = label
-            };
-
-            var builder = new RequestUriBuilder();
-            builder.Reset(new Uri("http://localhost/"));
-
-            ConfigurationClient.BuildBatchQuery(builder, selector, null);
-
-            Assert.AreEqual($"http://localhost/?label={label}", builder.ToUri().AbsoluteUri);
-        }
-
-        [Test]
-        public void SettingSomeFields()
-        {
-            var selector = new SettingSelector
-            {
-                KeyFilter = "key",
-                Fields = SettingFields.Key | SettingFields.Value
-            };
-
-            var builder = new RequestUriBuilder();
-            builder.Reset(new Uri("http://localhost/"));
-
-            ConfigurationClient.BuildBatchQuery(builder, selector, null);
-
-            Assert.AreEqual($"http://localhost/?key=key&$select=key%2C%20value", builder.ToUri().AbsoluteUri);
-        }
-
-        [Test]
-        public void SettingAllFields()
-        {
-            var selector = new SettingSelector
-            {
-                KeyFilter = "key",
-                Fields = SettingFields.All
-            };
-
-            var builder = new RequestUriBuilder();
-            builder.Reset(new Uri("http://localhost/"));
-
-            ConfigurationClient.BuildBatchQuery(builder, selector, null);
-
-            Assert.AreEqual($"http://localhost/?key=key", builder.ToUri().AbsoluteUri);
-        }
-
         [Test]
         public void ConfigurationSettingEquals()
         {