Skip to content

Commit

Permalink
Fix comparison of objects (#950)
Browse files Browse the repository at this point in the history
When comparing objects for unique values, a fatal error was thrown when doing `$object1 == $object2` whenever there was a circular reference.
  • Loading branch information
theofidry authored Sep 1, 2018
1 parent 323bc51 commit ffa4ce8
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 4 deletions.
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"php": "^7.1",
"fzaninotto/faker": "^1.6",
"myclabs/deep-copy": "^1.5.2",
"sebastian/comparator": "^3.0",
"symfony/property-access": "^2.8 || ^3.4 || ^4.0",
"symfony/yaml": "^2.8 || ^3.4 || ^4.0"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Nelmio\Alice\IsAServiceTrait;
use Nelmio\Alice\Throwable\Exception\FixtureBuilder\Denormalizer\DenormalizerExceptionFactory;
use Nelmio\Alice\Throwable\Exception\FixtureBuilder\Denormalizer\InvalidScopeException;
use function random_bytes;

final class UniqueValueDenormalizer implements ValueDenormalizerInterface
{
Expand Down Expand Up @@ -83,7 +84,7 @@ private function generateValue(FixtureInterface $scope, FlagBag $flags, $value):
}

if ($value instanceof DynamicArrayValue) {
$uniqueId = uniqid($uniqueId.'::', true);
$uniqueId = $uniqueId.'::__array_element_id#'.bin2hex(random_bytes(16));

return new DynamicArrayValue(
$value->getQuantifier(),
Expand Down
14 changes: 12 additions & 2 deletions src/Generator/Resolver/UniqueValuesPool.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
namespace Nelmio\Alice\Generator\Resolver;

use Nelmio\Alice\Definition\Value\UniqueValue;
use SebastianBergmann\Comparator\ComparisonFailure;
use SebastianBergmann\Comparator\Factory;

/**
* Class storing all the unique values.
Expand Down Expand Up @@ -51,7 +53,15 @@ private function isIdentical($val1, $val2): bool
}

if (is_object($val1)) {
return $val1 == $val2;
$comparator = Factory::getInstance()->getComparatorFor($val1, $val2);

try {
$comparator->assertEquals($val1, $val2);

return true;
} catch (ComparisonFailure $failure) {
return false;
}
}

if (is_scalar($val1) || null === $val1) {
Expand All @@ -74,7 +84,7 @@ private function isIdentical($val1, $val2): bool

return true;
}

private function arrayHasValue($value, array $array): bool
{
foreach ($array as $arrayValue) {
Expand Down
3 changes: 2 additions & 1 deletion src/Parser/IncludeProcessor/DefaultIncludeProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@

namespace Nelmio\Alice\Parser\IncludeProcessor;

use function array_reverse;
use Nelmio\Alice\FileLocatorInterface;
use Nelmio\Alice\IsAServiceTrait;
use Nelmio\Alice\Parser\IncludeProcessorInterface;
use Nelmio\Alice\ParserInterface;
use Nelmio\Alice\Throwable\Error\TypeErrorFactory;
use Nelmio\Alice\Throwable\Exception\InvalidArgumentExceptionFactory;
use function array_reverse;

final class DefaultIncludeProcessor implements IncludeProcessorInterface
{
Expand Down Expand Up @@ -136,6 +136,7 @@ private function retrieveIncludeData(ParserInterface $parser, string $file, arra

$this->included[$includeFile] = true;
}

return $data;
}
}
31 changes: 31 additions & 0 deletions tests/Loader/LoaderIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,37 @@ public function testUniqueValuesAreUniqueAcrossAClass()
$this->assertCount(10, $value[FixtureEntity\DummyWithPublicProperty::class]);
}

/**
* @see https://github.com/nelmio/alice/pull/950
*/
public function testUniqueCircularReferencesThrowNoFatal()
{
$data = [
stdClass::class => [
'member{1..5}' => [
'id' => '<current()>',
'self' => '@self',
],
'group' => [
'owner' => '@member*',
'members (unique)' => '100x @member*',
],
],
];

try {
$this->loader->loadData($data);
} catch (DebugUnexpectedValueException $exception) {
$this->assertNotNull($previous = $exception->getPrevious());

$this->assertInstanceOf(UnresolvableValueDuringGenerationException::class, $previous);

$this->assertNotNull($previous = $previous->getPrevious());

$this->assertInstanceOf(UniqueValueGenerationLimitReachedException::class, $previous);
}
}

/**
* @expectedException \Nelmio\Alice\Throwable\Exception\FixtureNotFoundException
* @expectedExceptionMessage Could not find the fixture "unknown".
Expand Down

0 comments on commit ffa4ce8

Please sign in to comment.