search

artsy / garner

A set of Rack middleware and cache helpers that implement various caching strategies.
MIT License
343 stars 24 forks source link

Where to include Garner::Mixins::Rack ? #53

Open monfresh opened 11 years ago

monfresh commented 11 years ago

Hi,

I'd like to try this gem out, but I can't get it to recognize the mixin in my Rails app. It would be great if the documentation were more specific as to where the include needs to go. Here is my setup:

app/api/api.rb:

module API
  class Root < Grape::API
    prefix 'api'
    format :json
    version 'v1', using: :header, vendor: 'ohanapi'

    mount Ohana::API
    Grape::Endpoint.send :include, LinkHeader
    add_swagger_documentation markdown: true, hide_documentation_path: true,
      hide_format: true, api_version: 'v1'
  end
end

app/api/ohana.rb:

module Ohana
  class API < Grape::API

  resource "/" do
...

I tried including it in both files, and also in the Grape::Endpoint.send line, but I keep getting uninitialized constant Garner::Mixins. What am I doing wrong?

monfresh commented 11 years ago

Same with the Mongoid mixins. I created config/initializers/garner.rb with this (and restarted the server):

module Mongoid
  module Document
    include Garner::Mixins::Mongoid::Document
  end
end

but it complains: uninitialized constant Garner::Mixins

monfresh commented 11 years ago

I also tried Garner::Cache::Context and I was able to get it to recognize garner by including it like so in app/api/api.rb:

Grape::Endpoint.send :include, LinkHeader, Garner::Cache::Context

However, I had some strange things happen. With the expires_in option set to 1.hour, everything seems to run normally, except it doesn't seem to be caching anything. I keep getting 200 response codes.

resource 'categories' do
  # GET /categories
  desc "Returns all categories"
  params do
    optional :page, type: Integer, default: 1
  end
  get do
    garner.options(expires_in: 1.hour) do
      categories = Category.page(params[:page]).per(400)
      categories.extend CategoriesRepresenter
    end
 end
end

However, if I change the time to 15.minutes, I get an exception: undefined class/module Category. Why would 15.minutes generate an exception and not 1.hour?

fancyremarker commented 11 years ago

@monfresh: Thanks for your issue! In response to the first point,

uninitialized constant Garner::Mixins

the issue is that you need to explicitly require "garner/mixins/rack" and require "garner/mixins/mongoid". I apologize for not including this in the README. I've just updated it. To your next point,

It would be great if the documentation were more specific as to where the include needs to go.

Yes, I agree. For a Grape application, I'd include the Garner mixin using helpers. You can see an example in the Grape integration spec, but in your specific case, it'd be:

module API
  class Root < Grape::API
    helpers Garner::Mixins::Rack

    # ...
  end
end

and

module Ohana
  class API < Grape::API
    helpers Garner::Mixins::Rack

    # ...
  end
end

I'll draft an edit to the README that references the Grape integration details.

fancyremarker commented 11 years ago

This point is a bit trickier to debug with certainty:

However, I had some strange things happen. With the expires_in option set to 1.hour, everything seems to run normally, except it doesn't seem to be caching anything.

Can you send the complete backtrace? It seems as though the cache store is correctly marshaling and caching the Categories objects, but is then unable to unmarshal them from cache. A backtrace would be helpful in determining whether this is what's happening, and how to fix it. I can submit a spec and fix for the issue tonight.

To this point:

I keep getting 200 response codes.

Are you looking to get 304 response codes for If-Modified-Since and If-None-Match requests? If so, we made a decision to stop supporting that within Garner, since many other libraries do it very succinctly and effectively. We recommend instead using Rack::ConditionalGet and Rack::Etag like so. This is also mentioned in the upgrading doc, which isn't exactly straightforward to find. Any suggestions on better exposing these points in the README are welcome.

class API < Grape::API
  use Rack::ConditionalGet
  use Rack::ETag
end
monfresh commented 11 years ago

Thanks for your detailed responses. I got the mixins working, but I think I found a reproducible case for undefined class/module Category.

  1. Remove the garner gem from your project
  2. Reinstall the gem
  3. Set up your Grape API with the mixins
  4. Add the garner block around your endpoint call:
get do
  garner.options(expires_in: 15.minutes) do
    categories = Category.page(params[:page]).per(400)
    categories.extend CategoriesRepresenter
  end
end
  1. Verify that a query to the categories endpoint returns 200 and that subsequent queries have a new etag that garner sends back
  2. Stop the server and comment out the garner portion of the block.
  3. Restart the server and perform the same query as in step 5.

Expected Result: a 200 response Actual Result: 500 response. See the full stack below.

If I wait a few minutes and try again, it starts working again. The same thing happens if I uncomment the garner block and try right after doing it.

