Issue #3048395 by Pancho: Cleanup FillPdfAccessController and FillPdfAccessHelper

src/FillPdfAccessController.php

@@ -10,41 +10,96 @@ use Drupal\Core\Logger\LoggerChannelTrait;
 use Drupal\Core\Messenger\MessengerTrait;
 use Drupal\Core\Session\AccountInterface;
 
+/**
+ * Defines a custom access controller for the FillPDF generation route.
+ */
 class FillPdfAccessController implements ContainerInjectionInterface {
 
   use MessengerTrait;
   use LoggerChannelTrait;
 
-  /** @var \Drupal\fillpdf\FillPdfAccessHelperInterface */
+  /**
+   * The FillPDF access helper.
+   *
+   * @var \Drupal\fillpdf\FillPdfAccessHelperInterface
+   */
   protected $accessHelper;
 
-  /** @var FillPdfLinkManipulatorInterface */
+  /**
+   * The FillPDF link manipulator.
+   *
+   * @var \Drupal\fillpdf\FillPdfLinkManipulatorInterface
+   */
   protected $linkManipulator;
 
-  /** @var RequestStack $requestStack */
-  protected $requestStack;
-
-  /** @var FillPdfContextManagerInterface */
+  /**
+   * The FillPDF context manager.
+   *
+   * @var \Drupal\fillpdf\FillPdfContextManagerInterface
+   */
   protected $contextManager;
 
-  /** @var AccountInterface $currentUser */
+  /**
+   * The request stack.
+   *
+   * @var \Symfony\Component\HttpFoundation\RequestStack
+   */
+  protected $requestStack;
+
+  /**
+   * The current user.
+   *
+   * @var \Drupal\Core\Session\AccountInterface
+   */
   protected $currentUser;
 
-  public function __construct(FillPdfAccessHelperInterface $access_helper, FillPdfLinkManipulatorInterface $link_manipulator, FillPdfContextManagerInterface $context_manager, RequestStack $request_stack, AccountInterface $current_user) {
+  /**
+   * Constructs a FillPdfAccessManager object.
+   *
+   * @param \Drupal\fillpdf\FillPdfAccessHelperInterface $access_helper
+   *   The FillPDF access helper.
+   * @param \Drupal\fillpdf\FillPdfLinkManipulatorInterface $link_manipulator
+   *   The FillPDF link manipulator.
+   * @param \Drupal\fillpdf\FillPdfContextManagerInterface $context_manager
+   *   The FillPDF context manager.
+   * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
+   *   The request stack.
+   * @param \Drupal\Core\Session\AccountInterface $current_user
+   *   The current user.
+   */
+  public function __construct(
+    FillPdfAccessHelperInterface $access_helper,
+    FillPdfLinkManipulatorInterface $link_manipulator,
+    FillPdfContextManagerInterface $context_manager,
+    RequestStack $request_stack,
+    AccountInterface $current_user
+  ) {
+    $this->accessHelper = $access_helper;
     $this->linkManipulator = $link_manipulator;
     $this->contextManager = $context_manager;
     $this->requestStack = $request_stack;
     $this->currentUser = $current_user;
-    $this->accessHelper = $access_helper;
   }
 
   /**
-   * {@inheritDoc}
+   * {@inheritdoc}
    */
   public static function create(ContainerInterface $container) {
-    return new static($container->get('fillpdf.access_helper'), $container->get('fillpdf.link_manipulator'), $container->get('fillpdf.context_manager'), $container->get('request_stack'), $container->get('current_user'));
+    return new static(
+      $container->get('fillpdf.access_helper'),
+      $container->get('fillpdf.link_manipulator'),
+      $container->get('fillpdf.context_manager'),
+      $container->get('request_stack'),
+      $container->get('current_user')
+    );
   }
 
+  /**
+   * Checks whether the current user has access to the current request.
+   *
+   * @return \Drupal\Core\Access\AccessResultInterface
+   *   Access result value object.
+   */
   public function checkLink() {
     try {
       $context = $this->linkManipulator->parseRequest($this->requestStack->getCurrentRequest());

src/FillPdfAccessHelper.php

@@ -3,11 +3,8 @@
 namespace Drupal\fillpdf;
 
 use Drupal\Core\Access\AccessResult;
-use Drupal\Core\Entity\EntityInterface;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Url;
-use Drupal\fillpdf\Service\FillPdfLinkManipulator;
-use Drupal\fillpdf\Service\FillPdfContextManager;
 
 /**
  * Class FillPdfAccessHelper.
@@ -17,27 +14,35 @@ use Drupal\fillpdf\Service\FillPdfContextManager;
 class FillPdfAccessHelper implements FillPdfAccessHelperInterface {
 
   /**
-   * Drupal\fillpdf\Service\FillPdfLinkManipulator definition.
+   * The FillPDF link manipulator.
    *
    * @var \Drupal\fillpdf\Service\FillPdfLinkManipulator
    */
   protected $linkManipulator;
 
   /**
-   * Drupal\fillpdf\Service\FillPdfContextManager definition.
+   * The FillPDF context manager.
    *
    * @var \Drupal\fillpdf\Service\FillPdfContextManager
    */
   protected $contextManager;
 
   /**
-   * Constructor.
+   * Constructs a FillPdfAccessManager object.
+   *
+   * @param \Drupal\fillpdf\FillPdfLinkManipulatorInterface $link_manipulator
+   *   The FillPDF link manipulator.
+   * @param \Drupal\fillpdf\FillPdfContextManagerInterface $context_manager
+   *   The FillPDF context manager.
    */
-  public function __construct(FillPdfLinkManipulator $link_manipulator, FillPdfContextManager $context_manager) {
+  public function __construct(FillPdfLinkManipulatorInterface $link_manipulator, FillPdfContextManagerInterface $context_manager) {
     $this->linkManipulator = $link_manipulator;
     $this->contextManager = $context_manager;
   }
 
+  /**
+   * {@inheritdoc}
+   */
   public function canGeneratePdfFromUrlString($url, AccountInterface $account) {
     $context = $this->linkManipulator->parseUrlString($url);
     return $this->canGeneratePdfFromContext($context, $account);
@@ -76,11 +81,11 @@ class FillPdfAccessHelper implements FillPdfAccessHelperInterface {
     if (!$is_sample && $can_publish) {
       $entities = $this->contextManager->loadEntities($context);
 
-      foreach ($entities as $entity_type => $entity_objects) {
-        // If there are any entities in the context that the user can't view,
-        // deny access.
-        /** @var EntityInterface $entity */
-        foreach ($entity_objects as $entity) {
+      /** @var \Drupal\Core\Entity\EntityInterface[] $type_entities */
+      foreach ($entities as $type_entities) {
+        foreach ($type_entities as $entity) {
+          // If there are any entities in the context that the user can't view,
+          // deny access.
           if (!$entity->access('view', $account)) {
             return $cachedForbidden;
           }

src/FillPdfAccessHelperInterface.php

 <?php
 
 namespace Drupal\fillpdf;
+
 use Drupal\Core\Access\AccessResultInterface;
 use Drupal\Core\Session\AccountInterface;
 use Drupal\Core\Url;
@@ -13,51 +14,51 @@ use Drupal\Core\Url;
 interface FillPdfAccessHelperInterface {
 
   /**
-   * Provides a way to pass in a FillPDF Link string to check access. Should
-   * ultimately pass control to self::canGeneratePdfFromContext().
+   * Provides a way to pass in a FillPDF Link string to check access.
+   *
+   * Should ultimately pass control to self::canGeneratePdfFromContext().
    *
    * @param string $url
    * The root-relative FillPDF URL that would be used to generate the PDF.
    * e.g. /fillpdf?fid=1&entity_type=node&entity_id=1
-   *
    * @param \Drupal\Core\Session\AccountInterface $account
    *
-   * @see canGeneratePdfFromContext()
+   * @return \Drupal\Core\Access\AccessResultInterface
    *
-   * @return AccessResultInterface
+   * @see \Drupal\fillpdf\FillPdfAccessHelperInterface::canGeneratePdfFromContext()
    */
   public function canGeneratePdfFromUrlString($url, AccountInterface $account);
 
   /**
-   * Provides a way to check access from a link argument. This function should
-   * build a FillPdfLinkManipulator-compatible $context and then pass control
-   * to self::canGeneratePdfFromLink().
+   * Provides a way to check access from a link argument.
    *
-   * @param \Drupal\Core\Url $link
-   * The FillPDF Link containing the entities whose access to check.
+   * This function should build a FillPdfLinkManipulator-compatible $context and
+   * then pass control to self::canGeneratePdfFromLink().
    *
+   * @param \Drupal\Core\Url $link
+   *   The FillPDF Link containing the entities whose access to check.
    * @param \Drupal\Core\Session\AccountInterface $account
-   * The user whose access is being checked.
+   *   The user whose access is being checked.
    *
-   * @see canGeneratePdfFromContext()
+   * @return \Drupal\Core\Access\AccessResultInterface
    *
-   * @return AccessResultInterface
+   * @see \Drupal\fillpdf\FillPdfAccessHelperInterface::canGeneratePdfFromContext()
    */
   public function canGeneratePdfFromLink(Url $link, AccountInterface $account);
 
   /**
    * This is the main access checking function of this class.
+   *
    * self::canGeneratePdfFromLinkUrl() should delegate to this one.
    *
    * @param array $context
-   * As returned by FillPdfLinkManipulator's parse functions.
-   *
+   *   As returned by FillPdfLinkManipulator's parse functions.
    * @param \Drupal\Core\Session\AccountInterface $account
-   * The user whose access is being checked.
+   *   The user whose access is being checked.
    *
-   * @see canGeneratePdfFromLink()
+   * @return \Drupal\Core\Access\AccessResultInterface
    *
-   * @return AccessResultInterface
+   * @see \Drupal\fillpdf\FillPdfAccessHelperInterface::canGeneratePdfFromLink()
    */
   public function canGeneratePdfFromContext(array $context, AccountInterface $account);