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

Fluent mapping path loader #48

Open
wants to merge 1 commit into
base: 1.1
Choose a base branch
from
Open
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
1 change: 0 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
language: php

php:
- 5.5
- 5.6
- 7.0
- 7.1
Expand Down
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
}
],
"require": {
"php": ">=5.5.0",
"php": ">=5.6.0",
"doctrine/orm": "2.5.*",
"doctrine/inflector": "^1.1"
},
"require-dev": {
"phpunit/phpunit": "~4.0",
"phpunit/phpunit": "~5.7",
"mockery/mockery": "~0.9",
"beberlei/DoctrineExtensions": "~1.0",
"zf1/zend-date": "~1.12",
Expand Down
93 changes: 91 additions & 2 deletions src/FluentDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,15 @@
use Doctrine\Common\Persistence\Mapping\Driver\MappingDriver;
use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder;
use Doctrine\ORM\Mapping\MappingException;
use FilesystemIterator;
use InvalidArgumentException;
use LaravelDoctrine\Fluent\Builders\Builder;
use LaravelDoctrine\Fluent\Mappers\MapperSet;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
use RecursiveRegexIterator;
use ReflectionClass;
use RegexIterator;

class FluentDriver implements MappingDriver
{
Expand All @@ -22,20 +28,37 @@ class FluentDriver implements MappingDriver
*/
protected $fluentFactory;

/**
* The file extension of mapping documents.
*
* @var string
*/
protected $fileExtension = '.php';

/**
* Initializes a new FileDriver that looks in the given path(s) for mapping
* documents and operates in the specified operating mode.
*
* @param string[] $mappings
* @param array $paths
*
* @throws MappingException
*/
public function __construct(array $mappings = [])
public function __construct(array $mappings = [], array $paths = [])
{
$this->fluentFactory = function (ClassMetadata $metadata) {
return new Builder(new ClassMetadataBuilder($metadata));
};

$this->mappers = new MapperSet();
$this->addMappings($mappings);

if (!empty($paths)) {
$this->loadPaths($paths);
}

if (!empty($mappings)) {
$this->addMappings($mappings);
}
}

/**
Expand Down Expand Up @@ -78,8 +101,74 @@ public function isTransient($className)
$this->mappers->getMapperFor($className)->isTransient();
}

/**
* @param array $paths
*
* @throws MappingException
*/
public function loadPaths(array $paths)
{
$includedFiles = [];

foreach ($paths as $path) {
if (!is_dir($path)) {
throw MappingException::fileMappingDriversRequireConfiguredDirectoryPath($path);
}

$iterator = new RegexIterator(
new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS),
RecursiveIteratorIterator::LEAVES_ONLY
),
'/^.+'.preg_quote($this->fileExtension).'$/i',
RecursiveRegexIterator::GET_MATCH
);

foreach ($iterator as $file) {
$sourceFile = $file[0];

if (!preg_match('(^phar:)i', $sourceFile)) {
$sourceFile = realpath($sourceFile);
}

require_once $sourceFile;
Copy link
Contributor

Choose a reason for hiding this comment

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

@patrickbrouwers I don't like this strategy, because it depends on those files not being previously loaded. I don't know what would happen with opcache, for example.
I think we talked about this in the previous PR.
I'd agree to merge as-is, but I wouldn't trust this strategy for my projects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is actually copy pasted from the AnnotationsDriver.


$includedFiles[] = $sourceFile;
}

$declared = get_declared_classes();

foreach ($declared as $className) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@patrickbrouwers this list could be huge. We could use some convention to skip building a ReflectionClass for each loaded class in the system... Or maybe parse filename and class name and only reflect on basename matches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that would require people to actually have a ...Mapping.php suffix.

Like previous comment, this is also the way the AnnotationsDriver does things. It's cached in production anyway.

$rc = new ReflectionClass($className);
$sourceFile = $rc->getFileName();

if (!in_array($sourceFile, $includedFiles)) {
continue;
}

if ($rc->isAbstract() || $rc->isInterface()) {
continue;
}

if (!$rc->implementsInterface(Mapping::class)) {
continue;
}

if ($this->isTransient($className)) {

/** @var Mapping $mapping */
$mapping = $rc->newInstanceWithoutConstructor();

$this->addMapping($mapping);
}
}
}
}

/**
* @param string[] $mappings
*
* @throws InvalidArgumentException
*/
public function addMappings(array $mappings = [])
{
Expand Down
97 changes: 94 additions & 3 deletions tests/FluentDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public function test_it_should_load_metadata_for_mappings_passed_as_constructor_
StubEntity::class,
new ClassMetadataInfo(StubEntity::class)
);

