From 5308f828fa3e3add047c958b7f674ab9e80d9e41 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Tue, 7 May 2024 11:01:15 +0200 Subject: Include value of 'idref' attribute when deciding which elements to keep when preprocessing We already include value of 'id' to key which is used to decide which elements are equal, but also need to include value of 'idref' so that we know which elements to keep e.g. in a list of searchers referred to by 'idref' in a search chain. --- .../config/application/ConfigDefinitionDir.java | 1 - .../yahoo/config/application/IncludeProcessor.java | 1 - .../config/application/OverrideProcessor.java | 22 +++---- .../config/application/ValidationProcessor.java | 4 +- .../config/application/OverrideProcessorTest.java | 69 ++++++++++++++++++++-- 5 files changed, 79 insertions(+), 18 deletions(-) diff --git a/config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java b/config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java index d4b257f0ba9..1329befbc9d 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/ConfigDefinitionDir.java @@ -11,7 +11,6 @@ import java.util.List; * but they cannot conflict with the existing ones. * * @author Ulf Lilleengen - * @since 5.1 */ public class ConfigDefinitionDir { private final File defDir; diff --git a/config-application-package/src/main/java/com/yahoo/config/application/IncludeProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/IncludeProcessor.java index bc48e7dd814..ac365fa5a3e 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/IncludeProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/IncludeProcessor.java @@ -22,7 +22,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; * Handles preprocess:include statements and returns a Document which has all the include statements resolved * * @author hmusum - * @since 5.22 */ class IncludeProcessor implements PreProcessor { diff --git a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java index bb456d95326..5f2046b1450 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/OverrideProcessor.java @@ -46,6 +46,7 @@ class OverrideProcessor implements PreProcessor { private final Tags tags; private static final String ID_ATTRIBUTE = "id"; + private static final String IDREF_ATTRIBUTE = "idref"; private static final String INSTANCE_ATTRIBUTE = "instance"; private static final String ENVIRONMENT_ATTRIBUTE = "environment"; private static final String REGION_ATTRIBUTE = "region"; @@ -200,7 +201,7 @@ class OverrideProcessor implements PreProcessor { /** Find the most specific element and remove all others. */ private void retainMostSpecific(Element parent, List children, Context context) { - // Keep track of elements with highest number of matches (might be more than one element with same tag, need a list) + // Keep track of elements with the highest number of matches (might be more than one element with same tag, need a list) List bestMatches = new ArrayList<>(); int bestMatch = 0; for (Element child : children) { @@ -307,42 +308,43 @@ class OverrideProcessor implements PreProcessor { private Set getInstances(Element element) { String instance = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, INSTANCE_ATTRIBUTE); - if (instance == null || instance.isEmpty()) return Set.of(); + if (instance.isEmpty()) return Set.of(); return Arrays.stream(instance.split(" ")).map(InstanceName::from).collect(Collectors.toSet()); } private Set getEnvironments(Element element) { String env = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, ENVIRONMENT_ATTRIBUTE); - if (env == null || env.isEmpty()) return Set.of(); + if (env.isEmpty()) return Set.of(); return Arrays.stream(env.split(" ")).map(Environment::from).collect(Collectors.toSet()); } private Set getRegions(Element element) { String reg = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, REGION_ATTRIBUTE); - if (reg == null || reg.isEmpty()) return Set.of(); + if (reg.isEmpty()) return Set.of(); return Arrays.stream(reg.split(" ")).map(RegionName::from).collect(Collectors.toSet()); } private Set getClouds(Element element) { String reg = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, CLOUD_ATTRIBUTE); - if (reg == null || reg.isEmpty()) return Set.of(); + if (reg.isEmpty()) return Set.of(); return Arrays.stream(reg.split(" ")).map(CloudName::from).collect(Collectors.toSet()); } private Tags getTags(Element element) { String env = element.getAttributeNS(XmlPreProcessor.deployNamespaceUri, TAGS_ATTRIBUTE); - if (env == null || env.isEmpty()) return Tags.empty(); + if (env.isEmpty()) return Tags.empty(); return Tags.fromString(env); } private Map> elementsByTagNameAndId(List children) { Map> elementsByTagName = new LinkedHashMap<>(); - // Index by tag name + // Index by tag name and optionally add "id" or "idref" to key if they are set for (Element child : children) { String key = child.getTagName(); - if (child.hasAttribute(ID_ATTRIBUTE)) { + if (child.hasAttribute(ID_ATTRIBUTE)) key += child.getAttribute(ID_ATTRIBUTE); - } + if (child.hasAttribute(IDREF_ATTRIBUTE)) + key += child.getAttribute(IDREF_ATTRIBUTE); if ( ! elementsByTagName.containsKey(key)) { elementsByTagName.put(key, new ArrayList<>()); } @@ -382,7 +384,7 @@ class OverrideProcessor implements PreProcessor { } /** - * Represents environment and region in a given context. + * Represents environments, regions, instances, clouds and tags in a given context. */ private static final class Context { diff --git a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java index b02ccc711c0..4dc40df61f4 100644 --- a/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java +++ b/config-application-package/src/main/java/com/yahoo/config/application/ValidationProcessor.java @@ -11,8 +11,8 @@ public class ValidationProcessor implements PreProcessor { @Override public Document process(Document input) throws IOException, TransformerException { - NodeList includeitems = input.getElementsByTagNameNS("http://www.w3.org/2001/XInclude", "*"); - if (includeitems.getLength() > 0) + NodeList includeItems = input.getElementsByTagNameNS("http://www.w3.org/2001/XInclude", "*"); + if (includeItems.getLength() > 0) throw new UnsupportedOperationException("XInclude not supported, use preprocess:include instead"); return input; } diff --git a/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java b/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java index e5e36615b09..c2b0770ab06 100644 --- a/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java +++ b/config-application-package/src/test/java/com/yahoo/config/application/OverrideProcessorTest.java @@ -2,6 +2,7 @@ package com.yahoo.config.application; import com.yahoo.config.provision.Cloud; +import com.yahoo.config.provision.CloudName; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.RegionName; @@ -12,6 +13,8 @@ import org.w3c.dom.Document; import javax.xml.transform.TransformerException; import java.io.StringReader; +import static com.yahoo.config.provision.Tags.empty; + /** * @author Ulf Lilleengen */ @@ -366,14 +369,72 @@ public class OverrideProcessorTest { assertOverride(input, Environment.dev, RegionName.defaultName(), expected); } + /** + * Tests that searchers referred to with idref are overridden per cloud + * and that searchers not referred to with idref are not overridden. + */ + @Test + public void testSearchersReferredWithIdRefPerCloud() throws TransformerException { + String input = + """ + + + + + + + + + + + + + + + """"; + + String expected = + """ + + + + + + + + + + + + + + """"; + + assertOverride(input, "aws", expected.formatted("AwsSearcher")); + assertOverride(input, "gcp", expected.formatted("GcpSearcher")); + } + private void assertOverride(Environment environment, RegionName region, String expected) throws TransformerException { assertOverride(input, environment, region, expected); } - private void assertOverride(String input, Environment environment, RegionName region, String expected) throws TransformerException { - Document inputDoc = Xml.getDocument(new StringReader(input)); - Document newDoc = new OverrideProcessor(InstanceName.from("default"), environment, region, Cloud.defaultCloud().name(), Tags.empty()).process(inputDoc); - TestBase.assertDocument(expected, newDoc); + private void assertOverride(String input, Environment environment, RegionName region, String expected) { + assertOverride(input, environment, region, Cloud.defaultCloud().name(), expected); + } + + private void assertOverride(String input, String cloudName, String expected) { + assertOverride(input, Environment.defaultEnvironment(), RegionName.defaultName(), CloudName.from(cloudName), expected); + } + + private void assertOverride(String input, Environment environment, RegionName region, CloudName cloudName, String expected) { + var inputDoc = Xml.getDocument(new StringReader(input)); + try { + var newDoc = new OverrideProcessor(InstanceName.from("default"), environment, region, cloudName, Tags.empty()) + .process(inputDoc); + TestBase.assertDocument(expected, newDoc); + } catch (TransformerException e) { + throw new RuntimeException(e); + } } } -- cgit v1.2.3