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.
>>> +
>>> + /**
>>> * @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_easy_unit_testing_of_file_operations
It might be overkill for now. Just a thought.
-Vincent
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
