Refactor processing flow

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    batch_api (0.0.1)
+    batch_api (0.0.3)
       rails (~> 3.2)
 
 GEM

app/controllers/batch_api/batch_controller.rb

@@ -3,8 +3,7 @@
 module BatchApi
   class BatchController < ::ApplicationController
     def batch
-      ops = params[:ops].map {|o| BatchApi::Operation.new(o, request.env)}
-      render :json => ops.map(&:execute)
+      render :json => BatchApi::Processor.new(params[:ops], request.env, params).execute!
     end
   end
 end

changelog.md

@@ -1,5 +1,12 @@
+v0.0.3
+* Encapsulate processing into a Processor module
+* Prepare for parallel processing in the future
+* Add specific errors
+* Allow controlling the routing target
+
 v0.0.2
 * Add config module
+* Add options for operation limit, endpoint, and verb
 
 v0.0.1
 * Initial build

lib/batch_api/operation.rb

@@ -3,6 +3,8 @@
 module BatchApi
   # Public: an individual batch operation.
   class Operation
+    class MalformedOperationError < ArgumentError; end
+
     attr_accessor :method, :url, :params, :headers
     attr_accessor :env, :result
 
@@ -17,6 +19,10 @@ def initialize(op, base_env)
       @params = op[:params]
       @headers = op[:headers]
 
+      raise MalformedOperationError,
+        "BatchAPI operation must include method (received #{@method.inspect}) " +
+        "and url (received #{@url.inspect})" unless @method && @url
+
       # deep_dup to avoid unwanted changes across requests
       @env = base_env.deep_dup
     end

lib/batch_api/processor.rb

@@ -0,0 +1,81 @@
+require 'batch_api/processor/strategies/sequential'
+
+module BatchApi
+  class Processor
+    # Raised when a user provides more Batch API requests than a service
+    # allows.
+    class OperationLimitExceeded < StandardError; end
+    # Raised if a provided option is invalid.
+    class BadOptionError < StandardError; end
+
+    attr_reader :ops, :options
+
+    # Public: create a new Processor.
+    #
+    # ops - an array of operations hashes
+    # options - any other options
+    #
+    # Raises OperationLimitExceeded if more operations are requested than
+    # allowed by the BatchApi configuration.
+    # Raises BadOptionError if other provided options are invalid.
+    # Raises ArgumentError if no operations are provided (nil or []).
+    #
+    # Returns the new Processor instance.
+    def initialize(ops, env, options = {})
+      raise ArgumentError, "No operations provided" if ops.blank?
+
+      @env = env
+      @ops = self.process_ops(ops)
+      @options = self.process_options(options)
+    end
+
+    # Public: the processing strategy to use, based on the options
+    # provided in BatchApi setup and the request.
+    # Currently only Sequential is supported.
+    def strategy
+      BatchApi::Processor::Strategies::Sequential
+    end
+
+    # Public: run the batch operations according to the appropriate strategy.
+    #
+    # Returns a set of BatchResponses
+    def execute!
+      strategy.execute!(@ops, @options)
+    end
+
+    protected
+
+    # Internal: Validate that an allowable number of operations have been
+    # provided, and turn them into BatchApi::Operation objects.
+    #
+    # ops - a series of operations
+    #
+    # Raises OperationLimitExceeded if more operations are requested than
+    # allowed by the BatchApi configuration.
+    #
+    # Returns an array of BatchApi::Operation objects
+    def process_ops(ops)
+      if ops.length > BatchApi.config.limit
+        raise OperationLimitExceeded,
+          "Only #{BatchApi.config.limit} operations can be submitted at once, " +
+          "#{ops.length} were provided"
+      else
+        ops.map do |op|
+          BatchApi::Operation.new(op, @env)
+        end
+      end
+    end
+
+    # Internal: Processes any other provided options for validity.
+    # Currently, the :sequential option is REQUIRED (until parallel
+    # implementation is created).
+    #
+    # options - an options hash
+    #
+    # Returns the valid options hash.
+    def process_options(options)
+      raise BadOptionError, "Sequential flag is currently required" unless options[:sequential]
+      options
+    end
+  end
+end

