support iri identification

[wildhaber]

Hi,

First of all I want to mention how much I appreciate your work. It makes working with JSON API resources a real pleasure.

In our project, we are using API-Platform to expose the API and consume them in a vue.js app.

API-Platform is exposing the id of a JSON API resource in the format of a relative IRI Internationalized Resource Identifier. In the end a JSON API resource looks like that:

{
    type: 'entity',
    id: '/entity/id'
}

Which is, as far as I understand the JSON API specs, totally valid.

Every resource object MUST contain an id member and a type member.
The values of the id and type members MUST be strings.

(https://jsonapi.org/format/#document-resource-object-identification)

The small dilemma with IRI's are, when in the next part of the specifications:

Within a given API, each resource object’s type and id pair MUST identify a single,
unique resource. (The set of URIs controlled by a server, or multiple servers acting
as one, constitute an API.)

In fact, the upper statement could result in a no-go of using IRI's in JSON API. But since we were not able to transform the URL in the middleware (See: #110) we made a small change of the URL-Method to support both formats {type}/{id} as well as {iri}.

This small change does not break the strict implementation of JSON API specs at all, but simplifies the integration of other popular frameworks like API-Platform.

Hope this PR would support the jsorm framework.

Keep up the great work 👍

[richmolj]

Hey @wildhaber, thanks for the kind words ❤️ Interesting and well-explained issue. I think I'm good with this, though I want to get @wadetandy's thoughts first.

[wadetandy]

@wildhaber Great issue description. I think this is a good feature to add, but from a performance and general debuggability perspective, instead of inspecting each incoming ID for whether it's following that convention (especially since who knows what crazy other ID patterns might be out there), I'd propose instead adding a static configuration flag that can be checked:

class ApplicationRecord extends JSORMBase {
  static usesFullPathIds = true // or something. open to suggestions on this
}

Then instead of inspecting each incoming id, the url method can just look at usesFullPathIds.

[wildhaber]

@wadetandy / @richmolj - thanks for your really quick response 👍

I've discussed this issue with a colleague of mine and I would like to share our key takeaways.

Confusion about terminology "resource"

I was a bit confused about what a resource is, in terms of JSON API. And was insecure if API-Platform is compliant with the specification. Especially in this part:

Within a given API, each resource object’s type and id pair MUST identify a single,
unique resource. (The set of URIs controlled by a server, or multiple servers acting
as one, constitute an API.)

After reading the thread resource vs entity where it became clear to me, that {type}{id} might not match any URI resource. So I was wrong with the implicit assumption that /{type}/{id} must point to a URI resource. Actually, there can be really crazy patterns out in the wild (as you already mentioned ;-) )

Who knows how to transform the path?

In our understanding, the only player that knows how to transform the type and id into a URL is the model. Since this might vary if for example the client consumes JSON API resources from different API's.

So we had the idea of introducing a new, but optionalresourceTransformer-property in the static part of the model (https://github.com/echonovum/jsorm/commit/a3a5ac55cb345f08396fc4b51c91425f68bf9a9d + https://github.com/echonovum/jsorm/commit/4b22e71f1abe83076403715a8c7486d5233f8b5e) which is a function that takes the id as a parameter and returns a string.

static resourceTransformer: ((id?: string | number) => string) | undefined

So there is just a quick check necessary in the url-method if there is a resourceTransformer-function defined and execute it - or just skip it and do the implied standard concepts.

Looking into an example of a JSON API resource like this:

// example from: https://demo.api-platform.com/books
{
  "data": {
    "id": "/books/20",
    "type": "Book",
    "attributes": {
      "isbn": "9780530239033",
      "title": "Iste modi accusantium autem suscipit quia et et dolorum.",
      "description": "Desc updated",
      "author": "Roslyn Morissette",
      "publicationDate": "2002-09-08T00:00:00+00:00"
    }
  }
}

the jsorm model would look like:

{
    static: {
        jsonapiType: 'Book',
        resourceTransformer(id) {
            return id || '/books';
        }
    },
    attrs: {
        isbn: attr(),
        title: attr(),
        description: attr(),
        author: attr(),
        publicationDate: attr(),        
    }
}

This approach also allows to use the regular .all() / .first()-requests since they would fail in the actual implementation, while they try to load example.com/Book instead of example.com/books.

Misc

Define a global settings static useFullPaths

Ding like suggested, this would not solve the "crazy patterns out there" issue. Which I totally agree with you - I am sure they exist :-) - Let's imagine there are 10+ properties ready for any strategy to support. The url-method would just be a mess.

Furthermore, there would still be no solution for the .all()/.first()-calls. They would fail because there is no id available.

Performance & Ease to debug

In terms of performance, I agree that there are some extra ticks needed to compute the paths, but in my opinion this is negligible in comparison to whats on the pro side. (And, I have never seen a API that runs a couple of thousands of requests per seconds per client so this would become an issue).

On the other hand I totally agree with the argument of the ease to debug. By defining a custom method in the model we can avoid hiding all the magic behind the scenes and create a transparent way of handling custom paths.

---

As a conclusion, I would like to hear what you think about this approach, and if you like I can adjust the PR with the implementation you prefer :-)

[wadetandy]

Thanks for all of this detail. A lot to parse, so forgive me if I misunderstand anything.

I think what you're suggesting is that the only change should be to support static resourceTransformer: (id?: string | number) => string as an optional method that can be defined on a given model? This would allow any consumers with "weird" API formats to do whatever they want without eventually supporting a billion config options or whatever.

The functionality you're describing is essentially how the library already works, but throws out the inheritance features that are already provided by typescript/javascript. You can simply override the url() static method to achieve what you want. Consider your example:

const Book = ApplicationRecord.extend({
    static: {
        jsonapiType: 'Book',
        resourceTransformer(id) {
            return id || '/books';
        }
    },
    attrs: { /***/ }
})

I could achieve the same thing right now by swapping out resourceTransformer with url:

const Book = ApplicationRecord.extend({
    static: {
        jsonapiType: 'Book',
        url(id) {
            return id || '/books';
        }
    },
    attrs: { /***/ }
})

Your patch in https://github.com/echonovum/jsorm/pull/2 is saying "if this method is defined, call it instead and short circuit this behavior", which is exactly what you'd get if you overrode the url method instead (if you wanted to conditionally fallback to parent-behavior, you could call ApplicationRecord.url() within your custom url() method).

Please let me know if this works, and if so, help me to identify what if anything we can do to improve here.

[wildhaber]

hey @wadetandy ,

Thanks for your response, yes, this is exactly what I was trying to achieve. Since I'm not really familiar with typescript I was not aware that it works that way.

I will give the overriding a try next week and let you know if this is already enough 👍

The only thing that is a difference, is that the model now needs to be aware of the "base path" while the approach with the extra resourceTransformer just converts the path after the base path. Or it will simply not be allowed to use arrow functions to access the proper this-context to get the base path elements. I'm not yet sure which way I prefer, but at least you helped me to understand the situation ;-)