Merge pull request #38 from sitegeist/CHORE/refactor

CHORE: Refactor code as a result of testing
sitegeist · Apr 25, 2024 · d6a2dd7 · d6a2dd7
2 parents a157400 + da92deb
commit d6a2dd7
Show file tree

Hide file tree

Showing 10 changed files with 172 additions and 96 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -13,11 +13,13 @@ jobs:
         strategy:
             fail-fast: false
             matrix:
-                php-versions: ['8.0']
+                php-versions: ['8.1']
                 neos-versions: ['7.3']
                 include:
                   - php-versions: '8.1'
                     neos-versions: '8.3'
+                  - php-versions: '8.2'
+                    neos-versions: '8.3'
         runs-on: ubuntu-latest
 
         steps:

diff --git a/Classes/Controller/LostInTranslationModuleController.php b/Classes/Controller/LostInTranslationModuleController.php
@@ -4,12 +4,11 @@
 
 namespace Sitegeist\LostInTranslation\Controller;
 
-use Neos\Cache\Frontend\StringFrontend;
 use Neos\Fusion\View\FusionView;
 use Neos\Neos\Controller\Module\AbstractModuleController;
 use Neos\Flow\Annotations as Flow;
+use Sitegeist\LostInTranslation\Infrastructure\DeepL\DeepLCustomAuthenticationKeyService;
 use Sitegeist\LostInTranslation\Infrastructure\DeepL\DeepLTranslationService;
