From 31e1dfb2ddf6571c85db8f82c2da69ca77b0b5d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ph=C3=A9na=20Proxima?= <adam@phenaproxima.net>
Date: Fri, 1 Apr 2022 12:30:06 -0400
Subject: [PATCH] Use properties and rename stuff

---
 .../1.0.0/src/PreUpdateService.php            | 20 -------
 .../1.0.0/updated_module.services.yml         | 10 +++-
 .../1.1.0/src/PostUpdateService.php           | 20 -------
 .../1.1.0/updated_module.services.yml         | 10 +++-
 .../package_manager_test_api.services.yml     |  8 ++-
 .../src/SystemChangeRecorder.php              | 57 ++++++++++++-------
 .../tests/src/Build/PackageUpdateTest.php     | 18 +++---
 7 files changed, 68 insertions(+), 75 deletions(-)
 delete mode 100644 package_manager/tests/fixtures/updated_module/1.0.0/src/PreUpdateService.php
 delete mode 100644 package_manager/tests/fixtures/updated_module/1.1.0/src/PostUpdateService.php

diff --git a/package_manager/tests/fixtures/updated_module/1.0.0/src/PreUpdateService.php b/package_manager/tests/fixtures/updated_module/1.0.0/src/PreUpdateService.php
deleted file mode 100644
index dfe58ac336..0000000000
--- a/package_manager/tests/fixtures/updated_module/1.0.0/src/PreUpdateService.php
+++ /dev/null
@@ -1,20 +0,0 @@
-<?php
-
-namespace Drupal\updated_module;
-
-/**
- * A generic service class to prove that services are reloaded after an update.
- */
-class PreUpdateService {
-
-  /**
-   * Returns a string value.
-   *
-   * @return string
-   *   The string value.
-   */
-  public function myMethod(): string {
-    return 'PreUpdateService:myMethod()';
-  }
-
-}
diff --git a/package_manager/tests/fixtures/updated_module/1.0.0/updated_module.services.yml b/package_manager/tests/fixtures/updated_module/1.0.0/updated_module.services.yml
index 942bacd72f..9eb7adf2b4 100644
--- a/package_manager/tests/fixtures/updated_module/1.0.0/updated_module.services.yml
+++ b/package_manager/tests/fixtures/updated_module/1.0.0/updated_module.services.yml
@@ -1,5 +1,9 @@
 services:
-  updated_module.changed_service:
-    class: Drupal\updated_module\PreUpdateService
+  updated_module.existing_service:
+    class: stdClass
+    properties:
+      value: 'Pre-update value'
   updated_module.deleted_service:
-    class: Drupal\updated_module\PreUpdateService
+    class: stdClass
+    properties:
+      value: 'Deleted service, should not exist after update'
diff --git a/package_manager/tests/fixtures/updated_module/1.1.0/src/PostUpdateService.php b/package_manager/tests/fixtures/updated_module/1.1.0/src/PostUpdateService.php
deleted file mode 100644
index e01fb5ec7f..0000000000
--- a/package_manager/tests/fixtures/updated_module/1.1.0/src/PostUpdateService.php
+++ /dev/null
@@ -1,20 +0,0 @@
-<?php
-
-namespace Drupal\updated_module;
-
-/**
- * A generic service class to prove that services are reloaded after an update.
- */
-class PostUpdateService {
-
-  /**
-   * Returns a string value.
-   *
-   * @return string
-   *   The string value.
-   */
-  public function myMethod(): string {
-    return 'PostUpdateService:myMethod()';
-  }
-
-}
diff --git a/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.services.yml b/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.services.yml
index 77373c65a9..fea0247fe2 100644
--- a/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.services.yml
+++ b/package_manager/tests/fixtures/updated_module/1.1.0/updated_module.services.yml
@@ -1,8 +1,12 @@
 services:
