search

joshfrench / rakismet

Easy Akismet and TypePad AntiSpam integration for Rails
MIT License
355 stars 46 forks source link

Making Rakismet Thread-safe #35

Open workmad3 opened 9 years ago

workmad3 commented 9 years ago

While investigating Rakismet to see how it obtained the request-related information, I noticed that a lot of the information was being stored in non-thread-safe class instance variables. Considering the larger number of people deploying onto Puma and other thread-capable servers nowadays, this seems like a large omission.

So here's a change set to store the request and responses in Thread-local variables to keep thread safety.

The tests also still pass, a large majority of them are now rewritten to use expect/allow from rspec 3.

dwbutler commented 7 years ago

This is a big problem. @joshfrench If this solution were brought up-to-date, would you merge it?

joshfrench commented 7 years ago

@dwbutler Absolutely! Sorry I dropped the ball on this before. Feel free to reopen with up-to-date code/tests/versioning.

dwbutler commented 7 years ago

@joshfrench I'm looking at this again. Would you be open to adding a dependency on RequestStore? It handles correctly storing a request-local variable that gets cleared at the end of each request. If not, it can be implemented in an equivalent way here.

joshfrench commented 7 years ago

@dwbutler Seems fine to me. If you do submit a PR please include mention of the additional setup for non-Rails apps. Thanks!