Backport fixes from LUCENE-5889, LUCENE-7564, and LUCENE-7670

Author: paulirwin

src/Lucene.Net.Suggest/Suggest/Analyzing/AnalyzingInfixSuggester.cs

@@ -92,12 +92,14 @@ public class AnalyzingInfixSuggester : Lookup, IDisposable
         private readonly Directory dir;
         internal readonly int minPrefixChars;
         private readonly bool commitOnBuild;
+        private readonly bool closeIndexWriterOnBuild; // LUCENENET specific - Support for LUCENE-7564.
         // LUCENENET specific - index writer config factory for extending classes
         private readonly IAnalyzingInfixSuggesterIndexWriterConfigFactory indexWriterConfigFactory;
 
         /// <summary>
         /// Used for ongoing NRT additions/updates. </summary>
-        private IndexWriter writer;
+        // LUCENENET specific - changed from private to protected internal for LUCENE-7564 test support.
+        protected internal IndexWriter writer;
 
         /// <summary>
         /// <see cref="IndexSearcher"/> used for lookups. </summary>
@@ -109,6 +111,12 @@ public class AnalyzingInfixSuggester : Lookup, IDisposable
         /// </summary>
         public const int DEFAULT_MIN_PREFIX_CHARS = 4;
 
+        /// <summary>
+        /// Default option to close the <see cref="IndexWriter"/> once the index has been built.
+        /// </summary>
+        // LUCENENET specific - Support for LUCENE-7564.
+        protected const bool DEFAULT_CLOSE_INDEXWRITER_ON_BUILD = true;
+
         /// <summary>
         /// How we sort the postings and search results. </summary>
         private static readonly Sort SORT = new Sort(new SortField("weight", SortFieldType.INT64, true));
