BREAKING: Remove covariance and LINQ use from Grouping, apache#1059 (apache#1066)

* Break GroupingSearch into subclasses, apache#1059

This still needs XML doc comments, but this breaks the GroupingSearch
"god class" into three subclasses that implement a common abstract
class. This should allow us to not need covariant interfaces for the
return types.

In order to randomly switch between these classes with incompatible
generic type arguments, the test shows how you can use a delegate to
work around this limitation.

* Fix failing tests

* Fix more failing unit tests; refactor out common AbstractFieldOrFunctionGroupingSearch<T> class

* Remove IAbstractAllGroupsCollector<T> interface

* Make AbstractAllGroupsCollector.Groups be ICollection, remove some LINQ use

* Remove ITopGroups and IAbstractSecondPassGroupingCollector interfaces

* Remove IGroupDocs interface

* Remove IAbstractFirstPassGroupingCollector and ISearchGroup interfaces, one failing test

* Remove IAbstractDistinctValuesCollector interface

* Remove IGroupCount interface

* Remove IGroupCountCollector interface

* Remove IEnumerable/LINQ use in Grouping

* Remove some O(n) extra list creation

* Remove some more IEnumerable spots

* Refactor to remove wrapping types and add back non-generic interfaces for collectors

* Return null if TopGroups returns null

* Add some XML doc comments

* Add back removed XML comments during refactoring

* Add XML doc comments to GroupingSearch

* Add IAbstractGroupingSearch interface for symmetry with the other types

* Add .idea/ folder to .gitignore

* Remove unnecessary cast

* Remove unnecessary non-generic overload of GetTopGroups

* Use CastingEnumeratorAdapter in CastingListAdapter; optimize as readonly struct to reduce allocations

* Remove UTF-8 BOMs

* Fix fluent API break due to base class; update docs and add tests

* Make ByDocBlock/DocBlockGroupingSearch non-generic to match Java's wildcard generic erasure for null

* Exclude Lucene3x codec from random grouping tests

* Remove setter for ISearchGroup.GroupValue

* Cast each element of CastingSetAdapter instead of casting the whole enumerable

* Remove BOM

* Rename abstract classes and refactor into a template method pattern

Author: paulirwin

.gitignore

@@ -36,6 +36,7 @@ packages/
 .vs/
 *.lock.json
 TestResults/
+.idea/
 test-files/analysis/data/
 [Nn]u[Gg]et[Pp]ackages/
 out.dot

src/Lucene.Net.Grouping/AbstractAllGroupsCollector.cs

@@ -1,6 +1,6 @@
 using Lucene.Net.Index;
+using Lucene.Net.Support;
 using System.Collections.Generic;
-using System.Linq;
 
 namespace Lucene.Net.Search.Grouping
 {
@@ -34,14 +34,14 @@ namespace Lucene.Net.Search.Grouping
     /// @lucene.experimental
     /// </summary>
     /// <typeparam name="TGroupValue"></typeparam>
-    public abstract class AbstractAllGroupsCollector<TGroupValue> : IAbstractAllGroupsCollector<TGroupValue>
+    public abstract class AbstractAllGroupsCollector<TGroupValue> : IAbstractAllGroupsCollector
     {
         /// <summary>
         /// Returns the total number of groups for the executed search.
         /// This is a convenience method. The following code snippet has the same effect: <code>GetGroups().Count</code>
         /// </summary>
         /// <returns>The total number of groups for the executed search</returns>
-        public virtual int GroupCount => Groups.Count();
+        public virtual int GroupCount => Groups.Count;
 
         /// <summary>
         /// Returns the group values
@@ -51,7 +51,7 @@ public abstract class AbstractAllGroupsCollector<TGroupValue> : IAbstractAllGrou
         /// </para>
         /// </summary>
         /// <returns>the group values</returns>
-        public abstract IEnumerable<TGroupValue> Groups { get; }
+        public abstract ICollection<TGroupValue> Groups { get; }
 
 
         // Empty not necessary
@@ -88,29 +88,34 @@ public virtual void SetScorer(Scorer scorer)
         public abstract void SetNextReader(AtomicReaderContext context);
 
         public virtual bool AcceptsDocsOutOfOrder => true;
+
+        #region Explicit interface implementations
+
+        /// <summary>
+        /// LUCENENET specific method to provide an object-based implementation of <see cref="Groups"/>.
+        /// </summary>
+        ICollection<object> IAbstractAllGroupsCollector.Groups => new CastingCollectionAdapter<TGroupValue, object>(Groups);
+
+        #endregion
     }
 
     /// <summary>
-    /// LUCENENET specific interface used to apply covariance to TGroupValue
+    /// LUCENENET specific interface to provide a non-generic abstraction
+    /// for <see cref="AbstractAllGroupsCollector{TGroupValue}"/>.
     /// </summary>
-    /// <typeparam name="TGroupValue"></typeparam>
-    public interface IAbstractAllGroupsCollector<out TGroupValue> : ICollector
+    public interface IAbstractAllGroupsCollector : ICollector
     {
         /// <summary>
         /// Returns the total number of groups for the executed search.
-        /// This is a convenience method. The following code snippet has the same effect: <code>GetGroups().Count</code>
         /// </summary>
-        /// <returns>The total number of groups for the executed search</returns>
         int GroupCount { get; }
 
         /// <summary>
         /// Returns the group values
-        /// <para>
+        /// <para />
         /// This is an unordered collections of group values. For each group that matched the query there is a <see cref="Util.BytesRef"/>
         /// representing a group value.
-        /// </para>
         /// </summary>
-        /// <returns>the group values</returns>
-        IEnumerable<TGroupValue> Groups { get; }
+        ICollection<object> Groups { get; }
     }
 }

