Skip to content

Commit

Permalink
[SHRINKWRAP-353] Alternate impl for shallowCopy to centralize copy lo…
Browse files Browse the repository at this point in the history
…gic; reveals design flaw noted by SHRINKWRAP-362
  • Loading branch information
ALRubinger committed Feb 9, 2012
1 parent ee19204 commit e95fbe3
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 167 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
public abstract class ArchiveBase<T extends Archive<T>> implements Archive<T>, Configurable, ArchiveFormatAssociable {

// -------------------------------------------------------------------------------------||
// Class Members ----------------------------------------------------------------------||
// Class Members -----------------------------------------------------------------------||
// -------------------------------------------------------------------------------------||

/**
Expand All @@ -69,7 +69,7 @@ public abstract class ArchiveBase<T extends Archive<T>> implements Archive<T>, C
private static final Logger log = Logger.getLogger(ArchiveBase.class.getName());

// -------------------------------------------------------------------------------------||
// Instance Members -------------------------------------------------------------------||
// Instance Members --------------------------------------------------------------------||
// -------------------------------------------------------------------------------------||

/**
Expand All @@ -83,7 +83,7 @@ public abstract class ArchiveBase<T extends Archive<T>> implements Archive<T>, C
private final Configuration configuration;

// -------------------------------------------------------------------------------------||
// Constructor ------------------------------------------------------------------------||
// Constructor -------------------------------------------------------------------------||
// -------------------------------------------------------------------------------------||

/**
Expand Down Expand Up @@ -486,6 +486,51 @@ public T merge(final Archive<?> source, final String path) throws IllegalArgumen
return this.merge(source, ArchivePaths.create(path));
}

/**
* {@inheritDoc}
*
* @see org.jboss.shrinkwrap.api.Archive#shallowCopy()
*/
@Override
public final Archive<T> shallowCopy() {
// Use existing configuration
final Configuration configuration = this.getConfiguration();

// Create a new archive to which we'll shallow copy all assets
final Archive<T> from = this;

//TODO SHRINKWRAP-362
/*
* The design of this particular hack is particularly offensive:
*
* First off, we're binding this abstract parent to use a specific
* implementation in creating new instances.
*
* Second, there's the unchecked casting.
*
* Mostly, however, is that this "shallowCopy" issue
* denoted by SHRINKWRAP-353 calls into question
* the whole design of having ArchiveBase implement
* the full contract set defined by Archive. Archive is
* targeted to be an end-user view; and ArchiveBase is really
* a set of operations for the underlying storage mechanism. What
* would be much more appropriate is a contract set for Archive, and
* a separate contract for the underlying storage (ie. file system).
* To be addressed in SHRINKWRAP-362.
*
*/
@SuppressWarnings("unchecked")
final Archive<T> to = Archive.class.cast(new MemoryMapArchiveImpl(configuration));

// Now loop through and add all content
for (final ArchivePath path : from.getContent().keySet()) {
to.add(from.get(path).getAsset(), path);
}

// Return
return to;
}

/**
* {@inheritDoc}
*
Expand Down Expand Up @@ -676,19 +721,14 @@ public Configuration getConfiguration() {
return configuration;
}

// -------------------------------------------------------------------------------------||
// Contracts --------------------------------------------------------------------------||
// -------------------------------------------------------------------------------------||

/**
* Returns the actual typed class for this instance, used in safe casting for covariant return types
*
* Exposes the actual class used in casting
* @return
*/
protected abstract Class<T> getActualClass();

// -------------------------------------------------------------------------------------||
// Internal Helper Methods ------------------------------------------------------------||
// Internal Helper Methods -------------------------------------------------------------||
// -------------------------------------------------------------------------------------||

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,6 @@ public GenericArchiveImpl(final Archive<?> delegate) {
super(GenericArchive.class, delegate);
}

/**
* {@inheritDoc}
*
* @see Archive#shallowCopy()
*/
@Override
public GenericArchiveImpl shallowCopy() {
GenericArchiveImpl newInstance = new GenericArchiveImpl(getArchive().shallowCopy());
ShallowCopy.shallowCopyContentTo(this, newInstance);
return newInstance;
}

/**
* {@inheritDoc}
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,23 +66,10 @@ public MemoryMapArchiveImpl(final String archiveName, final Configuration config

/**
* {@inheritDoc}
*
* @see Archive#shallowCopy()
*/
@Override
public MemoryMapArchiveImpl shallowCopy() {
MemoryMapArchiveImpl newInstance = new MemoryMapArchiveImpl(getConfiguration());
ShallowCopy.shallowCopyContentTo(this, newInstance);
return newInstance;
}

/**
* {@inheritDoc}
*
* @see org.jboss.shrinkwrap.impl.base.ArchiveBase#getActualClass()
*/
@Override
protected Class<MemoryMapArchive> getActualClass() {
public Class<MemoryMapArchive> getActualClass() {
return MemoryMapArchive.class;
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,18 @@
import java.util.Map;

import org.jboss.shrinkwrap.api.Archive;
import org.jboss.shrinkwrap.api.ArchiveFactory;
import org.jboss.shrinkwrap.api.ArchiveFormat;
import org.jboss.shrinkwrap.api.ArchivePath;
import org.jboss.shrinkwrap.api.ArchivePaths;
import org.jboss.shrinkwrap.api.ClassLoaderSearchUtilDelegator;
import org.jboss.shrinkwrap.api.Configuration;
import org.jboss.shrinkwrap.api.Domain;
import org.jboss.shrinkwrap.api.Filter;
import org.jboss.shrinkwrap.api.Filters;
import org.jboss.shrinkwrap.api.IllegalArchivePathException;
import org.jboss.shrinkwrap.api.Node;
import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.Asset;
import org.jboss.shrinkwrap.api.asset.ByteArrayAsset;
import org.jboss.shrinkwrap.api.asset.ClassAsset;
Expand Down Expand Up @@ -490,8 +494,28 @@ public String getName() {

/**
* {@inheritDoc}
*
* @see org.jboss.declarchive.api.Archive#toString(boolean)
* @see org.jboss.shrinkwrap.api.Archive#shallowCopy()
*/
@Override
public Archive<T> shallowCopy() {
// Get the actual class type and make a shallow copy of the the underlying archive,
// using the same underlying configuration
final Class<T> actualClass = this.getActualClass();
final Archive<?> underlyingArchive = this.getArchive();
final Configuration existingConfig= ((Configurable)underlyingArchive).getConfiguration();
final Domain domain = ShrinkWrap.createDomain(existingConfig);
final ArchiveFactory factory = domain.getArchiveFactory();
final Archive<T> newArchive = factory.create(actualClass, this.getName());
final Map<ArchivePath, Node> contents = underlyingArchive.getContent();
for (final ArchivePath path : contents.keySet()) {
newArchive.add(contents.get(path).getAsset(), path);
}
return newArchive;
}

/**
* {@inheritDoc}
* @see java.lang.Object#toString()
*/
@Override
public String toString() {
Expand All @@ -500,8 +524,7 @@ public String toString() {

/**
* {@inheritDoc}
*
* @see org.jboss.declarchive.api.Archive#toString(boolean)
* @see org.jboss.shrinkwrap.api.Archive#toString(boolean)
*/
@Override
public String toString(final boolean verbose) {
Expand All @@ -518,13 +541,21 @@ public String toString(final Formatter formatter) throws IllegalArgumentExceptio
return this.getArchive().toString(formatter);
}

/**
* {@inheritDoc}
* @see java.lang.Object#hashCode()
*/
@Override
public int hashCode() {
return this.getArchive().hashCode();
}

/**
* {@inheritDoc}
* @see java.lang.Object#equals(java.lang.Object)
*/
@Override
public boolean equals(Object obj) {
public boolean equals(final Object obj) {
if (obj instanceof ArchiveBase<?>) {
return this.getArchive().equals(obj);
}
Expand All @@ -548,20 +579,18 @@ public boolean equals(Object obj) {
*/
protected abstract ArchivePath getManifestPath();

/*
* (non-Javadoc)
*
* @see org.jboss.declarchive.api.container.ManifestContainer#setManifest(java.lang.String)
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#setManifest(java.lang.String)
*/
@Override
public final T setManifest(String resourceName) {
Validate.notNull(resourceName, "ResourceName should be specified");
return setManifest(new ClassLoaderAsset(resourceName));
}

/*
* (non-Javadoc)
*
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#setManifest(java.io.File)
*/
@Override
Expand All @@ -570,9 +599,8 @@ public T setManifest(File resource) throws IllegalArgumentException {
return setManifest(new FileAsset(resource));
}

/*
* (non-Javadoc)
*
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#setManifest(java.net.URL)
*/
@Override
Expand All @@ -581,22 +609,19 @@ public T setManifest(URL resource) throws IllegalArgumentException {
return setManifest(new UrlAsset(resource));
}

/*
* (non-Javadoc)
*
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#setManifest(org.jboss.shrinkwrap.api.Asset)
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#setManifest(org.jboss.shrinkwrap.api.asset.Asset)
*/
@Override
public T setManifest(Asset resource) throws IllegalArgumentException {
Validate.notNull(resource, "Resource should be specified");
return addAsManifestResource(resource, "MANIFEST.MF");
}

/*
* (non-Javadoc)
*
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#setManifestResource(java.lang.Package,
* java.lang.String)
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#setManifest(java.lang.Package, java.lang.String)
*/
@Override
public T setManifest(Package resourcePackage, String resourceName) throws IllegalArgumentException {
Expand All @@ -607,32 +632,29 @@ public T setManifest(Package resourcePackage, String resourceName) throws Illega
return setManifest(classloaderResourceName);
}

/*
* (non-Javadoc)
*
* @see org.jboss.declarchive.api.container.ManifestContainer#addManifestResource(java.lang.String)
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#addAsManifestResource(java.lang.String)
*/
@Override
public final T addAsManifestResource(String resourceName) {
Validate.notNull(resourceName, "ResourceName should be specified");
return addAsManifestResource(fileFromResource(resourceName), resourceName);
}

/*
* (non-Javadoc)
*
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#addManifestResource(java.io.File)
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#addAsManifestResource(java.io.File)
*/
@Override
public T addAsManifestResource(File resource) throws IllegalArgumentException {
Validate.notNull(resource, "Resource should be specified");
return addAsManifestResource(resource, resource.getName());
}

/*
* (non-Javadoc)
*
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#addManifestResource(java.lang.String, java.lang.String)
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#addAsManifestResource(java.lang.String, java.lang.String)
*/
@Override
public T addAsManifestResource(String resourceName, String target) throws IllegalArgumentException {
Expand All @@ -642,10 +664,9 @@ public T addAsManifestResource(String resourceName, String target) throws Illega
return addAsManifestResource(fileFromResource(resourceName), target);
}

/*
* (non-Javadoc)
*
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#addManifestResource(java.io.File, java.lang.String)
/**
* {@inheritDoc}
* @see org.jboss.shrinkwrap.api.container.ManifestContainer#addAsManifestResource(java.io.File, java.lang.String)
*/
@Override
public T addAsManifestResource(File resource, String target) throws IllegalArgumentException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.jboss.shrinkwrap.api.Archive;
import org.jboss.shrinkwrap.api.ArchivePath;
import org.jboss.shrinkwrap.api.spec.EnterpriseArchive;
import org.jboss.shrinkwrap.impl.base.ShallowCopy;
import org.jboss.shrinkwrap.impl.base.container.EnterpriseContainerBase;
import org.jboss.shrinkwrap.impl.base.path.BasicPath;

Expand Down Expand Up @@ -78,18 +77,6 @@ public EnterpriseArchiveImpl(final Archive<?> delegate) {
super(EnterpriseArchive.class, delegate);
}

/**
* {@inheritDoc}
*
* @see Archive#shallowCopy()
*/
@Override
public EnterpriseArchiveImpl shallowCopy() {
EnterpriseArchiveImpl newInstance = new EnterpriseArchiveImpl(getArchive().shallowCopy());
ShallowCopy.shallowCopyContentTo(this, newInstance);
return newInstance;
}

// -------------------------------------------------------------------------------------||
// Required Implementations -----------------------------------------------------------||
// -------------------------------------------------------------------------------------||
Expand Down

0 comments on commit e95fbe3

Please sign in to comment.