0

So I've written a small servlet to test file uploading with.

The form used to trigger the upload is very simple:

<form method="post" action="/webapp/upload" enctype="multipart/form-data">
    Choose the file(s) to upload:<br>
    <input type="file" name="files" multiple/>
    <input type="submit" value="Upload" />
</form>

The relevant servlet functionality structure can be summarised as

@Override
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
    PrintWriter out = resp.getWriter();
    resp.setContentType("text/html");

    req.getParts().parallelStream().forEach(p -> {
        //store uploaded file(s) to disk and communicate success/failure to client
    });


    req.getRequestDispatcher(uploadForm).include(req,resp);
}

BUT, I accidentally hit the upload button without having selected any files, and this is what was returned to me.

uploading '': application/octet-stream, 0KB...OK

And true enough, the upload folder on the server now contains a file named (1) (because my renaming method decided the empty name already existed) that is 0 bytes large.

And this made me aware that I have no idea how to check for "no file has been selected".

Spent enough time on this bloody thing that I may just as well back it up on stackoverflow for later reference. Who knows, maybe somebody else finds it useful, too. Or somebody has a helpful comment.

User1291
  • 7,664
  • 8
  • 51
  • 108

1 Answers1

1

I don't want to check for the file size because a file MAY just happen to be 0 bytes in size (a useless file, in the very most cases, admittedly, but still). And checking for empty file-names only allows me to decide whether or not a particular Part can be discarded.

If I can avoid it, I don't want to iterate through the parts more than once either.

So I guess what I could do is create a boolean flag that I set to true if I end up uploading anything at all ...

boolean uploadedAnything = false;

Since I'm using a parallelStream, updating this flag will require some synchronisation. I'm already synchronising on the out stream, so why not simply toss it into there?

synchronized (out) {
    out.print(String.format("uploading '%s': %s, %d%s...", fileName, p.getContentType(), sizeInKb, "KB"));
    if (success){
        out.println("OK<br>");
        uploadedAnything = true;
    }
    else out.println("FAILED<br>");
}

except Java refuses to compile that because

local variables referenced from a lambda expression must be final or effectively final

which has nothing to do with the synchronized block but more to the fact that the whole thing is encapsulated in a

boolean uploadedAnything = false;
req.getParts().parallelStream().forEach(p -> {
    //`uploadedAnything` gets changed here
});

I guess replacing the boolean with an AtomicBoolean could work, but I don't like that solution very much because I hate adding synchronisation that I KNOW serves no purpose.

So ... next idea:

Instead of going for a forEach, let's go for a map. That way, we turn our list of Parts into a list of statements whether or not uploading the file corresponding to that Part actually succeeded.

I.e.

boolean uploadedAnything = req.getParts().parallelStream().map(p -> {
    [...]
    return success;
}).matchAny(Predicate.isEqual(true));

except we can't do that either because matchAny then short-circuits out of handling all files. Whoops.

So ...

List<Boolean> uploadStatus = req.getParts().parallelStream().map(p -> {
    [...]
    return success;
}).collect(Collectors.toList());
boolean uploadedAnything = uploadStatus.stream().anyMatch(Predicate.isEqual(true));

... ok, that works, but now I've created a whole bloody list of booleans just to get the one boolean.

I guess we could do a fold...

boolean uploadedAnything = req.getParts().parallelStream().fold(false,(status,p) -> {
    [...]
    return status || success;
});

... except Java doesn't support folds, and the one thing that comes close to a somewhat similar functionality requires a third argument, a "Combiner". So we have to search the web to figure out why and how that Combiner actually comes into play.

Having found https://stackoverflow.com/a/24316429/3322533 , the snippet becomes

boolean uploadedAnything = req.getParts().parallelStream().reduce(false,(status,p) -> {
    [...]
    return status || success;
},(accA, accB) -> accA || accB);

which we can rewrite to

boolean uploadedAnything = req.getParts().parallelStream().reduce(false,(status,p) -> {
    [...]
    return status || success;
},Boolean::logicalOr);

This STILL is not a fold because it wreaks havoc on the operation order (which, thankfully, in this case doesn't matter), but it gets the job done.

User1291
  • 7,664
  • 8
  • 51
  • 108