From 1191298b4024f5c146c91e4f4a30a8c9a062b036 Mon Sep 17 00:00:00 2001
From: Marc-Andre Lafortune <github@marc-andre.ca>
Date: Tue, 24 Feb 2009 17:25:34 -0500
Subject: [PATCH] - Options hash passed when connecting is not modified anymore
 - Connection::Option simplified

---
 lib/aws/s3/connection.rb | 63 ++++++----------------------------------
 lib/aws/s3/exceptions.rb |  2 +-
 test/connection_test.rb  |  3 +-
 3 files changed, 11 insertions(+), 57 deletions(-)

diff --git a/lib/aws/s3/connection.rb b/lib/aws/s3/connection.rb
index 1c4557b..3429486 100644
--- a/lib/aws/s3/connection.rb
+++ b/lib/aws/s3/connection.rb
@@ -249,73 +249,28 @@ module AWS
       end
         
       class Options < Hash #:nodoc:
-        class << self
-          def valid_options
-            [:access_key_id, :secret_access_key, :server, :port, :use_ssl, :persistent, :proxy]
-          end
-        end
+        VALID_OPTIONS = [:access_key_id, :secret_access_key, :server, :port, :use_ssl, :persistent, :proxy].freeze
         
-        attr_reader :options
         def initialize(options = {})
           super()
-          @options = options
-          validate!
-          extract_proxy_settings!
-          extract_persistent!
-          extract_server!
-          extract_port!
-          extract_remainder!
+          validate(options)
+          replace(:server => DEFAULT_HOST, :port => (options[:use_ssl] ? 443 : 80))
+          merge!(options)
         end
-        
+
         def connecting_through_proxy?
           !self[:proxy].nil?
         end
         
         def proxy_settings
-          proxy_setting_keys.map do |proxy_key| 
-            self[:proxy][proxy_key]
-          end
+          self[:proxy].values_at(:host, :port, :user, :password)
         end
         
         private
-          def proxy_setting_keys
-            [:host, :port, :user, :password]
-          end
-          
-          def missing_proxy_settings?
-            !self[:proxy].keys.include?(:host)
-          end
-          
-          def extract_persistent!
-            self[:persistent] = options.has_key?(:persitent) ? options[:persitent] : false
-          end
-          
-          def extract_proxy_settings!
-            self[:proxy] = options.delete(:proxy) if options.include?(:proxy)
-            validate_proxy_settings!
-          end
-          
-          def extract_server!
-            self[:server] = options.delete(:server) || DEFAULT_HOST
-          end
-
-          def extract_port!
-            self[:port] = options.delete(:port) || (options[:use_ssl] ? 443 : 80)
-          end
-          
-          def extract_remainder!
-            update(options)
-          end
-          
-          def validate!
-            invalid_options = options.keys.select {|key| !self.class.valid_options.include?(key)}
+          def validate(options)
+            invalid_options = options.keys - VALID_OPTIONS
             raise InvalidConnectionOption.new(invalid_options) unless invalid_options.empty?
-          end
-          
-          def validate_proxy_settings!
-            if connecting_through_proxy? && missing_proxy_settings?
-              raise ArgumentError, "Missing proxy settings. Must specify at least :host."
-            end
+            raise ArgumentError, "Missing proxy settings. Must specify at least :host." if options[:proxy] && !options[:proxy][:host]
           end
       end
     end
diff --git a/lib/aws/s3/exceptions.rb b/lib/aws/s3/exceptions.rb
index 6e5a09c..9dab1a2 100644
--- a/lib/aws/s3/exceptions.rb
+++ b/lib/aws/s3/exceptions.rb
@@ -58,7 +58,7 @@ module AWS
     class InvalidConnectionOption < InvalidOption
       def initialize(invalid_options)
         message = "The following connection options are invalid: #{invalid_options.join(', ')}. "    +
-                  "The valid connection options are: #{Connection::Options.valid_options.join(', ')}."
+                  "The valid connection options are: #{Connection::Options::VALID_OPTIONS.join(', ')}."
         super(message)
       end
     end
diff --git a/test/connection_test.rb b/test/connection_test.rb
index d044de4..5098881 100644
--- a/test/connection_test.rb
+++ b/test/connection_test.rb
@@ -3,7 +3,7 @@ require File.dirname(__FILE__) + '/test_helper'
 class ConnectionTest < Test::Unit::TestCase
   attr_reader :keys
   def setup
-    @keys = {:access_key_id => '123', :secret_access_key => 'abc'}
+    @keys = {:access_key_id => '123', :secret_access_key => 'abc'}.freeze
   end
   
   def test_creating_a_connection
@@ -207,7 +207,6 @@ class ConnectionOptionsTest < Test::Unit::TestCase
   private
     def assert_key_transfered(key, value, options)
       assert_equal value, options[key]
-      assert !options.instance_variable_get('@options').has_key?(key)
     end
       
     def generate_options(options = {})