Skip to content

Conversation

@russcam
Copy link
Contributor

@russcam russcam commented Apr 23, 2018

Fixes #3100
Fixes #3096

@russcam
Copy link
Contributor Author

russcam commented Apr 24, 2018

I think the geo shape support should be revisited in 7.x. For example,

  • GeometryCollection should implement IGeoShape as it is a valid input for a geo_shape field datatype.
  • All geoshape queries could be condensed into one query

Copy link
Contributor

@codebrain codebrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - a minor observation - are we going to allow geoshapes on documents?

typeof(Attachment),
typeof(ILazyDocument)
typeof(ILazyDocument),
typeof(GeoCoordinate)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IGeoShape as well as IGeometryCollection ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled on lines 56 and 57 below. It's possible that a user could specify any type that implements IGeoShape e.g. IPolygonGeoShape, so json conversion should check if IGeoShape is assignable from the type.

}

// adapted from https://gis.stackexchange.com/a/103465/30046
private static IEnumerable<GeoCoordinate> GeneratePolygonCoordinates(Faker p, GeoCoordinate centroid, double maxDistance = 0.0002)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

russcam and others added 3 commits April 26, 2018 11:44
This commit changes GeometryCollection to implement IGeoShape, to allow a document to have an
IGeoShape member and accept a GeometryCollection. IGeoShape.Type is implemented explicitly so
that the existing Type member continues to be the implicit implementation of IGeometryCollection.

The ideal scenario would have been for IGeometryCollection to implement IGeoShape however this
would break backwards binary compatibility, since both interfaces have a Type member.
@russcam russcam merged commit 96c0d4d into master Apr 26, 2018
russcam added a commit that referenced this pull request Apr 26, 2018
* Support serialization of geo shapes on documents

Fixes #3100
Fixes #3096

* GeometryCollection implements IGeoShape

This commit changes GeometryCollection to implement IGeoShape, to allow a document to have an
IGeoShape member and accept a GeometryCollection. IGeoShape.Type is implemented explicitly so
that the existing Type member continues to be the implicit implementation of IGeometryCollection.

The ideal scenario would have been for IGeometryCollection to implement IGeoShape however this
would break backwards binary compatibility, since both interfaces have a Type member.

(cherry picked from commit 96c0d4d)
russcam added a commit that referenced this pull request Apr 26, 2018
* Support serialization of geo shapes on documents

Fixes #3100
Fixes #3096

* GeometryCollection implements IGeoShape

This commit changes GeometryCollection to implement IGeoShape, to allow a document to have an
IGeoShape member and accept a GeometryCollection. IGeoShape.Type is implemented explicitly so
that the existing Type member continues to be the implicit implementation of IGeometryCollection.

The ideal scenario would have been for IGeometryCollection to implement IGeoShape however this
would break backwards binary compatibility, since both interfaces have a Type member.

(cherry picked from commit 96c0d4d)

This commit includes changes from PR #3214, which were required to address incorrect
serialization introduced in 6.x
@russcam russcam deleted the bug/geoshape branch April 28, 2018 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants