Issue #2359213: Normalized token replacement.

Turns out tokens don't get replaced with empty values anyway, so reversing
the replacement order was good enough!

src/Controller/HandlePdfController.php

@@ -39,23 +39,19 @@ class HandlePdfController extends ControllerBase {
   /** @var QueryFactory $entityQuery */
   protected $entityQuery;
 
-  /** @var Token $token */
-  protected $token;
-
   /** @var FillPdfContextManagerInterface $contextManager */
   protected $contextManager;
 
   /** @var TokenResolverInterface */
   protected $tokenResolver;
 
-  public function __construct(FillPdfLinkManipulatorInterface $link_manipulator, FillPdfContextManagerInterface $context_manager, TokenResolverInterface $token_resolver, RequestStack $request_stack, FillPdfBackendManager $backend_manager, FillPdfActionPluginManager $action_manager, Token $token, QueryFactory $entity_query) {
+  public function __construct(FillPdfLinkManipulatorInterface $link_manipulator, FillPdfContextManagerInterface $context_manager, TokenResolverInterface $token_resolver, RequestStack $request_stack, FillPdfBackendManager $backend_manager, FillPdfActionPluginManager $action_manager, QueryFactory $entity_query) {
     $this->linkManipulator = $link_manipulator;
     $this->contextManager = $context_manager;
     $this->tokenResolver = $token_resolver;
     $this->requestStack = $request_stack;
     $this->backendManager = $backend_manager;
     $this->actionManager = $action_manager;
-    $this->token = $token;
     $this->entityQuery = $entity_query;
   }
 
@@ -70,7 +66,6 @@ class HandlePdfController extends ControllerBase {
       $container->get('request_stack'),
       $container->get('plugin.manager.fillpdf_backend'),
       $container->get('plugin.manager.fillpdf_action.processor'),
-      $container->get('token'),
       $container->get('entity.query')
     );
   }
@@ -123,7 +118,7 @@ class HandlePdfController extends ControllerBase {
       }
     }
 
-    $populated_pdf = $backend->populateWithFieldData($fillpdf_form, $field_mapping, $context);
+    $populated_pdf = $backend->populateWithFieldData($fillpdf_form, $field_mapping, $context, $entities);
 
     // @todo: When Rules integration ported, emit an event or whatever.
 
