Page MenuHomePhabricator

RFC: Disallow recursive FeatureCollection
Closed, ResolvedPublic

Description

It seems that according to the geojson spec, FeatureCollection is NOT a feature itself, and therefore cannot be recursively added. That said, it also seems that the Leaflet's implementation is more permissive and allows for that, as well as for an array of features.

Internally we use an array to combine features from multiple groups, but in the backend I am forced to convert top level array into a FeatureCollection, and recursively expand any FeatureCollections inside into lists of features.

Should we officially disallow any nested FeatureCollections (as part of the json schema), and keep using the Leaflet's array handling capability to avoid expanding top-level collections? On the backend I will expand the top-level featurecollections.

This will force us to be more strictly compatible with geojson (even though we do extend it with type: ExternalData

Details

Related Gerrit Patches:
mediawiki/extensions/Kartographer : masterAdd FeatureCollection tests

Event Timeline

Yurik created this task.Oct 2 2016, 6:36 PM
Restricted Application added a project: Discovery. · View Herald TranscriptOct 2 2016, 6:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
MaxSem closed this task as Declined.Oct 2 2016, 8:51 PM

Unfortunately, the spec allows nested FeatureCollections:

https://tools.ietf.org/html/rfc7946#section-3.3 :

The value of "features" is a JSON array.
Each element of the array is a Feature object as defined above. It
is possible for this array to be empty.

https://tools.ietf.org/html/rfc7946#section-3.2 :

A Feature object has a member with the name "geometry". The value
of the geometry member SHALL be either a Geometry object as
defined above or, in the case that the Feature is unlocated, a
JSON null value.

https://tools.ietf.org/html/rfc7946#section-3.1 :

The value of a Geometry object's "type" member MUST be one of the
seven geometry types (see Section 1.4).

https://tools.ietf.org/html/rfc7946#section-1.4 :

Inside this document, the term "geometry type" refers to seven
case-sensitive strings: "Point", "MultiPoint", "LineString",
"MultiLineString", "Polygon", "MultiPolygon", and
"GeometryCollection".

Yurik reopened this task as Open.EditedOct 2 2016, 9:04 PM

GeometryCollection is not the same as FeatureCollection. GeometryCollection could be nested, but it seems FeatureCollection is only one (top level).

MaxSem added a comment.Oct 2 2016, 9:23 PM

See my first link, it discusses specifically FC.

Yurik added a comment.Oct 2 2016, 9:34 PM

@MaxSem, you first link points to 3-3:

A GeoJSON object with the type "FeatureCollection" is a
FeatureCollection object. A FeatureCollection object has a member
with the name "features". The value of "features" is a JSON array.
Each element of the array is a Feature object as defined above. It
is possible for this array to be empty.

It does not say that a "FeatureCollection" is a type of "Feature"

MaxSem added a comment.Oct 2 2016, 9:39 PM

FeatureCollection contains Features => Feature's geometry is one of seven types => FeatureCollection is one of these types => Feature can contain FeatureCollections => FeatureCollections can be nested.

Yurik added a comment.Oct 2 2016, 9:42 PM

Nope, you are confusing FeatureCollection and GeometryCollection. geometry can be one of the seven types, none of which are FeatureCollection. FeatureCollection can only contain items of one type - Feature.

Change 313852 had a related patch set uploaded (by MaxSem):
Add FeatureCollection tests

https://gerrit.wikimedia.org/r/313852

MaxSem claimed this task.Oct 3 2016, 6:33 PM
MaxSem moved this task from Backlog to Needs review on the Maps-Sprint board.Oct 3 2016, 6:42 PM

Change 313852 merged by jenkins-bot:
Add FeatureCollection tests

https://gerrit.wikimedia.org/r/313852

MaxSem closed this task as Resolved.Oct 3 2016, 6:59 PM