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.

Sign up for GitHub

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

Check bean return type against return type annotation #40

Merged
merged 1 commit into from
Jun 15, 2016
Merged

Check bean return type against return type annotation #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 18 additions & 2 deletions src/bitExpert/Disco/Proxy/Configuration/MethodGenerator/BeanMethod.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,17 @@ public static function generateMethod(
$body .= $padding . ' $initializer = null;' . "\n";
$body .= $padding . ' $wrappedObject = parent::' . $methodName . '(' . $methodParamTpl . ');' . "\n";
$body .= $padding . ' $this->initializeBean($wrappedObject, "' . $methodName . '");' . "\n";
$body .= $padding . ' if (!is_a($wrappedObject, \'' . $beanType . '\')) {' . "\n";
$body .= $padding . ' throw new \\bitExpert\\Disco\\BeanException(sprintf(' . "\n";
$body .= $padding . ' \'Bean "%s" has declared "%s" as return type but returned "%s"\',' . "\n";
$body .= $padding . ' \'' . $originalMethod->getName() . '\',' . "\n";
$body .= $padding . ' \'' . $beanType . '\',' . "\n";
$body .= $padding . ' $wrappedObject ? get_class($wrappedObject) : \'null\'' . "\n";
$body .= $padding . ' ));' . "\n";
$body .= $padding . ' }'. "\n\n";
$body .= $padding . ' return true;' . "\n";
$body .= $padding . '};' . "\n\n";
$body .= $padding . '$instance = $factory->createProxy("' . $beanType . '", $initializer);' . "\n";
$body .= $padding . '$instance = $factory->createProxy("' . $beanType . '", $initializer);' . "\n\n";
} else {
$innerpadding = $padding;
if ($methodAnnotation->isSingleton()) {
Expand All @@ -120,7 +128,15 @@ public static function generateMethod(
}

$body .= $innerpadding . '$instance = parent::' . $methodName . '(' . $methodParamTpl . ');' . "\n";
$body .= $innerpadding . '$this->initializeBean($instance, "' . $methodName . '");' . "\n";
$body .= $innerpadding . '$this->initializeBean($instance, "' . $methodName . '");' . "\n\n";
$body .= $innerpadding . 'if (!is_a($instance, \'' . $beanType . '\')) {' . "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Can we replace the is_a() function call with an instanceof check? That might potentially be faster as it is not a method call.

In addition the check should be moved before the $this->initializeBean(). If the expected value is not returned, there is no need to run the initializer logic. Failing early makes sense to me in this case.

$body .= $innerpadding . ' throw new \\bitExpert\\Disco\\BeanException(sprintf(' . "\n";
$body .= $innerpadding . ' \'Bean "%s" has declared "%s" as return type but returned "%s"\',' . "\n";
$body .= $innerpadding . ' \'' . $originalMethod->getName() . '\',' . "\n";
$body .= $innerpadding . ' \'' . $beanType . '\',' . "\n";
$body .= $innerpadding . ' $instance ? get_class($instance) : \'null\'' . "\n";
$body .= $innerpadding . ' ));' . "\n";
$body .= $innerpadding . '}'. "\n\n";

if ($methodAnnotation->isSingleton()) {
$body .= $padding . '}' . "\n";
Expand Down
51 changes: 51 additions & 0 deletions tests/bitExpert/Disco/AnnotationBeanFactoryUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use bitExpert\Disco\Config\BeanConfigurationWithParameters;
use bitExpert\Disco\Config\BeanConfigurationWithPostProcessor;
use bitExpert\Disco\Config\BeanConfigurationWithProtectedMethod;
use bitExpert\Disco\Config\WrongReturnTypeConfiguration;
use bitExpert\Disco\Helper\BeanFactoryAwareService;
use bitExpert\Disco\Helper\MasterService;
use bitExpert\Disco\Helper\SampleService;
Expand Down Expand Up @@ -405,4 +406,54 @@ public function enablingProxyAutoloaderRegistersAdditionalAutoloader()
count($autoloaderFunctionsBeforeBeanFactoryInit) + 1 === count($autoloaderFunctionsAfterBeanFactoryInit)
);
}

/**
* @test
* @expectedException \bitExpert\Disco\BeanException
*/
public function throwsExceptionIfTypeOfReturnedObjectIsNotExpectedOfNonLazyBean()
{
$this->beanFactory = new AnnotationBeanFactory(WrongReturnTypeConfiguration::class);
BeanFactoryRegistry::register($this->beanFactory);

$this->beanFactory->get('nonLazyBeanReturningSomethingWrong');
}

/**
* @test
* @expectedException \bitExpert\Disco\BeanException
*/
public function throwsExceptionIfNonLazyBeanMethodDoesNotReturnAnything()
{
$this->beanFactory = new AnnotationBeanFactory(WrongReturnTypeConfiguration::class);
BeanFactoryRegistry::register($this->beanFactory);

$this->beanFactory->get('nonLazyBeanNotReturningAnything');
}

/**
* @test
* @expectedException \bitExpert\Disco\BeanException
*/
public function throwsExceptionIfTypeOfReturnedObjectIsNotExpectedOfLazyBean()
{
$this->beanFactory = new AnnotationBeanFactory(WrongReturnTypeConfiguration::class);
BeanFactoryRegistry::register($this->beanFactory);

$bean = $this->beanFactory->get('lazyBeanReturningSomethingWrong');
$bean->setTest('test');
}

/**
* @test
* @expectedException \bitExpert\Disco\BeanException
*/
public function throwsExceptionIfLazyBeanMethodDoesNotReturnAnything()
{
$this->beanFactory = new AnnotationBeanFactory(WrongReturnTypeConfiguration::class);
BeanFactoryRegistry::register($this->beanFactory);

$bean = $this->beanFactory->get('lazyBeanNotReturningAnything');
$bean->setTest('test');
}
}
58 changes: 58 additions & 0 deletions tests/bitExpert/Disco/Config/WrongReturnTypeConfiguration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

/*
* This file is part of the Disco package.
*
* (c) bitExpert AG
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace bitExpert\Disco\Config;

use bitExpert\Disco\Annotations\Bean;
use bitExpert\Disco\Annotations\Configuration;
use bitExpert\Disco\Helper\MasterService;
use bitExpert\Disco\Helper\SampleService;

/**
* @Configuration
*/
class WrongReturnTypeConfiguration
{
/**
* @Bean({"singleton"=false, "lazy"=false, "scope"="request"})
* @return SampleService
*/
public function nonLazyBeanNotReturningAnything()
{

}

/**
* @Bean({"singleton"=false, "lazy"=false, "scope"="request"})
* @return SampleService
*/
public function nonLazyBeanReturningSomethingWrong()
{
return new MasterService(new SampleService());
}

/**
* @Bean({"singleton"=false, "lazy"=true, "scope"="request"})
* @return SampleService
*/
public function lazyBeanNotReturningAnything()
{

}

/**
* @Bean({"singleton"=false, "lazy"=true, "scope"="request"})
* @return SampleService
*/
public function lazyBeanReturningSomethingWrong()
{
return new MasterService(new SampleService());
}
}