Refactor IAsyncReplicator support to not require two constructors

Author: paulirwin

src/Lucene.Net.Replicator/Http/HttpReplicator.cs

@@ -32,7 +32,7 @@ namespace Lucene.Net.Replicator.Http
     /// <remarks>
     /// @lucene.experimental
     /// </remarks>
-    public class HttpReplicator : HttpClientBase, IReplicator, IAsyncReplicator
+    public class HttpReplicator : HttpClientBase, IAsyncReplicator
     {
         /// <summary>
         /// Creates a new <see cref="HttpReplicator"/> with the given host, port and path.

src/Lucene.Net.Replicator/IAsyncReplicator.cs

@@ -7,9 +7,11 @@
 namespace Lucene.Net.Replicator
 {
     /// <summary>
-    /// Async version of <see cref="IReplicator"/> for non-blocking replication operations.
+    /// Async extension of <see cref="IReplicator"/> for non-blocking replication operations.
+    /// Implementations provide both synchronous (inherited from <see cref="IReplicator"/>)
+    /// and asynchronous methods.
     /// </summary>
-    public interface IAsyncReplicator
+    public interface IAsyncReplicator : IReplicator
     {
         /// <summary>
         /// Check whether the given version is up-to-date and returns a

src/Lucene.Net.Replicator/ReplicationClient.cs

@@ -129,7 +129,7 @@ public override void Run()
         /// </summary>
         public const string INFO_STREAM_COMPONENT = "ReplicationThread";
 
-        private readonly IReplicator? replicator;
+        private readonly IReplicator replicator;
         private readonly IAsyncReplicator? asyncReplicator;
         private readonly IReplicationHandler handler;
         private readonly ISourceDirectoryFactory factory;
@@ -147,26 +147,16 @@ public override void Run()
         /// <summary>
         /// Constructor.
         /// </summary>
-        /// <param name="replicator">The <see cref="IReplicator"/> used for checking for updates</param>
+        /// <param name="replicator">The <see cref="IReplicator"/> used for checking for updates.
+        /// If the replicator also implements <see cref="IAsyncReplicator"/>, the async
+        /// methods will use the native async implementation; otherwise, they will fall back
+        /// to wrapping the synchronous methods.</param>
         /// <param name="handler">The <see cref="IReplicationHandler"/> notified when new revisions are ready</param>
         /// <param name="factory">The <see cref="ISourceDirectoryFactory"/> for returning a <see cref="Directory"/> for a given source and session</param>
         public ReplicationClient(IReplicator replicator, IReplicationHandler handler, ISourceDirectoryFactory factory)
         {
             this.replicator = replicator ?? throw new ArgumentNullException(nameof(replicator));
-            this.handler = handler ?? throw new ArgumentNullException(nameof(handler));
-            this.factory = factory ?? throw new ArgumentNullException(nameof(factory));
-        }
-
-        /// <summary>
-        /// Constructor for async replicators.
-        /// </summary>
-        /// <param name="asyncReplicator"></param>
-        /// <param name="handler"></param>
-        /// <param name="factory"></param>
-        /// <exception cref="ArgumentNullException"></exception>
-        public ReplicationClient(IAsyncReplicator asyncReplicator, IReplicationHandler handler, ISourceDirectoryFactory factory)
-        {
-            this.asyncReplicator = asyncReplicator ?? throw new ArgumentNullException(nameof(asyncReplicator));
+            this.asyncReplicator = replicator as IAsyncReplicator;
             this.handler = handler ?? throw new ArgumentNullException(nameof(handler));
             this.factory = factory ?? throw new ArgumentNullException(nameof(factory));
         }
@@ -184,8 +174,6 @@ private void CopyBytes(IndexOutput output, Stream input)
         /// <exception cref="IOException"></exception>
         private void DoUpdate()
         {
-            if (replicator is null) throw new InvalidOperationException("Replicator not initialized.");
-
             SessionToken? session = null;
             Dictionary<string, Directory> sourceDirectory = new Dictionary<string, Directory>();
             Dictionary<string, IList<string>> copiedFiles = new Dictionary<string, IList<string>>();
@@ -285,13 +273,13 @@ private void DoUpdate()
         }
 
         /// <summary>
-        /// Performs the async update logic, mirrors DoUpdate but uses IAsyncReplicator.
+        /// Performs the async update logic. When the replicator implements
+        /// <see cref="IAsyncReplicator"/>, native async methods are used;
+        /// otherwise, the synchronous <see cref="IReplicator"/> methods are
+        /// called and their results wrapped.
         /// </summary>
         private async Task DoUpdateAsync(CancellationToken cancellationToken)
         {
-            if (asyncReplicator is null)
-                throw new InvalidOperationException("AsyncReplicator not initialized.");
-
             SessionToken? session = null;
             var sourceDirectory = new Dictionary<string, Directory>();
             var copiedFiles = new Dictionary<string, IList<string>>();
@@ -300,15 +288,17 @@ private async Task DoUpdateAsync(CancellationToken cancellationToken)
             try
             {
                 string? version = handler.CurrentVersion;
-                session = await asyncReplicator.CheckForUpdateAsync(version, cancellationToken).ConfigureAwait(false);
+
+                session = asyncReplicator is not null
+                    ? await asyncReplicator.CheckForUpdateAsync(version, cancellationToken).ConfigureAwait(false)
+                    : replicator.CheckForUpdate(version);
 
                 WriteToInfoStream($"DoUpdateAsync(): handlerVersion={version} session={session}");
 
                 if (session is null)
                     return;
 
                 IDictionary<string, IList<RevisionFile>> requiredFiles = RequiredFiles(session.SourceFiles);
-                WriteToInfoStream($"DoUpdateAsync(): handlerVersion={version} session={session}");
 
                 foreach (var pair in requiredFiles)
                 {
@@ -327,7 +317,10 @@ private async Task DoUpdateAsync(CancellationToken cancellationToken)
                         IndexOutput? output = null;
                         try
                         {
-                            input = await asyncReplicator.ObtainFileAsync(session.Id!, source, file.FileName, cancellationToken).ConfigureAwait(false);
+                            input = asyncReplicator is not null
+                                ? await asyncReplicator.ObtainFileAsync(session.Id!, source, file.FileName, cancellationToken).ConfigureAwait(false)
+                                : replicator.ObtainFile(session.Id!, source, file.FileName);
+
                             output = directory.CreateOutput(file.FileName, IOContext.DEFAULT);
 
                             int numBytes;
@@ -354,7 +347,10 @@ private async Task DoUpdateAsync(CancellationToken cancellationToken)
                 {
                     try
                     {
-                        await asyncReplicator.ReleaseAsync(session.Id, cancellationToken).ConfigureAwait(false);
+                        if (asyncReplicator is not null)
+                            await asyncReplicator.ReleaseAsync(session.Id, cancellationToken).ConfigureAwait(false);
+                        else
+                            replicator.Release(session.Id);
                     }
                     finally
                     {
@@ -367,18 +363,28 @@ private async Task DoUpdateAsync(CancellationToken cancellationToken)
                 }
             }
 
-            if (notify && !disposed)
+            // notify outside the try-finally above, so the session is released sooner.
+            // the handler may take time to finish acting on the copied files, but the
+            // session itself is no longer needed.
+            try
             {
-                handler.RevisionReady(session.Version, session.SourceFiles, Collections.AsReadOnly(copiedFiles), sourceDirectory);
+                if (notify && !disposed)
+                {
+                    // no use to notify if we are closed already
+                    // LUCENENET specific - pass the copiedFiles as read only
+                    handler.RevisionReady(session.Version, session.SourceFiles, Collections.AsReadOnly(copiedFiles), sourceDirectory);
+                }
             }
+            finally
+            {
+                IOUtils.Dispose(sourceDirectory.Values);
 
-            IOUtils.Dispose(sourceDirectory.Values);
-
-            // LUCENENET specific: removed redundant check. Code above either ensures it is not null (by returning early) or an exception is thrown.
-            // if (session != null)
-            // {
-                factory.CleanupSession(session.Id);
-            // }
+                // LUCENENET specific: removed redundant check. Code above either ensures it is not null (by returning early) or an exception is thrown.
+                // if (session != null)
+                // {
+                    factory.CleanupSession(session.Id);
+                // }
+            }
         }
 
         /// <summary>Throws <see cref="ObjectDisposedException"/> if the client has already been disposed.</summary>

src/Lucene.Net.Tests.Replicator/Http/HttpReplicatorTest.cs

@@ -152,7 +152,7 @@ public void TestBasic()
         [LuceneNetSpecific]
         public async Task TestBasicAsync()
         {
-            IAsyncReplicator replicator = new HttpReplicator(host, port, ReplicationService.REPLICATION_CONTEXT + "/s1", server.CreateHandler());
+            IReplicator replicator = new HttpReplicator(host, port, ReplicationService.REPLICATION_CONTEXT + "/s1", server.CreateHandler());
             ReplicationClient client = new ReplicationClient(replicator, new IndexReplicationHandler(handlerIndexDir, null),
                 new PerSessionDirectoryFactory(clientWorkDir.FullName));
 
@@ -208,7 +208,7 @@ public void TestServerErrors()
         public async Task TestServerErrorsAsync()
         {
             // tests the behaviour of the client when the server sends an error
-            IAsyncReplicator replicator = new HttpReplicator(host, port, ReplicationService.REPLICATION_CONTEXT + "/s1", server.CreateHandler());
+            IReplicator replicator = new HttpReplicator(host, port, ReplicationService.REPLICATION_CONTEXT + "/s1", server.CreateHandler());
             using ReplicationClient client = new ReplicationClient(replicator, new IndexReplicationHandler(handlerIndexDir, null),
                 new PerSessionDirectoryFactory(clientWorkDir.FullName));