From 2d380c9037669f1b07836137216848f3260fe4b5 Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Fri, 1 Apr 2022 17:28:26 +0000
Subject: [PATCH] Issue #3271144 by kunal.sachdev, phenaproxima, tedbow: Ensure
 new, changed or deleted service definitions are in the correct state after an
 update

---
 .../1.0.0/updated_module.services.yml         |  9 ++++++
 .../1.1.0/updated_module.services.yml         |  8 +++++
 .../src/SystemChangeRecorder.php              | 32 +++++++++++++++++--
 .../tests/src/Build/PackageUpdateTest.php     | 14 ++++++++
 4 files changed, 60 insertions(+), 3 deletions(-)
 create mode 100644 package_manager/tests/fixtures/updated_module/1.0.0/updated_module.services.yml

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
new file mode 100644
index 0000000000..9eb7adf2b4
--- /dev/null
+++ b/package_manager/tests/fixtures/updated_module/1.0.0/updated_module.services.yml
@@ -0,0 +1,9 @@
+services:
+  updated_module.existing_service:
+    class: stdClass
+    properties:
+      value: 'Pre-update value'
+  updated_module.deleted_service:
+    class: stdClass
+    properties:
+      value: 'Deleted service, should not exist after update'
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 1179d2fb06..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,4 +1,12 @@
 services:
+  updated_module.existing_service:
+    class: stdClass
+    properties:
+      value: 'Post-update value'
+  updated_module.added_service:
+    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/src/SystemChangeRecorder.php b/package_manager/tests/modules/package_manager_test_api/src/SystemChangeRecorder.php
index 4738a3157c..b48a5f3a58 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
@@ -54,14 +54,14 @@ 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.
    */
-  public function __construct(PathLocator $path_locator, StateInterface $state, RouterInterface $router, PermissionHandlerInterface $permissionHandler) {
+  public function __construct(PathLocator $path_locator, StateInterface $state, RouterInterface $router, PermissionHandlerInterface $permission_handler) {
     $this->pathLocator = $path_locator;
     $this->state = $state;
     $this->router = $router;
-    $this->permissionHandler = $permissionHandler;
+    $this->permissionHandler = $permission_handler;
   }
 
   /**
@@ -96,10 +96,36 @@ class SystemChangeRecorder implements EventSubscriberInterface {
     $results['deleted permission exists'] = array_key_exists('deleted permission', $permissions) ? 'exists' : 'not exists';
     // Check if a permission added in the updated module is available.
     $results['new permission exists'] = array_key_exists('added permission', $permissions) ? 'exists' : 'not exists';
+
+    // Check if changes to an existing service are picked up.
+    $this->recordServiceValue('updated_module.existing_service', $results);
+    // Check if a service removed from the updated module is available.
+    $this->recordServiceValue('updated_module.deleted_service', $results);
+    // Check if a service added in the updated module is available.
+    $this->recordServiceValue('updated_module.added_service', $results);
+
     $phase = $event instanceof PreApplyEvent ? 'pre' : 'post';
     $this->state->set("system_changes:$phase", $results);
   }
 
+  /**
+   * Checks if a given service exists, and records its ->value property.
+   *
+   * @param string $service_id
+   *   The ID of the service to check.
+   * @param array $results
+   *   The current set of results, passed by reference.
+   */
+  private function recordServiceValue(string $service_id, array &$results): void {
+    if (\Drupal::hasService($service_id)) {
+      $results["$service_id exists"] = 'exists';
+      $results["value of $service_id"] = \Drupal::service($service_id)->value;
+    }
+    else {
+      $results["$service_id exists"] = 'not exists';
+    }
+  }
+
   /**
    * Writes the results of ::recordSystemState() to file.
    *
diff --git a/package_manager/tests/src/Build/PackageUpdateTest.php b/package_manager/tests/src/Build/PackageUpdateTest.php
index 75c03353b9..1823508298 100644
--- a/package_manager/tests/src/Build/PackageUpdateTest.php
+++ b/package_manager/tests/src/Build/PackageUpdateTest.php
@@ -77,6 +77,11 @@ class PackageUpdateTest extends TemplateProjectTestBase {
       'title of changed permission' => 'permission',
       'deleted permission exists' => 'exists',
       'new permission exists' => 'not exists',
+      'updated_module.existing_service exists' => 'exists',
+      'value of updated_module.existing_service' => 'Pre-update value',
+      'updated_module.deleted_service exists' => 'exists',
+      'value of updated_module.deleted_service' => 'Deleted service, should not exist after update',
+      'updated_module.added_service exists' => 'not exists',
     ];
     $this->assertSame($expected_pre_apply_results, $results['pre']);
 
@@ -97,6 +102,15 @@ class PackageUpdateTest extends TemplateProjectTestBase {
       'deleted permission exists' => 'not exists',
       // Permissions added to the updated module should be available.
       'new permission exists' => 'exists',
+      // The existing generic service should have a new string value.
+      'updated_module.existing_service exists' => 'exists',
+      'value of updated_module.existing_service' => 'Post-update value',
+      // Services deleted from the updated module should not be available.
+      'updated_module.deleted_service exists' => 'not exists',
+      // Services added to the updated module should be available and return
+      // the expected value.
+      'updated_module.added_service exists' => 'exists',
+      'value of updated_module.added_service' => 'New service, should not exist before update',
     ];
     $this->assertSame($expected_post_apply_results, $results['post']);
   }
-- 
GitLab