Skip to content
Closed
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
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
},
"require-dev": {
"phpstan/phpstan": "1.12.7",
"phpstan/phpstan-phpunit": "^1",
"phpstan/phpstan-phpunit": ">=1.3.16",
"phpstan/phpstan-strict-rules": "^1.1",
"doctrine/coding-standard": "^12",
"phpunit/phpunit": "^9.6",
Expand Down
75 changes: 29 additions & 46 deletions src/Persistence/Mapping/Driver/ColocatedMappingDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,15 @@

namespace Doctrine\Persistence\Mapping\Driver;

use AppendIterator;
use Doctrine\Persistence\Mapping\MappingException;
use FilesystemIterator;
use Generator;
use Iterator;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
use ReflectionClass;
use RegexIterator;
use SplFileInfo;

use function array_merge;
use function array_unique;
use function assert;
use function get_declared_classes;
use function in_array;
use function is_dir;
use function preg_match;
use function preg_quote;
use function realpath;
use function sprintf;
use function str_contains;
use function str_replace;

Expand All @@ -33,8 +21,11 @@
*/
trait ColocatedMappingDriver
{
/** @var iterable<array-key,string> */
private iterable $filePaths;

/**
* The paths where to look for mapping files.
* The directory paths where to look for mapping files.
*
* @var array<int, string>
*/
Expand All @@ -51,7 +42,7 @@ trait ColocatedMappingDriver
protected string $fileExtension = '.php';

/**
* Cache for getAllClassNames().
* Cache for {@see getAllClassNames()}.
*
* @var array<int, string>|null
* @phpstan-var list<class-string>|null
Expand Down Expand Up @@ -79,7 +70,7 @@ public function getPaths(): array
}

/**
* Append exclude lookup paths to metadata driver.
* Append exclude lookup paths to a metadata driver.
*
* @param string[] $paths
*/
Expand Down Expand Up @@ -132,35 +123,22 @@ public function getAllClassNames(): array
return $this->classNames;
}

if ($this->paths === []) {
if ($this->paths === [] && ! isset($this->filePaths)) {
throw MappingException::pathRequiredForDriver(static::class);
}

/** @var AppendIterator<array-key,SplFileInfo,Iterator<array-key,SplFileInfo>> $filesIterator */
$filesIterator = new AppendIterator();

foreach ($this->paths as $path) {
if (! is_dir($path)) {
throw MappingException::fileMappingDriversRequireConfiguredDirectoryPath($path);
}
$dirFilesIterator = new DirectoryFilesIterator($this->paths, $this->fileExtension);

/** @var Iterator<array-key,SplFileInfo> $iterator */
$iterator = new RegexIterator(
new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS),
RecursiveIteratorIterator::LEAVES_ONLY,
),
sprintf('/%s$/', preg_quote($this->fileExtension, '/')),
RegexIterator::MATCH,
);

$filesIterator->append($iterator);
}
/** @var iterable<string> $filePathsIterator */
$filePathsIterator = $this->concatIterables(
$this->filePaths ?? [],
new FilePathNameIterator($dirFilesIterator),
);
Comment on lines +133 to +136
Copy link
Member

Choose a reason for hiding this comment

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

The AppendIterator would be perfect to replace concatIterables and avoid this private method to the class that uses the trait.

Suggested change
$filePathsIterator = $this->concatIterables(
$this->filePaths ?? [],
new FilePathNameIterator($dirFilesIterator),
);
$filePathsIterator = new \AppendIterator();
$filePathsIterator->append(new ArrayIterator($this->filePaths ?? []));
$filePathsIterator->append(new FilePathNameIterator($dirFilesIterator));

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 also considered doing it this way, but the problem is that ArrayIterator doesn't accept a generic iterable (nor Traversable). Although it works well with array|ArrayObject, that's the only input type it supports. It won't work with $filePaths being some Traversable instead.


$sourceFilePathNames = $this->pathNameIterator($filesIterator);
$includedFiles = [];
/** @var array<string,true> $includedFiles */
$includedFiles = [];

