New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce Profiles to v1.1 #1268

Merged
merged 30 commits into from Sep 18, 2018

Conversation

Projects
None yet
6 participants
@dgeb
Member

dgeb commented Apr 15, 2018

Note: Although this PR has my name attached to it, it represents a lot of hard work by @ethanresnick in particular over the past several years. Ethan, @wycats, and I have closely collaborated to reach a mutually acceptable solution in an attempt to finally push this over the finish line. This may yet be tweaked a little as some last minute adjustments have yet to be reviewed by Ethan and Yehuda. And of course we welcome feedback from the rest of the community.


Profiles are a means to describe additional semantics the JSON API media type, without altering the basic semantics. In other words, a profile describes a fully-compliant, yet more opinionated, implementation of the spec. It is hoped that profiles will unlock new layers of shared conventions.

A profile can essentially turn every MAY or SHOULD in the base spec into a MUST. Profiles might describe how the filter or page params are used. They might specify that every resource contains a self link, or that resources should include timestamps or versions as either attributes or in meta.

Profiles are identified by URI, which ideally should dereference to a document that describes the semantics of the profile.

One or more profiles may be associated with the JSON API media type through the profile media type parameter. The application of profiles to a particular document can be specified by clients and servers via the Content-Type header. The application of one or more profiles to a response can be requested by a client via the Accept header.

In order to require the application of one or more profiles, the profile(s) must be specified with the profile query parameter. This allows clients to request a profile or profiles and receive a hard failure if they can't be applied.

Any structural elements introduced by a profile can be aliased. This allows for better adaptability and composition of profiles. A new profile descriptor object is introduced which allows for declarations of aliases in a particular document.

Closes #1207
Closes #1195
Closes #762

Introduce Profiles to v1.1
Profiles are a means to describe additional semantics the JSON API media type, without altering the basic semantics.

Profiles are identified by URI, which ideally should dereference to a document that describes the semantics of the profile.

One or more profiles may be associated with the JSON API media type through the `profile` media type parameter. The application of profiles to a particular document can be specified by clients and servers  via the `Content-Type` header. The application of one or more profiles to a response can be requested by a client via the `Accept` header.

In order to require the application of one or more profiles, the profile(s) must be specified with the `profile` query parameter.

Any structural elements introduced by a profile can be aliased. This allows for better adaptability and composition of profiles. A new profile descriptor object is introduced which allows for declarations of aliases in a particular document.

Closes #1207
A client **MAY** use the `profile` media type parameter in conjunction with the
JSON API media type in an `Accept` header to _request_, but not _require_, that
the server apply one or more profiles to the response. When such a request is
received, a server **SHOULD** attempt to apply the requested profiles in

This comment has been minimized.

@joukevandermaas

joukevandermaas Apr 16, 2018

Contributor

How would a server signal that it did or did not succeed in this attempt?

This comment has been minimized.

@dgeb

dgeb Apr 16, 2018

Member

Profiles requested via Accept are considered optional. A server may apply a different set of profiles, or a limited set, to the response document. The actual applied profiles can be inspected by the client in the Content-Type header of the response.

If a client requires that a profile be applied, then that profile should be requested via the profile query parameter. The server will hard error if it can't apply all the required profiles. Profiles that affect processing rules, and not just representation, should be required this way.

Show resolved Hide resolved _format/1.1/index.md Outdated

ethanresnick added some commits Apr 17, 2018

profiles: tweak definitions a bit
E.g., clarify that profiles can cover any implementation-specific query
params, not just sort/filter/page
profiles: clarify that `Accept` may be totally ignored
This is key for servers that don’t want to add `Vary: Accept` for
better caching
profiles: clarify evolution requriments
The link to the w3c tag terminology about backwards and forwards
compatibility is more precise than “evolve additively”, though it
doesn’t change the intended meaning
> Note: When a client lists a profile in the `Accept` header, it's asking the server
> to compute its response as normal, but then send the response document with some
> extra information, as described in the requested profile. By contrarst, when a client

This comment has been minimized.

@marcoow

marcoow Apr 18, 2018

typo: contrarstcontrast

@wimleers

This comment has been minimized.

Contributor

wimleers commented Jun 4, 2018

First off: thanks to everyone for this epic work, in this issue and related issues!

