mycouch
mycouch copied to clipboard
Null values are serialized as empty (no property at all), but Dictionaries break the json
I think null values in object should be serialized as null value in object, instead the DocumentJsonWriter.cs implements WriteNull() like this:
public override void WriteNull()
{
base.WriteRaw(string.Empty);
}
Fair enought but this breaks serializing a dictionary, that's commonly used for data not in the structure by tagging it with the attribute [JsonExtensionData]. In an unstructured database it's a useful feature.
[JsonExtensionData]
private IDictionary<string, JToken> _additionalData;
I wrote a test that breaks with an exception:
[Fact]
public void When_serializing_anonymous_entity_It_will_contain_null_values_in_json()
{
var model = new
{
Id = "abc",
Rev = "505e07eb-41a4-4bb1-8a4c-fb6453f9927d",
Value = "Some value.",
Dict = new Dictionary<string, object> { { "prop1", (string)null }, { "prop2", (string)null } }
};
var json = SUT.Serialize(model);
json.Should().Contain("\"prop1\":null");
}
A solution is to remove the override void WriteNull() method, all the unit tests passes.
I came back to this issue, and really can't understand why no one care about it. The problem is that if the serializer doesn't include the null property, and when you have a Mango index on that property, the indexer doesn't include the document in the index, so the the query result would be wrong on query like this: {$eq: null}. I found myself a solution which seems to work:
Add this to the document classes (or the base class):
[JsonObject(ItemNullValueHandling = NullValueHandling.Include)]
Add this to the document _rev propery:
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
Change the bootstrapper so we can use a custom serializer
var bootstrapper = new MyCouchClientBootstrapper();
var documentSerializer = new Lazy<ISerializer>(() =>
{
var documentMetaProvider = new DocumentSerializationMetaProvider();
var contractResolver = new SerializationContractResolver();
var config = new SerializationConfiguration(contractResolver);
return new MyDefaultSerializer(config, documentMetaProvider, bootstrapper.EntityReflectorFn());
});
bootstrapper.DocumentSerializerFn = () => documentSerializer.Value;
and finally this basically does via reflection what I said in the first post
internal class MyDefaultSerializer : DefaultSerializer
{
public MyDefaultSerializer(SerializationConfiguration configuration, IDocumentSerializationMetaProvider documentMetaProvider, IEntityReflector entityReflector = null) :
base(configuration, documentMetaProvider, entityReflector)
{
}
protected override JsonTextWriter CreateWriterFor(Type docType, TextWriter writer)
{
if (EntityReflector == null)
return CreateWriter(writer);
var documentMeta = DocumentMetaProvider.Get(docType);
return documentMeta == null
? CreateWriter(writer)
: new MyDocumentJsonWriter(writer, documentMeta, Configuration.Conventions, EntityReflector);
}
}
internal class MyDocumentJsonWriter : DocumentJsonWriter
{
private Action baseWriteNull;
public MyDocumentJsonWriter(TextWriter textWriter, DocumentSerializationMeta documentMeta, SerializationConventions conventions, IEntityReflector entityReflector) :
base(textWriter, documentMeta, conventions, entityReflector)
{
var ptr = typeof(JsonTextWriter).GetMethod("WriteNull").MethodHandle.GetFunctionPointer();
baseWriteNull = (Action)Activator.CreateInstance(typeof(Action), this, ptr);
}
public override void WriteNull()
{
baseWriteNull();
}
}
We could configure the documentserializer to include the null value, instead of using the attuributes, but it happens that QueryAsync() (and maybe others) uses the document serializer for the whole response, this causes an issue deserializing the response (the offset value can be null, but the c# propery is not nullable).
I know that this is hard to fix, and I understand the need of having 2 different serializers, however the "include null values" settings could be handled someway with contract resolver by mycouch and documented as a feature.
Finally I don't understand why WriteNull() has been overriden so it returns an empty string instead of the "null" value, this causes the production of a corrupted json when the is used NullValueHandling.Include.
What happens to the tests if you remove:
public override void WriteNull()
{
base.WriteRaw(string.Empty);
}
If nothing seem to break I'm all for getting a PR that makes the change.
Perhaps the SerializationConfiguration could add a setting and the config can be injected to the writer so that it can be configured to behave as the "old" code did. That way it can be documented and I can make a major bump.
A solution is to remove the override void WriteNull() method, all the unit tests passes.
This is what I did, 1,5 years ago, I remember I removed the override and the test passed. I'll make the PR, when I got some time.
I did the change and all tests passed. I don't understand what do you mean with this:
Perhaps the
SerializationConfigurationcould add a setting and the config can be injected to the writer so that it can be configured to behave as the "old" code did. That way it can be documented and I can make a major bump.
Probably I wasn't clear. I see an additional problem, harder to fix, in this piece of code in MyCouchClientBootstrapper.cs:
protected virtual void ConfigureViewsFn()
{
ViewsFn = cn => new Views(
cn,
DocumentSerializerFn());
}
The problem is that the DocumentSerializer is not only used to deserialize the document from the response (in the "docs" field), indeed it's used also for the metadata like the field "offset", and if you query the view with reduce=true, the offset property is omitted:
{
"rows": [{
"key": null,
"value": 392
}
]
}
If the document serializer is set to include the null values, the omitted offset value is deserialized as null, the the ViewQueryResponse. The Offset prop is not nullable, and the deseralization fails.
All of this it to point out that configure globally the document serializer to include the null props could lead to unexpected effects that has to be tested more. I'm going to create the pull request to fix the main issue