Transaction metadata can be reused between transactions

[andreycha]

Hi,

We started to use TransactionConfig in 1.7.x driver to specify some metadata. Here's our code:

var config = new TransactionConfig();
config.Metadata.Add("correlationId", correlationId);
return config;

From the first run it became obvious that it's incorrect, because we started to get an exception "System.ArgumentException: An item with the same key has already been added.". Checking the source code showed that when one creates TransactionConfig metadata is initialize by static empty dictionary PackStream.EmptyDictionary.

Documentation (https://neo4j.com/docs/driver-manual/1.7/) doesn't state anything about how to use TransactionConfig. I believe this is an incorrect behaviour, and should be adjusted to always initialize Metadata with a new dictionary. If there is some performance consideration behind it, proper way of using config should be states in XML comments and/or in the documentation.

In 4.0.0 it is not an issue, because the config is always specified via builder where metadata is overwritten. Although some tests still use either default constructor or static Default property and are vulnerable to the same situation.

Neo4j Version: 3.5.12 Enterprise Neo4j Mode: Single instance
Driver version: .NET driver 1.7.91

[technige]

Hi @andreycha. Thanks for raising this. It looks like a bug. We have added to our backlog to address this.

[andreycha]

Hi @technige . Thanks for confirming. I could open PRs for 1.7 and 4.0.0 if you want.

[zhenlineo]

Hi @andreycha ,

In 1.7, we expect users to always config tx meta data with a new dictionary:

var config = new TransactionConfig();
config.Metadata = new Dictionary<string, object>{{"key", "value"}};

The reason of this design probably was to avoid creating a dictionary for each new TransactionConfig on creation. As we estimated that most users may not use transaction metadata.

Sorry for not coming back for a long time. Pls be free to re-open this issue if you have further questions.

[andreycha]

Hi @zhenlineo , I see, thanks for the explanation. Probably it makes sense to put a note in the documentation.