Problem/Motivation

We want to:

  1. Be compatible with JSON:API 2.x.
  2. Be compatible with JSON:API Extras 3.x.
  3. Have image styles on the image entities.
  4. Have image styles on the relationship items pointing to image entities.
  5. Have configurability based on the Consumer making the request.
  6. Refine the list of image styles on the relationship using JSON:API Extra's UI (not reinventing the wheel).

Proposed resolution

  1. Create a service to build the links for an image, given a list of configured image styles.
  2. Use Field Enhancers to provide field-level configurability.
  3. Override the entity normalizer for image entities following the same pattern in JSON:API Extras.
CommentFileSizeAuthor
#18 3027238--new-version--18.patch38.5 KBe0ipso
#18 3027238--interdiff--4-18.txt2.51 KBe0ipso
#9 3027238--interdiff--3-4.txt16.41 KBe0ipso
#9 3027238--new-version--4.patch38.32 KBe0ipso
#3 3027238--new-version--3.patch27.2 KBe0ipso

Comments

e0ipso created an issue. See original summary.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

Pasting from Slack.

e0ipso [31 minutes ago]
This is a thread for the upcoming release of _Consumer Image Styles_ which will be compatible with _JSON:API 2.x_.


e0ipso [30 minutes ago]
I know @wimleers and @gabesullice will be interested on this.


e0ipso [27 minutes ago]
The idea is to provide 2 features:
 :one: List of image style URLs on the Image entity normalizations based on the negotiated Consumer on every request. The configuration of the consumer contains the list of available styles.
 :two: List of image style URLs on the relationship _item_ of type image. The relationship can be hosted on any resource type with a reference to an image. This will be accomplished using field enhancers.


e0ipso [23 minutes ago]
The following example is an `articles` resource that contains an `image` relationship. The image relationship is configured to use the _Image Styles Field Enhancer_ using JSON:API Extras. This field enhancer allows having the image style URLs on the image field reference, avoiding an unnecessary include of the image entity in order to get the image style URLs.


e0ipso [22 minutes ago]

GET /api/articles?fields[articles]=image& include=image& page[limit]=1 HTTP/1.1
Host: vanilla-drupal.localhost
Accept: application/vnd.api+json
X-Consumer-ID: 2e41b077-9da7-4a63-ba22-7cef33086b40

e0ipso [22 minutes ago]


{
    "data": [
        {
            "type": "articles",
            "id": "c5565b94-5c23-479f-8eb6-e8efc707076c",
            "links": {
                "self": {
                    "href": "http://vanilla-drupal.localhost/api/articles/c5565b94-5c23-479f-8eb6-e8efc707076c"
                }
            },
            "relationships": {
                "image": {
                    "data": {
                        "type": "file--file",
                        "id": "43c0b045-13a3-4aff-b33f-7abb360b83b9",
                        "meta": {
                            "alt": "Fresh cut herbs including mint, parsley, thyme and dill",
                            "title": null,
                            "width": 768,
                            "height": 512,
                            "links": {
                                "thumbnail": {
                                    "href": "http://vanilla-drupal.localhost/sites/default/files/styles/thumbnail/public/home-grown-herbs.jpg?itok=DZLyNhq8",
                                    "meta": {
                                        "rel": [
                                            "drupal://jsonapi/extensions/consumer_image_styles/links/relation-types/derivative/"
                                        ]
                                    }
                                }
                            }
                        }
                    },
                    "links": {
                        "self": {
                            "href": "http://vanilla-drupal.localhost/api/articles/c5565b94-5c23-479f-8eb6-e8efc707076c/relationships/image"
                        },
                        "related": {
                            "href": "http://vanilla-drupal.localhost/api/articles/c5565b94-5c23-479f-8eb6-e8efc707076c/image"
                        }
                    }
                }
            }
        }
    ],
    "links": {
        "last": {
            "href": "http://vanilla-drupal.localhost/api/articles?fields%5Barticles%5D=image&include=image&page%5Boffset%5D=7&page%5Blimit%5D=1"
        },
        "next": {
            "href": "http://vanilla-drupal.localhost/api/articles?fields%5Barticles%5D=image&include=image&page%5Boffset%5D=1&page%5Blimit%5D=1"
        },
        "self": {
            "href": "http://vanilla-drupal.localhost/api/articles?fields%5Barticles%5D=image&include=image&page%5Blimit%5D=1"
        }
    },
    "included": [
        {
            "type": "file--file",
            "id": "43c0b045-13a3-4aff-b33f-7abb360b83b9",
            "links": {
                "self": {
                    "href": "http://vanilla-drupal.localhost/api/file/file/43c0b045-13a3-4aff-b33f-7abb360b83b9"
                },
                "max_650x650": {
                    "href": "http://vanilla-drupal.localhost/sites/default/files/styles/max_650x650/public/home-grown-herbs.jpg?itok=PDuwxS33",
                    "meta": {
                        "rel": [
                            "drupal://jsonapi/extensions/consumer_image_styles/links/relation-types/derivative/"
                        ]
                    }
                }
            },
            "attributes": {
                "uri": {
                    "value": "public://home-grown-herbs.jpg",
                    "url": "/sites/default/files/home-grown-herbs.jpg"
                },
            },
            "relationships": {}
        }
    ]
}
(edited)


