Add RegisterAll API to enable monitoring collections of key-values for refresh (#574)

* WIP

* WIP testing out client extensions methods

* WIP added selectors to multikeywatchers

* remove unused property

* WIP check for registerall changes to change refreshall

* WIP

* WIP fixing types and reslving errors

* WIP fixing client extensions class

* WIP

* WIP update feature flag logic

* WIP client extensions

* WIP reload all flags on change

* WIP

* WIP fixing tests to return response for getconfigurationsettingsasync

* WIP etag for tests

* fix watchedcollections null

* WIP tests, working for examples

* remove unused variables

* update to newest sdk version, remove unused

* WIP fixing tests

* WIP reworking testing to work with new etag approach

* tests passing, fix mockasyncpageable

* update sdk package version

* fix loghelper, tests

* WIP fixing aspages tests

* revert watchesfeatureflags test

* update test again

* WIP

* fixing watchconditions

* separate selected key value collections from feature flag collections, separate selectors, add new methods to support new logic

* comment and naming updates

* fixing unit tests, namespace of defining/calling code needs to be same

* fixing tests using AsPages

* fix tests with pageablemanager

* format

* fix tests

* fix tests

* remove unused extension test class

* fix comment, capitalization

* check etag on 200, fix tests

* add registerall test, fix refresh tests

* fix condition for pages and old match conditions

* WIP fixing PR comments, tests

* check status after advancing existing etag enumerator

* move around refresh logic

* null check page etag, revert break to existing keys check in getrefreshedcollections

* fix loadselected, replace selectedkvwatchers with registerall refresh time

* fix comment in options

* clean up tests

* PR comments

* PR comments

* don't allow both registerall and register

* fix check for calls to both register methods

* PR comments for rename/small changes

* fix compile error

* simplify refreshasync path, fix naming from comments

* remove redundant if check

* simplify logic for minrefreshinterval

* fix smaller comments

* call loadselected when refreshing collection, separate data for individual refresh

* in progress change to registerall include ff

* fix load order

* fix comments, rename logging constants to match new behavior

* pr comments, refactor refreshasync

* clean up etags dictionary creation

* PR comments

* add uncommitted changes to testhelper

* update tests for registerall with feature flags, check ff keys to remove flags on refresh

* PR comments

* PR comments

* use invalidoperationexception in configurerefresh, update loggingconstants to match behavior

* remove unused changes

Author: amerjusupovic

src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationOptions.cs

@@ -23,12 +23,13 @@ public class AzureAppConfigurationOptions
         private const int MaxRetries = 2;
         private static readonly TimeSpan MaxRetryDelay = TimeSpan.FromMinutes(1);
 
-        private List<KeyValueWatcher> _changeWatchers = new List<KeyValueWatcher>();
-        private List<KeyValueWatcher> _multiKeyWatchers = new List<KeyValueWatcher>();
+        private List<KeyValueWatcher> _individualKvWatchers = new List<KeyValueWatcher>();
+        private List<KeyValueWatcher> _ffWatchers = new List<KeyValueWatcher>();
         private List<IKeyValueAdapter> _adapters;
         private List<Func<ConfigurationSetting, ValueTask<ConfigurationSetting>>> _mappers = new List<Func<ConfigurationSetting, ValueTask<ConfigurationSetting>>>();
-        private List<KeyValueSelector> _kvSelectors = new List<KeyValueSelector>();
+        private List<KeyValueSelector> _selectors;
         private IConfigurationRefresher _refresher = new AzureAppConfigurationRefresher();
+        private bool _selectCalled = false;
 
         // The following set is sorted in descending order.
         // Since multiple prefixes could start with the same characters, we need to trim the longest prefix first.
@@ -62,19 +63,29 @@ public class AzureAppConfigurationOptions
         internal TokenCredential Credential { get; private set; }
 
         /// <summary>
-        /// A collection of <see cref="KeyValueSelector"/>.
+        /// A collection of <see cref="KeyValueSelector"/> specified by user.
         /// </summary>
-        internal IEnumerable<KeyValueSelector> KeyValueSelectors => _kvSelectors;
+        internal IEnumerable<KeyValueSelector> Selectors => _selectors;
+
+        /// <summary>
+        /// Indicates if <see cref="AzureAppConfigurationRefreshOptions.RegisterAll"/> was called.
+        /// </summary>
+        internal bool RegisterAllEnabled { get; private set; }
+
+        /// <summary>
+        /// Refresh interval for selected key-value collections when <see cref="AzureAppConfigurationRefreshOptions.RegisterAll"/> is called.
+        /// </summary>
+        internal TimeSpan KvCollectionRefreshInterval { get; private set; }
 
         /// <summary>
         /// A collection of <see cref="KeyValueWatcher"/>.
         /// </summary>
-        internal IEnumerable<KeyValueWatcher> ChangeWatchers => _changeWatchers;
+        internal IEnumerable<KeyValueWatcher> IndividualKvWatchers => _individualKvWatchers;
 
         /// <summary>
         /// A collection of <see cref="KeyValueWatcher"/>.
         /// </summary>
-        internal IEnumerable<KeyValueWatcher> MultiKeyWatchers => _multiKeyWatchers;
+        internal IEnumerable<KeyValueWatcher> FeatureFlagWatchers => _ffWatchers;
 
         /// <summary>
         /// A collection of <see cref="IKeyValueAdapter"/>.
@@ -96,11 +107,15 @@ internal IEnumerable<IKeyValueAdapter> Adapters
         internal IEnumerable<string> KeyPrefixes => _keyPrefixes;
 
         /// <summary>
-        /// An optional configuration client manager that can be used to provide clients to communicate with Azure App Configuration.
+        /// For use in tests only. An optional configuration client manager that can be used to provide clients to communicate with Azure App Configuration.
         /// </summary>
-        /// <remarks>This property is used only for unit testing.</remarks>
         internal IConfigurationClientManager ClientManager { get; set; }
 
+        /// <summary>
+        /// For use in tests only. An optional class used to process pageable results from Azure App Configuration.
+        /// </summary>
+        internal IConfigurationSettingPageIterator ConfigurationSettingPageIterator { get; set; }
+
         /// <summary>
         /// An optional timespan value to set the minimum backoff duration to a value other than the default.
         /// </summary>
@@ -142,6 +157,9 @@ public AzureAppConfigurationOptions()
                 new JsonKeyValueAdapter(),
                 new FeatureManagementKeyValueAdapter(FeatureFlagTracing)
             };
