Skip to content

Commit

Permalink
More thread safety: Create new Executor instance for each stack start.
Browse files Browse the repository at this point in the history
It in turn creates new instances of Middleware chain.
  • Loading branch information
ronen committed Mar 25, 2015
1 parent 828ae90 commit 35dc1b9
Showing 1 changed file with 43 additions and 32 deletions.
75 changes: 43 additions & 32 deletions lib/modware/stack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,70 +8,81 @@ def initialize(env:)
when Class then env
else KeyStruct[*env]
end
@middlewares = []
@middleware_mods = []
end

def add(mod)
middleware = Middleware.new(self, mod)
@middlewares.last._next = middleware if @middlewares.any?
@middlewares << middleware
@middleware_mods << mod
end

def start(*args, &implementation)
def start(*args, &base_implementation)
env = @env_klass.new(*args)
execute_stack(env, implementation)
Executor.new(@middleware_mods).execute(env, base_implementation)
env
end

private

def execute_stack(env, base_implementation)
return call_implementation(env, base_implementation) unless @middlewares.any?

@middlewares.each do |middleware|
middleware.before env if middleware.respond_to? :before
class Executor
def initialize(middleware_mods)
prev = nil
@middlewares = middleware_mods.map { |mod|
Middleware.new(self, mod).tap { |middleware|
prev._modware_next = middleware if prev
prev = middleware
}
}
end

@middlewares.first._call(env, base_implementation)
def execute(env, base_implementation)
return call_implementation(env, base_implementation) if @middlewares.empty?

@middlewares.each do |middleware|
middleware.before env if middleware.respond_to? :before
end

@middlewares.first._modware_call(env, base_implementation)

@middlewares.each do |middleware|
middleware.after env if middleware.respond_to? :after
@middlewares.each do |middleware|
middleware.after env if middleware.respond_to? :after
end
end
end

def call_implementation(env, base_implementation)
if middleware = @middlewares.select(&it.respond_to?(:implement)).last
middleware.implement(env)
elsif base_implementation
base_implementation.call env
else
raise StackError, "No base implementation nor middleware implementation in stack"
def call_implementation(env, base_implementation)
if middleware = @middlewares.select(&it.respond_to?(:implement)).last
middleware.implement(env)
elsif base_implementation
base_implementation.call env
else
raise StackError, "No base implementation nor middleware implementation in stack"
end
end
end


class Middleware
attr_accessor :_next
attr_accessor :_modware_next

def initialize(stack, mod)
@stack = stack
def initialize(executor, mod)
@executor = executor
singleton_class.send :include, mod
end

def _call(env, base_implementation)
def _modware_call(env, base_implementation)
if respond_to? :around
around(env) { |env|
_continue env, base_implementation
_modware_continue env, base_implementation
}
else
_continue env, base_implementation
_modware_continue env, base_implementation
end
end

def _continue(env, base_implementation)
if self._next
self._next._call(env, base_implementation)
def _modware_continue(env, base_implementation)
if self._modware_next
self._modware_next._modware_call(env, base_implementation)
else
@stack.send :call_implementation, env, base_implementation
@executor.call_implementation env, base_implementation
end
end
end
Expand Down

6 comments on commit 35dc1b9

@lowjoel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ronen sorry, I commented on the wrong fork... could you have a look at the comments in lowjoel/modware@35dc1b95bc348accc7987f1c58a96221b87181ee

@lowjoel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I think I know what you mean. The result must be explicitly captured by the middleware. That's a little counterintuitive, but I've fixed that (I think) in SchemaPlus/schema_plus_core#2

@ronen
Copy link
Member Author

@ronen ronen commented on 35dc1b9 Apr 26, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lowjoel Yes, that's a design choice / requirement. If the result is to be returned to the caller, the middleware "env" object must capture it, to give after and around methods the chance to modify it; and the wrapper extracts and return it.

@lowjoel
Copy link
Member

@lowjoel lowjoel commented on 35dc1b9 Apr 26, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ronen
Copy link
Member Author

@ronen ronen commented on 35dc1b9 Apr 26, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in the README for modware -- any way or place I can put it that would make it easier to find?

@lowjoel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, that would be the natural place to put it, it didn't occur to me.

I think it's because schema_plus_core and schema_monkey used modware implicitly so the natural instinct was to do as @ronen did =P the examples I referenced didn't use its return result, so it didn't occur to me that it was the pattern to follow

(and I checked schema_monkey, the readme did mention it... ohwell... I should have RTFM'ed...)

Please sign in to comment.