netstandard/neo4j v4/compatibility with Neo4jClient

[kYann]

• Migrate to new version of neo4j syntax
• Use same contractresolver than neo4jClient

[simonpinn]

HI @kYann - fantastic to see you pulled this up, it crossed my mind recently to revisit, I'm just not actively using Neo4j ATM...

Is there a reason the Microsoft.VisualBasic package is included?
Secondary comment would be to also remove the packages.config too, but I wouldn't worry for now

[simonpinn]

This should remain as camelCased, was there a reason for this change?

[kYann]

I'm using Neo4jClient and Neo4jClient.Extensions. When using Neo4jClient and the Where extension method, I noticed that Neo4jClient was not using camelCase. Which make both library incompatible on the same database.
As this is something that cannot be easily changed in Neo4jClient, I decided to unify the behavior and use the ContractResolver of Neo4jClient in Neo4jClient.Extensions

[simonpinn]

Ah, hmmm - well, I had to double to check, since I actually added camelCase support to the Neo4jClient library https://github.com/DotNet4Neo4j/Neo4jClient/pull/66/files - and while the default may not be that, it seems like an unnecessary change to the tests in this library... It'd be great if this change could limit it's self to the upgrade to netstandard2 which I absolutely agree is long over-due. Thanks

[kYann]

I know that I did not take the time to do a nice PR, and it should have been separated PR.
As I'm rushing to launch our product, this was not my priority (and I certainly should have waited to do that PR).

For the CamelCase things :

In Neo4jClient, the default JsonContractResolver is not CamelCase (as your can see here )
In Neo4JClient.Extensions, by default the resolver was CamelCase, I switched it to using the DefaultJsonContractResolver of the GraphClient instead of an hard coded one (See this change )

If you don't do this, Neo4jClient and Neo4jClient does not use (by default) the same casing.

[simonpinn]

Appreciated @kYann - ok then instead of using GraphClient.DefaultJsonContractResolver you should use the graphClient instance JsonContractResolver property, otherwise if the graph client in use has been set toCamelCasePropertyNamesContractResolver then you'll be out of sync. https://github.com/simonpinn/Neo4jClient.Extension/pull/30/files#diff-009515381c89ea310a3f146aae7e24aca34aff0e7aa8e5c3328ad9c2d8ff991eR23 Will need updating

tldr - If the user has set graphClient.JsonContractResolver = new CamelCasePropertyNamesContractResolver(); then GraphClient.DefaultJsonContractResolver will still be Neo4jContractResolver since https://github.com/DotNet4Neo4j/Neo4jClient/blob/master/Neo4jClient/GraphClient.cs#L42 is readonly

[kYann]

Yes, but in CypherExtensionContext I don't have access to neo4j current GraphClient. Maybe I need to change the signature of the constructor of this class to take the current GraphClient ? I will try to look at it when my product is released. Thank you

…

On Thu, Mar 18, 2021 at 11:00 PM Simon ***@***.***> wrote: Appreciated @kYann <https://github.com/kYann> - ok then instead of using GraphClient.DefaultJsonContractResolver you should use the graphClient instance JsonContractResolver property, otherwise if the graph client in use has been set toCamelCasePropertyNamesContractResolver then you'll be out of sync. https://github.com/simonpinn/Neo4jClient.Extension/pull/30/files#diff-009515381c89ea310a3f146aae7e24aca34aff0e7aa8e5c3328ad9c2d8ff991eR23 Will need updating tldr - If the user has set graphClient.JsonContractResolver = new CamelCasePropertyNamesContractResolver(); then GraphClient.DefaultJsonContractResolver will still be Neo4jContractResolver since https://github.com/DotNet4Neo4j/Neo4jClient/blob/master/Neo4jClient/GraphClient.cs#L42 is readonly — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#30 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAY2WFWIQA254HXDAQTBIDTEJZXPANCNFSM4ZAOG7ZA> .

-- Yann

[simonpinn]

closing in favour of recent PR #31