Support GeoJSON and 2dsphere queries #109

Merged
merged 16 commits into from Jul 11, 2013

Conversation

Projects
None yet
3 participants
@jmikola
Member

jmikola commented May 15, 2013

This PR:

Todo items:

  • Ensure $maxDistance works with $near 2dsphere queries
  • Support $centerSphere with $geoWithin
  • Tests for all geospatial operators
@EmiiKhaos

This comment has been minimized.

Show comment
Hide comment
@EmiiKhaos

EmiiKhaos Jun 30, 2013

Contributor

1) $geoWithin and $near

New in version 2.4: $geoWithin replaces $within which is deprecated.

For the most shapes it's a simple copy of the old within* methods and prefix them with geo and change the operator. Except polygons. How to differ if the given coordinates are legacy coordinate pairs or a geojson polygon? With a parameter $legacy? Or with spherical(true)? Or depending on the array depth? Legacy coordinates allowing only one array of coordinate tuples and no inner and outer rings as with a geojson polygon possible.

The same with the $near command.

2) $centerSphere with $geoWithin

The $centerSphere expects the radius in radians. Should the user take care of the calculation or the query builder? For the second I think of a method signature like

public function geoWithinCenterSphere($x, $x, $radius, $multiplier = SomeUtilClass::KILOMETERS)

Simply it does the same as the user can, but if the user forgets about the radius should be in radians, the $radius parameter is assumed to be in kilometers. The constant in the util class (which would fit?) are simple the right multipliers (6378.137 for km and 3963.192 miles), which allows the user to specify a custom multiplier.

Contributor

EmiiKhaos commented Jun 30, 2013

1) $geoWithin and $near

New in version 2.4: $geoWithin replaces $within which is deprecated.

For the most shapes it's a simple copy of the old within* methods and prefix them with geo and change the operator. Except polygons. How to differ if the given coordinates are legacy coordinate pairs or a geojson polygon? With a parameter $legacy? Or with spherical(true)? Or depending on the array depth? Legacy coordinates allowing only one array of coordinate tuples and no inner and outer rings as with a geojson polygon possible.

The same with the $near command.

2) $centerSphere with $geoWithin

The $centerSphere expects the radius in radians. Should the user take care of the calculation or the query builder? For the second I think of a method signature like

public function geoWithinCenterSphere($x, $x, $radius, $multiplier = SomeUtilClass::KILOMETERS)

Simply it does the same as the user can, but if the user forgets about the radius should be in radians, the $radius parameter is assumed to be in kilometers. The constant in the util class (which would fit?) are simple the right multipliers (6378.137 for km and 3963.192 miles), which allows the user to specify a custom multiplier.

@jmikola

This comment has been minimized.

Show comment
Hide comment
@jmikola

jmikola Jul 1, 2013

Member

How to differ if the given coordinates are legacy coordinate pairs or a geojson polygon?

I created a library two weeks ago, geojson, that I'm hoping will address this without the need to create duplicate methods. If the first argument is a Geometry object, we'd immediately know to use the GeoJSON operators under the hood. Additionally, the library does all the validation of shapes upon construction. I blogged about this last week here.

Should the user take care of the calculation or the query builder?

I think this is best left to the user. If the sphere happens to not be earth, the multiplier will differ anyway. Also, this would let Doctrine avoid having to default to accepting miles or kilometers. If the user did want to pass radians, and we defaulted to one of the other units, they'd have to specify a multiplier of 1.

Member

jmikola commented Jul 1, 2013

How to differ if the given coordinates are legacy coordinate pairs or a geojson polygon?

I created a library two weeks ago, geojson, that I'm hoping will address this without the need to create duplicate methods. If the first argument is a Geometry object, we'd immediately know to use the GeoJSON operators under the hood. Additionally, the library does all the validation of shapes upon construction. I blogged about this last week here.

Should the user take care of the calculation or the query builder?

I think this is best left to the user. If the sphere happens to not be earth, the multiplier will differ anyway. Also, this would let Doctrine avoid having to default to accepting miles or kilometers. If the user did want to pass radians, and we defaulted to one of the other units, they'd have to specify a multiplier of 1.

@jmikola

This comment has been minimized.

Show comment
Hide comment
@jmikola

jmikola Jul 2, 2013

Member

@patkar: I resolved the outstanding todo items and pushed the changes I discussed in my last comment. Let me know what you think.

I added deprecation doc tags to the older within*() methods, as $within is deprecated in MongoDB now. Those methods now refer to corresponding geoWithin*() methods, which use the $geoWithin operator. I also removed the new GeoJSON helper methods you created (e.g. intersecting lines, points, polygons). GeoJSON only shows up with the new geoIntersects() and geoWithin() methods, and optionally as the first argument (a point) in near() and nearSphere().