lib/batch_api/processor/strategies/sequential.rb

@@ -0,0 +1,18 @@
+module BatchApi
+  class Processor
+    module Strategies
+      module Sequential
+        # Public: execute all operations sequentially.
+        #
+        # ops - a set of BatchApi::Operations
+        # options - a set of options
+        #
+        # Returns an array of BatchApi::Response objects.
+        def self.execute!(ops, options = {})
+          ops.map(&:execute)
+        end
+      end
+    end
+  end
+end
+

lib/batch_api/routing_helper.rb

@@ -2,11 +2,13 @@ module BatchApi
   module RoutingHelper
     DEFAULT_VERB = :post
     DEFAULT_ENDPOINT = "/batch"
+    DEFAULT_TARGET = "batch_api/batch#batch"
 
     def batch_api(options = {})
       endpoint = options.delete(:endpoint) || DEFAULT_ENDPOINT
       verb = options.delete(:via) || DEFAULT_VERB
-      match({endpoint => "batch_api/batch#batch", via: verb}.merge(options))
+      target = options.delete(:target) || DEFAULT_TARGET
+      match({endpoint => target, via: verb}.merge(options))
     end
   end
 end

lib/batch_api/version.rb

@@ -1,3 +1,3 @@
 module BatchApi
-  VERSION = "0.0.2"
+  VERSION = "0.0.3"
 end

spec/controllers/batch_controller_spec.rb

@@ -3,25 +3,19 @@
 
 describe BatchApi::BatchController do
   describe "#batch" do
-    it "creates batch ops for each operation using the request environment" do
-      ops = 10.times.collect {|i| ({"operation" => i.to_s}) }
-      ops.each do |o|
-        BatchApi::Operation.should_receive(:new).with(o, request.env).and_return(stub(:execute => ""))
-      end
-
-      xhr :post, :batch, ops: ops
-    end
-
-    it "returns the resultof the batch operation's execution as JSON and in order" do
+    it "returns the result of the batch operation's execution as JSON and in order" do
+      env = request.env
       ops = 10.times.collect {|i| {"operation" => i.to_s} }
-      ops.each do |o|
-        BatchApi::Operation.should_receive(:new).and_return(stub(:execute => o["operation"]))
-      end
+      params = {ops: ops, sequential: true}
+      result = ops.map(&:to_s)
+
+      processor = stub("processor", :execute! => result)
+      BatchApi::Processor.should_receive(:new).with(ops, request.env, hash_including(params)).and_return(processor)
 
-      xhr :post, :batch, ops: ops
+      xhr :post, :batch, params
       json = JSON.parse(response.body)
       ops.each_with_index do |o, i|
-        json[i].should == i.to_s
+        json[i].should == ops[i].to_s
       end
     end
   end

spec/dummy/config/routes.rb

@@ -3,6 +3,7 @@
   # Test the Batch API in two formats: default and custom
   batch_api
   batch_api endpoint: "/foo/bar", via: "report"
+  batch_api endpoint: "/foo/baz", target: "endpoints#get"
 
   post "/endpoint" => "endpoints#post"
   get "/endpoint" => "endpoints#get"

spec/integration/batch_spec.rb

@@ -50,7 +50,7 @@ def headerize(hash)
   } }
 
   before :each do
-    xhr :post, "/batch", {ops: [get_request, post_request]}.to_json, "CONTENT_TYPE" => "application/json"
+    xhr :post, "/batch", {ops: [get_request, post_request], sequential: true}.to_json, "CONTENT_TYPE" => "application/json"
   end
 
   it "returns a 200" do

spec/lib/operation_spec.rb

@@ -45,6 +45,23 @@
         end
       end
     end