$this->assertInstanceOf(
EntityMapper::class,
$driver->getMappers()->getMapperFor(StubEntity::class)
Expand All @@ -91,6 +92,7 @@ public function test_it_should_load_metadata_for_mappings_passed_as_constructor_
StubEmbeddable::class,
new ClassMetadataInfo(StubEmbeddable::class)
);

$this->assertInstanceOf(
EmbeddableMapper::class,
$driver->getMappers()->getMapperFor(StubEmbeddable::class)
Expand All @@ -100,6 +102,7 @@ public function test_it_should_load_metadata_for_mappings_passed_as_constructor_
StubMappedSuperClass::class,
new ClassMetadataInfo(StubMappedSuperClass::class)
);

$this->assertInstanceOf(
MappedSuperClassMapper::class,
$driver->getMappers()->getMapperFor(StubMappedSuperClass::class)
Expand All @@ -126,9 +129,96 @@ public function test_can_add_array_of_new_mappings()
);
}

public function test_can_load_mappings_through_file_path()
{
$driver = new FluentDriver([], [__DIR__ . '/' . 'Stubs/Mappings']);

$this->assertContains(
StubEntity::class,
$driver->getAllClassNames()
);

$this->assertContains(
StubEmbeddable::class,
$driver->getAllClassNames()
);

$this->assertContains(
StubMappedSuperClass::class,
$driver->getAllClassNames()
);

$driver->loadMetadataForClass(
StubEntity::class,
new ClassMetadataInfo(StubEntity::class)
);

$this->assertInstanceOf(
EntityMapper::class,
$driver->getMappers()->getMapperFor(StubEntity::class)
);

$driver->loadMetadataForClass(
StubEmbeddable::class,
new ClassMetadataInfo(StubEmbeddable::class)
);

$this->assertInstanceOf(
EmbeddableMapper::class,
$driver->getMappers()->getMapperFor(StubEmbeddable::class)
);

$driver->loadMetadataForClass(
StubMappedSuperClass::class,
new ClassMetadataInfo(StubMappedSuperClass::class)
);

$this->assertInstanceOf(
MappedSuperClassMapper::class,
$driver->getMappers()->getMapperFor(StubMappedSuperClass::class)
);
}

public function test_can_load_paths()
{
$driver = new FluentDriver();
$driver->loadPaths([__DIR__ . '/' . 'Stubs/Mappings']);

$this->assertContains(
StubEntity::class,
$driver->getAllClassNames()
);

$this->assertContains(
StubEmbeddable::class,
$driver->getAllClassNames()
);

$this->assertContains(
StubMappedSuperClass::class,
$driver->getAllClassNames()
);
}

public function test_loading_by_paths_throws_exception_if_dir_not_exists()
{
$folder = __DIR__ . '/' . 'Stubs/non-existing-folder';

$this->setExpectedException(
MappingException::class,
'File mapping drivers must have a valid directory path, however the given path [' . $folder . '] seems to be incorrect!'
);

$driver = new FluentDriver();
$driver->loadPaths([$folder]);

$driver->getAllClassNames();
}

public function test_the_given_mapping_class_should_exist()
{
$this->setExpectedException(\InvalidArgumentException::class, 'Mapping class [Tests\DoesnExist] does not exist');
$this->setExpectedException(\InvalidArgumentException::class,
'Mapping class [Tests\DoesnExist] does not exist');

$driver = new FluentDriver;

Expand All @@ -139,7 +229,8 @@ public function test_the_given_mapping_class_should_exist()

public function test_the_given_mapping_class_should_implement_mapping()
{
$this->setExpectedException(\InvalidArgumentException::class, 'Mapping class [Tests\Stubs\Entities\StubEntity] should implement LaravelDoctrine\Fluent\Mapping');
$this->setExpectedException(\InvalidArgumentException::class,
'Mapping class [Tests\Stubs\Entities\StubEntity] should implement LaravelDoctrine\Fluent\Mapping');

$driver = new FluentDriver;

Expand Down Expand Up @@ -223,7 +314,7 @@ public function test_allow_other_fluent_implementations()
return new CustomBuilder(new ClassMetadataBuilder($metadata));
});

$mapping = $this->getMock(EntityMapping::class);
$mapping = $this->createMock(EntityMapping::class);
$mapping->expects($this->once())->method('map')->with($this->isInstanceOf(CustomBuilder::class));

$driver->getMappers()->addMapper('fake', new EntityMapper($mapping));
Expand Down