Decisions are now encapsulated in a separate object

src/Composer/DependencyResolver/Decisions.php

@@ -0,0 +1,215 @@
+<?php
+
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ *     Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\DependencyResolver;
+
+/**
+ * Stores decisions on installing, removing or keeping packages
+ *
+ * @author Nils Adermann <naderman@naderman.de>
+ */
+class Decisions implements \Iterator
+{
+    const DECISION_LITERAL = 0;
+    const DECISION_REASON = 1;
+
+    protected $pool;
+    protected $decisionMap;
+    protected $decisionQueue = array();
+    protected $decisionQueueFree = array();
+
+    public function __construct($pool)
+    {
+        $this->pool = $pool;
+
+        if (version_compare(PHP_VERSION, '5.3.4', '>=')) {
+            $this->decisionMap = new \SplFixedArray($this->pool->getMaxId() + 1);
+        } else {
+            $this->decisionMap = array_fill(0, $this->pool->getMaxId() + 1, 0);
+        }
+    }
+
+    protected function addDecision($literal, $level)
+    {
+        $packageId = abs($literal);
+
+        $previousDecision = $this->decisionMap[$packageId];
+        if ($previousDecision != 0) {
+            $literalString = $this->pool->literalToString($literal);
+            $package = $this->pool->literalToPackage($literal);
+            throw new SolverBugException(
+                "Trying to decide $literalString on level $level, even though $package was previously decided as ".(int) $previousDecision."."
+            );
+        }
+
+        if ($literal > 0) {
+            $this->decisionMap[$packageId] = $level;
+        } else {
+            $this->decisionMap[$packageId] = -$level;
+        }
+    }
+
+    public function decide($literal, $level, $why, $addToFreeQueue = false)
+    {
+        $this->addDecision($literal, $level);
+        $this->decisionQueue[] = array(
+            self::DECISION_LITERAL => $literal,
+            self::DECISION_REASON => $why,
+        );
+
+        if ($addToFreeQueue) {
+            $this->decisionQueueFree[count($this->decisionQueue) - 1] = true;
+        }
+    }
+
+    public function contain($literal)
+    {
+        $packageId = abs($literal);
+
+        return (
+            $this->decisionMap[$packageId] > 0 && $literal > 0 ||
+            $this->decisionMap[$packageId] < 0 && $literal < 0
+        );
+    }
+
+    public function satisfy($literal)
+    {
+        $packageId = abs($literal);
+
+        return (
+            $literal > 0 && $this->decisionMap[$packageId] > 0 ||
+            $literal < 0 && $this->decisionMap[$packageId] < 0
+        );
+    }
+
+    public function conflict($literal)
+    {
+        $packageId = abs($literal);
+
+        return (
+            ($this->decisionMap[$packageId] > 0 && $literal < 0) ||
+            ($this->decisionMap[$packageId] < 0 && $literal > 0)
+        );
+    }
+
+    public function decided($literalOrPackageId)
+    {
+        return $this->decisionMap[abs($literalOrPackageId)] != 0;
+    }
+
+    public function undecided($literalOrPackageId)
+    {
+        return $this->decisionMap[abs($literalOrPackageId)] == 0;
+    }
+
+    public function decidedInstall($literalOrPackageId)
+    {
+        return $this->decisionMap[abs($literalOrPackageId)] > 0;
+    }
+
+    public function decisionLevel($literalOrPackageId)
+    {
+        return abs($this->decisionMap[abs($literalOrPackageId)]);
+    }
+
+    public function decisionRule($literalOrPackageId)
+    {
+        $packageId = abs($literalOrPackageId);
+
+        foreach ($this->decisionQueue as $i => $decision) {
+            if ($packageId === abs($decision[self::DECISION_LITERAL])) {
+                return $decision[self::DECISION_REASON];
+            }
+        }
+
+        return null;
+    }
+
+    public function atOffset($queueOffset)
+    {
+        return $this->decisionQueue[$queueOffset];
+    }
+
+    public function validOffset($queueOffset)
+    {
+        return $queueOffset >= 0 && $queueOffset < count($this->decisionQueue);
+    }
+
+    public function lastReason()
+    {
+        return $this->decisionQueue[count($this->decisionQueue) - 1][self::DECISION_REASON];
+    }
+
+    public function lastLiteral()
+    {
+        return $this->decisionQueue[count($this->decisionQueue) - 1][self::DECISION_LITERAL];
+    }
+
+    public function reset()
+    {
+        while ($decision = array_pop($this->decisionQueue)) {
+            $this->decisionMap[abs($decision[self::DECISION_LITERAL])] = 0;
+        }
+
+        $this->decisionQueueFree = array();
+    }
+
+    public function resetToOffset($offset)
+    {
+        while (count($this->decisionQueue) > $offset + 1) {
+            $decision = array_pop($this->decisionQueue);
+            unset($this->decisionQueueFree[count($this->decisionQueue)]);
+            $this->decisionMap[abs($decision[self::DECISION_LITERAL])] = 0;
+        }
+    }
+
+    public function revertLast()
+    {
+        $this->decisionMap[abs($this->lastLiteral())] = 0;
+        array_pop($this->decisionQueue);
+    }
+
+    public function getMaxOffset()
+    {
+        return count($this->decisionQueue) - 1;
+    }
+
+    public function rewind()
+    {
+        end($this->decisionQueue);
+    }
+
+    public function current()
+    {
+        return current($this->decisionQueue);
+    }
+
+    public function key()
+    {
+        return key($this->decisionQueue);
+    }
+
+    public function next()
+    {
+        return prev($this->decisionQueue);
+    }
+
+    public function valid()
+    {
+        return false !== current($this->decisionQueue);
+    }
+
+    public function isEmpty()
+    {
+        return count($this->decisionQueue) === 0;
+    }
+}

