diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 059e1349..99e45752 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,8 +35,16 @@ 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 + # 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 @@ -52,3 +60,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/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)." + } +} 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_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_api/src/Client/QuantClient.php b/modules/quant_api/src/Client/QuantClient.php index 534ad18a..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; @@ -73,14 +74,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 +114,46 @@ public function __construct(Client $client, ConfigFactoryInterface $config_facto $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. + * + * @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 +179,7 @@ public function getOverrides() { * {@inheritdoc} */ public function ping() { + $this->refreshCredentials(); try { // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. @@ -157,6 +220,7 @@ public function ping() { * {@inheritdoc} */ public function project() { + $this->refreshCredentials(); try { // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. @@ -196,6 +260,7 @@ public function project() { * {@inheritdoc} */ public function search() { + $this->refreshCredentials(); try { // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. @@ -235,6 +300,12 @@ public function search() { * {@inheritdoc} */ 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, @@ -253,6 +324,12 @@ public function send(array $data) : array { * {@inheritdoc} */ 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, @@ -271,6 +348,11 @@ public function sendRedirect(array $data) : array { * {@inheritdoc} */ 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)) { @@ -319,6 +401,12 @@ public function sendFile(string $file, string $url, ?int $rid = NULL) : array { * The API response. */ 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' => [ @@ -343,12 +431,14 @@ 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 = [ 'Quant-Url' => $urls, ]; } + // @todo Switch from 'Quant-Customer' to 'Quant-Organization'. $response = $this->client->post($this->endpoint . '/url-meta', [ RequestOptions::JSON => $urls, @@ -367,6 +457,12 @@ public function getUrlMeta(array $urls) : array { * {@inheritdoc} */ 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, @@ -385,6 +481,12 @@ public function sendSearchRecords(array $records) : array { * {@inheritdoc} */ 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' => [ @@ -402,6 +504,12 @@ public function clearSearchIndex() : array { * {@inheritdoc} */ 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/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/src/EventSubscriber/QuantApi.php b/modules/quant_api/src/EventSubscriber/QuantApi.php index f4f084a4..9d28f029 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(); @@ -151,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_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_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..c67a2a36 100644 --- a/modules/quant_api/tests/src/Unit/QuantClientTest.php +++ b/modules/quant_api/tests/src/Unit/QuantClientTest.php @@ -2,372 +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 = []) { - $value = [ - 'api_account' => 'account', - 'api_token' => 'token', - 'api_endpoint' => 'http://test', - ] + $default; + protected $history = []; + + /** + * Temporary files created by a test, removed on teardown. + * + * @var string[] + */ + protected $tempFiles = []; - $stub = $this->prophesize(ConfigFactoryInterface::class); - $config = $this->prophesize(ImmutableConfig::class); + /** + * The credentials every test configures. + */ + const ACCOUNT = 'test-account'; + const PROJECT = 'test-project'; + const TOKEN = 'test-token'; - foreach ($config as $key => $value) { - $config->get($key)->willReturn($value); + /** + * The endpoint the client derives from the configured base. + */ + const ENDPOINT = 'http://test/v1'; + + /** + * {@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); + } + + /** + * {@inheritdoc} + */ + protected function tearDown() : void { + foreach ($this->tempFiles as $file) { + if (file_exists($file)) { + unlink($file); + } } + parent::tearDown(); + } - $stub->get('quant_api.settings')->willReturn($config); - return $stub; + /** + * 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 \Drupal\quant_api\Client\QuantClient + * The client under test. + */ + 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) + ); } /** - * Get a successful project response. + * Builds a config factory returning the test credentials. * - * @return GuzzleHttp\Psr7\Response - * A response object. + * @param array $overrides + * Configuration values to override. + * + * @return \Drupal\Core\Config\ConfigFactoryInterface + * The config factory double. */ - protected function getProjectResponse() { - // @todo should these be fixtures. - $body = [ - 'project' => 'test', - 'error' => FALSE, - 'errorMsg' => '', + 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/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_cron/quant_cron.module b/modules/quant_cron/quant_cron.module index 0c27e353..0e65f701 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(); @@ -270,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/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/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..95bdba2e 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,106 @@ function quant_purger_update_9102(&$sandbox) { $config->set('path_allowlist', ['']); $config->save(); } + +/** + * 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) { + $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; + } + + if (!$schema->indexExists('purge_queuer_quant', 'url_domain')) { + $schema->addUniqueKey('purge_queuer_quant', 'url_domain', ['url', 'domain']); + } + + $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_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/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..0cff09d9 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(); } @@ -65,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(); } /** @@ -73,7 +110,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 +139,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/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..776fed4f --- /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(); + $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/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_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 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_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/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/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..b6e2a006 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; @@ -142,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'); @@ -200,10 +207,17 @@ 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 + // 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/quant.module b/quant.module index 0d20e01d..f77a0f56 100644 --- a/quant.module +++ b/quant.module @@ -10,6 +10,9 @@ 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; @@ -19,6 +22,7 @@ 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(). @@ -53,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); @@ -84,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); @@ -109,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); @@ -140,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); @@ -196,7 +216,54 @@ 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'], $context['request'] ?? NULL); + } +} + +/** + * 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 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. + * + * @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, ?Request $request = NULL) { + $stack = \Drupal::service('request_stack'); + $pushed = FALSE; + + if ($stack->getCurrentRequest() === NULL) { + // 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; + } + + try { + $callback($args); + } + finally { + if ($pushed) { + $stack->pop(); + } } } @@ -360,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/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/CliDomainContext.php b/src/CliDomainContext.php new file mode 100644 index 00000000..3b256d3a --- /dev/null +++ b/src/CliDomainContext.php @@ -0,0 +1,110 @@ +moduleExists('domain')) { + return NULL; + } + + if (!\Drupal::hasService('domain.negotiator')) { + return NULL; + } + + $domain = \Drupal::service('domain.negotiator')->getActiveDomain(); + + if (empty($domain)) { + return NULL; + } + + static::$domainId = $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; + } + + /** + * 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; + } + + /** + * 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/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/EventSubscriber/DomainGuardSubscriber.php b/src/EventSubscriber/DomainGuardSubscriber.php new file mode 100644 index 00000000..a1a7db21 --- /dev/null +++ b/src/EventSubscriber/DomainGuardSubscriber.php @@ -0,0 +1,131 @@ +requestStack = $request_stack; + } + + /** + * {@inheritdoc} + */ + public static function getSubscribedEvents(): array { + 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], + // 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], + // 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 + } + + /** + * Stops the push when the serving host has no domain record. + * + * @param \Drupal\quant\Event\QuantEvent|\Drupal\quant\Event\QuantRedirectEvent $event + * 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 (!PublishGuard::refuses($host, $this->requestStack)) { + return; + } + + // 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(); + } + + /** + * 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(); + } + +} 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/FileItem.php b/src/Plugin/QueueItem/FileItem.php index a92c0597..357e5f21 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($data); } /** diff --git a/src/Plugin/QueueItem/NodeItem.php b/src/Plugin/QueueItem/NodeItem.php index 2e9789bb..204f4e80 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($data); } /** 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..b64cd640 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($data); } /** diff --git a/src/Plugin/QueueItem/RouteItem.php b/src/Plugin/QueueItem/RouteItem.php index 25908293..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. @@ -12,6 +13,8 @@ */ class RouteItem implements QuantQueueItemInterface { + use TargetProjectTrait; + /** * A Drupal entity. * @@ -47,11 +50,14 @@ 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, '?'); $this->filePath = $data['file_path'] ?? DRUPAL_ROOT . strtok($route, '?'); + + // Record the project this item is destined for. + $this->stampTargetProject($data); } /** @@ -90,7 +96,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' => [ diff --git a/src/Plugin/QueueItem/TargetProjectTrait.php b/src/Plugin/QueueItem/TargetProjectTrait.php new file mode 100644 index 00000000..67a6807f --- /dev/null +++ b/src/Plugin/QueueItem/TargetProjectTrait.php @@ -0,0 +1,62 @@ +targetProject = $data['target_project']; + return; + } + + $this->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..3f8d7fd4 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($data); } /** diff --git a/src/Plugin/QueueWorker/QuantSeedWorker.php b/src/Plugin/QueueWorker/QuantSeedWorker.php index b90a3880..595f3f44 100644 --- a/src/Plugin/QueueWorker/QuantSeedWorker.php +++ b/src/Plugin/QueueWorker/QuantSeedWorker.php @@ -2,7 +2,9 @@ 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; /** @@ -16,14 +18,73 @@ */ class QuantSeedWorker extends QueueWorkerBase { + /** + * Seconds to hold a mismatched item before it can be claimed again. + */ + const REQUEUE_DELAY = 60; + /** * {@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(); + + $this->assertTargetsActiveProject($item); + + \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. + * + * 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. + * + * @throws \Drupal\Core\Queue\DelayedRequeueException + * When the item belongs to a different project. + */ + 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; + } + + $active = \Drupal::service('quant_api.client')->getProject(); + + if ($target === $active) { + return; + } + + \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', + ]); + + // 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/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/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/src/Utility.php b/src/Utility.php index b19093bb..18fa15f7 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 .= ''; } @@ -299,14 +305,60 @@ 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. + * + * 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, 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); + $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)) { @@ -315,6 +367,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/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 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." + ); + } + } + +} diff --git a/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php b/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php new file mode 100644 index 00000000..a6ad9c4b --- /dev/null +++ b/tests/src/Kernel/QuantSeedWorkerProjectGuardTest.php @@ -0,0 +1,312 @@ +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'); + + 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. + * + * 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'); + + 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.'); + } + + /** + * 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()); + } + +} diff --git a/tests/src/Unit/DomainGuardSubscriberTest.php b/tests/src/Unit/DomainGuardSubscriberTest.php new file mode 100644 index 00000000..4068ab38 --- /dev/null +++ b/tests/src/Unit/DomainGuardSubscriberTest.php @@ -0,0 +1,281 @@ +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); + } + + /** + * 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()); + } + + /** + * 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. + * + * 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 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. + * + * @covers ::onOutput + */ + public function testPublishesWhenHostResolves() { + $event = $this->event(); + $this->subscriber(TRUE, ['clienta' => 'x', 'clientb' => 'y'], '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', 'clientb' => 'y'], '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, ['clienta' => 'x', 'clientb' => 'y'], 'clientb.example:8080', 'clientb.example') + ->onOutput($event); + + $this->assertTrue($event->isPropagationStopped()); + } + +} 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()); + } + } diff --git a/tests/src/Unit/UtilityNormalizePathTest.php b/tests/src/Unit/UtilityNormalizePathTest.php new file mode 100644 index 00000000..bf0f9d1c --- /dev/null +++ b/tests/src/Unit/UtilityNormalizePathTest.php @@ -0,0 +1,77 @@ + ['/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. + // 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', + ], + ]; + } + + /** + * 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)); + } + +}