lynndylanhurley / devise_token_auth

Token based authentication for Rails JSON APIs. Designed to work with jToker and ng-token-auth.
Do What The F*ck You Want To Public License
3.55k stars 1.14k forks source link

Handle multiple adapters #818

Open ValentinTrinque opened 7 years ago

ValentinTrinque commented 7 years ago

There has been an attempt to add JSON API in this gem (#566 by @Dschee). It also seems that it has been previously discussed with @booleanbetrayal. But this PR introduces unrelated changes to JSON API such as translation, rubocop warning etc. This makes me think we eed to refactor this PR to get it properly reviewed and done. Put aside the unrelated changes, we still have a major work to get this working and doing a one-shot monstrous PR is out of the question for several obvious reasons :

We then have to go for it more iteratively.

First, I propose to introduce the base architecture to handle multiple request and response adapters.

Architecture

Here is the architecture I propose :

1. Determine the response format We could have a controller concern ResponseFormat which will return a symbol of the format expected by the client. For now, it will only return :json. We expect it to also return :json_api in a future iteration.

The analysis will be based on the HTTP_ACCEPT content or the default reponse format.

2. Determine the response adapter We could have a concern ResponseAdapter deciding which adapter to use.

Here is the logic :

def response_adapter
  case response_format
  when :json then JsonAdapter
  when :json_api then JsonApiAdapter
  else NullAdapter
end

Then, response_adapter is called from the render_* methods living inside the controllers.

Example :

class PasswordsController
  def render_create_error_not_allowed_redirect_url
    response_adpater.create_error(...)
  end
end

Technical note : Each adapter will inherit from the NullAdapter. This adapter throw an exception warning about missing implementation.

Possible enhancement

We can push the concept forward by adding the support of other gems such as ActiveModel::Serializer (AMS) by adding a look-up in the adapters they tend to work with.

For example, we could have the JsonApiAdpater looking for AMS :

class JsonApiAdapter
  def call
    if defined?(ActiveModel::Serializer)
       JsonApiAdapter::AMS
    else
      JsonApiAdapter::Base
    end
  end
end

This way we can rely on the support of the most popular gem in the community instead of trying to re-implement the wheel. But this need to be discussed as it doesn't seem to be necessary neither.

Conclusion

This architecture tends to be designed with SRP (Single Responsibility Pattern) in mind and made to be easy to extend, not creating constraint.

With this changes in the architecture, we could easily implement the JSON API adapter and even allow custom adapter creation.

What do you think about that ?

ValentinTrinque commented 7 years ago

@booleanbetrayal Can I have a feedback on that ? I am really willing to help you on this project. Plus I want to add the support on EmberJS.

I would really appreciate something from you.

zachfeldman commented 7 years ago

@ValentinTrinque I read your work above. I like this idea! I think it would allow us to gradually introduce JSON API and any other desired adaptors. Feel free to do some work if this is still relevant to you. Otherwise you can also feel free to close this issue.