Re: [xwiki-devs] [xwiki-notifications] r29043 - in platform: core/trunk/xwiki-core core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/user/impl/xwiki core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/web web/trunk/standard/src/main/webapp/WEB-INF
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.
+ } 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
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
Hi,
+ actionMap.put("temp", "view");
why "temp" and not "tmp"?
I thought temp would be more meaningful like existing actions "view" and "edit", I could change it easily.
+ File tempFile = getTempFile(uri);
Using File is not recommended IMO. Couldn't you use a URL instead?
Is it because using java.io.File in a server environment might pose security restrictions etc. ? I don't know about it, can you explain?
+ private File getTempFile(String uri)
I'd prefer a more explicit name such as getTemporaryFile()
OK.
+ 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.
Did you mean to say that it's more simpler to use xwiki-url module to accomplish what is done above? I Thanks. - Asiri
Hi, 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.
I had a look at xwiki-url module and I think I cannot use it to extract all the necessary pieces of information form the temp resource URL. The classes that matters are: http://svn.xwiki.org/svnroot/xwiki/platform/core/trunk/xwiki-url/xwiki-url-d... and http://svn.xwiki.org/svnroot/xwiki/platform/core/trunk/xwiki-url/xwiki-url-a... EntityXWikiURLBuilder only handles first four path segments (rest is ignored), so there is no way for me to extract "module" or "filePath" segments. Also, XWikiEntityURL can only represent urlType, action, entityReference, locale and revision. I will have to modify xwiki-url module if I'm to use it for implementing tempresource action; unfortunately I don't have enough time for that :( Thanks. - Asiri
participants (2)
-
Asiri Rathnayake -
Vincent Massol