diff --git a/src/Composer/DependencyResolver/RuleSet.php b/src/Composer/DependencyResolver/RuleSet.php index 26771cef6..f4baa848b 100644 --- a/src/Composer/DependencyResolver/RuleSet.php +++ b/src/Composer/DependencyResolver/RuleSet.php @@ -62,13 +62,24 @@ class RuleSet implements \IteratorAggregate, \Countable $this->rules[$type] = array(); } + $hash = $rule->getHash(); + + // Do not add if rule already exists + if (isset($this->rulesByHash[$hash])) { + $potentialDuplicates = $this->rulesByHash[$hash]; + foreach ($potentialDuplicates as $potentialDuplicate) { + if ($rule->equals($potentialDuplicate)) { + return; + } + } + } + $this->rules[$type][] = $rule; $this->ruleById[$this->nextRuleId] = $rule; $rule->setType($type); $this->nextRuleId++; - $hash = $rule->getHash(); if (!isset($this->rulesByHash[$hash])) { $this->rulesByHash[$hash] = array($rule); } else { @@ -135,20 +146,6 @@ class RuleSet implements \IteratorAggregate, \Countable return array_keys($types); } - public function containsEqual($rule) - { - if (isset($this->rulesByHash[$rule->getHash()])) { - $potentialDuplicates = $this->rulesByHash[$rule->getHash()]; - foreach ($potentialDuplicates as $potentialDuplicate) { - if ($rule->equals($potentialDuplicate)) { - return true; - } - } - } - - return false; - } - public function getPrettyString(Pool $pool = null) { $string = "\n"; diff --git a/src/Composer/DependencyResolver/RuleSetGenerator.php b/src/Composer/DependencyResolver/RuleSetGenerator.php index 8ea24e742..e2ff33abe 100644 --- a/src/Composer/DependencyResolver/RuleSetGenerator.php +++ b/src/Composer/DependencyResolver/RuleSetGenerator.php @@ -137,7 +137,7 @@ class RuleSetGenerator */ private function addRule($type, Rule $newRule = null) { - if (!$newRule || $this->rules->containsEqual($newRule)) { + if (!$newRule) { return; } diff --git a/tests/Composer/Test/DependencyResolver/RuleSetTest.php b/tests/Composer/Test/DependencyResolver/RuleSetTest.php index eb968cf32..ea91c80dc 100644 --- a/tests/Composer/Test/DependencyResolver/RuleSetTest.php +++ b/tests/Composer/Test/DependencyResolver/RuleSetTest.php @@ -32,8 +32,8 @@ class RuleSetTest extends TestCase $rules = array( RuleSet::TYPE_PACKAGE => array(), RuleSet::TYPE_JOB => array( - new Rule(array(), Rule::RULE_JOB_INSTALL, null), - new Rule(array(), Rule::RULE_JOB_INSTALL, null), + new Rule(array(1), Rule::RULE_JOB_INSTALL, null), + new Rule(array(2), Rule::RULE_JOB_INSTALL, null), ), RuleSet::TYPE_LEARNED => array( new Rule(array(), Rule::RULE_INTERNAL_ALLOW_UPDATE, null), @@ -49,6 +49,25 @@ class RuleSetTest extends TestCase $this->assertEquals($rules, $ruleSet->getRules()); } + public function testAddIgnoresDuplicates() + { + $rules = array( + RuleSet::TYPE_JOB => array( + new Rule(array(), Rule::RULE_JOB_INSTALL, null), + new Rule(array(), Rule::RULE_JOB_INSTALL, null), + new Rule(array(), Rule::RULE_JOB_INSTALL, null), + ) + ); + + $ruleSet = new RuleSet; + + $ruleSet->add($rules[RuleSet::TYPE_JOB][0], RuleSet::TYPE_JOB); + $ruleSet->add($rules[RuleSet::TYPE_JOB][1], RuleSet::TYPE_JOB); + $ruleSet->add($rules[RuleSet::TYPE_JOB][2], RuleSet::TYPE_JOB); + + $this->assertCount(1, $ruleSet->getIteratorFor(array(RuleSet::TYPE_JOB))); + } + /** * @expectedException \OutOfBoundsException */ @@ -63,8 +82,8 @@ class RuleSetTest extends TestCase { $ruleSet = new RuleSet; - $ruleSet->add(new Rule(array(), Rule::RULE_JOB_INSTALL, null), RuleSet::TYPE_JOB); - $ruleSet->add(new Rule(array(), Rule::RULE_JOB_INSTALL, null), RuleSet::TYPE_JOB); + $ruleSet->add(new Rule(array(1), Rule::RULE_JOB_INSTALL, null), RuleSet::TYPE_JOB); + $ruleSet->add(new Rule(array(2), Rule::RULE_JOB_INSTALL, null), RuleSet::TYPE_JOB); $this->assertEquals(2, $ruleSet->count()); } @@ -83,8 +102,8 @@ class RuleSetTest extends TestCase { $ruleSet = new RuleSet; - $rule1 = new Rule(array(), Rule::RULE_JOB_INSTALL, null); - $rule2 = new Rule(array(), Rule::RULE_JOB_INSTALL, null); + $rule1 = new Rule(array(1), Rule::RULE_JOB_INSTALL, null); + $rule2 = new Rule(array(2), Rule::RULE_JOB_INSTALL, null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); $ruleSet->add($rule2, RuleSet::TYPE_LEARNED); @@ -98,8 +117,8 @@ class RuleSetTest extends TestCase public function testGetIteratorFor() { $ruleSet = new RuleSet; - $rule1 = new Rule(array(), Rule::RULE_JOB_INSTALL, null); - $rule2 = new Rule(array(), Rule::RULE_JOB_INSTALL, null); + $rule1 = new Rule(array(1), Rule::RULE_JOB_INSTALL, null); + $rule2 = new Rule(array(2), Rule::RULE_JOB_INSTALL, null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); $ruleSet->add($rule2, RuleSet::TYPE_LEARNED); @@ -112,8 +131,8 @@ class RuleSetTest extends TestCase public function testGetIteratorWithout() { $ruleSet = new RuleSet; - $rule1 = new Rule(array(), Rule::RULE_JOB_INSTALL, null); - $rule2 = new Rule(array(), Rule::RULE_JOB_INSTALL, null); + $rule1 = new Rule(array(1), Rule::RULE_JOB_INSTALL, null); + $rule2 = new Rule(array(2), Rule::RULE_JOB_INSTALL, null); $ruleSet->add($rule1, RuleSet::TYPE_JOB); $ruleSet->add($rule2, RuleSet::TYPE_LEARNED); @@ -123,38 +142,6 @@ class RuleSetTest extends TestCase $this->assertSame($rule2, $iterator->current()); } - public function testContainsEqual() - { - $ruleSet = new RuleSet; - - $rule = $this->getRuleMock(); - $rule->expects($this->any()) - ->method('getHash') - ->will($this->returnValue('rule_1_hash')); - $rule->expects($this->any()) - ->method('equals') - ->will($this->returnValue(true)); - - $rule2 = $this->getRuleMock(); - $rule2->expects($this->any()) - ->method('getHash') - ->will($this->returnValue('rule_2_hash')); - - $rule3 = $this->getRuleMock(); - $rule3->expects($this->any()) - ->method('getHash') - ->will($this->returnValue('rule_1_hash')); - $rule3->expects($this->any()) - ->method('equals') - ->will($this->returnValue(false)); - - $ruleSet->add($rule, RuleSet::TYPE_LEARNED); - - $this->assertTrue($ruleSet->containsEqual($rule)); - $this->assertFalse($ruleSet->containsEqual($rule2)); - $this->assertFalse($ruleSet->containsEqual($rule3)); - } - public function testPrettyString() { $repo = new ArrayRepository;