Simplification suggestions for easier understanding

[jkeen]

I'm building an API-only application in Rails with an Ember frontend and was surprised at how difficult it was to find something that let rails speak and understand json-api responses and requests. It felt like something I should be able to add a gem for and have it do Do The Right Thing™ without having to set up anything, but I was wrong.

I tried out several different options before coming across jsonapi-suite, and this is the best I've found and I'm super impressed how comprehensive it is!

But I've had some real trouble wrapping my head around everything that's going on and it got me digging into the source of the collection of gems and wishing it all felt more self-explanatory. So I thought I'd share the points I got hung up on, and throw in my two cents on possible improvements (and a future PR possibly) so I don't go off and try to build yet another json-api gem solution (relevant xkcd).

Don't get me wrong, the documentation and code comments are plentiful, but one of the reasons I like ruby/rails so much is because of the whole philosophy of having the code explain itself. And I'm a user-experience focused idealist which explains how I've derailed my productivity entirely this week wondering things like "Is resource the best name?" instead of just moving along with my project and accepting things the way they are. But I think a few key naming changes might make the whole suite so much easier to grok and allow people like me (who tend to avoid documentation and lengthy code comments) an easier time.

If we were just speaking normal json, some typical rails controller actions might look like this:

# GET
def show
  #1. Find Post
  @post = Post.find(params[:id])

  #2. Render post as json
  render json: @post # transformed into json
end

# POST
def create
  #1. Whitelist params
  post_params = params.require(:title, :author, :body)

  #2. Create post with params
  @post = Post.new(post_params)

  #3. Return status of operation
  if @post.save
    render json: @post, status: :created, location: @post
  else
    render json: @post.errors, status: :unprocessable_entity
  end
end

This all feels pretty normal, and going into this endeavor all I wanted a json-api gem to do was to convert the incoming json-api payload into something rails understood automatically and then convert the outgoing payload into json-api format leaving the controller looking almost the same as the default above. I understand that the suite does so more than just that, and in order to handle the nested creates and other features it's undoubtedly going to be a little more involved.

But the things that tripped me up going into the default controller setup for this were as follows:

class PostsController < ApplicationController
  jsonapi resource: PostResource

  strong_resource :post do
    has_many :comments, disassociate: true, destroy: true do
      belongs_to :post, disassociate: true, destroy: true
    end
  end

  before_action :apply_strong_params, only: [:create, :update]

