Permalink
Browse files

Add memoizing to AD::Request

  • Loading branch information...
1 parent 60bbf16 commit 146a5305d56b636d2fd2c2a09a58f5f3edc2d9c0 Carlhuda committed Mar 8, 2010
Showing with 8 additions and 0 deletions.
  1. +8 −0 actionpack/lib/action_dispatch/http/request.rb
@@ -30,6 +30,14 @@ def #{env.sub(/^HTTP_/n, '').downcase}
METHOD
end
+ def self.new(env)
+ if request = env["action_dispatch.request"] && request.instance_of?(self)
@GerryG

GerryG Dec 5, 2011

I think this line is buggy. Should be 'and', not '&&'

Request will be nil or true here, not a request.

@josevalim

josevalim Dec 5, 2011

Member

It doesn't seen though that env["action_dispatch.request"] is used. So I am unsure if I should fix this or simply get rid of the memoization.

@GerryG

GerryG Dec 5, 2011

I would not get rid of it. I found it because I'm trying to implement part of our app in middleware, and I need to be able to get the same request in another middleware from the env. When I saw this code, I thought, good it is doing what I need, but then I realized it was broken.

How can it ever be used if it is broken?

@josevalim

josevalim Dec 5, 2011

Member

Why do you need exactly the same request object? The AD::Request object should not have state, this code was added for performance only.

@josevalim

josevalim Dec 6, 2011

Member

Any news? If you don't have a good use case, I will just go ahead and remove this code. :)

+ return request
+ end
+
+ super
+ end
+
def key?(key)
@env.key?(key)
end

0 comments on commit 146a530

Please sign in to comment.