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

Does Garner work with Roar? #55

Open
monfresh opened this issue Sep 6, 2013 · 13 comments
Open

Does Garner work with Roar? #55

monfresh opened this issue Sep 6, 2013 · 13 comments

Comments

@monfresh
Copy link

monfresh commented Sep 6, 2013

Hi,
Me again 😄
I'm using Roar as my representer. I have 3 models: Organization, Location, and Services.
Here's my LocationRepresenter, which also includes some properties from the Organization and Service models:

require 'roar/representer/json'
require 'roar/representer/feature/hypermedia'

module LocationRepresenter
  include Roar::Representer::JSON
  include Roar::Representer::Feature::Hypermedia

  property :id
  # more location properties

  property :organization, extend: OrganizationRepresenter, class: Organization

  collection :services, :extend => ServiceRepresenter, :class => Service
end

Here's my Grape API location resource without Garner, which works fine:

...
get do
  locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
  locations.extend LocationsRepresenter
end

If I add Garner like this:

get do
  garner.options(expires_in: 15.minutes) do
    locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
    locations.extend LocationsRepresenter
  end
end

On the first request, I get the full representation. On the garnered request, I get the non-represented version of locations which is missing the organization and services stuff. I also tried garner.bind(Location) and garner.bind(Location).bind(Organization).bind(Service) and I'm still not getting the full representation back from garner.

If I try the same thing on an individual location:

get ':id' do
  garner.options(expires_in: 15.minutes) do
    location = Location.includes([:organization, :services]).find(params[:id])
    location.extend LocationRepresenter
  end
end

I get can't dump anonymous class #<Module:0x007f875130b3f0>.

I get the same dump error with the bind method:

get ':id' do
  garner.bind(Location) do
    location = Location.includes([:organization, :services]).find(params[:id])
    location.extend LocationRepresenter
  end
end

Any help would be greatly appreciated! Thanks!

@fancyremarker
Copy link
Contributor

Hi @monfresh, thanks again for the helpful and detailed issue! The short answer is that right now, Garner probably does not support caching Roar presenters. However, I'd like for it to be supported, so let me take a look at this issue today along with the outstanding issue on #53, and I'll let you know.

In the meantime, if you extend the class with your Roar presenters outside the Garner block, it should work, although perhaps not optimally. So, for example:

garner.options(expires_in: 15.minutes) do
  locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
end
locations.extend LocationsRepresenter

@dblock
Copy link
Contributor

dblock commented Sep 6, 2013

You probably want to cache JSON within the Garner block instead of the presenter. The presenter is some code that is going to do the presentation part, and caching it won't save you much - you want to cache the result that has been presented. So call to_json on the presenter (?).

@monfresh
Copy link
Author

monfresh commented Sep 6, 2013

@fancyremarker I forgot to mention that I did try calling the representer outside of the garner block.
If I do this:

garner.bind(Location) do
 @locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
end
@locations.extend LocationsRepresenter

Garner returns null when I request /api/locations.

If I use the options:

garner.options(expires_in: 15.minutes) do
  @locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
end
@locations.extend LocationsRepresenter

I get unknown class/module Location.

I also tried caching just the representation like this:

get do
  locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
  garner.options(expires_in: 15.minutes) do
    locations.extend LocationsRepresenter
  end
end

but that returns the original locations, not the representation.

@monfresh
Copy link
Author

monfresh commented Sep 7, 2013

I decided to give grape entities a try and was able to get Garner to work by doing this:

require "garner/mixins/rack"

module Ohana

  module Entities
    class Location < Grape::Entity
      expose :name
      expose :description
      ... 
    end
  end

  class Locations < Grape::API
    helpers Garner::Mixins::Rack
    use Rack::ConditionalGet
    use Rack::ETag

    desc 'Returns all locations'
    params do
      optional :page, type: Integer, default: 1
      optional :per_page, type: Integer
    end
    get '/locations' do
      garner.options(expires_in: 15.minutes) do
        locations = Location.page(params[:page]).per(params[:per_page])
        present locations, with: Ohana::Entities::Location
        locations.to_s
      end
    end
  end

