Skip to content

Commit f65ec25

Browse files
authored
Frontend module cleanup (#4699)
1 parent 1572e82 commit f65ec25

5 files changed

Lines changed: 223 additions & 106 deletions

File tree

modules/dkan_js_frontend/dkan_js_frontend.module

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* This is the DKAN JS Frontend module for using a decoupled frontend w/ DKAN.
66
*/
77

8+
use Drupal\dkan_js_frontend\Routing\RouteProvider;
89
use Symfony\Component\HttpFoundation\Request;
910
use Symfony\Component\Routing\Generator\UrlGenerator;
1011
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
@@ -13,10 +14,12 @@ use Symfony\Component\Routing\Route;
1314
use Symfony\Component\Routing\RouteCollection;
1415

1516
const DKAN_JS_FRONTEND_MISSING_DATASET_ROUTE_ERROR = 'Missing required DKAN route "dataset"; unable to build sitemap for dataset URLs.';
17+
1618
const DKAN_JS_FRONTEND_DEFAULT_STATIC_LINK = [
1719
'priority' => '0.7',
1820
'changefreq' => 'daily',
1921
];
22+
2023
const DKAN_JS_FRONTEND_DEFAULT_DATASET_LINK = [
2124
'priority' => '0.5',
2225
'changefreq' => 'weekly',
@@ -95,12 +98,15 @@ function dkan_js_frontend_library_info_build() {
9598
/**
9699
* Implements hook_page_attachments().
97100
*
98-
* Any route with a 'name' default with a value of dkan_js_frontend gets the
99-
* library attached.
101+
* Any route with a '_is_dkan_js_frontend' default with a value of true gets
102+
* the library attached.
103+
*
104+
* @see \Drupal\dkan_js_frontend\Routing\RouteProvider::addRoutesFromConfig()
100105
*/
101106
function dkan_js_frontend_page_attachments(array &$page) {
102-
$request = \Drupal::routeMatch()->getRouteObject()->getDefault('name');
103-
if ($request == 'dkan_js_frontend') {
107+
if (
108+
\Drupal::routeMatch()->getRouteObject()->getDefault('_is_dkan_js_frontend') === 'true'
109+
) {
104110
$page['#attached']['library'][] = 'dkan_js_frontend/dkan_js_frontend';
105111
}
106112
}
@@ -109,8 +115,9 @@ function dkan_js_frontend_page_attachments(array &$page) {
109115
* Implements hook_theme_suggestions_HOOK_alter().
110116
*/
111117
function dkan_js_frontend_theme_suggestions_page_alter(array &$suggestions, array $variables) {
112-
$request = \Drupal::routeMatch()->getRouteObject()->getDefault('name');
113-
if ($request == 'dkan_js_frontend') {
118+
if (
119+
\Drupal::routeMatch()->getRouteObject()->getDefault('_is_dkan_js_frontend') === 'true'
120+
) {
114121
$suggestions[] = 'page__dkan_js_frontend';
115122
}
116123
}
@@ -151,7 +158,7 @@ function dkan_js_frontend_simple_sitemap_arbitrary_links_alter(array &$arbitrary
151158
$request_context = _dkan_js_frontend_build_request_context();
152159
_dkan_js_frontend_add_static_links($arbitrary_links, $routes, $request_context);
153160
// If the 'dataset' route exists, add links for the dataset route.
154-
if ($dataset_route = $routes->get('dataset')) {
161+
if ($dataset_route = $routes->get(RouteProvider::ROUTE_PREFIX . 'dataset')) {
155162
_dkan_js_frontend_add_dataset_links($arbitrary_links, $dataset_route, $request_context);
156163
}
157164
else {
@@ -209,9 +216,10 @@ function _dkan_js_frontend_add_static_links(array &$arbitrary_links, RouteCollec
209216
* Request context containing base URL for sitemap URL generation.
210217
*/
211218
function _dkan_js_frontend_add_dataset_links(array &$arbitrary_links, Route $dataset_route, RequestContext $request_context): void {
219+
$dataset_route_name = RouteProvider::ROUTE_PREFIX . 'dataset';
212220
// Build route collection.
213221
$routes = new RouteCollection();
214-
$routes->add('dataset', $dataset_route);
222+
$routes->add($dataset_route_name, $dataset_route);
215223
// Build route URL generator.
216224
$url_generator = new UrlGenerator($routes, $request_context);
217225

@@ -220,7 +228,7 @@ function _dkan_js_frontend_add_dataset_links(array &$arbitrary_links, Route $dat
220228
// Add dataset routes using the fetched UUIDs.
221229
foreach ($dataset_uuids as $uuid) {
222230
$arbitrary_links[] = DKAN_JS_FRONTEND_DEFAULT_DATASET_LINK + [
223-
'url' => $url_generator->generate('dataset', ['id' => $uuid], UrlGeneratorInterface::ABSOLUTE_URL),
231+
'url' => $url_generator->generate($dataset_route_name, ['id' => $uuid], UrlGeneratorInterface::ABSOLUTE_URL),
224232
];
225233
}
226234

modules/dkan_js_frontend/src/Controller/Page.php

Lines changed: 98 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2,90 +2,137 @@
22

33
namespace Drupal\dkan_js_frontend\Controller;
44

5-
use Drupal\Core\Controller\ControllerBase;
5+
use Drupal\Core\Cache\Cache;
6+
use Drupal\Core\Cache\CacheableMetadata;
7+
use Drupal\Core\Config\ConfigFactoryInterface;
8+
use Drupal\Core\Config\ImmutableConfig;
69
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
7-
use Drupal\Core\Path\CurrentPathStack;
10+
use Drupal\Core\Http\Exception\CacheableNotFoundHttpException;
11+
use Drupal\Core\Render\RendererInterface;
12+
use Drupal\Core\Routing\RouteMatchInterface;
13+
use Drupal\dkan_js_frontend\Routing\RouteProvider;
814
use Drupal\dkan_metastore\Exception\MissingObjectException;
9-
use Drupal\dkan_metastore\MetastoreService;
15+
use Drupal\dkan_metastore\NodeWrapper\NodeDataFactory;
1016
use Symfony\Component\DependencyInjection\ContainerInterface;
11-
use Symfony\Component\HttpFoundation\RequestStack;
12-
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
17+
use Symfony\Component\HttpFoundation\Request;
1318

1419
/**
15-
* The Page controller.
20+
* Page controller.
21+
*
22+
* Routes defined in the dkan_js_frontend.config.routes configuration use this
23+
* controller.
24+
*
25+
* For datastore routes, we check if the datastore identifier is valid, and if
26+
* not throw an exception signaling a 404 response.
1627
*/
17-
class Page extends ControllerBase implements ContainerInjectionInterface {
28+
class Page implements ContainerInjectionInterface {
1829

1930
/**
20-
* Metastore service.
21-
*/
22-
private MetastoreService $metastoreService;
23-
24-
/**
25-
* The request stack.
31+
* Config for dkan_js_frontend.
32+
*
33+
* @var \Drupal\Core\Config\ImmutableConfig
2634
*/
27-
protected RequestStack $requestStack;
35+
protected readonly ImmutableConfig $frontendConfig;
2836

2937
/**
30-
* The current path.
38+
* Node data factory service.
39+
*
40+
* @var \Drupal\dkan_metastore\NodeWrapper\NodeDataFactory
3141
*/
32-
protected CurrentPathStack $currentPath;
42+
protected readonly NodeDataFactory $nodeDataFactory;
3343

3444
/**
35-
* Inherited.
36-
*
37-
* {@inheritdoc}
45+
* {@inheritDoc}
3846
*/
3947
public static function create(ContainerInterface $container) {
4048
return new static(
41-
$container->get('dkan.metastore.service'),
42-
$container->get('path.current'),
43-
$container->get('request_stack'),
49+
$container->get('config.factory'),
50+
$container->get('dkan.metastore.metastore_item_factory'),
4451
);
4552
}
4653

4754
/**
4855
* Constructor.
56+
*
57+
* @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
58+
* Config factory service.
59+
* @param \Drupal\dkan_metastore\NodeWrapper\NodeDataFactory $nodeDataFactory
60+
* Node data factory service.
4961
*/
50-
public function __construct(MetastoreService $service, CurrentPathStack $current_path, RequestStack $request_stack) {
51-
$this->metastoreService = $service;
52-
$this->currentPath = $current_path;
53-
$this->requestStack = $request_stack;
62+
public function __construct(
63+
ConfigFactoryInterface $configFactory,
64+
NodeDataFactory $nodeDataFactory,
65+
) {
66+
$this->frontendConfig = $configFactory->get('dkan_js_frontend.config');
67+
$this->nodeDataFactory = $nodeDataFactory;
5468
}
5569

5670
/**
57-
* Returns a render-able array.
58-
*/
59-
public function content() {
60-
// Checking for 404 prevents an infinite loop.
61-
if ($this->requestStack->getCurrentRequest()->query->get('_exception_statuscode') !== 404) {
62-
$this->handleInvalidDatasetId();
63-
}
64-
65-
return [
66-
'#theme' => 'page__dkan_js_frontend',
67-
];
68-
}
69-
70-
/**
71-
* If a dataset with an invalid ID is being requested, throw a 404 error.
71+
* Make a renderable page.
72+
*
73+
* @param \Drupal\Core\Routing\RouteMatchInterface $route_match
74+
* Route match for this request.
75+
* @param \Symfony\Component\HttpFoundation\Request $request
76+
* This request.
77+
*
78+
* @return array
79+
* Render array.
80+
*
81+
* @throws \Symfony\Component\HttpKernel\Exception\NotFoundHttpException
82+
* Throws a not-found exception for dataset routes which have an invalid
83+
* identifier.
7284
*/
73-
protected function handleInvalidDatasetId() {
74-
// Path should always have leading slash.
75-
// @see \Symfony\Component\HttpFoundation\Request::getPathInfo()
76-
// Match any path that equals or begins with /dataset/[ID].
77-
$dataset_path_match = '/^\/dataset\/(?P<id>[^\/]+)/';
85+
public function content(RouteMatchInterface $route_match, Request $request) {
86+
$cacheable_metadata = (new CacheableMetadata())
87+
// This is the default but let's make it explicit.
88+
->setCacheMaxAge(Cache::PERMANENT)
89+
// Allow cache invalidation if we change config for this module.
90+
->addCacheableDependency($this->frontendConfig)
91+
// Allow cache invalidation per NodeDataFactory. In practice, this is any
92+
// change to any data nodes. This is a wide net to allow for reasonably
93+
// easy cache invalidation.
94+
->addCacheTags(NodeDataFactory::getCacheTags());
7895

79-
$path = $this->currentPath->getPath();
80-
81-
if (preg_match($dataset_path_match, $path, $matches)) {
96+
// Check for valid dataset identifier on the dataset/api routes.
97+
// Checking for 404 prevents an infinite loop from the 404 we might cause
98+
// later.
99+
// @todo Make the dataset and dataset API routes their own controller and
100+
// config.
101+
if (
102+
in_array($route_match->getRouteName(), [
103+
RouteProvider::ROUTE_PREFIX . 'dataset',
104+
RouteProvider::ROUTE_PREFIX . 'datasetapi',
105+
]) &&
106+
($request->query->get('_exception_statuscode') !== 404)
107+
) {
82108
try {
83-
$this->metastoreService->get('dataset', $matches['id']);
109+
// Does this dataset exist?
110+
// @todo Figure out a way to find if the identifier exists without
111+
// triggering the lifecycle loading of tertiary nodes, etc.
112+
$dataset_item = $this->nodeDataFactory->getInstance(
113+
$route_match->getRawParameter('id') ?? ''
114+
);
115+
// Allow cache invalidation if our dataset item changes at all.
116+
// @todo Ideally DKAN would give us an easy way to also get
117+
// cacheability metadata from other related nodes, such as
118+
// distribution.
119+
$cacheable_metadata->addCacheableDependency($dataset_item);
84120
}
121+
// Handle if the dataset does not exist.
85122
catch (MissingObjectException) {
86-
throw new NotFoundHttpException();
123+
// Throw an exception that tells Drupal send back a 404. Also use the
124+
// same caching, so that we won't query the DB about a given known-bad
125+
// identifier until after the cache is invalidated.
126+
throw new CacheableNotFoundHttpException($cacheable_metadata);
87127
}
88128
}
129+
130+
$build = [
131+
'#theme' => 'page__dkan_js_frontend',
132+
];
133+
$cacheable_metadata->applyTo($build);
134+
135+
return $build;
89136
}
90137

91138
}

modules/dkan_js_frontend/src/Routing/RouteProvider.php

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
*/
1212
class RouteProvider {
1313

14+
const ROUTE_PREFIX = 'dkan_js_frontend.';
15+
1416
/**
1517
* Route-URL pairs, separated by a comma.
1618
*
@@ -46,8 +48,9 @@ public function routes(): RouteCollection {
4648
/**
4749
* Add all the routes specified in configuration.
4850
*
49-
* Routes added here are marked with a default property 'name' with a value
50-
* of 'dkan_js_frontend'. This allows for select attachment of libraries.
51+
* Routes added here are marked with a default property
52+
* '_is_dkan_js_frontend' with a value of 'true'. This allows for select
53+
* attachment of libraries.
5154
*
5255
* @param \Symfony\Component\Routing\RouteCollection $routes
5356
* The collection to add config routes to.
@@ -61,11 +64,14 @@ private function addRoutesFromConfig(RouteCollection $routes): void {
6164
'/' . $possible_page[1],
6265
[
6366
'_controller' => '\Drupal\dkan_js_frontend\Controller\Page::content',
64-
'name' => 'dkan_js_frontend',
65-
]
67+
'_is_dkan_js_frontend' => 'true',
68+
],
6669
);
6770
$route->setMethods(['GET']);
68-
$routes->add($possible_page[0], $route);
71+
$route->addRequirements([
72+
'_permission' => 'access content',
73+
]);
74+
$routes->add(self::ROUTE_PREFIX . $possible_page[0], $route);
6975
}
7076
}
7177

0 commit comments

Comments
 (0)