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