I want to be careful to leave the GeoJSON library as an optional dependency, so perhaps I can go a step further and remove the type-hints in geoIntersects() and geoWithin() and allow the user to send in an array, which will be used as-is. The array will be expected to be a GeoJSON geometry, such as:

[
  'type' => 'Point',
  'coordinates' => [1, 2],
]

If a GeoJSON object is passed in, I can call jsonSerialize() as is currently done in the Expr class. Likewise, we could do this for near() and nearSphere() as well. If their $x argument is an array, I'll expect it to be a GeoJSON point (just like above).

Member

jmikola commented Jul 2, 2013

@patkar: I resolved the outstanding todo items and pushed the changes I discussed in my last comment. Let me know what you think.

I added deprecation doc tags to the older within*() methods, as $within is deprecated in MongoDB now. Those methods now refer to corresponding geoWithin*() methods, which use the $geoWithin operator. I also removed the new GeoJSON helper methods you created (e.g. intersecting lines, points, polygons). GeoJSON only shows up with the new geoIntersects() and geoWithin() methods, and optionally as the first argument (a point) in near() and nearSphere().

I want to be careful to leave the GeoJSON library as an optional dependency, so perhaps I can go a step further and remove the type-hints in geoIntersects() and geoWithin() and allow the user to send in an array, which will be used as-is. The array will be expected to be a GeoJSON geometry, such as:

[
  'type' => 'Point',
  'coordinates' => [1, 2],
]

If a GeoJSON object is passed in, I can call jsonSerialize() as is currently done in the Expr class. Likewise, we could do this for near() and nearSphere() as well. If their $x argument is an array, I'll expect it to be a GeoJSON point (just like above).

@jmikola

This comment has been minimized.

Show comment
Hide comment
@jmikola

jmikola Jul 2, 2013

Member

87cb4b1 adds support for using basic arrays, removing the dependency on jmikola/geojson for $geoIntersects and $geoWithin queries.

Member

jmikola commented Jul 2, 2013

87cb4b1 adds support for using basic arrays, removing the dependency on jmikola/geojson for $geoIntersects and $geoWithin queries.

EmiiKhaos and others added some commits May 3, 2013

Support GeoJSON polygons (with holes)
This changes the polygon methods to accept an array of rings, which themselves are arrays of point tuples. Subsequent rings define holes in the first ring (i.e. outer polygon). In the future, it may be worthwhile to have a builder for polygons.
Use GeoJson objects for 2dsphere query builder/expr methods
This adds an optional dependency to a GeoJSON library (see: composer.json) for handling 2dsphere queries in query builder and expression methods. It was necessary to discern between GeoJSON points and legacy coordinates for $near and $nearSphere operators, due to how they incorporate the $maxDistance option. Additionally, using an external library for GeoJSON geometries removes the responsibility of validating those geometries from doctrine/mongodb.

Tests were updated and added for all geospatial methods in the builder and expression classes.
Support GeoJSON geometry arrays in addition to the library objects
This relaxes the GeoJSON library dependency, so 2dsphere queries (namely $geoIntersects and $geoWithin) can still be done without it.

jmikola added a commit that referenced this pull request Jul 11, 2013

Merge pull request #109 from doctrine/gh102-2dsphere
Support GeoJSON and 2dsphere queries

@jmikola jmikola merged commit edcacb1 into master Jul 11, 2013

@jmikola jmikola deleted the gh102-2dsphere branch Jul 11, 2013

@jmikola jmikola referenced this pull request in doctrine/mongodb-odm Jul 11, 2013

Open

Support GeoJSON and 2dsphere queries #631

1 of 3 tasks complete
@SteveTalbot

This comment has been minimized.

Show comment
Hide comment
@SteveTalbot

SteveTalbot Jul 24, 2013

Hi Jeremy,

The same change needs to be made in doctrine/mongodb-odm. Details here: doctrine/mongodb-odm#643

Guess this is an easy one for you to fix?

Cheers,
Steve

Hi Jeremy,

The same change needs to be made in doctrine/mongodb-odm. Details here: doctrine/mongodb-odm#643

Guess this is an easy one for you to fix?

Cheers,
Steve

This comment has been minimized.

Show comment
Hide comment
@jmikola

jmikola Jul 30, 2013

Member

@SteveTalbot: See my reply in doctrine/mongodb-odm#643. Thanks.

Member

jmikola replied Jul 30, 2013

@SteveTalbot: See my reply in doctrine/mongodb-odm#643. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment