diff options
author | Geir Storli <geirst@yahoo-inc.com> | 2017-03-24 09:44:22 +0100 |
---|---|---|
committer | Geir Storli <geirst@yahoo-inc.com> | 2017-03-24 09:44:22 +0100 |
commit | e3c766f2ecda6152ae7ead3a813b5a1196de66c2 (patch) | |
tree | 6cda019b7830da68b9783248ce46e826e91af289 /config-model | |
parent | e2e94f66e0faf5acb80e4831be6ec784d96b9e42 (diff) |
Disallow importing a field that is also an imported field.
Diffstat (limited to 'config-model')
2 files changed, 76 insertions, 25 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolver.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolver.java index c33c69b878f..4de582c60ee 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolver.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolver.java @@ -55,7 +55,11 @@ public class ImportedFieldsResolver extends Processor { private SDField validateTargetField(TemporaryImportedField importedField, DocumentReference reference) { String targetFieldName = importedField.targetFieldName(); - SDField targetField = reference.targetSearch().getField(targetFieldName); + Search targetSearch = reference.targetSearch(); + if (isImportedField(targetSearch, targetFieldName)) { + fail(importedField, targetFieldAsString(targetFieldName, reference) + ": Is an imported field. Importing such field not supported"); + } + SDField targetField = targetSearch.getField(targetFieldName); if (targetField == null) { fail(importedField, targetFieldAsString(targetFieldName, reference) + ": Not found"); } else if (!targetField.doesAttributing()) { @@ -68,6 +72,10 @@ public class ImportedFieldsResolver extends Processor { return targetField; } + private static boolean isImportedField(Search targetSearch, String targetFieldName) { + return targetSearch.importedFields().isPresent() && targetSearch.importedFields().get().fields().containsKey(targetFieldName); + } + private static String targetFieldAsString(String targetFieldName, DocumentReference reference) { return "Field '" + targetFieldName + "' via reference field '" + reference.referenceField().getName() + "'"; } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java index 5f8b4b2b38f..1cf90fcfcd1 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java @@ -39,7 +39,7 @@ public class ImportedFieldsResolverTestCase { @Test public void valid_imported_fields_are_resolved() { SearchModel model = new SearchModel(); - model.add(new TemporaryImportedField("my_attribute_field", "ref", "attribute_field")).resolve(); + model.addImportedField("my_attribute_field", "ref", "attribute_field").resolve(); assertEquals(1, model.importedFields.fields().size()); ImportedField myField = model.importedFields.fields().get("my_attribute_field"); @@ -55,7 +55,7 @@ public class ImportedFieldsResolverTestCase { exceptionRule.expect(IllegalArgumentException.class); exceptionRule.expectMessage("For search 'child', import field 'my_attribute_field': " + "Reference field 'not_ref' not found"); - new SearchModel().add(new TemporaryImportedField("my_attribute_field", "not_ref", "budget")).resolve(); + new SearchModel().addImportedField("my_attribute_field", "not_ref", "budget").resolve(); } @Test @@ -63,7 +63,7 @@ public class ImportedFieldsResolverTestCase { exceptionRule.expect(IllegalArgumentException.class); exceptionRule.expectMessage("For search 'child', import field 'my_attribute_field': " + "Field 'not_existing' via reference field 'ref': Not found"); - new SearchModel().add(new TemporaryImportedField("my_attribute_field", "ref", "not_existing")).resolve(); + new SearchModel().addImportedField("my_attribute_field", "ref", "not_existing").resolve(); } @Test @@ -71,7 +71,7 @@ public class ImportedFieldsResolverTestCase { exceptionRule.expect(IllegalArgumentException.class); exceptionRule.expectMessage("For search 'child', import field 'my_not_attribute': " + "Field 'not_attribute' via reference field 'ref': Is not an attribute"); - new SearchModel().add(new TemporaryImportedField("my_not_attribute", "ref", "not_attribute")).resolve(); + new SearchModel().addImportedField("my_not_attribute", "ref", "not_attribute").resolve(); } @Test @@ -81,7 +81,7 @@ public class ImportedFieldsResolverTestCase { "For search 'child', import field 'my_attribute_and_index': " + "Field 'attribute_and_index' via reference field 'ref': Index field not supported"); new SearchModel() - .add(new TemporaryImportedField("my_attribute_and_index", "ref", "attribute_and_index")) + .addImportedField("my_attribute_and_index", "ref", "attribute_and_index") .resolve(); } @@ -92,47 +92,90 @@ public class ImportedFieldsResolverTestCase { "For search 'child', import field 'my_tensor_field': " + "Field 'tensor_field' via reference field 'ref': Type 'tensor' not supported"); new SearchModel() - .add(new TemporaryImportedField("my_tensor_field", "ref", "tensor_field")) + .addImportedField("my_tensor_field", "ref", "tensor_field") + .resolve(); + } + + @Test + public void resolver_fails_if_imported_field_is_also_an_imported_field() { + exceptionRule.expect(IllegalArgumentException.class); + exceptionRule.expectMessage( + "For search 'child', import field 'my_ancient_field': " + + "Field 'ancient_field' via reference field 'ref': Is an imported field. Importing such field not supported"); + new SearchModel() + .addImportedField("my_ancient_field", "ref", "ancient_field") .resolve(); } private static class SearchModel { + + private final ApplicationPackage app = MockApplicationPackage.createEmpty(); + public final Search grandParentSearch; public final Search parentSearch; public final Search childSearch; public ImportedFields importedFields; + public SearchModel() { - ApplicationPackage app = MockApplicationPackage.createEmpty(); + grandParentSearch = createSearch("grandparent"); + grandParentSearch.getDocument().addField(createField("ancient_field", DataType.INT, "{ attribute }")); - parentSearch = new Search("parent", app); - parentSearch.addDocument(new SDDocumentType("parent")); + parentSearch = createSearch("parent"); parentSearch.getDocument().addField(createField("attribute_field", DataType.INT, "{ attribute }")); parentSearch.getDocument().addField(createField("attribute_and_index", DataType.INT, "{ attribute | index }")); parentSearch.getDocument().addField(new TemporarySDField("not_attribute", DataType.INT)); parentSearch.getDocument().addField(createField("tensor_field", new TensorDataType(TensorType.fromSpec("tensor(x[])")), "{ attribute }")); + addRefField(parentSearch, grandParentSearch, "ref"); + addImportedField(parentSearch, "ancient_field", "ref", "ancient_field"); - childSearch = new Search("child", app); - childSearch.addDocument(new SDDocumentType("child")); - SDField parentRefField = new TemporarySDField("ref", ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create("parent"))); - childSearch.getDocument().addField(parentRefField); - childSearch.getDocument().setDocumentReferences(new DocumentReferences(ImmutableMap.of(parentRefField.getName(), - new DocumentReference(parentRefField, parentSearch)))); + childSearch = createSearch("child"); + addRefField(childSearch, parentSearch, "ref"); + } + + private Search createSearch(String name) { + Search result = new Search(name, app); + result.addDocument(new SDDocumentType(name)); + return result; } - private TemporarySDField createField(String name, DataType dataType, String indexingScript) { + + private static TemporarySDField createField(String name, DataType dataType, String indexingScript) { TemporarySDField result = new TemporarySDField(name, dataType); result.parseIndexingScript(indexingScript); return result; } - public SearchModel add(TemporaryImportedField importedField) { - childSearch.temporaryImportedFields().get().add(importedField); + + private static SDField createRefField(String parentType, String fieldName) { + return new TemporarySDField(fieldName, ReferenceDataType.createWithInferredId(TemporaryStructuredDataType.create(parentType))); + } + + private static void addRefField(Search child, Search parent, String fieldName) { + SDField refField = createRefField(parent.getName(), fieldName); + child.getDocument().addField(refField); + child.getDocument().setDocumentReferences(new DocumentReferences(ImmutableMap.of(refField.getName(), + new DocumentReference(refField, parent)))); + } + + public SearchModel addImportedField(String fieldName, String referenceFieldName, String targetFieldName) { + return addImportedField(childSearch, fieldName, referenceFieldName, targetFieldName); + } + + private SearchModel addImportedField(Search search, String fieldName, String referenceFieldName, String targetFieldName) { + search.temporaryImportedFields().get().add(new TemporaryImportedField(fieldName, referenceFieldName, targetFieldName)); return this; } + public void resolve() { - assertNotNull(childSearch.temporaryImportedFields().get()); - assertFalse(childSearch.importedFields().isPresent()); - new ImportedFieldsResolver(childSearch, null, null, null).process(); - assertFalse(childSearch.temporaryImportedFields().isPresent()); - assertNotNull(childSearch.importedFields().get()); - importedFields = childSearch.importedFields().get(); + resolve(grandParentSearch); + resolve(parentSearch); + importedFields = resolve(childSearch); + } + + private static ImportedFields resolve(Search search) { + assertNotNull(search.temporaryImportedFields().get()); + assertFalse(search.importedFields().isPresent()); + new ImportedFieldsResolver(search, null, null, null).process(); + assertFalse(search.temporaryImportedFields().isPresent()); + assertNotNull(search.importedFields().get()); + return search.importedFields().get(); } } |