from:"Niels Charlier"

Re: [Geoserver-devel] build.geoserver geoserver-main-app-schema-online job maintenance / discussion.

2024-07-09 Thread Niels Charlier via Geoserver-devel

Seems you are missing the postgis extension, but I have no idea why that 
is in the script, that is strange indeed.


On 09/07/2024 08:28, Jody Garnett wrote:

Niels,

How do you run the app-schema tests?

 1. I create a ~/.geoserver/postgis.properties with connection
parameters including a database name
 2. And then run with:
mvn clean install -Papp-schema-online-test


When I run I get errors such as:

[ERROR] 
testNoPrimaryKey(org.geoserver.test.onlineTest.WfsOnlinePostgisTest) 
 Time elapsed: 0.294 s  <<< ERROR!

org.postgresql.util.PSQLException:
ERROR: function public.ndims(public.geometry) does not exist
  Hint: No function matches the given name and argument types. You 
might need to add explicit type casts.

  Position: 355

Looking at the build server job configuration I am shocked to see this 
is defined as part of the script:



#missing functions required for refdataset sql but removed in
postgis 2.1
export PGPASSWORD=''
psql -h localhost -p 5432 -U geotools -d geotoolstest -c 'create
or replace function public.ndims(g geometry) returns smallint as
$$ begin return public.st_ndims(g); end; $$ language plpgsql
immutable strict;'
psql -h localhost -p 5432 -U geotools -d geotoolstest -c 'create
or replace function public.srid(g geometry) returns integer as $$
begin return public.st_srid(g); end; $$ language plpgsql immutable
strict;'


Surely that cannot be right?
Is defining ndims part of the app schema installation instructions? No...

I How do you run the tests locally...
--
Jody Garnett


On Jul 8, 2024 at 5:29:42 AM, Niels Charlier  wrote:


Hello Jody

I have always run those tests locally whenever making changes that 
affect app-schema, not sure about Jenkins issues.


Kind Regards

Niels

On 06/07/2024 00:43, Jody Garnett wrote:
I have a PR 
<https://dficcfc.r.af.d.sendibt2.com/tr/cl/QnmEf4CiLzefQ8tvswpygu_mKOoVQYE_ymKm8YxI8KlC5Oky3xgemTOMFlheDxw7yfA0Ckx8oGqw94a_hRDOlVGXZewXdciN7xvD7M_rEd1R1rtu2mMm59pwZkkH2ZiMCWaeHCpmMWjPZd9E9F6M7TJtVQYL6Juaazw0CeqmAs90WSKZg5UI6sm8-3kOEC137-01Uu5tc2aY7FtDR-RwG1pkloVK2u99LweEcTF6j2KySuBjL90_-4IjweVKflwwU6cfARFQDGeH5iSioGKRuHZ7ZlMdX-73ykakFUTCQcrnS-EW> 
that looks at some gaps in the GeoTools feature model around 
validation... and Niels made the perfectly sensible recommendation 
to run the app-schema online tests...


 1. Checking build.geoserver.org

<https://dficcfc.r.af.d.sendibt2.com/tr/cl/XzwiyRYFMgufdjTffCOZKuLas1HzYo0A3G72jOEFBlOrZhk15PUdzFckDA1jFwcllUEiVQf6DKw68XxItKGLGa_2j5AP4TDPcUVDSLIaM6ZZkc1fnToljiZcp6hoYtejFQ9NY3IFWDnwTnMoyGKjIND376ahzpSRmwOuyi-qg9HKc_mfbNkdj2ZiWqCxPnIqI3RM8HSIx-C2D5RqmkXtDuQ-f-Ihutyv_OxLvVyT-j0MrOkLGFf2fr-6nKMioMiX750R24zo5Vw>
the geoserver-main-app-schema-online

<https://dficcfc.r.af.d.sendibt2.com/tr/cl/Ds5yMP0W1nLQwCo3m3ZabKQg8Jh23U86KCnuN506HyWCU_4ODqDvVJrIJzYbBRDZwAuXUBwR71PGYTp23w6jz_JmJtiPcq0MQTZygJYByi5KCStdM_mNHPWewpnAKJmLclqereIIZ0vgfvcNEtpqj82LW2kzfLFIpFvLJSVpK3L8sKE6Kq9c2I80ouevPJIq7lfJfNVHLgfK9fgV766Z2zy1Ky67C9bcm__h3JLGg8QycLVrCSz9SHnqqmb7T3Xh0n3NLvQjE3kM-iKcCDi8clbWOmRL7HOz2o-uC2ugxZml7bxBSUnbVUPUnUJjRQFX_fjZEPrpBXq4DznMFs1KD7p71tgFScz9>
is disabled
 2. Looking at the configuration it was java 8 based, so this has
been disabled for some time
 3. The setup has a lot of messing around to get the ojdbc7.jar to
talk to oracle-db.dev.boundlessgeo.com

