[PP-2] [META] Revisions support

There is a PR open on the JSON API specification about adding revisions as part of a resource. Is not closed yet, but it might help us to figure out how will work and how we can do it:

https://github.com/json-api/json-api/pull/824

https://github.com/json-api/json-api/pull/824 hasn't gone anywhere in a year's time. Last comment in May.

So to confirm, Is it true that there is no way to GET the latest revision (or a specific revision) using JSONAPI?

Thank you. I'll go ahead and write a custom endpoint.

This is blocked on https://github.com/json-api/json-api/pull/824.

While responding to a core issue, I had some thoughts about how we could handle this. I've updated the IS with my proposal.

See: #2937850-5: Mark revision_default as internal for REST consumers

I think we have both advantages and disadvantages when it comes to supporting revisions when compared to core REST. I think it's important that we're part of the conversation so that we don't get boxed into a corner if things become to REST-module-centric.

Fixed some formatting issues.

Again...

sci-fi atm

I'm glad people are already starting to think about this (go @gabesullice!) but for me personally, there's just too much other stuff we have tackle/stabilize first.

I also think translations are a bigger priority. They're A) more doable, B) at least partially working in core REST already, C) we'll need to handle revisions+translations eventually, so it's important we take "revisions of translations" into account while working on revisions!

Hi there!

I reading this issue with great interest. I seems that a general solution to support revisions in JSONAPI is currently quite complex. However I wonder if there might me quick and dirty way to alter the JSONAPI output to return the latest draft version instead of the public version of a node.

Would you suggest trying to alter JSONAPI output or better go with a custom endpoint to get the latest draft version of a node?

Thanks in advance!
Walter

Would you suggest trying to alter JSONAPI output or better go with a custom endpoint to get the latest draft version of a node?

For now, I personally think a custom endpoint is probably better, because then it's very explicit that this is an upstream feature that you need.

@Wim translations already, surprisingly, work with JSOON:API. If you prepend the langcode in front of the API endpoint, it'll return the translated content! /api/node--foo/12345 returns the default translation, /fr/api/node--foo/12345 returns the french!

w/r/t revisions, given that the referenced issue hasn't been updated in over 2 years, maybe its worthwhile experimenting a little? I think the put/patch usecase for revisions is much harder than the get</em> usecase, and we can probably bikeshed <em>a little</em> to come to a first-pass implementation of <code>getting a specific revision.

surprisingly, work with JSOON:API

That's the same way it "works" for core's rest.module: it works, but only semi-accidentally. There's no test coverage. I can see GET working fine, but I doubt PATCH, POST and DELETE work.

given that the referenced issue hasn't been updated in over 2 years, maybe its worthwhile experimenting a little?

Absolutely! That's exactly what's necessary. Nobody has simply started working on it. The reason I haven't prioritized it yet (as one of the API-First Initiative coordinators) is that there is was SO MUCH other stuff that was broken in simple cases (not involving revisions/translations). It's now finally getting to a point where things are okayish. Compare #2941316: REST: top priorities for Drupal 8.6.x with #2721489: REST: top priorities for Drupal 8.2.x to see how far we've come (there are summaries at the end of every one of those "top priorities" issues).

I can't wait to see your experiments with revision support! 😄 :partyparrot:

For now, I personally think a custom endpoint is probably better because then it's very explicit that this is an upstream feature that you need.

@Wim... you mention creating a custom endpoint to hit for displaying nodes in a "draft" or "unpublished" state/status. How would one go about doing so?

My current setup is such:

D8.5.3
Bricks Module
Jsonapi Module
Jsonapi Extras
Eck
Content Moderation
Vuejs on the FE

I have a content type called "Dynamic" where I have a Bricks field that references ECK entity types. The problem that I have at the moment are edits that are made inline to the ECK entity type no matter if I set the page to "draft" status the GET request is always displaying the latest. So edits that should not be displayed are. Is there a way to check for the current revision and or the status of the ECK entity type referenced within the page?

@TikaL13: your question is out of scope here. You want to create a new REST resource plugin. See https://www.drupal.org/docs/8/api/restful-web-services-api/restful-web-s... for getting started. This issue is specifically about revision support for JSON API. If you still have more questions about this, please create a Support request issue.

Ideally, the JSON API spec would describe this for us. But I don't think we can wait for https://github.com/json-api/json-api/pull/824 to happen. It'll take too long. We should implement it already, and do so in a way that allows the spec to still define it in the future.

