Skip to content

Commit

Permalink
fix: Setter does not get called if the property is also in the constr…
Browse files Browse the repository at this point in the history
…uctor (#58)

* fix: Setter does not get called if the property is also in the constructor

* the fix

* fix data_collector template
  • Loading branch information
priyadi authored May 1, 2024
1 parent 33a0e48 commit bf0ae7b
Show file tree
Hide file tree
Showing 11 changed files with 316 additions and 59 deletions.
13 changes: 4 additions & 9 deletions src/Transformer/Implementation/ObjectToObjectTransformer.php
Original file line number Diff line number Diff line change
Expand Up @@ -388,19 +388,14 @@ private function readSourcePropertyAndWriteTargetProperty(
PropertyMapping $propertyMapping,
Context $context
): void {
$targetWriteMode = $propertyMapping->getTargetWriteMode();
$targetWriteVisibility = $propertyMapping->getTargetWriteVisibility();
$targetSetterWriteMode = $propertyMapping->getTargetSetterWriteMode();
$targetSetterWriteVisibility = $propertyMapping->getTargetSetterWriteVisibility();

if (
in_array($targetWriteMode, [
WriteMode::None,
WriteMode::Constructor,
], true)
) {
if ($targetSetterWriteMode === WriteMode::None) {
return;
}

if ($targetWriteVisibility !== Visibility::Public) {
if ($targetSetterWriteVisibility !== Visibility::Public) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ public function createObjectToObjectMetadata(
->getReadInfo($sourceClass, $sourceProperty);
$targetReadInfo = $this->propertyReadInfoExtractor
->getReadInfo($targetClass, $targetProperty);
$targetWriteInfo = $this->propertyWriteInfoExtractor
->getWriteInfo($targetClass, $targetProperty);
$targetConstructorWriteInfo = $this
->getConstructorWriteInfo($targetClass, $targetProperty);
$targetSetterWriteInfo = $this
->getSetterWriteInfo($targetClass, $targetProperty);

// process source read mode

Expand Down Expand Up @@ -216,45 +218,59 @@ public function createObjectToObjectMetadata(
};
}

// process target write mode
// skip if target is not writable

if ($targetConstructorWriteInfo === null && $targetSetterWriteInfo === null) {
continue;
}

// process target constructor write info

if (
$targetWriteInfo === null
$targetConstructorWriteInfo === null
|| $targetConstructorWriteInfo->getType() !== PropertyWriteInfo::TYPE_CONSTRUCTOR
) {
continue;
} elseif ($targetWriteInfo->getType() === PropertyWriteInfo::TYPE_ADDER_AND_REMOVER) {
$targetWriteMode = WriteMode::AdderRemover;
$targetWriteName = $targetWriteInfo->getAdderInfo()->getName();
$targetWriteVisibility = match ($targetWriteInfo->getAdderInfo()->getVisibility()) {
$targetConstructorWriteMode = WriteMode::None;
$targetConstructorWriteName = null;
} else {
$targetConstructorWriteMode = WriteMode::Constructor;
$targetConstructorWriteName = $targetConstructorWriteInfo->getName();
}

// process target setter write mode

if ($targetSetterWriteInfo === null) {
$targetSetterWriteMode = WriteMode::None;
$targetSetterWriteName = null;
$targetSetterWriteVisibility = Visibility::None;
} elseif ($targetSetterWriteInfo->getType() === PropertyWriteInfo::TYPE_ADDER_AND_REMOVER) {
$targetSetterWriteMode = WriteMode::AdderRemover;
$targetSetterWriteName = $targetSetterWriteInfo->getAdderInfo()->getName();
$targetSetterWriteVisibility = match ($targetSetterWriteInfo->getAdderInfo()->getVisibility()) {
PropertyWriteInfo::VISIBILITY_PUBLIC => Visibility::Public,
PropertyWriteInfo::VISIBILITY_PROTECTED => Visibility::Protected,
PropertyWriteInfo::VISIBILITY_PRIVATE => Visibility::Private,
default => Visibility::None,
};
} elseif ($targetWriteInfo->getType() === PropertyWriteInfo::TYPE_CONSTRUCTOR) {
$targetWriteMode = WriteMode::Constructor;
$targetWriteName = $targetWriteInfo->getName();
$targetWriteVisibility = Visibility::None;
} else {
$targetWriteMode = match ($targetWriteInfo->getType()) {
$targetSetterWriteMode = match ($targetSetterWriteInfo->getType()) {
PropertyWriteInfo::TYPE_METHOD => WriteMode::Method,
PropertyWriteInfo::TYPE_PROPERTY => WriteMode::Property,
default => WriteMode::None,
};

if ($targetWriteMode === WriteMode::None) {
if ($targetSetterWriteMode === WriteMode::None) {
if ($targetAllowsDynamicProperties && $targetReadInfo === null) {
$targetWriteMode = WriteMode::DynamicProperty;
$targetWriteName = $targetProperty;
$targetWriteVisibility = Visibility::Public;
$targetSetterWriteMode = WriteMode::DynamicProperty;
$targetSetterWriteName = $targetProperty;
$targetSetterWriteVisibility = Visibility::Public;
} else {
$effectivePropertiesToMap[] = $targetProperty;

continue;
$targetSetterWriteName = null;
$targetSetterWriteVisibility = Visibility::None;
}
} else {
$targetWriteName = $targetWriteInfo->getName();
$targetWriteVisibility = match ($targetWriteInfo->getVisibility()) {
$targetSetterWriteName = $targetSetterWriteInfo->getName();
$targetSetterWriteVisibility = match ($targetSetterWriteInfo->getVisibility()) {
PropertyWriteInfo::VISIBILITY_PUBLIC => Visibility::Public,
PropertyWriteInfo::VISIBILITY_PROTECTED => Visibility::Protected,
PropertyWriteInfo::VISIBILITY_PRIVATE => Visibility::Private,
Expand Down Expand Up @@ -343,9 +359,11 @@ public function createObjectToObjectMetadata(
targetReadMode: $targetReadMode,
targetReadName: $targetReadName,
targetReadVisibility: $targetReadVisibility,
targetWriteMode: $targetWriteMode,
targetWriteName: $targetWriteName,
targetWriteVisibility: $targetWriteVisibility,
targetSetterWriteMode: $targetSetterWriteMode,
targetSetterWriteName: $targetSetterWriteName,
targetSetterWriteVisibility: $targetSetterWriteVisibility,
targetConstructorWriteMode: $targetConstructorWriteMode,
targetConstructorWriteName: $targetConstructorWriteName,
targetScalarType: $targetPropertyScalarType,
propertyMapper: $serviceMethodSpecification,
sourceLazy: $sourceLazy,
Expand Down Expand Up @@ -469,4 +487,36 @@ private function allowsDynamicProperties(\ReflectionClass $class): bool

return false;
}

private function getConstructorWriteInfo(
string $class,
string $property,
): ?PropertyWriteInfo {
$writeInfo = $this->propertyWriteInfoExtractor
->getWriteInfo($class, $property, [
'enable_getter_setter_extraction' => false,
'enable_magic_methods_extraction' => false,
'enable_adder_remover_extraction' => false,
]);

if ($writeInfo === null) {
return null;
}

if ($writeInfo->getType() === PropertyWriteInfo::TYPE_CONSTRUCTOR) {
return $writeInfo;
}

return null;
}

private function getSetterWriteInfo(
string $class,
string $property,
): ?PropertyWriteInfo {
return $this->propertyWriteInfoExtractor
->getWriteInfo($class, $property, [
'enable_constructor_extraction' => false,
]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,11 @@ public function __construct(
$propertyPropertyMappings = [];

foreach ($allPropertyMappings as $propertyMapping) {
if ($propertyMapping->getTargetWriteMode() === WriteMode::Constructor) {
if ($propertyMapping->getTargetConstructorWriteMode() === WriteMode::Constructor) {
$constructorPropertyMappings[] = $propertyMapping;
} else {
}

if ($propertyMapping->getTargetSetterWriteMode() !== WriteMode::None) {
$propertyPropertyMappings[] = $propertyMapping;

if ($propertyMapping->isSourceLazy()) {
Expand Down
36 changes: 24 additions & 12 deletions src/Transformer/ObjectToObjectMetadata/PropertyMapping.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,11 @@ public function __construct(
private ReadMode $targetReadMode,
private ?string $targetReadName,
private Visibility $targetReadVisibility,
private WriteMode $targetWriteMode,
private ?string $targetWriteName,
private Visibility $targetWriteVisibility,
private WriteMode $targetSetterWriteMode,
private ?string $targetSetterWriteName,
private Visibility $targetSetterWriteVisibility,
private WriteMode $targetConstructorWriteMode,
private ?string $targetConstructorWriteName,
private ?string $targetScalarType,
private ?ServiceMethodSpecification $propertyMapper,
private bool $sourceLazy,
Expand Down Expand Up @@ -134,14 +136,29 @@ public function getTargetReadName(): ?string
return $this->targetReadName;
}

public function getTargetWriteMode(): WriteMode
public function getTargetSetterWriteMode(): WriteMode
{
return $this->targetWriteMode;
return $this->targetSetterWriteMode;
}

public function getTargetWriteName(): ?string
public function getTargetSetterWriteName(): ?string
{
return $this->targetWriteName;
return $this->targetSetterWriteName;
}

public function getTargetSetterWriteVisibility(): Visibility
{
return $this->targetSetterWriteVisibility;
}

public function getTargetConstructorWriteMode(): WriteMode
{
return $this->targetConstructorWriteMode;
}

public function getTargetConstructorWriteName(): ?string
{
return $this->targetConstructorWriteName;
}

public function getSourceReadVisibility(): Visibility
Expand All @@ -154,11 +171,6 @@ public function getTargetReadVisibility(): Visibility
return $this->targetReadVisibility;
}

public function getTargetWriteVisibility(): Visibility
{
return $this->targetWriteVisibility;
}

public function isSourceLazy(): bool
{
return $this->sourceLazy;
Expand Down
12 changes: 6 additions & 6 deletions src/Transformer/Util/ReaderWriter.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,12 @@ public function readTargetProperty(
Context $context
): mixed {
if (
$propertyMapping->getTargetWriteMode() === WriteMode::AdderRemover
&& $propertyMapping->getTargetWriteVisibility() === Visibility::Public
$propertyMapping->getTargetSetterWriteMode() === WriteMode::AdderRemover
&& $propertyMapping->getTargetSetterWriteVisibility() === Visibility::Public
) {
return new AdderRemoverProxy(
$target,
$propertyMapping->getTargetWriteName(),
$propertyMapping->getTargetSetterWriteName(),
null
);
}
Expand Down Expand Up @@ -157,13 +157,13 @@ public function writeTargetProperty(
mixed $value,
Context $context
): void {
if ($propertyMapping->getTargetWriteVisibility() !== Visibility::Public) {
if ($propertyMapping->getTargetSetterWriteVisibility() !== Visibility::Public) {
return;
}

try {
$accessorName = $propertyMapping->getTargetWriteName();
$writeMode = $propertyMapping->getTargetWriteMode();
$accessorName = $propertyMapping->getTargetSetterWriteName();
$writeMode = $propertyMapping->getTargetSetterWriteMode();

if ($writeMode === WriteMode::Property) {
$target->{$accessorName} = $value;
Expand Down
40 changes: 36 additions & 4 deletions templates/data_collector.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@
</th>
</tr>
{% for mapping in metadata.constructorPropertyMappings %}
{{ _self.render_o2o_property_mapping(mapping, helper) }}
{{ _self.render_o2o_constructor_mapping(mapping, helper) }}
{% endfor %}
{% endif %}

Expand Down Expand Up @@ -357,6 +357,38 @@
</table>
{% endmacro %}

{% macro render_o2o_constructor_mapping(mapping, helper) %}
<tr>
<td>
{{ mapping.targetProperty }}
</td>
<td>
{{ helper.typeToHtml(mapping.sourceTypes)|raw }}
</td>
<td>
{{ helper.typeToHtml(mapping.targetTypes)|raw }}
</td>
<td style="white-space: nowrap">
{{ _self.render_o2o_property_mapping_read_mode(mapping.sourceReadMode, mapping.sourceReadName, mapping.sourceReadVisibility) }}
</td>
<td style="white-space: nowrap">
N/A
</td>
<td style="white-space: nowrap">
{{ _self.render_o2o_property_mapping_write_mode(mapping.targetConstructorWriteMode, mapping.targetConstructorWriteName, null) }}
</td>
<td>
{% if mapping.targetScalarType %}
<span class="badge badge-success">{{ mapping.targetScalarType }}</span>
{% endif %}

{% if mapping.targetCanAcceptNull %}
<span class="badge badge-success">nullable</span>
{% endif %}
</td>
</tr>
{% endmacro %}

{% macro render_o2o_property_mapping(mapping, helper) %}
<tr>
<td>
Expand All @@ -375,7 +407,7 @@
{{ _self.render_o2o_property_mapping_read_mode(mapping.targetReadMode, mapping.targetReadName, mapping.targetReadVisibility) }}
</td>
<td style="white-space: nowrap">
{{ _self.render_o2o_property_mapping_write_mode(mapping.targetWriteMode, mapping.targetWriteName, mapping.targetWriteVisibility) }}
{{ _self.render_o2o_property_mapping_write_mode(mapping.targetSetterWriteMode, mapping.targetSetterWriteName, mapping.targetSetterWriteVisibility) }}
</td>
<td>
{% if mapping.targetScalarType %}
Expand Down Expand Up @@ -404,13 +436,13 @@

{% macro render_o2o_property_mapping_write_mode(mode, name, visibility) %}
{%- if mode == constant('Rekalogika\\Mapper\\Transformer\\ObjectToObjectMetadata\\WriteMode::Method') -%}
-&gt;{{ name }}()
-&gt;{{ name }}(...)
{%- elseif mode == constant('Rekalogika\\Mapper\\Transformer\\ObjectToObjectMetadata\\WriteMode::Property') -%}
-&gt;{{ name }}
{%- elseif mode == constant('Rekalogika\\Mapper\\Transformer\\ObjectToObjectMetadata\\WriteMode::DynamicProperty') -%}
-&gt;{{ name }}
{%- elseif mode == constant('Rekalogika\\Mapper\\Transformer\\ObjectToObjectMetadata\\WriteMode::AdderRemover') -%}
-&gt;{{ name }}()
-&gt;{{ name }}(...)
{%- elseif mode == constant('Rekalogika\\Mapper\\Transformer\\ObjectToObjectMetadata\\WriteMode::Constructor') -%}
-&gt;__construct(...)
{%- else -%}
Expand Down
32 changes: 32 additions & 0 deletions tests/Fixtures/PropertyInSetterAndConstructor/ChildObject.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

/*
* This file is part of rekalogika/mapper package.
*
* (c) Priyadi Iman Nurcahyo <https://rekalogika.dev>
*
* For the full copyright and license information, please view the LICENSE file
* that was distributed with this source code.
*/

namespace Rekalogika\Mapper\Tests\Fixtures\PropertyInSetterAndConstructor;

class ChildObject
{
public function __construct(
private string $a,
) {
}

public function getA(): string
{
return $this->a;
}

public function setA(string $a): void
{
$this->a = $a;
}
}
Loading

0 comments on commit bf0ae7b

Please sign in to comment.