-  updated_module.changed_service:
-    class: Drupal\updated_module\PostUpdateService
+  updated_module.existing_service:
+    class: stdClass
+    properties:
+      value: 'Post-update value'
   updated_module.added_service:
-    class: Drupal\updated_module\PostUpdateService
+    class: stdClass
+    properties:
+      value: 'New service, should not exist before update'
   updated_module.post_apply_subscriber:
     class: Drupal\updated_module\PostApplySubscriber
     arguments:
diff --git a/package_manager/tests/modules/package_manager_test_api/package_manager_test_api.services.yml b/package_manager/tests/modules/package_manager_test_api/package_manager_test_api.services.yml
index cbfebc093b..ee523e2115 100644
--- a/package_manager/tests/modules/package_manager_test_api/package_manager_test_api.services.yml
+++ b/package_manager/tests/modules/package_manager_test_api/package_manager_test_api.services.yml
@@ -6,6 +6,12 @@ services:
       - '@state'
       - '@router.no_access_checks'
       - '@user.permissions'
-      - '@updated_module.changed_service'
+      # The following services are defined by updated_module in both its
+      # 1.0.0 and 1.1.0 versions, so they must be nullable in order for
+      # this event subscriber to be instantiated if updated_module is
+      # not installed.
+      - '?@updated_module.existing_service'
+      - '?@updated_module.added_service'
+      - '?@updated_module.deleted_service'
     tags:
       - { name: event_subscriber }
