Feature request: Option for ignoring client-side ids

[greena13]

Thank you for this library; it has saved us a lot of time.

If my reading of this previous issue is correct, it appears that initially this library started out with ignoring ids entirely, including those generated client-side, but support for ids was added for those who require it. However, I don't see an option to opt-out of this behaviour.

The spec permits ignoring client-generated IDs:

A server MAY accept a client-generated ID along with a request to create a resource.

and it would seem the transforming functionality that jsonapi_parameters provides, is a natural (and efficient) point to do that.

The particular issue we have is that we need POST and PATCH requests that support "nested attributes", which as part of the client serialization process get split out into relationships and included - necessitating client-generated ids to link them together until they're saved on the server and given server-allocated ids.

However, we don't want these ids making their way to ActiveRecord methods like create and update (which erroneously cause Rails to attempt to find and update records with those ids - resulting in ActiveRecord::RecordNotFound exceptions being thrown).

We can't simply ignore or reject client-generated ids as part of the Strong Parameters permit method, because our PATCH requests can include a mixture of client-generated ids (for new associated records) and server-side ids (for existing associated records).

The solution we're currently going with is to define custom handlers for to_one and to_many that strip out ids that are conformant with a particular convention for client-generated ids, but I suspect this is likely a use-case others may encounter and that it would be beneficial for the library to support a regular expression that would cause matching ids to be stripped out:

JsonApi::Parameters.blacklist_ids = /^client_id_/

Once I've confirm you may be receptive to such a feature, I don't mind working on a pull request.

Thanks.

[Marahin]

Hi! Thanks for reaching out.

First of all I'm supper happy to hear there are people using custom handlers :)

I must say I need some clarification on the issue you're having. My assumption here is that you're having a payload with included relationships, and JsonApi::Parameters is properly parsing the values including the ids that were generated / provided on the frontend. Similar example: https://github.com/visualitypl/jsonapi_parameters/wiki/Relationships#with-included-entity

It took me a while, but this is my current understanding. The issue stands within the nested ID, not the top-level resource (for an instance, you'd have an Author model with related Posts, and you'd like to create OR update Posts through the author). Skipping the nested posts_attributes: [:id, :name] and instead just passing posts_attributes: [:name] would probably work here, but if I got it right, the payload may consist of already existing records (where id is good, because we update something that is already in the database) with things that are not yet persisted, but also include an id (a frontend generated one just for linking purposes). Passing client-generated ID that is not persistent in the database will yield the error, while this is a clearly new record to be saved.

If my understanding is correct, this seems like an interesting use case that was probably not tested before. I think in this case it would be super cool if you could provide payload examples.

Let me know if I got it right and we can start thinking on how to resolve that issue :)

[greena13]

Thanks for getting back to me, @Marahin.

Yes, you seem to have it correct.

Please see below example of use-case:

class Post < ApplicationRecord
  has_many :quotations

  accepts_nested_attributes_for :quotations
end

class Quotation < ApplicationRecord
  belongs_to :post
end

The client has UI for updating an existing post with new quotations:

{
  type: 'post',
  body: 'These are my favourite quotes:'
  quotations: [
    {
      type: 'quotation',
      id: 123,
      body: 'Really old wisdom'
    },
    {
      type: 'quotation',
      body: 'Brand new insights'
    },
  ]
}

To serialize this into the JSON API format, quotations must be split into relationships and included:

{
  "type": "post",
  "attributes": {
    "body": "These are my favourite quotes:"
  },
  "relationships": {
    "quotations": {
       "data": [
        {
          "type": "quotation",
          "id": "123"                           // Server-allocated id of existing Quotation
        },
        {
          "type": "quotation",
          "id": "cid_quote1"                   // Client id required to match with included below
        }
       ]
    }
  },
  "included": [
    {
      "id": "123",
      "type": "quotation",
      "attributes": {
        "body": "Really old wisdom"
       }
    },
    {
      "id": "cid_quote1",                      // Required id to match with relationships above
      "type": "quotation",
      "attributes": {
        "body": "Brand new insights"
       }
    }
  ]
}

from_jsonapi needs this matching id to correctly re-assemble the quotation attributes:

params.from_jsonapi:

{
 "post" => {
    "body" => "These are my favourite quotes:",
    "quotations_attributes" => {
      "0" =>  {
         "id" => "123",
         "body" => "Really old wisdom"
      },
      "1" =>  {                                   # No id is present, so ActiveRecord#update correctly creates the new instance
         "body" => "Brand new insights"         
      }
    }
 }

Without filtering out client ids:

{
"post" =>  {
    "body" => "These are my favourite quotes:",
    "quotations_attributes" => {
      "0" => {
         "id" => "123",
         "body" =>"Really old wisdom"
      },
      "1" => {
        "id" => "cid_quote1",                    # Causes ActiveRecord to look for an existing Quotation with an id of "cid_quote1" - resulting in an ActiveRecord::RecordNotFound exception
        "body" => "Brand new insights"
      }                                        
    }
 }

[Marahin]

Alright, got it.

While I'm not yet sure about the design of blacklisting attributes with regexp, the idea of supporting creation and/or updating in the same request sounds like something that should be introduced.

I'll allow myself some more time to think how we can handle this scenario and will get back to this issue in a few days.

[greena13]

Ok, thanks for the consideration.

If it helps, the way we are currently achieving the desired result in our custom handlers is to recursively delete matching ids from each attributes hash (by overriding #prepare_relationship_vals for to_many and #handle for to_one) after they're constructed.

But the way I was thinking of pursuing a more robust solution would be to simply omit them from being added to the attribute hashes or array of ids in the first place, at the point that each is built (removing the need for any recursive post-processing).

[greena13]

Hi @Marahin,

Did you get a chance to think about this?

Thanks.

[Marahin]

Hey @greena13, indeed I have, but I couldn't think of a design that would work better than what you proposed.

Could you open a PR with the change that would resolve the issue?

[greena13]

Sure. It may be a while before I get one together, though.

[nikajukic]

@greena13 @Marahin I've opened a PR that follows suggested idea. I've tested it thoroughly on a project I'm currently working on and we haven't had any issues with this solution.

[greena13]

This is fantastic news, @nikajukic.

@Marahin is there any chance of getting that pull request reviewed, merged and released any time soon? There's definitely still an appetite for it, on our end.

[Marahin]

@nikajukic @greena13 thanks for reaching out. Yes, we are working on reviewing the PR and this issue. There is an ongoing internal discussion at Visuality's open source commitee that should be followed by action on this matter. I can't give exact details nor date or time when this will happen, as I'm no longer part of the commitee but just supporting the maintenance and development of this gem, but I will do my best to keep it going so this can be resolved :)

[greena13]

Thanks @Marahin, that's much appreciated.

For what it's worth, I've taken a look over @nikajukic's PR and tried to provide some (hopefully) useful suggestions, to help keep this alive.