Skip to content

Conversation

@RichardIrons-neo4j
Copy link
Contributor

Added specific tests for the case that caused the bug. Fixed using a struct wrapper for the dictionary that won't cause any allocations. Also a little docs update for specifying your own ITlsNegotiator.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request fixes a parameter bug and enhances documentation for the WithTlsNegotiator method while also improving the ToDictionary functionality and adding comprehensive tests.

  • Update XML docs for WithTlsNegotiator to warn users about certificate validation when overriding the default TLS negotiator.
  • Introduce a TryGetDictionaryOfStringKeys extension method along with a read-only dictionary wrapper to avoid allocations.
  • Add tests for parameter setting and dictionary conversion use cases.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
Neo4j.Driver/Public/ConfigBuilder.cs Updated method documentation for WithTlsNegotiator to clarify TLS certificate validation responsibilities.
Neo4j.Driver/Internal/Extensions/CollectionExtensions.cs Added TryGetDictionaryOfStringKeys logic and a read-only dictionary wrapper to support IDictionary of string keys.
Neo4j.Driver.Tests/TestUtil/CollectionExtensionsTests.cs Introduced tests covering various scenarios for the ToDictionary extension method.
Neo4j.Driver.Tests/ExecutableQuery/ExecutableQueryTests.cs Added tests to verify parameter handling using both anonymous objects and dictionaries.

…ions.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

@MaxAake MaxAake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RichardIrons-neo4j RichardIrons-neo4j merged commit 4bcf97f into neo4j:5.0 Jul 2, 2025
5 checks passed
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

Successfully merging this pull request may close these issues.

2 participants