Skip to content

Commit

Permalink
Only export ArchiveAssets that have content
Browse files Browse the repository at this point in the history
  • Loading branch information
@gpoul
gpoul committed Jul 16, 2011
1 parent dfb4b8f commit 31e9e33
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/asset/ArchiveAsset.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ public ArchiveAsset(final Archive<?> archive, final Class<? extends StreamExport
public InputStream openStream()
{
// Export via the specified exporter
return this.getArchive().as(this.exporter).exportAsInputStream();
if (!this.getArchive().getContent().isEmpty())
return this.getArchive().as(this.exporter).exportAsInputStream();
else
return null;
}

/**
Expand Down

8 comments on commit 31e9e33

@ALRubinger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this isEmpty() check necessary?

@gpoul
Copy link
Owner Author

@gpoul gpoul commented on 31e9e33 Jul 16, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there seems to be an edge case in the JVM-provided ZipOutputStream implementation that causes the underlying JdkZipExporterDelegate to bail on zero-length content archives.

https://github.com/gpoul/shrinkwrap/blob/master/impl-base/src/main/java/org/jboss/shrinkwrap/impl/base/exporter/zip/JdkZipExporterDelegate.java#L71

@ALRubinger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes. I've had fun with that before; glad I noted it as such. My memory is just about the worst. :)

@ALRubinger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even still, I think we have a gap here. Nowhere in the API docs does it note that "null" is an acceptable return value:

http://docs.jboss.org/shrinkwrap/1.0.0-beta-4/org/jboss/shrinkwrap/api/asset/Asset.html

How about instead: new ByteArrayInputStream(new byte[]{}); ?

@ALRubinger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or even better...whatever content represents a valid this.exporter will no content? We could get around the JDK ZIP limitation by using another tool to generate an empty ZIP archive, putting that in src/main/resources, and return a new InputStream with that as the byte content.

@gpoul
Copy link
Owner Author

@gpoul gpoul commented on 31e9e33 Jul 16, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that "If this returns null, this denotes that the Asset is to be viewed as a logical path (placeholder/directory) only with no backing content." would cover this case just fine... (?)

@ALRubinger
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hehe, but this isn't a logical path. It's an empty archive. Directories in ShrinkWrap are denoted by Node with getAsset==null. http://docs.jboss.org/shrinkwrap/1.0.0-beta-4/org/jboss/shrinkwrap/api/Node.html#getAsset()

@gpoul
Copy link
Owner Author

@gpoul gpoul commented on 31e9e33 Jul 16, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c7fb6d3

Please sign in to comment.