Skip to content

Commit d55218a

Browse files
priyadipriyadi
committed
fix(ObjectToObjectTransformer): If target is lazy and its constructor contains an eager argument, then the rest of the arguments must be eager.
1 parent 24b48f1 commit d55218a

8 files changed

+186
-29
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
* style(`Profiler`): Improve layout.
99
* feat(`Profiler`): Collect object to object metadata.
1010
* test: In 8.2, read only classes cannot be lazy.
11+
* fix(`ObjectToObjectTransformer`): If target is lazy and its constructor
12+
contains an eager argument, then the rest of the arguments must be eager.
1113

1214
## 0.7.2
1315

‎src/Transformer/ObjectToObjectMetadata/Implementation/ObjectToObjectMetadataFactory.php

+31-2
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@ public function createObjectToObjectMetadata(
331331
sourceModifiedTime: $sourceModifiedTime,
332332
targetModifiedTime: $targetModifiedTime,
333333
targetReadOnly: $targetReadOnly,
334+
constructorIsEager: false,
334335
);
335336

336337
// create proxy if possible
@@ -339,13 +340,41 @@ public function createObjectToObjectMetadata(
339340
$proxySpecification = $this->proxyGenerator
340341
->generateTargetProxy($objectToObjectMetadata);
341342

343+
// determine if the constructor contains eager properties. if it
344+
// does, then the constructor is eager
345+
346+
$constructorIsEager = false;
347+
348+
foreach ($objectToObjectMetadata->getConstructorPropertyMappings() as $propertyMapping) {
349+
if (!$propertyMapping->isSourceLazy()) {
350+
$constructorIsEager = true;
351+
break;
352+
}
353+
}
354+
355+
// if the constructor is eager, then every constructor argument is
356+
// eager
357+
358+
if ($constructorIsEager) {
359+
foreach ($objectToObjectMetadata->getConstructorPropertyMappings() as $propertyMapping) {
360+
$eagerProperties[] = $propertyMapping->getTargetProperty();
361+
}
362+
363+
$eagerProperties = \array_unique($eagerProperties);
364+
}
365+
366+
// skipped properties is the argument used by createLazyGhost()
367+
342368
$skippedProperties = self::getSkippedProperties(
343369
$targetClass,
344370
$eagerProperties
345371
);
346372

347-
$objectToObjectMetadata = $objectToObjectMetadata
348-
->withTargetProxy($proxySpecification, $skippedProperties);
373+
$objectToObjectMetadata = $objectToObjectMetadata->withTargetProxy(
374+
$proxySpecification,
375+
$skippedProperties,
376+
$constructorIsEager
377+
);
349378
} catch (ProxyNotSupportedException $e) {
350379
$objectToObjectMetadata = $objectToObjectMetadata
351380
->withReasonCannotUseProxy($e->getReason());

‎src/Transformer/ObjectToObjectMetadata/ObjectToObjectMetadata.php

+10-1
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public function __construct(
6565
private int $sourceModifiedTime,
6666
private int $targetModifiedTime,
6767
private bool $targetReadOnly,
68+
private bool $constructorIsEager,
6869
private ?string $targetProxyClass = null,
6970
private ?string $targetProxyCode = null,
7071
private array $targetProxySkippedProperties = [],
@@ -102,7 +103,8 @@ public function __construct(
102103
*/
103104
public function withTargetProxy(
104105
ProxySpecification $proxySpecification,
105-
array $targetProxySkippedProperties
106+
array $targetProxySkippedProperties,
107+
bool $constructorIsEager,
106108
): self {
107109
return new self(
108110
$this->sourceClass,
@@ -115,6 +117,7 @@ public function withTargetProxy(
115117
$this->sourceModifiedTime,
116118
$this->targetModifiedTime,
117119
$this->targetReadOnly,
120+
$constructorIsEager,
118121
$proxySpecification->getClass(),
119122
$proxySpecification->getCode(),
120123
$targetProxySkippedProperties,
@@ -136,6 +139,7 @@ public function withReasonCannotUseProxy(
136139
$this->sourceModifiedTime,
137140
$this->targetModifiedTime,
138141
$this->targetReadOnly,
142+
$this->constructorIsEager,
139143
null,
140144
null,
141145
[],
@@ -287,4 +291,9 @@ public function getCannotUseProxyReason(): ?string
287291
{
288292
return $this->cannotUseProxyReason;
289293
}
294+
295+
public function constructorIsEager(): bool
296+
{
297+
return $this->constructorIsEager;
298+
}
290299
}

‎src/Transformer/ObjectToObjectTransformer.php

+55-25
Original file line numberDiff line numberDiff line change
@@ -237,46 +237,27 @@ private function instantiateTargetProxy(
237237
// create proxy initializer. this initializer will be executed when the
238238
// proxy is first accessed
239239

240-
$initializer = function (object $instance) use (
240+
$initializer = function (object $target) use (
241241
$source,
242242
$objectToObjectMetadata,
243243
$context,
244-
$targetClass
245244
): void {
246-
// if constructor exists, process it
245+
// if the constructor is lazy, run it here
247246

248-
if (\method_exists($instance, '__construct')) {
249-
$constructorArguments = $this->generateConstructorArguments(
247+
if (!$objectToObjectMetadata->constructorIsEager()) {
248+
$target = $this->runConstructorManually(
250249
source: $source,
250+
target: $target,
251251
objectToObjectMetadata: $objectToObjectMetadata,
252252
context: $context
253253
);
254-
255-
$arguments = $constructorArguments->getArguments();
256-
257-
try {
258-
/**
259-
* @psalm-suppress DirectConstructorCall
260-
* @psalm-suppress MixedMethodCall
261-
*/
262-
$instance->__construct(...$arguments);
263-
} catch (\TypeError | \ReflectionException $e) {
264-
throw new InstantiationFailureException(
265-
source: $source,
266-
targetClass: $targetClass,
267-
constructorArguments: $constructorArguments->getArguments(),
268-
unsetSourceProperties: $constructorArguments->getUnsetSourceProperties(),
269-
previous: $e,
270-
context: $context
271-
);
272-
}
273254
}
274255

275256
// map lazy properties
276257

277258
$this->readSourceAndWriteTarget(
278259
source: $source,
279-
target: $instance,
260+
target: $target,
280261
propertyMappings: $objectToObjectMetadata->getLazyPropertyMappings(),
281262
context: $context
282263
);
@@ -295,6 +276,17 @@ private function instantiateTargetProxy(
295276
skippedProperties: $objectToObjectMetadata->getTargetProxySkippedProperties()
296277
);
297278

279+
// if the constructor is eager, run it here
280+
281+
if ($objectToObjectMetadata->constructorIsEager()) {
282+
$target = $this->runConstructorManually(
283+
source: $source,
284+
target: $target,
285+
objectToObjectMetadata: $objectToObjectMetadata,
286+
context: $context
287+
);
288+
}
289+
298290
// map eager properties
299291

300292
$this->readSourceAndWriteTarget(
@@ -307,6 +299,44 @@ private function instantiateTargetProxy(
307299
return $target;
308300
}
309301

302+
private function runConstructorManually(
303+
object $source,
304+
object $target,
305+
ObjectToObjectMetadata $objectToObjectMetadata,
306+
Context $context
307+
): object {
308+
if (!\method_exists($target, '__construct')) {
309+
return $target;
310+
}
311+
312+
$constructorArguments = $this->generateConstructorArguments(
313+
source: $source,
314+
objectToObjectMetadata: $objectToObjectMetadata,
315+
context: $context
316+
);
317+
318+
$arguments = $constructorArguments->getArguments();
319+
320+
try {
321+
/**
322+
* @psalm-suppress DirectConstructorCall
323+
* @psalm-suppress MixedMethodCall
324+
*/
325+
$target->__construct(...$arguments);
326+
} catch (\TypeError | \ReflectionException $e) {
327+
throw new InstantiationFailureException(
328+
source: $source,
329+
targetClass: $target::class,
330+
constructorArguments: $constructorArguments->getArguments(),
331+
unsetSourceProperties: $constructorArguments->getUnsetSourceProperties(),
332+
previous: $e,
333+
context: $context
334+
);
335+
}
336+
337+
return $target;
338+
}
339+
310340
private function generateConstructorArguments(
311341
object $source,
312342
ObjectToObjectMetadata $objectToObjectMetadata,

‎tests/Fixtures/LazyObject/ObjectWithIdAndNameInConstructorDto.php

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of rekalogika/mapper package.
7+
*
8+
* (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
9+
*
10+
* For the full copyright and license information, please view the LICENSE file
11+
* that was distributed with this source code.
12+
*/
13+
14+
namespace Rekalogika\Mapper\Tests\Fixtures\LazyObject;
15+
16+
class ObjectWithIdAndNameInConstructorDto
17+
{
18+
public function __construct(
19+
public string $id,
20+
public string $name
21+
) {
22+
}
23+
24+
public ?string $other = null;
25+
}

‎tests/Fixtures/LazyObject/ObjectWithIdAndNameMustBeCalled.php

+44
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* This file is part of rekalogika/mapper package.
7+
*
8+
* (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
9+
*
10+
* For the full copyright and license information, please view the LICENSE file
11+
* that was distributed with this source code.
12+
*/
13+
14+
namespace Rekalogika\Mapper\Tests\Fixtures\LazyObject;
15+
16+
class ObjectWithIdAndNameMustBeCalled
17+
{
18+
private bool $idCalled = false;
19+
private bool $nameCalled = false;
20+
21+
public function getId(): string
22+
{
23+
$this->idCalled = true;
24+
25+
return 'id';
26+
}
27+
28+
public function getName(): string
29+
{
30+
$this->nameCalled = true;
31+
32+
return 'name';
33+
}
34+
35+
public function getOther(): string
36+
{
37+
return 'other';
38+
}
39+
40+
public function isIdAndNameCalled(): bool
41+
{
42+
return $this->idCalled && $this->nameCalled;
43+
}
44+
}

‎tests/Fixtures/LazyObject/ObjectWithIdReadOnlyDto.php

-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
{
1818
public function __construct(
1919
public string $id,
20-
public string $name
2120
) {
2221
}
2322
}

‎tests/IntegrationTest/LazyObjectTest.php

+19
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616
use Rekalogika\Mapper\Tests\Common\AbstractFrameworkTest;
1717
use Rekalogika\Mapper\Tests\Fixtures\LazyObject\ChildObjectWithIdDto;
1818
use Rekalogika\Mapper\Tests\Fixtures\LazyObject\ObjectWithId;
19+
use Rekalogika\Mapper\Tests\Fixtures\LazyObject\ObjectWithIdAndNameInConstructorDto;
20+
use Rekalogika\Mapper\Tests\Fixtures\LazyObject\ObjectWithIdAndNameMustBeCalled;
1921
use Rekalogika\Mapper\Tests\Fixtures\LazyObject\ObjectWithIdDto;
2022
use Rekalogika\Mapper\Tests\Fixtures\LazyObject\ObjectWithIdFinalDto;
2123
use Rekalogika\Mapper\Tests\Fixtures\LazyObject\ObjectWithIdReadOnlyDto;
24+
use Symfony\Component\VarExporter\LazyObjectInterface;
2225

2326
class LazyObjectTest extends AbstractFrameworkTest
2427
{
@@ -94,4 +97,20 @@ public function testIdInParentClassInitialized(): void
9497
$target = $this->mapper->map($source, ChildObjectWithIdDto::class);
9598
$foo = $target->name;
9699
}
100+
101+
/**
102+
* If the constructor has an eager property, the constructor is eager.
103+
*/
104+
public function testEagerAndLazyPropertyInConstruct(): void
105+
{
106+
$source = new ObjectWithIdAndNameMustBeCalled();
107+
$target = $this->mapper->map($source, ObjectWithIdAndNameInConstructorDto::class);
108+
$this->assertInstanceOf(LazyObjectInterface::class, $target);
109+
110+
$this->assertTrue($source->isIdAndNameCalled());
111+
$this->assertFalse($target->isLazyObjectInitialized());
112+
113+
$this->assertEquals('other', $target->other);
114+
$this->assertTrue($target->isLazyObjectInitialized());
115+
}
97116
}

0 commit comments

Comments
 (0)