From aa48f60abf441e6526f824cbe876aa8e381b4962 Mon Sep 17 00:00:00 2001 From: ronaldtebrake Date: Mon, 28 Aug 2023 10:32:11 +0200 Subject: [PATCH 1/4] Backport patch from Drupal Core from issue #3231503 with cache improvements for hook_entity_extra_field_info() and EntityFieldManager::getExtraFields() As this patch only lands in 10.1.x (which is when we can remove this patch) we backport it as it could have significant performance impact, especially on pages layout builder or paragraphs driven. This way we can get the benefits already from the work done there, and remove it once we do our update towards 10.1.x, which is not yet planned at the time or writing. --- composer.json | 1 + 1 file changed, 1 insertion(+) diff --git a/composer.json b/composer.json index eedfddd3262..530a4683566 100644 --- a/composer.json +++ b/composer.json @@ -43,6 +43,7 @@ "Ajax not working when using non-default view mode": "https://www.drupal.org/files/issues/2023-01-30/ajax_comments-ajax_non_default_view_mode-2896916-beta5-60.patch" }, "drupal/core": { + "Issue #3231503 landed in 10.1.x: Cache the result of hook_entity_extra_field_info()": "https://www.drupal.org/files/issues/2023-08-28/3383704-backport-cache-fix-EntityFieldManager-getextrafields.patch", "Restrict images to this site blocks image style derivatives": "https://www.drupal.org/files/issues/2019-05-10/2528214-54.patch", "Optimize getCommentedEntity()": "https://www.drupal.org/files/issues/2018-12-28/2580551-72.patch", "Multiple usages of FieldPluginBase::getEntity do not check for NULL, leading to WSOD": "https://www.drupal.org/files/issues/2023-01-05/3007424-146-9.5.x.patch", From 4c893b7fd0c97628e1f2b4f9e2f054bc3987d905 Mon Sep 17 00:00:00 2001 From: Alexander Varwijk Date: Mon, 28 Aug 2023 17:23:53 +0200 Subject: [PATCH 2/4] Fix view name for groups search We can't just fix the change in `social_search_update_11403` because we would leave sites that had already executed the update in an indeterminate state. There's two scenarios: 1) The update has already run. In this case the change for the groups view hasn't happened due to the type, the new update hook will be executed and now all views are correct. 2) The update hook hasn't run already. In this case we do the correct 2 views (that sites from scenario 1 don't need help changing) in 11403 and we fix groups in 11404. --- .../social_search/social_search.install | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/modules/social_features/social_search/social_search.install b/modules/social_features/social_search/social_search.install index 2aea01bc7b8..51bc32dd9d5 100644 --- a/modules/social_features/social_search/social_search.install +++ b/modules/social_features/social_search/social_search.install @@ -307,7 +307,6 @@ function social_search_update_11403() : void { $views = [ "views.view.search_all", "views.view.search_content", - "views.view.search_group", ]; foreach ($views as $view) { @@ -323,3 +322,33 @@ function social_search_update_11403() : void { $config->save(TRUE); } } + +/** + * Revert search_api_language filter name/id change for groups view. + */ +function social_search_update_11404() : void { + // There was a typo in the view name of social_search_update_11403 which + // caused the groups view not to be updated, so we must do it again here. + $views = [ + "views.view.search_groups", + ]; + + foreach ($views as $view) { + $config = \Drupal::configFactory()->getEditable($view); + $display = $config->get("display"); + + $filter = $display['default']['display_options']['filters']['language_with_fallback']; + $filter['id'] = "search_api_language"; + $display['default']['display_options']['filters']["search_api_language"] = $filter; + unset($display['default']['display_options']['filters']['language_with_fallback']); + + $config->set('display', $display); + $config->save(TRUE); + } + + // Clean up the config created in our erroneous version of 11403. + $incorrect_view = \Drupal::configFactory()->getEditable("views.view.search_group"); + if (!$incorrect_view->isNew()) { + $incorrect_view->delete(); + } +} From a571e7c11fc354e3a3ceab629549ee2f44130827 Mon Sep 17 00:00:00 2001 From: Alexander Varwijk Date: Wed, 30 Aug 2023 13:05:46 +0200 Subject: [PATCH 3/4] Restructure EmailContext to provide better debug information The EmailContext mail handling was created quite some years ago. In case an email in a test didn't match, the developer only got information that there was no match, there was no useful information about what didn't match (for example that a subject wasn't found, or that a specific line didn't match). The test steps are rewritten so that the developer gets as much actionable information as possible. This can be that a specific subject wasn't found, or that one or more lines of the body did not match. This makes it easier for the developer based on just the test output and the changes they may have made to determine where the error comes from. Providing this information in the test output requires some new functions to be rewritten. This opportunity is used to also restructure the other functions of the file and provide PHPStan output: - Behat hooks are placed at the top - Followed by developer usable test steps - Followed by protected functions which are used to support the test steps, in the order in which they were first called. --- .../behat/features/bootstrap/EmailContext.php | 395 ++++++++++++------ 1 file changed, 259 insertions(+), 136 deletions(-) diff --git a/tests/behat/features/bootstrap/EmailContext.php b/tests/behat/features/bootstrap/EmailContext.php index 283a1b43e96..939944c5e3c 100644 --- a/tests/behat/features/bootstrap/EmailContext.php +++ b/tests/behat/features/bootstrap/EmailContext.php @@ -1,5 +1,4 @@ getEditable('swiftmailer.transport'); $swiftmailer_config->set('transport', 'spool'); @@ -33,7 +42,7 @@ public function enableEmailSpool() { * * @AfterScenario @email-spool */ - public function disableEmailSpool() { + public function disableEmailSpool() : void { // Update Drupal configuration. $swiftmailer_config = \Drupal::configFactory()->getEditable('swiftmailer.transport'); $swiftmailer_config->set('transport', 'native'); @@ -41,212 +50,326 @@ public function disableEmailSpool() { } /** - * Get a list of spooled emails. + * I run the digest cron. * - * @return Finder|null - * Returns a Finder if the directory exists. - * @throws Exception + * @Then I run the :arg1 digest cron */ - public function getSpooledEmails() { - $finder = new Finder(); - $spoolDir = $this->getSpoolDir(); + public function iRunTheDigestCron(string $frequency) : void { + // Update the timings in the digest table. + $query = \Drupal::database()->update('user_activity_digest'); + $query->fields(['timestamp' => 1]); + $query->condition('frequency', $frequency); + $query->execute(); - if(empty($spoolDir)) { - throw new \Exception('Could not retrieve the spool directory, or the directory does not exist.'); - } + // Update last run time to make sure we can run the digest cron. + \Drupal::state()->set('digest.' . $frequency . '.last_run', 1); - try { - $finder->files()->in($spoolDir); - return $finder; - } - catch (InvalidArgumentException $exception) { - return NULL; + \Drupal::service('cron')->run(); + + if (\Drupal::moduleHandler()->moduleExists('ultimate_cron')) { + $jobs = CronJob::loadMultiple(); + + /** @var CronJob $job */ + foreach($jobs as $job) { + $job->run(t('Launched by drush')); + } } } /** - * Get content of email. - * - * @param string $file - * Path to the file. + * I read an email. * - * @return string - * An unserialized email. + * @Then /^(?:|I )should have an email with subject "([^"]*)" and "([^"]*)" in the body$/ */ - public function getEmailContent($file) { - return unserialize(file_get_contents($file)); + public function iShouldHaveAnEmailWithTitleAndBody(string $subject, string $body) : void { + $this->assertEmailWithSubjectAndBody($subject, [$body]); } /** - * Get the path where the spooled emails are stored. + * I read an email with multiple content. * - * @return string - * The path where the spooled emails are stored. + * @Then I should have an email with subject :arg1 and in the content: */ - protected function getSpoolDir() { - $path = \Drupal::service('extension.list.profile')->getPath('social') . '/tests/behat/mail-spool'; - if (!file_exists($path)) { - mkdir($path, 0777, true); + public function iShouldHaveAnEmailWithTitleAndBodyMulti(string $subject, TableNode $table) : void { + $body = []; + $hash = $table->getHash(); + foreach ($hash as $row) { + $body[] = $row['content']; } - return $path; + $this->assertEmailWithSubjectAndBody($subject, $body); } /** - * Purge the messages in the spool. + * No emails have been sent. + * + * @Then no emails have been sent */ - protected function purgeSpool() { - $filesystem = new Filesystem(); + public function noEmailsHaveBeenSent() : void { $finder = $this->getSpooledEmails(); - if ($finder) { - /** @var File $file */ - foreach ($finder as $file) { - $filesystem->remove($file->getRealPath()); - } + $count = $finder->count(); + if ($count !== 0) { + throw new \Exception("No email messages should have been sent, but found $count."); } } /** - * Find an email with the given subject and body. + * I do not have an email with a specific subject. * - * @param string $subject - * The subject of the email. - * @param array $body - * Text that should be in the email. + * Can be used in case you don't want an email to be sent regardless of the + * body. * - * @return bool - * Email was found or not. - * @throws Exception + * @Then should not have an email with subject :subject + * @Then I should not have an email with subject :subject */ - protected function findSubjectAndBody($subject, $body) { - $finder = $this->getSpooledEmails(); - - $found_email = FALSE; - - if ($finder) { - /** @var File $file */ - foreach ($finder as $file) { - /** @var Swift_Message $email */ - $email = $this->getEmailContent($file); - $email_subject = $email->getSubject(); - $email_body = $email->getBody(); - - // Make it a traversable HTML doc. - $doc = new \DOMDocument(); - @$doc->loadHTML($email_body); - $xpath = new \DOMXPath($doc); - // Find the post header and email content in the HTML file. - $content = $xpath->evaluate('string(//*[contains(@class,"postheader")])'); - $content .= $xpath->evaluate('string(//*[contains(@class,"main")])'); - $content_found = 0; - - foreach ($body as $string) { - if (strpos($content, $string)) { - $content_found++; - } - } - - if ($email_subject == $subject && $content_found === count($body)) { - $found_email = TRUE; - } - } - } - else { - throw new \Exception('There are no email messages.'); + public function iShouldNotHaveAnEmailWithSubject(string $subject) : void { + $emails = $this->findEmailsWithSubject($subject); + $email_count = count($emails); + if ($email_count !== 0) { + throw new \Exception("Expected no emails with subject '$subject' but found $email_count"); } + } - return $found_email; + /** + * I do not have an email. + * + * @Then /^(?:|I )should not have an email with subject "([^"]*)" and "([^"]*)" in the body$/ + */ + public function iShouldNotHaveAnEmailWithTitleAndBody(string $subject, string $body) : void { + $this->assertNoEmailWithSubjectAndBody($subject, [$body]); } /** - * I run the digest cron. + * I do not have an email with multiple content. * - * @Then I run the :arg1 digest cron + * @Then I should not have an email with subject :arg1 and in the content: */ - public function iRunTheDigestCron($frequency) { - // Update the timings in the digest table. - $query = \Drupal::database()->update('user_activity_digest'); - $query->fields(['timestamp' => 1]); - $query->condition('frequency', $frequency); - $query->execute(); + public function iShouldNotHaveAnEmailWithTitleAndBodyMulti(string $subject, TableNode $table) : void { + $body = []; + $hash = $table->getHash(); + foreach ($hash as $row) { + $body[] = $row['content']; + } - // Update last run time to make sure we can run the digest cron. - \Drupal::state()->set('digest.' . $frequency . '.last_run', 1); + $this->assertEmailWithSubjectAndBody($subject, $body); + } - \Drupal::service('cron')->run(); + /** + * Ensure at least one email exists matching the provided subject and body. + * + * @param string $subject + * The exact subject the email should have. + * + * @param list $expected_lines + * A list of lines that should all be present in the body of the e-mail. + * + * @throws \Exception + * An exception that details to the developer what was wrong (e.g. no + * matching subject, no matching lines, or missing a specific line match). + */ + protected function assertEmailWithSubjectAndBody(string $subject, array $expected_lines) : void { + $emails = $this->findEmailsWithSubject($subject); + + // If no emails with the subject exist then we want to report that so a + // developer knows that may be the cause of their failure. + $subject_match_count = count($emails); + if ($subject_match_count === 0) { + throw new \Exception("No emails with subject '$subject' were found."); + } - if (\Drupal::moduleHandler()->moduleExists('ultimate_cron')) { - $jobs = CronJob::loadMultiple(); + $partial_matches = []; + $count_expected = count($expected_lines); + foreach ($emails as $email) { + $matched_lines = $this->getMatchingLinesForEmail($expected_lines, $email); - /** @var CronJob $job */ - foreach($jobs as $job) { - $job->run(t('Launched by drush')); + $count_matched = count($matched_lines); + // If we have a complete match then we're done. + if ($count_matched === $count_expected) { + return; } + + if ($count_matched > 0) { + $partial_matches[] = $matched_lines; + } + } + + $partial_match_count = count($partial_matches); + // If we had no partial matches then we can provide a simplified error message. + if ($partial_match_count === 0) { + $message = $subject_match_count === 1 + ? "One email with the subject '$subject' was found but none of the expected lines were found in the body of the email." + : "$subject_match_count emails with the subject '$subject' were found but none of the expected lines were found in the body of the email."; + throw new \Exception($message); + } + // If we had a single partial match then we tell the developer and there's + // probably a simple typo in the email or test. + if ($partial_match_count === 1) { + $missing_lines = array_diff($expected_lines, $partial_matches[0]); + $message = "One email was found which matched the subject and some lines but the following lines were missing from the e-mail: \n " . implode("\n ", $missing_lines); + throw new \Exception($message); + } + // With multiple partial matches we want to provide the developer as much + // information as possible. + $message = "$partial_match_count emails were found that matched the subject but all of them were missing some expected lines from the email:\n"; + foreach ($partial_matches as $i => $partial_match) { + $missing_lines = array_diff($expected_lines, $partial_match); + $message .= "------- Partial match $i --------\n " . implode("\n ", $missing_lines); } + throw new \Exception($message); } /** - * I read an email. + * Ensure there's no emails with the specified subject and body. * - * @Then /^(?:|I )should have an email with subject "([^"]*)" and "([^"]*)" in the body$/ + * @param string $subject + * The subject to match against. + * @param list $expected_lines + * An array of strings that match the body's contents. For an email to be + * considered a match, all lines in the array must match. + * + * @throws \Exception + * In case an email was found matching the sbuject and all expected lines of + * text. */ - public function iShouldHaveAnEmailWithTitleAndBody($subject, $body) { - $found_email = $this->findSubjectAndBody($subject, [$body]); + protected function assertNoEmailWithSubjectAndBody(string $subject, array $expected_lines) : void { + $emails = $this->findEmailsWithSubject($subject); - if (!$found_email) { - throw new \Exception('There is no email with that subject and body.'); + $count_expected = count($expected_lines); + foreach ($emails as $email) { + $matched_lines = $this->getMatchingLinesForEmail($expected_lines, $email); + + $count_matched = count($matched_lines); + if ($count_matched === $count_expected) { + throw new \Exception("An email exists with the specified subject and body."); + } } } /** - * I read an email with multiple content. + * Purge the messages in the spool. + */ + protected function purgeSpool() : void { + $filesystem = new Filesystem(); + $finder = $this->getSpooledEmails(); + + /** @var File $file */ + foreach ($finder as $file) { + $filesystem->remove($file->getRealPath()); + } + } + + /** + * Find all emails that were sent with a given subject. * - * @Then I should have an email with subject :arg1 and in the content: + * @param string $subject + * The subject to search for. + * + * @return array + * An array of matching emails. + * @throws \Exception + * An exception in case no emails were sent or email collection is + * incorrectly configured. */ - public function iShouldHaveAnEmailWithTitleAndBodyMulti($subject, TableNode $table) { - $body = []; - $hash = $table->getHash(); - foreach ($hash as $row) { - $body[] = $row['content']; + protected function findEmailsWithSubject(string $subject) : array { + $finder = $this->getSpooledEmails(); + + if ($finder->count() === 0) { + throw new \Exception('No email messages have been sent in the test.'); } - $found_email = $this->findSubjectAndBody($subject, $body); + $emails = []; + foreach ($finder as $file) { + $email = $this->getEmailContent($file); + assert($email instanceof \Swift_Message); - if (!$found_email) { - throw new \Exception('There is no email with that subject and body.'); + if ($email->getSubject() === $subject) { + $emails[] = $email; + } } + + return $emails; } /** - * I do not have an email. + * Get a list of spooled emails. * - * @Then /^(?:|I )should not have an email with subject "([^"]*)" and "([^"]*)" in the body$/ + * @return Finder + * Returns a Finder if the directory exists. + * @throws \Exception + * An exception is thrown in case the configured directory does not exist. */ - public function iShouldNotHaveAnEmailWithTitleAndBody($subject, $body) { - $found_email = $this->findSubjectAndBody($subject, [$body]); + protected function getSpooledEmails() : Finder { + $finder = new Finder(); + $spoolDir = $this->getSpoolDir(); - if ($found_email) { - throw new \Exception('There is an email with that subject and body.'); + try { + return $finder->files()->name("*.message")->in($spoolDir); + } + catch (\InvalidArgumentException $exception) { + throw new \Exception("The e-mail spool directory does not exist or is incorrectly configured, expected '{$spoolDir}' to exist."); } } /** - * I do not have an email with multiple content. + * Get the path where the spooled emails are stored. * - * @Then I should not have an email with subject :arg1 and in the content: + * @return string + * The path where the spooled emails are stored. */ - public function iShouldNotHaveAnEmailWithTitleAndBodyMulti($subject, TableNode $table) { - $body = []; - $hash = $table->getHash(); - foreach ($hash as $row) { - $body[] = $row['content']; - } + protected function getSpoolDir() : string { + // This path should exist within the repository and have a .gitignore file + // that ignores all e-mails so developers don't accidentally commit them. + return \Drupal::service('extension.list.profile')->getPath('social') . '/tests/behat/mail-spool'; + } - $found_email = $this->findSubjectAndBody($subject, $body); + /** + * Get content of email. + * + * @param \Symfony\Component\Finder\SplFileInfo $file + * The .message file to get content for. + * + * @return + * A deserialized email. + */ + protected function getEmailContent(SplFileInfo $file) { + assert($file->getExtension() === "message", "File passed to " . __FUNCTION__ . " must be a serialized .message file."); + return unserialize($file->getContents()); + } - if ($found_email) { - throw new \Exception('There is an email with that subject and body.'); + /** + * Find the matching lines in a specific email. + * + * @param list $expected_lines + * The list of lines of text that is expected to be in the email. + * @param $email + * The email to check. + * + * @return list + * A list of lines from $expected_lines that was found in the body of + * $email. + */ + protected function getMatchingLinesForEmail(array $expected_lines, $email) : array { + $body = $email->getBody(); + + // Make it a traversable HTML doc. + $doc = new \DOMDocument(); + @$doc->loadHTML($body); + $xpath = new \DOMXPath($doc); + // Find the post header and email content in the HTML file. + $content = $xpath->evaluate('string(//*[contains(@class,"postheader")])'); + $content .= $xpath->evaluate('string(//*[contains(@class,"main")])'); + + $matched_lines = []; + + foreach ($expected_lines as $string) { + if (str_contains($content, $string)) { + $matched_lines[] = $string; + } } + + return $matched_lines; } + + } From 9237ff07b16db854132cfe843e908c0c64f09785 Mon Sep 17 00:00:00 2001 From: Alexander Varwijk Date: Wed, 30 Aug 2023 14:30:59 +0200 Subject: [PATCH 4/4] Dump deserialized mail to spool directory for failed tests It's already helpful that the serialized mail is made available to developers in the mail-spool, which allows it to be output from the CI for easier debugging. However this requires developers to go through a serialized PHP object which can be difficult. This commit ads a postScenario hook that runs when a test fails and splits out the e-mail into multiple parts: html, text, and headers. These are stored in separate files which makes them a lot easier to inspect for developers, for example opening the HTML file in a browser. In case they need information that isn't split out, the original serialized object is still available in the .message file. --- .../behat/features/bootstrap/EmailContext.php | 48 +++++++++++++++++-- tests/behat/mail-spool/.gitignore | 4 ++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/tests/behat/features/bootstrap/EmailContext.php b/tests/behat/features/bootstrap/EmailContext.php index 939944c5e3c..8d121467161 100644 --- a/tests/behat/features/bootstrap/EmailContext.php +++ b/tests/behat/features/bootstrap/EmailContext.php @@ -3,12 +3,12 @@ namespace Drupal\social\Behat; use Behat\Behat\Context\Context; +use Behat\Behat\Hook\Scope\AfterScenarioScope; use Behat\Gherkin\Node\TableNode; use Drupal\ultimate_cron\Entity\CronJob; use Symfony\Component\Filesystem\Filesystem; use Symfony\Component\Finder\Finder; use Symfony\Component\Finder\SplFileInfo; -use Symfony\Component\HttpFoundation\File\File; /** * Provides helpful test steps around the handling of e-mails. @@ -49,6 +49,45 @@ public function disableEmailSpool() : void { $swiftmailer_config->save(); } + /** + * Extract all .message files into metadata, html and text. + * + * To help developers debug, in case a scenario failed we parse the serialized + * email into its text contents, html contents, and the header metadata. This + * is written in 3 files to the mail spool folder. This makes it easier for a + * developer to examine the contents of an email. + * + * @AfterScenario @email-spool + */ + public function extractSentEmails(AfterScenarioScope $afterScenario) : void { + if ($afterScenario->getTestResult()->isPassed()) { + return; + } + + $exts = [ + "text/html" => "html", + "text/plain" => "txt", + ]; + + $spool_directory = $this->getSpoolDir(); + $emails = $this->getSpooledEmails(); + foreach ($emails as $serialized) { + $path = $spool_directory . DIRECTORY_SEPARATOR . $serialized->getBasename($serialized->getExtension()); + + $email = $this->getEmailContent($serialized); + assert($email instanceof \Swift_Message); + + foreach ($email->getChildren() as $i => $part) { + file_put_contents($path . ($exts[$part->getContentType()] ?? "{$i}.unknown"), $part->getBody()); + } + + // The primary (preferred) body is not considered to be a child, so we + // must treat it separately. + file_put_contents($path . ($exts[$email->getBodyContentType()] ?? "unknown"), $email->getBody()); + file_put_contents($path . "metadata", $email->getHeaders()->toString()); + } + } + /** * I run the digest cron. * @@ -253,7 +292,7 @@ protected function purgeSpool() : void { $filesystem = new Filesystem(); $finder = $this->getSpooledEmails(); - /** @var File $file */ + /** @var \Symfony\Component\Finder\SplFileInfo $file */ foreach ($finder as $file) { $filesystem->remove($file->getRealPath()); } @@ -304,7 +343,10 @@ protected function getSpooledEmails() : Finder { $spoolDir = $this->getSpoolDir(); try { - return $finder->files()->name("*.message")->in($spoolDir); + // We don't provide a filter on extension here because we use the same + // finder in our purgeSpool function. extractSentEmails does create files + // with other extensions, but that should always happen last. + return $finder->files()->in($spoolDir); } catch (\InvalidArgumentException $exception) { throw new \Exception("The e-mail spool directory does not exist or is incorrectly configured, expected '{$spoolDir}' to exist."); diff --git a/tests/behat/mail-spool/.gitignore b/tests/behat/mail-spool/.gitignore index 843ff5d1a08..52d33cc907e 100644 --- a/tests/behat/mail-spool/.gitignore +++ b/tests/behat/mail-spool/.gitignore @@ -1 +1,5 @@ +*.html *.message +*.txt +*.metadata +*.unknown