Skip to content

GeoJson: Improper Deserialization of Document with a GeoJsonPolygon [DATAMONGO-2664] #3517

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

Closed
spring-projects-issues opened this issue Dec 1, 2020 · 14 comments
Assignees
Labels
in: core Issues in core support type: bug A general bug

Comments

@spring-projects-issues
Copy link

Bjorn Harvold opened DATAMONGO-2664 and commented

I'm trying to attach screenshots but getting "Try again. Invalid token". Will try to attach after I create the ticket.

The existing test that accompanies Spring Data MongoDb is in the class called GeoJsonModuleUnitTests:

 

@Test // DATAMONGO-1181	public void shouldDeserializeGeoJsonPolygonCorrectly() throws JsonParseException, JsonMappingException, IOException {
		String json = "{ \"type\": \"Polygon\", \"coordinates\": [ [ [100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0] ] ]}";
		assertThat(mapper.readValue(json, GeoJsonPolygon.class)).isEqualTo(new GeoJsonPolygon(				Arrays.asList(new Point(100, 0), new Point(101, 0), new Point(101, 1), new Point(100, 1), new Point(100, 0))));	}

 

 

And that passes just fine.

However, change those points to GeoJsonPoint objects and it fails:

 

@Test
public void shouldSerializeGeoJsonPolygonCorrectly() throws IOException {
    String json = "{ \"type\": \"Polygon\", \"coordinates\": [ [ [100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0] ] ]}";

    List<Point> points = Arrays.asList(new GeoJsonPoint(100, 0), new GeoJsonPoint(101, 0), new GeoJsonPoint(101, 1), new GeoJsonPoint(100, 1), new GeoJsonPoint(100, 0));
    GeoJsonPolygon polygon = new GeoJsonPolygon(points);
    
    assertThat(mapper.readValue(json, GeoJsonPolygon.class)).isEqualTo(polygon);
    assertThat(mapper.writeValueAsString(polygon)).isEqualTo(json);
}

 

 

And that is exactly what happens when I query MongoDb (simple findById()) for a document with Spring Data MongoDb. When I look in the object graph, I see GeoJsonPoint objects as coordinates in my Polygon.

When the Jackson ObjectMapper serializes that data, it looks like this:

 

"polygon" : {
        "points" : [ {
          "x" : 105.44848312743373,
          "y" : 13.049959719225617,
          "type" : "Point",
          "coordinates" : [ 105.44848312743373, 13.049959719225617 ]
        }, {
          "x" : 105.11889328368373,
          "y" : 11.891452140925736,
          "type" : "Point",
          "coordinates" : [ 105.11889328368373, 11.891452140925736 ]
        }, {
          "x" : 105.69018234618373,
          "y" : 11.50415959858532,
          "type" : "Point",
          "coordinates" : [ 105.69018234618373, 11.50415959858532 ]
        }, {
          "x" : 105.88793625243373,
          "y" : 11.998936428709188,
          "type" : "Point",
          "coordinates" : [ 105.88793625243373, 11.998936428709188 ]
        }, {
          "x" : 106.81078781493373,
          "y" : 11.998936428709188,
          "type" : "Point",
          "coordinates" : [ 106.81078781493373, 11.998936428709188 ]
        }, {
          "x" : 107.29418625243373,
          "y" : 12.600046596262514,
          "type" : "Point",
          "coordinates" : [ 107.29418625243373, 12.600046596262514 ]
        }, {
          "x" : 107.22826828368373,
          "y" : 13.584502655549848,
          "type" : "Point",
          "coordinates" : [ 107.22826828368373, 13.584502655549848 ]
        }, {
          "x" : 106.48119797118373,
          "y" : 12.964324199217307,
          "type" : "Point",
          "coordinates" : [ 106.48119797118373, 12.964324199217307 ]
        }, {
          "x" : 106.41528000243373,
          "y" : 14.543620849640376,
          "type" : "Point",
          "coordinates" : [ 106.41528000243373, 14.543620849640376 ]
        } ],
        "coordinates" : [ {
          "type" : "LineString",
          "coordinates" : [ {
            "x" : 105.44848312743373,
            "y" : 13.049959719225617,
            "type" : "Point",
            "coordinates" : [ 105.44848312743373, 13.049959719225617 ]
          }, {
            "x" : 105.11889328368373,
            "y" : 11.891452140925736,
            "type" : "Point",
            "coordinates" : [ 105.11889328368373, 11.891452140925736 ]
          }, {
            "x" : 105.69018234618373,
            "y" : 11.50415959858532,
            "type" : "Point",
            "coordinates" : [ 105.69018234618373, 11.50415959858532 ]
          }, {
            "x" : 105.88793625243373,
            "y" : 11.998936428709188,
            "type" : "Point",
            "coordinates" : [ 105.88793625243373, 11.998936428709188 ]
          }, {
            "x" : 106.81078781493373,
            "y" : 11.998936428709188,
            "type" : "Point",
            "coordinates" : [ 106.81078781493373, 11.998936428709188 ]
          }, {
            "x" : 107.29418625243373,
            "y" : 12.600046596262514,
            "type" : "Point",
            "coordinates" : [ 107.29418625243373, 12.600046596262514 ]
          }, {
            "x" : 107.22826828368373,
            "y" : 13.584502655549848,
            "type" : "Point",
            "coordinates" : [ 107.22826828368373, 13.584502655549848 ]
          }, {
            "x" : 106.48119797118373,
            "y" : 12.964324199217307,
            "type" : "Point",
            "coordinates" : [ 106.48119797118373, 12.964324199217307 ]
          }, {
            "x" : 106.41528000243373,
            "y" : 14.543620849640376,
            "type" : "Point",
            "coordinates" : [ 106.41528000243373, 14.543620849640376 ]
          } ]
        } ],
        "type" : "Polygon"
      }

 