+
+    it "raises an MalformedOperationError if method or URL are missing" do
+      no_method = op_params.dup.tap {|o| o.delete(:method) }
+      expect {
+        BatchApi::Operation.new(no_method, env)
+      }.to raise_exception(BatchApi::Operation::MalformedOperationError)
+
+      no_url = op_params.dup.tap {|o| o.delete(:url) }
+      expect {
+        BatchApi::Operation.new(no_url, env)
+      }.to raise_exception(BatchApi::Operation::MalformedOperationError)
+
+      nothing = op_params.dup.tap {|o| o.delete(:url); o.delete(:method) }
+      expect {
+        BatchApi::Operation.new(nothing, env)
+      }.to raise_exception(BatchApi::Operation::MalformedOperationError)
+    end
   end
 
   describe "#identify_routing" do

spec/lib/processor_spec.rb

@@ -0,0 +1,70 @@
+require 'spec_helper'
+
+describe BatchApi::Processor do
+  it "provides a OperationLimitExceeded error" do
+    BatchApi::Processor::OperationLimitExceeded.superclass.should == StandardError
+  end
+
+  it "provides a OperationLimitExceeded error" do
+    BatchApi::Processor::BadOptionError.superclass.should == StandardError
+  end
+
+  let(:ops) { [ {url: "/endpoint", method: "get"} ] }
+  let(:options) { { sequential: true } }
+  let(:env) { {} }
+  let(:processor) { BatchApi::Processor.new(ops, env, options) }
+
+  describe "#initialize" do
+    # this may be brittle...consider refactoring?
+    it "turns the ops provided into BatchApi::Operations stored at #ops" do
+      # simulate receiving several operations
+      operation_objects = 3.times.collect { stub("operation object") }
+      operation_params = 3.times.collect do |i|
+        stub("raw operation").tap do |o|
+          BatchApi::Operation.should_receive(:new).with(o, anything).and_return(operation_objects[i])
+        end
+      end
+
+      BatchApi::Processor.new(operation_params, env, options).ops.should == operation_objects
+    end
+
+    it "makes the options available" do
+      BatchApi::Processor.new(ops, env, options).options.should == options
+    end
+
+    context "error conditions" do
+      it "(currently) throws an error if sequential is not true" do
+        expect { BatchApi::Processor.new(ops, env, {}) }.to raise_exception(BatchApi::Processor::BadOptionError)
+      end
+
+      it "raise a OperationLimitExceeded error if too many ops provided" do
+        ops = (BatchApi.config.limit + 1).times.collect {|i| i}
+        expect { BatchApi::Processor.new(ops, env, options) }.to raise_exception(BatchApi::Processor::OperationLimitExceeded)
+      end
+
+      it "raises an ArgumentError if operations.blank?" do
+        expect { BatchApi::Processor.new(nil, env, options) }.to raise_exception(ArgumentError)
+        expect { BatchApi::Processor.new(nil, env, []) }.to raise_exception(ArgumentError)
+      end
+    end
+
+    describe "#strategy" do
+      it "returns BatchApi::Processor::Strategies::Sequential" do
+        processor.strategy.should == BatchApi::Processor::Strategies::Sequential
+      end
+    end
+
+    describe "#execute!" do
+      it "executes on the provided strategy" do
+        processor.strategy.should_receive(:execute!).with(processor.ops, processor.options)
+        processor.execute!
+      end
+
+      it "returns the result of the strategy" do
+        stubby = stub
+        processor.strategy.stub(:execute!).and_return(stubby)
+        processor.execute!.should == stubby
+      end
+    end
+  end
+end

spec/lib/sequential_spec.rb

@@ -0,0 +1,5 @@
+require 'spec_helper'
+
+describe BatchApi::Processor::Strategies::Sequential do
+  pending "needs a test"
+end

spec/routing/batch_api_routing_spec.rb

@@ -9,5 +9,10 @@
   it "draws the route using custom values" do
     expect("report" => "/foo/bar").to route_to("batch_api/batch#batch")
   end
+
+  it "draws the routes using a custom target" do
+    # almost like a generic router!
+    expect("post" => "/foo/baz").to route_to("endpoints#get")
+  end
 end