Re: [xwiki-devs] [xwiki-notifications] r17602 - in platform/core/trunk: xwiki-cache/xwiki-cache-tests/src/main/java/org/xwiki/cache/tests xwiki-containers/xwiki-container-api/src/main/java/org/xwiki/container xwiki-containers/xwiki-container-portlet/
On Fri, Mar 13, 2009 at 11:46, Vincent Massol <[email protected]> wrote:
On Mar 13, 2009, at 11:31 AM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi Sergiu,
On Mar 13, 2009, at 12:38 AM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2009-03-13 00:38:08 +0100 (Fri, 13 Mar 2009) New Revision: 17602
Modified: platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/java/ org/xwiki/cache/tests/AbstractTestCache.java platform/core/trunk/xwiki-containers/xwiki-container-api/src/main/ java/org/xwiki/container/ApplicationContext.java platform/core/trunk/xwiki-containers/xwiki-container-portlet/src/ main/java/org/xwiki/container/portlet/PortletApplicationContext.java platform/core/trunk/xwiki-containers/xwiki-container-servlet/src/ main/java/org/xwiki/container/servlet/ServletApplicationContext.java platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/test/ TestApplicationContext.java Log: XWIKI-3312: Add a getTemporaryDirectory() method to ApplicationContext Done. Patch submitted by Dan Miron, enhanced and extended.
Modified: platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/ main/ java/org/xwiki/cache/tests/AbstractTestCache.java =================================================================== --- platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/java/ org/xwiki/cache/tests/AbstractTestCache.java 2009-03-12 23:03:56 UTC (rev 17601) +++ platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/java/ org/xwiki/cache/tests/AbstractTestCache.java 2009-03-12 23:38:08 UTC (rev 17602) @@ -19,6 +19,7 @@ */ package org.xwiki.cache.tests;
+import java.io.File; import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; @@ -130,6 +131,21 @@ }
/** + * {@inheritDoc} + * + * @see org.xwiki.container.ApplicationContext#getTemporaryDirectory() + */ + public File getTemporaryDirectory() + { + try { + // The system temporary directory is a good place for temporary test data. + return new File(System.getProperty("java.io.tmpdir")); + } catch (SecurityException e) { + return new File("."); + } + }
I don't understand why you had to modify the cache module.
Because it implements ApplicationContext, don't ask me why it does that... I was pretty surprised by this also.
Thomas? This looks wrong and it certainly is since there's code duplication.
The cache test implement ApplicationContext because cache components use it to get configuration files, it's a mock.
+ + /** * @return a instance of the cache factory. * @throws Exception error when searching for cache factory component. */
Modified: platform/core/trunk/xwiki-containers/xwiki-container-api/ src/main/java/org/xwiki/container/ApplicationContext.java =================================================================== --- platform/core/trunk/xwiki-containers/xwiki-container-api/src/ main/java/org/xwiki/container/ApplicationContext.java 2009-03-12 23:03:56 UTC (rev 17601) +++ platform/core/trunk/xwiki-containers/xwiki-container-api/src/ main/java/org/xwiki/container/ApplicationContext.java 2009-03-12 23:38:08 UTC (rev 17602) @@ -20,6 +20,7 @@ */ package org.xwiki.container;
+import java.io.File; import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; @@ -27,5 +28,15 @@ public interface ApplicationContext { InputStream getResourceAsStream(String resourceName); + URL getResource(String resourceName) throws MalformedURLException; + + /** + * Gets the directory which the container must provide for storing temporary data. The contents of this directory + * may be deleted between container restarts (<em>temporary</ em>, as the name implies), so it is not a safe place to + * store permanent/important data. + * + * @return a {@link File} object pointing to a directory that the application can use for storing temporary files + */ + File getTemporaryDirectory();
I don't like this too much but it's ok for now. However I don't think this is a good generic solution for the future. I think it would be better to implement a temporary storage that uses the same storage api as the other storages (main storage, version storage, attachment storage, etc). The pb with returning a File is that 1) it ties us to a disk storage which might not be available in all environments for various reasons and 2) it's not extensible and you cannot swap another implementation (like it you want to use a memory storage for example).
For this we need the new storage/model of course which is why it's ok to have this now.
WDYT?
Yes, but this is not a storage, it's a temporary directory on the server. If we want to implement a temporary storage based on files, it will need to get a temporary directory. It seems normal to me that an environment should provide access to a place where to store data.
In cargo I had used commons vfs for all file storage and this was quite nice since it's independent of the underlying FS implementation and it was providing several implementations (FS, ftp, http, memory, etc). http://commons.apache.org/vfs/filesystems.html
That project is a bit dead now I think unfortunately but the idea was very nice and that's what I had in mind.
Once nice side effect was the ability to easily write unit tests for file-based IO easily without going to the file system: http://blogs.codehaus.org/people/vmassol/archives/think_tank.html#001370_eas...
It might be overkill for now. Just a thought.
-Vincent
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On Mar 13, 2009, at 12:01 PM, Thomas Mortagne wrote:
On Fri, Mar 13, 2009 at 11:46, Vincent Massol <[email protected]> wrote:
On Mar 13, 2009, at 11:31 AM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi Sergiu,
On Mar 13, 2009, at 12:38 AM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2009-03-13 00:38:08 +0100 (Fri, 13 Mar 2009) New Revision: 17602
Modified: platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/java/ org/xwiki/cache/tests/AbstractTestCache.java platform/core/trunk/xwiki-containers/xwiki-container-api/src/ main/ java/org/xwiki/container/ApplicationContext.java platform/core/trunk/xwiki-containers/xwiki-container-portlet/src/ main/java/org/xwiki/container/portlet/ PortletApplicationContext.java platform/core/trunk/xwiki-containers/xwiki-container-servlet/src/ main/java/org/xwiki/container/servlet/ ServletApplicationContext.java platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/test/ TestApplicationContext.java Log: XWIKI-3312: Add a getTemporaryDirectory() method to ApplicationContext Done. Patch submitted by Dan Miron, enhanced and extended.
Modified: platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/ main/ java/org/xwiki/cache/tests/AbstractTestCache.java = ================================================================== --- platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/ java/ org/xwiki/cache/tests/AbstractTestCache.java 2009-03-12 23:03:56 UTC (rev 17601) +++ platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/ java/ org/xwiki/cache/tests/AbstractTestCache.java 2009-03-12 23:38:08 UTC (rev 17602) @@ -19,6 +19,7 @@ */ package org.xwiki.cache.tests;
+import java.io.File; import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; @@ -130,6 +131,21 @@ }
/** + * {@inheritDoc} + * + * @see org.xwiki.container.ApplicationContext#getTemporaryDirectory() + */ + public File getTemporaryDirectory() + { + try { + // The system temporary directory is a good place for temporary test data. + return new File(System.getProperty("java.io.tmpdir")); + } catch (SecurityException e) { + return new File("."); + } + }
I don't understand why you had to modify the cache module.
Because it implements ApplicationContext, don't ask me why it does that... I was pretty surprised by this also.
Thomas? This looks wrong and it certainly is since there's code duplication.
The cache test implement ApplicationContext because cache components use it to get configuration files, it's a mock.
Sergiu, I suggest to throw a runtime exception saying "not implemented" instead, wdyt? Thanks -Vincent
Vincent Massol wrote:
On Mar 13, 2009, at 12:01 PM, Thomas Mortagne wrote:
On Fri, Mar 13, 2009 at 11:46, Vincent Massol <[email protected]> wrote:
On Mar 13, 2009, at 11:31 AM, Sergiu Dumitriu wrote:
Vincent Massol wrote:
Hi Sergiu,
On Mar 13, 2009, at 12:38 AM, sdumitriu (SVN) wrote:
Author: sdumitriu Date: 2009-03-13 00:38:08 +0100 (Fri, 13 Mar 2009) New Revision: 17602
Modified: platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/java/ org/xwiki/cache/tests/AbstractTestCache.java platform/core/trunk/xwiki-containers/xwiki-container-api/src/ main/ java/org/xwiki/container/ApplicationContext.java platform/core/trunk/xwiki-containers/xwiki-container-portlet/src/ main/java/org/xwiki/container/portlet/ PortletApplicationContext.java platform/core/trunk/xwiki-containers/xwiki-container-servlet/src/ main/java/org/xwiki/container/servlet/ ServletApplicationContext.java platform/core/trunk/xwiki-core/src/test/java/com/xpn/xwiki/test/ TestApplicationContext.java Log: XWIKI-3312: Add a getTemporaryDirectory() method to ApplicationContext Done. Patch submitted by Dan Miron, enhanced and extended.
Modified: platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/ main/ java/org/xwiki/cache/tests/AbstractTestCache.java = ================================================================== --- platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/ java/ org/xwiki/cache/tests/AbstractTestCache.java 2009-03-12 23:03:56 UTC (rev 17601) +++ platform/core/trunk/xwiki-cache/xwiki-cache-tests/src/main/ java/ org/xwiki/cache/tests/AbstractTestCache.java 2009-03-12 23:38:08 UTC (rev 17602) @@ -19,6 +19,7 @@ */ package org.xwiki.cache.tests;
+import java.io.File; import java.io.InputStream; import java.net.MalformedURLException; import java.net.URL; @@ -130,6 +131,21 @@ }
/** + * {@inheritDoc} + * + * @see org.xwiki.container.ApplicationContext#getTemporaryDirectory() + */ + public File getTemporaryDirectory() + { + try { + // The system temporary directory is a good place for temporary test data. + return new File(System.getProperty("java.io.tmpdir")); + } catch (SecurityException e) { + return new File("."); + } + } I don't understand why you had to modify the cache module. Because it implements ApplicationContext, don't ask me why it does that... I was pretty surprised by this also. Thomas? This looks wrong and it certainly is since there's code duplication. The cache test implement ApplicationContext because cache components use it to get configuration files, it's a mock.
Sergiu, I suggest to throw a runtime exception saying "not implemented" instead, wdyt?
K. Done. -- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (3)
-
Sergiu Dumitriu -
Thomas Mortagne -
Vincent Massol