relationships: fixed removing entity from nullable oneHasMany relatio…

…nship # Conflicts: # src/Relationships/HasMany.php # src/Relationships/ManyHasMany.php # src/Relationships/OneHasMany.php
nextras · May 26, 2017 · 9b7b8b1 · 9b7b8b1
1 parent ed90dce
commit 9b7b8b1
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 28 deletions.
diff --git a/src/Relationships/HasMany.php b/src/Relationships/HasMany.php
@@ -51,9 +51,6 @@ abstract class HasMany extends Object implements IRelationshipCollection
 	/** @var bool */
 	protected $isModified = false;
 
-	/** @var bool */
-	protected $wasLoaded = false;
-
 	/** @var IRelationshipMapper */
 	protected $relationshipMapper;
 
@@ -102,7 +99,6 @@ public function add($entity)
 
 		$this->updateRelationshipAdd($entity);
 		$this->modify();
-		$this->wasLoaded = $this->wasLoaded || $this->collection !== null;
 		$this->collection = null;
 		return $entity;
 	}
@@ -126,7 +122,6 @@ public function remove($entity)
 
 		$this->updateRelationshipRemove($entity);
 		$this->modify();
-		$this->wasLoaded = $this->wasLoaded || $this->collection !== null;
 		$this->collection = null;
 		return $entity;
 	}
@@ -288,7 +283,7 @@ protected function getCollection($forceNew = false)
 				$all[$hash] = $entity;
 			}
 			foreach ($this->toRemove as $hash => $entity) {
-				unset($all[$hash]);
+				unset($all[$hash], $this->tracked[$hash]);
 			}
 
 			$collection = new ArrayCollection(array_values($all), $this->getTargetRepository());
@@ -352,7 +347,6 @@ public function __clone()
 	{
 		$this->added = [];
 		$this->removed = [];
-		$this->wasLoaded = false;
 		$this->isModified = false;
 		$this->collection = null;
 	}

diff --git a/src/Relationships/ManyHasMany.php b/src/Relationships/ManyHasMany.php
@@ -15,12 +15,7 @@ class ManyHasMany extends HasMany
 {
 	public function getEntitiesForPersistence()
 	{
-		if ($this->collection !== null || $this->wasLoaded) {
-			return iterator_to_array($this->getIterator());
-
-		} else {
-			return $this->added + $this->toAdd;
-		}
+		return $this->added + $this->toAdd;
 	}
 
 

diff --git a/src/Relationships/OneHasMany.php b/src/Relationships/OneHasMany.php
@@ -15,21 +15,15 @@ class OneHasMany extends HasMany
 {
 	public function getEntitiesForPersistence()
 	{
-		if ($this->collection !== null || $this->wasLoaded) {
-			return iterator_to_array($this->getIterator());
-
-		} else {
-			$entities = $this->added + $this->toAdd;
-
-			foreach ($this->toRemove as $hash => $remove) {
-				if ($remove->isPersisted()) {
-					$entities[$hash] = $remove;
-				} else {
-					unset($entities[$hash]);
-				}
+		$entities = $this->added + $this->toAdd;
+		foreach ($this->toRemove as $hash => $remove) {
+			if ($remove->isPersisted()) {
+				$entities[$hash] = $remove;
+			} else {
+				unset($entities[$hash]);
 			}
-			return $entities;
 		}
+		return $entities;
 	}
 
 

diff --git a/tests/cases/integration/Relationships/relationships.oneHasMany.collection.phpt b/tests/cases/integration/Relationships/relationships.oneHasMany.collection.phpt
@@ -323,9 +323,10 @@ class RelationshipsOneHasManyCollectionTest extends DataTestCase
 			$bookA = $this->getExistingBook(1); // THIS FIRES UNNECESSARY QUERY: SELECT * FROM authors WHERE id IN (1)
 			$bookA->author = $this->authorB;
 			Assert::count(1, iterator_to_array($this->books));
-			Assert::count(1, $this->books->getEntitiesForPersistence());
+			Assert::count(2, $this->books->getEntitiesForPersistence()); // one tracked + one to remove
 
 			$this->orm->persist($bookA);
+			$this->orm->persist($this->authorA);
 			Assert::count(1, iterator_to_array($this->books));
 			Assert::count(1, $this->books->getEntitiesForPersistence());
 
@@ -335,7 +336,7 @@ class RelationshipsOneHasManyCollectionTest extends DataTestCase
 		});
 
 		if ($queries) {
-			Assert::count(5, $queries); // SELECT all, SELECT 1 book's author, BEGIN, UPDATE, COMMIT
+			Assert::count(6, $queries); // SELECT all, SELECT 1 book's author, BEGIN, UPDATE, SELECT, COMMIT
 		}
 	}
 
@@ -382,6 +383,25 @@ class RelationshipsOneHasManyCollectionTest extends DataTestCase
 	}
 
 
+	public function testRemoveD()
+	{
+		$queries = $this->getQueries(function () {
+			Assert::count(0, $this->authorA->translatedBooks->getEntitiesForPersistence());
+
+			$book1 = $this->orm->books->getById(1); // SELECT
+			Assert::count(0, $this->authorA->translatedBooks->getEntitiesForPersistence());
+
+			iterator_to_array($this->authorA->translatedBooks); // SELECT ALL
+			$this->authorA->translatedBooks->remove($book1);
+			Assert::count(1, $this->authorA->translatedBooks->getEntitiesForPersistence());
+		});
+
+		if ($queries) {
+			Assert::count(2, $queries);
+		}
+	}
+
+
 	private function createBook()
 	{
 		static $id = 0;