src/Lucene.Net.Grouping/AbstractDistinctValuesCollector.cs

@@ -1,4 +1,5 @@
 using Lucene.Net.Index;
+using Lucene.Net.Support;
 using System.Collections.Generic;
 using JCG = J2N.Collections.Generic;
 
@@ -26,15 +27,37 @@ namespace Lucene.Net.Search.Grouping
     ///
     /// @lucene.experimental
     /// </summary>
-    /// <typeparam name="GC"></typeparam>
-    public abstract class AbstractDistinctValuesCollector<GC> : IAbstractDistinctValuesCollector<GC>
-        where GC : AbstractDistinctValuesCollector.IGroupCount<object>
+    /// <typeparam name="GC">The type of group counts</typeparam>
+    /// <typeparam name="TGroupValue">The type of the group values</typeparam>
+    /// <remarks>
+    /// The <typeparamref name="TGroupValue"/> type parameter is LUCENENET specific to allow for
+    /// strongly-typed group values.
+    /// <para/>
+    /// <b>Migration from 4.8.0-beta00017 and earlier:</b> This class previously had a single type
+    /// parameter <c>GC</c> with a constraint of
+    /// <c>AbstractDistinctValuesCollector.IGroupCount&lt;object&gt;</c>. It now has two type
+    /// parameters: <typeparamref name="GC"/> and <typeparamref name="TGroupValue"/>. When migrating,
+    /// add the group value type (typically <c>BytesRef</c> or <c>MutableValue</c>) as the second
+    /// type argument. For example:
+    /// <code>
+    /// // Before:
+    /// AbstractDistinctValuesCollector&lt;MyGroupCount&gt;
+    ///
+    /// // After:
+    /// AbstractDistinctValuesCollector&lt;MyGroupCount, BytesRef&gt;
+    /// </code>
+    /// Additionally, the <c>GC</c> constraint changed from
+    /// <c>AbstractDistinctValuesCollector.IGroupCount&lt;object&gt;</c> to
+    /// <c>AbstractDistinctValuesCollector.GroupCount&lt;TGroupValue&gt;</c>.
+    /// </remarks>
+    public abstract class AbstractDistinctValuesCollector<GC, TGroupValue> : IAbstractDistinctValuesCollector
+        where GC : AbstractDistinctValuesCollector.GroupCount<TGroupValue>
     {
         /// <summary>
         /// Returns all unique values for each top N group.
         /// </summary>
         /// <returns>all unique values for each top N group</returns>
-        public abstract IEnumerable<GC> Groups { get; }
+        public abstract IList<GC> Groups { get; }
 
         public virtual bool AcceptsDocsOutOfOrder => true;
 
@@ -69,6 +92,16 @@ public virtual void SetScorer(Scorer scorer)
         /// </summary>
         /// <param name="context">next atomic reader context </param>
         public abstract void SetNextReader(AtomicReaderContext context);
+
+        #region Explicit interface implementations
+
+        /// <summary>
+        /// LUCENENET specific implementation to provide a non-generic abstraction
+        /// </summary>
+        IList<AbstractDistinctValuesCollector.IGroupCount> IAbstractDistinctValuesCollector.Groups
+            => new CastingListAdapter<GC, AbstractDistinctValuesCollector.IGroupCount>(Groups);
+
+        #endregion
     }
 
     /// <summary>
