Author: andrei_exadel Date: 2008-02-21 06:37:38 -0500 (Thu, 21 Feb 2008) New Revision: 6232 Modified: trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/ui/renderkit/FileUploadRendererBase.java trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/web/MultipartFilter.java trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/web/MultipartRequest.java trunk/sandbox/ui/fileUpload/src/main/resources/org/richfaces/renderkit/html/js/FileUpload.js Log: Fix code review notes Modified: trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/ui/renderkit/FileUploadRendererBase.java =================================================================== --- trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/ui/renderkit/FileUploadRendererBase.java 2008-02-21 10:21:54 UTC (rev 6231) +++ trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/ui/renderkit/FileUploadRendererBase.java 2008-02-21 11:37:38 UTC (rev 6232) @@ -15,7 +15,6 @@ import javax.faces.render.RenderKit; import javax.faces.render.RenderKitFactory; import javax.servlet.ServletRequest; -import javax.servlet.http.HttpSession; import org.ajax4jsf.context.AjaxContext; import org.ajax4jsf.context.AjaxContextImpl; @@ -25,11 +24,11 @@ import org.ajax4jsf.javascript.JSLiteral; import org.ajax4jsf.javascript.JSReference; import org.ajax4jsf.renderkit.AjaxRendererUtils; -import org.ajax4jsf.renderkit.RendererUtils; import org.ajax4jsf.resource.CountingOutputWriter; import org.richfaces.component.UIProgressBar; import org.richfaces.event.UploadEvent; import org.richfaces.org.jboss.seam.ui.component.UIFileUpload; +import org.richfaces.org.jboss.seam.web.MultipartFilter; import org.richfaces.org.jboss.seam.web.MultipartRequest; import org.richfaces.renderkit.TemplateEncoderRendererBase; @@ -42,17 +41,6 @@ public abstract class FileUploadRendererBase extends TemplateEncoderRendererBase { - /** File name parameter name */ - private static final String FILE_NAME_PARAMETER = "fileName"; - - /** Session bean name to store the percent value of uploading process */ - public static final String _percentBeanName = "__percentValue$"; - - /** Session bean name to store the percent value of uploading process */ - public static final String _progressInfoBeanName = "__ProgressInfo$"; - - private static final String _reRenderAllFlag = "reRenderAll"; - /* * (non-Javadoc) * @@ -71,29 +59,21 @@ .getRequestParameterValuesMap(); if ("progress".equals(request.getParameter("action"))) { - AjaxContext ajaxContext = AjaxContextImpl.getCurrentInstance(context); + AjaxContext ajaxContext = AjaxContextImpl + .getCurrentInstance(context); String uid = request.getParameter("uid"); - /* - * - * FIXME: Maksim --> Andrei - * Why not use standard JSF constructs? - * + ExternalContext externalContext = context.getExternalContext(); - Map <String, Object> sessionMap = externalContext.getSessionMap(); - - */ + Map<String, Object> sessionMap = externalContext.getSessionMap(); - HttpSession session = (HttpSession)context.getExternalContext().getSession(false); - Map<String, MultipartRequest> sessions = (Map<String, MultipartRequest>) session - .getAttribute("_richfaces_upload_sessions"); - - if (sessions != null) { - MultipartRequest multipartRequest = sessions.get(uid); - if (multipartRequest != null) { - ajaxContext.setResponseData(multipartRequest.getSize()); - } + Map<String, MultipartRequest> sessions = (Map<String, MultipartRequest>) sessionMap + .get(MultipartFilter.REQUESTS_SESSIONS_BEAN_NAME); + if (sessions != null) { + MultipartRequest multipartRequest = sessions.get(uid); + if (multipartRequest != null) { + ajaxContext.setResponseData(multipartRequest.getSize()); } - + } } else { new AjaxEvent(component).queue(); } @@ -117,6 +97,12 @@ } + /** + * Put uploaded file into data attribute defined + * @param context + * @param fileUpload + * @param file + */ private void storeData(FacesContext context, UIFileUpload fileUpload, File file) { ValueExpression data = fileUpload.getValueExpression("data"); if (data != null) { @@ -124,13 +110,16 @@ if (clazz.isAssignableFrom(Map.class)) { Map map = (Map) data.getValue(context.getELContext()); map.put(fileUpload.getLocalFileName(), file); - //FIXME: Maksim --> Andrei - // What does Map.empty mean? - map.isEmpty(); } } } + /** + * Call with method after uploading completed + * @param context + * @param file + * @param fileUpload + */ private void onUploadComplete(FacesContext context, File file, UIFileUpload fileUpload) { storeData(context, fileUpload, file); @@ -146,27 +135,28 @@ * * @see org.ajax4jsf.renderkit.RendererBase#getComponentClass() */ - //FIXME: Maksim --> Andrei - //Better use UIFileUpload here. Otherwise you can get NPE here @Override protected Class<? extends UIComponent> getComponentClass() { - // TODO Auto-generated method stub - return null; + return UIFileUpload.class; } + /** + * Generates common JS script by action value + * @param context + * @param component + * @param action + * @param oncomplete + * @return + * @throws IOException + */ private String getActionScript(FacesContext context, UIComponent component, String action, Object oncomplete) throws IOException { JSFunction ajaxFunction = AjaxRendererUtils.buildAjaxFunction( component, context); - - //FIXME: Maksim --> Andrei - //Why multiple map.get? In eclipse, use Ctrl+2 then L to introduce local variable - // - Map options = AjaxRendererUtils.buildEventOptions(context, component); - - ((Map) options.get("parameters")).put("action", action); - ((Map) options.get("parameters")).put("uid", new JSReference("uid")); + Map parameters = (Map) options.get("parameters"); + parameters.put("action", action); + parameters.put("uid", new JSReference("uid")); if (oncomplete != null) { options.put("onbeforedomupdate", oncomplete); } @@ -179,13 +169,26 @@ return function.toScript(); } + /** + * Generates JS script for stopping uploading process + * @param context + * @param component + * @return + * @throws IOException + */ public String getStopScript(FacesContext context, UIComponent component) throws IOException { return getActionScript(context, component, "stop", null); } - // TODO : complete this one + /** + * Generates JS script for getting file size from server + * @param context + * @param component + * @return + * @throws IOException + */ public String getFileSizeScript(FacesContext context, UIComponent component) throws IOException { JSFunctionDefinition oncomplete = new JSFunctionDefinition(); @@ -200,9 +203,13 @@ } - //FIXME: Maksim --> Andrei - //Method not used anywhere - //WHY? + /** + * Generates progress label markup + * @param context + * @param component + * @return + * @throws IOException + */ public Object getLabelMarkup(FacesContext context, UIComponent component) throws IOException { CountingOutputWriter customWriter = new CountingOutputWriter(); @@ -226,7 +233,6 @@ ResponseWriter responseWriter = renderKit.createResponseWriter( customWriter, null, "UTF-8"); context.setResponseWriter(responseWriter); - //renderChild(context, label); writeScriptBody(context, label, false); if (writer != null) { context.setResponseWriter(writer); @@ -279,17 +285,36 @@ } + /** + * Gets progress bar Id + * @param context + * @param component + * @return + * @throws IOException + */ public String getProgressBarId(FacesContext context, UIComponent component) throws IOException { return getProgressBar(context, component).getClientId(context); } + /** + * Renders progress bar + * @param context + * @param component + * @throws IOException + */ public void renderProgress(FacesContext context, UIComponent component) throws IOException { UIComponent progressBar = getProgressBar(context, component); renderChild(context, progressBar); } + /** + * Creates progress bar component + * @param context + * @param fileUpload + * @return + */ private UIComponent createProgressBar(FacesContext context, UIComponent fileUpload) { UIComponent progressBar = fileUpload.getFacet("progress"); @@ -301,6 +326,12 @@ return progressBar; } + /** + * Returns progress bar + * @param context + * @param component + * @return + */ private UIComponent getProgressBar(FacesContext context, UIComponent component) { UIComponent progressBar = component.getFacet("progress"); @@ -308,45 +339,18 @@ progressBar = createProgressBar(context, component); } - //FIXME: Maksim --> Andrei - //What happens if we have two fileuploads on the page? - //Two progress bars with identical value? - String exprStr = "#{" + _percentBeanName + "}"; + //We associate progress bar's percent value in session scope with file upload component id + String exprStr = "#{" + MultipartFilter.PERCENT_BEAN_NAME + "['"+component.getClientId(context)+"']}"; ValueExpression ex = context.getApplication().getExpressionFactory() .createValueExpression(context.getELContext(), exprStr, Integer.class); progressBar.setValueExpression("value", ex); - - - //FIXME: Maksim --> Andrei - //Why set id for user-defined component - progressBar.setId("progressBar"); progressBar.getAttributes().put("minValue", -1); progressBar.getAttributes().put("enabled", false); progressBar.setTransient(false); - if (getProgressLabel(component) != null) { - progressBar.getAttributes().put("reRender", "progressLabel"); - } - return progressBar; } - private StringBuffer getJSScriptStart(String clientId) { - StringBuffer buffer = new StringBuffer("$('"); - buffer.append(clientId).append("').component"); - return buffer; - } - - private UIComponent getProgressLabel(UIComponent component) { - UIComponent label = component.getFacet("progressLabel"); - if (label != null) { - //FIXME: Maksim --> Andrei - //Why set id for user-defined component - label.setId("progressLabel"); - } - return label; - } - /* * (non-Javadoc) * Modified: trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/web/MultipartFilter.java =================================================================== --- trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/web/MultipartFilter.java 2008-02-21 10:21:54 UTC (rev 6231) +++ trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/web/MultipartFilter.java 2008-02-21 11:37:38 UTC (rev 6232) @@ -16,8 +16,6 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; -import org.richfaces.org.jboss.seam.ui.renderkit.FileUploadRendererBase; - /** * A filter for decoding multipart requests, for * use with the file upload control. @@ -34,6 +32,10 @@ { public static final String MULTIPART = "multipart/"; + public static final String REQUESTS_SESSIONS_BEAN_NAME = "_richfaces_upload_sessions"; + + public static final String PERCENT_BEAN_NAME = "_richfaces_upload_percents"; + /** * Flag indicating whether a temporary file should be used to cache the uploaded file */ @@ -73,34 +75,36 @@ HttpServletRequest httpRequest = (HttpServletRequest) request; String uid = httpRequest.getParameter("uid"); + String componentId = httpRequest.getParameter("clientId"); if (isMultipartRequest(httpRequest)) { MultipartRequest multipartRequest = new MultipartRequest( - httpRequest, createTempFiles, maxRequestSize); + httpRequest, createTempFiles, maxRequestSize, componentId); Map<String, MultipartRequest> sessionsMap = null; + Map<String, Object> percentMap = null; try { HttpSession session = httpRequest.getSession(); synchronized (session) { sessionsMap = (Map<String, MultipartRequest>) session - .getAttribute("_richfaces_upload_sessions"); + .getAttribute(REQUESTS_SESSIONS_BEAN_NAME); + percentMap = (Map<String, Object>) session.getAttribute(PERCENT_BEAN_NAME); if (sessionsMap == null) { sessionsMap = Collections.synchronizedMap(new HashMap<String, MultipartRequest>()); session.setAttribute( - "_richfaces_upload_sessions", sessionsMap); + REQUESTS_SESSIONS_BEAN_NAME, sessionsMap); } + if (percentMap == null) { + percentMap = new HashMap<String, Object>(); + session.setAttribute(PERCENT_BEAN_NAME, percentMap); + } } - //FIXME: Maksim --> Andrei - //Can we make _percentBeanName uid-dependent? - - session.setAttribute(FileUploadRendererBase._percentBeanName, 0); + percentMap.put(componentId, 0); // associate percent value with file upload component ID + sessionsMap.put(uid, multipartRequest); - sessionsMap.put(uid, multipartRequest); - if (multipartRequest.parseRequest()) { chain.doFilter(multipartRequest, response); } else { - // TODO HttpServletResponse httpResponse = (HttpServletResponse) response; httpResponse.setStatus(HttpServletResponse.SC_OK); httpResponse.setContentType("text/html"); @@ -119,7 +123,7 @@ if ("stop".equals(httpRequest.getParameter("action"))) { HttpSession session = httpRequest.getSession(); Map<String, MultipartRequest> sessions = (Map<String, MultipartRequest>) session - .getAttribute("_richfaces_upload_sessions"); + .getAttribute(REQUESTS_SESSIONS_BEAN_NAME); if (sessions != null) { MultipartRequest multipartRequest = sessions.get(uid); @@ -159,7 +163,6 @@ } public void destroy() { - // TODO Auto-generated method stub } Modified: trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/web/MultipartRequest.java =================================================================== --- trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/web/MultipartRequest.java 2008-02-21 10:21:54 UTC (rev 6231) +++ trunk/sandbox/ui/fileUpload/src/main/java/org/richfaces/org/jboss/seam/web/MultipartRequest.java 2008-02-21 11:37:38 UTC (rev 6232) @@ -12,7 +12,6 @@ import java.rmi.server.UID; import java.util.ArrayList; import java.util.Collections; -import java.util.Date; import java.util.Enumeration; import java.util.HashMap; import java.util.List; @@ -23,9 +22,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequestWrapper; -import org.richfaces.org.jboss.seam.ui.component.ProgressData; -import org.richfaces.org.jboss.seam.ui.renderkit.FileUploadRendererBase; - /** * Request wrapper for supporting multipart requests, used for file uploading. * @@ -40,6 +36,8 @@ private static final int CHUNK_SIZE = 512; private boolean createTempFiles; + + private String clientId; private String encoding = null; @@ -47,9 +45,9 @@ private int read = 0; - private String beanName; - private Map<String, Param> parameters = null; + + private Map<String, Object> percentMap = null; private enum ReadState { BOUNDARY, HEADERS, DATA @@ -248,14 +246,14 @@ private boolean shouldStop = false; public MultipartRequest(HttpServletRequest request, - boolean createTempFiles, int maxRequestSize) { + boolean createTempFiles, int maxRequestSize, String clientId) { super(request); this.request = request; this.createTempFiles = createTempFiles; + this.clientId = clientId; String contentLength = request.getHeader("Content-Length"); this.contentLength = Integer.parseInt(contentLength); - //initProgressInfo(this.contentLength); if (contentLength != null && maxRequestSize > 0 && this.contentLength > maxRequestSize) { throw new FileUploadException( @@ -292,6 +290,7 @@ encoding = request.getCharacterEncoding(); parameters = new HashMap<String, Param>(); + this.percentMap = getProgressData(); try { byte[] buffer = new byte[BUFFER_SIZE]; @@ -430,36 +429,17 @@ } } - private ProgressData getProgressData () { - ProgressData progressData = null; - String beanName = (String)getSession().getAttribute(FileUploadRendererBase._progressInfoBeanName); - if (beanName != null) { - progressData = (ProgressData)getSession().getAttribute(beanName); - } - return progressData; + @SuppressWarnings("unchecked") + private Map<String, Object> getProgressData () { + percentMap = (Map<String, Object>)getSession().getAttribute(MultipartFilter.PERCENT_BEAN_NAME); + return percentMap; } private void fillProgressInfo () { Double percent = (Double) (100.0 * this.read / this.contentLength); - getSession().setAttribute(FileUploadRendererBase._percentBeanName,percent); - /*if (this.progressData != null) { - progressData.setPercent(percent); - progressData.setUploaded((long)this.read); - progressData.setTime((new Date().getTime() / 1000)- this.time); - }*/ + percentMap.put(clientId, percent); } - - /* private void initProgressInfo(int size) { - time = new Date().getTime()/1000; - progressData = getProgressData(); - if (progressData != null) { - progressData.setSize((long)size); - progressData.setTime(0L); - } - } - */ - private byte[] getBoundaryMarker(String contentType) { Map<String, Object> params = parseParams(contentType, ";"); String boundaryStr = (String) params.get("boundary"); Modified: trunk/sandbox/ui/fileUpload/src/main/resources/org/richfaces/renderkit/html/js/FileUpload.js =================================================================== --- trunk/sandbox/ui/fileUpload/src/main/resources/org/richfaces/renderkit/html/js/FileUpload.js 2008-02-21 10:21:54 UTC (rev 6231) +++ trunk/sandbox/ui/fileUpload/src/main/resources/org/richfaces/renderkit/html/js/FileUpload.js 2008-02-21 11:37:38 UTC (rev 6232) @@ -139,6 +139,7 @@ }, setupLabelUpdate: function () { + this.updateLabel(); this.labelUpdateInterval = setInterval(function () { this.updateLabel(); }.bind(this), this.uploadObject.progressBar.options['pollinterval']); }, @@ -164,20 +165,12 @@ }, _clearInput: function() { - //remove input type="file" element Richfaces.removeNode(this.fileInput); - /*var inputs = $(this.uploadObject.id + ":add1") - try{ - inputs.removeChild(this.fileInput); - }catch(e) {}*/ this.fileInput = null; }, _clearEntry: function() { Richfaces.removeNode(this.entryElement); - /*try{ - this.uploadObject.items.removeChild(this.entryElement); - }catch(e) {}*/ this.entryElement = null; }, @@ -365,7 +358,6 @@ initialize: function(id, stopScript, getFileSizeScript, progressBarId, classes, label, maxFiles, options) { this.id = id; this.element = $(this.id); - //this.progressBarId = progressBarId; this._progressBar = $(progressBarId); this.progressBar = this._progressBar.component; @@ -402,6 +394,7 @@ prepareProgressBar: function () { this.progressBar.setValue(0); Element.show(this._progressBar); + this.progressBar.enable(); }, finishProgressBar: function () { @@ -439,7 +432,6 @@ entry.clear(); this.entries = this.entries.without(entry); - //this.disableAddButton(this.maxFileBatchSize && this.entries.length >= this.maxFileBatchSize); }, _selectEntryForUpload: function() { @@ -576,9 +568,6 @@ _endUpload: function() { this.activeEntry = null; - //this.progressBar.setValue(100); - //this.progressBar.disable(); - //Element.hide(this._progressBar); }, _updateEntriesState: function() { @@ -628,7 +617,6 @@ } this._updateEntriesState(); - //this.disableUploadButton(false); } else if (newState == FileUploadEntry.UPLOAD_CANCELED || newState == FileUploadEntry.UPLOAD_TRANSFER_ERROR || @@ -641,17 +629,12 @@ this._updateEntriesState(); - //this.disableUploadButton(false); - } else if (newState == FileUploadEntry.UPLOAD_IN_PROGRESS) { this.activeEntry = entry; - this.progressBar.setValue(0); - this.progressBar.enable(); this._updateEntriesState(); - //this.disableUploadButton(true); } this.processButtons(); }, @@ -676,7 +659,7 @@ parentForm.encoding = "multipart/form-data"; parentForm.enctype = "multipart/form-data"; parentForm.target = this.id; - parentForm.action = oldAction + (/\?/.test(oldAction) ? '&uid' : '?uid') + '=' + encodeURI(entry.uid); + parentForm.action = oldAction + (/\?/.test(oldAction) ? '&uid' : '?uid') + '=' + encodeURI(entry.uid) + '&clientId=' + this.id; try { var inputs = parentForm.elements;