Let's do this! 🤘 🚀

#27 sounds SUPER fascinating! 😍 Thank you! 🙏

I really like the idea of getting to a point where we can implement the memento protocol. It's a great concept.

But I think it's getting a little ahead of ourselves. A prerequisite to negotiating revisions at a particular time means that those revisions are actually identified by a URI.

   When needed for extra clarity, the following conventions are used:

   o  URI-R is used to denote the URI of an Original Resource.

   o  URI-G is used to denote the URI of a TimeGate.

   o  URI-M is used to denote the URI of a Memento.

   o  URI-T is used to denote the URI of a TimeMap.

- https://tools.ietf.org/html/rfc7089#section-1.2

In essence, each memento needs a URI.

To start, we first need to be able to GET different versions of a resource. From there, we can add a way to negotiate which of those versions corresponds to a datetime.

FWIW, even though I put URLs in the IS, I don't think that's the best way any longer.

I'd like to propose a query parameter based strategy with heavy inspiration from this RFC: 5829: Link Relation Types for Simple Version Navigation between Web Resources

For example, /jsonapi/node/article?resource_version={type}:{value} where {type} is a pluggable strategy for evaluating {value} to a specific revision ID. The default strategies might be: id, rel and datetime. ID would obviously just be the raw revision ID, rel would use the link relation types registered by RFC 5829 to resolve a revision ID, and datetime would resolve the revision ID of the resource that was canonical at the given datetime.

So, using the rel strategy, one could GET /jsonapi/node/article?resource_version=rel:latest or /jsonapi/node/article?resource_version=rel:predecessor-version:working-copy or /jsonapi/node/article?resource_version=rel:predecessor-version:{int} to get the predecessor version of the latest draft or the predecessor version of the given revision ID.

Thanks for summarizing the key discussion items and outcomes in the call!

wow, indeed, those proposed endpoints are magical, I can see them very useful in many scenarios!

@skyredwang Wonderful to have you chiming in! I'd love to hear from you in a separate issue how JSON API has been working for you in the past few months, as we've been working to stabilize it, expand test coverage, and so on.

@pwolanin created #2993557: Allow optional creation of new revision when PATCHing revisionable entities to support autosave functionality via JSON:API..

This is a great feature, just curious when would this be implemented (time range)?

I realize the team has all these other issues and stuff to work on. My boss wants this feature though, so I just want to give him a ballpark, (not official estimate) about when this might be released.

He might let me work on it on my own for a day or two, or just here and there. Right now we have "bigger fish to fry" for the next coming weeks for this sprint, but I might have time to contribute after that depending on how things go. I'll let you know in a few weeks.

Right now, I think* his way of sponsoring will be like telling me "go and hotwire the jsonapi to make it do what I want it to do" type of thing :D

#2992833 just reached RTBC in #2992833-279: Add a version negotiation to revisionable resource types!

Next: #2992836-15: Provide links to resource versions (entity revisions), which was blocked on that. And then: #3009588: Provide a collection resource where a version history can be obtained (`version-history`, `predecessor-version` and `successor-version` link relations).

#2992833: Add a version negotiation to revisionable resource types landed!

#2992836: Provide links to resource versions (entity revisions) landed!

Yay :)

I think we need an updated issue summary here, to indicate what is blocking the remainder. Because AFAICT neither the CUD in CRUD nor version history are achievable with the current set of core APIs?

Updated.

The most often needed functionality wrt revisions has shipped! 🥳

What remains is:

1. version history: #3009588: Provide a collection resource where a version history can be obtained (`version-history`, `predecessor-version` and `successor-version` link relations), blocked on #2725523: Add a revision_parent field to revisionable entities in core
2. revision READ support for entity types other than Media and Node, blocked on #2350939: Implement a generic revision UI in core
3. CUD support, blocked on #2350939: Implement a generic revision UI in core

I think we should consider creating two concrete feature request issues for points 2 and 3 and then close this issue? On the other hand, it may be useful to keep this around :) For now, keeping it around and marking it explicitly postponed on those two core issues.

Something not captured in in #47 is revision creation is an essential part of updating a revisionable entity for entity types that are revisionable. @Wim Leers, @gabesullice, @effulgentsia, and I discussed this today.