+
+            // Adds the default query to App Configuration if <see cref="Select"/> and <see cref="SelectSnapshot"/> are never called.
+            _selectors = new List<KeyValueSelector> { new KeyValueSelector { KeyFilter = KeyFilter.Any, LabelFilter = LabelFilter.Null } };
         }
 
         /// <summary>
@@ -170,22 +188,30 @@ public AzureAppConfigurationOptions Select(string keyFilter, string labelFilter
                 throw new ArgumentNullException(nameof(keyFilter));
             }
 
+            // Do not support * and , for label filter for now.
+            if (labelFilter != null && (labelFilter.Contains('*') || labelFilter.Contains(',')))
+            {
+                throw new ArgumentException("The characters '*' and ',' are not supported in label filters.", nameof(labelFilter));
+            }
+
             if (string.IsNullOrWhiteSpace(labelFilter))
             {
                 labelFilter = LabelFilter.Null;
             }
 
-            // Do not support * and , for label filter for now.
-            if (labelFilter.Contains('*') || labelFilter.Contains(','))
+            if (!_selectCalled)
             {
-                throw new ArgumentException("The characters '*' and ',' are not supported in label filters.", nameof(labelFilter));
+                _selectors.Clear();
+
+                _selectCalled = true;
             }
 
-            _kvSelectors.AppendUnique(new KeyValueSelector
+            _selectors.AppendUnique(new KeyValueSelector
             {
                 KeyFilter = keyFilter,
                 LabelFilter = labelFilter
             });
+
             return this;
         }
 
@@ -201,7 +227,14 @@ public AzureAppConfigurationOptions SelectSnapshot(string name)
                 throw new ArgumentNullException(nameof(name));
             }
 
