Hi Martin, thank you very much for reviewing!
Jep - if it is finished we can of course port it to the Wicket 7.x branch. :-) 1) Are you able to help me a bit out with this task? I am afraid to break the functionality - that's the reason why I decided to handle it within a FSRR and don't change something in the Resource/Stream-Layer. 2) Good points! I used static because the API can be invoked without creating an object (because it has to be created after the path has been received) I used a HashMap because I didn't find a way to check if the file system already exists. I am going to work through the review as fast as possible! kind regards Tobias > Am 01.11.2015 um 16:18 schrieb Martin Grigorov <mgrigo...@apache.org>: > > 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 < > tobiassolosche...@googlemail.com> 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 < >>> tobiassolosche...@googlemail.com> 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. >>
