Fix issues with nested Sets

[petergebala]

Hi,

Code there is quite complex I am not 100% convinced that my solution is correct.

        def fresh_cluster_composition_fetched(composition_lookup_result)
          @log.debug("Fetched cluster composition for database '#{@database_name.description}'. #{composition_lookup_result.cluster_composition}")
          @routing_table.update(composition_lookup_result.cluster_composition)
          @routing_table_registry.remove_aged
          addresses_to_retain = @routing_table_registry.all_servers.map(&:unicast_stream).reduce(&:+)

          composition_lookup_result.resolved_initial_routers&.then do |addresses|
            addresses_to_retain << addresses
          end

          @connection_pool.retain_all(addresses_to_retain)

          @log.debug("Updated routing table for database '#{@database_name.description}'. #{routing_table}")
          @routing_table
        rescue => error
          cluster_composition_lookup_failed(error)
        end

I think problem is with << method here. Looks like both lines:

@routing_table_registry.all_servers.map(&:unicast_stream).reduce(&:+)

composition_lookup_result.resolved_initial_routers

Return a Set:

irb(main):152> composition_lookup_result.resolved_initial_routers
=> #<Set: {#<Neo4j::Driver::Internal::BoltServerAddress:0x00007f39d5a8eb78 @connection_host="XXXXXX", @host="XXXXXX", @port=7687>}>
irb(main):153> ActiveGraph::Base.driver.session_factory.connection_provider.routing_table_registry.all_servers.map(&:unicast_stream).reduce(&:+)
=>
#<Set:
 {#<Neo4j::Driver::Internal::BoltServerAddress:0x00007f39d00503a0 @connection_host="XXXXXX", @host="XXXXXX", @port=7687>,
  #<Neo4j::Driver::Internal::BoltServerAddress:0x00007f39d0050080 @connection_host="XXXXXX", @host="XXXXXX", @port=7687>,
  #<Neo4j::Driver::Internal::BoltServerAddress:0x00007f39d0050710 @connection_host="XXXXXX", @host="XXXXXX", @port=7687>}>

By using << you create nested structure:

irb(main):001> a = Set.new
=> #<Set: {}>
irb(main):002> b = Set.new
=> #<Set: {}>
irb(main):003> a << 1
=> #<Set: {1}>
irb(main):004> b << 2
=> #<Set: {2}>
irb(main):005> a << b
=> #<Set: {1, #<Set: {2}>}>

Where we expect there probably sth more like:

irb(main):001> a = Set.new
=> #<Set: {}>
irb(main):002> b = Set.new
=> #<Set: {}>
irb(main):003> a << 1
=> #<Set: {1}>
irb(main):004> b << 2
=> #<Set: {2}>
irb(main):005> a.merge(b)
=> #<Set: {1, 2}>
irb(main):006> a
=> #<Set: {1, 2}>
irb(main):007> b
=> #<Set: {2}>

Having nested set later fails in comparison in here:

addresses_to_retain.include?(address)

neo4j-ruby-driver/ruby/neo4j/driver/internal/async/pool/connection_pool_impl.rb

Lines 33 to 35 in 19abaac

	@address_to_pool.each do |address, pool|
	unless addresses_to_retain.include?(address)
	unless pool.busy?

Which later uses == method:

neo4j-ruby-driver/lib/neo4j/driver/internal/bolt_server_address.rb

Lines 73 to 75 in 19abaac

	def ==(other)
	attributes == other&.attributes
	end

If we have there nested Sets Neo4j Driver throws exceptions like:

Should solve issue: #220

[klobuczek]

Thank you Piotrek. Before we release this we need to fix the failing specs, which I know are not due to your change. Do you have any spare time you could dedicate to open source? I promise it would be extremely educative.

[petergebala]

Hi @klobuczek . Sure, I can help. I will have a look at failing specs and try to fix it. If you have anything else that needs attention, please let me know.

[klobuczek]

@petergebala could you have another look at your PR. I think we all missed the fact that the error might have been resolved but the code is not doing what was intended.
The result of the merge is not used. I think it should be merge! to achieve the intended outcome.
CC: @nsauk @JuliaKovtun

[petergebala]

@klobuczek, very likely you are right. I am on PTO till 12th May (don't have access to computer). I will fix it once I return.

[petergebala]

I asked a friend to check. Looks like merg works as well:

What is more patched version we were using in production for months without any issues.