1. What's the difference between the strong resource and the other resource? (answered it myself eventually, but doubling up on terms there tripped me up).
2. I defined strong_resources in an initializer, why do I need to define the has_many, belongs_to relationships with disassociate, destroy, etc in the controller? (still don't quite understand this, really)
3. Is strong params something different? What's this doing (answered it myself eventually)

  def index
    posts = Post.all
    render_jsonapi(posts)
  end

  def show
    scope = jsonapi_scope(Post.where(id: params[:id]))
    instance = scope.resolve.first
    raise JsonapiCompliable::Errors::RecordNotFound unless instance
    render_jsonapi(instance, scope: false)
  end

4. What is scope: false doing? I see that quite a bit.
5. I defined a PostResource, assigned it to the controller, and it knows we're dealing with Post models, how is the controller using that if I'm still having to call into the Post model directly?

  def create
    post, success = jsonapi_create.to_a

    if success
      render_jsonapi(post, scope: false)
    else
      render_errors_for(post)
    end
  end

  def update
    post, success = jsonapi_update.to_a

    if success
      render_jsonapi(post, scope: false)
    else
      render_errors_for(post)
    end
  end

  def destroy
    post, success = jsonapi_destroy.to_a

    if success
      render json: { meta: {} }
    else
      render_errors_for(post)
    end
  end

6. Why do each of those calls have an explicit .to_a call? If that’s the expected return, why not just have the internals do that?

As I dug through the source code I understood better what was going on, but sketched out a possibility that might make things feel less opaque and also help reduce the need for explanatory comments.

class PostsController < ApplicationController
  jsonapi resource: PostResource, allow_sideload: [:comments]

  before_action :validate_request_params, only: [:create, :update]

  def index
    resources = find_resources(deserialized_params)
    render json: resources.response
  end

  def show
    resource = find_resource(params[:id])

    if resource.found?
      render json: resource.response
    else
      render json: resource.errors, status: :not_found
    end
  end

  def create
    resource = build_resource(deserialized_params)

    if resource.save # persist with relationships, returns boolean
      render json: resource.response, status: :created, location: resource.location
    else
      render json: resource.errors, status: resource.status
    end
  end

  def update
    resource = find_resource(params[:id])

    if resource.found? && resource.update(deserialized_params) # persist with relationships, returns boolean
      render json: resource.response, status: :ok, location: resource.location
    else
      render json: resource.errors, status: resource.status
    end
  end

  def destroy
    resource = find_resource(params[:id])

    if resource.found? && resource.destroy
      render json: { meta: {} }
    else
      render json: resource.errors, status: resource.status
    end
  end
end

The main change here is having a new object that manipulates resources behind the scenes that the consumer of this gem interacts directly with, making it clearer on what's going on and much closer to the rails default pattern that everyone's familiar with. I implemented a hacky/incomplete version of it here

I think this pattern might have some potential.

Second piece of setup simplification that I haven't dug into as much yet is the serializer, resource, and strong_resource piece (some of the points of confusion I covered above), but is there a reason those three concepts couldn't be combined? Currently the resource object's type needs to match the serializer object's type but they're in separate files and some of the interaction/terminology was tripping me up.

What if it were something like this, and you only had to define one file/class?

class PostResource
  type :post
  model Post

  request do
    validate_request_params do
      allow :title, :string
      allow :author, :string
      allow :published_date, :date_time
    end

    allow_sideload :comments
  end

  response do
    attribute :title
    attribute :author
    attribute :published_date
    attribute :created_at
    attribute :updated_at
    has_many :comments
  end
end

Anyway, that was much longer than I intended it to be and it's all I have for now! Great work on this thing overall, and I hope I can help out somehow!

[richmolj]

Hey @jkeen!

Thanks for the constructive comments ❤️ I agree with a lot of what you've put here.

It felt like something I should be able to add a gem for and have it do Do The Right Thing™ without having to set up anything, but I was wrong.

I started the same way 🙂 My original intent was less to create a gem that does the right thing, and more to collect a bunch of existing gems and record the wiring code to make them play nicely together (hence "suite"). This is why you see things liketype repeated in the serializer - it's a separate library and originally I was going for the minimal amount of "magic" to make things work, so we could get out of dev's way. It was also an attempt to be less-prescriptive and see what solutions would naturally evolve before committing to another layer of abstraction.

And there is also just a lot to do 😅 So I appreciate the help!

That said, we've had enough usage (open-source and internally within my company) that it's time to take these learnings, sand a lot of these edges and commit to a more cohesive whole. I've actually been doing some work independently to this end, and plan some major revamps in a 1.0.0-dev branch soon (like, really soon). I'm excited to finally get here.

Some of your suggestions may hit technical or other limitations, but I think there's a lot we can take from this. On some specific points:

I defined strong_resources in an initializer, why do I need to define the has_many, belongs_to relationships with disassociate, destroy, etc in the controller? (still don't quite understand this, really)

The initializer holds templates, the controller references those templates. In other words, the controller can limit what level of nesting is allowed in sideposts, or what attributes should be allowed in create but not update. I agree with your comments on the word resource - it makes more sense from a library perspective than a user perspective.

What is scope: false doing? I see that quite a bit.

By default render_jsonapi will try to "scope" what you pass in (apply filters, sorting, etc). scope:false says, "leave me alone, I already did it myself". I like the fundamentals here but not the naming, and we can probably get away with avoiding the option or elsewise handling it.

I defined a PostResource, assigned it to the controller, and it knows we're dealing with Post models, how is the controller using that if I'm still having to call into the Post model directly?

The premise here is that the controller defines a "default scope" that is then passed to the Resource. So /comments would start with Comment.all but /top_comments could start with Comment.where("likes > 500"). Controllers adjust the behavior of a Resource, but this isn't well spelled out (and took some time to evolve this way).

Why do each of those calls have an explicit .to_a call? If that’s the expected return, why not just have the internals do that?

This was mostly to try and get the controller looking Rails-y as possible, but I've never been wild about it. I like your controller visually, but methods like find_resource return a ResourceRequest object and not a Resource, which also seems confusing. But I like the direction you are going down; I'd like to play with it a bit as part of a more fundamental refactor and see if we can land something along these lines.

only had to define one file/class?

This is the area I've been focusing my attention recently. I agree with the high-level premise, but what happens when you have custom serialization logic? Now our logic for serialization and persistence, not to mention filtering etc is all dumped into one god class. My current thought is for attributes to be defined here, but with ways to punt to other objects/classes when you need to customize. I've been getting some good feedback on initial thoughts (which I could share on Slack if interested) and think this area will be the beginning of 1.0.0-dev.

Again, great feedback! I'd like to get some of the foundational ball rolling first, then hopefully continue this discussion in 1.0.0-dev. Slack me if this is something you'd like to help with.

[jkeen]

Alright, great insight!

but methods like find_resource return a ResourceRequest object and not a Resource, which also seems confusing.

Yeah, that naming was hard. I really wanted it to be a Resource object, but that was already taken and it looked like more work (for POC anyway) to stuff all that logic inside the resource itself.

By default render_jsonapi will try to "scope" what you pass in (apply filters, sorting, etc). scope:false says, "leave me alone, I already did it myself". I like the fundamentals here but not the naming, and we can probably get away with avoiding the option or elsewise handling it.

Couldn't we detect if it's already scoped and do the right thing?

I'll hit ya up on slack to join that group and we can chat about the 1.0.0-dev stuff

[richmolj]

Couldn't we detect if it's already scoped and do the right thing?

I had done this at some point, but it was introducing certain headaches so I opted to be explicit. I agree we should be doing something like that in 1.0 though

[richmolj]

Done with Graphiti!