Skip to content

Commit

Permalink
Merge pull request #54 from City-of-Helsinki/UHF-9978
Browse files Browse the repository at this point in the history
UHF-9978: Limit main navigation to 2 levels, autowire services
  • Loading branch information
tuutti authored Apr 18, 2024
2 parents e0d8aa4 + 7a69c36 commit dfe38c9
Show file tree
Hide file tree
Showing 16 changed files with 102 additions and 70 deletions.
8 changes: 5 additions & 3 deletions helfi_navigation.module
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\Core\Utility\Error;
use Drupal\helfi_navigation\ApiAuthorization;
use Drupal\helfi_navigation\ApiManager;
use Drupal\menu_link_content\Entity\MenuLinkContent;
use Drupal\menu_link_content\MenuLinkContentInterface;
use Drupal\system\MenuInterface;
Expand Down Expand Up @@ -123,7 +125,7 @@ function helfi_navigation_menu_link_content_delete(MenuLinkContentInterface $ent
* Create menu update queue item.
*/
function _helfi_navigation_queue_item(string $menuName, string $langcode, string $action) : void {
if (!\Drupal::service('helfi_navigation.api_authorization')->getAuthorization()) {
if (!Drupal::service(ApiAuthorization::class)->getAuthorization()) {
return;
}
$queue = Drupal::queue('helfi_navigation_menu_queue');
Expand All @@ -150,7 +152,7 @@ function helfi_navigation_page_attachments(array &$attachments) : void {
$langcode = $defaultLanguageResolver->getCurrentOrFallbackLanguage();

/** @var \Drupal\helfi_navigation\ApiManager $apiManager */
$apiManager = Drupal::service('helfi_navigation.api_manager');
$apiManager = Drupal::service(ApiManager::class);

try {
// Ensure that drupalSettings library is enabled (menu library is not
Expand All @@ -164,7 +166,7 @@ function helfi_navigation_page_attachments(array &$attachments) : void {
],
];
}
catch (\Exception $e) {
catch (Exception $e) {
/** @var \Psr\Log\LoggerInterface $logger */
$logger = Drupal::service('logger.channel.helfi_navigation');

Expand Down
53 changes: 14 additions & 39 deletions helfi_navigation.services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,50 +2,25 @@ parameters:
helfi_navigation.request_timeout: 15
helfi_navigation.absolute_url_always: null
services:
_defaults:
autoconfigure: true
autowire: true

logger.channel.helfi_navigation:
parent: logger.channel_base
arguments: ['helfi_navigation']
helfi_navigation.external_menu_tree_builder:
class: Drupal\helfi_navigation\ExternalMenuTreeBuilder
arguments:
- '@helfi_api_base.internal_domain_resolver'
- '@request_stack'

Drupal\helfi_navigation\ExternalMenuTreeBuilder: ~

helfi_navigation.api_client:
parent: helfi_api_base.api_client_base
arguments:
- '@logger.channel.helfi_navigation'
- { timeout: '%helfi_navigation.request_timeout%' }
helfi_navigation.api_manager:
class: Drupal\helfi_navigation\ApiManager
arguments:
- '@helfi_navigation.api_client'
- '@helfi_api_base.environment_resolver'
- '@helfi_navigation.api_authorization'
helfi_navigation.menu_manager:
class: Drupal\helfi_navigation\MainMenuManager
arguments:
- '@language_manager'
- '@config.factory'
- '@helfi_navigation.api_manager'
- '@helfi_navigation.menu_tree_builder'
- '@account_switcher'
helfi_navigation.menu_tree_builder:
class: Drupal\helfi_navigation\Menu\MenuTreeBuilder
arguments:
- '@entity_type.manager'
- '@helfi_api_base.internal_domain_resolver'
- '@menu.link_tree'
- '@plugin.manager.menu.link'
- '@event_dispatcher'
helfi_navigation.api_authorization:
class: Drupal\helfi_navigation\ApiAuthorization
arguments:
- '@config.factory'
- '@helfi_api_base.vault_manager'
helfi_api_base.absolute_url_menu_tree_builder_subscriber:
class: Drupal\helfi_navigation\EventSubscriber\AbsoluteUrlMenuTreeBuilderLinkSubscriber
arguments:
- '@helfi_api_base.environment_resolver'
- '%helfi_navigation.absolute_url_always%'
tags:
- { name: event_subscriber }

Drupal\helfi_navigation\ApiManager: ~

Drupal\helfi_navigation\MainMenuManager: ~
Drupal\helfi_navigation\Menu\MenuTreeBuilder: ~
Drupal\helfi_navigation\ApiAuthorization: ~
Drupal\helfi_navigation\EventSubscriber\AbsoluteUrlMenuTreeBuilderLinkSubscriber: ~
2 changes: 0 additions & 2 deletions phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
colors="true"
cacheResultFile=".phpunit.cache/test-results"
executionOrder="depends,defects"
forceCoversAnnotation="true"
beStrictAboutTestsThatDoNotTestAnything="true"
beStrictAboutOutputDuringTests="true"
beStrictAboutChangesToGlobalState="true"
beStrictAboutCoversAnnotation="true"
printerClass="\Drupal\Tests\Listeners\HtmlOutputPrinter"
failOnRisky="true"
failOnWarning="true"
Expand Down
12 changes: 10 additions & 2 deletions src/ApiManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Drupal\helfi_api_base\Cache\CacheKeyTrait;
use Drupal\helfi_api_base\Environment\EnvironmentResolverInterface;
use Drupal\helfi_api_base\Environment\Project;
use Symfony\Component\DependencyInjection\Attribute\Autowire;

/**
* Service class for global navigation-related functions.
Expand All @@ -19,7 +20,14 @@ class ApiManager {

public const GLOBAL_MENU_ENDPOINT = '/api/v1/global-menu';
public const MENU_ENDPOINT = '/api/v1/menu';
public const TTL = 180;

/**
* Cache menu data for one month.
*
* The response cache is flushed by 'helfi_navigation_menu_queue'
* queue worker.
*/
public const TTL = 2629800;

use CacheKeyTrait;

Expand All @@ -34,7 +42,7 @@ class ApiManager {
* The API authorization service.
*/
public function __construct(
private ApiClient $client,
#[Autowire(service: 'helfi_navigation.api_client')] private ApiClient $client,
private readonly EnvironmentResolverInterface $environmentResolver,
private readonly ApiAuthorization $apiAuthorization,
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Drupal\helfi_api_base\Environment\EnvironmentResolverInterface;
use Drupal\helfi_api_base\Environment\Project;
use Drupal\helfi_navigation\Event\MenuTreeBuilderLink;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
Expand All @@ -24,7 +25,7 @@ final class AbsoluteUrlMenuTreeBuilderLinkSubscriber implements EventSubscriberI
*/
public function __construct(
private readonly EnvironmentResolverInterface $environmentResolver,
private ?bool $mustBeAbsoluteUrl = NULL,
#[Autowire('%helfi_navigation.absolute_url_always%')] private ?bool $mustBeAbsoluteUrl = NULL,
) {
}

Expand Down
2 changes: 1 addition & 1 deletion src/HelfiNavigationServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public function register(ContainerBuilder $container) {
$modules = $container->getParameter('container.modules');

if (isset($modules['redirect'])) {
$container->register('helfi_navigation.redirect_subscriber', RedirectEventSubscriber::class)
$container->register(RedirectEventSubscriber::class)
->addTag('event_subscriber')
->addArgument(new Reference('redirect.repository'))
->addArgument(new Reference('config.factory'));
Expand Down
3 changes: 2 additions & 1 deletion src/MainMenuManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Drupal\Core\Url;
use Drupal\helfi_navigation\Menu\MenuTreeBuilder;
use Drupal\language\ConfigurableLanguageManagerInterface;
use Symfony\Component\DependencyInjection\Attribute\Autowire;

/**
* Menu manager service.
Expand All @@ -31,7 +32,7 @@ class MainMenuManager {
* The account switcher service.
*/
public function __construct(
private readonly ConfigurableLanguageManagerInterface $languageManager,
#[Autowire(service: 'language_manager')] private readonly ConfigurableLanguageManagerInterface $languageManager,
private readonly ConfigFactoryInterface $config,
private readonly ApiManager $apiManager,
private readonly MenuTreeBuilder $menuTreeBuilder,
Expand Down
4 changes: 2 additions & 2 deletions src/Menu/MenuTreeBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use Drupal\helfi_api_base\Link\InternalDomainResolver;
use Drupal\helfi_navigation\Event\MenuTreeBuilderLink;
use Drupal\menu_link_content\MenuLinkContentInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;

/**
* Create menu tree from Drupal menu.
Expand All @@ -34,7 +34,7 @@ final class MenuTreeBuilder {
* The 'menu link tree builder' service.
* @param \Drupal\Core\Menu\MenuLinkManagerInterface $menuLinkManager
* The menu link manager.
* @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $eventDispatcher
* @param \Symfony\Contracts\EventDispatcher\EventDispatcherInterface $eventDispatcher
* The event dispatcher.
*/
public function __construct(
Expand Down
15 changes: 13 additions & 2 deletions src/Plugin/Block/ExternalMenuBlockBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ abstract class ExternalMenuBlockBase extends MenuBlockBase implements ExternalMe
*/
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) : static {
$instance = parent::create($container, $configuration, $plugin_id, $plugin_definition);
$instance->apiManager = $container->get('helfi_navigation.api_manager');
$instance->menuTreeBuilder = $container->get('helfi_navigation.external_menu_tree_builder');
$instance->apiManager = $container->get(ApiManager::class);
$instance->menuTreeBuilder = $container->get(ExternalMenuTreeBuilder::class);
$instance->languageManager = $container->get('language_manager');
$instance->defaultLanguageResolver = $container->get('helfi_api_base.default_language_resolver');
return $instance;
Expand Down Expand Up @@ -88,6 +88,16 @@ public function getCacheContexts() : array {
*/
abstract protected function getTreeFromResponse(ApiResponse $response) : mixed;

/**
* Gets the request options.
*
* @return array
* The request options.
*/
protected function getRequestOptions() : array {
return [];
}

/**
* {@inheritdoc}
*/
Expand All @@ -109,6 +119,7 @@ public function build() : array {
$response = $this->apiManager->get(
$langcode,
$menuId,
$this->getRequestOptions(),
);
$menuTree = $this->menuTreeBuilder
->build($this->getTreeFromResponse($response), $this->getOptions());
Expand Down
9 changes: 9 additions & 0 deletions src/Plugin/Block/MainNavigationMenuBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ public function getDerivativeId() : string {
return 'main';
}

/**
* {@inheritdoc}
*/
protected function getRequestOptions() : array {
return [
'query' => 'max-depth=' . $this->getMaxDepth(),
];
}

/**
* {@inheritdoc}
*/
Expand Down
2 changes: 1 addition & 1 deletion src/Plugin/Block/MobileMenuFallbackBlock.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public static function create(
$instance = parent::create($container, $configuration, $plugin_id, $plugin_definition);
$instance->configFactory = $container->get('config.factory');
$instance->pathMatcher = $container->get('path.matcher');
$instance->apiManager = $container->get('helfi_navigation.api_manager');
$instance->apiManager = $container->get(ApiManager::class);
$instance->defaultLanguageResolver = $container->get('helfi_api_base.default_language_resolver');
$instance->filterByLanguage = $container->has('menu_block_current_language_tree_manipulator');
$instance->menuLinkManager = $container->get('plugin.manager.menu.link');
Expand Down
8 changes: 4 additions & 4 deletions src/Plugin/QueueWorker/MenuQueue.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\Core\Queue\QueueWorkerBase;
use Drupal\helfi_api_base\Cache\CacheTagInvalidator;
use Drupal\helfi_api_base\Cache\CacheTagInvalidatorInterface;
use Drupal\helfi_api_base\Environment\Project;
use Drupal\helfi_navigation\MainMenuManager;
use Symfony\Component\DependencyInjection\ContainerInterface;
Expand All @@ -32,16 +32,16 @@ final class MenuQueue extends QueueWorkerBase implements ContainerFactoryPluginI
/**
* The cache tag invalidator service.
*
* @var \Drupal\helfi_api_base\Cache\CacheTagInvalidator
* @var \Drupal\helfi_api_base\Cache\CacheTagInvalidatorInterface
*/
private CacheTagInvalidator $cacheTagInvalidator;
private CacheTagInvalidatorInterface $cacheTagInvalidator;

/**
* {@inheritdoc}
*/
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) : self {
$instance = new self($configuration, $plugin_id, $plugin_definition);
$instance->mainMenuManager = $container->get('helfi_navigation.menu_manager');
$instance->mainMenuManager = $container->get(MainMenuManager::class);
$instance->cacheTagInvalidator = $container->get('helfi_api_base.cache_tag_invalidator');
return $instance;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Plugin/rest/resource/GlobalMobileMenu.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ public static function create(ContainerInterface $container, array $configuratio
$instance = parent::create($container, $configuration, $plugin_id, $plugin_definition);
$instance->configFactory = $container->get('config.factory');
$instance->languageManager = $container->get('language_manager');
$instance->apiManager = $container->get('helfi_navigation.api_manager');
$instance->apiManager = $container->get(ApiManager::class);
$instance->environmentResolver = $container->get('helfi_api_base.environment_resolver');
$instance->mainMenuManager = $container->get('helfi_navigation.menu_manager');
$instance->mainMenuManager = $container->get(MainMenuManager::class);

return $instance;
}
Expand Down
6 changes: 3 additions & 3 deletions tests/src/Kernel/MenuSyncTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ private function getQueue() : QueueInterface {
* The menu manager service.
*/
private function getMenuManager() : MainMenuManager {
return $this->container->get('helfi_navigation.menu_manager');
return $this->container->get(MainMenuManager::class);
}

/**
Expand Down Expand Up @@ -144,7 +144,7 @@ public function testConfigTranslation(string $langcode) : void {
],
]);
}));
$this->container->set('helfi_navigation.api_manager', $apiManager);
$this->container->set(ApiManager::class, $apiManager);

$this->getMenuManager()->sync($langcode);
}
Expand Down Expand Up @@ -172,7 +172,7 @@ public function testEmptyStatus() : void {
$apiManager->expects($this->once())
->method('update')
->willReturn(new ApiResponse([]));
$this->container->set('helfi_navigation.api_manager', $apiManager);
$this->container->set(ApiManager::class, $apiManager);

$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Failed to parse entity published state.');
Expand Down
2 changes: 1 addition & 1 deletion tests/src/Kernel/MenuTreeBuilderTestBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected function setUp(): void {
* The menu tree builder service.
*/
protected function getMenuTreeBuilder() : MenuTreeBuilder {
return $this->container->get('helfi_navigation.menu_tree_builder');
return $this->container->get(MenuTreeBuilder::class);
}

/**
Expand Down
Loading

0 comments on commit dfe38c9

Please sign in to comment.