Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

USAGOV-1937-clear-menu-link: #1923

Merged
merged 35 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
fa48c23
USAGOV-1937-clear-menu-link: Adds an event subscriber to clear menu l…
omerida Sep 4, 2024
5fa0b27
USAGOV-1937-clear-menu-link: Increase path count
omerida Sep 4, 2024
d17a82e
Merge branch 'dev' into USAGOV-1773-clear-menu-link
akf Sep 12, 2024
2c05ec8
Merge branch 'dev' into USAGOV-1773-clear-menu-link
omerida Sep 13, 2024
f979b4e
USAGOV-1773-clear-menu-link: Use correct interface type hint
omerida Sep 13, 2024
51a88ec
USAGOV-1773-clear-menu-link: Setting path count to 1 until we figure …
omerida Sep 13, 2024
42ad75c
Merge branch 'dev' into USAGOV-1773-clear-menu-link
akf Sep 13, 2024
f14bb1f
USAGOV-1773-clear-menu-link: Merge branch 'refs/heads/dev' into USAGO…
omerida Sep 17, 2024
5d83857
USAGOV-1773-clear-menu-link: Resets the alias path manager cache betw…
omerida Sep 17, 2024
979ca50
Merge branch 'dev' into USAGOV-1773-clear-menu-link
akf Sep 18, 2024
549fd04
USAGOV-1773-clear-menu-link: Merge remote-tracking branch 'origin/USA…
omerida Sep 18, 2024
b3f5197
USAGOV-1773-clear-menu-link: Add a check ot ensure we don't crawl the…
omerida Sep 23, 2024
b417c77
USAGOV-1773-clear-menu-link: Add an event listener to prevent exporti…
omerida Sep 23, 2024
cbf8f0b
USAGOV-1773-clear-menu-link: Apply a patch for changes to tome so it …
omerida Sep 24, 2024
db60ff0
USAGOV-1773-clear-menu-link: Remove verbose flag, set path count to 1…
omerida Sep 24, 2024
c340185
USAGOV-1773-clear-menu-link: Merge branch 'refs/heads/dev' into USAGO…
omerida Sep 24, 2024
7051984
USAGOV-1773-clear-menu-link: Fixing linting error
omerida Sep 24, 2024
dd52424
USAGOV-1773-clear-menu-link: Merge branch 'refs/heads/dev' into USAGO…
omerida Sep 24, 2024
5a336ad
USAGOV-1937-clear-menu-link: Merge branch 'refs/heads/dev' into USAGO…
omerida Oct 2, 2024
92d6e5a
USAGOV-1937-clear-menu-link: Moves resetting the menu active trail ca…
omerida Oct 3, 2024
044301f
Merge branch 'dev' into USAGOV-1773-clear-menu-link
omerida Oct 4, 2024
f58594a
Merge branch 'dev' into USAGOV-1773-clear-menu-link
akf Oct 9, 2024
61045a0
USAGOV-1937-clear-menu-link: Reset menu link storage cache
omerida Oct 11, 2024
9ed27c8
USAGOV-1937-clear-menu-link: Reset current route match between runs.
omerida Oct 11, 2024
e6b05b6
USAGOV-1937-clear-menu-link: Merge branch 'dev' into USAGOV-1937-clea…
omerida Oct 17, 2024
af565c6
USAGOV-1937-clear-menu-link: Add patch for menu_breadcrumb to add url…
omerida Oct 18, 2024
99a8cef
USAGOV-1937-clear-menu-link: Merge branch 'dev' into USAGOV-1937-clea…
omerida Oct 18, 2024
1b8c654
USAGOV-1937-clear-menu-link: Need to clear the menu cache between req…
omerida Oct 21, 2024
cfcd51a
Merge branch 'dev' into USAGOV-1773-clear-menu-link
omerida Oct 21, 2024
0cfb5c9
USAGOV-1937-clear-menu-link: Reapply menu breadcrumb patch.
omerida Oct 22, 2024
7265951
USAGOV-1937-clear-menu-link: Merge branch 'dev' into USAGOV-1937-clea…
omerida Oct 22, 2024
2c19d14
Merge branch 'dev' into USAGOV-1773-clear-menu-link
omerida Oct 22, 2024
bc05659
USAGOV-19370-clear-menu-link: Update patch to track invoked paths as …
omerida Oct 22, 2024
c205668
Merge branch 'dev' into USAGOV-1773-clear-menu-link
akf Oct 22, 2024
700b0c7
Merge branch 'dev' into USAGOV-1773-clear-menu-link
akf Oct 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@
"drupal/tome": {
"CSS on Amazon S3 via Flysystem not processing properly": "https://www.drupal.org/files/issues/2020-08-06/3161384-4.patch",
"De-duplicate invoke paths in StaticCommand's exportPaths": "./patches/drupal/deduplicateTomeInvokePaths.patch",
"Make tome work with drush 12": "https://www.drupal.org/files/issues/2023-08-02/tome_drush12-support.patch"
"Make tome work with drush 12": "https://www.drupal.org/files/issues/2023-08-02/tome_drush12-support.patch",
"Make tome work with path count > 1 without re-exporting pages multiple times" : "./patches/drupal/tomePathCountFixes.patch"
},
"drupal/core": {
"Enable MenuActiveTrail to find the original menu item created by node_menus": "./patches/drupal/correctActiveTrail_d10.patch",
Expand All @@ -151,6 +152,9 @@
},
"drupal/views_menu_children_filter": {
"Use the technique from correctActiveTrail.patch to return the original menu item": "./patches/drupal/correctMenuChildren_d10.patch"
},
"drupal/menu_breadcrumb": {
"Add url.path to cache context for menu breadcrumb": "https://www.drupal.org/files/issues/2024-09-11/menu_breadcrumb-3230481-fix-cache-context-mr22.patch"
}
},
"drupal-lenient": {
Expand Down
92 changes: 92 additions & 0 deletions patches/drupal/tomePathCountFixes.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
diff --git a/modules/tome_static/src/Commands/StaticCommand.php b/modules/tome_static/src/Commands/StaticCommand.php
index 56d963b5607c3ffdeebdf6c7854f0d0b1ece1b06..d6fbaca2cb30308f156e9927529673dfbf81f778 100644
--- a/modules/tome_static/src/Commands/StaticCommand.php
+++ b/modules/tome_static/src/Commands/StaticCommand.php
@@ -53,6 +53,8 @@ class StaticCommand extends CommandBase {
*/
protected $state;

+ protected $init_paths = [];
+
/**
* Constructs a StaticCommand instance.
*
@@ -123,6 +125,9 @@ class StaticCommand extends CommandBase {
}

$this->io->writeln('Generating static HTML...');
+
+ $this->setInitialPaths($paths);
+
$this->exportPaths($paths, [], $options['process-count'], $options['path-count'], TRUE, $options['retry-count'], $options['uri']);
$this->io->success('Exported static HTML and related assets.');

@@ -135,6 +140,14 @@ class StaticCommand extends CommandBase {
return 0;
}

+ protected function setInitialPaths(array $paths): void {
+ $this->init_paths = $paths;
+ }
+
+ protected function getInitialPaths(): array {
+ return $this->init_paths;
+ }
+
/**
* Exports the given paths to the static directory.
*
@@ -175,15 +188,18 @@ class StaticCommand extends CommandBase {
$show_progress && $this->io->progressStart(count($paths));

$invoke_paths = [];
- $collected_errors = $this->runCommands($commands, $process_count, $retry_count, function (Process $process) use ($show_progress, &$invoke_paths, $path_count) {
+ $collected_errors = $this->runCommands($commands, $process_count, $retry_count, function (Process $process) use ($show_progress, &$invoke_paths, $path_count, $old_paths) {
$show_progress && $this->io->progressAdvance($path_count);
$output = $process->getOutput();
if (!empty($output) && $json = json_decode($output, TRUE)) {
$invoke_paths = array_unique(array_merge($invoke_paths, $json));
+
+ // don't include paths that are going to be exported by another process
+ $invoke_paths = array_diff($invoke_paths, $old_paths, $this->getInitialPaths());
}
});

- $invoke_paths = array_diff($invoke_paths, $old_paths);
+ $invoke_paths = array_diff($invoke_paths, $old_paths, $this->getInitialPaths());
$old_paths = array_merge($old_paths, $invoke_paths);

$show_progress && $this->io->progressFinish();
@@ -191,6 +207,7 @@ class StaticCommand extends CommandBase {
$this->displayErrors($collected_errors);
}
if (count($invoke_paths)) {
+ $this->init_paths = array_merge($this->init_paths, $invoke_paths);
$this->io->writeln('Processing related assets and paths...');
$this->exportPaths($invoke_paths, $old_paths, $process_count, $path_count, $show_progress, $retry_count, $uri);
}
diff --git a/modules/tome_static/src/Commands/StaticExportPathCommand.php b/modules/tome_static/src/Commands/StaticExportPathCommand.php
index 933321846edd33204522259f1411ff5eab404376..4ff8dd3e79a9cd9e5db72b8b7d0d0f2e73007d4a 100644
--- a/modules/tome_static/src/Commands/StaticExportPathCommand.php
+++ b/modules/tome_static/src/Commands/StaticExportPathCommand.php
@@ -68,6 +68,7 @@ class StaticExportPathCommand extends StaticCommand {
$this->requestPreparer->prepareForRequest();
try {
$invoke_paths = array_merge($this->static->requestPath($path), $invoke_paths);
+ $invoke_paths = array_unique($invoke_paths);
}
catch (\Exception $e) {
$this->io->getErrorStyle()->error($this->formatPathException($path, $e));
diff --git a/modules/tome_static/src/RequestPreparer.php b/modules/tome_static/src/RequestPreparer.php
index be08b0460e494b74492f3940634b8ab71aa1f900..7d514d82f689ddf203b6280dc1f702cfa4327159 100644
--- a/modules/tome_static/src/RequestPreparer.php
+++ b/modules/tome_static/src/RequestPreparer.php
@@ -110,7 +110,7 @@ class RequestPreparer {
}
// Reset active trail cache.
if ($this->menuActiveTrail instanceof CacheCollectorInterface) {
- $this->menuActiveTrail->reset();
+ $this->menuActiveTrail->clear();
}
// Reset the language manager.
$this->languageManager->reset();
2 changes: 1 addition & 1 deletion scripts/tome-static.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ echo "Starting Static Site Generation : "$(date)
mkdir -p /var/www/html
# time drush -vvv tome:static --uri=$URI --process-count=1 --path-count=1
# time drush tome:static -y --uri=$URI --process-count=5 --path-count=1
time drush tome:static -y --uri=$URI --process-count=$TOME_PROCESS_COUNT --path-count=1
time drush tome:static -y --uri=$URI --process-count=$TOME_PROCESS_COUNT --path-count=10
omerida marked this conversation as resolved.
Show resolved Hide resolved
TOME_SUCCESS=$?
echo "Finished Static Site Generation : "$(date)
exit $TOME_SUCCESS
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

namespace Drupal\usagov_ssg_postprocessing\EventSubscriber;

use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Routing\CurrentRouteMatch;
use Drupal\path_alias\AliasManager;
use Drupal\tome_static\Event\TomeStaticEvents;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
* Clears non-core caches between Tome requests.
*
* This event is useful when running tome with more than one path per process to
* clear any caches used by contrib modules.
*/
class RequestPrepareSubscriber implements EventSubscriberInterface {

public function __construct(
private AliasManager $alias_manager,
private EntityTypeManagerInterface $entity_type_manager,
private CurrentRouteMatch $currentRouteMatch,
) {}

/**
* Clear additional caches.
*
* Fixes issues found when tome export path count is greater than 1.
*/
public function requestPrepare(): void {
// Fixes redirects exporting with the target node's content instead
// of an HTML redirect.
$this->alias_manager->cacheClear();

$menuLinkStorage = $this->entity_type_manager->getStorage('menu_link_content');
$menuLinkStorage->resetCache();

$menuStorage = $this->entity_type_manager->getStorage('menu');
$menuStorage->resetCache();

$this->currentRouteMatch->resetRouteMatch();
}

/**
* {@inheritdoc}
*/
public static function getSubscribedEvents() {
$events[TomeStaticEvents::REQUEST_PREPARE][] = ['requestPrepare'];
return $events;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Drupal\Core\Site\Settings;
use Drupal\tome_static\Event\CollectPathsEvent;
use Drupal\tome_static\Event\ModifyHtmlEvent;
use Drupal\tome_static\Event\PathPlaceholderEvent;
use Drupal\tome_static\Event\TomeStaticEvents;
use Masterminds\HTML5;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
Expand Down Expand Up @@ -168,19 +169,38 @@ public function modifyHtml(ModifyHtmlEvent $event) {
}
}

// Never crawl the rewritten Spanish path. It might be treated like a redirect by
// Tome and overwrite the original homepage HTML
$event->addExcludePath('/es/');

if ($changes) {
// Render it as HTML5:
$modifiedHtml = $html5->saveHTML($document);
$event->setHtml($modifiedHtml);
}
}

/**
* Prevent exporting paths Tome might discover after the collect paths event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, specifically "prevent exporting node-based paths" ...

This is probably a separate can of worms, but I look at this simple function and wonder whether it might not be a better approach than what we currently have for excludeDirectories? That is, go ahead and let the all the paths be collected, and then mark the ones that match our exclusion rules as invalid?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be somewhat faster if you don't have to load the node entities too, no? Though I bet those are cached already...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect it to be. I'm certain that when I wrote that, I didn't think there was a way to get at the path alias without loading the node entity.

*
* @param PathPlaceholderEvent $event
* @return void
*/
public function excludeInvalidPaths(PathPlaceholderEvent $event) {
$path = $event->getPath();

if (preg_match('/(es\/)?node\/\d+$/', $path)) {
$event->setInvalid();
}
}

/**
* {@inheritdoc}
*/
public static function getSubscribedEvents() {
$events[TomeStaticEvents::MODIFY_HTML][] = ['modifyHtml'];
$events[TomeStaticEvents::COLLECT_PATHS][] = ['excludeDirectories'];
$events[TomeStaticEvents::PATH_PLACEHOLDER][] = ['excludeInvalidPaths'];
return $events;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,8 @@ services:
class: '\Drupal\usagov_ssg_postprocessing\EventSubscriber\PublishedPagesSubscriber'
tags:
- { name: 'event_subscriber' }
usagov_ssg_postprocessing_request_prepare_events_subscriber:
class: '\Drupal\usagov_ssg_postprocessing\EventSubscriber\RequestPrepareSubscriber'
arguments: ['@path_alias.manager', '@entity_type.manager', '@current_route_match']
tags:
- { name: 'event_subscriber' }
Loading