I'm reviewing this as one of the people doing the work to bring JSON API support to Drupal core. Note that there are already >5000 Drupal 8 sites using JSON API (that usage count is an underestimation: it's the lower bound).

As mentioned in #1207, we're working to minimize/reduce risk as much as possible, by having detailed attention to API surface and its implications: backwards compatibility, maintainability, supportability. We managed to achieve zero configuration: install this module and all Drupal data is instantly exposed via JSON API, respecting the existing access control logic. If users can view foobar resources in Drupal (as HTML), they can also GET them via JSON API.

Specifically, the https://www.drupal.org/project/jsonapi module for Drupal (that we're working to move into Drupal core itself) uses two extensions: "Fancy Filters" and "Partial Success" — see https://www.drupal.org/project/jsonapi/issues/2955020. Without the former, no querying of resources (e.g. only published articles written by Wim), without the latter, impossible to do per-resource access control (e.g. Wim can see unpublished articles, but Gabe can't).

Today, and for the past ~2 years, those extensions are always running, but their existence is not communicated explicitly, because the JSON API 1.0 spec does not support extensions, per http://jsonapi.org/extensions/.

Therefore this issue/PR is essential for JSON API support to land in Drupal core. Otherwise we risk future JSON API specs moving in directions incompatible with what Drupal's JSON API implementation does … and then we would have a massive backwards compatibility problem.


@effulgentsia, @gabesullice and I commented in #1207.

I wanted to call out a two specific concerns they raised, and whether this PR addresses them:

  1. From the OP of this PR:

The application of one or more profiles to a response can be requested by a client via the Accept header.
In order to require the application of one or more profiles, the profile(s) must be specified with the profile query parameter.

This addresses @effulgentsia's concern in #1207 (comment).

This would mean no BC break in the current jsonapi module (version 1.x), because everything would continue to work. We're about to create version 2.x, with many soft BC breaks to address shortcomings in the original implementation. In version 2.x, I don't think we'd require clients to specify ?profile=fancyfilters,partialsuccess for every request (because bad Developer Experience)

  1. Gabe's concern at #1207 (comment)

Request
GET /valid-url?unknownProfileParameter
Accept: json-api
Link: ; rel=profile

Response
404 (if server doesn’t support the profile);
400 if the server doesn't understand the qp value
200 otherwise.
What the client provides in Accept doesn't actually influence this response.

I see this as a minor improvement to avoid the conflicts that were worked around in this sentence:

So, the first/primary extension the server wants to support for filtering can claim ?filter, and the second ends up with ?profiles[other-filter]

I don't think this concern is fully addressed? I think the conflict @gabesullice explained/predicted there still exists. I can understand that his Link request header proposal may not be accepted, but I don't see a reason why his 404 proposal (in case the server doesn't support the profile) is not in the PR.

Looking forward to your thoughts about this :)


Review of the actual PR to follow.

@wimleers

Several nitpicks, but also some actual questions.

A profile **MAY** assign meaning to particular elements of the document
structure, such as resource attributes or members of meta objects, and even
to query parameters whose behavior is left up to each implementation, such
as `filter` and all those that contain a non a-z character.

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

I don't think a non a-z character is precise enough. Perhaps it's better to point to http://jsonapi.org/format/#document-member-names-allowed-characters?

This comment has been minimized.

@ethanresnick

ethanresnick Jun 13, 2018

Member

