Sounds excellent, thanks for your hard work.
--
Jody Garnett
On 21 December 2015 at 02:12, Niels Charlier <ni...@scitus.be> wrote:
> I'd suggest merging ResourceLoader and GeoServerDataDirectory. Both of
> them provide mostly a lot of convenience methods and a little bit of extra
> logic on top of the ResourceStore. We can remove a lot of the depecrated
> stuff now.
>
>
> On 19-12-15 01:43, Jody Garnett wrote:
>
> And utility methods ...
>
> 2) New utility methods
>>>
>>> Resources.toURL( resource )
>>> - makes a lot of sense and reads better in the code
>>>
>>> I am not too worried about additional utility methods, instead I am
>>> concerned about changes to objects we may have to implement (such as
>>> GeoServerResourceLoader).
>>>
>>> GeoServerResourceLoader.fromURL(String url)
>>> GeoServerResourceLoader.fromURL(URL url)
>>> - yeah I know you did not write this - just added one that takes a URL
>>> (do you really need both?)
>>>
>>
>> Yes, you do. The fromURL(String) method accepts things that would be
>> rejected by a normal URL parser, abolishing it would be a serious
>> regression causing many configurations to fail.
>>
>> Oh, I forgot to mention. I did NOT add one that takes a URL. This method
>> was merely moved from another spot (GeoServerDataDirectory). An illogical
>> spot, the only reason for it being there is because it is being used there.
>> None of this URL stuff is new, this is just refactoring, moving things
>> together that belong together and make them consistent.
>
>
> Okay now we are making sense :)
>
> Here is the annoying difference in class responsibility (you can also
> check the javadocs):
> * GeoServerResourceLoader is supposed to obtain a resource from the
> configuration system (even if that is a JDBC blob).
> * GeoServerDataDirectory is supposed to *be* the GEOSERVER_DATA_DIRECTORY.
>
> So this is why they were seperate - I like us gradually gathering up all
> the useful methods in GeoServerResourceLoader. This "fromURL" is always
> going to be strange because it is used resolve data references
>
> public Resource fromPath(String path) {
>>> return Resources.fromPath(path, resources.get(Paths.BASE));
>>> }
>>>
>>> So no need to add helper method here use:
>>> Resource r = Resources.fromPath( path, loader.get(Paths.BASE));
>>>
>>> Only thing I can think of is that you are trying to hide the fact we
>>> have a DATA_DIRECTORY used as starting point?
>>>
>>
>> These are pure convenience methods. I made them in analogy to the
>> previously already existing GeoServerResourceLoader.url convenience method,
>> which works on the exact same principle and was already there. If you
>> insist they can be removed, but I personally don't see the harm and I find
>> them neater to use than having to call Paths.BASE etc... But it's up to
>> you, say the word.
>>
>
> Keep them, but be very clear about what they do :)
>
> But really we should chat next month about removing deprecated methods so
> this API is less complex over time.
>
>
>
>
>
------------------------------------------------------------------------------
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
