diff options
author | Arne H Juul <arnej@yahooinc.com> | 2022-03-07 13:24:08 +0000 |
---|---|---|
committer | Arne H Juul <arnej@yahooinc.com> | 2022-03-07 13:34:21 +0000 |
commit | 343ec9de039f2da9f29a93b658e5b69b6bc859e6 (patch) | |
tree | 7228634e5a028d56c5055547bc63e7293250a3e0 /config-model | |
parent | a1099560e37f44a184d102e5dc6a927910777fe0 (diff) |
resolve document references also
* we need to check for cycles with both inheritance and
document references
* also, allow different names schema / document
Diffstat (limited to 'config-model')
5 files changed, 94 insertions, 17 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java index d9132d3aa24..4d011c1b596 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java @@ -15,6 +15,7 @@ public class InheritanceResolver { private final Map<String, ParsedSchema> parsedSchemas; private final Map<String, ParsedDocument> parsedDocs = new HashMap<>(); + private final Map<String, ParsedSchema> schemaForDocs = new HashMap<>(); public InheritanceResolver(Map<String, ParsedSchema> parsedSchemas) { this.parsedSchemas = parsedSchemas; @@ -24,7 +25,7 @@ public class InheritanceResolver { String name = schema.name(); if (seen.contains(name)) { seen.add(name); - throw new IllegalArgumentException("Inheritance cycle for schemas: " + + throw new IllegalArgumentException("Inheritance/reference cycle for schemas: " + String.join(" -> ", seen)); } seen.add(name); @@ -64,9 +65,13 @@ public class InheritanceResolver { if (old != null) { throw new IllegalArgumentException("duplicate document declaration for " + doc.name()); } + schemaForDocs.put(doc.name(), schema); for (String docInherit : doc.getInherited()) { schema.inheritByDocument(docInherit); } + for (String docReferenced : doc.getReferencedDocuments()) { + schema.inheritByDocument(docReferenced); + } } for (ParsedDocument doc : parsedDocs.values()) { for (String inherit : doc.getInherited()) { @@ -76,13 +81,20 @@ public class InheritanceResolver { } doc.resolveInherit(inherit, parentDoc); } + for (String docRefName : doc.getReferencedDocuments()) { + var refDoc = parsedDocs.get(docRefName); + if (refDoc == null) { + throw new IllegalArgumentException("document " + doc.name() + " references unavailable document " + docRefName); + } + doc.resolveReferenced(refDoc); + } } for (ParsedSchema schema : parsedSchemas.values()) { - for (String inherit : schema.getInheritedByDocument()) { - var parent = parsedSchemas.get(inherit); + for (String docName : schema.getInheritedByDocument()) { + var parent = schemaForDocs.get(docName); assert(parent.hasDocument()); - assert(parent.getDocument().name().equals(inherit)); - schema.resolveInheritByDocument(inherit, parent); + assert(parent.getDocument().name().equals(docName)); + schema.resolveInheritByDocument(docName, parent); } } } @@ -91,7 +103,7 @@ public class InheritanceResolver { String name = document.name(); if (seen.contains(name)) { seen.add(name); - throw new IllegalArgumentException("Inheritance cycle for documents: " + + throw new IllegalArgumentException("Inheritance/reference cycle for documents: " + String.join(" -> ", seen)); } seen.add(name); diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java index 065b66e22b1..ed975238067 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchdefinition.parser; - import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; @@ -34,6 +33,19 @@ public class ParsedDocument extends ParsedBlock { ParsedStruct getStruct(String name) { return docStructs.get(name); } ParsedAnnotation getAnnotation(String name) { return docAnnotations.get(name); } + List<String> getReferencedDocuments() { + var result = new ArrayList<String>(); + for (var field : docFields.values()) { + var type = field.getType(); + if (type.getVariant() == ParsedType.Variant.DOC_REFERENCE) { + var docType = type.getReferencedDocumentType(); + assert(docType.getVariant() == ParsedType.Variant.DOCUMENT); + result.add(docType.name()); + } + } + return result; + } + void inherit(String other) { inherited.add(other); } void addField(ParsedField field) { @@ -54,6 +66,7 @@ public class ParsedDocument extends ParsedBlock { verifyThat(! docAnnotations.containsKey(annName), "already has annotation", annName); docAnnotations.put(annName, annotation); annotation.tagOwner(name()); + annotation.getStruct().ifPresent(s -> s.tagOwner(name())); } public String toString() { return "document " + name(); } @@ -64,5 +77,12 @@ public class ParsedDocument extends ParsedBlock { verifyThat(! resolvedInherits.containsKey(name), "double resolveInherit for", name); resolvedInherits.put(name, parsed); } + + void resolveReferenced(ParsedDocument parsed) { + // TODO - not really inheritance: + var old = resolvedInherits.put(parsed.name(), parsed); + assert(old == null || old == parsed); + } + } diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java index a381d7c2745..0004094e1c2 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java @@ -82,8 +82,9 @@ public class ParsedSchema extends ParsedBlock { void addDocument(ParsedDocument document) { verifyThat(myDocument == null, "already has", myDocument, "so cannot add", document); - verifyThat(name().equals(document.name()), - "schema " + name() + " can only contain document named " + name() + ", was: "+ document.name()); + // TODO - disallow? + // verifyThat(name().equals(document.name()), + // "schema " + name() + " can only contain document named " + name() + ", was: "+ document.name()); this.myDocument = document; } @@ -156,16 +157,15 @@ public class ParsedSchema extends ParsedBlock { verifyThat(name.equals(parsed.name()), "resolveInherit name mismatch for", name); verifyThat(! resolvedInherits.containsKey(name), "double resolveInherit for", name); resolvedInherits.put(name, parsed); - var old = allResolvedInherits.put(name, parsed); + var old = allResolvedInherits.put("schema " + name, parsed); verifyThat(old == null || old == parsed, "conflicting resolveInherit for", name); } void resolveInheritByDocument(String name, ParsedSchema parsed) { verifyThat(inheritedByDocument.contains(name), "resolveInheritByDocument for non-inherited name", name); - verifyThat(name.equals(parsed.name()), "resolveInheritByDocument name mismatch for", name); - var old = allResolvedInherits.put(name, parsed); - verifyThat(old == null || old == parsed, "conflicting resolveInherit for", name); + var old = allResolvedInherits.put("document " + name, parsed); + verifyThat(old == null || old == parsed, "conflicting resolveInheritByDocument for", name); } } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java index 10f45e6c17b..e29e4833856 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java @@ -36,6 +36,20 @@ public class IntermediateCollectionTestCase { } @Test + public void names_may_differ() throws Exception { + String input = joinLines + ("schema foo_search {", + " document foo {", + " }", + "}"); + var collection = new IntermediateCollection(); + ParsedSchema schema = collection.addSchemaFromString(input); + assertEquals("foo_search", schema.name()); + assertTrue(schema.hasDocument()); + assertEquals("foo", schema.getDocument().name()); + } + + @Test public void can_add_schema_files() throws Exception { var collection = new IntermediateCollection(); collection.addSchemaFromFile("src/test/derived/deriver/child.sd"); @@ -167,7 +181,7 @@ public class IntermediateCollectionTestCase { assertEquals(collection.getParsedSchemas().size(), 3); var ex = assertThrows(IllegalArgumentException.class, () -> collection.resolveInternalConnections()); - assertTrue(ex.getMessage().startsWith("Inheritance cycle for schemas: ")); + assertTrue(ex.getMessage().startsWith("Inheritance/reference cycle for schemas: ")); } @Test @@ -180,7 +194,7 @@ public class IntermediateCollectionTestCase { var ex = assertThrows(IllegalArgumentException.class, () -> collection.resolveInternalConnections()); System.err.println("ex: "+ex.getMessage()); - assertTrue(ex.getMessage().startsWith("Inheritance cycle for documents: ")); + assertTrue(ex.getMessage().startsWith("Inheritance/reference cycle for documents: ")); } @Test @@ -194,4 +208,30 @@ public class IntermediateCollectionTestCase { assertEquals("document foo inherits from unavailable document bar", ex.getMessage()); } + @Test + public void can_detect_document_reference_cycle() throws Exception { + var collection = new IntermediateCollection(); + collection.addSchemaFromString("schema foo { document foo { field oneref type reference<bar> {} } }"); + collection.addSchemaFromString("schema bar { document bar { field tworef type reference<foo> {} } }"); + assertEquals(collection.getParsedSchemas().size(), 2); + var ex = assertThrows(IllegalArgumentException.class, () -> + collection.resolveInternalConnections()); + System.err.println("ex: "+ex.getMessage()); + assertTrue(ex.getMessage().startsWith("Inheritance/reference cycle for documents: ")); + } + + @Test + public void can_detect_cycles_with_reference() throws Exception { + var collection = new IntermediateCollection(); + collection.addSchemaFromString("schema foo { document foodoc inherits bardoc {} }"); + collection.addSchemaFromString("schema bar { document bardoc { field myref type reference<qux> { } } }"); + collection.addSchemaFromString("schema qux inherits foo { document qux inherits foodoc {} }"); + assertEquals(collection.getParsedSchemas().size(), 3); + var ex = assertThrows(IllegalArgumentException.class, () -> + collection.resolveInternalConnections()); + System.err.println("ex: "+ex.getMessage()); + assertTrue(ex.getMessage().startsWith("Inheritance/reference cycle for documents: ")); + } + + } diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateParserTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateParserTestCase.java index 9d93dedb709..190a8fc0a19 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateParserTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateParserTestCase.java @@ -22,8 +22,12 @@ public class IntermediateParserTestCase { var deployLogger = new BaseDeployLogger(); var modelProperties = new TestProperties(); var stream = new SimpleCharStream(input); - var parser = new IntermediateParser(stream, deployLogger, modelProperties); - return parser.schema(); + try { + var parser = new IntermediateParser(stream, deployLogger, modelProperties); + return parser.schema(); + } catch (ParseException pe) { + throw new ParseException(stream.formatException(pe.getMessage())); + } } ParsedSchema parseFile(String fileName) throws Exception { @@ -208,6 +212,7 @@ public class IntermediateParserTestCase { checkFileParses("src/test/derived/uri_wset/uri_wset.sd"); checkFileParses("src/test/examples/arrays.sd"); checkFileParses("src/test/examples/arraysweightedsets.sd"); + checkFileParses("src/test/examples/attributeposition.sd"); checkFileParses("src/test/examples/attributesettings.sd"); checkFileParses("src/test/examples/attributesexactmatch.sd"); checkFileParses("src/test/examples/casing.sd"); |