bug 335190: PAR and Bundle bridges now fallback to file name to determine identity. Illegal version causes exceptionbug335190-bridge-identity

9 files changed, 208 insertions, 196 deletions

diff --git a/org.eclipse.virgo.kernel.artifact/src/main/java/org/eclipse/virgo/kernel/artifact/bundle/BundleBridge.java b/org.eclipse.virgo.kernel.artifact/src/main/java/org/eclipse/virgo/kernel/artifact/bundle/BundleBridge.java
index d7298b8d..9106e42d 100644
--- a/org.eclipse.virgo.kernel.artifact/src/main/java/org/eclipse/virgo/kernel/artifact/bundle/BundleBridge.java
+++ b/org.eclipse.virgo.kernel.artifact/src/main/java/org/eclipse/virgo/kernel/artifact/bundle/BundleBridge.java
@@ -40,9 +40,9 @@ import org.eclipse.virgo.util.osgi.manifest.FragmentHost.Extension;
  * either as a JAR file, or a directory.
  * <p />
  * <strong>Concurrent Semantics</strong><br />
- * 
+ *
  * This class is thread-safe
- * 
+ *
  */
 public final class BundleBridge implements ArtifactBridge {
 
@@ -53,9 +53,9 @@ public final class BundleBridge implements ArtifactBridge {
     public static final String RAW_HEADER_PREFIX = "RAW_HEADER:";
 
     public static final String BRIDGE_TYPE = "bundle";
-    
+
     private final HashGenerator hashGenerator;
-    
+
     public BundleBridge(HashGenerator hashGenerator) {
         this.hashGenerator = hashGenerator;
     }
@@ -72,17 +72,47 @@ public final class BundleBridge implements ArtifactBridge {
         BundleManifest bundleManifest;
 
         try {
-            bundleManifest = BundleManifestUtils.readBundleManifest(artifactFile, JAR_SUFFIX, WAR_SUFFIX);            
+            bundleManifest = BundleManifestUtils.readBundleManifest(artifactFile, JAR_SUFFIX, WAR_SUFFIX);
         } catch (RuntimeException re) {
             throw new RuntimeException(String.format("Error occurred while parsing the manifest of file '%s'.", artifactFile.getPath()),  re);
         } catch (Exception e) {
             throw new ArtifactGenerationException("Error occurred while parsing the manifest.", BRIDGE_TYPE, e);
         }
-        
-        if (bundleManifest == null) {
-            return null;
+
+        ArtifactDescriptor descriptor = null;
+
+        if (bundleManifest != null) {
+            descriptor = createArtifactDescriptorFromManifest(artifactFile, bundleManifest);
         }
 
+        if (descriptor == null) {
+            descriptor = createArtifactDescriptorFromFile(artifactFile);
+        }
+
+        return descriptor;
+    }
+
+    private ArtifactDescriptor createArtifactDescriptorFromFile(File artifactFile) {
+        String fileName = artifactFile.getName();
+
+        if (fileName.endsWith(JAR_SUFFIX) || fileName.endsWith(WAR_SUFFIX)) {
+            String name = fileName.substring(0, fileName.length() - JAR_SUFFIX.length());
+
+            ArtifactDescriptorBuilder artifactDescriptorBuilder = new ArtifactDescriptorBuilder();
+            artifactDescriptorBuilder.setUri(artifactFile.toURI());
+            artifactDescriptorBuilder.setName(name);
+            artifactDescriptorBuilder.setType(BRIDGE_TYPE);
+            artifactDescriptorBuilder.setVersion(Version.emptyVersion);
+
+            this.hashGenerator.generateHash(artifactDescriptorBuilder, artifactFile);
+
+            return artifactDescriptorBuilder.build();
+        }
+
+        return null;
+    }
+
+    private ArtifactDescriptor createArtifactDescriptorFromManifest(File artifactFile, BundleManifest bundleManifest) throws ArtifactGenerationException {
         try {
             ArtifactDescriptorBuilder artifactDescriptorBuilder = new ArtifactDescriptorBuilder();
 
diff --git a/org.eclipse.virgo.kernel.artifact/src/main/java/org/eclipse/virgo/kernel/artifact/par/ParBridge.java b/org.eclipse.virgo.kernel.artifact/src/main/java/org/eclipse/virgo/kernel/artifact/par/ParBridge.java
index 0219e9cc..35791e57 100644
--- a/org.eclipse.virgo.kernel.artifact/src/main/java/org/eclipse/virgo/kernel/artifact/par/ParBridge.java
+++ b/org.eclipse.virgo.kernel.artifact/src/main/java/org/eclipse/virgo/kernel/artifact/par/ParBridge.java
@@ -25,16 +25,17 @@ import org.eclipse.virgo.repository.builder.ArtifactDescriptorBuilder;
 import org.eclipse.virgo.repository.builder.AttributeBuilder;
 import org.eclipse.virgo.util.common.StringUtils;
 import org.eclipse.virgo.util.osgi.manifest.BundleManifest;
+import org.osgi.framework.Version;
 
 /**
  * An <code>ArtifactBridge</code> for PAR files.
- * 
+ *
  * <p />
- * 
+ *
  * <strong>Concurrent Semantics</strong><br />
- * 
+ *
  * Thread-safe.
- * 
+ *
  */
 public final class ParBridge implements ArtifactBridge {
 
@@ -50,6 +51,8 @@ public final class ParBridge implements ArtifactBridge {
 
     public static final String BRIDGE_TYPE = "par";
 
+    public static final String PAR_SUFFIX = ".par";
+
     private final HashGenerator hashGenerator;
 
     public ParBridge(HashGenerator hashGenerator) {
@@ -61,7 +64,7 @@ public final class ParBridge implements ArtifactBridge {
         BundleManifest manifest;
 
         try {
-            manifest = BundleManifestUtils.readBundleManifest(artifactFile, ".par");
+            manifest = BundleManifestUtils.readBundleManifest(artifactFile, PAR_SUFFIX);
         } catch (IOException ioe) {
             throw new ArtifactGenerationException("Failed to read manifest from " + artifactFile, ioe);
         }
@@ -69,8 +72,25 @@ public final class ParBridge implements ArtifactBridge {
         if (manifest != null) {
             return createDescriptorFromManifest(manifest, artifactFile);
         } else {
-            return null;
+            return createDescriptorFromFile(artifactFile);
+        }
+    }
+
+    private ArtifactDescriptor createDescriptorFromFile(File artifactFile) throws ArtifactGenerationException {
+        String fileName = artifactFile.getName();
+
+        if (fileName.endsWith(PAR_SUFFIX)) {
+            String symbolicName = fileName.substring(0, fileName.length() - PAR_SUFFIX.length());
+
+            ArtifactDescriptorBuilder builder = new ArtifactDescriptorBuilder();
+            builder.setType(BRIDGE_TYPE).setName(symbolicName).setVersion(Version.emptyVersion).setUri(artifactFile.toURI());
+
+            this.hashGenerator.generateHash(builder, artifactFile);
+
+            return builder.build();
         }
+
+        return null;
     }
 
     private ArtifactDescriptor createDescriptorFromManifest(BundleManifest manifest, File artifactFile) throws ArtifactGenerationException {
@@ -88,7 +108,7 @@ public final class ParBridge implements ArtifactBridge {
 
         applyAttributeIfPresent(HEADER_APPLICATION_NAME, manifest, builder);
         applyAttributeIfPresent(HEADER_APPLICATION_DESCRIPTION, manifest, builder);
-        
+
         this.hashGenerator.generateHash(builder, artifactFile);
 
         return builder.build();
@@ -102,7 +122,7 @@ public final class ParBridge implements ArtifactBridge {
         }
     }
 
-    private Version getApplicationVersion(BundleManifest manifest) throws ArtifactGenerationException {
+    private Version getApplicationVersion(BundleManifest manifest) {
         String versionString = manifest.getHeader(HEADER_APPLICATION_VERSION);
         Version version;
 
@@ -112,7 +132,7 @@ public final class ParBridge implements ArtifactBridge {
             try {
                 version = new Version(versionString);
             } catch (IllegalArgumentException iae) {
-                throw new ArtifactGenerationException("Version '" + versionString + "' is ill-formed", iae);
+                throw new IllegalArgumentException("Version '" + versionString + "' is ill-formed", iae);
             }
         }
         return version;
diff --git a/org.eclipse.virgo.kernel.artifact/src/test/java/org/eclipse/virgo/kernel/artifact/bundle/BundleBridgeTests.java b/org.eclipse.virgo.kernel.artifact/src/test/java/org/eclipse/virgo/kernel/artifact/bundle/BundleBridgeTests.java
index 8d606dba..ef06b0a2 100644
--- a/org.eclipse.virgo.kernel.artifact/src/test/java/org/eclipse/virgo/kernel/artifact/bundle/BundleBridgeTests.java
+++ b/org.eclipse.virgo.kernel.artifact/src/test/java/org/eclipse/virgo/kernel/artifact/bundle/BundleBridgeTests.java
@@ -37,21 +37,21 @@ import org.osgi.framework.Version;
  * Unit tests for {@link org.eclipse.virgo.kernel.artifact.bundle.BundleBridge BundleBridge}. Uses a combination of real bundle files and
  * static test data.
  * </p>
- * 
+ *
  * <strong>Concurrent Semantics</strong><br />
- * 
+ *
  * Threadsafe test case
- * 
+ *
  */
 public class BundleBridgeTests {
-    
+
     //Test Data
-    
+
     private final static String ARTEFACT_ATTRIBUTE_NAME = "name";
 
     private final static String ARTEFACT_ATTRIBUTE_VERSION = "version";
     //End Test Data
-    
+
     private static BundleBridge BUNDLE_BRIDGE;
 
     private static final StubBundleArtefactBridge STUB_ARTEFACT_DEFINITION = new StubBundleArtefactBridge();
@@ -162,7 +162,7 @@ public class BundleBridgeTests {
     public void webBundleWar() throws ArtifactGenerationException {
         ArtifactDescriptor descriptor = BUNDLE_BRIDGE.generateArtifactDescriptor(new File("src/test/resources/wars/test.war"));
         assertNotNull(descriptor);
-        assertEquals("bundle", descriptor.getType());
+        assertEquals(BundleBridge.BRIDGE_TYPE, descriptor.getType());
         assertEquals("com.springsource.server.admin.web", descriptor.getName());
         assertEquals(new Version(2, 0, 0), descriptor.getVersion());
     }
@@ -171,14 +171,32 @@ public class BundleBridgeTests {
     public void explodedBundle() throws ArtifactGenerationException {
         ArtifactDescriptor descriptor = BUNDLE_BRIDGE.generateArtifactDescriptor(new File("src/test/resources/bundle.jar"));
         assertNotNull(descriptor);
-        assertEquals("bundle", descriptor.getType());
+        assertEquals(BundleBridge.BRIDGE_TYPE, descriptor.getType());
         assertEquals("exploded.bundle", descriptor.getName());
         assertEquals(new Version(1, 0, 0), descriptor.getVersion());
     }
 
+    @Test
+    public void noSymbolicName() throws ArtifactGenerationException {
+        ArtifactDescriptor descriptor = BUNDLE_BRIDGE.generateArtifactDescriptor(new File("src/test/resources/jars/no-symbolic-name.jar"));
+        assertNotNull(descriptor);
+        assertEquals(BundleBridge.BRIDGE_TYPE, descriptor.getType());
+        assertEquals("no-symbolic-name", descriptor.getName());
+        assertEquals(new Version(0, 0, 0), descriptor.getVersion());
+    }
+
+    @Test
+    public void noManifest() throws ArtifactGenerationException {
+        ArtifactDescriptor descriptor = BUNDLE_BRIDGE.generateArtifactDescriptor(new File("src/test/resources/jars/no-manifest.jar"));
+        assertNotNull(descriptor);
+        assertEquals(BundleBridge.BRIDGE_TYPE, descriptor.getType());
+        assertEquals("no-manifest", descriptor.getName());
+        assertEquals(Version.emptyVersion, descriptor.getVersion());
+    }
+
     private Set<ArtifactDescriptor> generateArtefacts(File directory) throws ArtifactGenerationException {
         Set<ArtifactDescriptor> artefacts = new HashSet<ArtifactDescriptor>();
-        
+
         for (File fileInDir : directory.listFiles()) {
             if(!fileInDir.getName().endsWith(".jar") && !fileInDir.getName().contains("sources")){
                 ArtifactDescriptor artefact = BUNDLE_BRIDGE.generateArtifactDescriptor(fileInDir);
diff --git a/org.eclipse.virgo.kernel.artifact/src/test/java/org/eclipse/virgo/kernel/artifact/par/ParBridgeTests.java b/org.eclipse.virgo.kernel.artifact/src/test/java/org/eclipse/virgo/kernel/artifact/par/ParBridgeTests.java
index 459889f3..8a47a352 100644
--- a/org.eclipse.virgo.kernel.artifact/src/test/java/org/eclipse/virgo/kernel/artifact/par/ParBridgeTests.java
+++ b/org.eclipse.virgo.kernel.artifact/src/test/java/org/eclipse/virgo/kernel/artifact/par/ParBridgeTests.java
@@ -22,56 +22,51 @@ import java.io.File;
 import org.junit.Test;
 import org.osgi.framework.Version;
 
-
 import org.eclipse.virgo.kernel.artifact.StubHashGenerator;
 import org.eclipse.virgo.kernel.artifact.par.ParBridge;
 import org.eclipse.virgo.repository.ArtifactDescriptor;
 import org.eclipse.virgo.repository.ArtifactGenerationException;
 
-
-/**
- *
- */
 public class ParBridgeTests {
-    
+
     private final ParBridge parBridge = new ParBridge(new StubHashGenerator());
-    
+
     @Test
     public void descriptorGeneration() throws ArtifactGenerationException {
         ArtifactDescriptor descriptor = this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/basic.par"));
         assertDescriptor(descriptor, "par", "basic", new Version(1,2,3), "Basic Par", "A basic PAR file for the unit tests");
     }
-    
+
     @Test
     public void generationWithNoManifest() throws ArtifactGenerationException {
         ArtifactDescriptor descriptor = this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/no-manifest.par"));
-        assertNull(descriptor);
+        assertDescriptor(descriptor, "par", "no-manifest", Version.emptyVersion, null, null);
     }
-    
+
     @Test
     public void generationWithNoApplicationSymbolicName() throws ArtifactGenerationException {
         ArtifactDescriptor descriptor = this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/no-asn.par"));
         assertNull(descriptor);
     }
-    
+
     @Test
     public void generationWithNoApplicationVersion() throws ArtifactGenerationException {
         ArtifactDescriptor descriptor = this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/no-version.par"));
         assertDescriptor(descriptor, "par", "basic", Version.emptyVersion, "Basic Par", "A basic PAR file for the unit tests");
     }
-    
+
     @Test
     public void generationWithNoName() throws ArtifactGenerationException {
         ArtifactDescriptor descriptor = this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/no-name.par"));
         assertDescriptor(descriptor, "par", "basic", new Version(1,2,3), null, "A basic PAR file for the unit tests");
     }
-    
+
     @Test
     public void generationWithNoDescription() throws ArtifactGenerationException {
         ArtifactDescriptor descriptor = this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/no-description.par"));
         assertDescriptor(descriptor, "par", "basic", new Version(1,2,3), "Basic Par", null);
     }
-    
+
     @Test
     public void generationWithIllegalApplicationSymbolicName() throws Exception {
         try {
@@ -79,24 +74,24 @@ public class ParBridgeTests {
             fail("Illegal Application-SymbolicName did not throw an ArtifactGenerationException");
         } catch (ArtifactGenerationException age) {
             assertEquals("Application-SymbolicName '.@$%' contains illegal characters", age.getMessage());
-        }        
+        }
     }
-    
+
     @Test
     public void generationWithIllegalApplicationVersion() throws Exception {
         try {
             this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/illegal-version.par"));
-            fail("Illegal Application-Version did not throw an ArtifactGenerationException");
-        } catch (ArtifactGenerationException age) {
-            assertEquals("Version 'alpha' is ill-formed", age.getMessage());
-        }        
+            fail("Illegal Application-Version did not throw an IllegalArgumentException");
+        } catch (IllegalArgumentException iae) {
+            assertEquals("Version 'alpha' is ill-formed", iae.getMessage());
+        }
     }
-    
+
     @Test(expected=ArtifactGenerationException.class)
     public void generationWithMissingPar() throws ArtifactGenerationException {
-        this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/not-there.par"));   
+        this.parBridge.generateArtifactDescriptor(new File("src/test/resources/pars/not-there.par"));
     }
-    
+
     private void assertDescriptor(ArtifactDescriptor descriptor, String type, String symbolicName, Version version, String name, String description) {
         assertNotNull(descriptor);
         assertEquals(type, descriptor.getType());
@@ -107,7 +102,7 @@ public class ParBridgeTests {
         } else {
             assertTrue(descriptor.getAttribute("Application-Name").isEmpty());
         }
-        
+
         if (description != null) {
             assertEquals(description, descriptor.getAttribute("Application-Description").iterator().next().getValue());
         } else {
diff --git a/org.eclipse.virgo.kernel.artifact/src/test/resources/jars/no-manifest.jar b/org.eclipse.virgo.kernel.artifact/src/test/resources/jars/no-manifest.jar
new file mode 100644
index 00000000..f63054be
--- /dev/null
+++ b/org.eclipse.virgo.kernel.artifact/src/test/resources/jars/no-manifest.jar
diff --git a/org.eclipse.virgo.kernel.artifact/src/test/resources/jars/no-symbolic-name.jar b/org.eclipse.virgo.kernel.artifact/src/test/resources/jars/no-symbolic-name.jar
new file mode 100644
index 00000000..7ab5fb31
--- /dev/null
+++ b/org.eclipse.virgo.kernel.artifact/src/test/resources/jars/no-symbolic-name.jar
diff --git a/org.eclipse.virgo.kernel.deployer/src/main/java/org/eclipse/virgo/kernel/install/artifact/internal/StandardArtifactIdentityDeterminer.java b/org.eclipse.virgo.kernel.deployer/src/main/java/org/eclipse/virgo/kernel/install/artifact/internal/StandardArtifactIdentityDeterminer.java
index 2603feeb..91b1612f 100644
--- a/org.eclipse.virgo.kernel.deployer/src/main/java/org/eclipse/virgo/kernel/install/artifact/internal/StandardArtifactIdentityDeterminer.java
+++ b/org.eclipse.virgo.kernel.deployer/src/main/java/org/eclipse/virgo/kernel/install/artifact/internal/StandardArtifactIdentityDeterminer.java
@@ -1,20 +1,19 @@
 /*******************************************************************************
- * Copyright (c) 2008, 2010 VMware Inc.
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- *
- * Contributors:
- *   VMware Inc. - initial contribution
- *******************************************************************************/
+* Copyright (c) 2008, 2010 VMware Inc.
+* All rights reserved. This program and the accompanying materials
+* are made available under the terms of the Eclipse Public License v1.0
+* which accompanies this distribution, and is available at
+* http://www.eclipse.org/legal/epl-v10.html
+*
+* Contributors:
+*   VMware Inc. - initial contribution
+*******************************************************************************/
 
 package org.eclipse.virgo.kernel.install.artifact.internal;
 
 import java.io.File;
 import java.util.Set;
 
-import org.osgi.framework.Version;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -25,95 +24,44 @@ import org.eclipse.virgo.repository.ArtifactDescriptor;
 import org.eclipse.virgo.repository.ArtifactGenerationException;
 
 /**
- * {@link StandardArtifactIdentityDeterminer} is a {@link ArtifactIdentityDeterminer} that can determine basic kernel artifact
- * identities.
- * <p />
- * 
- * <strong>Concurrent Semantics</strong><br />
- * 
- * This class is thread safe.
- * 
- */
+* {@link StandardArtifactIdentityDeterminer} is a {@link ArtifactIdentityDeterminer} that can determine basic kernel artifact
+* identities.
+* <p />
+*
+* <strong>Concurrent Semantics</strong><br />
+*
+* This class is thread safe.
+*
+*/
 final class StandardArtifactIdentityDeterminer implements ArtifactIdentityDeterminer {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(StandardArtifactIdentityDeterminer.class);
 
-    private static final String JAR_EXTENSION = ".jar";
-
-    private static final String PLAN_EXTENSION = ".plan";
-
-    private static final String PROPERTIES_EXTENSION = ".properties";
-
-    private static final String PAR_EXTENSION = ".par";
-
     private final Set<ArtifactBridge> bridges;
 
     public StandardArtifactIdentityDeterminer(Set<ArtifactBridge> bridges) {
         this.bridges = bridges;
     }
 
-    private ArtifactIdentity determineIdentityFromFile(File file, String scopeName) {
-        String type = null;
-        String name = null;
-        
-        String filename = file.getName();
-        
-        name = trimExtension(file);
-        
-        if (filename.endsWith(JAR_EXTENSION)) {
-            type = BUNDLE_TYPE;
-        } else if (filename.endsWith(PLAN_EXTENSION)) {
-            type = PLAN_TYPE;
-        } else if (filename.endsWith(PROPERTIES_EXTENSION)) {
-            type = CONFIGURATION_TYPE;
-        } else if (filename.endsWith(PAR_EXTENSION)) {
-            type = PAR_TYPE;
-        }
-
-        
-        if (type != null && name != null) {
-            return new ArtifactIdentity(type, name, Version.emptyVersion, scopeName);
-        } else {
-            return null;
-        }
-    }
-    
-    private String trimExtension(File file) {
-        String filename = file.getName();
-        
-        int lastIndexOf = filename.lastIndexOf('.');
-        
-        if (lastIndexOf > 0) {
-            return filename.substring(0, lastIndexOf);
-        } else {
-            return filename;
-        }
-    }
-    
     public ArtifactIdentity determineIdentity(File file, String scopeName) {
         ArtifactDescriptor artifactDescriptor = null;
         for (ArtifactBridge artifactBridge : this.bridges) {
             try {
                 artifactDescriptor = artifactBridge.generateArtifactDescriptor(file);
             } catch (ArtifactGenerationException e) {
-                LOGGER.warn(String.format("Error occurred while determining the type of an Artifact '%s' with the bridge '%s'.", file,
+                LOGGER.error(String.format("Error occurred while determining the identity of an Artifact '%s' with the bridge '%s'.", file,
                     artifactBridge.getClass().getSimpleName()), e);
+                return null;
             }
             if (artifactDescriptor != null) {
                 break;
             }
         }
+
         if (artifactDescriptor == null) {
-            return this.determineIdentityFromFile(file, scopeName);
-        } else {
-            String type = artifactDescriptor.getType();
-            String name = artifactDescriptor.getName();
-            Version version = artifactDescriptor.getVersion();
-            
-            name = name == null ? trimExtension(file) : name;
-            version = version == null ? Version.emptyVersion : version;
-            
-            return new ArtifactIdentity(type, name, version, scopeName);
+            return null;
         }
+
+        return new ArtifactIdentity(artifactDescriptor.getType(), artifactDescriptor.getName(), artifactDescriptor.getVersion(), scopeName);
     }
 }
diff --git a/org.eclipse.virgo.kernel.deployer/src/test/java/org/eclipse/virgo/kernel/install/artifact/internal/StandardArtifactIdentityDeterminerTests.java b/org.eclipse.virgo.kernel.deployer/src/test/java/org/eclipse/virgo/kernel/install/artifact/internal/StandardArtifactIdentityDeterminerTests.java
index e1681e27..f5011a04 100644
--- a/org.eclipse.virgo.kernel.deployer/src/test/java/org/eclipse/virgo/kernel/install/artifact/internal/StandardArtifactIdentityDeterminerTests.java
+++ b/org.eclipse.virgo.kernel.deployer/src/test/java/org/eclipse/virgo/kernel/install/artifact/internal/StandardArtifactIdentityDeterminerTests.java
@@ -11,99 +11,97 @@
 
 package org.eclipse.virgo.kernel.install.artifact.internal;
 
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
+import org.eclipse.virgo.kernel.install.artifact.ArtifactIdentity;
+import org.eclipse.virgo.repository.ArtifactBridge;
+import org.eclipse.virgo.repository.ArtifactDescriptor;
+import org.eclipse.virgo.repository.ArtifactGenerationException;
+import org.junit.Test;
+import org.osgi.framework.Version;
 
 import java.io.File;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 
-import org.junit.Before;
-import org.junit.Test;
-import org.osgi.framework.Version;
+import static org.easymock.EasyMock.*;
+import static org.junit.Assert.*;
 
-import org.eclipse.virgo.kernel.install.artifact.ArtifactIdentity;
-import org.eclipse.virgo.kernel.install.artifact.ArtifactIdentityDeterminer;
-import org.eclipse.virgo.kernel.install.artifact.internal.StandardArtifactIdentityDeterminer;
-import org.eclipse.virgo.repository.ArtifactBridge;
-import org.eclipse.virgo.repository.ArtifactDescriptor;
-import org.eclipse.virgo.repository.ArtifactGenerationException;
-
-/**
- */
 public class StandardArtifactIdentityDeterminerTests {
 
-    private ArtifactIdentityDeterminer artifactIdentityDeterminer;
-
-    @Before
-    public void setUp() {
-        this.artifactIdentityDeterminer = new StandardArtifactIdentityDeterminer(Collections.<ArtifactBridge>emptySet());
-    }
+    private static final File TEST_FILE = new File("test");
 
     @Test
-    public void testJarFileType() {
-        ArtifactIdentity identity = this.artifactIdentityDeterminer.determineIdentity(new File("test.jar"), null);
-        assertEquals(ArtifactIdentityDeterminer.BUNDLE_TYPE, identity.getType());
-        assertEquals("test", identity.getName());
-        assertEquals(Version.emptyVersion, identity.getVersion());
+    public void identityWithoutBridge() {
+        ArtifactIdentity identity = new StandardArtifactIdentityDeterminer(Collections.<ArtifactBridge>emptySet()).determineIdentity(TEST_FILE, null);
+        assertNull(identity);
     }
 
     @Test
-    public void testPlanFileType() {
-        ArtifactIdentity identity = this.artifactIdentityDeterminer.determineIdentity(new File("test.plan"), null);
-        assertEquals(ArtifactIdentityDeterminer.PLAN_TYPE, identity.getType());
-        assertEquals("test", identity.getName());
-        assertEquals(Version.emptyVersion, identity.getVersion());
-    }
+    public void identityFromBridge() throws ArtifactGenerationException {
+        ArtifactDescriptor artifactDescriptor = createArtifactDescriptorMock();
+        ArtifactBridge bridge = createArtifactBridgeMock(artifactDescriptor);
 
-    @Test
-    public void testPropertiesFileType() {
-        ArtifactIdentity identity = this.artifactIdentityDeterminer.determineIdentity(new File("test.properties"), null);
-        assertEquals(ArtifactIdentityDeterminer.CONFIGURATION_TYPE, identity.getType());
-        assertEquals("test", identity.getName());
-        assertEquals(Version.emptyVersion, identity.getVersion());
-    }
+        checkIdentityDeterminer(bridge);
 
-    @Test
-    public void testParFileType() {
-        ArtifactIdentity identity = this.artifactIdentityDeterminer.determineIdentity(new File("test.par"), null);
-        assertEquals(ArtifactIdentityDeterminer.PAR_TYPE, identity.getType());
-        assertEquals("test", identity.getName());
-        assertEquals(Version.emptyVersion, identity.getVersion());
+        verify(artifactDescriptor, bridge);
     }
 
     @Test
-    public void testWarFileType() {
-        assertNull(this.artifactIdentityDeterminer.determineIdentity(new File("test.war"), null));
+    public void identityFromBridgeThrowingException() throws ArtifactGenerationException {
+        ArtifactDescriptor artifactDescriptor = createArtifactDescriptorMock();
+        ArtifactBridge interestedBridge = createArtifactBridgeMock(artifactDescriptor);
+
+        ArtifactBridge throwingBridge = createMock(ArtifactBridge.class);
+        expect(throwingBridge.generateArtifactDescriptor(new File("test"))).andThrow(new ArtifactGenerationException("Illegal argument"));
+
+        replay(throwingBridge);
+
+        assertNull(new StandardArtifactIdentityDeterminer(new LinkedHashSet<ArtifactBridge>(Arrays.asList(throwingBridge, interestedBridge))).determineIdentity(new File("test"), null));
     }
 
     @Test
-    public void testNoFileType() {
-        assertNull(this.artifactIdentityDeterminer.determineIdentity(new File("test"), null));
+    public void identityFromSeveralBridges() throws ArtifactGenerationException {
+        ArtifactDescriptor artifactDescriptor = createArtifactDescriptorMock();
+        ArtifactBridge interestedBridge = createArtifactBridgeMock(artifactDescriptor);
+
+        ArtifactBridge throwingBridge = createMock(ArtifactBridge.class);
+
+        ArtifactBridge uninterestedBridge = createMock(ArtifactBridge.class);
+        expect(uninterestedBridge.generateArtifactDescriptor(TEST_FILE)).andReturn(null);
+
+        replay(throwingBridge, uninterestedBridge);
+
+        checkIdentityDeterminer(uninterestedBridge,  interestedBridge, throwingBridge);
+
+        verify(artifactDescriptor, throwingBridge, interestedBridge, uninterestedBridge);
     }
-    
-    @Test
-    public void identityFromBridge() throws ArtifactGenerationException {
+
+    private ArtifactDescriptor createArtifactDescriptorMock() throws ArtifactGenerationException {
         ArtifactDescriptor artifactDescriptor = createMock(ArtifactDescriptor.class);
         expect(artifactDescriptor.getType()).andReturn("foo");
         expect(artifactDescriptor.getName()).andReturn("bar");
-        expect(artifactDescriptor.getVersion()).andReturn(new Version(1,2,3));
-        
+        expect(artifactDescriptor.getVersion()).andReturn(new Version(1, 2, 3));
+
+        replay(artifactDescriptor);
+
+        return artifactDescriptor;
+    }
+
+    private ArtifactBridge createArtifactBridgeMock(ArtifactDescriptor descriptor) throws ArtifactGenerationException {
         ArtifactBridge bridge = createMock(ArtifactBridge.class);
-        expect(bridge.generateArtifactDescriptor(new File("test"))).andReturn(artifactDescriptor);
-        
-        replay(artifactDescriptor, bridge);
-        
-        ArtifactIdentity artifactIdentity = new StandardArtifactIdentityDeterminer(new HashSet<ArtifactBridge>(Arrays.asList(bridge))).determineIdentity(new File("test"), null);
+        expect(bridge.generateArtifactDescriptor(new File("test"))).andReturn(descriptor);
+
+        replay(bridge);
+
+        return bridge;
+    }
+
+    private void checkIdentityDeterminer(ArtifactBridge... bridges) {
+        ArtifactIdentity artifactIdentity = new StandardArtifactIdentityDeterminer(new LinkedHashSet<ArtifactBridge>(Arrays.asList(bridges))).determineIdentity(TEST_FILE, null);
+        assertNotNull(artifactIdentity);
         assertEquals("foo", artifactIdentity.getType());
         assertEquals("bar", artifactIdentity.getName());
-        assertEquals(new Version(1,2,3), artifactIdentity.getVersion());
-        
-        verify(artifactDescriptor, bridge);
+        assertEquals(new Version(1, 2, 3), artifactIdentity.getVersion());
     }
+
 }
diff --git a/org.eclipse.virgo.kernel.deployer/src/test/java/org/eclipse/virgo/kernel/install/artifact/internal/StubArtifactBridge.java b/org.eclipse.virgo.kernel.deployer/src/test/java/org/eclipse/virgo/kernel/install/artifact/internal/StubArtifactBridge.java
index c3994058..48308a5b 100644
--- a/org.eclipse.virgo.kernel.deployer/src/test/java/org/eclipse/virgo/kernel/install/artifact/internal/StubArtifactBridge.java
+++ b/org.eclipse.virgo.kernel.deployer/src/test/java/org/eclipse/virgo/kernel/install/artifact/internal/StubArtifactBridge.java
@@ -49,9 +49,12 @@ public class StubArtifactBridge implements ArtifactBridge {
      */
     public ArtifactDescriptor generateArtifactDescriptor(File artifactFile) throws ArtifactGenerationException {
         boolean matched = false;
+        final String fileName = artifactFile.getName();
+
         for(String ending : this.matchs){
-            if(artifactFile.getName().endsWith(ending)){
+            if(fileName.endsWith(ending)){
                 matched = true;
+                break;
             }
         }
         
@@ -59,7 +62,7 @@ public class StubArtifactBridge implements ArtifactBridge {
             return new ArtifactDescriptor() {
                 
                 public Version getVersion() {
-                    return null;
+                    return Version.emptyVersion;
                 }
                 
                 public URI getUri() {
@@ -71,11 +74,11 @@ public class StubArtifactBridge implements ArtifactBridge {
                 }
                 
                 public String getName() {
-                    return null;
+                    return fileName.substring(0, fileName.lastIndexOf('.'));
                 }
                 
                 public String getFilename() {
-                    return null;
+                    return fileName;
                 }
                 
                 public Set<Attribute> getAttributes() {