Hi Tobias,

I've added some comments to the Pull Request.

I think it is a useful addition1 I don't see any Java 8 APIs used there so
it can go even in Wicket 7.

The main issues I see:
1) I think we should extract most of the logic in FileSystemResource to
PathResourceStream (a new class), similar to FileResourceStream. This way
it will benefit from locale/style/variation support.
2) The helper methods #getPath() in FSRR bother me a bit. At the moment
there are concurrency issues with it. This is easy to fix. But the usage of
'static' is a usually problematic. If caching is really needed then I think
we should use Application's metadata instead.

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov

On Sun, Nov 1, 2015 at 10:38 AM, Tobias Soloschenko <
[email protected]> wrote:

> I wrote some unit tests even with mime type detection and created a pull
> request to review the implementation:
>
> https://github.com/apache/wicket/pull/142
>
> kind regards
>
> Tobias
>
> Am 01.11.15 um 07:13 schrieb Maxim Solodovnik:
>
> +1
>> this new component will be very useful in our project
>>
>> On Sat, Oct 31, 2015 at 3:47 PM, Tobias Soloschenko <
>> [email protected]> wrote:
>>
>> Hi,
>>>
>>> I just wanted to ask if we should integrate the
>>> FileSystemResourceReference into wicket core which is mentioned and
>>> already
>>> discussed here:
>>>
>>>
>>>
>>> http://apache-wicket.1842946.n4.nabble.com/Generic-resource-reference-for-video-audio-playback-td4672337.html
>>>
>>> I provided two examples:
>>>
>>> * Reading jar / zip files (Example >
>>> jar:file:///myFolder/test.zip!/myFolderInsideTheZip/video1.mp4)
>>> * Unified reading of files out of the file system (unix, windows,
>>> solaris)
>>> (Example > file:///myFolder/video1.mp4)
>>>
>>>
>>>
>>> https://github.com/klopfdreh/wicket-components-playground/blob/master/wicket-components-playground/src/main/java/org/apache/wicket/markup/html/media/FileSystemExamplePage.java
>>>
>>> Further FileSystemProviders and the corresponding FileSystems can be
>>> implemented as described here:
>>>
>>>
>>>
>>> http://docs.oracle.com/javase/7/docs/technotes/guides/io/fsp/filesystemprovider.html
>>>
>>> WDYT?
>>>
>>> kind regards
>>>
>>> Tobias
>>>
>>> P.S.: If all tend the FileSystemResourceReference to be integrated I am
>>> going to create a PR and write some unit tests.
>>>
>>>
>>
>>
>

Reply via email to