@@ -80,48 +113,61 @@ public virtual void SetScorer(Scorer scorer)
     public static class AbstractDistinctValuesCollector // LUCENENET specific: CA1052 Static holder types should be Static or NotInheritable
     {
         /// <summary>
-        /// Returned by <see cref="AbstractDistinctValuesCollector{GC}.Groups"/>,
+        /// Returned by <see cref="AbstractDistinctValuesCollector{GC, TGroupValue}.Groups"/>,
         /// representing the value and set of distinct values for the group.
         /// </summary>
         /// <typeparam name="TGroupValue"></typeparam>
         /// <remarks>
         /// LUCENENET - removed this class from being a nested class of
-        /// <see cref="AbstractDistinctValuesCollector{GC}"/> and renamed
-        /// from GroupCount to AbstractGroupCount
+        /// <see cref="AbstractDistinctValuesCollector{GC, TGroupValue}"/>
         /// </remarks>
-        public abstract class GroupCount<TGroupValue> : IGroupCount<TGroupValue>
+        public abstract class GroupCount<TGroupValue> : IGroupCount
         {
             public TGroupValue GroupValue { get; protected set; }
-            public IEnumerable<TGroupValue> UniqueValues { get; protected set; }
+            public ISet<TGroupValue> UniqueValues { get; protected set; }
 
             protected GroupCount(TGroupValue groupValue) // LUCENENET: CA1012: Abstract types should not have constructors (marked protected)
             {
                 this.GroupValue = groupValue;
                 this.UniqueValues = new JCG.HashSet<TGroupValue>();
             }
+
+            #region Explicit interface implementations
+
+            /// <summary>
+            /// LUCENENET specific method to provide an object-based implementation of <see cref="GroupValue"/>.
+            /// </summary>
+            object IGroupCount.GroupValue => GroupValue;
+
+            /// <summary>
+            /// LUCENENET specific method to provide an object-based implementation of <see cref="UniqueValues"/>.
+            /// </summary>
+            ISet<object> IGroupCount.UniqueValues => new CastingSetAdapter<TGroupValue, object>(UniqueValues);
+
+            #endregion
         }
 
         /// <summary>
-        /// LUCENENET specific interface used to apply covariance to TGroupValue
+        /// LUCENENET specific interface to provide a non-generic abstraction
+        /// for <see cref="GroupCount{TGroupValue}"/>.
         /// </summary>
-        /// <typeparam name="TGroupValue"></typeparam>
-        public interface IGroupCount<out TGroupValue>
+        public interface IGroupCount
         {
-            TGroupValue GroupValue { get; }
-            IEnumerable<TGroupValue> UniqueValues { get; }
+            object GroupValue { get; }
+
+            ISet<object> UniqueValues { get; }
         }
     }
 
     /// <summary>
-    /// LUCENENET specific interface used to apply covariance to GC
+    /// LUCENENET specific interface to provide a non-generic abstraction
+    /// for <see cref="AbstractDistinctValuesCollector{GC, TGroupValue}"/>.
     /// </summary>
-    /// <typeparam name="GC"></typeparam>
-    public interface IAbstractDistinctValuesCollector<out GC> : ICollector
+    public interface IAbstractDistinctValuesCollector : ICollector
     {
         /// <summary>
         /// Returns all unique values for each top N group.
         /// </summary>
-        /// <returns>all unique values for each top N group</returns>
-        IEnumerable<GC> Groups { get; }
+        IList<AbstractDistinctValuesCollector.IGroupCount> Groups { get; }
     }
 }

src/Lucene.Net.Grouping/AbstractFirstPassGroupingCollector.cs

@@ -1,9 +1,9 @@
 using Lucene.Net.Diagnostics;
 using Lucene.Net.Index;
+using Lucene.Net.Support;
 using Lucene.Net.Support.Threading;
 using System;
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using JCG = J2N.Collections.Generic;
@@ -40,7 +40,7 @@ namespace Lucene.Net.Search.Grouping
     /// @lucene.experimental
     /// </summary>
     /// <typeparam name="TGroupValue"></typeparam>
