Use Apache Fileupload streaming API
The Apache Commons Fileupload library offers two different APIs, a
traditional one that stores uploads in temp files before making them
available, and a streaming API that provides direct access to the
upload stream.
The streaming API has many benefits for this add-on:
* it fits better with the concept of a receiver that handles uploaded
contents
* it makes this add-on independent from the filesystem
* it does not require cleaning up temporary files, we can get rid of
the CleaningTracker
* it's faster because we don't need to copy streams twice
As a side effect of this change, it's not directly possible to support
the method FileDetails.getContentLength() anymore. However, returning
-1 for "unknown" is in conformance with the contract of this method.
diff --git a/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/CleaningTracker.java b/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/CleaningTracker.java
deleted file mode 100644
index 00e4fe9..0000000
--- a/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/CleaningTracker.java
+++ /dev/null
@@ -1,87 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2013 EclipseSource and others.
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- *
- * Contributors:
- * EclipseSource - initial API and implementation
- ******************************************************************************/
-package org.eclipse.rap.addons.fileupload.internal;
-
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.commons.io.FileCleaningTracker;
-import org.apache.commons.io.FileDeleteStrategy;
-
-/*
- * Simpler file cleaning tracker implementation due to bug
- *
- * FileCleaningTracker Reaper thread not ended
- * https://issues.apache.org/jira/browse/FILEUPLOAD-205
- */
-public class CleaningTracker extends FileCleaningTracker {
-
- private final List<String> filesToDelete;
- private final List<String> deleteFailures;
-
- public CleaningTracker() {
- filesToDelete = new ArrayList<String>();
- deleteFailures = new ArrayList<String>();
- }
-
- @Override
- public void track( File file, Object marker ) {
- track( file, marker, null );
- }
-
- @Override
- public void track( File file, Object marker, FileDeleteStrategy deleteStrategy ) {
- if( file == null ) {
- throw new NullPointerException( "The file must not be null" );
- }
- filesToDelete.add( file.getAbsolutePath() );
- }
-
- @Override
- public void track( String path, Object marker ) {
- track( path, marker, null );
- }
-
- @Override
- public void track( String path, Object marker, FileDeleteStrategy deleteStrategy ) {
- if( path == null ) {
- throw new NullPointerException( "The path must not be null" );
- }
- filesToDelete.add( path );
- }
-
- @Override
- public int getTrackCount() {
- return filesToDelete.size();
- }
-
- @Override
- public List<String> getDeleteFailures() {
- return deleteFailures;
- }
-
- @Override
- public void exitWhenFinished() {
- deleteTemporaryFiles();
- }
-
- public void deleteTemporaryFiles() {
- for( String path : filesToDelete ) {
- File file = new File( path );
- if( !file.delete() ) {
- deleteFailures.add( path );
- }
- }
- filesToDelete.clear();
- }
-
-}
diff --git a/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/FileUploadProcessor.java b/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/FileUploadProcessor.java
index 10a59fd..d9f84a8 100644
--- a/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/FileUploadProcessor.java
+++ b/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/FileUploadProcessor.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011, 2013 EclipseSource and others.
+ * Copyright (c) 2011, 2014 EclipseSource and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -11,17 +11,15 @@
package org.eclipse.rap.addons.fileupload.internal;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
+import java.io.InputStream;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import org.apache.commons.fileupload.FileItem;
import org.apache.commons.fileupload.FileUploadBase.FileSizeLimitExceededException;
-import org.apache.commons.fileupload.FileUploadException;
+import org.apache.commons.fileupload.FileItemIterator;
+import org.apache.commons.fileupload.FileItemStream;
import org.apache.commons.fileupload.ProgressListener;
-import org.apache.commons.fileupload.disk.DiskFileItemFactory;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.eclipse.rap.addons.fileupload.FileDetails;
import org.eclipse.rap.addons.fileupload.FileUploadHandler;
@@ -32,68 +30,48 @@
private final FileUploadHandler handler;
private final FileUploadTracker tracker;
- private final CleaningTracker cleaningTracker;
FileUploadProcessor( FileUploadHandler handler ) {
this.handler = handler;
tracker = new FileUploadTracker( handler );
- cleaningTracker = new CleaningTracker();
}
void handleFileUpload( HttpServletRequest request, HttpServletResponse response )
throws IOException
{
try {
- List<FileItem> fileItems = readUploadedFileItems( request );
- if( !fileItems.isEmpty() ) {
- FileUploadReceiver receiver = handler.getReceiver();
- for( FileItem fileItem : fileItems ) {
- String fileName = stripFileName( fileItem.getName() );
- String contentType = fileItem.getContentType();
- long contentLength = fileItem.getSize();
- FileDetails details = new FileDetailsImpl( fileName, contentType, contentLength );
- receiver.receive( fileItem.getInputStream(), details );
- tracker.addFile( details );
+ ServletFileUpload upload = createUpload();
+ FileItemIterator iter = upload.getItemIterator( request );
+ while( iter.hasNext() ) {
+ FileItemStream item = iter.next();
+ if( !item.isFormField() ) {
+ receive( item );
}
- tracker.handleFinished();
- } else {
+ }
+ if( tracker.isEmpty() ) {
String errorMessage = "No file upload data found in request";
tracker.setException( new Exception( errorMessage ) );
tracker.handleFailed();
response.sendError( HttpServletResponse.SC_BAD_REQUEST, errorMessage );
+ } else {
+ tracker.handleFinished();
}
- } catch( FileSizeLimitExceededException exception ) {
- tracker.setException( exception );
- tracker.handleFailed();
- response.sendError( HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, exception.getMessage() );
} catch( Exception exception ) {
+ Throwable cause = exception.getCause();
+ if( cause instanceof FileSizeLimitExceededException ) {
+ exception = ( Exception )cause;
+ }
tracker.setException( exception );
tracker.handleFailed();
- response.sendError( HttpServletResponse.SC_INTERNAL_SERVER_ERROR, exception.getMessage() );
+ int errorCode = exception instanceof FileSizeLimitExceededException
+ ? HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE
+ : HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+ response.sendError( errorCode, exception.getMessage() );
}
- cleaningTracker.deleteTemporaryFiles();
- }
-
- private List<FileItem> readUploadedFileItems( HttpServletRequest request )
- throws FileUploadException
- {
- List<FileItem> result = new ArrayList<FileItem>();
- ServletFileUpload upload = createUpload();
- List uploadedItems = upload.parseRequest( request );
- for( Object uploadedItem : uploadedItems ) {
- FileItem fileItem = ( FileItem )uploadedItem;
- // Don't check for file size == 0 because this would prevent uploading empty files
- if( !fileItem.isFormField() ) {
- result.add( fileItem );
- }
- }
- return result;
}
private ServletFileUpload createUpload() {
- DiskFileItemFactory factory = new DiskFileItemFactory();
- factory.setFileCleaningTracker( cleaningTracker );
- ServletFileUpload upload = new ServletFileUpload( factory );
+ ServletFileUpload upload = new ServletFileUpload();
upload.setFileSizeMax( handler.getMaxFileSize() );
upload.setProgressListener( createProgressListener() );
return upload;
@@ -117,6 +95,20 @@
return result;
}
+ private void receive( FileItemStream item ) throws IOException {
+ InputStream stream = item.openStream();
+ try {
+ String fileName = stripFileName( item.getName() );
+ String contentType = item.getContentType();
+ FileDetails details = new FileDetailsImpl( fileName, contentType, -1 );
+ FileUploadReceiver receiver = handler.getReceiver();
+ receiver.receive( stream, details );
+ tracker.addFile( details );
+ } finally {
+ stream.close();
+ }
+ }
+
private static String stripFileName( String name ) {
String result = name;
int lastSlash = result.lastIndexOf( '/' );
diff --git a/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/FileUploadTracker.java b/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/FileUploadTracker.java
index 3745173..f7a5601 100644
--- a/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/FileUploadTracker.java
+++ b/bundles/org.eclipse.rap.addons.fileupload/src/org/eclipse/rap/addons/fileupload/internal/FileUploadTracker.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011, 2013 EclipseSource and others.
+ * Copyright (c) 2011, 2014 EclipseSource and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -35,6 +35,10 @@
files.add( details );
}
+ boolean isEmpty() {
+ return files.isEmpty();
+ }
+
void setContentLength( long contentLength ) {
this.contentLength = contentLength;
}
diff --git a/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/FileUploadHandler_Test.java b/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/FileUploadHandler_Test.java
index 6218397..e8b9600 100644
--- a/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/FileUploadHandler_Test.java
+++ b/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/FileUploadHandler_Test.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2011, 2013 EclipseSource and others.
+ * Copyright (c) 2011, 2014 EclipseSource and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -10,23 +10,12 @@
******************************************************************************/
package org.eclipse.rap.addons.fileupload;
-import static org.eclipse.rap.rwt.internal.service.ContextProvider.getApplicationContext;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
import java.io.IOException;
import java.io.InputStream;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletResponse;
-import org.eclipse.rap.addons.fileupload.FileUploadHandler;
-import org.eclipse.rap.addons.fileupload.FileUploadReceiver;
-import org.eclipse.rap.addons.fileupload.FileDetails;
import org.eclipse.rap.addons.fileupload.internal.FileUploadHandlerStore;
import org.eclipse.rap.addons.fileupload.internal.FileUploadServiceHandler;
import org.eclipse.rap.addons.fileupload.test.FileUploadTestUtil;
@@ -41,6 +30,17 @@
import org.junit.Before;
import org.junit.Test;
+import static org.eclipse.rap.rwt.internal.service.ContextProvider.getApplicationContext;
+
+import static org.hamcrest.CoreMatchers.containsString;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+
@SuppressWarnings( "restriction" )
public class FileUploadHandler_Test {
@@ -77,7 +77,7 @@
@Test
public void testInitialized() {
- assertTrue( handler.getUploadUrl().indexOf( handler.getToken() ) != -1 );
+ assertThat( handler.getUploadUrl(), containsString( handler.getToken() ) );
assertSame( handler, getRegisteredHandler( handler.getToken() ) );
}
@@ -225,7 +225,7 @@
serviceHandler.service( ContextProvider.getRequest(), ContextProvider.getResponse() );
assertEquals( HttpServletResponse.SC_REQUEST_ENTITY_TOO_LARGE, getResponseErrorStatus() );
- assertTrue( getResponseContent().indexOf( "file exceeds its maximum permitted size" ) != -1 );
+ assertThat( getResponseContent(), containsString( "file exceeds its maximum permitted size" ) );
}
@Test
@@ -242,7 +242,7 @@
serviceHandler.service( ContextProvider.getRequest(), ContextProvider.getResponse() );
assertEquals( HttpServletResponse.SC_INTERNAL_SERVER_ERROR, getResponseErrorStatus() );
- assertTrue( getResponseContent().indexOf( "the error message" ) != -1 );
+ assertThat( getResponseContent(), containsString( "the error message" ) );
}
@Test
@@ -278,4 +278,5 @@
TestResponse response = ( TestResponse )ContextProvider.getResponse();
return response.getContent();
}
+
}
diff --git a/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/internal/CleaningTracker_Test.java b/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/internal/CleaningTracker_Test.java
deleted file mode 100644
index 878df6b..0000000
--- a/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/internal/CleaningTracker_Test.java
+++ /dev/null
@@ -1,123 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2013 EclipseSource and others.
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- *
- * Contributors:
- * EclipseSource - initial API and implementation
- ******************************************************************************/
-package org.eclipse.rap.addons.fileupload.internal;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.mock;
-
-import java.io.File;
-import java.io.IOException;
-
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
-
-
-public class CleaningTracker_Test {
-
- private File tempFile;
- private CleaningTracker cleaningTracker;
-
- @Before
- public void setUp() throws IOException {
- tempFile = File.createTempFile( "temp-", null );
- cleaningTracker = new CleaningTracker();
- }
-
- @After
- public void tearDown() {
- tempFile.delete();
- }
-
- @Test( expected = NullPointerException.class )
- public void testTrack_nullFile() {
- cleaningTracker.track( ( File )null, null );
- }
-
- @Test( expected = NullPointerException.class )
- public void testTrack_nullPath() {
- cleaningTracker.track( ( String )null, null );
- }
-
- @Test
- public void testDeleteTemporaryFiles_trackedByFile_1() {
- cleaningTracker.track( tempFile, null );
-
- cleaningTracker.deleteTemporaryFiles();
-
- assertFalse( tempFile.exists() );
- assertEquals( 0, cleaningTracker.getDeleteFailures().size() );
- }
-
- @Test
- public void testDeleteTemporaryFiles_trackedByFile_2() {
- cleaningTracker.track( tempFile, null, null );
-
- cleaningTracker.deleteTemporaryFiles();
-
- assertFalse( tempFile.exists() );
- assertEquals( 0, cleaningTracker.getDeleteFailures().size() );
- }
-
- @Test
- public void testDeleteTemporaryFiles_trackedByPath_1() {
- cleaningTracker.track( tempFile.getAbsolutePath(), null );
-
- cleaningTracker.deleteTemporaryFiles();
-
- assertFalse( tempFile.exists() );
- assertEquals( 0, cleaningTracker.getDeleteFailures().size() );
- }
-
- @Test
- public void testDeleteTemporaryFiles_trackedByPath_2() {
- cleaningTracker.track( tempFile.getAbsolutePath(), null, null );
-
- cleaningTracker.deleteTemporaryFiles();
-
- assertFalse( tempFile.exists() );
- assertEquals( 0, cleaningTracker.getDeleteFailures().size() );
- }
-
- @Test
- public void testExitWhenFinished_deletesTemporaryFiles() {
- cleaningTracker.track( tempFile, null );
-
- cleaningTracker.exitWhenFinished();
-
- assertFalse( tempFile.exists() );
- assertEquals( 0, cleaningTracker.getDeleteFailures().size() );
- }
-
- @Test
- public void testGetTrackCount() {
- cleaningTracker.track( tempFile, null );
-
- assertEquals( 1, cleaningTracker.getTrackCount() );
- }
-
- @Test
- public void testGetDeleteFailures() {
- String path = tempFile.getAbsolutePath();
- tempFile.delete();
- tempFile = mock( File.class );
- doReturn( Boolean.FALSE ).when( tempFile ).delete();
- doReturn( path ).when( tempFile ).getAbsolutePath();
- cleaningTracker.track( tempFile, null );
-
- cleaningTracker.deleteTemporaryFiles();
-
- assertEquals( 1, cleaningTracker.getDeleteFailures().size() );
- }
-
-}
diff --git a/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/internal/FileUploadProcessor_Test.java b/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/internal/FileUploadProcessor_Test.java
index fbdb181..1158483 100644
--- a/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/internal/FileUploadProcessor_Test.java
+++ b/tests/org.eclipse.rap.addons.fileupload.test/src/org/eclipse/rap/addons/fileupload/internal/FileUploadProcessor_Test.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2013 EclipseSource and others.
+ * Copyright (c) 2013, 2014 EclipseSource and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
@@ -13,6 +13,8 @@
import static org.eclipse.rap.addons.fileupload.test.FileUploadTestUtil.fakeUploadRequest;
import static org.junit.Assert.assertEquals;
import static org.mockito.Matchers.any;
+
+import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
@@ -30,6 +32,7 @@
import org.eclipse.rap.addons.fileupload.FileUploadReceiver;
import org.eclipse.rap.addons.fileupload.test.FileUploadTestUtil.FileData;
import org.eclipse.rap.addons.fileupload.test.TestFileUploadListener;
+import org.eclipse.rap.addons.fileupload.test.TestFileUploadReceiver;
import org.eclipse.rap.rwt.RWT;
import org.eclipse.rap.rwt.testfixture.Fixture;
import org.eclipse.rap.rwt.testfixture.TestResponse;
@@ -37,6 +40,8 @@
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
public class FileUploadProcessor_Test {
@@ -82,7 +87,7 @@
FileDetails[] fileDetails = event.getFileDetails();
assertEquals( "foo.txt", fileDetails[ 0 ].getFileName() );
assertEquals( "text/plain", fileDetails[ 0 ].getContentType() );
- assertEquals( 3, fileDetails[ 0 ].getContentLength() );
+ assertEquals( -1, fileDetails[ 0 ].getContentLength() );
assertEquals( event.getContentLength(), event.getBytesRead() );
}
@@ -94,7 +99,7 @@
ArgumentCaptor<FileDetails> captor = ArgumentCaptor.forClass( FileDetails.class );
verify( receiver ).receive( any( InputStream.class ), captor.capture() );
FileDetails uploadDetails = captor.getValue();
- assertEquals( 3, uploadDetails.getContentLength() );
+ assertEquals( -1, uploadDetails.getContentLength() );
assertEquals( "text/plain", uploadDetails.getContentType() );
assertEquals( "foo.txt", uploadDetails.getFileName() );
}
@@ -124,10 +129,10 @@
FileDetails[] fileDetails = event.getFileDetails();
assertEquals( "foo.txt", fileDetails[ 0 ].getFileName() );
assertEquals( "text/plain", fileDetails[ 0 ].getContentType() );
- assertEquals( 3, fileDetails[ 0 ].getContentLength() );
+ assertEquals( -1, fileDetails[ 0 ].getContentLength() );
assertEquals( "bar.png", fileDetails[ 1 ].getFileName() );
assertEquals( "image/png", fileDetails[ 1 ].getContentType() );
- assertEquals( 3, fileDetails[ 1 ].getContentLength() );
+ assertEquals( -1, fileDetails[ 1 ].getContentLength() );
assertEquals( event.getContentLength(), event.getBytesRead() );
}
@@ -141,10 +146,10 @@
ArgumentCaptor<FileDetails> captor = ArgumentCaptor.forClass( FileDetails.class );
verify( receiver, times( 2 ) ).receive( any( InputStream.class ), captor.capture() );
List<FileDetails> values = captor.getAllValues();
- assertEquals( 3, values.get( 0 ).getContentLength() );
+ assertEquals( -1, values.get( 0 ).getContentLength() );
assertEquals( "text/plain", values.get( 0 ).getContentType() );
assertEquals( "foo.txt", values.get( 0 ).getFileName() );
- assertEquals( 3, values.get( 1 ).getContentLength() );
+ assertEquals( -1, values.get( 1 ).getContentLength() );
assertEquals( "image/png", values.get( 1 ).getContentType() );
assertEquals( "bar.png", values.get( 1 ).getFileName() );
}
@@ -164,6 +169,7 @@
public void testHandleFileUpload_fileExceedsMaxSize() throws IOException {
uploadHandler.setMaxFileSize( 5 );
uploadHandler.addUploadListener( testListener );
+ stubReceiveMethod( receiver );
FileData file1 = new FileData( "foo", "text/plain", "foo.txt" );
FileData file2 = new FileData( "bar bar", "image/png", "bar.png" );
@@ -187,6 +193,16 @@
assertEquals( HttpServletResponse.SC_INTERNAL_SERVER_ERROR, getResponseErrorStatus() );
}
+ private static void stubReceiveMethod( FileUploadReceiver receiver ) throws IOException {
+ Answer answer = new Answer() {
+ public Object answer( InvocationOnMock invocation ) throws Throwable {
+ new TestFileUploadReceiver().receive( ( InputStream )invocation.getArguments()[0], null );
+ return null;
+ }
+ };
+ doAnswer( answer ).when( receiver ).receive( any( InputStream.class ), any( FileDetails.class ) );
+ }
+
private static int getResponseErrorStatus() {
TestResponse response = ( TestResponse )RWT.getResponse();
return response.getErrorStatus();
