Merge pull request #35 from creative-commoners/pulls/2.0/update-for-t…

…inymce4 API Refactor input and output format for TinyMCE 4 compatibility
silverstripe · Dec 13, 2017 · 434e3d6 · 434e3d6
2 parents c8aa2ce + dc94796
commit 434e3d6
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 85 deletions.
diff --git a/readme.md b/readme.md
@@ -15,6 +15,11 @@ Ensure that your server is setup with [hunspell](http://hunspell.sourceforge.net
 Install the spellcheck module with composer, using `composer require silverstripe/spellcheck ^2.0`, or downloading
 the module and extracting to the 'spellcheck' directory under your project root.
 
+## Requirements
+
+* SilverStripe 4.0.2 or above
+* Hunspell
+
 **Note:** this version is compatible with SilverStripe 4. For SilverStripe 3, please see [the 1.x release line](https://github.com/silverstripe/silverstripe-spellcheck/tree/1.0).
 
 ## Configuration

diff --git a/src/Handling/SpellCheckMiddleware.php b/src/Handling/SpellCheckMiddleware.php
@@ -2,6 +2,7 @@
 
 namespace SilverStripe\SpellCheck\Handling;
 
+use SilverStripe\Control\Director;
 use SilverStripe\Control\HTTPRequest;
 use SilverStripe\Control\Middleware\HTTPMiddleware;
 use SilverStripe\Core\Config\Configurable;
@@ -28,20 +29,21 @@ public function process(HTTPRequest $request, callable $delegate)
         HTMLEditorConfig::get($editor)->enablePlugins('spellchecker');
         HTMLEditorConfig::get($editor)->addButtonsToLine(2, 'spellchecker');
         $token = SecurityToken::inst();
-        HTMLEditorConfig::get($editor)->setOption('spellchecker_rpc_url', $token->addToUrl('spellcheck/'));
-        HTMLEditorConfig::get($editor)->setOption('browser_spellcheck', false);
-        HTMLEditorConfig::get($editor)->setOption(
-            'spellchecker_languages',
-            '+'.implode(', ', $this->getLanguages())
-        );
+        HTMLEditorConfig::get($editor)
+            ->setOption('spellchecker_rpc_url', Director::absoluteURL($token->addToUrl('spellcheck/')))
+            ->setOption('browser_spellcheck', false)
+            ->setOption(
+                'spellchecker_languages',
+                implode(',', $this->getLanguages())
+            );
 
         return $delegate($request);
     }
 
     /**
      * Check languages to set
      *
-     * @return array
+     * @return string[]
      */
     public function getLanguages()
     {

diff --git a/src/Handling/SpellController.php b/src/Handling/SpellController.php
@@ -99,26 +99,20 @@ public function setProvider(SpellProvider $provider)
     /**
      * Parse the output response
      *
-     * @param string $id Request ID
      * @param array|null $result Result data
      * @param array|null $error Error data
      * @param int $code HTTP Response code
      */
-    protected function result($id, $result, $error = null, $code = 200)
+    protected function result($result, $code = 200)
     {
         $this->response->setStatusCode($code);
-        $this->response->setBody(json_encode(array(
-            'id' => $id ? preg_replace('/\W/', '', $id) : null, // Cleanup id
-            'result' => $result,
-            'error' => $error
-        )));
+        $this->response->setBody(json_encode($result));
         return $this->response;
     }
 
     protected function success($result)
     {
-        $data = $this->getRequestData();
-        return $this->result($data['id'], $result);
+        return $this->result($result);
     }
 
     /**
@@ -129,14 +123,11 @@ protected function success($result)
      */
     protected function error($message, $code)
     {
-        $error = array(
-            'errstr' => $message,
-            'errfile' => '',
-            'errline' => null,
-            'errcontext' => '',
-            'level' => 'FATAL'
-        );
-        return $this->result(null, null, $error, $code);
+        $error = [
+            'error' => $message,
+        ];
+
+        return $this->result($error, $code);
     }
 
     public function index()
@@ -169,14 +160,13 @@ public function index()
         }
 
         // Check params and request type
-        if (!Director::is_ajax() || empty($data['method']) || empty($data['params']) || count($data['params']) < 2) {
+        if (!Director::is_ajax() || empty($data['method']) || empty($data['lang'])) {
             return $this->error(_t(__CLASS__ . '.InvalidRequest', 'Invalid request'), 400);
         }
 
         // Check locale
-        $params = $data['params'];
-        $locale = $params[0];
-        if (!in_array($locale, self::get_locales())) {
+        $locale = $this->getLocale($data);
+        if (!$locale) {
             return $this->error(_t(__CLASS__ . '.InvalidLocale', 'Not supported locale'), 400);
         }
 
@@ -189,12 +179,10 @@ public function index()
         // Perform action
         try {
             $method = $data['method'];
-            $words = $params[1];
+            $words = explode(' ', $data['text']);
             switch ($method) {
-                case 'checkWords':
-                    return $this->success($provider->checkWords($locale, $words));
-                case 'getSuggestions':
-                    return $this->success($provider->getSuggestions($locale, $words));
+                case 'spellcheck':
+                    return $this->success($this->assembleData($locale, $words));
                 default:
                     return $this->error(
                         _t(
@@ -210,6 +198,29 @@ public function index()
         }
     }
 
+    /**
+     * Assemble an output data structure that is expected for TinyMCE 4
+     *
+     * @see https://www.tinymce.com/docs/plugins/spellchecker/#spellcheckerresponseformat
+     *
+     * @param string $locale
+     * @param string[] $words
+     * @return array
+     */
+    protected function assembleData($locale, $words)
+    {
+        $result = [
+            'words' => [],
+        ];
+
+        $misspelledWords = $this->getProvider()->checkWords($locale, $words);
+        foreach ($misspelledWords as $word) {
+            $result['words'][$word] = $this->getProvider()->getSuggestions($locale, $word);
+        }
+
+        return $result;
+    }
+
     /**
      * Ensures the response has the correct headers
      */
@@ -234,10 +245,30 @@ protected function getRequestData()
         // Check if data needs to be parsed
         if ($this->data === null) {
             // Parse data from input
-            $result = $this->request->requestVar('json_data')
-                ?: file_get_contents("php://input");
-            $this->data = $result ? json_decode($result, true) : array();
+            $this->data = $this->request->postVars();
         }
         return $this->data;
     }
+
+    /**
+     * Get the locale from the provided "lang" argument, which could be either a language code or locale
+     *
+     * @param array $data
+     * @return string|false
+     */
+    protected function getLocale(array $data)
+    {
+        $locale = $data['lang'];
+
+        // Check if the locale is actually a language
+        if (strpos($locale, '_') === false) {
+            $locale = i18n::getData()->localeFromLang($locale);
+        }
+
+        if (!in_array($locale, self::get_locales())) {
+            return false;
+        }
+
+        return $locale;
+    }
 }
diff --git a/tests/SpellControllerTest.php b/tests/SpellControllerTest.php
@@ -71,21 +71,21 @@ public function testSecurityID()
         $response = $this->get('spellcheck', Injector::inst()->create(Session::class, $session));
         $this->assertEquals(400, $response->getStatusCode());
         $jsonBody = json_decode($response->getBody());
-        $this->assertEquals($tokenError, $jsonBody->error->errstr);
+        $this->assertEquals($tokenError, $jsonBody->error);
 
         // Test request with correct token (will fail with an unrelated error)
         $response = $this->get(
             'spellcheck/?SecurityID='.urlencode($token),
             Injector::inst()->create(Session::class, $session)
         );
         $jsonBody = json_decode($response->getBody());
-        $this->assertNotEquals($tokenError, $jsonBody->error->errstr);
+        $this->assertNotEquals($tokenError, $jsonBody->error);
 
         // Test request with check disabled
         Config::modify()->set(SpellController::class, 'enable_security_token', false);
         $response = $this->get('spellcheck', Injector::inst()->create(Session::class, $session));
         $jsonBody = json_decode($response->getBody());
-        $this->assertNotEquals($tokenError, $jsonBody->error->errstr);
+        $this->assertNotEquals($tokenError, $jsonBody->error);
     }
 
     /**
@@ -102,20 +102,69 @@ public function testPermissions()
         $this->logInWithPermission('ADMIN');
         $response = $this->get('spellcheck');
         $jsonBody = json_decode($response->getBody());
-        $this->assertNotEquals($securityError, $jsonBody->error->errstr);
+        $this->assertNotEquals($securityError, $jsonBody->error);
 
         // Test insufficient permissions
         $this->logInWithPermission('CMS_ACCESS_CMSMain');
         $response = $this->get('spellcheck');
         $this->assertEquals(403, $response->getStatusCode());
         $jsonBody = json_decode($response->getBody());
-        $this->assertEquals($securityError, $jsonBody->error->errstr);
+        $this->assertEquals($securityError, $jsonBody->error);
 
         // Test disabled permissions
         Config::modify()->set(SpellController::class, 'required_permission', false);
         $response = $this->get('spellcheck');
         $jsonBody = json_decode($response->getBody());
-        $this->assertNotEquals($securityError, $jsonBody->error->errstr);
+        $this->assertNotEquals($securityError, $jsonBody->error);
+    }
+
+    /**
+     * @param string $lang
+     * @param int $expectedStatusCode
+     * @dataProvider langProvider
+     */
+    public function testBothLangAndLocaleInputResolveToLocale($lang, $expectedStatusCode)
+    {
+        $this->logInWithPermission('ADMIN');
+        Config::modify()->set(SpellController::class, 'enable_security_token', false);
+
+        $mockData = [
+            'ajax' => true,
+            'method' => 'spellcheck',
+            'lang' => $lang,
+            'text' => 'Collor is everywhere',
+        ];
+        $response = $this->post('spellcheck', $mockData);
+        $this->assertEquals($expectedStatusCode, $response->getStatusCode());
+    }
+
+    /**
+     * @return array[]
+     */
+    public function langProvider()
+    {
+        return [
+            'english_language' => [
+                'en', // assumes en_US is the default locale for "en" language
+                200,
+            ],
+            'english_locale' => [
+                'en_NZ',
+                200,
+            ],
+            'invalid_language' => [
+                'ru',
+                400,
+            ],
+            'other_valid_language' => [
+                'fr', // assumes fr_FR is the default locale for "en" language
+                200,
+            ],
+            'other_valid_locale' => [
+                'fr_FR',
+                200,
+            ],
+        ];
     }
 
     /**
@@ -128,47 +177,29 @@ public function testInputRejection()
         Config::modify()->set(SpellController::class, 'required_permission', false);
         $invalidRequest = _t('SilverStripe\\SpellCheck\\Handling\\SpellController.InvalidRequest', 'Invalid request');
 
-        // Test checkWords acceptance
-        $dataCheckWords = array(
-            'id' => 'c0',
-            'method' => 'checkWords',
-            'params' => array(
-                'en_NZ',
-                array('collor', 'colour', 'color', 'onee', 'correct')
-            )
-        );
-        $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataCheckWords)));
+        // Test spellcheck acceptance
+        $mockData = [
+            'method' => 'spellcheck',
+            'lang' => 'en_NZ',
+            'text' => 'Collor is everywhere',
+        ];
+        $response = $this->post('spellcheck', ['ajax' => true] + $mockData);
         $this->assertEquals(200, $response->getStatusCode());
         $jsonBody = json_decode($response->getBody());
-        $this->assertEquals('c0', $jsonBody->id);
-        $this->assertEquals(array("collor", "color", "onee"), $jsonBody->result);
-
-        // Test getSuggestions acceptance
-        $dataGetSuggestions = array(
-            'id' => '//c1//', // Should be reduced to only alphanumeric characters
-            'method' => 'getSuggestions',
-            'params' => array(
-                'en_NZ',
-                'collor'
-
-            )
-        );
-        $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataGetSuggestions)));
-        $this->assertEquals(200, $response->getStatusCode());
-        $jsonBody = json_decode($response->getBody());
-        $this->assertEquals('c1', $jsonBody->id);
-        $this->assertEquals(array('collar', 'colour'), $jsonBody->result);
+        $this->assertNotEmpty($jsonBody->words);
+        $this->assertNotEmpty($jsonBody->words->collor);
+        $this->assertEquals(['collar', 'colour'], $jsonBody->words->collor);
 
         // Test non-ajax rejection
-        $response = $this->post('spellcheck', array('json_data' => json_encode($dataCheckWords)));
+        $response = $this->post('spellcheck', $mockData);
         $this->assertEquals(400, $response->getStatusCode());
         $jsonBody = json_decode($response->getBody());
-        $this->assertEquals($invalidRequest, $jsonBody->error->errstr);
+        $this->assertEquals($invalidRequest, $jsonBody->error);
 
         // Test incorrect method
-        $dataInvalidMethod = $dataCheckWords;
+        $dataInvalidMethod = $mockData;
         $dataInvalidMethod['method'] = 'validate';
-        $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataInvalidMethod)));
+        $response = $this->post('spellcheck', ['ajax' => true] + $dataInvalidMethod);
         $this->assertEquals(400, $response->getStatusCode());
         $jsonBody = json_decode($response->getBody());
         $this->assertEquals(
@@ -177,29 +208,27 @@ public function testInputRejection()
                 "Unsupported method '{method}'",
                 array('method' => 'validate')
             ),
-            $jsonBody->error->errstr
+            $jsonBody->error
         );
 
         // Test missing method
-        $dataNoMethod = $dataCheckWords;
+        $dataNoMethod = $mockData;
         unset($dataNoMethod['method']);
-        $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataNoMethod)));
+        $response = $this->post('spellcheck', ['ajax' => true] + $dataNoMethod);
         $this->assertEquals(400, $response->getStatusCode());
         $jsonBody = json_decode($response->getBody());
-        $this->assertEquals($invalidRequest, $jsonBody->error->errstr);
+        $this->assertEquals($invalidRequest, $jsonBody->error);
 
         // Test unsupported locale
-        $dataWrongLocale = $dataCheckWords;
-        $dataWrongLocale['params'] = array(
-            'de_DE',
-            array('collor', 'colour', 'color', 'onee', 'correct')
-        );
-        $response = $this->post('spellcheck', array('ajax' => 1, 'json_data' => json_encode($dataWrongLocale)));
+        $dataWrongLocale = $mockData;
+        $dataWrongLocale['lang'] = 'de_DE';
+
+        $response = $this->post('spellcheck', ['ajax' => true] + $dataWrongLocale);
         $this->assertEquals(400, $response->getStatusCode());
         $jsonBody = json_decode($response->getBody());
         $this->assertEquals(_t(
             'SilverStripe\\SpellCheck\\Handling\\.InvalidLocale',
             'Not supported locale'
-        ), $jsonBody->error->errstr);
+        ), $jsonBody->error);
     }
 }