From e5b225f17599e6285b28366cc7eda8b1cee7e787 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Tue, 8 Aug 2023 13:05:54 +0000 Subject: Always use stricter validation for complex fields with struct field attributes. --- .../com/yahoo/schema/derived/AttributeFields.java | 4 ++-- .../com/yahoo/schema/derived/ImportedFields.java | 4 ++-- .../document/ComplexAttributeFieldUtils.java | 25 +++++++++------------- .../schema/processing/ImportedFieldsResolver.java | 4 ++-- ...exFieldsWithStructFieldAttributesValidator.java | 18 ++++++---------- .../ComplexAttributeFieldUtilsTestCase.java | 4 ++-- .../validation/ComplexFieldsValidatorTestCase.java | 12 ++++++----- 7 files changed, 32 insertions(+), 39 deletions(-) (limited to 'config-model') 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) { -- cgit v1.2.3