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

Collection representation #23

Open
Kris-LIBIS opened this issue Mar 15, 2017 · 8 comments
Open

Collection representation #23

Kris-LIBIS opened this issue Mar 15, 2017 · 8 comments
Assignees
Labels

Comments

@Kris-LIBIS
Copy link

Kris-LIBIS commented Mar 15, 2017

I have some issues using the JSONAPI output for collections. I'm generating the collection output with:

orgs = paginate(Libis::Ingester::Organization.all)
Representer::OrganizationRepresenter.for_collection.prepare(orgs).to_hash(pagination_hash(orgs))

The options generated by 'pagination_hash' looks like this:

    {
      user_options:
      {
        ...,
        links:
        {
          self: "http://localhost:9393/api/organizations?per_page=2&page=1",
          first: "http://localhost:9393/api/organizations?per_page=2&page=1",
          last: "http://localhost:9393/api/organizations?per_page=2&page=3",
          next: "http://localhost:9393/api/organizations?per_page=2&page=2"
        }
      },
      meta: { per_page:2, :total=>5, :page=>1, :max_page=>3, :next_page=>2, :prev_page=>nil}
    }

The generated hash looks like this:

{
    "data": [
        {
            "id": "58b7ccd7e852dd0775f51cae",
            "attributes": {
                "name": "Test Ingest Organization"
            },
            "type": "organization",
            "links": {
                "self": "http://localhost:9393/api/organizations/58b7ccd7e852dd0775f51cae",
                "all": "http://localhost:9393/api/organizations"
            },
            "meta": {
                "per_page": 2,
                "total": 5,
                "page": 1,
                "max_page": 3,
                "next_page": 2,
                "prev_page": null
            }
        },
        {
            "id": "58b7ccd7e852dd0775f51caf",
            "attributes": {
                "name": "Dummy test organization"
            },
            "type": "organization",
            "links": {
                "self": "http://localhost:9393/api/organizations/58b7ccd7e852dd0775f51caf",
                "all": "http://localhost:9393/api/organizations"
            },
            "meta": {
                "per_page": 2,
                "total": 5,
                "page": 1,
                "max_page": 3,
                "next_page": 2,
                "prev_page": null
            }
        }
    ],
    "meta": {
        "per_page": 2,
        "total": 5,
        "page": 1,
        "max_page": 3,
        "next_page": 2,
        "prev_page": null
    }
}

There are two issues with this:

  • the 'meta' section is present on each 'organization', but should only be present as a top level entry.
  • the links are missing on the top-level (the links on the individual 'organization' entries are generated by the Decorator itself and are correct).

The output I'm expecting is:

{
    "data": [
        {
            "id": "58b7ccd7e852dd0775f51cae",
            "attributes": {
                "name": "Test Ingest Organization"
            },
            "type": "organization",
            "links": {
                "self": "http://localhost:9393/api/organizations/58b7ccd7e852dd0775f51cae",
                "all": "http://localhost:9393/api/organizations"
            }
        },
        {
            "id": "58b7ccd7e852dd0775f51caf",
            "attributes": {
                "name": "Dummy test organization"
            },
            "type": "organization",
            "links": {
                "self": "http://localhost:9393/api/organizations/58b7ccd7e852dd0775f51caf",
                "all": "http://localhost:9393/api/organizations"
            }
        }
    ],
    "links": {
        "self": "http://localhost:9393/api/organizations?per_page=2&page=1",
        "first": "http://localhost:9393/api/organizations?per_page=2&page=1",
        "last": "http://localhost:9393/api/organizations?per_page=2&page=3",
        "next": "http://localhost:9393/api/organizations?per_page=2&page=2"
    },
    "meta": {
        "per_page": 2,
        "total": 5,
        "page": 1,
        "max_page": 3,
        "next_page": 2
    }
}

BTW: I'm using latest releases of grape, grape-roar, roar and roar-jsonapi on Ruby 2.3.1.

@myabc myabc self-assigned this Mar 17, 2017
@myabc myabc added the bug label Mar 17, 2017
@myabc
Copy link
Collaborator

myabc commented Mar 17, 2017

@Kris-LIBIS could you post your Representer? I'm assuming you have a block that looks something like this?

link :self, toplevel: true do |options|
  "/api/organizations?page=#{options[:page]}&per_page=#{options[:per_page]}"
end

myabc added a commit that referenced this issue Mar 17, 2017
Top-level meta information should not be rendered on individual
resource objects in the primary data array.