The text might warrant some clarification, but I believe "those" refers to "query parameters left up to each implementation", the rule for which is non a-z (see http://jsonapi.org/format/#query-parameters)

to query parameters whose behavior is left up to each implementation, such
as `filter` and all those that contain a non a-z character.
The scope of a profile **MUST** be clearly delineated. The elements reserved by

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

This is the first time the spec would be referring to elements as a concept. There's only one mention of "element" in http://jsonapi.org/format/ today, and it's only mentioned in passing:

If sorting is supported by the server and requested by the client via query parameter sort, the server MUST return elements of the top-level data array
(Emphasis mine.)

I think it would be valuable to disambiguate this.

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

Never mind; it's being defined earlier in this PR:

particular elements of the document structure, such as resource attributes or members of meta objects, and …

object. But it could not make that member required, nor could it introduce
a new sibling to `timestamps`.
Profiles **SHOULD** reserve at least one object-valued member, and **SHOULD**

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

This comes as a surprise to me. This means that a spec like Drupal's Fancy Filters is violates this recommendation. OTOH, Partial Success would comply (it reserves the errors key in meta objects).

So what about profiles that define behavior for reserved query parameters, such as filter?

This comment has been minimized.

@ethanresnick

ethanresnick Jun 13, 2018

Member

Yeah, this was written with profiles that add data to the document in mind, not those that just use query parameters. Although, on second thought, it's probably a good idea even for a profile like fancy filters to reserve a key (probably somewhere in meta) so that, if it ever wants to add some metadata to the response (e.g., the size of the unfiltered set, or a deprecation warning or something), it can do so.

This comment has been minimized.

@dgeb

dgeb Jul 1, 2018

Member

We can probably clarify intent here by saying something like:

In order to support the potential to add features over time, profiles SHOULD reserve at least one object-valued member in any portion of the document in which they may realistically expand in the future.

A profile **SHOULD** explicitly declare "keywords" for any elements that it
introduces to the document structure. If a profile does not explicitly declare a
keyword for an element, then the name of the element itself is considered to be its

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

I don't think I fully comprehend this, nor its intent. I think a concrete example would help.

This comment has been minimized.

@dgeb

dgeb Jun 24, 2018

Member

That's fair. An example would probably go a long way here.

The point of keywords is to allow any member that's defined by profile to be aliased in a particular implementation. This aliasing might be as simple as ensuring that an API that uses camelCase throughtout does not have to use dasherized-case to comply with a particular profile (e.g. totalCount vs. total-count)

> Note: When serializing the `profile` media type parameter, the HTTP
> specification requires that its value be surrounded by quotation marks
> (U+0022 QUOTATION MARK, "\"") if it contains more than one URI.

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

Excellent note!

Show resolved Hide resolved _format/1.1/index.md Outdated
`http://jsonapi.org/extensions/last-modified` profile if it is able to.
```http
Accept: application/vnd.api+json;profile="http://jsonapi.org/extensions/last-modified", application/vnd.api+json

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

profile="URL1 URL2 URL3" is very verbose. In an Accept or Content-Type header, this is annoying but manageable. Once you're using the profile query parameter, it becomes rather unwieldy.
Wouldn't it be reasonable to rely on IDs?

I'm guessing that long-term, you want to see this supported, but that would require a JSON API profile registry, which doesn't exist yet.

This comment has been minimized.

@ethanresnick

ethanresnick Jun 13, 2018

Member

Once you're using the profile query parameter, it becomes rather unwieldy.

It is a bit unweildy, and I saw that in an earlier comment you said:

In version 2.x, I don't think we'd require clients to specify ?profile=fancyfilters,partialsuccess for every request (because bad Developer Experience)

I want to think this over and take it seriously. If the query parameter is too unwieldy that people don't bother, then it's really problematic to hinge negotiation on it. IDs could be part of the answer, but they'd seem to have the inadvertent side effect of creating two different identifiers for each profile (its URI and registry id), which is probably bad for caching + implementations would have to remember/be careful to handle both identifiers the same way.

This comment has been minimized.

@dgeb

dgeb Jun 24, 2018

Member

I think your suggestion below is the right direction here @ethanresnick.

in that document that it does not understand. The only exception to this is profiles
whose support is required using the `profile` query parameter, as described below.
#### <a href="#profiles-sending" id="profiles-sending" class="headerlink"></a> Sending Profiled Documents

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

I think this section would be better placed after the next one; I think reading is more common than mutating. I first didn't realize that this section was only about mutating.

Likewise, clients and servers applying profiles to a JSON API document **MUST**
include a [top-level][top level] [`links` object][links] with a `profile` key,
and that `profile` key **MUST** include a [link] to the URI of each profile

This comment has been minimized.

@wimleers

wimleers Jun 4, 2018

Contributor

When no profiles are specified in the request document, but they are specified in the Content-Type request header or the profile URL query parameter, should a 400 error response be sent?

IOW: are JSON API documents in request bodies required to match the Content-Type and/or profile URL query argument?

This comment has been minimized.

@wimleers

wimleers Jun 5, 2018

Contributor

On second thought, I don't think this makes sense, because it's already valid to just send

{
  "data": { … }
}

Because http://jsonapi.org/format/#document-top-level already says that only one of data, errors and meta is a MUST-have.

This comment has been minimized.

@ethanresnick

ethanresnick Jun 13, 2018

Member

When no profiles are specified in the request document, but they are specified in the Content-Type request header or the profile URL query parameter, should a 400 error response be sent?

This is a good point. I think we want to say that, on requests, if the profiles listed in Content-Type don't match those listed in the document body, the server should return a 400.

On second thought, I don't think this makes sense, because it's already valid to just send

I don't think that prevents us from requiring a 400 in this case, because none of the requests that are valid today will contain profile in the Content-Type.

```json
"links": {
"profile": [{ "href": "http://jsonapi.org/ext/example-ext" }]

This comment has been minimized.

@wimleers

wimleers Jun 6, 2018

Contributor

Nit: Still mention of ext instead of profile.

### <a href="#profile-keywords" id="profile-keywords" class="headerlink"></a> Profile Keywords
A profile **SHOULD** explicitly declare "keywords" for any elements that it

This comment has been minimized.

@wimleers

wimleers Jun 6, 2018

Contributor

How can a profile explicitly declare a keyword?

This comment has been minimized.

@ethanresnick

ethanresnick Jun 13, 2018

Member

The profile would declare the keyword in its specification. Part of the problem is that this PR never actually says what a profile is. We go straight from talking about what they do ("a way to indicate additional semantics") to how they're identified ("by URI"). If we import some language from #1195:

A [profile] is a small, separate specification that defines...

(We have to update what's after the ...)

Then, it's clear that you declare keywords in that specification

For instance, the following document includes a single profile descriptor for a
profile that, let's say, assigns meaning to a `version` keyword that can be
included in any resource's `meta` object. This descriptor provides an alias for

This comment has been minimized.

@wimleers

wimleers Jun 6, 2018

Contributor

Once we have a mechanism to explicitly declare a keyword, that also has as a side benefit that that declaration could also state whether the keyword applies to meta, a query param, or something else still.

@wimleers

This comment has been minimized.

Contributor

wimleers commented Jun 11, 2018

FYI: this is being implemented for Drupal at https://www.drupal.org/project/jsonapi/issues/2955020.

declared by the profile, and the value **MUST** be an alias that applies to this
particular representation. This aliasing mechanism allows profiles to be applied
in a way that is both consistent with the rest of the representation and does
not conflict with other profiles.

This comment has been minimized.

@wimleers

wimleers Jun 11, 2018

Contributor

Does this mean JSON API servers are required to resolve/process the aliases defined in a request document? (It's possible that the client uses different aliases than the server.)

This comment has been minimized.

@ethanresnick
@ethanresnick

This comment has been minimized.

Member

ethanresnick commented Jun 13, 2018

@wimleers Thanks for the review, and sorry for my delay in addressing your comments. I've been swamped.

I don't think any of your comments suggest that this PR needs serious changes, which is reassuring. The possible exception is the around the developer experience of requiring long urls in the profile query parameter for every request. I'll bring that up with @dgeb, though I imagine he already thought about it when making the proposal, and I know that that was an area where it was tricky to find any consensus.

EDIT: I think the solution here is simple: the server can interpret each query parameter as coming from the profile of its choice by default (though it can't change that choice over time). In other words, it can implicitly interpret ?filter=..... as coming from fancy filters in the absence of a ?profiles parameter to the contrary, as long as it always interprets it that way. If the ?profile parameter is used, though, it still must be respected (e.g., ?filter=....&profile=https://drupal.org/filter-spec-2 is no longer a fancy-filters request). Finally, ?filter=....&profile=https://drupal.org/fancy-filters is just equivalent to the request without the ?profile param.

Does that approach work for everyone?

I don't think this concern is fully addressed? I think the conflict @gabesullice explained/predicted there still exists. I can understand that his Link request header proposal may not be accepted, but I don't see a reason why his 404 proposal (in case the server doesn't support the profile) is not in the PR.

I'm not sure I understand this. Can you give an example?

@dgeb

This comment has been minimized.

Member

dgeb commented Jun 24, 2018

I think the solution here is simple: the server can interpret each query parameter as coming from the profile of its choice by default (though it can't change that choice over time).

@ethanresnick I agree that this is the most simple and elegant solution. The critical part here is that the JSON:API resource(s) (i.e. HTTP resource) identified by a request does not change for a given URL. If usage of a profile targets different resource(s), then the same URL must not be used. And we explicitly provide the profile query param for the purpose of guaranteeing a unique URL when this is the case. I think it's probably ideal for a given API to choose a default set of profiles carefully, so that conventional usage of that API requires no profile query param at all. I'm going to attempt to rework the phrasing in this PR to this effect over the next day or two.

@wimleers

This comment has been minimized.

Contributor

wimleers commented Jun 28, 2018

I've been swamped.

No need to apologize — I know the feeling all too well!

I don't think any of your comments suggest that this PR needs serious changes, which is reassuring.

Indeed! :) 👍

I'm not sure I understand this. Can you give an example?

I'll let @gabesullice reply to this — it's possible I'm misinterpreting what he wrote.

I think the solution here is simple: the server can interpret each query parameter as coming from the profile of its choice by default (though it can't change that choice over time).

Agreed!

@ethanresnick++
@dgeb++

ethanresnick added some commits Jul 22, 2018

profiles: add must-ignore rule to example timestamps profile
Technically, this rule is required to make it legal to add even a new,
optional member to the timestamps object.
profiles: create separate authoring section
This commit just pulls the existing guidelines about authoring profiles
into a dedicated section, without changing any of the substance.

I’m pulling this out because there are a few requirements for authoring
profiles that I think are missing, and it was getting a bit messy to
have the rules for authoring profiles all mixed in with the rules for
using them.
@gabesullice

This comment has been minimized.

gabesullice commented on db1e970 Sep 18, 2018

😚👌

+1

Show resolved Hide resolved _format/1.1/index.md Outdated
Show resolved Hide resolved _format/1.1/index.md
Show resolved Hide resolved _format/1.1/index.md Outdated
Show resolved Hide resolved _format/1.1/index.md
@dgeb

This comment has been minimized.

Member

dgeb commented Sep 18, 2018

@ethanresnick Thanks for making those adjustments. I think this is finally ready to squash and merge! 🚀

@ethanresnick

This comment has been minimized.

Member

ethanresnick commented Sep 18, 2018

@dgeb AWESOME. I'm going squash this down a bit (though maybe keep more than one commit, as having some distinct history is useful I think) and then will merge.

@ethanresnick

This comment has been minimized.

Member

ethanresnick commented Sep 18, 2018

Actually, jk, there aren't really neat commit divisions anymore anyway, so squash and merge it is.

@ethanresnick ethanresnick merged commit 55cd721 into gh-pages Sep 18, 2018

@dgeb dgeb deleted the profiles branch Sep 18, 2018

@dgeb

This comment has been minimized.

Member

dgeb commented Sep 18, 2018

Woot! Thanks for all your work on profiles over the years, @ethanresnick. This one is more yours than mine. :shipit:

@ethanresnick

This comment has been minimized.

Member

ethanresnick commented Sep 18, 2018

Thanks Dan! Next up: operations :)

@gabesullice gabesullice referenced this pull request Sep 18, 2018

Closed

Profile Registration #1295

@wimleers

This comment has been minimized.

Contributor

wimleers commented Sep 19, 2018

Exciting! 🎉 Congrats! And thanks :)

This means that the Drupal JSON API module — which is currently on a path to inclusion into Drupal core itself — now actually could implement this. How certain are we that there will be no more breaking changes in the spec wrt profiles?

I reopened the Drupal.org issue: https://www.drupal.org/project/jsonapi/issues/2955020#comment-12781015.

@ethanresnick

This comment has been minimized.

Member

ethanresnick commented Sep 19, 2018

Exciting! 🎉 Congrats! And thanks :)
Thanks!

This means that the Drupal JSON API module — which is currently on a path to inclusion into Drupal core itself — now actually could implement this.
🎉

How certain are we that there will be no more breaking changes in the spec wrt profiles?

I can't forsee any changes, except if implementation experience proves that some part of this design is really unworkable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment