Remove rest.settings.yml, use rest_resource config entities

This currently serves as documentation I guess, so we should probably move it somewhere?

Also, having a single config file for this is likely going to cause trouble and for example makes it harder to deploy default service configuration with a module, but I doubt we're able to change that now.

+1 for committing this patch.

Maybe comment the node settings instead to have an in code example?

We already have docs at https://www.drupal.org/node/2096019 .

I agree with clemens, I think we should comment out the example config since it is surely useful.

In case this is really helpful, we could just go with an "example." file put in some directory

@larowlan how often is the config file scanned? In the past it was copied over to the config/active once.

OTOH using REST UI makes it's content unimportant.

Yep, comments don't survive an import into active, so it should only affect the initial import, I don't think that makes a measurable difference.

Cool, so the only remaining thing to do is a test case that demonstrates the spectacular site blow up that this patch fixes :-)

Ok, looks good to me!

Adding a web test solely for this purpose seems odd. How about we remove node from the list of modules to enable in RESTTestBase. Which is a doubly good thing since faster tests!

Kernel tests don't install default config, so that will not make it fail unless we'd also force it to import the config.

@Berdir but abstract class RESTTestBase extends WebTestBase

Tests failed because RESTTestBase was using self:: in the setUp()

-    // Create a test content type for node testing.
-    $this->drupalCreateContentType(array('name' => 'resttest', 'type' => 'resttest'));
+    // Create a test content type for node testing if appropriate.
+    if (in_array('node', static::$modules)) {
+      $this->drupalCreateContentType(array('name' => 'resttest', 'type' => 'resttest'));
+    }

Looks proper to me.

+++ b/core/modules/rest/src/Tests/RESTTestBase.php
@@ -48,15 +48,17 @@
-    // Create a test content type for node testing.
-    $this->drupalCreateContentType(array('name' => 'resttest', 'type' => 'resttest'));
+    // Create a test content type for node testing if appropriate.
+    if (in_array('node', static::$modules)) {
+      $this->drupalCreateContentType(array('name' => 'resttest', 'type' => 'resttest'));
+    }

In the off chance that ATest has 'node' in $modules, and BTest extends ATest, I guess BTest will not have the content type created, but it should? Better change the in_array() check to \Drupal::moduleHandler()->moduleExists('node')?

