Skip to content
This repository has been archived by the owner on Nov 16, 2017. It is now read-only.

RF-12224: FileUpload - initial support for multiple file selection #78

Closed
wants to merge 6 commits into from
Closed

RF-12224: FileUpload - initial support for multiple file selection #78

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
71c9ccc
RF-12224: FileUpload - initial support for multiple file selection
simone83 Jun 11, 2013
403de53
RF-12224: support hiding button on reaching maxFilesQuantity
simone83 Jun 14, 2013
6c4b153
RF-12224: multiple file upload - cleanup of sources
Jul 2, 2013
3e7eeb9
RF-12224: rejectedFileNames array emptiness check fix
Jul 3, 2013
1104f3e
RF-12224: make the file quantity checking thread-safe
Jul 26, 2013
e8ac096
RF-12224: support case of empty extension
Jul 26, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions input/api/src/main/java/org/richfaces/model/UploadedFile.java
View file
Open in desktop
Expand Up @@ -53,4 +53,11 @@ public interface UploadedFile {
Collection<String> getHeaders(String headerName);

String getParameterName();

/**
* Returns the files extension - the substring after last period of this file's name.
*
* If there is no period in file name, empty string is returned instead.
*/
String getFileExtension();
}
63 changes: 60 additions & 3 deletions input/ui/src/main/java/org/richfaces/component/AbstractFileUpload.java
View file
Open in desktop
Expand Up @@ -21,6 +21,9 @@
*/
package org.richfaces.component;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;

import javax.faces.FacesException;
Expand All @@ -29,6 +32,7 @@
import javax.faces.context.FacesContext;
import javax.faces.event.AbortProcessingException;
import javax.faces.event.ComponentSystemEvent;
import javax.faces.event.FacesEvent;
import javax.faces.event.ListenerFor;
import javax.faces.event.PostAddToViewEvent;

Expand All @@ -37,7 +41,9 @@
import org.richfaces.cdk.annotations.JsfComponent;
import org.richfaces.cdk.annotations.JsfRenderer;
import org.richfaces.cdk.annotations.Tag;
import org.richfaces.event.FileUploadEvent;
import org.richfaces.event.FileUploadListener;
import org.richfaces.model.UploadedFile;
import org.richfaces.renderkit.RenderKitUtils;

/**
Expand All @@ -50,9 +56,12 @@
"events-mouse-props.xml", "events-key-props.xml", "core-props.xml", "ajax-props.xml", "i18n-props.xml", "fileUploadListener-props.xml" })
@ListenerFor(systemEventClass = PostAddToViewEvent.class)
public abstract class AbstractFileUpload extends UIComponentBase {

public static final String COMPONENT_TYPE = "org.richfaces.FileUpload";
public static final String COMPONENT_FAMILY = "org.richfaces.FileUpload";

private int queuedFileUploadEvents = 0;

/**
* Defines comma separated list of file extensions accepted by component.
* The component does not provide any feedback when rejecting file.
Expand All @@ -64,10 +73,10 @@ public abstract class AbstractFileUpload extends UIComponentBase {
/**
* Defines maximum number of files allowed to be uploaded. After a number of files in the list equals to the value
* of this attribute, "Add" button disappears and nothing could be uploaded even if you clear the whole list.
* In order to upload files again you should rerender the component
* In order to upload files again you should rerender the component. (Negative numbers means no limits; default value -1).
*/
@Attribute
public abstract String getMaxFilesQuantity();
@Attribute(defaultValue = "-1")
public abstract Integer getMaxFilesQuantity();

/**
* If "true", this component is disabled
Expand Down Expand Up @@ -233,4 +242,52 @@ public FileUploadListener[] getFileUploadListeners() {
public void removeFileUploadListener(FileUploadListener listener) {
removeFacesListener(listener);
}

/**
* Get a list of accepted types from {@link #getAcceptedTypes()} attribute.
*/
public List<String> getAcceptedTypesList() {
String acceptedTypes = this.getAcceptedTypes();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value should be cached in the component using the StateHelper, something like:
String acceptedTypes = (String) getStateHelper().eval("acceptedTypes");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the we delegate to abstract method, which is implemented by UIFileUpload as:

public String getAcceptedTypes() {
        String value = (String) getStateHelper().eval(Properties.acceptedTypes);
        return value;
    }

if (acceptedTypes != null) {
return Arrays.asList(acceptedTypes.toLowerCase().replaceAll("\\s+", "").split(","));
} else {
return Collections.emptyList();
}
}

