diff --git a/src/ControllersTree/Core/Utils/ControllerCompositeDisposable.cs b/src/ControllersTree/Core/Utils/ControllerCompositeDisposable.cs index 434a0d5..a78d451 100644 --- a/src/ControllersTree/Core/Utils/ControllerCompositeDisposable.cs +++ b/src/ControllersTree/Core/Utils/ControllerCompositeDisposable.cs @@ -11,6 +11,7 @@ namespace Playtika.Controllers public class ControllerCompositeDisposable : IDisposable { private readonly List _disposables = ListPool.Get(); + private bool _disposed = false; /// /// Adds a disposable object to the internal list of disposables. @@ -18,7 +19,14 @@ public class ControllerCompositeDisposable : IDisposable /// The disposable object to add to the list. public void Add(IDisposable disposable) { - _disposables.Add(disposable); + if (_disposed) + { + disposable?.Dispose(); + } + else if (disposable != null) + { + _disposables.Add(disposable); + } } /// @@ -27,18 +35,50 @@ public void Add(IDisposable disposable) /// The collection of disposable objects to add to the list. public void AddRange(IEnumerable collection) { - _disposables.AddRange(collection); + if (collection == null) + { + return; + } + + if (_disposed) + { + using var pooledObject = ListPool.Get(out var disposablesList); + disposablesList.AddRange(collection.Where(c => c != null)); + DisposeMany(disposablesList); + } + else + { + _disposables.AddRange(collection); + } } public void Dispose() { - using var pooledObject = ListPool.Get(out var exceptionList); + if (_disposed) + { + return; + } + + _disposed = true; + + try + { + DisposeMany(_disposables); + } + finally + { + ListPool.Release(_disposables); + } + } - foreach (var disposable in _disposables) + private static void DisposeMany(IEnumerable disposables) + { + using var pooledObject = ListPool.Get(out var exceptionList); + foreach (var disposable in disposables) { try { - disposable.Dispose(); + disposable?.Dispose(); } catch (Exception e) { @@ -46,14 +86,12 @@ public void Dispose() } } - _disposables.Clear(); - - ListPool.Release(_disposables); - - if (exceptionList.Any()) + switch (exceptionList.Count) { - throw new AggregateException(exceptionList); + case 0: return; + case 1: throw exceptionList[0]; + default: throw new AggregateException(exceptionList.ToList()); } } } -} +} \ No newline at end of file diff --git a/src/ControllersTree/Tests/ControllerCompositeDisposableTests.cs b/src/ControllersTree/Tests/ControllerCompositeDisposableTests.cs new file mode 100644 index 0000000..d2dc66a --- /dev/null +++ b/src/ControllersTree/Tests/ControllerCompositeDisposableTests.cs @@ -0,0 +1,267 @@ +using System; +using NUnit.Framework; +using Playtika.Controllers; + +namespace UnitTests.Controllers +{ + [TestFixture] + public class ControllerCompositeDisposableTests + { + [Test] + public void ControllerCompositeDisposable_SingleDisposable_Disposes() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var disposable = new TestDisposable(); + + composite.Add(disposable); + + // Act + composite.Dispose(); + + // Assert + Assert.AreEqual(1, disposable.DisposeCallCount, "Disposable should be disposed exactly once"); + } + + [Test] + public void ControllerCompositeDisposable_DisposeEmpty_DoesNotThrow() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + + // Act / Assert + Assert.DoesNotThrow( + () => + { + composite.Dispose(); + }); + } + + [Test] + public void ControllerCompositeDisposable_DisposeTwice_IsIdempotent() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var disposable = new TestDisposable(); + composite.Add(disposable); + + // Act + composite.Dispose(); + composite.Dispose(); + + // Assert + Assert.AreEqual(1, disposable.DisposeCallCount, "Disposable should still be disposed exactly once"); + } + + [Test] + public void ControllerCompositeDisposable_AddAfterDisposed_DisposesImmediately() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var disposable = new TestDisposable(); + + composite.Dispose(); + + // Act + composite.Add(disposable); + + // Assert + Assert.AreEqual(1, disposable.DisposeCallCount, "Disposable added after composite dispose should be disposed immediately"); + } + + [Test] + public void ControllerCompositeDisposable_AddNull_IgnoredAndOtherDisposablesWork() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var disposable = new TestDisposable(); + + composite.Add(null); + composite.Add(disposable); + composite.Add(null); + + // Act + composite.Dispose(); + + // Assert + Assert.AreEqual(1, disposable.DisposeCallCount, "Non-null disposable should be disposed exactly once"); + } + + [Test] + public void ControllerCompositeDisposable_AddRangeBeforeDispose_DisposesAll() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var d1 = new TestDisposable(); + var d2 = new TestDisposable(); + var collection = new[] { d1, d2 }; + + composite.AddRange(collection); + + // Act + composite.Dispose(); + + // Assert + Assert.AreEqual(1, d1.DisposeCallCount, "First disposable should be disposed exactly once"); + Assert.AreEqual(1, d2.DisposeCallCount, "Second disposable should be disposed exactly once"); + } + + [Test] + public void ControllerCompositeDisposable_AddRangeNullCollection_DoesNotThrow() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + + // Act / Assert + Assert.DoesNotThrow( + () => + { + composite.AddRange(null); + composite.Dispose(); + }); + } + + [Test] + public void ControllerCompositeDisposable_AddRangeWithNullItems_DisposesNonNullOnly() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var d1 = new TestDisposable(); + var d3 = new TestDisposable(); + var collection = new[] { d1, null, d3 }; + + composite.AddRange(collection); + + // Act + composite.Dispose(); + + // Assert + Assert.AreEqual(1, d1.DisposeCallCount, "First non-null disposable should be disposed exactly once"); + Assert.AreEqual(1, d3.DisposeCallCount, "Second non-null disposable should be disposed exactly once"); + } + + [Test] + public void ControllerCompositeDisposable_AddRangeAfterDisposed_DisposesAllImmediately() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var d1 = new TestDisposable(); + var d2 = new TestDisposable(); + var collection = new[] { d1, d2 }; + + composite.Dispose(); + + // Act + composite.AddRange(collection); + + // Assert + Assert.AreEqual(1, d1.DisposeCallCount, "First disposable should be disposed immediately"); + Assert.AreEqual(1, d2.DisposeCallCount, "Second disposable should be disposed immediately"); + } + + [Test] + public void ControllerCompositeDisposable_OneDisposableThrows_ThrowsSingleExceptionAndDisposesOthers() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var throwingDisposable = new TestDisposable { ThrowOnDispose = true }; + var normalDisposable = new TestDisposable(); + + composite.Add(throwingDisposable); + composite.Add(normalDisposable); + + // Act + var exception = Assert.Throws( + () => composite.Dispose(), + "Expected TestControllersException from throwing disposable"); + + // Assert + Assert.IsNotNull(exception, "Exception should not be null"); + Assert.AreEqual(1, throwingDisposable.DisposeCallCount, "Throwing disposable should be disposed exactly once"); + Assert.AreEqual(1, normalDisposable.DisposeCallCount, "Normal disposable should still be disposed exactly once"); + } + + [Test] + public void ControllerCompositeDisposable_MultipleDisposablesThrow_ThrowsAggregateExceptionWithAllInner() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var d1 = new TestDisposable("d1") { ThrowOnDispose = true }; + var d2 = new TestDisposable("d2") { ThrowOnDispose = true }; + var d3 = new TestDisposable("d3"); + + composite.Add(d1); + composite.Add(d2); + composite.Add(d3); + + // Act + var aggregate = Assert.Throws( + () => composite.Dispose(), + "Expected AggregateException when multiple disposables throw"); + + // Assert + Assert.IsNotNull(aggregate, "AggregateException should not be null"); + Assert.AreEqual(2, aggregate.InnerExceptions.Count, "AggregateException should contain all throwing disposables"); + foreach (var inner in aggregate.InnerExceptions) + { + Assert.IsInstanceOf(inner, "Inner exception should be TestControllersException"); + } + + Assert.AreEqual(1, d1.DisposeCallCount, "First throwing disposable should be disposed exactly once"); + Assert.AreEqual(1, d2.DisposeCallCount, "Second throwing disposable should be disposed exactly once"); + Assert.AreEqual(1, d3.DisposeCallCount, "Non-throwing disposable should be disposed exactly once"); + } + + [Test] + public void ControllerCompositeDisposable_AddRangeAfterDisposed_MultipleDisposablesThrow_ThrowsAggregateException() + { + // Arrange + var composite = new ControllerCompositeDisposable(); + var d1 = new TestDisposable("d1") { ThrowOnDispose = true }; + var d2 = new TestDisposable("d2") { ThrowOnDispose = true }; + var d3 = new TestDisposable("d3"); // does not throw + var collection = new[] { d1, d2, d3 }; + + composite.Dispose(); + + // Act + var aggregate = Assert.Throws( + () => composite.AddRange(collection), + "Expected AggregateException when multiple disposables added after dispose throw"); + + // Assert + Assert.IsNotNull(aggregate, "AggregateException should not be null"); + Assert.AreEqual(2, aggregate.InnerExceptions.Count, "AggregateException should contain all throwing disposables"); + foreach (var inner in aggregate.InnerExceptions) + { + Assert.IsInstanceOf(inner, "Inner exception should be TestControllersException"); + } + + Assert.AreEqual(1, d1.DisposeCallCount, "First throwing disposable should be disposed exactly once"); + Assert.AreEqual(1, d2.DisposeCallCount, "Second throwing disposable should be disposed exactly once"); + Assert.AreEqual(1, d3.DisposeCallCount, "Non-throwing disposable should be disposed exactly once"); + } + + private sealed class TestDisposable : IDisposable + { + public int DisposeCallCount { get; private set; } + public bool ThrowOnDispose { get; set; } + public string Id { get; } + + public TestDisposable(string id = null) + { + Id = id; + } + + public void Dispose() + { + DisposeCallCount++; + + if (ThrowOnDispose) + { + throw new TestControllersException($"Dispose:{Id ?? "unknown"}"); + } + } + } + } +} diff --git a/src/ControllersTree/Tests/ControllerCompositeDisposableTests.cs.meta b/src/ControllersTree/Tests/ControllerCompositeDisposableTests.cs.meta new file mode 100644 index 0000000..6a8e539 --- /dev/null +++ b/src/ControllersTree/Tests/ControllerCompositeDisposableTests.cs.meta @@ -0,0 +1,3 @@ +fileFormatVersion: 2 +guid: 11651f4ad5f64d2492004bcb1627bb16 +timeCreated: 1763135352 \ No newline at end of file diff --git a/src/ControllersTree/Tests/ControllersWithResultBaseTests.cs b/src/ControllersTree/Tests/ControllersWithResultBaseTests.cs index 913151b..30f7097 100644 --- a/src/ControllersTree/Tests/ControllersWithResultBaseTests.cs +++ b/src/ControllersTree/Tests/ControllersWithResultBaseTests.cs @@ -215,7 +215,7 @@ public async Task ControllerWithResultBase_Exception_Dispose() .LaunchAsync( args, _controllerFactory, CancellationToken); } - catch (AggregateException) + catch (TestControllersException) { exceptionThrown = true; }