-    public abstract class AbstractFirstPassGroupingCollector<TGroupValue> : IAbstractFirstPassGroupingCollector<TGroupValue>
+    public abstract class AbstractFirstPassGroupingCollector<TGroupValue> : IAbstractFirstPassGroupingCollector
     {
         private readonly Sort groupSort;
         private readonly FieldComparer[] comparers;
@@ -106,7 +106,7 @@ protected AbstractFirstPassGroupingCollector(Sort groupSort, int topNGroups) //
         /// <param name="groupOffset">The offset in the collected groups</param>
         /// <param name="fillFields">Whether to fill to <see cref="SearchGroup{TGroupValue}.SortValues"/></param>
         /// <returns>top groups, starting from offset</returns>
-        public virtual IEnumerable<ISearchGroup<TGroupValue>> GetTopGroups(int groupOffset, bool fillFields)
+        public virtual ICollection<SearchGroup<TGroupValue>> GetTopGroups(int groupOffset, bool fillFields)
         {
 
             //System.out.println("FP.getTopGroups groupOffset=" + groupOffset + " fillFields=" + fillFields + " groupMap.size()=" + groupMap.size());
@@ -126,7 +126,7 @@ public virtual IEnumerable<ISearchGroup<TGroupValue>> GetTopGroups(int groupOffs
                 BuildSortedSet();
             }
 
-            ICollection<ISearchGroup<TGroupValue>> result = new JCG.List<ISearchGroup<TGroupValue>>();
+            ICollection<SearchGroup<TGroupValue>> result = new JCG.List<SearchGroup<TGroupValue>>();
             int upto = 0;
             int sortFieldCount = groupSort.GetSort().Length;
             foreach (CollectedSearchGroup<TGroupValue> group in m_orderedGroups)
@@ -421,27 +421,39 @@ public virtual void SetNextReader(AtomicReaderContext context)
         /// <returns>a copy of the specified group value</returns>
         protected abstract TGroupValue CopyDocGroupValue(TGroupValue groupValue, TGroupValue reuse);
 
+        #region Explicit interface implementations
+
+        /// <summary>
+        /// LUCENENET specific method to provide an <see cref="ISearchGroup"/>-based implementation of <see cref="GetTopGroups(int, bool)"/>.
+        /// </summary>
+        /// <param name="groupOffset">The offset in the collected groups</param>
+        /// <param name="fillFields">Whether to fill to <see cref="ISearchGroup.SortValues"/></param>
+        /// <returns>top groups, starting from offset</returns>
+        ICollection<ISearchGroup> IAbstractFirstPassGroupingCollector.GetTopGroups(int groupOffset, bool fillFields)
+        {
+            var topGroups = GetTopGroups(groupOffset, fillFields);
+            return topGroups != null
+                ? new CastingCollectionAdapter<SearchGroup<TGroupValue>, ISearchGroup>(topGroups)
+                : null;
+        }
+
+        #endregion
     }
 
     /// <summary>
-    /// LUCENENET specific interface used to apply covariance to TGroupValue
-    /// to simulate Java's wildcard generics.
+    /// LUCENENET specific interface to provide a non-generic abstraction
+    /// for <see cref="AbstractFirstPassGroupingCollector{TGroupValue}"/>.
     /// </summary>
-    /// <typeparam name="TGroupValue"></typeparam>
-    public interface IAbstractFirstPassGroupingCollector<out TGroupValue> : ICollector
+    public interface IAbstractFirstPassGroupingCollector : ICollector
     {
         /// <summary>
         /// Returns top groups, starting from offset.  This may
         /// return null, if no groups were collected, or if the
         /// number of unique groups collected is &lt;= offset.
         /// </summary>
         /// <param name="groupOffset">The offset in the collected groups</param>
-        /// <param name="fillFields">Whether to fill to <see cref="SearchGroup{TGroupValue}.SortValues"/></param>
+        /// <param name="fillFields">Whether to fill to <see cref="ISearchGroup.SortValues"/></param>
         /// <returns>top groups, starting from offset</returns>
-        /// <remarks>
-        /// LUCENENET NOTE: We must use <see cref="IEnumerable{TGroupValue}"/> rather than
-        /// <see cref="ICollection{TGroupValue}"/> here because we need this to be covariant
-        /// </remarks>
-        IEnumerable<ISearchGroup<TGroupValue>> GetTopGroups(int groupOffset, bool fillFields);
+        ICollection<ISearchGroup> GetTopGroups(int groupOffset, bool fillFields);
     }
 }