Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve no app exception #369

Merged
merged 1 commit into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions src/AppScopeTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace Atk4\Core;

use Atk4\Ui\App;

/**
* Typical software design will create the application scope. Most frameworks
* relies on "static" properties, methods and classes. This does puts some
Expand All @@ -14,7 +16,7 @@
*/
trait AppScopeTrait
{
/** @var \Atk4\Ui\App Always points to current application. */
/** @var App */
private $_app;

/**
Expand Down Expand Up @@ -47,8 +49,8 @@ trait AppScopeTrait

protected function assertInstanceOfApp(object $app): void
{
if (!$app instanceof \Atk4\Ui\App) {
// called from phpunit, allow to use/test this trait without \Atk4\Ui\App class
if (!$app instanceof App) {
// called from phpunit, allow to test this trait without Atk4\Ui\App class
if (class_exists(\PHPUnit\Framework\TestCase::class, false)) {
foreach (debug_backtrace(\DEBUG_BACKTRACE_IGNORE_ARGS) as $frame) {
if (str_starts_with($frame['class'] ?? '', 'Atk4\Core\Tests\\')) {
Expand All @@ -57,7 +59,7 @@ protected function assertInstanceOfApp(object $app): void
}
}

throw new Exception('App must be instance of \Atk4\Ui\App');
throw new Exception('App must be instance of Atk4\Ui\App');
}
}

Expand All @@ -67,27 +69,29 @@ public function issetApp(): bool
}

/**
* @return \Atk4\Ui\App
* @return App
*/
public function getApp()
{
$this->assertInstanceOfApp($this->_app);
$app = $this->_app;
if ($app === null) {
throw new Exception('App is not set');
}

return $this->_app;
return $app;
}

/**
* @param \Atk4\Ui\App $app
* @param App $app
*
* @return static
*/
public function setApp(object $app)
{
$this->assertInstanceOfApp($app);
if ($this->issetApp() && $this->getApp() !== $app) {
if ($this->getApp()->catchExceptions || $this->getApp()->alwaysRun) { // allow to replace App created by AbstractView::initDefaultApp() - TODO fix
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no longer needed thanks to atk4/ui#1968

throw new Exception('App cannot be replaced');
}

if ($this->issetApp()) {
throw new Exception('App is already set');
}

$this->_app = $app;
Expand Down
4 changes: 3 additions & 1 deletion src/CollectionTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ public function _addIntoCollection(string $name, object $item, string $collectio
$this->{$collection}[$name] = $item;

// Carry on reference to application if we have appScopeTraits set
if (TraitUtil::hasAppScopeTrait($this) && TraitUtil::hasAppScopeTrait($item)) {
if ((TraitUtil::hasAppScopeTrait($this) && TraitUtil::hasAppScopeTrait($item))
&& (!$item->issetApp() || $item->getApp() !== $this->getApp())
) {
$item->setApp($this->getApp());
}

Expand Down
4 changes: 3 additions & 1 deletion src/ContainerTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ public function add($obj, $args = []): object
protected function _addContainer(object $element, array $args): void
{
// Carry on reference to application if we have appScopeTraits set
if (TraitUtil::hasAppScopeTrait($this) && TraitUtil::hasAppScopeTrait($element)) {
if (TraitUtil::hasAppScopeTrait($this) && TraitUtil::hasAppScopeTrait($element)
&& (!$element->issetApp() || $element->getApp() !== $this->getApp())
) {
$element->setApp($this->getApp());
}

Expand Down
2 changes: 1 addition & 1 deletion src/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ protected function _mergeSeeds(...$seeds)

if (count($injection) > 0) {
if (!TraitUtil::hasDiContainerTrait($obj)) {
throw (new Exception('Property injection is possible only to objects that use \Atk4\Core\DiContainerTrait trait'))
throw (new Exception('Property injection is possible only to objects that use Atk4\Core\DiContainerTrait trait'))
->addMoreInfo('object', $obj)
->addMoreInfo('injection', $injection);
}
Expand Down
2 changes: 1 addition & 1 deletion src/TrackableTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function getOwner(): object
public function setOwner(object $owner)
{
if ($this->issetOwner()) {
throw new Exception('Owner already set');
throw new Exception('Owner is already set');
}

$this->_owner = $owner;
Expand Down
2 changes: 1 addition & 1 deletion src/TraitUtil.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public static function hasTrait($class, string $traitName): bool
// prevent mass use for other than internal use then we can decide
// if we want to keep support this or replace with pure interfaces
if (!str_starts_with($traitName, 'Atk4\Core\\')) {
throw new Exception('Core::hasTrait is not indended for use with other than \Atk4\Core\* traits');
throw new Exception(self::class . '::hasTrait is not indended for use with other than Atk4\Core\* traits');
}

if (!isset(self::$_hasTraitCache[$class][$traitName])) {
Expand Down
21 changes: 21 additions & 0 deletions tests/AppScopeTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Atk4\Core\AppScopeTrait;
use Atk4\Core\ContainerTrait;
use Atk4\Core\Exception;
use Atk4\Core\NameTrait;
use Atk4\Core\Phpunit\TestCase;
use Atk4\Core\TrackableTrait;
Expand Down Expand Up @@ -39,6 +40,26 @@ public function testConstruct(): void
static::assertNull($this->getProtected($child, '_app'));
static::assertFalse($child->issetOwner());
}

public function testAppNotSetException(): void
{
$m = new AppScopeMock();

$this->expectException(Exception::class);
$this->expectErrorMessage('App is not set');
$m->getApp();
}

public function testAppSetTwiceException(): void
{
$m = new AppScopeMock();
$fakeApp = new \stdClass();
$m->setApp($fakeApp);

$this->expectException(Exception::class);
$this->expectErrorMessage('App is already set');
$m->setApp($fakeApp);
}
}

class AppScopeMock
Expand Down