Re: [Geoserver-devel] resource store and base directory discussion

[Niels Charlier]

Hi Jody,

I have made a few minor edits to 
https://github.com/geoserver/geoserver/wiki/Cleaning-up-File-References


Now we just need to decide which of the two possibilities we prefer. I 
really don't mind which one.


Regards
Niels

On 18-11-15 01:48, Jody Garnett wrote:
Here is a branch where I am exploring the ideas brought up in our 
conversation:


https://github.com/jodygarnett/geoserver/commits/base_dir

So what is going to happen if you are not use the
FileSystemResourcestore?


The method 
GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext) should 
be used to obtain the GEOSERVER_DATA_DIRECTORY location, it should be 
retrieved from GeoServerResourceLoader.getBaseDirectory(). If we need 
to adjust method / wrappers / constructor order to make that happen so 
be it :)


The baseDirectory is really necessary, several pieces of code
depend on it, including classes loaded by Spring as well that need
it to be there.


I understand, it would help if we can short-list those cases so we can 
use them to know "when we are done".


The GeoServerResourceLoader is created around the ResourceStore,
so the ResourceStore itself cannot set a property of the
ResourceLoader on its initialisation. So what's going to happen?
If the ResourceStore doesn't provide it, what will? And if
something else provides it, why the need for the
FileSystemResourceStore still providing it?


The only reason FileSystemResourceStore is providing something is to 
fake out the system for testing purposes. This is what I meant about 
separating out the "real use" from the "test case" use.


We use a subclass of FileSystemResourceStore that obtains the value 
from 
GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext) 
during a real run.


I know that you think that the JDBCResourceStore should not
advertise a directory, but the point is this: even then GeoServer
still relies on a file system directory and the JDBCResourceStore
knows that directory (for importing & caching).


We can adjust the api for JDBCResourceStore to provide a directory, we 
just need to ensure it gets that value from 
GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext).


In my opinion the only real alternative to having a
getBaseDirectory for every resource store is to get rid of the
GeoServerResourceLoader.baseDirectory altogether. That would be
the only way to be truly consistent with your philosophy: that
there should be no assumption by GeoServerResourceLoader that the
store is file/directory based.


That is fine, to pull that off we still need to find a home for the 
value (for the code that needs file access). We could take this 
responsibility away from ResourceLoader and keep the 
GeoServerDataDirectory.root() method alive (I am glad to see most of 
the other methods there are deprecated and being replaced).


We could also force each ResourceStore (including JDBCResourceStore) 
to provide a getBaseDirectory() method as outlined above.


Then we could still cache the result of
GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext)
in a static variable, which was my second suggestion, and perhaps
slowly moving away from using it altogether.


I hope to quickly move away from it. Even for uses like geowebcache 
and importer they are mostly looking up a directory of their own 
choosing (rather than assuming a path relative to 
GEOSERVER_DATA_DIRECTORY).


--
___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Re: [Geoserver-devel] resource store and base directory discussion

[Jody Garnett]

Thanks Niels,

I think the responsible thing during application startup is to
have GeoServerResourceLoader look up the correct value
 using lookupGeoServerDataDirectory(ServletContext) during application
startup.

You have four examples of code wanting to do the file thing, they can call
GeoServerResourceLoader.getBaseDirectory() and have a party.

This is better than doing anything with GeoServerDataDirectory.root() which
is mostly deprecated  - hopefully you can finish it off.

For test cases we can have the base directory value supplied by the
injected FileSystemResourceLoader and short cut checking the ServletContext.


--
Jody Garnett

On 20 November 2015 at 01:27, Niels Charlier  wrote:

> Hi Jody,
>
> I have made a few minor edits to
> https://github.com/geoserver/geoserver/wiki/Cleaning-up-File-References
>
> Now we just need to decide which of the two possibilities we prefer. I
> really don't mind which one.
>
> Regards
> Niels
>
>
> On 18-11-15 01:48, Jody Garnett wrote:
>
> Here is a branch where I am exploring the ideas brought up in our
> conversation:
>
> https://github.com/jodygarnett/geoserver/commits/base_dir
>
> So what is going to happen if you are not use the FileSystemResourcestore?
>
>
> The method 
> GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext) should
> be used to obtain the GEOSERVER_DATA_DIRECTORY location, it should be
> retrieved from GeoServerResourceLoader.getBaseDirectory(). If we need to
> adjust method / wrappers / constructor order to make that happen so be it :)
>
>
>> The baseDirectory is really necessary, several pieces of code depend on
>> it, including classes loaded by Spring as well that need it to be there.
>
>
> I understand, it would help if we can short-list those cases so we can use
> them to know "when we are done".
>
> The GeoServerResourceLoader is created around the ResourceStore, so the
>> ResourceStore itself cannot set a property of the ResourceLoader on its
>> initialisation. So what's going to happen? If the ResourceStore doesn't
>> provide it, what will? And if something else provides it, why the need for
>> the FileSystemResourceStore still providing it?
>>
>
> The only reason FileSystemResourceStore is providing something is to fake
> out the system for testing purposes. This is what I meant about separating
> out the "real use" from the "test case" use.
>
> We use a subclass of FileSystemResourceStore that obtains the value from
> GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext)
> during a real run.
>
> I know that you think that the JDBCResourceStore should not advertise a
>> directory, but the point is this: even then GeoServer still relies on a
>> file system directory and the JDBCResourceStore knows that directory (for
>> importing & caching).
>>
>
> We can adjust the api for JDBCResourceStore to provide a directory, we
> just need to ensure it gets that value from GeoServerResourceLoader.lookup
> GeoServerDataDirectory(servletContext).
>
>
>> In my opinion the only real alternative to having a getBaseDirectory for
>> every resource store is to get rid of the
>> GeoServerResourceLoader.baseDirectory altogether. That would be the only
>> way to be truly consistent with your philosophy: that there should be no
>> assumption by GeoServerResourceLoader that the store is file/directory
>> based.
>>
>
> That is fine, to pull that off we still need to find a home for the value
> (for the code that needs file access). We could take this responsibility
> away from ResourceLoader and keep the GeoServerDataDirectory.root()
> method alive (I am glad to see most of the other methods there are
> deprecated and being replaced).
>
> We could also force each ResourceStore (including JDBCResourceStore) to
> provide a getBaseDirectory() method as outlined above.
>
> Then we could still cache the result of
>> GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext) in a
>> static variable, which was my second suggestion, and perhaps slowly moving
>> away from using it altogether.
>>
>
> I hope to quickly move away from it. Even for uses like geowebcache and
> importer they are mostly looking up a directory of their own choosing
> (rather than assuming a path relative to GEOSERVER_DATA_DIRECTORY).
>
>
>
--
___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Re: [Geoserver-devel] resource store and base directory discussion

[Niels Charlier]

On 17-11-15 02:45, Jody Garnett wrote:
> public GeoServerResourceLoader(ResourceStore resourceStore) {
> if( resourceStore instanceof FileSystemResourceStore){
> FileSystemResourceStore files = (FileSystemResourceStore) 
> resourceStore;
> this.baseDirectory = files.getBaseDirectory();
> }
> else {
> this.baseDirectory = null; // client code will supply via 
> setBaseDirectory
> }
> this.resources = resourceStore;
> }

So what is going to happen if you are not use the 
FileSystemResourcestore? The baseDirectory is really necessary, several 
pieces of code depend on it, including classes loaded by Spring as well 
that need it to be there. The GeoServerResourceLoader is created around 
the ResourceStore, so the ResourceStore itself cannot set a property of 
the ResourceLoader on its initialisation. So what's going to happen? If 
the ResourceStore doesn't provide it, what will? And if something else 
provides it, why the need for the FileSystemResourceStore still 
providing it?

I know that you think that the JDBCResourceStore should not advertise a 
directory, but the point is this: even then GeoServer still relies on a 
file system directory and the JDBCResourceStore knows that directory 
(for importing & caching).

In my opinion the only real alternative to having a getBaseDirectory for 
every resource store is to get rid of the 
GeoServerResourceLoader.baseDirectory altogether. That would be the only 
way to be truly consistent with your philosophy: that there should be no 
assumption by GeoServerResourceLoader that the store is file/directory 
based.

Then we could still cache the result of 
GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext) in 
a static variable, which was my second suggestion, and perhaps slowly 
moving away from using it altogether.

Regards
Niels

--
___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Re: [Geoserver-devel] resource store and base directory discussion

[Jody Garnett]

Here is a branch where I am exploring the ideas brought up in our
conversation:

https://github.com/jodygarnett/geoserver/commits/base_dir

So what is going to happen if you are not use the FileSystemResourcestore?


The method GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext)
should
be used to obtain the GEOSERVER_DATA_DIRECTORY location, it should be
retrieved from GeoServerResourceLoader.getBaseDirectory(). If we need to
adjust method / wrappers / constructor order to make that happen so be it :)


> The baseDirectory is really necessary, several pieces of code depend on
> it, including classes loaded by Spring as well that need it to be there.


I understand, it would help if we can short-list those cases so we can use
them to know "when we are done".

The GeoServerResourceLoader is created around the ResourceStore, so the
> ResourceStore itself cannot set a property of the ResourceLoader on its
> initialisation. So what's going to happen? If the ResourceStore doesn't
> provide it, what will? And if something else provides it, why the need for
> the FileSystemResourceStore still providing it?
>

The only reason FileSystemResourceStore is providing something is to fake
out the system for testing purposes. This is what I meant about separating
out the "real use" from the "test case" use.

We use a subclass of FileSystemResourceStore that obtains the value from
GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext) during
a real run.

I know that you think that the JDBCResourceStore should not advertise a
> directory, but the point is this: even then GeoServer still relies on a
> file system directory and the JDBCResourceStore knows that directory (for
> importing & caching).
>

We can adjust the api for JDBCResourceStore to provide a directory, we just
need to ensure it gets that value from GeoServerResourceLoader.lookup
GeoServerDataDirectory(servletContext).


> In my opinion the only real alternative to having a getBaseDirectory for
> every resource store is to get rid of the
> GeoServerResourceLoader.baseDirectory altogether. That would be the only
> way to be truly consistent with your philosophy: that there should be no
> assumption by GeoServerResourceLoader that the store is file/directory
> based.
>

That is fine, to pull that off we still need to find a home for the value
(for the code that needs file access). We could take this responsibility
away from ResourceLoader and keep the GeoServerDataDirectory.root() method
alive (I am glad to see most of the other methods there are deprecated and
being replaced).

We could also force each ResourceStore (including JDBCResourceStore) to
provide a getBaseDirectory() method as outlined above.

Then we could still cache the result of
> GeoServerResourceLoader.lookupGeoServerDataDirectory(servletContext) in a
> static variable, which was my second suggestion, and perhaps slowly moving
> away from using it altogether.
>

I hope to quickly move away from it. Even for uses like geowebcache and
importer they are mostly looking up a directory of their own choosing
(rather than assuming a path relative to GEOSERVER_DATA_DIRECTORY).
--
___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel