Make the action parameter non-nullable in Discovery::getWopiClientURL().

Author: donquixote

src/Controller/ViewerController.php

@@ -70,7 +70,14 @@ public function editor(MediaInterface $media, Request $request, $edit = FALSE):
     try {
       // @todo Get client url for the correct MIME type.
       $discovery = $this->discoveryFetcher->getDiscovery();
-      $wopi_client_url = $discovery->getWopiClientURL();
+      $wopi_client_url = $edit
+        ? $discovery->getWopiClientURL(action: 'edit')
+        : ($discovery->getWopiClientURL(action: 'view')
+          // With the typical discovery.xml from Collabora, some MIME types that
+          // are viewable have an 'edit' or 'view_comment' action but no 'view'
+          // action.
+          ?? $discovery->getWopiClientURL(action: 'edit')
+          ?? $discovery->getWopiClientURL(action: 'view_comment'));
     }
     catch (CollaboraNotAvailableException $e) {
       $this->logger->warning(

src/Discovery/Discovery.php

@@ -32,16 +32,11 @@ public function __construct(
   /**
    * {@inheritdoc}
    */
-  public function getWopiClientURL(string $mimetype = 'text/plain', ?string $action = NULL): ?string {
+  public function getWopiClientURL(string $mimetype = 'text/plain', string $action = 'view'): ?string {
     $result = $this->parsedXml->xpath(sprintf(
-      '/wopi-discovery/net-zone/app[@name=\'%s\']/action%s',
+      "/wopi-discovery/net-zone/app[@name='%s']/action[@name='%s']",
       $mimetype,
-      match ($action) {
-        'edit',
-        'view',
-        'view_comment' => "[@name='$action']",
-        NULL => '',
-      }
+      $action,
     ));
     if (empty($result[0]['urlsrc'][0])) {
       return NULL;

src/Discovery/DiscoveryInterface.php

@@ -25,15 +25,15 @@ interface DiscoveryInterface {
    * @param string $mimetype
    *   Mime type for which to get the WOPI client URL.
    *   This refers to config entries in the discovery.xml file.
-   * @param 'view'|'edit'|'view_comment'|null $action
-   *   Name of the action/operation for which to get the url, or NULL for any
-   *   action/operation.
+   * @param string $action
+   *   Name of the action/operation for which to get the url.
+   *   Typical values are 'view', 'edit' or 'view_comment'.
    *
    * @return string|null
    *   The WOPI client URL, or NULL if none provided for the MIME type and
    *   operation.
    */
-  public function getWopiClientURL(string $mimetype = 'text/plain', ?string $action = NULL): ?string;
+  public function getWopiClientURL(string $mimetype = 'text/plain', string $action = 'view'): ?string;
 
   /**
    * Gets the public key used for proofing.

tests/src/Unit/CollaboraDiscoveryTest.php

@@ -29,12 +29,10 @@ public function testWopiClientUrl(): void {
     );
     $this->assertSame(
       'http://spreadsheet.collabora.test:9980/browser/61cf2b4/cool.html?',
-      $discovery->getWopiClientURL('text/spreadsheet'),
+      $discovery->getWopiClientURL('text/spreadsheet', 'edit'),
     );
     // Test unknown mime type.
-    $this->assertNull(
-      $discovery->getWopiClientURL('text/unknown'),
-    );
+    $this->assertNull($discovery->getWopiClientURL('text/unknown', 'view'));
     $this->assertSame(
       'http://csv.collabora.test:9980/browser/61cf2b4/cool.html?',
       $discovery->getWopiClientURL('text/csv', 'edit'),
@@ -52,7 +50,6 @@ public function testWopiClientUrl(): void {
     // well-defined behavior in that case.
     $this->assertNull($discovery->getWopiClientURL('image/png', 'edit'));
     $this->assertNull($discovery->getWopiClientURL('image/png', 'view'));
-    $this->assertNotNull($discovery->getWopiClientURL('image/png'));
   }
 
   /**
@@ -81,11 +78,6 @@ public function testRealisticDiscoveryXml(): void {
           $type_supported_actions[] = $action;
         }
       }
-      $null_url = $discovery->getWopiClientURL($mimetype);
-      $this->assertSame($null_url === NULL, $type_supported_actions === []);
-      if ($null_url !== NULL) {
-        $this->assertSame($known_url, $null_url);
-      }
       sort($type_supported_actions);
       $supported_action_types[implode(',', $type_supported_actions)][] = $mimetype;
     }