<https://dficcfc.r.af.d.sendibt2.com/tr/cl/unOkFWXeyJmZIY8qRTRfEOWS7MGyOk4vkdzoJyUiwkcH9yRsDojjPS_a77MX248fX9iZJGLRLX_qf5Rhme4iUNeTWwPvePI2TTI46ny9WtEcprgjPVkdaKgW5yZV8sbuW4InphYTDd0l1E7xMMxaUvthsp69SyJB2DiWJ4IJiYU7u2Kgb2xPk_qYdiLx57HZKfzX2_SwJtpS5nD-ZmeJm7F5DF2VZKTj6qgTUd6HnHQnvGM9X1JvFm60YhkBoEIJfXLt7P-eHDHbvODtvxVsyQK_lQ>
... so quite a long time.


Do we have an active module maintainer (or experienced developer) 
for application schema?


Trying it out:

  * I do not think the steps to copy ojdbc7.jar are required any
more (since such a jar is now available via maven dependency)
  * Changing to Java 11 and commenting out the oracle.properties
test fixed and running the job shows

<https://dficcfc.r.af.d.sendibt2.com/tr/cl/LfbWvC9tnFQ2Kw5ymOBeMXXeT8KmZn9PWK0NdXq52y5AYgfeX8mz06QT5ntFE-CKVt-ayEtPArTWSd2tixGSIEsOCw19g25HJNXHOpr7XEwluNq1_BUiIrzgr0flipi2EIBE8KbuuRpPJXKaGRA2Lc0Nq-UGcqd0p5tdbGWmOc5AA4-VnX7OxNt0O9ruU8yWc999jWsXXc3VeywtedDPUsdFmae6wzsRP-4lxATCFNfq7_TZn2NA9YJxiX_sa-razs2GEF-J8k9jk_XwnjqgwFLRgTL0v7uOSSiOdKI3ZO1kYTVxFfvfGfeD35FNb9SHic25FOFs2A6n63ff920JMw9Ed0YNZZmfuzP9lex9>
...


/[INFO] Application Schema Support .
SUCCESS [  4.694 s]/
/[INFO] Application Schema Module ..
SUCCESS [  9.101 s]/
/[INFO] Application Schema Integration Test 
FAILURE [01:28 min]/
/[INFO] Sample DataAccess Integration Test .
SUCCESS [  4.919 s]/
/[INFO] Application Schema Integration Online Test w

Re: [Geoserver-devel] build.geoserver geoserver-main-app-schema-online job maintenance / discussion.

2024-07-08 Thread Niels Charlier via Geoserver-devel


Hello Jody

I have always run those tests locally whenever making changes that 
affect app-schema, not sure about Jenkins issues.


Kind Regards

Niels

On 06/07/2024 00:43, Jody Garnett wrote:
I have a PR  that 
looks at some gaps in the GeoTools feature model around validation... 
and Niels made the perfectly sensible recommendation to run the 
app-schema online tests...


 1. Checking build.geoserver.org   the
geoserver-main-app-schema-online


is disabled
 2. Looking at the configuration it was java 8 based, so this has been
disabled for some time
 3. The setup has a lot of messing around to get the ojdbc7.jar to
talk to oracle-db.dev.boundlessgeo.com
 ... so quite a long time.


Do we have an active module maintainer (or experienced developer) for 
application schema?


Trying it out:

  * I do not think the steps to copy ojdbc7.jar are required any more
(since such a jar is now available via maven dependency)
  * Changing to Java 11 and commenting out the oracle.properties test
fixed and running the job shows


...


/[INFO] Application Schema Support .
SUCCESS [  4.694 s]/
/[INFO] Application Schema Module ..
SUCCESS [  9.101 s]/
/[INFO] Application Schema Integration Test 
FAILURE [01:28 min]/
/[INFO] Sample DataAccess Integration Test .
SUCCESS [  4.919 s]/
/[INFO] Application Schema Integration Online Test with Postgis
Database SUCCESS [01:14 min]/
/[INFO] Application Schema Solr Integration Test ...
SUCCESS [  2.659 s]/
/[INFO] Application Schema Indexes Integration Test 
SUCCESS [  1.319 s]/
/[INFO] Application Schema Geopackage Integration Test .
FAILURE [ 11.730 s]/

*
*
So what do you think? Does this test have value? For now I will enable 
it but not schedule it @daily (in case anyone else wants to look).


Thanks
--
Jody Garnett


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

[Geoserver-devel] Hibernate version

2024-02-19 Thread Niels Charlier via Geoserver-devel

