diff --git a/src/Couchbase.Analytics/Cluster.cs b/src/Couchbase.Analytics/Cluster.cs index 67fc2ae..a7c2015 100644 --- a/src/Couchbase.Analytics/Cluster.cs +++ b/src/Couchbase.Analytics/Cluster.cs @@ -228,6 +228,9 @@ public Database Database(string databaseName) public void Dispose() { - // TODO release managed resources here + if (_serviceProvider is IDisposable disposableProvider) + { + disposableProvider.Dispose(); + } } } diff --git a/src/Couchbase.Analytics/Internal/DI/CouchbaseServiceProvider.cs b/src/Couchbase.Analytics/Internal/DI/CouchbaseServiceProvider.cs index 370179b..8db04a5 100644 --- a/src/Couchbase.Analytics/Internal/DI/CouchbaseServiceProvider.cs +++ b/src/Couchbase.Analytics/Internal/DI/CouchbaseServiceProvider.cs @@ -19,6 +19,7 @@ * ************************************************************/ #endregion +using System; using System.Collections.ObjectModel; namespace Couchbase.AnalyticsClient.Internal.DI; @@ -80,4 +81,38 @@ public bool IsService(Type serviceType) return false; } + + private bool _disposed; + + public void Dispose() + { + if (_disposed) + { + return; + } + + _disposed = true; + + var exceptions = new List(); + + foreach (var factory in _services.Values) + { + if (factory is IDisposable disposableFactory) + { + try + { + disposableFactory.Dispose(); + } + catch (Exception ex) + { + exceptions.Add(ex); + } + } + } + + if (exceptions.Count > 0) + { + throw new AggregateException(exceptions); + } + } } diff --git a/src/Couchbase.Analytics/Internal/DI/ICouchbaseServiceProvider.cs b/src/Couchbase.Analytics/Internal/DI/ICouchbaseServiceProvider.cs index ce7e6b9..6c80c12 100644 --- a/src/Couchbase.Analytics/Internal/DI/ICouchbaseServiceProvider.cs +++ b/src/Couchbase.Analytics/Internal/DI/ICouchbaseServiceProvider.cs @@ -24,7 +24,7 @@ namespace Couchbase.AnalyticsClient.Internal.DI; /// /// Extends with a method to test for service registration. /// -internal interface ICouchbaseServiceProvider : IServiceProvider +internal interface ICouchbaseServiceProvider : IServiceProvider, IDisposable { /// /// Determines if the specified service type is available from the . diff --git a/src/Couchbase.Analytics/Internal/DI/IServiceFactory.cs b/src/Couchbase.Analytics/Internal/DI/IServiceFactory.cs index 7246b72..bb17920 100644 --- a/src/Couchbase.Analytics/Internal/DI/IServiceFactory.cs +++ b/src/Couchbase.Analytics/Internal/DI/IServiceFactory.cs @@ -24,7 +24,7 @@ namespace Couchbase.AnalyticsClient.Internal.DI; /// /// A factory capable of returning a service. /// -internal interface IServiceFactory +internal interface IServiceFactory : IDisposable { /// /// Initializes the factory, making it owned by the given . diff --git a/src/Couchbase.Analytics/Internal/DI/SingletonGenericServiceFactory.cs b/src/Couchbase.Analytics/Internal/DI/SingletonGenericServiceFactory.cs index 186b703..ada36a9 100644 --- a/src/Couchbase.Analytics/Internal/DI/SingletonGenericServiceFactory.cs +++ b/src/Couchbase.Analytics/Internal/DI/SingletonGenericServiceFactory.cs @@ -131,4 +131,29 @@ private object Factory(Type requestedType) return constructor.Invoke(constructorArgs); } + + public void Dispose() + { + var exceptions = new List(); + + foreach (var singleton in _singletons.Values) + { + if (singleton is IDisposable disposable) + { + try + { + disposable.Dispose(); + } + catch (Exception ex) + { + exceptions.Add(ex); + } + } + } + + if (exceptions.Count > 0) + { + throw new AggregateException(exceptions); + } + } } diff --git a/src/Couchbase.Analytics/Internal/DI/SingletonServiceFactory.cs b/src/Couchbase.Analytics/Internal/DI/SingletonServiceFactory.cs index 92c37c7..3727612 100644 --- a/src/Couchbase.Analytics/Internal/DI/SingletonServiceFactory.cs +++ b/src/Couchbase.Analytics/Internal/DI/SingletonServiceFactory.cs @@ -19,6 +19,7 @@ * ************************************************************/ #endregion +using System; using System.Diagnostics.CodeAnalysis; using Couchbase.AnalyticsClient.Utils; @@ -108,4 +109,12 @@ object Factory(IServiceProvider serviceProvider) return Factory; } + + public void Dispose() + { + if (_singleton is IDisposable disposable) + { + disposable.Dispose(); + } + } } diff --git a/src/Couchbase.Analytics/Internal/DI/TransientServiceFactory.cs b/src/Couchbase.Analytics/Internal/DI/TransientServiceFactory.cs index 5f80374..f482b5b 100644 --- a/src/Couchbase.Analytics/Internal/DI/TransientServiceFactory.cs +++ b/src/Couchbase.Analytics/Internal/DI/TransientServiceFactory.cs @@ -96,4 +96,9 @@ object Factory(IServiceProvider serviceProvider) return constructor.Invoke(constructorArgs); } } + + public void Dispose() + { + // Transient factories do not manage singleton resources, so disposal is a no-op + } } diff --git a/src/Couchbase.Analytics/Internal/HTTP/CouchbaseHttpClientFactory.cs b/src/Couchbase.Analytics/Internal/HTTP/CouchbaseHttpClientFactory.cs index 804f751..7b2fe34 100644 --- a/src/Couchbase.Analytics/Internal/HTTP/CouchbaseHttpClientFactory.cs +++ b/src/Couchbase.Analytics/Internal/HTTP/CouchbaseHttpClientFactory.cs @@ -223,5 +223,10 @@ private static async Task DisposeAfterDelayAsync(AuthenticationHandler handler) handler.Dispose(); } + public void Dispose() + { + _sharedHandler?.Dispose(); + } + public HttpCompletionOption DefaultCompletionOption { get; set; } = HttpCompletionOption.ResponseHeadersRead; } diff --git a/src/Couchbase.Analytics/Internal/HTTP/ICouchbaseHttpClientFactory.cs b/src/Couchbase.Analytics/Internal/HTTP/ICouchbaseHttpClientFactory.cs index ff250fd..86b7615 100644 --- a/src/Couchbase.Analytics/Internal/HTTP/ICouchbaseHttpClientFactory.cs +++ b/src/Couchbase.Analytics/Internal/HTTP/ICouchbaseHttpClientFactory.cs @@ -25,7 +25,7 @@ namespace Couchbase.AnalyticsClient.Internal.HTTP; /// Creates an which may be safely configured and disposed, but while /// reusing inner handlers for connection pooling and HTTP keep-alives. /// -internal interface ICouchbaseHttpClientFactory +internal interface ICouchbaseHttpClientFactory : IDisposable { /// /// Creates an which may be safely configured and disposed, but while diff --git a/tests/Couchbase.Analytics.UnitTests/ClusterDisposalTests.cs b/tests/Couchbase.Analytics.UnitTests/ClusterDisposalTests.cs new file mode 100644 index 0000000..ce54bd5 --- /dev/null +++ b/tests/Couchbase.Analytics.UnitTests/ClusterDisposalTests.cs @@ -0,0 +1,28 @@ +using System; +using Couchbase.AnalyticsClient; +using Couchbase.AnalyticsClient.HTTP; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Couchbase.Analytics.UnitTests; + +public class ClusterDisposalTests +{ + [Fact] + public void Cluster_Dispose_DisposesAllRegisteredSingletons() + { + var mockLoggerFactory = new Mock(); + + var cluster = Cluster.Create( + "http://localhost:8095", + new Credential("Administrator", "password"), + opts => opts.WithLogging(mockLoggerFactory.Object)); + + // Trigger the top-level Dispose, which must cascade down the DI structural tree + // and aggressively purge the singleton cache. + cluster.Dispose(); + + mockLoggerFactory.Verify(x => x.Dispose(), Times.Once, "The Cluster failed to actively dispose its registered singleton resources (like the logging framework and SocketsHttpHandler) during destruction!"); + } +} diff --git a/tests/Couchbase.Analytics.UnitTests/Internal/DI/CouchbaseServiceProviderTests.cs b/tests/Couchbase.Analytics.UnitTests/Internal/DI/CouchbaseServiceProviderTests.cs new file mode 100644 index 0000000..1eec904 --- /dev/null +++ b/tests/Couchbase.Analytics.UnitTests/Internal/DI/CouchbaseServiceProviderTests.cs @@ -0,0 +1,92 @@ +using System; +using System.Collections.Generic; +using Couchbase.AnalyticsClient.Internal.DI; +using Moq; +using Xunit; + +namespace Couchbase.Analytics.UnitTests.Internal.DI; + +public class CouchbaseServiceProviderTests +{ + [Fact] + public void Dispose_IteratesAndDisposesAllDisposableFactories() + { + // Arrange + var mockNonDisposableFactory = new Mock(); + var mockDisposableFactory = new Mock(); + + // Explicitly project the second mock to support the IDisposable interface + var disposableInterface = mockDisposableFactory.As(); + + var services = new Dictionary + { + { typeof(string), mockNonDisposableFactory.Object }, + { typeof(int), mockDisposableFactory.Object } + }; + + var provider = new CouchbaseServiceProvider(services); + + // Act + provider.Dispose(); + + // Assert + // The normal factory should simply receive its baseline Initialize call from the constructor, and nothing else. + mockNonDisposableFactory.Verify(f => f.Initialize(It.IsAny()), Times.Once); + + // ...and definitively fires exactly once against any factory holding the IDisposable pattern. + disposableInterface.Verify(d => d.Dispose(), Times.Once); + } + + [Fact] + public void Dispose_IsIdempotent() + { + // Arrange + var mockDisposableFactory = new Mock(); + var disposableInterface = mockDisposableFactory.As(); + + var services = new Dictionary + { + { typeof(int), mockDisposableFactory.Object } + }; + + var provider = new CouchbaseServiceProvider(services); + + // Act + provider.Dispose(); + provider.Dispose(); // Call second time + provider.Dispose(); // Call third time + + // Assert + // Due to the _disposed flag, multiple explicit invocations must collapse harmlessly, + // firing downstream teardowns EXACTLY once. + disposableInterface.Verify(d => d.Dispose(), Times.Once); + } + + [Fact] + public void Dispose_ExceptionInOneFactory_DoesNotPreventDisposalOfOthers() + { + // Arrange + var mockFailingFactory = new Mock(); + var failingDisposable = mockFailingFactory.As(); + failingDisposable.Setup(d => d.Dispose()).Throws(new InvalidOperationException("Boom")); + + var mockWorkingFactory = new Mock(); + var workingDisposable = mockWorkingFactory.As(); + + var services = new Dictionary + { + { typeof(int), mockFailingFactory.Object }, + { typeof(string), mockWorkingFactory.Object } + }; + + var provider = new CouchbaseServiceProvider(services); + + // Act + var aggregateEx = Assert.Throws(() => provider.Dispose()); + + // Assert + // We guarantee the loop continued executing despite the explosion. + Assert.Contains(aggregateEx.InnerExceptions, ex => ex is InvalidOperationException && ex.Message == "Boom"); + workingDisposable.Verify(d => d.Dispose(), Times.Once); + } +} diff --git a/tests/Couchbase.Analytics.UnitTests/Internal/DI/SingletonGenericServiceFactoryTests.cs b/tests/Couchbase.Analytics.UnitTests/Internal/DI/SingletonGenericServiceFactoryTests.cs new file mode 100644 index 0000000..605350f --- /dev/null +++ b/tests/Couchbase.Analytics.UnitTests/Internal/DI/SingletonGenericServiceFactoryTests.cs @@ -0,0 +1,55 @@ +using System; +using Couchbase.AnalyticsClient.Internal.DI; +using Moq; +using Xunit; + +namespace Couchbase.Analytics.UnitTests.Internal.DI; + +public interface IFakeGenericService : IDisposable { } + +public class FakeGenericService : IFakeGenericService +{ + public static Action? OnDispose { get; set; } + + public FakeGenericService() { } + + public void Dispose() + { + OnDispose?.Invoke(); + } +} + +public class SingletonGenericServiceFactoryTests +{ + [Fact] + public void SingletonGenericServiceFactory_Dispose_DisposesAllCachedPermutations() + { + // Arrange + var factory = new SingletonGenericServiceFactory(typeof(FakeGenericService<>)); + + var mockServiceProvider = new Mock(); + factory.Initialize(mockServiceProvider.Object); + + // Populate the concurrent dictionary with different permutations of the generic + factory.CreateService(typeof(IFakeGenericService)); + factory.CreateService(typeof(IFakeGenericService)); + factory.CreateService(typeof(IFakeGenericService)); + + int disposeCount = 0; + FakeGenericService.OnDispose = () => disposeCount++; + FakeGenericService.OnDispose = () => disposeCount++; + FakeGenericService.OnDispose = () => disposeCount++; + + // Act + factory.Dispose(); + + // Assert + // The factory should naturally iterate and call Dispose exactly 3 times (once per instantiated T) + Assert.Equal(3, disposeCount); + + // Cleanup statics + FakeGenericService.OnDispose = null; + FakeGenericService.OnDispose = null; + FakeGenericService.OnDispose = null; + } +} diff --git a/tests/Couchbase.Analytics.UnitTests/Internal/DI/SingletonServiceFactoryTests.cs b/tests/Couchbase.Analytics.UnitTests/Internal/DI/SingletonServiceFactoryTests.cs new file mode 100644 index 0000000..91a35ce --- /dev/null +++ b/tests/Couchbase.Analytics.UnitTests/Internal/DI/SingletonServiceFactoryTests.cs @@ -0,0 +1,23 @@ +using System; +using Couchbase.AnalyticsClient.Internal.DI; +using Moq; +using Xunit; + +namespace Couchbase.Analytics.UnitTests.Internal.DI; + +public class SingletonServiceFactoryTests +{ + [Fact] + public void SingletonServiceFactory_Dispose_DisposesInnerSingleton() + { + // Arrange + var mockDisposable = new Mock(); + var factory = new SingletonServiceFactory(mockDisposable.Object); + + // Act + factory.Dispose(); + + // Assert + mockDisposable.Verify(d => d.Dispose(), Times.Once); + } +} diff --git a/tests/Couchbase.Analytics.UnitTests/Internal/DI/TransientServiceFactoryTests.cs b/tests/Couchbase.Analytics.UnitTests/Internal/DI/TransientServiceFactoryTests.cs new file mode 100644 index 0000000..6038e06 --- /dev/null +++ b/tests/Couchbase.Analytics.UnitTests/Internal/DI/TransientServiceFactoryTests.cs @@ -0,0 +1,32 @@ +using System; +using Couchbase.AnalyticsClient.Internal.DI; +using Moq; +using Xunit; + +namespace Couchbase.Analytics.UnitTests.Internal.DI; + +public class TransientServiceFactoryTests +{ + [Fact] + public void TransientServiceFactory_Dispose_DoesNotDisposeGeneratedInstances() + { + // Arrange + var mockDisposable = new Mock(); + var factory = new TransientServiceFactory(_ => mockDisposable.Object); + + var mockServiceProvider = new Mock(); + factory.Initialize(mockServiceProvider.Object); + + // Act + // We simulate the lifetime: The user asks for a Transient object, then later the Cluster shuts down entirely. + var instance = factory.CreateService(typeof(IDisposable)); + + factory.Dispose(); + + // Assert + // We explicitly confirm that the Transient factory completely ignores the instances + // it generates, thus avoiding long-term memory leaks in the Cluster's DI container! + Assert.NotNull(instance); + mockDisposable.Verify(d => d.Dispose(), Times.Never); + } +} diff --git a/tests/Couchbase.Analytics.UnitTests/Internal/HTTP/CouchbaseHttpClientFactoryTests.cs b/tests/Couchbase.Analytics.UnitTests/Internal/HTTP/CouchbaseHttpClientFactoryTests.cs new file mode 100644 index 0000000..1272a5c --- /dev/null +++ b/tests/Couchbase.Analytics.UnitTests/Internal/HTTP/CouchbaseHttpClientFactoryTests.cs @@ -0,0 +1,38 @@ +using System; +using System.Net.Http; +using System.Threading.Tasks; +using Couchbase.AnalyticsClient; +using Couchbase.AnalyticsClient.HTTP; +using Couchbase.AnalyticsClient.Internal.HTTP; +using Couchbase.AnalyticsClient.Options; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace Couchbase.Analytics.UnitTests.Internal.HTTP; + +public class CouchbaseHttpClientFactoryTests +{ + [Fact] + public async Task CouchbaseHttpClientFactory_Dispose_CascadesToUnderlyingHandler() + { + // Arrange + var options = new ClusterOptions { ConnectionString = "http://localhost:8095" }; + var factory = new CouchbaseHttpClientFactory( + () => new Credential("Administrator", "password"), + options, + new NullLogger() + ); + + // Create an active HTTP client wrapping the shared AuthenticationHandler -> SocketsHttpHandler + var httpClient = factory.Create(); + + // Act + // Executing Dispose must trigger teardowns down the chain. + factory.Dispose(); + + // Assert + // A perfectly disposed downstream handler natively refuses any new HttpClient execution + // by immediately throwing an ObjectDisposedException preflight. + await Assert.ThrowsAsync(() => httpClient.GetAsync("http://127.0.0.1:8095")); + } +}