diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/CHANGELOG.md b/sdk/appconfiguration/Azure.Data.AppConfiguration/CHANGELOG.md index 0f8524ef1e7e8..117755569b405 100644 --- 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 index 6be2f9404c2b4..1eac8ff51bcd9 100644 --- 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 index 34e6080482797..15724903e77e6 100644 --- 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 GetConfigurationSettingsAsync var label = selector.LabelFilter; RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken); - IEnumerable fieldsString = selector.Fields == SettingFields.All ? null : selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(','); + IEnumerable 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 GetConfigurationSettings(SettingSe var dateTime = selector.AcceptDateTime?.UtcDateTime.ToString(AcceptDateTimeFormat, CultureInfo.InvariantCulture); RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken); - IEnumerable fieldsString = selector.Fields == SettingFields.All ? null : selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(','); + IEnumerable 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 GetConfigurationSettingsForSn Argument.AssertNotNullOrEmpty(snapshotName, nameof(snapshotName)); RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken); - IEnumerable fieldsString = fields == SettingFields.All ? null : fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(','); + IEnumerable 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 GetConfigurationSettingsForSnapsho Argument.AssertNotNullOrEmpty(snapshotName, nameof(snapshotName)); RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken); - IEnumerable fieldsString = fields == SettingFields.All ? null : fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(','); + IEnumerable 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 GetRevisionsAsync(SettingSele var label = selector.LabelFilter; var dateTime = selector.AcceptDateTime?.UtcDateTime.ToString(AcceptDateTimeFormat, CultureInfo.InvariantCulture); RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken); - IEnumerable fieldsString = selector.Fields == SettingFields.All ? null : selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(','); + IEnumerable 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 GetRevisions(SettingSelector selec var label = selector.LabelFilter; var dateTime = selector.AcceptDateTime?.UtcDateTime.ToString(AcceptDateTimeFormat, CultureInfo.InvariantCulture); RequestContext context = CreateRequestContext(ErrorOptions.Default, cancellationToken); - IEnumerable fieldsString = selector.Fields == SettingFields.All ? null : selector.Fields.ToString().ToLowerInvariant().Replace("isreadonly", "locked").Split(','); + IEnumerable 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 index 114a7bb7a5472..f211883ea397f 100644 --- 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 /// /// 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 new file mode 100644 index 0000000000000..081c605d29ce1 --- /dev/null +++ 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 + { + /// + /// Maps a SettingFields member to its corresponding service name in accordance with the REST API specification. + /// + private static readonly IReadOnlyDictionary s_serviceNameMap = new Dictionary() + { + { 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" } + }; + + /// + /// Splits flags into their corresponding service names. + /// + /// The flags to split. + /// An enumerable containing the names of the flags. The method returns null for . + public static IEnumerable Split(this SettingFields fields) + { + if (fields == SettingFields.All) + { + return null; + } + + var splitFields = new List(); + + 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 index d42439a635c5e..6070aca7fc430 100644 --- 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 index d0b5e6c29ca58..096f99e0b1b72 100644 --- 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() { diff --git a/sdk/appconfiguration/Azure.Data.AppConfiguration/tests/SettingFieldsExtensionsTests.cs b/sdk/appconfiguration/Azure.Data.AppConfiguration/tests/SettingFieldsExtensionsTests.cs new file mode 100644 index 0000000000000..36da6df865bed --- /dev/null +++ b/sdk/appconfiguration/Azure.Data.AppConfiguration/tests/SettingFieldsExtensionsTests.cs @@ -0,0 +1,67 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using NUnit.Framework; + +namespace Azure.Data.AppConfiguration.Tests +{ + public class SettingFieldsExtensionsTests + { + [Test] + public void SplitReturnsNullForAll() + { + Assert.IsNull(SettingFields.All.Split()); + } + + [Test] + [TestCase(SettingFields.Key, "key")] + [TestCase(SettingFields.Label, "label")] + [TestCase(SettingFields.Value, "value")] + [TestCase(SettingFields.ContentType, "content_type")] + [TestCase(SettingFields.ETag, "etag")] + [TestCase(SettingFields.LastModified, "last_modified")] + [TestCase(SettingFields.IsReadOnly, "locked")] + [TestCase(SettingFields.Tags, "tags")] + public void SplitWithSingleField(SettingFields fields, string expectedFieldString) + { + IEnumerable splitFields = fields.Split(); + string fieldString = splitFields.Single(); + + Assert.AreEqual(fieldString, expectedFieldString); + } + + [Test] + public void SplitWithMultipleFields() + { + SettingFields fields = SettingFields.Key | SettingFields.ContentType | SettingFields.LastModified | SettingFields.IsReadOnly; + IEnumerable splitFields = fields.Split(); + + Assert.AreEqual(splitFields.Count(), 4); + CollectionAssert.Contains(splitFields, "key"); + CollectionAssert.Contains(splitFields, "content_type"); + CollectionAssert.Contains(splitFields, "last_modified"); + CollectionAssert.Contains(splitFields, "locked"); + } + + [Test] + public void SplitSupportsAllPossibleSettingFields() + { + foreach (SettingFields fields in Enum.GetValues(typeof(SettingFields))) + { + if (fields == SettingFields.All) + { + continue; + } + + IEnumerable splitFields = fields.Split(); + + // If this assertion fails, it's likely that a new enum value has been added to SettingFields + // but a corresponding entry has not been added to s_serviceNameMap in SettingFieldsExtensions. + Assert.AreEqual(splitFields.Count(), 1, $"{nameof(SettingFields)} enum value {fields} could not be mapped to a string."); + } + } + } +}