Create upload URLs only when stream variable is attached by drop handler (#9301)

* Create upload URLs only when stream variable is attached by drop handler

* Add file size limit to test ui and remove label drop target

3 files changed, 54 insertions, 86 deletions

diff --git a/server/src/main/java/com/vaadin/event/dnd/FileDropEvent.java b/server/src/main/java/com/vaadin/event/dnd/FileDropEvent.java
index 42d270ebe4..7ad5f410bf 100644
--- a/server/src/main/java/com/vaadin/event/dnd/FileDropEvent.java
+++ b/server/src/main/java/com/vaadin/event/dnd/FileDropEvent.java
@@ -15,6 +15,7 @@
  */
 package com.vaadin.event.dnd;
 
+import java.util.Collection;
 import java.util.List;
 
 import com.vaadin.ui.AbstractComponent;
@@ -22,7 +23,7 @@ import com.vaadin.ui.Component;
 import com.vaadin.ui.Html5File;
 
 /**
- * File drop event that contains the list of files dropped on a file drop
+ * File drop event that contains the collection of files dropped on a file drop
  * target.
  *
  * @param <T>
@@ -34,7 +35,7 @@ import com.vaadin.ui.Html5File;
 public class FileDropEvent<T extends AbstractComponent> extends
         Component.Event {
 
-    private final List<Html5File> files;
+    private final Collection<Html5File> files;
 
     /**
      * Creates a file drop event.
@@ -42,21 +43,21 @@ public class FileDropEvent<T extends AbstractComponent> extends
      * @param target
      *         The file drop target component.
      * @param files
-     *         List of files.
+     *         Collection of files.
      */
-    public FileDropEvent(T target, List<Html5File> files) {
+    public FileDropEvent(T target, Collection<Html5File> files) {
         super(target);
 
         this.files = files;
     }
 
     /**
-     * Gets the list of files dropped onto the file drop target component.
+     * Gets the collection of files dropped onto the file drop target component.
      *
-     * @return List of files that were dropped onto the file drop target
+     * @return Collection of files that were dropped onto the file drop target
      * component.
      */
-    public List<Html5File> getFiles() {
+    public Collection<Html5File> getFiles() {
         return files;
     }
 }
diff --git a/server/src/main/java/com/vaadin/ui/FileDropTarget.java b/server/src/main/java/com/vaadin/ui/FileDropTarget.java
index 831d6723d9..88d2550691 100644
--- a/server/src/main/java/com/vaadin/ui/FileDropTarget.java
+++ b/server/src/main/java/com/vaadin/ui/FileDropTarget.java
@@ -16,9 +16,7 @@
 package com.vaadin.ui;
 
 import java.io.OutputStream;
-import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 
 import com.vaadin.event.dnd.DropTargetExtension;
@@ -72,22 +70,40 @@ public class FileDropTarget<T extends AbstractComponent> extends
         registerRpc(new FileDropTargetRpc() {
             @Override
             public void drop(Map<String, FileParameters> fileParams) {
-                List<Html5File> files = new ArrayList<>();
+                Map<String, Html5File> files = new HashMap<>();
                 Map<String, String> urls = new HashMap<>();
 
+                // Create a collection of html5 files
                 fileParams.forEach((id, fileParameters) -> {
-                    Html5File html5File = new Html5File(fileParameters.getName(),
-                            fileParameters.getSize(), fileParameters.getMime());
-                    String url = createUrl(html5File, id);
+                    Html5File html5File = new Html5File(
+                            fileParameters.getName(), fileParameters.getSize(),
+                            fileParameters.getMime());
+                    files.put(id, html5File);
+                });
+
+                // Call drop handler with the collection of dropped files
+                FileDropEvent<T> event = new FileDropEvent<>(target,
+                        files.values());
+                fileDropHandler.drop(event);
+
+                // Create upload URLs for the files that the drop handler
+                // attached stream variable to
+                files.entrySet().stream()
+                        .filter(entry -> entry.getValue().getStreamVariable()
+                                != null).forEach(entry -> {
+                    String id = entry.getKey();
+                    Html5File file = entry.getValue();
 
-                    files.add(html5File);
+                    String url = createUrl(file, id);
                     urls.put(id, url);
                 });
 
-                getRpcProxy(FileDropTargetClientRpc.class).sendUploadUrl(urls);
-
-                FileDropEvent<T> event = new FileDropEvent<>(target, files);
-                fileDropHandler.drop(event);
+                // Send upload URLs to the client if there are files to be
+                // uploaded
+                if (urls.size() > 0) {
+                    getRpcProxy(FileDropTargetClientRpc.class)
+                            .sendUploadUrl(urls);
+                }
             }
 
             @Override
diff --git a/uitest/src/main/java/com/vaadin/tests/dnd/Html5FileDragAndDropUpload.java b/uitest/src/main/java/com/vaadin/tests/dnd/Html5FileDragAndDropUpload.java
index 211d50d047..c01e0293c1 100644
--- a/uitest/src/main/java/com/vaadin/tests/dnd/Html5FileDragAndDropUpload.java
+++ b/uitest/src/main/java/com/vaadin/tests/dnd/Html5FileDragAndDropUpload.java
@@ -29,75 +29,17 @@ import com.vaadin.ui.Grid;
 import com.vaadin.ui.Html5File;
 import com.vaadin.ui.Label;
 import com.vaadin.ui.Layout;
+import com.vaadin.ui.Notification;
 import com.vaadin.ui.VerticalLayout;
 
 public class Html5FileDragAndDropUpload extends AbstractTestUIWithLog {
 
+    private static final int FILE_SIZE_LIMIT = 1024 * 1024 * 5; // 5 MB
+
     @Override
     protected void setup(VaadinRequest request) {
 
-        Label dropArea = new Label("Drop files here");
-
-        FileDropTarget<Label> dropTarget = new FileDropTarget<>(dropArea,
-                event -> {
-                    List<Html5File> files = event.getFiles();
-                    if (files != null) {
-                        files.forEach(file -> {
-                            file.setStreamVariable(new StreamVariable() {
-                                @Override
-                                public OutputStream getOutputStream() {
-                                    return new OutputStream() {
-                                        @Override
-                                        public void write(int b) throws
-                                                IOException {
-                                            // NOP
-                                        }
-                                    };
-                                }
-
-                                @Override
-                                public boolean listenProgress() {
-                                    return true;
-                                }
-
-                                @Override
-                                public void onProgress(
-                                        StreamingProgressEvent event) {
-                                    log("Progress, bytesReceived=" + event
-                                            .getBytesReceived());
-                                }
-
-                                @Override
-                                public void streamingStarted(
-                                        StreamingStartEvent event) {
-                                    log("Stream started, fileName=" + event
-                                            .getFileName());
-                                }
-
-                                @Override
-                                public void streamingFinished(
-                                        StreamingEndEvent event) {
-                                    log("Stream finished, fileName=" + event
-                                            .getFileName());
-                                }
-
-                                @Override
-                                public void streamingFailed(
-                                        StreamingErrorEvent event) {
-                                    log("Stream failed, fileName=" + event
-                                            .getFileName());
-                                }
-
-                                @Override
-                                public boolean isInterrupted() {
-                                    return false;
-                                }
-                            });
-                        });
-                    }
-                });
-
-        Grid<FileParameters> grid = new Grid<>();
+        Grid<FileParameters> grid = new Grid<>("Drop files on the Grid");
         grid.addColumn(FileParameters::getName).setCaption("File name");
         grid.addColumn(FileParameters::getSize).setCaption("File size");
         grid.addColumn(FileParameters::getMime).setCaption("Mime type");
@@ -106,6 +48,12 @@ public class Html5FileDragAndDropUpload extends AbstractTestUIWithLog {
 
         new FileDropTarget<Grid<FileParameters>>(grid, event -> {
             event.getFiles().forEach(html5File -> {
+                if (html5File.getFileSize() > FILE_SIZE_LIMIT) {
+                    Notification.show(html5File.getFileName()
+                            + " is too large (max 5 MB)");
+                    return;
+                }
+
                 html5File.setStreamVariable(new StreamVariable() {
                     @Override
                     public OutputStream getOutputStream() {
@@ -119,17 +67,18 @@ public class Html5FileDragAndDropUpload extends AbstractTestUIWithLog {
 
                     @Override
                     public boolean listenProgress() {
-                        return false;
+                        return true;
                     }
 
                     @Override
                     public void onProgress(StreamingProgressEvent event) {
-                        // NOP
+                        log("Progress, bytesReceived=" + event
+                                .getBytesReceived());
                     }
 
                     @Override
                     public void streamingStarted(StreamingStartEvent event) {
-                        // NOP
+                        log("Stream started, fileName=" + event.getFileName());
                     }
 
                     @Override
@@ -137,11 +86,13 @@ public class Html5FileDragAndDropUpload extends AbstractTestUIWithLog {
                         gridItems.add(new FileParameters(event.getFileName(),
                                 event.getContentLength(), event.getMimeType()));
                         grid.setItems(gridItems);
+
+                        log("Stream finished, fileName=" + event.getFileName());
                     }
 
                     @Override
                     public void streamingFailed(StreamingErrorEvent event) {
-
+                        log("Stream failed, fileName=" + event.getFileName());
                     }
 
                     @Override
@@ -152,13 +103,13 @@ public class Html5FileDragAndDropUpload extends AbstractTestUIWithLog {
             });
         });
 
-        Layout layout = new VerticalLayout(dropArea, grid);
+        Layout layout = new VerticalLayout(grid);
 
         addComponent(layout);
     }
 
     @Override
     protected String getTestDescription() {
-        return "Drop and upload files onto file drop target";
+        return "Drop files onto the Grid to upload them";
     }
 }