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_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
>



-- 
Thomas Mortagne
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs

Reply via email to