ArgumentError - undefined class/module Category:
  activesupport (3.2.13) lib/active_support/cache.rb:587:in `load'
  activesupport (3.2.13) lib/active_support/cache.rb:587:in `value'
  activesupport (3.2.13) lib/active_support/cache.rb:294:in `fetch'
  garner (0.4.4) lib/garner/cache.rb:12:in `fetch'
  garner (0.4.4) lib/garner/cache/identity.rb:22:in `fetch'
  garner (0.4.4) lib/garner/cache/identity.rb:54:in `options'
  app/api/ohana.rb:134:in `block (2 levels) in <class:API>'
  grape (0.5.0) lib/grape/endpoint.rb:31:in `call'
  grape (0.5.0) lib/grape/endpoint.rb:31:in `block in generate_api_method'
  grape (0.5.0) lib/grape/endpoint.rb:400:in `call'
  grape (0.5.0) lib/grape/endpoint.rb:400:in `run'
  grape (0.5.0) lib/grape/endpoint.rb:164:in `block in call!'
  grape (0.5.0) lib/grape/middleware/base.rb:24:in `call'
  grape (0.5.0) lib/grape/middleware/base.rb:24:in `call!'
  grape (0.5.0) lib/grape/middleware/base.rb:18:in `call'
  grape (0.5.0) lib/grape/middleware/base.rb:24:in `call!'
  grape (0.5.0) lib/grape/middleware/base.rb:18:in `call'
  grape (0.5.0) lib/grape/middleware/error.rb:26:in `block in call!'
  grape (0.5.0) lib/grape/middleware/error.rb:25:in `catch'
  grape (0.5.0) lib/grape/middleware/error.rb:25:in `call!'
  grape (0.5.0) lib/grape/middleware/base.rb:18:in `call'
  rack (1.4.5) lib/rack/head.rb:9:in `call'
  rack (1.4.5) lib/rack/builder.rb:134:in `call'
  grape (0.5.0) lib/grape/endpoint.rb:165:in `call!'
  grape (0.5.0) lib/grape/endpoint.rb:155:in `call'
  rack-mount (0.8.3) lib/rack/mount/route_set.rb:152:in `block in call'
  rack-mount (0.8.3) lib/rack/mount/code_generation.rb:96:in `block in recognize'
  rack-mount (0.8.3) lib/rack/mount/code_generation.rb:75:in `optimized_each'
  rack-mount (0.8.3) lib/rack/mount/code_generation.rb:95:in `recognize'
  rack-mount (0.8.3) lib/rack/mount/route_set.rb:141:in `call'
  grape (0.5.0) lib/grape/api.rb:480:in `call'
  grape (0.5.0) lib/grape/api.rb:49:in `call!'
  grape (0.5.0) lib/grape/api.rb:45:in `call'
  journey (1.0.4) lib/journey/router.rb:68:in `block in call'
  journey (1.0.4) lib/journey/router.rb:56:in `each'
  journey (1.0.4) lib/journey/router.rb:56:in `call'
  actionpack (3.2.13) lib/action_dispatch/routing/route_set.rb:612:in `call'
  rack-pjax (0.7.0) lib/rack/pjax.rb:12:in `call'
  newrelic_rpm (3.6.6.147) lib/new_relic/rack/error_collector.rb:43:in `call'
  newrelic_rpm (3.6.6.147) lib/new_relic/rack/agent_hooks.rb:22:in `call'
  newrelic_rpm (3.6.6.147) lib/new_relic/rack/browser_monitoring.rb:16:in `call'
  newrelic_rpm (3.6.6.147) lib/new_relic/rack/developer_mode.rb:28:in `call'
  bullet (4.6.0) lib/bullet/rack.rb:13:in `call'
  mongoid (3.1.4) lib/rack/mongoid/middleware/identity_map.rb:34:in `block in call'
  mongoid (3.1.4) lib/mongoid/unit_of_work.rb:39:in `unit_of_work'
  mongoid (3.1.4) lib/rack/mongoid/middleware/identity_map.rb:34:in `call'
  rack-cors (0.2.8) lib/rack/cors.rb:54:in `call'
  warden (1.2.3) lib/warden/manager.rb:35:in `block in call'
  warden (1.2.3) lib/warden/manager.rb:34:in `catch'
  warden (1.2.3) lib/warden/manager.rb:34:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/best_standards_support.rb:17:in `call'
  rack (1.4.5) lib/rack/etag.rb:23:in `call'
  rack (1.4.5) lib/rack/conditionalget.rb:25:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/head.rb:14:in `call'
  remotipart (1.2.1) lib/remotipart/middleware.rb:27:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/params_parser.rb:21:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/flash.rb:242:in `call'
  rack (1.4.5) lib/rack/session/abstract/id.rb:210:in `context'
  rack (1.4.5) lib/rack/session/abstract/id.rb:205:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/cookies.rb:341:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/callbacks.rb:28:in `block in call'
  activesupport (3.2.13) lib/active_support/callbacks.rb:405:in `_run__4523929805245771996__call__4335334493411686009__callbacks'
  activesupport (3.2.13) lib/active_support/callbacks.rb:405:in `__run_callback'
  activesupport (3.2.13) lib/active_support/callbacks.rb:385:in `_run_call_callbacks'
  activesupport (3.2.13) lib/active_support/callbacks.rb:81:in `run_callbacks'
  actionpack (3.2.13) lib/action_dispatch/middleware/callbacks.rb:27:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/reloader.rb:65:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/remote_ip.rb:31:in `call'
  better_errors (0.9.0) lib/better_errors/middleware.rb:84:in `protected_app_call'
  better_errors (0.9.0) lib/better_errors/middleware.rb:79:in `better_errors_call'
  better_errors (0.9.0) lib/better_errors/middleware.rb:56:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/debug_exceptions.rb:16:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/show_exceptions.rb:56:in `call'
  railties (3.2.13) lib/rails/rack/logger.rb:32:in `call_app'
  railties (3.2.13) lib/rails/rack/logger.rb:16:in `block in call'
  activesupport (3.2.13) lib/active_support/tagged_logging.rb:22:in `tagged'
  railties (3.2.13) lib/rails/rack/logger.rb:16:in `call'
  quiet_assets (1.0.2) lib/quiet_assets.rb:18:in `call_with_quiet_assets'
  actionpack (3.2.13) lib/action_dispatch/middleware/request_id.rb:22:in `call'
  rack (1.4.5) lib/rack/methodoverride.rb:21:in `call'
  rack (1.4.5) lib/rack/runtime.rb:17:in `call'
  activesupport (3.2.13) lib/active_support/cache/strategy/local_cache.rb:72:in `call'
  lib/api_defender.rb:47:in `call'
  rack (1.4.5) lib/rack/lock.rb:15:in `call'
  actionpack (3.2.13) lib/action_dispatch/middleware/static.rb:63:in `call'
  rack-timeout (0.0.4) lib/rack/timeout.rb:16:in `block in call'
  /Users/moncef/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/timeout.rb:66:in `timeout'
  rack-timeout (0.0.4) lib/rack/timeout.rb:16:in `call'
  railties (3.2.13) lib/rails/engine.rb:479:in `call'
  railties (3.2.13) lib/rails/application.rb:223:in `call'
  railties (3.2.13) lib/rails/railtie/configurable.rb:30:in `method_missing'
  rack (1.4.5) lib/rack/lint.rb:48:in `_call'
  rack (1.4.5) lib/rack/lint.rb:36:in `call'
  rack (1.4.5) lib/rack/showexceptions.rb:24:in `call'
  rack (1.4.5) lib/rack/commonlogger.rb:33:in `call'
  rack (1.4.5) lib/rack/chunked.rb:43:in `call'
  rack (1.4.5) lib/rack/content_length.rb:14:in `call'
  unicorn (4.6.3) lib/unicorn/http_server.rb:552:in `process_client'
  unicorn (4.6.3) lib/unicorn/http_server.rb:632:in `worker_loop'
  unicorn (4.6.3) lib/unicorn/http_server.rb:500:in `spawn_missing_workers'
  unicorn (4.6.3) lib/unicorn/http_server.rb:142:in `start'
  unicorn (4.6.3) bin/unicorn:126:in `<top (required)>'
   () Users/moncef/.rvm/gems/ruby-2.0.0-p247@ohana-api/bin/unicorn:23:in `load'
   () Users/moncef/.rvm/gems/ruby-2.0.0-p247@ohana-api/bin/unicorn:23:in `<main>'
   () Users/moncef/.rvm/gems/ruby-2.0.0-p247@ohana-api/bin/ruby_noexec_wrapper:14:in `eval'
   () Users/moncef/.rvm/gems/ruby-2.0.0-p247@ohana-api/bin/ruby_noexec_wrapper:14:in `<main>'
monfresh commented 11 years ago

It seems like any change you make to the garner block results in a 500. It's probably not unmarshalling like you suspected. Is there something I can do to force it to unmarshal?

monfresh commented 11 years ago

Quick unrelated question: Is this:

garner.bind(Location.identify(params[:id])) do
  location = Location.find(params[:id])
end

equivalent to this?

location = Location.garnered_find(params[:id])
fancyremarker commented 11 years ago

Those two calls are nearly equivalent. The only difference is that garnered_find bypasses all context key strategies (like the Caller strategy), so that the same cache result is used everywhere in the application where Location.garnered_find(id) is called.

fancyremarker commented 11 years ago

@monfresh: I tried unsuccessfully to reproduce the bug you're seeing in 9c87cf49908c468c41e53d8cbbe6f432268e8d46. The stopping/starting of the Grape server is the hard part to simulate in a spec. Have you seen the bug manifest itself in other ways, without stopping/starting the server? Alternatively, do you have suggestions for how to test this in an RSpec environment?