optimizing flash code; added full coverage tests

Refs [#.0.x]

phalcon/Flash/AbstractFlash.zep

@@ -255,71 +255,42 @@ abstract class AbstractFlash extends AbstractInjectionAware implements FlashInte
      */
     public function outputMessage(string type, var message)
     {
-        bool implicitFlush;
-        var content, msg, htmlMessage, preparedMsg;
+        var content, html, item, prepared;
 
-        let implicitFlush = this->implicitFlush;
+        let content = "";
 
-        if typeof message == "array" {
-            /**
-             * We create the message with implicit flush or other
-             */
-            if !implicitFlush {
-                let content = "";
-            }
+        if typeof message !== "array" && typeof message !== "string" {
+            throw new Exception("The message must be an array or a string");
+        }
 
-            /**
-             * We create the message with implicit flush or other
-             */
-            for msg in message {
-                /**
-                 * Check if the message needs to be escaped
-                 */
-                let preparedMsg = this->prepareEscapedMessage(msg);
-
-                /**
-                 * We create the applying formatting or not
-                 */
-                let htmlMessage = this->prepareHtmlMessage(type, preparedMsg);
-
-                if implicitFlush {
-                    echo htmlMessage;
-                } else {
-                    let content .= htmlMessage;
-                    let this->messages[] = htmlMessage;
-                }
-            }
+        /**
+         * Make this an array. Same code processes string and array
+         */
+        if typeof message !== "array" {
+            let message = [message];
+        }
 
-            /**
-             * We return the message as a string if the implicitFlush is turned
-             * off
-             */
-            if !implicitFlush {
-                return content;
-            }
-        } else {
-            /**
-             * Check if the message needs to be escaped
-             */
-            let preparedMsg = this->prepareEscapedMessage(message);
-
-            /**
-             * We create the applying formatting or not
-             */
-            let htmlMessage = this->prepareHtmlMessage(type, preparedMsg);
-
-            /**
-             * We return the message as a string if the implicitFlush is turned
-             * off
-             */
-            if implicitFlush {
-                echo htmlMessage;
-            } else {
-                let this->messages[] = htmlMessage;
+        for item in message {
+            let prepared = this->prepareEscapedMessage(item),
+                html     = this->prepareHtmlMessage(type, prepared);
 
-                return htmlMessage;
+            if (true === this->implicitFlush) {
+                echo html;
+            } else {
+                let content          .= html,
+                    this->messages[] = html;
             }
         }
+
+        /**
+         * We return the message as a string if the implicitFlush is turned
+         * off
+         */
+        if (true !== this->implicitFlush) {
+            return content;
+        }
+
+        return null;
     }
 
     /**
@@ -343,18 +314,18 @@ abstract class AbstractFlash extends AbstractInjectionAware implements FlashInte
     {
         string divString, iconString, template;
 
-        let template     = "<div%divString%>%iconString%%message%</div>" . PHP_EOL,
-            divString    = "",
-            iconString   = "";
+        let template   = "<div%divString%>%iconString%%message%</div>" . PHP_EOL,
+            divString  = "",
+            iconString = "";
 
         if !empty this->customTemplate {
             return this->customTemplate;
         }
 
-        if !empty cssClassses || !empty cssIconClasses {
+        if !empty cssClassses {
             let divString = " class=\"%cssClass%\"";
             if !empty cssIconClasses {
-                let iconString = "<i class=\"%cssIconClasses%\"></i> ";
+                let iconString = "<i class=\"%cssIconClass%\"></i> ";
             }
         }
 
@@ -392,45 +363,47 @@ abstract class AbstractFlash extends AbstractInjectionAware implements FlashInte
      */
     private function prepareHtmlMessage(string type, string message) -> string
     {
-        var classes, cssClasses, cssIconClasses, typeClasses, typeIconClasses, automaticHtml;
-
-        let automaticHtml = this->automaticHtml;
+        var cssClasses, cssIconClasses;
 
-        if true !== automaticHtml {
+        if true !== this->automaticHtml {
             return message;
         }
 
-        let classes        = this->cssClasses,
-            cssIconClasses = this->cssIconClasses;
-
-        if fetch typeClasses, classes[type] {
-            if typeof typeClasses == "array" {
-                let cssClasses = join(" ", typeClasses);
-            } else {
-                let cssClasses = typeClasses;
-            }
-        } else {
-            let cssClasses = "";
-        }
-
-        if fetch typeIconClasses, cssIconClasses[type] {
-            if typeof typeIconClasses == "array" {
-                let cssIconClasses = join(" ", typeIconClasses);
-            } else {
-                let cssIconClasses = typeIconClasses;
-            }
-        } else {
-            let cssIconClasses = "";
-        }
-
+        let cssClasses     = this->checkClasses(this->cssClasses, type),
+            cssIconClasses = this->checkClasses(this->cssIconClasses, type);
 
         return this->interpolator->__invoke(
             this->getTemplate(cssClasses, cssIconClasses),
             [
                 "cssClass"     : cssClasses,
-                "iconCssClass" : cssIconClasses,
+                "cssIconClass" : cssIconClasses,
                 "message"      : message
             ]
         );
     }
+
+    /**
+     * Checks the collection and returns the content as a string
+     * (array is joined)
+     */
+    private function checkClasses(array collection, string type) -> string
+    {
+        var content;
+
+        let content = "";
+
+        if isset collection[type] {
+            let content = collection[type];
+        }
+
+        if true !== empty(content) {
+            if typeof content !== "array" {
+                let content = [content];
+            }
+
+            let content = join(" ", content);
+        }
+
+        return content;
+    }
 }

phalcon/Flash/Session.zep

@@ -92,7 +92,6 @@ class Session extends AbstractFlash
         var type, message, messages;
 
         let messages = this->getSessionMessages(remove);
-
         for type, message in messages {
             this->outputMessage(type, message);
         }
@@ -108,7 +107,7 @@ class Session extends AbstractFlash
      *
      * @return array
      */
-    protected function getSessionMessages(bool remove, var type = null) -> array
+    protected function getSessionMessages(bool remove, string type = null) -> array
     {
         var session, messages, returnMessages;
 
@@ -118,11 +117,11 @@ class Session extends AbstractFlash
         /**
          * Session might be empty
          */
-        if typeof messages != "array" {
+        if typeof messages !== "array" {
             let messages = [];
         }
 
-        if typeof type == "string" {
+        if true !== empty(type) {
             if fetch returnMessages, messages[type] {
                 if remove {
                     unset(messages[type]);

tests/unit/Flash/Direct/GetSetDICest.php

@@ -13,7 +13,7 @@
 
 namespace Phalcon\Tests\Unit\Flash\Direct;
 
-use Phalcon\Di\Di;
+use Phalcon\Di;
 use Phalcon\Flash\Direct;
 use UnitTester;
 

tests/unit/Flash/Direct/GetSetEscaperServiceCest.php

@@ -13,10 +13,10 @@
 
 namespace Phalcon\Tests\Unit\Flash\Direct;
 
-use Phalcon\Di\Di;
+use Phalcon\Di;
 use Phalcon\Flash\Direct;
 use Phalcon\Flash\Exception;
-use Phalcon\Html\Escaper;
+use Phalcon\Escaper;
 use UnitTester;
 
 use function spl_object_hash;
@@ -91,7 +91,7 @@ public function flashDirectGetEscaperServiceException(UnitTester $I)
         $I->expectThrowable(
             new Exception(
                 'A dependency injection container is required to ' .
-                'access the "escaper" service'
+                "access the 'escaper' service"
             ),
             function () {
                 $flash = new Direct();

tests/unit/Flash/Direct/OutputCest.php

@@ -15,7 +15,7 @@
 
 use Codeception\Example;
 use Phalcon\Flash\Direct;
-use Phalcon\Html\Escaper;
+use Phalcon\Escaper;
 use UnitTester;
 
 use function ob_end_clean;

tests/unit/Flash/Direct/OutputMessageCest.php

@@ -16,7 +16,7 @@
 use Codeception\Example;
 use Phalcon\Flash\Direct;
 use Phalcon\Flash\Exception;
-use Phalcon\Html\Escaper;
+use Phalcon\Escaper;
 use UnitTester;
 
 use const PHP_EOL;
@@ -82,7 +82,7 @@ public function flashDirectOutputMessage(UnitTester $I, Example $example)
     }
 
     /**
-     * @return array
+     * @return \string[][]
      */
     private function getExamples(): array
     {

tests/unit/Flash/Session/ConstructCest.php

@@ -63,7 +63,7 @@ public function flashSessionConstructNoSessionService(UnitTester $I)
         $I->expectThrowable(
             new Exception(
                 'A dependency injection container is required to ' .
-                'access the "session" service'
+                "access the 'session' service"
             ),
             function () {
                 $flash = new Session();

tests/unit/Flash/Session/GetSetDICest.php

@@ -13,7 +13,7 @@
 
 namespace Phalcon\Tests\Unit\Flash\Session;
 
-use Phalcon\Di\Di;
+use Phalcon\Di;
 use Phalcon\Flash\Session;
 use UnitTester;
 

tests/unit/Flash/Session/GetSetEscaperServiceCest.php

@@ -15,7 +15,7 @@
 
 use Phalcon\Flash\Exception;
 use Phalcon\Flash\Session;
-use Phalcon\Html\Escaper;
+use Phalcon\Escaper;
 use Phalcon\Tests\Fixtures\Traits\DiTrait;
 use UnitTester;
 
@@ -104,7 +104,7 @@ public function flashSessionGetEscaperServiceException(UnitTester $I)
         $I->expectThrowable(
             new Exception(
                 'A dependency injection container is required to ' .
-                'access the "escaper" service'
+                "access the 'escaper' service"
             ),
             function () {
                 $flash = new Session();

tests/unit/Flash/Session/OutputCest.php

@@ -86,6 +86,7 @@ public function flashSessionOutputTypes(UnitTester $I)
 
         $flash = new Session();
         $flash->setDI($this->container);
+        $flash->clear();
 
         $message1 = uniqid('m-');
         $message2 = uniqid('m-');

tests/unit/Support/Version/GetIdCest.php

@@ -14,10 +14,8 @@
 namespace Phalcon\Tests\Unit\Support\Version;
 
 use Codeception\Example;
-use Phalcon\Tests\Fixtures\Support\Version\VersionAlphaFixture;
-use Phalcon\Tests\Fixtures\Support\Version\VersionBetaFixture;
-use Phalcon\Tests\Fixtures\Support\Version\VersionRcFixture;
-use Phalcon\Tests\Fixtures\Support\Version\VersionStableFixture;
+use Codeception\Util\Stub;
+use Phalcon\Support\Version;
 use Phalcon\Tests\Fixtures\Traits\VersionTrait;
 use UnitTester;
 
@@ -42,9 +40,14 @@ public function supportVersionGetId(UnitTester $I, Example $example)
     {
         $I->wantToTest('Version - getId() - ' . $example[0]);
 
-        $version = new $example[1]();
+        $version = Stub::make(
+            Version::class,
+            [
+                'getVersion' => $example[2],
+            ]
+        );
 
-        $expected = $example[2];
+        $expected = $example[1];
         $actual   = $version->getId();
         $I->assertTrue(is_string($actual));
         $I->assertEquals($expected, $actual);
@@ -58,23 +61,23 @@ private function getExamples(): array
         return [
             [
                 'alpha',
-                VersionAlphaFixture::class,
                 '5000011',
+                [5, 0, 0, 1, 1],
             ],
             [
                 'beta',
-                VersionBetaFixture::class,
                 '5000022',
+                [5, 0, 0, 2, 2],
             ],
             [
                 'rc',
-                VersionRcFixture::class,
                 '5000033',
+                [5, 0, 0, 3, 3],
             ],
             [
                 'stable',
-                VersionStableFixture::class,
-                '5000000',
+                '5000040',
+                [5, 0, 0, 4, 0],
             ],
         ];
     }