Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How does the model have access to the request environment? #44

Open
jjb opened this issue Oct 30, 2016 · 7 comments
Open

How does the model have access to the request environment? #44

jjb opened this issue Oct 30, 2016 · 7 comments

Comments

@jjb
Copy link

jjb commented Oct 30, 2016

(hi Josh!)

The readme says:

This means that if you are not storing the request variables, you must call spam? from within the controller action that handles comment submissions. That way the IP, user agent, and referer will belong to the person submitting the comment.

I chased user_ip around the code in a loop and am too stupid to figure out how a model can have access to the request environment. Where does this happen?

@joshfrench
Copy link
Owner

Hey John!

If you're using Rails there's a default middleware which sets those vars for the current request: https://github.com/joshfrench/rakismet/blob/master/lib/rakismet/middleware.rb (Which is why you need to call spam? in situ, or store the details for later recall.)

If you're not using Rails, you'd have to adapt that or set those vars manually.

@jjb
Copy link
Author

jjb commented Oct 30, 2016

Gotcha — but where/how in the model does it access those vars? Is that a
normal thing that an AR object can do when it's instantiated in a
controller action?

@joshfrench
Copy link
Owner

So that middleware sets the current request data as class-level vars on the Rakismet class, which is just a plain old Ruby object: https://github.com/joshfrench/rakismet/blob/master/lib/rakismet.rb#L30

Then spam? and friends just look up whatever's currently stored in those Rakismet class vars: https://github.com/joshfrench/rakismet/blob/master/lib/rakismet/model.rb#L76

Rakismet has no knowledge of AR, you could include rakismet/model into any kind of object and get the same behavior.

@jjb
Copy link
Author

jjb commented Oct 31, 2016

Ahhh.

So I suppose that means rakismet isn't thread-safe?

@joshfrench
Copy link
Owner

It's not. There's an out-of-date PR that I'd gladly merge if someone wanted to bring that branch up to speed with master: #35

(Hint hint, nudge nudge.)

@jjb
Copy link
Author

jjb commented Oct 31, 2016

Instead of explicitly supporting it with thread-local variables, what do you think of these ideas…

  1. pass the request object into spam? and friends, which will then invoke .user_ip etc on it. this can even be used outside of the context of a controller by passing in some other object with the same api.

    thing = Thing.new(params)
    raise "spam!" if thing.spam?(request)
    thing.save
  2. have include Rakismet::Model invoke attr_accessor :user_ip, :user_agent, :referrer (or maybe namespaced with something safer like rakismet_ or request_). then there is the option to set these non-persited values in whatever context, be it a controller or otherwise. this might simplify the case of

    thing = Thing.new(params)
    thing.user_ip, thing.user_agent, thing.referrer = request.user_ip, request.user_agent, request.referer
    raise "spam!" if thing.spam?
    thing.save

I like 1 a lot better but maybe there are reasons that 2 is more flexible or something.

WDYT?

@joshfrench
Copy link
Owner

They're both breaking changes, but in hindsight I think explicitly passing the request data in some form is preferable to magically gleaning it from the environment.

The first option would be my choice, as it relieves the user of needing to know which request vars are important.

Open question: if I decide to store my own request info --

@comment = Comment.new({ user_ip: foo, ... })

And later I supply a different request env:

@comment.spam?({ user_ip: bar, ... })

Which value would you expect to take precedence?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants