Merge pull request #27367 from vespa-engine/hmusum/revert-validate-semantic-rules

Hmusum/revert validate semantic rules

8 files changed, 30 insertions, 98 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/SemanticRuleBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/SemanticRuleBuilder.java
index 4a295d49a32..ce999603439 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/SemanticRuleBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/SemanticRuleBuilder.java
@@ -4,18 +4,11 @@ package com.yahoo.vespa.model.container.search;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.io.IOUtils;
 import com.yahoo.io.reader.NamedReader;
-import com.yahoo.language.simple.SimpleLinguistics;
-import com.yahoo.prelude.semantics.RuleBase;
-import com.yahoo.prelude.semantics.RuleImporter;
-import com.yahoo.prelude.semantics.SemanticRulesConfig;
-import com.yahoo.prelude.semantics.parser.ParseException;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
+import java.util.stream.Collectors;
 
 /**
  * Reads the semantic rules from the application package by delegating to SemanticRules.
@@ -25,22 +18,16 @@ import java.util.Map;
 // TODO: Move into SemanticRules
 public class SemanticRuleBuilder {
 
-    /** Build the set of semantic rules for an application package and validates them */
+    /** Build the set of semantic rules for an application package */
     public SemanticRules build(ApplicationPackage applicationPackage) {
-        var ruleFiles = applicationPackage.getFiles(ApplicationPackage.RULES_DIR, "sr");
-        var rules = new SemanticRules(ruleFiles.stream().map(this::toRuleBaseConfigView).toList());
-
-        // Create config to make sure rules are valid, config is validated in call to toMap() below
-        var builder = new SemanticRulesConfig.Builder();
-        rules.getConfig(builder);
-        SemanticRulesConfig config = builder.build();
+        List<NamedReader> ruleBaseFiles = null;
         try {
-            toMap(config);  // validates config
-            ensureZeroOrOneDefaultRule(config);
-        } catch (ParseException | IOException e) {
-            throw new RuntimeException(e);
+            ruleBaseFiles = applicationPackage.getFiles(ApplicationPackage.RULES_DIR, "sr");
+            return new SemanticRules(ruleBaseFiles.stream().map(this::toRuleBaseConfigView).toList());
+        }
+        finally {
+            NamedReader.closeAll(ruleBaseFiles);
         }
-        return rules;
     }
 
     private SemanticRules.RuleBase toRuleBaseConfigView(NamedReader reader) {
@@ -56,32 +43,7 @@ public class SemanticRuleBuilder {
 
     private String toName(String fileName) {
         String shortName = new File(fileName).getName();
-        return shortName.substring(0, shortName.length() - ".sr".length());
-    }
-
-    private void ensureZeroOrOneDefaultRule(SemanticRulesConfig config) {
-        String defaultName = null;
-        for (SemanticRulesConfig.Rulebase ruleBase : config.rulebase()) {
-            if (defaultName != null && ruleBase.isdefault()) {
-                List<String> defaultRules = new ArrayList<>(List.of(defaultName, ruleBase.name()));
-                defaultRules.sort(String::compareTo);
-                throw new IllegalArgumentException("Rules " + defaultRules + " are both marked as the default rule, there can only be one");
-            }
-            if (ruleBase.isdefault())
-                defaultName = ruleBase.name();
-        }
-    }
-
-    static Map<String, RuleBase> toMap(SemanticRulesConfig config) throws ParseException, IOException {
-        RuleImporter ruleImporter = new RuleImporter(config, new SimpleLinguistics());
-        Map<String, RuleBase> ruleBaseMap = new HashMap<>();
-        for (SemanticRulesConfig.Rulebase ruleBaseConfig : config.rulebase()) {
-            RuleBase ruleBase = ruleImporter.importConfig(ruleBaseConfig);
-            if (ruleBaseConfig.isdefault())
-                ruleBase.setDefault(true);
-            ruleBaseMap.put(ruleBase.getName(), ruleBase);
-        }
-        return ruleBaseMap;
+        return shortName.substring(0, shortName.length()-".sr".length());
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/SemanticRules.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/SemanticRules.java
index 0083fc6b886..46c8577c6e8 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/SemanticRules.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/SemanticRules.java
@@ -6,14 +6,14 @@ import java.io.Serializable;
 import java.util.List;
 
 /**
- * Returns the semantic rules config from a set of rule bases.
+ * Returns the semantic rules config form a set of rule bases.
  * Owned by a container cluster
  *
  * @author bratseth
  */
 public class SemanticRules implements Serializable, SemanticRulesConfig.Producer {
 
-    private final List<RuleBase> ruleBases;
+    private List<RuleBase> ruleBases;
 
     public SemanticRules(List<RuleBase> ruleBases) {
         this.ruleBases = ruleBases;
@@ -46,6 +46,7 @@ public class SemanticRules implements Serializable, SemanticRulesConfig.Producer
             return ruleBaseBuilder;
         }
 
+
     }
 
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/SemanticRulesTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/SemanticRulesTest.java
index e20e14f9465..d9e2ae59ef6 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/SemanticRulesTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/SemanticRulesTest.java
@@ -2,66 +2,52 @@
 package com.yahoo.vespa.model.container.search;
 
 import com.yahoo.config.model.application.provider.FilesApplicationPackage;
+import com.yahoo.language.simple.SimpleLinguistics;
 import com.yahoo.prelude.semantics.RuleBase;
+import com.yahoo.prelude.semantics.RuleImporter;
 import com.yahoo.prelude.semantics.SemanticRulesConfig;
 import com.yahoo.prelude.semantics.parser.ParseException;
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.*;
+
 import java.io.File;
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.Map;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
-
 /**
  * @author bratseth
  */
 public class SemanticRulesTest {
 
-    private static final String basePath = "src/test/java/com/yahoo/vespa/model/container/search/";
-    private static final String root = basePath + "semanticrules";
-    private static final String rootWithErrors = basePath + "semanticrules_with_errors";
-    private static final String rootWithDuplicateDefault = basePath + "semanticrules_with_duplicate_default_rule";
+    private final static String root = "src/test/java/com/yahoo/vespa/model/container/search/semanticrules";
 
     @Test
-    void semanticRulesTest() throws ParseException, IOException {
+    void semanticRulesTest()  throws ParseException, IOException {
         SemanticRuleBuilder ruleBuilder = new SemanticRuleBuilder();
         SemanticRules rules = ruleBuilder.build(FilesApplicationPackage.fromFile(new File(root)));
         SemanticRulesConfig.Builder configBuilder = new SemanticRulesConfig.Builder();
         rules.getConfig(configBuilder);
         SemanticRulesConfig config = new SemanticRulesConfig(configBuilder);
-        Map<String, RuleBase> ruleBases = SemanticRuleBuilder.toMap(config);
+        Map<String, RuleBase> ruleBases = toMap(config);
         assertEquals(2, ruleBases.size());
         assertTrue(ruleBases.containsKey("common"));
         assertTrue(ruleBases.containsKey("other"));
         assertFalse(ruleBases.get("common").isDefault());
         assertTrue(ruleBases.get("other").isDefault());
-        assertTrue(ruleBases.get("other").includes("common"));
-        assertNotNull(ruleBases.get("other").getCondition("stopword"));
     }
 
-    @Test
-    void rulesWithErrors() {
-        try {
-            new SemanticRuleBuilder().build(FilesApplicationPackage.fromFile(new File(rootWithErrors)));
-            fail("should fail with exception");
-        } catch (Exception e) {
-            assertEquals("com.yahoo.prelude.semantics.parser.ParseException: Could not parse 'semantic-rules.cfg'", e.getMessage());
-        }
-    }
-
-    @Test
-    void rulesWithDuplicateDefault() {
-        try {
-            new SemanticRuleBuilder().build(FilesApplicationPackage.fromFile(new File(rootWithDuplicateDefault)));
-            fail("should fail with exception");
-        } catch (Exception e) {
-            assertEquals("Rules [one, other] are both marked as the default rule, there can only be one", e.getMessage());
+    private static Map<String, RuleBase> toMap(SemanticRulesConfig config) throws ParseException, IOException {
+        RuleImporter ruleImporter = new RuleImporter(config, new SimpleLinguistics());
+        Map<String, RuleBase> ruleBaseMap = new HashMap<>();
+        for (SemanticRulesConfig.Rulebase ruleBaseConfig : config.rulebase()) {
+            RuleBase ruleBase = ruleImporter.importConfig(ruleBaseConfig);
+            if (ruleBaseConfig.isdefault())
+                ruleBase.setDefault(true);
+            ruleBaseMap.put(ruleBase.getName(), ruleBase);
         }
+        return ruleBaseMap;
     }
 
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_duplicate_default_rule/rules/one.sr b/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_duplicate_default_rule/rules/one.sr
deleted file mode 100644
index 4f2271e91ba..00000000000
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_duplicate_default_rule/rules/one.sr
+++ /dev/null
@@ -1,5 +0,0 @@
-# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-@default
-
-# Spelling correction
-bahc -> bach;
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_duplicate_default_rule/rules/other.sr b/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_duplicate_default_rule/rules/other.sr
deleted file mode 100644
index 29f7e85967f..00000000000
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_duplicate_default_rule/rules/other.sr
+++ /dev/null
@@ -1,5 +0,0 @@
-# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-@default
-
-# Spelling correction
-list-> liszt;
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_errors/rules/invalid.sr b/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_errors/rules/invalid.sr
deleted file mode 100644
index 9d89cab7e31..00000000000
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/semanticrules_with_errors/rules/invalid.sr
+++ /dev/null
@@ -1,7 +0,0 @@
-# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-# Stopwords
-[stopword] -> ;
-[stopword] :- and, or, the, what, why, how;
-
-# Synonyms, with wrong character at end of line
-[bill] :- Bill, bill, William:
diff --git a/container-search/src/main/resources/configdefinitions/prelude.semantics.semantic-rules.def b/container-search/src/main/resources/configdefinitions/prelude.semantics.semantic-rules.def
index d8e521ed940..71fb907ffe2 100644
--- a/container-search/src/main/resources/configdefinitions/prelude.semantics.semantic-rules.def
+++ b/container-search/src/main/resources/configdefinitions/prelude.semantics.semantic-rules.def
@@ -3,7 +3,6 @@
 namespace=prelude.semantics
 
 # Whether we should use these rule bases in pre-Vespa 2.2 compatibility mode
-# TODO: Unused, remove in Vespa 9
 compatibility bool default=false
 
 # The name of a rule base
diff --git a/container-search/src/test/java/com/yahoo/prelude/semantics/test/rulebases/cjk-rules.cfg b/container-search/src/test/java/com/yahoo/prelude/semantics/test/rulebases/cjk-rules.cfg
index 2e3299f7723..b040a44ad90 100644
--- a/container-search/src/test/java/com/yahoo/prelude/semantics/test/rulebases/cjk-rules.cfg
+++ b/container-search/src/test/java/com/yahoo/prelude/semantics/test/rulebases/cjk-rules.cfg
@@ -1,3 +1,4 @@
+compatibility false
 rulebase[1]
 rulebase[0].name "cjk"
 rulebase[0].isdefault false