Adding ability to use resource from other namespace

[senid231]

Routing:

resource for jsonapi_resource(s), jsonapi_related_resource(s), jsonapi_link(s), jsonapi_relationships will be founded through controller
Breaking changes - application will not start if route's controller is missing

Controller:

provides resource_klass to RequestParser (previously it was provided only for resource_serializer, RequestParser detected resource_klass by params[:controller])

usage example:

# config/routes.rb
TestApp.routes.draw do
  namespace :api do
    namespace :v1 do
      jsonapi_resources :people
      jsonapi_resources :countries do
        jsonapi_related_resources :cities, controller: 'country_cities'
        jsonapi_links :cities
      end
      jsonapi_resources :cities
    end
  end

 namespace :api do
    namespace :v2 do
      jsonapi_resources :people
      jsonapi_resources :countries do
        jsonapi_relationships
      end
      jsonapi_resources :cities, controller: 'not_nested_cities'
    end
  end
end

# app/controllers/json_api_controller.rb
class JsonApiController < ApplicationController
  skip_before_filter :verify_authenticity_token
  include JSONAPI::ActsAsResourceController
end

# app/controllers/api/v1/people_controller.rb
class Api::V1::PeopleController < JsonApiController
end

# app/controllers/api/v1/countries_controller.rb
class Api::V1::CountriesController < JsonApiController
  def self.resource_klass_name
    'CountryResource'
  end
end

# app/controllers/api/v1/country_cities_controller.rb
class Api::V1::CountryCitiesController < Api::V1::CitiesController
  def context
    super.merge skip_country_id: true
  end
end

# app/controllers/api/v1/cities_controller.rb
class Api::V1::CitiesController < JsonApiController
  def self.resource_klass_name
    'CityResource'
  end
end

# app/controllers/api/v2/people_controller.rb
class Api::V2::PeopleController < JsonApiController
end

# app/controllers/api/v1/countries_controller.rb
class Api::V2::CountriesController < JsonApiController
  def self.resource_klass_name
    'CountryResource'
  end
end

# app/controllers/api/v2/cities_controller.rb
class Api::V2::CitiesController < JsonApiController
  def self.resource_klass_name
    'CityResource'
  end
  def context
    super.merge skip_country_id: true
  end
end

# app/controllers/api/v2/not_nested_cities_controller.rb
class Api::V2::NotNestedCitiesController < Api::V2::CitiesController
   def context
    super.merge skip_country_id: false
  end
end


# app/resources/base_resource.rb
class BaseResource < JSONAPI::Resource
  abstract
end

# app/resources/country_resource.rb
class CountryResource < BaseResource
  immutable
  attributes :name, :iso_code, :prefix
  filters :name, :iso_code, :prefix
  has_many :cities, class_name: 'City'
end

# app/resources/city_resource.rb
class CityResource < BaseResource
  immutable
  attributes :name, :country_id
  filters :name
  def self.fetchable_fields(context = {})
    res = super
    res - [:country_id] if context[:skip_country_id]
   res
  end
end

# app/resources/api/v1/person_resource.rb
class Api::V1::PersonResource < ::BaseResource
  model_name 'Person'
  attributes :first_name, :last_name
  filters :first_name, :last_name
end

# app/resources/api/v2/person_resource.rb
class Api::V2::PersonResource < Api::V1::PersonResource
  attributes :middle_name
  filter :middle_name
end

[senid231]

@lgebhardt, what do you think about it?

[lgebhardt]

My primary concerns are around edge cases. One example, we make a GET to the ApiRest::PeopleController with includes for comments,comments.author:

    get :index, params: {include: 'comments,comments.author'}

Here are the resources from the tests:

module ApiRest
  class PersonResource < BaseResource
    model_name 'Person'
    attributes :name, :email
    attribute :date_joined, format: :date_with_timezone
    has_many :comments, class_name: '::Comment'
  end
end

class CommentResource < JSONAPI::Resource
  attributes :body
  has_one :post
  has_one :author, class_name: 'Person'
  has_many :tags

  filters :body
end

class PersonResource < BaseResource
  model_name 'Person'
  attributes :name, :email
  attribute :date_joined, format: :date_with_timezone
  has_many :comments
end

Since comments are backed by the ::Comment resource, which has it's Author backed by the Person resource it's unclear how the included author should be serialized. Is it a PersonResource (root namespace), which will result in a separate record for the author in the included section? Or is it serialized as a ApiRest::PersonResource, which is how it seems to function in this PR but has some issues in not giving consistent results if the comment was fetched directly.

I think technically the first option is "correct", but it's a confusing response, at least in this case.

Note: this was edited to add the ::PersonResource to help with the example below.

[Fivell]

@lgebhardt , can you please add more details why you think response is confusing in this case?

[lgebhardt]

@Fivell In the example I gave above the request would be GET /apirest/people/1?include=comments,comments.author. Note I edited the example for clarity and to make it a closer match for the tests in the PR.

Without the namespace differences between comments and authors the response would return the person, their comments sideloaded, and then the author of each comment, which you would expect to be the original person, so no side loaded person data, since it's returned in the primary data. The types specified for these resource will not be decorated with the namespace.

The reason the author is not side loaded if it exists in the primary data is in the spec, which states: "A compound document MUST NOT include more than one resource object for each type and id pair."

However because of the namespace on ApiRest::PersonResource it's not clear what should be returned. As it is you will get the person in the primary data, and the comments side-loaded. But since the CommentResource author relationship is to a Person not in theApiRest namespace it is unclear how to serialize the person (which we know is the same as the primary data). But since it has a different resource definition the serialized version will/may be different (especially in the generated links).

The JSON API spec does not address namespace issues. I think to make it work completely we'd need to promote the namespace up into the type attribute, which would be a major change.

If that's clear as mud I'll see if I can write up some example responses, but I don't have the time at the moment.

[senid231]

@lgebhardt, I understand your concerns, but for now i think that in jsonapi_resources we should have possibility to use resources from another namespace as relationships (exception should be thrown in edge case that you describe).

for this example

module Api
    module System
      class Employee < BaseResource
        model_name 'Employee'
        attributes :full_name, :email
        has_one :country, class_name: 'Api::Country'
      end
    end

    class Country < BaseResource
      model_name 'Country'
      attributes :iso, :name
    end

    class Client < BaseResource
      model_name 'Client'
      attributes :full_name, :email, :phone_number
      has_one :country, class_name: 'Country'
      has_one :support, class_name: 'Api::System::Employee'
    end
end

such requests should be valid

get 'api/system/employees', params: {include: 'country'}
get 'api/clients', params: {include: 'country,support'}

and country relation always has link to 'api/countries/{id}'

but if we add resource

module Api
    module System
      class Country < BaseResource
        model_name 'Country'
        attributes :iso, :name
      end
    end
end

these requests should raise some internal exception.

With old behavior we can have to many duplicated endpoint

get 'api/system/country'
get 'api/country'

and link in country relationship will be '/api/system/countries/{id}' or '/api/system/countries/{id}' depending on parent resource

if we add another namespace with endpoint that relates to country we must add another chain of route + controller + resource with same functionality

Maybe some recommendations should be added to README

what do you think about this?

[lgebhardt]

Please review #927.