-use Sitegeist\LostInTranslation\Package;
 
 class LostInTranslationModuleController extends AbstractModuleController
 {
@@ -20,10 +19,10 @@ class LostInTranslationModuleController extends AbstractModuleController
     protected $translationService;
 
     /**
-     * @var StringFrontend
      * @Flow\Inject
+     * @var DeepLCustomAuthenticationKeyService
      */
-    public $apiKeyCache;
+    protected $customAuthenticationKeyService;
 
     /**
      * @var FusionView
@@ -43,13 +42,13 @@ public function setCustomKeyAction(): void
 
     public function storeCustomKeyAction(string $key): void
     {
-        $this->apiKeyCache->set(Package::API_KEY_CACHE_ID, $key);
+        $this->customAuthenticationKeyService->set($key);
         $this->forward('index');
     }
 
     public function removeCustomKeyAction(): void
     {
-        $this->apiKeyCache->remove(Package::API_KEY_CACHE_ID);
+        $this->customAuthenticationKeyService->remove();
         $this->forward('index');
     }
 }
diff --git a/Classes/Infrastructure/DeepL/DeepLAuthenticationKey.php b/Classes/Infrastructure/DeepL/DeepLAuthenticationKey.php
@@ -6,40 +6,12 @@
 
 class DeepLAuthenticationKey
 {
-    /**
-     * @var string
-     */
-    protected $authenticationKey;
-
-    /**
-     * @var bool
-     */
-    protected $isFree;
-
-    public function __construct(string $authenticationKey)
-    {
-        if (empty($authenticationKey)) {
-            throw new \InvalidArgumentException('Empty strings are not allowed as authentication key');
-        }
-
-        $this->authenticationKey = $authenticationKey;
-        $this->isFree = substr($authenticationKey, -3, 3) === ':fx';
-    }
-
-    /**
-     * @return string
-     */
-    public function getAuthenticationKey(): string
-    {
-        return $this->authenticationKey;
-    }
-
-    /**
-     * @return bool
-     */
-    public function isFree(): bool
-    {
-        return $this->isFree;
+    public bool $isFree;
+    public function __construct(
+        public readonly string $authenticationKey,
+        public readonly bool $isCustomKey = false
+    ) {
+        $this->isFree = str_ends_with($authenticationKey, ':fx');
     }
 
     /**

diff --git a/Classes/Infrastructure/DeepL/DeepLAuthenticationKeyFactory.php b/Classes/Infrastructure/DeepL/DeepLAuthenticationKeyFactory.php
@@ -0,0 +1,37 @@
+<?php
+
+namespace Sitegeist\LostInTranslation\Infrastructure\DeepL;
+
+use InvalidArgumentException;
+use Neos\Flow\Annotations as Flow;
+
+/**
+ * @Flow\Scope("singleton")
+ */
+class DeepLAuthenticationKeyFactory
+{
+    /**
+     * @var array{authenticationKey: string}
+     * @Flow\InjectConfiguration(path="DeepLApi")
+     */
+    protected array $settings;
+
+    /**
+     * @Flow\Inject
+     * @var DeepLCustomAuthenticationKeyService
+     */
+    protected $customAuthenticationKeyService;
+
+    /**
+     * @return DeepLAuthenticationKey
+     */
+    public function create(): DeepLAuthenticationKey
+    {
+        $customKey = $this->customAuthenticationKeyService->get();
+        $settingsKey = $this->settings['authenticationKey'] ?? null;
+        if (!isset($settingsKey) && !isset($customKey)) {
+            throw new InvalidArgumentException('Empty strings are not allowed as authentication key');
+        }
+        return new DeepLAuthenticationKey($customKey ?? $settingsKey, !is_null($customKey));
+    }
+}
diff --git a/Classes/Infrastructure/DeepL/DeepLCustomAuthenticationKeyService.php b/Classes/Infrastructure/DeepL/DeepLCustomAuthenticationKeyService.php
@@ -0,0 +1,29 @@
+<?php
+
+namespace Sitegeist\LostInTranslation\Infrastructure\DeepL;
+
+use Neos\Cache\Frontend\StringFrontend;
+
+class DeepLCustomAuthenticationKeyService
+{
+    private const API_KEY_CACHE_ID = 'lostInTranslationApiKey';
+    /**
+     * @var StringFrontend
+     */
+    protected $apiKeyCache;
+
+    public function get(): ?string
+    {
+        return $this->apiKeyCache->get(self::API_KEY_CACHE_ID) ?: null;
+    }
+
+    public function set(string $key): void
+    {
+        $this->apiKeyCache->set(self::API_KEY_CACHE_ID, $key);
+    }
+
+    public function remove(): void
+    {
+        $this->apiKeyCache->remove(self::API_KEY_CACHE_ID);
+    }
+}
diff --git a/Classes/Infrastructure/DeepL/DeepLTranslationService.php b/Classes/Infrastructure/DeepL/DeepLTranslationService.php
@@ -16,10 +16,11 @@
 use Neos\Flow\Http\Client\CurlEngineException;
 use Neos\Http\Factories\ServerRequestFactory;
 use Neos\Http\Factories\StreamFactory;
+use Psr\Http\Message\ServerRequestInterface;
 use Psr\Log\LoggerInterface;
 use Sitegeist\LostInTranslation\Domain\ApiStatus;
 use Sitegeist\LostInTranslation\Domain\TranslationServiceInterface;
-use Sitegeist\LostInTranslation\Package;
+use Sitegeist\LostInTranslation\Utility\IgnoredTermsUtility;
 
 /**
  * @Flow\Scope("singleton")
@@ -52,15 +53,21 @@ class DeepLTranslationService implements TranslationServiceInterface
 
     /**
      * @Flow\Inject
-     * @var StringFrontend
+     * @var DeepLCustomAuthenticationKeyService
      */
-    protected $apiKeyCache;
+    protected $customAuthenticationKeyService;
 
     /**
      * @var StringFrontend
      */
     protected $translationCache;
 
+    /**
+     * @Flow\Inject
+     * @var DeepLAuthenticationKeyFactory
+     */
+    protected $authenticationKeyFactory;
+
     /**
      * @param array<string,string> $texts
      * @param string $targetLanguage
@@ -75,7 +82,7 @@ public function translate(array $texts, string $targetLanguage, ?string $sourceL
 
         if ($isCacheEnabled) {
             foreach ($texts as $i => $text) {
-                $entryIdentifier = $this->getEntryIdentifier($text, $targetLanguage, $sourceLanguage);
+                $entryIdentifier = self::getEntryIdentifier($text, $targetLanguage, $sourceLanguage);
                 if ($this->translationCache->has($entryIdentifier)) {
                     $cachedEntries[$i] = $this->translationCache->get($entryIdentifier);
                     unset($texts[$i]);
@@ -91,9 +98,6 @@ public function translate(array $texts, string $targetLanguage, ?string $sourceL
         $keys = array_keys($texts);
         $values = array_values($texts);
 
-        $deeplAuthenticationKey = $this->getDeeplAuthenticationKey();
-        $baseUri = $deeplAuthenticationKey->isFree() ? $this->settings['baseUriFree'] : $this->settings['baseUri'];
-
         // request body ... this has to be done manually because of the non php ish format
         // with multiple text arguments
         $body = http_build_query($this->settings['defaultOptions']);
@@ -105,25 +109,16 @@ public function translate(array $texts, string $targetLanguage, ?string $sourceL
             // All ignored terms will be wrapped in a <ignored> tag
             // which will be ignored by DeepL
             if (isset($this->settings['ignoredTerms']) && count($this->settings['ignoredTerms']) > 0) {
-                /**
-                 * @var string $part
-                 */
-                $part = preg_replace('/(' . implode('|', $this->settings['ignoredTerms']) . ')/i', '<ignore>$1</ignore>', $part);
+                $part = IgnoredTermsUtility::wrapIgnoredTerms($part, $this->settings['ignoredTerms']);
             }
 
             $body .= '&text=' . urlencode($part);
         }
 
-        $apiRequest = $this->serverRequestFactory->createServerRequest('POST', $baseUri . 'translate')
-            ->withHeader('Accept', 'application/json')
-            ->withHeader('Authorization', sprintf('DeepL-Auth-Key %s', $deeplAuthenticationKey->getAuthenticationKey()))
-            ->withHeader('Content-Type', 'application/x-www-form-urlencoded')
-            ->withBody($this->streamFactory->createStream($body));
+        $apiRequest = $this->createRequest('translate', 'POST');
+        $apiRequest->withBody($this->streamFactory->createStream($body));
 
-        $browser = new Browser();
-        $engine = new CurlEngine();
-        $engine->setOption(CURLOPT_TIMEOUT, 0);
-        $browser->setRequestEngine($engine);
+        $browser = $this->getBrowser();
 
         $attempt = 0;
         $maximumAttempts = $this->settings['numberOfAttempts'];
@@ -152,7 +147,7 @@ public function translate(array $texts, string $targetLanguage, ?string $sourceL
             }
             $translations = array_map(
                 function ($part) {
-                    return preg_replace('/(<ignore>|<\/ignore>)/i', '', $part['text']);
+                    return IgnoredTermsUtility::unwrapIgnoredTerms($part);
                 },
                 $returnedData['translations']
             );
