From 81a4087da7ec3f4692ac589b75c135181594d5ec Mon Sep 17 00:00:00 2001 From: Jean-Francois Denise Date: Wed, 23 Oct 2024 19:03:46 +0200 Subject: [PATCH] Fix Issue #95, Introduce a rule for a layer to express that it requires some user selected add-ons --- .../org/wildfly/glow/AddOnSelectionMode.java | 66 ++++++++++ .../java/org/wildfly/glow/GlowSession.java | 94 ++++++++++++- .../java/org/wildfly/glow/LayerMapping.java | 8 ++ .../java/org/wildfly/glow/LayerMetadata.java | 2 + .../src/main/java/org/wildfly/glow/Utils.java | 14 +- .../standalone/f1-add-on/layer-spec.xml | 16 +++ .../standalone/f1-add-on2/layer-spec.xml | 16 +++ .../standalone/f2-add-on/layer-spec.xml | 16 +++ .../standalone/f2-add-on2/layer-spec.xml | 16 +++ .../select-family-consumer/layer-spec.xml | 10 ++ .../layer-spec.xml | 10 ++ .../test/AbstractLayerMetaDataTestCase.java | 3 + .../select/family/SelectFamilyTestCase.java | 123 ++++++++++++++++++ 13 files changed, 392 insertions(+), 2 deletions(-) create mode 100644 core/src/main/java/org/wildfly/glow/AddOnSelectionMode.java create mode 100644 tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f1-add-on/layer-spec.xml create mode 100644 tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f1-add-on2/layer-spec.xml create mode 100644 tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f2-add-on/layer-spec.xml create mode 100644 tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f2-add-on2/layer-spec.xml create mode 100644 tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/select-family-consumer/layer-spec.xml create mode 100644 tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/select-only-one-family-consumer/layer-spec.xml create mode 100644 tests/test-feature-pack/layer-metadata-tests/src/test/java/org/wildfly/glow/rules/test/addon/select/family/SelectFamilyTestCase.java diff --git a/core/src/main/java/org/wildfly/glow/AddOnSelectionMode.java b/core/src/main/java/org/wildfly/glow/AddOnSelectionMode.java new file mode 100644 index 00000000..f4566975 --- /dev/null +++ b/core/src/main/java/org/wildfly/glow/AddOnSelectionMode.java @@ -0,0 +1,66 @@ +/* + * Copyright The WildFly Authors + * SPDX-License-Identifier: Apache-2.0 + */ +package org.wildfly.glow; + +/** + * A layer expects add-ons from a given family to be explicitly selected. + * @author jdenise + */ +public class AddOnSelectionMode { + private final Layer targetLayer; + private final String family; + private final int maxNumber; + + /** + * AddOnSelectionMode constructor. + * @param targetLayer The layer that contains the rule. + * @param family The family that the targetlayer expect add-ons from. + * @param maxNumber The max number of expected add-ons. Can be 1 or -1 (unbounded). + */ + public AddOnSelectionMode(Layer targetLayer, String family, int maxNumber) throws Exception { + this.targetLayer = targetLayer; + this.family = family; + if (maxNumber != 1 && maxNumber != -1) { + throw new Exception("The max number of add-on selection rule can only be 1 or -1 (unbounded)"); + } + this.maxNumber = maxNumber; + } + + /** + * Is the expected number of add-ons unbounded. + * @return true if the number of expected add-ons is unbounded. + */ + public boolean isUnBounded() { + return maxNumber < 0; + } + /** + * Is the expected number of add-ons exactly one. + * @return true if the number of expected add-ons is one. + */ + public boolean isExactlyOne() { + return maxNumber == 1; + } + + /** + * @return the maxNumber of add-ons a user can select. -1 means unbounded. + */ + public boolean isOne() { + return maxNumber == 1; + } + + /** + * @return the targetLayer + */ + public Layer getTargetLayer() { + return targetLayer; + } + + /** + * @return the family + */ + public String getFamily() { + return family; + } +} diff --git a/core/src/main/java/org/wildfly/glow/GlowSession.java b/core/src/main/java/org/wildfly/glow/GlowSession.java index e2929895..b1114bc2 100644 --- a/core/src/main/java/org/wildfly/glow/GlowSession.java +++ b/core/src/main/java/org/wildfly/glow/GlowSession.java @@ -430,7 +430,7 @@ public ScanResults scan() throws Exception { } // Fix addOns Map disabledAddOns = new TreeMap<>(); - fixAddOns(errorSession, layers, mapping, allEnabledAddOns, possibleAddOns, disabledAddOns, arguments); + fixAddOns(errorSession, layers, decorators, mapping, allEnabledAddOns, possibleAddOns, disabledAddOns, arguments); // END ADD-ON // DECORATORS CLEANUP @@ -819,6 +819,7 @@ public void close() throws Exception { private static void fixAddOns(ErrorIdentificationSession errorSession, Set layers, + Set decorators, LayerMapping mapping, Set allEnabledAddOns, Set possibleAddOns, @@ -828,6 +829,97 @@ private static void fixAddOns(ErrorIdentificationSession errorSession, Set familyOfAddOnsComplete = new TreeSet<>(); Map> membersInFamily = new HashMap<>(); Map> defaultMembersInFamily = new HashMap<>(); + // Handle inclusion mode + Map> selectedAddOnsForFamily = new HashMap<>(); + Map> candidatesForFamily = new HashMap<>(); + Set toBeRemoved = new HashSet<>(); + for(AddOn ao : allEnabledAddOns) { + AddOnSelectionMode mode = mapping.getFamilySelectionMode().get(ao.getFamily()); + // We have a selection mode for this family + // A selection means that such add-ons must be included by the user and not automatically discovered. + if (mode != null) { + Set selectedAddOns = selectedAddOnsForFamily.computeIfAbsent(ao.getFamily(), s -> new HashSet<>()); + if (!arguments.getUserEnabledAddOns().contains(ao.getName())) { + toBeRemoved.add(ao); + for(Layer l : ao.getLayers()) { + for(Layer dep : l.getDependencies()) { + AddOn addOn = dep.getAddOn(); + if (addOn != null) { + if(!arguments.getUserEnabledAddOns().contains(addOn.getName())) { + toBeRemoved.add(addOn); + } + } + } + } + Set candidates = candidatesForFamily.computeIfAbsent(ao.getFamily(), s -> new HashSet<>()); + candidates.add(ao); + } else { + selectedAddOns.add(ao); + } + } + } + // At this point we know the addOn that have been included by the user + // and the one that should be removed because not explicitly included. + for(String k : selectedAddOnsForFamily.keySet()) { + // Empty selection... + if (selectedAddOnsForFamily.get(k).isEmpty()) { + Set candidates = candidatesForFamily.get(k); + // None have been selected but we found candidates in the set of discovered ones + // Or we remove them and create an error, asking user to select somes. + if (candidates != null) { + if(candidates.size() == 1) { + // Simply keep it, we have a match + // The add-on will be not removed. + toBeRemoved.remove(candidates.iterator().next()); + } else { + AddOnSelectionMode mode = mapping.getFamilySelectionMode().get(k); + StringBuilder errorBuilder = new StringBuilder(); + if (mode.isExactlyOne()) { + errorBuilder.append("one "); + } else { + errorBuilder.append("At least one "); + } + errorBuilder.append("add-on of the " + k + " family is expected by the " + + mode.getTargetLayer() + " layer\n" + + " To correct this error, enable one of the following add-ons:\n"); + Iterator it = candidates.iterator(); + while(it.hasNext()) { + AddOn ao = it.next(); + errorBuilder.append(" - ").append(ao.getName()); + if(it.hasNext()) { + errorBuilder.append("\n"); + } + } + IdentifiedError error = new IdentifiedError("missing-add-on", errorBuilder.toString(), ERROR); + errorSession.addError(error); + } + } + } else { + // We have a selection of add-ons, is it in the boundaries of the max number? + AddOnSelectionMode mode = mapping.getFamilySelectionMode().get(k); + if (!mode.isUnBounded() && selectedAddOnsForFamily.get(k).size() > 1) { + StringBuilder errorBuilder = new StringBuilder(); + errorBuilder.append("Only one add-on of the " + k + " family is expected by the " + + mode.getTargetLayer() + " layer\n" + + " To correct this error, remove some of the following add-ons:\n"); + Iterator it = selectedAddOnsForFamily.get(k).iterator(); + while (it.hasNext()) { + AddOn ao = it.next(); + errorBuilder.append(" - ").append(ao.getName()); + if (it.hasNext()) { + errorBuilder.append("\n"); + } + } + IdentifiedError error = new IdentifiedError("too-many-add-on", errorBuilder.toString(), ERROR); + errorSession.addError(error); + } + } + } + for(AddOn ao : toBeRemoved) { + allEnabledAddOns.remove(ao); + layers.removeAll(ao.getLayers()); + decorators.removeAll(ao.getLayers()); + } for (AddOn addOn : allEnabledAddOns) { if (addOn.isDefault()) { Set members = defaultMembersInFamily.get(addOn.getFamily()); diff --git a/core/src/main/java/org/wildfly/glow/LayerMapping.java b/core/src/main/java/org/wildfly/glow/LayerMapping.java index 50a72e59..cb8df679 100644 --- a/core/src/main/java/org/wildfly/glow/LayerMapping.java +++ b/core/src/main/java/org/wildfly/glow/LayerMapping.java @@ -64,6 +64,7 @@ public enum RULE { private final Map noConfigurationConditions = new HashMap<>(); private final Map hiddenConditions = new HashMap<>(); + private final Map familySelectionMode = new HashMap<>(); /** * @return the constantPoolClassInfos */ @@ -85,6 +86,13 @@ public Map getActiveProfilesLayers() { return activeProfilesLayers; } + /** + * @return the selectionModes + */ + public Map getFamilySelectionMode() { + return familySelectionMode; + } + /** * @return the allProfilesLayers */ diff --git a/core/src/main/java/org/wildfly/glow/LayerMetadata.java b/core/src/main/java/org/wildfly/glow/LayerMetadata.java index 430493ec..6138dc2a 100644 --- a/core/src/main/java/org/wildfly/glow/LayerMetadata.java +++ b/core/src/main/java/org/wildfly/glow/LayerMetadata.java @@ -38,6 +38,7 @@ public abstract class LayerMetadata { public static final String CLASS = PREFIX + "class"; public static final String CONFIGURATION = PREFIX + "configuration"; public static final String EXPECT_ADD_ON_FAMILY = PREFIX + "expect-add-on-family"; + public static final String SELECT_ADD_ON_FAMILY = PREFIX + "select-add-on-family-"; public static final String EXPECTED_FILE = PREFIX + "expected-file"; public static final String HIDDEN_IF = PREFIX + "hidden-if"; public static final String INCLUSION_MODE = PREFIX + "inclusion-mode"; @@ -72,6 +73,7 @@ public abstract class LayerMetadata { RULES_WITH_SUFFIX.add(PROFILE); RULES_WITH_SUFFIX.add(PROPERTIES_FILE_MATCH); RULES_WITH_SUFFIX.add(XML_PATH); + RULES_WITH_SUFFIX.add(SELECT_ADD_ON_FAMILY); CONDITION_RULES.add(HIDDEN_IF); CONDITION_RULES.add(NO_CONFIGURATION_IF); diff --git a/core/src/main/java/org/wildfly/glow/Utils.java b/core/src/main/java/org/wildfly/glow/Utils.java index 28d21691..9f1c59e0 100644 --- a/core/src/main/java/org/wildfly/glow/Utils.java +++ b/core/src/main/java/org/wildfly/glow/Utils.java @@ -403,7 +403,7 @@ public static boolean isPattern(String s) { return s.contains("*"); } - public static LayerMapping buildMapping(Map layers, Set profiles) { + public static LayerMapping buildMapping(Map layers, Set profiles) throws Exception { LayerMapping mapping = new LayerMapping(); for (Layer l : layers.values()) { for (String k : l.getProperties().keySet()) { @@ -455,6 +455,18 @@ public static LayerMapping buildMapping(Map layers, Set p l.setExpectFamily(l.getProperties().get(k)); continue; } + if (k.startsWith(LayerMetadata.SELECT_ADD_ON_FAMILY)) { + int i = LayerMetadata.SELECT_ADD_ON_FAMILY.length(); + String family = k.substring(i); + AddOnSelectionMode mode = mapping.getFamilySelectionMode().get(family); + if (mode != null) { + throw new Exception("More than one layer defines a selection mode for the family " + + family + " : " + mode.getTargetLayer().getName() + " and " + l.getName()); + } + mode = new AddOnSelectionMode(l, family, Integer.parseInt(l.getProperties().get(k))); + mapping.getFamilySelectionMode().put(family, mode); + continue; + } if (LayerMetadata.ADD_ON.equals(k)) { String familyAndName = l.getProperties().get(k); String[] split = familyAndName.split(","); diff --git a/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f1-add-on/layer-spec.xml b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f1-add-on/layer-spec.xml new file mode 100644 index 00000000..70fdc677 --- /dev/null +++ b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f1-add-on/layer-spec.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f1-add-on2/layer-spec.xml b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f1-add-on2/layer-spec.xml new file mode 100644 index 00000000..564c113f --- /dev/null +++ b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f1-add-on2/layer-spec.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f2-add-on/layer-spec.xml b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f2-add-on/layer-spec.xml new file mode 100644 index 00000000..da9003bb --- /dev/null +++ b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f2-add-on/layer-spec.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f2-add-on2/layer-spec.xml b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f2-add-on2/layer-spec.xml new file mode 100644 index 00000000..1876f03f --- /dev/null +++ b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/f2-add-on2/layer-spec.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/select-family-consumer/layer-spec.xml b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/select-family-consumer/layer-spec.xml new file mode 100644 index 00000000..33df5ed7 --- /dev/null +++ b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/select-family-consumer/layer-spec.xml @@ -0,0 +1,10 @@ + + + + + + + \ No newline at end of file diff --git a/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/select-only-one-family-consumer/layer-spec.xml b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/select-only-one-family-consumer/layer-spec.xml new file mode 100644 index 00000000..437a5847 --- /dev/null +++ b/tests/test-feature-pack/galleon-pack/src/main/resources/layers/standalone/select-only-one-family-consumer/layer-spec.xml @@ -0,0 +1,10 @@ + + + + + + + \ No newline at end of file diff --git a/tests/test-feature-pack/layer-metadata-tests/src/test/java/org/wildfly/glow/rules/test/AbstractLayerMetaDataTestCase.java b/tests/test-feature-pack/layer-metadata-tests/src/test/java/org/wildfly/glow/rules/test/AbstractLayerMetaDataTestCase.java index 54050a6f..dc575859 100644 --- a/tests/test-feature-pack/layer-metadata-tests/src/test/java/org/wildfly/glow/rules/test/AbstractLayerMetaDataTestCase.java +++ b/tests/test-feature-pack/layer-metadata-tests/src/test/java/org/wildfly/glow/rules/test/AbstractLayerMetaDataTestCase.java @@ -130,6 +130,9 @@ protected static String createXmlElementWithContent(String content, String... pa return sb.toString(); } + protected void manualCheck() { + checkMethodCalled = true; + } protected Set checkLayersForArchive(Path archivePath, String...expectedLayers) { return checkLayersForArchive(archivePath, null, expectedLayers); } diff --git a/tests/test-feature-pack/layer-metadata-tests/src/test/java/org/wildfly/glow/rules/test/addon/select/family/SelectFamilyTestCase.java b/tests/test-feature-pack/layer-metadata-tests/src/test/java/org/wildfly/glow/rules/test/addon/select/family/SelectFamilyTestCase.java new file mode 100644 index 00000000..00d5580a --- /dev/null +++ b/tests/test-feature-pack/layer-metadata-tests/src/test/java/org/wildfly/glow/rules/test/addon/select/family/SelectFamilyTestCase.java @@ -0,0 +1,123 @@ +/* + * JBoss, Home of Professional Open Source. + * Copyright 2024 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.wildfly.glow.rules.test.addon.select.family; + +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.Assert; +import org.junit.Test; +import org.wildfly.glow.Arguments; +import org.wildfly.glow.GlowMessageWriter; +import org.wildfly.glow.GlowSession; +import org.wildfly.glow.ScanArguments; +import org.wildfly.glow.ScanResults; +import org.wildfly.glow.error.IdentifiedError; +import org.wildfly.glow.maven.MavenResolver; +import org.wildfly.glow.rules.test.AbstractLayerMetaDataTestCase; + +public class SelectFamilyTestCase extends AbstractLayerMetaDataTestCase { + + @Test + public void testError() throws Exception { + manualCheck(); + Set layers = new HashSet<>(); + layers.add("select-family-consumer"); + ScanArguments.Builder argumentsBuilder = Arguments.scanBuilder().setJndiLayers(layers); + Arguments arguments = argumentsBuilder.build(); + try (ScanResults scanResults = GlowSession.scan(MavenResolver.newMavenResolver(), arguments, GlowMessageWriter.DEFAULT)) { + List errors = scanResults.getErrorSession().getErrors(); + Assert.assertEquals(1, errors.size()); + IdentifiedError error = errors.get(0); + Assert.assertEquals("missing-add-on", error.getId()); + } + } + + @Test + public void testValid() throws Exception { + manualCheck(); + Set layers = new HashSet<>(); + layers.add("select-family-consumer"); + Set addOns = new HashSet<>(); + addOns.add("add-on1"); + ScanArguments.Builder argumentsBuilder = Arguments.scanBuilder().setJndiLayers(layers).setUserEnabledAddOns(addOns); + Arguments arguments = argumentsBuilder.build(); + try (ScanResults scanResults = GlowSession.scan(MavenResolver.newMavenResolver(), arguments, GlowMessageWriter.DEFAULT)) { + List errors = scanResults.getErrorSession().getErrors(); + Assert.assertEquals(0, errors.size()); + Set foundLayers = scanResults.getDiscoveredLayers().stream().map(l -> l.getName()).collect(Collectors.toSet()); + Assert.assertEquals(2, foundLayers.size()); + Assert.assertTrue(foundLayers.toString(), foundLayers.contains("select-family-consumer")); + Assert.assertTrue(foundLayers.toString(),foundLayers.contains("f1-add-on")); + + } + } + + @Test + public void testOnlyOneError() throws Exception { + manualCheck(); + Set layers = new HashSet<>(); + layers.add("select-only-one-family-consumer"); + ScanArguments.Builder argumentsBuilder = Arguments.scanBuilder().setJndiLayers(layers); + Arguments arguments = argumentsBuilder.build(); + try (ScanResults scanResults = GlowSession.scan(MavenResolver.newMavenResolver(), arguments, GlowMessageWriter.DEFAULT)) { + List errors = scanResults.getErrorSession().getErrors(); + Assert.assertEquals(1, errors.size()); + IdentifiedError error = errors.get(0); + Assert.assertEquals("missing-add-on", error.getId()); + } + } + + @Test + public void testOnlyOneValid() throws Exception { + manualCheck(); + Set layers = new HashSet<>(); + layers.add("select-only-one-family-consumer"); + Set addOns = new HashSet<>(); + addOns.add("f2-add-on"); + ScanArguments.Builder argumentsBuilder = Arguments.scanBuilder().setJndiLayers(layers).setUserEnabledAddOns(addOns); + Arguments arguments = argumentsBuilder.build(); + try (ScanResults scanResults = GlowSession.scan(MavenResolver.newMavenResolver(), arguments, GlowMessageWriter.DEFAULT)) { + List errors = scanResults.getErrorSession().getErrors(); + Assert.assertEquals(0, errors.size()); + Set foundLayers = scanResults.getDiscoveredLayers().stream().map(l -> l.getName()).collect(Collectors.toSet()); + Assert.assertEquals(2, foundLayers.size()); + Assert.assertTrue(foundLayers.toString(), foundLayers.contains("select-only-one-family-consumer")); + Assert.assertTrue(foundLayers.toString(),foundLayers.contains("f2-add-on")); + + } + } + + @Test + public void testOnlyOneErrorTooMuch() throws Exception { + manualCheck(); + Set layers = new HashSet<>(); + layers.add("select-only-one-family-consumer"); + Set addOns = new HashSet<>(); + addOns.add("f2-add-on"); + addOns.add("f2-add-on2"); + ScanArguments.Builder argumentsBuilder = Arguments.scanBuilder().setJndiLayers(layers).setUserEnabledAddOns(addOns); + Arguments arguments = argumentsBuilder.build(); + try (ScanResults scanResults = GlowSession.scan(MavenResolver.newMavenResolver(), arguments, GlowMessageWriter.DEFAULT)) { + List errors = scanResults.getErrorSession().getErrors(); + Assert.assertEquals(1, errors.size()); + IdentifiedError error = errors.get(0); + Assert.assertEquals("too-many-add-on", error.getId()); + } + } +}