code cleanup / improvements

8 files changed, 184 insertions, 122 deletions

diff --git a/core/Translate/Filter/ByBaseTranslations.php b/core/Translate/Filter/ByBaseTranslations.php
index dd5f98e5bd..001a293c2e 100644
--- a/core/Translate/Filter/ByBaseTranslations.php
+++ b/core/Translate/Filter/ByBaseTranslations.php
@@ -19,6 +19,18 @@ use Piwik\Translate\Filter\FilterAbstract;
  */
 class ByBaseTranslations extends FilterAbstract
 {
+    protected $_baseTranslations = array();
+
+    /**
+     * Sets base translations
+     *
+     * @param array $baseTranslations
+     */
+    public function __construct($baseTranslations=array())
+    {
+        $this->_baseTranslations = $baseTranslations;
+    }
+
     /**
      * Removes all translations that aren't present in the base translations set in constructor
      *
diff --git a/core/Translate/Filter/ByParameterCount.php b/core/Translate/Filter/ByParameterCount.php
index a98ef13f96..1a1e716294 100644
--- a/core/Translate/Filter/ByParameterCount.php
+++ b/core/Translate/Filter/ByParameterCount.php
@@ -19,6 +19,18 @@ use Piwik\Translate\Filter\FilterAbstract;
  */
 class ByParameterCount extends FilterAbstract
 {
+    protected $_baseTranslations = array();
+
+    /**
+     * Sets base translations
+     *
+     * @param array $baseTranslations
+     */
+    public function __construct($baseTranslations=array())
+    {
+        $this->_baseTranslations = $baseTranslations;
+    }
+
     /**
      * Removes all translations where the placeholder parameter count differs to base translation
      *
diff --git a/core/Translate/Filter/FilterAbstract.php b/core/Translate/Filter/FilterAbstract.php
index f61bb614b5..eca3489bdf 100644
--- a/core/Translate/Filter/FilterAbstract.php
+++ b/core/Translate/Filter/FilterAbstract.php
@@ -19,18 +19,6 @@ abstract class FilterAbstract
 {
     protected $_filteredData = array();
 
-    protected $_baseTranslations = array();
-
-    /**
-     * Sets base translations
-     *
-     * @param array $baseTranslations
-     */
-    public function __construct($baseTranslations=array())
-    {
-        $this->_baseTranslations = $baseTranslations;
-    }
-
     /**
      * Filter the given translations
      *
diff --git a/core/Translate/Filter/UnnecassaryWhitespaces.php b/core/Translate/Filter/UnnecassaryWhitespaces.php
index 7a80a21612..1de5c50a1f 100644
--- a/core/Translate/Filter/UnnecassaryWhitespaces.php
+++ b/core/Translate/Filter/UnnecassaryWhitespaces.php
@@ -19,6 +19,18 @@ use Piwik\Translate\Filter\FilterAbstract;
  */
 class UnnecassaryWhitespaces extends FilterAbstract
 {
+    protected $_baseTranslations = array();
+
+    /**
+     * Sets base translations
+     *
+     * @param array $baseTranslations
+     */
+    public function __construct($baseTranslations=array())
+    {
+        $this->_baseTranslations = $baseTranslations;
+    }
+
     /**
      * Removes all unnecassary whitespaces and newlines from the given translations
      *
@@ -32,14 +44,17 @@ class UnnecassaryWhitespaces extends FilterAbstract
         foreach ($translations AS $pluginName => $pluginTranslations) {
             foreach ($pluginTranslations AS $key => $translation) {
 
-                $baseTranslation  = $this->_baseTranslations[$pluginName][$key];
+                $baseTranslation = '';
+                if (isset($this->_baseTranslations[$pluginName][$key])) {
+                    $baseTranslation  = $this->_baseTranslations[$pluginName][$key];
+                }
 
                 // remove excessive line breaks (and leading/trailing whitespace) from translations
                 $stringNoLineBreak = trim($translation);
                 $stringNoLineBreak = str_replace("\r", "", $stringNoLineBreak); # remove useless carrige renturns
                 $stringNoLineBreak = preg_replace('/(\n[ ]+)/', "\n", $stringNoLineBreak); # remove useless white spaces after line breaks
                 $stringNoLineBreak = preg_replace('/([\n]{2,})/', "\n\n", $stringNoLineBreak); # remove excessive line breaks
-                if (!isset($baseTranslation) || !substr_count($baseTranslation, "\n")) {
+                if (empty($baseTranslation) || !substr_count($baseTranslation, "\n")) {
                     $stringNoLineBreak = preg_replace("/[\n]+/", " ", $stringNoLineBreak); # remove all line breaks if english string doesn't contain any
                 }
                 $stringNoLineBreak = preg_replace('/([ ]{2,})/', " ", $stringNoLineBreak); # remove excessive white spaces again as there might be any now, after removing line breaks
diff --git a/core/Translate/Validate/CoreTranslations.php b/core/Translate/Validate/CoreTranslations.php
index d1f53e93d0..068d1c3cbc 100644
--- a/core/Translate/Validate/CoreTranslations.php
+++ b/core/Translate/Validate/CoreTranslations.php
@@ -32,6 +32,18 @@ class CoreTranslations extends ValidateAbstract
     const __ERRORSTATE_LOCALEINVALIDLANGUAGE__      = 'Locale is invalid - invalid language code';
     const __ERRORSTATE_LOCALEINVALIDCOUNTRY__       = 'Locale is invalid - invalid country code';
 
+    protected $_baseTranslations = array();
+
+    /**
+     * Sets base translations
+     *
+     * @param array $baseTranslations
+     */
+    public function __construct($baseTranslations=array())
+    {
+        $this->_baseTranslations = $baseTranslations;
+    }
+
     /**
      * Validates the given translations
      *  * There need to be more than 250 translations presen
@@ -46,30 +58,32 @@ class CoreTranslations extends ValidateAbstract
      */
     public function isValid($translations)
     {
+        $this->_message = null;
+
         if (250 > count($translations, COUNT_RECURSIVE)) {
-            $this->_error = self::__ERRORSTATE_MINIMUMTRANSLATIONS__;
+            $this->_message = self::__ERRORSTATE_MINIMUMTRANSLATIONS__;
             return false;
         }
 
         if (empty($translations['General']['Locale'])) {
-            $this->_error = self::__ERRORSTATE_LOCALEREQUIRED__;
+            $this->_message = self::__ERRORSTATE_LOCALEREQUIRED__;
             return false;
         }
 
         if (empty($translations['General']['TranslatorName'])) {
-            $this->_error = self::__ERRORSTATE_TRANSLATORINFOREQUIRED__;
+            $this->_message = self::__ERRORSTATE_TRANSLATORINFOREQUIRED__;
             return false;
         }
 
         if (empty($translations['General']['TranslatorEmail'])) {
-            $this->_error = self::__ERRORSTATE_TRANSLATOREMAILREQUIRED__;
+            $this->_message = self::__ERRORSTATE_TRANSLATOREMAILREQUIRED__;
             return false;
         }
 
         if (!empty($translations['General']['LayoutDirection']) &&
             !in_array($translations['General']['LayoutDirection'], array('ltr', 'rtl'))
         ) {
-            $this->_error = self::__ERRORSTATE_LAYOUTDIRECTIONINVALID__;
+            $this->_message = self::__ERRORSTATE_LAYOUTDIRECTIONINVALID__;
             return false;
         }
 
@@ -77,13 +91,13 @@ class CoreTranslations extends ValidateAbstract
         $allCountries = Common::getCountriesList();
 
         if (!preg_match('/^([a-z]{2})_([A-Z]{2})\.UTF-8$/', $translations['General']['Locale'], $matches)) {
-            $this->_error = self::__ERRORSTATE_LOCALEINVALID__;
+            $this->_message = self::__ERRORSTATE_LOCALEINVALID__;
             return false;
         } else if (!array_key_exists($matches[1], $allLanguages)) {
-            $this->_error = self::__ERRORSTATE_LOCALEINVALIDLANGUAGE__;
+            $this->_message = self::__ERRORSTATE_LOCALEINVALIDLANGUAGE__;
             return false;
         } else if (!array_key_exists(strtolower($matches[2]), $allCountries)) {
-            $this->_error = self::__ERRORSTATE_LOCALEINVALIDCOUNTRY__;
+            $this->_message = self::__ERRORSTATE_LOCALEINVALIDCOUNTRY__;
             return false;
         }
 
diff --git a/core/Translate/Validate/NoScripts.php b/core/Translate/Validate/NoScripts.php
index cdba2c608f..7116e409a8 100644
--- a/core/Translate/Validate/NoScripts.php
+++ b/core/Translate/Validate/NoScripts.php
@@ -31,14 +31,14 @@ class NoScripts extends ValidateAbstract
      */
     public function isValid($translations)
     {
-        $this->_error = null;
+        $this->_message = null;
 
         // check if any translation contains restricted script tags
         $serializedStrings = serialize($translations);
         $invalids = array("<script", 'document.', 'javascript:', 'src=', 'background=', 'onload=');
         foreach ($invalids as $invalid) {
             if (stripos($serializedStrings, $invalid) !== false) {
-                $this->_error = 'script tags restricted for language files';
+                $this->_message = 'script tags restricted for language files';
                 return false;
             }
         }
diff --git a/core/Translate/Validate/ValidateAbstract.php b/core/Translate/Validate/ValidateAbstract.php
index 6fdee45ed2..ca71850ed3 100644
--- a/core/Translate/Validate/ValidateAbstract.php
+++ b/core/Translate/Validate/ValidateAbstract.php
@@ -17,19 +17,7 @@ namespace Piwik\Translate\Validate;
  */
 abstract class ValidateAbstract
 {
-    protected $_baseTranslations = array();
-
-    protected $_error = null;
-
-    /**
-     * Sets base translations
-     *
-     * @param array $baseTranslations
-     */
-    public function __construct($baseTranslations=array())
-    {
-        $this->_baseTranslations = $baseTranslations;
-    }
+    protected $_message = null;
 
     /**
      * Returns if the given translations are valid
@@ -42,11 +30,13 @@ abstract class ValidateAbstract
     abstract public function isValid($translations);
 
     /**
-     * Returns the error occured while validating
-     * @return mixed
+     * Returns an array of messages that explain why the most recent isValid()
+     * call returned false.
+     *
+     * @return array
      */
-    public function getError()
+    public function getMessage()
     {
-        return $this->_error;
+        return $this->_message;
     }
 }
\ No newline at end of file
diff --git a/core/Translate/Writer.php b/core/Translate/Writer.php
index 66e34449ec..dea88a7103 100644
--- a/core/Translate/Writer.php
+++ b/core/Translate/Writer.php
@@ -14,13 +14,8 @@ namespace Piwik\Translate;
 use Exception;
 use Piwik\Common;
 use Piwik\PluginsManager;
-use Piwik\Translate\Filter\ByBaseTranslations;
-use Piwik\Translate\Filter\ByParameterCount;
-use Piwik\Translate\Filter\EncodedEntities;
-use Piwik\Translate\Filter\EmptyTranslations;
-use Piwik\Translate\Filter\UnnecassaryWhitespaces;
-use Piwik\Translate\Validate\CoreTranslations;
-use Piwik\Translate\Validate\NoScripts;
+use Piwik\Translate\Filter\FilterAbstract;
+use Piwik\Translate\Validate\ValidateAbstract;
 
 /**
  * Writes clean translations to file
@@ -45,30 +40,44 @@ class Writer
     protected $_pluginName = null;
 
     /**
-     * base translations (english) for the current instance
+     * translations to write to file
      *
      * @var array
      */
-    protected $_baseTranslations = array();
+    protected $_translations = array();
 
     /**
-     * translations to write to file
+     * Validators to check translations with
      *
-     * @var array
+     * @var ValidateAbstract[]
      */
-    protected $_translations = array();
+    protected $_validators = array();
+
+    /**
+     * Message why validation failed
+     *
+     * @var string|null
+     */
+    protected $_validationMessage = null;
+
+    /**
+     * Filters to to apply to translations
+     *
+     * @var FilterAbstract[]
+     */
+    protected $_filters = array();
 
     /**
-     * Errors occured while cleaning the translations
+     * Messages which filter changed the data
      *
      * @var array
      */
-    protected $_cleanErrors = array();
+    protected $_filterMessages = array();
 
-    const __UNCLEANED__  = 'uncleaned';
-    const __CLEANED__    = 'cleaned';
+    const __UNFILTERED__  = 'unfiltered';
+    const __FILTERED__    = 'filtered';
 
-    protected $_currentState = self::__UNCLEANED__;
+    protected $_currentState = self::__UNFILTERED__;
 
     /**
      * If $pluginName is given, Writer will be initialized for the given plugin if it exists
@@ -92,9 +101,6 @@ class Writer
 
             $this->_pluginName = $pluginName;
         }
-
-        $this->_baseTranslations = $this->_loadTranslation('en');
-        $this->setTranslations($this->_loadTranslation($this->getLanguage()));
     }
 
     /**
@@ -125,7 +131,7 @@ class Writer
      */
     public function hasTranslations()
     {
-        return !empty($this->_baseTranslations) && !empty($this->_translations);
+        return !empty($this->_translations);
     }
 
     /**
@@ -135,19 +141,19 @@ class Writer
      */
     public function setTranslations($translations)
     {
-        $this->_currentState = self::__UNCLEANED__;
+        $this->_currentState = self::__UNFILTERED__;
         $this->_translations = $translations;
-        $this->_cleanTranslations();
+        $this->_applyFilters();
     }
 
     /**
-     * Load translations from file
+     * Get translations from file
      *
      * @param  string  $lang  ISO 639-1 alpha-2 language code
      * @throws Exception
      * @return array   Array of translations ( plugin => ( key => translated string ) )
      */
-    protected function _loadTranslation($lang)
+    public function getTranslations($lang)
     {
         $path = $this->_getTranslationPath('lang', $lang);
         if (!is_readable($path)) {
@@ -224,10 +230,17 @@ class Writer
     /**
      * Save translations to file; translations should already be cleaned.
      *
+     * @throws \Exception
      * @return bool|int  False if failure, or number of bytes written
      */
     public function save()
     {
+        $this->_applyFilters();
+
+        if (!$this->hasTranslations() || !$this->isValid()) {
+            throw new Exception('unable to save empty or invalid translations');
+        }
+
         $path = $this->getTranslationPath();
 
         Common::mkdir(dirname($path));
@@ -238,10 +251,17 @@ class Writer
     /**
      * Save translations to  temporary file; translations should already be cleansed.
      *
+     * @throws \Exception
      * @return bool|int False if failure, or number of bytes written
      */
     public function saveTemporary()
     {
+        $this->_applyFilters();
+
+        if (!$this->hasTranslations() || !$this->isValid()) {
+            throw new Exception('unable to save empty or invalid translations');
+        }
+
         $path = $this->getTemporaryTranslationPath();
 
         Common::mkdir(dirname($path));
@@ -250,13 +270,54 @@ class Writer
     }
 
     /**
+     * Adds an validator to check before saving
+     *
+     * @param ValidateAbstract $validator
+     */
+    public function addValidator(ValidateAbstract $validator)
+    {
+        $this->_validators[] = $validator;
+    }
+
+    /**
+     * Returns if translations are valid to save or not
+     *
+     * @return bool
+     */
+    public function isValid()
+    {
+        $this->_applyFilters();
+
+        $this->_validationMessage = null;
+
+        foreach ($this->_validators AS $validator) {
+            if (!$validator->isValid($this->_translations)) {
+                $this->_validationMessage = $validator->getMessage();
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    /**
+     * Returns last validation message
+     *
+     * @return null|string
+     */
+    public function getValidationMessage()
+    {
+        return $this->_validationMessage;
+    }
+
+    /**
      * Returns if the were translations removed while cleaning
      *
      * @return bool
      */
-    public function hasErrors()
+    public function wasFiltered()
     {
-        return !empty($this->_cleanErrors);
+        return !empty($this->_filterMessages);
     }
 
     /**
@@ -264,9 +325,17 @@ class Writer
      *
      * @return array
      */
-    public function getErrors()
+    public function getFilterMessages()
     {
-        return $this->_cleanErrors;
+        return $this->_filterMessages;
+    }
+
+    /**
+     * @param FilterAbstract $filter
+     */
+    public function addFilter(FilterAbstract $filter)
+    {
+        $this->_filters[] = $filter;
     }
 
     /**
@@ -274,77 +343,39 @@ class Writer
      *
      * @return bool   error state
      */
-    protected function _cleanTranslations()
+    protected function _applyFilters()
     {
         // skip if already cleaned
-        if ($this->_currentState == self::__CLEANED__) {
-            return $this->hasErrors();
+        if ($this->_currentState == self::__FILTERED__) {
+            return $this->wasFiltered();
         }
 
-        $this->_cleanErrors = array();
+        $this->_filterMessages = array();
 
         // skip if not translations available
         if (!$this->hasTranslations()) {
-            $this->_currentState = self::__CLEANED__;
+            $this->_currentState = self::__FILTERED__;
             return false;
         }
 
-        $basefilter = new ByBaseTranslations($this->_baseTranslations);
-        $cleanedTranslations = $basefilter->filter($this->_translations);
-        $filteredData = $basefilter->getFilteredData();
-        if (!empty($filteredData)) {
-            $this->_cleanErrors[] = "removed translations that are not present in base translations: " .var_export($filteredData, 1);
-        }
-
-        $emptyfilter = new EmptyTranslations($this->_baseTranslations);
-        $cleanedTranslations = $emptyfilter->filter($cleanedTranslations);
-        $filteredData = $emptyfilter->getFilteredData();
-        if (!empty($filteredData)) {
-            $this->_cleanErrors[] = "removed empty translations: " .var_export($filteredData, 1);
-        }
-
-        $parameterFilter = new ByParameterCount($this->_baseTranslations);
-        $cleanedTranslations = $parameterFilter->filter($cleanedTranslations);
-        $filteredData = $parameterFilter->getFilteredData();
-        if (!empty($filteredData)) {
-            $this->_cleanErrors[] = "removed translations that had diffrent parameter counts: " .var_export($filteredData, 1);
-        }
-
-        $whitespaceFilter = new UnnecassaryWhitespaces($this->_baseTranslations);
-        $cleanedTranslations = $whitespaceFilter->filter($cleanedTranslations);
-        $filteredData = $whitespaceFilter->getFilteredData();
-        if (!empty($filteredData)) {
-            $this->_cleanErrors[] = "filtered unnecassary whitespaces in some translations: " .var_export($filteredData, 1);
-        }
-
-        $entityFilter = new EncodedEntities($this->_baseTranslations);
-        $cleanedTranslations = $entityFilter->filter($cleanedTranslations);
-        $filteredData = $entityFilter->getFilteredData();
-        if (!empty($filteredData)) {
-            $this->_cleanErrors[] = "converting entities to characters in some translations: " .var_export($filteredData, 1);
-        }
-
-        $noscriptValidator = new NoScripts();
-        if (!$noscriptValidator->isValid($cleanedTranslations)) {
-            throw new Exception($noscriptValidator->getError());
-        }
+        $cleanedTranslations = $this->_translations;
 
-        // check requirements for core translations
-        if (empty($this->_pluginName)) {
+        foreach ($this->_filters AS $filter) {
 
-            $baseValidator = new CoreTranslations($this->_baseTranslations);
-            if(!$baseValidator->isValid($cleanedTranslations)) {
-                throw new Exception($baseValidator->getError());
+            $cleanedTranslations = $filter->filter($cleanedTranslations);
+            $filteredData = $filter->getFilteredData();
+            if (!empty($filteredData)) {
+                $this->_filterMessages[] = get_class($filter) . " changed: " .var_export($filteredData, 1);
             }
         }
 
-        $this->_currentState = self::__CLEANED__;
+        $this->_currentState = self::__FILTERED__;
 
         if ($cleanedTranslations != $this->_translations) {
-            $this->_cleanErrors[] = 'translations have been cleaned';
+            $this->_filterMessages[] = 'translations have been cleaned';
         }
 
         $this->_translations = $cleanedTranslations;
-        return $this->hasErrors();
+        return $this->wasFiltered();
     }
 }