From 553a9806012658182bcf86c09f06675dbba09a6e Mon Sep 17 00:00:00 2001 From: "P.S.V.R" Date: Fri, 4 Jul 2014 17:35:53 +0800 Subject: [PATCH 1/7] Use Ruby 2.x in lieu of the out-dated Ruby 1.9 * Update travis config * Use pry since debugger is no longer supported for Ruby 2.1 * Use better group syntax in Gemfile --- .ruby-version | 2 +- .travis.yml | 7 +------ Gemfile | 4 ++-- Gemfile.lock | 9 +-------- 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/.ruby-version b/.ruby-version index 8e17352..00b5647 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -ruby-1.9.3-p448 +ruby-2.1.2 \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index efd10ac..8a8ac56 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,3 @@ language: ruby rvm: - - 1.9.3 - - rbx-19mode - - jruby-19mode -matrix: - allow_failures: - - rvm: jruby-19mode + - 2.1.2 diff --git a/Gemfile b/Gemfile index 86d3189..4baeaed 100644 --- a/Gemfile +++ b/Gemfile @@ -12,9 +12,9 @@ group :development, :test do gem 'guard-rspec' gem 'faker' gem 'timecop' - gem "debugger", platform: :mri + gem 'pry' - if RUBY_PLATFORM =~ /darwin/ + group :darwin do # OS X integration gem "ruby_gntp" gem "rb-fsevent" diff --git a/Gemfile.lock b/Gemfile.lock index a1e58ed..b8b60ae 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -37,13 +37,6 @@ GEM arel (3.0.2) builder (3.0.4) coderay (1.0.8) - columnize (0.3.6) - debugger (1.6.1) - columnize (>= 0.3.1) - debugger-linecache (~> 1.2.0) - debugger-ruby_core_source (~> 1.2.3) - debugger-linecache (1.2.0) - debugger-ruby_core_source (1.2.3) diff-lcs (1.1.3) erubis (2.7.0) faker (1.1.2) @@ -142,10 +135,10 @@ PLATFORMS DEPENDENCIES batch_api! - debugger faker guard guard-rspec + pry rack-contrib rails (~> 3.2) rb-fsevent From 49e980990e28ca680b1b225f16676db1a89292b9 Mon Sep 17 00:00:00 2001 From: bharathpbhat Date: Thu, 2 Oct 2014 11:30:21 -0700 Subject: [PATCH 2/7] CMA-3391 batch api should not try to parse when response is blank --- .../internal_middleware/decode_json_body.rb | 2 +- .../internal_middleware/decode_json_body_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/batch_api/internal_middleware/decode_json_body.rb b/lib/batch_api/internal_middleware/decode_json_body.rb index 0d49aa9..8b59d7d 100644 --- a/lib/batch_api/internal_middleware/decode_json_body.rb +++ b/lib/batch_api/internal_middleware/decode_json_body.rb @@ -10,7 +10,7 @@ def initialize(app) def call(env) @app.call(env).tap do |result| - result.body = MultiJson.load(result.body) if should_decode?(result) + result.body = MultiJson.load(result.body) if (should_decode?(result) && !result.body.blank?) end end diff --git a/spec/lib/internal_middleware/decode_json_body_spec.rb b/spec/lib/internal_middleware/decode_json_body_spec.rb index c724338..bccf5f4 100644 --- a/spec/lib/internal_middleware/decode_json_body_spec.rb +++ b/spec/lib/internal_middleware/decode_json_body_spec.rb @@ -12,6 +12,13 @@ [MultiJson.dump(json)] ]) } + let(:blank_result) { + BatchApi::Response.new([ + 200, + {"Content-Type" => "application/json"}, + [''] + ]) + } describe "#call" do context "for json results" do @@ -33,5 +40,12 @@ decoder.call(env).body.should == MultiJson.dump(json) end end + + context "for blank responses" do + it "doesn't try to parse" do + result.body = '' + decoder.call(env).body.should == '' + end + end end end From 99f48aa1dda2121b5a149c73748db484e58c62a3 Mon Sep 17 00:00:00 2001 From: bharathpbhat Date: Thu, 2 Oct 2014 11:31:08 -0700 Subject: [PATCH 3/7] CMA-3391 remove unnecessary code --- spec/lib/internal_middleware/decode_json_body_spec.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/spec/lib/internal_middleware/decode_json_body_spec.rb b/spec/lib/internal_middleware/decode_json_body_spec.rb index bccf5f4..1374850 100644 --- a/spec/lib/internal_middleware/decode_json_body_spec.rb +++ b/spec/lib/internal_middleware/decode_json_body_spec.rb @@ -12,13 +12,6 @@ [MultiJson.dump(json)] ]) } - let(:blank_result) { - BatchApi::Response.new([ - 200, - {"Content-Type" => "application/json"}, - [''] - ]) - } describe "#call" do context "for json results" do From 1e86286045d21abf62b42b5b5445f2b5db5ee149 Mon Sep 17 00:00:00 2001 From: Trung Pham Date: Mon, 16 Feb 2015 11:17:40 -0800 Subject: [PATCH 4/7] upgrade to ruby 2.2.0 --- .ruby-version | 2 +- .travis.yml | 2 +- Gemfile | 1 + Gemfile.lock | 8 ++++++-- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.ruby-version b/.ruby-version index 00b5647..d538d61 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -ruby-2.1.2 \ No newline at end of file +ruby-2.2.0 \ No newline at end of file diff --git a/.travis.yml b/.travis.yml index 8a8ac56..457cdc9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,3 +1,3 @@ language: ruby rvm: - - 2.1.2 + - 2.2.0 diff --git a/Gemfile b/Gemfile index 4baeaed..03d6807 100644 --- a/Gemfile +++ b/Gemfile @@ -13,6 +13,7 @@ group :development, :test do gem 'faker' gem 'timecop' gem 'pry' + gem 'test-unit' group :darwin do # OS X integration diff --git a/Gemfile.lock b/Gemfile.lock index b8b60ae..7c124c1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -52,7 +52,7 @@ GEM hike (1.2.1) i18n (0.6.1) journey (1.0.4) - json (1.7.5) + json (1.8.2) listen (0.5.3) lumberjack (1.0.2) mail (2.4.4) @@ -64,6 +64,7 @@ GEM mime-types (1.19) multi_json (1.3.6) polyglot (0.3.3) + power_assert (0.2.2) pry (0.9.10) coderay (~> 1.0.5) method_source (~> 0.8) @@ -121,7 +122,9 @@ GEM hike (~> 1.2) rack (~> 1.0) tilt (~> 1.1, != 1.3.0) - sqlite3 (1.3.6) + sqlite3 (1.3.10) + test-unit (3.0.9) + power_assert thor (0.16.0) tilt (1.3.3) timecop (0.5.3) @@ -147,4 +150,5 @@ DEPENDENCIES ruby_gntp sinatra sqlite3 + test-unit timecop From 66f6815ce637e6dec29b190339db7657a294f2d7 Mon Sep 17 00:00:00 2001 From: Trung Pham Date: Mon, 16 Feb 2015 15:16:13 -0800 Subject: [PATCH 5/7] integrate parallel processing --- Gemfile.lock | 6 +++ batch_api.gemspec | 3 +- lib/batch_api/configuration.rb | 1 + lib/batch_api/internal_middleware.rb | 2 +- lib/batch_api/parallel_actor.rb | 14 +++++++ lib/batch_api/processor.rb | 9 +---- lib/batch_api/processor/parallel.rb | 31 ++++++++++++++++ lib/batch_api/rack_middleware.rb | 1 + spec/integration/rails_spec.rb | 3 +- spec/integration/shared_examples.rb | 4 +- spec/integration/sinatra_integration_spec.rb | 3 +- spec/lib/processor/parallel_spec.rb | 39 ++++++++++++++++++++ spec/lib/processor_spec.rb | 13 ++++--- spec/spec_helper.rb | 1 + 14 files changed, 110 insertions(+), 20 deletions(-) create mode 100644 lib/batch_api/parallel_actor.rb create mode 100644 lib/batch_api/processor/parallel.rb create mode 100644 spec/lib/processor/parallel_spec.rb diff --git a/Gemfile.lock b/Gemfile.lock index 7c124c1..2a1cefd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,6 +2,7 @@ PATH remote: . specs: batch_api (0.2.1) + celluloid middleware GEM @@ -36,6 +37,8 @@ GEM multi_json (~> 1.0) arel (3.0.2) builder (3.0.4) + celluloid (0.16.0) + timers (~> 4.0.0) coderay (1.0.8) diff-lcs (1.1.3) erubis (2.7.0) @@ -50,6 +53,7 @@ GEM guard (>= 1.1) rspec (~> 2.11) hike (1.2.1) + hitimes (1.2.2) i18n (0.6.1) journey (1.0.4) json (1.8.2) @@ -128,6 +132,8 @@ GEM thor (0.16.0) tilt (1.3.3) timecop (0.5.3) + timers (4.0.1) + hitimes treetop (1.4.11) polyglot polyglot (>= 0.3.1) diff --git a/batch_api.gemspec b/batch_api.gemspec index f374eb7..f97a32e 100644 --- a/batch_api.gemspec +++ b/batch_api.gemspec @@ -18,7 +18,8 @@ Gem::Specification.new do |s| s.test_files = Dir["spec/**/*"] s.add_runtime_dependency("middleware") - + s.add_runtime_dependency("celluloid") + s.add_development_dependency("rails", "~> 3.2") s.add_development_dependency("sinatra") s.add_development_dependency("rspec") diff --git a/lib/batch_api/configuration.rb b/lib/batch_api/configuration.rb index fee2a1f..d15a0d4 100644 --- a/lib/batch_api/configuration.rb +++ b/lib/batch_api/configuration.rb @@ -19,6 +19,7 @@ module BatchApi verb: :post, endpoint: "/batch", limit: 50, + parallel_size: 10, batch_middleware: InternalMiddleware::DEFAULT_BATCH_MIDDLEWARE, operation_middleware: InternalMiddleware::DEFAULT_OPERATION_MIDDLEWARE } diff --git a/lib/batch_api/internal_middleware.rb b/lib/batch_api/internal_middleware.rb index d33d2cd..075f26f 100644 --- a/lib/batch_api/internal_middleware.rb +++ b/lib/batch_api/internal_middleware.rb @@ -1,5 +1,6 @@ require 'middleware' require 'batch_api/processor/sequential' +require 'batch_api/processor/parallel' require 'batch_api/processor/executor' require 'batch_api/internal_middleware/decode_json_body' require 'batch_api/internal_middleware/response_filter' @@ -68,7 +69,6 @@ def self.batch_stack(processor) Middleware::Builder.new do # evaluate these in the context of the middleware stack self.instance_eval &BatchApi.config.batch_middleware - # for now, everything's sequential, but that will change use processor.strategy end end diff --git a/lib/batch_api/parallel_actor.rb b/lib/batch_api/parallel_actor.rb new file mode 100644 index 0000000..b9e4e3e --- /dev/null +++ b/lib/batch_api/parallel_actor.rb @@ -0,0 +1,14 @@ +require 'celluloid' + +module BatchApi + class ParallelActor + include ::Celluloid + + def run(env) + middleware = InternalMiddleware.operation_stack + middleware.call(env).tap {|r| env.delete(:op) } + end + end +end + + \ No newline at end of file diff --git a/lib/batch_api/processor.rb b/lib/batch_api/processor.rb index 40738fd..f37b112 100644 --- a/lib/batch_api/processor.rb +++ b/lib/batch_api/processor.rb @@ -28,7 +28,7 @@ def initialize(request, app) # provided in BatchApi setup and the request. # Currently only Sequential is supported. def strategy - BatchApi::Processor::Sequential + @request.params["sequential"] ? BatchApi::Processor::Sequential : BatchApi::Processor::Parallel end # Public: run the batch operations according to the appropriate strategy. @@ -95,18 +95,11 @@ def self.operation_klass end # Internal: Processes any other provided options for validity. - # Currently, the :sequential option is REQUIRED (until parallel - # implementation is created). # # options - an options hash # - # Raises Errors::BadOptionError if sequential is not provided. - # # Returns the valid options hash. def process_options - unless @request.params["sequential"] - raise Errors::BadOptionError, "Sequential flag is currently required" - end @request.params end end diff --git a/lib/batch_api/processor/parallel.rb b/lib/batch_api/processor/parallel.rb new file mode 100644 index 0000000..551b3bd --- /dev/null +++ b/lib/batch_api/processor/parallel.rb @@ -0,0 +1,31 @@ +require 'batch_api/parallel_actor' +module BatchApi + class Processor + class Parallel + + # Public: initialize with the app. + def initialize(app) + @app = app + end + + # Public: execute all operations sequentially. + # + # ops - a set of BatchApi::Operations + # options - a set of options + # + # Returns an array of BatchApi::Response objects. + def call(env) + futures = env[:ops].map do |op| + _env = BatchApi::Utils.deep_dup(env) + _env[:op] = op + Celluloid::Actor[:batch_parallel_pool].future.run(_env) + end + futures.map do |future| + future.value + end + end + + end + end +end + diff --git a/lib/batch_api/rack_middleware.rb b/lib/batch_api/rack_middleware.rb index 87a0177..52a68fc 100644 --- a/lib/batch_api/rack_middleware.rb +++ b/lib/batch_api/rack_middleware.rb @@ -3,6 +3,7 @@ class RackMiddleware def initialize(app, &block) @app = app yield BatchApi.config if block + Celluloid::Actor[:batch_parallel_pool] = BatchApi::ParallelActor.pool(size: BatchApi.config.parallel_size) end def call(env) diff --git a/spec/integration/rails_spec.rb b/spec/integration/rails_spec.rb index 29dda87..db90fb7 100644 --- a/spec/integration/rails_spec.rb +++ b/spec/integration/rails_spec.rb @@ -6,5 +6,6 @@ BatchApi.stub(:rails?).and_return(true) end - it_should_behave_like "integrating with a server" + it_should_behave_like "integrating with a server", true + it_should_behave_like "integrating with a server", false end diff --git a/spec/integration/shared_examples.rb b/spec/integration/shared_examples.rb index 7ba6b20..b8c4c7a 100644 --- a/spec/integration/shared_examples.rb +++ b/spec/integration/shared_examples.rb @@ -19,7 +19,7 @@ end end -shared_examples_for "integrating with a server" do +shared_examples_for "integrating with a server" do |sequential| def headerize(hash) Hash[hash.map do |k, v| ["HTTP_#{k.to_s.upcase}", v.to_s] @@ -153,7 +153,7 @@ def headerize(hash) failed_silent_request, get_by_default_request ], - sequential: true + sequential: sequential }.to_json, "CONTENT_TYPE" => "application/json" end diff --git a/spec/integration/sinatra_integration_spec.rb b/spec/integration/sinatra_integration_spec.rb index 3b30254..d391777 100644 --- a/spec/integration/sinatra_integration_spec.rb +++ b/spec/integration/sinatra_integration_spec.rb @@ -10,5 +10,6 @@ def app SinatraApp end - it_should_behave_like "integrating with a server" + it_should_behave_like "integrating with a server", true + it_should_behave_like "integrating with a server", false end diff --git a/spec/lib/processor/parallel_spec.rb b/spec/lib/processor/parallel_spec.rb new file mode 100644 index 0000000..d66f2f3 --- /dev/null +++ b/spec/lib/processor/parallel_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe BatchApi::Processor::Parallel do + + let(:app) { stub("app", call: stub) } + let(:parallel) { BatchApi::Processor::Parallel.new(app) } + + describe "#call" do + let(:call_results) { 3.times.collect {|i| stub("called #{i}") } } + let(:env) { { + ops: 3.times.collect {|i| stub("op #{i}") } + } } + let(:op_middleware) { stub("middleware", call: {}) } + + before :each do + BatchApi::InternalMiddleware. + stub(:operation_stack).and_return(op_middleware) + op_middleware.stub(:call).and_return(*call_results) + end + + it "creates an operation middleware stack and calls it for each op" do + env[:ops].each {|op| + op_middleware.should_receive(:call). + with(hash_including(op: op)) + } + parallel.call(env) + end + + it "includes the rest of the env in the calls" do + op_middleware.should_receive(:call). + with(hash_including(env)).exactly(3).times + parallel.call(env) + end + + it "returns the results of the calls" do + parallel.call(env).should =~ call_results + end + end +end \ No newline at end of file diff --git a/spec/lib/processor_spec.rb b/spec/lib/processor_spec.rb index 9830daa..7b506b8 100644 --- a/spec/lib/processor_spec.rb +++ b/spec/lib/processor_spec.rb @@ -59,12 +59,6 @@ end context "error conditions" do - it "(currently) throws an error if sequential is not true" do - request.params.delete("sequential") - expect { - BatchApi::Processor.new(request, app) - }.to raise_exception(BatchApi::Errors::BadOptionError) - end it "raise a OperationLimitExceeded error if too many ops provided" do ops = (BatchApi.config.limit + 1).to_i.times.collect {|i| i} @@ -91,6 +85,13 @@ it "returns BatchApi::Processor::Sequential" do processor.strategy.should == BatchApi::Processor::Sequential end + it "returns BatchApi::Processor::Parallel" do + request = Rack::Request.new(env).tap do |r| + r.stub(:params).and_return({}.merge("ops" => ops).merge("sequential" => false)) + end + processor = BatchApi::Processor.new(request, app) + processor.strategy.should == BatchApi::Processor::Parallel + end end describe "#execute!" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 7fbf890..9dc714a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -20,6 +20,7 @@ RSpec.configure do |config| config.before :each do BatchApi.config.limit = 20 + BatchApi.config.parallel_size = 11 BatchApi.config.endpoint = "/batch" BatchApi.config.verb = :post From e73a63c167d388d6d41087d03bf53f8c5d55d6d5 Mon Sep 17 00:00:00 2001 From: Trung Pham Date: Mon, 16 Feb 2015 16:18:39 -0800 Subject: [PATCH 6/7] integrate parallel processing --- lib/batch_api/processor/parallel.rb | 5 ++++- lib/batch_api/rack_middleware.rb | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/batch_api/processor/parallel.rb b/lib/batch_api/processor/parallel.rb index 551b3bd..e12000f 100644 --- a/lib/batch_api/processor/parallel.rb +++ b/lib/batch_api/processor/parallel.rb @@ -18,13 +18,16 @@ def call(env) futures = env[:ops].map do |op| _env = BatchApi::Utils.deep_dup(env) _env[:op] = op - Celluloid::Actor[:batch_parallel_pool].future.run(_env) + self.class.get_actor_pool.future.run(_env) end futures.map do |future| future.value end end + def self.get_actor_pool + Celluloid::Actor[:batch_parallel_pool] ||= BatchApi::ParallelActor.pool(size: BatchApi.config.parallel_size) + end end end end diff --git a/lib/batch_api/rack_middleware.rb b/lib/batch_api/rack_middleware.rb index 52a68fc..87a0177 100644 --- a/lib/batch_api/rack_middleware.rb +++ b/lib/batch_api/rack_middleware.rb @@ -3,7 +3,6 @@ class RackMiddleware def initialize(app, &block) @app = app yield BatchApi.config if block - Celluloid::Actor[:batch_parallel_pool] = BatchApi::ParallelActor.pool(size: BatchApi.config.parallel_size) end def call(env) From 02141f5cb2aad34b28a7919e3a4ea8ceaba35cfc Mon Sep 17 00:00:00 2001 From: Trung Pham Date: Mon, 16 Feb 2015 16:29:56 -0800 Subject: [PATCH 7/7] integrate parallel processing. use separate sql connection --- lib/batch_api/parallel_actor.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/batch_api/parallel_actor.rb b/lib/batch_api/parallel_actor.rb index b9e4e3e..b361b87 100644 --- a/lib/batch_api/parallel_actor.rb +++ b/lib/batch_api/parallel_actor.rb @@ -6,7 +6,14 @@ class ParallelActor def run(env) middleware = InternalMiddleware.operation_stack - middleware.call(env).tap {|r| env.delete(:op) } + + if defined?(ActiveRecord) + ActiveRecord::Base.connection_pool.with_connection do + middleware.call(env).tap {|r| env.delete(:op) } + end + else + middleware.call(env).tap {|r| env.delete(:op) } + end end end end