Merge pull request #39 from wikimedia/url-normalization

Normalize URLs of the translate page
wikimedia · Jan 3, 2019 · 6c0954a · 6c0954a
2 parents e266472 + 785f8bf
commit 6c0954a
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 36 deletions.
diff --git a/src/Controller/SearchController.php b/src/Controller/SearchController.php
@@ -3,6 +3,7 @@
 
 namespace App\Controller;
 
+use App\Model\Title;
 use App\OOUI\SearchWidget;
 use Krinkle\Intuition\Intuition;
 use OOUI\ActionFieldLayout;
@@ -59,12 +60,6 @@ public function search(Request $request):Response
             return $this->redirectToRoute('home');
         }
 
-        $filePrefixPos = stripos($filename, 'File:');
-        if (false !== $filePrefixPos) {
-            // Strip any 'File:' prefix, including if a URL has been supplied.
-            $filename = substr($filename, $filePrefixPos + strlen('File:'));
-        }
-
-        return $this->redirectToRoute('translate', ['filename' => $filename]);
+        return $this->redirectToRoute('translate', ['filename' => Title::normalize($filename)]);
     }
 }
diff --git a/src/Controller/TranslateController.php b/src/Controller/TranslateController.php
@@ -23,24 +23,34 @@ class TranslateController extends AbstractController
 {
 
     /**
-     * Get the filename parameter from the given Request.
-     * @param Request $request Normalized filename.
-     * @return string
+     * The translate page.
+     *
+     * The 'prefix' part of this route is actually a fixed string 'File', but in order to make it
+     * case-insensitive we have to make it a route variable with a regex requirement. Then we can
+     * also check it and redirect to the canonical form when required.
+     * @Route( "/{prefix}:{filename}",
+     *     name="translate",
+     *     methods={"GET"},
+     *     requirements={"prefix"="(?i:File)", "filename"="(.+)"}
+     *     )
      */
-    protected function getFilename(Request $request):string
-    {
-        return str_replace('_', ' ', $request->get('filename'));
-    }
+    public function translate(
+        Request $request,
+        Intuition $intuition,
+        Session $session,
+        FileCache $cache,
+        string $filename,
+        string $prefix = 'File'
+    ): Response {
+        $normalizedFilename = Title::normalize($filename);
+
+        // Redirect to normalized URL if required.
+        if ('File' !== $prefix || $filename !== $normalizedFilename) {
+            return $this->redirectToRoute('translate', ['filename' => $normalizedFilename]);
+        }
 
-    /**
-     * @Route("/File:{filename<.+>}", name="translate", methods={"GET"})
-     */
-    public function translate(Request $request, Intuition $intuition, Session $session, FileCache $cache):Response
-    {
         // Fetch the SVG from Commons.
-        $filename = $this->getFilename($request);
-        $fileName = Title::normalize($filename);
-        $path = $cache->getPath($fileName);
+        $path = $cache->getPath($filename);
         $svgFile = new SvgFile($path);
 
         // Upload and download buttons.
@@ -130,8 +140,8 @@ public function translate(Request $request, Intuition $intuition, Session $sessi
 
         return $this->render('translate.html.twig', [
             'page_class' => 'translate',
-            'title' => $filename,
-            'filename' => $filename,
+            'title' => Title::text($filename),
+            'filename' => $normalizedFilename,
             'fields' => $formFields,
             'download_button' => $downloadButton,
             'upload_button' => $uploadButton,
@@ -140,14 +150,4 @@ public function translate(Request $request, Intuition $intuition, Session $sessi
             'target_lang' => $targetLangDefault,
         ]);
     }
-
-    /**
-     * @Route("/File:{filename<.+>}", name="svg", methods={"POST"}))
-     */
-    public function svg(Request $request, Intuition $intuition, Session $session):Response
-    {
-        $filename = $this->getFilename($request);
-        return $this->redirectToRoute('translate', ['filename' => $filename]);
-        // @TODO Modify and return SVG.
-    }
 }
diff --git a/src/Model/Title.php b/src/Model/Title.php
@@ -14,10 +14,11 @@ final class Title
      */
     public static function removeNamespace(string $title): string
     {
-        return preg_replace('/^file\s*:\s*/i', '', $title);
+        return preg_replace('/.*file\s*:\s*/i', '', $title);
     }
 
     /**
+     * Get the 'DB key' form of a title (underscores not spaces, and uppercase first character).
      * @param string $title
      * @return string
      */
@@ -30,4 +31,14 @@ public static function normalize(string $title): string
 
         return $title;
     }
+
+    /**
+     * Get the 'text' form of a title (spaces not underscores).
+     * @param string $title
+     * @return string
+     */
+    public static function text(string $title): string
+    {
+        return str_replace('_', ' ', static::normalize($title));
+    }
 }
diff --git a/templates/translate.html.twig b/templates/translate.html.twig
@@ -5,7 +5,7 @@
 {% endblock %}
 
 {% block description %}
-    <h1>{{ filename }}</h1>
+    <h1>{{ title }}</h1>
     <p>
         <a href="https://commons.wikimedia.org/wiki/File:{{ filename }}" target="_blank"
            title="{{ msg('opens-in-new-tab') }}">{{ msg('view-on-commons') }}</a>

diff --git a/tests/Model/TitleTest.php b/tests/Model/TitleTest.php
@@ -32,6 +32,10 @@ public function provideNormalize(): array
             ['file:тест_123.svg', 'Тест_123.svg'],
             ['Тест.svg', 'Тест.svg'],
             ['тест_123.svg', 'Тест_123.svg'],
+            [
+                'https://commons.wikimedia.org/wiki/File:Flag_of_Pakistan_Construction.svg',
+                'Flag_of_Pakistan_Construction.svg',
+            ],
         ];
     }
 }