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

Unable to deserialize object #311

Open
simendsjo opened this issue Nov 19, 2018 · 5 comments
Open

Unable to deserialize object #311

simendsjo opened this issue Nov 19, 2018 · 5 comments

Comments

@simendsjo
Copy link
Contributor

simendsjo commented Nov 19, 2018

I think 33a78fd might have broken something

class Record {
  public Guid A;
  public string B;
  public string C;
  public string D;
  public string E;
}

It's actually an F# type, so it would look more like this:

public sealed class Record : IEquatable<Record>, IStructuralEquatable, IComparable<Record>, IComparable, IStructuralComparable {
  public Guid A { get; internal set; }
  // ...
}

Here's the output which fails:

{ "columns":["records"]
, "data":[[
      "B":"B"
    , "C":null
    , "D":null
    , "A":"a6ceb0a4-0268-4424-b734-2813a5787f6d"
    ]]
}

EDIT: This is only a problem with the Bolt client.

@simendsjo
Copy link
Contributor Author

I can verify that all our tests work if we uncomment the two lines here: 33a78fd#diff-c9f5f40cbc0efb158b9bf3d487cb18c9R122

image

@simendsjo
Copy link
Contributor Author

I don't understand why a Complex type shouldn't be enclosed in {} as this generates invalid json. But the change looks very intentional.

@simendsjo
Copy link
Contributor Author

Not sure how to fix this. Note the following patch, which works for me, but changes the semantics.
See the DeserializesAnonymousNestedObjectCorrectly where I had to remove one level of {} to make the test pass. To my untrained eye, the original test looks wrong (dict of dict == { {} } as it is now), but I haven't looked at how the format is supposed to be. I hope the test is wrong and that this patch fixes the issue.

diff --git a/Neo4jClient.Shared/StatementResultHelper.cs b/Neo4jClient.Shared/StatementResultHelper.cs
index b35daa6..92935b1 100644
--- a/Neo4jClient.Shared/StatementResultHelper.cs
+++ b/Neo4jClient.Shared/StatementResultHelper.cs
@@ -119,8 +119,6 @@ namespace Neo4jClient
                     output.Add(s);
                 }
 
-                if (isConcreteType)
-                    return string.Join(",", output);
                 return $"{{ {string.Join(",", output)} }}";
             }
 
@@ -146,8 +144,7 @@ namespace Neo4jClient
                     }
                     else if (eType == typeof(Dictionary<string, object>))
                     {
-                        //here!!
-                        output.Add($"{{{e.ToJsonString(inSet, false, true, isConcreteType) ?? "null"}}}");
+                        output.Add($"{e.ToJsonString(inSet, false, true, isConcreteType) ?? "null"}");
                         onlyKvp = false;
                     }
                     else
diff --git a/Neo4jClient.Tests.Shared/StatementResultHelperTests.cs b/Neo4jClient.Tests.Shared/StatementResultHelperTests.cs
index 59238ae..b2b0c28 100644
--- a/Neo4jClient.Tests.Shared/StatementResultHelperTests.cs
+++ b/Neo4jClient.Tests.Shared/StatementResultHelperTests.cs
@@ -209,7 +209,7 @@ namespace Neo4jClient.Test
                 record.Keys.Returns(new List<string> {"Data"});
                 record["Data"].Returns(list);
               
-                var expectedContent = "{ \"columns\":[\"Data\"], \"data\":[[ [{\"Id\":1}] ]] }";
+                var expectedContent = "{ \"columns\":[\"Data\"], \"data\":[[ [{ \"Id\":1 }] ]] }";
                 var mockDeserializer = new Mock<ICypherJsonDeserializer<RecordWithListOfComplex>>();
                 mockDeserializer
                     .Setup(d => d.Deserialize(It.IsAny<string>()))
@@ -257,7 +257,7 @@ namespace Neo4jClient.Test
                     }
                 );
 
-                var expectedContent = "{ \"columns\":[\"x\"], \"data\":[[ [{{ \"Info\":{\"A\":\"a\",\"B\":\"b\"} }}] ]] }";
+                var expectedContent = "{ \"columns\":[\"x\"], \"data\":[[ [{ \"Info\":{\"A\":\"a\",\"B\":\"b\"} }] ]] }";
                 var mockDeserializer = new Mock<ICypherJsonDeserializer<string>>();
                 mockDeserializer
                     .Setup(d => d.Deserialize(It.IsAny<string>()))

@cskardon
Copy link
Member

Unfortunately - if I remember correctly - the test is correct - the way that the REST client returns the data is as the test has it - not as it should be.

As it happens it didn't fix the issue I was trying to - so I'm happy to roll it back for now and try to get it done another way.

Can you PR your patch?

simendsjo added a commit to simendsjo/Neo4jClient that referenced this issue Nov 20, 2018
@simendsjo
Copy link
Contributor Author

Unfortunately - if I remember correctly - the test is correct (...)
(...) Can you PR your patch?

Then this patch isn't correct. I think the best quickfix would be to revert the commit: #312

cskardon pushed a commit that referenced this issue Nov 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants