From bb9473e77c00f1fdda9eb4fdbbbeefbf985a3e8f Mon Sep 17 00:00:00 2001
From: Willem Verstraeten <willem.verstraeten@gmail.com>
Date: Mon, 21 Oct 2019 19:15:23 +0200
Subject: Make cancellation of uploads work regardless of Push configuration
 (#11743)

- Checking the push configuration outside of session lock threw
  an AssertionError, so the push configuration is not checked anymore.

- The original problem with cancelling Upload was due to a subtle
  ordering issue that depended on the Push configuration.
  In the case of PushMode.AUTOMATIC, a new StreamVariable was
  added by the `Upload` component _before_ the `FileUploadHandler`
  got a chance to remove the old `StreamVariable`. As a result, the
  `FileUploadHandler` actually removed the fresh `StreamVariable`,
  breaking future uploads.

Fixes #11682
---
 .../server/communication/FileUploadHandler.java    | 23 +++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

(limited to 'server')

diff --git a/server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java b/server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java
index 64e0df1b06..bd20bea9ed 100644
--- a/server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java
+++ b/server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java
@@ -621,11 +621,7 @@ public class FileUploadHandler implements RequestHandler {
             } finally {
                 session.unlock();
             }
-            boolean pushEnabled = UI.getCurrent().getPushConfiguration()
-                    .getPushMode().isEnabled();
-            if (!pushEnabled) {
-                return true;
-            }
+            return true;
 
             // Note, we are not throwing interrupted exception forward as it is
             // not a terminal level error like all other exception.
@@ -704,7 +700,20 @@ public class FileUploadHandler implements RequestHandler {
 
     private void cleanStreamVariable(VaadinSession session, final UI ui,
             final ClientConnector owner, final String variableName) {
-        session.accessSynchronously(() -> ui.getConnectorTracker()
-                .cleanStreamVariable(owner.getConnectorId(), variableName));
+        session.accessSynchronously(() -> {
+            ui.getConnectorTracker().cleanStreamVariable(owner.getConnectorId(),
+                    variableName);
+
+            // in case of automatic push mode, the client connector
+            // could already have refreshed its StreamVariable
+            // in the ConnectorTracker. For instance, the Upload component
+            // adds its stream variable in its paintContent method, which is
+            // called (indirectly) on each session unlock in case of automatic
+            // pushes.
+            // To cover this case, mark the client connector as dirty, so that
+            // the unlock after this runnable refreshes the StreamVariable
+            // again.
+            owner.markAsDirty();
+        });
     }
 }
-- 
cgit v1.2.3