e0ipso [17 minutes ago]

@gabesullice, @wimleers this follows the linking proposal we have going on. You'll note the presence of extra links in the included image entity. A client could determine it's a downloadable image style URL by the presence of the specific `rel`. Otherwise it's a regular link.


e0ipso [15 minutes ago]

the most icky part (and this is a known problem) is that relationships can have links, but relationship *items* cannot (according to the spec). In other words, a resource linkage object cannot have links. That's why the links are placed in the `"meta"` on the _image_ relationship item.


e0ipso [15 minutes ago]

(NOTE: the example response above has been pruned for readability reasons)


e0ipso [12 minutes ago]

Thoughts, impressions? I am trying to be forward compatible, if possible.


e0ipso [< 1 minute ago]

I have pasted this conversation in the corresponding drupal.org issue.
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Active » Needs review
Issue tags: +API-First Initiative
StatusFileSize
new 3027238--new-version--3.patch27.2 KB

First patch draft. This creates the output described above.

This doesn't adapt any of the tests, so expect red.

szeidler’s picture

szeidler commented

Thanks for your great effort @e0ipso.

I tested patch #3 with JSON:API 2.x and JSON:API Extras 3.x. It seems simply to work like a charm!

Derivatives for image fields on nodes are exposed directly. Derivatives for media (image) field on nodes are exposed when including the media field. Refining the image style list with JSON:API Extras also worked. Perfect!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
  1. 👍 Computed links instead of attributes (which is what I was doing at #3007268-13: Make Consumer Image Styles compatible with JSON API 2.0-beta2)
  2. 👍 Using the ability to add extra links, as is being introduced in #2994193: [META] The Last Link: A Hypermedia Story
gabesullice’s picture

gabesullice
Primary language English
commented

Overall, I like the idea. But, like you said, the ickiest bit is that the links are under the meta bit of the relationship object.

I love the use of an extension link relation type. FWIW, those aren't actually serialized under the link meta right now, since I have https://github.com/json-api/json-api/pull/1348 open, which would serialize it elsewhere.

I see that you've got derivatives on the relationship object and on the included image, but they're not the same. I don't understand that.

Last thought: why use the key links as all? Why not have a links object with a key of imageDerivatives? There's no particular spec reason it has to be links. Having the camel-casing makes it profile-compatible and has the nice side-effect that it doesn't appear so blatantly "non-spec'd".

Alternatively, you could have an imageDerivatives object, which itself has a links object. This gives you the most flexibility because it'll permit you to have non-link keys/values if they ever become necessary or desirable.

gabesullice’s picture

gabesullice
Primary language English
commented

Passing thought:

If you could add imageDerivatives as a computed property of the image field type (either by altering or extending), then it would work for REST, JSON:API and (presumably) GraphQL. You'd have to consider if that is worth the effort.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

Thanks all for your feedback!

why use the key links as all?

I was hoping that a client could then leverage a link parser module if we ensured links compliance. Do you think it's better to stick them out?

I see that you've got derivatives on the relationship object and on the included image, but they're not the same. I don't understand that.

You can specify the image styles to apply for a given consumer. However since JSON:API Extras gives you extra configurability for fields, you can refine those on a field-by-field basis if necessary. TL;DR if you don't manually add the field enhancer to a file field you won't get field level image styles.

So why the field-level image styles? For better UX and performance. Typically images are hosted on other entities via relationships, so we should skip the includes for the most common task of all in that kind of relationship. That will lower parsing complexity, reduce response size and improve server performance.

So why not only add those in the entity reference then? Because then you can't select an entity with its image styles. IIRC the core patch for image styles misses this.

If you could add imageDerivatives as a computed property of the image field type (either by altering or extending), then it would work for REST, JSON:API and (presumably) GraphQL. You'd have to consider if that is worth the effort.

I had the same temptation. I think the core patch takes this approach. The problem is that you will end up generating those image style URLs (a potentially costy operation) whenever you are loading the host entity, which can lead to very bad performance (given the right combination of popular modules).

Alternatively, you could have an imageDerivatives object, which itself has a links object. This gives you the most flexibility because it'll permit you to have non-link keys/values if they ever become necessary or desirable.

Would you be kind enough to provide an example?

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
StatusFileSize
new 3027238--new-version--4.patch38.32 KB
new 3027238--interdiff--3-4.txt16.41 KB

This should be green now.

gabesullice’s picture

gabesullice
Primary language English
commented
why use the key links as all?

I was hoping that a client could then leverage a link parser module if we ensured links compliance. Do you think it's better to stick them out?

Alternatively, you could have an imageDerivatives object, which itself has a links object.

Would you be kind enough to provide an example?

I think you might have misunderstood my first question about "why use the key "links" as all?" I didn't mean, don't use links, I meant don't use the key name "links". Just use the key name imageDerivatives, the value for that key would be identical to a links object and a parser should handle it just fine.

So, this:

"meta": {
  "imageDerivatives": {
    "thumbnail: {"href": "..."},
    "hero: {"href": "..."}
  }
}

Instead of this:

"meta": {
  "links": {
    "thumbnail: {"href": "..."},
    "hero: {"href": "..."}
  }
}

And finally, "an imageDerivatives object, which itself has a links object":

"meta": {
  "imageDerivatives": {
    "links": {
      "thumbnail: {"href": "..."},
      "hero: {"href": "..."}
    }
  }
}

^ This last one is my favorite because you could have a profile for it + you still have the key name links which will be more familiar to users + it's more resilient to change (f.e. you could add a "recommendedDerivative": "thumbnail" w/o breaking BC)

  1. +++ b/src/ImageStylesProvider.php
@@ -18,6 +18,8 @@ use Drupal\image\ImageStyleInterface;
+  const DERIVATIVE_LINK_REL = 'drupal://jsonapi/extensions/consumer_image_styles/links/relation-types/derivative/';

    Maybe derivative should be a fragment (i.e. /relation-types/#derivative) instead of a path segment. I'm just thinking about documenting these link relations in the future and that might be a nicer pattern. WDYT? 

  2. +++ b/src/ImageStylesProvider.php
@@ -81,9 +83,7 @@ class ImageStylesProvider {
-          'drupal://jsonapi/extensions/consumer_image_styles/links/relation-types/derivative/'
...
+        'rel' => [static::DERIVATIVE_LINK_REL],

    ❤️

dustinleblanc’s picture

dustinleblanc
he/him/his
Primary language English
Location Ithaca, NY
commented

Just chiming in, I tried applying these patches on the module to get rolling with contenta 3.0^ and for some reason, I am not getting any image styles in the responses. My Requests are making use of the includes key and the sub-requests module to cut down on round trips, not sure if that is impacting my results.

dustinleblanc’s picture

dustinleblanc
he/him/his
Primary language English
Location Ithaca, NY
commented

Just spun up a vanilla D8 install with JSON API/Extras and consumer image styles so I can test without our constraints. This works there when I use just a node and an image on that node. In our actual use case, there is a paragraph being included and an image on that paragraph. I just confirmed that if I fetch the paragraph directly, rather than using an includes chain, everything works as it should.

For reference, here is my request structure of the _non_ working request

const routerRequest = {
      requestId: "router",
      action: "view",
      uri: `/router/translate-path?path=${params.vertical}/${params.slug}`
    };
    const pageRequest = {
      requestId: "Page",
      action: "view",
      headers: {
        Accept: "application/json"
      },
      uri: `{{router.body@$.jsonapi.individual}}?include=field_content_sections.image,highlighted_content.field_image`,
      waitFor: ["router"]
    };
    return app.$axios
      .post("/subrequests?_format=json", [routerRequest, pageRequest], {
        headers: {
          "Content-Type": "application/json",
          Accept: "application/json"
        }
      })
      .then(response => {
       // response handling...

Note that this request works like a charm to get me parsable data with all my relationships, except the issue with getting the image styles. I refactored this from a previous JSONAPI 1.x version that used a lot more requests and was able to pull the images.

I think what is required to get the total magic package together is to somehow allow these resources to get transformed when they are included via a more complex include chain.

dustinleblanc’s picture

dustinleblanc
he/him/his
Primary language English
Location Ithaca, NY
commented

The plot thickens!

I had to make sure to set "X-CONSUMER-ID" as a header inside each sub-request.

I now have all the magicks, thanks for your hard work on this @e0ipso!

dustinleblanc’s picture

dustinleblanc
he/him/his
Primary language English
Location Ithaca, NY
commented

The plot thickens!

I had to make sure to set "X-CONSUMER-ID" as a header inside each sub-request.

I now have all the magicks, thanks for your hard work on this @e0ipso!

dustinleblanc’s picture

dustinleblanc
he/him/his
Primary language English
Location Ithaca, NY
commented

The plot thickens!

I had to make sure to set "X-CONSUMER-ID" as a header inside each sub-request.

I now have all the magicks, thanks for your hard work on this @e0ipso!

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

@dustinleblanc ah! Yes, that makes sense. Thanks for digging in and looping back here. How do you think we can improve our docs so the next dev doesn't get tripped by that?

dustinleblanc’s picture

dustinleblanc
he/him/his
Primary language English
Location Ithaca, NY
commented

@e0ipso, I think that the new 3.x branch should just include a section on sub-requests in the readme. I'd be happy to help with that. As a starter something like:

### Usage with the Subrequests Module

The subrequests module can work in conjunction with the consumer image styles module to drastically reduce the number of requests required to render a resource and it's related entities. When issueing subrequests, ensure that the X-CONSUMER-ID header is added to each subrequest or image styles will not be returned in the response. It is insufficient to simply add the header to the 'outer' request.

I'd be happy to submit a patch once the language is good and there is a 3.x branch on D.O or GH to submit it to (I think trying to add to your existing patch here might be confusing?).

Thanks again! It's taking me a bit to wrap my head around how to 'do this right', but modules like this one make the end result much better.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
StatusFileSize
new 3027238--interdiff--4-18.txt2.51 KB
new 3027238--new-version--18.patch38.5 KB

Thanks for the feedback in #10 @gabesullice. I like your suggestions. This patch implements them.

I have not run tests locally. If tests pass, I'll commit to 3.x.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

@dustinleblanc thanks for that paragraph! I'm happy to get that in. Would you mind opening a new ticket for it?

Status: Needs review » Needs work

The last submitted patch, 18: 3027238--new-version--18.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs work » Fixed

I pushed this to 8.x-3.x. Let's follow up on any lingering concerns in new issues.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Fixed » Active

I'm surprised this suddenly got committed, with tests failing :(

Most importantly, it's not clear to me why ImageEntityNormalizer is still needed, why isn't a @FieldEnhancer plugin alone not enough?