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
[email protected]
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