Please advise,

Bjorn

 

 


Affects: 3.1.1 (2020.0.1)

Attachments:

@spring-projects-issues
Copy link
Author

Bjorn Harvold commented

I wanted to follow up on this with another unit test. If I modify the existing GeoJsonPolygon test, in GeoJsonModuleUnitTests, slightly and try to serialize it back to string JSON, it also fails.

@Test
public void shouldSerializeGeoJsonPolygonCorrectly() throws IOException {
    String json = "{ \"type\": \"Polygon\", \"coordinates\": [ [ [100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0] ] ]}";

    List<Point> points = Arrays.asList(new Point(100, 0), new Point(101, 0), new Point(101, 1), new Point(100, 1), new Point(100, 0));
    GeoJsonPolygon polygon = new GeoJsonPolygon(points);

    assertThat(mapper.readValue(json, GeoJsonPolygon.class)).isEqualTo(polygon);
    assertThat(mapper.writeValueAsString(polygon)).isEqualTo(json);
}

Results:

Expected :"{ "type": "Polygon", "coordinates": [ [ [100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0] ] ]}"
Actual   :"{"points":[{"x":100.0,"y":0.0},{"x":101.0,"y":0.0},{"x":101.0,"y":1.0},{"x":100.0,"y":1.0},{"x":100.0,"y":0.0}],"coordinates":[{"type":"LineString","coordinates":[{"x":100.0,"y":0.0},{"x":101.0,"y":0.0},{"x":101.0,"y":1.0},{"x":100.0,"y":1.0},{"x":100.0,"y":0.0}]}],"type":"Polygon"}"

 

@spring-projects-issues
Copy link
Author

Bjorn Harvold commented

The issue with this is I need to keep 2 separate data models for Spring Web MVC GET and POST. In other words, it won't let me submit the same payload I received from the server back to the server. The server will require proper GeoJson

@spring-projects-issues
Copy link
Author

Bjorn Harvold commented

More than happy to contribute time to fix this if it will help and if it speeds up the release / patch process

@spring-projects-issues
Copy link
Author

Christoph Strobl commented

Thanks for bringing this up!

The GeoJsonModule only contains a defined set of JsonDeserializer. Since there are no JsonSerializer in place en-/decoding is not symmetric.

You might want to add those to Jackson for now to work around that issue. In case you've got time to open a PR including the required `JsonSerializer`s we'd be happy to review it

@spring-projects-issues spring-projects-issues added status: waiting-for-feedback We need additional information before we can continue type: bug A general bug in: core Issues in core support labels Dec 30, 2020
@bjornharvold
Copy link
Contributor

@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 labels Jan 6, 2021
@bjornharvold
Copy link
Contributor

bjornharvold commented Jan 6, 2021

Suggestion:

public class ImprovedGeoJsonModule extends GeoJsonModule {
    public ImprovedGeoJsonModule() {
        super();
        addSerializer(GeoJsonPoint.class, new GeoJsonPointSerializer());
        addSerializer(GeoJsonMultiPoint.class, new GeoJsonMultiPointSerializer());
        addSerializer(GeoJsonLineString.class, new GeoJsonLineStringSerializer());
        addSerializer(GeoJsonMultiLineString.class, new GeoJsonMultiLineStringSerializer());
        addSerializer(GeoJsonPolygon.class, new GeoJsonPolygonSerializer());
        addSerializer(GeoJsonMultiPolygon.class, new GeoJsonMultiPolygonSerializer());
    }

    public static class GeoJsonPointSerializer extends JsonSerializer<GeoJsonPoint> {
        @Override
        public void serialize(GeoJsonPoint value, JsonGenerator gen, SerializerProvider serializers) throws IOException, JsonProcessingException {
            gen.writeStartObject();
            gen.writeStringField("type", value.getType());
            gen.writeObjectField("coordinates", value.getCoordinates());
            gen.writeEndObject();
        }

    }

    public static class GeoJsonLineStringSerializer extends JsonSerializer<GeoJsonLineString> {

        @Override
        public void serialize(GeoJsonLineString value, JsonGenerator gen, SerializerProvider serializers) throws IOException, JsonProcessingException {
            gen.writeStartObject();
            gen.writeStringField("type", value.getType());
            gen.writeArrayFieldStart("coordinates");
            for (Point p : value.getCoordinates()) {
                gen.writeObject(new double[]{p.getX(), p.getY()});
            }
            gen.writeEndArray();
            gen.writeEndObject();
        }
    }

    public static class GeoJsonMultiPointSerializer extends JsonSerializer<GeoJsonMultiPoint> {

        @Override
        public void serialize(GeoJsonMultiPoint value, JsonGenerator gen, SerializerProvider serializers) throws IOException, JsonProcessingException {
            gen.writeStartObject();
            gen.writeStringField("type", value.getType());
            gen.writeArrayFieldStart("coordinates");
            for (Point p : value.getCoordinates()) {
                gen.writeObject(new double[]{p.getX(), p.getY()});
            }
            gen.writeEndArray();
            gen.writeEndObject();
        }
    }

    public static class GeoJsonMultiLineStringSerializer extends JsonSerializer<GeoJsonMultiLineString> {

        @Override
        public void serialize(GeoJsonMultiLineString value, JsonGenerator gen, SerializerProvider serializers) throws IOException, JsonProcessingException {
            gen.writeStartObject();
            gen.writeStringField("type", value.getType());
            gen.writeArrayFieldStart("coordinates");
            for (GeoJsonLineString lineString : value.getCoordinates()) {
                List<double[]> arrayList = new ArrayList<>();
                for (Point p : lineString.getCoordinates()) {
                    arrayList.add(new double[]{p.getX(), p.getY()});
                }
                double[][] doubles = arrayList.toArray(new double[0][0]);
                gen.writeObject(doubles);
            }
            gen.writeEndArray();
            gen.writeEndObject();
        }
    }

    public static class GeoJsonPolygonSerializer extends JsonSerializer<GeoJsonPolygon> {

        @Override
        public void serialize(GeoJsonPolygon value, JsonGenerator gen, SerializerProvider serializers) throws IOException, JsonProcessingException {
            gen.writeStartObject();
            gen.writeStringField("type", value.getType());
            gen.writeArrayFieldStart("coordinates");
            for (GeoJsonLineString ls : value.getCoordinates()) {
                gen.writeStartArray();
                for (Point p : ls.getCoordinates()) {
                    gen.writeObject(new double[]{p.getX(), p.getY()});
                }
                gen.writeEndArray();
            }
            gen.writeEndArray();
            gen.writeEndObject();
        }
    }

    public static class GeoJsonMultiPolygonSerializer extends JsonSerializer<GeoJsonMultiPolygon> {

        @Override
        public void serialize(GeoJsonMultiPolygon value, JsonGenerator gen, SerializerProvider serializers) throws IOException, JsonProcessingException {
            gen.writeStartObject();
            gen.writeStringField("type", value.getType());
            gen.writeArrayFieldStart("coordinates");
            for (GeoJsonPolygon polygon : value.getCoordinates()) {

                gen.writeStartArray();

                gen.writeStartArray();
                for (GeoJsonLineString lineString : polygon.getCoordinates()) {

                    for (Point p : lineString.getCoordinates()) {
                        gen.writeObject(new double[]{p.getX(), p.getY()});
                    }

                }

                gen.writeEndArray();
                gen.writeEndArray();
            }
            gen.writeEndArray();
            gen.writeEndObject();
        }
    }
}

