Stricter timeout options parsing

lib/http/chainable.rb

@@ -93,19 +93,15 @@ def build_request(*args)
     #   @param [Numeric] global_timeout
     def timeout(options)
       klass, options = case options
-                       when Numeric then [HTTP::Timeout::Global, {global: options}]
-                       when Hash    then [HTTP::Timeout::PerOperation, options.dup]
-                       when :null   then [HTTP::Timeout::Null, {}]
-                       else raise ArgumentError, "Use `.timeout(global_timeout_in_seconds)` or `.timeout(connect: x, write: y, read: z)`."
-
+                       when Numeric then [HTTP::Timeout::Global, {:global_timeout => options}]
+                       when Hash
+                         [HTTP::Timeout::PerOperation, HTTP::Timeout::PerOperation.normalize_options(options)]
+                       when :null then [HTTP::Timeout::Null, {}]
+                       else raise ArgumentError, "Use `.timeout(:null)`, " \
+                                                 "`.timeout(global_timeout_in_seconds)` or " \
+                                                 "`.timeout(connect: x, write: y, read: z)`."
                        end
 
-      %i[global read write connect].each do |k|
-        next unless options.key? k
-
-        options[:"#{k}_timeout"] = options.delete k
-      end
-
       branch default_options.merge(
         timeout_class:   klass,
         timeout_options: options

lib/http/timeout/per_operation.rb

@@ -11,6 +11,28 @@ class PerOperation < Null
       WRITE_TIMEOUT = 0.25
       READ_TIMEOUT = 0.25
 
+      KEYS = %i[read write connect].to_h { |k| [k, :"#{k}_timeout"] }.freeze
+
+      class << self
+        def normalize_options(options)
+          normalized = {}
+          original   = options.dup
+
+          KEYS.each do |short, long|
+            next if !original.key?(short) && !original.key?(long)
+            raise ArgumentError, "can't pass both #{short} and #{long}" if original.key?(short) && original.key?(long)
+
+            normalized[long] = original.key?(long) ? original.delete(long) : original.delete(short)
+            raise ArgumentError, "#{long} must be numeric" unless normalized[long].is_a?(Numeric)
+          end
+
+          raise ArgumentError, "unknown timeout options: #{original.keys.join(', ')}" if original.size.positive?
+          raise ArgumentError, "no timeout options given" if normalized.empty?
+
+          normalized
+        end
+      end
+
       def initialize(*args)
         super
 

spec/lib/http_spec.rb

@@ -335,8 +335,50 @@
       end
 
       it "sets given timeout options" do
+        client = HTTP.timeout :read => 125
+
         expect(client.default_options.timeout_options).
-          to eq read_timeout: 123
+          to eq read_timeout: 125
+      end
+
+      it "sets given timeout options" do
+        client = HTTP.timeout :read_timeout => 321
+
+        expect(client.default_options.timeout_options).
+          to eq :read_timeout => 321
+      end
+
+      it "sets all available options" do
+        client = HTTP.timeout :read => 123, :write => 12, :connect => 1
+
+        expect(client.default_options.timeout_options).
+          to eq(:connect_timeout => 1, :write_timeout => 12, :read_timeout => 123)
+      end
+
+      it "raises an error is empty hash is passed" do
+        expect { HTTP.timeout({}) }
+          .to raise_error(ArgumentError)
+      end
+
+      it "raises if an invalid key is passed" do
+        expect { HTTP.timeout({:timeout => 2}) }
+          .to raise_error(ArgumentError)
+      end
+
+      it "raises if both read and read_timeout is passed" do
+        expect { HTTP.timeout({:read => 2, :read_timeout => 2}) }
+          .to raise_error(ArgumentError)
+      end
+
+      it "raises if a string is passed as read timeout" do
+        expect { HTTP.timeout({:connect => 1, :read => "2"}) }
+          .to raise_error(ArgumentError)
+      end
+
+      it "don't accept string keys" do
+        expect { HTTP.timeout({:connect => 1, "read" => 2}) }
+          .to raise_error(ArgumentError)
+>>>>>>> 0b150cb (Be stricter of allowed values for timeout)
       end
     end
 
@@ -362,6 +404,18 @@
         expect(client.default_options.timeout_options).
           to eq global_timeout: 123
       end
+
+      it "accepts floats argument" do
+        client = HTTP.timeout 2.5
+
+        expect(client.default_options.timeout_options).
+          to eq(:global_timeout => 2.5)
+      end
+
+      it "raises expect if a string is passed" do
+        expect { HTTP.timeout "1" }
+          .to raise_error(ArgumentError)
+      end
     end
   end