• Out of the box, in Standard, updating a node creates a new revision of that node and sets it as the default revision.
• Privileged users can override this behavior by unchecking the "Create new revision" checkbox in the UI.
• Per @Wim Leers, JSON:API doesn't have access to that currently since that "create new revision" checkbox is (apparently) just a form element rather than a generic part of the Entity API.
• However, updating an entity over JSON:API (and reportedly over REST as well) might update the current revision rather than creating a new revision.
• Apparently, based on manual testing that @effulgentsia did, this somehow changes when Content Moderation is enabled, and new revisions are then created when updating an entity over JSON:API. (? Needs verification and investigation of why.)

This mismatch could be considered data loss for sites that require a new revision to be created every time content is updated. It could also potentially lead to orphaned revisions if the latest revision is not the default revision.

I promoted #2838395: [PP-1] Entity API bug affecting REST: "last changed" timestamp not updated and "create new revision" setting not respected when PATCHing via REST and #1863258: Move entity preparation from form controller to entity class: "last changed" timestamp not updated and "create new revision" setting not respected when updating entities via Entity API to critical based on the discussion.

Apparently, based on manual testing that @effulgentsia did, this somehow changes when Content Moderation is enabled, and new revisions are then created when updating an entity over JSON:API. (? Needs verification and investigation of why.)

Content moderation makes sure that is set by default for all bundles - see \Drupal\content_moderation\Entity\Handler\NodeModerationHandler::enforceRevisionsBundleFormAlter and \Drupal\content_moderation\Entity\Handler\NodeModerationHandler::enforceRevisionsEntityFormAlter for the form alters and \Drupal\content_moderation\Entity\Handler\ModerationHandler::onPresave for the non-form route.

#50: that ::onPresave() is crucial here. Related: @hchonov's comments at #1863258-35: Move entity preparation from form controller to entity class: "last changed" timestamp not updated and "create new revision" setting not respected when updating entities via Entity API and #1863258-37: Move entity preparation from form controller to entity class: "last changed" timestamp not updated and "create new revision" setting not respected when updating entities via Entity API.

Apparently, based on manual testing that @effulgentsia did, this somehow changes when Content Moderation is enabled, and new revisions are then created when updating an entity over JSON:API. (? Needs verification and investigation of why.)

Issue created: #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists.

I discussed this issue with @larowlan yesterday. One thing @larowlan pointed out about the "safe" revisionable types (Nodes and Media) defined in \Drupal\jsonapi\ResourceType\ResourceTypeRepository::isVersionableResourceType was:

no api to override it short of swapping out the resource type repo service

We'd also like to see hard failures instead of partial support for things that aren't supported, and test coverage.

@gaborhojtsy listed out these concerns for the translation support meta, but I think they apply here as well:

• What "contracts" does JSON:API want to keep for multilingual functionality, since they need to keep BC with these bugs for X (define X, if we know that is only until D9 do we have something to mark deprecated?)
• Are there code paths that need @todos added?
• Is there tests to cover that this is how broken it is and to the ensure we keep supporting how broken it is now until we promise we do, etc.?

So, i.e., do we have test coverage:

1. Proving that all revision entity types besides Media and Node are inaccessible with any method?
2. That Media and Node revisions are read only?
3. Tests of what happens when you do update a Media or node revision?
4. A version of #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists for when Content Moderation isn't enabled?
5. Test coverage for what happens when a revisionable entity that does not have revision support from JSON:API is updated? E.g. Block Content entities.

1. Yes: \Drupal\Tests\jsonapi\Functional\ResourceTestBase::testRevisions().
2. Yes: \Drupal\Tests\jsonapi\Functional\ResourceTestBase::testRevisions() + #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists
3. Yes: #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists
4. Yes: #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists
5. Yes: \Drupal\Tests\jsonapi\Functional\ResourceTestBase::testPatchIndividual() and #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists

One thing @larowlan pointed out about the "safe" revisionable types (Nodes and Media) defined in \Drupal\jsonapi\ResourceType\ResourceTypeRepository::isVersionableResourceType was:

no api to override it short of swapping out the resource type repo service

… because overriding that would be extremely insecure, since there's no revision access control API yet:

  protected static function isVersionableResourceType(EntityTypeInterface $entity_type) {
    // @todo: remove the following line and uncomment the next one when revisions have standardized access control. For now, it is unsafe to support all revisionable entity types.
    return in_array($entity_type->id(), ['node', 'media']);

We'd also like to see hard failures instead of partial support for things that aren't supported, and test coverage.

\ResourceTestBase::testRevisions() was already doing much of this. #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists added more.

Thanks @Wim Leers!

Re: #55:

… because overriding that would be extremely insecure, since there's no revision access control API yet:

The point I believe is that a contrib module could easily write its own access control e.g. for Block Content, but would have no way to add support for it.

Sorry, I don't think #55 is necessarily a blocker; it's just something that came up in review and may need an issue.

The point I believe is that a contrib module could easily write its own access control e.g. for Block Content, but would have no way to add support for it.

They could not "easily" write this. Precisely because there is no API for this.

class NodeRevisionAccessCheck implements AccessInterface {…} and class MediaRevisionAccessCheck implements AccessInterface {…} are route access checks that happen to be written in a way that we can call them too.

There is no interface for a contrib module to implement, no class to subclass, no place to inform Drupal that a certain class is supposed to be used for. If there were an EntityAccessControlHandler-like structure for checking revisions too, we'd definitely support it. We already had to go the extra mile to support Node and Mediasince there is no official revision access checking API. Workflow Initiative lead and Entity API maintainer @amateescu confirmed this in #2992833-151: Add a version negotiation to revisionable resource types.

We already have an explicit issue for this: #3031271: Support version negotiation for any entity type (currently only Node & Media are supported). Updated the issue summary to point to that instead.

EDIT: heh, I already had a response for your new comment that got posted after I started writing this:

Sorry, I don't think #55 is necessarily a blocker; it's just something that came up in review and may need an issue.

So: that issue exists 🙂

What "contracts" does JSON:API want to keep for revision functionality […]

https://www.drupal.org/docs/8/modules/jsonapi/revisions documents how to retrieve a revision of an entity via JSON:API using either a specific revision ID or a RFC5829 link relation type. (Change record: https://www.drupal.org/node/3015434.)

Are there code paths that need @todos added?

No, all necessary @todos are already present.

Is there tests to cover that this is how broken it is and to the ensure we keep supporting how broken it is now until we promise we do, etc.?

1. For revisions, there is nothing "broken", it's just not as feature complete as it'd ideally be, due to missing foundations in Drupal core. When Drupal core gains those, we can expose more functionality: GETting revisions for all revisionable entity types, and mutating revisions once that infrastructure exists.
2. From the moment we had any revision support, there was explicit test coverage verifying it only worked for Node and Media; it ensures an precise and helpful error response is sent for other entity types.
3. #3037989-8: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists's interdiff also added explicit test coverage ensuring that POST, PATCH, DELETE (i.e. anything other than reading) results in an error response.

There are at least two things that are potentially broken as per our call: The existing revision being updated instead of a new one being created when CM is not enabled, and orphaned revisions potentially being created when an entity is updated (needs testing/confirmation). These are the things @effulgentsia raised and they are serious bugs. Maybe we can get @effulgentsia to help with testing and documenting potential issues also.

To be clear, we need to test (automated tests, but also manually) what happens with updating not only supported things like nodes but also something like block_content, which is revisioned by default, but json_api doesn't support revisions for.

1. If the entity is edited, is a new revision created as per the standard profile expectation? I think it's not, because of #1863258: Move entity preparation from form controller to entity class: "last changed" timestamp not updated and "create new revision" setting not respected when updating entities via Entity API, and that's a critical bug because it's "data loss" of the previous revision according to that site's expectations.
2. Similarly, if a block_content entity has a default revision that is not the latest revision, and is updated, what happens? Do we get orphaned revisions?

Those two things are what I'm concerned about. They should be tested with both nodes and block content, both with and without content moderation.

what happens with updating not only supported things like nodes but also something like block_content, which is revisioned by default, but json_api doesn't support revisions for.

I think you may have a bit of confusion WRT what our support for node and media is :) Let me try to clear that up.

The extent of any special handling of revisions (per entity type) in JSON:API is limited to GETing a particular revision by its ID (or a shortcut, like "working-copy" which GETs the latest revision) using a query parameter. For example, /jsonapi/node/article/{uuid}?resourceVersion=id:42. It's impossible to use this query parameter with POST, PATCH or DELETE. For all other entities, JSON:API behaves in exactly the same way (including block_content):

1. gets whatever core's entity ParamConverter provides by default
2. if the entity is revisionable, it checks that is is the default and latest revision (400 Bad Request if not)
3. updates it
4. validates it
5. saves it

