You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2018/07/25 14:57:28 UTC

[sling-org-apache-sling-feature] branch master updated: SLING-7783 : Unable to remove bundles without specifying a version

This is an automated email from the ASF dual-hosted git repository.

cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature.git


The following commit(s) were added to refs/heads/master by this push:
     new 67be682  SLING-7783 : Unable to remove bundles without specifying a version
67be682 is described below

commit 67be6821193366288ee995db37b4787ce1eccede
Author: Carsten Ziegeler <cz...@adobe.com>
AuthorDate: Wed Jul 25 16:57:22 2018 +0200

    SLING-7783 : Unable to remove bundles without specifying a version
---
 .../java/org/apache/sling/feature/Feature.java     |  10 +-
 .../sling/feature/builder/FeatureBuilder.java      |  56 +++++-
 .../sling/feature/builder/FeatureBuilderTest.java  | 205 +++++++++++++++++++++
 3 files changed, 260 insertions(+), 11 deletions(-)

diff --git a/src/main/java/org/apache/sling/feature/Feature.java b/src/main/java/org/apache/sling/feature/Feature.java
index 98f1e0b..d9c7627 100644
--- a/src/main/java/org/apache/sling/feature/Feature.java
+++ b/src/main/java/org/apache/sling/feature/Feature.java
@@ -16,15 +16,15 @@
  */
 package org.apache.sling.feature;
 
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
+
 import org.apache.felix.utils.resource.CapabilityImpl;
 import org.apache.felix.utils.resource.RequirementImpl;
 import org.osgi.resource.Capability;
 import org.osgi.resource.Requirement;
 
-import java.util.ArrayList;
-import java.util.Enumeration;
-import java.util.List;
-
 /**
  * A feature consists of
  * <ul>
@@ -332,7 +332,7 @@ public class Feature implements Comparable<Feature> {
             c.getConfigurationRemovals().addAll(i.getConfigurationRemovals());
             c.getExtensionRemovals().addAll(i.getExtensionRemovals());
             c.getFrameworkPropertiesRemovals().addAll(i.getFrameworkPropertiesRemovals());
-            c.getArtifactExtensionRemovals().putAll(c.getArtifactExtensionRemovals());
+            c.getArtifactExtensionRemovals().putAll(i.getArtifactExtensionRemovals());
 
             result.getIncludes().add(c);
         }
diff --git a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
index ca9e38c..2977c8d 100644
--- a/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
+++ b/src/main/java/org/apache/sling/feature/builder/FeatureBuilder.java
@@ -29,6 +29,7 @@ import org.apache.sling.feature.Configuration;
 import org.apache.sling.feature.Extension;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.Include;
+import org.osgi.framework.Version;
 
 public class FeatureBuilder {
 
@@ -218,12 +219,33 @@ public class FeatureBuilder {
         // process removals
         // bundles
         for(final ArtifactId a : i.getBundleRemovals()) {
-            base.getBundles().removeExact(a);
+            boolean removed = false;
+            final boolean ignoreVersion = a.getOSGiVersion().equals(Version.emptyVersion);
+            if ( ignoreVersion ) {
+                // remove any version of that bundle
+                while (base.getBundles().removeSame(a)) {
+                    // continue to remove
+                    removed = true;
+                }
+            } else {
+                // remove exact version
+                removed = base.getBundles().removeExact(a);
+            }
+            if ( !removed ) {
+                throw new IllegalStateException("Bundle " + a + " can't be removed from feature " + base.getId() + " as it is not part of that feature.");
+            }
             final Iterator<Configuration> iter = base.getConfigurations().iterator();
             while ( iter.hasNext() ) {
                 final Configuration cfg = iter.next();
                 final String bundleId = (String)cfg.getProperties().get(Configuration.PROP_ARTIFACT);
-                if ( a.toMvnId().equals(bundleId) ) {
+                final ArtifactId bundleArtifactId = ArtifactId.fromMvnId(bundleId);
+                boolean remove = false;
+                if ( ignoreVersion ) {
+                    remove = bundleArtifactId.isSame(a);
+                } else {
+                    remove = bundleArtifactId.equals(a);
+                }
+                if (  remove) {
                     iter.remove();
                 }
             }
@@ -271,13 +293,35 @@ public class FeatureBuilder {
         for(final Map.Entry<String, List<ArtifactId>> entry : i.getArtifactExtensionRemovals().entrySet()) {
             for(final Extension ext : base.getExtensions()) {
                 if ( ext.getName().equals(entry.getKey()) ) {
-                    for(final ArtifactId id : entry.getValue() ) {
-                        for(final Artifact a : ext.getArtifacts()) {
-                            if ( a.getId().equals(id) ) {
-                                ext.getArtifacts().remove(a);
+                    for(final ArtifactId toRemove : entry.getValue() ) {
+                        boolean removed = false;
+                        final boolean ignoreVersion = toRemove.getOSGiVersion().equals(Version.emptyVersion);
+                        final Iterator<Artifact> iter = ext.getArtifacts().iterator();
+                        while ( iter.hasNext() ) {
+                            final Artifact a = iter.next();
+
+                            boolean remove = false;
+                            if ( ignoreVersion ) {
+                                // remove any version of that bundle
+                                if ( a.getId().isSame(toRemove) ) {
+                                    remove = true;
+                                }
+                            } else {
+                                // remove exact version
+
+                                remove = a.getId().equals(toRemove);
+                            }
+                            if ( remove ) {
+                                iter.remove();
+                                removed = true;
+                            }
+                            if ( remove && !ignoreVersion ) {
                                 break;
                             }
                         }
+                        if ( !removed ) {
+                            throw new IllegalStateException("Artifact " + toRemove + " can't be removed from feature " + base.getId() + " as it is not part of that feature.");
+                        }
                     }
                     break;
                 }
diff --git a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
index 74122c3..3c8e76c 100644
--- a/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
+++ b/src/test/java/org/apache/sling/feature/builder/FeatureBuilderTest.java
@@ -20,12 +20,16 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.felix.utils.resource.CapabilityImpl;
 import org.apache.felix.utils.resource.RequirementImpl;
@@ -33,6 +37,7 @@ import org.apache.sling.feature.Artifact;
 import org.apache.sling.feature.ArtifactId;
 import org.apache.sling.feature.Configuration;
 import org.apache.sling.feature.Extension;
+import org.apache.sling.feature.ExtensionType;
 import org.apache.sling.feature.Feature;
 import org.apache.sling.feature.Include;
 import org.junit.Test;
@@ -346,4 +351,204 @@ public class FeatureBuilderTest {
         assertEquals(1, features.length);
         assertEquals(idB, features[0].getId());
     }
+
+    @Test public void testBundleRemoveWithExactVersion() throws Exception {
+        final ArtifactId bundleA1 = ArtifactId.fromMvnId("g:a:1.0.0");
+        final ArtifactId bundleA2 = ArtifactId.fromMvnId("g:a:1.1.0");
+        final ArtifactId bundleB = ArtifactId.fromMvnId("g:b:1.1.0");
+
+        final Feature a = new Feature(ArtifactId.fromMvnId("g:a-base:1"));
+        a.getBundles().add(new Artifact(bundleA1));
+        a.getBundles().add(new Artifact(bundleA2));
+        a.getBundles().add(new Artifact(bundleB));
+        final Feature b = new Feature(ArtifactId.fromMvnId("g:a-include:1"));
+        final Include inc = new Include(a.getId());
+        inc.getBundleRemovals().add(bundleA2);
+        b.getIncludes().add(inc);
+
+        // assemble feature include
+        Feature feature = FeatureBuilder.assemble(b, new BuilderContext(new FeatureProvider() {
+
+            @Override
+            public Feature provide(ArtifactId id) {
+                if ( id.equals(a.getId()) ) {
+                    return a;
+                }
+                return null;
+            }
+        }));
+        final Set<ArtifactId> set = new HashSet<>();
+        for(final Artifact c : feature.getBundles()) {
+            set.add(c.getId());
+        }
+        assertEquals(2, set.size());
+        assertTrue(set.contains(bundleA1));
+        assertTrue(set.contains(bundleB));
+    }
+
+    @Test public void testBundleRemoveWithAnyVersion() throws Exception {
+        final ArtifactId bundleA1 = ArtifactId.fromMvnId("g:a:1.0.0");
+        final ArtifactId bundleA2 = ArtifactId.fromMvnId("g:a:1.1.0");
+        final ArtifactId bundleB = ArtifactId.fromMvnId("g:b:1.1.0");
+
+        final Feature a = new Feature(ArtifactId.fromMvnId("g:a-base:1"));
+        a.getBundles().add(new Artifact(bundleA1));
+        a.getBundles().add(new Artifact(bundleA2));
+        a.getBundles().add(new Artifact(bundleB));
+        final Feature b = new Feature(ArtifactId.fromMvnId("g:a-include:1"));
+        final Include inc = new Include(a.getId());
+        inc.getBundleRemovals().add(ArtifactId.fromMvnId("g:a:0.0.0"));
+        b.getIncludes().add(inc);
+
+        // assemble feature include
+        Feature feature = FeatureBuilder.assemble(b, new BuilderContext(new FeatureProvider() {
+
+            @Override
+            public Feature provide(ArtifactId id) {
+                if ( id.equals(a.getId()) ) {
+                    return a;
+                }
+                return null;
+            }
+        }));
+        final Set<ArtifactId> set = new HashSet<>();
+        for(final Artifact c : feature.getBundles()) {
+            set.add(c.getId());
+        }
+        assertEquals(1, set.size());
+        assertTrue(set.contains(bundleB));
+    }
+
+    @Test public void testBundleRemoveNoMatch() throws Exception {
+        final ArtifactId bundleA1 = ArtifactId.fromMvnId("g:a:1.0.0");
+        final ArtifactId bundleA2 = ArtifactId.fromMvnId("g:a:1.1.0");
+        final ArtifactId bundleB = ArtifactId.fromMvnId("g:b:1.1.0");
+
+        final Feature a = new Feature(ArtifactId.fromMvnId("g:a-base:1"));
+        a.getBundles().add(new Artifact(bundleA1));
+        a.getBundles().add(new Artifact(bundleB));
+        final Feature b = new Feature(ArtifactId.fromMvnId("g:a-include:1"));
+        final Include inc = new Include(a.getId());
+        inc.getBundleRemovals().add(bundleA2);
+        b.getIncludes().add(inc);
+
+        try {
+            FeatureBuilder.assemble(b, new BuilderContext(new FeatureProvider() {
+
+                @Override
+                public Feature provide(ArtifactId id) {
+                    if ( id.equals(a.getId()) ) {
+                        return a;
+                    }
+                    return null;
+                }
+            }));
+            fail();
+        } catch ( final IllegalStateException ise) {
+            // expected
+        }
+    }
+
+    @Test public void testExtensionArtifactRemoveWithExactVersion() throws Exception {
+        final ArtifactId bundleA1 = ArtifactId.fromMvnId("g:a:1.0.0");
+        final ArtifactId bundleA2 = ArtifactId.fromMvnId("g:a:1.1.0");
+        final ArtifactId bundleB = ArtifactId.fromMvnId("g:b:1.1.0");
+
+        final Feature a = new Feature(ArtifactId.fromMvnId("g:a-base:1"));
+        final Extension e = new Extension(ExtensionType.ARTIFACTS, "foo", false);
+        e.getArtifacts().add(new Artifact(bundleA1));
+        e.getArtifacts().add(new Artifact(bundleA2));
+        e.getArtifacts().add(new Artifact(bundleB));
+        a.getExtensions().add(e);
+        final Feature b = new Feature(ArtifactId.fromMvnId("g:a-include:1"));
+        final Include inc = new Include(a.getId());
+        inc.getArtifactExtensionRemovals().put("foo", Arrays.asList(bundleA2));
+        b.getIncludes().add(inc);
+
+        // assemble feature include
+        Feature feature = FeatureBuilder.assemble(b, new BuilderContext(new FeatureProvider() {
+
+            @Override
+            public Feature provide(ArtifactId id) {
+                if ( id.equals(a.getId()) ) {
+                    return a;
+                }
+                return null;
+            }
+        }));
+        final Set<ArtifactId> set = new HashSet<>();
+        for(final Artifact c : feature.getExtensions().getByName("foo").getArtifacts()) {
+            set.add(c.getId());
+        }
+        assertEquals(2, set.size());
+        assertTrue(set.contains(bundleA1));
+        assertTrue(set.contains(bundleB));
+    }
+
+    @Test public void testExtensionArtifactRemoveWithAnyVersion() throws Exception {
+        final ArtifactId bundleA1 = ArtifactId.fromMvnId("g:a:1.0.0");
+        final ArtifactId bundleA2 = ArtifactId.fromMvnId("g:a:1.1.0");
+        final ArtifactId bundleB = ArtifactId.fromMvnId("g:b:1.1.0");
+
+        final Feature a = new Feature(ArtifactId.fromMvnId("g:a-base:1"));
+        final Extension e = new Extension(ExtensionType.ARTIFACTS, "foo", false);
+        e.getArtifacts().add(new Artifact(bundleA1));
+        e.getArtifacts().add(new Artifact(bundleA2));
+        e.getArtifacts().add(new Artifact(bundleB));
+        a.getExtensions().add(e);
+        final Feature b = new Feature(ArtifactId.fromMvnId("g:a-include:1"));
+        final Include inc = new Include(a.getId());
+        inc.getArtifactExtensionRemovals().put("foo", Arrays.asList(ArtifactId.fromMvnId("g:a:0.0.0")));
+        b.getIncludes().add(inc);
+
+        // assemble feature include
+        Feature feature = FeatureBuilder.assemble(b, new BuilderContext(new FeatureProvider() {
+
+            @Override
+            public Feature provide(ArtifactId id) {
+                if ( id.equals(a.getId()) ) {
+                    return a;
+                }
+                return null;
+            }
+        }));
+        final Set<ArtifactId> set = new HashSet<>();
+        for(final Artifact c : feature.getExtensions().getByName("foo").getArtifacts()) {
+            set.add(c.getId());
+        }
+        assertEquals(1, set.size());
+        assertTrue(set.contains(bundleB));
+    }
+
+    @Test public void testExtensionArtifactRemoveNoMatch() throws Exception {
+        final ArtifactId bundleA1 = ArtifactId.fromMvnId("g:a:1.0.0");
+        final ArtifactId bundleA2 = ArtifactId.fromMvnId("g:a:1.1.0");
+        final ArtifactId bundleB = ArtifactId.fromMvnId("g:b:1.1.0");
+
+        final Feature a = new Feature(ArtifactId.fromMvnId("g:a-base:1"));
+        final Extension e = new Extension(ExtensionType.ARTIFACTS, "foo", false);
+        e.getArtifacts().add(new Artifact(bundleA1));
+        e.getArtifacts().add(new Artifact(bundleB));
+        a.getExtensions().add(e);
+        final Feature b = new Feature(ArtifactId.fromMvnId("g:a-include:1"));
+        final Include inc = new Include(a.getId());
+        inc.getArtifactExtensionRemovals().put("foo", Arrays.asList(bundleA2));
+        b.getIncludes().add(inc);
+
+        try {
+            FeatureBuilder.assemble(b, new BuilderContext(new FeatureProvider() {
+
+                @Override
+                public Feature provide(ArtifactId id) {
+                    if ( id.equals(a.getId()) ) {
+                        return a;
+                    }
+                    return null;
+                }
+            }));
+            fail();
+        } catch ( final IllegalStateException ise) {
+            // expected
+        }
+    }
 }