Without adding locations.to_s, I get no _dump_data is defined for class StringIO as reported in issue #17. Is adding locations.to_s the right solution? Are there any adverse effects?

Is there something similar that can work with Roar? Instead of to_s, I tried to_json (in my Roar branch) as @dblock suggested, but that returns a String instead of JSON.

@monfresh
Copy link
Author

monfresh commented Sep 7, 2013

I take that back. It's not working as expected with grape entities. The first request gives me the expected representation, but the second request returns this string: "#<Mongoid::Criteria:0x007faaa36d4db8>"

@dblock
Copy link
Contributor

dblock commented Sep 7, 2013

Call as_json on both.

@monfresh
Copy link
Author

monfresh commented Sep 7, 2013

Thanks, that makes sense! It works great now. There was another part missing that I can't believe I overlooked. In order for garner to send back the representation, I have to explicitly set the locations variable to the represented version, then call as_json on that:

garner.options(expires_in: 15.minutes) do
  locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
  locations = present locations, with: Ohana::Entities::Location
  locations.as_json
end

@monfresh
Copy link
Author

monfresh commented Sep 7, 2013

Note that the same thing does not work with Roar. I still get the non-represented version when I do this:

garner.options(expires_in: 15.minutes) do
  locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
  locations = locations.extend LocationsRepresenter
  locations.as_json
end

@ekosz
Copy link

ekosz commented Aug 31, 2014

@dblock Has there been any update on this on getting Roar to work with Garner?

@dblock
Copy link
Contributor

dblock commented Aug 31, 2014

I've been using these two together but without grape-roar and with my own representer, like so:

module Grape
  module Roar
    module Representer
      def self.included(base)
        base.extend(ClassMethods)
      end

      module ClassMethods
        def represent(object, options = {})
          object.extend self
          # our own version of Grape::Roar::Representer
          # returns a serializable object within a garner block
          object.to_json(options)
        end

        def entity_name
          self.name.split('::').last.gsub(/Presenter$/, '')
        end

        def exposures
          {}
        end

        def documentation
          Hash[representable_attrs.map do |attribute|
            property_name = attribute[:as].evaluate nil, nil
            next if property_name == '_links'
            [ property_name, { desc: attribute[:desc], type: attribute[:type] }]
          end.compact]
        end

        def build_definition(name, options, &block)
          options[:render_nil] = true
          options[:render_empty] = true
          super
        end
      end
    end
  end
end

Furthermore we render strings directly without encoding them as JSON:

# The original Grape formatter is more strict and doesn't allow String or nil.
module Grape
  module Formatter
    module Json
      class << self
        def call(object, env)
          return object if ! object || object.is_a?(String)
          return MultiJson.dump(object) if object.respond_to?(:to_json) # to_json will use ActiveSupport::JSON.encode, which double-encodes UTF-8 strings
          raise Grape::Exceptions::InvalidFormatter.new(object.class, 'json')
        end
      end
    end
  end
end

Maybe both should be combined into a grape-roar-garner gem, but seems all too hacky to me.

@dblock
Copy link
Contributor

dblock commented Aug 31, 2014

The above btw is ruby-grape/grape-roar#3. I think you could help by addressing this issue and adding a new Grape::Roar::Representer::JSON class that can be swapped. That will fix the garner part of the issue.

@hadwinzhy
Copy link

@monfresh i found just add to_hash make it work with representer.
but i don't known why.
@dblock can u help to explain?

garner.options(expires_in: 15.minutes) do
  locations = Location.includes([:organization, :services]).page(params[:page]).per(params[:per_page])
  locations = locations.extend LocationsRepresenter
  locations.to_hash
end

@dblock
Copy link
Contributor

dblock commented Oct 23, 2014

The outer garner call caches the result of the block, which ends up being an array of locations extended with LocationsRepresenter. However that extend part is not actually serialized. When this is de-serialized the next time around the cached result doesn't know how to render itself.

By calling to_hash you serialize the actual output, not the array of instances that produces the output.

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

No branches or pull requests

5 participants