Skip to content

Commit

Permalink
Fix #381 - JsonConverters check CanRead/CanWrite before branching (#382)
Browse files Browse the repository at this point in the history
* Fix #381 - JsonConverters check CanRead/CanWrite before branching

* Added unit tests

* Added one more check, for good measure
  • Loading branch information
adam8797 committed Nov 2, 2020
1 parent 3967b84 commit a880a9e
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 7 deletions.
50 changes: 50 additions & 0 deletions Neo4jClient.Tests/Extensions/Neo4jDriverExtensionsTests.cs
Expand Up @@ -6,6 +6,7 @@
using Moq;
using Neo4jClient.Cypher;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Xunit;

namespace Neo4jClient.Tests.Extensions
Expand All @@ -30,6 +31,27 @@ internal class ClassWithDateTimeOffset
public DateTimeOffset Dt { get; set; }
}

internal class ClassWithString
{
public string String { get; set; }
}

internal class ClassWithStringReadOnlyConverter : JsonConverter<ClassWithString>
{
public override bool CanRead => true;
public override ClassWithString ReadJson(JsonReader reader, Type objectType, ClassWithString existingValue, bool hasExistingValue, JsonSerializer serializer)
{
var jo = JObject.Load(reader);
return jo.ToObject<ClassWithString>();
}

public override bool CanWrite => false;
public override void WriteJson(JsonWriter writer, ClassWithString value, JsonSerializer serializer)
{
throw new NotImplementedException();
}
}

public class Neo4jDriverExtensionsTests
{
public class ToNeo4jDriverParametersMethod : IClassFixture<CultureInfoSetupFixture>
Expand Down Expand Up @@ -275,6 +297,34 @@ public void UsesCustomJsonSerializersWhereItCan()

serializedObj["Dt"].Should().Be(dateTime.ToUniversalTime().Ticks);
}

[Fact]
// Issue 381 - https://github.com/DotNet4Neo4j/Neo4jClient/issues/381
public void UsesCustomJsonSerializersAndRespectsReadOnly()
{
var obj = new ClassWithString
{
String = "This is a String"
};

var mockGc = MockGc;
mockGc
.Setup(x => x.JsonConverters)
.Returns(new List<JsonConverter> { new ClassWithStringReadOnlyConverter() });

var query = new CypherFluentQuery(mockGc.Object)
.Create("(n:Node $value)")
.WithParam("value", obj);

var actual = query.Query.ToNeo4jDriverParameters(mockGc.Object);
actual.Keys.Should().Contain("value");
// Upon regression, actual["value"] will be of type JObject
actual["value"].Should().NotBeOfType<JObject>();
actual["value"].Should().BeOfType<Dictionary<string, object>>();
var serializedObj = (Dictionary<string, object>)actual["value"];

serializedObj["String"].Should().Be(obj.String);
}
}
}
}
2 changes: 1 addition & 1 deletion Neo4jClient/Cypher/CypherReturnExpressionBuilder.cs
Expand Up @@ -524,7 +524,7 @@ static bool IsSupportedElementForAs(Type type, IList<JsonConverter> jsonConverte
return true;

jsonConvertersThatTheDeserializerWillUse = jsonConvertersThatTheDeserializerWillUse ?? GraphClient.DefaultJsonConverters;
if (jsonConvertersThatTheDeserializerWillUse.Any(c => c.CanConvert(type)))
if (jsonConvertersThatTheDeserializerWillUse.Any(c => c.CanConvert(type) && c.CanRead))
return true;

var hasDefaultConstructor = type.GetConstructor(Type.EmptyTypes) != null;
Expand Down
4 changes: 2 additions & 2 deletions Neo4jClient/Neo4jDriverExtensions.cs
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
Expand Down Expand Up @@ -67,7 +67,7 @@ private static object Serialize(object value, IList<JsonConverter> converters, I
var type = value.GetType();
var typeInfo = type.GetTypeInfo();

var converter = converters.FirstOrDefault(c => c.CanConvert(type));
var converter = converters.FirstOrDefault(c => c.CanConvert(type) && c.CanWrite);
if (converter != null)
{
var serializer = new CustomJsonSerializer{JsonConverters = converters, JsonContractResolver = ((IRawGraphClient)gc).JsonContractResolver};
Expand Down
2 changes: 1 addition & 1 deletion Neo4jClient/Serialization/CommonDeserializerMethods.cs
Expand Up @@ -310,7 +310,7 @@ public static object CreateAndMap(DeserializationContext context, Type type, JTo
static bool TryJsonConverters(DeserializationContext context, Type type, JToken element, out object instance)
{
instance = null;
var converter = context.JsonConverters?.FirstOrDefault(c => c.CanConvert(type));
var converter = context.JsonConverters?.FirstOrDefault(c => c.CanConvert(type) && c.CanRead);
if (converter == null) return false;
using (var reader = element.CreateReader())
{
Expand Down
4 changes: 2 additions & 2 deletions Neo4jClient/Serialization/CustomJsonSerializer.cs
Expand Up @@ -37,7 +37,7 @@ public string Serialize(object obj)

if (JsonConverters != null)
{
foreach (var converter in JsonConverters.Reverse())
foreach (var converter in JsonConverters.Reverse().Where(writer => writer.CanWrite))
serializer.Converters.Add(converter);
}

Expand All @@ -62,7 +62,7 @@ public T Deserialize<T>(string content)

if (JsonConverters != null)
{
foreach (var converter in JsonConverters.Reverse())
foreach (var converter in JsonConverters.Reverse().Where(writer => writer.CanRead))
serializer.Converters.Add(converter);
}

Expand Down
2 changes: 1 addition & 1 deletion Neo4jClient/StatementResultHelper.cs
Expand Up @@ -284,7 +284,7 @@ private static T Parse<T>(this IRecord record, string identifier, IGraphClient g
};

foreach (var jsonConverter in converters)
if (jsonConverter.CanConvert(typeof(T)))
if (jsonConverter.CanConvert(typeof(T)) && jsonConverter.CanRead)
return JsonConvert.DeserializeObject<T>(record[identifier].As<INode>().ToJsonString(), serializerSettings);


Expand Down

0 comments on commit a880a9e

Please sign in to comment.