@@ -138,8 +146,9 @@ public AnalyzingInfixSuggester(LuceneVersion matchVersion, Directory dir, Analyz
         ///     Prefixes shorter than this are indexed as character
         ///     ngrams (increasing index size but making lookups
         ///     faster). </param>
-        // LUCENENET specific - LUCENE-5889, a 4.11.0 feature. calls new constructor with extra param.
-        // LUCENENET UPGRADE TODO: Remove method at version 4.11.0. Was retained for perfect 4.8 compatibility
+        // LUCENENET specific - backported from LUCENE-5889 (4.11.0), LUCENE-7564 (6.4.0), LUCENE-7670 (6.4.1).
+        // Calls new constructor with default values for commitOnBuild and closeIndexWriterOnBuild.
+        // Retained for backwards compatibility.
         public AnalyzingInfixSuggester(LuceneVersion matchVersion, Directory dir, Analyzer indexAnalyzer,
             Analyzer queryAnalyzer, int minPrefixChars)
             : this(matchVersion, dir, indexAnalyzer, queryAnalyzer, minPrefixChars, commitOnBuild: false)
@@ -165,7 +174,33 @@ public AnalyzingInfixSuggester(LuceneVersion matchVersion, Directory dir, Analyz
         // LUCENENET specific - LUCENE-5889, a 4.11.0 feature. (Code moved from other constructor to here.)
         public AnalyzingInfixSuggester(LuceneVersion matchVersion, Directory dir, Analyzer indexAnalyzer,
             Analyzer queryAnalyzer, int minPrefixChars, bool commitOnBuild)
-            : this(new AnalyzingInfixSuggesterIndexWriterConfigFactory(SORT), matchVersion, dir, indexAnalyzer, queryAnalyzer, minPrefixChars, commitOnBuild)
+            : this(matchVersion, dir, indexAnalyzer, queryAnalyzer, minPrefixChars, commitOnBuild, DEFAULT_CLOSE_INDEXWRITER_ON_BUILD)
+        {
+        }
+
+        /// <summary>
+        /// Create a new instance, loading from a previously built
+        /// <see cref="AnalyzingInfixSuggester"/> directory, if it exists.  This directory must be
+        /// private to the infix suggester (i.e., not an external
+        /// Lucene index).  Note that <see cref="Dispose()"/>
+        /// will also dispose the provided directory.
+        /// </summary>
+        ///  <param name="minPrefixChars"> Minimum number of leading characters
+        ///     before <see cref="PrefixQuery"/> is used (default 4).
+        ///     Prefixes shorter than this are indexed as character
+        ///     ngrams (increasing index size but making lookups
+        ///     faster). </param>
+        ///  <param name="commitOnBuild"> Call commit after the index has finished building. This
+        ///  would persist the suggester index to disk and future instances of this suggester can
+        ///  use this pre-built dictionary. </param>
+        ///  <param name="closeIndexWriterOnBuild"> If <c>true</c>, the <see cref="IndexWriter"/> will be closed
+        ///  after the index has finished building. </param>
+        // LUCENENET specific - closeIndexWriterOnBuild backported from LUCENE-7564.
+        // Note: Java's equivalent constructor also has allTermsRequired and highlight parameters, which
+        // are not present here because those are method-level parameters in this version of Lucene.NET.
+        public AnalyzingInfixSuggester(LuceneVersion matchVersion, Directory dir, Analyzer indexAnalyzer,
+            Analyzer queryAnalyzer, int minPrefixChars, bool commitOnBuild, bool closeIndexWriterOnBuild)
+            : this(new AnalyzingInfixSuggesterIndexWriterConfigFactory(SORT), matchVersion, dir, indexAnalyzer, queryAnalyzer, minPrefixChars, commitOnBuild, closeIndexWriterOnBuild)
         {
         }
 
@@ -186,14 +221,41 @@ public AnalyzingInfixSuggester(LuceneVersion matchVersion, Directory dir, Analyz
         ///  use this pre-built dictionary. </param>
         /// <param name="indexWriterConfigFactory"> Factory for creating the <see cref="IndexWriterConfig"/>. </param>
         // LUCENENET specific - added indexWriterConfigFactory parameter to allow for customizing the index writer config.
+        // Retained for backwards compatibility.
         public AnalyzingInfixSuggester(IAnalyzingInfixSuggesterIndexWriterConfigFactory indexWriterConfigFactory, LuceneVersion matchVersion,
             Directory dir, Analyzer indexAnalyzer, Analyzer queryAnalyzer, int minPrefixChars, bool commitOnBuild)
+            : this(indexWriterConfigFactory, matchVersion, dir, indexAnalyzer, queryAnalyzer, minPrefixChars, commitOnBuild, DEFAULT_CLOSE_INDEXWRITER_ON_BUILD)
+        {
+        }
+
+        /// <summary>
+        /// Create a new instance, loading from a previously built
+        /// <see cref="AnalyzingInfixSuggester"/> directory, if it exists.  This directory must be
+        /// private to the infix suggester (i.e., not an external
+        /// Lucene index).  Note that <see cref="Dispose()"/>
+        /// will also dispose the provided directory.
+        /// </summary>
+        ///  <param name="minPrefixChars"> Minimum number of leading characters
+        ///     before <see cref="PrefixQuery"/> is used (default 4).
+        ///     Prefixes shorter than this are indexed as character
+        ///     ngrams (increasing index size but making lookups
+        ///     faster). </param>
+        ///  <param name="commitOnBuild"> Call commit after the index has finished building. This
+        ///  would persist the suggester index to disk and future instances of this suggester can
+        ///  use this pre-built dictionary. </param>
+        ///  <param name="closeIndexWriterOnBuild"> If <c>true</c>, the <see cref="IndexWriter"/> will be closed
+        ///  after the index has finished building. </param>
+        /// <param name="indexWriterConfigFactory"> Factory for creating the <see cref="IndexWriterConfig"/>. </param>
+        // LUCENENET specific - added indexWriterConfigFactory and closeIndexWriterOnBuild parameters.
+        public AnalyzingInfixSuggester(IAnalyzingInfixSuggesterIndexWriterConfigFactory indexWriterConfigFactory, LuceneVersion matchVersion,
+            Directory dir, Analyzer indexAnalyzer, Analyzer queryAnalyzer, int minPrefixChars, bool commitOnBuild, bool closeIndexWriterOnBuild)
         {
             if (minPrefixChars < 0)
             {
                 throw new ArgumentOutOfRangeException(nameof(minPrefixChars), "minPrefixChars must be >= 0; got: " + minPrefixChars);// LUCENENET specific - changed from IllegalArgumentException to ArgumentOutOfRangeException (.NET convention)
             }
 
+            // LUCENENET specific - moved IndexWriterConfig to AnalyzingInfixSuggesterIndexWriterConfigFactory
             if (indexWriterConfigFactory is null) throw new ArgumentNullException(nameof(indexWriterConfigFactory));
 
             this.m_queryAnalyzer = queryAnalyzer;
@@ -202,14 +264,19 @@ public AnalyzingInfixSuggester(IAnalyzingInfixSuggesterIndexWriterConfigFactory
             this.dir = dir;
             this.minPrefixChars = minPrefixChars;
             this.commitOnBuild = commitOnBuild;
+            this.closeIndexWriterOnBuild = closeIndexWriterOnBuild;
             this.indexWriterConfigFactory = indexWriterConfigFactory;
 
             if (DirectoryReader.IndexExists(dir))
             {
                 // Already built; open it:
-                var config = indexWriterConfigFactory.Get(matchVersion, GetGramAnalyzer(), OpenMode.APPEND);
-                writer = new IndexWriter(dir, config);
-                m_searcherMgr = new SearcherManager(writer, true, null);
+
+                // LUCENENET specific - backported fix from Lucene 6.4.1 to fix #1242. previously was:
+                // var config = indexWriterConfigFactory.Get(matchVersion, GetGramAnalyzer(), OpenMode.APPEND);
+                // writer = new IndexWriter(dir, config);
+                // m_searcherMgr = new SearcherManager(writer, true, null);
+
+                m_searcherMgr = new SearcherManager(dir, null);
             }
         }
 
@@ -268,7 +335,7 @@ public override void Build(IInputEnumerator enumerator)
                 }
 
                 //System.out.println("initial indexing time: " + ((System.nanoTime()-t0)/1000000) + " msec");
-                if (commitOnBuild)                      //LUCENENET specific -Support for LUCENE - 5889.
+                if (commitOnBuild || closeIndexWriterOnBuild) // LUCENENET specific - Support for LUCENE-5889, LUCENE-7564.
                 {
                     Commit();
                 }
@@ -279,24 +346,40 @@ public override void Build(IInputEnumerator enumerator)
             {
                 if (success)
                 {
+                    if (closeIndexWriterOnBuild)  // LUCENENET specific - Support for LUCENE-7564.
+                    {
+                        writer.Dispose();
+                        writer = null;
+                    }
                     IOUtils.Dispose(r);
                 }
                 else
                 {
-                    IOUtils.DisposeWhileHandlingException(writer, r);
-                    writer = null;
+                    if (writer != null)
+                    {
+                        writer.Rollback();
+                        writer = null;
+                    }
+                    IOUtils.DisposeWhileHandlingException(r);
                 }
             }
         }
 
-        // LUCENENET specific -Support for LUCENE-5889.
+        // LUCENENET specific - Support for LUCENE-5889, LUCENE-7564.
         public void Commit()
         {
             if (writer is null)
             {
-                throw IllegalStateException.Create("Cannot commit on an closed writer. Add documents first");
+                if (m_searcherMgr is null || closeIndexWriterOnBuild == false)
+                {
+                    throw IllegalStateException.Create("Cannot commit on an closed writer. Add documents first");
+                }
+                // else no-op: writer was committed and closed after the index was built, so commit is unnecessary
+            }
+            else
+            {
+                writer.Commit();
             }
-            writer.Commit();
         }
 
         private Analyzer GetGramAnalyzer()
@@ -335,7 +418,7 @@ protected override TokenStreamComponents WrapComponents(string fieldName, TokenS
             }
         }
 
-        //LUCENENET specific -Support for LUCENE - 5889.
+        // LUCENENET specific - Support for LUCENE-5889, LUCENE-7564.
         private void EnsureOpen()
         {
             if (writer != null)
@@ -346,13 +429,21 @@ private void EnsureOpen()
             {
                 if (writer is null)
                 {
-                    if (m_searcherMgr != null)
+                    if (DirectoryReader.IndexExists(dir))
+                    {
+                        // Already built; open it:
+                        writer = new IndexWriter(dir, indexWriterConfigFactory.Get(matchVersion, GetGramAnalyzer(), OpenMode.APPEND));
+                    }
+                    else
                     {
-                        m_searcherMgr.Dispose();
-                        m_searcherMgr = null;
+                        writer = new IndexWriter(dir, indexWriterConfigFactory.Get(matchVersion, GetGramAnalyzer(), OpenMode.CREATE));
                     }
-                    writer = new IndexWriter(dir, indexWriterConfigFactory.Get(matchVersion, GetGramAnalyzer(), OpenMode.CREATE));
+                    SearcherManager oldSearcherMgr = m_searcherMgr;
                     m_searcherMgr = new SearcherManager(writer, true, null);
+                    if (oldSearcherMgr != null)
+                    {
+                        oldSearcherMgr.Dispose();
+                    }
                 }
             }
             finally
@@ -370,21 +461,22 @@ private void EnsureOpen()
         /// </summary>
         public virtual void Add(BytesRef text, IEnumerable<BytesRef> contexts, long weight, BytesRef payload)
         {
-            EnsureOpen();    //LUCENENET specific -Support for LUCENE - 5889.
+            EnsureOpen();    // LUCENENET specific - Support for LUCENE-5889.
             writer.AddDocument(BuildDocument(text, contexts, weight, payload));
         }
 
         /// <summary>
         /// Updates a previous suggestion, matching the exact same
         /// text as before.  Use this to change the weight or
-        /// payload of an already added suggstion.  If you know
+        /// payload of an already added suggestion.  If you know
         /// this text is not already present you can use <see cref="Add"/>
         /// instead.  After adding or updating a batch of
         /// new suggestions, you must call <see cref="Refresh()"/> in the
         /// end in order to see the suggestions in <see cref="DoLookup(string, IEnumerable{BytesRef}, int, bool, bool, CancellationToken)"/>
         /// </summary>
         public virtual void Update(BytesRef text, IEnumerable<BytesRef> contexts, long weight, BytesRef payload)
         {
+            EnsureOpen();    // LUCENENET specific - Support for LUCENE-5889.
             writer.UpdateDocument(new Term(EXACT_TEXT_FIELD_NAME, text.Utf8ToString()), BuildDocument(text, contexts, weight, payload));
         }
 
@@ -424,11 +516,16 @@ private Document BuildDocument(BytesRef text, IEnumerable<BytesRef> contexts, lo
         /// </summary>
         public virtual void Refresh()
         {
-            if (m_searcherMgr is null) // LUCENENET specific -Support for LUCENE-5889.
+            if (m_searcherMgr is null) // LUCENENET specific - Support for LUCENE-5889.
             {
                 throw IllegalStateException.Create("suggester was not built");
             }
-            m_searcherMgr.MaybeRefreshBlocking();
+            if (writer != null) // LUCENENET specific - Support for LUCENE-7564.
+            {
+                m_searcherMgr.MaybeRefreshBlocking();
+            }
+            // else no-op: writer was committed and closed after the index was built
+            //             and before searcherMgr was constructed, so refresh is unnecessary
         }
 
         /// <summary>
@@ -851,9 +948,12 @@ protected virtual void Dispose(bool disposing) // LUCENENET specific - implement
                 if (writer != null)
                 {
                     writer.Dispose();
-                    dir.Dispose();
                     writer = null;
                 }
+                if (dir != null) // LUCENENET specific - Support for LUCENE-7564. Close dir even when writer is null.
+                {
+                    dir.Dispose();
+                }
             }
         }
 

src/Lucene.Net.Suggest/Suggest/Analyzing/BlendedInfixSuggester.cs

@@ -105,8 +105,8 @@ public BlendedInfixSuggester(LuceneVersion matchVersion, Directory dir, Analyzer
         /// <param name="blenderType"> Type of blending strategy, see BlenderType for more precisions </param>
         /// <param name="numFactor">   Factor to multiply the number of searched elements before ponderate </param>
         /// <exception cref="IOException"> If there are problems opening the underlying Lucene index. </exception>
-        // LUCENENET specific - LUCENE-5889, a 4.11.0 feature. calls new constructor with extra param.
-        // LUCENENET UPGRADE TODO: Remove method at version 4.11.0. Was retained for perfect 4.8 compatibility
+        // LUCENENET specific - retained for backwards compatibility. Calls new constructor with default
+        // commitOnBuild value. Java does not have this overload without commitOnBuild post-LUCENE-5889.
         public BlendedInfixSuggester(LuceneVersion matchVersion, Directory dir, Analyzer indexAnalyzer, Analyzer queryAnalyzer, int minPrefixChars,
                                      BlenderType blenderType, int numFactor)
             : this(matchVersion, dir, indexAnalyzer, queryAnalyzer, minPrefixChars, blenderType, numFactor, commitOnBuild: false)