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 <niels@anonymised.com> 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.
-
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.
-
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’).
-
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.
-
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