Fix #106: ISM Plugin is not functional (#133)

* Warn but don't fail on attempt to install duplicate bundle * Address style issue
opensecuritycontroller · Apr 11, 2017 · 6b4d2fb · 6b4d2fb
1 parent a787701
commit 6b4d2fb
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 12 deletions.
diff --git a/osc-installer/src/main/java/org/osc/core/server/installer/Artifact.java b/osc-installer/src/main/java/org/osc/core/server/installer/Artifact.java
@@ -22,6 +22,8 @@
 public interface Artifact {
 
 	String getName();
+
+	String getVersion();
 
 	/**
 	 * Returns the hash for the artifact content if available, otherwise

diff --git a/osc-installer/src/main/java/org/osc/core/server/installer/impl/ArtifactImpl.java b/osc-installer/src/main/java/org/osc/core/server/installer/impl/ArtifactImpl.java
@@ -22,11 +22,13 @@
 class ArtifactImpl implements Artifact {
 
 	private final String name;
+	private final String version;
 	private final String location;
 	private final Hash hash;
 
-	ArtifactImpl(String name, String location, Hash hash) {
-		if (location == null) {
+	ArtifactImpl(String name, String version, String location, Hash hash) {
+		this.version = version;
+        if (location == null) {
             throw new IllegalArgumentException("Artifact location may not be null");
         }
 		this.name = name;
@@ -38,6 +40,11 @@ class ArtifactImpl implements Artifact {
 	public String getName() {
 		return this.name;
 	}
+
+	@Override
+	public String getVersion() {
+	    return version;
+	}
 
 	@Override
 	public Hash getHash() {
@@ -51,7 +58,12 @@ public String getLocation() {
 
 	@Override
 	public String toString() {
-		return new StringBuilder().append(this.name).append(":").append(this.hash != null ? this.hash : "<no-hash>").append("@").append(this.location).toString();
+		return new StringBuilder()
+		        .append(this.name)
+		        .append('/').append(this.version)
+		        .append(":").append(this.hash != null ? this.hash : "<no-hash>")
+		        .append("@").append(this.location)
+		        .toString();
 	}
 
 }
diff --git a/osc-installer/src/main/java/org/osc/core/server/installer/impl/DeploymentInstaller.java b/osc-installer/src/main/java/org/osc/core/server/installer/impl/DeploymentInstaller.java
@@ -65,6 +65,7 @@
 import org.osgi.framework.BundleListener;
 import org.osgi.framework.FrameworkEvent;
 import org.osgi.framework.FrameworkListener;
+import org.osgi.framework.Version;
 import org.osgi.framework.namespace.IdentityNamespace;
 import org.osgi.framework.wiring.BundleRevision;
 import org.osgi.resource.Capability;
@@ -278,6 +279,9 @@ private List<Bundle> installArtifacts(InstallableUnitImpl unit) {
                 oldState = unit.getState();
                 unit.setState(State.ERROR);
                 unit.setErrorMessage(e.getMessage());
+                if (log != null) {
+                    log.log(LogService.LOG_ERROR, "Error installing artifact(s)", e);
+                }
                 notifyListeners(Collections.singleton(new InstallableUnitEvent(oldState, State.ERROR, unit)));
                 return Collections.emptyList();
             }
@@ -347,7 +351,8 @@ private InstallableUnitImpl performResolve(File file) {
 
         List<Artifact> artifacts = new ArrayList<>(result.getResources().size());
         for (Entry<Resource, String> resourceEntry : result.getResources().entrySet()) {
-            ArtifactImpl artifact = new ArtifactImpl(getIdentity(resourceEntry.getKey()), resourceEntry.getValue(), getContentHash(resourceEntry.getKey()));
+            Capability idCap = getIdentityCapability(resourceEntry.getKey());
+            ArtifactImpl artifact = new ArtifactImpl(getIdentity(idCap), getVersion(idCap), resourceEntry.getValue(), getContentHash(resourceEntry.getKey()));
             artifacts.add(artifact);
         }
         debug("Sucessful resolve for file %s: Deployment-Name=%s, Deployment-SymbolicName=%s, Deployment-Version= %s, Deployment-Type=%s", file, request.getName(), request.getSymbolicName(), request.getVersion(), request.getType());
@@ -472,19 +477,33 @@ public void install(File file) {
         log(LogService.LOG_INFO, null, "Installing bundle archive: %s", file.getAbsolutePath());
         putResolveJob(file);
     }
+
+    private static String getIdentity(Capability identityCap) {
+        Object idObj = identityCap.getAttributes().get(IdentityNamespace.IDENTITY_NAMESPACE);
+        if (!(idObj instanceof String)) {
+            throw new IllegalArgumentException("Missing identity capability on resource, or incorrect type");
+        }
+
+        return (String) idObj;
+    }
+
+    private static String getVersion(Capability identityCap) {
+        Object versionObj = identityCap.getAttributes().get(IdentityNamespace.CAPABILITY_VERSION_ATTRIBUTE);
+        if (versionObj == null)
+            return Version.emptyVersion.toString();
+        if (versionObj instanceof Version)
+            return ((Version) versionObj).toString();
+        if (versionObj instanceof String)
+            return Version.parseVersion((String) versionObj).toString();
+        throw new IllegalArgumentException("Incorrect type on identity version");
+    }
 
-    private static String getIdentity(Resource resource) {
+    private static Capability getIdentityCapability(Resource resource) {
         List<Capability> caps = resource.getCapabilities(IdentityNamespace.IDENTITY_NAMESPACE);
         if (caps == null || caps.isEmpty()) {
             throw new IllegalArgumentException("Missing identity capability on resource");
         }
-
-        Object idObj = caps.get(0).getAttributes().get(IdentityNamespace.IDENTITY_NAMESPACE);
-        if (!(idObj instanceof String)) {
-            throw new IllegalArgumentException("Missing identity capability on resource, or incorrect type");
-        }
-
-        return (String) idObj;
+        return caps.get(0);
     }
 
     @Override

diff --git a/...staller/src/main/java/org/osc/core/server/installer/impl/FrameworkInstallerComponent.java b/...staller/src/main/java/org/osc/core/server/installer/impl/FrameworkInstallerComponent.java
@@ -91,6 +91,11 @@ public synchronized List<Bundle> addLocations(Object sponsor, List<String> bundl
 						Set<Object> sponsors = new HashSet<>();
 						sponsors.add(sponsor);
 						this.bundleSponsors.put(bundle.getBundleId(), sponsors);
+					} catch (BundleException e) {
+					    if (e.getType() == BundleException.DUPLICATE_BUNDLE_ERROR)
+					        log.log(LogService.LOG_WARNING, "Duplicate bundle symbolic-name/version in install for location " + location, e);
+					    else
+					        throw e;
 					}
 				} catch (URISyntaxException e) {
 					throw new IOException("Invalid bundle location URI: " + location, e);