Add support for ignoring client generated IDs

[nikajukic]

Solves issue #48

Added configurable client_id_prefix parameter which is taken into consideration when parsing parameters.

All IDs starting with client_id_prefix will be ignored.

This feature gives you the option to create new AND update existing nested records on parent update.

{
  "type": "multitracks",
  "attributes": {
    "title": "Multitrack"
  },
  "relationships": {
    "tracks": {
       "data": [
        {
          "type": "tracks",
          "id": "123"                           // Existing ID for existing resources
        },
        {
          "type": "tracks",
          "id": "cid_new_track"                 // Client ID for new resources -> needs to match ID in included below
        }
       ]
    }
  },
  "included": [
    {
      "id": "123",                              // Existing ID for existing resources
      "type": "tracks",
      "attributes": {
        "name": "Piano"
       }
    },
    {
      "id": "cid_new_track",                    // Client ID for new resources -> needs to match ID in relationships below
      "type": "tracks",
      "attributes": {
        "name": "Drums"
       }
    }
  ]
}

params.from_jsonapi

{
  "multitrack" => {
    "title" => "Multitrack",
    "tracks_attributes" => {
      "0" =>  {
         "id" => "123",
         "name" => "Piano"
      },
      "1" =>  {                                 // No ID is present, so ActiveRecord#update correctly creates the new instance
         "name" => "Drums"
      }
    }
  }
}

[greena13]

Thanks for all your efforts, @nikajukic. This looks like a great solution and would solve our needs discussed in the original issue.

Thinking of other users of the library, however, having a predefined prefix may be an unnecessarily breaking change. Possibly a default value of nil with a guard clause would allow a smoother upgrade path for library users, while allowing those who want to opt-in to the behaviour, the ability to do so.

The current implementation also doesn't provide a way to opt out of this behaviour, either.

Something like:

def client_generated_id?(related_id)
  return false unless JsonApi::Parameters.client_id_prefix

  related_id.to_s.starts_with?(JsonApi::Parameters.client_id_prefix)
end

Presuming a prefix may also be unnecessarily prescriptive, and perhaps it would be better to allow specifying a lambda:

module JsonApi
  module Parameters
    @ensure_underscore_translation = false
    @ignore_id_filter = (id) -> { false }

    class << self
      attr_accessor :ensure_underscore_translation
      attr_accessor :ignore_id_filter
    end
  end
end

# ...
def build_included_object(included_object, related_id)
  included_object_base(included_object).tap do |body|
    body[:id] = related_id unless JsonApi::Parameters.ignore_id_filter(related_id)
  # ...

[choosen]

Thanks for your opinion @greena13
I would add some additional information
Instead of cid_ prefix in examples, we should use lid_ as it is stated in draft of json api 1.1 (draft but still better than custom 😄 ) https://jsonapi.org/format/1.1/#document-resource-object-identification

I think it's fair point to disable it as default for backward compatibility.

And we thought about renaming prefix to ignore_ids_with_prefix, but @greena13 @ignore_id_filter is also good.
I am not sure if we should provide lambda, such elasticity is not needed probably.
What do you think issue#48 participants❔ 🎛️

[greena13]

I'm happy with any of the possibilities you've outlined.

The minimal set of changes appears to be changing from cid_ to lid_ (with it being part of the draft standard, I rescind any concerns about introducing "breaking changes" that could not be disabled, and would probably just suggest a major increment instead).

[greena13]

Actually, we have some legacy clients for which the breaking change of moving from cid_ to lid_ would present a problem.

Retaining the ability to specify an alternative prefix would remain very valuable for us.

[nikajukic]

Thanks for your comments and suggestions 👍

I think the best solution is the one @greena13 provided, with setting a default client_id_prefix to nil and checking if it's defined.
I also like the ignore_ids_with_prefix config name instead of client_id_prefix.

Regarding the lambda, I'm not sure if it's needed here. I think having the prefix config option is good enough, but if you insist, we could add a lambda filter as well.

[greena13]

Regarding the lambda, I'm not sure if it's needed here. I think having the prefix config option is good enough, but if you insist, we could add a lambda filter as well.

To help focus the remaining conversation, I'll rescind my original suggestion for a lambda (it won't currently serve our needs and if absolutely needed, consumers of the library can override the appropriate method - as one would expect when deviating from the intended usage of a library).

[greena13]

@nikajukic have there been any updates on this since last month?

[choosen]

@greena13 actually we should use lid as local id key instead of id with the prefix according to draft.
it solves some problems but requires some additional changes for frontend.
How do you see that change ?

[greena13]

@choosen this is a very good point and a distinction that was somehow missed (did the proposal change, or was I just not attentive enough the first time?)

I would imagine at first thought, using a additional id field to match resources on (but then discard it) would be a different approach/algorithm that should be done independently and regardless of this issue.

But then we still have this separate issue of wanting to migrate legacy clients that use a value prefix to determine local only IDs. It doesn't perhaps feel like it should be part of the critical functionality of the gem, but is there some way of separating out the algorithm for including ids into a method or module that can be easily extended and overridden, so we could provide users with instructions on how to support this legacy workaround if needed?

[nikajukic]

@greena13 @choosen sorry for the late response and feature update.

I've applied the changes you wanted:

• renamed client_id_prefix to ignore_ids_with_prefix
• set default value for ignore_ids_with_prefix config to nil
• updated tests

Can you please check how does it look now?

[choosen]

@greena13 what do you think ?

[greena13]

Thanks @nikajukic and @choosen . This looks good to me as a solution to the problem as originally stated, and this will likely prove a valuable tool for consumers of the library looking to upgrade or move to something more standard.

However, long term, I expect to move to a lid (separate attribute) based solution. Although waiting for the specification to exit draft status, and/or all of the various serialisation/deserialisation libraries used in the particular stack I'm working with, to support it, will take some time yet.

So 👍 as a likely useful stop-gap in the interim, and possibly for other uses I haven't anticipated in the long term.

[choosen]

Some small change to readme only.
I think we can leave spec as they are. Don't have to align them with readme example

[choosen]

Suggested change

	You can specify ignore_ids_with_prefix:
	```
	JsonApi::Parameters.ignore_ids_with_prefix = 'client_'
	```
	ignore_ids_with_prefix is by default set to `nil`
	If defined, all IDs starting with `JsonApi::Parameters.ignore_ids_with_prefix` will be removed from params.
	In case of creating new nested resources, client will need to generate IDs sent in `relationships` and `included` parts of request.
	```
	{
	"type": "multitracks",
	"attributes": {
	"title": "Multitrack"
	},
	"relationships": {
	"tracks": {
	"data": [
	{
	"type": "tracks",
	"id": "cid_new_track" // Client ID for new resources -> needs to match ID in included below
	}
	]
	}
	},
	"included": [
	{
	"id": "cid_new_track", // Client ID for new resources -> needs to match ID in relationships below
	"type": "tracks",
	"attributes": {
	"name": "Drums"
	}
	}
	]
	}
	```
	```
	params.from_jsonapi
	{
	"multitrack" => {
	"title" => "Multitrack",
	"tracks_attributes" => {
	"0" => { // No ID is present, so ActiveRecord#create correctly creates the new instance
	"name" => "Drums"
	}
	}
	}
	}
	```
	In case of updating existing nested resources and creating new ones in the same request, client needs to generate IDs for new resources and use existing ones for existing resources. Client IDs will be removed from params.
	```
	{
	"type": "multitracks",
	"attributes": {
	"title": "Multitrack"
	},
	"relationships": {
	"tracks": {
	"data": [
	{
	"type": "tracks",
	"id": "123" // Existing ID for existing resources
	},
	{
	"type": "tracks",
	"id": "cid_new_track" // Client ID for new resources -> needs to match ID in included below
	}
	]
	}
	},
	"included": [
	{
	"id": "123", // Existing ID for existing resources
	"type": "tracks",
	"attributes": {
	"name": "Piano"
	}
	},
	{
	"id": "cid_new_track", // Client ID for new resources -> needs to match ID in relationships below
	Library will add relations to record. Nested resources will receive attributes from _included_ section by `id` match.
	By default id field is treated as Client-Generated ID, so it will identify record in database.
	If you want to add related object attributes without providing Client-Generated ID then
	you can specify ignore_ids_with_prefix:
	```ruby
	JsonApi::Parameters.ignore_ids_with_prefix = 'local_id_' # library default is nil
	```
	If defined, all IDs starting with `local_id_` will be removed from parsed params.
	In case of creating new nested resources, client will need to generate local IDs sent in `relationships` and `included` parts of request.
	```js
	{
	"type": "multitracks",
	"attributes": {
	"title": "Multitrack"
	},
	"relationships": {
	"tracks": {
	"data": [
	{
	"type": "tracks",
	"id": "local_id_new_track" // Local ID for new resources -> needs to match ID in included below
	}
	]
	}
	},
	"included": [
	{
	"id": "local_id_new_track", // Local ID for new resources -> needs to match ID in relationships above
	"type": "tracks",
	"attributes": {
	"name": "Drums"
	}
	}
	]
	}
	```
	```js
	params.from_jsonapi
	{
	"multitrack" => {
	"title" => "Multitrack",
	"tracks_attributes" => {
	"0" => { // No ID is present, so ActiveRecord#create correctly creates
	"name" => "Drums" // the new instance without Client-Generated ID
	}
	}
	}
	}
	```
	In case of updating existing nested resources and creating new ones in the same request, client needs to generate local IDs for new resources and use existing ones for existing resources. Local IDs will be removed from params.
	```js
	{
	"type": "multitracks",
	"attributes": {
	"title": "Multitrack"
	},
	"relationships": {
	"tracks": {
	"data": [
	{
	"type": "tracks",
	"id": "123" // Existing ID for existing resources
	},
	{
	"type": "tracks",
	"id": "local_id_new_track" // Local ID for new resources -> needs to match ID in included below
	}
	]
	}
	},
	"included": [
	{
	"id": "123", // Existing ID for existing resources
	"type": "tracks",
	"attributes": {
	"name": "Piano"
	}
	},
	{
	"id": "local_id_new_track", // Local ID for new resources -> needs to match ID in relationships above
	```

[choosen]

@nikajukic Can I fix those and merge it ?

[nikajukic]

@choosen Sorry, I thought you already did it above, now I see it was only a suggestion 🙈

Sure, go for it!

[choosen]

JSON:API v1.1 was finalized September 30, 2022! 🎉
CC: @greena13