From 12e1dcbcde2c4e0377b2f44cd0b01b2f21c233ec Mon Sep 17 00:00:00 2001 From: = Date: Tue, 2 Aug 2016 15:46:23 +0200 Subject: [PATCH] Fixing issues with QtiIdentifiableCollection from master. --- composer.json | 2 +- qtism/data/QtiIdentifiableCollection.php | 54 ++++++- qtism/data/storage/xml/XmlDocument.php | 3 +- .../data/QTIIdentifiableCollectionTest.php | 141 +++++++++++++++--- 4 files changed, 166 insertions(+), 34 deletions(-) diff --git a/composer.json b/composer.json index 481493843..f3a58a2f7 100644 --- a/composer.json +++ b/composer.json @@ -2,7 +2,7 @@ "name": "qtism/qtism", "description": "OAT QTI Software Module Library", "type": "library", - "version": "0.10.5", + "version": "0.10.6", "authors": [ { "name": "Open Assessment Technologies S.A.", diff --git a/qtism/data/QtiIdentifiableCollection.php b/qtism/data/QtiIdentifiableCollection.php index 0d17fc24d..0114cad78 100644 --- a/qtism/data/QtiIdentifiableCollection.php +++ b/qtism/data/QtiIdentifiableCollection.php @@ -27,6 +27,7 @@ use \ReflectionClass; use \InvalidArgumentException; use \OutOfRangeException; +use \UnexpectedValueException; use \SplObserver; use \SplSubject; @@ -147,6 +148,7 @@ public function offsetUnset($offset) { if (gettype($offset) === 'string') { $data = &$this->getDataPlaceHolder(); if (isset($data[$offset])) { + $data[$offset]->detach($this); unset($data[$offset]); } } @@ -155,6 +157,50 @@ public function offsetUnset($offset) { throw new OutOfRangeException($msg); } } + + /** + * Replace an $object in the collection by another $replacement $object. + * + * @param mixed $object An object to be replaced. + * @param mixed $replacement An object to be used as a replacement. + * @throws \InvalidArgumentException If $object or $replacement are not compliant with the current collection typing. + * @throws \UnexpectedValueException If $object is not contained in the collection. + */ + public function replace($object, $replacement) + { + $this->checkType($object); + $this->checkType($replacement); + + if (($search = array_search($object, $this->dataPlaceHolder, true)) !== false) { + + $objectKey = $search; + $replacementKey = $replacement->getIdentifier(); + + if ($objectKey === $replacementKey) { + // If they share the same key, just replace. + $this->dataPlaceHolder[$objectKey] = $replacement; + } else { + // Otherwise, we have to insert the $replacement object at the appropriate offset (just before $object), + // and then remove the former $object. + $objectOffset = array_search($objectKey, array_keys($this->dataPlaceHolder)); + + $this->dataPlaceHolder = array_merge( + array_slice($this->dataPlaceHolder, 0, $objectOffset), + array($replacementKey => $replacement), + array_slice($this->dataPlaceHolder, $objectOffset, null) + ); + + $this->offsetUnset($objectKey); + } + + $replacement->attach($this); + $object->detach($this); + + } else { + $msg = "The object you want to replace could not be found."; + throw new UnexpectedValueException($msg); + } + } /** * Implementation of SplObserver::update. @@ -164,13 +210,7 @@ public function offsetUnset($offset) { public function update(SplSubject $subject) { // -- case 1 (QtiIdentifiable) // If it is a QtiIdentifiable, it has changed its identifier. - $data = &$this->getDataPlaceHolder(); - foreach (array_keys($data) as $k) { - if ($data[$k] === $subject && $k !== $subject->getIdentifier()) { - unset($data[$k]); - $this->offsetSet(null, $subject); - } - } + $this->replace($subject, $subject); } public function __clone() { diff --git a/qtism/data/storage/xml/XmlDocument.php b/qtism/data/storage/xml/XmlDocument.php index 42ac7fd21..75d1f31aa 100644 --- a/qtism/data/storage/xml/XmlDocument.php +++ b/qtism/data/storage/xml/XmlDocument.php @@ -392,8 +392,7 @@ public function includeAssessmentSectionRefs($validate = false) $collection = $parent->getSectionParts(); } - $collection->detach($assessmentSectionRef); - $collection->attach($sectionRoot); + $collection->replace($assessmentSectionRef, $sectionRoot); } } diff --git a/test/qtism/data/QTIIdentifiableCollectionTest.php b/test/qtism/data/QTIIdentifiableCollectionTest.php index 2824045f2..1d4bd2cfa 100644 --- a/test/qtism/data/QTIIdentifiableCollectionTest.php +++ b/test/qtism/data/QTIIdentifiableCollectionTest.php @@ -7,27 +7,120 @@ class QtiIdentifiableCollectionTest extends QtiSmTestCase { - public function testWithWeights() { - - $weight1 = new Weight('weight1', 1.0); - $weight2 = new Weight('weight2', 1.1); - $weight3 = new Weight('weight3', 1.2); - $weights = new WeightCollection(array($weight1, $weight2, $weight3)); - - $this->assertTrue($weights['weight1'] === $weight1); - $this->assertTrue($weights['weight2'] === $weight2); - $this->assertTrue($weights['weight3'] === $weight3); - - $this->assertTrue($weights['weightX'] === null); - - - // Can I address the by identifier? - $this->assertTrue($weights['weight2'] === $weight2); - - // What happens if I change the identifier of an object. - // Is it adressable with the new identifier? - $weight2->setIdentifier('weightX'); - $this->assertTrue($weights['weightX'] === $weight2); - $this->assertFalse(isset($weights['weight2'])); - } -} \ No newline at end of file + public function testWithWeights() { + + $weight1 = new Weight('weight1', 1.0); + $weight2 = new Weight('weight2', 1.1); + $weight3 = new Weight('weight3', 1.2); + $weights = new WeightCollection(array($weight1, $weight2, $weight3)); + + $this->assertTrue($weights['weight1'] === $weight1); + $this->assertTrue($weights['weight2'] === $weight2); + $this->assertTrue($weights['weight3'] === $weight3); + + $this->assertTrue($weights['weightX'] === null); + $this->assertFalse(isset($weights['weightX'])); + + // Can I address the by identifier? + $this->assertTrue($weights['weight2'] === $weight2); + + // What happens if I change the identifier of an object. + // Is it adressable with the new identifier? + $weight2->setIdentifier('weightX'); + $this->assertTrue($weights['weightX'] === $weight2); + $this->assertFalse(isset($weights['weight2'])); + $this->assertTrue(isset($weights['weightX'])); + + // What happens if I remove an object? + unset($weights['weightX']); + $this->assertFalse(isset($weights['weightX'])); + } + + /** + * @depends testWithWeights + */ + public function testReplace() + { + $weight1 = new Weight('weight1', 1.0); + $weight2 = new Weight('weight2', 1.1); + $weight3 = new Weight('weight3', 1.2); + $weights = new WeightCollection(array($weight1, $weight2, $weight3)); + + // Let's replace weight2 with another Weight object having the same identifier. + $this->assertSame($weight2, $weights['weight2']); + + $weightBis = new Weight('weight2', 2.0); + $weights->replace($weight2, $weightBis); + + $this->assertFalse($weight2 === $weights['weight2']); + $this->assertCount(3, $weights); + + // Is the order still respected? + $this->assertSame( + array('weight1', 'weight2', 'weight3'), + $weights->getKeys() + ); + + // Let's replace (the new) weight2 with another Weight object having different identifiers. + $weight4 = new Weight('weight4', 1.4); + $weights->replace($weights['weight2'], $weight4); + + $this->assertCount(3, $weights); + $this->assertFalse(isset($weights['weight2'])); + + // Now check the order of things, let's get the keys and compare them. + $this->assertSame( + array('weight1', 'weight4', 'weight3'), + $weights->getKeys() + ); + } + + /** + * @depends testWithWeights + */ + public function testEventsUnset() + { + $weight1 = new Weight('weight1', 1.0); + $weight2 = new Weight('weight2', 1.2); + $weights = new WeightCollection(array($weight1, $weight2)); + + $this->assertCount(2, $weights); + $this->assertTrue(isset($weights['weight1'])); + $this->assertTrue(isset($weights['weight2'])); + + $weight1->setIdentifier('weightX'); + $this->assertCount(2, $weights); + $this->assertFalse(isset($weights['weight1'])); + $this->assertTrue(isset($weights['weight2'])); + $this->assertTrue(isset($weights['weightX'])); + + unset($weights['weightX']); + $this->assertCount(1, $weights); + $this->assertFalse(isset($weights['weightX'])); + $this->assertTrue(isset($weights['weight2'])); + + $weight1->setIdentifier('weight2'); + $this->assertFalse($weight1 === $weights['weight2']); + } + + public function testRenamingOrder() + { + $weight1 = new Weight('weight1', 1.0); + $weight2 = new Weight('weight2', 1.2); + $weight3 = new Weight('weight3', 1.2); + $weights = new WeightCollection(array($weight1, $weight2, $weight3)); + + // If weight2 gets a new identifier "weight4", it should still be in second position in the collection. + $this->assertSame( + array('weight1', 'weight2', 'weight3'), + $weights->getKeys() + ); + + $weight2->setIdentifier('weight4'); + + $this->assertSame( + array('weight1', 'weight4', 'weight3'), + $weights->getKeys() + ); + } +}