Skip to content

Commit

Permalink
Fix: Prevent NPE with autoUpload=false and multiple files are being u…
Browse files Browse the repository at this point in the history
…ploaded (#919)

Fixes: vaadin/vaadin-upload#358
  • Loading branch information
tulioag committed May 7, 2021
1 parent 1088c16 commit d2ffb7d
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@
<groupId>com.vaadin</groupId>
<artifactId>flow-polymer-template</artifactId>
</dependency>
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
<scope>provided</scope>
</dependency>
</dependencies>
<build>
<plugins>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.vaadin.flow.component.upload.tests.it;

import com.vaadin.flow.component.AttachEvent;
import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.html.Span;
import com.vaadin.flow.component.upload.Upload;
import com.vaadin.flow.component.upload.receivers.MultiFileMemoryBuffer;
import com.vaadin.flow.router.Route;

@Route("vaadin-upload/non-immediate-upload")
public class NonImmediateUploadView extends Div {

private Span result = new Span("No errors");

public NonImmediateUploadView() {
result.setId("error-handler-message");

MultiFileMemoryBuffer buffer = new MultiFileMemoryBuffer();
Upload upload = new Upload(buffer);
upload.setAutoUpload(false);
add(upload, result);
}

@Override
public void onAttach(AttachEvent attachEvent) {
attachEvent.getUI().getSession().setErrorHandler(event -> {
result.setText("There was an error");
event.getThrowable().printStackTrace();
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package com.vaadin.flow.component.upload.tests;

import java.io.File;

import org.junit.Assert;
import org.junit.AssumptionViolatedException;
import org.junit.Before;
import org.junit.Test;
import org.openqa.selenium.WebElement;

import com.vaadin.flow.component.upload.testbench.UploadElement;
import com.vaadin.flow.testutil.TestPath;
import com.vaadin.testbench.TestBenchElement;

@TestPath("vaadin-upload/non-immediate-upload")
public class NonImmediateUploadIT extends AbstractUploadIT {

@Before
public void init() {
if (getRunLocallyBrowser() == null) {
// Multiple file upload does not work with Remotewebdriver
// and autoUpload=false
// Related to
// https://github.com/SeleniumHQ/selenium/issues/7408
throw new AssumptionViolatedException(
"Skipped <Multiple file upload does not work with Remotewebdriver>");
}
}

@Test
public void uploadMultipleFiles_shouldNotThrowException_onStart()
throws Exception {
uploadMultipleFiles_shouldNotThrowException("start-button");
}

@Test
public void uploadMultipleFiles_shouldNotThrowException_onRemove()
throws Exception {
uploadMultipleFiles_shouldNotThrowException("clear-button");
}

private void uploadMultipleFiles_shouldNotThrowException(String buttonType)
throws Exception {
open();
File file1 = createTempFile();
File file2 = createTempFile();
UploadElement upload = $(UploadElement.class).waitForFirst();
WebElement input = getInput(upload);
fillPathToUploadInput(input, file1.getPath(), file2.getPath());
WebElement button = findButtonInVaadinUploadFile(upload, buttonType);
button.click();
TestBenchElement element = $("span").id("error-handler-message");
Assert.assertEquals("No errors", element.getText());
}

private WebElement findButtonInVaadinUploadFile(UploadElement upload,
String buttonType) {
final String QUERY = String.format(
"return arguments[0]"
+ ".shadowRoot.querySelector('vaadin-upload-file')"
+ ".shadowRoot.querySelector('[part=\"%s\"]')",
buttonType);
return (WebElement) getCommandExecutor().executeScript(QUERY, upload);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,12 @@ public Upload() {
DomEventListener allFinishedListener = e -> {
JsonArray files = e.getEventData().getArray(elementFiles);

boolean isUploading = IntStream.range(0, files.length()).anyMatch(
index -> files.getObject(index).getBoolean("uploading"));
boolean isUploading = IntStream.range(0, files.length())
.anyMatch(index -> {
final String KEY = "uploading";
JsonObject object = files.getObject(index);
return object.hasKey(KEY) && object.getBoolean(KEY);
});

if (this.uploading && !isUploading) {
this.fireAllFinish();
Expand Down

0 comments on commit d2ffb7d

Please sign in to comment.