Elfocrash / Cosmonaut

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

WIP. Adds query interception, resolves #60 #63

Open da1rren opened 5 years ago

da1rren commented 5 years ago

This pull request allows the developer to specify typed predicate which will then be applied to allow LINQ -> SQL query requests.

An example of how to configure a global filter:

var cosmosStoreSettings = new CosmosStoreSettings(_databaseId, _emulatorUri, _emulatorKey,
    settings =>
    {
        settings.AddInterceptor<Cat>(x => x.Name == "Nick");
    });

I've added integration tests covering most of the basic scenarios.

todo

da1rren commented 5 years ago

Looks like my history rewrite went a little wrong, just fixing it now.

Elfocrash commented 5 years ago

I think, moving it to the CosmosStore constructor as it's own thing would be more appropriate. The reason is that there are two ways to initialise the CosmosStore.

1) Using the CosmosStoreSettings 2) Passing your own ICosmonautClient

If you do add it in the settings then you are limiting the feature to a specific initialisation method. I's rather have it in both.

Wouldn't it be better if we have a separate class that both constructors take where you store things like query interceptors, or operations before and after like a middle-ware? See https://github.com/Elfocrash/Cosmonaut/issues/58 for example. Mats needs an Action that's invoked before the operation happens. Interceptors and request delegates could sit together on a new class.

It would also allows us to control the TEntity and validate it in compile time.

What do you think about this idea?

EDIT: Also this whole feature is more of a Query Filter than a Query Interceptor. I know it's using query interception behind the scenes but the CosmosStore Query Filter is what will end up being exposed to the user.

da1rren commented 5 years ago

@Elfocrash I agree it makes far more sense for it to exist on the CosmosStore constructor. I'll add some overrides to the constructors. It also saves a filter operation and resolves the issue with the IReadOnly collection.

da1rren commented 5 years ago

@Elfocrash The integration tests appear to be failing on Az Devops. I cannot replicate locally and the error looks suspiciously similar to some issues with the emulator I've been having recently. Would you be able to investigate?

Elfocrash commented 5 years ago

Don't worry about integration tests. the Cosmos DB team has been doing some stuff with the emulator and it's failing on some agents. I will deal with that problem once the feature is done.