Class property inheritance visibility may lead to a confusion

[llaville]

Hello,

While I continue my learning phase of your great library https://github.com/patchlevel/event-sourcing (currently 3.10.1 tested), I found a situation that make me confused before I realized the error !

I've the same context as your test fixtures :

• https://github.com/patchlevel/hydrator/blob/1.8.x/tests/Unit/Fixture/ParentDto.php
• https://github.com/patchlevel/hydrator/blob/1.8.x/tests/Unit/Fixture/ChildDto.php

But instead of putting the private visibility to Email, I've putted the protected visibility.

The issue I got then make me confused, and search why I got it !

 [ERROR] field name "recordedDate" is duplicated due to inheritance between
         Bartlett\CompatInfoDb\Domain\Event\DistributionCreated and
         Bartlett\CompatInfoDb\Domain\Event\AbstractDomainEvent


array (
  'file' => '/shared/backups/bartlett/php-compatinfo-db/vendor/patchlevel/hydrator/src/Metadata/AttributeMetadataFactory.php',
  'line' => 301,
  'function' => 'byInheritance',
  'class' => 'Patchlevel\\Hydrator\\Metadata\\DuplicatedFieldNameInMetadata',
  'type' => '::',
  'args' =>
  array (
    0 => 'recordedDate',
    1 => 'Bartlett\\CompatInfoDb\\Domain\\Event\\DistributionCreated',
    2 => 'Bartlett\\CompatInfoDb\\Domain\\Event\\AbstractDomainEvent',
  ),
)

NB: Don't try to look on my project source code : I've not put it yet online

AbstractDomainEvent

<?php declare(strict_types=1);

namespace Bartlett\CompatInfoDb\Domain\Event;

use DateTimeImmutable;

abstract class AbstractDomainEvent
{
    public function __construct(
        protected readonly DateTimeImmutable $recordedDate,
    ) {
    }

    public function occurredOn(): DateTimeImmutable
    {
        return $this->recordedDate;
    }
}

DistributionCreated

<?php declare(strict_types=1);

namespace Bartlett\CompatInfoDb\Domain\Event;

use Bartlett\CompatInfoDb\Domain\DomainEvent;
use Bartlett\CompatInfoDb\Domain\ValueObject\CustomId;

use Patchlevel\EventSourcing\Attribute\Event;

use DateTimeImmutable;

#[Event('distribution.created')]
final class DistributionCreated extends AbstractDomainEvent implements DomainEvent
{
    public function __construct(
        public CustomId $distributionId,
        public string $distributionName,
        DateTimeImmutable $distributionDate,
    ) {
        parent::__construct($distributionDate);
    }
}

Reproductible context

Change on your fixtures source code

diff --git a/tests/Unit/Fixture/BrokenParentDto.php b/tests/Unit/Fixture/BrokenParentDto.php
index fed4de9..e7178ae 100644
--- a/tests/Unit/Fixture/BrokenParentDto.php
+++ b/tests/Unit/Fixture/BrokenParentDto.php
@@ -9,7 +9,7 @@ final class BrokenParentDto extends ChildDto
     public function __construct(
         #[IdNormalizer]
         public ProfileId $profileId,
-        private Email $email,
+        protected Email $email,
     ) {
         parent::__construct($email);
     }
diff --git a/tests/Unit/Fixture/ChildDto.php b/tests/Unit/Fixture/ChildDto.php
index 9f205e0..96b4e4c 100644
--- a/tests/Unit/Fixture/ChildDto.php
+++ b/tests/Unit/Fixture/ChildDto.php
@@ -8,7 +8,7 @@ abstract class ChildDto
 {
     public function __construct(
         #[EmailNormalizer]
-        private Email $email,
+        protected Email $email,
     ) {
     }
 }
diff --git a/tests/Unit/Fixture/IgnoreParentDto.php b/tests/Unit/Fixture/IgnoreParentDto.php
index 9af5186..932e5a8 100644
--- a/tests/Unit/Fixture/IgnoreParentDto.php
+++ b/tests/Unit/Fixture/IgnoreParentDto.php
@@ -12,7 +12,7 @@ final class IgnoreParentDto extends ChildDto
         #[IdNormalizer]
         public ProfileId $profileId,
         #[Ignore]
-        private Email $email,
+        protected Email $email,
     ) {
         parent::__construct($email);
     }

And run unit tests, and you'll get something like

There were 3 errors:

1) Patchlevel\Hydrator\Tests\Unit\Metadata\AttributeMetadataFactoryTest::testExtends
Patchlevel\Hydrator\Metadata\DuplicatedFieldNameInMetadata: field name "email" is duplicated due to inheritance between Patchlevel\Hydrator\Tests\Unit\Fixture\ParentDto and Patchlevel\Hydrator\Tests\Unit\Fixture\ChildDto

/shared/backups/github/hydrator/src/Metadata/DuplicatedFieldNameInMetadata.php:24
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:301
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:112
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:76
/shared/backups/github/hydrator/tests/Unit/Metadata/AttributeMetadataFactoryTest.php:227

2) Patchlevel\Hydrator\Tests\Unit\MetadataHydratorTest::testExtractWithInheritance
Patchlevel\Hydrator\Metadata\DuplicatedFieldNameInMetadata: field name "email" is duplicated due to inheritance between Patchlevel\Hydrator\Tests\Unit\Fixture\ParentDto and Patchlevel\Hydrator\Tests\Unit\Fixture\ChildDto

/shared/backups/github/hydrator/src/Metadata/DuplicatedFieldNameInMetadata.php:24
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:301
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:112
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:76
/shared/backups/github/hydrator/src/MetadataHydrator.php:134
/shared/backups/github/hydrator/tests/Unit/MetadataHydratorTest.php:74

3) Patchlevel\Hydrator\Tests\Unit\MetadataHydratorTest::testHydrateWithInheritance
Patchlevel\Hydrator\Metadata\DuplicatedFieldNameInMetadata: field name "email" is duplicated due to inheritance between Patchlevel\Hydrator\Tests\Unit\Fixture\ParentDto and Patchlevel\Hydrator\Tests\Unit\Fixture\ChildDto

/shared/backups/github/hydrator/src/Metadata/DuplicatedFieldNameInMetadata.php:24
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:301
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:112
/shared/backups/github/hydrator/src/Metadata/AttributeMetadataFactory.php:76
/shared/backups/github/hydrator/src/MetadataHydrator.php:44
/shared/backups/github/hydrator/tests/Unit/MetadataHydratorTest.php:195

[llaville]

Of course, I understand and adhere to apply private visibility on property especially when parent class is marked as final, but we should not know by advance in which context the child (abstract) class will be used : With a final parent class or not

[DavidBadura]

Hello and welcome,

it seems to be a bug, which I will look into today.

It has nothing to do with final. By default, the property name is used as the field name. If you have inheritance and two properties with the same name, then the Hydrator does not know which property is meant. To get around this, you can change the name with FieldName or ignore one of the fields with Ignore.

But in your example, I don't see two properties with recordedDate, which is why the error is actually wrong. I will look into it. Thank you for the report!

[DavidBadura]

Should now be fixed with hydrator version 1.7.2.

[llaville]

Should now be fixed with hydrator version 1.7.2.

I confirmed. Thanks