Elfocrash / Cosmonaut

🌐 A supercharged Azure CosmosDB .NET SDK with ORM support
https://cosmonaut.readthedocs.io
MIT License
341 stars 44 forks source link

Added ability to configure unique keys for collection #91

Closed florisrobbemont closed 5 years ago

florisrobbemont commented 5 years ago

We needed to add a unique key policy when creating the collection. Since we like to let Cosmonaut handle collection provisioning I added this feature to the codebase.

Elfocrash commented 5 years ago

Hello. The code looks good and I like the idea of the feature but before I approve this I need to test how the shared collection behaviour would work with this.

florisrobbemont commented 5 years ago

I’ve built that into the code and into the tests. You cannot have a unique key policy for a shared collection, since not all of the entities might have those unique properties. I also can’t think of a scenario where this would be useful in a shared collection.

So if you configure an entity with a unique key and a shared collection attribute it will throw an exception.

Would like to hear your thoughts on this.

Elfocrash commented 5 years ago

What would the behaviour be if the collection exists with a different unique key policy?

florisrobbemont commented 5 years ago

In that case it would also crash on initialization:

https://github.com/florisrobbemont/Cosmonaut/blob/0c1b48cea9b2c016f551e019f8dde7a8b514d47a/src/Cosmonaut/Storage/CosmosCollectionCreator.cs#L49

The check is in the CollectionCreator, before it goes into the CosmosClient. So if you try to configure this on an existing collection/entitity you will get an exception on init.

Elfocrash commented 5 years ago

I would rather have it behave differently. If the collection doesn't exist then create it with the uniquekeypolicy. If the collection does exist then query then don't do anything. It should go on par with the indexing policy and the RUs.

florisrobbemont commented 5 years ago

I've ran some tests, and Cosmos DB accepts it when you push a unique key that doesn't exists. I don't know how this will hold up in runtime, since the non existing property will probably result in 'null', and will therefor not be unique accross the documents.

The code doesn't throw an exception anymore for both existing and non-existing collection, leaving it up to the user to make the correct decisions.