Covarient returns from a storage delagate

[johnbailey]

I ran into some interesting gotchas when implementing the MemoryMapArchive. The chaining API breaks down a bit when implementing a storage archive. The difficulty is the return type of the storage implementation is set to be the actual type of the archive (JavaArchive, WebArchive, etc...). So it would be expected that each method that allows chaining would have a covarient return type of the actual archive type (JavaArchive, etc...). This can't be handled just by capturing the actual class to use and performing a cast, as the storage archive is only a delegate and is not guaranteed to be an instance of the return archive type. Without multiple inheritance, I don't think we can get this to work without one of the following:

1. The storage archives don't follow the same chaining API:

 void add(Path target, String name, Asset asset) throws IllegalArgumentException;

instead of...

T add(Path target, String name, Asset asset) throws IllegalArgumentException;

2. The storage archive must maintain a reference to be used by any chaining methods.

 /** From some storage archive **/

 private T primaryArchive;

 public void initialize(T archive)
 {
 this.primaryArchive = archive;
 }

 protected T covarientReturn()
 {
 return primaryArchive;
 }

It would be unfortunate to go the first route as Storage archives would no longer be API compliant. Option two seems to make more sense. Since it is assumed the storage archive is going to be provided to the containers, this may be problematic as there will not be a strict contract to make sure the reference is set on the storage archive. I have an impl working with the second approach, but as this is early on I want to get input on it.

Any thoughts?

[aslak]

The storage engines will be wrapped up/delegated to by the Java/WebArchive spec impls and never exposed directly through the chaining in the Archive interface.

The thought was to let the Storage Engines have there own Archive interfaces and not depended on the specs.

The spec impls would wrap up the storage engines(handeled by a support Superclass) like:

 public JavaArchive add(Asset... assets)
 {
 archive.add(assets);
 return this;
 }

They do not extend the storage engines nor do they expose them directly in this way:

 public JavaArchiveImpl extends MemoryArchiveImpl {
 }


 public JavaArchive add(Asset... assets)
 {
 return archive.add(assets);
 }

Here is a more complete stack example:

 public interface ServiceProviderDelegate
 {
 Archive<?> getDelegate();
 }

 public interface MemoryArchive extends Archive<MemoryArchive>
 {
 }

 public class MemoryArchiveImpl implements MemoryArchive
 {
 }

 public class JavaArchiveImpl implements JavaArchive, ServiceProviderDelegate
 {
 Archive<?> archive;

 public JavaArchiveImpl(Archive<?> archive)
 {
 this.archive = archive;
 }

 public JavaArchive add(Asset... assets)
 {
 archive.add(assets);
 return this;
 }

 @Override
 public Archive<?> getDelegate()
 {
 return archive;
 }
 }

 JavaArchive jar = new JavaArchiveImpl(new MemoryArchiveImpl());

[alrubinger]

As Aslak describes.

With the remaining question mark of:

public JavaArchive add(Asset... assets)

...pursuant to our Asset.getPath discussion. But definitely:

public JavaArchive addClasses(Class<?>... classes)
{
 for(Class<?> clazz : classes)
 {
 ClassAsset asset = new ClassAsset(clazz);
 Path path = new BasicPath(ClassAssetUtil.convertClassNameToPathName(clazz));
 storageDelegate.add(path,asset);
 }
}

S,
ALR

[johnbailey]

Yeah my assumption was there was no reason we would need to return the delegating spec archives. My initial implemantation of the memory map did not have any reference to the delegating archive instance or type. The only thing that led me to ask the question was Aslak's patch was maintaining the actual type as a parameter and returning that on the covarient return, which really just causes a class cast exception. Just wanted to make sure I wasn't missing something.

[aslak]

Yea, the patch was work in progress. It was after I submitted the patch we realised that the MemoryMapArchive should have its own interface.