Feature Request: ZipFile::uncompressEntry into MemoryOutputStream


#1

This feature requests avoids creating temporary files on the HDD.

Please add two functions:

Result ZipFile::uncompressEntry (int index, MemoryOutputStream &output)

and

Result ZipFile::uncompressEntry (const String& fileName, MemoryOutputStream &output, bool ignoreCase = false)

so we can uncompress entries without having to write them to HDD first and then reading them back in.

Thanks,
Mike


#2

better use OutputStream instead of MemoryOutputStream . more generic
and the file impl could just call this one with the FileOutputStream

my 2 cents


#3
MemoryOutputStream data;

if (ScopedPointer<InputStream> in = zipFile.createStreamForEntry (xyz))
    data << *in;

You’ll find with library authors everywhere that you’ll get a lot of resistance if you ask for features that can be written very trivially using the functionality that they already provide.

Only when features are very very commonly used or difficult/error-prone to write does it make sense to add them to a library class.

Things like the C++ standard library take this principle to an extreme - JUCE takes a much more lenient and practical approach, but this one doesn’t sound like it’d be worth adding.


#4

I understand, but can I please leave this example in the hopes you re-consider?

Suppose I have a zip-file with multiple user-generated files in them, so I don’t know if they’re even in there or not.

Now ideally I could simply use:

MemoryOutputStream	output;
if (skinZip->uncompressEntry ("skin.json", output).wasOk ())
{
    // Parse json etc.
}

for each potentially existing file

vs.

 MemoryOutputStream  output;
 if (auto entry = skinZip->getEntry ("skin.json", true))
 {
     if (ScopedPointer<InputStream> in = skinZip->createStreamForEntry (entry))
         output << *in;

     if (output.getDataSize () != 0)
     {
         // Parse json etc.
     }
 }

which seems unnecessarily verbose.

Thanks for your consideration.

Cheers,
Mike


#5

You see, this is something that’s annoying about developing libraries… People always give us very specific requests, for particular tiny bits of functionality. Then you dig a bit deeper into what they’re trying to do, and it turns out that what they’re asking for is not what they really need.

e.g. why not just write yourself a free function like this?

static var getJSONFileFromZip (const ZipFile& zf, StringRef name)
{
    if (auto e = zf.getEntry (name, true))
        if (ScopedPointer<InputStream> in = zf.createStreamForEntry (e))
            return JSON::parse (in->readEntireStreamAsString());

    return {};
}

#6

Oh, I think you misunderstood. I’m not talking about reading a single JSON file here and wanting to avoid 4-5 lines of code.

I’m talking about reading 4-5 different files (albeit from the same ZIP) and reacting to them differently:

Here an example from my code:

	MemoryOutputStream	output;
	if (skinZip->uncompressEntry ("skin.json", output).wasOk ())
	{
		skinJSON = json::parse (output.toString ().toStdString ());
	}

	output.reset ();
	if (skinZip->uncompressEntry ("background.svg", output).wasOk ())
	{
		ScopedPointer<XmlElement> svg (XmlDocument::parse (output.toString ()));
		back_svg = Drawable::createFromSVG (*svg);
	}
	else if (skinZip->uncompressEntry ("background.jpg", output).wasOk ())
	{
		back_jpg = ImageFileFormat::loadFrom (output.getData (), output.getDataSize ());
	}

	output.reset ();
	if (skinZip->uncompressEntry ("background.png", output).wasOk ())
	{
		back_png = ImageFileFormat::loadFrom (output.getData (), output.getDataSize ());
	}

Maybe it’s clearer now?

Thanks,
Mike


#7

OK, but that’s still a clunky way of writing it. I’d write myself a couple of helpers:

static String loadString (const ZipFile& zf, StringRef name)
{
    if (auto e = zf.getEntry (name, true))
        if (ScopedPointer<InputStream> in = zf.createStreamForEntry (e))
            return in->readEntireStreamAsString();

    return {};
}

static Image loadImage (const ZipFile& zf, StringRef name)
{
    if (auto e = zf.getEntry (name, true))
        if (ScopedPointer<InputStream> in = zf.createStreamForEntry (e))
            return ImageFileFormat::loadFrom (*in);

    return {};
}

It’d make no sense to copy an image into a MemoryOutputStream and then stream that into an image decoder. Nor would it be very elegant to have multiple places where you convert a MemoryOutputStream into a string and then use it for different things.


#8

But I don’t convert the stream into a string and then into an image. Only two of those convert to a string. The JSON and the SVG. Both are strings. The others are converted directly from the stream into the images.

But I get your point.

Thanks,
Mike


#9

I didn’t say you converted the image to a string, but you were copying all the data into an intermediate stream when there was no need.

But yes, I hope you see what I’m trying to explain. It’s all about getting the right abstractions at the right level.


#10

lambda function allows to have the helper function Jules talks about with an std::function as second argument and have different way to react to the output
easily using one liner.

my 2 cents