foreach ($sourceFilePathNames as $sourceFile) {
foreach ($filePathsIterator as $sourceFile) {
if (preg_match('(^phar:)i', $sourceFile) === 0) {
$sourceFile = realpath($sourceFile);
assert($sourceFile !== false);
Expand All @@ -179,7 +157,7 @@ public function getAllClassNames(): array

require_once $sourceFile;

$includedFiles[] = $sourceFile;
$includedFiles[$sourceFile] = true;
}

$classes = [];
Expand All @@ -190,7 +168,7 @@ public function getAllClassNames(): array

$sourceFile = $rc->getFileName();

if (! in_array($sourceFile, $includedFiles, true) || $this->isTransient($className)) {
if (! isset($includedFiles[$sourceFile]) || $this->isTransient($className)) {
continue;
}

Expand All @@ -203,14 +181,19 @@ public function getAllClassNames(): array
}

/**
* @param iterable<SplFileInfo> $filesIterator
* @internal
*
* @return Generator<int,string>
* @param iterable<TKey, T> $iterable1
* @param iterable<TKey, T> $iterable2
*
* @return iterable<TKey, T>
*
* @template TKey
* @template T
*/
private function pathNameIterator(iterable $filesIterator): Generator
Copy link
Member

Choose a reason for hiding this comment

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

Technically, removing a private method from a trait is a breaking change. It could be used by the class that uses it in an other package. But this code has not been released yet.

That's why I thing it's better not introducing concatIterables in this trait.

private function concatIterables(iterable $iterable1, iterable $iterable2): iterable
{
foreach ($filesIterator as $file) {
yield $file->getPathname();
}
yield from $iterable1;
yield from $iterable2;
}
}
61 changes: 61 additions & 0 deletions src/Persistence/Mapping/Driver/DirectoryFilesIterator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

namespace Doctrine\Persistence\Mapping\Driver;

use AppendIterator;
use Doctrine\Persistence\Mapping\MappingException;
use FilesystemIterator;
use Iterator;
use IteratorAggregate;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
use RegexIterator;
use SplFileInfo;

use function is_dir;
use function preg_quote;
use function sprintf;

/** @implements IteratorAggregate<array-key,SplFileInfo> */
final class DirectoryFilesIterator implements IteratorAggregate
{
public function __construct(
/** @var list<string> */
private readonly array $paths,
private readonly string $fileExtension = '.php',
) {
}

/**
* @return Iterator<array-key,SplFileInfo>
*
* @throws MappingException
*/
public function getIterator(): Iterator
{
/** @var AppendIterator<array-key,SplFileInfo,Iterator<array-key,SplFileInfo>> $filesIterator */
$filesIterator = new AppendIterator();

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

/** @var Iterator<array-key,SplFileInfo> $iterator */
$iterator = new RegexIterator(
new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($path, FilesystemIterator::SKIP_DOTS),
RecursiveIteratorIterator::LEAVES_ONLY,
),
sprintf('/%s$/', preg_quote($this->fileExtension, '/')),
RegexIterator::MATCH,
);

$filesIterator->append($iterator);
}

return $filesIterator;
}
}
27 changes: 27 additions & 0 deletions src/Persistence/Mapping/Driver/FilePathNameIterator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Doctrine\Persistence\Mapping\Driver;

use Generator;
use IteratorAggregate;
use SplFileInfo;

/** @implements IteratorAggregate<array-key,string> */
final class FilePathNameIterator implements IteratorAggregate
{
public function __construct(
/** @var iterable<SplFileInfo> */
private readonly iterable $filesIterator,
) {
}

/** @return Generator<array-key,string> */
public function getIterator(): Generator
{
foreach ($this->filesIterator as $key => $file) {
yield $key => $file->getPathname();
}
}
}
2 changes: 1 addition & 1 deletion src/Persistence/Mapping/MappingException.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static function classNotFoundInNamespaces(
public static function pathRequiredForDriver(string $driverClassName): self
{
return new self(sprintf(
'Specifying the paths to your entities is required when using %s to retrieve all class names.',
'Specifying source file paths to your entities is required when using %s to retrieve all class names.',
$driverClassName,
));
}
Expand Down
66 changes: 54 additions & 12 deletions tests/Persistence/Mapping/ColocatedMappingDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,16 @@
use Doctrine\Tests\Persistence\Mapping\_files\colocated\TestClass;
use Generator;
use PHPUnit\Framework\TestCase;
use Traversable;

use function is_file;
use function sort;

class ColocatedMappingDriverTest extends TestCase
{
public function testAddGetPaths(): void
{
$driver = $this->createDriver(__DIR__ . '/_files/colocated');
$driver = $this->createDirectoryPathDriver(__DIR__ . '/_files/colocated');
self::assertSame([
__DIR__ . '/_files/colocated',
], $driver->getPaths());
Expand All @@ -35,7 +37,7 @@ public function testAddGetPaths(): void

public function testAddGetExcludePaths(): void
{
$driver = $this->createDriver(__DIR__ . '/_files/colocated');
$driver = $this->createDirectoryPathDriver(__DIR__ . '/_files/colocated');
self::assertSame([], $driver->getExcludePaths());

$driver->addExcludePaths(['/test/path1', '/test/path2']);
Expand All @@ -48,46 +50,86 @@ public function testAddGetExcludePaths(): void

public function testGetSetFileExtension(): void
{
$driver = $this->createDriver(__DIR__ . '/_files/colocated');
$driver = $this->createDirectoryPathDriver(__DIR__ . '/_files/colocated');
self::assertSame('.php', $driver->getFileExtension());

$driver->setFileExtension('.php1');

self::assertSame('.php1', $driver->getFileExtension());
}

/** @dataProvider pathProvider */
public function testGetAllClassNames(string $path): void
/** @dataProvider directoryPathProvider */
public function testGetAllClassNamesForDirectory(string $dirPath): void
{
$driver = $this->createDriver($path);
$driver = $this->createDirectoryPathDriver($dirPath);

$classes = $driver->getAllClassNames();

sort($classes);
self::assertSame([Entity::class, EntityFixture::class], $classes);
}

public function testGetAllClassNamesForFilePaths(): void
{
$driver = $this->createFilePathsDriver([
__DIR__ . '/_files/colocated/Entity.php',
__DIR__ . '/_files/colocated/TestClass.php',
]);

$classes = $driver->getAllClassNames();

self::assertSame([Entity::class], $classes, 'The driver should only return the class names for the provided file path names, excluding transient class names.');
}

public function testGetAllClassNamesWorksBothForFilePathsAndRetroactivelyAddedDirectoryPaths(): void
{
$driver = $this->createFilePathsDriver([__DIR__ . '/_files/colocated/Entity.php']);

$driver->addPaths([__DIR__ . '/_files/colocated/']);

$classes = $driver->getAllClassNames();
sort($classes);

self::assertSame(
[Entity::class, EntityFixture::class],
$classes,
'The driver should return class names from both the provided file path names and the retroactively added directory paths (these should not be ignored).',
);
}

/** @return Generator<string, array{string}> */
public static function pathProvider(): Generator
public static function directoryPathProvider(): Generator
{
yield 'straigthforward path' => [__DIR__ . '/_files/colocated'];
yield 'winding path' => [__DIR__ . '/../Mapping/_files/colocated'];
}

private function createDriver(string $path): MyDriver
private function createDirectoryPathDriver(string $dirPath): MyDriver
{
return new MyDriver([$path]);
return new MyDriver([$dirPath]);
}

/** @param list<string> $filePaths */
private function createFilePathsDriver(array $filePaths): MyDriver
{
return new MyDriver($filePaths);
}
}

final class MyDriver implements MappingDriver
{
use ColocatedMappingDriver;

/** @param non-empty-list<string> $paths One or multiple paths where mapping classes can be found. */
public function __construct(array $paths)
/** @param iterable<string> $paths One or multiple paths where mapping classes can be found. */
public function __construct(iterable $paths)
{
$this->addPaths($paths);
$isFilePaths = $paths instanceof Traversable || ($paths !== [] && is_file($paths[0]));

if (! $isFilePaths) {
$this->paths = $paths;
} else {
$this->filePaths = $paths;
}
}

/**
Expand Down
Loading