Azure / azure-cosmos-dotnet-v3

.NET SDK for Azure Cosmos DB for the core SQL API
MIT License
745 stars 496 forks source link

Unseal GeoJson classes? #1980

Open jaybo opened 4 years ago

jaybo commented 4 years ago

The GeoJson specification (https://tools.ietf.org/html/rfc7946) allows for extensions and additional foreign members:

6.1. Foreign Members Members not described in this specification ("foreign members") MAY be used in a GeoJSON document.

But since all of the GeoJson classes in the Spatial namespace are marked sealed, subclassing is not available, and hence foreign members are not possible.

What is the justification for the sealed modifier and why not remove it to make the Spatial namespace classes usable for a larger audience?

One glaring example is the need to add id and other Cosmos related document properties. Or is it assumed Spatial namespace classes will never be root documents?

bchong95 commented 4 years ago

As the rfc mentions while geojsons MAY have foreign members, they are frowned upon and the RFC states that you lose interoperability if you have them.

As you have correctly brought up in a previous issue:

https://github.com/Azure/azure-cosmos-dotnet-v3/issues/1854

The cleaner way to achieve this is to use composition with feature and feature collection.

For instead of:

{
         "type": "Point",
         "coordinates": [100.0, 0.0],
         "some foriegn property A": "asdf"
}

You can model your object as:

{
       "type": "Feature",
       "geometry": {
           "type": "Point",
         "coordinates": [100.0, 0.0],
       },
       "properties":{
           "A": "asdf"
        }
}
jaybo commented 4 years ago

Right, but CosmosDB requires that an "id" property at the root document level. Hence, none of the Spatial data types can exist at the root document level in a container at present. Simply making these classes unsealed allows for that possibility.