Move "app" and "owner" props to getter/setter (#277)

* Move "app" prop to getter/setter * fix return type check * Move "owner" prop to getter/setter * add unsetOwner() * revert by-ref TestCase::callProtected() params * allow to call setApp() with the same instance more than once * allow implicit replace App created by View::initDefaultApp()
atk4 · Oct 6, 2020 · 88412f2 · 88412f2
1 parent 196e8a3
commit 88412f2
Show file tree

Hide file tree

Showing 16 changed files with 238 additions and 99 deletions.
diff --git a/docs/debug.rst b/docs/debug.rst
@@ -40,7 +40,7 @@ DebugTrait::
     $this->debug('User {user} created', ['user'=>$user]);
 
 The Application itself can use DebugTrait too and normally should do, making it
-possible to use ``$this->app->debug()``.
+possible to use ``$this->getApp()->debug()``.
 
 Various objects may implement DebugTrait and also invoke $this->debug(), but in
 most cases this will simply be ignored right away unless you manually enable
@@ -53,15 +53,15 @@ debugging for the object::
     $obj1->debug('test1'); // will go to logger
     $obj2->debug('test2'); // will not go to logger because debug is not enabled for this object
 
-Executing debug will look for ``$this->app`` link and if the application
-implements ``Psr\Log\LoggerInterface``, then ``$this->app->log()`` will be
+Executing debug will look for ``$this->getApp()`` link and if the application
+implements ``Psr\Log\LoggerInterface``, then ``$this->getApp()->log()`` will be
 called using LogLevel DEBUG.
 
 Log
 ---
 
 Log method will log message every time. DebugTrait implements the ``log()``
-method which will either display information on the STDOUT (if ``$this->app``
+method which will either display information on the STDOUT (if ``$this->getApp()``
 does not exist or does not implement PSR-3)
 
 Message

diff --git a/docs/dynamicmethod.rst b/docs/dynamicmethod.rst
@@ -20,7 +20,7 @@ If object has application scope :php:trait:`AppScopeTrait` and the application
 implements :php:trait:`HookTrait` then executing $object->test() will also
 look for globally-registered method inside the application::
 
-    $object->app->addGlobalMethod('test', function($app, $o, $args){
+    $object->getApp()->addGlobalMethod('test', function($app, $o, $args){
         echo 'hello, '.$args[0];
     });
 

diff --git a/src/AppScopeTrait.php b/src/AppScopeTrait.php
@@ -22,11 +22,16 @@ trait AppScopeTrait
     public $_appScopeTrait = true;
 
     /**
-     * Always points to current Application.
+     * @internal to be removed in Jan 2021, keep until then to prevent wrong assignments
+     */
+    private $app;
+
+    /**
+     * Always points to current application.
      *
-     * @var \atk4\ui\App
+     * @var App
      */
-    public $app;
+    private $_app;
 
     /**
      * When using mechanism for ContainerTrait, they inherit name of the
@@ -59,4 +64,64 @@ trait AppScopeTrait
      * @var array
      */
     public $unique_hashes = [];
+
+    private function assertInstanceOfApp(object $app): void
+    {
+        // called from phpunit, allow to use/test this trait without \atk4\ui\App class
+        if (class_exists(\PHPUnit\Framework\TestCase::class, false)) {
+            return;
+        }
+
+        if (!$app instanceof \atk4\ui\App) {
+            throw new Exception('App must be instance of \atk4\ui\App');
+        }
+    }
+
+    /**
+     * To be removed in Jan 2021.
+     */
+    private function assertNoDirectAppAssignment(): void
+    {
+        if ($this->app !== null) {
+            throw new Exception('App can not be assigned directly');
+        }
+    }
+
+    public function issetApp(): bool
+    {
+        $this->assertNoDirectAppAssignment();
+
+        return $this->_app !== null;
+    }
+
+    /**
+     * @return \atk4\ui\App
+     */
+    public function getApp()
+    {
+        $this->assertNoDirectAppAssignment();
+        $this->assertInstanceOfApp($this->_app);
+
+        return $this->_app;
+    }
+
+    /**
+     * @param \atk4\ui\App $app
+     *
+     * @return static
+     */
+    public function setApp(object $app)
+    {
+        $this->assertNoDirectAppAssignment();
+        $this->assertInstanceOfApp($app);
+        if ($this->issetApp() && $this->getApp() !== $app) {
+            if ($this->getApp()->catch_exceptions || $this->getApp()->always_run) { // allow to replace App created by AbstractView::initDefaultApp() - TODO fix
+                throw new Exception('App can not be replaced');
+            }
+        }
+
+        $this->_app = $app;
+
+        return $this;
+    }
 }
diff --git a/src/AtkPhpunit/TestCase.php b/src/AtkPhpunit/TestCase.php
@@ -49,7 +49,30 @@ public function &getProtected(object $obj, string $name)
     }
 
     /**
-     * Fake test. Otherwise Travis gives warning that there are no tests in here.
+     * Sets protected property value.
+     *
+     * NOTE: this method must only be used for low-level functionality, not
+     * for general test-scripts.
+     *
+     * @param mixed $value
+     *
+     * @return static
+     */
+    public function setProtected(object $obj, string $name, &$value, bool $byReference = false)
+    {
+        \Closure::bind(static function () use ($obj, &$value, $byReference) {
+            if ($byReference) {
+                $obj->{$name} = &$value;
+            } else {
+                $obj->{$name} = $value;
+            }
+        }, null, $obj)();
+
+        return $this;
+    }
+
+    /**
+     * Fake test. Otherwise phpunit gives warning that there are no tests in here.
      *
      * @doesNotPerformAssertions
      */

diff --git a/src/CollectionTrait.php b/src/CollectionTrait.php
@@ -48,16 +48,13 @@ public function _addIntoCollection(string $name, object $item, string $collectio
 
         // Carry on reference to application if we have appScopeTraits set
         if (isset($this->_appScopeTrait) && isset($item->_appScopeTrait)) {
-            $item->app = $this->app;
+            $item->setApp($this->getApp());
         }
 
         // Calculate long "name" but only if both are trackables
         if (isset($item->_trackableTrait)) {
             $item->short_name = $name;
-            if ($item->owner !== null) {
-                throw new Exception('Element owner is already set');
-            }
-            $item->owner = $this;
+            $item->setOwner($this);
             if (isset($this->_trackableTrait)) {
                 $item->name = $this->_shorten_ml($this->name . '-' . $collection . '_' . $name);
             }
@@ -102,8 +99,8 @@ public function _cloneCollection(string $collectionName): void
     {
         $this->{$collectionName} = array_map(function ($item) {
             $item = clone $item;
-            if (isset($item->owner)) {
-                $item->owner = $this;
+            if (isset($item->_trackableTrait) && $item->issetOwner()) {
+                $item->unsetOwner()->setOwner($this);
             }
 
             return $item;
@@ -159,11 +156,11 @@ public function shorten(?object $app, string $desired): string
                         $this->_appScopeTrait = $app !== null;
 
                         try {
-                            $this->app = $app;
+                            $this->setApp($app);
 
                             return $this->_shorten($desired);
                         } finally {
-                            $this->app = null; // important for GC
+                            $this->_app = null; // important for GC
                         }
                     }
                 };
@@ -172,6 +169,6 @@ public function shorten(?object $app, string $desired): string
             return $factory->collectionTraitHelper;
         }, null, Factory::class)();
 
-        return $collectionTraitHelper->shorten($this->_appScopeTrait ? $this->app : null, $desired);
+        return $collectionTraitHelper->shorten($this->_appScopeTrait ? $this->getApp() : null, $desired);
     }
 }
diff --git a/src/ContainerTrait.php b/src/ContainerTrait.php
@@ -89,7 +89,7 @@ protected function _add_Container(object $element, $args = []): object
     {
         // Carry on reference to application if we have appScopeTraits set
         if (isset($this->_appScopeTrait) && isset($element->_appScopeTrait)) {
-            $element->app = $this->app;
+            $element->setApp($this->getApp());
         }
 
         // If element is not trackable, then we don't need to do anything with it
@@ -130,10 +130,7 @@ protected function _add_Container(object $element, $args = []): object
                 ->addMoreInfo('arg2', $args);
         }
 
-        if ($element->owner !== null) {
-            throw new Exception('Element owner is already set');
-        }
-        $element->owner = $this;
+        $element->setOwner($this);
         $element->short_name = $args[0];
         if (isset($this->_nameTrait)) {
             $element->name = $this->_shorten($this->name . '_' . $element->short_name);
@@ -182,27 +179,25 @@ public function removeElement($short_name)
      */
     protected function _shorten(string $desired): string
     {
-        if (
-            isset($this->_appScopeTrait) &&
-            isset($this->app->max_name_length) &&
-            mb_strlen($desired) > $this->app->max_name_length
-        ) {
+        if (isset($this->_appScopeTrait)
+            && isset($this->getApp()->max_name_length)
+            && mb_strlen($desired) > $this->getApp()->max_name_length) {
             /*
              * Basic rules: hash is 10 character long (8+2 for separator)
              * We need at least 5 characters on the right side. Total must not exceed
              * max_name_length. First chop will be max-10, then chop size will increase by
              * max-15
              */
             $len = mb_strlen($desired);
-            $left = $len - ($len - 10) % ($this->app->max_name_length - 15) - 5;
+            $left = $len - ($len - 10) % ($this->getApp()->max_name_length - 15) - 5;
 
             $key = mb_substr($desired, 0, $left);
             $rest = mb_substr($desired, $left);
 
-            if (!isset($this->app->unique_hashes[$key])) {
-                $this->app->unique_hashes[$key] = '_' . dechex(crc32($key));
+            if (!isset($this->getApp()->unique_hashes[$key])) {
+                $this->getApp()->unique_hashes[$key] = '_' . dechex(crc32($key));
             }
-            $desired = $this->app->unique_hashes[$key] . '__' . $rest;
+            $desired = $this->getApp()->unique_hashes[$key] . '__' . $rest;
         }
 
         return $desired;

diff --git a/src/DebugTrait.php b/src/DebugTrait.php
@@ -49,7 +49,7 @@ public function debug($message = true, array $context = [])
 
         // if debug is enabled, then log it
         if ($this->debug) {
-            if (!isset($this->app) || !isset($this->app->logger) || !$this->app->logger instanceof \Psr\Log\LoggerInterface) {
+            if (!isset($this->_appScopeTrait) || !$this->issetApp() || !$this->getApp()->logger instanceof \Psr\Log\LoggerInterface) {
                 $message = '[' . static::class . ']: ' . $message;
             }
             $this->log(LogLevel::DEBUG, $message, $context);
@@ -68,8 +68,8 @@ public function debug($message = true, array $context = [])
      */
     public function log($level, $message, array $context = [])
     {
-        if (isset($this->app) && isset($this->app->logger) && $this->app->logger instanceof \Psr\Log\LoggerInterface) {
-            $this->app->logger->log($level, $message, $context);
+        if (isset($this->_appScopeTrait) && $this->issetApp() && $this->getApp()->logger instanceof \Psr\Log\LoggerInterface) {
+            $this->getApp()->logger->log($level, $message, $context);
         } else {
             $this->_echo_stderr($message . "\n");
         }
@@ -85,10 +85,10 @@ public function log($level, $message, array $context = [])
      */
     public function userMessage(string $message, array $context = [])
     {
-        if (isset($this->app) && $this->app instanceof \atk4\core\AppUserNotificationInterface) {
-            $this->app->userNotification($message, $context);
-        } elseif (isset($this->app) && $this->app instanceof \Psr\Log\LoggerInterface) {
-            $this->app->log('warning', 'Could not notify user about: ' . $message, $context);
+        if (isset($this->_appScopeTrait) && $this->issetApp() && $this->getApp() instanceof \atk4\core\AppUserNotificationInterface) {
+            $this->getApp()->userNotification($message, $context);
+        } elseif (isset($this->_appScopeTrait) && $this->issetApp() && $this->getApp() instanceof \Psr\Log\LoggerInterface) {
+            $this->getApp()->log('warning', 'Could not notify user about: ' . $message, $context);
         } else {
             $this->_echo_stderr("Could not notify user about: {$message}\n");
         }

diff --git a/src/DynamicMethodTrait.php b/src/DynamicMethodTrait.php
@@ -55,9 +55,9 @@ public function tryCall($name, $args)
             return $ret;
         }
 
-        if (isset($this->_appScopeTrait) && isset($this->app->_hookTrait)) {
+        if (isset($this->_appScopeTrait) && isset($this->getApp()->_hookTrait)) {
             array_unshift($args, $this);
-            if ($ret = $this->app->hook($this->buildMethodHookName($name, true), $args)) {
+            if ($ret = $this->getApp()->hook($this->buildMethodHookName($name, true), $args)) {
                 return $ret;
             }
         }
@@ -136,7 +136,7 @@ public function removeMethod(string $name)
     public function addGlobalMethod(string $name, \Closure $fx): void
     {
         // AppScopeTrait and HookTrait for app are mandatory
-        if (!isset($this->_appScopeTrait) || !isset($this->app->_hookTrait)) {
+        if (!isset($this->_appScopeTrait) || !isset($this->getApp()->_hookTrait)) {
             throw new Exception('You need AppScopeTrait and HookTrait traits, see docs');
         }
 
@@ -145,7 +145,7 @@ public function addGlobalMethod(string $name, \Closure $fx): void
                 ->addMoreInfo('name', $name);
         }
 
-        $this->app->onHook($this->buildMethodHookName($name, true), $fx);
+        $this->getApp()->onHook($this->buildMethodHookName($name, true), $fx);
     }
 
     /**
@@ -155,10 +155,9 @@ public function addGlobalMethod(string $name, \Closure $fx): void
      */
     public function hasGlobalMethod(string $name): bool
     {
-        return
-            isset($this->_appScopeTrait) &&
-            isset($this->app->_hookTrait) &&
-            $this->app->hookHasCallbacks($this->buildMethodHookName($name, true));
+        return isset($this->_appScopeTrait)
+            && isset($this->getApp()->_hookTrait)
+            && $this->getApp()->hookHasCallbacks($this->buildMethodHookName($name, true));
     }
 
     /**
@@ -168,8 +167,8 @@ public function hasGlobalMethod(string $name): bool
      */
     public function removeGlobalMethod(string $name): void
     {
-        if (isset($this->_appScopeTrait) && isset($this->app->_hookTrait)) {
-            $this->app->removeHook($this->buildMethodHookName($name, true));
+        if (isset($this->_appScopeTrait) && isset($this->getApp()->_hookTrait)) {
+            $this->getApp()->removeHook($this->buildMethodHookName($name, true));
         }
     }
 }