Fixed issues raised by code review

Author: tuj

README.md

@@ -126,9 +126,11 @@ itk_dev_entity:
     enabled: true
 ```
 
-When enabled, the `BlameableListener` (Doctrine `onFlush`) sets `createdBy` on insert and `modifiedBy` on every flush
-from `Symfony\Bundle\SecurityBundle\Security`. Bundle code types these as `?UserInterface`; the concrete class is
-resolved at runtime via Doctrine `resolve_target_entities`, configured by `itk_dev_entity.user_class`.
+When enabled, the `BlameableListener` (Doctrine `onFlush`) reads the current user from
+`Symfony\Bundle\SecurityBundle\Security` and sets `createdBy` + `modifiedBy` on insert, and `modifiedBy` on every
+update. Flushes that happen outside an authenticated request (workers, console commands) leave the existing blame
+fields untouched. Bundle code types these as `?UserInterface`; the concrete class is resolved at runtime via Doctrine
+`resolve_target_entities`, configured by `itk_dev_entity.user_class`.
 
 ### Opt-in: archivable
 

docker-compose.postgres.yml

@@ -9,11 +9,11 @@ services:
     image: postgres:16
     ports: !reset []
     environment: !override
-      POSTGRES_DB: app_bundle_test
-      POSTGRES_USER: app
-      POSTGRES_PASSWORD: "!ChangeMe!"
+      POSTGRES_DB: db
+      POSTGRES_USER: db
+      POSTGRES_PASSWORD: db
     healthcheck:
-      test: ["CMD-SHELL", "pg_isready -U app -d app_bundle_test"]
+      test: ["CMD-SHELL", "pg_isready -U db -d db"]
       start_period: 10s
       interval: 10s
       timeout: 5s
@@ -29,7 +29,7 @@ services:
       args:
         PHP_FPM_IMAGE: ${PHP_FPM_IMAGE:-itkdev/php8.4-fpm:latest}
     environment:
-      - DATABASE_URL=postgresql://app:!ChangeMe!@database:5432/app_bundle_test?serverVersion=16&charset=utf8
+      - DATABASE_URL=postgresql://db:db@database:5432/db?serverVersion=16&charset=utf8
 
 volumes:
   postgres-data:

src/DependencyInjection/ITKDevEntityExtension.php

@@ -14,6 +14,7 @@
 use ITKDev\EntityBundle\Doctrine\Listener\SoftDeleteListener;
 use ITKDev\EntityBundle\Doctrine\Listener\TimestampableListener;
 use ITKDev\EntityBundle\Privacy\Attribute\Anonymize;
+use ITKDev\EntityBundle\Privacy\Strategy;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\Config\Resource\DirectoryResource;
@@ -180,6 +181,10 @@ private function mergeAnonymizationRules(array $discovered, array $configRules):
                 if (!\is_string($property) || !\is_array($spec) || !isset($spec['strategy']) || !\is_string($spec['strategy'])) {
                     throw new InvalidConfigurationException(sprintf('itk_dev_entity.anonymization.rules[%s][%s] must be { strategy: ..., replacement?: ... }', $class, (string) $property));
                 }
+                if (null === Strategy::tryFrom($spec['strategy'])) {
+                    $valid = implode(', ', array_map(static fn (Strategy $s): string => $s->value, Strategy::cases()));
+                    throw new InvalidConfigurationException(sprintf('itk_dev_entity.anonymization.rules[%s][%s].strategy must be one of: %s. Got "%s".', $class, $property, $valid, $spec['strategy']));
+                }
                 $replacement = $spec['replacement'] ?? null;
                 if (null !== $replacement && !\is_string($replacement)) {
                     throw new InvalidConfigurationException(sprintf('itk_dev_entity.anonymization.rules[%s][%s].replacement must be a string or null.', $class, $property));

src/Doctrine/Filter/ArchivableFilter.php

@@ -16,7 +16,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, string $targetT
             return '';
         }
 
-        $column = $targetEntity->getColumnName('archivedAt');
+        $column = $this->getConnection()->quoteIdentifier($targetEntity->getColumnName('archivedAt'));
 
         return sprintf('%s.%s IS NULL', $targetTableAlias, $column);
     }

src/Doctrine/Filter/SoftDeleteFilter.php

@@ -16,7 +16,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, string $targetT
             return '';
         }
 
-        $column = $targetEntity->getColumnName('deletedAt');
+        $column = $this->getConnection()->quoteIdentifier($targetEntity->getColumnName('deletedAt'));
 
         return sprintf('%s.%s IS NULL', $targetTableAlias, $column);
     }

src/Doctrine/Listener/BlameableListener.php

@@ -41,10 +41,17 @@ public function onFlush(OnFlushEventArgs $args): void
             }
         }
 
+        if (null === $user) {
+            return;
+        }
+
         foreach ($uow->getScheduledEntityUpdates() as $entity) {
             if (!$entity instanceof BlameableInterface) {
                 continue;
             }
+            if ($entity->getModifiedBy() === $user) {
+                continue;
+            }
             $entity->setModifiedBy($user);
             $uow->recomputeSingleEntityChangeSet($em->getClassMetadata($entity::class), $entity);
         }

src/Doctrine/Listener/SoftDeleteListener.php

@@ -33,6 +33,8 @@ public function onFlush(OnFlushEventArgs $args): void
             $entity->setDeletedAt($now);
 
             $meta = $em->getClassMetadata($entity::class);
+            // remove() left the entity in STATE_REMOVED; persist() brings it back to
+            // STATE_MANAGED so scheduleExtraUpdate() will accept it.
             $em->persist($entity);
             $uow->scheduleExtraUpdate($entity, [
                 'deletedAt' => [null, $now],
@@ -43,6 +45,12 @@ public function onFlush(OnFlushEventArgs $args): void
         }
     }
 
+    /**
+     * Doctrine ORM 3 has no public API to cancel a scheduled deletion, so we reach into
+     * the UnitOfWork's private $entityDeletions map and remove the entry directly. The
+     * composer.json constraint pins doctrine/orm to ^3 — if that constraint is widened,
+     * verify this property still exists (see SoftDeleteListenerInternalsTest).
+     */
     private static function cancelDeletion(UnitOfWork $uow, object $entity): void
     {
         $ref = new \ReflectionProperty(UnitOfWork::class, 'entityDeletions');

src/Entity/Trait/TimestampableTrait.php

@@ -14,6 +14,9 @@ trait TimestampableTrait
     #[ORM\Column(type: 'datetime_immutable')]
     private \DateTimeImmutable $updatedAt;
 
+    // Properties are filled by TimestampableListener on first flush, so they may be
+    // uninitialized between `new Entity()` and `$em->flush()`. `?? null` is safe on
+    // uninitialized typed properties (PHP 8+) and lets callers null-check before flush.
     public function getCreatedAt(): ?\DateTimeImmutable
     {
         return $this->createdAt ?? null;

src/Privacy/StrategyApplier.php

@@ -19,10 +19,10 @@ public function apply(Strategy $strategy, mixed $value, ?string $replacement = n
         return match ($strategy) {
             Strategy::NullValue => null,
             Strategy::Redact => $replacement ?? '[REDACTED]',
-            Strategy::Hash => null === $value ? null : hash('sha256', (string) $value),
+            Strategy::Hash => null === $value ? null : hash('sha256', (string) $value.$this->pepper),
             Strategy::Pseudonymize => null === $value
                 ? null
-                : 'user_'.substr(sha1((string) $value.$this->pepper), 0, 12),
+                : 'user_'.substr(hash('sha256', (string) $value.$this->pepper), 0, 12),
         };
     }
 }

tests/Fixtures/Inheritance/GrandchildLeaf.php

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ITKDev\EntityBundle\Tests\Fixtures\Inheritance;
+
+use ITKDev\EntityBundle\Audit\Attribute\Auditable;
+
+/**
+ * Concrete leaf two levels below the class carrying #[ITKDevEntity]. The bundle's
+ * parent-chain walk must reach Grandparent for this class to be discovered.
+ */
+#[Auditable]
+final class GrandchildLeaf extends IntermediateParent
+{
+}