/**
* Checks whether this component can accept given {@link UploadedFile}..
*
* First, the number of enqueued {@link FileUploadEvent} events can't exceed {@link #getMaxFilesQuantity()}.
*
* Then, the file extension of uploaded file needs to be acceptable by this component (see {@link #getAcceptedTypes()}).
*/
public boolean acceptsFile(UploadedFile file) {
final String clientId = this.getClientId();
final int maxFilesQuantity = this.getMaxFilesQuantity();
final List<String> acceptedTypes = this.getAcceptedTypesList();

if ((maxFilesQuantity > 0) && (queuedFileUploadEvents >= maxFilesQuantity))
return false;

if (clientId.equals(file.getParameterName())) {
if (acceptedTypes.isEmpty()) {
return true;
}

if (acceptedTypes.contains(file.getFileExtension().toLowerCase())) {
return true;
}
}

return false;
}

@Override
public void queueEvent(FacesEvent event) {
if (event instanceof FileUploadEvent) {
queuedFileUploadEvents += 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any threading concerns here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, right, I did wrong assumption that JSF isn't multi-threaded.. ;-) going to fix that

}
super.queueEvent(event);
}
}
22 changes: 14 additions & 8 deletions input/ui/src/main/java/org/richfaces/renderkit/FileUploadRendererBase.java
View file
Open in desktop
Expand Up @@ -25,29 +25,35 @@
import javax.faces.context.ExternalContext;
import javax.faces.context.FacesContext;

import org.richfaces.component.AbstractFileUpload;
import org.richfaces.event.FileUploadEvent;
import org.richfaces.model.UploadedFile;
import org.richfaces.request.MultipartRequest;

/**
* @author Konstantin Mishin
* @author Nick Belaevski
* @author Lukas Fryc
* @author Simone Cinti
*
*/

public class FileUploadRendererBase extends RendererBase {

@Override
protected void doDecode(FacesContext context, UIComponent component) {
ExternalContext externalContext = context.getExternalContext();
final AbstractFileUpload fileUpload = (AbstractFileUpload) component;
final ExternalContext externalContext = context.getExternalContext();

MultipartRequest multipartRequest = (MultipartRequest) externalContext.getRequestMap().get(
MultipartRequest.REQUEST_ATTRIBUTE_NAME);
if (multipartRequest != null) {
String clientId = component.getClientId(context);
MultipartRequest.REQUEST_ATTRIBUTE_NAME);

if (multipartRequest != null) {
for (UploadedFile file : multipartRequest.getUploadedFiles()) {
if (clientId.equals(file.getParameterName())) {
component.queueEvent(new FileUploadEvent(component, file));
break;
if (fileUpload.acceptsFile(file)) {
fileUpload.queueEvent(new FileUploadEvent(fileUpload, file));
}
}
}
}
}
}
12 changes: 12 additions & 0 deletions input/ui/src/main/java/org/richfaces/request/BaseUploadedFile.java
View file
Open in desktop
Expand Up @@ -73,4 +73,16 @@ public byte[] getData() {
Closeables.closeQuietly(is);
}
}