@@ -146,12 +141,12 @@ class HandlePdfController extends ControllerBase {
    *   The FillPDF request context as parsed by
    *   \Drupal\fillpdf\Service\LinkManipulator.
    *
-   * @param array $token_objects
+   * @param array $entities
    *   An array of objects to be used in replacing tokens.
    *   Here, specifically, it's for generating the filename of the handled PDF.
    * @return NULL|\Symfony\Component\HttpFoundation\Response
    */
-  protected function handlePopulatedPdf(FillPdfFormInterface $fillpdf_form, $pdf_data, $context, array $token_objects) {
+  protected function handlePopulatedPdf(FillPdfFormInterface $fillpdf_form, $pdf_data, $context, array $entities) {
     $force_download = FALSE;
     if (!empty($context['force_download'])) {
       $force_download = TRUE;
@@ -159,7 +154,7 @@ class HandlePdfController extends ControllerBase {
 
     // Generate the filename of downloaded PDF from title of the PDF set in
     // admin/structure/fillpdf/%fid
-    $output_name = $this->buildFilename($fillpdf_form->title->value, $token_objects);
+    $output_name = $this->buildFilename($fillpdf_form->title->value, $entities);
 
     // Determine the appropriate action for the PDF.
     $destination_path_set = !empty($fillpdf_form->destination_path->value);
@@ -177,7 +172,7 @@ class HandlePdfController extends ControllerBase {
     $action_configuration = [
       'form' => $fillpdf_form,
       'context' => $context,
-      'token_objects' => $token_objects,
+      'token_objects' => $entities,
       'data' => $pdf_data,
       'filename' => $output_name,
     ];
@@ -200,10 +195,10 @@ class HandlePdfController extends ControllerBase {
     return $response;
   }
 
-  public function buildFilename($original, array $token_objects) {
+  protected function buildFilename($original, array $entities) {
     // Replace tokens *before* sanitization
-    if (count($token_objects)) {
-      $original = $this->token->replace($original, $token_objects);
+    if (count($entities)) {
+      $original = $this->tokenResolver->replace($original, $entities);
     }
 
     $output_name = str_replace(' ', '_', $original);

src/TokenResolver.php

@@ -22,6 +22,7 @@ class TokenResolver implements TokenResolverInterface {
    * @var \Drupal\Core\Utility\Token
    */
   protected $token;
+
   /**
    * Constructor.
    */
@@ -34,34 +35,20 @@ class TokenResolver implements TokenResolverInterface {
    */
   public function replace($original, array $entities) {
     // Whichever entity matches the token last wins.
-    $replaced_string = '';
+    $replaced_string = $original;
 
-    // TODO: Do these in reverse order, and only apply the value when the token gets replaced with something. Probably have to use Token::scan() myself because otherwise I'm not sure if I will know if any tokens matched or not. Maybe I can pass in the third argument and check what the BubbleableMetadata says? Check my StackExchange question about this.
     foreach ($entities as $entity_type => $entity_objects) {
-      foreach ($entity_objects as $entity_id => $entity) {
-        // @todo: Refactor so the token context can be re-used for title replacement later OR do title replacement here so that it works the same way as value replacement and doesn't just use the last value...like it does in Drupal 7 :(
-        // @todo: What if one fill pattern has tokens from multiple types in it? Figure out the best way to deal with that and rewrite this section accordingly. Probably some form of parallel arrays. Basically we'd have to run all combinations, although our logic still might not be smart enough to tell if *all* tokens in the source text have been replaced, or in which case both of them have been replaced last (which is what we want). I could deliberately pass each entity context separately and then count how many of them match, and only overwrite it if the match count is higher than the current one. Yeah, that's kind of inefficient but also a good start. I might just be able to scan for tokens myself and then check if they're still in the $uncleaned_base output, or do the cleaning myself so I only have to call Token::replace once. TBD.
-        $maybe_replaced_string = $this->token->replace($original, [
-          $entity_type => $entity,
-        ], [
-          'clean' => TRUE,
-        ]);
-        // Generate a non-cleaned version of the token string so we can
-        // tell if the non-empty string we got back actually replaced
-        // some tokens.
-        $uncleaned_base = $this->token->replace($original, [
+      // Our rule is "last value wins." So use the last entities's values first.
+      $seititne = array_reverse($entity_objects); // Get it?
+
+      foreach ($seititne as $entity_id => $entity) {
+        $replaced_string = $this->token->replace($replaced_string, [
           $entity_type => $entity,
         ]);
-
-        // If we got a result that isn't what we put in, update the value
-        // for this field..
-        if ($maybe_replaced_string && $original !== $uncleaned_base) {
-          $replaced_string = $maybe_replaced_string;
-        }
       }
     }
 
-    return $replaced_string;
+    return $this->token->replace($replaced_string, [], ['clear' => TRUE]);
   }
 
 }

tests/features/token_replacement.feature

@@ -28,9 +28,8 @@ Feature: Replace tokens using entity data
     But the Body field value from the first entity should be used for the [node:body] token
     And non-matching tokens should be cleared
 
-    # TODO: Fix this case. It's failing. fillpdf?fid=5&entity_ids[]=node:1&entity_ids[]=node:2&&entity_ids[]=node:3
   Scenario: Populate PDF with 3+ entities - two tokens in pattern
-    Given the FillPDF Form's Body field is mapped to "Title [node:title] Body [node:body]"
+    Given the FillPDF Form's Body field is mapped to "Body [node:body]"
     When I generate the PDF with multiple entities
     Then I should see all matching tokens replaced
     And the last passed-in entity's tokens should win (if they are not blank)
@@ -44,3 +43,10 @@ Feature: Replace tokens using entity data
     And the last passed-in entity's tokens should win (if they are not blank)
     But the Body field value from the first entity should be used for the [node:body] token
     And non-matching tokens should be cleared
+
+  Scenario: Unreplaced tokens properly cleaned
+    Given the FillPDF Form's Body field is mapped to "Title [node:title] Body [node:body]"
+    And the passed-in entity has an empty Body field
+    When I generate the PDF
+    Then I should see "Title <title> Body" in the PDF's Text2 field
+