This behavior is exactly the same all entities of any type (node and block_content included).

Additionally, it's explicitly disallowed to PATCH any entity using the resourceVersion query parameter (which works only for node and media). This was given explicit test coverage in #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists.

#61.1a:

If the entity is edited, is a new revision created as per the standard profile expectation?

JSON:API does not call setNewRevision() on any entity, regardless of any form defaults.

However, when Content Moderation is installed, CM will execute a hook_entity_presave which does create new revisions under certain circumstances, according to its own logic. We have no input in that process.

#61.1b:

that's a critical bug because it's "data loss" of the previous revision according to that site's expectations.

Without CM installed, the revision ID is never incremented. This is explicitly tested in #3037989 too. I'm not sure that we can be certain of what the site's expectations are, given that the setting you're talking about explicitly says it's a form default. Any developer developing against JSON:API would quickly realize that new revisions are not being created. We have an existing issue to make it possible to create new revisions according to a more clear set of expectations: #2993557: Allow optional creation of new revision when PATCHing revisionable entities to support autosave functionality via JSON:API.. Right now, it is not clear what user expectations are.

With CM installed, the behavior depends on whether the entity type has a moderation handler (which causes the presave behavior I described above). If it does have a moderation handler, it is explicitly tested that the revision is incremented. If it does not, then it asserted that the revision ID is not incremented:

if ($this->entity->getEntityType()->hasHandlerClass('moderation')) {
  $this->assertNotSame($original_revision_id, (int) $updated_entity->getRevisionId());
}
else {
  $this->assertSame($original_revision_id, (int) $updated_entity->getRevisionId());
}

#61.2

Similarly, if a block_content entity has a default revision that is not the latest revision, and is updated, what happens? Do we get orphaned revisions?

This is explicitly tested in #3037989: Test coverage with and without Content Moderation: POST/PATCH/DELETE revisions results in 400, auto-revisioning PATCH with CM, PATCH/DELETE when pending revision exists. Attempting to PATCH any revisionable entity, including block_content, that is not the default and latest revision will result in a 400 Bad Request.

They should be tested with both nodes and block content, both with and without content moderation.

All the PATCH tests apply to all entities and the revision specific bits apply to all revisionable entities, included node and block content.

It's impossible to test the #61.2 case without content moderation, because it's impossible to have a default revision that is not the latest revision unless CM is installed.

Thanks @gabesullice!

Regarding:

if the entity is revisionable, it checks that is is the default and latest revision (400 Bad Request if not)

👍

Then, regarding:

Without CM installed, the revision ID is never incremented. This is explicitly tested in #3037989 too. I'm not sure that we can be certain of what the site's expectations are, given that the setting you're talking about explicitly says it's a form default.

Until the upstream bug is fixed, I think we should go the other direction and always increment the revision ID. Extra revisions are better than lost revisions from a risk management perspective.

Then hopefully, we might (as a temporary measure, until the entity itself has the field for this) check the bundle settings for the default value of the checkbox, and obey that always.

Whether or not we check the bundle setting, we could form alter a message onto the existing "create new revision by default" checkboxes informing the user that JSON:API will ignore the user's choice if the user overrides the default (or always if we can't check the bundle setting).

Unassigning myself.

#61 has been addressed in #3038453: When RevisionableEntityBundleInterface::shouldCreateNewRevision is TRUE, create a new revision on PATCH requests and add an entry in the revision log., after @plach RTBC'd it! 🎉

Added an issue to address the revision access api, as the UI one shouldn't be the place we do that, for the sake of scope

+1

We linked to #2350939: Implement a generic revision UI for that, since that's what @amateescu linked to, but #2795279: [PP-2] [META] Revisions support is definitely a better fit: tighter scope.

Updated issue summary. Thanks!

The second link in #67 should of course have been #3043321: Use generic access API for node and media revision UI, not #2795279: [PP-2] [META] Revisions support 😅

Adding #2993557: Allow optional creation of new revision when PATCHing revisionable entities to support autosave functionality via JSON:API. to Full CRUD on Revisions.

We need to clear up whether #3009588: Provide a collection resource where a version history can be obtained (`version-history`, `predecessor-version` and `successor-version` link relations) truly is blocked or not. But even then, this is blocked on at least 4 issues: three direct blockers, one indirect blocker.

#3043321: Use generic access API for node and media revision UI landed

#2350939: Implement a generic revision UI is fixed.