diff --git a/src/Entity/SavedSearchType.php b/src/Entity/SavedSearchType.php index 6abfeb9..3ec8940 100644 --- a/src/Entity/SavedSearchType.php +++ b/src/Entity/SavedSearchType.php @@ -510,9 +510,9 @@ public function onDependencyRemoval(array $dependencies): bool { $call_on_removal = []; foreach ($dependencies as $dependency_type => $dependency_objects) { - // Annoyingly, modules and theme dependencies come not keyed by dependency - // name here, while entities do. Flip the array for modules and themes to - // make the code simpler. + // Annoyingly, modules and theme dependencies do not come keyed by + // dependency name here, while entities do. Flip the array for modules + // and themes to make the code simpler. if (in_array($dependency_type, ['module', 'theme'])) { $dependency_objects = array_flip($dependency_objects); } @@ -541,7 +541,7 @@ public function onDependencyRemoval(array $dependencies): bool { // However this plays out, it will lead to a change. $changed = TRUE; - foreach ($dependency_sources['optional'] as $plugin_type => $plugins) { + foreach ($dependency_sources['optional'] as $plugins) { // Type entities currently have no soft dependencies, so this has to // be a plugin dependency. We want to call onDependencyRemoval() on // that plugin. @@ -550,7 +550,7 @@ public function onDependencyRemoval(array $dependencies): bool { $plugins = array_intersect_key($plugins, $all_plugins); foreach ($plugins as $plugin_id => $plugin) { - $call_on_removal[$plugin_type][$plugin_id][$dependency_type][$name] = $dependency_objects[$name]; + $call_on_removal[$plugin_id][$dependency_type][$name] = $dependency_objects[$name]; } } } @@ -560,19 +560,13 @@ public function onDependencyRemoval(array $dependencies): bool { // Now for all plugins with optional dependencies (stored in // $call_on_removal, mapped to their removed dependencies) call their // onDependencyRemoval() methods. - $updated_config = []; - foreach ($call_on_removal as $plugin_type => $plugins) { - foreach ($plugins as $plugin_id => $plugin_dependencies) { - $removal_successful = $all_plugins[$plugin_id]->onDependencyRemoval($plugin_dependencies); - // If the plugin was successfully changed to remove the dependency, - // remember the new configuration to later set it. Otherwise, remove the - // plugin from the index so the dependency still gets removed. - if ($removal_successful) { - $updated_config[$plugin_type][$plugin_id] = $all_plugins[$plugin_id]->getConfiguration(); - } - else { - unset($all_plugins[$plugin_id]); - } + foreach ($call_on_removal as $plugin_id => $plugin_dependencies) { + $plugin = $all_plugins[$plugin_id]; + $removal_successful = $plugin->onDependencyRemoval($plugin_dependencies); + // If the plugin could not be changed to remove the dependency, remove it + // from the type so the dependency still gets removed. + if (!$removal_successful) { + unset($all_plugins[$plugin_id]); } } diff --git a/tests/search_api_saved_searches_test/config/schema/search_api_saved_searches_test.schema.yml b/tests/search_api_saved_searches_test/config/schema/search_api_saved_searches_test.schema.yml new file mode 100644 index 0000000..c3e320a --- /dev/null +++ b/tests/search_api_saved_searches_test/config/schema/search_api_saved_searches_test.schema.yml @@ -0,0 +1,7 @@ +plugin.plugin_configuration.search_api_saved_searches_notification.search_api_saved_searches_test: + type: mapping + label: Test notification plugin + mapping: + dependencies: + type: config_dependencies + label: Dependencies diff --git a/tests/search_api_saved_searches_test/search_api_saved_searches_test.info.yml b/tests/search_api_saved_searches_test/search_api_saved_searches_test.info.yml new file mode 100644 index 0000000..e5ac3ae --- /dev/null +++ b/tests/search_api_saved_searches_test/search_api_saved_searches_test.info.yml @@ -0,0 +1,7 @@ +name: 'Search API Saved Searches – Test' +type: module +description: 'Support module for Search API Saved Searches tests' +package: Testing +dependencies: + - search_api_saved_searches:search_api_saved_searches + - search_api:search_api_test diff --git a/tests/search_api_saved_searches_test/src/Plugin/search_api_saved_searches/notification/TestNotificationPlugin.php b/tests/search_api_saved_searches_test/src/Plugin/search_api_saved_searches/notification/TestNotificationPlugin.php new file mode 100644 index 0000000..ba0bda6 --- /dev/null +++ b/tests/search_api_saved_searches_test/src/Plugin/search_api_saved_searches/notification/TestNotificationPlugin.php @@ -0,0 +1,48 @@ +configuration['dependencies'] ?? []; + } + + /** + * {@inheritdoc} + */ + public function onDependencyRemoval(array $dependencies): bool { + $remove = $this->getReturnValue(__FUNCTION__, FALSE); + if ($remove) { + unset($this->configuration['dependencies']); + } + return $remove; + } + + /** + * {@inheritdoc} + */ + public function notify(SavedSearchInterface $search, ResultSetInterface $results): void { + } + +} diff --git a/tests/src/Kernel/DependencyRemovalTest.php b/tests/src/Kernel/DependencyRemovalTest.php new file mode 100644 index 0000000..814af85 --- /dev/null +++ b/tests/src/Kernel/DependencyRemovalTest.php @@ -0,0 +1,140 @@ +installEntitySchema('search_api_saved_search'); + $this->installConfig('search_api_saved_searches'); + $this->installSchema('user', 'users_data'); + } + + /** + * Tests that removal of optional dependencies is handled correctly. + * + * @param string $dependency_type + * The type of dependency to use, "module" or "config". + * @param bool $remove + * Whether the plugin should successfully adapt upon removal of the + * dependency. + * + * @dataProvider optionalDependencyRemovalTestDataProvider + */ + public function testOptionalDependencyRemoval(string $dependency_type, bool $remove): void { + if ($dependency_type === 'module') { + $dependency_key = 'module'; + $dependency_name = 'help'; + } + else { + assert($dependency_type === 'config'); + // Use a saved search type as the dependency, since we have that available + // anyways. The entity type should not matter at all, though. + $dependency = SavedSearchType::create([ + 'id' => 'dependency', + 'name' => 'Test dependency', + 'backend' => 'search_api_test', + ]); + $dependency->save(); + $dependency_key = $dependency->getConfigDependencyKey(); + $dependency_name = $dependency->getConfigDependencyName(); + } + + $type = SavedSearchType::load('default'); + $plugins = \Drupal::getContainer() + ->get('plugin.manager.search_api_saved_searches.notification') + ->createPlugins($type, ['search_api_saved_searches_test'], [ + 'search_api_saved_searches_test' => [ + 'dependencies' => [ + $dependency_key => [ + $dependency_name, + ], + ], + ], + ]); + $type->setNotificationPlugins($plugins); + $type->save(); + + // Check that the dependencies were calculated correctly. + $type_dependencies = $type->getDependencies(); + $this->assertContains($dependency_name, $type_dependencies[$dependency_key]); + + // Tell the backend plugin whether it should successfully remove the + // dependency. + $this->setReturnValue('notification', 'onDependencyRemoval', $remove); + + // Now remove the dependency. + if ($dependency_type === 'module') { + \Drupal::getContainer()->get('module_installer')->uninstall(['help']); + } + else { + if (empty($dependency)) { + throw new \RuntimeException('$dependency not set.'); + } + $dependency->delete(); + } + + // Reload the type. + $storage = \Drupal::entityTypeManager()->getStorage($type->getEntityTypeId()); + $storage->resetCache(); + $type = $storage->load($type->id()); + $this->assertInstanceOf(SavedSearchType::class, $type); + $plugins = $type->getNotificationPlugins(); + + if ($remove) { + $this->assertArrayHasKey('search_api_saved_searches_test', $plugins); + $this->assertArrayNotHasKey('dependencies', $plugins['search_api_saved_searches_test']->getConfiguration()); + } + else { + $this->assertArrayNotHasKey('search_api_saved_searches_test', $plugins); + } + } + + /** + * Provides test data sets for testOptionalDependencyRemoval(). + * + * @return array[] + * An associative array of argument arrays for + * testOptionalDependencyRemoval(), keyed by data set labels. + * + * @see testOptionalDependencyRemoval() + */ + public function optionalDependencyRemovalTestDataProvider(): array { + return [ + ['module', FALSE], + ['module', TRUE], + ['config', FALSE], + ['config', TRUE], + ]; + } + +}