aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHenning Baldersheim <balder@yahoo-inc.com>2022-02-25 17:39:47 +0100
committerGitHub <noreply@github.com>2022-02-25 17:39:47 +0100
commit937f3b4efd4df819484a58e535c928927687ad09 (patch)
tree9e26189b0b9b82669b84f4455f74da9cd38d705e
parent4d42745d588c809b49de187c030ba97b7310e1e6 (diff)
parentc08469d34727330e2e783ce75c43b4bae2d8e92f (diff)
Merge pull request #21406 from vespa-engine/arnej/intermediate-parser-7
Arnej/intermediate parser 7
-rw-r--r--config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java105
-rw-r--r--config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java26
-rw-r--r--config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedDocument.java11
-rw-r--r--config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedSchema.java10
-rw-r--r--config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java6
-rw-r--r--config-model/src/test/java/com/yahoo/searchdefinition/parser/IntermediateCollectionTestCase.java63
6 files changed, 210 insertions, 11 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
new file mode 100644
index 00000000000..488464ccd1f
--- /dev/null
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/InheritanceResolver.java
@@ -0,0 +1,105 @@
+// 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.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Class resolving some inheritance relationships.
+ *
+ * @author arnej27959
+ **/
+public class InheritanceResolver {
+
+ private final Map<String, ParsedSchema> parsedSchemas;
+ private final Map<String, ParsedDocument> parsedDocs = new HashMap<>();
+
+ public InheritanceResolver(Map<String, ParsedSchema> parsedSchemas) {
+ this.parsedSchemas = parsedSchemas;
+ }
+
+ private void inheritanceCycleCheck(ParsedSchema schema, List<String> seen) {
+ String name = schema.name();
+ if (seen.contains(name)) {
+ seen.add(name);
+ throw new IllegalArgumentException("Inheritance cycle for schemas: " +
+ String.join(" -> ", seen));
+ }
+ seen.add(name);
+ for (ParsedSchema parent : schema.getResolvedInherits()) {
+ inheritanceCycleCheck(parent, seen);
+ }
+ seen.remove(name);
+ }
+
+ private void resolveSchemaInheritance() {
+ for (ParsedSchema schema : parsedSchemas.values()) {
+ for (String inherit : schema.getInherited()) {
+ var parent = parsedSchemas.get(inherit);
+ if (parent == null) {
+ throw new IllegalArgumentException("schema " + schema.name() + " inherits from unavailable schema " + inherit);
+ }
+ schema.resolveInherit(inherit, parent);
+ }
+ }
+ }
+
+ private void checkSchemaCycles() {
+ List<String> seen = new ArrayList<>();
+ for (ParsedSchema schema : parsedSchemas.values()) {
+ inheritanceCycleCheck(schema, seen);
+ }
+ }
+
+ private void resolveDocumentInheritance() {
+ for (ParsedSchema schema : parsedSchemas.values()) {
+ if (! schema.hasDocument()) {
+ // TODO: is schema without a document even valid?
+ continue;
+ }
+ ParsedDocument doc = schema.getDocument();
+ var old = parsedDocs.put(doc.name(), doc);
+ assert(old == null);
+ }
+ for (ParsedDocument doc : parsedDocs.values()) {
+ for (String inherit : doc.getInherited()) {
+ var parentDoc = parsedDocs.get(inherit);
+ if (parentDoc == null) {
+ throw new IllegalArgumentException("document " + doc.name() + " inherits from unavailable document " + inherit);
+ }
+ doc.resolveInherit(inherit, parentDoc);
+ }
+ }
+ }
+
+ private void inheritanceCycleCheck(ParsedDocument document, List<String> seen) {
+ String name = document.name();
+ if (seen.contains(name)) {
+ seen.add(name);
+ throw new IllegalArgumentException("Inheritance cycle for documents: " +
+ String.join(" -> ", seen));
+ }
+ seen.add(name);
+ for (ParsedDocument parent : document.getResolvedInherits()) {
+ inheritanceCycleCheck(parent, seen);
+ }
+ seen.remove(name);
+ }
+
+ private void checkDocumentCycles() {
+ List<String> seen = new ArrayList<>();
+ for (ParsedDocument doc : parsedDocs.values()) {
+ inheritanceCycleCheck(doc, seen);
+ }
+ }
+
+ public void resolveInheritance() {
+ resolveSchemaInheritance();
+ resolveDocumentInheritance();
+ checkSchemaCycles();
+ checkDocumentCycles();
+ }
+
+}
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java
index 1f011883bfe..a93fb441563 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/IntermediateCollection.java
@@ -47,7 +47,7 @@ public class IntermediateCollection {
var parser = new IntermediateParser(stream, deployLogger, modelProperties);
var schema = parser.schema();
if (parsedSchemas.containsKey(schema.name())) {
- throw new IllegalArgumentException("Duplicate schemas named: "+schema.name());
+ throw new IllegalArgumentException("Duplicate schemas named: " + schema.name());
}
parsedSchemas.put(schema.name(), schema);
return schema;
@@ -60,14 +60,14 @@ public class IntermediateCollection {
throw new IllegalArgumentException("The file containing schema '"
+ parsed.name() + "' must be named '"
+ parsed.name() + ApplicationPackage.SD_NAME_SUFFIX
- + "', not " + fileName);
+ + "', was '" + stripDirs(fileName) + "'");
}
}
private String baseName(String filename) {
int pos = filename.lastIndexOf('/');
if (pos != -1) {
- filename = filename.substring(pos+1);
+ filename = filename.substring(pos + 1);
}
pos = filename.lastIndexOf('.');
if (pos != -1) {
@@ -76,6 +76,14 @@ public class IntermediateCollection {
return filename;
}
+ private String stripDirs(String filename) {
+ int pos = filename.lastIndexOf('/');
+ if (pos != -1) {
+ return filename.substring(pos + 1);
+ }
+ return filename;
+ }
+
/**
* parse a schema from the given reader and add result to collection
**/
@@ -85,7 +93,7 @@ public class IntermediateCollection {
} catch (java.io.IOException ex) {
throw new IllegalArgumentException("Failed reading from " + reader.getName() + ": " + ex.getMessage());
} catch (ParseException ex) {
- throw new IllegalArgumentException("Failed parsing schema from " + reader.getName() + ": "+ex.getMessage());
+ throw new IllegalArgumentException("Failed parsing schema from " + reader.getName() + ": " + ex.getMessage());
}
}
@@ -97,7 +105,7 @@ public class IntermediateCollection {
} catch (java.io.IOException ex) {
throw new IllegalArgumentException("Could not read file " + fileName + ": " + ex.getMessage());
} catch (ParseException ex) {
- throw new IllegalArgumentException("Failed parsing schema file " + fileName + ": "+ex.getMessage());
+ throw new IllegalArgumentException("Failed parsing schema file " + fileName + ": " + ex.getMessage());
}
}
@@ -109,7 +117,7 @@ public class IntermediateCollection {
try {
ParsedSchema schema = parsedSchemas.get(schemaName);
if (schema == null) {
- throw new IllegalArgumentException("No schema named: "+schemaName);
+ throw new IllegalArgumentException("No schema named: " + schemaName);
}
var stream = new SimpleCharStream(IOUtils.readAll(reader.getReader()));
var parser = new IntermediateParser(stream, deployLogger, modelProperties);
@@ -117,7 +125,7 @@ public class IntermediateCollection {
} catch (java.io.IOException ex) {
throw new IllegalArgumentException("Failed reading from " + reader.getName() + ": " + ex.getMessage());
} catch (ParseException ex) {
- throw new IllegalArgumentException("Failed parsing rank-profile from " + reader.getName() + ": "+ex.getMessage());
+ throw new IllegalArgumentException("Failed parsing rank-profile from " + reader.getName() + ": " + ex.getMessage());
}
}
@@ -131,4 +139,8 @@ public class IntermediateCollection {
}
}
+ void resolveInternalConnections() {
+ var resolver = new InheritanceResolver(parsedSchemas);
+ resolver.resolveInheritance();
+ }
}
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 0b7d0507161..8cd64ef16f7 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
@@ -17,6 +17,7 @@ import java.util.Optional;
**/
public class ParsedDocument extends ParsedBlock {
private final List<String> inherited = new ArrayList<>();
+ private final Map<String, ParsedDocument> resolvedInherits = new HashMap();
private final Map<String, ParsedField> docFields = new HashMap<>();
private final Map<String, ParsedStruct> docStructs = new HashMap<>();
private final Map<String, ParsedAnnotation> docAnnotations = new HashMap<>();
@@ -27,6 +28,7 @@ public class ParsedDocument extends ParsedBlock {
List<String> getInherited() { return List.copyOf(inherited); }
List<ParsedAnnotation> getAnnotations() { return List.copyOf(docAnnotations.values()); }
+ List<ParsedDocument> getResolvedInherits() { return List.copyOf(resolvedInherits.values()); }
List<ParsedField> getFields() { return List.copyOf(docFields.values()); }
List<ParsedStruct> getStructs() { return List.copyOf(docStructs.values()); }
@@ -50,6 +52,13 @@ public class ParsedDocument extends ParsedBlock {
docAnnotations.put(annName, annotation);
}
- public String toString() { return "document "+name(); }
+ public String toString() { return "document " + name(); }
+
+ void resolveInherit(String name, ParsedDocument parsed) {
+ verifyThat(inherited.contains(name), "resolveInherit for non-inherited name", name);
+ verifyThat(name.equals(parsed.name()), "resolveInherit name mismatch for", name);
+ verifyThat(! resolvedInherits.containsKey(name), "double resolveInherit for", name);
+ resolvedInherits.put(name, 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 1916f236aa9..bf448b31dd2 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
@@ -36,6 +36,7 @@ public class ParsedSchema extends ParsedBlock {
private final List<OnnxModel> onnxModels = new ArrayList<>();
private final List<RankingConstant> rankingConstants = new ArrayList<>();
private final List<String> inherited = new ArrayList<>();
+ private final Map<String, ParsedSchema> resolvedInherits = new HashMap();
private final Map<String, ParsedAnnotation> extraAnnotations = new HashMap<>();
private final Map<String, ParsedDocumentSummary> docSums = new HashMap<>();
private final Map<String, ParsedField> extraFields = new HashMap<>();
@@ -64,6 +65,7 @@ public class ParsedSchema extends ParsedBlock {
List<RankingConstant> getRankingConstants() { return List.copyOf(rankingConstants); }
List<String> getInherited() { return List.copyOf(inherited); }
Map<String, ParsedRankProfile> getRankProfiles() { return Map.copyOf(rankProfiles); }
+ List<ParsedSchema> getResolvedInherits() { return List.copyOf(resolvedInherits.values()); }
void addAnnotation(ParsedAnnotation annotation) {
String annName = annotation.name();
@@ -136,4 +138,12 @@ public class ParsedSchema extends ParsedBlock {
"already has stemming", defaultStemming, "cannot also set", value);
defaultStemming = value;
}
+
+ void resolveInherit(String name, ParsedSchema parsed) {
+ verifyThat(inherited.contains(name), "resolveInherit for non-inherited name", name);
+ verifyThat(name.equals(parsed.name()), "resolveInherit name mismatch for", name);
+ verifyThat(! resolvedInherits.containsKey(name), "double resolveInherit for", name);
+ resolvedInherits.put(name, parsed);
+ }
+
}
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java
index 4d5280d29eb..9f02c5247ef 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/parser/ParsedType.java
@@ -104,14 +104,14 @@ class ParsedType {
void setCreateIfNonExistent(boolean value) {
if (variant != Variant.WSET) {
- throw new IllegalArgumentException("CreateIfNonExistent only valid for weightedset, not "+variant);
+ throw new IllegalArgumentException("CreateIfNonExistent only valid for weightedset, not " + variant);
}
this.createIfNonExistent = value;
}
void setRemoveIfZero(boolean value) {
if (variant != Variant.WSET) {
- throw new IllegalArgumentException("RemoveIfZero only valid for weightedset, not "+variant);
+ throw new IllegalArgumentException("RemoveIfZero only valid for weightedset, not " + variant);
}
this.removeIfZero = value;
}
@@ -119,7 +119,7 @@ class ParsedType {
void setVariant(Variant value) {
if (variant == value) return; // already OK
if (variant != Variant.UNKNOWN) {
- throw new IllegalArgumentException("setVariant("+value+") only valid for UNKNOWN, not: "+variant);
+ throw new IllegalArgumentException("setVariant(" + value + ") only valid for UNKNOWN, not: " + variant);
}
// maybe even more checking would be useful
this.variant = value;
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 c20991f9de3..1ee7ae4937a 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
@@ -107,6 +107,15 @@ public class IntermediateCollectionTestCase {
}
@Test
+ public void name_mismatch_throws() throws Exception {
+ var collection = new IntermediateCollection();
+ var ex = assertThrows(IllegalArgumentException.class, () ->
+ collection.addSchemaFromReader(readerOf("src/test/cfg/application/sdfilenametest/schemas/notmusic.sd")));
+ assertEquals("The file containing schema 'music' must be named 'music.sd', was 'notmusic.sd'",
+ ex.getMessage());
+ }
+
+ @Test
public void bad_parse_throws() throws Exception {
var collection = new IntermediateCollection();
var ex = assertThrows(IllegalArgumentException.class, () ->
@@ -122,4 +131,58 @@ public class IntermediateCollectionTestCase {
assertTrue(ex.getMessage().startsWith("Failed parsing rank-profile from src/test/examples/structoutsideofdocument.sd: "));
}
+ @Test
+ public void can_resolve_document_inheritance() throws Exception {
+ var collection = new IntermediateCollection();
+ collection.addSchemaFromFile("src/test/derived/deriver/child.sd");
+ collection.addSchemaFromFile("src/test/derived/deriver/grandparent.sd");
+ collection.addSchemaFromFile("src/test/derived/deriver/parent.sd");
+ collection.resolveInternalConnections();
+ var schemes = collection.getParsedSchemas();
+ assertEquals(schemes.size(), 3);
+ var childDoc = schemes.get("child").getDocument();
+ var inherits = childDoc.getResolvedInherits();
+ assertEquals(inherits.size(), 1);
+ var parentDoc = inherits.get(0);
+ assertEquals(parentDoc.name(), "parent");
+ inherits = parentDoc.getResolvedInherits();
+ assertEquals(inherits.size(), 1);
+ assertEquals(inherits.get(0).name(), "grandparent");
+ }
+
+ @Test
+ public void can_detect_schema_inheritance_cycles() throws Exception {
+ var collection = new IntermediateCollection();
+ collection.addSchemaFromString("schema foo inherits bar {}");
+ collection.addSchemaFromString("schema bar inherits qux {}");
+ collection.addSchemaFromString("schema qux inherits foo {}");
+ assertEquals(collection.getParsedSchemas().size(), 3);
+ var ex = assertThrows(IllegalArgumentException.class, () ->
+ collection.resolveInternalConnections());
+ assertTrue(ex.getMessage().startsWith("Inheritance cycle for schemas: "));
+ }
+
+ @Test
+ public void can_detect_document_inheritance_cycles() throws Exception {
+ var collection = new IntermediateCollection();
+ collection.addSchemaFromString("schema foo { document foo inherits bar {} }");
+ collection.addSchemaFromString("schema bar { document bar inherits qux {} }");
+ collection.addSchemaFromString("schema qux { document qux inherits foo {} }");
+ assertEquals(collection.getParsedSchemas().size(), 3);
+ var ex = assertThrows(IllegalArgumentException.class, () ->
+ collection.resolveInternalConnections());
+ assertTrue(ex.getMessage().startsWith("Inheritance cycle for documents: "));
+ }
+
+ @Test
+ public void can_detect_missing_doc() throws Exception {
+ var collection = new IntermediateCollection();
+ collection.addSchemaFromString("schema foo { document foo inherits bar {} }");
+ collection.addSchemaFromString("schema qux { document qux inherits foo {} }");
+ assertEquals(collection.getParsedSchemas().size(), 2);
+ var ex = assertThrows(IllegalArgumentException.class, () ->
+ collection.resolveInternalConnections());
+ assertEquals("document foo inherits from unavailable document bar", ex.getMessage());
+ }
+
}