-            _kvSelectors.AppendUnique(new KeyValueSelector
+            if (!_selectCalled)
+            {
+                _selectors.Clear();
+
+                _selectCalled = true;
+            }
+
+            _selectors.AppendUnique(new KeyValueSelector
             {
                 SnapshotName = name
             });
@@ -212,7 +245,7 @@ public AzureAppConfigurationOptions SelectSnapshot(string name)
         /// <summary>
         /// Configures options for Azure App Configuration feature flags that will be parsed and transformed into feature management configuration.
         /// If no filtering is specified via the <see cref="FeatureFlagOptions"/> then all feature flags with no label are loaded.
-        /// All loaded feature flags will be automatically registered for refresh on an individual flag level.
+        /// All loaded feature flags will be automatically registered for refresh as a collection.
         /// </summary>
         /// <param name="configure">A callback used to configure feature flag options.</param>
         public AzureAppConfigurationOptions UseFeatureFlags(Action<FeatureFlagOptions> configure = null)
@@ -237,25 +270,22 @@ public AzureAppConfigurationOptions UseFeatureFlags(Action<FeatureFlagOptions> c
                 options.FeatureFlagSelectors.Add(new KeyValueSelector
                 {
                     KeyFilter = FeatureManagementConstants.FeatureFlagMarker + "*",
-                    LabelFilter = options.Label == null ? LabelFilter.Null : options.Label
+                    LabelFilter = string.IsNullOrWhiteSpace(options.Label) ? LabelFilter.Null : options.Label,
+                    IsFeatureFlagSelector = true
                 });
             }
 
-            foreach (var featureFlagSelector in options.FeatureFlagSelectors)
+            foreach (KeyValueSelector featureFlagSelector in options.FeatureFlagSelectors)
             {
-                var featureFlagFilter = featureFlagSelector.KeyFilter;
-                var labelFilter = featureFlagSelector.LabelFilter;
+                _selectors.AppendUnique(featureFlagSelector);
 
-                Select(featureFlagFilter, labelFilter);
-
-                _multiKeyWatchers.AppendUnique(new KeyValueWatcher
+                _ffWatchers.AppendUnique(new KeyValueWatcher
                 {
-                    Key = featureFlagFilter,
-                    Label = labelFilter,
+                    Key = featureFlagSelector.KeyFilter,
+                    Label = featureFlagSelector.LabelFilter,
                     // If UseFeatureFlags is called multiple times for the same key and label filters, last refresh interval wins
                     RefreshInterval = options.RefreshInterval
                 });
-
             }
 
             return this;
@@ -376,18 +406,41 @@ public AzureAppConfigurationOptions ConfigureClientOptions(Action<ConfigurationC
         /// <param name="configure">A callback used to configure Azure App Configuration refresh options.</param>
         public AzureAppConfigurationOptions ConfigureRefresh(Action<AzureAppConfigurationRefreshOptions> configure)
         {
+            if (RegisterAllEnabled)
+            {
+                throw new InvalidOperationException($"{nameof(ConfigureRefresh)}() cannot be invoked multiple times when {nameof(AzureAppConfigurationRefreshOptions.RegisterAll)} has been invoked.");
+            }
+
             var refreshOptions = new AzureAppConfigurationRefreshOptions();
             configure?.Invoke(refreshOptions);
 
-            if (!refreshOptions.RefreshRegistrations.Any())
+            bool isRegisterCalled = refreshOptions.RefreshRegistrations.Any();
+            RegisterAllEnabled = refreshOptions.RegisterAllEnabled;
+
+            if (!isRegisterCalled && !RegisterAllEnabled)
+            {
+                throw new InvalidOperationException($"{nameof(ConfigureRefresh)}() must call either {nameof(AzureAppConfigurationRefreshOptions.Register)}()" +
+                    $" or {nameof(AzureAppConfigurationRefreshOptions.RegisterAll)}()");
+            }
+
+            // Check if both register methods are called at any point
+            if (RegisterAllEnabled && (_individualKvWatchers.Any() || isRegisterCalled))
             {
-                throw new ArgumentException($"{nameof(ConfigureRefresh)}() must have at least one key-value registered for refresh.");
+                throw new InvalidOperationException($"Cannot call both {nameof(AzureAppConfigurationRefreshOptions.RegisterAll)} and "
+                + $"{nameof(AzureAppConfigurationRefreshOptions.Register)}.");
             }
 
-            foreach (var item in refreshOptions.RefreshRegistrations)
+            if (RegisterAllEnabled)
+            {
+                KvCollectionRefreshInterval = refreshOptions.RefreshInterval;
+            }
+            else
             {
-                item.RefreshInterval = refreshOptions.RefreshInterval;
-                _changeWatchers.Add(item);
+                foreach (KeyValueWatcher item in refreshOptions.RefreshRegistrations)
+                {
+                    item.RefreshInterval = refreshOptions.RefreshInterval;
+                    _individualKvWatchers.Add(item);
+                }
             }
 
             return this;