Issue #3047303 by Pancho: Reliably replace tokens in filenames

src/Controller/HandlePdfController.php

@@ -253,9 +253,7 @@ class HandlePdfController extends ControllerBase {
    */
   protected function buildFilename($original, array $entities) {
     // Replace tokens *before* sanitization.
-    if (count($entities)) {
-      $original = $this->tokenResolver->replace($original, $entities);
-    }
+    $original = $this->tokenResolver->replace($original, $entities);
 
     $output_name = str_replace(' ', '_', $original);
     $output_name = preg_replace('/\.pdf$/i', '', $output_name);

tests/src/Functional/HandlePdfControllerTest.php

@@ -12,15 +12,17 @@ use Drupal\file\Entity\File;
  * and \Drupal\fillpdf\OutputHandler.
  *
  * @group fillpdf
+ *
+ * @todo Convert into a unit test.
  */
 class HandlePdfControllerTest extends FillPdfUploadTestBase {
 
   /**
    * A test node.
    *
-   * @var \Drupal\node\NodeInterface
+   * @var \Drupal\node\NodeInterface[]
    */
-  protected $testNode;
+  protected $testNodes;
 
   /**
    * {@inheritdoc}
@@ -30,19 +32,20 @@ class HandlePdfControllerTest extends FillPdfUploadTestBase {
 
     $this->configureBackend();
 
-    $this->testNode = $this->createNode([
+    $this->testNode[1] = $this->createNode([
       'title' => 'Hello',
       'type' => 'article',
     ]);
+    $this->testNode[2] = $this->createNode([
+      'title' => 'Goodbye',
+      'type' => 'article',
+    ]);
   }
 
   /**
    * Tests filename and destination of a populated PDF file.
    */
   public function testTokenFilenameDestination() {
-    $node_id = $this->testNode->id();
-    $user_id = $this->adminUser->id();
-
     $this->uploadTestPdf('fillpdf_test_v3.pdf');
     $form_id = $this->getLatestFillPdfForm();
     $edit = [
@@ -51,16 +54,54 @@ class HandlePdfControllerTest extends FillPdfUploadTestBase {
     ];
     $this->drupalPostForm("admin/structure/fillpdf/{$form_id}", $edit, 'Save');
 
-    $entities = [
-      'node' => $node_id,
-      'user' => $user_id,
-    ];
-    foreach ($entities as $type => $id) {
+    $year = date('Y');
+    $node1_id = $this->testNode[1]->id();
+    $node1_title = $this->testNode[1]->getTitle();
+    $node2_id = $this->testNode[2]->id();
+    $node2_title = $this->testNode[2]->getTitle();
+    $user_id = $this->adminUser->id();
+    $user_name = $this->adminUser->getAccountName();
+
+    $testcases = [];
+    // Test case 0: no entity.
+    $testcases[1]['entities'] = [];
+    $testcases[1]['expected'] = "{$year}--";
+
+    // Test case 1: existing node.
+    $testcases[1]['entities'] = ["node:{$node1_id}"];
+    $testcases[1]['expected'] = "{$year}--{$node1_title}";
+
+    // Test case 2: two existing nodes.
+    $testcases[2]['entities'] = ["node:{$node1_id}", "node:{$node2_id}"];
+    $testcases[2]['expected'] = "{$year}--{$node2_title}";
+
+    // Test case 3: twice the same node.
+    $testcases[3]['entities'] = ["node:{$node1_id}", "node:{$node1_id}"];
+    $testcases[3]['expected'] = "{$year}--{$node1_title}";
+
+    // Test case 4: existing user.
+    $testcases[4]['entities'] = ["user:{$user_id}"];
+    $testcases[4]['expected'] = "{$year}-{$user_name}-";
+
+    // Test case 5: existing node and existing user.
+    $testcases[5]['entities'] = ["node:{$node1_id}", "user:{$user_id}"];
+    $testcases[5]['expected'] = "{$year}-{$user_name}-{$node1_title}";
+
+    // Test case 6: non-existing node.
+    $testcases[6]['entities'] = ["node:123"];
+    $testcases[6]['expected'] = "{$year}--";
+
+    // Test case 7: existing node and non-existing user.
+    $testcases[7]['entities'] = ["node:{$node1_id}", "user:456"];
+    $testcases[7]['expected'] = "{$year}--{$node1_title}";
+
+    foreach ($testcases as $id => $case) {
       // Hit the generation route.
+      $entities = $case['entities'];
       $fillpdf_route = Url::fromRoute('fillpdf.populate_pdf', [], [
         'query' => [
           'fid' => $form_id,
-          'entity_id' => "{$type}:{$id}",
+          'entity_ids' => $entities,
         ],
       ]);
       $this->drupalGet($fillpdf_route);
@@ -70,31 +111,19 @@ class HandlePdfControllerTest extends FillPdfUploadTestBase {
       $filename = $file->getFilename();
       $uri = $file->getFileUri();
 
-      $current_year = date('Y');
-      switch ($type) {
-        case 'node':
-          $node_title = $this->testNode->getTitle();
-          $expected = "{$current_year}--{$node_title}";
-          break;
-
-        case 'user':
-          $account_name = $this->adminUser->getAccountName();
-          $expected = "{$current_year}-{$account_name}-";
-          break;
-
-        default:
-          $expected = "{$current_year}--";
-          break;
-      }
-      $this->assertEquals("{$expected}.pdf", $filename, "Populated with a $type, the file has the filename $filename.");
-      $this->assertEquals("public://fillpdf/{$expected}/{$expected}.pdf", $uri, "Populated with a $type, the file has the expected URI.");
+      $expected = $case['expected'];
+      $this->assertEquals("{$expected}.pdf", $filename, "Test case $id: The file has the filename $filename.");
+      $this->assertEquals("public://fillpdf/{$expected}/{$expected}.pdf", $uri, "Test case $id: The file has the expected URI.");
 
       // Check if file is permanent and has the right format.
       $this->assertFileIsPermanent($file);
       $this->drupalGet(file_create_url($uri));
       $maybe_pdf = $this->getSession()->getPage()->getContent();
       $finfo = new \finfo(FILEINFO_MIME_TYPE);
-      static::assertEquals('application/pdf', $finfo->buffer($maybe_pdf), 'The file has the correct application/pdf MIME type.');
+      static::assertEquals('application/pdf', $finfo->buffer($maybe_pdf), "Test case $id: The file has the correct MIME type.");
+
+      // Delete the file, so we don't run into conflicts with the next testcase.
+      $file->delete();
     }
   }