From 8125102ae62abe2c7eb1dc116af2966fe2eb0312 Mon Sep 17 00:00:00 2001
From: Nick Townsend <ntownsend@iparadigms.com>
Date: Fri, 6 Feb 2015 10:31:40 -0800
Subject: [PATCH] Match servers only on hostname not user and port

Servers must now be kept in an array and not a Set as user and port
are part of the hash value for an SSHKit Host. So changing them after
they're in a Set is not advised.

More role_properties tests
---
 lib/capistrano/configuration/server.rb  |  2 +-
 lib/capistrano/configuration/servers.rb | 18 +++++++++---------
 spec/integration/dsl_spec.rb            | 20 ++++++++++++++++++++
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/lib/capistrano/configuration/server.rb b/lib/capistrano/configuration/server.rb
index 9cab82b1..6d3cbd67 100644
--- a/lib/capistrano/configuration/server.rb
+++ b/lib/capistrano/configuration/server.rb
@@ -62,7 +62,7 @@ module Capistrano
       end
 
       def matches?(other)
-        user == other.user && hostname == other.hostname && port == other.port
+        hostname == other.hostname
       end
 
       private
diff --git a/lib/capistrano/configuration/servers.rb b/lib/capistrano/configuration/servers.rb
index 9f2809a9..7bc2fddc 100644
--- a/lib/capistrano/configuration/servers.rb
+++ b/lib/capistrano/configuration/servers.rb
@@ -8,7 +8,14 @@ module Capistrano
       include Enumerable
 
       def add_host(host, properties={})
-        servers.add server(host, properties).with(properties)
+        new_host = Server[host]
+        if server = servers.find { |s| s.matches? new_host }
+          server.user = new_host.user if new_host.user
+          server.port = new_host.port if new_host.port
+          server.with(properties)
+        else
+          servers << new_host.with(properties)
+        end
       end
 
       def add_role(role, hosts, options={})
@@ -50,15 +57,8 @@ module Capistrano
 
       private
 
-      def server(host, properties)
-        new_host = Server[host]
-        new_host.with({user: properties[:user]}) unless properties[:user].nil?
-        new_host.with({port: properties[:port]}) unless properties[:port].nil?
-        servers.find { |server| server.matches? new_host } || new_host
-      end
-
       def servers
-        @servers ||= Set.new
+        @servers ||= []
       end
 
       def extract_options(array)
diff --git a/spec/integration/dsl_spec.rb b/spec/integration/dsl_spec.rb
index a43b4bbc..49653a3d 100644
--- a/spec/integration/dsl_spec.rb
+++ b/spec/integration/dsl_spec.rb
@@ -558,6 +558,26 @@ describe Capistrano::DSL do
         recipient.doit(host, role, props)
       end
     end
+
+    it 'yields the merged properties for multiple roles' do
+      recipient = mock('recipient')
+      recipient.expects(:doit).with('example1.com', :redis, { port: 6379, type: :slave})
+      recipient.expects(:doit).with('example2.com', :redis, { port: 6379, type: :master})
+      recipient.expects(:doit).with('example1.com', :web, { port: 80 })
+      recipient.expects(:doit).with('example2.com', :web, { port: 81 })
+      dsl.role_properties(:redis, :web) do |host, role, props|
+        recipient.doit(host, role, props)
+      end
+    end
+
+    it 'honours a property filter before yielding' do
+      recipient = mock('recipient')
+      recipient.expects(:doit).with('example1.com', :redis, { port: 6379, type: :slave})
+      recipient.expects(:doit).with('example1.com', :web, { port: 80 })
+      dsl.role_properties(:redis, :web, select: :active) do |host, role, props|
+        recipient.doit(host, role, props)
+      end
+    end
   end
 
 end