diff --git a/package_manager/tests/modules/package_manager_test_api/src/SystemChangeRecorder.php b/package_manager/tests/modules/package_manager_test_api/src/SystemChangeRecorder.php
index 6fb48ea54a..ec8bff1257 100644
--- a/package_manager/tests/modules/package_manager_test_api/src/SystemChangeRecorder.php
+++ b/package_manager/tests/modules/package_manager_test_api/src/SystemChangeRecorder.php
@@ -46,11 +46,25 @@ class SystemChangeRecorder implements EventSubscriberInterface {
   private $permissionHandler;
 
   /**
-   * The changed service.
+   * A generic service which is different before and after the update.
    *
-   * @var \Drupal\updated_module\PreUpdateService|\Drupal\updated_module\PostUpdateService
+   * @var \stdClass
    */
-  private $changedService;
+  private $existingService;
+
+  /**
+   * A generic service which was added in the newly updated module.
+   *
+   * @var \stdClass
+   */
+  private $addedService;
+
+  /**
+   * A generic service which was deleted from the newly updated module.
+   *
+   * @var \stdClass
+   */
+  private $deletedService;
 
   /**
    * Constructs a SystemChangeRecorder object.
@@ -61,17 +75,25 @@ class SystemChangeRecorder implements EventSubscriberInterface {
    *   The state service.
    * @param \Symfony\Component\Routing\RouterInterface $router
    *   The router service.
-   * @param \Drupal\user\PermissionHandlerInterface $permissionHandler
+   * @param \Drupal\user\PermissionHandlerInterface $permission_handler
    *   The permission handler service.
-   * @param object $changedService
+   * @param \stdClass $existing_service
    *   A generic service which will be different after the update.
+   * @param \stdClass|null $added_service
+   *   A generic service which was added in the newly updated module, or NULL
+   *   if it is not defined.
+   * @param \stdClass|null $deleted_service
+   *   A generic service which was deleted from the newly updated module, or
+   *   NULL if it is not defined.
    */
-  public function __construct(PathLocator $path_locator, StateInterface $state, RouterInterface $router, PermissionHandlerInterface $permissionHandler, object $changedService) {
+  public function __construct(PathLocator $path_locator, StateInterface $state, RouterInterface $router, PermissionHandlerInterface $permission_handler, \stdClass $existing_service, ?\stdClass $added_service, ?\stdClass $deleted_service) {
     $this->pathLocator = $path_locator;
     $this->state = $state;
     $this->router = $router;
-    $this->permissionHandler = $permissionHandler;
-    $this->changedService = $changedService;
+    $this->permissionHandler = $permission_handler;
+    $this->existingService = $existing_service;
+    $this->addedService = $added_service;
+    $this->deletedService = $deleted_service;
   }
 
   /**
@@ -108,27 +130,24 @@ class SystemChangeRecorder implements EventSubscriberInterface {
     $results['new permission exists'] = array_key_exists('added permission', $permissions) ? 'exists' : 'not exists';
 
     // Check if changes to an existing service are picked up.
-    $results['return value of changed service'] = $this->changedService->myMethod();
+    $results['value of existing service'] = $this->existingService->value;
+
     // Check if a service removed from the updated module is available.
-    if (\Drupal::hasService('updated_module.deleted_service')) {
+    if ($this->deletedService) {
       $results['deleted service exists'] = 'exists';
-      /** @var \Drupal\updated_module\PreUpdateService $deletedService */
-      $deletedService = \Drupal::service('updated_module.deleted_service');
-      $results['return value of deleted service'] = $deletedService->myMethod();
+      $results['value of deleted service'] = $this->deletedService->value;
     }
     else {
       $results['deleted service exists'] = 'not exists';
     }
 
     // Check if a service added in the updated module is available.
-    if (\Drupal::hasService('updated_module.added_service')) {
-      $results['added service exists'] = 'exists';
-      /** @var \Drupal\updated_module\PostUpdateService $addedService */
-      $addedService = \Drupal::service('updated_module.added_service');
-      $results['return value of added service'] = $addedService->myMethod();
+    if ($this->addedService) {
+      $results['new service exists'] = 'exists';
+      $results['value of added service'] = $this->addedService->value;
     }
     else {
-      $results['added service exists'] = 'not exists';
+      $results['new service exists'] = 'not exists';
     }
 
     $phase = $event instanceof PreApplyEvent ? 'pre' : 'post';
diff --git a/package_manager/tests/src/Build/PackageUpdateTest.php b/package_manager/tests/src/Build/PackageUpdateTest.php
index 84b23e00ff..b69a6015eb 100644
--- a/package_manager/tests/src/Build/PackageUpdateTest.php
+++ b/package_manager/tests/src/Build/PackageUpdateTest.php
@@ -77,10 +77,10 @@ class PackageUpdateTest extends TemplateProjectTestBase {
       'title of changed permission' => 'permission',
       'deleted permission exists' => 'exists',
       'new permission exists' => 'not exists',
-      'return value of changed service' => 'PreUpdateService:myMethod()',
+      'value of existing service' => 'Pre-update value',
       'deleted service exists' => 'exists',
-      'return value of deleted service' => 'PreUpdateService:myMethod()',
-      'added service exists' => 'not exists',
+      'value of deleted service' => 'Deleted service, should not exist after update',
+      'new service exists' => 'not exists',
     ];
     $this->assertSame($expected_pre_apply_results, $results['pre']);
 
@@ -101,14 +101,14 @@ class PackageUpdateTest extends TemplateProjectTestBase {
       'deleted permission exists' => 'not exists',
       // Permissions added to the updated module should be available.
       'new permission exists' => 'exists',
-      // Method called on changed service should now return the updated value.
-      'return value of changed service' => 'PostUpdateService:myMethod()',
+      // The existing generic service should have a new string value.
+      'value of existing service' => 'Post-update value',
       // Services deleted from the updated module should not be available.
       'deleted service exists' => 'not exists',
-      // Services added to the updated module should be available.
-      'added service exists' => 'exists',
-      // Method called on new service should return the value.
-      'return value of added service' => 'PostUpdateService:myMethod()',
+      // Services added to the updated module should be available and return
+      // the expected value.
+      'new service exists' => 'exists',
+      'value of new service' =>  'New service, should not exist before update',
     ];
     $this->assertSame($expected_post_apply_results, $results['post']);
   }
-- 
GitLab