Hello,Currently, the managed version of Hibernate is 3.6.9. This version is rather ancient, from 2011.  It is used by one extension: geofence-server; and one community module: monitor-hibernate.Taskmanager, a community module that I maintain, uses a more recent version of hibernate: 5.3.20. This does not pose any issues in building and deploying as long as you do not combine it with either one of the two other modules. However, the plugin ZIP file is broken and this cannot be fixed because the 'release' module must choose one version for everything.  (This was reported by a user on jira: https://osgeo-org.atlassian.net/browse/GEOS-10690)Are there any plans to upgrade the hibernate version of the other modules? I don't think it even involves even any considerable coding work, although it may generate a lot of 'deprecated' warnings.  I noticed there has been talk about removing 'monitor-hibernate' altogether. Since the community dir has its own release module, I suppose it would be technically possible to change the version only there, but perhaps rather confusing.Kind RegardsNuiels

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

Re: [Geoserver-devel] Upcoming 2.25-RC Release candidate planning

2024-02-19 Thread Niels Charlier via Geoserver-devel


Hi Gabe,

I added more information in the pull request.

Kind Regards

Niels

On 16/02/2024 15:36, Gabriel Roldan wrote:

Hi,

I do want to get involved in the ResourceStore API discussion.

Unfortunately, I'll be on vacation next week, so if you don't mind I 
propose we leave it off from this release.


Meanwhile, I'd like to ask Niels to update the PR description with a 
(rather succinct) description of the issue and breakdown of the 
approach. That helps a lot in reviewing, and going through the very 
long discussion on the email list is draining.


TIA,
Gabe
*camptocamp*
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTS

*Gabriel Roldán*
Geospatial Developer*
*



On Fri, Feb 16, 2024 at 4:59 AM Jody Garnett  
wrote:


We are a couple weeks a way from the 2.25-RC and I am aware of a
number of outstanding pull-requests that are wishing review.

  * [GEOS-11284] Promote community module "datadir catalog loader"
to core 
  o Andrea is presently reviewing
  * [GEOS-11050] Refactor Resources and Paths API to support
alternative 
  o Jody (and possibly Gabe) to review
  * If there is anything else please speak up!


NOTICE: As per last meeting I volunteered to do this release,
checking my work calendar I may be doing this a few days early
(Feb 28-29).
--
Jody Garnett
___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel



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


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

Re: [Geoserver-devel] Upcoming 2.25-RC Release candidate planning

2024-02-17 Thread Niels Charlier via Geoserver-devel


Hello Jody,

Just to be clear, in my pull request I did take account of our 
discussion and your concerns, we had arrived at a vague outline for a 
compromise that I tried to respect as much as possible, filling in the gaps.


I will provide more information soon.

Kind Regards

Niels

On 16/02/2024 20:49, Jody Garnett wrote:
Thanks Gabe for being clear - we are operating with reduced resources 
these days and I need to ask for help where it can be effective.


Niels I have set aside time next week to fix the bug getting in your 
way (for sure); and go over the javadocs API changes to see if we can 
work out a compromise.

--
Jody Garnett


On Feb 16, 2024 at 6:49:35 AM, Niels Charlier via Geoserver-devel 
 wrote:


Hello Gabe,

Okay, I will do that.

I wish I had done it earlier actually, because it would have helped 
me remember the details which are already fading :)


Regards

Niels

On 16/02/2024 15:36, Gabriel Roldan wrote:

Hi,

I do want to get involved in the ResourceStore API discussion.

Unfortunately, I'll be on vacation next week, so if you don't mind I 
propose we leave it off from this release.


Meanwhile, I'd like to ask Niels to update the PR description with a 
(rather succinct) description of the issue and breakdown of the 
approach. That helps a lot in reviewing, and going through the very 
long discussion on the email list is draining.


TIA,
Gabe
*camptocamp*
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTS

*Gabriel Roldán*
Geospatial Developer*
*



On Fri, Feb 16, 2024 at 4:59 AM Jody Garnett 
 wrote:


We are a couple weeks a way from the 2.25-RC and I am aware of a
number of outstanding pull-requests that are wishing review.

  * [GEOS-11284] Promote community module "datadir catalog
loader" to core

<https://dficcfc.r.af.d.sendibt2.com/tr/cl/uy2GjYiTnyfYzry_ryOcCtUfK1cFmz7nQcOz5pB4znyVuRZ8gKnn6WAccArwEtvngkFKPhDWQd9igi0SIm3Jo3_haZoSTyJHPrUUePjeAQIrHKF7F74Ghj63wnkddVLgvsXVS2uJKDfJgrNKPcje3HtClqHEuqR7vLIYEt28MF1r1NQPXn-YmBcKIY5CcOu9hB6n3zI_BaVcx3slejOQNhwypnOj5UmbLNFc2JaDR0yJiFvMbNTl7gnn1Vo_WU3993Q3G-Tjk8HP6G5ePQ76OI6kzJU_j9wqAtZ_yVwriyDMVj4gr_RYLHw6_xCTkQVmuDosYEY>
  o Andrea is presently reviewing
  * [GEOS-11050] Refactor Resources and Paths API to support
alternative

<https://dficcfc.r.af.d.sendibt2.com/tr/cl/li4NVJ7rI9pcvsHNOrTXSL4ky3IZENeOhT2vNHM8QeuzVPgGHzRCiudEL4DZyHs1Pn_9uk1D8RwumUGFgPaBufAnTkOEoHl9RQoQm-k1Uh4FgV9-llvZXa0KPVF3PH4MP7JuXUKfSvSlMZt0CsIAVKEaz74GXQo3bzdlHVOwb7tnaAhRstmYlEkay-tU-qjRx2W_-8M6rTYZh85EPiRdvZLASpXNmw6FKC6KeB8P4Zlo9n7-bwpE2Bk7M7SA8hcny5sTe7eZnwJIJsM70pcU3VY0C8FdOT3rwRxFC6kEXN7BsN6Bw5438jw_DOorn7Ol_lKdG9Y>
  o Jody (and possibly Gabe) to review
  * If there is anything else please speak up!


NOTICE: As per last meeting I volunteered to do this release,
checking my work calendar I may be doing this a few days early
(Feb 28-29).
--
Jody Garnett
___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel



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

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


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

Re: [Geoserver-devel] Upcoming 2.25-RC Release candidate planning

2024-02-16 Thread Niels Charlier via Geoserver-devel


Hello Gabe,

Okay, I will do that.

I wish I had done it earlier actually, because it would have helped me 
remember the details which are already fading :)


Regards

Niels

On 16/02/2024 15:36, Gabriel Roldan wrote:

Hi,

I do want to get involved in the ResourceStore API discussion.

Unfortunately, I'll be on vacation next week, so if you don't mind I 
propose we leave it off from this release.


Meanwhile, I'd like to ask Niels to update the PR description with a 
(rather succinct) description of the issue and breakdown of the 
approach. That helps a lot in reviewing, and going through the very 
long discussion on the email list is draining.


TIA,
Gabe
*camptocamp*
INNOVATIVE SOLUTIONS
BY OPEN SOURCE EXPERTS

*Gabriel Roldán*
Geospatial Developer*
*



On Fri, Feb 16, 2024 at 4:59 AM Jody Garnett  
wrote:


We are a couple weeks a way from the 2.25-RC and I am aware of a
number of outstanding pull-requests that are wishing review.

  * [GEOS-11284] Promote community module "datadir catalog loader"
to core 
  o Andrea is presently reviewing
  * [GEOS-11050] Refactor Resources and Paths API to support
alternative 
  o Jody (and possibly Gabe) to review
  * If there is anything else please speak up!


NOTICE: As per last meeting I volunteered to do this release,
checking my work calendar I may be doing this a few days early
(Feb 28-29).
--
Jody Garnett
___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel



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


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

Re: [Geoserver-devel] Random failure in gs-metadata keeps on breaking builds...

2023-12-01 Thread Niels Charlier via Geoserver-devel


Hello Andrea,

You are right on both accounts; that stacktrace should not pollute the 
output.


The other error indeed... shouldn't happen. The spring test runner does 
not change the behavior of the order of before calls. I cannot find any 
information on this happening to other people, and the code of that that 
runner class suggests it should be the same. I am quite perplexed. I am 
looking for a work-around that should prevent this from ever happening.


Pull request is on its way.

Regards

Niels

On 27/11/2023 19:16, Andrea Aime wrote:

Hi all,
I was checking a random failure in gs-metadata module, that is 
breaking the builds on Github and the build server (there are others, 
this one is popping up frequently though).


The logs are ridden with 300+ instances of errors similar to this one:

Nov 27, 2023 6:22:16 PM 
org.geoserver.metadata.data.service.impl.ConfigurationServiceImpl 
readConfiguration
SEVERE: Cannot construct instance of 
`org.geoserver.metadata.data.dto.impl.MetadataConfigurationImpl` 
(although at least one Creator exists): no String-argument 
constructor/factory method to deserialize from String value 
('klopt-niets-van')
 at [Source: 
(org.geoserver.platform.resource.FileSystemResourceStore$FileSystemResource$1); 
line: 1, column: 1]
com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot 
construct instance of 
`org.geoserver.metadata.data.dto.impl.MetadataConfigurationImpl` 
(although at least one Creator exists): no String-argument 
constructor/factory method to deserialize from String value 
('klopt-niets-van')
 at [Source: 
(org.geoserver.platform.resource.FileSystemResourceStore$FileSystemResource$1); 
line: 1, column: 1]
at 
com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:63)
at 
com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1739)
at 
com.fasterxml.jackson.databind.DeserializationContext.handleMissingInstantiator(DeserializationContext.java:1364)
at 
com.fasterxml.jackson.databind.deser.std.StdDeserializer._deserializeFromString(StdDeserializer.java:311)
at 
com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromString(BeanDeserializerBase.java:1504)
at 
com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:197)
at 
com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:187)
at 
com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:323)
at 
com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4825)
at 
com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3809)
at 
org.geoserver.metadata.data.service.impl.ConfigurationServiceImpl.readConfiguration(ConfigurationServiceImpl.java:195)


A few things:

  * The exception is per se legit, one cannot load
a MetadataConfigurationImpl from a YAML file that contains only
the string 'klopt-niets-van')
  * The code is just logging the error... but making a lot of noise
about it, especially at the current SEVERE logging level. More so
considering that the code is blindly looping over files in a
directory and trying to deserialize 3 different types of clases in
turn, try the next if one fails

.
This needs to be fixed, we cannot have tests logging so loudly for
nothing (nor we should have runtime code doing so).
  * For all the noise it's making, it's not the actual cause of the
failure.

Niels could you please remove this wall of text from our build execution?

Now, going to the actual failure, it's just one:

[ERROR] Tests run: 6, Failures: 0, Errors: 1, Skipped: 0, Time 
elapsed: 4.447 s <<< FAILURE! - in 
org.geoserver.metadata.web.TemplatePageTest [ERROR] 
testPageNewSave(org.geoserver.metadata.web.TemplatePageTest) Time 
elapsed: 0.043 s <<< ERROR! java.lang.NullPointerException

at 
org.geoserver.metadata.web.TemplatePageTest.before(TemplatePageTest.java:37)
At line 37 the only NPE I see happening 
, is having the tester field being null.
That should not be happening, because JUnit ensures that the @before 
methods of a parent class are run before the ones of the children, and 
we have this 
.


However... the tests are not using vanilla JUnit, but Spring own test 
runner 
, 
which runs the tests in a different way

Re: [Geoserver-devel] reviews pull requests metadata and csw

2023-10-08 Thread Niels Charlier via Geoserver-devel


It is not that urgent, it can be backported.

Kind Regards

Niels

On 05/10/2023 17:49, Jody Garnett wrote:

Niels,

Are these changes intended for the 2.24 release? I do not personally 
use this extension but I can review if this is a change you are trying 
to get in as part of RC feedback.

--
Jody Garnett


On Oct 5, 2023 at 5:51:26 AM, Niels Charlier via Geoserver-devel 
 wrote:

Hello,

I have two pull requests for metadata and csw extensions:

https://github.com/geoserver/geoserver/pull/7072

https://github.com/geoserver/geoserver/pull/7073

Is anyone able/willing to review them?

@Andrea, you have already commented on both of them, and I addressed
your remarks. I absolutely do not wish to sound demanding or impatient,
but could you clarify if you will still approve them or if I should find
another reviewer? Either way I am grateful for any help.

Kind Regards

Niels

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


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

[Geoserver-devel] reviews pull requests metadata and csw

2023-10-05 Thread Niels Charlier via Geoserver-devel

Hello,I have two pull requests for metadata and csw extensions:https://github.com/geoserver/geoserver/pull/7072https://github.com/geoserver/geoserver/pull/7073Is anyone able/willing to review them?@Andrea, you have already commented on both of them, and I addressed your remarks. I absolutely do not wish to sound demanding or impatient, but could you clarify if you will still approve them or if I should find another reviewer? Either way I am grateful for any help.Kind RegardsNiels

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

Re: [Geoserver-devel] Paths utility class

2023-09-13 Thread Niels Charlier via Geoserver-devel


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  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 specifi

Re: [Geoserver-devel] Paths utility class

2023-09-13 Thread Niels Charlier via Geoserver-devel


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

[Geoserver-devel] [JIRA] (GEOS-11101) metadata: multiple tabs

2023-08-13 Thread Niels Charlier (JIRA) via Geoserver-devel

Niels Charlier ( 
https://osgeo-org.atlassian.net/secure/ViewProfile.jspa?accountId=557058%3A1d2c4019-de4e-4624-8b27-2486941601cf
 ) *created* an issue

GeoServer ( 
https://osgeo-org.atlassian.net/browse/GEOS?atlOrigin=eyJpIjoiY2QwMjA0NTE2MmRmNDI2NGIzZGM4MDI1M2JkZTYyYWYiLCJwIjoiaiJ9
 ) / New Feature ( 
https://osgeo-org.atlassian.net/browse/GEOS-11101?atlOrigin=eyJpIjoiY2QwMjA0NTE2MmRmNDI2NGIzZGM4MDI1M2JkZTYyYWYiLCJwIjoiaiJ9
 ) GEOS-11101 ( 
https://osgeo-org.atlassian.net/browse/GEOS-11101?atlOrigin=eyJpIjoiY2QwMjA0NTE2MmRmNDI2NGIzZGM4MDI1M2JkZTYyYWYiLCJwIjoiaiJ9
 ) metadata: multiple tabs ( 
https://osgeo-org.atlassian.net/browse/GEOS-11101?atlOrigin=eyJpIjoiY2QwMjA0NTE2MmRmNDI2NGIzZGM4MDI1M2JkZTYyYWYiLCJwIjoiaiJ9
 )

Issue Type: New Feature Assignee: Niels Charlier ( 
https://osgeo-org.atlassian.net/secure/ViewProfile.jspa?accountId=557058%3A1d2c4019-de4e-4624-8b27-2486941601cf
 ) Created: 14/Aug/23 7:55 AM Priority: Medium Reporter: Niels Charlier ( 
https://osgeo-org.atlassian.net/secure/ViewProfile.jspa?accountId=557058%3A1d2c4019-de4e-4624-8b27-2486941601cf
 )

Sometimes there are a lot of custom metadata fields, we’d like to split them in 
multiple tabs to navigate.

( 
https://osgeo-org.atlassian.net/browse/GEOS-11101#add-comment?atlOrigin=eyJpIjoiY2QwMjA0NTE2MmRmNDI2NGIzZGM4MDI1M2JkZTYyYWYiLCJwIjoiaiJ9
 ) Add Comment ( 
https://osgeo-org.atlassian.net/browse/GEOS-11101#add-comment?atlOrigin=eyJpIjoiY2QwMjA0NTE2MmRmNDI2NGIzZGM4MDI1M2JkZTYyYWYiLCJwIjoiaiJ9
 )

