Always use stricter validation for complex fields with struct field attributes.

7 files changed, 32 insertions, 39 deletions

diff --git a/config-model/src/main/java/com/yahoo/schema/derived/AttributeFields.java b/config-model/src/main/java/com/yahoo/schema/derived/AttributeFields.java
index 12ca67bf2c9..c3531d03d3f 100644
--- a/config-model/src/main/java/com/yahoo/schema/derived/AttributeFields.java
+++ b/config-model/src/main/java/com/yahoo/schema/derived/AttributeFields.java
@@ -51,9 +51,9 @@ public class AttributeFields extends Derived implements AttributesConfig.Produce
         if (unsupportedFieldType(field)) {
             return; // Ignore complex struct and map fields for indexed search (only supported for streaming search)
         }
-        if (isArrayOfSimpleStruct(field, false)) {
+        if (isArrayOfSimpleStruct(field)) {
             deriveArrayOfSimpleStruct(field);
-        } else if (isMapOfSimpleStruct(field, false)) {
+        } else if (isMapOfSimpleStruct(field)) {
             deriveMapOfSimpleStruct(field);
         } else if (isMapOfPrimitiveType(field)) {
             deriveMapOfPrimitiveType(field);
diff --git a/config-model/src/main/java/com/yahoo/schema/derived/ImportedFields.java b/config-model/src/main/java/com/yahoo/schema/derived/ImportedFields.java
index 122048d02b9..fa3f49f06d5 100644
--- a/config-model/src/main/java/com/yahoo/schema/derived/ImportedFields.java
+++ b/config-model/src/main/java/com/yahoo/schema/derived/ImportedFields.java
@@ -61,9 +61,9 @@ public class ImportedFields extends Derived implements ImportedFieldsConfig.Prod
         ImmutableSDField targetField = field.targetField();
         if (GeoPos.isAnyPos(targetField)) {
             // no action needed
-        } else if (isArrayOfSimpleStruct(targetField, false)) {
+        } else if (isArrayOfSimpleStruct(targetField)) {
             considerNestedFields(builder, field);
-        } else if (isMapOfSimpleStruct(targetField, false)) {
+        } else if (isMapOfSimpleStruct(targetField)) {
             considerSimpleField(builder, field.getNestedField("key"));
             considerNestedFields(builder, field.getNestedField("value"));
         } else if (isMapOfPrimitiveType(targetField)) {
diff --git a/config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java b/config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java
index 5e4ee6d4b27..ebafd8f1d24 100644
--- a/config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java
+++ b/config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java
@@ -22,31 +22,26 @@ import com.yahoo.document.StructDataType;
 public class ComplexAttributeFieldUtils {
 
     public static boolean isSupportedComplexField(ImmutableSDField field) {
-        return isSupportedComplexField(field, false);
-    }
-
-    // TODO: Remove the stricterValidation flag when this is changed to being always on.
-    public static boolean isSupportedComplexField(ImmutableSDField field, boolean stricterValidation) {
-        return (isArrayOfSimpleStruct(field, stricterValidation) ||
-                isMapOfSimpleStruct(field, stricterValidation) ||
+        return (isArrayOfSimpleStruct(field) ||
+                isMapOfSimpleStruct(field) ||
                 isMapOfPrimitiveType(field));
     }
 
-    public static boolean isArrayOfSimpleStruct(ImmutableSDField field, boolean stricterValidation) {
+    public static boolean isArrayOfSimpleStruct(ImmutableSDField field) {
         if (field.getDataType() instanceof ArrayDataType) {
             ArrayDataType arrayType = (ArrayDataType)field.getDataType();
-            return isStructWithPrimitiveStructFieldAttributes(arrayType.getNestedType(), field, stricterValidation);
+            return isStructWithPrimitiveStructFieldAttributes(arrayType.getNestedType(), field);
         } else {
             return false;
         }
     }
 
-    public static boolean isMapOfSimpleStruct(ImmutableSDField field, boolean stricterValidation) {
+    public static boolean isMapOfSimpleStruct(ImmutableSDField field) {
         if (field.getDataType() instanceof MapDataType) {
             MapDataType mapType = (MapDataType)field.getDataType();
             return isPrimitiveType(mapType.getKeyType()) &&
                     isStructWithPrimitiveStructFieldAttributes(mapType.getValueType(),
-                            field.getStructField("value"), stricterValidation);
+                            field.getStructField("value"));
         } else {
             return false;
         }
@@ -62,7 +57,7 @@ public class ComplexAttributeFieldUtils {
         }
     }
 
-    private static boolean isStructWithPrimitiveStructFieldAttributes(DataType type, ImmutableSDField field, boolean stricterValidation) {
+    private static boolean isStructWithPrimitiveStructFieldAttributes(DataType type, ImmutableSDField field) {
         if (type instanceof StructDataType && ! GeoPos.isPos(type)) {
             for (ImmutableSDField structField : field.getStructFields()) {
                 Attribute attribute = structField.getAttributes().get(structField.getName());
@@ -75,7 +70,7 @@ public class ComplexAttributeFieldUtils {
                         return false;
                     }
                 }
-                if (stricterValidation && !structField.isImportedField() && hasStructFieldAttributes(structField)) {
+                if (!structField.isImportedField() && hasStructFieldAttributes(structField)) {
                     return false;
                 }
             }
@@ -113,9 +108,9 @@ public class ComplexAttributeFieldUtils {
     }
 
     public static boolean isComplexFieldWithOnlyStructFieldAttributes(ImmutableSDField field) {
-        if (isArrayOfSimpleStruct(field, false)) {
+        if (isArrayOfSimpleStruct(field)) {
             return hasOnlyStructFieldAttributes(field);
-        } else if (isMapOfSimpleStruct(field, false)) {
+        } else if (isMapOfSimpleStruct(field)) {
             return (field.getStructField("key").hasSingleAttribute()) &&
                     hasOnlyStructFieldAttributes(field.getStructField("value"));
         } else if (isMapOfPrimitiveType(field)) {
diff --git a/config-model/src/main/java/com/yahoo/schema/processing/ImportedFieldsResolver.java b/config-model/src/main/java/com/yahoo/schema/processing/ImportedFieldsResolver.java
index 8e44bd026a3..ee465be44f2 100644
--- a/config-model/src/main/java/com/yahoo/schema/processing/ImportedFieldsResolver.java
+++ b/config-model/src/main/java/com/yahoo/schema/processing/ImportedFieldsResolver.java
@@ -52,9 +52,9 @@ public class ImportedFieldsResolver extends Processor {
         ImmutableSDField targetField = getTargetField(importedField, reference);
         if (GeoPos.isAnyPos(targetField)) {
             resolveImportedPositionField(importedField, reference, targetField, validate);
-        } else if (isArrayOfSimpleStruct(targetField, false)) {
+        } else if (isArrayOfSimpleStruct(targetField)) {
             resolveImportedArrayOfStructField(importedField, reference, targetField, validate);
-        } else if (isMapOfSimpleStruct(targetField, false)) {
+        } else if (isMapOfSimpleStruct(targetField)) {
             resolveImportedMapOfStructField(importedField, reference, targetField, validate);
         } else if (isMapOfPrimitiveType(targetField)) {
             resolveImportedMapOfPrimitiveField(importedField, reference, targetField, validate);
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java
index d2999a24775..1d67b0c023f 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java
@@ -39,19 +39,15 @@ public class ComplexFieldsWithStructFieldAttributesValidator extends Validator {
     }
 
     private static void validateComplexFields(String clusterName, Schema schema, DeployLogger logger) {
-        String unsupportedFields = validateComplexFields(clusterName, schema, false);
+        String unsupportedFields = validateComplexFields(schema);
         if (!unsupportedFields.isEmpty()) {
             throw new IllegalArgumentException(getErrorMessage(clusterName, schema, unsupportedFields));
         }
-        unsupportedFields = validateComplexFields(clusterName, schema, true);
-        if (!unsupportedFields.isEmpty()) {
-            logger.logApplicationPackage(Level.WARNING, getErrorMessage(clusterName, schema, unsupportedFields));
-        }
     }
 
-    private static String validateComplexFields(String clusterName, Schema schema, boolean stricterValidation) {
+    private static String validateComplexFields(Schema schema) {
         return schema.allFields()
-                .filter(field -> isUnsupportedComplexField(field, stricterValidation))
+                .filter(field -> isUnsupportedComplexField(field))
                 .map(ComplexFieldsWithStructFieldAttributesValidator::toString)
                 .collect(Collectors.joining(", "));
     }
@@ -63,14 +59,14 @@ public class ComplexFieldsWithStructFieldAttributesValidator extends Validator {
                              clusterName, schema.getName(), unsupportedFields);
     }
 
-    private static boolean isUnsupportedComplexField(ImmutableSDField field, boolean stricterValidation) {
+    private static boolean isUnsupportedComplexField(ImmutableSDField field) {
         return (field.usesStructOrMap() &&
-                !isSupportedComplexField(field, stricterValidation) &&
+                !isSupportedComplexField(field) &&
                 hasStructFieldAttributes(field.getStructFields()));
     }
 
-    private static boolean isSupportedComplexField(ImmutableSDField field, boolean stricterValidation) {
-        return (ComplexAttributeFieldUtils.isSupportedComplexField(field, stricterValidation) ||
+    private static boolean isSupportedComplexField(ImmutableSDField field) {
+        return (ComplexAttributeFieldUtils.isSupportedComplexField(field) ||
                 GeoPos.isAnyPos(field));
     }
 
diff --git a/config-model/src/test/java/com/yahoo/schema/document/ComplexAttributeFieldUtilsTestCase.java b/config-model/src/test/java/com/yahoo/schema/document/ComplexAttributeFieldUtilsTestCase.java
index 310ede6bae2..7a89f52268f 100644
--- a/config-model/src/test/java/com/yahoo/schema/document/ComplexAttributeFieldUtilsTestCase.java
+++ b/config-model/src/test/java/com/yahoo/schema/document/ComplexAttributeFieldUtilsTestCase.java
@@ -30,11 +30,11 @@ public class ComplexAttributeFieldUtilsTestCase {
         }
 
         boolean isArrayOfSimpleStruct() {
-            return ComplexAttributeFieldUtils.isArrayOfSimpleStruct(field(), false);
+            return ComplexAttributeFieldUtils.isArrayOfSimpleStruct(field());
         }
 
         boolean isMapOfSimpleStruct() {
-            return ComplexAttributeFieldUtils.isMapOfSimpleStruct(field(), false);
+            return ComplexAttributeFieldUtils.isMapOfSimpleStruct(field());
         }
 
         boolean isMapOfPrimitiveType() {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexFieldsValidatorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexFieldsValidatorTestCase.java
index 04abd4e4836..8f8918b5140 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexFieldsValidatorTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexFieldsValidatorTestCase.java
@@ -71,9 +71,9 @@ public class ComplexFieldsValidatorTestCase {
     }
 
     @Test
-    void logs_warning_when_struct_field_inside_nested_struct_array_is_specified_as_attribute() throws IOException, SAXException {
-        var logger = new MyLogger();
-        createModelAndValidate(joinLines(
+    void throws_exception_when_struct_field_inside_nested_struct_array_is_specified_as_attribute() throws IOException, SAXException {
+        Throwable exception = assertThrows(IllegalArgumentException.class, () -> {
+            createModelAndValidate(joinLines(
                     "schema test {",
                     "document test {",
                     "struct item {",
@@ -92,8 +92,10 @@ public class ComplexFieldsValidatorTestCase {
                         "}",
                     "}",
                     "}",
-                    "}"), logger);
-        assertTrue(logger.message.toString().contains(getExpectedMessage("cabinet (cabinet.value.items.name, cabinet.value.items.color)")));
+                    "}"));
+
+        });
+        assertTrue(exception.getMessage().contains(getExpectedMessage("cabinet (cabinet.value.items.name, cabinet.value.items.color)")));
     }
 
     private String getExpectedMessage(String unsupportedFields) {