Review comments: inherit hasValues from base class, error messages, j…

…avadocs Signed-off-by: Martin Gaievski <[email protected]>
opensearch-project · Jul 6, 2022 · 0325d0b · 0325d0b
1 parent 65c0944
commit 0325d0b
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 12 deletions.
diff --git a/server/src/main/java/org/opensearch/index/mapper/DenseVectorFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DenseVectorFieldMapper.java
@@ -29,8 +29,10 @@
 
 /**
  * Field Mapper for Dense vector type. Extends ParametrizedFieldMapper in order to easily configure mapping parameters.
+ *
+ * @opensearch.internal
  */
-public class DenseVectorFieldMapper extends ParametrizedFieldMapper {
+public final class DenseVectorFieldMapper extends ParametrizedFieldMapper {
 
     public static final String CONTENT_TYPE = "dense_vector";
 
@@ -55,12 +57,12 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
             int value = XContentMapValues.nodeIntegerValue(o);
             if (value > MAX_DIMENSION) {
                 throw new IllegalArgumentException(
-                    String.format(Locale.ROOT, "[dimension] value cannot be greater than %d for vector [%s]", MAX_DIMENSION, name)
+                    String.format(Locale.ROOT, "[dimension] value %d cannot be greater than %d for vector [%s]", value, MAX_DIMENSION, name)
                 );
             }
             if (value <= 0) {
                 throw new IllegalArgumentException(
-                    String.format(Locale.ROOT, "[dimension] value must be greater than 0 for vector [%s]", name)
+                    String.format(Locale.ROOT, "[dimension] value %d must be greater than 0 for vector [%s]", value, name)
                 );
             }
             return value;
@@ -130,17 +132,15 @@ public static class DenseVectorFieldType extends MappedFieldType {
 
         private final int dimension;
         private final KnnContext knnContext;
-        private final boolean hasDocValues;
 
         public DenseVectorFieldType(String name, int dimension, KnnContext knnContext) {
             this(name, Collections.emptyMap(), dimension, knnContext, false);
         }
 
         public DenseVectorFieldType(String name, Map<String, String> meta, int dimension, KnnContext knnContext, boolean hasDocValues) {
-            super(name, false, false, false, TextSearchInfo.NONE, meta);
+            super(name, false, false, hasDocValues, TextSearchInfo.NONE, meta);
             this.dimension = dimension;
             this.knnContext = knnContext;
-            this.hasDocValues = hasDocValues;
         }
 
         @Override

diff --git a/server/src/main/java/org/opensearch/index/mapper/KnnContext.java b/server/src/main/java/org/opensearch/index/mapper/KnnContext.java
@@ -17,8 +17,10 @@
 
 /**
  * Abstracts KNN segment of dense_vector field type
+ *
+ * @opensearch.internal
  */
-public class KnnContext implements ToXContentFragment, Writeable {
+public final class KnnContext implements ToXContentFragment, Writeable {
 
     private final Metric metric;
     private final KnnAlgorithmContext knnAlgorithmContext;

diff --git a/server/src/main/java/org/opensearch/index/mapper/Metric.java b/server/src/main/java/org/opensearch/index/mapper/Metric.java
@@ -13,8 +13,10 @@
 
 /**
  * Abstracts supported metrics for dense_vector and KNN search
+ *
+ * @opensearch.internal
  */
-public enum Metric {
+enum Metric {
     L2,
     COSINE,
     DOT_PRODUCT;

diff --git a/server/src/test/java/org/opensearch/index/mapper/DenseVectorFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/DenseVectorFieldTypeTests.java
@@ -273,10 +273,7 @@ public void testInvalidVectorDimension() throws Exception {
             IllegalArgumentException.class,
             () -> parser.parse("type", new CompressedXContent(Strings.toString(mappingInvalidDimension)))
         );
-        org.hamcrest.MatcherAssert.assertThat(
-            exceptionInvalidDimension.getMessage(),
-            containsString("[dimension] value cannot be greater than 1024 for vector")
-        );
+        assertEquals(exceptionInvalidDimension.getMessage(), "[dimension] value 1200 cannot be greater than 1024 for vector [field]");
 
         XContentBuilder mappingDimentionsMismatch = XContentFactory.jsonBuilder()
             .startObject()