Also tighten up wording in tests and documentation.

Closes #23

Signed-off-by: Alex Coles <alex@alexbcoles.com>
myabc added a commit that referenced this issue Mar 17, 2017
Top-level meta information should not be rendered on individual
resource objects in the primary data array.

Also tighten up wording in tests and documentation.

Closes #23

Signed-off-by: Alex Coles <alex@alexbcoles.com>
@myabc myabc closed this as completed in #24 Mar 17, 2017
@myabc myabc reopened this Mar 17, 2017
@Kris-LIBIS
Copy link
Author

@myabc I might have something like that at a given point. In the mean time I worked around the meta problem by putting it in my Representer i.o. the user_options. But I still cannot work out the links part. I have fiddled around with my Representers so much, that I don't know anymore how I got the results above.

But I have currently this:

base.rb

require 'roar/coercion'
module Libis::Ingester::API::Representer
  module Base
    def self.included(klass)
      klass.include Roar::JSON
      klass.include Roar::Coercion
      klass.include Representable::Hash
      klass.include Representable::Hash::AllowSymbols
      klass.include Roar::JSON::JSONAPI::Mixin
      klass.property :id, exec_context: :decorator, writable: false,
                     type: String, desc: 'Object\'s unique identifier'
    end
    def id
      represented.id.to_s
    end
  end
end

item_list.rb

require_relative 'base'
module Libis::Ingester::API::Representer
  module ItemList
    def self.included(klass)
      klass.include Base
      klass.link :self do |opts|
        "#{opts[:base_url]}/#{represented.id}"
      end
      klass.link :all do |opts|
        "#{opts[:base_url]}"
      end
      [:self, :next, :prev, :first, :last, :all].each do |ref|
        klass.link ref, toplevel: true do |opts|
          opts[:links][ref] rescue nil
        end
      end
      klass.meta toplevel: true do
        property :limit_value, as: :per_page
        property :total_count
        property :current_page
        property :total_pages
        property :next_page
        property :prev_page
      end
    end
  end
end

organization.rb

require_relative 'item_list'
module Libis::Ingester::API::Representer
  class OrganizationRepresenter < Grape::Roar::Decorator
    include ItemList
    type :organization
    attributes do
      property :name, type: String, desc: 'organization name'
    end
  end
end

The reason I do it this way is because I have a lot of object types to represent and I want to keep as much as possible the object and api specific data out of the Representer.

I do get the meta section on the top level only and I get the links on the top level too, but I cannot get the links :self and :all to appear on the individual objects anymore:

        orgs = paginate(Libis::Ingester::Organization.all)
        Representer::OrganizationRepresenter.for_collection.prepare(orgs).extend(Representable::Debug).
            to_hash(pagination_hash(orgs).merge(fields_opts(declared(params).fields, {organization: [:name]})))

with the to_hash argument evaluation to:

{
  user_options: {
    base_url: "http://localhost:9393/api/organizations",
    links: {
      self: "http://localhost:9393/api/organizations?per_page=2&page=1",
      first: "http://localhost:9393/api/organizations?per_page=2&page=1",
      last: "http://localhost:9393/api/organizations?per_page=2&page=3",
      next: "http://localhost:9393/api/organizations?per_page=2&page=2"
    }
  },
  fields: {
    organization: [:name]
  }
}

Gives me:

{
  "data": [
    {
      "id": "58b7ccd7e852dd0775f51cae",
      "attributes": {
        "name": "Test Ingest Organization"
      },
      "type": "organization"
    },
    {
      "id": "58b7ccd7e852dd0775f51caf",
      "attributes": {
        "name": "Dummy test organization"
      },
      "type": "organization"
    }
  ],
  "links": {
    "self": "http://localhost:9393/api/organizations?per_page=2&page=1",
    "next": "http://localhost:9393/api/organizations?per_page=2&page=2",
    "first": "http://localhost:9393/api/organizations?per_page=2&page=1",
    "last": "http://localhost:9393/api/organizations?per_page=2&page=3"
  },
  "meta": {
    "per_page": 2,
    "total-count": 5,
    "current-page": 1,
    "total-pages": 3,
    "next-page": 2
  }
}

Note that there is no links section on the I don't see what I am doing wrong here. Especially since it used to work at some point.

@Kris-LIBIS
Copy link
Author

@myabc I also looked at the code change you committed for #24. If I interpret that correctly you are stripping both the meta and user_options for the individual document rendering, right?

