Handle core 8.2.x changing REST configuration to use configuration entities

Thanks for the heads up @tedbow!

This patch looks good AFAIK. We need to add tests + create a new branch? And maybe improve the code, anyway needs review.

Well it is important to update it asap because manually could be complex :)

IMHO this is Critical.

@tedbow

Sorry I wrote too fast :) Manually is not the correct word. I mean it is more complex to enable the resources from code instead of using the UI.

This patch fixes the problems with the branch 8.2.x. I'm not verifying the version from this patch.

Thanks for reviewing!

I've just checked latest 8.2.x without patch ... seems to work. So why should we fix this (now) and introduce a new branch?

I see a CR REST config converted to config entities for #2308745: Remove rest.settings.yml, use rest_resource config entities

The CR does not tell us about a BC break either :-(

Don't get me wrong as the patch looks sensible to me for a 8.2.x!

@tedbow thanks for the explanation.

I've tested with and without the patch. Issuing drush @drupal.d8 cache-rebuild removes all but content settings from http://drupal.d8/admin/config/services/rest (content/user/comment) which is weird.

So we must create a new branch indicating it is for 8.2.x. Not sure what current naming strategy is.

This definitely needs a require section in composer.json (https://www.drupal.org/node/2514612)

I tried settings below to see whether drupal checks for it but that doesn't seem to work :-(

  "require": {
    "drupal/drupal": ">=8.3"
  }

Checking https://www.drupal.org/node/1938140/qa we pass the tests which could not be right :-(

Just tried REST UI HEAD in Drupal 8.3 HEAD. Form works fine, but trying to use REST resources gets expected:

{
  message: "Not acceptable format: json"
}

Applying the patch in #4, my previously saved configuration settings/enabled resources are gone. Going back in, I can set the same settings as before node with json, hal_json, and cookie auth. It saves fine, and restores on revisiting the edit form. The enabled formats are no longer displayed on the resource listing page (unless I am misremembering that they ever were).

Navigating back to /node/1?_format=hal_json, the resource loads as expected.

Tested with Drupal 8.2-rc1, restui 8..x-1.x-dev and patch from #4 applied.

Resources are being updated, but it turns out that there's still a small problem with displaying the enabled methods, authentication and formats. Attached patch fixes this, includes the patch from #4.

For what it's worth, the patch in #12 works like a charm on today's 8.2 release.

Just from reading the change, this looks pretty alright. One could create a follow up to integrate the simpler configuration file into the UI, aka. just configure formats/authentication per resource, not per HTTP verb, but this is out of scope of this issue of course.

I try the patch of #12 and work fine.

Here is a small change which just tries to minimizes the change required as part of the patch. For example it doesn't rename the variable, even config might be better. From a pure functionality point of view its not needed and distracts people from reviewing it.

I'm not sure if this is related to this issue, but I did notice after upgrading to 8.2 in the permissions.

Before upgrade:

After upgrade:

@blackbull77
That's right. As of 8.2.x we no longer use REST specific permissions, but rather fallback to the entity specific ones, because well, if you can create an entity, you should be able to create it via the UI or via REST. Its just another way to do so. This let's the permissions to no longer be available.

Just in case someone wants to opt out.

https://www.drupal.org/node/2733435

I'm a new to REST so I'm still learning. Any materials about the new way?

Well, checkout the two change records: https://www.drupal.org/node/2747231 and https://www.drupal.org/node/2733435

I haven't read comments on creating a new branch ... what will this patch do regarding BC? Please advise.

Interdiff from #19 does a nice cleanup. No \Drupal and str_replace :-)

Tested the minimalist patch in #19 on a fresh D8.2 running in a Kalabox app environment, and it applies cleanly and does what it's supposed to. However, this same patch applied to the restui-8.x-1.x-dev module in another Kalabox-ed site that's been upgraded from D8.1 > D8.2 fails to fix the problem, so something's still not right here.

Further details:

• Upgraded site is experiencing the "before upgrade" permissions anomaly screenshotted in comment #22, wherein permissions are added or subtracted from the RESTful Web Services section depending on whether or not the corresponding REST service is enabled or disabled in the Rest UI screen.
• The upgraded sites rest.settings.yml file is set to bc_entity_resource_permissions: false
• Upgraded site uses a minimal install profile, while the fresh site uses the standard install profile. (not sure if relevant yet)

I haven't read comments on creating a new branch ... what will this patch do regarding BC? Please advise.

Well, all what this patch is doing is to enable the usage of the module with the core 8.2.0 version of REST. I don't think there is any BC layer needed, as this is all provided by core itself.

I guess I didn't expressed myself correctly. When I apply the patch I am pretty sure it will break on D 8.1.x/8.0.x installs. And I'm not sure what to do as mentioned in #9

- Should we create a new branch for _every_ new Drupal minor release?!?
- What should be this new branch be. Semantic versioned or plan old Drupal style?
- What did I wrong with the require settings from #9?

I'm happy to commit ASAP but I don't want to break D8.1.x/8.0.x sites so please advise.

I guess I didn't expressed myself correctly. When I apply the patch I am pretty sure it will break on D 8.1.x/8.0.x installs. And I'm not sure what to do as mentioned in #9

Well, why do you think supporting D 8.1.x/8.0.x installations out there is a good idea? The entire point of SEMV is that you can update to the next minor version, and well you should update to 8.1.x. Its in fact unsupported: security patches for example won't be backported.

IF you are hypercritical you could create a 8.x-2.x branch and continue there and let the 8.x-1.x branch die.
I think this patch should actually add a dependency:

dependencies:
  - system (>=8.2.0)

@dawehner: 8.x-2.x would suggest the second major version of this module, compatible with the entire Drupal 8.x range.

I would suggest creating a new 8.2-1.x branch with the patch in this issue applied, and renaming the curent branch to 8.1-1.x This way we would clearly indicatie core version compatibility, and at the same time allow the module to still be used with Drupal 8.0 and 8.1 where there are no issues with the current branch.

I agree that a version dependency / restriction should be added to both branches, altough that won't work until https://www.drupal.org/node/2641658 is fixed...

I would suggest creating a new 8.2-1.x branch with the patch in this issue applied, and renaming the curent branch to 8.1-1.x This way we would clearly indicatie core version compatibility, and at the same time allow the module to still be used with Drupal 8.0 and 8.1 where there are no issues with the current branch.

Well, if we care about the BC issues with it, than we need to follow semantic versioning, which tells you that breaking changes belong into major versions.

Hmm, after digging around a bit more I believe you are right. We are supposed to be always using the 8.x prefix for Drupal 8 modules as minor versions are not supposed to break BC. (Which 8.2 does do anyway IMHO, this issue being one)

I updated https://www.drupal.org/docs/8/api/restful-web-services-api/restful-web-services-api-overview to reflect the current state of the REST UI module and included a pointer to this issue.
If that is not desired, the link can be removed, but the existing documentation lacked clarity.

In response to #28:

• For the fresh D8.2, did you apply the patch to " restui-8.x-1.x-dev " as you did in the second, upgraded site?
• Does "apply the patch to restui-8.x-1.x-dev" mean you applied the patch to a fresh checkout of that branch of the git repository?

Let's fix the failures first

@kaymccormick Yes, all tests were done with the latest 1.x-dev cloned to my local, and patch applied cleanly, and then tested manually. The virgin D8.2 site worked fine, the 8.1->8.2 upgraded site functions, but has serious usability issues because the permissions are still showing up and disappearing according to whether a Rest UI component is enabled or disabled.

@Senpai
Good point. I agree we should fix that, but that is a bit of an orthogonal problem, isnt' it? Having its own issue for that would probably help to get this first critical bit in.

Hi. I upgraded my D8 site from D8.1.10 to D8.2.0 last week, and have upgraded the restui module to the latest 8.x-1.11+7-dev version. I applied the patch from #38, and find the following:

1. The rest types I have enabled are showing was enabled again in the form "/admin/config/services/rest" (they were missing after the upgrade to D8.2).
2. The form "/admin/config/services/rest/resource/entity%3Anode/edit" does not show any of the methods, accepted formats, or authentication providers that are set in the YML file (all checkboxes are blank).
3. The user permissions for each HTTP method / resource entity combination are still showing with the same settings that applied under D8.1.

The contents of my config file "rest.resource.entity.node.yml" are:

uuid: 2bd48846-3162-48db-846f-6fbbdf8328f3
langcode: en
status: true
dependencies:
module:
- basic_auth
- hal
- node
- serialization
- user
id: entity.node
plugin_id: 'entity:node'
granularity: resource
configuration:
methods:
- GET
- POST
- DELETE
- PATCH
formats:
- hal_json
- json
authentication:
- basic_auth
- cookie

After editing the settings in "/admin/config/services/rest/resource/entity%3Anode/edit" to re-establish the actual settings, and exporting the changed configuration, my config file "rest.resource.entity.node.yml" has the following content:

uuid: 2bd48846-3162-48db-846f-6fbbdf8328f3
langcode: en
status: true
dependencies:
module:
- basic_auth
- hal
- node
- serialization
- user
id: entity.node
plugin_id: 'entity:node'
granularity: resource
configuration:
methods:
- GET
- POST
- DELETE
- PATCH
formats:
- hal_json
- json
authentication:
- basic_auth
- cookie
GET:
supported_formats:
- hal_json
- json
supported_auth:
- basic_auth
- cookie
POST:
supported_formats:
- hal_json
- json
supported_auth:
- basic_auth
- cookie
DELETE:
supported_formats:
- hal_json
- json
supported_auth:
- basic_auth
- cookie
PATCH:
supported_formats:
- hal_json
- json
supported_auth:
- basic_auth
- cookie

which appears to be a mixture of the old and new rest configurations. There was no change to the "REST" user permissions.

I've added the dependencies from #31.

@Senpai and @jlscott please file a separate issue regarding the upgrade.

I've will apply this patch

Use Rest UI 8.x-1.12 for Drupal core < 8.2

In Drupal 8.2.1, there is a default rest config for entity:node located in core/modules/rest/config/optional/rest.resource.entity.node.yml, which sets the default value of granularity to "resource". We need to override the value with "method".

Also need to remove following resource-level attributes in the config, so that changing granularity from "resource" to "method" would not give errors

configuration:
  methods:
    - GET
    - POST
    - PATCH
    - DELETE
  formats:
    - hal_json
  authentication:
    - basic_auth

=========
I am so sorry that I hide the attachments uploaded by dawehner and clemens.tolboom, because I clicked something wrong when posting my comments. However in the comment edit page I cannot find a way to fix it, do anyone have experience about this and could you teach me how to fix it? Sorry and thank you!

@Arthur Chan thanks for your finding regarding granularity. Can you please file a new issue and reapply your patches to latest dev version? Thanks :)

I guess @Arthur Chan was spot on regarding the upgrade quirks regarding upgrading mentioned by @Senpai and @jlscott

Do REST requests work after this patch was committed? I'm trying to GET/POST on node/{node}?_format=json and I get Not acceptable format: json

I'm using Drupal 8.2 and Restui 8.1.3. I have also enabled the necessary permissions

@gvso I encountered something similar, you may have a look at #2799993: REST resource endpoint returns "NotAcceptableHttpException" if no HTML route is defined at the same URL.

I'm having the same error as @gvso. Requests with node/{node}?_format=json will end with the error message Not acceptable format: json and the error message will be rendered in the specified format.

The REST service for /user/{user} is working well.

I am still getting the same error on accessing nodes - node/{node}?_format=json
{"message":"Not acceptable format: json"}
Drupal Version - 8.2.3
ResUI - 8.x-1.13

I've added #2831716: REST UI does not support "resource" granularity as a related issue

Not sure if that is a helper, but I managed to resolve the problem by this code;

<?php
$config = \Drupal::service('config.factory')->getEditable('rest.resource.entity.node');

  $config->set("configuration.formats.1", "json")
    ->set("configuration.formats.2", "xml")
    ->set("configuration.authentication.1", "cookie")
    ->save();
?>

Now I am getting all information in json about nodes. It is a workaround, however problem still persists.

The solution here was inadequate. See #2831716-13: REST UI does not support "resource" granularity.