[Geoserver-devel] IOUtils.zipDirectory issue

Hey,

for the GeoNode process I'm using the org.geoserver.data.util.IOUtils.zipDirectory method (indirectly through shape-zipping).
Problem is that this method is taking ownership of its argument ZipOutputStream since its calling zipout.finish(), essentially preventing the calling code (which owns the zip output stream) to append more content to the zip archive.

As far as I can tell the only code using this is the ShapeZipOutputFormat? (for which I'm going to propose a refactor in a separate email).

So I would like to prevent the utility method from calling ZipOutputStream.finish() at all and instead leave the responsibility to the calling code, which I think would be more appropriate.

thoughts?

Cheers,
Gabriel

--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Gabriel Roldan ha scritto:

Hey,

for the GeoNode process I'm using the org.geoserver.data.util.IOUtils.zipDirectory method (indirectly through shape-zipping).
Problem is that this method is taking ownership of its argument ZipOutputStream since its calling zipout.finish(), essentially preventing the calling code (which owns the zip output stream) to append more content to the zip archive.

As far as I can tell the only code using this is the ShapeZipOutputFormat? (for which I'm going to propose a refactor in a separate email).

Dangerous business, every time I touch that class it breaks in some
way (it's handling a ton of use cases)

So I would like to prevent the utility method from calling ZipOutputStream.finish() at all and instead leave the responsibility to the calling code, which I think would be more appropriate.

Works for me

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

So I would like to prevent the utility method from calling
ZipOutputStream.finish() at all and instead leave the responsibility
to the calling code, which I think would be more appropriate.

Works for me

Ok cool. GeoNode is using 2.0.2 so I would need to apply to the stable branch. Concerns?

Cheers
Andrea

--
Gabriel Roldan
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Gabriel Roldan ha scritto:

So I would like to prevent the utility method from calling
ZipOutputStream.finish() at all and instead leave the responsibility
to the calling code, which I think would be more appropriate.

Works for me

Ok cool. GeoNode is using 2.0.2 so I would need to apply to the stable branch. Concerns?

None, it's a one liner, no?

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

On 4/1/10 3:35 AM, Andrea Aime wrote:

Gabriel Roldan ha scritto:

As far as I can tell the only code using this is the
ShapeZipOutputFormat? (for which I'm going to propose a refactor in a
separate email).

Dangerous business, every time I touch that class it breaks in some
way (it's handling a ton of use cases)

Well, intent is only to decouple the outputformat business from the actual shape-zipping process.
Attempt results in the following patch: <http://pastebin.com/RzkeVmeJ&gt;
Meaning the output format write method ends up as: <http://pastebin.com/4yNZxQKe&gt; and the utility class as <http://pastebin.com/FtXS8YVz&gt;\.

Rationale being I need exactly this functionality for a geonode process (the one in the utility class) but not the wfs output format's specific concerns, and I would better avoid just copying and pasting?

comments welcomed.

Cheers,
Gabriel

Gabriel Roldan ha scritto:

On 4/1/10 3:35 AM, Andrea Aime wrote:

Gabriel Roldan ha scritto:

As far as I can tell the only code using this is the
ShapeZipOutputFormat? (for which I'm going to propose a refactor in a
separate email).

Dangerous business, every time I touch that class it breaks in some
way (it's handling a ton of use cases)

Well, intent is only to decouple the outputformat business from the actual shape-zipping process.
Attempt results in the following patch: <http://pastebin.com/RzkeVmeJ&gt;
Meaning the output format write method ends up as: <http://pastebin.com/4yNZxQKe&gt; and the utility class as <http://pastebin.com/FtXS8YVz&gt;\.

Rationale being I need exactly this functionality for a geonode process (the one in the utility class) but not the wfs output format's specific concerns, and I would better avoid just copying and pasting?

Generally speaking it seems ok, besides the bit that exposes a static
method out of the utility writer (I think it's easy to make the method
non static as you create the utility objects a few lines later).

However I would like to look at it a bit closer, as I'm still half
asleep :slight_smile:

Can you open a jira and attach the patch there?

Cheers
Andrea

--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.

Can you open a jira and attach the patch there?

sure. First thing when I get up (as I didn't got bed yet and am right about to :slight_smile:

Cheers,
Gabriel

On 4/1/10 12:21 AM, Gabriel Roldan wrote:

Hey,

for the GeoNode process I'm using the
org.geoserver.data.util.IOUtils.zipDirectory method (indirectly through
shape-zipping).
Problem is that this method is taking ownership of its argument
ZipOutputStream since its calling zipout.finish(), essentially
preventing the calling code (which owns the zip output stream) to append
more content to the zip archive.

As far as I can tell the only code using this is the
ShapeZipOutputFormat? (for which I'm going to propose a refactor in a
separate email).

I thought restconfig might be using it as well to send zip files back to the client... but I think that could be another method I am thinking of. Might want to double check.

So I would like to prevent the utility method from calling
ZipOutputStream.finish() at all and instead leave the responsibility to
the calling code, which I think would be more appropriate.

How about overloading the method with a flag called "finish" and by default have it set to true. Then your code can just pass in false. Ensures that no client code can break.

thoughts?

Cheers,
Gabriel

--
Justin Deoliveira
OpenGeo - http://opengeo.org
Enterprise support for open source geospatial.