From 7edf8a6a12397221959c52cb0a3c2b24af2602a4 Mon Sep 17 00:00:00 2001
From: "kunal.sachdev" <kunal.sachdev@3685163.no-reply.drupal.org>
Date: Wed, 6 Apr 2022 15:25:43 +0000
Subject: [PATCH] Issue #3271371 by kunal.sachdev, phenaproxima: Ensure new,
changed or deleted classes are in the correct state after an update
---
.../updated_module/1.0.0/src/ChangedClass.php | 17 +++++++++
.../updated_module/1.0.0/src/DeletedClass.php | 17 +++++++++
.../1.0.0/src/LoadedAndDeletedClass.php | 17 +++++++++
.../updated_module/1.1.0/src/AddedClass.php | 17 +++++++++
.../updated_module/1.1.0/src/ChangedClass.php | 17 +++++++++
.../src/SystemChangeRecorder.php | 36 +++++++++++++++++++
.../tests/src/Build/PackageUpdateTest.php | 18 ++++++++++
7 files changed, 139 insertions(+)
create mode 100644 package_manager/tests/fixtures/updated_module/1.0.0/src/ChangedClass.php
create mode 100644 package_manager/tests/fixtures/updated_module/1.0.0/src/DeletedClass.php
create mode 100644 package_manager/tests/fixtures/updated_module/1.0.0/src/LoadedAndDeletedClass.php
create mode 100644 package_manager/tests/fixtures/updated_module/1.1.0/src/AddedClass.php
create mode 100644 package_manager/tests/fixtures/updated_module/1.1.0/src/ChangedClass.php
diff --git a/package_manager/tests/fixtures/updated_module/1.0.0/src/ChangedClass.php b/package_manager/tests/fixtures/updated_module/1.0.0/src/ChangedClass.php
new file mode 100644
index 0000000000..1ec6c3b956
--- /dev/null
+++ b/package_manager/tests/fixtures/updated_module/1.0.0/src/ChangedClass.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace Drupal\updated_module;
+
+/**
+ * This class will have a different value in updated_module 1.1.0.
+ */
+class ChangedClass {
+
+ /**
+ * The value.
+ *
+ * @var string
+ */
+ public static $value = 'Before Update';
+
+}
diff --git a/package_manager/tests/fixtures/updated_module/1.0.0/src/DeletedClass.php b/package_manager/tests/fixtures/updated_module/1.0.0/src/DeletedClass.php
new file mode 100644
index 0000000000..e638683ba8
--- /dev/null
+++ b/package_manager/tests/fixtures/updated_module/1.0.0/src/DeletedClass.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace Drupal\updated_module;
+
+/**
+ * This class is removed from updated_module 1.1.0.
+ */
+class DeletedClass {
+
+ /**
+ * The value.
+ *
+ * @var string
+ */
+ public static $value = 'This class will be deleted';
+
+}
diff --git a/package_manager/tests/fixtures/updated_module/1.0.0/src/LoadedAndDeletedClass.php b/package_manager/tests/fixtures/updated_module/1.0.0/src/LoadedAndDeletedClass.php
new file mode 100644
index 0000000000..6dd93dd98b
--- /dev/null
+++ b/package_manager/tests/fixtures/updated_module/1.0.0/src/LoadedAndDeletedClass.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace Drupal\updated_module;
+
+/**
+ * This class is loaded in updated_module 1.0.0 and removed in 1.1.0.
+ */
+class LoadedAndDeletedClass {
+
+ /**
+ * The value.
+ *
+ * @var string
+ */
+ public static $value = 'This class will be loaded and then deleted';
+
+}
diff --git a/package_manager/tests/fixtures/updated_module/1.1.0/src/AddedClass.php b/package_manager/tests/fixtures/updated_module/1.1.0/src/AddedClass.php
new file mode 100644
index 0000000000..ad8c441bd9
--- /dev/null
+++ b/package_manager/tests/fixtures/updated_module/1.1.0/src/AddedClass.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace Drupal\updated_module;
+
+/**
+ * This class only exists in updated_module 1.1.0.
+ */
+class AddedClass {
+
+ /**
+ * The value.
+ *
+ * @var string
+ */
+ public static $value = 'This class will be added';
+
+}
diff --git a/package_manager/tests/fixtures/updated_module/1.1.0/src/ChangedClass.php b/package_manager/tests/fixtures/updated_module/1.1.0/src/ChangedClass.php
new file mode 100644
index 0000000000..2bf7ca010c
--- /dev/null
+++ b/package_manager/tests/fixtures/updated_module/1.1.0/src/ChangedClass.php
@@ -0,0 +1,17 @@
+<?php
+
+namespace Drupal\updated_module;
+
+/**
+ * This class exists in updated_module 1.0.0, but with a different value.
+ */
+class ChangedClass {
+
+ /**
+ * The value.
+ *
+ * @var string
+ */
+ public static $value = 'After Update';
+
+}
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 b48a5f3a58..a86a6c24f7 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
@@ -105,9 +105,45 @@ class SystemChangeRecorder implements EventSubscriberInterface {
$this->recordServiceValue('updated_module.added_service', $results);
$phase = $event instanceof PreApplyEvent ? 'pre' : 'post';
+
+ // Check if changes to an existing class are picked up.
+ $this->recordClassValue('ChangedClass', $results);
+ // Check if changes to a class that has not been loaded before the update is
+ // applied, are picked up.
+ $this->recordClassValue('LoadedAndDeletedClass', $results);
+ // We can't check AddedClass and DeletedClass in the "pre" phase, because
+ // class_exists() uses auto-loading, so if we checked these classes in the
+ // "pre" phase, the results will persist into the "post" phase.
+ if ($phase === 'post') {
+ // Check if a class that was removed in the updated module is still
+ // loaded.
+ $this->recordClassValue('DeletedClass', $results);
+ // Check if a class that was added in the updated module is available.
+ $this->recordClassValue('AddedClass', $results);
+ }
+
$this->state->set("system_changes:$phase", $results);
}
+ /**
+ * Checks if a given class exists, and records its 'value' property.
+ *
+ * @param string $class_name
+ * The name of the class to check, not including the namespace.
+ * @param array $results
+ * The current set of results, passed by reference.
+ */
+ private function recordClassValue(string $class_name, array &$results): void {
+ $full_class_name = "Drupal\updated_module\\$class_name";
+ if (class_exists($full_class_name)) {
+ $results["$class_name exists"] = 'exists';
+ $results["value of $class_name"] = $full_class_name::$value;
+ }
+ else {
+ $results["$class_name exists"] = 'not exists';
+ }
+ }
+
/**
* Checks if a given service exists, and records its ->value property.
*
diff --git a/package_manager/tests/src/Build/PackageUpdateTest.php b/package_manager/tests/src/Build/PackageUpdateTest.php
index 7db4a10773..bb4485d099 100644
--- a/package_manager/tests/src/Build/PackageUpdateTest.php
+++ b/package_manager/tests/src/Build/PackageUpdateTest.php
@@ -82,6 +82,10 @@ class PackageUpdateTest extends TemplateProjectTestBase {
'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',
+ 'ChangedClass exists' => 'exists',
+ 'value of ChangedClass' => 'Before Update',
+ 'LoadedAndDeletedClass exists' => 'exists',
+ 'value of LoadedAndDeletedClass' => 'This class will be loaded and then deleted',
];
$this->assertSame($expected_pre_apply_results, $results['pre']);
@@ -111,6 +115,20 @@ class PackageUpdateTest extends TemplateProjectTestBase {
// the expected value.
'updated_module.added_service exists' => 'exists',
'value of updated_module.added_service' => 'New service, should not exist before update',
+ // Existing class should be available.
+ 'ChangedClass exists' => 'exists',
+ // Existing class will still use the pre-update version.
+ 'value of ChangedClass' => 'Before Update',
+ // Classes loaded in pre-apply or before and deleted from the updated module should
+ // be available.
+ 'LoadedAndDeletedClass exists' => 'exists',
+ 'value of LoadedAndDeletedClass' => 'This class will be loaded and then deleted',
+ // Classes not loaded before the apply operation and deleted from the updated module
+ // should not be available.
+ 'DeletedClass exists' => 'not exists',
+ // Classes added to the updated module should be available.
+ 'AddedClass exists' => 'exists',
+ 'value of AddedClass' => 'This class will be added',
];
$this->assertSame($expected_post_apply_results, $results['post']);
}
--
GitLab