Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CosmosSerializer and CosmosSerializationOptions in the Cosmos Provider #24334

Closed
wants to merge 1 commit into from

Conversation

Marusyk
Copy link
Member

@Marusyk Marusyk commented Mar 7, 2021

Added support for CosmosSerializationOptions and CosmosSerializer

It is a very useful feature when we want to ignore null values when saving data in CosmosDb with Entity Framework
https://stackoverflow.com/questions/66424039/ignore-null-values-when-saving-data-in-cosmosdb-with-entity-framework

Fixes #20670

Please review
Thank you in advance

@@ -560,6 +621,8 @@ public override long GetServiceProviderHashCode()
hashCode = (hashCode * 131) ^ (Extension._gatewayModeMaxConnectionLimit?.GetHashCode() ?? 0);
hashCode = (hashCode * 397) ^ (Extension._maxTcpConnectionsPerEndpoint?.GetHashCode() ?? 0);
hashCode = (hashCode * 131) ^ (Extension._maxRequestsPerTcpConnection?.GetHashCode() ?? 0);
hashCode = (hashCode * 131) ^ (Extension._serializer?.GetHashCode() ?? 0);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about this code. can anyone explain how to implement it in a proper way?

Copy link
Member Author

@Marusyk Marusyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add more tests and fix the behavior.

@@ -127,6 +127,34 @@ public async Task Should_throw_if_specified_connection_mode_is_wrong()
});
}

[ConditionalFact]
public async Task Should_use_serialization_options()
Copy link
Member Author

@Marusyk Marusyk Mar 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is just for example. I actually don't know where I should put tests for this.
Also, after SaveChanges, the Name will be stored with null. It seems I missed something. Can it be some Tests configuration? Could you please help?

@AndriySvyryd
Copy link
Member

CosmosSerializationOptions and CosmosSerializer shouldn't be exposed on any public API as they will move to Azure.Cosmos.Serialization in SDK v4, see: CosmosSerializer.cs, CosmosSerializationOptions.cs

#20670 and #17306 should be implemented after #18753 and #14570 are done

@Marusyk
Copy link
Member Author

Marusyk commented Mar 11, 2021

Hi @AndriySvyryd
Is it means that we should wait for SDK v4 release (November 2021) after that, we can implement this feature in EF and release it in November 2022? 😮

@AndriySvyryd
Copy link
Member

@Marusyk We can upgrade to a feature complete preview of V4, this should give us enough time before release

@AndriySvyryd
Copy link
Member

In any case a better approach would be to expose the options on OptionsBuilder directly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Support for CosmosSerializationOptions in the Cosmos Provider
2 participants