@Override
public String getFileExtension() {
String name = this.getName();
if (name != null) {
int i = name.lastIndexOf('.');
if (i > 0) {
return name.substring(i + 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will we get an exception here for a file named: "emptyextension." ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I've been tested it and it will throw an exception if you try to upload a file with empty extension. Right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An empty file extension is legal on unix-like OS'es, and may be valid for certain use cases.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

surely, is legal in winblows os'es too, but the question is that: the user specifies an acceptedTypes list-comma separated string inside the page in the rich:fileUpload component, and this must be a list of extensions without the '.' dot. In my use-case it was correctly rejected because i have specified a list of acceptedTypes="GIF, PNG", excluding the empty extension.

Note: Inside the component reference spec. the sintax (with or without dots?) is not explicitly reported...

acceptedTypes

The acceptedTypes parameter defines comma separated list of file extensions accepted by component. The component does not provide any feedback when rejecting file. For introducing feedback for rejection, use ontyperejected parameter

...so if we implicit take the assumption (as the example source code in showcase does) that we must pass a string of comma separated extensions without dot '.' prefixes, the only way the user can specify an empty file extension is when he inserts a comma followed by zero or more spaces (or vice-versa if it is the first entry in the acceptedTypes string).

If you try to do that (but now defining the acceptedTypes attribute, including the empty string between commas) the actual implementation will upload correctly the "withoutextension." file and rejects the incorrect file types at server-side, but at client-side there's something wrong because it will display in list all the files however (rejected or not)...

I investigate about this problem and we must change a line inside fileupload.js@__accept() method:
fileupload.js@line:309
CURRENT: var extension = this.acceptedTypes[i];
NEW FIX : var extension = "." + this.acceptedTypes[i];

with this NEW FIX the next line of code will behave as we expect:
result = fileName.indexOf(extension, fileName.length - extension.length) !== -1;
otherwise the indexOf will fail because extension is an empty string in our case
(I supposed acceptedTypes =", PNG, GIF" to include the empty extesions files)

I think, however, the use of empty string inside the acceptedTypes for the empty extension it is not the best choice in terms of clairness, so, for instance, we could modify the acceptedTypes requiring the "." prefix, to emprove that, then the user will specify an acceptedTypes=".,.JPG,.PNG" for example...but we're wasting time IMHO.

So, finally, i think we can:

  1. leave all as-is in the actual implemention, and
  2. correct the bug i reported before related to missed rejects displayed in list at client-side, and
  3. eventually, we could better explain the usage/syntax of acceptedTypes field inside the VDL doc of 4.3.3 release, either in case of empty file extension and, more generally, reporting explicitly the correct sintax of this field without the '.' prefix. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it is sufficient to provide option to pass "" empty extension,
e.g.: "jpg,png,"

It will mean "no extension" as well as "empty extension" (isn't it
incredibly rare case?).

On Sat, Jul 6, 2013 at 4:29 PM, simone83 notifications@github.com wrote:

In input/ui/src/main/java/org/richfaces/request/BaseUploadedFile.java:

@@ -73,4 +73,16 @@ public String getContentType() {
Closeables.closeQuietly(is);
}
}
+

  • @OverRide
  • public String getFileExtension() {
  •    String name = this.getName();

  •    if (name != null) {

  •        int i = name.lastIndexOf('.');

  •        if (i > 0) {

  •            return name.substring(i + 1);

surely, is legal in winblows os'es too, but the question is that: the user
specifies an acceptedTypes list-comma separated string inside the page in
the rich:fileUpload component, and this must be a list of extensions
without the '.' dot.
Inside the component reference spec. the use of dots however is not

explicitly required...

acceptedTypes

The acceptedTypes parameter defines comma separated list of file
extensions accepted by component. The component does not provide any
feedback when rejecting file. For introducing feedback for rejection, use

ontyperejected parameter

...so if we implicit take the assumption (as the example source code in
showcase does) that we must pass a string of comma separated extensions
without dot '.' prefixes, the only way the user can specify an empty file
extension is when he inserts a comma followed by 0 or more spaces or
vice-versa if it is the first entry in the acceptedTypes string.
If you try to do that, the actual implementation will upload each file
without care of extension (i have to look inside code deeply to understand
if it's a bug or it is a sort of failed validation of acceptedTypes list
behaviour, then it will disable the effects of acceptedTypes attribute at
all...try it yourself!).
So we have to modify the acceptedTypes to require the "." prefix, then the
user can specify an acceptedTypes=".,.JPG,.PNG" for example...otherwise, as
it is now, it make sense to reject the "noextension." files because they
have and empty extension that it will represented by an empty string and
the users actually cannot specify that inside the acceptedTypes because as
i sayed before inserting a comma followed by an empty string will change
the expected behaviour, disabling the per-extension upload filter ...WDYT?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/78/files#r5052766
.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, for server-side, but the problem still remain at client-side (so i proposed the var extension = "." + this.acceptedTypes[i]; at line 309 of fileupload.js)...try it yourself to upload (in multiple upload mode) two files: one file with no extesions and the other with an extension that will be rejected, specyfing the acceptedTypes=",png,jpg" for example and see what happens...

}
}
return "";
}
}
67 changes: 59 additions & 8 deletions input/ui/src/main/resources/META-INF/resources/org.richfaces/fileupload.js
View file
Open in desktop
Expand Up @@ -50,7 +50,7 @@
this.progressBarElement = this.iframe.next();
this.progressBar = richfaces.$(this.progressBarElement);
this.cleanInput = this.input.clone();
this.addProxy = jQuery.proxy(this.__addItem, this);
this.addProxy = jQuery.proxy(this.__addItems, this);
this.input.change(this.addProxy);
this.addButton.mousedown(pressButton).mouseup(unpressButton).mouseout(unpressButton);
this.uploadButton.click(jQuery.proxy(this.__startUpload, this)).mousedown(pressButton)
Expand Down Expand Up @@ -100,6 +100,13 @@
};

richfaces.BaseComponent.extend(richfaces.ui.FileUpload);


function TypeRejectedException(fileName) {
this.name = "TypeRejectedException";
this.message = "The type of file " + fileName + " is not accepted";
this.fileName = fileName;
}

$.extend(richfaces.ui.FileUpload.prototype, (function () {

Expand All @@ -113,8 +120,52 @@
clearLabel: "Clear",
deleteLabel: "Delete",

__addItem: function() {
var fileName = this.input.val();
__addItems : function() {
var context = {
acceptedFileNames: [],
rejectedFileNames: []
};

var multipleInputFiles = this.input.prop("files");
if (multipleInputFiles) {
for (var i = 0 ; i < multipleInputFiles.length; i++) {
this.__tryAddItem(context, multipleInputFiles[i].name);

if (this.maxFilesQuantity && (context.acceptedFileNames.length >= this.maxFilesQuantity)) {
this.addButton.hide();
break;
}
}
} else {
var fileName = this.input.val();
this.__tryAddItem(context, fileName);
}

if (context.rejectedFileNames.length > 0) {
richfaces.Event.fire(this.element, "ontyperejected", context.rejectedFileNames.join(','));
}

if (this.immediateUpload) {
this.__startUpload();
}
},

__tryAddItem: function(context, fileName) {
try {
if (this.__addItem(fileName)) {
context.acceptedFileNames.push(fileName);
}
} catch (e) {
if (e instanceof TypeRejectedException) {
context.rejectedFileNames.push(fileName);
} else {
throw e;
}
}
},

__addItem: function(fileName) {
var fileName = fileName;
if (!navigator.platform.indexOf("Win")) {
fileName = fileName.match(/[^\\]*$/)[0];
} else {
Expand All @@ -135,10 +186,10 @@
this.input.change(this.addProxy);
this.__updateButtons();
richfaces.Event.fire(this.element, "onfileselect", fileName);
if (this.immediateUpload) {
this.__startUpload();
}
return true;
}

return false;
},

__removeItem: function(item) {
Expand Down Expand Up @@ -259,7 +310,7 @@
result = fileName.indexOf(extension, fileName.length - extension.length) !== -1;
}
if (!result) {
richfaces.Event.fire(this.element, "ontyperejected", fileName);
throw new TypeRejectedException(fileName);
}
return result;
},
Expand Down Expand Up @@ -352,4 +403,4 @@
this.model.state = state;
}
});
}(window.RichFaces, jQuery));
}(window.RichFaces, jQuery));
4 changes: 2 additions & 2 deletions input/ui/src/main/templates/fileupload.template.xml
View file
Open in desktop
Expand Up @@ -48,7 +48,7 @@ Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
<c:if test="#{!disabled}">
<span class="rf-fu-inp-cntr"> <!-- This span is needed for IE7 only. -->
<!-- name attribute added dynamically in fileupload.js#startUploading -->
<input type="file" class="rf-fu-inp" />
<input type="file" class="rf-fu-inp" multiple="multiple" />
</span>
</c:if>
<cdk:object name="addLabel" value="#{attributes['addLabel']}" />
Expand Down Expand Up @@ -89,4 +89,4 @@ Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
</c:if>
</div>
</cc:implementation>
</cdk:root>
</cdk:root>