Clean Code Post Lucene 10.0.1 change

Signed-off-by: Vikasht34 <[email protected]>
Vikasht34 · Feb 20, 2025 · ef720ce · ef720ce
1 parent 45ecb5b
commit ef720ce
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 60 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Maintenance
 * Update package name to fix compilation issue [#2513](https://github.com/opensearch-project/k-NN/pull/2513)
 ### Refactoring
+* Small Refactor Post Lucene 10.0.1 upgrade [#2541](https://github.com/opensearch-project/k-NN/pull/2541)
 
 ## [Unreleased 2.x](https://github.com/opensearch-project/k-NN/compare/2.19...2.x)
 ### Features

diff --git a/src/main/java/org/opensearch/knn/index/KNNVectorScriptDocValues.java b/src/main/java/org/opensearch/knn/index/KNNVectorScriptDocValues.java
@@ -74,26 +74,33 @@ public float[] get(int i) {
     /**
      * Creates a KNNVectorScriptDocValues object based on the provided parameters.
      *
-     * @param values          The DocIdSetIterator representing the vector values.
+     * @param knnVectorValues          The DocIdSetIterator representing the vector values.
      * @param fieldName       The name of the field.
      * @param vectorDataType  The data type of the vector.
      * @return A KNNVectorScriptDocValues object based on the type of the values.
      * @throws IllegalArgumentException If the type of values is unsupported.
      */
-    public static KNNVectorScriptDocValues create(Object values, String fieldName, VectorDataType vectorDataType) {
-        Objects.requireNonNull(values, "values must not be null");
-
-        if (values instanceof FloatVectorValues) {
-            return new KNNFloatVectorScriptDocValues((FloatVectorValues) values, fieldName, vectorDataType);
-        } else if (values instanceof ByteVectorValues) {
-            return new KNNByteVectorScriptDocValues((ByteVectorValues) values, fieldName, vectorDataType);
-        } else if (values instanceof BinaryDocValues) {
-            return new KNNNativeVectorScriptDocValues((BinaryDocValues) values, fieldName, vectorDataType);
+    public static KNNVectorScriptDocValues create(KnnVectorValues knnVectorValues, String fieldName, VectorDataType vectorDataType) {
+        Objects.requireNonNull(knnVectorValues, "values must not be null");
+        if (knnVectorValues instanceof FloatVectorValues) {
+            return new KNNFloatVectorScriptDocValues((FloatVectorValues) knnVectorValues, fieldName, vectorDataType);
+        } else if (knnVectorValues instanceof ByteVectorValues) {
+            return new KNNByteVectorScriptDocValues((ByteVectorValues) knnVectorValues, fieldName, vectorDataType);
         } else {
-            throw new IllegalArgumentException("Unsupported values type: " + values.getClass());
+            throw new IllegalArgumentException("Unsupported values type: " + knnVectorValues.getClass());
         }
     }
 
+    public static KNNVectorScriptDocValues create(DocIdSetIterator docIdSetIterator, String fieldName, VectorDataType vectorDataType) {
+        Objects.requireNonNull(docIdSetIterator, "values must not be null");
+        if (docIdSetIterator instanceof BinaryDocValues) {
+            return new KNNNativeVectorScriptDocValues((BinaryDocValues) docIdSetIterator, fieldName, vectorDataType);
+        } else {
+            throw new IllegalArgumentException("Unsupported values type: " + docIdSetIterator.getClass());
+        }
+
+    }
+
     private static final class KNNByteVectorScriptDocValues extends KNNVectorScriptDocValues {
         private final ByteVectorValues values;
         private final KnnVectorValues.DocIndexIterator iterator;

diff --git a/src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java b/src/main/java/org/opensearch/knn/index/vectorvalues/KNNVectorValuesIterator.java
@@ -67,32 +67,15 @@ public interface KNNVectorValuesIterator {
     VectorValueExtractorStrategy getVectorExtractorStrategy();
 
     /**
-     * A DocIdsIteratorValues provides a common iteration logic for all Values that implements
-     * {@link DocIdSetIterator} interface. Example: {@link BinaryDocValues}, {@link FloatVectorValues} etc.
+     * Abstract base class for KNN vector iterators, encapsulating common iteration logic.
      */
-    class DocIdsIteratorValues implements KNNVectorValuesIterator {
-        private final DocIdSetIterator docIdSetIterator;
-        private KnnVectorValues knnVectorValues = null; // Added reference to KnnVectorValues
-        @Getter
-        @Setter
-        private int lastOrd = -1;
-        @Getter
-        @Setter
-        private Object lastAccessedVector = null;
-
-        DocIdsIteratorValues(@NonNull final KnnVectorValues knnVectorValues) {
-            this.docIdSetIterator = knnVectorValues.iterator();
-            this.knnVectorValues = knnVectorValues;
-        }
+    abstract class AbstractVectorValuesIterator implements KNNVectorValuesIterator {
+        protected final DocIdSetIterator docIdSetIterator;
 
-        DocIdsIteratorValues(final DocIdSetIterator docIdSetIterator) {
+        AbstractVectorValuesIterator(@NonNull final DocIdSetIterator docIdSetIterator) {
             this.docIdSetIterator = docIdSetIterator;
         }
 
-        public KnnVectorValues getKnnVectorValues() {
-            return knnVectorValues;
-        }
-
         @Override
         public int docId() {
             return docIdSetIterator.docID();
@@ -113,6 +96,41 @@ public DocIdSetIterator getDocIdSetIterator() {
             return docIdSetIterator;
         }
 
+        @Override
+        public long liveDocs() {
+            return docIdSetIterator.cost();
+        }
+    }
+
+    /**
+     * A DocIdsIteratorValues provides a common iteration logic for all Values that implements
+     * {@link DocIdSetIterator} interface. Example: {@link BinaryDocValues}, {@link FloatVectorValues} etc.
+     */
+    class DocIdsIteratorValues extends AbstractVectorValuesIterator {
+
+        private final KnnVectorValues knnVectorValues;
+
+        @Getter
+        @Setter
+        private int lastOrd = -1;
+        @Getter
+        @Setter
+        private Object lastAccessedVector = null;
+
+        DocIdsIteratorValues(@NonNull final KnnVectorValues knnVectorValues) {
+            super(knnVectorValues.iterator());
+            this.knnVectorValues = knnVectorValues;
+        }
+
+        DocIdsIteratorValues(@NonNull final DocIdSetIterator docIdSetIterator) {
+            super(docIdSetIterator);
+            this.knnVectorValues = null;
+        }
+
+        public KnnVectorValues getKnnVectorValues() {
+            return knnVectorValues;
+        }
+
         @Override
         public long liveDocs() {
             if (docIdSetIterator instanceof BinaryDocValues) {
@@ -129,34 +147,19 @@ public long liveDocs() {
         public VectorValueExtractorStrategy getVectorExtractorStrategy() {
             return new VectorValueExtractorStrategy.DISIVectorExtractor();
         }
+
     }
 
     /**
      * A FieldWriterIteratorValues is mainly used when Vectors are stored in {@link KnnFieldVectorsWriter} interface.
      */
-    class FieldWriterIteratorValues<T> implements KNNVectorValuesIterator {
-        private final DocIdSetIterator docIdSetIterator;
+    class FieldWriterIteratorValues<T> extends AbstractVectorValuesIterator {
         private final Map<Integer, T> vectors;
 
         FieldWriterIteratorValues(@NonNull final DocsWithFieldSet docsWithFieldSet, @NonNull final Map<Integer, T> vectors) {
+            super(docsWithFieldSet.iterator());
             assert docsWithFieldSet.iterator().cost() == vectors.size();
             this.vectors = vectors;
-            this.docIdSetIterator = docsWithFieldSet.iterator();
-        }
-
-        @Override
-        public int docId() {
-            return docIdSetIterator.docID();
-        }
-
-        @Override
-        public int advance(int docId) throws IOException {
-            return docIdSetIterator.advance(docId);
-        }
-
-        @Override
-        public int nextDoc() throws IOException {
-            return docIdSetIterator.nextDoc();
         }
 
         /**
@@ -167,16 +170,6 @@ public T vectorsValue() {
             return vectors.get(docId());
         }
 
-        @Override
-        public DocIdSetIterator getDocIdSetIterator() {
-            return docIdSetIterator;
-        }
-
-        @Override
-        public long liveDocs() {
-            return docIdSetIterator.cost();
-        }
-
         @Override
         public VectorValueExtractorStrategy getVectorExtractorStrategy() {
             return new VectorValueExtractorStrategy.FieldWriterIteratorVectorExtractor();