summaryrefslogtreecommitdiffstats
path: root/config-model
diff options
context:
space:
mode:
authorGeir Storli <geirst@yahooinc.com>2023-07-04 11:41:19 +0000
committerGeir Storli <geirst@yahooinc.com>2023-07-04 11:41:19 +0000
commitc10aa1c8336a46a8106781fddcf1c99c6bf1d02e (patch)
treeecba54814a3ea45cff090b4851176ba8ba694660 /config-model
parent60819f2638b4b3d7bcb27e62d6b56431055c9ab9 (diff)
Only log warning when the stricter validation triggers.
The stricter validation will eventually be made default after all cloud applications are upgraded.
Diffstat (limited to 'config-model')
-rw-r--r--config-model/src/main/java/com/yahoo/schema/derived/AttributeFields.java4
-rw-r--r--config-model/src/main/java/com/yahoo/schema/derived/ImportedFields.java4
-rw-r--r--config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java25
-rw-r--r--config-model/src/main/java/com/yahoo/schema/processing/ImportedFieldsResolver.java4
-rw-r--r--config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java44
-rw-r--r--config-model/src/test/java/com/yahoo/schema/document/ComplexAttributeFieldUtilsTestCase.java4
-rw-r--r--config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexFieldsValidatorTestCase.java15
7 files changed, 57 insertions, 43 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 c3531d03d3f..12ca67bf2c9 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)) {
+ if (isArrayOfSimpleStruct(field, false)) {
deriveArrayOfSimpleStruct(field);
- } else if (isMapOfSimpleStruct(field)) {
+ } else if (isMapOfSimpleStruct(field, false)) {
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 fa3f49f06d5..122048d02b9 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)) {
+ } else if (isArrayOfSimpleStruct(targetField, false)) {
considerNestedFields(builder, field);
- } else if (isMapOfSimpleStruct(targetField)) {
+ } else if (isMapOfSimpleStruct(targetField, false)) {
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 ebafd8f1d24..5e4ee6d4b27 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,26 +22,31 @@ import com.yahoo.document.StructDataType;
public class ComplexAttributeFieldUtils {
public static boolean isSupportedComplexField(ImmutableSDField field) {
- return (isArrayOfSimpleStruct(field) ||
- isMapOfSimpleStruct(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) ||
isMapOfPrimitiveType(field));
}
- public static boolean isArrayOfSimpleStruct(ImmutableSDField field) {
+ public static boolean isArrayOfSimpleStruct(ImmutableSDField field, boolean stricterValidation) {
if (field.getDataType() instanceof ArrayDataType) {
ArrayDataType arrayType = (ArrayDataType)field.getDataType();
- return isStructWithPrimitiveStructFieldAttributes(arrayType.getNestedType(), field);
+ return isStructWithPrimitiveStructFieldAttributes(arrayType.getNestedType(), field, stricterValidation);
} else {
return false;
}
}
- public static boolean isMapOfSimpleStruct(ImmutableSDField field) {
+ public static boolean isMapOfSimpleStruct(ImmutableSDField field, boolean stricterValidation) {
if (field.getDataType() instanceof MapDataType) {
MapDataType mapType = (MapDataType)field.getDataType();
return isPrimitiveType(mapType.getKeyType()) &&
isStructWithPrimitiveStructFieldAttributes(mapType.getValueType(),
- field.getStructField("value"));
+ field.getStructField("value"), stricterValidation);
} else {
return false;
}
@@ -57,7 +62,7 @@ public class ComplexAttributeFieldUtils {
}
}
- private static boolean isStructWithPrimitiveStructFieldAttributes(DataType type, ImmutableSDField field) {
+ private static boolean isStructWithPrimitiveStructFieldAttributes(DataType type, ImmutableSDField field, boolean stricterValidation) {
if (type instanceof StructDataType && ! GeoPos.isPos(type)) {
for (ImmutableSDField structField : field.getStructFields()) {
Attribute attribute = structField.getAttributes().get(structField.getName());
@@ -70,7 +75,7 @@ public class ComplexAttributeFieldUtils {
return false;
}
}
- if (!structField.isImportedField() && hasStructFieldAttributes(structField)) {
+ if (stricterValidation && !structField.isImportedField() && hasStructFieldAttributes(structField)) {
return false;
}
}
@@ -108,9 +113,9 @@ public class ComplexAttributeFieldUtils {
}
public static boolean isComplexFieldWithOnlyStructFieldAttributes(ImmutableSDField field) {
- if (isArrayOfSimpleStruct(field)) {
+ if (isArrayOfSimpleStruct(field, false)) {
return hasOnlyStructFieldAttributes(field);
- } else if (isMapOfSimpleStruct(field)) {
+ } else if (isMapOfSimpleStruct(field, false)) {
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 ee465be44f2..8e44bd026a3 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)) {
+ } else if (isArrayOfSimpleStruct(targetField, false)) {
resolveImportedArrayOfStructField(importedField, reference, targetField, validate);
- } else if (isMapOfSimpleStruct(targetField)) {
+ } else if (isMapOfSimpleStruct(targetField, false)) {
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 a3dc2b52b93..d2999a24775 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
@@ -1,6 +1,7 @@
// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.model.application.validation;
+import com.yahoo.config.application.api.DeployLogger;
import com.yahoo.config.model.deploy.DeployState;
import com.yahoo.schema.Schema;
import com.yahoo.schema.derived.SchemaInfo;
@@ -13,6 +14,7 @@ import com.yahoo.vespa.model.search.SearchCluster;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.logging.Level;
import java.util.stream.Collectors;
/**
@@ -31,34 +33,44 @@ public class ComplexFieldsWithStructFieldAttributesValidator extends Validator {
if (cluster.isStreaming()) continue;
for (SchemaInfo spec : cluster.schemas().values()) {
- validateComplexFields(cluster.getClusterName(), spec.fullSchema());
+ validateComplexFields(cluster.getClusterName(), spec.fullSchema(), deployState.getDeployLogger());
}
}
}
- private static void validateComplexFields(String clusterName, Schema schema) {
- String unsupportedFields = schema.allFields()
- .filter(field -> isUnsupportedComplexField(field))
- .map(ComplexFieldsWithStructFieldAttributesValidator::toString)
- .collect(Collectors.joining(", "));
-
+ private static void validateComplexFields(String clusterName, Schema schema, DeployLogger logger) {
+ String unsupportedFields = validateComplexFields(clusterName, schema, false);
+ if (!unsupportedFields.isEmpty()) {
+ throw new IllegalArgumentException(getErrorMessage(clusterName, schema, unsupportedFields));
+ }
+ unsupportedFields = validateComplexFields(clusterName, schema, true);
if (!unsupportedFields.isEmpty()) {
- throw new IllegalArgumentException(
- String.format("For cluster '%s', search '%s': The following complex fields do not support using struct field attributes: %s. " +
- "Only supported for the following complex field types: array or map of struct with primitive types, map of primitive types. " +
- "The supported primitive types are: byte, int, long, float, double and string",
- clusterName, schema.getName(), unsupportedFields));
+ logger.logApplicationPackage(Level.WARNING, getErrorMessage(clusterName, schema, unsupportedFields));
}
}
- private static boolean isUnsupportedComplexField(ImmutableSDField field) {
+ private static String validateComplexFields(String clusterName, Schema schema, boolean stricterValidation) {
+ return schema.allFields()
+ .filter(field -> isUnsupportedComplexField(field, stricterValidation))
+ .map(ComplexFieldsWithStructFieldAttributesValidator::toString)
+ .collect(Collectors.joining(", "));
+ }
+
+ private static String getErrorMessage(String clusterName, Schema schema, String unsupportedFields) {
+ return String.format("For cluster '%s', search '%s': The following complex fields do not support using struct field attributes: %s. " +
+ "Only supported for the following complex field types: array or map of struct with primitive types, map of primitive types. " +
+ "The supported primitive types are: byte, int, long, float, double and string",
+ clusterName, schema.getName(), unsupportedFields);
+ }
+
+ private static boolean isUnsupportedComplexField(ImmutableSDField field, boolean stricterValidation) {
return (field.usesStructOrMap() &&
- !isSupportedComplexField(field) &&
+ !isSupportedComplexField(field, stricterValidation) &&
hasStructFieldAttributes(field.getStructFields()));
}
- private static boolean isSupportedComplexField(ImmutableSDField field) {
- return (ComplexAttributeFieldUtils.isSupportedComplexField(field) ||
+ private static boolean isSupportedComplexField(ImmutableSDField field, boolean stricterValidation) {
+ return (ComplexAttributeFieldUtils.isSupportedComplexField(field, stricterValidation) ||
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 7a89f52268f..310ede6bae2 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());
+ return ComplexAttributeFieldUtils.isArrayOfSimpleStruct(field(), false);
}
boolean isMapOfSimpleStruct() {
- return ComplexAttributeFieldUtils.isMapOfSimpleStruct(field());
+ return ComplexAttributeFieldUtils.isMapOfSimpleStruct(field(), false);
}
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 f3517f9ef48..04abd4e4836 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
@@ -18,7 +18,6 @@ import java.util.List;
import java.util.logging.Level;
import static com.yahoo.config.model.test.TestUtil.joinLines;
-import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -72,9 +71,9 @@ public class ComplexFieldsValidatorTestCase {
}
@Test
- void throws_exception_when_struct_field_inside_nested_struct_array_is_specified_as_attribute() throws IOException, SAXException {
- Throwable exception = assertThrows(IllegalArgumentException.class, () -> {
- createModelAndValidate(joinLines(
+ void logs_warning_when_struct_field_inside_nested_struct_array_is_specified_as_attribute() throws IOException, SAXException {
+ var logger = new MyLogger();
+ createModelAndValidate(joinLines(
"schema test {",
"document test {",
"struct item {",
@@ -93,12 +92,10 @@ public class ComplexFieldsValidatorTestCase {
"}",
"}",
"}",
- "}"));
- });
- assertTrue(exception.getMessage().contains(getExpectedMessage("cabinet (cabinet.value.items.name, cabinet.value.items.color)")));
+ "}"), logger);
+ assertTrue(logger.message.toString().contains(getExpectedMessage("cabinet (cabinet.value.items.name, cabinet.value.items.color)")));
}
-
private String getExpectedMessage(String unsupportedFields) {
return "For cluster 'mycluster', search 'test': " +
"The following complex fields do not support using struct field attributes: " +
@@ -133,7 +130,7 @@ public class ComplexFieldsValidatorTestCase {
"}",
"}",
"}"), logger);
- assertThat(logger.message.toString().contains(
+ assertTrue(logger.message.toString().contains(
"For cluster 'mycluster', schema 'test': " +
"The following complex fields have struct fields with 'indexing: index' which is not supported and has no effect: " +
"topics (topics.id, topics.label). " +