On May 28, 2010, at 1:18 PM, Vincent Massol wrote:
Hi Asiri,
Some comments below.
On May 28, 2010, at 1:03 PM, asiri (SVN) wrote:
Author: asiri Date: 2010-05-28 13:03:53 +0200 (Fri, 28 May 2010) New Revision: 29043
Added: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/TempResourceAction.java Modified: platform/core/trunk/xwiki-core/pom.xml platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/XWikiRightServiceImpl.java platform/web/trunk/standard/src/main/webapp/WEB-INF/struts-config.xml Log: XWIKI-5227: Introduce a temp resource action.
* Applying patch after review, refer http://jira.xwiki.org/jira/browse/XWIKI-5227 for more details.
Modified: platform/core/trunk/xwiki-core/pom.xml =================================================================== --- platform/core/trunk/xwiki-core/pom.xml 2010-05-28 09:34:16 UTC (rev 29042) +++ platform/core/trunk/xwiki-core/pom.xml 2010-05-28 11:03:53 UTC (rev 29043) @@ -257,6 +257,13 @@ <groupId>commons-beanutils</groupId> <artifactId>commons-beanutils</artifactId> </dependency> + + <!-- For extracting metadata from various document types --> + <dependency> + <groupId>org.apache.tika</groupId> + <artifactId>tika-core</artifactId> + <version>0.7</version> + </dependency>
<!-- Logging --> <dependency>
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/XWikiRightServiceImpl.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/XWikiRightServiceImpl.java 2010-05-28 09:34:16 UTC (rev 29042) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki/XWikiRightServiceImpl.java 2010-05-28 11:03:53 UTC (rev 29043) @@ -137,6 +137,7 @@ actionMap.put("jsx", "view"); actionMap.put("ssx", "view"); actionMap.put("tex", "view"); + actionMap.put("temp", "view");
why "temp" and not "tmp"?
actionMap.put("unknown", "view"); }
Added: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/TempResourceAction.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/TempResourceAction.java (rev 0) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web/TempResourceAction.java 2010-05-28 11:03:53 UTC (rev 29043) @@ -0,0 +1,128 @@ +/* + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * + * This is free software; you can redistribute it and/or modify it + * under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation; either version 2.1 of + * the License, or (at your option) any later version. + * + * This software is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this software; if not, write to the Free + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA, or see the FSF site: http://www.fsf.org. + */ +package com.xpn.xwiki.web; + +import java.io.File; +import java.io.IOException; +import java.net.URI; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.apache.commons.io.FileUtils; +import org.apache.commons.io.IOUtils; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.tika.Tika; +import org.apache.tika.mime.MimeTypes; +import org.xwiki.container.Container; + +import com.xpn.xwiki.XWikiContext; +import com.xpn.xwiki.XWikiException; + +/** + * Action responsible for downloading temporary resources created by various modules. Refer JIRA issue: + * <a>http://jira.xwiki.org/jira/browse/XWIKI-5227</a>. + * + * @version $Id$ + * @since 2.4M1 + */ +public class TempResourceAction extends XWikiAction +{ + /** + * URI pattern for this action. + */ + public static final Pattern URI_PATTERN = Pattern.compile(".*?/temp/([^/]*+)/([^/]*+)/([^/]*+)/(.*+)"); + + /** + * Logging support. + */ + private static final Log LOG = LogFactory.getLog(TempResourceAction.class); + + /** + * Used for detecting mime-types of files. + */ + private Tika tika = new Tika(); + + /** + * Used to resolve temporary working dir. + */ + private Container container = Utils.getComponent(Container.class); + + /** + * {@inheritDoc} + */ + public String render(XWikiContext context) throws XWikiException + { + XWikiRequest request = context.getRequest(); + XWikiResponse response = context.getResponse(); + String uri = request.getRequestURI(); + + // Locate the temporary file. + File tempFile = getTempFile(uri);
Using File is not recommended IMO. Couldn't you use a URL instead?
+ if (null == tempFile) { + throw new XWikiException(XWikiException.MODULE_XWIKI_APP, XWikiException.ERROR_XWIKI_APP_URL_EXCEPTION, + "Invalid temporary resource URL"); + } + + // Write temporary file into response. + response.setDateHeader("Last-Modified", tempFile.lastModified()); + String contentType = MimeTypes.OCTET_STREAM; + try { + contentType = tika.detect(tempFile); + } catch (IOException ex) { + LOG.warn(String.format("Unable to determine mime type for temporary resource [%s]", tempFile + .getAbsolutePath()), ex); + } + response.setContentType(contentType); + try { + response.setContentLength((int) tempFile.length()); + IOUtils.copy(FileUtils.openInputStream(tempFile), response.getOutputStream());
This is very bad for performance. You should stream the content instead.
hmm I may be wrong here... need to check what this signature does. I thought you were copying the full file into memory but it seems it's ok after all. Sorry about that. Thanks -Vincent
+ } catch (IOException e) { + throw new XWikiException(XWikiException.MODULE_XWIKI_APP, + XWikiException.ERROR_XWIKI_APP_SEND_RESPONSE_EXCEPTION, "Exception while sending response", e); + } + return null; + } + + /** + * Returns the temporary file corresponding to the specified URI. + * + * @param uri request URI. + * @return temporary file corresponding to the specified URI or null if no such file can be located. + */ + private File getTempFile(String uri)
I'd prefer a more explicit name such as getTemporaryFile()
+ { + Matcher matcher = URI_PATTERN.matcher(uri); + File result = null; + if (matcher.find()) { + String space = matcher.group(1); + String page = matcher.group(2); + String module = matcher.group(3); + String filePath = matcher.group(4); + String prefix = String.format("temp/%s/%s/%s/", module, space, page); + String path = URI.create(prefix + filePath).normalize().toString(); + if (path.startsWith(prefix)) { + result = new File(container.getApplicationContext().getTemporaryDirectory(), path); + result = result.exists() ? result : null; + }
You should use the xwiki-url module to extract information from the URL IMO. It's much more complicated than what is written above. See code in xwiki-url for details.
+ } + return result; + } +}
Modified: platform/web/trunk/standard/src/main/webapp/WEB-INF/struts-config.xml =================================================================== --- platform/web/trunk/standard/src/main/webapp/WEB-INF/struts-config.xml 2010-05-28 09:34:16 UTC (rev 29042) +++ platform/web/trunk/standard/src/main/webapp/WEB-INF/struts-config.xml 2010-05-28 11:03:53 UTC (rev 29043) @@ -257,6 +257,11 @@ name="download" scope="request"> </action> + <action path="/temp/" + type="com.xpn.xwiki.web.TempResourceAction" + name="temp" + scope="request"> + </action> <action path="/downloadrev/" type="com.xpn.xwiki.web.DownloadRevAction" name="dowloadrev"
Thanks -Vincent