Create an implementation for Storage Provider
Closed this issue · 17 comments
The current implementation is lacking an implementation for IStorageProvider. It will be great if an implementation is provided for this.
@sujesharukil I have considered implementing a storage provider but there is already an implementation.: https://github.com/weitaolee/Orleans.Storage.MongoDB. I haven't used it yet but it's been around for quite a while.
I don't think there would be an advantage other than only using one nuget package.
The package you have listed is 2 years old. It internally references Orleans v 1.0. I have tried using it but ran into issues. I can fork that but would prefer this to support storage provider.
Is there a way to inject a dependency to a storage provider? I spun my own but would like to control the collection name where the state gets stored.
@sujesharukil, @AwsomeCode If the nuget package I referred to isn't working I don't mind adding a storage provider to the package. I'll only have a chance to look at it this weekend though.
I'm not sure about injecting, but maybe we can just add a collection name to the configuration. I will have a look at the options once I implement.
@AwsomeCode I started implementing the Storage provider from the link you gave. I end up with an issue where the de-serialized object contains $type. When this is written to Mongo I get an error that this is an invalid field name:
This is the deserialized string: {"$id":"1","$type":"Orleans.Providers.MongoDB.Test.Grains.EmployeeState, Orleans.Providers.MongoDB.Test.Grains","Level":100}
Have you experienced this before?
@laredoza looks like there's a clash between JSON.NET's use of $type and MongoDB's: https://docs.mongodb.com/manual/reference/mongodb-extended-json/
https://stackoverflow.com/questions/9490345/json-net-change-type-field-to-another-name might help
@ReubenBond Thanks, I'll have a look at it tonight
@ReubenBond @sujesharukil @AwsomeCode I've decided to keep it simple:
public async Task Write(string collectionName, string key, string entityData)
{
var doc = global::MongoDB.Bson.Serialization.BsonSerializer.Deserialize<BsonDocument>(entityData);
// NewtonSoft generates a $type & $id which is incompatible with Mongo. Replacing $ with __
this.SwapValues(doc, "$type", "__type");
this.SwapValues(doc, "$id", "__id");
}
public async Task<string> Read(string collectionName, string key)
{
var existing = await collection.Find(builder).FirstOrDefaultAsync();
// NewtonSoft generates a $type & $id which is incompatible with Mongo. Replacing $ with __
this.SwapValues(existing, "__type", "$type");
this.SwapValues(existing, "__id", "$id");
}
private void SwapValues(BsonDocument doc, string oldFieldName, string newFieldName)
{
if (doc.Contains(oldFieldName))
{
doc[newFieldName] = doc[oldFieldName];
doc.Remove(oldFieldName);
}
}
Initial tests indicate that it's working. I will implement the Orleans unit tests tomorrow. Once this is done I will update the nuget package.
I'm also trying to get MongoDB to work with Orleans and ran into this $type field issue.
@laredoza your solution only works if the state doesn't reference other complex objects.
Nested objects will still have the invalid field names:
{
{
"$id" : "1",
"$type" : "GrainCollection.EmployeeState, GrainCollection",
"Manager" : {
"$id" : "2", "$type" : "GrainInterfaces.OrleansCodeGenManagerReference, GrainInterfaces",
"GrainId" : "424dd8d2b715c40f0a2a0abcaf761896030000006b1e46ba",
"GenericArguments" : ""
},
"MyObject" : {
"$id" : "3",
"$type" : "GrainCollection.SomeObject, GrainCollection",
"Field" : 10
}
}
}
I think the solution may be either a string replacement of the whole entityData (ouch), or do something on the Json.NET level...
Thanks @fschuh I guess my solution was a little too simple. I've implemented another solution. If you look at the employeeState class it now has an address sub class & an array. It saves successfully now. I"ve used a combination of the a JsonTextReader and JsonTextWriter. I don't think this is the most elegant solution but it seems to work. This will require more testing. I'd appreciate it if you have a look.
Example:
{
"_id" : ObjectId("594c169ea7fd7a49a458cbbb"),
"__id" : "1",
"__type" : "Orleans.Providers.MongoDB.Test.Grains.EmployeeState, Orleans.Providers.MongoDB.Test.Grains",
"Level" : 100,
"Address" : {
"__id" : "2",
"__type" : "Orleans.Providers.MongoDB.Test.Grains.EmployeeAddress, Orleans.Providers.MongoDB.Test.Grains",
"Code" : "0190",
"StreetName" : "My Street"
},
"FavouriteColours" : {
"__type" : "System.String[], mscorlib",
"__values" : [
"Red",
"Blue",
"Green"
]
},
"key" : "GrainReference=00000000000000000000000000000001030000002a285065"
}
@laredoza Thanks for quickly submitting a fix.
I was able to test it, and I can confirm it works for serialization, but it will fail deserialization if your object references any grains (it can't be instantiated because the grain reference is just an Interface).
The fix is quite simple though, I'll just post it here. In the class BaseJSONStorageProvider:
protected void ConvertFromStorageFormat(IGrainState grainState, string entityData)
{
JsonConvert.PopulateObject(entityData, grainState.State, this.serializerSettings);
}
You just need to add the serializerSettings as a parameter to the PopulateObject() call. Due to the dependency on serializerSettings I made it an instance method instead of static.
Thanks @fschuh, I'll implement this tonight. I wonder if I can make you the person in charge of testing the storage provider as I'm not using it at the moment :) It would be apreciated.
I've implemented your change @fschuh. The key field is also indexed now when a storage collection is created.
I've also updated the nuget package
Thanks @laredoza.
I was going to mention the lack of index in the key field, glad you've added it!
We're a heavy MongoDB user here where I'm working and we'll be going to do a lot of MongoDB testing with Orleans, so I'd be happy to test this storage provider. Depending on how it goes maybe even contribute as well.
Ideally I think we should do direct to/from BSON serialization (we do that in Java), completely skipping the Json.NET step which would be much more efficient. However that would take some considerable effort to implement properly. The current solution is appropriate for some testing.
Thanks @fschuh any help would be appreciated.
I agree that there are a lot of performance enhancements that still need to be done. Once everything has stabalized I'll run it through a profiler and start that process. At the moment I'm quite happy with things working. It's a good first step :)
Do you think we're at a point where we can close this issue? I think we can create seperate issues for any new bugs.
Ah yes, from my side we can definitely close this issue, unless someone else objects :)
It's been working well so far, as far as basic functionality goes.
I guess the point was create a working implementation of a Storage Provider, and we do have one now.
I'll open a new ticket if I find any bugs.
Please create a new issue if any bugs are found. Thanks for all the help