OrleansContrib / Orleans.Providers.MongoDB

A MongoDb implementation of the Orleans Providers: Membership, Storage and Reminders.
MIT License
105 stars 44 forks source link

Support for BSON Serializer #106

Closed wassim-k closed 1 year ago

wassim-k commented 1 year ago

Currently, this provider only supports using Json.NET for serializing grain state to MongoDB.
It would be great to support the option to use BSON Serializer shipped with mongodb driver instead, since it has great documentation, support and many features that are specific for mongo db storage.

For example a more generic interface like this one might serve the purpose:

public interface IMongoGrainStorageSerializer
{
    void Deserialize<T>(IGrainState<T> grainState, BsonDocument document);

    BsonDocument Serialize<T>(IGrainState<T> grainState);
}

Current implementation can be achieved with something like this:

public class MongoGrainStateSerializer : IMongoGrainStorageSerializer
{
    private readonly IGrainStateSerializer _serializer;

    public MongoGrainStateSerializer(IGrainStateSerializer serializer)
    {
        _serializer = serializer;
    }

    public void Deserialize<T>(IGrainState<T> grainState, BsonDocument document)
    {
        _serializer.Deserialize(grainState, document.ToJToken());
    }

    public BsonDocument Serialize<T>(IGrainState<T> grainState)
    {
        var jObj = _serializer.Serialize(grainState);
        return jObj.ToBson();
    }
}

And finally using BSON Serializer can be done like this:

public class MongoBsonGrainStorageSerializer : IMongoGrainStorageSerializer
{
    public void Deserialize<T>(IGrainState<T> grainState, BsonDocument document)
    {
        grainState.State = BsonSerializer.Deserialize<T>(document);
    }

    public BsonDocument Serialize<T>(IGrainState<T> grainState)
    {
        return BsonDocumentWrapper.Create(grainState.State);
    }t
}

The one issue I can see with using BSON Serialize is the lack of support for some Json.NET Converters that are implemented by Orleans, but I believe it wouldn't be too difficult to implement those converters using BSON Serializer, and I'd gladly contribute that work to your repo if you're happy with this feature suggestion.

SebastianStehle commented 1 year ago

The serializer already exists, but is coupled to json now. Should be easy to change it return BsonDocument now.

But I would probably go with BsonValue instead of BsonDocument to support scalars line int, `string and so on. Do you want to provide a PR for that?

wassim-k commented 1 year ago

Is it okay to introduce a breaking change by changing the current definition of IGrainStateSerializer?

SebastianStehle commented 1 year ago

For 4.0...yes

wassim-k commented 1 year ago

Sounds good, once the 4.0 changes are merged, I'll raise a PR for this.

SebastianStehle commented 1 year ago

Great. I am not planning any migration before Orleans 4.0 is released. So nothing for prereleases.

wassim-k commented 1 year ago

Just a heads up, the work on this is already done, let me know when it's time to merge it into the 7.0 migration.