Paths.names (method used by multiple ResourceStores) not removing root slash

Description

As a consequence, when creating a resource "/some/file/in/the/data/dir" or "some/file/in/the/data/dir" results in two unequal resources, even though they are one and the same.

Environment

None

Activity

Ben Caradoc-Davies
April 24, 2016 at 8:36 PM

I checked and this was included in 2.9-beta2.

Niels Charlier
April 21, 2016 at 9:36 AM

Oh and why this was necessary. In the tests, path logic is used to
create new resources.
That is why it is necessary the paths make consistent sense.

For example, If there are two files
1) one has relative path "/some/file" inside "/some/base"
2) the other one has relative path "/some/file" inside "/some/other/base"

You make a resourcewrapper from each of those. Both will return the same
path().
If you create a new resource from that path, at least one of them is
going to be pointing at a different file.
That doesn't make any sense.

And again, all I did was follow the official API which says relative
paths are not supported.

Regards
Niels

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Niels Charlier
April 21, 2016 at 9:25 AM

When you to take the path of a resource, it isn't possible to derive
from the path whether it is absolute or relative in Linux (Resource
paths according to the specs do not support relative paths and make no
difference between a starting slash and no starting slash).

I don't think this would be dangerous or affect anything.
ResourceWrappers are used to provide simple files to objects and methods
that expect a Resource. They should be threaten like a normal resource,
and are mostly used to directly read and write to it.

Returning a relative file path from a resource provides incomplete,
nonsensical information to somebody who just wants to use the resource API.

This change simply guarantees that the path returned is unique and makes
some kind of consistent sense to somebody who is following the resource API.

Regards
Niels

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Jody Garnett
April 20, 2016 at 10:33 PM

We have three things:

  • Configuration stuff in the data directory --> resource api

  • Files ( relative files in the data directory, absolute files for external
    references ) - should only be used as connection parameters now

  • and this thing ResourceAdaptor which served to bridge the two worlds
    during the transition

So I guess I want context? If the transition is done (I think it is!) then
yes ResourceAdaptor could be restricted to only using absolute file
references.

What was the need to convert a relative filename to an absolute file name?
Whatever the need was it strikes me as a gap in the story above....


Jody Garnett

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Ben Caradoc-Davies
April 20, 2016 at 10:27 PM

Jody, it is not essential to get this into 2.9-beta2. If it is not
included, then please change the Fix Version of https://osgeo-org.atlassian.net/browse/GEOS-7493#icft=GEOS-7493 to 2.9-RC1.

I considered the impact of the change you noted below, and also whether
it should be getCanonicalFile (and I thought not). As far as I know,
there is no new requirement for the file to exist, and this change did
not break any resource theory tests. How do you think it changes the
capability of the class? It is a change that caught my attention too.
Thanks for your eagle-eyed scrutiny!

I could speculate why this change was made but I would rather let Niels
answer for himself. Niels?

Kind regards,
Ben.


Ben Caradoc-Davies <ben@transient.nz>
Director
Transient Software Limited <http://transient.nz/>
New Zealand

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Fixed

Details

Assignee

Reporter

Fix versions

Priority

Created April 12, 2016 at 2:33 PM
Updated April 24, 2016 at 8:36 PM
Resolved April 24, 2016 at 8:36 PM