Merge pull request #12120 from greg0ire/fix-broken-comments

Fix broken comments

.github/workflows/continuous-integration.yml

@@ -1,4 +1,4 @@
-name: "Continuous Integration"
+name: "CI"
 
 on:
   pull_request:

README.md

@@ -1,7 +1,7 @@
-|                      [4.0.x][4.0]                      |                      [3.4.x][3.4]                      |                      [3.3.x][3.3]                      |                      [2.21.x][2.21]                      |                      [2.20.x][2.20]                      |
+|                      [4.0.x][4.0]                      |                      [3.6.x][3.6]                      |                      [3.5.x][3.5]                      |                      [2.21.x][2.21]                      |                      [2.20.x][2.20]                      |
 |:------------------------------------------------------:|:------------------------------------------------------:|:------------------------------------------------------:|:--------------------------------------------------------:|:--------------------------------------------------------:|
-|           [![Build status][4.0 image]][4.0]            |           [![Build status][3.4 image]][3.4]            |           [![Build status][3.3 image]][3.3]            |           [![Build status][2.21 image]][2.21]            |           [![Build status][2.20 image]][2.20]            |
-| [![Coverage Status][4.0 coverage image]][4.0 coverage] | [![Coverage Status][3.4 coverage image]][3.4 coverage] | [![Coverage Status][3.3 coverage image]][3.3 coverage] | [![Coverage Status][2.21 coverage image]][2.21 coverage] | [![Coverage Status][2.20 coverage image]][2.20 coverage] |
+|       [![Build status][4.0 image]][4.0 workflow]       |       [![Build status][3.6 image]][3.6 workflow]       |       [![Build status][3.5 image]][3.5 workflow]       |       [![Build status][2.21 image]][2.21 workflow]       |       [![Build status][2.20 image]][2.20 workflow]       |
+| [![Coverage Status][4.0 coverage image]][4.0 coverage] | [![Coverage Status][3.6 coverage image]][3.6 coverage] | [![Coverage Status][3.5 coverage image]][3.5 coverage] | [![Coverage Status][2.21 coverage image]][2.21 coverage] | [![Coverage Status][2.20 coverage image]][2.20 coverage] |
 
 [<h1 align="center">🇺🇦 UKRAINE NEEDS YOUR HELP NOW!</h1>](https://www.doctrine-project.org/stop-war.html)
 
@@ -20,21 +20,26 @@ without requiring unnecessary code duplication.
 
   [4.0 image]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml/badge.svg?branch=4.0.x
   [4.0]: https://github.com/doctrine/orm/tree/4.0.x
+  [4.0 workflow]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml?query=branch%3A4.0.x
   [4.0 coverage image]: https://codecov.io/gh/doctrine/orm/branch/4.0.x/graph/badge.svg
   [4.0 coverage]: https://codecov.io/gh/doctrine/orm/branch/4.0.x
-  [3.4 image]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml/badge.svg?branch=3.4.x
-  [3.4]: https://github.com/doctrine/orm/tree/3.4.x
-  [3.4 coverage image]: https://codecov.io/gh/doctrine/orm/branch/3.4.x/graph/badge.svg
-  [3.4 coverage]: https://codecov.io/gh/doctrine/orm/branch/3.4.x
-  [3.3 image]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml/badge.svg?branch=3.3.x
-  [3.3]: https://github.com/doctrine/orm/tree/3.3.x
-  [3.3 coverage image]: https://codecov.io/gh/doctrine/orm/branch/3.3.x/graph/badge.svg
-  [3.3 coverage]: https://codecov.io/gh/doctrine/orm/branch/3.3.x
+  [3.6 image]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml/badge.svg?branch=3.6.x
+  [3.6]: https://github.com/doctrine/orm/tree/3.6.x
+  [3.6 workflow]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml?query=branch%3A3.6.x
+  [3.6 coverage image]: https://codecov.io/gh/doctrine/orm/branch/3.6.x/graph/badge.svg
+  [3.6 coverage]: https://codecov.io/gh/doctrine/orm/branch/3.6.x
+  [3.5 image]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml/badge.svg?branch=3.5.x
+  [3.5]: https://github.com/doctrine/orm/tree/3.5.x
+  [3.5 workflow]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml?query=branch%3A3.5.x
+  [3.5 coverage image]: https://codecov.io/gh/doctrine/orm/branch/3.5.x/graph/badge.svg
+  [3.5 coverage]: https://codecov.io/gh/doctrine/orm/branch/3.5.x
   [2.21 image]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml/badge.svg?branch=2.21.x
   [2.21]: https://github.com/doctrine/orm/tree/2.21.x
+  [2.21 workflow]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml?query=branch%3A2.21.x
   [2.21 coverage image]: https://codecov.io/gh/doctrine/orm/branch/2.21.x/graph/badge.svg
   [2.21 coverage]: https://codecov.io/gh/doctrine/orm/branch/2.21.x
   [2.20 image]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml/badge.svg?branch=2.20.x
   [2.20]: https://github.com/doctrine/orm/tree/2.20.x
+  [2.20 workflow]: https://github.com/doctrine/orm/actions/workflows/continuous-integration.yml?query=branch%3A2.20.x
   [2.20 coverage image]: https://codecov.io/gh/doctrine/orm/branch/2.20.x/graph/badge.svg
   [2.20 coverage]: https://codecov.io/gh/doctrine/orm/branch/2.20.x

src/Persisters/Entity/BasicEntityPersister.php

@@ -168,14 +168,6 @@ class BasicEntityPersister implements EntityPersister
      */
     protected $quotedColumns = [];
 
-    /**
-     * The INSERT SQL statement used for entities handled by this persister.
-     * This SQL is only generated once per request, if at all.
-     *
-     * @var string|null
-     */
-    private $insertSql;
-
     /**
      * The quote strategy.
      *
@@ -310,8 +302,8 @@ class BasicEntityPersister implements EntityPersister
                 $this->assignDefaultVersionAndUpsertableValues($entity, $id);
             }
 
-            // Unset this queued insert, so that the prepareUpdateData() method knows right away
-            // (for the next entity already) that the current entity has been written to the database
+            // Unset this queued insert, so that the prepareUpdateData() method (called via prepareInsertData() method)
+            // knows right away (for the next entity already) that the current entity has been written to the database
             // and no extra updates need to be scheduled to refer to it.
             //
             // In \Doctrine\ORM\UnitOfWork::executeInserts(), the UoW already removed entities
@@ -1473,22 +1465,17 @@ class BasicEntityPersister implements EntityPersister
      */
     public function getInsertSQL()
     {
-        if ($this->insertSql !== null) {
-            return $this->insertSql;
-        }
-
         $columns   = $this->getInsertColumnList();
         $tableName = $this->quoteStrategy->getTableName($this->class, $this->platform);
 
-        if (empty($columns)) {
-            $identityColumn  = $this->quoteStrategy->getColumnName($this->class->identifier[0], $this->class, $this->platform);
-            $this->insertSql = $this->platform->getEmptyIdentityInsertSQL($tableName, $identityColumn);
+        if ($columns === []) {
+            $identityColumn = $this->quoteStrategy->getColumnName($this->class->identifier[0], $this->class, $this->platform);
 
-            return $this->insertSql;
+            return $this->platform->getEmptyIdentityInsertSQL($tableName, $identityColumn);
         }
 
-        $values  = [];
-        $columns = array_unique($columns);
+        $placeholders = [];
+        $columns      = array_unique($columns);
 
         foreach ($columns as $column) {
             $placeholder = '?';
@@ -1502,15 +1489,13 @@ class BasicEntityPersister implements EntityPersister
                 $placeholder = $type->convertToDatabaseValueSQL('?', $this->platform);
             }
 
-            $values[] = $placeholder;
+            $placeholders[] = $placeholder;
         }
 
-        $columns = implode(', ', $columns);
-        $values  = implode(', ', $values);
+        $columns      = implode(', ', $columns);
+        $placeholders = implode(', ', $placeholders);
 
-        $this->insertSql = sprintf('INSERT INTO %s (%s) VALUES (%s)', $tableName, $columns, $values);
-
-        return $this->insertSql;
+        return sprintf('INSERT INTO %s (%s) VALUES (%s)', $tableName, $columns, $placeholders);
     }
 
     /**

src/Persisters/Entity/JoinedSubclassPersister.php

@@ -149,7 +149,7 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister
         // Execute all inserts. For each entity:
         // 1) Insert on root table
         // 2) Insert on sub tables
-        foreach ($this->queuedInserts as $entity) {
+        foreach ($this->queuedInserts as $key => $entity) {
             $insertData = $this->prepareInsertData($entity);
 
             // Execute insert on root table
@@ -194,9 +194,16 @@ class JoinedSubclassPersister extends AbstractEntityInheritancePersister
             if ($this->class->requiresFetchAfterChange) {
                 $this->assignDefaultVersionAndUpsertableValues($entity, $id);
             }
-        }
 
-        $this->queuedInserts = [];
+            // Unset this queued insert, so that the prepareUpdateData() method (called via prepareInsertData() method)
+            // knows right away (for the next entity already) that the current entity has been written to the database
+            // and no extra updates need to be scheduled to refer to it.
+            //
+            // In \Doctrine\ORM\UnitOfWork::executeInserts(), the UoW already removed entities
+            // from its own list (\Doctrine\ORM\UnitOfWork::$entityInsertions) right after they
+            // were given to our addInsert() method.
+            unset($this->queuedInserts[$key]);
+        }
     }
 
     /**

src/Tools/Console/Command/MappingDescribeCommand.php

@@ -98,7 +98,7 @@ EOT
                     $this->formatField('Embedded class?', $metadata->isEmbeddedClass),
                     $this->formatField('Parent classes', $metadata->parentClasses),
                     $this->formatField('Sub classes', $metadata->subClasses),
-                    $this->formatField('Embedded classes', $metadata->subClasses),
+                    $this->formatField('Embedded classes', $metadata->embeddedClasses),
                     $this->formatField('Named queries', $metadata->namedQueries),
                     $this->formatField('Named native queries', $metadata->namedNativeQueries),
                     $this->formatField('SQL result set mappings', $metadata->sqlResultSetMappings),

src/UnitOfWork.php

@@ -1060,7 +1060,9 @@ class UnitOfWork implements PropertyChangedListener
 
         $this->entityStates[$oid] = self::STATE_MANAGED;
 
-        $this->scheduleForInsert($entity);
+        if (! isset($this->entityInsertions[$oid])) {
+            $this->scheduleForInsert($entity);
+        }
     }
 
     /** @param mixed[] $idValue */

tests/Tests/ORM/Functional/PrePersistEventTest.php

@@ -0,0 +1,112 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\ORM\Event\PrePersistEventArgs;
+use Doctrine\ORM\Events;
+use Doctrine\ORM\Mapping\Column;
+use Doctrine\ORM\Mapping\Entity;
+use Doctrine\ORM\Mapping\GeneratedValue;
+use Doctrine\ORM\Mapping\Id;
+use Doctrine\ORM\Mapping\ManyToOne;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+use function uniqid;
+
+class PrePersistEventTest extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->createSchemaForModels(
+            EntityWithUnmappedEntity::class,
+            EntityWithCascadeAssociation::class
+        );
+    }
+
+    public function testCallingPersistInPrePersistHook(): void
+    {
+        $entityWithUnmapped = new EntityWithUnmappedEntity();
+        $entityWithCascade  = new EntityWithCascadeAssociation();
+
+        $entityWithUnmapped->unmapped = $entityWithCascade;
+        $entityWithCascade->cascaded  = $entityWithUnmapped;
+
+        $this->_em->getEventManager()->addEventListener(Events::prePersist, new PrePersistUnmappedPersistListener());
+        $this->_em->persist($entityWithUnmapped);
+
+        $this->assertTrue($this->_em->getUnitOfWork()->isScheduledForInsert($entityWithCascade));
+        $this->assertTrue($this->_em->getUnitOfWork()->isScheduledForInsert($entityWithUnmapped));
+    }
+}
+
+class PrePersistUnmappedPersistListener
+{
+    public function prePersist(PrePersistEventArgs $args): void
+    {
+        $object = $args->getObject();
+
+        if ($object instanceof EntityWithUnmappedEntity) {
+            $uow = $args->getObjectManager()->getUnitOfWork();
+
+            if ($object->unmapped && ! $uow->isInIdentityMap($object->unmapped) && ! $uow->isScheduledForInsert($object->unmapped)) {
+                $args->getObjectManager()->persist($object->unmapped);
+            }
+        }
+    }
+}
+
+/** @Entity */
+#[Entity]
+class EntityWithUnmappedEntity
+{
+    /**
+     * @var string
+     * @Id
+     * @Column(type="string", length=255)
+     * @GeneratedValue(strategy="NONE")
+     */
+    #[Id]
+    #[Column(type: 'string', length: 255)]
+    #[GeneratedValue(strategy: 'NONE')]
+    public $id;
+
+    /** @var ?EntityWithCascadeAssociation  */
+    public $unmapped = null;
+
+    public function __construct()
+    {
+        $this->id = uniqid(self::class, true);
+    }
+}
+
+/** @Entity */
+#[Entity]
+class EntityWithCascadeAssociation
+{
+    /**
+     * @var string
+     * @Id
+     * @Column(type="string", length=255)
+     * @GeneratedValue(strategy="NONE")
+     */
+    #[Id]
+    #[Column(type: 'string', length: 255)]
+    #[GeneratedValue(strategy: 'NONE')]
+    public $id;
+
+    /**
+     * @var ?EntityWithUnmappedEntity
+     * @ManyToOne(targetEntity=EntityWithUnmappedEntity::class, cascade={"persist"})
+     */
+    #[ManyToOne(targetEntity: EntityWithUnmappedEntity::class, cascade: ['persist'])]
+    public $cascaded = null;
+
+    public function __construct()
+    {
+        $this->id = uniqid(self::class, true);
+    }
+}

tests/Tests/ORM/Functional/SecondLevelCacheCountQueriesTest.php

@@ -0,0 +1,180 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional;
+
+use Doctrine\ORM\Id\AssignedGenerator;
+use Doctrine\ORM\Mapping\ClassMetadata;
+use Doctrine\ORM\Mapping\ClassMetadataInfo;
+use Doctrine\Tests\Models\Cache\Country;
+use ReflectionProperty;
+
+use function array_diff;
+use function array_filter;
+use function file_exists;
+use function rmdir;
+use function scandir;
+use function strpos;
+use function sys_get_temp_dir;
+
+use const DIRECTORY_SEPARATOR;
+
+/**
+ * @group DDC-2183
+ * @phpstan-type SupportedCacheUsage 0|ClassMetadata::CACHE_USAGE_NONSTRICT_READ_WRITE|ClassMetadata::CACHE_USAGE_READ_WRITE
+ */
+class SecondLevelCacheCountQueriesTest extends SecondLevelCacheFunctionalTestCase
+{
+    /** @var string */
+    private $tmpDir;
+
+    protected function tearDown(): void
+    {
+        if ($this->tmpDir !== null && file_exists($this->tmpDir)) {
+            foreach (array_diff(scandir($this->tmpDir), ['.', '..']) as $f) {
+                rmdir($this->tmpDir . DIRECTORY_SEPARATOR . $f);
+            }
+
+            rmdir($this->tmpDir);
+        }
+
+        parent::tearDown();
+    }
+
+    /** @param SupportedCacheUsage $cacheUsage */
+    private function setupCountryModel(int $cacheUsage): void
+    {
+        $metadata = $this->_em->getClassMetaData(Country::class);
+
+        if ($cacheUsage === 0) {
+            $metadataCacheReflection = new ReflectionProperty(ClassMetadataInfo::class, 'cache');
+            $metadataCacheReflection->setAccessible(true);
+            $metadataCacheReflection->setValue($metadata, null);
+
+            return;
+        }
+
+        if ($cacheUsage === ClassMetadata::CACHE_USAGE_READ_WRITE) {
+            $this->tmpDir = sys_get_temp_dir() . DIRECTORY_SEPARATOR . self::class;
+            $this->secondLevelCacheFactory->setFileLockRegionDirectory($this->tmpDir);
+        }
+
+        $metadata->enableCache(['usage' => $cacheUsage]);
+    }
+
+    private function loadFixturesCountriesWithoutPostInsertIdentifier(): void
+    {
+        $metadata = $this->_em->getClassMetaData(Country::class);
+        $metadata->setIdGenerator(new AssignedGenerator());
+
+        $c1 = new Country('Brazil');
+        $c1->setId(10);
+        $c2 = new Country('Germany');
+        $c2->setId(20);
+
+        $this->countries[] = $c1;
+        $this->countries[] = $c2;
+
+        $this->_em->persist($c1);
+        $this->_em->persist($c2);
+        $this->_em->flush();
+    }
+
+    /** @param 'INSERT'|'UPDATE'|'DELETE' $type */
+    private function assertQueryCountByType(string $type, int $expectedCount): void
+    {
+        $queries = array_filter($this->getQueryLog()->queries, static function (array $entry) use ($type): bool {
+            return strpos($entry['sql'], $type) === 0;
+        });
+
+        self::assertCount($expectedCount, $queries);
+    }
+
+    /**
+     * @param SupportedCacheUsage $cacheUsage
+     *
+     * @dataProvider cacheUsageProvider
+     */
+    public function testInsertWithPostInsertIdentifier(int $cacheUsage): void
+    {
+        $this->setupCountryModel($cacheUsage);
+
+        self::assertQueryCountByType('INSERT', 0);
+
+        $this->loadFixturesCountries();
+
+        self::assertCount(2, $this->countries);
+        self::assertQueryCountByType('INSERT', 2);
+    }
+
+    /**
+     * @param SupportedCacheUsage $cacheUsage
+     *
+     * @dataProvider cacheUsageProvider
+     */
+    public function testInsertWithoutPostInsertIdentifier(int $cacheUsage): void
+    {
+        $this->setupCountryModel($cacheUsage);
+
+        self::assertQueryCountByType('INSERT', 0);
+
+        $this->loadFixturesCountriesWithoutPostInsertIdentifier();
+
+        self::assertCount(2, $this->countries);
+        self::assertQueryCountByType('INSERT', 2);
+    }
+
+    /**
+    /* @param SupportedCacheUsage $cacheUsage
+     *
+     * @dataProvider cacheUsageProvider
+     */
+    public function testDelete(int $cacheUsage): void
+    {
+        $this->setupCountryModel($cacheUsage);
+        $this->loadFixturesCountries();
+
+        $c1 = $this->_em->find(Country::class, $this->countries[0]->getId());
+        $c2 = $this->_em->find(Country::class, $this->countries[1]->getId());
+
+        $this->_em->remove($c1);
+        $this->_em->remove($c2);
+        $this->_em->flush();
+
+        self::assertQueryCountByType('DELETE', 2);
+    }
+
+    /**
+     * @param SupportedCacheUsage $cacheUsage
+     *
+     * @dataProvider cacheUsageProvider
+     */
+    public function testUpdate(int $cacheUsage): void
+    {
+        $this->setupCountryModel($cacheUsage);
+        $this->loadFixturesCountries();
+
+        $c1 = $this->_em->find(Country::class, $this->countries[0]->getId());
+        $c2 = $this->_em->find(Country::class, $this->countries[1]->getId());
+
+        $c1->setName('Czech Republic');
+        $c2->setName('Hungary');
+
+        $this->_em->persist($c1);
+        $this->_em->persist($c2);
+        $this->_em->flush();
+
+        self::assertQueryCountByType('UPDATE', 2);
+    }
+
+    /** @return list<array{SupportedCacheUsage}> */
+    public static function cacheUsageProvider(): array
+    {
+        return [
+            [0],
+            [ClassMetadata::CACHE_USAGE_NONSTRICT_READ_WRITE],
+            [ClassMetadata::CACHE_USAGE_READ_WRITE],
+        ];
+    }
+}