Add IDiscoveryClient.InstancesFetched event (#1672)

* Add IDiscoveryClient.InstancesFetched event

* Consul: don't use UseNetworkInterfaces to determine ports (reverts change from #1666)

* Cleanup queries, fix case insensivity

Author: bart-vmware

src/Common/src/Common/Discovery/DiscoveryInstancesFetchedEventArgs.cs

@@ -0,0 +1,23 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the Apache 2.0 License.
+// See the LICENSE file in the project root for more information.
+
+namespace Steeltoe.Common.Discovery;
+
+/// <summary>
+/// Provides data for the <see cref="IDiscoveryClient.InstancesFetched" /> event.
+/// </summary>
+public sealed class DiscoveryInstancesFetchedEventArgs : EventArgs
+{
+    /// <summary>
+    /// Gets the updated list of service instances, grouped by service ID.
+    /// </summary>
+    public IReadOnlyDictionary<string, IReadOnlyList<IServiceInstance>> InstancesByServiceId { get; }
+
+    public DiscoveryInstancesFetchedEventArgs(IReadOnlyDictionary<string, IReadOnlyList<IServiceInstance>> instancesByServiceId)
+    {
+        ArgumentNullException.ThrowIfNull(instancesByServiceId);
+
+        InstancesByServiceId = instancesByServiceId;
+    }
+}

src/Common/src/Common/Discovery/IDiscoveryClient.cs

@@ -14,6 +14,11 @@ public interface IDiscoveryClient
     /// </summary>
     string Description { get; }
 
+    /// <summary>
+    /// Occurs when service instances have been fetched from the discovery server.
+    /// </summary>
+    public event EventHandler<DiscoveryInstancesFetchedEventArgs> InstancesFetched;
+
     /// <summary>
     /// Gets information used to register the local service instance (this app) to the discovery server.
     /// </summary>

src/Common/src/Common/PublicAPI.Unshipped.txt

@@ -1 +1,5 @@
 #nullable enable
+Steeltoe.Common.Discovery.DiscoveryInstancesFetchedEventArgs
+Steeltoe.Common.Discovery.DiscoveryInstancesFetchedEventArgs.DiscoveryInstancesFetchedEventArgs(System.Collections.Generic.IReadOnlyDictionary<string!, System.Collections.Generic.IReadOnlyList<Steeltoe.Common.Discovery.IServiceInstance!>!>! instancesByServiceId) -> void
+Steeltoe.Common.Discovery.DiscoveryInstancesFetchedEventArgs.InstancesByServiceId.get -> System.Collections.Generic.IReadOnlyDictionary<string!, System.Collections.Generic.IReadOnlyList<Steeltoe.Common.Discovery.IServiceInstance!>!>!
+Steeltoe.Common.Discovery.IDiscoveryClient.InstancesFetched -> System.EventHandler<Steeltoe.Common.Discovery.DiscoveryInstancesFetchedEventArgs!>!

src/Configuration/test/ConfigServer.Discovery.Test/ConfigServerDiscoveryServiceTest.cs

@@ -114,6 +114,10 @@ private sealed class TestDiscoveryClient : IDiscoveryClient
     {
         public string Description => throw new NotImplementedException();
 
+#pragma warning disable CS0067 // The event is never used
+        public event EventHandler<DiscoveryInstancesFetchedEventArgs>? InstancesFetched;
+#pragma warning restore CS0067 // The event is never used
+
         public Task<ISet<string>> GetServiceIdsAsync(CancellationToken cancellationToken)
         {
             throw new NotImplementedException();

src/Discovery/src/Configuration/ConfigurationDiscoveryClient.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the Apache 2.0 License.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.ObjectModel;
 using Microsoft.Extensions.Options;
 using Steeltoe.Common.Discovery;
 
@@ -10,17 +11,69 @@ namespace Steeltoe.Discovery.Configuration;
 /// <summary>
 /// A discovery client that reads service instances from app configuration.
 /// </summary>
-public sealed class ConfigurationDiscoveryClient : IDiscoveryClient
+public sealed class ConfigurationDiscoveryClient : IDiscoveryClient, IDisposable
 {
     private readonly IOptionsMonitor<ConfigurationDiscoveryOptions> _optionsMonitor;
+    private readonly IDisposable? _changeTokenRegistration;
 
     public string Description => "A discovery client that returns service instances from app configuration.";
 
+    /// <summary>
+    /// Occurs when the configuration of service instances has been reloaded.
+    /// </summary>
+    public event EventHandler<DiscoveryInstancesFetchedEventArgs>? InstancesFetched;
+
     public ConfigurationDiscoveryClient(IOptionsMonitor<ConfigurationDiscoveryOptions> optionsMonitor)
     {
         ArgumentNullException.ThrowIfNull(optionsMonitor);
 
         _optionsMonitor = optionsMonitor;
+        _changeTokenRegistration = optionsMonitor.OnChange(OnOptionsChanged);
+    }
+
+    private void OnOptionsChanged(ConfigurationDiscoveryOptions options)
+    {
+        if (InstancesFetched != null)
+        {
+            ReadOnlyDictionary<string, IReadOnlyList<IServiceInstance>> instancesByServiceId = ToServiceInstanceMap(options.Services);
+            var eventArgs = new DiscoveryInstancesFetchedEventArgs(instancesByServiceId);
+            RaiseFetchEvent(eventArgs);
+        }
+    }
+
+    private static ReadOnlyDictionary<string, IReadOnlyList<IServiceInstance>> ToServiceInstanceMap(IList<ConfigurationServiceInstance> services)
+    {
+        // @formatter:wrap_chained_method_calls chop_always
+        // @formatter:wrap_before_first_method_call true
+
+        return services
+            .Where(service => service.ServiceId != null)
+            .GroupBy(service => service.ServiceId!, StringComparer.OrdinalIgnoreCase)
+            .ToDictionary(grouping => grouping.Key, grouping => (IReadOnlyList<IServiceInstance>)grouping
+                .Cast<IServiceInstance>()
+                .ToList()
+                .AsReadOnly(), StringComparer.OrdinalIgnoreCase)
+            .AsReadOnly();
+
+        // @formatter:wrap_before_first_method_call restore
+        // @formatter:wrap_chained_method_calls restore
+    }
+
+    private void RaiseFetchEvent(DiscoveryInstancesFetchedEventArgs eventArgs)
+    {
+        // Execute on separate thread, so we won't block the configuration system in case the handler logic is expensive.
+        ThreadPool.QueueUserWorkItem(_ =>
+        {
+            try
+            {
+                InstancesFetched?.Invoke(this, eventArgs);
+            }
+            catch (Exception)
+            {
+                // Intentionally left empty. Adding a logger to the constructor is a breaking change.
+                // Adding an extra constructor confuses the service container.
+            }
+        });
     }
 
     /// <inheritdoc />
@@ -54,4 +107,10 @@ public Task ShutdownAsync(CancellationToken cancellationToken)
     {
         return Task.CompletedTask;
     }
+
+    /// <inheritdoc />
+    public void Dispose()
+    {
+        _changeTokenRegistration?.Dispose();
+    }
 }

src/Discovery/src/Configuration/PublicAPI.Unshipped.txt

@@ -1 +1,3 @@
 #nullable enable
+Steeltoe.Discovery.Configuration.ConfigurationDiscoveryClient.Dispose() -> void
+Steeltoe.Discovery.Configuration.ConfigurationDiscoveryClient.InstancesFetched -> System.EventHandler<Steeltoe.Common.Discovery.DiscoveryInstancesFetchedEventArgs!>?

src/Discovery/src/Consul/Configuration/ConsulDiscoveryOptions.cs

@@ -176,8 +176,7 @@ public sealed class ConsulDiscoveryOptions
     /// <summary>
     /// Gets or sets a value indicating whether to register with the port number ASP.NET Core is listening on. Default value: true.
     /// <para />
-    /// This property is ignored when <see cref="Port" /> or <see cref="Scheme" /> is explicitly configured, or when <see cref="UseNetworkInterfaces" /> is
-    /// <c>true</c>.
+    /// This property is ignored when <see cref="Port" /> or <see cref="Scheme" /> is explicitly configured.
     /// </summary>
     public bool UseAspNetCoreUrls { get; set; } = true;
 }

src/Discovery/src/Consul/ConfigurationSchema.json

@@ -188,7 +188,7 @@
             },
             "UseAspNetCoreUrls": {
               "type": "boolean",
-              "description": "Gets or sets a value indicating whether to register with the port number ASP.NET Core is listening on. Default value: true.\n\nThis property is ignored when 'Steeltoe.Discovery.Consul.Configuration.ConsulDiscoveryOptions.Port' or 'Steeltoe.Discovery.Consul.Configuration.ConsulDiscoveryOptions.Scheme' is explicitly configured, or when 'Steeltoe.Discovery.Consul.Configuration.ConsulDiscoveryOptions.UseNetworkInterfaces' is true."
+              "description": "Gets or sets a value indicating whether to register with the port number ASP.NET Core is listening on. Default value: true.\n\nThis property is ignored when 'Steeltoe.Discovery.Consul.Configuration.ConsulDiscoveryOptions.Port' or 'Steeltoe.Discovery.Consul.Configuration.ConsulDiscoveryOptions.Scheme' is explicitly configured."
             },
             "UseNetworkInterfaces": {
               "type": "boolean",

src/Discovery/src/Consul/ConsulDiscoveryClient.cs

@@ -30,6 +30,13 @@ public sealed class ConsulDiscoveryClient : IDiscoveryClient
     /// <inheritdoc />
     public string Description => "A discovery client for HashiCorp Consul.";
 
+    /// <summary>
+    /// This event is never raised. The Consul client doesn't implement caching.
+    /// </summary>
+#pragma warning disable CS0067 // The event is never used
+    public event EventHandler<DiscoveryInstancesFetchedEventArgs>? InstancesFetched;
+#pragma warning restore CS0067 // The event is never used
+
     /// <summary>
     /// Initializes a new instance of the <see cref="ConsulDiscoveryClient" /> class.
     /// </summary>

src/Discovery/src/Consul/PostConfigureConsulDiscoveryOptions.cs

@@ -62,7 +62,7 @@ public void PostConfigure(string? name, ConsulDiscoveryOptions options)
             options.HostName = options.IPAddress;
         }
 
-        if (options is { UseAspNetCoreUrls: true, Port: 0, Scheme: null, UseNetworkInterfaces: false })
+        if (options is { UseAspNetCoreUrls: true, Port: 0, Scheme: null })
         {
             ICollection<string> addresses = _configuration.GetListenAddresses();
             SetSchemeWithPortFromListenAddresses(options, addresses);