From fa81b4b8b963442be6ef50d56c6b00895fb13c7c Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 14:13:03 -0700 Subject: [PATCH 01/15] Fix cross-project content leakage on multi-domain sites. Sites that serve many domains from one Drupal instance give each domain its own Quant project via config overrides. That routing was silently ignored: every domain's content published to the base project instead. Reproduced with two domains, two projects and a recording API endpoint. Before this change, ten of ten pushes across both domains arrived at the base project. After it, five arrive at each domain's own project. Four defects combined to cause it: - Workers forked by quant:run-queue inherited no --uri, so they booted on the default domain regardless of the parent's context. - getLockFileLocation() and the seed preparation read config through getEditable(), which bypasses overrides. Every domain shared one lock file, and each seeded with the base site's settings. - QuantClient captured its credentials in the constructor. The container is built before the domain is negotiated, so the project it captured was always the base one. - Queue items recorded no destination, leaving the worker to resolve the project from whatever context it happened to boot in. Domain negotiation is also forced explicitly in CLI. The Domain module populates its negotiation context from a kernel.request subscriber, and Drush never dispatches that event, so overrides are otherwise absent even when --uri names a valid domain. Queue items now carry the project they were queued for, and the worker refuses to send an item whose stamp does not match the project it is publishing to. Items queued before this change carry no stamp and are sent as before. This makes a leak impossible rather than unlikely: cron drains the queue in a single domain context, so without the check it would still misroute every other domain's items. Adds QuantClientProjectTest covering call-time project resolution, and repairs the config stub in QuantClientTest so the factory can answer more than one read. --- modules/quant_api/src/Client/QuantClient.php | 50 +++++++- .../src/Client/QuantClientInterface.php | 11 ++ .../tests/src/Unit/QuantClientProjectTest.php | 115 ++++++++++++++++++ .../tests/src/Unit/QuantClientTest.php | 16 ++- src/CliDomainContext.php | 66 ++++++++++ src/Commands/QuantDrushCommands.php | 58 ++++++++- src/Plugin/QueueItem/FileItem.php | 5 + src/Plugin/QueueItem/NodeItem.php | 5 + .../QueueItem/QuantQueueItemInterface.php | 13 ++ src/Plugin/QueueItem/RedirectItem.php | 5 + src/Plugin/QueueItem/RouteItem.php | 5 + src/Plugin/QueueItem/TargetProjectTrait.php | 49 ++++++++ src/Plugin/QueueItem/TaxonomyTermItem.php | 5 + src/Plugin/QueueWorker/QuantSeedWorker.php | 55 ++++++++- 14 files changed, 446 insertions(+), 12 deletions(-) create mode 100644 modules/quant_api/tests/src/Unit/QuantClientProjectTest.php create mode 100644 src/CliDomainContext.php create mode 100644 src/Plugin/QueueItem/TargetProjectTrait.php diff --git a/modules/quant_api/src/Client/QuantClient.php b/modules/quant_api/src/Client/QuantClient.php index 534ad18a..874c1e27 100644 --- a/modules/quant_api/src/Client/QuantClient.php +++ b/modules/quant_api/src/Client/QuantClient.php @@ -73,14 +73,39 @@ class QuantClient implements QuantClientInterface { */ protected $tlsDisabled = FALSE; + /** + * The configuration factory. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory; + /** * {@inheritdoc} */ public function __construct(Client $client, ConfigFactoryInterface $config_factory, LoggerChannelFactoryInterface $logger_factory) { - $this->config = $config_factory->get('quant_api.settings'); + $this->configFactory = $config_factory; $this->client = $client; $this->logger = $logger_factory->get('quant_api'); + $this->refreshCredentials(); + } + + /** + * Re-reads the API credentials from configuration. + * + * This service is a singleton for the lifetime of the process. On a + * multi-domain site the target project is not known until the active + * domain is negotiated, and that happens after the container is built. + * Values captured once in the constructor pin every subsequent request to + * whichever project the base configuration names, so they are re-read + * immediately before each API call. + */ + protected function refreshCredentials() : void { + // Re-fetch from the factory rather than reuse a cached config object: a + // domain switch resets the factory and produces a new object. + $this->config = $this->configFactory->get('quant_api.settings'); + $this->username = $this->config->get('api_account'); $this->token = $this->config->get('api_token'); $this->project = $this->config->get('api_project'); @@ -88,10 +113,22 @@ public function __construct(Client $client, ConfigFactoryInterface $config_facto $this->tlsDisabled = $this->config->get('api_tls_disabled'); } + /** + * Returns the Quant project this client currently targets. + * + * @return string|null + * The project machine name, or NULL when none is configured. + */ + public function getProject() : ?string { + $this->refreshCredentials(); + return $this->project ?: NULL; + } + /** * Get API overrides. */ public function getOverrides() { + $this->refreshCredentials(); // Note this has to be processed in this class instead of in the // SettingsForm because the overrides aren't available in the form. $overrides = []; @@ -117,6 +154,7 @@ public function getOverrides() { * {@inheritdoc} */ public function ping() { + $this->refreshCredentials(); try { // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. @@ -157,6 +195,7 @@ public function ping() { * {@inheritdoc} */ public function project() { + $this->refreshCredentials(); try { // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. @@ -196,6 +235,7 @@ public function project() { * {@inheritdoc} */ public function search() { + $this->refreshCredentials(); try { // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. @@ -235,6 +275,7 @@ public function search() { * {@inheritdoc} */ public function send(array $data) : array { + $this->refreshCredentials(); // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint, [ RequestOptions::JSON => $data, @@ -253,6 +294,7 @@ public function send(array $data) : array { * {@inheritdoc} */ public function sendRedirect(array $data) : array { + $this->refreshCredentials(); // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint . '/redirect', [ RequestOptions::JSON => $data, @@ -271,6 +313,7 @@ public function sendRedirect(array $data) : array { * {@inheritdoc} */ public function sendFile(string $file, string $url, ?int $rid = NULL) : array { + $this->refreshCredentials(); // Ensure the file is accessible before attempting to send to the API. if (!file_exists($file) || !is_readable($file) || !is_file($file)) { @@ -319,6 +362,7 @@ public function sendFile(string $file, string $url, ?int $rid = NULL) : array { * The API response. */ public function unpublish(string $url) : array { + $this->refreshCredentials(); // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->patch($this->endpoint . '/unpublish', [ 'headers' => [ @@ -343,6 +387,7 @@ public function unpublish(string $url) : array { * The API response. */ public function getUrlMeta(array $urls) : array { + $this->refreshCredentials(); // Format array if it's not already. if (!array_key_exists('Quant-Url', $urls)) { $urls = [ @@ -367,6 +412,7 @@ public function getUrlMeta(array $urls) : array { * {@inheritdoc} */ public function sendSearchRecords(array $records) : array { + $this->refreshCredentials(); // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint . '/search', [ RequestOptions::JSON => $records, @@ -385,6 +431,7 @@ public function sendSearchRecords(array $records) : array { * {@inheritdoc} */ public function clearSearchIndex() : array { + $this->refreshCredentials(); // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->delete($this->endpoint . '/search/all', [ 'headers' => [ @@ -402,6 +449,7 @@ public function clearSearchIndex() : array { * {@inheritdoc} */ public function addFacets(array $facets) : array { + $this->refreshCredentials(); // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint . '/search/facet', [ RequestOptions::JSON => $facets, diff --git a/modules/quant_api/src/Client/QuantClientInterface.php b/modules/quant_api/src/Client/QuantClientInterface.php index 7266618b..ae518487 100644 --- a/modules/quant_api/src/Client/QuantClientInterface.php +++ b/modules/quant_api/src/Client/QuantClientInterface.php @@ -15,6 +15,17 @@ interface QuantClientInterface { */ public function ping(); + /** + * Returns the Quant project this client currently targets. + * + * Resolved at call time, because a multi-domain site changes the target + * project once the active domain is negotiated. + * + * @return string|null + * The project machine name, or NULL when none is configured. + */ + public function getProject() : ?string; + /** * Retrieves project data. * diff --git a/modules/quant_api/tests/src/Unit/QuantClientProjectTest.php b/modules/quant_api/tests/src/Unit/QuantClientProjectTest.php new file mode 100644 index 00000000..7a57be5c --- /dev/null +++ b/modules/quant_api/tests/src/Unit/QuantClientProjectTest.php @@ -0,0 +1,115 @@ +createMock(ImmutableConfig::class); + $config->method('get')->willReturnMap([ + ['api_account', 'test-org'], + ['api_token', 'token-' . $project], + ['api_project', $project], + ['api_endpoint', 'http://test'], + ['api_tls_disabled', FALSE], + ]); + return $config; + } + + /** + * Builds a client whose config factory yields the given configs in order. + * + * @param \Drupal\Core\Config\ImmutableConfig ...$configs + * The config objects to return on successive reads. + * + * @return \Drupal\quant_api\Client\QuantClient + * The client under test. + */ + protected function client(ImmutableConfig ...$configs) : QuantClient { + $factory = $this->createMock(ConfigFactoryInterface::class); + $factory->method('get')->willReturnOnConsecutiveCalls(...$configs); + + return new QuantClient( + $this->createMock(Client::class), + $factory, + $this->createMock(LoggerChannelFactoryInterface::class) + ); + } + + /** + * The project is read from configuration on construction. + * + * @covers ::getProject + */ + public function testProjectResolvesFromConfig() { + $config = $this->config('project-a'); + // Two reads: one in the constructor, one in getProject(). + $client = $this->client($config, $config); + + $this->assertEquals('project-a', $client->getProject()); + } + + /** + * The project follows a configuration change after construction. + * + * This is the multi-domain case: the container is built before the domain + * is negotiated, so the project the constructor saw is not the project the + * request must publish to. + * + * @covers ::getProject + * @covers ::refreshCredentials + */ + public function testProjectFollowsDomainSwitch() { + $base = $this->config('base-project'); + $overridden = $this->config('project-client-a'); + + // Read one is the constructor, before the domain is negotiated. Read two + // is the first getProject(). Read three is after the switch. + $client = $this->client($base, $base, $overridden); + + $this->assertEquals('base-project', $client->getProject()); + $this->assertEquals('project-client-a', $client->getProject()); + } + + /** + * An unconfigured project reports NULL rather than an empty string. + * + * @covers ::getProject + */ + public function testUnconfiguredProjectIsNull() { + $empty = $this->createMock(ImmutableConfig::class); + $empty->method('get')->willReturn(''); + + $this->assertNull($this->client($empty, $empty)->getProject()); + } + +} diff --git a/modules/quant_api/tests/src/Unit/QuantClientTest.php b/modules/quant_api/tests/src/Unit/QuantClientTest.php index 184d50fb..85d853ae 100644 --- a/modules/quant_api/tests/src/Unit/QuantClientTest.php +++ b/modules/quant_api/tests/src/Unit/QuantClientTest.php @@ -25,21 +25,27 @@ class QuantClientTest extends UnitTestCase { * The config interface. */ protected function getConfigStub($default = []) { - $value = [ + $values = [ 'api_account' => 'account', 'api_token' => 'token', 'api_endpoint' => 'http://test', ] + $default; - $stub = $this->prophesize(ConfigFactoryInterface::class); $config = $this->prophesize(ImmutableConfig::class); - foreach ($config as $key => $value) { + // Iterate the values, not the prophecy. Keys absent here resolve to NULL, + // which is what an unconfigured setting returns. + foreach ($values as $key => $value) { $config->get($key)->willReturn($value); } - $stub->get('quant_api.settings')->willReturn($config); - return $stub; + $stub = $this->prophesize(ConfigFactoryInterface::class); + + // Reveal both doubles. The client re-reads its credentials before every + // request, so the factory must answer get() more than once. + $stub->get('quant_api.settings')->willReturn($config->reveal()); + + return $stub->reveal(); } /** diff --git a/src/CliDomainContext.php b/src/CliDomainContext.php new file mode 100644 index 00000000..00d34b1e --- /dev/null +++ b/src/CliDomainContext.php @@ -0,0 +1,66 @@ +moduleExists('domain')) { + return NULL; + } + + if (!\Drupal::hasService('domain.negotiator')) { + return NULL; + } + + $domain = \Drupal::service('domain.negotiator')->getActiveDomain(); + + if (empty($domain)) { + return NULL; + } + + // Any config object built before negotiation was cached without the + // override applied. Drop the static cache so the overridden values + // resolve on the next read. + \Drupal::configFactory()->reset(); + + return $domain->id(); + } + + /** + * Returns the Quant project the current context resolves to. + * + * @return string|null + * The project machine name, or NULL when none is configured. + */ + public static function getActiveProject() : ?string { + return \Drupal::config('quant_api.settings')->get('api_project') ?: NULL; + } + +} diff --git a/src/Commands/QuantDrushCommands.php b/src/Commands/QuantDrushCommands.php index b37a0a40..79522ef4 100644 --- a/src/Commands/QuantDrushCommands.php +++ b/src/Commands/QuantDrushCommands.php @@ -3,7 +3,9 @@ namespace Drupal\quant\Commands; use Drush\Commands\DrushCommands; +use Drush\Drush; use Drupal\Core\Form\FormState; +use Drupal\quant\CliDomainContext; use Drupal\quant\Seed; use Drupal\quant\Event\CollectEntitiesEvent; use Drupal\quant\Event\CollectFilesEvent; @@ -29,12 +31,42 @@ class QuantDrushCommands extends DrushCommands { /** * Returns lock file location (project specific). + * + * Reads through the overridable config factory. getEditable() returns the + * base values only, which on a multi-domain site gives every domain the + * same lock file: the first seed run blocks all the others. */ private function getLockFileLocation() { - $config = \Drupal::configFactory()->getEditable('quant_api.settings'); + $config = \Drupal::config('quant_api.settings'); return sys_get_temp_dir() . '/' . $config->get('api_project') . '_quant_seed_worker.lock'; } + /** + * Returns the options forked workers must inherit from this process. + * + * Forked workers start as bare drush processes that share nothing with + * their parent. Without --uri the worker boots on the default domain, so + * per-domain config overrides resolve to the base project and every + * domain's content is published there. + * + * @return string + * Options to append to the forked command, escaped for the shell. + */ + private function getForkOptions() : string { + $options = []; + $bootstrapManager = Drush::bootstrapManager(); + + if ($uri = $bootstrapManager->getUri()) { + $options[] = '--uri=' . escapeshellarg($uri); + } + + if ($root = $bootstrapManager->getRoot()) { + $options[] = '--root=' . escapeshellarg($root); + } + + return $options ? ' ' . implode(' ', $options) : ''; + } + /** * Returns path to drush binary for process forking. * @@ -82,12 +114,20 @@ private function getDrushPath() { * @usage quant:run-queue --threads=5 */ public function message($options = ['threads' => 5]) { + // Resolve per-domain config before anything reads the project name. + $domainId = CliDomainContext::initialize(); + $this->output()->writeln("Forking seed worker."); $drushPath = $this->getDrushPath(); $lockFilePath = $this->getLockFileLocation(); - $cmd = $drushPath . ' queue:run quant_seed_worker'; + $cmd = $drushPath . ' queue:run quant_seed_worker' . $this->getForkOptions(); $this->output()->writeln("Using drush binary at $drushPath. Override with \$DRUSH_PATH if required."); + $project = CliDomainContext::getActiveProject(); + if ($domainId) { + $this->output()->writeln("Active domain: {$domainId}. Publishing to project: {$project}."); + } + // Bail if another run is in progress. if (file_exists($lockFilePath)) { $this->output()->writeln("Seeding bailed. Another seed run is in progress (lockfile is present: {$lockFilePath})"); @@ -127,6 +167,9 @@ public function message($options = ['threads' => 5]) { * @usage quant:unlock-queue */ public function unlock($options = []) { + // The lock file is named after the resolved project, so the domain must + // be negotiated before the path can be built. + CliDomainContext::initialize(); $lockFilePath = $this->getLockFileLocation(); unlink($lockFilePath); @@ -157,9 +200,18 @@ public function clear($options = []) { * @usage quant:seed-queue */ public function prepare($options = ['reset' => 'true']) { + // Resolve per-domain config before the seed settings are read, so that + // each domain seeds with its own bundles, routes and file paths. + $domainId = CliDomainContext::initialize(); + $this->output()->writeln("Preparing seed..."); - $config = \Drupal::configFactory()->getEditable('quant.settings'); + if ($domainId) { + $project = CliDomainContext::getActiveProject(); + $this->output()->writeln("Active domain: {$domainId}. Target project: {$project}."); + } + + $config = \Drupal::config('quant.settings'); $queue_factory = QuantQueueFactory::getInstance(); $queue = $queue_factory->get('quant_seed_worker'); diff --git a/src/Plugin/QueueItem/FileItem.php b/src/Plugin/QueueItem/FileItem.php index a92c0597..2e90695e 100644 --- a/src/Plugin/QueueItem/FileItem.php +++ b/src/Plugin/QueueItem/FileItem.php @@ -11,6 +11,8 @@ */ class FileItem implements QuantQueueItemInterface { + use TargetProjectTrait; + /** * The file path. * @@ -47,6 +49,9 @@ public function __construct(array $data = []) { $this->url = $data['url'] ?? NULL; $this->fullPath = $data['full_path'] ?? NULL; $this->originalPath = $data['original_path'] ?? NULL; + + // Record the project this item is destined for. + $this->stampTargetProject(); } /** diff --git a/src/Plugin/QueueItem/NodeItem.php b/src/Plugin/QueueItem/NodeItem.php index 2e9789bb..3ed66358 100644 --- a/src/Plugin/QueueItem/NodeItem.php +++ b/src/Plugin/QueueItem/NodeItem.php @@ -11,6 +11,8 @@ */ class NodeItem implements QuantQueueItemInterface { + use TargetProjectTrait; + /** * The entity id. * @@ -47,6 +49,9 @@ public function __construct(array $data = []) { $this->id = $data['id']; $this->vid = $data['vid'] ?? FALSE; $this->filter = isset($data['lang_filter']) && is_array($data['lang_filter']) ? array_filter($data['lang_filter']) : []; + + // Record the project this item is destined for. + $this->stampTargetProject(); } /** diff --git a/src/Plugin/QueueItem/QuantQueueItemInterface.php b/src/Plugin/QueueItem/QuantQueueItemInterface.php index fe7e94a1..dbc47b21 100644 --- a/src/Plugin/QueueItem/QuantQueueItemInterface.php +++ b/src/Plugin/QueueItem/QuantQueueItemInterface.php @@ -14,6 +14,19 @@ interface QuantQueueItemInterface { */ public function send(); + /** + * Returns the Quant project this item was queued for. + * + * Recorded at enqueue time so the worker can confirm it is publishing to + * the intended destination. Multi-domain sites resolve a different project + * per domain, and the worker does not always boot in the domain that + * created the item. + * + * @return string|null + * The project machine name, or NULL when the item carries no stamp. + */ + public function getTargetProject() : ?string; + /** * Describe the current item. * diff --git a/src/Plugin/QueueItem/RedirectItem.php b/src/Plugin/QueueItem/RedirectItem.php index 7bdbb670..b76197b5 100644 --- a/src/Plugin/QueueItem/RedirectItem.php +++ b/src/Plugin/QueueItem/RedirectItem.php @@ -11,6 +11,8 @@ */ class RedirectItem implements QuantQueueItemInterface { + use TargetProjectTrait; + /** * The source path. * @@ -39,6 +41,9 @@ public function __construct(array $data = []) { $this->source = $data['source']; $this->destination = $data['destination']; $this->statusCode = $data['status_code']; + + // Record the project this item is destined for. + $this->stampTargetProject(); } /** diff --git a/src/Plugin/QueueItem/RouteItem.php b/src/Plugin/QueueItem/RouteItem.php index 25908293..73950d03 100644 --- a/src/Plugin/QueueItem/RouteItem.php +++ b/src/Plugin/QueueItem/RouteItem.php @@ -12,6 +12,8 @@ */ class RouteItem implements QuantQueueItemInterface { + use TargetProjectTrait; + /** * A Drupal entity. * @@ -52,6 +54,9 @@ public function __construct(array $data = []) { $this->route = $route; $this->uri = $data['uri'] ?? strtok($route, '?'); $this->filePath = $data['file_path'] ?? DRUPAL_ROOT . strtok($route, '?'); + + // Record the project this item is destined for. + $this->stampTargetProject(); } /** diff --git a/src/Plugin/QueueItem/TargetProjectTrait.php b/src/Plugin/QueueItem/TargetProjectTrait.php new file mode 100644 index 00000000..842bf2cb --- /dev/null +++ b/src/Plugin/QueueItem/TargetProjectTrait.php @@ -0,0 +1,49 @@ +targetProject = \Drupal::config('quant_api.settings')->get('api_project') ?: NULL; + } + + /** + * Returns the project this item was queued for. + * + * @return string|null + * The project machine name, or NULL for items queued before this stamp + * existed. A NULL stamp is not checked, so old items keep working. + */ + public function getTargetProject() : ?string { + return $this->targetProject; + } + +} diff --git a/src/Plugin/QueueItem/TaxonomyTermItem.php b/src/Plugin/QueueItem/TaxonomyTermItem.php index 79768cae..fe526bfc 100644 --- a/src/Plugin/QueueItem/TaxonomyTermItem.php +++ b/src/Plugin/QueueItem/TaxonomyTermItem.php @@ -11,6 +11,8 @@ */ class TaxonomyTermItem implements QuantQueueItemInterface { + use TargetProjectTrait; + /** * The taxonomy term id. * @@ -23,6 +25,9 @@ class TaxonomyTermItem implements QuantQueueItemInterface { */ public function __construct(array $data = []) { $this->tid = $data['tid']; + + // Record the project this item is destined for. + $this->stampTargetProject(); } /** diff --git a/src/Plugin/QueueWorker/QuantSeedWorker.php b/src/Plugin/QueueWorker/QuantSeedWorker.php index b90a3880..c5f18f7b 100644 --- a/src/Plugin/QueueWorker/QuantSeedWorker.php +++ b/src/Plugin/QueueWorker/QuantSeedWorker.php @@ -3,6 +3,7 @@ namespace Drupal\quant\Plugin\QueueWorker; use Drupal\Core\Queue\QueueWorkerBase; +use Drupal\quant\CliDomainContext; use Drupal\quant\Plugin\QueueItem\QuantQueueItemInterface; /** @@ -20,10 +21,58 @@ class QuantSeedWorker extends QueueWorkerBase { * {@inheritdoc} */ public function processItem($item) { - if (is_a($item, QuantQueueItemInterface::class)) { - \Drupal::logger('quant_seed')->notice($item->log()); - return $item->send(); + if (!is_a($item, QuantQueueItemInterface::class)) { + return NULL; } + + // Resolve the active domain before reading the target project. Workers + // forked by quant:run-queue inherit --uri but never dispatch a + // kernel.request, so the domain context is otherwise empty. + CliDomainContext::initialize(); + + if (!$this->targetsActiveProject($item)) { + return NULL; + } + + \Drupal::logger('quant_seed')->notice($item->log()); + return $item->send(); + } + + /** + * Confirms the item belongs to the project this process publishes to. + * + * Publishing an item to the wrong project puts one site's content on + * another's domain. On a shared Drupal instance serving many clients that + * is a content leak, so a mismatch stops the send rather than risking it. + * + * @param \Drupal\quant\Plugin\QueueItem\QuantQueueItemInterface $item + * The queue item. + * + * @return bool + * TRUE when the item may be sent. + */ + protected function targetsActiveProject(QuantQueueItemInterface $item) : bool { + $target = $item->getTargetProject(); + + // Items queued before the stamp existed carry no target. Send them, to + // keep existing single-domain queues working across an update. + if (empty($target)) { + return TRUE; + } + + $active = \Drupal::service('quant_api.client')->getProject(); + + if ($target === $active) { + return TRUE; + } + + \Drupal::logger('quant_seed')->error('Skipped @item: queued for project @target but this worker publishes to @active. Run the queue with --uri set to the domain that owns @target.', [ + '@item' => $item->log(), + '@target' => $target, + '@active' => $active ?: 'none', + ]); + + return FALSE; } } From 7e7b13fed62f2b89a3a1dc8560006b0f956c29ef Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 14:26:06 -0700 Subject: [PATCH 02/15] Route submodule publishing per domain, and repair the client tests. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit quant_cron sends synchronously rather than queueing, so it published to whichever project the base configuration named. It now negotiates the domain first, making "drush --uri=... cron" target the right project. Verified against two domains: three nodes to each, none crossing over. quant_tome has the same shape. Its deploy command resolves the domain before checkConfig() reads the API settings, and the batch callback resolves it again because batch operations may run in a forked process. quant_search pushes index records straight to the API from its batch, so that resolves the domain too. Records ride the content push and were already correct once the queue was fixed; the end-to-end run confirms each domain's records reach only its own index. quant_sitemap needs no change: it contributes routes to the seed and inherits its context. quant_purger queues stamped items from HTTP, where the domain is already negotiated, so it is correct as well — but its traffic registry stores paths with no host, so a tag invalidation only purges one domain's copy. That is an under-purge rather than a leak and is left for a follow-up that needs a schema change. CliDomainContext now caches its result, so batch and loop callers can call it freely, and it only drops the config cache under CLI. A web request negotiates its domain from kernel.request before any Quant code runs, so resetting there would discard the config cache for nothing. QuantClientTest was broken long before this branch: 6 errors and 3 failures of 20, unnoticed because the CI job named phpunit only installs the module and never runs it. It never called reveal(), so the doubles were prophecies; getStatusCode was read as a property rather than called; RequestException was built with one argument; and the expected requests omitted Quant-Project, used 'exception' for 'exceptions' and expected the endpoint without its /v1 suffix. Rather than patch those expectations, requests now run through a Guzzle MockHandler with the history middleware, so the assertions describe the method, URI, headers and body that reach the wire. That makes the Quant-Project header — the one thing that decides which site content is published to — explicitly asserted on every call. Coverage extends to unpublish, getUrlMeta, search records, index clearing, facets, TLS verification and override reporting. The php built-in stubs are gone; the upload tests use real temporary files. quant_api unit tests: 22 of 22 pass, from 20 with 9 broken. --- .../tests/src/Unit/QuantClientTest.php | 657 ++++++++++-------- modules/quant_cron/quant_cron.module | 14 + modules/quant_search/quant_search.module | 7 + .../src/Commands/QuantTomeCommands.php | 10 + modules/quant_tome/src/QuantTomeBatch.php | 6 + src/CliDomainContext.php | 51 +- tests/src/Unit/FileItemTest.php | 33 + 7 files changed, 502 insertions(+), 276 deletions(-) diff --git a/modules/quant_api/tests/src/Unit/QuantClientTest.php b/modules/quant_api/tests/src/Unit/QuantClientTest.php index 85d853ae..c67a2a36 100644 --- a/modules/quant_api/tests/src/Unit/QuantClientTest.php +++ b/modules/quant_api/tests/src/Unit/QuantClientTest.php @@ -2,378 +2,493 @@ namespace Drupal\Tests\quant_api\Unit; -use Drupal\Core\Logger\LoggerChannelFactoryInterface; use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Config\ImmutableConfig; +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\Logger\LoggerChannelFactoryInterface; +use Drupal\Core\Messenger\MessengerInterface; use Drupal\quant_api\Client\QuantClient; use Drupal\quant_api\Exception\InvalidPayload; use Drupal\Tests\UnitTestCase; use GuzzleHttp\Client; use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Handler\MockHandler; +use GuzzleHttp\HandlerStack; +use GuzzleHttp\Middleware; +use GuzzleHttp\Psr7\Request; use GuzzleHttp\Psr7\Response; -use GuzzleHttp\RequestOptions; /** - * Ensure that the client responds correctly. + * Ensures the client forms correct requests and reads responses. + * + * Requests are driven through a Guzzle MockHandler and captured with the + * history middleware, so the assertions describe what reaches the wire — + * method, URI, headers and body — rather than the shape of an options array. + * Every request must carry Quant-Project, because that header alone decides + * which site the content is published to. + * + * @coversDefaultClass \Drupal\quant_api\Client\QuantClient + * + * @group quant_api */ class QuantClientTest extends UnitTestCase { /** - * Get a stubbed config factory. + * Requests recorded by the history middleware. * - * @return \Drupal\Core\Config\ConfigFactoryInterface - * The config interface. + * @var array */ - protected function getConfigStub($default = []) { - $values = [ - 'api_account' => 'account', - 'api_token' => 'token', - 'api_endpoint' => 'http://test', - ] + $default; + protected $history = []; - $config = $this->prophesize(ImmutableConfig::class); + /** + * Temporary files created by a test, removed on teardown. + * + * @var string[] + */ + protected $tempFiles = []; - // Iterate the values, not the prophecy. Keys absent here resolve to NULL, - // which is what an unconfigured setting returns. - foreach ($values as $key => $value) { - $config->get($key)->willReturn($value); - } + /** + * The credentials every test configures. + */ + const ACCOUNT = 'test-account'; + const PROJECT = 'test-project'; + const TOKEN = 'test-token'; - $stub = $this->prophesize(ConfigFactoryInterface::class); + /** + * The endpoint the client derives from the configured base. + */ + const ENDPOINT = 'http://test/v1'; - // Reveal both doubles. The client re-reads its credentials before every - // request, so the factory must answer get() more than once. - $stub->get('quant_api.settings')->willReturn($config->reveal()); + /** + * {@inheritdoc} + */ + protected function setUp() : void { + parent::setUp(); + + // The client reports transport and subscription errors through + // \Drupal::messenger() rather than an injected service, so a container + // has to exist before those paths run. + $container = new ContainerBuilder(); + $container->set('messenger', $this->createMock(MessengerInterface::class)); + $container->set('string_translation', $this->getStringTranslationStub()); + \Drupal::setContainer($container); + } - return $stub->reveal(); + /** + * {@inheritdoc} + */ + protected function tearDown() : void { + foreach ($this->tempFiles as $file) { + if (file_exists($file)) { + unlink($file); + } + } + parent::tearDown(); } /** - * Get a successful project response. + * Builds a client whose transport returns the given responses in order. + * + * @param array $responses + * Responses or exceptions for the MockHandler to yield. + * @param array $overrides + * Configuration values to override. * - * @return GuzzleHttp\Psr7\Response - * A response object. + * @return \Drupal\quant_api\Client\QuantClient + * The client under test. */ - protected function getProjectResponse() { - // @todo should these be fixtures. - $body = [ - 'project' => 'test', - 'error' => FALSE, - 'errorMsg' => '', + protected function client(array $responses, array $overrides = []) : QuantClient { + $stack = HandlerStack::create(new MockHandler($responses)); + $stack->push(Middleware::history($this->history)); + + return new QuantClient( + new Client(['handler' => $stack]), + $this->configFactory($overrides), + $this->createMock(LoggerChannelFactoryInterface::class) + ); + } + + /** + * Builds a config factory returning the test credentials. + * + * @param array $overrides + * Configuration values to override. + * + * @return \Drupal\Core\Config\ConfigFactoryInterface + * The config factory double. + */ + protected function configFactory(array $overrides = []) : ConfigFactoryInterface { + $values = $overrides + [ + 'api_account' => self::ACCOUNT, + 'api_project' => self::PROJECT, + 'api_token' => self::TOKEN, + 'api_endpoint' => 'http://test', + 'api_tls_disabled' => FALSE, ]; - $res = $this->prophesize(Response::class); - $res->getStatusCode->willReturn(200); - $res->getBody()->willReturn(json_encode($body)); + $config = $this->createMock(ImmutableConfig::class); + $config->method('get')->willReturnCallback( + fn($key) => $values[$key] ?? NULL + ); + + $factory = $this->createMock(ConfigFactoryInterface::class); + $factory->method('get')->willReturn($config); - return $res; + return $factory; } /** - * A valid redirect response. + * Returns the request the client sent. + * + * @param int $index + * Which recorded request to return. * - * @return GuzzleHttp\Psr7\Response - * A response object. + * @return \Psr\Http\Message\RequestInterface + * The captured request. */ - protected function getRedirectResponse() { - $body = [ - 'redirect_url' => '/b', - 'quant_revision' => 1, - 'url' => '/a', - 'redirect_http_code' => 302, - 'errorMsg' => '', - 'error' => FALSE, - ]; + protected function request(int $index = 0) { + $this->assertArrayHasKey($index, $this->history, 'The client sent a request.'); + return $this->history[$index]['request']; + } - $res = $this->prophesize(Response::class); - $res->getStatusCode->willReturn(200); - $res->getBody()->willReturn(json_encode($body)); + /** + * Asserts the request carries the credentials that route it to a project. + * + * @param \Psr\Http\Message\RequestInterface $request + * The captured request. + */ + protected function assertAuthHeaders($request) : void { + $this->assertEquals(self::ACCOUNT, $request->getHeaderLine('Quant-Customer')); + $this->assertEquals(self::PROJECT, $request->getHeaderLine('Quant-Project')); + $this->assertEquals(self::TOKEN, $request->getHeaderLine('Quant-Token')); + } - return $res; + /** + * Creates a real temporary file for the upload tests. + * + * @param string $extension + * The file extension to use. + * + * @return string + * The path to the file. + */ + protected function tempFile(string $extension = 'jpg') : string { + $path = tempnam(sys_get_temp_dir(), 'quant') . '.' . $extension; + file_put_contents($path, 'test contents'); + $this->tempFiles[] = $path; + return $path; } /** - * Get an invalid response. + * A successful ping returns TRUE. * - * @return GuzzleHttp\Psr7\Response - * A response object. + * @covers ::ping */ - protected function getInvalidResponse() { - $body = [ - 'error' => TRUE, - 'errorMsg' => 'Error', - ]; + public function testPingValid() { + $client = $this->client([new Response(200, [], json_encode(['project' => 'test']))]); - $res = $this->prophesize(Response::class); - $res->getStatusCode->willReturn(400); - $res->getBody->willReturn(json_encode($body)); + $this->assertTrue($client->ping()); - return $res; + $request = $this->request(); + $this->assertEquals('GET', $request->getMethod()); + $this->assertEquals(self::ENDPOINT . '/ping', (string) $request->getUri()); + $this->assertAuthHeaders($request); } /** - * Ensure that the client handles a failed ping to QuantAPI. + * A non-200 ping returns FALSE. + * + * @covers ::ping */ - public function testPingClientError() { - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); - - // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. - $http->get('http://test/ping', [ - 'http_errors' => FALSE, - 'headers' => [ - 'Quant-Customer' => 'account', - 'Quant-Token' => 'token', - ], - 'exception' => FALSE, - ])->willThrow(new RequestException('ERROR')); - - $client = new QuantClient($http, $config, $logger); + public function testPingInvalid() { + $client = $this->client([new Response(500, [], json_encode(['error' => TRUE]))]); + $this->assertFalse($client->ping()); } /** - * Ensure a valid ping can be made. + * A transport failure during ping is caught and reported as FALSE. + * + * @covers ::ping */ - public function testPingValid() { - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); - $res = $this->getProjectResponse(); - - // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. - $http->get('http://test/ping', [ - 'http_errors' => FALSE, - 'headers' => [ - 'Quant-Customer' => 'account', - 'Quant-Token' => 'token', - ], - 'exception' => FALSE, - ])->willReturn($res); - - $client = new QuantClient($http, $config, $logger); - $project = $client->ping(); - - $this->assertEquals($project, 'test'); + public function testPingClientError() { + $error = new RequestException('ERROR', new Request('GET', self::ENDPOINT . '/ping')); + + $this->assertFalse($this->client([$error])->ping()); } /** - * Ensure that ping handles an invalid response from the server. + * The project endpoint decodes the response body. + * + * @covers ::project */ - public function testPingInvalid() { - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); - $res = $this->getInvalidResponse(); - - // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. - $http->get('http://test/ping', [ - 'http_errors' => FALSE, - 'headers' => [ - 'Quant-Customer' => 'account', - 'Quant-Token' => 'token', - ], - 'exception' => FALSE, - ])->willReturn($res); - - $client = new QuantClient($http, $config, $logger); + public function testProjectValid() { + $body = ['project' => 'test', 'config' => ['search_enabled' => TRUE]]; + $client = $this->client([new Response(200, [], json_encode($body))]); - $this->assertFalse($client->ping()); + $project = $client->project(); + + $this->assertEquals('test', $project->project); + $this->assertTrue($project->config->search_enabled); + $this->assertAuthHeaders($this->request()); } /** - * Ensure that send can send a valid payload. + * A valid send returns the decoded payload. + * + * @covers ::send */ public function testSendValid() { - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); - $res = $this->getProjectResponse(); - - // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. - $http->post('http://test', [ - RequestOptions::JSON => [], - 'headers' => [ - 'Quant-Customer' => 'account', - 'Quant-Token' => 'token', - ], - ])->willReturn($res); - - $client = new QuantClient($http, $config, $logger); - $this->assertEquals(['project' => 'test'], $client->send([])); + $client = $this->client([new Response(200, [], json_encode(['project' => 'test']))]); + + $this->assertEquals(['project' => 'test'], $client->send(['url' => '/a'])); + + $request = $this->request(); + $this->assertEquals('POST', $request->getMethod()); + $this->assertEquals(self::ENDPOINT, (string) $request->getUri()); + $this->assertEquals(['url' => '/a'], json_decode((string) $request->getBody(), TRUE)); + $this->assertAuthHeaders($request); } /** - * Ensure that send handles server errors. + * A transport failure during send is not swallowed. + * + * @covers ::send */ public function testSendError() { + $error = new RequestException('ERROR', new Request('POST', self::ENDPOINT)); + $this->expectException(RequestException::class); - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); - $res = $this->getInvalidResponse(); - - // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. - $http->post('http://test', [ - RequestOptions::JSON => [], - 'headers' => [ - 'Quant-Customer' => 'account', - 'Quant-Token' => 'token', - ], - ])->willReturn($res); - - $client = new QuantClient($http, $config, $logger); - $client->send([]); + $this->client([$error])->send([]); } /** - * Ensure a valid redirect response is sent. + * A valid redirect send returns the decoded payload. + * + * @covers ::sendRedirect */ public function testSendRedirectValid() { - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); - $res = $this->getRedirectResponse(); - - // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. - $http->post('http://test/redirect', [ - RequestOptions::JSON => [], - 'headers' => [ - 'Quant-Customer' => 'account', - 'Quant-Token' => 'token', - ], - ])->willReturn($res); - - $client = new QuantClient($http, $config, $logger); - $redirect = $client->sendRedirect([]); - - $this->assertEquals([ - 'redirect_url' => '/b', - 'quant_revision' => 1, + $body = [ 'url' => '/a', + 'redirect_url' => '/b', 'redirect_http_code' => 302, - 'errorMsg' => '', 'error' => FALSE, - ], $redirect); + ]; + $client = $this->client([new Response(200, [], json_encode($body))]); + + $this->assertEquals($body, $client->sendRedirect(['url' => '/a'])); + $this->assertEquals(self::ENDPOINT . '/redirect', (string) $this->request()->getUri()); } /** - * Ensure a valid redirect response is sent. + * A transport failure during a redirect send is not swallowed. + * + * @covers ::sendRedirect */ public function testSendRedirectError() { + $error = new RequestException('ERROR', new Request('POST', self::ENDPOINT . '/redirect')); + $this->expectException(RequestException::class); - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); - $res = $this->getInvalidResponse(); - - // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. - $http->post('http://test/redirect', [ - RequestOptions::JSON => [], - 'headers' => [ - 'Quant-Customer' => 'account', - 'Quant-Token' => 'token', - ], - ])->willReturn($res); - - $client = new QuantClient($http, $config, $logger); - $client->sendRedirect([]); + $this->client([$error])->sendRedirect([]); } /** - * Ensure files are validated before sending. + * A missing file is rejected before any request is made. + * + * @covers ::sendFile */ public function testSendFileFileNoExist() { - $this->expectException(InvalidPayload::class); - // phpcs:ignore - global $exists_return; - // phpcs:ignore - global $readable_return; + $client = $this->client([]); - $exists_return = FALSE; - $readable_return = FALSE; + $this->expectException(InvalidPayload::class); + $client->sendFile('/tmp/quant-does-not-exist-' . uniqid(), '/url'); + } - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); + /** + * A directory is rejected, since it is readable but not a file. + * + * @covers ::sendFile + */ + public function testSendFileDirectoryRejected() { + $client = $this->client([]); - $client = new QuantClient($http, $config, $logger); - $client->sendFile('/tmp/test', '/url'); + $this->expectException(InvalidPayload::class); + $client->sendFile(sys_get_temp_dir(), '/url'); } /** - * Ensure files are validated before sending. + * A readable file is uploaded as multipart with the target url attached. + * + * @covers ::sendFile */ public function testSendFileValid() { - // phpcs:ignore - global $exists_return; - // phpcs:ignore - global $readable_return; - - $exists_return = TRUE; - $readable_return = TRUE; - - $http = $this->prophesize(Client::class); - $logger = $this->prophesize(LoggerChannelFactoryInterface::class); - $config = $this->getConfigStub(); - $res = $this->getProjectResponse(); - - // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. - $http->post('http://test', [ - 'headers' => [ - 'Quant-File-Url' => '/url', - 'Quant-Customer' => 'account', - 'Quant-Token' => 'token', - ], - 'multipart' => [ - [ - 'name' => 'filename', - 'filename' => 'test.jpg', - 'contents' => [], - ], - ], - ])->willReturn($res); - - $client = new QuantClient($http, $config, $logger); - $client->sendFile('/tmp/test.jpg', '/url'); + $file = $this->tempFile(); + $client = $this->client([new Response(200, [], json_encode(['project' => 'test']))]); + + $this->assertEquals(['project' => 'test'], $client->sendFile($file, '/url')); + + $request = $this->request(); + $this->assertEquals('POST', $request->getMethod()); + $this->assertEquals(self::ENDPOINT, (string) $request->getUri()); + $this->assertEquals('/url', $request->getHeaderLine('Quant-File-Url')); + $this->assertAuthHeaders($request); + + // The body is a multipart stream naming the file and carrying its bytes. + $body = (string) $request->getBody(); + $this->assertStringContainsString(basename($file), $body); + $this->assertStringContainsString('test contents', $body); } -} + /** + * Unpublish sends a PATCH naming the url to withdraw. + * + * @covers ::unpublish + */ + public function testUnpublish() { + $client = $this->client([new Response(200, [], json_encode(['published' => FALSE]))]); -// -// Hacky... this is a hack to stub php built-ins so we -// can correctly test the send file method. -// -namespace Drupal\quant_api\Client; + $this->assertEquals(['published' => FALSE], $client->unpublish('/a')); -/** - * Stub file_exists. - */ -function file_exists($path) { - // phpcs:ignore - global $exists_return; - if (isset($exists_return)) { - return $exists_return; + $request = $this->request(); + $this->assertEquals('PATCH', $request->getMethod()); + $this->assertEquals(self::ENDPOINT . '/unpublish', (string) $request->getUri()); + $this->assertEquals('/a', $request->getHeaderLine('Quant-Url')); + $this->assertAuthHeaders($request); } - return call_user_func_array('\file_exists', func_get_args()); -} -/** - * Stub is_readable. - */ -function is_readable($path) { - // phpcs:ignore - global $readable_return; - if (isset($readable_return)) { - return $readable_return; + /** + * A bare url list is wrapped in the key the API expects. + * + * @covers ::getUrlMeta + */ + public function testGetUrlMetaWrapsBareList() { + $client = $this->client([new Response(200, [], json_encode(['meta' => []]))]); + + $client->getUrlMeta(['/a', '/b']); + + $request = $this->request(); + $this->assertEquals(self::ENDPOINT . '/url-meta', (string) $request->getUri()); + $this->assertEquals( + ['Quant-Url' => ['/a', '/b']], + json_decode((string) $request->getBody(), TRUE) + ); + } + + /** + * An already-wrapped url list is passed through unchanged. + * + * @covers ::getUrlMeta + */ + public function testGetUrlMetaKeepsWrappedList() { + $client = $this->client([new Response(200, [], json_encode(['meta' => []]))]); + + $client->getUrlMeta(['Quant-Url' => ['/a']]); + + $this->assertEquals( + ['Quant-Url' => ['/a']], + json_decode((string) $this->request()->getBody(), TRUE) + ); + } + + /** + * Search records are posted to the search endpoint. + * + * @covers ::sendSearchRecords + */ + public function testSendSearchRecords() { + $records = [['title' => 'A', 'url' => '/a']]; + $client = $this->client([new Response(200, [], json_encode(['count' => 1]))]); + + $this->assertEquals(['count' => 1], $client->sendSearchRecords($records)); + + $request = $this->request(); + $this->assertEquals(self::ENDPOINT . '/search', (string) $request->getUri()); + $this->assertEquals($records, json_decode((string) $request->getBody(), TRUE)); + $this->assertAuthHeaders($request); + } + + /** + * Clearing the index deletes the whole search collection. + * + * @covers ::clearSearchIndex + */ + public function testClearSearchIndex() { + $client = $this->client([new Response(200, [], json_encode(['cleared' => TRUE]))]); + + $this->assertEquals(['cleared' => TRUE], $client->clearSearchIndex()); + + $request = $this->request(); + $this->assertEquals('DELETE', $request->getMethod()); + $this->assertEquals(self::ENDPOINT . '/search/all', (string) $request->getUri()); + $this->assertAuthHeaders($request); + } + + /** + * Facets are posted to the facet endpoint. + * + * @covers ::addFacets + */ + public function testAddFacets() { + $facets = ['category', 'tags']; + $client = $this->client([new Response(200, [], json_encode(['ok' => TRUE]))]); + + $client->addFacets($facets); + + $request = $this->request(); + $this->assertEquals(self::ENDPOINT . '/search/facet', (string) $request->getUri()); + $this->assertEquals($facets, json_decode((string) $request->getBody(), TRUE)); + } + + /** + * TLS verification is on by default and off when disabled. + * + * @covers ::send + */ + public function testTlsVerificationFollowsConfig() { + $this->client([new Response(200, [], '{}')])->send([]); + $this->assertTrue($this->history[0]['options']['verify']); + + $this->history = []; + + $this->client([new Response(200, [], '{}')], ['api_tls_disabled' => TRUE])->send([]); + $this->assertFalse($this->history[0]['options']['verify']); + } + + /** + * Overrides are reported by comparing active values against the original. + * + * @covers ::getOverrides + */ + public function testGetOverridesReportsChangedKeys() { + $config = $this->createMock(ImmutableConfig::class); + $config->method('get')->willReturnCallback(fn($key) => match ($key) { + 'api_project' => 'overridden-project', + 'api_account' => self::ACCOUNT, + 'api_token' => self::TOKEN, + 'api_endpoint' => 'http://test', + 'api_tls_disabled' => FALSE, + default => NULL, + }); + // getOriginal() reports the pre-override value for the project only. + $config->method('getOriginal')->willReturnCallback(fn($key) => match ($key) { + 'api_project' => 'base-project', + 'api_account' => self::ACCOUNT, + 'api_token' => self::TOKEN, + 'api_endpoint' => 'http://test', + 'api_tls_disabled' => FALSE, + default => NULL, + }); + + $factory = $this->createMock(ConfigFactoryInterface::class); + $factory->method('get')->willReturn($config); + + $client = new QuantClient( + new Client(['handler' => HandlerStack::create(new MockHandler([]))]), + $factory, + $this->createMock(LoggerChannelFactoryInterface::class) + ); + + $this->assertEquals(['api_project' => 'overridden-project'], $client->getOverrides()); } - return call_user_func_array('\file_exists', func_get_args()); -} -/** - * Stub fopen. - */ -function fopen($file, $opts) { - return []; } diff --git a/modules/quant_cron/quant_cron.module b/modules/quant_cron/quant_cron.module index 0c27e353..5b5bb965 100644 --- a/modules/quant_cron/quant_cron.module +++ b/modules/quant_cron/quant_cron.module @@ -5,6 +5,7 @@ * Add cron support for Quant processing. */ +use Drupal\quant\CliDomainContext; use Drupal\quant\Seed; use Drupal\Core\Form\FormState; use Drupal\quant\Plugin\QueueItem\RouteItem; @@ -24,6 +25,19 @@ function quant_cron_cron() { return; } + // This hook sends content synchronously rather than queueing it, so the + // target project must be resolved before the first send. Drush dispatches + // no kernel.request, so on a multi-domain site the domain has not been + // negotiated yet and every domain would publish to the base project. + $domainId = CliDomainContext::initialize(); + + if ($domainId) { + \Drupal::logger('quant_cron')->notice('Running for domain @domain, publishing to project @project.', [ + '@domain' => $domainId, + '@project' => CliDomainContext::getActiveProject(), + ]); + } + // Load the settings form. $form_state = new FormState(); $form_state->setRebuild(); diff --git a/modules/quant_search/quant_search.module b/modules/quant_search/quant_search.module index e2063519..d39b5266 100644 --- a/modules/quant_search/quant_search.module +++ b/modules/quant_search/quant_search.module @@ -7,6 +7,7 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\node\Entity\Node; +use Drupal\quant\CliDomainContext; use Drupal\quant\Utility; use Drupal\quant_search\Controller\Search; @@ -46,6 +47,12 @@ function quant_search_theme($existing, $type, $theme, $path) { */ function quant_search_run_index($nids, $languages, array &$context) { + // Records are pushed straight to the index rather than queued, so the + // target project must be resolved first. This batch runs from the admin + // form, where the domain is already negotiated, but also from CLI where + // it is not. + CliDomainContext::initialize(); + $client = \Drupal::service('quant_api.client'); $config = \Drupal::config('quant_search.entities.settings'); diff --git a/modules/quant_tome/src/Commands/QuantTomeCommands.php b/modules/quant_tome/src/Commands/QuantTomeCommands.php index 3355f483..4b07a711 100644 --- a/modules/quant_tome/src/Commands/QuantTomeCommands.php +++ b/modules/quant_tome/src/Commands/QuantTomeCommands.php @@ -3,6 +3,7 @@ namespace Drupal\quant_tome\Commands; use Drush\Commands\DrushCommands; +use Drupal\quant\CliDomainContext; use Drupal\quant\Commands\QuantDrushCommands; use Drupal\quant_tome\QuantTomeBatch; @@ -34,8 +35,17 @@ public function __construct(QuantTomeBatch $batch) { * @command quant:tome:deploy */ public function deploy(array $options = ['threads' => 5]) { + // Resolve the domain before checkConfig() reads the API settings, + // otherwise the connection test and the whole deploy target the base + // project rather than this domain's. + $domainId = CliDomainContext::initialize(); + $this->io()->writeln('Preparing Tome output for Quant...'); + if ($domainId) { + $this->io()->writeln(sprintf('Active domain: %s. Target project: %s.', $domainId, CliDomainContext::getActiveProject())); + } + if (!$this->batch->checkConfig()) { $this->io()->error('Cannot connect to the Quant API. Please check the Quant configuration.'); return 1; diff --git a/modules/quant_tome/src/QuantTomeBatch.php b/modules/quant_tome/src/QuantTomeBatch.php index f38d1c95..cde4e633 100644 --- a/modules/quant_tome/src/QuantTomeBatch.php +++ b/modules/quant_tome/src/QuantTomeBatch.php @@ -6,6 +6,7 @@ use Drupal\Core\DependencyInjection\DependencySerializationTrait; use Drupal\Core\File\FileSystemInterface; use Drupal\Core\Queue\QueueFactory; +use Drupal\quant\CliDomainContext; use Drupal\quant\Plugin\QueueItem\RedirectItem; use Drupal\quant\Plugin\QueueItem\RouteItem; use Drupal\quant_api\Client\QuantClient; @@ -204,6 +205,11 @@ public function pathToUri($file_path) { * The file item to send to Quant API. */ public function deploy($item, array &$context) { + // Batch operations may run in a forked process that never negotiated a + // domain, so resolve it here rather than relying on the caller. The call + // is cached per process and costs nothing after the first item. + CliDomainContext::initialize(); + \Drupal::logger('quant_tome')->notice('Sending %s', [ '%s' => $item->log(), ]); diff --git a/src/CliDomainContext.php b/src/CliDomainContext.php index 00d34b1e..89ade19e 100644 --- a/src/CliDomainContext.php +++ b/src/CliDomainContext.php @@ -19,16 +19,41 @@ */ class CliDomainContext { + /** + * The domain id resolved by the first initialize() call. + * + * @var string|null + */ + protected static $domainId = NULL; + + /** + * Whether initialize() has already run in this process. + * + * @var bool + */ + protected static $initialized = FALSE; + /** * Negotiates the active domain so configuration overrides apply. * * Safe to call when the Domain module is absent. In that case it is a no-op * and base configuration continues to apply. * + * The result is cached for the life of the process. A single CLI process + * serves one domain, and resetting the config factory repeatedly would + * discard every cached config object for no gain, so callers in loops and + * batch callbacks can call this freely. + * * @return string|null * The active domain id, or NULL when no domain was negotiated. */ public static function initialize() : ?string { + if (static::$initialized) { + return static::$domainId; + } + + static::$initialized = TRUE; + $moduleHandler = \Drupal::moduleHandler(); if (!$moduleHandler->moduleExists('domain')) { @@ -45,12 +70,18 @@ public static function initialize() : ?string { return NULL; } - // Any config object built before negotiation was cached without the - // override applied. Drop the static cache so the overridden values - // resolve on the next read. - \Drupal::configFactory()->reset(); + static::$domainId = $domain->id(); - return $domain->id(); + // An HTTP request negotiates its domain from kernel.request, before any + // Quant code runs, so the config factory already holds the overridden + // values. Only CLI reaches this point with stale objects cached, and + // dropping them in a web request would discard the whole config cache + // for no benefit. + if (PHP_SAPI === 'cli') { + \Drupal::configFactory()->reset(); + } + + return static::$domainId; } /** @@ -63,4 +94,14 @@ public static function getActiveProject() : ?string { return \Drupal::config('quant_api.settings')->get('api_project') ?: NULL; } + /** + * Forgets the negotiated domain. + * + * Only needed by tests, which exercise several domains in one process. + */ + public static function reset() : void { + static::$initialized = FALSE; + static::$domainId = NULL; + } + } diff --git a/tests/src/Unit/FileItemTest.php b/tests/src/Unit/FileItemTest.php index 2ead2a80..4e2091cd 100644 --- a/tests/src/Unit/FileItemTest.php +++ b/tests/src/Unit/FileItemTest.php @@ -2,6 +2,8 @@ namespace Drupal\Tests\quant\Unit; +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Config\ImmutableConfig; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Logger\LoggerChannelFactoryInterface; use Drupal\Core\Logger\LoggerChannelInterface; @@ -20,6 +22,11 @@ */ class FileItemTest extends UnitTestCase { + /** + * The Quant project the container resolves to. + */ + const PROJECT = 'test-project'; + /** * The mocked event dispatcher. * @@ -61,10 +68,19 @@ protected function setUp(): void { $logger_factory = $this->prophesize(LoggerChannelFactoryInterface::class); $logger_factory->get('quant')->willReturn($this->logger->reveal()); + // Queue items stamp the project they are created for, which reads + // quant_api.settings through the config factory. + $config = $this->prophesize(ImmutableConfig::class); + $config->get('api_project')->willReturn(self::PROJECT); + + $config_factory = $this->prophesize(ConfigFactoryInterface::class); + $config_factory->get('quant_api.settings')->willReturn($config->reveal()); + $container = new ContainerBuilder(); $container->set('event_dispatcher', $this->eventDispatcher->reveal()); $container->set('quant.asset_generator', $this->assetGenerator->reveal()); $container->set('logger.factory', $logger_factory->reveal()); + $container->set('config.factory', $config_factory->reveal()); \Drupal::setContainer($container); } @@ -167,4 +183,21 @@ public function testSendIgnoresMissingFileWithoutOriginalPath() { $item->send(); } + /** + * Items record the project resolved when they were created. + * + * The worker compares this stamp against the project it is publishing to, + * so that an item queued for one site is never sent to another. + * + * @covers ::getTargetProject + */ + public function testItemStampsTargetProject() { + $item = new FileItem([ + 'file' => '/sites/default/files/example.pdf', + 'url' => '/sites/default/files/example.pdf', + ]); + + $this->assertEquals(self::PROJECT, $item->getTargetProject()); + } + } From 0010006dfd4130b8999257ecdb34f11ffca26ae9 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 14:39:01 -0700 Subject: [PATCH 03/15] Add config schema, real CI test runs, and per-domain cache purging. Kernel tests could not install this module's configuration, because most of it had no schema at all: quant.settings, quant.token_settings, quant_api.settings, quant_cron.settings and quant_search.entities.settings were all undeclared, and quant_purger's schema still described the tag_blacklist and path_blacklist keys that were renamed to blocklist and allowlist several updates ago. All are now declared and validate against the configuration the forms actually write. The CI job named phpunit installed the module and stopped there, which is how a comprehensively broken test file survived unnoticed. It now installs the test dependencies the image's phpunit bootstrap needs, and runs every unit and kernel test in the module and its submodules. Verified inside quantcdn/drupal-ci:11.1.x-dev. SitemapManagerTest extended KernelTestBase from a Unit namespace, so it failed for want of a database whenever it was run at all. Moved to Kernel, and the case that doubles a simple_sitemap class now skips where that optional module is absent. quant_purger recorded traffic against a bare path. Every client's /about collapsed into one row, so invalidating a cache tag refreshed whichever domain wrote that row last and left the others stale. The registry now records the domain alongside the path, and returns matches grouped by domain so the queuer can raise one item per domain, each stamped with the project that owns it. Queue items accept an explicit target project for exactly this case; everything else still stamps from the current context. Sites without the Domain module store an empty domain and behave as they did before. Update 9103 adds the column and the unique key. Fixes a latent bug found on the way: TrafficRegistry::add() passed an array to Merge::key(), which takes a single field name and has asserted on an array since Drupal 10. The call is now keys(), which is what it always meant. Test coverage across the module: 57 tests, from 20 of which 9 were broken. End to end, seeding and cron across two domains put 8 pushes in each client's project and none in the base project. --- .github/workflows/ci.yml | 20 ++ config/schema/quant.schema.yml | 116 +++++++ .../config/schema/quant_api.schema.yml | 23 ++ .../config/schema/quant_cron.schema.yml | 48 +++ .../config/schema/quant_purger.schema.yml | 30 +- modules/quant_purger/quant_purger.install | 38 +++ .../src/Plugin/Purge/Queuer/QuantPurger.php | 45 ++- modules/quant_purger/src/TrafficRegistry.php | 67 ++++- .../src/TrafficRegistryInterface.php | 17 ++ .../src/Kernel/TrafficRegistryDomainTest.php | 196 ++++++++++++ .../config/schema/quant_search.schema.yml | 62 ++++ .../{Unit => Kernel}/SitemapManagerTest.php | 10 +- src/Plugin/QueueItem/FileItem.php | 2 +- src/Plugin/QueueItem/NodeItem.php | 2 +- src/Plugin/QueueItem/RedirectItem.php | 2 +- src/Plugin/QueueItem/RouteItem.php | 2 +- src/Plugin/QueueItem/TargetProjectTrait.php | 17 +- src/Plugin/QueueItem/TaxonomyTermItem.php | 2 +- .../QuantSeedWorkerProjectGuardTest.php | 284 ++++++++++++++++++ 19 files changed, 957 insertions(+), 26 deletions(-) create mode 100644 config/schema/quant.schema.yml create mode 100644 modules/quant_api/config/schema/quant_api.schema.yml create mode 100644 modules/quant_cron/config/schema/quant_cron.schema.yml create mode 100644 modules/quant_purger/tests/src/Kernel/TrafficRegistryDomainTest.php create mode 100644 modules/quant_search/config/schema/quant_search.schema.yml rename modules/quant_sitemap/tests/src/{Unit => Kernel}/SitemapManagerTest.php (94%) create mode 100644 tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 059e1349..0b9b38bd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,6 +39,12 @@ jobs: run: composer --no-interaction --no-progress require drupal/token working-directory: /var/www/drupal + # The image ships phpunit globally, but its bootstrap needs the test + # dependencies that core-dev pulls in. + - name: Install test dependencies + run: composer --no-interaction --no-progress require --dev -W drupal/core-dev:^11 + working-directory: /var/www/drupal + - name: Set up custom module run: mkdir -p /var/www/drupal/web/modules/custom/quant @@ -52,3 +58,17 @@ jobs: - name: Enable quant run: drush en quant -y working-directory: /var/www/drupal + + - name: Prepare test output directory + run: mkdir -p /var/www/drupal/web/sites/simpletest/browser_output + working-directory: /var/www/drupal + + # Runs every unit and kernel test in the module and its submodules. + # Previously this job installed the module and stopped, so a broken + # test file went unnoticed. + - name: Run tests + env: + SIMPLETEST_DB: mysql://root:drupal@mariadb:3306/drupal11 + BROWSERTEST_OUTPUT_DIRECTORY: /var/www/drupal/web/sites/simpletest/browser_output + run: vendor/bin/phpunit -c web/core --display-deprecations web/modules/custom/quant + working-directory: /var/www/drupal diff --git a/config/schema/quant.schema.yml b/config/schema/quant.schema.yml new file mode 100644 index 00000000..e4a53c40 --- /dev/null +++ b/config/schema/quant.schema.yml @@ -0,0 +1,116 @@ +quant.settings: + type: config_object + label: 'Quant settings' + mapping: + quant_enabled: + type: boolean + label: 'Enable Quant tracking' + quant_enabled_nodes: + type: boolean + label: 'Track node changes' + quant_enabled_taxonomy: + type: boolean + label: 'Track taxonomy term changes' + quant_enabled_views: + type: boolean + label: 'Track view changes' + quant_enabled_redirects: + type: boolean + label: 'Track redirect changes' + quant_show_page_info_block: + type: boolean + label: 'Show the page info block' + quant_routes_export: + type: string + label: 'Routes to export' + nullable: true + ssl_cert_verify: + type: boolean + label: 'Verify SSL certificates' + disable_content_drafts: + type: boolean + label: 'Do not export unpublished drafts' + proxy_override: + type: boolean + label: 'Override existing proxy configuration' + xpath_selectors: + type: string + label: 'XPath selectors used to discover pager links' + local_server: + type: string + label: 'Local webserver address used for the crawl' + host_domain: + type: string + label: 'HTTP Host header sent with the crawl' + host_domain_strip: + type: boolean + label: 'Strip the host domain from generated content' + entity_node: + type: boolean + label: 'Export nodes' + entity_node_revisions: + type: boolean + label: 'Export node revisions' + entity_node_bundles: + type: sequence + label: 'Node bundles to export' + sequence: + type: string + label: 'Bundle' + entity_node_languages: + type: sequence + label: 'Languages to export' + sequence: + type: string + label: 'Language code' + entity_taxonomy_term: + type: boolean + label: 'Export taxonomy terms' + theme_assets: + type: boolean + label: 'Export theme assets' + views_pages: + type: boolean + label: 'Export view pages' + redirects: + type: boolean + label: 'Export redirects' + robots: + type: boolean + label: 'Export robots.txt' + routes: + type: boolean + label: 'Export custom routes' + routes_textarea: + type: string + label: 'Custom routes, one per line' + file_paths: + type: boolean + label: 'Export custom file paths' + file_paths_textarea: + type: string + label: 'Custom file paths, one per line' + lunr: + type: boolean + label: 'Export Lunr search assets' + export_sitemap: + type: boolean + label: 'Export the XML sitemap' + +quant.token_settings: + type: config_object + label: 'Quant token settings' + mapping: + timeout: + type: string + label: 'Token lifetime, as a relative time string' + disable: + type: boolean + label: 'Disable token verification' + secret: + type: string + label: 'Shared secret used to sign tokens' + nullable: true + strict: + type: boolean + label: 'Reject requests whose token does not match the route' diff --git a/modules/quant_api/config/schema/quant_api.schema.yml b/modules/quant_api/config/schema/quant_api.schema.yml new file mode 100644 index 00000000..80b8c5e3 --- /dev/null +++ b/modules/quant_api/config/schema/quant_api.schema.yml @@ -0,0 +1,23 @@ +quant_api.settings: + type: config_object + label: 'Quant API settings' + mapping: + api_endpoint: + type: string + label: 'API endpoint' + api_account: + type: string + label: 'API organization' + api_project: + type: string + label: 'API project' + # Multi-domain sites override this per domain, so that each domain + # publishes to its own project. + nullable: true + api_token: + type: string + label: 'API token' + nullable: true + api_tls_disabled: + type: boolean + label: 'Disable TLS verification' diff --git a/modules/quant_cron/config/schema/quant_cron.schema.yml b/modules/quant_cron/config/schema/quant_cron.schema.yml new file mode 100644 index 00000000..746b291f --- /dev/null +++ b/modules/quant_cron/config/schema/quant_cron.schema.yml @@ -0,0 +1,48 @@ +quant_cron.settings: + type: config_object + label: 'Quant cron settings' + mapping: + entity_node: + type: boolean + label: 'Export nodes' + entity_node_bundles: + type: sequence + label: 'Node bundles to export' + sequence: + type: string + label: 'Bundle' + entity_node_languages: + type: sequence + label: 'Languages to export' + sequence: + type: string + label: 'Language code' + entity_taxonomy_term: + type: boolean + label: 'Export taxonomy terms' + theme_assets: + type: boolean + label: 'Export theme assets' + views_pages: + type: boolean + label: 'Export view pages' + robots: + type: boolean + label: 'Export robots.txt' + routes: + type: boolean + label: 'Export custom routes' + routes_export: + type: string + label: 'Custom routes, one per line' + nullable: true + file_paths: + type: boolean + label: 'Export custom file paths' + file_paths_textarea: + type: string + label: 'Custom file paths, one per line' + nullable: true + lunr: + type: boolean + label: 'Export Lunr search assets' diff --git a/modules/quant_purger/config/schema/quant_purger.schema.yml b/modules/quant_purger/config/schema/quant_purger.schema.yml index 887cf0d5..0d8be960 100644 --- a/modules/quant_purger/config/schema/quant_purger.schema.yml +++ b/modules/quant_purger/config/schema/quant_purger.schema.yml @@ -1,21 +1,37 @@ - # Schema for the configuration files of the purge_queuer_url module. +# Schema for the configuration files of the quant_purger module. quant_purger.settings: type: config_object label: 'Quant purger settings.' mapping: - tag_blacklist: - label: 'A list of string tags that will not trigger a queue.' + tag_blocklist: + label: 'Cache tags that will not trigger a queue.' type: sequence translatable: false sequence: type: string - label: 'String that cannot be present in the ccache tag.' + label: 'String that cannot be present in the cache tag.' translatable: false - path_blacklist: - label: 'A list of string patterns that will not get queued.' + path_blocklist: + label: 'Path patterns that will not get queued.' type: sequence translatable: false sequence: type: string - label: 'String that cannot be present in a fully qualified URL.' + label: 'String that cannot be present in a path.' + translatable: false + tag_allowlist: + label: 'Cache tags that will always trigger a queue.' + type: sequence + translatable: false + sequence: + type: string + label: 'String that must be present in the cache tag.' + translatable: false + path_allowlist: + label: 'Path patterns that will always get queued.' + type: sequence + translatable: false + sequence: + type: string + label: 'String that must be present in a path.' translatable: false diff --git a/modules/quant_purger/quant_purger.install b/modules/quant_purger/quant_purger.install index 77ba85bb..fcfa1acf 100644 --- a/modules/quant_purger/quant_purger.install +++ b/modules/quant_purger/quant_purger.install @@ -23,6 +23,13 @@ function quant_purger_schema() { 'length' => 255, 'not null' => TRUE, ], + 'domain' => [ + 'description' => 'The domain the URL was requested on, empty on single-domain sites', + 'type' => 'varchar', + 'length' => 128, + 'not null' => TRUE, + 'default' => '', + ], 'tags' => [ 'description' => 'Space-separated list of cache tag IDs for this entry', 'type' => 'text', @@ -31,6 +38,11 @@ function quant_purger_schema() { ], ], 'primary key' => ['urlid'], + // The same path exists on every domain, so a URL is only unique when + // paired with the domain it was served from. + 'unique keys' => [ + 'url_domain' => ['url', 'domain'], + ], ]; return $schema; } @@ -99,3 +111,29 @@ function quant_purger_update_9102(&$sandbox) { $config->set('path_allowlist', ['']); $config->save(); } + +/** + * Record the domain each tracked URL was served from. + */ +function quant_purger_update_9103(&$sandbox) { + $schema = \Drupal::database()->schema(); + + if (!$schema->fieldExists('purge_queuer_quant', 'domain')) { + $schema->addField('purge_queuer_quant', 'domain', [ + 'description' => 'The domain the URL was requested on, empty on single-domain sites', + 'type' => 'varchar', + 'length' => 128, + 'not null' => TRUE, + 'default' => '', + ]); + } + + // Existing rows keep an empty domain, which is how a single-domain site + // continues to behave. A multi-domain site repopulates the registry as + // traffic arrives. + if (!$schema->indexExists('purge_queuer_quant', 'url_domain')) { + $schema->addUniqueKey('purge_queuer_quant', 'url_domain', ['url', 'domain']); + } + + return t('Added the domain column to the Quant purger traffic registry.'); +} diff --git a/modules/quant_purger/src/Plugin/Purge/Queuer/QuantPurger.php b/modules/quant_purger/src/Plugin/Purge/Queuer/QuantPurger.php index bc215d98..042b3292 100644 --- a/modules/quant_purger/src/Plugin/Purge/Queuer/QuantPurger.php +++ b/modules/quant_purger/src/Plugin/Purge/Queuer/QuantPurger.php @@ -101,15 +101,54 @@ public function invalidateTags(array $tags) { return; } - $paths = $this->registry->getPaths($tags); + $pathsByDomain = $this->registry->getPathsByDomain($tags); foreach ($tags as $tag) { $this->invalidatedTags[] = $tag; } - foreach ($paths as $path) { - $this->quantSeedQueue->createItem(new RouteItem(['route' => $path])); + // A page shown on several domains has to be purged on each of them, and + // each domain publishes to its own project. Stamp every item with the + // project that owns it rather than the one this request happens to be + // serving, otherwise only the current domain is refreshed. + foreach ($pathsByDomain as $domainId => $paths) { + $project = $this->getProjectForDomain($domainId); + + foreach ($paths as $path) { + $this->quantSeedQueue->createItem(new RouteItem([ + 'route' => $path, + 'target_project' => $project, + ])); + } + } + } + + /** + * Resolves the Quant project a given domain publishes to. + * + * @param string $domainId + * The domain id, or an empty string on a single-domain site. + * + * @return string|null + * The project machine name, or NULL when none is configured. + */ + protected function getProjectForDomain($domainId) { + $container = $this->container ?: \Drupal::getContainer(); + + // Without a domain, or without the override service, the site has one + // project and the base configuration names it. + if (!empty($domainId) && $container->has('domain.config_factory_override')) { + $override = $container->get('domain.config_factory_override') + ->getOverride($domainId, 'quant_api.settings'); + + if ($project = $override->get('api_project')) { + return $project; + } } + + return $container->get('config.factory') + ->get('quant_api.settings') + ->get('api_project') ?: NULL; } } diff --git a/modules/quant_purger/src/TrafficRegistry.php b/modules/quant_purger/src/TrafficRegistry.php index b2658026..1a5346cf 100644 --- a/modules/quant_purger/src/TrafficRegistry.php +++ b/modules/quant_purger/src/TrafficRegistry.php @@ -38,17 +38,44 @@ public function __construct(Connection $connection) { $this->config = \Drupal::configFactory()->get('quant_purger.settings'); } + /** + * Returns the domain currently being served. + * + * A site serving several domains from one Drupal instance has the same + * path on every domain, so the registry has to keep them apart. Sites + * without the Domain module record an empty string and behave as before. + * + * @return string + * The active domain id, or an empty string. + */ + protected function getActiveDomainId() : string { + if (!\Drupal::moduleHandler()->moduleExists('domain')) { + return ''; + } + + if (!\Drupal::hasService('domain.negotiator')) { + return ''; + } + + $domain = \Drupal::service('domain.negotiator')->getActiveDomain(); + + return $domain ? $domain->id() : ''; + } + /** * {@inheritdoc} */ public function add($url, array $tags) { $tags = ';' . implode(';', $tags) . ';'; - $fields = ['url' => $url, 'tags' => $tags]; + $domain = $this->getActiveDomainId(); + $fields = ['url' => $url, 'domain' => $domain, 'tags' => $tags]; + // keys(), not key(): the latter takes a single field name and asserts on + // an array, so the previous call broke under Drupal 10 and later. $this->connection->merge('purge_queuer_quant') ->insertFields($fields) ->updateFields($fields) - ->key(['url' => $url]) + ->keys(['url' => $url, 'domain' => $domain]) ->execute(); } @@ -58,6 +85,7 @@ public function add($url, array $tags) { public function remove($url) { $this->connection->delete('purge_queuer_quant') ->condition('url', $url) + ->condition('domain', $this->getActiveDomainId()) ->execute(); } @@ -73,7 +101,27 @@ public function clear() { */ public function getPaths(array $tags) { $urls = []; + + foreach ($this->getPathsByDomain($tags) as $paths) { + foreach ($paths as $path) { + $urls[$path] = $path; + } + } + + return array_values($urls); + } + + /** + * {@inheritdoc} + */ + public function getPathsByDomain(array $tags) { + $paths = []; $tags = $this->getAcceptedCacheTags($tags); + + if (empty($tags)) { + return $paths; + } + $or = new Condition('OR'); foreach ($tags as $tag) { $condition = '%;' . $this->connection->escapeLike($tag) . ';%'; @@ -82,22 +130,25 @@ public function getPaths(array $tags) { try { $results = $this->connection->select('purge_queuer_quant', 'q') - ->fields('q', ['url']) + ->fields('q', ['url', 'domain']) ->condition($or) ->execute(); } catch (\Exception $e) { - // During install and uninstall the purge_queue_quant table may not + // During install and uninstall the purge_queuer_quant table may not // be available which can result in a race condition with this query, - // return an empty URL list if the query fails. - return $urls; + // return an empty list if the query fails. + return $paths; } + // The same page exists on every domain that serves it, and each domain + // publishes to a different project, so the caller needs to know which + // domain each path came from. foreach ($results as $result) { - $urls[] = $result->url; + $paths[$result->domain][] = $result->url; } - return $urls; + return $paths; } } diff --git a/modules/quant_purger/src/TrafficRegistryInterface.php b/modules/quant_purger/src/TrafficRegistryInterface.php index 39d80bf3..08f571f7 100644 --- a/modules/quant_purger/src/TrafficRegistryInterface.php +++ b/modules/quant_purger/src/TrafficRegistryInterface.php @@ -44,4 +44,21 @@ public function clear(); */ public function getPaths(array $tags); + /** + * Gets the paths matching the given cache tags, grouped by domain. + * + * A site serving several domains from one Drupal instance shows the same + * page on more than one of them, and each domain publishes to its own + * Quant project. Invalidating a tag therefore has to purge the page on + * every domain that serves it, addressed to that domain's project. + * + * @param array $tags + * The cache tags being invalidated. + * + * @return array + * Lists of paths keyed by domain id. Sites without the Domain module + * return everything under an empty-string key. + */ + public function getPathsByDomain(array $tags); + } diff --git a/modules/quant_purger/tests/src/Kernel/TrafficRegistryDomainTest.php b/modules/quant_purger/tests/src/Kernel/TrafficRegistryDomainTest.php new file mode 100644 index 00000000..f206e652 --- /dev/null +++ b/modules/quant_purger/tests/src/Kernel/TrafficRegistryDomainTest.php @@ -0,0 +1,196 @@ +installSchema('quant_purger', ['purge_queuer_quant']); + $this->installConfig(['quant_purger']); + + $this->registry = $this->container->get('quant_purger.registry'); + } + + /** + * Writes a registry row for a given domain. + * + * The registry resolves the domain itself, which needs the Domain module. + * Writing directly keeps this test focused on the storage behaviour. + * + * @param string $url + * The path. + * @param string $domain + * The domain id. + * @param array $tags + * The cache tags. + */ + protected function record(string $url, string $domain, array $tags) : void { + $fields = [ + 'url' => $url, + 'domain' => $domain, + 'tags' => ';' . implode(';', $tags) . ';', + ]; + + $this->container->get('database')->merge('purge_queuer_quant') + ->insertFields($fields) + ->updateFields($fields) + ->keys(['url' => $url, 'domain' => $domain]) + ->execute(); + } + + /** + * The same path on two domains is kept as two rows. + * + * @covers ::getPathsByDomain + */ + public function testSamePathOnTwoDomainsIsNotCollapsed() { + $this->record('/about', 'clienta', ['node:1']); + $this->record('/about', 'clientb', ['node:1']); + + $byDomain = $this->registry->getPathsByDomain(['node:1']); + + $this->assertEqualsCanonicalizing(['clienta', 'clientb'], array_keys($byDomain)); + $this->assertEquals(['/about'], $byDomain['clienta']); + $this->assertEquals(['/about'], $byDomain['clientb']); + } + + /** + * Only the domains serving a tag are returned. + * + * @covers ::getPathsByDomain + */ + public function testUnrelatedDomainIsNotReturned() { + $this->record('/about', 'clienta', ['node:1']); + $this->record('/contact', 'clientb', ['node:2']); + + $byDomain = $this->registry->getPathsByDomain(['node:1']); + + $this->assertEquals(['clienta'], array_keys($byDomain)); + } + + /** + * A path registered on one domain only purges that domain. + * + * @covers ::getPathsByDomain + */ + public function testDistinctPathsPerDomain() { + $this->record('/a', 'clienta', ['node:1']); + $this->record('/b', 'clienta', ['node:1']); + $this->record('/c', 'clientb', ['node:1']); + + $byDomain = $this->registry->getPathsByDomain(['node:1']); + + $this->assertEqualsCanonicalizing(['/a', '/b'], $byDomain['clienta']); + $this->assertEquals(['/c'], $byDomain['clientb']); + } + + /** + * A site without the Domain module groups everything under one empty key. + * + * @covers ::getPathsByDomain + */ + public function testSingleDomainSiteUsesEmptyKey() { + $this->record('/about', '', ['node:1']); + + $byDomain = $this->registry->getPathsByDomain(['node:1']); + + $this->assertEquals([''], array_keys($byDomain)); + $this->assertEquals(['/about'], $byDomain['']); + } + + /** + * The flat path list still works and reports each path once. + * + * @covers ::getPaths + */ + public function testGetPathsDeduplicatesAcrossDomains() { + $this->record('/about', 'clienta', ['node:1']); + $this->record('/about', 'clientb', ['node:1']); + $this->record('/other', 'clientb', ['node:1']); + + $this->assertEqualsCanonicalizing( + ['/about', '/other'], + $this->registry->getPaths(['node:1']) + ); + } + + /** + * An unmatched tag returns nothing rather than everything. + * + * @covers ::getPathsByDomain + */ + public function testUnmatchedTagReturnsNothing() { + $this->record('/about', 'clienta', ['node:1']); + + $this->assertEquals([], $this->registry->getPathsByDomain(['node:999'])); + } + + /** + * Adding a path records it against the active domain. + * + * Without the Domain module that is the empty string, and the row is + * updated in place rather than duplicated. + * + * @covers ::add + */ + public function testAddIsIdempotentForOneDomain() { + $this->registry->add('/about', ['node:1']); + $this->registry->add('/about', ['node:1', 'node:2']); + + $count = $this->container->get('database') + ->select('purge_queuer_quant', 'q') + ->countQuery() + ->execute() + ->fetchField(); + + $this->assertEquals(1, $count); + $this->assertEquals(['/about'], $this->registry->getPaths(['node:2'])); + } + +} diff --git a/modules/quant_search/config/schema/quant_search.schema.yml b/modules/quant_search/config/schema/quant_search.schema.yml new file mode 100644 index 00000000..2732c172 --- /dev/null +++ b/modules/quant_search/config/schema/quant_search.schema.yml @@ -0,0 +1,62 @@ +quant_search.entities.settings: + type: config_object + label: 'Quant search record settings' + mapping: + quant_search_entity_node: + type: boolean + label: 'Keep search records for nodes updated' + quant_search_entity_node_bundles: + type: sequence + label: 'Node bundles to index' + sequence: + type: string + label: 'Bundle' + quant_search_entity_node_languages: + type: sequence + label: 'Languages to index' + sequence: + type: string + label: 'Language code' + quant_search_entity_taxonomy_term: + type: boolean + label: 'Keep search records for taxonomy terms updated' + quant_search_title_token: + type: string + label: 'Token used for the record title' + quant_search_summary_token: + type: string + label: 'Token used for the record summary' + quant_search_image_token: + type: string + label: 'Token used for the record image' + quant_search_content_viewmode: + type: string + label: 'View mode rendered into the record body' + +# Per bundle overrides of the defaults above, keyed by node type. +quant_search.entities.settings.*: + type: config_object + label: 'Quant search record settings for a content type' + mapping: + enabled: + type: boolean + label: 'Override the default values for this content type' + exclude: + type: boolean + label: 'Exclude this content type from the search index' + quant_search_title_token: + type: string + label: 'Token used for the record title' + nullable: true + quant_search_summary_token: + type: string + label: 'Token used for the record summary' + nullable: true + quant_search_image_token: + type: string + label: 'Token used for the record image' + nullable: true + quant_search_content_viewmode: + type: string + label: 'View mode rendered into the record body' + nullable: true diff --git a/modules/quant_sitemap/tests/src/Unit/SitemapManagerTest.php b/modules/quant_sitemap/tests/src/Kernel/SitemapManagerTest.php similarity index 94% rename from modules/quant_sitemap/tests/src/Unit/SitemapManagerTest.php rename to modules/quant_sitemap/tests/src/Kernel/SitemapManagerTest.php index 28e52704..c69e3029 100644 --- a/modules/quant_sitemap/tests/src/Unit/SitemapManagerTest.php +++ b/modules/quant_sitemap/tests/src/Kernel/SitemapManagerTest.php @@ -1,8 +1,9 @@ markTestSkipped('The simple_sitemap module is not installed.'); + } + $module_handler_mock = $this->createMock(ModuleHandler::class); $module_handler_mock->expects($this->once()) ->method('moduleExists') diff --git a/src/Plugin/QueueItem/FileItem.php b/src/Plugin/QueueItem/FileItem.php index 2e90695e..357e5f21 100644 --- a/src/Plugin/QueueItem/FileItem.php +++ b/src/Plugin/QueueItem/FileItem.php @@ -51,7 +51,7 @@ public function __construct(array $data = []) { $this->originalPath = $data['original_path'] ?? NULL; // Record the project this item is destined for. - $this->stampTargetProject(); + $this->stampTargetProject($data); } /** diff --git a/src/Plugin/QueueItem/NodeItem.php b/src/Plugin/QueueItem/NodeItem.php index 3ed66358..204f4e80 100644 --- a/src/Plugin/QueueItem/NodeItem.php +++ b/src/Plugin/QueueItem/NodeItem.php @@ -51,7 +51,7 @@ public function __construct(array $data = []) { $this->filter = isset($data['lang_filter']) && is_array($data['lang_filter']) ? array_filter($data['lang_filter']) : []; // Record the project this item is destined for. - $this->stampTargetProject(); + $this->stampTargetProject($data); } /** diff --git a/src/Plugin/QueueItem/RedirectItem.php b/src/Plugin/QueueItem/RedirectItem.php index b76197b5..b64cd640 100644 --- a/src/Plugin/QueueItem/RedirectItem.php +++ b/src/Plugin/QueueItem/RedirectItem.php @@ -43,7 +43,7 @@ public function __construct(array $data = []) { $this->statusCode = $data['status_code']; // Record the project this item is destined for. - $this->stampTargetProject(); + $this->stampTargetProject($data); } /** diff --git a/src/Plugin/QueueItem/RouteItem.php b/src/Plugin/QueueItem/RouteItem.php index 73950d03..85cd89c3 100644 --- a/src/Plugin/QueueItem/RouteItem.php +++ b/src/Plugin/QueueItem/RouteItem.php @@ -56,7 +56,7 @@ public function __construct(array $data = []) { $this->filePath = $data['file_path'] ?? DRUPAL_ROOT . strtok($route, '?'); // Record the project this item is destined for. - $this->stampTargetProject(); + $this->stampTargetProject($data); } /** diff --git a/src/Plugin/QueueItem/TargetProjectTrait.php b/src/Plugin/QueueItem/TargetProjectTrait.php index 842bf2cb..67a6807f 100644 --- a/src/Plugin/QueueItem/TargetProjectTrait.php +++ b/src/Plugin/QueueItem/TargetProjectTrait.php @@ -29,9 +29,22 @@ trait TargetProjectTrait { protected $targetProject = NULL; /** - * Records the project resolved in the current domain context. + * Records the project this item must be published to. + * + * Defaults to the project the current domain context resolves to. Callers + * that queue work on behalf of a domain other than the one they are + * serving — cache invalidation touching every domain that shows a page, + * for example — pass the project explicitly. + * + * @param array $data + * The queue item data. An explicit 'target_project' wins. */ - protected function stampTargetProject() : void { + protected function stampTargetProject(array $data = []) : void { + if (!empty($data['target_project'])) { + $this->targetProject = $data['target_project']; + return; + } + $this->targetProject = \Drupal::config('quant_api.settings')->get('api_project') ?: NULL; } diff --git a/src/Plugin/QueueItem/TaxonomyTermItem.php b/src/Plugin/QueueItem/TaxonomyTermItem.php index fe526bfc..3f8d7fd4 100644 --- a/src/Plugin/QueueItem/TaxonomyTermItem.php +++ b/src/Plugin/QueueItem/TaxonomyTermItem.php @@ -27,7 +27,7 @@ public function __construct(array $data = []) { $this->tid = $data['tid']; // Record the project this item is destined for. - $this->stampTargetProject(); + $this->stampTargetProject($data); } /** diff --git a/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php b/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php new file mode 100644 index 00000000..e52c046e --- /dev/null +++ b/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php @@ -0,0 +1,284 @@ +installConfig(['quant_api']); + + // The negotiated domain is cached per process; tests exercise several. + CliDomainContext::reset(); + + $this->worker = new QuantSeedWorker([], 'quant_seed_worker', []); + } + + /** + * Points the site at a given Quant project. + * + * @param string|null $project + * The project machine name. + */ + protected function setActiveProject(?string $project) : void { + \Drupal::configFactory() + ->getEditable('quant_api.settings') + ->set('api_project', $project) + ->save(); + } + + /** + * Builds a queue item that records whether it was sent. + * + * @param string|null $target + * The project the item is stamped for. + * + * @return \Drupal\quant\Plugin\QueueItem\QuantQueueItemInterface + * The recording item. + */ + protected function recordingItem(?string $target) : QuantQueueItemInterface { + return new class($target) implements QuantQueueItemInterface { + + /** + * Whether send() ran. + * + * @var bool + */ + public $sent = FALSE; + + /** + * The stamped project. + * + * @var string|null + */ + protected $target; + + /** + * Constructs the item. + */ + public function __construct(?string $target) { + $this->target = $target; + } + + /** + * {@inheritdoc} + */ + public function send() { + $this->sent = TRUE; + return TRUE; + } + + /** + * {@inheritdoc} + */ + public function info() { + return 'recording item'; + } + + /** + * {@inheritdoc} + */ + public function log() { + return 'recording item'; + } + + /** + * {@inheritdoc} + */ + public function getTargetProject() : ?string { + return $this->target; + } + + }; + } + + /** + * An item is sent when its stamp matches the active project. + * + * @covers ::processItem + * @covers ::targetsActiveProject + */ + public function testSendsWhenProjectMatches() { + $this->setActiveProject('project-a'); + $item = $this->recordingItem('project-a'); + + $this->worker->processItem($item); + + $this->assertTrue($item->sent, 'The item was published.'); + } + + /** + * An item is withheld when its stamp names a different project. + * + * This is the case that would otherwise put one client's page on another + * client's domain. + * + * @covers ::processItem + * @covers ::targetsActiveProject + */ + public function testWithholdsWhenProjectDiffers() { + $this->setActiveProject('project-a'); + $item = $this->recordingItem('project-b'); + + $this->worker->processItem($item); + + $this->assertFalse($item->sent, 'The item was not published to the wrong project.'); + } + + /** + * Items queued before stamping existed are still sent. + * + * Queues survive a module update, and a single-domain site has no stamp to + * compare, so an absent stamp must not block publishing. + * + * @covers ::targetsActiveProject + */ + public function testSendsUnstampedLegacyItem() { + $this->setActiveProject('project-a'); + $item = $this->recordingItem(NULL); + + $this->worker->processItem($item); + + $this->assertTrue($item->sent, 'An unstamped item was published.'); + } + + /** + * An item is withheld when the worker has no project configured at all. + * + * @covers ::targetsActiveProject + */ + public function testWithholdsWhenNoActiveProject() { + $this->setActiveProject(NULL); + $item = $this->recordingItem('project-a'); + + $this->worker->processItem($item); + + $this->assertFalse($item->sent, 'The item was not published without a target.'); + } + + /** + * Anything that is not a queue item is ignored rather than fatal. + * + * @covers ::processItem + */ + public function testIgnoresForeignQueueItem() { + $this->assertNull($this->worker->processItem(new \stdClass())); + } + + /** + * A real queue item records the project configured when it was built. + * + * @covers \Drupal\quant\Plugin\QueueItem\TargetProjectTrait::getTargetProject + */ + public function testRealItemStampsActiveProject() { + $this->setActiveProject('project-a'); + + $this->assertEquals('project-a', (new RouteItem(['route' => '/a']))->getTargetProject()); + } + + /** + * The stamp follows the configuration in force at enqueue time. + * + * @covers \Drupal\quant\Plugin\QueueItem\TargetProjectTrait::stampTargetProject + */ + public function testStampFollowsProjectAtEnqueueTime() { + $this->setActiveProject('project-a'); + $first = new RouteItem(['route' => '/a']); + + $this->setActiveProject('project-b'); + $second = new RouteItem(['route' => '/b']); + + $this->assertEquals('project-a', $first->getTargetProject()); + $this->assertEquals('project-b', $second->getTargetProject()); + } + + /** + * Without the Domain module, negotiation is a no-op. + * + * @covers \Drupal\quant\CliDomainContext::initialize + */ + public function testDomainContextIsNoopWithoutDomainModule() { + $this->assertFalse(\Drupal::moduleHandler()->moduleExists('domain')); + $this->assertNull(CliDomainContext::initialize()); + } + + /** + * The negotiated domain is resolved once and reused. + * + * Batch and loop callers invoke this per item, so it must not repeat the + * config cache reset. + * + * @covers \Drupal\quant\CliDomainContext::initialize + * @covers \Drupal\quant\CliDomainContext::reset + */ + public function testDomainContextCachesItsResult() { + $this->assertNull(CliDomainContext::initialize()); + $this->assertNull(CliDomainContext::initialize()); + + CliDomainContext::reset(); + + $this->assertNull(CliDomainContext::initialize()); + } + + /** + * The active project is read through the overridable config factory. + * + * @covers \Drupal\quant\CliDomainContext::getActiveProject + */ + public function testGetActiveProjectReadsConfig() { + $this->setActiveProject('project-a'); + $this->assertEquals('project-a', CliDomainContext::getActiveProject()); + + $this->setActiveProject(NULL); + $this->assertNull(CliDomainContext::getActiveProject()); + } + +} From 10db0208475a1b9faf3e56701e7ffbf542495e8f Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 15:13:29 -0700 Subject: [PATCH 04/15] Fix live entity saves failing to publish, and guard unknown hosts. Driving a real node save through the browser showed that saving from the UI published nothing at all. The hooks queue their work with drupal_register_shutdown_function, and by the time those callbacks run Drupal has already popped the request off the stack. Info metadata calls $this->token->replace(), token info asks the request for its base path, and the resulting Error is caught by the shutdown handler, which can only reach error_log(). The save succeeded, the page rendered, watchdog stayed silent, and the content never reached Quant. Seed callbacks now run with a request rebuilt from the globals, which still describe the request that triggered the save, so the correct host and therefore the correct domain stay in scope. That also fixes the same crash reached through quant_search, whose subscriber renders the entity and runs ahead of the publisher, so its failure suppressed the push too. Utility::getPageInfo() declared a string return type but only assigned $output inside the branch handling a URL that Quant already knows about. Viewing any not-yet-synced page as an administrator, with the page info block enabled, returned NULL and produced a 500. It now starts from an empty string. Its unmatched-URL list also closed itself once per URL rather than once, so that markup is repaired. The browser test then caught something worse. A save on one domain was published to another client's project. When the request host matches no domain record the Domain module falls back to the default domain, and every Quant push follows it: the wrong customer's site changes and nothing reports it. The queue item stamp cannot catch this, because the stamp is taken from the same mistaken context. A guard subscriber now refuses to publish when the serving host has no domain record, ahead of both the search and publish subscribers, naming the host and the project it would otherwise have written to. It applies to the command line as well: a --uri that names no configured domain, or a cron run with no --uri, falls back identically and would republish an entire site into the default domain's project. Sites without the Domain module, or with no domains configured, are unaffected. Verified in the browser across both domains: each save reaches only its own project, with its search record. With a hostname deliberately broken, zero pushes leave Drupal and the refusal is logged. Command line seeding and cron behave the same way. --- quant.module | 40 +++- quant.services.yml | 7 + src/EventSubscriber/DomainGuardSubscriber.php | 114 ++++++++++++ src/Utility.php | 14 +- tests/src/Unit/DomainGuardSubscriberTest.php | 175 ++++++++++++++++++ 5 files changed, 345 insertions(+), 5 deletions(-) create mode 100644 src/EventSubscriber/DomainGuardSubscriber.php create mode 100644 tests/src/Unit/DomainGuardSubscriberTest.php diff --git a/quant.module b/quant.module index 0d20e01d..a5890cf5 100644 --- a/quant.module +++ b/quant.module @@ -15,6 +15,7 @@ use Drupal\Core\Site\Settings; use Drupal\Core\Url; use Drupal\node\NodeInterface; use Drupal\quant\Exception\TokenValidationDisabledException; +use Symfony\Component\HttpFoundation\Request; use Drupal\quant\Plugin\QueueItem\RouteItem; use Drupal\quant\QuantQueueFactory; use Drupal\quant\Seed; @@ -196,7 +197,44 @@ function quant_shutdown(array $context = []) { } if (is_callable($context['callback'])) { - drupal_register_shutdown_function($context['callback'], $context['args']); + drupal_register_shutdown_function('_quant_run_with_request', $context['callback'], $context['args']); + } +} + +/** + * Runs a seed callback with a request on the stack. + * + * Drupal pops the request once the response has been sent, so by the time + * these callbacks run \Drupal::request() is NULL. Anything that renders the + * entity or replaces a token then fails: token info asks the request for its + * base path and fatals on NULL. The error surfaces only in the PHP log, + * because the shutdown handler cannot reach the logger, so the page silently + * never reaches Quant. + * + * The globals still describe the request that triggered the save, including + * its host, so the rebuilt request also keeps the correct domain in scope. + * + * @param callable $callback + * The seed callback to run. + * @param mixed $args + * The argument to pass to it. + */ +function _quant_run_with_request(callable $callback, $args) { + $stack = \Drupal::service('request_stack'); + $pushed = FALSE; + + if ($stack->getCurrentRequest() === NULL) { + $stack->push(Request::createFromGlobals()); + $pushed = TRUE; + } + + try { + $callback($args); + } + finally { + if ($pushed) { + $stack->pop(); + } } } diff --git a/quant.services.yml b/quant.services.yml index 60113216..9c62d16a 100644 --- a/quant.services.yml +++ b/quant.services.yml @@ -18,6 +18,13 @@ services: tags: - { name: 'event_subscriber' } + quant.domain_guard: + class: Drupal\quant\EventSubscriber\DomainGuardSubscriber + arguments: + - '@request_stack' + tags: + - { name: 'event_subscriber' } + quant.collect_entity_subscriber: class: Drupal\quant\EventSubscriber\CollectionSubscriber arguments: diff --git a/src/EventSubscriber/DomainGuardSubscriber.php b/src/EventSubscriber/DomainGuardSubscriber.php new file mode 100644 index 00000000..0d704d86 --- /dev/null +++ b/src/EventSubscriber/DomainGuardSubscriber.php @@ -0,0 +1,114 @@ +requestStack = $request_stack; + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents(): array { + // Ahead of quant_search (1) and the API publisher (0), so that stopping + // propagation prevents the push entirely. + return [QuantEvent::OUTPUT => ['onOutput', 100]]; + } + + /** + * Stops the push when the serving host has no domain record. + * + * @param \Drupal\quant\Event\QuantEvent $event + * The event. + */ + public function onOutput(QuantEvent $event) { + if (!$this->hostIsUnknown($host)) { + return; + } + + \Drupal::logger('quant')->error('Refused to publish @path: the host @host matches no domain, so the Domain module fell back to the default and this content would be published to project @project. Add a domain for @host, or correct the Host header reaching Drupal.', [ + '@path' => $event->getLocation(), + '@host' => $host, + '@project' => \Drupal::config('quant_api.settings')->get('api_project') ?: 'unknown', + ]); + + $event->stopPropagation(); + } + + /** + * Determines whether the serving host has no domain record. + * + * @param string|null $host + * Set to the offending host when the check fails. + * + * @return bool + * TRUE when the push must be stopped. + */ + protected function hostIsUnknown(&$host = NULL) : bool { + $host = NULL; + + $moduleHandler = \Drupal::moduleHandler(); + + if (!$moduleHandler->moduleExists('domain')) { + return FALSE; + } + + $request = $this->requestStack->getCurrentRequest(); + + if (!$request) { + return FALSE; + } + + $host = $request->getHttpHost(); + $storage = \Drupal::entityTypeManager()->getStorage('domain'); + + // No domains configured at all means the site is not using per-domain + // projects, so there is nothing to get wrong. + if (empty($storage->loadMultiple())) { + return FALSE; + } + + return empty($storage->loadByHostname($host)); + } + +} diff --git a/src/Utility.php b/src/Utility.php index b19093bb..3872c436 100644 --- a/src/Utility.php +++ b/src/Utility.php @@ -248,6 +248,10 @@ public static function getPageInfo(?array $urls = NULL) : string { $client = \Drupal::service('quant_api.client'); $response = $client->getUrlMeta($urls); + // A page that has never been synced has no records, and the method is + // declared to return a string, so start from one. + $output = ''; + if (isset($response['global_meta']['records'])) { // Show meta information for the pages in Quant. $found_urls = []; @@ -277,12 +281,14 @@ public static function getPageInfo(?array $urls = NULL) : string { else { $output .= '' . t('Page info could not be found for the following URLs:') . ''; } + $output .= '
    '; - } - foreach ($urls as $url) { - if (!in_array($url, $found_urls)) { - $output .= '
  • ' . $url . '
    • '; + foreach ($urls as $url) { + if (!in_array($url, $found_urls)) { + $output .= '
  • ' . $url . '
    • '; + } } + // Closing the list inside the loop repeated it once per URL. $output .= '
'; } diff --git a/tests/src/Unit/DomainGuardSubscriberTest.php b/tests/src/Unit/DomainGuardSubscriberTest.php new file mode 100644 index 00000000..edf89c73 --- /dev/null +++ b/tests/src/Unit/DomainGuardSubscriberTest.php @@ -0,0 +1,175 @@ +createMock(ModuleHandlerInterface::class); + $moduleHandler->method('moduleExists')->willReturnCallback( + fn($name) => $name === 'domain' ? $domainEnabled : FALSE + ); + + // loadByHostname() belongs to the Domain module's storage handler, which + // is not present here, so it is added to the double explicitly. + $storage = $this->getMockBuilder(EntityStorageInterface::class) + ->addMethods(['loadByHostname']) + ->getMockForAbstractClass(); + + $storage->method('loadMultiple')->willReturn($domains); + $storage->method('loadByHostname')->willReturnCallback( + fn($host) => $host === $matchedHost ? (object) ['id' => 'matched'] : NULL + ); + + $entityTypeManager = $this->createMock(EntityTypeManagerInterface::class); + $entityTypeManager->method('getStorage')->willReturn($storage); + + $config = $this->createMock(ImmutableConfig::class); + $config->method('get')->willReturn('project-default'); + $configFactory = $this->createMock(ConfigFactoryInterface::class); + $configFactory->method('get')->willReturn($config); + + $loggerFactory = $this->createMock(LoggerChannelFactoryInterface::class); + $loggerFactory->method('get')->willReturn($this->createMock(LoggerChannelInterface::class)); + + $container = new ContainerBuilder(); + $container->set('module_handler', $moduleHandler); + $container->set('entity_type.manager', $entityTypeManager); + $container->set('config.factory', $configFactory); + $container->set('logger.factory', $loggerFactory); + $container->set('string_translation', $this->getStringTranslationStub()); + \Drupal::setContainer($container); + + $stack = new RequestStack(); + $stack->push(Request::create('http://' . $requestHost . '/node/1')); + + return new DomainGuardSubscriber($stack); + } + + /** + * Builds an output event for a page. + * + * @return \Drupal\quant\Event\QuantEvent + * The event. + */ + protected function event() : QuantEvent { + return new QuantEvent('', '/node/1', [], NULL); + } + + /** + * The guard runs ahead of the search and publish subscribers. + * + * Stopping propagation only prevents the push if this runs first. + * + * @covers ::getSubscribedEvents + */ + public function testGuardRunsBeforePublishing() { + $events = DomainGuardSubscriber::getSubscribedEvents(); + + $this->assertGreaterThan(1, $events[QuantEvent::OUTPUT][1]); + } + + /** + * A site without the Domain module publishes as normal. + * + * @covers ::onOutput + */ + public function testPublishesWithoutDomainModule() { + $event = $this->event(); + $this->subscriber(FALSE)->onOutput($event); + + $this->assertFalse($event->isPropagationStopped()); + } + + /** + * A site with the module but no domains configured publishes as normal. + * + * @covers ::onOutput + */ + public function testPublishesWhenNoDomainsConfigured() { + $event = $this->event(); + $this->subscriber(TRUE, [])->onOutput($event); + + $this->assertFalse($event->isPropagationStopped()); + } + + /** + * A host that resolves to a domain publishes as normal. + * + * @covers ::onOutput + */ + public function testPublishesWhenHostResolves() { + $event = $this->event(); + $this->subscriber(TRUE, ['clientb' => 'x'], 'clientb.example', 'clientb.example') + ->onOutput($event); + + $this->assertFalse($event->isPropagationStopped()); + } + + /** + * A host with no domain record stops the push. + * + * This is the case where the Domain module silently falls back to the + * default domain and the content would reach another client's project. + * + * @covers ::onOutput + * @covers ::hostIsUnknown + */ + public function testStopsWhenHostHasNoDomain() { + $event = $this->event(); + $this->subscriber(TRUE, ['clienta' => 'x'], 'clienta.example', 'unregistered.example') + ->onOutput($event); + + $this->assertTrue($event->isPropagationStopped()); + } + + /** + * The port is part of the host, so a port mismatch also stops the push. + * + * @covers ::hostIsUnknown + */ + public function testPortMismatchStopsPush() { + $event = $this->event(); + $this->subscriber(TRUE, ['clientb' => 'x'], 'clientb.example:8080', 'clientb.example') + ->onOutput($event); + + $this->assertTrue($event->isPropagationStopped()); + } + +} From 8cfba0abf6ea6aa0ea46a0718b501575d83b3ab0 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 15:22:02 -0700 Subject: [PATCH 05/15] Scope the domain guard to multi-domain sites, and correct the shutdown note. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard blocked any publish from a host with no domain record, including on the command line. That is wrong for the many sites that run cron and seeds without a --uri: with a single domain there is only one project to publish to, so the fallback cannot misdirect anything, and refusing would stop publishing for no safety gain. It now engages only where more than one domain is configured, which is the only arrangement in which a page can reach a different site's project. Verified: a single-domain site running cron with no --uri publishes as before. A two-domain site still refuses an unrecognised host, on the web and on the command line alike. Also corrects the previous commit's account of the shutdown crash, which overstated its reach. Live saves were not broken everywhere. The failing call is the contrib token module collecting token info, where the site:base-path token describes itself by asking the request for its base path. Core's token service does not do this, so the crash only occurred where contrib token was installed — which in practice means sites running quant_search, since it depends on it. Confirmed by removing quant_search and token and watching a save publish correctly with the fix reverted. Keeping the fix regardless: running these callbacks without a request on the stack is fragile whatever happens to be listening, and it is what made the failure invisible. --- quant.module | 17 ++++++++++---- src/EventSubscriber/DomainGuardSubscriber.php | 16 ++++++++----- tests/src/Unit/DomainGuardSubscriberTest.php | 23 ++++++++++++++++--- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/quant.module b/quant.module index a5890cf5..c84cc93f 100644 --- a/quant.module +++ b/quant.module @@ -205,11 +205,18 @@ function quant_shutdown(array $context = []) { * Runs a seed callback with a request on the stack. * * Drupal pops the request once the response has been sent, so by the time - * these callbacks run \Drupal::request() is NULL. Anything that renders the - * entity or replaces a token then fails: token info asks the request for its - * base path and fatals on NULL. The error surfaces only in the PHP log, - * because the shutdown handler cannot reach the logger, so the page silently - * never reaches Quant. + * these callbacks run \Drupal::request() is NULL. Anything that then asks the + * request for something fails. + * + * The known case is the contrib token module, which quant_search depends on. + * It collects token info during a replacement, and the site:base-path token + * describes itself by calling \Drupal::request()->getBasePath(). The Info + * metadata plugin replaces [user:name] by default, so on a site with + * quant_search installed every entity save hit this. Core's token service + * alone does not, which is why sites without quant_search were unaffected. + * + * The error surfaces only in the PHP log, because the shutdown handler cannot + * reach the logger, so the page silently never reaches Quant. * * The globals still describe the request that triggered the save, including * its host, so the rebuilt request also keeps the correct domain in scope. diff --git a/src/EventSubscriber/DomainGuardSubscriber.php b/src/EventSubscriber/DomainGuardSubscriber.php index 0d704d86..7ffb9bab 100644 --- a/src/EventSubscriber/DomainGuardSubscriber.php +++ b/src/EventSubscriber/DomainGuardSubscriber.php @@ -21,9 +21,12 @@ * Host header. Publishing on a guess is worse than not publishing, so this * stops the push and says why. * - * This applies to the command line too. A --uri that names no configured - * domain, or a cron run given no --uri at all, falls back the same way and - * publishes an entire site into the default domain's project. + * This only engages where more than one domain is configured, because that is + * the only arrangement in which the fallback can reach a different site's + * project. A site with one domain, or none, has a single destination and + * publishes as it always has, whether or not a --uri was given. That matters + * on the command line especially, where plenty of sites run cron and seeds + * without one. * * @ingroup quant */ @@ -102,9 +105,10 @@ protected function hostIsUnknown(&$host = NULL) : bool { $host = $request->getHttpHost(); $storage = \Drupal::entityTypeManager()->getStorage('domain'); - // No domains configured at all means the site is not using per-domain - // projects, so there is nothing to get wrong. - if (empty($storage->loadMultiple())) { + // With a single domain there is only one project to publish to, so the + // fallback cannot send content anywhere unexpected. Only a genuine + // multi-domain site can lose a page to another site's project. + if (count($storage->loadMultiple()) < 2) { return FALSE; } diff --git a/tests/src/Unit/DomainGuardSubscriberTest.php b/tests/src/Unit/DomainGuardSubscriberTest.php index edf89c73..2cdbf313 100644 --- a/tests/src/Unit/DomainGuardSubscriberTest.php +++ b/tests/src/Unit/DomainGuardSubscriberTest.php @@ -129,6 +129,23 @@ public function testPublishesWhenNoDomainsConfigured() { $this->assertFalse($event->isPropagationStopped()); } + /** + * A single-domain site publishes even when the host does not resolve. + * + * There is only one project to reach, so the fallback cannot misdirect + * anything. Plenty of such sites run cron and seeds without a --uri, and + * blocking those would stop publishing for no safety gain. + * + * @covers ::hostIsUnknown + */ + public function testPublishesOnSingleDomainSiteWithUnknownHost() { + $event = $this->event(); + $this->subscriber(TRUE, ['clienta' => 'x'], 'clienta.example', 'unregistered.example') + ->onOutput($event); + + $this->assertFalse($event->isPropagationStopped()); + } + /** * A host that resolves to a domain publishes as normal. * @@ -136,7 +153,7 @@ public function testPublishesWhenNoDomainsConfigured() { */ public function testPublishesWhenHostResolves() { $event = $this->event(); - $this->subscriber(TRUE, ['clientb' => 'x'], 'clientb.example', 'clientb.example') + $this->subscriber(TRUE, ['clienta' => 'x', 'clientb' => 'y'], 'clientb.example', 'clientb.example') ->onOutput($event); $this->assertFalse($event->isPropagationStopped()); @@ -153,7 +170,7 @@ public function testPublishesWhenHostResolves() { */ public function testStopsWhenHostHasNoDomain() { $event = $this->event(); - $this->subscriber(TRUE, ['clienta' => 'x'], 'clienta.example', 'unregistered.example') + $this->subscriber(TRUE, ['clienta' => 'x', 'clientb' => 'y'], 'clienta.example', 'unregistered.example') ->onOutput($event); $this->assertTrue($event->isPropagationStopped()); @@ -166,7 +183,7 @@ public function testStopsWhenHostHasNoDomain() { */ public function testPortMismatchStopsPush() { $event = $this->event(); - $this->subscriber(TRUE, ['clientb' => 'x'], 'clientb.example:8080', 'clientb.example') + $this->subscriber(TRUE, ['clienta' => 'x', 'clientb' => 'y'], 'clientb.example:8080', 'clientb.example') ->onOutput($event); $this->assertTrue($event->isPropagationStopped()); From af6a1381c89fc58fad583e8524db3630aa00df93 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 15:23:49 -0700 Subject: [PATCH 06/15] Correct contrib dependency project prefixes. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dependencies are declared as project:module. quant_search named all three of its dependencies under the drupal project, which claims quant, quant_api and token all ship with core. token in particular is contrib, and getting its project wrong means Drupal cannot point an administrator at what to install when it is missing. quant_api named quant the same way. The other submodules already do this correctly, with webform:webform, purge:purge and tome:tome_static. Note for later: quant depends on quant_api while quant_api depends on quant, and the code matches — the API subscriber uses QuantEvent, Utility and QuantQueueFactory from quant. Drupal tolerates the cycle today. Left alone here because breaking it means moving shared classes, which is not this branch's business. --- modules/quant_api/quant_api.info.yml | 2 +- modules/quant_search/quant_search.info.yml | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/quant_api/quant_api.info.yml b/modules/quant_api/quant_api.info.yml index 0afdadb7..2377cb2b 100644 --- a/modules/quant_api/quant_api.info.yml +++ b/modules/quant_api/quant_api.info.yml @@ -6,4 +6,4 @@ type: module core_version_requirement: ^11 configure: quant_api.settings_form dependencies: - - drupal:quant + - quant:quant diff --git a/modules/quant_search/quant_search.info.yml b/modules/quant_search/quant_search.info.yml index e6443325..1d9b123e 100644 --- a/modules/quant_search/quant_search.info.yml +++ b/modules/quant_search/quant_search.info.yml @@ -6,6 +6,6 @@ type: module core_version_requirement: ^11 configure: quant_search.main dependencies: - - drupal:quant - - drupal:quant_api - - drupal:token + - quant:quant + - quant:quant_api + - token:token From 3214201208b136fd00544efc29163d9b2c027622 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 15:34:12 -0700 Subject: [PATCH 07/15] Declare the contrib modules the submodules need in composer.json. The published metadata for 2.0.0 requires drupal/quantcdn, drupal/core and drupal/quant-quant_api, and nothing else. drupal/token is absent, because quant_search declared it under the drupal project and the facade read that as core and dropped it. drupal/purge, drupal/webform and drupal/tome are absent too, from which it is clear the facade does not carry submodule dependencies up into the project requirement at all. Correcting the prefixes was necessary but was never going to be enough on its own. These are listed as suggestions rather than requirements. Each belongs to one optional submodule, and a site using none of them should not be made to install four contrib projects. With the prefixes now correct, Drupal names the right project when it refuses to enable a submodule whose dependency is missing, so the path from error to fix is clear. No drupal/core requirement is declared, so the facade keeps deriving it from core_version_requirement. That leaves one source of truth and stops the 1.x and 2.x branches drifting apart. Worth checking the generated metadata on the next dev release: adding a composer.json changes what the facade contributes. --- composer.json | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 composer.json diff --git a/composer.json b/composer.json new file mode 100644 index 00000000..39578498 --- /dev/null +++ b/composer.json @@ -0,0 +1,18 @@ +{ + "name": "drupal/quant", + "description": "Static site generator for Drupal, integrating with the QuantCDN static edge.", + "type": "drupal-module", + "license": "GPL-2.0-or-later", + "homepage": "https://www.quantcdn.io", + "support": { + "issues": "https://github.com/quantcdn/drupal/issues", + "docs": "https://docs.quantcdn.io/docs/integrations/drupal", + "source": "https://github.com/quantcdn/drupal" + }, + "suggest": { + "drupal/token": "Required by Quant Search (quant_search).", + "drupal/purge": "Required by Quant Purger (quant_purger).", + "drupal/webform": "Required by Quant Webform (quant_webform).", + "drupal/tome": "Required by Quant Tome (quant_tome)." + } +} From bcfc9ae71e3e10f1599c61c4b307be786a5be809 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 15:47:18 -0700 Subject: [PATCH 08/15] Make the purger update survive duplicate rows, and fix two static findings. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The update that adds the domain column also adds a unique key on (url, domain). The table never had one, so a site can hold several rows for the same URL, and the key was refused with an integrity constraint — the update failed part applied, mid deploy. Reproduced with three rows for one URL. Duplicates are now merged before the key is added, combining their tags rather than keeping one row's and discarding the rest: a tag dropped here is a page that stops being purged when its content changes. The work runs in phases over batches, because this table grows with traffic. Verified against six rows collapsing to three with every tag preserved. Config::get() takes one argument. Six calls passed a default as a second, which PHP discards silently, so the default never applied and the caller got NULL. Today the config/install defaults hide it, but any new setting whose default is not falsy would have been silently wrong. Rewritten with ??. QuantSearchPageForm built its facet rows in a foreach and then attached the "Add facet" button at $i, which is undefined when a page has no facets yet. Checked but deliberately left alone: $item->data in quant_process_queue is guarded by an earlier falsy check, and QuantSearchPageForm::save() declares no return type of its own so it cannot fatal on the missing return. --- modules/quant_api/src/Form/SettingsForm.php | 4 +- .../quant_cron/src/Form/CronSettingsForm.php | 2 +- modules/quant_purger/quant_purger.install | 105 +++++++++++++++--- .../src/Form/QuantSearchPageForm.php | 4 + src/Form/ConfigForm.php | 2 +- src/Plugin/Quant/Metadata/ProxyOverride.php | 2 +- src/Plugin/QueueItem/RouteItem.php | 2 +- 7 files changed, 101 insertions(+), 20 deletions(-) diff --git a/modules/quant_api/src/Form/SettingsForm.php b/modules/quant_api/src/Form/SettingsForm.php index c932eb6e..0c95c3ce 100644 --- a/modules/quant_api/src/Form/SettingsForm.php +++ b/modules/quant_api/src/Form/SettingsForm.php @@ -75,7 +75,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#type' => 'textfield', '#title' => $this->t('API Endpoint'), '#description' => $this->t('The fully-qualified domain name for the API endpoint, e.g. https://api.quantcdn.io, shown on the Integrations page. Update via drush or settings.php if necessary.'), - '#default_value' => $config->get('api_endpoint', 'https://api.quantcdn.io'), + '#default_value' => $config->get('api_endpoint') ?? 'https://api.quantcdn.io', '#required' => TRUE, '#disabled' => TRUE, ]; @@ -109,7 +109,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#type' => 'checkbox', '#title' => $this->t('Disable TLS verification'), '#description' => $this->t('You can optionally disable TLS verification for all Quant API requests. This is not recommended, but may be necessary in some configurations. For example, old web servers may have issues validating modern TSL/SSL certificates.'), - '#default_value' => $config->get('api_tls_disabled', FALSE), + '#default_value' => $config->get('api_tls_disabled') ?? FALSE, ]; // API values might be overridden in the settings file. diff --git a/modules/quant_cron/src/Form/CronSettingsForm.php b/modules/quant_cron/src/Form/CronSettingsForm.php index e374796b..db344a7d 100644 --- a/modules/quant_cron/src/Form/CronSettingsForm.php +++ b/modules/quant_cron/src/Form/CronSettingsForm.php @@ -53,7 +53,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#type' => 'checkbox', '#title' => $this->t('Nodes'), '#description' => $this->t('Exports the latest revision of each node.'), - '#default_value' => !empty($config->get('entity_node', '')), + '#default_value' => !empty($config->get('entity_node')), ]; // Seed by language. diff --git a/modules/quant_purger/quant_purger.install b/modules/quant_purger/quant_purger.install index fcfa1acf..95bdba2e 100644 --- a/modules/quant_purger/quant_purger.install +++ b/modules/quant_purger/quant_purger.install @@ -114,26 +114,103 @@ function quant_purger_update_9102(&$sandbox) { /** * Record the domain each tracked URL was served from. + * + * The table never had a unique key, so a site can hold several rows for the + * same URL. Adding the key on top of those fails with an integrity + * constraint and leaves the update half applied, so the duplicates are + * merged first. Their tags are combined rather than discarded: a tag lost + * here is a page that stops being purged when its content changes. + * + * Done in phases over batches, because this table grows with traffic and can + * be large. */ function quant_purger_update_9103(&$sandbox) { - $schema = \Drupal::database()->schema(); - - if (!$schema->fieldExists('purge_queuer_quant', 'domain')) { - $schema->addField('purge_queuer_quant', 'domain', [ - 'description' => 'The domain the URL was requested on, empty on single-domain sites', - 'type' => 'varchar', - 'length' => 128, - 'not null' => TRUE, - 'default' => '', - ]); + $database = \Drupal::database(); + $schema = $database->schema(); + + if (!isset($sandbox['phase'])) { + $sandbox['phase'] = 'add_column'; + $sandbox['merged'] = 0; + } + + if ($sandbox['phase'] === 'add_column') { + if (!$schema->fieldExists('purge_queuer_quant', 'domain')) { + $schema->addField('purge_queuer_quant', 'domain', [ + 'description' => 'The domain the URL was requested on, empty on single-domain sites', + 'type' => 'varchar', + 'length' => 128, + 'not null' => TRUE, + 'default' => '', + ]); + } + + // Existing rows keep an empty domain, which is how a single-domain site + // continues to behave. A multi-domain site repopulates as traffic + // arrives. + $sandbox['phase'] = 'dedupe'; + $sandbox['#finished'] = 0; + return t('Added the domain column to the Quant purger traffic registry.'); + } + + if ($sandbox['phase'] === 'dedupe') { + $duplicates = $database->select('purge_queuer_quant', 'q') + ->fields('q', ['url', 'domain']) + ->groupBy('q.url') + ->groupBy('q.domain') + ->havingCondition('total', 1, '>') + ->range(0, 50); + $duplicates->addExpression('COUNT(*)', 'total'); + $rows = $duplicates->execute()->fetchAll(); + + if (empty($rows)) { + $sandbox['phase'] = 'add_key'; + $sandbox['#finished'] = 0; + return NULL; + } + + foreach ($rows as $row) { + $records = $database->select('purge_queuer_quant', 'q') + ->fields('q', ['urlid', 'tags']) + ->condition('url', $row->url) + ->condition('domain', $row->domain) + ->orderBy('urlid') + ->execute() + ->fetchAll(); + + $keep = array_shift($records); + $tags = []; + + foreach (array_merge([$keep], $records) as $record) { + foreach (explode(';', (string) $record->tags) as $tag) { + if ($tag !== '') { + $tags[$tag] = $tag; + } + } + } + + $database->update('purge_queuer_quant') + ->fields(['tags' => ';' . implode(';', $tags) . ';']) + ->condition('urlid', $keep->urlid) + ->execute(); + + $database->delete('purge_queuer_quant') + ->condition('urlid', array_column($records, 'urlid'), 'IN') + ->execute(); + + $sandbox['merged'] += count($records); + } + + $sandbox['#finished'] = 0; + return NULL; } - // Existing rows keep an empty domain, which is how a single-domain site - // continues to behave. A multi-domain site repopulates the registry as - // traffic arrives. if (!$schema->indexExists('purge_queuer_quant', 'url_domain')) { $schema->addUniqueKey('purge_queuer_quant', 'url_domain', ['url', 'domain']); } - return t('Added the domain column to the Quant purger traffic registry.'); + $sandbox['#finished'] = 1; + + return t('Quant purger traffic registry is now per domain. Merged @count duplicate rows.', [ + '@count' => $sandbox['merged'], + ]); } diff --git a/modules/quant_search/src/Form/QuantSearchPageForm.php b/modules/quant_search/src/Form/QuantSearchPageForm.php index b2d8c47f..3649a995 100644 --- a/modules/quant_search/src/Form/QuantSearchPageForm.php +++ b/modules/quant_search/src/Form/QuantSearchPageForm.php @@ -261,6 +261,10 @@ public function form(array $form, FormStateInterface $form_state) { $existingFacets[] = []; } + // The "Add facet" button below attaches to the last row, so this has to + // hold a value even when the page has no facets configured yet. + $i = 0; + // Configuration fields for all the facets. foreach ($existingFacets as $i => $facet) { diff --git a/src/Form/ConfigForm.php b/src/Form/ConfigForm.php index 5dd015a7..dea31954 100644 --- a/src/Form/ConfigForm.php +++ b/src/Form/ConfigForm.php @@ -54,7 +54,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#type' => 'checkbox', '#title' => $this->t('Track content change'), '#description' => $this->t('Automatically push content changes to Quant (recommended).'), - '#default_value' => $config->get('quant_enabled', TRUE), + '#default_value' => $config->get('quant_enabled') ?? TRUE, ]; $form['tracking_fieldset'] = [ diff --git a/src/Plugin/Quant/Metadata/ProxyOverride.php b/src/Plugin/Quant/Metadata/ProxyOverride.php index 48c1fad3..420b6191 100644 --- a/src/Plugin/Quant/Metadata/ProxyOverride.php +++ b/src/Plugin/Quant/Metadata/ProxyOverride.php @@ -53,7 +53,7 @@ public function build(EntityInterface $entity) : array { // Proxies are created manually and usually not something you want to // replace. This is a globally configurable to allow override just in case. $config = \Drupal::config('quant.settings'); - $proxy_override = boolval($config->get('proxy_override', TRUE)); + $proxy_override = boolval($config->get('proxy_override') ?? TRUE); return ['proxy_override' => $proxy_override]; } diff --git a/src/Plugin/QueueItem/RouteItem.php b/src/Plugin/QueueItem/RouteItem.php index 85cd89c3..2d9ac334 100644 --- a/src/Plugin/QueueItem/RouteItem.php +++ b/src/Plugin/QueueItem/RouteItem.php @@ -95,7 +95,7 @@ public function send() { [$markup, $content_type] = $response; $config = \Drupal::config('quant.settings'); - $proxy_override = boolval($config->get('proxy_override', FALSE)); + $proxy_override = boolval($config->get('proxy_override') ?? FALSE); $meta = [ 'info' => [ From 15cb9b85d3ce99e540cd09ec18cb1ad479618933 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 15:59:29 -0700 Subject: [PATCH 09/15] Stamp tome's queue items where they are created. quant_tome queues its work in checkRequiredFiles(), which runs as a batch operation and may be handed to a separate process. That process had not negotiated a domain, so items were stamped with the base project while the sender resolved the real one, and every item was refused by the worker's project check. A deploy published nothing. Reproduced with tome installed: the run reported "Skipped [route_item] /rss.xml: queued for project SINGLE-SITE but this worker publishes to PROJECT-CLIENT-A" for each item. The domain is now resolved where the items are built, not only where they are sent. Verified with tome and webform installed: a single-site deploy publishes 7 pages, and a per-domain deploy sends every request to that domain's project and nothing to the other. --- modules/quant_tome/src/QuantTomeBatch.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/modules/quant_tome/src/QuantTomeBatch.php b/modules/quant_tome/src/QuantTomeBatch.php index cde4e633..7f9d4518 100644 --- a/modules/quant_tome/src/QuantTomeBatch.php +++ b/modules/quant_tome/src/QuantTomeBatch.php @@ -143,6 +143,12 @@ public function getHashes(array $files, &$context) { * The batch context. */ public function checkRequiredFiles(&$context) { + // Items are stamped with their destination project as they are created, + // and this runs as a batch operation, which drush may hand to a separate + // process that never negotiated a domain. Resolve it here, where the + // items are built, rather than only where they are sent. + CliDomainContext::initialize(); + $file_hashes = $context['results']['files']; $queue = $this->queueFactory->get('quant_seed_worker'); From c44a958076fded0e5ab1c4a6ec13078458e9c795 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 16:30:50 -0700 Subject: [PATCH 10/15] Fix malformed multilingual redirects, and guard redirects like content. Utility::getPathPrefix() already carries its leading slash, and returns a bare slash for a language that has no prefix. handleInternalPathRedirects() prepended another, so every translated page published a redirect at //fr/node/1 instead of /fr/node/1. The truthiness check was wrong for the same reason: a bare slash is truthy, so a default-language page with an alias produced //node/1 too. Verified on a trilingual site: content reaches /fr/page-une and /de/seite-eins, the prefixed internal redirect is /fr/node/1, and no push carries a double slash. The domain guard covered content but not redirects, which are written to the same project by the same route. A run on an unrecognised host was therefore refused for pages and allowed for redirects, quietly rewriting another client's redirect map. It now guards both. That gap survived because the regression matrix counted any request carrying a url as a content push, and redirect payloads carry one. It now separates the two endpoints, and asserts routing rather than exact counts: everything reached the expected project and nothing reached another. Exact counts moved as soon as the fixture gained languages, which is precisely when the assertions should have kept working. --- src/EventSubscriber/DomainGuardSubscriber.php | 21 +++++++---- src/Seed.php | 13 ++++--- tests/src/Unit/DomainGuardSubscriberTest.php | 35 +++++++++++++++++++ 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/EventSubscriber/DomainGuardSubscriber.php b/src/EventSubscriber/DomainGuardSubscriber.php index 7ffb9bab..1b96eaa2 100644 --- a/src/EventSubscriber/DomainGuardSubscriber.php +++ b/src/EventSubscriber/DomainGuardSubscriber.php @@ -3,6 +3,7 @@ namespace Drupal\quant\EventSubscriber; use Drupal\quant\Event\QuantEvent; +use Drupal\quant\Event\QuantRedirectEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\RequestStack; @@ -53,24 +54,30 @@ public function __construct(RequestStack $request_stack) { * {@inheritdoc} */ public static function getSubscribedEvents(): array { - // Ahead of quant_search (1) and the API publisher (0), so that stopping - // propagation prevents the push entirely. - return [QuantEvent::OUTPUT => ['onOutput', 100]]; + return [ + // Ahead of quant_search (1) and the API publisher (0), so that stopping + // propagation prevents the push entirely. + QuantEvent::OUTPUT => ['onOutput', 100], + // Redirects reach the same project by the same route, so guarding only + // content would still let a misdirected run rewrite another client's + // redirect map. The publisher listens at -999. + QuantRedirectEvent::UPDATE => ['onOutput', 100], + ]; } /** * Stops the push when the serving host has no domain record. * - * @param \Drupal\quant\Event\QuantEvent $event - * The event. + * @param \Drupal\quant\Event\QuantEvent|\Drupal\quant\Event\QuantRedirectEvent $event + * The content or redirect event. */ - public function onOutput(QuantEvent $event) { + public function onOutput($event) { if (!$this->hostIsUnknown($host)) { return; } \Drupal::logger('quant')->error('Refused to publish @path: the host @host matches no domain, so the Domain module fell back to the default and this content would be published to project @project. Add a domain for @host, or correct the Host header reaching Drupal.', [ - '@path' => $event->getLocation(), + '@path' => $event instanceof QuantEvent ? $event->getLocation() : $event->getSourceUrl(), '@host' => $host, '@project' => \Drupal::config('quant_api.settings')->get('api_project') ?: 'unknown', ]); diff --git a/src/Seed.php b/src/Seed.php index d8e72a7f..4190656a 100644 --- a/src/Seed.php +++ b/src/Seed.php @@ -583,12 +583,17 @@ public static function handleInternalPathRedirects($entity, $langcode, $url) { $defaultUrl = $languageUrl; } + // getPathPrefix() already carries its leading slash, and returns a bare + // slash for a language that has no prefix. Concatenating another one + // published redirects at //fr/node/1 for every translation. + $hasPrefix = $prefix !== '' && $prefix !== '/'; + // Only create redirects if the content has an alias. if ($internalPath != $url) { \Drupal::service('event_dispatcher')->dispatch(new QuantRedirectEvent($internalPath, $defaultUrl, 301), QuantRedirectEvent::UPDATE); - if ($prefix) { + if ($hasPrefix) { // Handle redirects with path prefix too. - \Drupal::service('event_dispatcher')->dispatch(new QuantRedirectEvent("/{$prefix}{$internalPath}", $languageUrl, 301), QuantRedirectEvent::UPDATE); + \Drupal::service('event_dispatcher')->dispatch(new QuantRedirectEvent("{$prefix}{$internalPath}", $languageUrl, 301), QuantRedirectEvent::UPDATE); } } @@ -596,9 +601,9 @@ public static function handleInternalPathRedirects($entity, $langcode, $url) { if (!$defaultPublished) { Utility::unpublishUrl($internalPath, 'Unpublished internal path'); } - if (!$published && $prefix) { + if (!$published && $hasPrefix) { // Handle redirects with path prefix too. - Utility::unpublishUrl("/{$prefix}{$internalPath}", 'Unpublished internal path'); + Utility::unpublishUrl("{$prefix}{$internalPath}", 'Unpublished internal path'); } } diff --git a/tests/src/Unit/DomainGuardSubscriberTest.php b/tests/src/Unit/DomainGuardSubscriberTest.php index 2cdbf313..dde6a96e 100644 --- a/tests/src/Unit/DomainGuardSubscriberTest.php +++ b/tests/src/Unit/DomainGuardSubscriberTest.php @@ -11,6 +11,7 @@ use Drupal\Core\Logger\LoggerChannelFactoryInterface; use Drupal\Core\Logger\LoggerChannelInterface; use Drupal\quant\Event\QuantEvent; +use Drupal\quant\Event\QuantRedirectEvent; use Drupal\quant\EventSubscriber\DomainGuardSubscriber; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; @@ -92,6 +93,40 @@ protected function event() : QuantEvent { return new QuantEvent('', '/node/1', [], NULL); } + /** + * Redirects are guarded as well as content. + * + * A redirect is written to the same project by the same route, so guarding + * only content would still let a misdirected run rewrite another client's + * redirect map. + * + * @covers ::getSubscribedEvents + * @covers ::onOutput + */ + public function testRedirectsAreGuarded() { + $events = DomainGuardSubscriber::getSubscribedEvents(); + $this->assertArrayHasKey(QuantRedirectEvent::UPDATE, $events); + + $event = new QuantRedirectEvent('/old', '/new', 301); + $this->subscriber(TRUE, ['clienta' => 'x', 'clientb' => 'y'], 'clienta.example', 'unregistered.example') + ->onOutput($event); + + $this->assertTrue($event->isPropagationStopped()); + } + + /** + * A redirect on a recognised host is left alone. + * + * @covers ::onOutput + */ + public function testRedirectPassesOnKnownHost() { + $event = new QuantRedirectEvent('/old', '/new', 301); + $this->subscriber(TRUE, ['clienta' => 'x', 'clientb' => 'y'], 'clientb.example', 'clientb.example') + ->onOutput($event); + + $this->assertFalse($event->isPropagationStopped()); + } + /** * The guard runs ahead of the search and publish subscribers. * From 9d442c3698128d0962907032abfd98cd6dcf1940 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 16:41:19 -0700 Subject: [PATCH 11/15] Guard deletes, and stop malformed paths reaching the API. Deleting content was not guarded. unpublishUrl() dispatches QuantEvent::UNPUBLISH, a different event from the one the guard watched, so a delete on an unrecognised host withdrew the matching URL from whichever project the fallback landed on. That is the worst of the three: publishing to the wrong project adds a page, but unpublishing takes a live one down. Worse, the domain was only ever negotiated by Quant's own drush commands. Deleting a node through drush php:eval, a migration, or any other command resolved the base project even with --uri set. Verified: a delete as clienta withdrew /node/8 and /fr/node/8 from the base project rather than the domain's. Negotiation now happens in the guard subscriber, which every publish, redirect and unpublish passes through, so it no longer depends on which entry point started the work. The call is cached per process. Separately, nothing should ever be published at //fr/node/1. The handleInternalPathRedirects fix removed the cause found so far, but paths are assembled from prefixes, base paths and aliases all over the module, and any of them can be empty. Utility::normalizePath() collapses repeated slashes and is applied where routes enter the queue and again at the API boundary for content, redirects and unpublishes. Normalising before the self-redirect check also means a malformed source is recognised as equal to its destination instead of being published as a redirect to itself. quant_cron had one more producer, filtering out the empty default prefix. The query string is left alone: an oEmbed route carries a whole URL in one, and those slashes are not ours to collapse. Regression matrix now covers deletion in both shapes and multilingual: 25 cases, all passing. 76 unit and kernel tests. --- .../src/EventSubscriber/QuantApi.php | 12 +++- modules/quant_cron/quant_cron.module | 6 +- src/EventSubscriber/DomainGuardSubscriber.php | 14 +++++ src/Plugin/QueueItem/RouteItem.php | 3 +- src/Utility.php | 39 +++++++++++-- tests/src/Unit/UtilityNormalizePathTest.php | 57 +++++++++++++++++++ 6 files changed, 121 insertions(+), 10 deletions(-) create mode 100644 tests/src/Unit/UtilityNormalizePathTest.php diff --git a/modules/quant_api/src/EventSubscriber/QuantApi.php b/modules/quant_api/src/EventSubscriber/QuantApi.php index f4f084a4..c4090424 100644 --- a/modules/quant_api/src/EventSubscriber/QuantApi.php +++ b/modules/quant_api/src/EventSubscriber/QuantApi.php @@ -79,8 +79,11 @@ public static function getSubscribedEvents(): array { * The redirect event. */ public function onRedirect(QuantRedirectEvent $event) { - $source = $event->getSourceUrl(); - $dest = $event->getDestinationUrl(); + // Normalise before the self-redirect check, so that a malformed source + // like //fr/node/1 is recognised as the same path as its destination + // rather than published as a redirect to itself. + $source = Utility::normalizePath($event->getSourceUrl()); + $dest = Utility::normalizePath($event->getDestinationUrl()); $statusCode = $event->getStatusCode(); if ($source == $dest) { @@ -114,7 +117,10 @@ public function onRedirect(QuantRedirectEvent $event) { public function onOutput(QuantEvent $event) { $config = \Drupal::config('quant.settings'); - $path = $event->getLocation(); + // Last line of defence: a path assembled from an empty prefix or base + // must not reach the API as //fr/node/1, which would publish a duplicate + // resource alongside the real one. + $path = Utility::normalizePath($event->getLocation()); $content = $event->getContents(); $meta = $event->getMetadata(); diff --git a/modules/quant_cron/quant_cron.module b/modules/quant_cron/quant_cron.module index 5b5bb965..0e65f701 100644 --- a/modules/quant_cron/quant_cron.module +++ b/modules/quant_cron/quant_cron.module @@ -284,9 +284,11 @@ function quant_cron_get_views_routes() { $paths[] = $path; - // Language negotiation may also provide path prefixes. + // Language negotiation may also provide path prefixes. The default + // language usually has an empty one, which would otherwise produce + // //path. if ($prefixes = \Drupal::config('language.negotiation')->get('url.prefixes')) { - foreach ($prefixes as $prefix) { + foreach (array_filter($prefixes) as $prefix) { $paths[] = "/{$prefix}/{$path}"; } } diff --git a/src/EventSubscriber/DomainGuardSubscriber.php b/src/EventSubscriber/DomainGuardSubscriber.php index 1b96eaa2..64f2414f 100644 --- a/src/EventSubscriber/DomainGuardSubscriber.php +++ b/src/EventSubscriber/DomainGuardSubscriber.php @@ -2,6 +2,7 @@ namespace Drupal\quant\EventSubscriber; +use Drupal\quant\CliDomainContext; use Drupal\quant\Event\QuantEvent; use Drupal\quant\Event\QuantRedirectEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -62,6 +63,11 @@ public static function getSubscribedEvents(): array { // content would still let a misdirected run rewrite another client's // redirect map. The publisher listens at -999. QuantRedirectEvent::UPDATE => ['onOutput', 100], + // Unpublishing is the one that cannot be walked back. Deleting a node + // on an unrecognised host would withdraw the matching URL from + // whichever project the fallback landed on, taking down a live page + // belonging to another client. + QuantEvent::UNPUBLISH => ['onOutput', 100], ]; } @@ -72,6 +78,14 @@ public static function getSubscribedEvents(): array { * The content or redirect event. */ public function onOutput($event) { + // Every publish, redirect and unpublish passes through here, which makes + // it the one place guaranteed to run no matter what triggered the work. + // Quant's own drush commands negotiate the domain themselves, but a node + // deleted by drush php:eval, a migration, or any other command does not + // reach them, and would resolve the base project instead of the domain's. + // The call is cached per process, so this costs nothing after the first. + CliDomainContext::initialize(); + if (!$this->hostIsUnknown($host)) { return; } diff --git a/src/Plugin/QueueItem/RouteItem.php b/src/Plugin/QueueItem/RouteItem.php index 2d9ac334..f035b599 100644 --- a/src/Plugin/QueueItem/RouteItem.php +++ b/src/Plugin/QueueItem/RouteItem.php @@ -4,6 +4,7 @@ use Drupal\quant\Event\QuantEvent; use Drupal\quant\Seed; +use Drupal\quant\Utility; /** * A Quant queue item for a redirect. @@ -49,7 +50,7 @@ public function __construct(array $data = []) { if (substr($route, 0, 1) != '/') { $route = "/{$route}"; } - $route = trim($route); + $route = Utility::normalizePath(trim($route)); $this->route = $route; $this->uri = $data['uri'] ?? strtok($route, '?'); diff --git a/src/Utility.php b/src/Utility.php index 3872c436..4d161c55 100644 --- a/src/Utility.php +++ b/src/Utility.php @@ -305,14 +305,43 @@ public static function getPageInfo(?array $urls = NULL) : string { } /** - * Unpublish the given URL and optionally log a message. + * Collapses repeated slashes in a path. + * + * Paths are assembled from language prefixes, base paths and aliases, and + * any of those can be empty, which leaves a stray slash behind. A path like + * //fr/node/1 is published as a distinct resource from /fr/node/1, so the + * project accumulates duplicates nobody asked for. Callers should build the + * path correctly; this is the backstop that keeps a malformed one off the + * wire. + * + * @param string $path + * The path, which may carry a query string. + * + * @return string + * The path with repeated slashes collapsed. + */ + public static function normalizePath(string $path) : string { + // Only the path can pick up stray slashes. A query string may legitimately + // contain them, in an oEmbed url for instance, so it is left alone. + $parts = explode('?', $path, 2); + $parts[0] = preg_replace('#/{2,}#', '/', $parts[0]); + + if ($parts[0] === '') { + $parts[0] = '/'; + } + + return implode('?', $parts); + } + + /** + * Unpublishes a url from Quant. * * @param string $url - * The URL to unpublish. + * The url to unpublish. * @param string $message - * The message to log. + * Message to log. * @param bool $log - * Whether or not to log the message. + * Whether to log the action. */ public static function unpublishUrl(string $url, string $message = '', bool $log = TRUE) : void { if (!trim($url)) { @@ -321,6 +350,8 @@ public static function unpublishUrl(string $url, string $message = '', bool $log return; } + $url = self::normalizePath($url); + \Drupal::service('event_dispatcher')->dispatch(new QuantEvent('', $url, [], NULL), QuantEvent::UNPUBLISH); if ($log) { diff --git a/tests/src/Unit/UtilityNormalizePathTest.php b/tests/src/Unit/UtilityNormalizePathTest.php new file mode 100644 index 00000000..0c50bc88 --- /dev/null +++ b/tests/src/Unit/UtilityNormalizePathTest.php @@ -0,0 +1,57 @@ + ['/fr/node/1', '/fr/node/1'], + 'doubled prefix slash' => ['//fr/node/1', '/fr/node/1'], + 'doubled root slash' => ['//node/1', '/node/1'], + 'tripled' => ['///node/1', '/node/1'], + 'interior double' => ['/fr//node/1', '/fr/node/1'], + 'trailing double' => ['/fr/node//', '/fr/node/'], + 'root stays root' => ['/', '/'], + 'empty becomes root' => ['', '/'], + 'query string kept' => ['//fr/search?page=2', '/fr/search?page=2'], + // An oEmbed route carries a whole URL in its query string, and the + // slashes in that URL are not ours to touch. + 'slashes in query untouched' => [ + '//media/oembed?url=https://example.com/a//b', + '/media/oembed?url=https://example.com/a//b', + ], + ]; + } + + /** + * Repeated slashes collapse, without disturbing the query string. + * + * @dataProvider pathProvider + * @covers ::normalizePath + */ + public function testNormalizePath(string $input, string $expected) { + $this->assertEquals($expected, Utility::normalizePath($input)); + } + +} From aaad0ef7a0ee78ecb1cb2d6d72d28e21612a01f7 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 17:48:46 -0700 Subject: [PATCH 12/15] Address review: absolute urls, requeue, clear() scope, host trust. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit normalizePath() collapsed the // in a scheme, so https://example.com became https:/example.com. Reachable: the redirect module supports external destinations, Seed builds them with Url::toString(), and this branch put that value through the normaliser. Every redirect pointing off site would have published broken. Reproduced with a real redirect entity. An absolute url is now returned untouched. A protocol-relative one is still collapsed, because it cannot be told apart from a malformed path by inspection and nothing here generates one; that is written down rather than left implicit. A mismatched queue item returned NULL, and a worker that returns normally has its item deleted. On a shared queue that meant the first domain's worker to claim consumed every other domain's work and it was never published. Traded a leak for silent loss. The worker now throws DelayedRequeueException, which drush and cron both honour, and the batch runner in quant_process_queue is taught to as well since it deleted unconditionally. Verified: 8 items queued for clientb, drained as clienta, all 8 requeued and none lost; drained again as clientb after the delay, all 8 published to its project and the queue empties. TrafficRegistry::clear() deleted every row while add() and remove() are domain scoped, so an administrator on one client's domain wiped the registry for all of them and each silently stopped purging. Now scoped to the active domain, which on a single-domain site is every row. The shutdown handler rebuilt its request with createFromGlobals(), taking $_SERVER['HTTP_HOST'] verbatim and bypassing trusted host checking — on the path where the host decides which project receives the work. The hooks now capture the live request while it is still on the stack and hand it over. Writing the test that asserts the guard covers every event QuantApi publishes on failed immediately: QuantFileEvent::OUTPUT was unguarded, so a misdirected run would have uploaded one client's files and media into another's project. That is the fourth event, found the way the review predicted a fourth would be. Now guarded, and the parity test will fail if a fifth appears. Also closes the coverage the review named: absolute urls in the normalizePath provider, UNPUBLISH asserted rather than merely subscribed, QuantPurger::getProjectForDomain(), and the update 9103 dedupe including that no tag is lost when rows merge. Import ordering and the CliDomainContext caching comment tidied. 91 unit and kernel tests, 39 regression cases, phpcs clean. --- .../src/Form/ConfigurationForm.php | 2 +- modules/quant_purger/src/TrafficRegistry.php | 11 +- .../src/Kernel/PurgerUpdateDedupeTest.php | 160 ++++++++++++++++++ .../src/Kernel/QuantPurgerProjectTest.php | 109 ++++++++++++ quant.module | 50 +++++- src/CliDomainContext.php | 3 + src/EventSubscriber/DomainGuardSubscriber.php | 32 +++- src/Plugin/QueueWorker/QuantSeedWorker.php | 32 ++-- src/Utility.php | 19 ++- .../QuantSeedWorkerProjectGuardTest.php | 32 +++- tests/src/Unit/DomainGuardSubscriberTest.php | 54 ++++++ tests/src/Unit/UtilityNormalizePathTest.php | 20 +++ 12 files changed, 502 insertions(+), 22 deletions(-) create mode 100644 modules/quant_purger/tests/src/Kernel/PurgerUpdateDedupeTest.php create mode 100644 modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php diff --git a/modules/quant_purger/src/Form/ConfigurationForm.php b/modules/quant_purger/src/Form/ConfigurationForm.php index cde184a4..1229bb30 100644 --- a/modules/quant_purger/src/Form/ConfigurationForm.php +++ b/modules/quant_purger/src/Form/ConfigurationForm.php @@ -170,7 +170,7 @@ public function submitFormClear(array &$form, FormStateInterface $form_state) { if (!$form_state->getErrors()) { \Drupal::service('quant_purger.registry')->clear(); $status = 'status'; - $message = $this->t('Successfully cleared the traffic registry. All content must be re-seeded so the database reflects the configuration.'); + $message = $this->t('Successfully cleared the traffic registry for this domain. All content must be re-seeded so the database reflects the configuration.'); } else { $status = 'error'; diff --git a/modules/quant_purger/src/TrafficRegistry.php b/modules/quant_purger/src/TrafficRegistry.php index 1a5346cf..0cff09d9 100644 --- a/modules/quant_purger/src/TrafficRegistry.php +++ b/modules/quant_purger/src/TrafficRegistry.php @@ -93,7 +93,16 @@ public function remove($url) { * {@inheritdoc} */ public function clear() { - $this->connection->delete('purge_queuer_quant')->execute(); + $delete = $this->connection->delete('purge_queuer_quant'); + + // Scoped to the active domain, to match add() and remove(). An + // administrator working on one client's domain would otherwise wipe the + // registry for every other client, and each would silently stop purging + // until its content was re-seeded. On a single-domain site the active + // domain is the empty string, which is every row, so nothing changes. + $delete->condition('domain', $this->getActiveDomainId()); + + $delete->execute(); } /** diff --git a/modules/quant_purger/tests/src/Kernel/PurgerUpdateDedupeTest.php b/modules/quant_purger/tests/src/Kernel/PurgerUpdateDedupeTest.php new file mode 100644 index 00000000..6878162a --- /dev/null +++ b/modules/quant_purger/tests/src/Kernel/PurgerUpdateDedupeTest.php @@ -0,0 +1,160 @@ +installSchema('quant_purger', ['purge_queuer_quant']); + + // Return the table to its pre-update shape. + $schema = $this->container->get('database')->schema(); + if ($schema->indexExists('purge_queuer_quant', 'url_domain')) { + $schema->dropUniqueKey('purge_queuer_quant', 'url_domain'); + } + if ($schema->fieldExists('purge_queuer_quant', 'domain')) { + $schema->dropField('purge_queuer_quant', 'domain'); + } + + require_once $this->root . '/' . \Drupal::service('extension.list.module')->getPath('quant_purger') . '/quant_purger.install'; + } + + /** + * Writes a pre-update row. + */ + protected function seedRow(string $url, string $tags) : void { + $this->container->get('database')->insert('purge_queuer_quant') + ->fields(['url' => $url, 'tags' => $tags]) + ->execute(); + } + + /** + * Runs the update to completion, as the update system would. + * + * @return array + * The sandbox, carrying the merged count. + */ + protected function runUpdate() : array { + $sandbox = []; + $passes = 0; + + do { + quant_purger_update_9103($sandbox); + $passes++; + } while (($sandbox['#finished'] ?? 1) < 1 && $passes < 100); + + $this->assertLessThan(100, $passes, 'The update terminated.'); + + return $sandbox; + } + + /** + * Duplicate rows are merged and the unique key is added. + */ + public function testDuplicatesAreMergedNotRejected() { + $this->seedRow('/about', ';node:1;'); + $this->seedRow('/about', ';node:2;'); + $this->seedRow('/about', ';node:3;'); + $this->seedRow('/contact', ';node:4;'); + + $sandbox = $this->runUpdate(); + + $database = $this->container->get('database'); + $this->assertEquals(2, $database->select('purge_queuer_quant')->countQuery()->execute()->fetchField()); + $this->assertEquals(2, $sandbox['merged']); + $this->assertTrue($database->schema()->indexExists('purge_queuer_quant', 'url_domain')); + } + + /** + * No tag is lost when rows are merged. + * + * A dropped tag is a page that stops being purged when its content changes, + * which is silent until someone notices stale content. + */ + public function testMergedRowsKeepEveryTag() { + $this->seedRow('/about', ';node:1;'); + $this->seedRow('/about', ';node:2;node:3;'); + $this->seedRow('/about', ';node:4;'); + + $this->runUpdate(); + + $tags = $this->container->get('database')->select('purge_queuer_quant', 'q') + ->fields('q', ['tags']) + ->condition('url', '/about') + ->execute() + ->fetchField(); + + foreach (['node:1', 'node:2', 'node:3', 'node:4'] as $tag) { + $this->assertStringContainsString($tag, $tags, "Tag $tag survived the merge."); + } + } + + /** + * A table with no duplicates is upgraded without touching its rows. + */ + public function testCleanTableIsUnchanged() { + $this->seedRow('/a', ';node:1;'); + $this->seedRow('/b', ';node:2;'); + + $sandbox = $this->runUpdate(); + + $this->assertEquals(0, $sandbox['merged']); + $this->assertEquals(2, $this->container->get('database') + ->select('purge_queuer_quant')->countQuery()->execute()->fetchField()); + } + + /** + * Existing rows land in the empty domain, which is single-site behaviour. + */ + public function testExistingRowsGetTheEmptyDomain() { + $this->seedRow('/a', ';node:1;'); + + $this->runUpdate(); + + $domain = $this->container->get('database')->select('purge_queuer_quant', 'q') + ->fields('q', ['domain']) + ->condition('url', '/a') + ->execute() + ->fetchField(); + + $this->assertSame('', $domain); + } + +} diff --git a/modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php b/modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php new file mode 100644 index 00000000..ec2e0583 --- /dev/null +++ b/modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php @@ -0,0 +1,109 @@ +installSchema('quant_purger', ['purge_queuer_quant']); + $this->installConfig(['quant_api', 'quant_purger']); + + \Drupal::configFactory()->getEditable('quant_api.settings') + ->set('api_project', 'base-project') + ->save(); + } + + /** + * Calls the plugin's project resolver for a given domain. + * + * @param string $domainId + * The domain id, empty for a single-domain site. + * + * @return string|null + * The resolved project. + */ + protected function resolve(string $domainId) { + $plugin = new QuantPurger([], 'quant', []); + $plugin->setContainer($this->container); + + $method = new \ReflectionMethod($plugin, 'getProjectForDomain'); + $method->setAccessible(TRUE); + + return $method->invoke($plugin, $domainId); + } + + /** + * Without a domain, the base configuration names the only project. + * + * @covers ::getProjectForDomain + */ + public function testEmptyDomainResolvesBaseProject() { + $this->assertEquals('base-project', $this->resolve('')); + } + + /** + * Without the Domain module, a domain id still falls back to the base. + * + * The override service is absent, so there is nowhere else to look. This + * must not fatal. + * + * @covers ::getProjectForDomain + */ + public function testUnknownDomainFallsBackToBaseProject() { + $this->assertFalse(\Drupal::hasService('domain.config_factory_override')); + $this->assertEquals('base-project', $this->resolve('clienta')); + } + + /** + * An unconfigured project reports NULL rather than an empty string. + * + * @covers ::getProjectForDomain + */ + public function testUnconfiguredProjectIsNull() { + \Drupal::configFactory()->getEditable('quant_api.settings') + ->set('api_project', '') + ->save(); + + $this->assertNull($this->resolve('')); + } + +} diff --git a/quant.module b/quant.module index c84cc93f..f77a0f56 100644 --- a/quant.module +++ b/quant.module @@ -10,16 +10,19 @@ use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\Display\EntityViewDisplayInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Queue\DelayableQueueInterface; +use Drupal\Core\Queue\DelayedRequeueException; +use Drupal\Core\Queue\RequeueException; use Drupal\Core\Session\AccountInterface; use Drupal\Core\Site\Settings; use Drupal\Core\Url; use Drupal\node\NodeInterface; use Drupal\quant\Exception\TokenValidationDisabledException; -use Symfony\Component\HttpFoundation\Request; use Drupal\quant\Plugin\QueueItem\RouteItem; use Drupal\quant\QuantQueueFactory; use Drupal\quant\Seed; use Drupal\quant\Utility; +use Symfony\Component\HttpFoundation\Request; /** * Implements hook_menu_local_tasks_alter(). @@ -54,6 +57,10 @@ function quant_node_insert(EntityInterface $entity) { $context = [ 'callback' => '_quant_entity_update_op', 'args' => $entity, + // Capture the live request while it is still on the stack. Rebuilding one + // from globals after the response has gone bypasses Drupal's trusted host + // checking, and the host is what decides which project this publishes to. + 'request' => \Drupal::requestStack()->getCurrentRequest(), ]; drupal_register_shutdown_function('quant_shutdown', $context); @@ -85,6 +92,10 @@ function quant_node_update(EntityInterface $entity) { $context = [ 'callback' => '_quant_entity_update_op', 'args' => $entity, + // Capture the live request while it is still on the stack. Rebuilding one + // from globals after the response has gone bypasses Drupal's trusted host + // checking, and the host is what decides which project this publishes to. + 'request' => \Drupal::requestStack()->getCurrentRequest(), ]; drupal_register_shutdown_function('quant_shutdown', $context); @@ -110,6 +121,10 @@ function quant_taxonomy_term_insert(EntityInterface $entity) { $context = [ 'callback' => '_quant_entity_update_op', 'args' => $entity, + // Capture the live request while it is still on the stack. Rebuilding one + // from globals after the response has gone bypasses Drupal's trusted host + // checking, and the host is what decides which project this publishes to. + 'request' => \Drupal::requestStack()->getCurrentRequest(), ]; drupal_register_shutdown_function('quant_shutdown', $context); @@ -141,6 +156,10 @@ function quant_taxonomy_term_update(EntityInterface $entity) { $context = [ 'callback' => '_quant_entity_update_op', 'args' => $entity, + // Capture the live request while it is still on the stack. Rebuilding one + // from globals after the response has gone bypasses Drupal's trusted host + // checking, and the host is what decides which project this publishes to. + 'request' => \Drupal::requestStack()->getCurrentRequest(), ]; drupal_register_shutdown_function('quant_shutdown', $context); @@ -197,7 +216,7 @@ function quant_shutdown(array $context = []) { } if (is_callable($context['callback'])) { - drupal_register_shutdown_function('_quant_run_with_request', $context['callback'], $context['args']); + drupal_register_shutdown_function('_quant_run_with_request', $context['callback'], $context['args'], $context['request'] ?? NULL); } } @@ -226,12 +245,15 @@ function quant_shutdown(array $context = []) { * @param mixed $args * The argument to pass to it. */ -function _quant_run_with_request(callable $callback, $args) { +function _quant_run_with_request(callable $callback, $args, ?Request $request = NULL) { $stack = \Drupal::service('request_stack'); $pushed = FALSE; if ($stack->getCurrentRequest() === NULL) { - $stack->push(Request::createFromGlobals()); + // Prefer the request the hook captured: it went through Drupal's trusted + // host checking. createFromGlobals() reads $_SERVER['HTTP_HOST'] verbatim, + // and that host decides which project the work publishes to. + $stack->push($request ?: Request::createFromGlobals()); $pushed = TRUE; } @@ -405,8 +427,24 @@ function quant_process_queue(array &$context) { return FALSE; } - $worker->processItem($item->data); - $queue->deleteItem($item); + // Drush and cron both understand a requeue; this batch runner has to be + // told, or an item belonging to another domain would be deleted here after + // the worker declined to send it. + try { + $worker->processItem($item->data); + $queue->deleteItem($item); + } + catch (DelayedRequeueException $e) { + if ($queue instanceof DelayableQueueInterface) { + $queue->delayItem($item, $e->getDelay()); + } + else { + $queue->releaseItem($item); + } + } + catch (RequeueException) { + $queue->releaseItem($item); + } $context['sandbox']['progress']++; $context['message'] = t('Processed @i of @t', [ diff --git a/src/CliDomainContext.php b/src/CliDomainContext.php index 89ade19e..3b256d3a 100644 --- a/src/CliDomainContext.php +++ b/src/CliDomainContext.php @@ -52,6 +52,9 @@ public static function initialize() : ?string { return static::$domainId; } + // Set before the early returns below. A process that asks before the + // container has domain.negotiator caches a NULL and keeps it, which is + // correct here: a CLI process serves one domain for its whole life. static::$initialized = TRUE; $moduleHandler = \Drupal::moduleHandler(); diff --git a/src/EventSubscriber/DomainGuardSubscriber.php b/src/EventSubscriber/DomainGuardSubscriber.php index 64f2414f..e480a0e9 100644 --- a/src/EventSubscriber/DomainGuardSubscriber.php +++ b/src/EventSubscriber/DomainGuardSubscriber.php @@ -4,6 +4,7 @@ use Drupal\quant\CliDomainContext; use Drupal\quant\Event\QuantEvent; +use Drupal\quant\Event\QuantFileEvent; use Drupal\quant\Event\QuantRedirectEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\RequestStack; @@ -68,7 +69,15 @@ public static function getSubscribedEvents(): array { // whichever project the fallback landed on, taking down a live page // belonging to another client. QuantEvent::UNPUBLISH => ['onOutput', 100], + // Files and media go to the same project by the same route, so a + // misdirected run would upload one client's assets into another's. + QuantFileEvent::OUTPUT => ['onOutput', 100], ]; + + // Every event the API publisher listens to must appear above. A new one + // added there and missed here would publish unguarded, and nothing would + // fail loudly. + // @see \Drupal\quant\Tests\Unit\DomainGuardSubscriberTest } /** @@ -91,7 +100,7 @@ public function onOutput($event) { } \Drupal::logger('quant')->error('Refused to publish @path: the host @host matches no domain, so the Domain module fell back to the default and this content would be published to project @project. Add a domain for @host, or correct the Host header reaching Drupal.', [ - '@path' => $event instanceof QuantEvent ? $event->getLocation() : $event->getSourceUrl(), + '@path' => self::describe($event), '@host' => $host, '@project' => \Drupal::config('quant_api.settings')->get('api_project') ?: 'unknown', ]); @@ -99,6 +108,27 @@ public function onOutput($event) { $event->stopPropagation(); } + /** + * Names the thing being published, for the log message. + * + * @param object $event + * A content, file or redirect event. + * + * @return string + * The path or url the event concerns. + */ + protected static function describe($event) : string { + if ($event instanceof QuantEvent) { + return $event->getLocation(); + } + + if ($event instanceof QuantFileEvent) { + return $event->getUrl(); + } + + return $event->getSourceUrl(); + } + /** * Determines whether the serving host has no domain record. * diff --git a/src/Plugin/QueueWorker/QuantSeedWorker.php b/src/Plugin/QueueWorker/QuantSeedWorker.php index c5f18f7b..595f3f44 100644 --- a/src/Plugin/QueueWorker/QuantSeedWorker.php +++ b/src/Plugin/QueueWorker/QuantSeedWorker.php @@ -2,6 +2,7 @@ namespace Drupal\quant\Plugin\QueueWorker; +use Drupal\Core\Queue\DelayedRequeueException; use Drupal\Core\Queue\QueueWorkerBase; use Drupal\quant\CliDomainContext; use Drupal\quant\Plugin\QueueItem\QuantQueueItemInterface; @@ -17,6 +18,11 @@ */ class QuantSeedWorker extends QueueWorkerBase { + /** + * Seconds to hold a mismatched item before it can be claimed again. + */ + const REQUEUE_DELAY = 60; + /** * {@inheritdoc} */ @@ -30,9 +36,7 @@ public function processItem($item) { // kernel.request, so the domain context is otherwise empty. CliDomainContext::initialize(); - if (!$this->targetsActiveProject($item)) { - return NULL; - } + $this->assertTargetsActiveProject($item); \Drupal::logger('quant_seed')->notice($item->log()); return $item->send(); @@ -45,34 +49,42 @@ public function processItem($item) { * another's domain. On a shared Drupal instance serving many clients that * is a content leak, so a mismatch stops the send rather than risking it. * + * The item is put back rather than dropped. A worker returning normally has + * its item deleted, so simply declining to send would consume work queued + * for another domain and that content would never be published at all. The + * queue is a single shared table, so whichever domain's worker claims first + * would quietly eat the rest. + * * @param \Drupal\quant\Plugin\QueueItem\QuantQueueItemInterface $item * The queue item. * - * @return bool - * TRUE when the item may be sent. + * @throws \Drupal\Core\Queue\DelayedRequeueException + * When the item belongs to a different project. */ - protected function targetsActiveProject(QuantQueueItemInterface $item) : bool { + protected function assertTargetsActiveProject(QuantQueueItemInterface $item) : void { $target = $item->getTargetProject(); // Items queued before the stamp existed carry no target. Send them, to // keep existing single-domain queues working across an update. if (empty($target)) { - return TRUE; + return; } $active = \Drupal::service('quant_api.client')->getProject(); if ($target === $active) { - return TRUE; + return; } - \Drupal::logger('quant_seed')->error('Skipped @item: queued for project @target but this worker publishes to @active. Run the queue with --uri set to the domain that owns @target.', [ + \Drupal::logger('quant_seed')->error('Requeued @item: queued for project @target but this worker publishes to @active. Run the queue with --uri set to the domain that owns @target.', [ '@item' => $item->log(), '@target' => $target, '@active' => $active ?: 'none', ]); - return FALSE; + // Long enough that a single run cannot spin on the same item, short + // enough that the correct worker picks it up on its next pass. + throw new DelayedRequeueException(self::REQUEUE_DELAY); } } diff --git a/src/Utility.php b/src/Utility.php index 4d161c55..18fa15f7 100644 --- a/src/Utility.php +++ b/src/Utility.php @@ -314,13 +314,30 @@ public static function getPageInfo(?array $urls = NULL) : string { * path correctly; this is the backstop that keeps a malformed one off the * wire. * + * An absolute url is returned untouched. Redirect destinations may point at + * another host, and the // in a scheme is not a stray slash: collapsing it + * turns https://example.com into https:/example.com and publishes a broken + * redirect. + * + * A protocol-relative url is normalised rather than preserved. It cannot be + * told apart from a malformed path by inspection — //fr/node/1 and + * //example.com/x have the same shape — and within this module a bare // + * is always the malformed path. Nothing here generates protocol-relative + * urls: an external redirect destination arrives from + * Url::toString() with its scheme intact. + * * @param string $path * The path, which may carry a query string. * * @return string - * The path with repeated slashes collapsed. + * The path with repeated slashes collapsed, or the input unchanged if it + * is an absolute url. */ public static function normalizePath(string $path) : string { + if (!empty(parse_url($path, PHP_URL_SCHEME))) { + return $path; + } + // Only the path can pick up stray slashes. A query string may legitimately // contain them, in an oEmbed url for instance, so it is left alone. $parts = explode('?', $path, 2); diff --git a/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php b/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php index e52c046e..a6ad9c4b 100644 --- a/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php +++ b/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php @@ -2,6 +2,7 @@ namespace Drupal\Tests\quant\Kernel; +use Drupal\Core\Queue\DelayedRequeueException; use Drupal\KernelTests\KernelTestBase; use Drupal\quant\CliDomainContext; use Drupal\quant\Plugin\QueueItem\QuantQueueItemInterface; @@ -168,11 +169,33 @@ public function testWithholdsWhenProjectDiffers() { $this->setActiveProject('project-a'); $item = $this->recordingItem('project-b'); - $this->worker->processItem($item); + try { + $this->worker->processItem($item); + $this->fail('A mismatched item should be requeued, not consumed.'); + } + catch (DelayedRequeueException $e) { + $this->assertGreaterThan(0, $e->getDelay(), 'The item is held before it can be claimed again.'); + } $this->assertFalse($item->sent, 'The item was not published to the wrong project.'); } + /** + * A mismatched item is put back rather than dropped. + * + * A worker that returns normally has its item deleted. Declining to send + * without requeueing would consume another domain's work, and that content + * would never be published at all. + * + * @covers ::assertTargetsActiveProject + */ + public function testMismatchedItemIsRequeuedNotConsumed() { + $this->setActiveProject('project-a'); + + $this->expectException(DelayedRequeueException::class); + $this->worker->processItem($this->recordingItem('project-b')); + } + /** * Items queued before stamping existed are still sent. * @@ -199,7 +222,12 @@ public function testWithholdsWhenNoActiveProject() { $this->setActiveProject(NULL); $item = $this->recordingItem('project-a'); - $this->worker->processItem($item); + try { + $this->worker->processItem($item); + } + catch (DelayedRequeueException $e) { + // Expected: held for a worker that knows where it belongs. + } $this->assertFalse($item->sent, 'The item was not published without a target.'); } diff --git a/tests/src/Unit/DomainGuardSubscriberTest.php b/tests/src/Unit/DomainGuardSubscriberTest.php index dde6a96e..4068ab38 100644 --- a/tests/src/Unit/DomainGuardSubscriberTest.php +++ b/tests/src/Unit/DomainGuardSubscriberTest.php @@ -13,6 +13,7 @@ use Drupal\quant\Event\QuantEvent; use Drupal\quant\Event\QuantRedirectEvent; use Drupal\quant\EventSubscriber\DomainGuardSubscriber; +use Drupal\quant_api\EventSubscriber\QuantApi; use Drupal\Tests\UnitTestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; @@ -127,6 +128,59 @@ public function testRedirectPassesOnKnownHost() { $this->assertFalse($event->isPropagationStopped()); } + /** + * Unpublishing is guarded, on the same terms as publishing. + * + * This is the irreversible one: a delete on an unrecognised host withdraws + * the matching URL from whichever project the fallback landed on, taking a + * live page down on another client's site. + * + * @covers ::getSubscribedEvents + * @covers ::onOutput + */ + public function testUnpublishIsGuarded() { + $events = DomainGuardSubscriber::getSubscribedEvents(); + $this->assertArrayHasKey(QuantEvent::UNPUBLISH, $events); + + $event = new QuantEvent('', '/node/1', [], NULL); + $this->subscriber(TRUE, ['clienta' => 'x', 'clientb' => 'y'], 'clienta.example', 'unregistered.example') + ->onOutput($event); + + $this->assertTrue($event->isPropagationStopped()); + } + + /** + * An unpublish on a recognised host proceeds. + * + * @covers ::onOutput + */ + public function testUnpublishPassesOnKnownHost() { + $event = new QuantEvent('', '/node/1', [], NULL); + $this->subscriber(TRUE, ['clienta' => 'x', 'clientb' => 'y'], 'clientb.example', 'clientb.example') + ->onOutput($event); + + $this->assertFalse($event->isPropagationStopped()); + } + + /** + * The guard covers every event the API publisher listens to. + * + * Each of these was unguarded until a test exercised that verb. A fourth + * would go unguarded too, and nothing would fail loudly, so this asserts + * the two sets stay in step. + * + * @covers ::getSubscribedEvents + */ + public function testGuardCoversEveryPublishingEvent() { + $guarded = array_keys(DomainGuardSubscriber::getSubscribedEvents()); + $published = array_keys(QuantApi::getSubscribedEvents()); + + $this->assertEmpty( + array_diff($published, $guarded), + 'Every event QuantApi publishes on must also be guarded.' + ); + } + /** * The guard runs ahead of the search and publish subscribers. * diff --git a/tests/src/Unit/UtilityNormalizePathTest.php b/tests/src/Unit/UtilityNormalizePathTest.php index 0c50bc88..bf0f9d1c 100644 --- a/tests/src/Unit/UtilityNormalizePathTest.php +++ b/tests/src/Unit/UtilityNormalizePathTest.php @@ -37,6 +37,26 @@ public static function pathProvider() : array { 'query string kept' => ['//fr/search?page=2', '/fr/search?page=2'], // An oEmbed route carries a whole URL in its query string, and the // slashes in that URL are not ours to touch. + // An absolute url must survive intact: the // in its scheme is not a + // stray slash, and collapsing it publishes a broken redirect. + 'absolute https untouched' => [ + 'https://example.com/a//b', + 'https://example.com/a//b', + ], + 'absolute http untouched' => [ + 'http://other.org/foo', + 'http://other.org/foo', + ], + 'absolute with query untouched' => [ + 'https://example.com/a?b=//c', + 'https://example.com/a?b=//c', + ], + // Protocol-relative is indistinguishable from a malformed path, and in + // this module a bare // is always the latter, so it is collapsed. + 'protocol relative is treated as a path' => [ + '//example.com/x', + '/example.com/x', + ], 'slashes in query untouched' => [ '//media/oembed?url=https://example.com/a//b', '/media/oembed?url=https://example.com/a//b', From 67704851139c90d225b5de945876a866a2bbe45e Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 18:03:09 -0700 Subject: [PATCH 13/15] Guard every write at the client, not only the ones that emit events. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewing the branch for anything the last round missed turned up the same gap a fourth time, in a place events never reach. Search records, facets and index clearing call QuantClient directly and dispatch nothing, so the subscriber never saw them. Clearing is the destructive one: on an unrecognised host it wipes another client's entire search index. Guarding those three call sites would have repeated the mistake, so the decision moved into Drupal\quant\PublishGuard and both the subscriber and the client consult it. The subscriber still runs first, and still stops early enough to skip the render and search work; the client is the backstop that covers a write method nobody has written yet. Applied to the seven methods that change something — send, sendRedirect, sendFile, unpublish, sendSearchRecords, clearSearchIndex, addFacets — and deliberately not to ping, project, search or getUrlMeta, which are reads and are how the settings form reports whether the connection works. Verified against two domains on an unrecognised host: the three search writes refused with zero requests reaching the API, while ping still answers. A refused write returns an empty array, which QuantApi::onOutput could not survive: it read $res['attachments']['js'] straight into array_merge(), which fatals on NULL. The guard makes that reachable, but the API was always entitled to answer without attachments, so the keys are no longer assumed. Static analysis on the changed files also flagged a test constructing QuantPurger with plugin arguments it has no constructor for, and an @var where a @param belonged in the tome batch. 94 unit and kernel tests, 39 regression cases, phpcs clean. --- modules/quant_api/src/Client/QuantClient.php | 60 +++++++++ .../src/EventSubscriber/QuantApi.php | 12 +- .../src/Kernel/QuantPurgerProjectTest.php | 2 +- modules/quant_tome/src/QuantTomeBatch.php | 4 +- src/EventSubscriber/DomainGuardSubscriber.php | 48 +------ src/PublishGuard.php | 100 +++++++++++++++ tests/src/Kernel/PublishGuardWriteTest.php | 119 ++++++++++++++++++ 7 files changed, 299 insertions(+), 46 deletions(-) create mode 100644 src/PublishGuard.php create mode 100644 tests/src/Kernel/PublishGuardWriteTest.php diff --git a/modules/quant_api/src/Client/QuantClient.php b/modules/quant_api/src/Client/QuantClient.php index 874c1e27..424023f9 100644 --- a/modules/quant_api/src/Client/QuantClient.php +++ b/modules/quant_api/src/Client/QuantClient.php @@ -4,6 +4,7 @@ use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Logger\LoggerChannelFactoryInterface; +use Drupal\quant\PublishGuard; use Drupal\quant_api\Exception\InvalidPayload; use GuzzleHttp\Client; use GuzzleHttp\Exception\RequestException; @@ -113,6 +114,30 @@ protected function refreshCredentials() : void { $this->tlsDisabled = $this->config->get('api_tls_disabled'); } + /** + * Refuses a write the current context cannot be trusted to address. + * + * The event subscriber stops most work earlier, and more cheaply. This + * catches the writes that never dispatch an event — search records, facets + * and index clearing all call this client directly — so a method added + * later is covered without anyone remembering to guard it. + * + * @param string $operation + * What is being attempted, for the log. + * + * @return bool + * TRUE when the caller must not proceed. + */ + protected function refusesWrite(string $operation) : bool { + if (!PublishGuard::refuses($host)) { + return FALSE; + } + + PublishGuard::logRefusal($operation, $host); + + return TRUE; + } + /** * Returns the Quant project this client currently targets. * @@ -276,6 +301,11 @@ public function search() { */ public function send(array $data) : array { $this->refreshCredentials(); + + if ($this->refusesWrite('to publish ' . ($data['url'] ?? 'content'))) { + return []; + } + // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint, [ RequestOptions::JSON => $data, @@ -295,6 +325,11 @@ public function send(array $data) : array { */ public function sendRedirect(array $data) : array { $this->refreshCredentials(); + + if ($this->refusesWrite('to publish a redirect for ' . ($data['url'] ?? 'a url'))) { + return []; + } + // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint . '/redirect', [ RequestOptions::JSON => $data, @@ -315,6 +350,10 @@ public function sendRedirect(array $data) : array { public function sendFile(string $file, string $url, ?int $rid = NULL) : array { $this->refreshCredentials(); + if ($this->refusesWrite('to upload ' . $url)) { + return []; + } + // Ensure the file is accessible before attempting to send to the API. if (!file_exists($file) || !is_readable($file) || !is_file($file)) { throw new InvalidPayload($file); @@ -363,6 +402,11 @@ public function sendFile(string $file, string $url, ?int $rid = NULL) : array { */ public function unpublish(string $url) : array { $this->refreshCredentials(); + + if ($this->refusesWrite('to unpublish ' . $url)) { + return []; + } + // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->patch($this->endpoint . '/unpublish', [ 'headers' => [ @@ -394,6 +438,7 @@ public function getUrlMeta(array $urls) : array { 'Quant-Url' => $urls, ]; } + // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint . '/url-meta', [ RequestOptions::JSON => $urls, @@ -413,6 +458,11 @@ public function getUrlMeta(array $urls) : array { */ public function sendSearchRecords(array $records) : array { $this->refreshCredentials(); + + if ($this->refusesWrite('to write ' . count($records) . ' search records')) { + return []; + } + // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint . '/search', [ RequestOptions::JSON => $records, @@ -432,6 +482,11 @@ public function sendSearchRecords(array $records) : array { */ public function clearSearchIndex() : array { $this->refreshCredentials(); + + if ($this->refusesWrite('to clear the search index')) { + return []; + } + // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->delete($this->endpoint . '/search/all', [ 'headers' => [ @@ -450,6 +505,11 @@ public function clearSearchIndex() : array { */ public function addFacets(array $facets) : array { $this->refreshCredentials(); + + if ($this->refusesWrite('to write search facets')) { + return []; + } + // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint . '/search/facet', [ RequestOptions::JSON => $facets, diff --git a/modules/quant_api/src/EventSubscriber/QuantApi.php b/modules/quant_api/src/EventSubscriber/QuantApi.php index c4090424..9d28f029 100644 --- a/modules/quant_api/src/EventSubscriber/QuantApi.php +++ b/modules/quant_api/src/EventSubscriber/QuantApi.php @@ -157,7 +157,17 @@ public function onOutput(QuantEvent $event) { return FALSE; } - $media = array_merge($res['attachments']['js'], $res['attachments']['css'], $res['attachments']['media']['images'], $res['attachments']['media']['documents'], $res['attachments']['media']['video']); + // A refused write returns an empty array, and the API is entitled to + // answer without attachments, so none of these keys can be assumed. + // array_merge() fatals on a NULL argument. + $attachments = $res['attachments'] ?? []; + $media = array_merge( + $attachments['js'] ?? [], + $attachments['css'] ?? [], + $attachments['media']['images'] ?? [], + $attachments['media']['documents'] ?? [], + $attachments['media']['video'] ?? [] + ); $queue_factory = QuantQueueFactory::getInstance(); $queue = $queue_factory->get('quant_seed_worker'); diff --git a/modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php b/modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php index ec2e0583..776fed4f 100644 --- a/modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php +++ b/modules/quant_purger/tests/src/Kernel/QuantPurgerProjectTest.php @@ -62,7 +62,7 @@ protected function setUp() : void { * The resolved project. */ protected function resolve(string $domainId) { - $plugin = new QuantPurger([], 'quant', []); + $plugin = new QuantPurger(); $plugin->setContainer($this->container); $method = new \ReflectionMethod($plugin, 'getProjectForDomain'); diff --git a/modules/quant_tome/src/QuantTomeBatch.php b/modules/quant_tome/src/QuantTomeBatch.php index 7f9d4518..b6e2a006 100644 --- a/modules/quant_tome/src/QuantTomeBatch.php +++ b/modules/quant_tome/src/QuantTomeBatch.php @@ -207,8 +207,10 @@ public function pathToUri($file_path) { /** * Deploy a file to Quant. * - * @var \Drupal\quant\Plugin\QueueItem $item + * @param \Drupal\quant\Plugin\QueueItem\QuantQueueItemInterface $item * The file item to send to Quant API. + * @param array $context + * The batch context. */ public function deploy($item, array &$context) { // Batch operations may run in a forked process that never negotiated a diff --git a/src/EventSubscriber/DomainGuardSubscriber.php b/src/EventSubscriber/DomainGuardSubscriber.php index e480a0e9..a1a7db21 100644 --- a/src/EventSubscriber/DomainGuardSubscriber.php +++ b/src/EventSubscriber/DomainGuardSubscriber.php @@ -6,6 +6,7 @@ use Drupal\quant\Event\QuantEvent; use Drupal\quant\Event\QuantFileEvent; use Drupal\quant\Event\QuantRedirectEvent; +use Drupal\quant\PublishGuard; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpFoundation\RequestStack; @@ -95,15 +96,13 @@ public function onOutput($event) { // The call is cached per process, so this costs nothing after the first. CliDomainContext::initialize(); - if (!$this->hostIsUnknown($host)) { + if (!PublishGuard::refuses($host, $this->requestStack)) { return; } - \Drupal::logger('quant')->error('Refused to publish @path: the host @host matches no domain, so the Domain module fell back to the default and this content would be published to project @project. Add a domain for @host, or correct the Host header reaching Drupal.', [ - '@path' => self::describe($event), - '@host' => $host, - '@project' => \Drupal::config('quant_api.settings')->get('api_project') ?: 'unknown', - ]); + // Stopping here also skips the render and search work the later + // subscribers would do, which the client-level backstop cannot. + PublishGuard::logRefusal('to publish ' . self::describe($event), $host); $event->stopPropagation(); } @@ -129,41 +128,4 @@ protected static function describe($event) : string { return $event->getSourceUrl(); } - /** - * Determines whether the serving host has no domain record. - * - * @param string|null $host - * Set to the offending host when the check fails. - * - * @return bool - * TRUE when the push must be stopped. - */ - protected function hostIsUnknown(&$host = NULL) : bool { - $host = NULL; - - $moduleHandler = \Drupal::moduleHandler(); - - if (!$moduleHandler->moduleExists('domain')) { - return FALSE; - } - - $request = $this->requestStack->getCurrentRequest(); - - if (!$request) { - return FALSE; - } - - $host = $request->getHttpHost(); - $storage = \Drupal::entityTypeManager()->getStorage('domain'); - - // With a single domain there is only one project to publish to, so the - // fallback cannot send content anywhere unexpected. Only a genuine - // multi-domain site can lose a page to another site's project. - if (count($storage->loadMultiple()) < 2) { - return FALSE; - } - - return empty($storage->loadByHostname($host)); - } - } diff --git a/src/PublishGuard.php b/src/PublishGuard.php new file mode 100644 index 00000000..787b8aef --- /dev/null +++ b/src/PublishGuard.php @@ -0,0 +1,100 @@ +moduleExists('domain')) { + return FALSE; + } + + if (!$requestStack) { + if (!\Drupal::hasService('request_stack')) { + return FALSE; + } + $requestStack = \Drupal::service('request_stack'); + } + + if (!\Drupal::hasService('entity_type.manager')) { + return FALSE; + } + + $request = $requestStack->getCurrentRequest(); + + if (!$request) { + return FALSE; + } + + $host = $request->getHttpHost(); + $storage = \Drupal::entityTypeManager()->getStorage('domain'); + + // With a single domain there is only one project to write to, so the + // fallback cannot reach anywhere unexpected. Only a genuine multi-domain + // site can lose a page to another site's project. + if (count($storage->loadMultiple()) < 2) { + return FALSE; + } + + return empty($storage->loadByHostname($host)); + } + + /** + * Logs a refusal, naming the host and the project that would have received. + * + * @param string $what + * The path, url or operation being refused. + * @param string|null $host + * The host that matched no domain. + */ + public static function logRefusal(string $what, ?string $host) : void { + \Drupal::logger('quant')->error('Refused @what: the host @host matches no domain, so the Domain module fell back to the default and this would have been written to project @project. Add a domain for @host, or correct the Host header reaching Drupal.', [ + '@what' => $what, + '@host' => $host ?? 'unknown', + '@project' => \Drupal::config('quant_api.settings')->get('api_project') ?: 'unknown', + ]); + } + +} diff --git a/tests/src/Kernel/PublishGuardWriteTest.php b/tests/src/Kernel/PublishGuardWriteTest.php new file mode 100644 index 00000000..16bc73b7 --- /dev/null +++ b/tests/src/Kernel/PublishGuardWriteTest.php @@ -0,0 +1,119 @@ +installConfig(['quant_api']); + } + + /** + * Without the Domain module nothing is refused. + * + * @covers ::refuses + */ + public function testAllowsWhenDomainModuleAbsent() { + $this->assertFalse(\Drupal::moduleHandler()->moduleExists('domain')); + $this->assertFalse(PublishGuard::refuses()); + } + + /** + * Every write method on the client consults the guard. + * + * Asserted by reading the source rather than by exercising each one, + * because the point is that a method added later is covered too. A new + * write that skips the check fails this. + * + * @covers ::refuses + */ + public function testEveryWriteMethodIsGuarded() { + $path = \Drupal::service('extension.list.module')->getPath('quant_api'); + $source = file_get_contents($this->root . '/' . $path . '/src/Client/QuantClient.php'); + + // Methods that change something in a Quant project. + $writes = [ + 'send', + 'sendRedirect', + 'sendFile', + 'unpublish', + 'sendSearchRecords', + 'clearSearchIndex', + 'addFacets', + ]; + + foreach ($writes as $method) { + $start = strpos($source, 'public function ' . $method . '('); + $this->assertNotFalse($start, "QuantClient::$method() exists."); + + // The guard should be among the first statements, before any request. + $body = substr($source, $start, 500); + $this->assertStringContainsString( + 'refusesWrite(', + $body, + "QuantClient::$method() consults the publish guard." + ); + } + } + + /** + * Read-only methods are left alone. + * + * The settings form reports whether the connection works by calling + * ping() and project(), so those must keep answering on any host. + * + * @covers ::refuses + */ + public function testReadMethodsAreNotGuarded() { + $path = \Drupal::service('extension.list.module')->getPath('quant_api'); + $source = file_get_contents($this->root . '/' . $path . '/src/Client/QuantClient.php'); + + foreach (['ping', 'project', 'search', 'getUrlMeta'] as $method) { + $start = strpos($source, 'public function ' . $method . '('); + $body = substr($source, $start, 400); + $this->assertStringNotContainsString( + 'refusesWrite(', + $body, + "QuantClient::$method() is a read and stays unguarded." + ); + } + } + +} From 1d967a3bfc6256bc412a193c219675fa8abab67b Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 19:09:21 -0700 Subject: [PATCH 14/15] Install purge in CI, so the kernel tests can actually run. Wiring the phpunit job was verified by running the unit tests inside the CI image locally. That missed the kernel tests, which install quant_purger and therefore need drupal/purge. CI only installed drupal/token, so all 14 purger kernel tests errored with "Unavailable module: 'purge'" on every push since the job was added. The suite passes locally because the harness has purge installed for the end-to-end work. Verifying a CI change by running part of it locally was not verifying it; the run on the pull request was. --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0b9b38bd..99e45752 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,8 +35,10 @@ jobs: run: drush si --db-url=mysql://root:drupal@mariadb:3306/drupal11 -y working-directory: /var/www/drupal + # token is required by quant_search. purge is required by quant_purger, + # whose kernel tests install it, so the suite cannot run without it. - name: Install module dependencies - run: composer --no-interaction --no-progress require drupal/token + run: composer --no-interaction --no-progress require drupal/token drupal/purge working-directory: /var/www/drupal # The image ships phpunit globally, but its bootstrap needs the test From b6b3d3c0053a1350d0469aff0139b410973010a6 Mon Sep 17 00:00:00 2001 From: Stuart Rowlands Date: Tue, 11 Aug 2026 19:51:43 -0700 Subject: [PATCH 15/15] Commit the regression harness, and finish the purger end-to-end check. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The harness that found most of this branch's bugs existed only on one laptop. It drives every publishing path against a recording endpoint and asserts which project each request reached, which the unit and kernel suites cannot: they check the decision, this checks what leaves Drupal. Every bug it was written for was silent in production, and one reached only the PHP error log. Moved into tests/regression with a README, and the hard-coded path replaced. Also drove a real cache tag invalidation through the queuer, which had only been tested a layer at a time. /node/2 registered on both domains, invalidating node:2 from clienta's context queued two items stamped PROJECT-CLIENT-A and PROJECT-CLIENT-B, draining as clienta published A's and held B's, and draining as clientb published B's and emptied the queue. That is the fan-out and the per-domain project resolution confirmed against real data rather than doubles. Found while doing it, and left alone: the traffic registry only records requests carrying a Quant token, and the crawl only sends one when disable_content_drafts is off. That setting is on by default, so cache-tag purging does nothing on a default configuration — a full seed populated zero rows, and five once drafts were enabled. It predates this branch and is unrelated to multi-domain, and the fix has to reason about what that token is for, so it is written up as a known issue rather than rushed in here. --- tests/regression/README.md | 68 +++++++ tests/regression/mock-quant-api.py | 113 +++++++++++ tests/regression/regression.sh | 291 +++++++++++++++++++++++++++++ 3 files changed, 472 insertions(+) create mode 100644 tests/regression/README.md create mode 100644 tests/regression/mock-quant-api.py create mode 100755 tests/regression/regression.sh diff --git a/tests/regression/README.md b/tests/regression/README.md new file mode 100644 index 00000000..e0777d63 --- /dev/null +++ b/tests/regression/README.md @@ -0,0 +1,68 @@ +# Publishing regression harness + +Drives every path that publishes to Quant and asserts **which project each +request reached**. Publishing to the wrong project is the failure that matters +on a site serving several clients from one Drupal instance, and it is invisible +to the unit and kernel suites: those check the decision, this checks what +actually leaves Drupal. + +Every bug this harness was written for was silent in production. A page +published to the wrong client's site, a delete withdrawing a live page from +another client's project, a translated page publishing a redirect at +`//fr/node/1` — none raised an error, and one of them only reached the PHP +error log where nobody looks. + +## What it covers + +39 cases across the shapes a customer can actually have: + +| Section | What it asserts | +|---|---| +| Single site | Seed, cron, tome deploy, webform page, direct `seedNode`, deletion — all reaching the one project, nothing elsewhere | +| Multi domain | The same per domain, with `--uri`, asserting nothing crosses between projects | +| Guard cases | An unrecognised host publishes nothing: no content, no redirects, no withdrawals | +| Multilingual | Translations publish at their aliases, no path carries a doubled slash | +| Multilingual x multi domain | Every language reaching only its own domain's project, including search records and deletion | + +Assertions are on routing, not counts. "Everything reached the expected +project and nothing reached another" survives a fixture gaining content or +languages; an exact count does not, and a brittle count is how a real gap hid +once already. + +## Requirements + +- A ddev Drupal 11 site with this module installed +- `drupal/domain`, `drupal/domain_config`, `drupal/token`, `drupal/purge`, + `drupal/tome`, `drupal/webform` +- Two extra hostnames on the ddev project, `clienta` and `clientb` +- Content in three languages with aliases, matching the paths asserted in the + multilingual sections + +## Running it + +Start the recording endpoint on the host, then point the site at it: + +``` +python3 mock-quant-api.py # listens on :8899, logs to requests.jsonl +drush config:set quant_api.settings api_endpoint http://host.docker.internal:8899 +``` + +Then: + +``` +./regression.sh +``` + +It prints a pass or fail line per case and exits non-zero on any failure. It +sets up and tears down its own domains, so it is safe to re-run. + +## Why a mock rather than the live API + +The mock records the `Quant-Project` header of every request, which is the +single thing that decides whose site changes. Against the live API that is +invisible without going and looking in each project, and a mistake would +publish real content to a real customer. + +Run one seed against a live throwaway project as well, to confirm the wire +format is still accepted. The mock proves routing; only the real API proves +the payload. diff --git a/tests/regression/mock-quant-api.py b/tests/regression/mock-quant-api.py new file mode 100644 index 00000000..337e0493 --- /dev/null +++ b/tests/regression/mock-quant-api.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python3 +"""Mock Quant API for the multi-domain PoC. + +Purpose: record which Quant project each push is addressed to. + +The Drupal module identifies the target project with the `Quant-Project` +request header. This server accepts every request, returns the minimum +response shape the module expects, and appends one JSON line per request +to requests.jsonl. + +Run on the host. ddev containers reach it at http://host.docker.internal:8899 +""" + +import json +import os +import sys +from datetime import datetime, timezone +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer + +PORT = 8899 +LOG_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), "requests.jsonl") + +# The response shape QuantApi::onOutput() destructures after a send(). +SEND_RESPONSE = { + "attachments": { + "js": [], + "css": [], + "media": {"images": [], "documents": [], "video": []}, + } +} + +# The shape quant_search / SeedForm expect from /v1/ping. +PING_RESPONSE = { + "project": "mock", + "config": {"search_enabled": True}, +} + + +class Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def log_message(self, fmt, *args): + # Silence the default stderr access log; we keep our own. + pass + + def _record(self, method): + length = int(self.headers.get("Content-Length") or 0) + raw = self.rfile.read(length) if length else b"" + + pushed_url = None + has_search_record = False + try: + if raw: + payload = json.loads(raw) + pushed_url = payload.get("url") + has_search_record = "search_record" in payload + except (ValueError, AttributeError): + pass + + entry = { + "ts": datetime.now(timezone.utc).isoformat(), + "method": method, + "path": self.path, + # The three headers that identify the destination. + "quant_project": self.headers.get("Quant-Project"), + "quant_customer": self.headers.get("Quant-Customer"), + "quant_token": self.headers.get("Quant-Token"), + # The content being published. + "pushed_url": pushed_url, + # unpublish sends its target in a header rather than the body. + "quant_url_header": self.headers.get("Quant-Url"), + "has_search_record": has_search_record, + } + + with open(LOG_PATH, "a") as handle: + handle.write(json.dumps(entry) + "\n") + + print( + f"{method:6} {self.path:24} project={entry['quant_project']!s:20} " + f"url={entry['pushed_url']}", + flush=True, + ) + return entry + + def _respond(self, payload): + body = json.dumps(payload).encode() + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def do_GET(self): + self._record("GET") + if self.path.startswith("/v1/ping"): + self._respond(PING_RESPONSE) + else: + self._respond({"status": "ok"}) + + def do_POST(self): + self._record("POST") + self._respond(SEND_RESPONSE) + + def do_PATCH(self): + self._record("PATCH") + self._respond({"status": "ok"}) + + +if __name__ == "__main__": + if "--reset" in sys.argv and os.path.exists(LOG_PATH): + os.remove(LOG_PATH) + print(f"Mock Quant API on :{PORT}, logging to {LOG_PATH}", flush=True) + ThreadingHTTPServer(("0.0.0.0", PORT), Handler).serve_forever() diff --git a/tests/regression/regression.sh b/tests/regression/regression.sh new file mode 100755 index 00000000..bad51cc9 --- /dev/null +++ b/tests/regression/regression.sh @@ -0,0 +1,291 @@ +#!/bin/bash +# Regression matrix for the multi-domain branch. +# +# Runs every publishing path in both shapes a customer can have: +# single - no domain module, one project, no --uri (how most sites run) +# multi - domain module, two domains, two projects, --uri per domain +# +# Prints a pass/fail line per case. Reports which project each push reached, +# because publishing to the wrong project is the failure that matters. + +# The ddev project root, i.e. wherever mock-quant-api.py is writing its log. +# Override with QUANT_REGRESSION_LOG when running from elsewhere. +LOG="${QUANT_REGRESSION_LOG:-$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/requests.jsonl}" +PASS=0 +FAIL=0 + +reset_log() { : > "$LOG"; } + +# pushes_to -> count of content pushes addressed to that project +pushes_to() { + python3 -c " +import json,sys +n=0 +for line in open('$LOG'): + line=line.strip() + if not line: continue + r=json.loads(line) + if r['pushed_url'] and r['path']=='/v1' and r['quant_project']=='$1': n+=1 +print(n)" +} + +total_pushes() { + python3 -c " +import json +n=0 +for line in open('$LOG'): + line=line.strip() + if not line: continue + r=json.loads(line) + if r['pushed_url'] and r['path']=='/v1': n+=1 +print(n)" +} + +# Everything the API received that was NOT addressed to the given project. +# Exact counts move with the fixture; "all of it went to the right place, and +# none of it went anywhere else" is the property that actually matters. +pushes_elsewhere() { + python3 -c " +import json +rows=[json.loads(l) for l in open('$LOG') if l.strip()] +print(len([r for r in rows if r['quant_project'] != '$1']))" +} + +check_routed() { + local name="$1" project="$2" + local mine elsewhere + mine=$(pushes_to "$project") + elsewhere=$(pushes_elsewhere "$project") + if [ "$mine" -gt 0 ] && [ "$elsewhere" -eq 0 ]; then + echo " PASS $name ($mine to $project, 0 elsewhere)" + PASS=$((PASS+1)) + else + echo " FAIL $name ($mine to $project, $elsewhere elsewhere)" + FAIL=$((FAIL+1)) + fi +} + +# Unpublish sends its target in a header, not the body. +unpublishes_to() { + python3 -c " +import json +rows=[json.loads(l) for l in open('$LOG') if l.strip()] +print(len([r for r in rows if r['path']=='/v1/unpublish' and r['quant_project']=='$1']))" +} + +unpublishes_total() { + python3 -c " +import json +rows=[json.loads(l) for l in open('$LOG') if l.strip()] +print(len([r for r in rows if r['path']=='/v1/unpublish']))" +} + +# Creates a translated node, deletes it, and leaves the calls in the log. +delete_translated_node() { + local uri_arg="$1" + local nid + nid=$(ddev drush $uri_arg php:eval ' +use Drupal\node\Entity\Node; +$n = Node::create(["type"=>"page","title"=>"regression delete","status"=>1]); +$n->save(); +$n->addTranslation("fr", ["title"=>"regression delete fr","status"=>1])->save(); +print $n->id();' 2>/dev/null | tr -d "[:space:]") + reset_log + ddev drush $uri_arg php:eval "\\Drupal\\node\\Entity\\Node::load($nid)->delete();" >/dev/null 2>&1 +} + +check() { + local name="$1" actual="$2" expected="$3" + if [ "$actual" = "$expected" ]; then + echo " PASS $name (got $actual)" + PASS=$((PASS+1)) + else + echo " FAIL $name (expected $expected, got $actual)" + FAIL=$((FAIL+1)) + fi +} + +# Always start from a single-site shape, whatever the last run left behind. +teardown_domains() { + ddev drush php:eval '$s=\Drupal::entityTypeManager()->getStorage("domain"); $s->delete($s->loadMultiple());' >/dev/null 2>&1 + ddev drush pmu domain_config -y >/dev/null 2>&1 + ddev drush pmu domain -y >/dev/null 2>&1 + ddev drush cr >/dev/null 2>&1 +} + +has_path() { + python3 -c " +import json +rows=[json.loads(l) for l in open('$LOG') if l.strip()] +print('yes' if any(r['pushed_url']=='$1' for r in rows) else 'no')" +} + +setup_domains() { + ddev drush en domain domain_config -y >/dev/null 2>&1 + ddev drush php:eval ' +use Drupal\domain\Entity\Domain; +$s = \Drupal::entityTypeManager()->getStorage("domain"); +foreach ([["clienta_ddev_site","clienta.ddev.site:33000","Client A",1,1],["clientb_ddev_site","clientb.ddev.site:33000","Client B",2,0]] as [$id,$host,$name,$w,$def]) { + if (!$s->load($id)) { + Domain::create(["id"=>$id,"hostname"=>$host,"name"=>$name,"scheme"=>"http","status"=>1,"weight"=>$w,"is_default"=>$def])->save(); + } +} +$base = \Drupal::service("config.storage"); +foreach (["clienta_ddev_site"=>["PROJECT-CLIENT-A","token-a","clienta.ddev.site:33000"],"clientb_ddev_site"=>["PROJECT-CLIENT-B","token-b","clientb.ddev.site:33000"]] as $id=>[$p,$t,$h]) { + $c = $base->createCollection("domain.$id"); + $c->write("quant_api.settings", ["api_project"=>$p,"api_token"=>$t]); + $c->write("quant.settings", ["host_domain"=>$h]); +}' >/dev/null 2>&1 + ddev drush cr >/dev/null 2>&1 +} + +trap teardown_domains EXIT + +echo "================ SINGLE SITE (no domain module) ================" +teardown_domains + +reset_log +ddev drush quant:seed-queue >/dev/null 2>&1 +ddev drush quant:run-queue --threads=3 >/dev/null 2>&1 +check_routed "seed + run-queue, no --uri" SINGLE-SITE + +reset_log +ddev drush cron >/dev/null 2>&1 +check_routed "cron, no --uri" SINGLE-SITE + +reset_log +ddev drush --uri=http://quant-domain-poc.ddev.site:33000 quant:seed-queue >/dev/null 2>&1 +ddev drush --uri=http://quant-domain-poc.ddev.site:33000 quant:run-queue --threads=2 >/dev/null 2>&1 +check_routed "seed + run-queue, with --uri" SINGLE-SITE + +reset_log +ddev drush php:eval '\Drupal\quant\Seed::seedNode(\Drupal\node\Entity\Node::load(1), "en");' >/dev/null 2>&1 +check_routed "direct seedNode" SINGLE-SITE + +reset_log +ddev drush tome:static -y >/dev/null 2>&1 +ddev drush quant:tome:deploy >/dev/null 2>&1 +check_routed "tome deploy, no --uri" SINGLE-SITE + +# quant_webform alters webform's libraries, so a webform page still has to +# render and publish with it enabled. +reset_log +ddev drush php:eval '(new \Drupal\quant\Plugin\QueueItem\RouteItem(["route" => "/form/contact"]))->send();' >/dev/null 2>&1 +check_routed "webform page publishes" SINGLE-SITE + +# Deleting withdraws pages from the edge, so a misdirected delete takes down +# a live page rather than merely adding a wrong one. +delete_translated_node "" +check "delete withdraws every language" "$(unpublishes_to SINGLE-SITE)" "2" +check " nothing withdrawn elsewhere" "$(unpublishes_total)" "2" + +echo +echo "================ MULTI DOMAIN (two domains) ================" +ddev drush en domain domain_config -y >/dev/null 2>&1 +ddev drush php:eval ' +use Drupal\domain\Entity\Domain; +$s = \Drupal::entityTypeManager()->getStorage("domain"); +foreach ([["clienta_ddev_site","clienta.ddev.site:33000","Client A",1,1],["clientb_ddev_site","clientb.ddev.site:33000","Client B",2,0]] as [$id,$host,$name,$w,$def]) { + if (!$s->load($id)) { + Domain::create(["id"=>$id,"hostname"=>$host,"name"=>$name,"scheme"=>"http","status"=>1,"weight"=>$w,"is_default"=>$def])->save(); + } +} +$base = \Drupal::service("config.storage"); +foreach (["clienta_ddev_site"=>["PROJECT-CLIENT-A","token-a","clienta.ddev.site:33000"],"clientb_ddev_site"=>["PROJECT-CLIENT-B","token-b","clientb.ddev.site:33000"]] as $id=>[$p,$t,$h]) { + $c = $base->createCollection("domain.$id"); + $c->write("quant_api.settings", ["api_project"=>$p,"api_token"=>$t]); + $c->write("quant.settings", ["host_domain"=>$h]); +}' >/dev/null 2>&1 +ddev drush cr >/dev/null 2>&1 + +for D in clienta clientb; do + UP=$(echo "$D" | tr 'a-z' 'A-Z' | sed 's/CLIENT/CLIENT-/') + reset_log + ddev drush --uri="http://$D.ddev.site:33000" quant:seed-queue >/dev/null 2>&1 + ddev drush --uri="http://$D.ddev.site:33000" quant:run-queue --threads=2 >/dev/null 2>&1 + check_routed "seed + run-queue as $D" "PROJECT-$UP" + + reset_log + ddev drush --uri="http://$D.ddev.site:33000" cron >/dev/null 2>&1 + check_routed "cron as $D" "PROJECT-$UP" + + reset_log + ddev drush --uri="http://$D.ddev.site:33000" quant:tome:deploy >/dev/null 2>&1 + check_routed "tome deploy as $D" "PROJECT-$UP" + + delete_translated_node "--uri=http://$D.ddev.site:33000" + check "delete as $D withdraws from PROJECT-$UP only" "$(unpublishes_to "PROJECT-$UP")" "2" + check " nothing withdrawn elsewhere" "$(unpublishes_total)" "2" +done + +echo +echo " -- guard cases --" +reset_log +ddev drush quant:seed-queue >/dev/null 2>&1 +ddev drush quant:run-queue --threads=2 >/dev/null 2>&1 +check "multi-domain, no --uri, no content published" "$(total_pushes)" "0" +# Redirects reach the same project, so the guard has to stop those too. +check " no redirects published either" "$(python3 -c " +import json +rows=[json.loads(l) for l in open('$LOG') if l.strip()] +print(len([r for r in rows if r['path']=='/v1/redirect']))")" "0" + +# The destructive path matters most: a delete on an unrecognised host would +# withdraw a live page from whichever project the fallback landed on. +delete_translated_node "" +check " no pages withdrawn either" "$(unpublishes_total)" "0" + +echo +echo "================ MULTILINGUAL ================" +teardown_domains + +# Paths and redirects are built from language prefixes, and getPathPrefix() +# already carries its leading slash. Concatenating another one published +# redirects at //fr/node/1 for every translation. +reset_log +ddev drush php:eval '\Drupal\quant\Seed::seedNode(\Drupal\node\Entity\Node::load(1), "fr");' >/dev/null 2>&1 +ddev drush php:eval '\Drupal\quant\Seed::seedNode(\Drupal\node\Entity\Node::load(1), "de");' >/dev/null 2>&1 + +malformed=$(python3 -c " +import json +rows=[json.loads(l) for l in open('$LOG') if l.strip()] +print(len([r for r in rows if r['pushed_url'] and '//' in r['pushed_url']]))") +check "no malformed // paths in any push" "$malformed" "0" + +check "french translation published at its alias" "$(has_path /fr/page-une)" "yes" +check "german translation published at its alias" "$(has_path /de/seite-eins)" "yes" +check "prefixed internal path redirect created" "$(has_path /fr/node/1)" "yes" + +echo +echo "================ MULTILINGUAL x MULTI DOMAIN ================" +# The shape the multi-client customers actually run: several domains, each +# publishing to its own project, every page in several languages. A leak here +# puts one client's translated page on another client's site. +setup_domains + +for D in clienta clientb; do + UP=$(echo "$D" | tr 'a-z' 'A-Z' | sed 's/CLIENT/CLIENT-/') + + reset_log + ddev drush --uri="http://$D.ddev.site:33000" quant:seed-queue >/dev/null 2>&1 + ddev drush --uri="http://$D.ddev.site:33000" quant:run-queue --threads=2 >/dev/null 2>&1 + check_routed "seed as $D, all languages" "PROJECT-$UP" + + for path in /page-one /fr/page-une /de/seite-eins; do + check " $path reached PROJECT-$UP" "$(has_path "$path")" "yes" + done + + check " search records only in PROJECT-$UP" "$(python3 -c " +import json +rows=[json.loads(l) for l in open('$LOG') if l.strip()] +print(len([r for r in rows if r.get('has_search_record') and r['quant_project'] != 'PROJECT-$UP']))")" "0" + + delete_translated_node "--uri=http://$D.ddev.site:33000" + check "delete as $D withdraws all languages from PROJECT-$UP" "$(unpublishes_to "PROJECT-$UP")" "2" + check " nothing withdrawn from another project" "$(unpublishes_total)" "2" +done + +echo +echo "================ RESULT ================" +echo " passed: $PASS failed: $FAIL" +[ "$FAIL" -eq 0 ] || exit 1