From 017778df3e369217d806aa7e26f62e53acb6b365 Mon Sep 17 00:00:00 2001 From: Alexey Loubyansky Date: Mon, 25 Nov 2019 17:55:26 +0100 Subject: [PATCH] [GAL-300] ProvisioningLayout collecting updates may iterate over duplicates with maven coords --- .../galleon/layout/ProvisioningLayout.java | 77 ++++++++++++------- .../galleon/universe/FeaturePackLocation.java | 2 +- ...ForFpInstalledWithMavenCoordsTestCase.java | 4 +- 3 files changed, 52 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/jboss/galleon/layout/ProvisioningLayout.java b/core/src/main/java/org/jboss/galleon/layout/ProvisioningLayout.java index 5df372e28..93d3f1dfc 100644 --- a/core/src/main/java/org/jboss/galleon/layout/ProvisioningLayout.java +++ b/core/src/main/java/org/jboss/galleon/layout/ProvisioningLayout.java @@ -328,6 +328,7 @@ public void close() { private Set transitiveDeps; private Map> conflicts = Collections.emptyMap(); private Map featurePacks = new HashMap<>(); + private Map mavenProducers = null; private ArrayList ordered = new ArrayList<>(); private Map allPatches = Collections.emptyMap(); private Map> fpPatches = Collections.emptyMap(); @@ -407,7 +408,7 @@ private ProvisioningLayout(ProvisioningLayout o while(--i >= 0) { final O otherFp = other.ordered.get(i); final F fp = transformer.transform(otherFp); - featurePacks.put(fp.getFPID().getProducer(), fp); + registerFeaturePack(fp.getFPID().getProducer(), fp); ordered.add(fp); } Collections.reverse(ordered); @@ -773,9 +774,12 @@ public boolean hasFeaturePack(ProducerSpec producer) { } public F getFeaturePack(ProducerSpec producer) throws ProvisioningException { - final F p = featurePacks.get(producer); + F p = featurePacks.get(producer); if(p == null) { - throw new ProvisioningException(Errors.unknownFeaturePack(producer.getLocation().getFPID())); + p = mavenProducers == null ? null : mavenProducers.get(producer); + if (p == null) { + throw new ProvisioningException(Errors.unknownFeaturePack(producer.getLocation().getFPID())); + } } return p; } @@ -1040,26 +1044,24 @@ private void layout(FeaturePackDepsConfig config, Map branch transitiveDeps = new HashSet<>(); } for(FeaturePackConfig transitiveConfig : config.getTransitiveDeps()) { - FPID fpid = transitiveConfig.getLocation().getFPID(); + FeaturePackLocation fpl = transitiveConfig.getLocation(); if(transitiveConfig.hasPatches()) { addPatches(transitiveConfig); } - final FPID branchId = branch.get(fpid.getProducer()); + final FPID branchId = branch.get(fpl.getProducer()); if (branchId != null) { - if (!branchId.getChannel().getName().equals(fpid.getChannel().getName())) { - addConflict(fpid, branchId); + if (!branchId.getChannel().getName().equals(fpl.getChannel().getName())) { + addConflict(fpl.getFPID(), branchId); } continue; } - if(transitiveConfig.getLocation().isMavenCoordinates()) { - buildTracker.processing(transitiveConfig.getLocation().getFPID()); - fpid = layoutFactory.resolveFeaturePack(transitiveConfig.getLocation(), FeaturePackLayout.TRANSITIVE_DEP, fpFactory).getSpec().getFPID(); - buildTracker.processed(transitiveConfig.getLocation().getFPID()); - registerResolvedVersion(transitiveConfig.getLocation().getProducer(), fpid.getLocation()); + if(fpl.isMavenCoordinates()) { + fpl = resolveFeaturePack(fpl, FeaturePackLayout.TRANSITIVE_DEP).getSpec().getFPID().getLocation(); + registerResolvedVersion(transitiveConfig.getLocation().getProducer(), fpl); } - transitiveDeps.add(fpid.getProducer()); - branch.put(fpid.getProducer(), fpid); - added = CollectionUtils.add(added, fpid.getProducer()); + transitiveDeps.add(fpl.getProducer()); + branch.put(fpl.getProducer(), fpl.getFPID()); + added = CollectionUtils.add(added, fpl.getProducer()); } } @@ -1081,9 +1083,7 @@ private void layout(FeaturePackDepsConfig config, Map branch continue; } } - buildTracker.processing(fpl.getFPID()); - fp = layoutFactory.resolveFeaturePack(fpl, type, fpFactory); - buildTracker.processed(fpl.getFPID()); + fp = resolveFeaturePack(fpl, type); if(fpl.isMavenCoordinates()) { if(branchId == null) { branchId = branch.get(fp.getFPID().getProducer()); @@ -1092,19 +1092,22 @@ private void layout(FeaturePackDepsConfig config, Map branch F resolvedFp = featurePacks.get(resolvedFpl.getProducer()); if(resolvedFp != null) { converge(branchId, resolvedFpl.getFPID(), resolvedFp.getFPID()); - featurePacks.put(fpl.getProducer(), resolvedFp); + if(!fpl.equals(resolvedFpl)) { + registerMavenProducer(fpl.getProducer(), resolvedFp); + } continue; - } else if(branchId != null) { - buildTracker.processing(resolvedFpl.getFPID()); - fp = layoutFactory.resolveFeaturePack(resolvedFpl, type, fpFactory); - buildTracker.processed(resolvedFpl.getFPID()); - } else if (!fpl.equals(resolvedFpl)) { - registerResolvedVersion(fpl.getProducer(), resolvedFpl); } - featurePacks.put(fpl.getProducer(), fp); - fpl = resolvedFpl; + if (!fpl.equals(resolvedFpl)) { + if (branchId != null) { + fp = resolveFeaturePack(resolvedFpl, type); + } else { + registerResolvedVersion(fpl.getProducer(), resolvedFpl); + } + registerMavenProducer(fpl.getProducer(), fp); + fpl = resolvedFpl; + } } - featurePacks.put(fpl.getProducer(), fp); + registerFeaturePack(fpl.getProducer(), fp); queue.add(fp); @@ -1131,6 +1134,24 @@ private void layout(FeaturePackDepsConfig config, Map branch } } + private void registerFeaturePack(ProducerSpec producer, F f) { + featurePacks.put(producer, f); + } + + private void registerMavenProducer(ProducerSpec producer, F f) { + if(mavenProducers == null) { + mavenProducers = new HashMap<>(); + } + mavenProducers.put(producer, f); + } + + private F resolveFeaturePack(FeaturePackLocation fpl, int type) throws ProvisioningException { + buildTracker.processing(fpl.getFPID()); + final F fp = layoutFactory.resolveFeaturePack(fpl, type, fpFactory); + buildTracker.processed(fpl.getFPID()); + return fp; + } + protected FeaturePackLocation resolveVersion(FeaturePackLocation fpl, final FPID branchId) throws ProvisioningException { if(branchId == null) { return normalize(fpl); diff --git a/core/src/main/java/org/jboss/galleon/universe/FeaturePackLocation.java b/core/src/main/java/org/jboss/galleon/universe/FeaturePackLocation.java index 2709e7517..e1484d90e 100644 --- a/core/src/main/java/org/jboss/galleon/universe/FeaturePackLocation.java +++ b/core/src/main/java/org/jboss/galleon/universe/FeaturePackLocation.java @@ -453,7 +453,7 @@ public FeaturePackLocation replaceBuild(String build) { } public boolean isMavenCoordinates() { - return universeSpec != null && universeSpec.getLocation() == null && universeSpec.getFactory().equals(Constants.MAVEN); + return universeSpec != null && universeSpec.getLocation() == null && Constants.MAVEN.equals(universeSpec.getFactory()); } @Override diff --git a/testsuite/src/test/java/org/jboss/galleon/layout/update/test/UpdateDiscoveryForFpInstalledWithMavenCoordsTestCase.java b/testsuite/src/test/java/org/jboss/galleon/layout/update/test/UpdateDiscoveryForFpInstalledWithMavenCoordsTestCase.java index 1346759bb..e601f8fbd 100644 --- a/testsuite/src/test/java/org/jboss/galleon/layout/update/test/UpdateDiscoveryForFpInstalledWithMavenCoordsTestCase.java +++ b/testsuite/src/test/java/org/jboss/galleon/layout/update/test/UpdateDiscoveryForFpInstalledWithMavenCoordsTestCase.java @@ -144,9 +144,9 @@ protected ProvisioningConfig provisioningConfig() throws ProvisioningException { protected FeaturePackUpdatePlan[] expectedUpdatePlans() { return new FeaturePackUpdatePlan[] { FeaturePackUpdatePlan.request(prodC101).setNewLocation(prodC102).buildPlan(), - FeaturePackUpdatePlan.request(prodB101, true).setNewLocation(prodB102).buildPlan(), FeaturePackUpdatePlan.request(prodD100).setNewLocation(prodD102).buildPlan(), - FeaturePackUpdatePlan.request(prodA101, true).setNewLocation(prodA102).buildPlan() + FeaturePackUpdatePlan.request(prodA101, true).setNewLocation(prodA102).buildPlan(), + FeaturePackUpdatePlan.request(prodB101, true).setNewLocation(prodB102).buildPlan() }; } }