Browse Source

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
tags/8.10.0.alpha1
Willem Verstraeten 4 years ago
parent
commit
bb9473e77c

+ 16
- 7
server/src/main/java/com/vaadin/server/communication/FileUploadHandler.java View File

} finally { } finally {
session.unlock(); 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 // Note, we are not throwing interrupted exception forward as it is
// not a terminal level error like all other exception. // not a terminal level error like all other exception.


private void cleanStreamVariable(VaadinSession session, final UI ui, private void cleanStreamVariable(VaadinSession session, final UI ui,
final ClientConnector owner, final String variableName) { 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();
});
} }
} }

+ 27
- 3
uitest/src/main/java/com/vaadin/tests/components/upload/InterruptUpload.java View File



private Upload sample; private Upload sample;
private UploadInfoWindow uploadInfoWindow; private UploadInfoWindow uploadInfoWindow;
private final boolean pushManually;

public InterruptUpload() {
this(false);
}

public InterruptUpload(boolean pushManually) {
this.pushManually = pushManually;
}


@Override @Override
protected void setup(VaadinRequest request) { protected void setup(VaadinRequest request) {
UI.getCurrent().addWindow(uploadInfoWindow); UI.getCurrent().addWindow(uploadInfoWindow);
} }
uploadInfoWindow.setClosable(false); uploadInfoWindow.setClosable(false);
pushIfManual();
});
sample.addFinishedListener(event -> {
uploadInfoWindow.setClosable(true);
pushIfManual();
}); });
sample.addFinishedListener(event -> uploadInfoWindow.setClosable(true));


addComponent(sample);
addComponents(new Label(getDescription()), sample);
} }


private static class UploadInfoWindow extends Window
private class UploadInfoWindow extends Window
implements Upload.StartedListener, Upload.ProgressListener, implements Upload.StartedListener, Upload.ProgressListener,
Upload.FailedListener, Upload.SucceededListener, Upload.FailedListener, Upload.SucceededListener,
Upload.FinishedListener { Upload.FinishedListener {
textualProgress.setVisible(false); textualProgress.setVisible(false);
cancelButton.setVisible(false); cancelButton.setVisible(false);
UI.getCurrent().setPollInterval(-1); UI.getCurrent().setPollInterval(-1);
pushIfManual();
} }


@Override @Override
fileName.setValue(event.getFilename()); fileName.setValue(event.getFilename());


cancelButton.setVisible(true); cancelButton.setVisible(true);
pushIfManual();
} }


@Override @Override
textualProgress.setValue( textualProgress.setValue(
"Processed " + readBytes + " bytes of " + contentLength); "Processed " + readBytes + " bytes of " + contentLength);
result.setValue(counter.getLineBreakCount() + " (counting...)"); result.setValue(counter.getLineBreakCount() + " (counting...)");
pushIfManual();
} }


@Override @Override
public void uploadSucceeded(final SucceededEvent event) { public void uploadSucceeded(final SucceededEvent event) {
result.setValue(counter.getLineBreakCount() + " (total)"); result.setValue(counter.getLineBreakCount() + " (total)");
pushIfManual();
} }


@Override @Override
result.setValue( result.setValue(
counter.getLineBreakCount() + " (counting interrupted at " counter.getLineBreakCount() + " (counting interrupted at "
+ Math.round(100 * progressBar.getValue()) + "%)"); + Math.round(100 * progressBar.getValue()) + "%)");
pushIfManual();
}
}

private void pushIfManual() {
if (pushManually) {
push();
} }
} }



+ 23
- 0
uitest/src/main/java/com/vaadin/tests/components/upload/InterruptUploadWithManualPush.java View File

package com.vaadin.tests.components.upload;

import com.vaadin.annotations.Push;
import com.vaadin.shared.communication.PushMode;

@Push(PushMode.MANUAL)
public class InterruptUploadWithManualPush extends InterruptUpload {

public InterruptUploadWithManualPush() {
super(true);
}

@Override
protected Integer getTicketNumber() {
return 11616;
}

@Override
public String getDescription() {
return "Interrupting an upload with @Push shouldn't prevent uploading that same file immediately afterwards.";
}

}

+ 42
- 3
uitest/src/main/java/com/vaadin/tests/dnd/Html5FileDragAndDropUpload.java View File

import java.io.OutputStream; import java.io.OutputStream;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;


import com.vaadin.server.StreamVariable; import com.vaadin.server.StreamVariable;
import com.vaadin.server.VaadinRequest; import com.vaadin.server.VaadinRequest;
import com.vaadin.shared.ui.dnd.FileParameters; import com.vaadin.shared.ui.dnd.FileParameters;
import com.vaadin.shared.ui.grid.DropMode; import com.vaadin.shared.ui.grid.DropMode;
import com.vaadin.tests.components.AbstractTestUIWithLog; import com.vaadin.tests.components.AbstractTestUIWithLog;
import com.vaadin.ui.Button;
import com.vaadin.ui.Grid; import com.vaadin.ui.Grid;
import com.vaadin.ui.Layout;
import com.vaadin.ui.Notification; import com.vaadin.ui.Notification;
import com.vaadin.ui.ProgressBar;
import com.vaadin.ui.UI;
import com.vaadin.ui.VerticalLayout; import com.vaadin.ui.VerticalLayout;
import com.vaadin.ui.components.grid.GridDropTarget; import com.vaadin.ui.components.grid.GridDropTarget;
import com.vaadin.ui.dnd.FileDropTarget; import com.vaadin.ui.dnd.FileDropTarget;


private static final int FILE_SIZE_LIMIT = 1024 * 1024 * 5; // 5 MB private static final int FILE_SIZE_LIMIT = 1024 * 1024 * 5; // 5 MB


private final boolean pushManually;

public Html5FileDragAndDropUpload() {
this(false);
}

public Html5FileDragAndDropUpload(boolean pushManually) {
this.pushManually = pushManually;
}

@Override @Override
protected void setup(VaadinRequest request) { protected void setup(VaadinRequest request) {


grid.addColumn(FileParameters::getMime).setCaption("Mime type"); grid.addColumn(FileParameters::getMime).setCaption("Mime type");


List<FileParameters> gridItems = new ArrayList<>(); List<FileParameters> gridItems = new ArrayList<>();
AtomicBoolean cancelled = new AtomicBoolean(false);

ProgressBar progressBar = new ProgressBar(0.0f);


new FileDropTarget<Grid<FileParameters>>(grid, new FileDropTarget<Grid<FileParameters>>(grid,
event -> event.getFiles().forEach(html5File -> { event -> event.getFiles().forEach(html5File -> {
+ " is too large (max 5 MB)"); + " is too large (max 5 MB)");
return; return;
} }
UI.getCurrent().setPollInterval(200);


html5File.setStreamVariable(new StreamVariable() { html5File.setStreamVariable(new StreamVariable() {
@Override @Override


@Override @Override
public void onProgress(StreamingProgressEvent event) { public void onProgress(StreamingProgressEvent event) {
float progress = (float) event.getBytesReceived()
/ event.getContentLength();
progressBar.setValue(progress);
log("Progress, bytesReceived=" log("Progress, bytesReceived="
+ event.getBytesReceived()); + event.getBytesReceived());
pushIfManual();
} }


@Override @Override
public void streamingStarted( public void streamingStarted(
StreamingStartEvent event) { StreamingStartEvent event) {
cancelled.set(false);
progressBar.setValue(0.0f);
log("Stream started, fileName=" log("Stream started, fileName="
+ event.getFileName()); + event.getFileName());
pushIfManual();
} }


@Override @Override
public void streamingFinished(StreamingEndEvent event) { public void streamingFinished(StreamingEndEvent event) {
progressBar.setValue(1.0f);
gridItems gridItems
.add(new FileParameters(event.getFileName(), .add(new FileParameters(event.getFileName(),
event.getContentLength(), event.getContentLength(),


log("Stream finished, fileName=" log("Stream finished, fileName="
+ event.getFileName()); + event.getFileName());
pushIfManual();
UI.getCurrent().setPollInterval(-1);
} }


@Override @Override
public void streamingFailed(StreamingErrorEvent event) { public void streamingFailed(StreamingErrorEvent event) {
progressBar.setValue(0.0f);
log("Stream failed, fileName=" log("Stream failed, fileName="
+ event.getFileName()); + event.getFileName());
pushIfManual();
UI.getCurrent().setPollInterval(-1);
} }


@Override @Override
public boolean isInterrupted() { public boolean isInterrupted() {
return false;
return cancelled.get();
} }
}); });
})); }));
Notification.show(event.getDataTransferText()); Notification.show(event.getDataTransferText());
}); });


Layout layout = new VerticalLayout(grid);
Button cancelButton = new Button("Cancel",
click -> cancelled.set(true));
VerticalLayout layout = new VerticalLayout(grid, cancelButton,
progressBar);


addComponent(layout); addComponent(layout);
} }


private void pushIfManual() {
if (pushManually) {
push();
}
}

@Override @Override
protected String getTestDescription() { protected String getTestDescription() {
return "Drop files onto the Grid to upload them or text"; return "Drop files onto the Grid to upload them or text";

+ 12
- 0
uitest/src/main/java/com/vaadin/tests/dnd/Html5FileDragAndDropUploadWithManualPush.java View File

package com.vaadin.tests.dnd;

import com.vaadin.annotations.Push;
import com.vaadin.shared.communication.PushMode;

@Push(PushMode.MANUAL)
public class Html5FileDragAndDropUploadWithManualPush
extends Html5FileDragAndDropUpload {
public Html5FileDragAndDropUploadWithManualPush() {
super(true);
}
}

+ 8
- 0
uitest/src/main/java/com/vaadin/tests/dnd/Html5FileDragAndDropUploadWithPush.java View File

package com.vaadin.tests.dnd;

import com.vaadin.annotations.Push;

@Push
public class Html5FileDragAndDropUploadWithPush
extends Html5FileDragAndDropUpload {
}

Loading…
Cancel
Save