Re: [xwiki-devs] [xwiki-notifications] r14565 - in platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki: api doc
On Dec 5, 2008, at 7:55 AM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2008-12-05 07:55:30 +0100 (Fri, 05 Dec 2008) New Revision: 14565
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ Document.java platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/doc/ XWikiDocument.java Log: XWIKI-2549: Make Document.addAttachment public Done.
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ api/Document.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ Document.java 2008-12-05 04:30:58 UTC (rev 14564) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ Document.java 2008-12-05 06:55:30 UTC (rev 14565) @@ -20,7 +20,6 @@ */ package com.xpn.xwiki.api;
-import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; @@ -32,7 +31,6 @@ import java.util.Vector;
import org.apache.commons.fileupload.FileItem; -import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; import org.suigeneris.jrcs.diff.DifferentiationFailedException; import org.suigeneris.jrcs.rcs.Version; @@ -1721,7 +1719,7 @@ filename = filename.replaceAll("\\+", " ");
if ((data != null) && (data.length > 0)) { - XWikiAttachment attachment = addAttachment(filename, data); + XWikiAttachment attachment = this.doc.addAttachment(filename, data, getXWikiContext()); getDoc().saveAttachmentContent(attachment, getXWikiContext()); // commenting because this was already done by addAttachment // getDoc().getAttachmentList().add(attachment); @@ -1736,39 +1734,26 @@ return nb; }
- protected XWikiAttachment addAttachment(String fileName, InputStream iStream) throws XWikiException, IOException + public Attachment addAttachment(String fileName, InputStream iStream) { - ByteArrayOutputStream bAOut = new ByteArrayOutputStream(); - IOUtils.copy(iStream, bAOut); - return addAttachment(fileName, bAOut.toByteArray()); + try { + return new Attachment(this, this.doc.addAttachment(fileName, iStream, getXWikiContext()), getXWikiContext()); + } catch (XWikiException e) { + // TODO Log the error and let the user know about it + } catch (IOException e) { + // TODO Log the error and let the user know about it
I don't like this... Don't we have a solution to handle it in a better way? With the current solution scripts calling this method need to check for null and take action if null which I'm pretty sure none care about so it'll silently fail. The best current solution is probably to store the result in the "error" variable in the Velocity context (we have a variable for this) and modify a main template to check for errors in page execution and if there are redirect to an error page printing them. The script should then be able to do a check for null itself and have the ability to clear the error if it can handle it gracefully so that the error page doesn't appear for the user. WDYT? [snip] Thanks -Vincent
Vincent Massol wrote:
On Dec 5, 2008, at 7:55 AM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2008-12-05 07:55:30 +0100 (Fri, 05 Dec 2008) New Revision: 14565
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ Document.java platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/doc/ XWikiDocument.java Log: XWIKI-2549: Make Document.addAttachment public Done.
Modified: platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/ api/Document.java =================================================================== --- platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ Document.java 2008-12-05 04:30:58 UTC (rev 14564) +++ platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/api/ Document.java 2008-12-05 06:55:30 UTC (rev 14565) @@ -20,7 +20,6 @@ */ package com.xpn.xwiki.api;
-import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; @@ -32,7 +31,6 @@ import java.util.Vector;
import org.apache.commons.fileupload.FileItem; -import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; import org.suigeneris.jrcs.diff.DifferentiationFailedException; import org.suigeneris.jrcs.rcs.Version; @@ -1721,7 +1719,7 @@ filename = filename.replaceAll("\\+", " ");
if ((data != null) && (data.length > 0)) { - XWikiAttachment attachment = addAttachment(filename, data); + XWikiAttachment attachment = this.doc.addAttachment(filename, data, getXWikiContext()); getDoc().saveAttachmentContent(attachment, getXWikiContext()); // commenting because this was already done by addAttachment // getDoc().getAttachmentList().add(attachment); @@ -1736,39 +1734,26 @@ return nb; }
- protected XWikiAttachment addAttachment(String fileName, InputStream iStream) throws XWikiException, IOException + public Attachment addAttachment(String fileName, InputStream iStream) { - ByteArrayOutputStream bAOut = new ByteArrayOutputStream(); - IOUtils.copy(iStream, bAOut); - return addAttachment(fileName, bAOut.toByteArray()); + try { + return new Attachment(this, this.doc.addAttachment(fileName, iStream, getXWikiContext()), getXWikiContext()); + } catch (XWikiException e) { + // TODO Log the error and let the user know about it + } catch (IOException e) { + // TODO Log the error and let the user know about it
I don't like this... Don't we have a solution to handle it in a better way? With the current solution scripts calling this method need to check for null and take action if null which I'm pretty sure none care about so it'll silently fail.
The best current solution is probably to store the result in the "error" variable in the Velocity context (we have a variable for this) and modify a main template to check for errors in page execution and if there are redirect to an error page printing them. The script should then be able to do a check for null itself and have the ability to clear the error if it can handle it gracefully so that the error page doesn't appear for the user.
WDYT?
Yes, that's what I wanted to do, too. But since there's no easy access to that variable (I was expecting a context.setError and/or pushError), I decided to leave it as a later improvement (thus the TODO). This should not happen, anyway. The thrown exception is mostly a bogeyman thrown when things go really bad, in which case the failed attachment is just a minor problem. We could ignore the exception (let it pass to the interface), if you think that's better. -- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (2)
-
Sergiu Dumitriu -
Vincent Massol