diff --git a/Gemfile.lock b/Gemfile.lock index 6dbe841..76c1ceb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - batch_api (0.0.1) + batch_api (0.0.3) rails (~> 3.2) GEM diff --git a/app/controllers/batch_api/batch_controller.rb b/app/controllers/batch_api/batch_controller.rb index b19b8f6..3baf458 100644 --- a/app/controllers/batch_api/batch_controller.rb +++ b/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 diff --git a/changelog.md b/changelog.md index fb48853..399e327 100644 --- a/changelog.md +++ b/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 diff --git a/lib/batch_api/operation.rb b/lib/batch_api/operation.rb index 908a27b..c667b58 100644 --- a/lib/batch_api/operation.rb +++ b/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 diff --git a/lib/batch_api/processor.rb b/lib/batch_api/processor.rb new file mode 100644 index 0000000..31bc94b --- /dev/null +++ b/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 diff --git a/lib/batch_api/processor/strategies/sequential.rb b/lib/batch_api/processor/strategies/sequential.rb new file mode 100644 index 0000000..9225b65 --- /dev/null +++ b/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 + diff --git a/lib/batch_api/routing_helper.rb b/lib/batch_api/routing_helper.rb index dfc0f94..62ca7cf 100644 --- a/lib/batch_api/routing_helper.rb +++ b/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 diff --git a/lib/batch_api/version.rb b/lib/batch_api/version.rb index 76b317a..3c2f40f 100644 --- a/lib/batch_api/version.rb +++ b/lib/batch_api/version.rb @@ -1,3 +1,3 @@ module BatchApi - VERSION = "0.0.2" + VERSION = "0.0.3" end diff --git a/spec/controllers/batch_controller_spec.rb b/spec/controllers/batch_controller_spec.rb index e647bea..6e15e19 100644 --- a/spec/controllers/batch_controller_spec.rb +++ b/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 diff --git a/spec/dummy/config/routes.rb b/spec/dummy/config/routes.rb index d6c8152..b545e6b 100644 --- a/spec/dummy/config/routes.rb +++ b/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" diff --git a/spec/integration/batch_spec.rb b/spec/integration/batch_spec.rb index d85b1f5..87f8cd1 100644 --- a/spec/integration/batch_spec.rb +++ b/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 diff --git a/spec/lib/operation_spec.rb b/spec/lib/operation_spec.rb index 8b66a29..fd0e265 100644 --- a/spec/lib/operation_spec.rb +++ b/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 diff --git a/spec/lib/processor_spec.rb b/spec/lib/processor_spec.rb new file mode 100644 index 0000000..6ee67dd --- /dev/null +++ b/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 diff --git a/spec/lib/sequential_spec.rb b/spec/lib/sequential_spec.rb new file mode 100644 index 0000000..7c5fb77 --- /dev/null +++ b/spec/lib/sequential_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe BatchApi::Processor::Strategies::Sequential do + pending "needs a test" +end diff --git a/spec/routing/batch_api_routing_spec.rb b/spec/routing/batch_api_routing_spec.rb index 6da11b5..b37fb5c 100644 --- a/spec/routing/batch_api_routing_spec.rb +++ b/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