Merge pull request postrank-labs#54 from wicz/master

Class-based map never gets executed

Author: dj2

examples/rack_routes.rb

@@ -27,8 +27,11 @@ def response(env)
 end
 
 classHola < Goliath::API
+useGoliath::Rack::Params
+useGoliath::Rack::Validation::RequiredParam,{:key=>"foo"}
+
 defresponse(env)
-[200,{},"¡hola!"]
+[200,{},"hola!"]
 end
 end
 
@@ -55,7 +58,6 @@ class RackRoutes < Goliath::API
 
 map"/hola"do
 useGoliath::Rack::Validation::RequestMethod,%w(GET)
-
 runHola.new
 end
 
@@ -64,4 +66,9 @@ class RackRoutes < Goliath::API
 map'/'do
 runProc.new{ |env| [404,{"Content-Type"=>"text/html"},["Try /version /hello_world, /bonjour, or /hola"]]}
 end
+
+# You must use either maps or response, but never both!
+defresponse(env)
+raiseRuntimeException.new("#response is ignored when using maps, so this exception won't raise. See spec/integration/rack_routes_spec.")
+end
 end

lib/goliath/api.rb

@@ -83,6 +83,10 @@ def maps
 @maps ||= []
 end
 
+defmaps?
+ !maps.empty?
+end
+
 # Specify a router map to be used by the API
 #
 # @example

lib/goliath/rack/builder.rb

@@ -12,26 +12,17 @@ def self.build(klass, api)
 use(mw_klass, *args, &blk)
 end
 
-# If you use map you can't use run as
-# the rack builder will blowup.
-ifklass.maps.empty?
-runapi
-else
-klass.maps.eachdo |(path,route_klass,blk)|
-ifroute_klass
-map(path)do
-route_klass.middlewares.eachdo |mw_klass,args,blk|
-use(mw_klass, *args, &blk)
-end
-runklass.new
-end
-else
-map(path, &blk)
-end
-end
+klass.maps.eachdo |path,route_klass,blk|
+blk ||= Proc.new{
+runBuilder.build(route_klass,route_klass.new)
+}
+map(path, &blk)
 end
+
+runapiunlessklass.maps?
 end
 end
+
 end
 end
 end

spec/integration/rack_routes_spec.rb

@@ -5,41 +5,67 @@
 describeRackRoutesdo
 let(:err){Proc.new{fail"API request failed"}}
 
-it'routes to the correct API'do
-with_api(RackRoutes)do
-get_request({:path=>'/bonjour'},err)do |c|
-c.response_header.status.should == 200
-c.response.should == 'bonjour!'
+context"when using maps"do
+
+it"ignores #response"do
+expect{
+with_api(RackRoutes)do
+get_request({:path=>'/'},err){}
+end
+}.to_notraise_error
+end
+
+it'fallback not found to /'do
+with_api(RackRoutes)do
+get_request({:path=>'/donkey'},err)do |cb|
+cb.response_header.status.should == 404
+cb.response.should == 'Try /version /hello_world, /bonjour, or /hola'
+end
 end
 end
-end
 
-it'routes to the default'do
-with_api(RackRoutes)do
-get_request({:path=>'/donkey'},err)do |c|
-c.response_header.status.should == 404
-c.response.should == 'Try /version /hello_world, /bonjour, or /hola'
+it'routes to the correct API'do
+with_api(RackRoutes)do
+get_request({:path=>'/bonjour'},err)do |c|
+c.response_header.status.should == 200
+c.response.should == 'bonjour!'
+end
 end
 end
-end
 
-it'uses API middleware'do
-with_api(RackRoutes)do
-post_request({:path=>'/hola'},err)do |c|
-# the /hola route only supports GET requests
-c.response_header.status.should == 400
-c.response.should == '[:error, "Invalid request method"]'
+context"defined in blocks"do
+it'uses middleware defined in the block'do
+with_api(RackRoutes)do
+post_request({:path=>'/hola'},err)do |c|
+# the /hola route only supports GET requests
+c.response_header.status.should == 400
+c.response.should == '[:error, "Invalid request method"]'
+end
+end
+end
+
+it"doesn't use middleware defined in the API"do
+with_api(RackRoutes)do
+get_request({:path=>'/hola'},err)do |cb|
+# it doesn't raise required param error
+cb.response_header.status.should == 200
+cb.response.should == "hola!"
+end
+end
 end
 end
-end
 
-it'uses API middleware'do
-with_api(RackRoutes)do
-post_request({:path=>'/aloha'},err)do |c|
-# the /hola route only supports GET requests
-c.response_header.status.should == 400
-c.response.should == '[:error, "Invalid request method"]'
+context"defined in classes"do
+it'uses API middleware'do
+with_api(RackRoutes)do
+post_request({:path=>'/aloha'},err)do |c|
+# the /hola route only supports GET requests
+c.response_header.status.should == 400
+c.response.should == '[:error, "Invalid request method"]'
+end
+end
 end
 end
+
 end
-end
+end

spec/spec_helper.rb

@@ -5,8 +5,10 @@
 
 require'goliath/test_helper'
 
+Goliath.env=:test
+
 RSpec.configuredo |c|
 c.includeGoliath::TestHelper,:example_group=>{
 :file_path=>/spec\/integration/
 }
-end
+end

spec/unit/api_spec.rb

@@ -2,9 +2,29 @@
 require'goliath/api'
 
 describeGoliath::APIdo
+
+DummyApi=Class.new(Goliath::API)
+
 describe'middlewares'do
 it"doesn't change after multi calls"do
 2.times{Goliath::API.shouldhave(2).middlewares}
 end
 end
+
+describe".maps?"do
+context"when not using maps"do
+it"returns false"do
+DummyApi.maps?.shouldbe_false
+end
+end
+
+context"when using maps"do
+it"returns true"do
+DummyApi.map"/foo"
+DummyApi.maps?.shouldbe_true
+end
+end
+end
+
+
 end

spec/unit/rack/builder_spec.rb

@@ -0,0 +1,59 @@
+# encoding: utf-8
+require'spec_helper'
+
+describeGoliath::Rack::Builderdo
+
+SimpleMiddleware=Class.new
+NestedMiddleware=Class.new
+
+classNestedClassApi < Goliath::API
+useNestedMiddleware,'class'
+end
+
+classNestedBlockApi < Goliath::API
+useNestedMiddleware,'block'
+end
+
+classSimpleRouter < Goliath::API
+useSimpleMiddleware
+map"/class",NestedClassApi
+map"/block"do
+runNestedBlockApi.new
+end
+end
+
+let(:router){SimpleRouter.new}
+
+describe'.build'do
+
+it"builds rack app for api"do
+Rack::Builder.should_receive(:app)
+Goliath::Rack::Builder.build(router.class,router)
+end
+
+it"loads rack/goliath middlewares"do
+SimpleMiddleware.should_receive(:new)
+Goliath::Rack::Builder.build(router.class,router)
+end
+
+context"when API using maps"do
+
+it"loads mappings from classes"do
+NestedClassApi.should_receive(:new)# once in Builder
+Goliath::Rack::Builder.build(router.class,router)
+end
+
+it"loads mappings from blocks"do
+NestedBlockApi.should_receive(:new)# once in SimpleRouter
+Goliath::Rack::Builder.build(router.class,router)
+end
+
+it"loads middlewares only for class mappings"do
+NestedMiddleware.should_receive(:new)# once in NestedClassApi
+Goliath::Rack::Builder.build(router.class,router)
+end
+
+end
+
+end
+end