src/Composer/DependencyResolver/RuleWatchGraph.php

@@ -72,16 +72,11 @@ class RuleWatchGraph
      * @param int $decidedLiteral The literal which was decided (A in our example)
      * @param int $level          The level at which the decision took place and at which
      *     all resulting decisions should be made.
-     * @param Callable $decisionsSatisfyCallback A callback which checks if a
-     *     literal has already been positively decided and the rule is thus
-     *     already true and can be skipped.
-     * @param Callable $conflictCallback A callback which checks if a literal
-     *     would conflict with previously made decisions on the same package
-     * @param Callable $decideCallback A callback which is responsible for
-     *     registering decided literals resulting from unit clauses
+     * @param Decisions $decisions Used to check previous decisions and to
+     *     register decisions resulting from propagation
      * @return Rule|null If a conflict is found the conflicting rule is returned
      */
-    public function propagateLiteral($decidedLiteral, $level, $decisionsSatisfyCallback, $conflictCallback, $decideCallback)
+    public function propagateLiteral($decidedLiteral, $level, $decisions)
     {
         // we invert the decided literal here, example:
         // A was decided => (-A|B) now requires B to be true, so we look for
@@ -99,13 +94,13 @@ class RuleWatchGraph
             $node = $chain->current();
             $otherWatch = $node->getOtherWatch($literal);
 
-            if (!$node->getRule()->isDisabled() && !call_user_func($decisionsSatisfyCallback, $otherWatch)) {
+            if (!$node->getRule()->isDisabled() && !$decisions->contain($otherWatch)) {
                 $ruleLiterals = $node->getRule()->getLiterals();
 
-                $alternativeLiterals = array_filter($ruleLiterals, function ($ruleLiteral) use ($literal, $otherWatch, $conflictCallback) {
+                $alternativeLiterals = array_filter($ruleLiterals, function ($ruleLiteral) use ($literal, $otherWatch, $decisions) {
                     return $literal !== $ruleLiteral &&
                         $otherWatch !== $ruleLiteral &&
-                        !call_user_func($conflictCallback, $ruleLiteral);
+                        !$decisions->conflict($ruleLiteral);
                 });
 
                 if ($alternativeLiterals) {
@@ -114,11 +109,11 @@ class RuleWatchGraph
                     continue;
                 }
 
-                if (call_user_func($conflictCallback, $otherWatch)) {
+                if ($decisions->conflict($otherWatch)) {
                     return $node->getRule();
                 }
 
-                call_user_func($decideCallback, $otherWatch, $level, $node->getRule());
+                $decisions->decide($otherWatch, $level, $node->getRule());
             }
 
             $chain->next();

src/Composer/DependencyResolver/RuleWatchNode.php

@@ -47,9 +47,9 @@ class RuleWatchNode
      * Useful for learned rules where the literal for the highest rule is most
      * likely to quickly lead to further decisions.
      *
-     * @param SplFixedArray $decisionMap A package to decision lookup table
+     * @param Decisions $decisions The decisions made so far by the solver
      */
-    public function watch2OnHighest($decisionMap)
+    public function watch2OnHighest(Decisions $decisions)
     {
         $literals = $this->rule->getLiterals();
 
@@ -61,7 +61,7 @@ class RuleWatchNode
         $watchLevel = 0;
 
         foreach ($literals as $literal) {
-            $level = abs($decisionMap[abs($literal)]);
+            $level = $decisions->decisionLevel($literal);
 
             if ($level > $watchLevel) {
                 $this->rule->watch2 = $literal;

src/Composer/DependencyResolver/Solver.php

@@ -29,12 +29,9 @@ class Solver
     protected $addedMap = array();
     protected $updateMap = array();
     protected $watchGraph;
-    protected $decisionMap;
+    protected $decisions;
     protected $installedMap;
 
-    protected $decisionQueue = array();
-    protected $decisionQueueWhy = array();
-    protected $decisionQueueFree = array();
     protected $propagateIndex;
     protected $branches = array();
     protected $problems = array();
@@ -48,21 +45,10 @@ class Solver
         $this->ruleSetGenerator = new RuleSetGenerator($policy, $pool);
     }
 
-    private function findDecisionRule($packageId)
-    {
-        foreach ($this->decisionQueue as $i => $literal) {
-            if ($packageId === abs($literal)) {
-                return $this->decisionQueueWhy[$i];
-            }
-        }
-
-        return null;
-    }
-
     // aka solver_makeruledecisions
     private function makeAssertionRuleDecisions()
     {
-        $decisionStart = count($this->decisionQueue);
+        $decisionStart = $this->decisions->getMaxOffset();
 
         for ($ruleIndex = 0; $ruleIndex < count($this->rules); $ruleIndex++) {
             $rule = $this->rules->ruleById($ruleIndex);
@@ -74,12 +60,12 @@ class Solver
             $literals = $rule->getLiterals();
             $literal = $literals[0];
 
-            if (!$this->decided(abs($literal))) {
-                $this->decide($literal, 1, $rule);
+            if (!$this->decisions->decided(abs($literal))) {
+                $this->decisions->decide($literal, 1, $rule);
                 continue;
             }
 
-            if ($this->decisionsSatisfy($literal)) {
+            if ($this->decisions->satisfy($literal)) {
                 continue;
             }
 
@@ -89,7 +75,7 @@ class Solver
                 continue;
             }
 
-            $conflict = $this->findDecisionRule(abs($literal));
+            $conflict = $this->decisions->decisionRule($literal);
 
             if ($conflict && RuleSet::TYPE_PACKAGE === $conflict->getType()) {
 
@@ -126,13 +112,7 @@ class Solver
             }
             $this->problems[] = $problem;
 
-            // start over
-            while (count($this->decisionQueue) > $decisionStart) {
-                $decisionLiteral = array_pop($this->decisionQueue);
-                array_pop($this->decisionQueueWhy);
-                unset($this->decisionQueueFree[count($this->decisionQueue)]);
-                $this->decisionMap[abs($decisionLiteral)] = 0;
-            }
+            $this->resetToOffset($decisionStart);
             $ruleIndex = -1;
         }
     }
@@ -177,11 +157,7 @@ class Solver
 
         $this->setupInstalledMap();
 
-        if (version_compare(PHP_VERSION, '5.3.4', '>=')) {
-            $this->decisionMap = new \SplFixedArray($this->pool->getMaxId() + 1);
-        } else {
-            $this->decisionMap = array_fill(0, $this->pool->getMaxId() + 1, 0);
-        }
+        $this->decisions = new Decisions($this->pool);
 
         $this->rules = $this->ruleSetGenerator->getRulesFor($this->jobs, $this->installedMap);
         $this->watchGraph = new RuleWatchGraph;
@@ -199,7 +175,7 @@ class Solver
             throw new SolverProblemsException($this->problems);
         }
 
-        $transaction = new Transaction($this->policy, $this->pool, $this->installedMap, $this->decisionMap, $this->decisionQueue, $this->decisionQueueWhy);
+        $transaction = new Transaction($this->policy, $this->pool, $this->installedMap, $this->decisions);
 
         return $transaction->getOperations();
     }
@@ -211,77 +187,6 @@ class Solver
         return new Literal($package, $id > 0);
     }
 
-    protected function addDecision($literal, $level)
-    {
-        $packageId = abs($literal);
-
-        $previousDecision = $this->decisionMap[$packageId];
-        if ($previousDecision != 0) {
-            $literalString = $this->pool->literalToString($literal);
-            $package = $this->pool->literalToPackage($literal);
-            throw new SolverBugException(
-                "Trying to decide $literalString on level $level, even though $package was previously decided as ".(int) $previousDecision."."
-            );
-        }
-
-        if ($literal > 0) {
-            $this->decisionMap[$packageId] = $level;
-        } else {
-            $this->decisionMap[$packageId] = -$level;
-        }
-    }
-
-    public function decide($literal, $level, $why)
-    {
-        $this->addDecision($literal, $level);
-        $this->decisionQueue[] = $literal;
-        $this->decisionQueueWhy[] = $why;
-    }
-
-    public function decisionsContain($literal)
-    {
-        $packageId = abs($literal);
-
-        return (
-            $this->decisionMap[$packageId] > 0 && $literal > 0 ||
-            $this->decisionMap[$packageId] < 0 && $literal < 0
-        );
-    }
-
-    protected function decisionsSatisfy($literal)
-    {
-        $packageId = abs($literal);
-
-        return (
-            $literal > 0 && $this->decisionMap[$packageId] > 0 ||
-            $literal < 0 && $this->decisionMap[$packageId] < 0
-        );
-    }
-
-    public function decisionsConflict($literal)
-    {
-        $packageId = abs($literal);
-
-        return (
-            ($this->decisionMap[$packageId] > 0 && $literal < 0) ||
-            ($this->decisionMap[$packageId] < 0 && $literal > 0)
-        );
-    }
-    protected function decided($packageId)
-    {
-        return $this->decisionMap[$packageId] != 0;
-    }
-
-    protected function undecided($packageId)
-    {
-        return $this->decisionMap[$packageId] == 0;
-    }
-
-    protected function decidedInstall($packageId)
-    {
-        return $this->decisionMap[$packageId] > 0;
-    }
-
     /**
      * Makes a decision and propagates it to all rules.
      *
@@ -292,15 +197,17 @@ class Solver
      */
     protected function propagate($level)
     {
-        while ($this->propagateIndex < count($this->decisionQueue)) {
+        while ($this->decisions->validOffset($this->propagateIndex)) {
+            $decision = $this->decisions->atOffset($this->propagateIndex);
+
             $conflict = $this->watchGraph->propagateLiteral(
-                $this->decisionQueue[$this->propagateIndex++],
+                $decision[Decisions::DECISION_LITERAL],
                 $level,
-                array($this, 'decisionsContain'),
-                array($this, 'decisionsConflict'),
-                array($this, 'decide')
+                $this->decisions
             );
 
+            $this->propagateIndex++;
+
             if ($conflict) {
                 return $conflict;
             }
@@ -314,24 +221,21 @@ class Solver
      */
     private function revert($level)
     {
-        while (!empty($this->decisionQueue)) {
-            $literal = $this->decisionQueue[count($this->decisionQueue) - 1];
+        while (!$this->decisions->isEmpty()) {
+            $literal = $this->decisions->lastLiteral();
 
-            if (!$this->decisionMap[abs($literal)]) {
+            if ($this->decisions->undecided($literal)) {
                 break;
             }
 
-            $decisionLevel = abs($this->decisionMap[abs($literal)]);
+            $decisionLevel = $this->decisions->decisionLevel($literal);
 
             if ($decisionLevel <= $level) {
                 break;
             }
 
-            $this->decisionMap[abs($literal)] = 0;
-            array_pop($this->decisionQueue);
-            array_pop($this->decisionQueueWhy);
-
-            $this->propagateIndex = count($this->decisionQueue);
+            $this->decisions->revertLast();
+            $this->propagateIndex = $this->decisions->getMaxOffset() + 1;
         }
 
         while (!empty($this->branches)) {
@@ -349,7 +253,7 @@ class Solver
      *
      * setpropagatelearn
      *
-     * add free decision (solvable to install) to decisionq
+     * add free decision (a positive literal) to decision queue
      * increase level and propagate decision
      * return if no conflict.
      *
@@ -364,8 +268,7 @@ class Solver
     {
         $level++;
 
-        $this->decide($literal, $level, $rule);
-        $this->decisionQueueFree[count($this->decisionQueueWhy) - 1] = true;
+        $this->decisions->decide($literal, $level, $rule, true);
 
         while (true) {
             $rule = $this->propagate($level);
@@ -400,10 +303,10 @@ class Solver
             $this->learnedWhy[$newRule->getId()] = $why;
 
             $ruleNode = new RuleWatchNode($newRule);
-            $ruleNode->watch2OnHighest($this->decisionMap);
+            $ruleNode->watch2OnHighest($this->decisions);
             $this->watchGraph->insert($ruleNode);
 
-            $this->decide($learnLiteral, $level, $newRule);
+            $this->decisions->decide($learnLiteral, $level, $newRule);
         }
 
         return $level;
@@ -433,7 +336,7 @@ class Solver
         $seen = array();
         $learnedLiterals = array(null);
 
-        $decisionId = count($this->decisionQueue);
+        $decisionId = $this->decisions->getMaxOffset() + 1;
 
         $this->learnedPool[] = array();
 
@@ -442,7 +345,7 @@ class Solver
 
             foreach ($rule->getLiterals() as $literal) {
                 // skip the one true literal
-                if ($this->decisionsSatisfy($literal)) {
+                if ($this->decisions->satisfy($literal)) {
                     continue;
                 }
 
@@ -451,7 +354,7 @@ class Solver
                 }
                 $seen[abs($literal)] = true;
 
-                $l = abs($this->decisionMap[abs($literal)]);
+                $l = $this->decisions->decisionLevel($literal);
 
                 if (1 === $l) {
                     $l1num++;
@@ -485,7 +388,8 @@ class Solver
 
                     $decisionId--;
 
-                    $literal = $this->decisionQueue[$decisionId];
+                    $decision = $this->decisions->atOffset($decisionId);
+                    $literal = $decision[Decisions::DECISION_LITERAL];
 
                     if (isset($seen[abs($literal)])) {
                         break;
@@ -512,7 +416,8 @@ class Solver
                 }
             }
 
-            $rule = $this->decisionQueueWhy[$decisionId];
+            $decision = $this->decisions->atOffset($decisionId);
+            $rule = $decision[Decisions::DECISION_REASON];
         }
 
         $why = count($this->learnedPool) - 1;
@@ -565,34 +470,35 @@ class Solver
 
         foreach ($literals as $literal) {
             // skip the one true literal
-            if ($this->decisionsSatisfy($literal)) {
+            if ($this->decisions->satisfy($literal)) {
                 continue;
             }
             $seen[abs($literal)] = true;
         }
 
-        $decisionId = count($this->decisionQueue);
+        $decisionId = $this->decisions->getMaxOffset() + 1;
 
         while ($decisionId > 0) {
             $decisionId--;
 
-            $literal = $this->decisionQueue[$decisionId];
+            $decision = $this->decisions->atOffset($decisionId);
+            $literal = $decision[Decisions::DECISION_LITERAL];
 
             // skip literals that are not in this rule
             if (!isset($seen[abs($literal)])) {
                 continue;
             }
 
-            $why = $this->decisionQueueWhy[$decisionId];
-            $problem->addRule($why);
+            $why = $decision[Decisions::DECISION_REASON];
 
+            $problem->addRule($why);
             $this->analyzeUnsolvableRule($problem, $why);
 
             $literals = $why->getLiterals();
 
             foreach ($literals as $literal) {
                 // skip the one true literal
-                if ($this->decisionsSatisfy($literal)) {
+                if ($this->decisions->satisfy($literal)) {
                     continue;
                 }
                 $seen[abs($literal)] = true;
@@ -630,12 +536,8 @@ class Solver
 
     private function resetSolver()
     {
-        while ($literal = array_pop($this->decisionQueue)) {
-            $this->decisionMap[abs($literal)] = 0;
-        }
+        $this->decisions->reset();
 
-        $this->decisionQueueWhy = array();
-        $this->decisionQueueFree = array();
         $this->propagateIndex = 0;
         $this->branches = array();
 
@@ -717,11 +619,11 @@ class Solver
                         $noneSatisfied = true;
 
                         foreach ($rule->getLiterals() as $literal) {
-                            if ($this->decisionsSatisfy($literal)) {
+                            if ($this->decisions->satisfy($literal)) {
                                 $noneSatisfied = false;
                                 break;
                             }
-                            if ($literal > 0 && $this->undecided($literal)) {
+                            if ($literal > 0 && $this->decisions->undecided($literal)) {
                                 $decisionQueue[] = $literal;
                             }
                         }
@@ -794,14 +696,14 @@ class Solver
                 //
                 foreach ($literals as $literal) {
                     if ($literal <= 0) {
-                        if (!$this->decidedInstall(abs($literal))) {
+                        if (!$this->decisions->decidedInstall(abs($literal))) {
                             continue 2; // next rule
                         }
                     } else {
-                        if ($this->decidedInstall(abs($literal))) {
+                        if ($this->decisions->decidedInstall(abs($literal))) {
                             continue 2; // next rule
                         }
-                        if ($this->undecided(abs($literal))) {
+                        if ($this->decisions->undecided(abs($literal))) {
                             $decisionQueue[] = $literal;
                         }
                     }
@@ -839,7 +741,7 @@ class Solver
                     list($literals, $level) = $this->branches[$i];
 
                     foreach ($literals as $offset => $literal) {
-                        if ($literal && $literal > 0 && $this->decisionMap[abs($literal)] > $level + 1) {
+                        if ($literal && $literal > 0 && $this->decisions->decisionLevel($literal) > $level + 1) {
                             $lastLiteral = $literal;
                             $lastBranchIndex = $i;
                             $lastBranchOffset = $offset;
@@ -855,7 +757,7 @@ class Solver
                     $level = $lastLevel;
                     $this->revert($level);
 
-                    $why = $this->decisionQueueWhy[count($this->decisionQueueWhy) - 1];
+                    $why = $this->decisions->lastReason();
 
                     $oLevel = $level;
                     $level = $this->setPropagateLearn($level, $lastLiteral, $disableRules, $why);

src/Composer/DependencyResolver/Transaction.php

@@ -23,26 +23,30 @@ class Transaction
     protected $policy;
     protected $pool;
     protected $installedMap;
-    protected $decisionMap;
-    protected $decisionQueue;
-    protected $decisionQueueWhy;
+    protected $decisions;
+    protected $transaction;
 
-    public function __construct($policy, $pool, $installedMap, $decisionMap, array $decisionQueue, $decisionQueueWhy)
+    public function __construct($policy, $pool, $installedMap, $decisions)
     {
         $this->policy = $policy;
         $this->pool = $pool;
         $this->installedMap = $installedMap;
-        $this->decisionMap = $decisionMap;
-        $this->decisionQueue = $decisionQueue;
-        $this->decisionQueueWhy = $decisionQueueWhy;
+        $this->decisions = $decisions;
+        $this->transaction = array();
     }
 
     public function getOperations()
     {
-        $transaction = array();
         $installMeansUpdateMap = array();
 
-        foreach ($this->decisionQueue as $i => $literal) {
+        foreach ($this->installedMap as $packageId => $void) {
+            if ($this->decisions->undecided($packageId)) {
+                $this->decisions->decide(-$packageId, 1, null);
+            }
+        }
+
+        foreach ($this->decisions as $i => $decision) {
+            $literal = $decision[Decisions::DECISION_LITERAL];
             $package = $this->pool->literalToPackage($literal);
 
             // !wanted & installed
@@ -63,7 +67,8 @@ class Transaction
             }
         }
 
-        foreach ($this->decisionQueue as $i => $literal) {
+        foreach ($this->decisions as $i => $decision) {
+            $literal = $decision[Decisions::DECISION_LITERAL];
             $package = $this->pool->literalToPackage($literal);
 
             // wanted & installed || !wanted & !installed
@@ -73,9 +78,7 @@ class Transaction
 
             if ($literal > 0) {
                 if ($package instanceof AliasPackage) {
-                    $transaction[] = new Operation\MarkAliasInstalledOperation(
-                        $package, $this->decisionQueueWhy[$i]
-                    );
+                    $this->markAliasInstalled($package, $decision[Decisions::DECISION_REASON]);
                     continue;
                 }
 
@@ -83,85 +86,64 @@ class Transaction
 
                     $source = $installMeansUpdateMap[abs($literal)];
 
-                    $transaction[] = new Operation\UpdateOperation(
-                        $source, $package, $this->decisionQueueWhy[$i]
-                    );
+                    $this->update($source, $package, $decision[Decisions::DECISION_REASON]);
 
                     // avoid updates to one package from multiple origins
                     unset($installMeansUpdateMap[abs($literal)]);
                     $ignoreRemove[$source->getId()] = true;
                 } else {
-                    $transaction[] = new Operation\InstallOperation(
-                        $package, $this->decisionQueueWhy[$i]
-                    );
-                }
-            } elseif (!isset($ignoreRemove[$package->getId()])) {
-                if ($package instanceof AliasPackage) {
-                    $transaction[] = new Operation\MarkAliasInstalledOperation(
-                        $package, $this->decisionQueueWhy[$i]
-                    );
-                } else {
-                    $transaction[] = new Operation\UninstallOperation(
-                        $package, $this->decisionQueueWhy[$i]
-                    );
+                    $this->install($package, $decision[Decisions::DECISION_REASON]);
                 }
             }
         }
 
-        $allDecidedMap = $this->decisionMap;
-        foreach ($this->decisionMap as $packageId => $decision) {
-            if ($decision != 0) {
-                $package = $this->pool->packageById($packageId);
-                if ($package instanceof AliasPackage) {
-                    $allDecidedMap[$package->getAliasOf()->getId()] = $decision;
-                }
-            }
-        }
+        foreach ($this->decisions as $i => $decision) {
+            $literal = $decision[Decisions::DECISION_LITERAL];
+            $package = $this->pool->literalToPackage($literal);
 
-        foreach ($allDecidedMap as $packageId => $decision) {
-            if ($packageId === 0) {
+            // wanted & installed || !wanted & !installed
+            if (($literal > 0) == (isset($this->installedMap[$package->getId()]))) {
                 continue;
             }
 
-            if (0 == $decision && isset($this->installedMap[$packageId])) {
-                $package = $this->pool->packageById($packageId);
-
-                if ($package instanceof AliasPackage) {
-                    $transaction[] = new Operation\MarkAliasInstalledOperation(
-                        $package, null
-                    );
-                } else {
-                    $transaction[] = new Operation\UninstallOperation(
-                        $package, null
-                    );
-                }
-
-                $this->decisionMap[$packageId] = -1;
+            if ($literal <= 0 && !isset($ignoreRemove[$package->getId()])) {
+                $this->uninstall($package, $decision[Decisions::DECISION_REASON]);
             }
         }
 
-        foreach ($allDecidedMap as $packageId => $decision) {
-            if ($packageId === 0) {
-                continue;
-            }
+        return $this->transaction;
+    }
 
-            if (0 == $decision && isset($this->installedMap[$packageId])) {
-                $package = $this->pool->packageById($packageId);
+    protected function install($package, $reason)
+    {
+        if ($package instanceof AliasPackage) {
+            return $this->markAliasInstalled($package, $reason);
+        }
 
-                if ($package instanceof AliasPackage) {
-                    $transaction[] = new Operation\MarkAliasInstalledOperation(
-                        $package, null
-                    );
-                } else {
-                    $transaction[] = new Operation\UninstallOperation(
-                        $package, null
-                    );
-                }
+        $this->transaction[] = new Operation\InstallOperation($package, $reason);
+    }
 
-                $this->decisionMap[$packageId] = -1;
-            }
+    protected function update($from, $to, $reason)
+    {
+        $this->transaction[] = new Operation\UpdateOperation($from, $to, $reason);
+    }
+
+    protected function uninstall($package, $reason)
+    {
+        if ($package instanceof AliasPackage) {
+            return $this->markAliasUninstalled($package, $reason);
         }
 
-        return array_reverse($transaction);
+        $this->transaction[] = new Operation\UninstallOperation($package, $reason);
+    }
+
+    protected function markAliasInstalled($package, $reason)
+    {
+        $this->transaction[] = new Operation\MarkAliasInstalledOperation($package, $reason);
+    }
+
+    protected function markAliasUninstalled($package, $reason)
+    {
+        $this->transaction[] = new Operation\MarkAliasUninstalledOperation($package, $reason);
     }
 }

tests/Composer/Test/DependencyResolver/SolverTest.php

@@ -291,15 +291,16 @@ class SolverTest extends TestCase
         $this->request->update('A', $this->getVersionConstraint('=', '1.0.0.0'));
 
         $this->checkSolverResult(array(
-            array(
-                'job' => 'remove',
-                'package' => $packageB,
-            ),
             array(
                 'job' => 'update',
                 'from' => $packageA,
                 'to' => $newPackageA,
-        )));
+            ),
+            array(
+                'job' => 'remove',
+                'package' => $packageB,
+            ),
+        ));
     }
 
     public function testSolverAllJobs()
@@ -324,8 +325,8 @@ class SolverTest extends TestCase
         $this->checkSolverResult(array(
             array('job' => 'update',  'from' => $oldPackageC, 'to' => $packageC),
             array('job' => 'install', 'package' => $packageB),
-            array('job' => 'remove',  'package' => $packageD),
             array('job' => 'install', 'package' => $packageA),
+            array('job' => 'remove',  'package' => $packageD),
         ));
     }