@@ -162,7 +157,7 @@ function ($part) {
             if ($isCacheEnabled) {
                 foreach ($translationWithOriginalIndex as $i => $translatedString) {
                     $originalString = $texts[$i];
-                    $this->translationCache->set($this->getEntryIdentifier($originalString, $targetLanguage, $sourceLanguage), $translatedString);
+                    $this->translationCache->set(self::getEntryIdentifier($originalString, $targetLanguage, $sourceLanguage), $translatedString);
                 }
             }
 
@@ -193,30 +188,21 @@ function ($part) {
     public function getStatus(): ApiStatus
     {
         $hasSettingsKey =  $this->settings['authenticationKey'] ? true : false;
-        $hasCustomKey = $this->apiKeyCache->has(Package::API_KEY_CACHE_ID);
+        $hasCustomKey = !is_null($this->customAuthenticationKeyService->get());
 
         try {
             $deeplAuthenticationKey = $this->getDeeplAuthenticationKey();
-            $baseUri = $deeplAuthenticationKey->isFree() ? $this->settings['baseUriFree'] : $this->settings['baseUri'];
-
-            $apiRequest = $this->serverRequestFactory->createServerRequest('POST', $baseUri . 'usage')
-                ->withHeader('Accept', 'application/json')
-                ->withHeader('Authorization', sprintf('DeepL-Auth-Key %s', $deeplAuthenticationKey->getAuthenticationKey()))
-                ->withHeader('Content-Type', 'application/x-www-form-urlencoded');
-
-            $browser = new Browser();
-            $engine = new CurlEngine();
-            $engine->setOption(CURLOPT_TIMEOUT, 0);
-            $browser->setRequestEngine($engine);
 
+            $apiRequest = $this->createRequest('usage');
+            $browser = $this->getBrowser();
             $apiResponse = $browser->sendRequest($apiRequest);
 
 
             if ($apiResponse->getStatusCode() == 200) {
                 $json = json_decode($apiResponse->getBody()->getContents(), true);
-                return new ApiStatus(true, $json['character_count'], $json['character_limit'], $hasSettingsKey, $hasCustomKey, $deeplAuthenticationKey->isFree());
+                return new ApiStatus(true, $json['character_count'], $json['character_limit'], $hasSettingsKey, $hasCustomKey, $deeplAuthenticationKey->isFree);
             } else {
-                return new ApiStatus(false, 0, 0, $hasSettingsKey, $hasCustomKey, $deeplAuthenticationKey->isFree());
+                return new ApiStatus(false, 0, 0, $hasSettingsKey, $hasCustomKey, $deeplAuthenticationKey->isFree);
             }
         } catch (\Exception $exception) {
             return new ApiStatus(false, 0, 0, $hasSettingsKey, $hasCustomKey, false);
@@ -225,19 +211,48 @@ public function getStatus(): ApiStatus
 
     protected function getDeeplAuthenticationKey(): DeepLAuthenticationKey
     {
-        $customKey = $this->apiKeyCache->get(Package::API_KEY_CACHE_ID) ?: null;
-        $settingsKey = $this->settings['authenticationKey'] ?? null;
-        return new DeepLAuthenticationKey($customKey ?? $settingsKey);
+        return $this->authenticationKeyFactory->create();
     }
 
     /**
-     * @param  string  $text
-     * @param  string  $targetLanguage
-     * @param  string|null  $sourceLanguage
+     * @param  string      $text
+     * @param  string      $targetLanguage
+     * @param  string|null $sourceLanguage
+     *
      * @return string
      */
-    protected function getEntryIdentifier(string $text, string $targetLanguage, string $sourceLanguage = null): string
+    public static function getEntryIdentifier(string $text, string $targetLanguage, string $sourceLanguage = null): string
     {
         return sha1($text . $targetLanguage . $sourceLanguage);
     }
+
+    /**
+     * @return Browser
+     */
+    protected function getBrowser(): Browser
+    {
+        $browser = new Browser();
+        $engine = new CurlEngine();
+        $engine->setOption(CURLOPT_TIMEOUT, 0);
+        $browser->setRequestEngine($engine);
+        return $browser;
+    }
+
+    /**
+     * @param string $method
+     * @param string $endpoint
+     *
+     * @return ServerRequestInterface
+     */
+    protected function createRequest(
+        string $endpoint,
+        string $method = 'GET'
+    ): ServerRequestInterface {
+        $deeplAuthenticationKey = $this->getDeeplAuthenticationKey();
+        $baseUri = $deeplAuthenticationKey->isFree ? $this->settings['baseUriFree'] : $this->settings['baseUri'];
+        return $this->serverRequestFactory->createServerRequest($method, $baseUri . $endpoint)
+            ->withHeader('Accept', 'application/json')
+            ->withHeader('Authorization', sprintf('DeepL-Auth-Key %s', $deeplAuthenticationKey->authenticationKey))
+            ->withHeader('Content-Type', 'application/x-www-form-urlencoded');
+    }
 }
diff --git a/Classes/Package.php b/Classes/Package.php
@@ -16,8 +16,6 @@
  */
 class Package extends BasePackage
 {
-    const API_KEY_CACHE_ID = 'lostInTranslationApiKey';
-
     /**
      * @param Bootstrap $bootstrap The current bootstrap
      * @return void