SHRINKWRAP-163: Making the Archive name optional
Ken's submitted a patch to:
https://jira.jboss.org/browse/SHRINKWRAP-163
Some review.
1) JavaDocs. As noted in Ken's TODOs, yep, we'll put these in before commit.
2) ExtensionType:
@Override
public String toString()
{
return "." + extension;
}
The "." can be a final static char.
3) ConfigurationBuilder / Configuration:
public ConfigurationBuilder assignableExtensionTypeMapping(final Map<Class<?>, ExtensionType> assignableExtensionTypeMapping)
Can we find a cleaner name? "getExtensionMapping"?
4) ShrinkWrap/ArchiveFactory:
@Deprecated public static <T extends Assignable> T create(final String archiveName, final Class<T> type)
Let's note in the JavaDocs why it's deprecated, and point to the new stuff.
5) ShrinkWrapTestCase:
@Test
public void shouldCreateArchiveWithCorrectExtensionBasedOnType() throws Exception
{
JavaArchive javaArchive = ShrinkWrap.create(JavaArchive.class);
Assert.assertTrue("JavaArchive should have proper extension: " + ExtensionType.JAR, javaArchive.getName().endsWith(ExtensionType.JAR.toString()));
WebArchive webArchive = ShrinkWrap.create(WebArchive.class);
Assert.assertTrue("WebArchive should have proper extension: " + ExtensionType.WAR, webArchive.getName().endsWith(ExtensionType.WAR.toString()));
EnterpriseArchive enterpriseArchive = ShrinkWrap.create(EnterpriseArchive.class);
Assert.assertTrue("EnterpriseArchive should have proper extension: " + ExtensionType.EAR, enterpriseArchive.getName().endsWith(ExtensionType.EAR.toString()));
ResourceAdapterArchive resourceAdapterArchive = ShrinkWrap.create(ResourceAdapterArchive.class);
Assert.assertTrue("ResourceAdapterArchive should have proper extension: " + ExtensionType.RAR, resourceAdapterArchive.getName().endsWith(ExtensionType.RAR.toString()));
}
If we honor Kent Beck fully, these checks may each get their own test.
6) ArchiveFactory:
public <T extends Assignable> T create(final Class<T> type)
throws IllegalArgumentException
{
// Precondition checks
if (type == null)
{
throw new IllegalArgumentException("Type must be specified");
}
String archiveName = UUID.randomUUID().toString();
ExtensionType extensionType = configuration.getAssignableExtensionTypeMapping().get(type);
if (extensionType != null)
{
archiveName += extensionType;
}
return create(type, archiveName);
}
I try to avoid logic like this. If there's a mapping, we append to the name. Else we don't. I'd probably opt for an unchecked exception saying something like "The current configuration has no mapping for type X, unable to determine extension. Either add a mapping via Configuration.getExtensionMapping or manually assign a name." Thoughts?
7) Test Coverage
Coburtura shows we're missing tests for:
ShrinkWrap:
public static <T extends Assignable> T create(final Class<T> type, final String archiveName)
throws IllegalArgumentException
public static <T extends Assignable> T create(final Class<T> type, final String archiveName, final ExtensionType extensionType)
throws IllegalArgumentException
ArchiveFactory:
public <T extends Assignable> T create(final Class<T> type, final String archiveName, final ExtensionType extensionType)
throws IllegalArgumentException
Otherwise real nice.
S,
ALR