With accompanying tests:

public class ObjectMapperTest {
    ObjectMapper mapper;

    @BeforeEach
    public void setUp() {

        mapper = new ObjectMapper();
        mapper.registerModule(new ImprovedGeoJsonModule());
    }

    @Test
    public void shouldDeserializeJsonPointCorrectly() throws IOException {

        String json = StringUtils.deleteWhitespace("{ \"type\": \"Point\", \"coordinates\": [10.0, 20.0] }");
        GeoJsonPoint point = new GeoJsonPoint(10D, 20D);

        assertThat(mapper.readValue(json, GeoJsonPoint.class)).isEqualTo(point);

        String backToJSON =  StringUtils.deleteWhitespace(mapper.writeValueAsString(point));

        assertThat(backToJSON).isEqualTo(json);
    }

    @Test
    public void shouldDeserializeGeoJsonLineStringCorrectly()
            throws IOException {

        String json = StringUtils.deleteWhitespace("{ \"type\": \"LineString\", \"coordinates\": [ [10.0, 20.0], [30.0, 40.0], [50.0, 60.0] ]}");

        GeoJsonLineString lineString = new GeoJsonLineString(Arrays.asList(new Point(10, 20), new Point(30, 40), new Point(50, 60)));

        assertThat(mapper.readValue(json, GeoJsonLineString.class)).isEqualTo(lineString);

        String backToJSON =  StringUtils.deleteWhitespace(mapper.writeValueAsString(lineString));

        assertThat(backToJSON).isEqualTo(json);
    }

    @Test
    public void shouldDeserializeGeoJsonMultiPointCorrectly() throws IOException {

        String json = StringUtils.deleteWhitespace("{ \"type\": \"MultiPoint\", \"coordinates\": [ [10.0, 20.0], [30.0, 40.0], [50.0, 60.0] ]}");
        GeoJsonMultiPoint multiPoint = new GeoJsonMultiPoint(Arrays.asList(new Point(10, 20), new Point(30, 40), new Point(50, 60)));

        assertThat(mapper.readValue(json, GeoJsonMultiPointString.class)).isEqualTo(multiPoint);

        String backToJSON =  StringUtils.deleteWhitespace(mapper.writeValueAsString(multiPoint));

        assertThat(backToJSON).isEqualTo(json);
    }

    @Test
    @SuppressWarnings("unchecked")
    public void shouldDeserializeGeoJsonMultiLineStringCorrectly() throws IOException {

        String json = StringUtils.deleteWhitespace("{ \"type\": \"MultiLineString\", \"coordinates\": [ [ [10.0, 20.0], [30.0, 40.0] ], [ [50.0, 60.0] , [70.0, 80.0] ] ]}");
        GeoJsonMultiLineString multiLineString = new GeoJsonMultiLineString(Arrays.asList(new Point(10, 20), new Point(30, 40)), Arrays.asList(new Point(50, 60), new Point(70, 80)));

        assertThat(mapper.readValue(json, GeoJsonMultiLineString.class)).isEqualTo(multiLineString);

        String backToJSON =  StringUtils.deleteWhitespace(mapper.writeValueAsString(multiLineString));

        assertThat(backToJSON).isEqualTo(json);
    }

    @Test
    public void shouldDeserializeGeoJsonPolygonCorrectly() throws IOException {
        String json = StringUtils.deleteWhitespace("{ \"type\": \"Polygon\", \"coordinates\": [ [ [100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0] ] ]}");

        List<Point> points = Arrays.asList(new Point(100, 0), new Point(101, 0), new Point(101, 1), new Point(100, 1), new Point(100, 0));
        GeoJsonPolygon polygon = new GeoJsonPolygon(points);

        assertThat(mapper.readValue(json, GeoJsonPolygon.class)).isEqualTo(polygon);

        String backToJSON =  StringUtils.deleteWhitespace(mapper.writeValueAsString(polygon));

        assertThat(backToJSON).isEqualTo(json);
    }