It is a bit sad, because this makes it harder for people to enable REST support for nodes. It no longer is simple :(

No, I think that would be worse, but meh, we seem to have to go with #31

As berdir mentioned at #2, a single file for providing all entities config causes troubles like this (I faced this in a profile, where I had to set the settings in hook_install() because providing a config file failed.

It could also help with dawehner concerns at #33, as we could provide a rest_node module in core only with the proper config, and it would be simple again.

Is this feasible at this stage?

I think the least disruptive solution is to fix it like this #2397271: REST configuration fails if it contains a plugin reference that does not exist

But the correct solution is to have a configuration entity rest.settings.node so that it is deployable.

@larowlan well the fix in #2397271: REST configuration fails if it contains a plugin reference that does not exist removes the dependency on the node module since with that change wherever rest.settings is used we check that we have a plugin for it. So that's the quick fix.

I think this issue should be repurposed to consider changing the rest module to use config entities. It makes little sense that ...

      $plugin = $this->manager->getInstance(array('id' => $id));

      foreach ($plugin->routes() as $name => $route) {
        $method = $route->getRequirement('_method');
        // Only expose routes where the method is enabled in 

We're checking if the route is enabled in the configuration... the plugin should only return configured routes.

Is the following assumption true:
If rest would use config entities for the endpoint configuration then the node module could provide such a configuration entity as optional configuration?

Oh, yes, optional configuration should make this easier. But it seems to me like rest should better ship with that optional configuration for all relevant core modules. (Although possibly "all relevant core modules" really means only "node".)

How about something like this to get started?

I don't see any problems with this. The RestEndpointRepository seems like a slightly unnecessary layer on top of the storage, and the actual endpoint config is missing. Also the changes to RestTestBase from the earlier patches should be applied. Let's add it and see some test runs?

Please fix issue summary as per #40

I think this issue should be repurposed to consider changing the rest module to use config entities.

May I assume we now will #2300677: JSON:API POST/PATCH support for fully validatable config entities? If so please close that set of issues preferably with a motivation.

1. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,49 @@
   \ No newline at end of file
   

   No newline

2. +++ b/core/modules/rest/src/RestEndpointInterface.php
   @@ -0,0 +1,12 @@
   \ No newline at end of file
   

   No newline

3. +++ b/core/modules/rest/src/RestEndpointRepository.php
   @@ -0,0 +1,40 @@
   +} ¶
   \ No newline at end of file
   

   Whitespace
   No newline

4. +++ b/core/modules/rest/src/RestEndpointRepositoryInterface.php
   @@ -0,0 +1,18 @@
   \ No newline at end of file
   

   No newline

No, those two issues have nothing in common. This is about using config entities to store rest.module's configuration.

@Berdir thanks. Fixing

Updates issue summary.

I'll work on this now.
I first wanted to get the config entity done first. Now i will work on the feedback and start working down the remaining tasks as state in the IS:

1. Switch to config entities for REST endpoint configuration
2. Fix HEAD tests
3. Incorporate test changes from earlier patches
4. Use PluginCollections for easy dependency generation and plugin instantiation
5. Try to move some methods from the Rescource Plugin onto the Configuration entity

Let's see if HEAD tests work now.

@44: Removed EndpointRepository
@45: Fixed those stupid ones ...

Added Task to IS:

• Decide what endpoints should be shipped with the REST module as optional configuration

Node should probably be be only one as the current design would would automatically enable an endpoint if it was shipped as optional configuration if all dependencies are met.
We could also add an active flag to denote whether an endpoint is enabled. In that case it might be better to for each/some core module/s to provide a default configuration instead of the REST module providing one for each of them.

This should fix those test's & also @44: This should contain all relevant changes from #31.

This will hopefully fix the "Invalid character in Config object name" Exceptions.

I changed the the resource plugin ids generated by the EntityDeriver to look like 'entity__*' instead of 'entity:*'.

Fixed lines missed in refactoring of RESTTestBase.

REST endpoint entities now have an correctly working id() ....

This should only have the page cache test's failing.

Apparently neither . nor : are valid as part of a config entity name, switched back to __.
Added optional configuration for the node module as the failing page cache test expects an enabled node rest endpoint after REST module installation.
The plugin id is generated automatically on entity creation.

Fixed cache tag assertion in REST module's PageCacheTest.
Fixed permission generation.

This should remove the last fatal error. Still no idea why id is not set ...

This should fix all fails except the ones in Drupal\page_cache\Tests\PageCacheTest.

This is a long shot but i think that fails because in the REST endpoint for node I hard coded the module dependencies which it should have:

• node
• basic_auth
• hal

Added interdiff against the patch from #31.
This patch also removes the rest.settings.yml and its schema now.
Updated IS to reflect the current progress.

Updated IS with API changes.

The

1. +++ b/core/modules/rest/config/optional/rest.endpoint.entity__node.yml
   @@ -0,0 +1,28 @@
   +dependencies:
   +  module:
   +    - node
   +    - basic_auth
   +    - hal
   

   These dependencies must be calculable and atm I don't see how they are. Also I think that this might belong in node/config/optional

2. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,85 @@
   +class RestEndpoint extends ConfigEntityBase implements RestEndpointInterface {
   
   +++ b/core/modules/rest/src/RestEndpointInterface.php
   @@ -0,0 +1,22 @@
   +interface RestEndpointInterface extends ConfigEntityInterface {
   

   This should implement EntityWithPluginCollectionInterface

@73: That's on the todo list ;-)

@73.1: I thought about that as well, whether to put the endpoint into the REST module or node. ++ for node from me

Added methods to manage the configuration array of the config entity & removed the getter & setter.

The config entity now implements EntityWithPluginCollectionInterface which should calculate the additional dependencies for the resource plugin.

I also wanted to calculate additional dependencies based on the supported formats and authentication provider but those are not identified easily. Apparently both formats and authentication provider are registered via tagged services but there seems to be no way of identifying which module provides that service.
The module could probably be identified by the namespace used for the service class but is there an easier way that i may have overlooked?

Not reviewed yet, just cross-referencing with #2472321: Move rest.module routes to /api, which will conflict a lot I guess.

Added code to ensure that the request method is upper case and throw an exception if a not matching method is passed.
I could not find an error in the API but maybe there is an error in the usage of the API in tests?

Realized there is a tag for that ;-)

Well this should fix that ...
Apparently did not see that in protected function enableService($resource_type, $method = 'GET', $format = NULL, $auth = NULL) { the $auth parameter is expected to be an array -_-
Added an array type hint with this patch.

Well this showed that i missed something during refactoring ...
Should be green now.

Just some quick drive by review.

1. +++ b/core/modules/rest/config/optional/rest.endpoint.entity__node.yml
   @@ -0,0 +1,28 @@
   +plugin_id: "entity:node"
   
   +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,239 @@
   +    // The config entity id looks like the plugin id but uses _ instead of : because : is not valid for config entities.
   +    if (!isset($this->plugin_id) && isset($this->id)) {
   +      $this->plugin_id = str_replace('__', ':', $this->id);
   +    }
   

   Why do you have to care about this here, given that the entity itself seems to have stored that information?

2. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,239 @@
   +<?php
   +
   

   Nitpick: Missing @file Contains bit.

3. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,239 @@
   +    $this->pluginManager = \Drupal::service('plugin.manager.rest');
   ...
   +    $plugin_definition = \Drupal::service('plugin.manager.rest')
   +      ->getDefinition(['id' => $this->getResourcePluginID()]);
   

   Do we want to use the "injected" value?

#87.1: That is in there for the first time the entity is created. The plugin id is generated at that time and stored after that. On each subsequent load, the plugin id will be loaded from storage and won't be generated again.

I will fix #87.2 & #87.3 in a subsequent patch hopefully along with the test failures.

That single missing ! -_-

For the test bot.

Grasping for straws now ...
Trying to backtrack to find out what is causing those changes.

To evaluate whether we should make this change during the beta, we need to update the issue summary according to https://www.drupal.org/core/beta-changes to explain what improvements it makes and what the disruption of the change would be. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

I tried my best to fill out the template appropriately.

How could I ever have expected that the last patch would be green -_-
Maybe this one.

Updated IS example with the new methods.

• Switched to DefaultSingleLazyPluginCollection as we are dealing with only 1 plugin.
• Updated EntityResource to implement DependentPluginInterface & identify the entity type provider as module dependency.

This should include into the dependencies: the resource plugin provider & in case of an EntityResource derived plugin, the entity type provider

Renamed <hasSupportedAuthenticationProviders($method) to supportsAuthenticationProviders($method).
Also in lack of any other proposal, implemented my idea from #77:

I also wanted to calculate additional dependencies based on the supported formats and authentication provider but those are not identified easily. Apparently both formats and authentication provider are registered via tagged services but there seems to be no way of identifying which module provides that service.
The module could probably be identified by the namespace used for the service class but is there an easier way that i may have overlooked?

This works as follows:

• Dependency list generation:
  1. On container rebuild a dependency list for both available authentication providers & formats is generated in RestServiceProvider along the idea of #77
  2. Those lists are saved as a container parameters: rest.dependencies.auth & rest.dependencies.format
• Dependency calculation for the config entity:
  1. When dependencies for the config entities are calculated in calculateDependencies(), those parameters are accessed via the ContainerAwareTrait
  2. Based on the dependency lists the supported authentication provides and formats for each request method are checked and necessary module dependencies are added.
  3. After this point the config entity should have the appropriate dependencies (At least I could verify that the dependencies looked like the ones on the optional configuration)
• Fix missing dependencies:
  1. The config entity also implements onDependencyRemoval(array $dependencies)
  2. If a module is uninstalled, the dependency lists are checked as well
  3. If the affected dependencies are related to authentication providers or formats the available request methods are checked for those an the corresponding auth providers and formats are removed from those request methods
  4. Also if the request method has no supported authentication providers or formats, it's removed from the configuration
  5. If there is any request method configuration left afterwards the method will return TRUE marking the dependency problems as resolved
  6. If all dependency problems resolved successfully the config entity will remain in case not, it's deletion won't be prevented(Like for exampe the resource plugin provider was uninstalled)

I would probably want to cache the affected authentication provider & formats from onDependencyRemoval(array $dependencies) in a static property on the class to improve performance if multiple endpoints are affected.

But before that:

• Let's see what fails
• Decide whether to keep this dependency code

Tackled "Try to move some methods from the Rescource Plugin onto the Configuration entity".

The patch contains:

• Made getBaseRoute($canonical_path, $method) public so it can be accessed inside RestEndpoint.
• Moved route generation from ResourceBase to the config entity type.
• Now put the REST endpoint id on the route instead of the plugin id.

I only moved the route generation part, because it explicitly depends on configuration.

Once this is green we only have to

• Decide whether to use the elaborate dependency generation code introduced with #2308745-103: Remove rest.settings.yml, use rest_resource config entities or to rely only on resource plugin dependencies or maybe find another solution
• Decide what endpoints should be shipped with the REST module as optional configuration or maybe even moved to their respective modules

That should be it for now ;-)

Attempted reroll.

Tried to incorporate changes changes from other since the last patch:

• #2489912: Update Drupal.org URL in core modules
• #2463009: Introduce CacheableResponseInterface: consolidate ways of setting X-Drupal-Cache-Tags/Contexts headers
• #2336247: Make Relation and Type domain configurable based on context

Let's see what breaks ;-)

This should fix the failure by enabling the missing `basic_auth` module. The rest endpoint that this patch ships with the rest module as optional dependency depends on:

• node
• basic_auth
• hal

Wrong patch .....

Reroll.

Also, I guess this is unfortunately not welcome in 8.0.x anymore? :)

Yes this could only happen in a minor release.

Also we're going to have to look at a backwards compatibility layer for this somehow, not sure what that looks like though.

One possible option

- when saving a config entity, convert the old config key to config entities and strip them out of the config object itself.

- when loading a config entity, convert the config entities back to the old config format on the config object

This would cover modules loading and saving the old config object, including default configuration.

When we load, we'd have to add some kind of identifier to indicate that the items are 'fake' to avoid doing the work on save.

Could put this into its own event listener so it's out of the way of everything else, and then potentially could be disabled too. And that event listener is then a place we could trigger E_USER_DEPRECATED in a later release if modules haven't been ported.

If that's actually enough to provide bc, it could be quite a small thing to add.

1. +++ b/core/modules/rest/config/schema/rest.schema.yml
   @@ -45,3 +38,17 @@ rest_request:
   +rest.endpoint.*:
   +  type: config_entity
   +  label: 'REST endpooint'
   

   Pedantic point: There is no such thing as a "rest endpoint". This should be called "rest resource". Endpoints only exist in RPC.

2. +++ b/core/modules/rest/rest.permissions.yml
   @@ -1,2 +1,5 @@
   +administer rest endpoints:
   +  title: 'Administer REST endpoint configuration'
   

   Ibid.

3. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,440 @@
   + * Defines a RestEndpoint configuration entity class.
   + *
   + * @ConfigEntityType(
   + *   id = "rest_endpoint",
   

   Ibid. (Will not repeat further.)

4. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,440 @@
   +  public function __construct(array $values, $entity_type) {
   +    $this->pluginManager = \Drupal::service('plugin.manager.rest');
   +    $this->logger = \Drupal::service('logger.factory')->get('rest');
   +    $this->setContainer(\Drupal::getContainer());
   +    parent::__construct($values, $entity_type);
   

   It's a plugin. Just inject properly with the create() static method. No ContainerAware.

5. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,440 @@
   +  protected function normaliseRestMethod($method) {
   +    $valid_methods = [ 'GET', 'POST', 'PATCH', 'DELETE' ];
   

   Is there a reason PUT isn't whitelisted? We're not using it for Entities but in concept there's no reason for us to not support it.

While there's a lot of new code here, from an API perspective we're just changing the configuration storage format, no? The API for defining a new REST resource is the same, unless you need dependencies and then it's just an addition. So why do we need a BC layer? I'd think all we need is an upgrade hook to convert the old config file to the new config entities and call it a day.

@Crell per my previous comment, modules can provide default configuration.

@catch modules can only change the config via hook_install() now, because otherwise you will get config import error about the config rest.settings exists (installed with rest module)

That's the second point to get that fixed - provide a way to import rest settings for each resource without affecting a whole config object. The same was done for content_translation.settings and language.settings for scalability & performance reasons already.

So maybe better to re-use third party settings like #2363155: content_translation.settings config is not scalable

EDIT: language settings was changed to config entity #2355909: language.settings config is not scalable

#119: "I'd think all we need is an upgrade hook to convert the old config file to the new config entities and call it a day."

Did this, and it fixes some fails. Some other fails are caused by what I think is a bug in system_update_8004(), so I'm suggesting a fix there as well. Also suggesting some helpful fail() messages to UpdatePathTestBase.

#121: "modules can only change the config via hook_install() now"

Not sure what you mean by this, @andypost? The update hook seems to work fine. And if we were to use third_party_settings, what entity type would we hang them on to?

Ignored all of the code/naming review points in #119 for now, I want to make the test greener first (and get someone's confirmation on the changes in this patch).

"modules can only change the config via hook_install() now"

It means that currently, modules can implement hook_install() to modify the rest configuration object - this is what we need a bc layer for per #118. Similarly with install profile configuration overrides.

The rest update should be numbered 8100 since this will go into 8.1.x

Just browsing through the patch file...

1. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,446 @@
   +  public function __construct(array $values, $entity_type) {
   

   Missing documentation.

2. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,446 @@
   +    // the dependency lists for authentication providers & formats
   +    //   generated on container build
   ...
   +      // add dependencies based on the supported authentication providers
   ...
   +      // add dependencies based on the supported authentication formats
   

   Punctuation and formatting.

3. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,446 @@
   +    // Only module related dependencies can be fixed as all other types
   +    //   of dependencies can't as they where not generated based on
   +    //   supported authentication providers or formats
   

   More punctuation and formatting all throughout this method...

4. +++ b/core/modules/rest/src/Plugin/ResourceBase.php
   @@ -184,23 +130,13 @@ public function availableMethods() {
   -  protected function getBaseRoute($canonical_path, $method) {
   +  public function getBaseRoute($canonical_path, $method) {
   

   Doesn't this break existing plugins that override the method and keep the old visibility?

5. +++ b/core/modules/rest/src/Plugin/ResourceInterface.php
   @@ -20,18 +21,6 @@
   -  public function routes();
   
   @@ -51,4 +40,16 @@ public function permissions();
   +  public function getBaseRoute($canonical_path, $method);
   

   ...or is it ok to be changing ResourceBase and ResourceInterface?

6. +++ b/core/modules/rest/src/RestServiceProvider.php
   @@ -0,0 +1,65 @@
   +    // Build a dependency list for available authentication providers
   ...
   +        // extract module name based on class namespace
   ...
   +      // Register the dependency list in the container
   ...
   +    // Build a dependency list for available formats
   ...
   +        // extract module name based on class namespace
   ...
   +      // Register the dependency list in the container
   

   Periods and stuff.

7. +++ b/core/modules/rest/src/RestServiceProvider.php
   @@ -0,0 +1,65 @@
   +    foreach ($container->findTaggedServiceIds('authentication_provider') AS $service_id => $service_tags) {
   ...
   +    foreach ($container->findTaggedServiceIds('encoder') AS $service_id => $service_tags) {
   ...
   +          foreach ($service_tags AS $service_tag) {
   

   AS => as

Reroll.

Now that it applies again let's put some love in it.

It addresses the formatting issues, adds missing doc blocks (#124) and renames the update hook to rest_update_8100 (#123).

1. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,452 @@
   +    $this->pluginManager = \Drupal::service('plugin.manager.rest');
   +    $this->logger = \Drupal::service('logger.factory')->get('rest');
   +    $this->setContainer(\Drupal::getContainer());
   ...
   +    $auth_dependencies_list = $this->container->getParameter('rest.dependencies.auth');
   +    $format_dependencies_list = $this->container->getParameter('rest.dependencies.format');
   ...
   +    $format_dependencies_list = $this->container->getParameter('rest.dependencies.format');
   +    $auth_dependencies_list = $this->container->getParameter('rest.dependencies.auth');
   

   At least inject the parameters directly instead.

2. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,452 @@
   +  public function routes() {
   

   IMHO this should stay on the plugin somehow. This kind of method is something people really might want to override on the plugin level. The config entity is just about storage for me, so it should not contain that kind of logic.

3. +++ b/core/modules/rest/src/Entity/RestEndpoint.php
   @@ -0,0 +1,452 @@
   +  protected function normaliseRestMethod($method) {
   +    $valid_methods = [ 'GET', 'POST', 'PATCH', 'DELETE' ];
   

   nitpick: use remove the space after "["

4. +++ b/core/modules/rest/src/Plugin/rest/resource/EntityResource.php
   @@ -31,7 +35,49 @@
   +    parent::__construct($configuration, $plugin_id, $plugin_definition, $serializer_formats, $logger);
   +    $this->entity_type_definition = $entity_manager->getDefinition($plugin_definition['entity_type']);
   

   A, let's use $this->entityType and $entity_type_manager instead.

Let's especially not forget to consider 124.4 and 124.5 either.

I'll work a bit on this

Fixed nitpicks. Big issues remain though:

• ResourceInterface should not be changed (#124)
• A config load/save BC layer needs to be added (#118)
• The "endpoint" entity should be renamed? (#119)

#119.4: EntityInterface::create() ≠ ContainerFactoryPluginInterface::create()... right?

Setting to Needs review to trigger bots although it needs work as per above.

Some changed on top of #133

#119.4: EntityInterface::create() ≠ ContainerFactoryPluginInterface::create()... right?

Yeah :( I'm thinking that it would actually make sense to add this big amount of logic into its own service

I was sent the following feedback by Edward Faulkner of Ember.js:

There's no good reason to choose auth methods and serialization formats per endpoint per verb.

I too have been frustrated/surprised by this.

Technically this is off-topic for this issue. But, this issue is completely changing (well, removing) rest.settings.yml. So, if we ever have a chance of changing this, it's in this issue.

Thoughts? An explanation justifying the current design would be welcome too, of course.

Well, we end to support everything what we supported before, otherwise we have an API break, so I don't see how we can change this here.
This though sounds like a discussion for D9. As far as I understand the rest module it tries to be quite flexible in its configuration, so it can fit into more usecases than a normal simple REST integration.

Changing title to clarify what this issue actually does.

Wouldn't rest_resource config entities make more sense than rest_endpoint config entities?

That'd also be consistent with the plugin interface name (https://api.drupal.org/api/drupal/core%21modules%21rest%21src%21Plugin%2...) and the annotation (https://api.drupal.org/api/drupal/core%21modules%21rest%21src%21Annotati...).

Re #135

@Wim Leers
While I understand your points here, we need to take into account two things

a) We want to make progress on this issue itself. It solves a ton of problems rest has at the moment
b) People might have setup crazy things, so let's not try to break things.

IMHO We should split up rest though in two parts:

1. The generic code, usable by probably all REST implementations
2. The concrete rest implementation using the plugin system to register recourses. The second part could then be over time replaced by another module doing something more sane

Yep, I understand that, and I agree. I split off #135 into #2664780: Remove REST's resource- and verb-specific permissions for EntityResource, but provide BC and document why it's necessary for other resources.

My comments #137 and #138 are on-topic/do not make out-of-scope requests, and are just trying to move this issue forward.

Yeah, this would be pretty nice. Having everything in rest.settings can be a pain in testing as well because you pull along all the other endpoint requirements when testing an individual endpoint. This would allow more targeted, less error prone units of testing for site developers.

Switching REST configuration to config entities makes a ton of sense for a myriad of reasons, so +1 on that. However, "endpoint" is the completely wrong terminology here. :-) There is no such thing as a "REST endpoint". REST has resources.

I'm a little unclear from the patch what the scope of the new config entity is. Is it one entity per content entity being exposed (ie, one for node, one for user, one for term, etc.) or one per piece of the content entity being exposed (GET node, POST node, DELETE node, GET user, POST user, DELETE user, etc.)? That will help determine the proper name. (REST Resource is already taken by the main plugin of REST module.)

I'm a little unclear from the patch what the scope of the new config entity is. Is it one entity per content entity being exposed (ie, one for node, one for user, one for term, etc.

The patch uses one per entity type / rest plugin.

So the config entity is the configuration for a rest resource plugin.

Agreed that Resource is a better name for it than Endpoint. Leaves the naming overlap.

A pattern that I quite like for that is a Config suffix. Like FieldConfig for example. Classname would be RestResourceConfig, entity type could be rest_resource or rest_resource_config, and the config prefix either way just rest_resource I think.

Per #138, #142 and #144.

I don't see why the naming overlap needs to be a problem per se, because one is an annotation, this is a config entity. Confusion seems unlikely. Consistency with FieldConfig is a good argument though.

#144 sounds good to me.

Rename.

Removed the test added in #2397271: REST configuration fails if it contains a plugin reference that does not exist. The other fail I cannot find the cause of.

+1 to #149. Now the patch removes everything that #2397271: REST configuration fails if it contains a plugin reference that does not exist added.

If you run Drupal\Tests\rest\Kernel\RequestHandlerTest locally, does it pass?