Skip to content

Commit

Permalink
Fix: FileBuffer getInputStream is closed (#405) (#413)
Browse files Browse the repository at this point in the history
Warranty: Fixes FileBuffer getInputStream is closed
Fixes: vaadin/vaadin-upload#334
  • Loading branch information
Tulio Garcia authored Nov 20, 2020
1 parent 8ba9bdd commit 21c8d43
Show file tree
Hide file tree
Showing 11 changed files with 353 additions and 52 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright 2000-2020 Vaadin Ltd.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/
package com.vaadin.flow.component.upload.tests.it;

import com.vaadin.flow.component.html.Div;
import com.vaadin.flow.component.upload.SucceededEvent;
import com.vaadin.flow.component.upload.Upload;
import com.vaadin.flow.component.upload.receivers.FileBuffer;
import com.vaadin.flow.component.upload.receivers.MultiFileBuffer;
import com.vaadin.flow.router.Route;
import org.apache.commons.io.IOUtils;

import java.io.IOException;
import java.io.InputStream;
import java.util.function.Function;

/**
* View for {@link Upload} tests using {@link FileBuffer} and {@link MultiFileBuffer}.
*
* @author Vaadin Ltd
*/
@Route("vaadin-upload/filebuffer")
public class FileBufferView extends Div {
public FileBufferView() {
add(createSingleFileUpload());
add(createMultiFileUpload());
}

private static Div createSingleFileUpload() {
final FileBuffer buffer = new FileBuffer();
final Upload singleFileUpload = new Upload(buffer);
singleFileUpload.setId("single-upload");
singleFileUpload.setMaxFiles(1);
return setupUploadSection(singleFileUpload,
e -> buffer.getInputStream());

}

private static Div createMultiFileUpload() {
final MultiFileBuffer buffer = new MultiFileBuffer();
final Upload multiFileUpload = new Upload(buffer);
multiFileUpload.setId("multi-upload");
return setupUploadSection(multiFileUpload,
e -> buffer.getInputStream(e.getFileName()));
}

private static Div setupUploadSection(Upload upload,
Function<SucceededEvent, InputStream> streamProvider) {
final String uploadId = upload.getId().orElse("");
final Div output = new Div();
output.setId(uploadId + "-output");
final Div eventsOutput = new Div();
eventsOutput.setId(uploadId + "-event-output");

upload.addSucceededListener(event -> {
try {
output.add(event.getFileName());
output.add(
IOUtils.toString(streamProvider.apply(event), "UTF-8"));
} catch (IOException e) {
throw new RuntimeException(e);
}
eventsOutput.add("-succeeded");
});

upload.addAllFinishedListener(event -> eventsOutput.add("-finished"));

return new Div(output, eventsOutput, upload);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package com.vaadin.flow.component.upload.tests;

import com.vaadin.tests.AbstractComponentIT;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.internal.WrapsElement;
import org.openqa.selenium.remote.LocalFileDetector;
import org.openqa.selenium.remote.RemoteWebElement;

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;

public abstract class AbstractUploadIT extends AbstractComponentIT {
/**
* @return The generated temp file handle
* @throws IOException
*/
File createTempFile() throws IOException {
File tempFile = File.createTempFile("TestFileUpload", ".txt");
BufferedWriter writer = new BufferedWriter(new FileWriter(tempFile));
writer.write(getTempFileContents());
writer.close();
tempFile.deleteOnExit();
return tempFile;
}

String getTempFileContents() {
return "This is a test file! Row 2 Row3";
}

void fillPathToUploadInput(WebElement input, String... tempFileNames)
throws Exception {
// create a valid path in upload input element. Instead of selecting a
// file by some file browsing dialog, we use the local path directly.
setLocalFileDetector(input);
input.sendKeys(String.join(System.lineSeparator(), tempFileNames));
}

private void setLocalFileDetector(WebElement element) throws Exception {
if (getRunLocallyBrowser() != null) {
return;
}

if (element instanceof WrapsElement) {
element = ((WrapsElement) element).getWrappedElement();
}
if (element instanceof RemoteWebElement) {
((RemoteWebElement) element)
.setFileDetector(new LocalFileDetector());
} else {
throw new IllegalArgumentException(
"Expected argument of type RemoteWebElement, received "
+ element.getClass().getName());
}
}

WebElement getInput(WebElement upload) {
return getInShadowRoot(upload, By.id("fileInput"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package com.vaadin.flow.component.upload.tests;

import com.vaadin.flow.component.upload.testbench.UploadElement;
import com.vaadin.flow.testutil.TestPath;
import org.junit.Assert;
import org.junit.AssumptionViolatedException;
import org.junit.Test;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;

import java.io.File;

/**
* Tests for Upload using FileBuffer and MultiFileBuffer.
*/
@TestPath("vaadin-upload/filebuffer")
public class FileBufferIT extends AbstractUploadIT {
@Test
public void testUploadAnyFile() throws Exception {
open();
final UploadElement upload = $(UploadElement.class).id("single-upload");
waitUntil(driver -> upload.isDisplayed());

File tempFile = createTempFile();
fillPathToUploadInput(getInput(upload),tempFile.getPath());

WebElement uploadOutput = getDriver().findElement(By.id("single-upload-output"));

String content = uploadOutput.getText();

String expectedContent = tempFile.getName() + getTempFileContents();

Assert.assertEquals("Upload content does not match expected",
expectedContent, content);
}

@Test
public void testUploadMultipleEventOrder() throws Exception {
if (getRunLocallyBrowser() == null) {
// Multiple file upload does not work with Remotewebdriver
// https://github.com/SeleniumHQ/selenium/issues/7408
throw new AssumptionViolatedException(
"Skipped <Multiple file upload does not work with Remotewebdriver>");
}
open();

final UploadElement upload = $(UploadElement.class).id("multi-upload");
waitUntil(driver -> upload.isDisplayed());

File tempFile = createTempFile();

fillPathToUploadInput(getInput(upload),tempFile.getPath(), tempFile.getPath(),
tempFile.getPath());

WebElement eventsOutput = getDriver()
.findElement(By.id("multi-upload-event-output"));

Assert.assertEquals("Upload event order does not match expected",
"-succeeded-succeeded-succeeded-finished",
eventsOutput.getText());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@
*/
package com.vaadin.flow.component.upload.tests;

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.util.List;
import java.util.logging.Level;

Expand All @@ -28,12 +25,8 @@
import org.junit.Test;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.internal.WrapsElement;
import org.openqa.selenium.logging.LogEntry;
import org.openqa.selenium.remote.LocalFileDetector;
import org.openqa.selenium.remote.RemoteWebElement;

import com.vaadin.tests.AbstractComponentIT;
import com.vaadin.flow.testutil.TestPath;

import static org.junit.Assert.assertThat;
Expand All @@ -42,7 +35,7 @@
* Upload component test class.
*/
@TestPath("vaadin-upload")
public class UploadIT extends AbstractComponentIT {
public class UploadIT extends AbstractUploadIT {

@Test
public void testUploadAnyFile() throws Exception {
Expand Down Expand Up @@ -135,30 +128,10 @@ public void i18nUploadTest() {
Assert.assertEquals("Перетащите файл сюда...", dropLabel.getText());
}

/**
* @return The generated temp file handle
* @throws IOException
*/
private File createTempFile() throws IOException {
File tempFile = File.createTempFile("TestFileUpload", ".txt");
BufferedWriter writer = new BufferedWriter(new FileWriter(tempFile));
writer.write(getTempFileContents());
writer.close();
tempFile.deleteOnExit();
return tempFile;
}

private String getTempFileContents() {
return "This is a test file! Row 2 Row3";
}

private void fillPathToUploadInput(String... tempFileNames)
throws Exception {
// create a valid path in upload input element. Instead of selecting a
// file by some file browsing dialog, we use the local path directly.
WebElement input = getInput();
setLocalFileDetector(input);
input.sendKeys(String.join(System.lineSeparator(), tempFileNames));
fillPathToUploadInput(getInput(),tempFileNames);
}

private WebElement getUpload() {
Expand All @@ -172,24 +145,8 @@ private WebElement getUpload() {
* @return actual upload button
*/
private WebElement getInput() {
return getInShadowRoot(getUpload(), By.id("fileInput"));
return getInput(getUpload());
}

private void setLocalFileDetector(WebElement element) throws Exception {
if (getRunLocallyBrowser() != null) {
return;
}

if (element instanceof WrapsElement) {
element = ((WrapsElement) element).getWrappedElement();
}
if (element instanceof RemoteWebElement) {
((RemoteWebElement) element)
.setFileDetector(new LocalFileDetector());
} else {
throw new IllegalArgumentException(
"Expected argument of type RemoteWebElement, received "
+ element.getClass().getName());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public AbstractFileBuffer(FileFactory factory) {
*/
protected FileOutputStream createFileOutputStream(String fileName) {
try {
return new FileOutputStream(factory.createFile(fileName));
return new UploadOutputStream(factory.createFile(fileName));
} catch (IOException e) {
getLogger().log(Level.SEVERE,
"Failed to create file output stream for: '" + fileName
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.vaadin.flow.component.upload.receivers;

import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.FileDescriptor;
import java.io.FileInputStream;
import java.io.FileOutputStream;
Expand Down Expand Up @@ -89,9 +90,9 @@ public FileDescriptor getFileDescriptor() {
*/
public InputStream getInputStream() {
if (file != null) {
final File path = file.getFile();
try {
return new FileInputStream(
((FileOutputStream) file.getOutputBuffer()).getFD());
return new FileInputStream(path);
} catch (IOException e) {
getLogger().log(Level.WARNING,
"Failed to create InputStream for: '" + getFileName()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.vaadin.flow.component.upload.receivers;

import java.io.File;
import java.io.OutputStream;
import java.io.Serializable;

Expand Down Expand Up @@ -65,4 +66,23 @@ public String getFileName() {
public OutputStream getOutputBuffer() {
return outputBuffer;
}

/**
*
* @return Temporary file containing the uploaded data.
* @throws NullPointerException if outputBuffer is null
* @throws UnsupportedOperationException if outputBuffer is not an {@link UploadOutputStream}
*/
File getFile() {
if (outputBuffer == null) {
throw new NullPointerException("OutputBuffer is null");
}
if (outputBuffer instanceof UploadOutputStream) {
return ((UploadOutputStream) outputBuffer).getFile();
}
final String MESSAGE = String
.format("%s not supported. Use a UploadOutputStream",
outputBuffer.getClass());
throw new UnsupportedOperationException(MESSAGE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,7 @@ public FileDescriptor getFileDescriptor(String fileName) {
public InputStream getInputStream(String fileName) {
if (files.containsKey(fileName)) {
try {
return new FileInputStream(
((FileOutputStream) files.get(fileName)
.getOutputBuffer()).getFD());
return new FileInputStream(files.get(fileName).getFile());
} catch (IOException e) {
getLogger().log(Level.WARNING,
"Failed to create InputStream for: '" + fileName + "'",
Expand Down
Loading

0 comments on commit 21c8d43

Please sign in to comment.