    @Test
    public void shouldDeserializeGeoJsonMultiPolygonCorrectly()
            throws IOException {

        String json = StringUtils.deleteWhitespace("{ \"type\": \"MultiPolygon\", \"coordinates\": ["
                + "[ "
                + "[ "
                + "[102.0, 2.0], [103.0, 2.0], [103.0, 3.0], [102.0, 3.0], [102.0, 2.0]"
                + "]"
                + "],"
                + "["
                + "["
                + "[100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0] "
                + "]"
                + "],"
                + "[ "
                + "[ "
                + "[100.2, 0.2], [100.8, 0.2], [100.8, 0.8], [100.2, 0.8], [100.2, 0.2] "
                + "]"
                + "]"
                + "]"
                + "}");

        GeoJsonMultiPolygon multiPolygon = new GeoJsonMultiPolygon(Arrays.asList(
                new GeoJsonPolygon(Arrays.asList(new Point(102, 2), new Point(103, 2), new Point(103, 3), new Point(102, 3),
                        new Point(102, 2))),
                new GeoJsonPolygon(Arrays.asList(new Point(100, 0), new Point(101, 0), new Point(101, 1), new Point(100, 1),
                        new Point(100, 0))),
                new GeoJsonPolygon(Arrays.asList(new Point(100.2, 0.2), new Point(100.8, 0.2), new Point(100.8, 0.8),
                        new Point(100.2, 0.8), new Point(100.2, 0.2)))));

        assertThat(mapper.readValue(json, GeoJsonMultiPolygon.class)).isEqualTo(multiPolygon);

        String backToJSON =  StringUtils.deleteWhitespace(mapper.writeValueAsString(multiPolygon));

        assertThat(backToJSON).isEqualTo(json);

    }
}

Notice that I changed the json for MultiPolygon as your original test had set the type to "Polygon" and the 3 polygons had some weirdly nested 2nd and 3rd polygon. I don't know if that was intentional but I found it impossible to serialize with the data I was given from GeoJsonMultiPolygon.

@christophstrobl
Copy link
Member

Thanks @bjornharvold care to create a PR with your changes?

@bjornharvold
Copy link
Contributor

Hi @christophstrobl

I'll leave it here for your team to figure out how you want to integrate it. Some projects might not want to use it automatically when the next upgrade from spring data mongodb occurs. So renaming the current module to GeoJsonDeserializersModule() and create a new one based on the code above in a GeoJsonSerializersModule() would probably make sense to most.

Anyway, I leave it to you. We are currently using the code above in production without any issues for a two weeks now.

Cheers,
Bjorn

@christophstrobl
Copy link
Member

We'd take care of integrating the PR addressing the issues you pointed out. A PR would make sure your contribution is linked to your GH profile and that it follows the CLA.

@bjornharvold
Copy link
Contributor

Sure thing @christophstrobl. I'll create a PR for you guys. I've already signed your CLA.

@bjornharvold
Copy link
Contributor

#3539

@christophstrobl christophstrobl linked a pull request Jan 20, 2021 that will close this issue
4 tasks
@mp911de mp911de added this to the 3.2 M3 (2021.0.0) milestone Feb 2, 2021
@mp911de mp911de closed this as completed in cba9088 Feb 2, 2021
mp911de pushed a commit that referenced this issue Feb 2, 2021
This commit reduces the visibility of the GeoJsonSerializersModule and instead offers static methods on GeoJsonModule allowing to obtain those via a static method.
The actual serialization was simpified by moving some of its code to a common base class.
Updated reference documentation - relates to: spring-projects/spring-data-commons#2288

We also did, by intent, not change the current GeoJsonModule to add the serializers by default in order to give users time to prepare for that change which will be done with the next major release.

Original pull request: #3539.
Closes #3517
mp911de added a commit that referenced this issue Feb 2, 2021
Move registerSerializersIn(…) entirely to GeoJsonSerializersModule. Tweak Javadoc and wording.

Original pull request: #3539.
Closes #3517
mp911de pushed a commit that referenced this issue Feb 2, 2021
mp911de pushed a commit that referenced this issue Feb 2, 2021
@mp911de
Copy link
Member

mp911de commented Feb 2, 2021

We've backported the documentation about registering only GeoJson serializers to 3.1.x and 3.0.x.

@jwill490
Copy link

Hi @mp911de @christophstrobl @bjornharvold , I'm using the latest version of Spring Data MongoDB Reactive (2.7.5), and I'm getting this exact same serialization issue (where it's returning an array of points with x, y coordinates, instead of the proper coordinates array):

{ "x": -77.11191, "y": 16.32832, "type": "Point", "coordinates": [ -77.11191, 16.32832 ] }, { "x": -77.12373, "y": 16.3343, "type": "Point", "coordinates": [ -77.12373, 16.3343 ] },

Could you possibly advise me on how to implement this serializer?

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

Successfully merging a pull request may close this issue.

5 participants