Get Jira notifications on your phone! Download the Jira Cloud app for Android ( 
https://play.google.com/store/apps/details?id=com.atlassian.android.jira.core&referrer=utm_source%3DNotificationLink%26utm_medium%3DEmail
 ) or iOS ( 
https://itunes.apple.com/app/apple-store/id1006972087?pt=696495&ct=EmailNotificationLink&mt=8
 ) This message was sent by Atlassian Jira (v1001.0.0-SNAPSHOT#100234- 
sha1:39b0f46 )___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

[Geoserver-devel] [JIRA] (GEOS-11096) csw: support multiple mappings

2023-08-06 Thread Niels Charlier (JIRA) via Geoserver-devel

Niels Charlier ( 
https://osgeo-org.atlassian.net/secure/ViewProfile.jspa?accountId=557058%3A1d2c4019-de4e-4624-8b27-2486941601cf
 ) *created* an issue

GeoServer ( 
https://osgeo-org.atlassian.net/browse/GEOS?atlOrigin=eyJpIjoiZThjOTYwNDJkZjkzNDlmZDg3MTU3ODdiM2EwYjkzOTkiLCJwIjoiaiJ9
 ) / New Feature ( 
https://osgeo-org.atlassian.net/browse/GEOS-11096?atlOrigin=eyJpIjoiZThjOTYwNDJkZjkzNDlmZDg3MTU3ODdiM2EwYjkzOTkiLCJwIjoiaiJ9
 ) GEOS-11096 ( 
https://osgeo-org.atlassian.net/browse/GEOS-11096?atlOrigin=eyJpIjoiZThjOTYwNDJkZjkzNDlmZDg3MTU3ODdiM2EwYjkzOTkiLCJwIjoiaiJ9
 ) csw: support multiple mappings ( 
https://osgeo-org.atlassian.net/browse/GEOS-11096?atlOrigin=eyJpIjoiZThjOTYwNDJkZjkzNDlmZDg3MTU3ODdiM2EwYjkzOTkiLCJwIjoiaiJ9
 )

Issue Type: New Feature Assignee: Niels Charlier ( 
https://osgeo-org.atlassian.net/secure/ViewProfile.jspa?accountId=557058%3A1d2c4019-de4e-4624-8b27-2486941601cf
 ) Components: CSW Created: 07/Aug/23 8:57 AM Priority: Medium Reporter: Niels 
Charlier ( 
https://osgeo-org.atlassian.net/secure/ViewProfile.jspa?accountId=557058%3A1d2c4019-de4e-4624-8b27-2486941601cf
 )

We want to support multiple mappings files for the same type/scheme (such as 
iso metadata)

One layer would then create multiple records, one for each mapping.

( 
https://osgeo-org.atlassian.net/browse/GEOS-11096#add-comment?atlOrigin=eyJpIjoiZThjOTYwNDJkZjkzNDlmZDg3MTU3ODdiM2EwYjkzOTkiLCJwIjoiaiJ9
 ) Add Comment ( 
https://osgeo-org.atlassian.net/browse/GEOS-11096#add-comment?atlOrigin=eyJpIjoiZThjOTYwNDJkZjkzNDlmZDg3MTU3ODdiM2EwYjkzOTkiLCJwIjoiaiJ9
 )

Get Jira notifications on your phone! Download the Jira Cloud app for Android ( 
https://play.google.com/store/apps/details?id=com.atlassian.android.jira.core&referrer=utm_source%3DNotificationLink%26utm_medium%3DEmail
 ) or iOS ( 
https://itunes.apple.com/app/apple-store/id1006972087?pt=696495&ct=EmailNotificationLink&mt=8
 ) This message was sent by Atlassian Jira (v1001.0.0-SNAPSHOT#100233- 
sha1:06f5c37 )___
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Re: [Geoserver-devel] ResourceStore API workparty

2023-08-04 Thread Niels Charlier via Geoserver-devel


Jody,

My apologies if I sound or have sounded agitated in this discussion. I 
do hope we find a solution.


Thanks for the analysis I believe we are now working on the same 
problem: clarifying api design
Yes, I share your goal but we don't seem to find a way to agree? I have 
done a proposal for a possible compromise, but afterwards I found 
another problem (the Paths class). If you read the entire javadoc at the 
top, you must agree something doesn't make sense there.


org.geosever.platform.resource.Files and 
org.geoserver.platform.resource.Paths are not strictly a ResourceStore 
utility class. These classes helped the GeoServer codebase adjust to 
having ResourceStore be such a strict API.


org.geoserver.platform.resource.Files is indeed about handling Files. 
That would have been the right place to put OS & file system related 
stuff. Not Paths, the original javadoc (everything apart from the 
sentence you added) clearly states that this is resourcestore path 
utility class, and that relative paths are not supported (by the 
resource store that is).


Your sentence about operating systems and absolute paths on the file 
system seems entirely out of place there.


Files and Paths utility classes work with absolute paths (because our 
code base does).
ResourceStore does not, and I believe we should take steps to remove 
the test case that assumes it is willing to.


I agree about removing the test case, but I feel you are still missing 
the point.


Relative/absolute have different meaning when you are talking about 
resource store versus file system, because they have a different root. 
As said, the resource store in fact *only* supports absolute paths, 
absolute for its own root that is. Windows paths have nothing to do with 
it. The resource store API should be considered as entirely separate 
from the file system, that is just one implementation.


Perhaps the confusion is over the "path" that is used to retrieve 
resources from the resource store versus the "path" that the file system 
resource store specifically uses to access the files behind the resources.


And take steps to provide a warning if an linux absolute path is used 
(and the ResourceStore will assume that such a path is intended to be 
relative to the data directory). I have found several cases where such 
paths were used (notably in the rest Resource Controller) which have 
been addressed.


But how so? The resource controller is for the resource store, it should 
not have access to the file system? That would be a security issue too.


I think what probably should have happened, is that was made sure the 
file system resource store specifically never uses absolute files, only 
files relative to the data directory (but again, just one implementation).


Warning instead of an exception is perhaps a good idea. But which 
classes / methods do you suggest would provide this warning?


Kind Regards

Niels






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

Re: [Geoserver-devel] ResourceStore API workparty

2023-08-03 Thread Niels Charlier via Geoserver-devel


Jody,

Note that the current javadoc oforg.geoserver.platform.resource.Files is 
highly problematic.


From the original javadoc we get:

"Utility class for handling Resource paths in a consistent fashion.

This utility class is primarily aimed at implementations of 
ResourceStore "


This utility class was indeed intended for resource paths and the 
resource store, not file paths and the file system.


"The base location is represented with "",  * relative paths are not 
supported.



The sentence 'relative paths are not supported' was meant with respect 
to the resource store. The meaning here is that all paths in the 
resource store are absolute, with respect to the _REsource Store root_, 
not the file system root!



And then comes the new sentence:


" Absolute paths are supported, with Linux systems using a leading 
{@code /}, and windows using* {@code L:}."


This sentence contradicts everything before it. If this is a 
resourcestore utility class,


why are we suddenly talking about absolute paths in the operating 
system? The resource store has nothing to do with this.


There never should have been anything inside this class

testing on operating system. There should never have been the method 
called "isAbsolute". This should have been put inside another class that 
is used for file paths.


This will incredibly confuse new developers. They will not at all 
understand the purpose of the


resource store as an abstraction that should not be assumed to be part 
of the file system.


Instead of clarifying things for developers, make sure they understand 
how to use it and not abuse


it, it seems we are giving up on its intended purpose completely and 
just making the confusion


worse by writing nonsensical javadoc about it. Unless this is rolled 
back, we may as well


delete the 'jdbcstore' module and give up entirely on the resource API.

If that is the chose direction, fine, it is not up to me, but then be 
clear about it.


Kind Regards

Niels




On 03/08/2023 08:46, Niels Charlier via Geoserver-devel wrote:


Jody,

Perhaps I can offer a suggestion that may be considered a reasonable 
compromise.


The "resource:" URLs are not used because they were never documented 
in user docs. (The javadoc however clearly states that it always was 
the intention to be used for access to the 'data directory' via URLs.) 
I can see that one of the issues you have been working to resolve is 
the usage of URLs without a prefix, which are just naked paths and 
where there is ambiguity in interpretation.


The resourcestore is low level API only accessible to developers, not 
users. Users will always access it via some layer in between, like 
rest or something that calls Resources.fromURL. When a developer call 
resourcestore.get he knows he is not necessarily using the file 
system, and the paths should not be considered file system paths. With 
Resources.fromURL this is not so clear.


My suggestion is that we bring back / leave the resource store API to 
how it was, being lenient on the leading slash because there is no 
possible confusion there, and it avoids a huge amount of work. We may 
however agree (and document this) that the Resources.fromURL, as well 
as REST or other services accessible to users use the following 
guidelines (and we document this for both developers as well as users):


- file: URLS will use the file system
- resource: URLs will use the resource store
- naked path without URL prefix: absolute paths are interpreted as 
file: URLs, relative paths are interpreted as resource: URLs.


This way the issue is addressed at the level where it actually occurs, 
and OS/filesystem stuff does not creep into the resource store API.


Kind Regards

Niels

On 03/08/2023 01:49, Jody Garnett wrote:

Niels:

Comments inline
--
Jody Garnett


On Aug 2, 2023 at 12:20:43 PM, Niels Charlier  wrote:


Hi Jody,

I do support checking everything is handled correctly.



Thanks Niels, sorry I am a bit frustrated these days so am probably 
not explaining clearly.


I just don't seem the point in breaking huge amounts of code for 
nothing. And that is what will happen if we start throwing 
exceptions in the resource store if the path starts with a "/". 
Which is actually normal, because the resource store has a root.



ResourceStore no longer uses `/` as a root.
The work done earlier this year removed the use leading |/|  from the 
codebase (including the REST API Controller).


Please: define "absolute path". Is this OS dependent? Should the 
behaviour of the resource store be OS dependent? What /exactly/


I put the definitions into the document I shared and the javadocs 
(not sure why I cannot find the javadocs online?).


When I say "path" I mean the strings used by the class 
Paths.isAbsolute(String)


  * 
https://github.com/geoserver/geoserver/blob/main/src/platform/src/ma

Re: [Geoserver-devel] ResourceStore API workparty

2023-08-02 Thread Niels Charlier via Geoserver-devel

Jody,

Perhaps I can offer a suggestion that may be considered a reasonable
compromise.

The "resource:" URLs are not used because they were never documented in
user docs. (The javadoc however clearly states that it always was the
intention to be used for access to the 'data directory' via URLs.) I can
see that one of the issues you have been working to resolve is the usage
of URLs without a prefix, which are just naked paths and where there is
ambiguity in interpretation.

The resourcestore is low level API only accessible to developers, not
users. Users will always access it via some layer in between, like rest
or something that calls Resources.fromURL. When a developer call
resourcestore.get he knows he is not necessarily using the file system,
and the paths should not be considered file system paths. With
Resources.fromURL this is not so clear.

My suggestion is that we bring back / leave the resource store API to
how it was, being lenient on the leading slash because there is no
possible confusion there, and it avoids a huge amount of work. We may
however agree (and document this) that the Resources.fromURL, as well as
REST or other services accessible to users use the following guidelines
(and we document this for both developers as well as users):

- file: URLS will use the file system
- resource: URLs will use the resource store
- naked path without URL prefix: absolute paths are interpreted as file:
URLs, relative paths are interpreted as resource: URLs.

This way the issue is addressed at the level where it actually occurs,
and OS/filesystem stuff does not creep into the resource store API.

Kind Regards

Niels

On 03/08/2023 01:49, Jody Garnett wrote:

Niels:

Comments inline
--
Jody Garnett

On Aug 2, 2023 at 12:20:43 PM, Niels Charlier wrote:

Hi Jody,

I do support checking everything is handled correctly.

Thanks Niels, sorry I am a bit frustrated these days so am probably
not explaining clearly.

I just don't seem the point in breaking huge amounts of code for
nothing. And that is what will happen if we start throwing exceptions
in the resource store if the path starts with a "/". Which is
actually normal, because the resource store has a root.

ResourceStore no longer uses `/` as a root.
The work done earlier this year removed the use leading |/| from the
codebase (including the REST API Controller).

Please: define "absolute path". Is this OS dependent? Should the
behaviour of the resource store be OS dependent? What /exactly/

I put the definitions into the document I shared and the javadocs (not
sure why I cannot find the javadocs online?).

When I say "path" I mean the strings used by the class
Paths.isAbsolute(String)

*
https://github.com/geoserver/geoserver/blob/main/src/platform/src/main/java/org/geoserver/platform/resource/Paths.java#L550
* This has nothing to do with the ResourceStore API.

This is covered in the developers guide here:
https://docs.geoserver.org/latest/en/developer/programming-guide/config/resource.html#paths

Paths are broken down into a sequence of names, as listed by
|Paths.names(path)|:

On linux |Path.names("/src/gis/cadaster/district.geopkg")| starts
with a marker to indicate an absolute path, resulting in |/|,
|src|, |gis|, |cadaster|, |district.geopkg|.

On windows |Path.names("D:/gis/cadaster/district.geopkg")| starts
with a marker to indicate an absolute path, resulting in |D:/|,
|gis|, |cadaster|, |district.geopkg|.

are you suggesting should be allowed or not allowed to access a
resource from the resource store?

I am suggesting paths that return true for this method be not allowed
to access resources from the resource store.

Yes, "paths" are used across the application. But "path" is a vague
term, 'xpaths' are paths too, they have no relationship with file
paths. Resource paths are no different, they live in their own world
and are not part of the file system. The notion of a path being
'absolute' or 'relative' has no meaning when you are talking about
paths in general, this is file system terminology.

Earlier in this email thread I shared the developer guide
documentation which was written to define what Paths mean for GeoServer.

When someone gets a resource directly from the resource store, they
are using the resource store as a black box, and the "path" is always
relative to the _root_ of the resource store.

Correct, relative paths are with respect to the resource store
location. Actually when I did he QA I found that there was at one
point a search path that could be scanned to allow "data directory" to
be spread across several folders (terrifying).

Resource.fromURL

Re: [Geoserver-devel] ResourceStore API workparty

2023-08-02 Thread Niels Charlier via Geoserver-devel

Hello Jody,

You write "I am suggesting paths that return true for this method be not
allowed to access resources from the resource store. "

I think this is problematic: the method is OS dependent: a path with
leading slash will return "false" on windows and "true" on linux. So
your suggestion is that resource paths can start with a leading slash on
windows, but not on linux. I don't think the syntax of the resource
store should be OS dependent. Because the resource store is not part of
the file system or the OS, it is an internal system of geoserver.

I think the recent changes are problematic: you added a test method to
the resource theory test that can only work for a file system based
resource store, and not for alternative implementations. It makes the
existence of the resource store abstract API obsolete. There is a lot of
confusion going on and creeping into the code base.

My position is that this should be pulled back. Removing all leading
slashes from resource stores paths will require a huge amount of
unnecessary work, and I still do not see a reason why this is necessary.

Regards