Make ResourceConfig an actual value object: immutable, with no services injected

.

Note that #2 is relative to #2841048: Remove ResourceConfig::getGlobalConfig(), and stop injecting the config factory service in ResourceConfig value objects, it will fail.

1. +++ b/src/Configuration/ResourceConfig.php
   @@ -68,16 +60,6 @@ class ResourceConfig implements ResourceConfigInterface {
   -  public function setEntityTypeId($entity_type_id) {
   
   @@ -85,13 +67,6 @@ class ResourceConfig implements ResourceConfigInterface {
   -  public function setTypeName($type_name) {
   
   @@ -99,13 +74,6 @@ class ResourceConfig implements ResourceConfigInterface {
   -  public function setPath($path) {
   
   @@ -113,20 +81,6 @@ class ResourceConfig implements ResourceConfigInterface {
   -  public function setBundleId($bundle_id) {
   

   We remove all setters, because they make the object mutable.

2. +++ b/src/Configuration/ResourceConfig.php
   @@ -113,20 +81,6 @@ class ResourceConfig implements ResourceConfigInterface {
   -  public function getStorage() {
   -    return $this->entityTypeManager->getStorage($this->entityTypeId);
   -  }
   

   We also delete this method, because it always needs the service to be injected, i.e. it's a hard blocker to making ResourceConfig a value object.

3. +++ b/src/Configuration/ResourceConfig.php
   @@ -134,11 +88,23 @@ class ResourceConfig implements ResourceConfigInterface {
   -  public function __construct(EntityTypeManagerInterface $entity_type_manager) {
   -    $this->entityTypeManager = $entity_type_manager;
   ...
   +  public function __construct($entity_type_id, $bundle_id, $path, $type_name, $deserialization_target_class) {
   

   Rather than retrieving all this "automatically", we move the responsibility for doing this to the ResourceManager service.

4. +++ b/src/Configuration/ResourceManager.php
   @@ -58,11 +58,13 @@ class ResourceManager implements ResourceManagerInterface {
   -          $resource_config = new ResourceConfig($this->entityTypeManager);
   -          $resource_config->setEntityTypeId($entity_type_id);
   -          $resource_config->setBundleId($bundle);
   -          $resource_config->setPath(sprintf('/%s/%s', $entity_type_id, $bundle));
   -          $resource_config->setTypeName(sprintf('%s--%s', $entity_type_id, $bundle));
   +          $resource_config = new ResourceConfig(
   +            $entity_type_id,
   +            $bundle,
   +            sprintf('/%s/%s', $entity_type_id, $bundle),
   +            sprintf('%s--%s', $entity_type_id, $bundle),
   +            $this->entityTypeManager->getDefinition($entity_type_id)->getClass()
   +          );
   

   And as you can tell, it's actually barely makes an difference here!

   If you would prefer that, we could move this logic to a public static ResourceConfig::create() method.

5. +++ b/src/Normalizer/EntityNormalizer.php
   @@ -142,7 +142,9 @@ class EntityNormalizer extends NormalizerBase implements DenormalizerInterface,
   -    return $resource_config->getStorage()->create($data);
   +    return $this->currentContext->getResourceManager()
   +      ->getEntityTypeManager()
   +      ->getStorage($resource_config->getEntityTypeId())->create($data);
   

   This was the only place that was using ResourceConfig::getStorage()! So, it was not at all hard to remove that method.

6. +++ /dev/null
   @@ -1,55 +0,0 @@
   -class ResourceConfigTest extends UnitTestCase {
   

   A unit test for a pure value object is nonsensical. So, we remove this.

This is specifically blocking #2841056: Remove use of plugins.

Also, this is a net reduction: 7 files changed, 34 insertions, 162 deletions :)

Yay! The retested patch came back green, see https://www.drupal.org/pift-ci-job/566430.

Done.

All this made me realize I could've gone slightly further in this patch: since the path and "JSON API type" or "type name" are parameters 3 and 4 and are calculated entirely from parameters 1 and 2 (entity type ID + bundle), I could just move that into the constructor for ResourceConfig. Doing that next.

Now also finished that additional step.

1. +++ b/src/Configuration/ResourceConfig.php
   @@ -92,19 +92,16 @@ class ResourceConfig implements ResourceConfigInterface {
   -   * @param string $path
   -   *   The path at which this entity bundle will be made accessible.
   -   * @param string $type_name
   -   *   The JSON API type.
       * @param string $deserialization_target_class
       *   The deserialization target class.
       */
   -  public function __construct($entity_type_id, $bundle_id, $path, $type_name, $deserialization_target_class) {
   +  public function __construct($entity_type_id, $bundle_id, $deserialization_target_class) {
        $this->entityTypeId = $entity_type_id;
        $this->bundleId = $bundle_id;
   -    $this->path = $path;
   -    $this->typeName = $type_name;
        $this->deserializationTargetClass = $deserialization_target_class;
   +
   +    $this->typeName = sprintf('%s--%s', $this->entityTypeId, $this->bundleId);
   +    $this->path= sprintf('/%s/%s', $this->entityTypeId, $this->bundleId);
      }
   

   So this is the key change.

2. +++ b/src/Configuration/ResourceManager.php
   @@ -61,8 +61,6 @@ class ResourceManager implements ResourceManagerInterface {
              $resource_config = new ResourceConfig(
                $entity_type_id,
                $bundle,
   -            sprintf('/%s/%s', $entity_type_id, $bundle),
   -            sprintf('%s--%s', $entity_type_id, $bundle),
                $this->entityTypeManager->getDefinition($entity_type_id)->getClass()
              );
   

   Which enables this simplification in the actual module logic.

3. +++ b/tests/src/Unit/Context/CurrentContextTest.php
   @@ -82,7 +82,7 @@ class CurrentContextTest extends UnitTestCase {
   -    $resource_config = new ResourceConfig('node', 'article', '/node/article', 'node--article', NodeInterface::class);
   +    $resource_config = new ResourceConfig('node', 'article', NodeInterface::class);
   
   +++ b/tests/src/Unit/Normalizer/ConfigEntityNormalizerTest.php
   @@ -42,7 +42,7 @@ class ConfigEntityNormalizerTest extends UnitTestCase {
   -      ->willReturn(new ResourceConfig('dolor', 'sid', NULL, NULL, NULL));
   +      ->willReturn(new ResourceConfig('dolor', 'sid', NULL));
   
   +++ b/tests/src/Unit/Normalizer/EntityReferenceFieldNormalizerTest.php
   @@ -101,7 +101,7 @@ class EntityReferenceFieldNormalizerTest extends UnitTestCase {
   -    $resource_config = new ResourceConfig('fake_entity_type', 'dummy_bundle', NULL, NULL, NULL);
   +    $resource_config = new ResourceConfig('fake_entity_type', 'dummy_bundle', NULL);
   

   And simplifications like this in tests.

4. +++ b/tests/src/Unit/Normalizer/Value/EntityNormalizerValueTest.php
   @@ -78,7 +79,7 @@ class EntityNormalizerValueTest extends UnitTestCase {
   -    $context = ['resource_config' => new ResourceConfig(NULL, NULL, NULL, 'node', NULL)];
   +    $context = ['resource_config' => new ResourceConfig('node', 'article', NodeInterface::class)];
   
   @@ -109,7 +110,7 @@ class EntityNormalizerValueTest extends UnitTestCase {
   -      'type' => 'node',
   +      'type' => 'node--article',
   

   But quite importantly, it also forces us to use 'realistic' JSON API types in our tests.

   In this test for example, we were using node, but that's impossible to ever see in the real world; in the real world, Drupal 8's JSON API module will always include the bundle in the JSON API type as well, so something like node--article or node--foobarbazsomething.

   In other words: this change forces us to clean things up in tests, to match the real-world.

5. +++ b/tests/src/Unit/Plugin/Deriver/ResourceDeriverTest.php
   @@ -37,7 +37,7 @@ class ResourceDeriverTest extends UnitTestCase {
   -    $resource_manager->all()->willReturn([new ResourceConfig('entity_type_1', 'bundle_1_1', '/entity_type_1/bundle_path_1', 'resource_type_1', EntityInterface::class)]);
   +    $resource_manager->all()->willReturn([new ResourceConfig('entity_type_1', 'bundle_1_1', EntityInterface::class)]);
        $resource_manager->hasBundle(Argument::type('string'))->willReturn(FALSE);
    
        $container = $this->prophesize(ContainerInterface::class);
   @@ -50,14 +50,14 @@ class ResourceDeriverTest extends UnitTestCase {
   
   @@ -50,14 +50,14 @@ class ResourceDeriverTest extends UnitTestCase {
       * @covers ::getDerivativeDefinitions
       */
      public function testGetDerivativeDefinitions() {
   -    $expected = ['jsonapi.resource_type_1' => [
   -      'id' => 'jsonapi.resource_type_1',
   +    $expected = ['jsonapi.entity_type_1--bundle_1_1' => [
   +      'id' => 'jsonapi.entity_type_1--bundle_1_1',
          'entityType' => 'entity_type_1',
          'bundle' => 'bundle_1_1',
          'hasBundle' => FALSE,
   -      'type' => 'resource_type_1',
   +      'type' => 'entity_type_1--bundle_1_1',
          'data' => [
   -        'partialPath' => '/entity_type_1/bundle_path_1',
   +        'partialPath' => '/entity_type_1/bundle_1_1',
   

   Most notably, it forces this clean-up, where the path and type were completely independent from the entity type ID and bundle ID.

6. +++ b/tests/src/Unit/RequestHandlerTest.php
   @@ -45,7 +45,7 @@ class RequestHandlerTest extends UnitTestCase  {
   -    $resource_config = new ResourceConfig(NULL, NULL, NULL, NULL, NULL);
   +    $resource_config = new ResourceConfig($this->randomMachineName(), $this->randomMachineName(), NULL);
   

   And also quite importantly, it allows us to show more explicitly in the test that we can use random entity type ID and random bundle ID in those cases where we're fabricating a ResourceConfig object just to comply with requirements for other objects. In other words: the use of random machine names here shows provably that these objects are effectively unused.

So with #14 and #15, we have even a further net reduction:

18 files changed, 88 insertions, 260 deletions

compared to
7 files changed, 34 insertions, 162 deletions

in #2.

Net reduction of 172 LoC vs 128.

#18: exactly! And yay, this means #2841056: Remove use of plugins is now unblocked!