Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GeoConverters does not convert inner rings of GeoJsonPolygon #4104

Closed
lennertdefeyter opened this issue Jul 8, 2022 · 6 comments
Closed
Labels
type: bug A general bug

Comments

@lennertdefeyter
Copy link

lennertdefeyter commented Jul 8, 2022

Hi,

I noticed that the GeojsonPolygonconvertor does not convert the inner rings. If you want to convert a polygon with holes. It only looks at the outer ring.

This is very apparent in the code in line 759 of GeoConverters.java.
static GeoJsonPolygon toGeoJsonPolygon(List dbList) { return new GeoJsonPolygon(toListOfPoint((List) dbList.get(0))); }

As you can see, it only looks at the first item in the list of rings (e.g. the outer ring).

I've already fixed it with a custom convertor (see attachment, although this is for multipolygons), but it might be nice to have this in the core.

I think that in order to fix it, the toGeoJsonPolygon(List dbList) function in GeoConverters.java needs to change to:

static GeoJsonPolygon toGeoJsonPolygon(List dbList) { Iterator<GeoJsonLineString> it = ((List) dbList).iterator(); GeoJsonPolygon g = new GeoJsonPolygon(toListOfPoint((List) it.next())); while (it.hasNext()) g = g.withInnerRing(toListOfPoint((List) it.next())); return g; }

GeoJsonMultiPolygonDeserializer.java.txt
.

@lennertdefeyter lennertdefeyter changed the title GeoJsonconvertor does not convert inner rings. GeoJsonPolygonconvertor does not convert inner rings. Jul 8, 2022
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 8, 2022
@christophstrobl
Copy link
Member

Which version are you using? It would be great if you could please take the time to provide a complete minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem.

@christophstrobl christophstrobl added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 21, 2022
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Jul 28, 2022
@lennertdefeyter
Copy link
Author

Hi @christophstrobl ,

Sorry for the delay. This is version 2.7.2.

Attached is a zip that should show the problem.

it inserts a polygon correctly with a donut hole in the center: outer ring is a box with bounds: 0,0 -> 10,10 . Inner ring is a box 4,4 -> 6,6

It is stored correctly in the database. But when querying it clearly has only the outer ring:
image

mongodbdemo.zip

Thank you for your help.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Jul 28, 2022
@christophstrobl
Copy link
Member

Sorry for long silence - good catch! Thank you for the reproducer!

@christophstrobl christophstrobl added type: bug A general bug and removed status: feedback-provided Feedback has been provided labels Sep 8, 2022
mp911de added a commit that referenced this issue Sep 14, 2022
Fix generics. Add warning suppressions for nullability checks.

See: #4104
Original pull request: #4156.
mp911de pushed a commit that referenced this issue Sep 14, 2022
mp911de added a commit that referenced this issue Sep 14, 2022
Fix generics. Add warning suppressions for nullability checks.

See: #4104
Original pull request: #4156.
mp911de pushed a commit that referenced this issue Sep 14, 2022
mp911de added a commit that referenced this issue Sep 14, 2022
Fix generics. Add warning suppressions for nullability checks.

See: #4104
Original pull request: #4156.
@mp911de mp911de changed the title GeoJsonPolygonconvertor does not convert inner rings. GeoConverters does not convert inner rings Sep 14, 2022
@mp911de mp911de changed the title GeoConverters does not convert inner rings GeoConverters does not convert inner rings of GeoJsonPolygon Sep 14, 2022
@mp911de mp911de added this to the 3.3.7 (2021.1.7) milestone Sep 14, 2022
@lennertdefeyter
Copy link
Author

@mp911de
Hi,

Thank you in advance for the help! Unfortunately this will only fix the issue if it has only one donut hole. A lot of polygons have multiple, for example, if you have a layer for all countries in the world, Italy would have holes for San Marino and Vatican city.

For an example on a full fix, you can reference the deserializer I added in the original post. Otherwise I'll try to make my own PR this weekend.

Kind regards and again thank you for your time,

Lennert

@mp911de
Copy link
Member

mp911de commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants