Hello Jody,
Yes, I admit it's my own fault for missing this discussion at the time.
I think it would be a shame to let the codebase further drift away from
the intended use of the resource store. We have done the work to make
the entirety of geoserver generic with respect to the implementation of
the data directory and it is only a minimal effort to keep it this way.
Even though jdbc store is still a community module and the Redis based
geoserver project has been discontinued, it has been proven that
alternative implementations for the data directory work and the jdbc
store module is actually being used in production successfully.
Now, interestingly, I have discovered that there are contradictions in
how it works / is documented now.
- I discovered that while 'theoryRootIsAbsolute' test is successful, it
is actually very misleading. At least on linux, paths starting with '/'
*still create a file relative to the data directory*!. The only
difference is the string returned by the path() method. So while the
path might seem absolute, the file you are accessing is not. So the
path() method is misleading and in reality the leading slash is still
being ignored (again, at least on linux).
- Note that the javadoc of ResourceStore still says:
This abstraction assumes a unix like file system, *all paths are relative*
There it says it: all paths are relative for the resourcestore. Absolute
paths have no meaning or function when it comes to the resource store.
- I think this contradiction could be resolved in two possible ways:
1) the test 'theoryIsRootSlashIsIgnored' should come back instead of
'theoryRootIsAbsolute'. The root slash is ignored and removed from the path.
2) The resource store throws an exception when you ask for an absolute path.
Either way, all absolute paths should be handled _outside of_ the
ResourceStore, for instance by calling Files.asResource(). So problems
with absolute paths in the rest service should be resolved in the rest
service, or some other layer that is used by the rest service.
Kind Regards
Niels
On 04/07/2023 21:59, Jody Garnett wrote:
Hello Niels,
The PRs for this activity contain extensive discussion.
The fundamental issue was the handling of absolute paths which was
done differently by different sections of code. Specifically we found
that the REST API endpoint was allowing paths *//data* and */data *to
both reference content in the data directory, rather than treating the
first one as an absolute path. In response we tightened up the
javadocs and added test cases including the one you mentioned.
I agree that this goes against the goal of resource store, but the
codebase had drifted away from the intended use over time.
Now that you are present in the discussion it would be a good
opportunity to discuss this with the parties involved.
--
Jody Garnett
On Jul 3, 2023 at 2:50:02 PM, Niels Charlier <ni...@scitus.be> wrote:
Hello Jody and others,
I am having trouble understanding the changes that were made about 6
months ago to the ResourceStore's expected behaviour.
In particular, in the class
'org.geoserver.platform.resource.ResourceTheoryTest', the unit test
'theoryRootSlashIsIgnored' was replaced by 'theoryRootIsAbsolute'. I
cannot make sense out of this theory test at all.
This seems to be entirely contradictory to the whole reason that the
ResourceStore API was created, that is to make an abstraction of the
/Data Directory/, so that it can be replaced by something else (such
as jdbc store or other implementations that have been made). There
was already support for absolute file paths in all circumstances by
using "file:" URLs. This will bypass the resource store and call
Files.asResource instead. But resource: URLs are for the data
directory or alternative resource store only. How does it make sense
to get absolute paths from the resource store?
In order to make jdbc-config pass the tests, I will have to turn off
this particular method. But why should the test even be there if the
file resource store is the only one that could ever support it?
Programmers and users will rely on this behaviour and support for all
alternative implementations of ResourceStore will be broken. In this
case we may as well do away with the API and just use the file system
directly again.
Kind Regards
Niels
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
