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

Added workaround for #327 #328

Merged
merged 5 commits into from
Jul 12, 2017
Merged
Show file tree
Hide file tree
Changes from 4 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
24 changes: 22 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Features
* Ability to change the return value of any methods/functions via `Around` type of advice.
* Rich pointcut grammar syntax for defining pointcuts in the source code.
* Native debugging for AOP with XDebug. The code with weaved aspects is fully readable and native. You can put a breakpoint in the original class or in the aspect and it will work (for debug mode)!
* Can be integrated with any existing PHP frameworks and libraries.
* Can be integrated with any existing PHP frameworks and libraries (with or without additional configuration).
* Highly optimized for production use: support of opcode cachers, lazy loading of advices and aspects, joinpoints caching, no runtime checks of pointcuts, no runtime annotations parsing, no evals and `__call` methods, no slow proxies and `call_user_func_array()`. Fast bootstraping process (2-20ms) and advice invocation.


Expand Down Expand Up @@ -194,7 +194,27 @@ use Aspect\MonitorAspect;

Now you are ready to use the power of aspects! Feel free to change anything everywhere. If you like this project, you could support it <a href="https://flattr.com/submit/auto?fid=83r77w&url=https%3A%2F%2Fgithub.com%2Fgoaop%2Fframework" target="_blank"><img src="https://button.flattr.com/flattr-badge-large.png" alt="Flattr this" title="Flattr this" border="0"></a> [![Gratipay](https://img.shields.io/gratipay/lisachenko.svg)](https://gratipay.com/lisachenko/)

### 6. Contribution
### 6. Optional configurations

#### 6.1 Support for weaving Doctrine entities

Weaving Doctrine entities can not be supported out of the box due to the fact
that Go! AOP generates two sets of classes for each weaved entity, a concrete class and
proxy with pointcuts. Doctrine will interpret both of those classes as concrete entities
and assign for both of them same metadata, which would mess up the database and relations
(see [https://github.com/goaop/framework/issues/327](https://github.com/goaop/framework/issues/327)).

Therefore, a workaround is provided with this library which will sort out
mapping issue in Doctrine. Workaround is in form of event subscriber,
`Go\Bridge\Doctrine\MetadataLoadInterceptor` which has to be registered
when Doctrine is bootstraped in your project. For details how to do that,
see [http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/events.html](http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/events.html).

Event subscriber will modify metadata entity definition for generated Go! Aop proxies
as mapped superclass. That would sort out issues on which you may stumble upon when
weaving Doctrine entities.

### 7. Contribution

To contribute changes see the [Contribute Readme](CONTRIBUTE.md)

Expand Down
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"require-dev": {
"symfony/console": "^2.7|^3.0",
"adlawson/vfs": "^0.12",
"phpunit/phpunit": "^4.8"
"phpunit/phpunit": "^4.8",
"doctrine/orm": "^2.5"
},

"suggest": {
Expand Down
148 changes: 148 additions & 0 deletions src/Bridge/Doctrine/MetadataLoadInterceptor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
<?php
/*
* Go! AOP framework
*
* @copyright Copyright 2012, Lisachenko Alexander <[email protected]>
*
* This source file is subject to the license that is bundled
* with this source code in the file LICENSE.
*/
namespace Go\Bridge\Doctrine;

use Doctrine\Common\EventSubscriber;
use Doctrine\ORM\Event\LoadClassMetadataEventArgs;
use Doctrine\ORM\Events;
use Doctrine\ORM\Mapping\ClassMetadata;
use Go\Core\AspectContainer;

/**
* Class MetadataLoadInterceptor
*
* Support for weaving Doctrine entities.
*/
final class MetadataLoadInterceptor implements EventSubscriber
{
/**
* {@inheritdoc}
*/
public function getSubscribedEvents()
{
return [
Events::loadClassMetadata
];
}

/**
* Handles \Doctrine\ORM\Events::loadClassMetadata event by modifying metadata of Go! AOP proxied classes.
*
* This method intercepts loaded metadata of Doctrine's entities which are weaved by Go! AOP,
* and denotes them as mapped superclass. If weaved entities uses mappings from traits
* (such as Timestampable, Blameable, etc... from https://github.com/Atlantic18/DoctrineExtensions),
* it will remove all mappings from proxied class for fields inherited from traits in order to prevent
* collision with concrete subclass of weaved entity. Fields from trait will be present in concrete subclass
* of weaved entitites.
*
* @see http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/inheritance-mapping.html#mapped-superclasses
* @see https://github.com/Atlantic18/DoctrineExtensions
*
* @param LoadClassMetadataEventArgs $args
*/
public function loadClassMetadata(LoadClassMetadataEventArgs $args)
{
/**
* @var ClassMetadata $metadata
*/
$metadata = $args->getClassMetadata();

if (1 === preg_match(sprintf('/.+(%s)$/', AspectContainer::AOP_PROXIED_SUFFIX), $metadata->name)) {

Choose a reason for hiding this comment

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

Expected 1 newline after opening brace; 2 found

$metadata->isMappedSuperclass = true;
$metadata->isEmbeddedClass = false;
$metadata->table = [];
$metadata->customRepositoryClassName = null;

$this->removeMappingsFromTraits($metadata);
}
}

/**
* Remove fields in Go! AOP proxied class metadata that are inherited
* from traits.
*
* @param ClassMetadata $metadata
*/
private function removeMappingsFromTraits(ClassMetadata $metadata)
{
$traits = $this->getTraits($metadata->name);

foreach ($traits as $trait) {

Choose a reason for hiding this comment

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

Expected 1 newline after opening brace; 2 found

$trait = new \ReflectionClass($trait);

/**
* @var \ReflectionProperty $property
*/
foreach ($trait->getProperties() as $property) {

Choose a reason for hiding this comment

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

Expected 1 newline after opening brace; 2 found

$name = $property->getName();

if (isset($metadata->fieldMappings[$name])) {
$mapping = $metadata->fieldMappings[$name];

unset(
$metadata->fieldMappings[$name],
$metadata->fieldNames[$mapping['columnName']],
$metadata->columnNames[$name]
);
}
}
}
}

/**
* Get ALL traits used by one class.
*
* This method is copied from https://github.com/RunOpenCode/traitor-bundle/blob/master/src/RunOpenCode/Bundle/Traitor/Utils/ClassUtils.php
*
* @param object|string $objectOrClass Instance of class or FQCN
* @param bool $autoload Weather to autoload class.
*
* @throws \InvalidArgumentException
* @throws \RuntimeException
*
* @return array Used traits.
*/
private function getTraits($objectOrClass, $autoload = true)
{
if (is_object($objectOrClass)) {
$objectOrClass = get_class($objectOrClass);
}

if (!is_string($objectOrClass)) {
throw new \InvalidArgumentException(sprintf('Full qualified class name expected, got: "%s".', gettype($objectOrClass)));
}

if (!class_exists($objectOrClass)) {
throw new \RuntimeException(sprintf('Class "%s" does not exists or it can not be autoloaded.', $objectOrClass));
}

$traits = [];
// Get traits of all parent classes
do {
$traits = array_merge(class_uses($objectOrClass, $autoload), $traits);
} while ($objectOrClass = get_parent_class($objectOrClass));

$traitsToSearch = $traits;

while (count($traitsToSearch) > 0) {
$newTraits = class_uses(array_pop($traitsToSearch), $autoload);
$traits = array_merge($newTraits, $traits);
$traitsToSearch = array_merge($newTraits, $traitsToSearch);
}

foreach ($traits as $trait => $same) {
$traits = array_merge(class_uses($trait, $autoload), $traits);
}

return array_unique(array_map(function ($fqcn) {
return ltrim($fqcn, '\\');
}, $traits));
}
}
82 changes: 82 additions & 0 deletions tests/Go/Aop/Bridge/Doctrine/MetadataLoadInterceptorTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php
/*
* Go! AOP framework
*
* @copyright Copyright 2014, Lisachenko Alexander <[email protected]>
*
* This source file is subject to the license that is bundled
* with this source code in the file LICENSE.
*/
namespace Go\Aop\Bridge\Doctrine;

use Doctrine\ORM\EntityManager;
use Doctrine\ORM\Event\LoadClassMetadataEventArgs;
use Doctrine\ORM\Mapping\ClassMetadata;
use Go\Bridge\Doctrine\MetadataLoadInterceptor;
use Go\Core\AspectContainer;

class MetadataLoadInterceptorTest extends \PHPUnit_Framework_TestCase
{
public function testItWillNotModifyClassMetadataForNonProxiedClasses()
{
$metadatas = [
new ClassMetadata('\\Some\\Class\\Name'),
new ClassMetadata(sprintf('%s\\Some\\Class\\Name', AspectContainer::AOP_PROXIED_SUFFIX)),
new ClassMetadata(AspectContainer::AOP_PROXIED_SUFFIX),
];

$metadataInterceptor = new MetadataLoadInterceptor();
$entityManager = $this->getMockBuilder(EntityManager::class)->disableOriginalConstructor()->getMock();

/**
* @var ClassMetadata $metadata
*/
foreach ($metadatas as $metadata) {

Choose a reason for hiding this comment

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

Expected 1 newline after opening brace; 2 found

$metadata->isMappedSuperclass = false;
$metadataInterceptor->loadClassMetadata(new LoadClassMetadataEventArgs($metadata, $entityManager));

$this->assertFalse($metadata->isMappedSuperclass);
}
}

public function testItWillModifyClassMetadataForNonProxiedClasses()
{
$metadata = new ClassMetadata(Entity__AopProxied::class);
$metadataInterceptor = new MetadataLoadInterceptor();
$entityManager = $this->getMockBuilder(EntityManager::class)->disableOriginalConstructor()->getMock();

$metadata->isMappedSuperclass = false;
$metadata->isEmbeddedClass = true;
$metadata->table = ['table_name'];
$metadata->customRepositoryClassName = 'CustomRepositoryClass';

$metadata->fieldMappings['mappedField'] = [
'columnName' => 'mapped_field',
'fieldName' => 'mappedField'
];
$metadata->fieldNames['mapped_field'] = 'mappedField';
$metadata->columnNames['mappedField'] = 'mapped_field';

$metadataInterceptor->loadClassMetadata(new LoadClassMetadataEventArgs($metadata, $entityManager));

$this->assertTrue($metadata->isMappedSuperclass);
$this->assertFalse($metadata->isEmbeddedClass);
$this->assertEquals(0, count($metadata->table));
$this->assertNull($metadata->customRepositoryClassName);

$this->assertFalse(isset($metadata->fieldMappings['mappedField']));
$this->assertFalse(isset($metadata->fieldNames['mapped_field']));
$this->assertFalse(isset($metadata->columnNames['mappedField']));
}
}

trait SimpleTrait

Choose a reason for hiding this comment

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

Each class must be in a file by itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can not fix this, except to pull out to separate files - however, this class is used only for this test suite, so usual practice is to place it inline within test suite.

Copy link
Member

Choose a reason for hiding this comment

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

We can ignore this, ok

{
private $mappedField;
}


class Entity__AopProxied
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can not fix this, except to pull out to separate files - however, this class is used only for this test suite, so usual practice is to place it inline within test suite.

Class name must stay same, that is clear.

Choose a reason for hiding this comment

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

  • Each trait must be in a file by itself
  • Class name Entity__AopProxied is not in camel caps format

Choose a reason for hiding this comment

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

  • Each trait must be in a file by itself
  • Class name Entity__AopProxied is not in camel caps format

{
use SimpleTrait;
}

Choose a reason for hiding this comment

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

Expected 1 blank line at end of file; 2 found