I am afraid that stripping the user_options may break my code again, because the :base_url will not be available for the individual objects any more and would cause the :self and :all links to be broken.

Since one can always use if clauses and differentiate with toplevel: true I do not see any harm in passing the user_options to the individual documents. Would you consider reverting that part of the change?

@Kris-LIBIS
Copy link
Author

I played a bit more with the Representer and discovered by accident that the :self and :all links appear again when I do not supply the :fields option in the to_hash argument.

Is that a bug or is it intentional or am I doing something wrong? If it is a bug I'd gladly open a separate issue for it if you want me to.

@Kris-LIBIS
Copy link
Author

BTW the same happens when I add the :include option.

@twiduch
Copy link

twiduch commented May 16, 2017

I also have the problem with include:
Adding that removes links. Did you find any solution for that?

@Kris-LIBIS
Copy link
Author

@twiduch Yes I did. I pathced ResourceCollection#to_hash like so:

module Roar
  module JSON
    module JSONAPI
      module ResourceCollection
        def to_hash(options = {})
          meta = options.delete(:meta)
          document = super(to_a: options, user_options: options[:user_options]) # [{data: {..}, data: {..}}]

          links = Renderer::Links.new.(document, options)
          meta  = render_meta(meta: meta)
          included = []
          document['data'].each do |single|
            included += single.delete('included') || []
          end

          HashUtils.store_if_any(document, 'included',
                                 Fragment::Included.(included, options))
          HashUtils.store_if_any(document, 'links',    links)
          HashUtils.store_if_any(document, 'meta',     meta)

          document
        end
      end
    end
  end
end

Note that I refrain from using relationships and include as they do not play nice with grape-swagger in my setup. So you luck may vary using my patch.

@Kris-LIBIS
Copy link
Author

@twiduch My patch is not that different from the code change 0063d6d. I only remove the :meta information from the options because I need the :user_options when processing the data section.

Additionally, I created a special module for paginated collections:

  module Pagination

    def for_pagination
      for_collection.tap do |representer|
        representer.class_eval do
          def page_url(opts, page = nil, offset = nil)
            url = opts[:this_url]
            return url if page.nil? && offset.nil?
            return url unless opts && opts[:pagination] && opts[:pagination][:total] > 1
            page = (page || opts[:pagination][:page] rescue 1) + offset.to_i
            uri = URI::parse(url)
            uri.query_params['page'] = page
            uri.to_s
          end

          def next_url(opts)
            page_url(opts, nil, 1) if (opts[:pagination][:page] < opts[:pagination][:total] rescue false)
          end

          def prev_url(opts)
            page_url(opts, nil, -1) if (opts[:pagination][:page] > 1 rescue false)
          end

          def first_url(opts)
            page_url(opts, 1) if (opts[:pagination][:total] > 1 rescue false)
          end

          def last_url(opts)
            page_url(opts, (opts[:pagination][:total] rescue 1)) if (opts[:pagination][:total] > 1 rescue false)
          end

          link(:self) {|opts| page_url(opts)}
          link(:next) {|opts| next_url opts}
          link(:prev) {|opts| prev_url opts}
          link(:first) {|opts| first_url opts}
          link(:last) {|opts| last_url opts}

          # meta do
          #   property :current_page, exec_context: :decorator
          #   property :total_pages, exec_context: :decorator
          #   property :next_page, exec_context: :decorator
          #   property :prev_page, exec_context: :decorator
          #   property :total_count, exec_context: :decorator
          #   property :limit_value, as: :per_page, exec_context: :decorator
          # end
          #
          # def current_page
          #   respresented.respond_to?(:current_page) ? represented.current_page : nil
          # end

        end
      end
    end
  end

I include the module in the representer and now use #for_pagination instead of #for_collection and supply the pagination info in the options hash with this code snippet:

  def pagination_hash(collection, default = {})
    option_hash(default).tap do |h|
      h[:user_options].merge!(
          pagination: {
              page: collection.current_page,
              total: collection.total_pages,
              per: declared(params).per_page
          }
      )
      h.merge!(
        meta: {
            current_page: collection.current_page,
            total_pages: collection.total_pages,
            total_count: collection.total_count,
            per_page: collection.limit_value
        }
      )
      h[:meta][:next_page] = collection.next_page if collection.next_page
      h[:meta][:prev_page] = collection.prev_page if collection.prev_page
    end
  end

The option_hash() call supplies the basic options like :base_url and :this_url that I feed to any representer#to_hash call and is needed fo my code to work.

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

No branches or pull requests

3 participants