Hello Jody,
We have a very different way of looking at things, but what matters most
is to find a solution moving forward even if we cannot convince each
other. So let's try to focus on what we each find most important and try
to find a compromise that respects both sides.
The resource store is an abstract entity that is part of geoserver that
makes no assumption on file system or OS, the syntax is the same
regardless of OS, so to me resource paths are not file paths and vice
versa, they live in separate worlds. What is most important to me here
is that resource paths are OS independent.
As far as point (3) concerns, the javadoc at the moment is confused and
confusing so that needs to be resolved one way or another. It starts
with talking about resource paths, but then suddenly it starts talking
about differences between operating systems. Resource paths should
remain to be OS independent, so we need to be clear about this. My
preference would be that resource paths and OS-dependent file paths are
clearly separated from each other, and therefore these methods should
not be part of the same utility class, but the bare minimum for me would
be that the javadoc is at least clear about the distinction.
As far as point (1) concerns. The user has already been able to use
"resource:" paths for many years, but it is in no way obligated to so
so, their experience will indeed not change at all. (Admittedly, the
only application I could find though are image references in SLD files.
) The javadoc from Resources.fromURL specifies: "File URL - will support
relative file references - this is deprecated, use resource: instead".
This was the original idea and the reasoning is 1) to make the
distinction between OS dependent file paths and OS independent resource
paths; 2) relative file paths violate the URL standard. So from my
perspective, this would be the logical way forward but since we cannot
agree on this I would suggest to not provide any warning and continue to
allow both. On the same token, I do not consider resource paths with a
leading slash to be 'relative', in fact to me they are absolute and make
perfect sense and as long as they are part of a resource: URL or used
directly with the resource store they cannot be misinterpreted, so I
would prefer not to generate a warning for them.
So the way forward I suggest would be to have no warnings and make sure
that the API functionality itself cannot be misinterpreted or misused,
whichever way you want to look at it.
Concerning (4), there is indeed a page about the resources API. But for
new developers, this could be interpreted as an API that is free to use
or ignore. I mean to write some documentation with guidelines that all
developers should know to keep geoserver generic with respect to usage
of the data directory.
Kind Regards
Niels
On 13/09/2023 16:34, Jody Garnett wrote:
You are arriving with this right as we try and find volunteers to make
the RC :)
Please make a PR immediately and we can try and get it in.
I am unclear about (1) - Can I confirm that users would not see
resource: URLs, only developers? I do not think relative paths should
produce a warning as they are the vast majority of cases - and the
point of what we are trying to do?
The end user experience is what matters here and should not change. It
is slightly better if the rest api changes as (I think it could
produce warnings if absolute path is sent and ignored - indeed it may
already).
I strongly agree with (2). It can provide a warning for the leading
slash please, and correct it. The point is a path string should be
consistent everywhere in the codebase.
I do not agree with (3) the point is to break resource stor me paths
so that we have consistency in our code base internals.
(4) the page exists and can be clarified.
Niels I am not understanding why you wish paths to be different? Each
opportunity to convert between paths produced problems in our codebase
that we are seeking to resolve by being strict? What am I missing …
On Wed, Sep 13, 2023 at 5:30 AM Niels Charlier <ni...@scitus.be> wrote:
Hello Jody and Gabriel,
Sorry for my late response. I have a suggestion to resolve the
issue, please let me know what you think,
My suggestion is that I would make a pull request with the
following proposed changes, and you would review it.
1. The methods Resources.fromPath as well as Resources.fromURL
with a "file:" URL follow your guidelines: relative paths are
interpreted as resource store paths, absolute paths are
interpreted as file system paths. However, I suggest that a
relative "file:" URL produces a warning message that says that
relative "file:" URLs are deprecated and that one should use a
"resource:" URL instead.
2. The resource store implementation is separate from the two
methods as specified above, and only provides access to files that
are either in the data directory (file system resource store) or
part of an external emulation of a file/folder hierarchy (such as
jdbc database for the jdbc resource store). It has its own "root",
and therefore the ResourceStore.get(...) method returns the same
result with or without leading slash. A unit tests should check
that the store does not provide any access to absolute paths on
the file system. Javadoc should explain that Resources.fromPath or
Resources.fromURL should be used for access to paths and URLs in
general, and that direct usage of the resource store is only for
access to configuration files and directories with a hard coded
location (relative to the 'data directory').
3. The Paths class javadoc is fixed or the class is refactored so
that there is a clear distinction between resource store paths and
file paths.
4. A page is added to the developer guide that specifies the above
guidelines as well as general guidelines on how geoserver
developers should deal with resources, URLs, files, etc. so that
the generic nature of the resource store API is preserved in the
future.
I look forward to your feedback.
Kind Regards
Niels
On 18/08/2023 17:35, Jody Garnett wrote:
Niels:
I checked if we were having a work party today, and put a little
time into the activity.
And I finally have a useful answer for you: Why did Resource
absolute path change from being relative to the data directory?
The answer is ... the concept of a Path was used all over our
codebase, with different conversions happening, especially around
absolute paths. This caused problems with a Java 11 change for
windows which revealed how many different conventions we had in
the codebase.
So something had to change - paths was made consistent - so if
you stopped with in a debugger, at any point, and saw a "path"
you could have confidence in what it means.
To that end a relative path (anywhere in the codebase) is now
relative to the data directory location.
There is a little bit of code in the REST Resource API that is
forgiving for external scripts that use an absolute path. But it
is changed to be consistent (ie relative path) as soon as possible.
If you find any other special cases we can make note of them.
--
Jody Garnett
--
--
Jody Garnett
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel
