Merge branch 'MC-5465-requirement-ordering' into MC-5465-merged

Author: pdohogne-magento

docs/class_descriptions.md

@@ -150,13 +150,17 @@ This is accomplished by comparing `composer.json` fields between the original Ma
  - **`findResolution()`**
  - For an individual field value, compare the original Magento value to the target Magento value, and if a delta is found, check if the user's installation has a customized value for the field. If the user has changed the value, resolve the conflict according to the CLI command options: use the user's custom value, override with the target Magento value, or interactively ask the user which of the two values should be used
  - **`resolveLinkSection()`**
- - For a given `composer.json` section that consists of links to package versions/constraints (such as the `require` and `conflict` sections), call `findResolution()` for each package constraint found in either the original Magento root or the target Magento root
+ - For a given `composer.json` section that consists of links to package versions/constraints (such as the `require` and `conflict` sections), call `findLinkResolution()` for each package constraint found in either the original Magento root or the target Magento root
  - **`resolveArraySection()`**
  - For a given `composer.json` section that consists of data that is not package links (such as the `"autoload"` or `"extra"` sections), call `resolveNestedArray()` and accept the new values if changes were made
  - **`resolveNestedArray()`**
  - Recursively processes changes to a `composer.json` value that could be a nested array, calling `findResolution()` for each "leaf" value found in either the original Magento root or the target Magento root
- - **`linksToMap()`**
- - Helper function to convert a set of package links to an associative array for use by `resolveLinkSection()`
+ - **`findLinkResolution()`**
+ - Helper function to call `findResolution()` for a particular package for use by `resolveLinkSection()`
+ - **`getLinkOrderOverride()`**
+ - Determine the order to use for a link section when the user's order disagrees with the target Magento section order
+ - **`buildLinkOrderComparator()`**
+ - Construct the comparator function to use for sorting a set of links according to `getLinkOverride()` results followed by the order in the target Magento version followed by the order of custom values in the user's installation
 
 #### [**MagentoRootUpdater**](../src/Magento/ComposerRootUpdatePlugin/Updater/MagentoRootUpdater.php)
 

src/Magento/ComposerRootUpdatePlugin/Updater/DeltaResolver.php

@@ -89,21 +89,41 @@ public function resolveRootDeltas()
  $target = $this->targetMageRootPackage;
  $user = $this->userRootPackage;
 
- $this->resolveLinkSection('require', $original->getRequires(), $target->getRequires(), $user->getRequires());
+ $this->resolveLinkSection(
+ 'require',
+ $original->getRequires(),
+ $target->getRequires(),
+ $user->getRequires(),
+ true
+ );
  $this->resolveLinkSection(
  'require-dev',
  $original->getDevRequires(),
  $target->getDevRequires(),
- $user->getDevRequires()
+ $user->getDevRequires(),
+ true
  );
  $this->resolveLinkSection(
  'conflict',
  $original->getConflicts(),
  $target->getConflicts(),
- $user->getConflicts()
+ $user->getConflicts(),
+ false
+ );
+ $this->resolveLinkSection(
+ 'provide',
+ $original->getProvides(),
+ $target->getProvides(),
+ $user->getProvides(),
+ false
+ );
+ $this->resolveLinkSection(
+ 'replace',
+ $original->getReplaces(),
+ $target->getReplaces(),
+ $user->getReplaces(),
+ false
  );
- $this->resolveLinkSection('provide', $original->getProvides(), $target->getProvides(), $user->getProvides());
- $this->resolveLinkSection('replace', $original->getReplaces(), $target->getReplaces(), $user->getReplaces());
 
  $this->resolveArraySection('autoload', $original->getAutoload(), $target->getAutoload(), $user->getAutoload());
  $this->resolveArraySection(
@@ -215,53 +235,26 @@ public function findResolution(
  * @param Link[] $originalMageLinks
  * @param Link[] $targetMageLinks
  * @param Link[] $userLinks
+ * @param bool $verifyOrder
  * @return array
  */
- public function resolveLinkSection($section, $originalMageLinks, $targetMageLinks, $userLinks)
+ public function resolveLinkSection($section, $originalMageLinks, $targetMageLinks, $userLinks, $verifyOrder)
  {
- /** @var Link[] $originalLinkMap */
- $originalLinkMap = static::linksToMap($originalMageLinks);
-
- /** @var Link[] $targetLinkMap */
- $targetLinkMap = static::linksToMap($targetMageLinks);
-
- /** @var Link[] $userLinkMap */
- $userLinkMap = static::linksToMap($userLinks);
-
  $adds = [];
  $removes = [];
  $changes = [];
- $magePackages = array_unique(array_merge(array_keys($originalLinkMap), array_keys($targetLinkMap)));
+ $magePackages = array_unique(array_merge(array_keys($originalMageLinks), array_keys($targetMageLinks)));
  foreach ($magePackages as $pkg) {
  if ($section === 'require' && PackageUtils::getMagentoProductEdition($pkg)) {
  continue;
  }
- $field = "$section:$pkg";
- $originalConstraint = key_exists($pkg, $originalLinkMap) ? $originalLinkMap[$pkg]->getConstraint() : null;
- $originalMageVal = ($originalConstraint === null) ? null : $originalConstraint->__toString();
- $prettyOriginalMageVal = ($originalConstraint === null) ? null : $originalConstraint->getPrettyString();
- $targetConstraint = key_exists($pkg, $targetLinkMap) ? $targetLinkMap[$pkg]->getConstraint() : null;
- $targetMageVal = ($targetConstraint === null) ? null : $targetConstraint->__toString();
- $prettyTargetMageVal = ($targetConstraint === null) ? null : $targetConstraint->getPrettyString();
- $userConstraint = key_exists($pkg, $userLinkMap) ? $userLinkMap[$pkg]->getConstraint() : null;
- $userVal = ($userConstraint === null) ? null : $userConstraint->__toString();
- $prettyUserVal = ($userConstraint === null) ? null : $userConstraint->getPrettyString();
-
- $action = $this->findResolution(
- $field,
- $originalMageVal,
- $targetMageVal,
- $userVal,
- $prettyOriginalMageVal,
- $prettyTargetMageVal,
- $prettyUserVal
- );
+ $action = $this->findLinkResolution($section, $pkg, $originalMageLinks, $targetMageLinks, $userLinks);
  if ($action == static::ADD_VAL) {
- $adds[$pkg] = $targetLinkMap[$pkg];
+ $adds[$pkg] = $targetMageLinks[$pkg];
  } elseif ($action == static::REMOVE_VAL) {
  $removes[] = $pkg;
  } elseif ($action == static::CHANGE_VAL) {
- $changes[$pkg] = $targetLinkMap[$pkg];
+ $changes[$pkg] = $targetMageLinks[$pkg];
  }
  }
 
@@ -287,11 +280,26 @@ public function resolveLinkSection($section, $originalMageLinks, $targetMageLink
  $this->console->labeledVerbose("Updating $section constraints: " . implode(', ', $prettyChanges));
  }
 
+ $enforcedOrder = [];
+ if ($verifyOrder) {
+ $enforcedOrder = $this->getLinkOrderOverride(
+ $section,
+ array_keys($originalMageLinks),
+ array_keys($targetMageLinks),
+ array_keys($userLinks)
+ );
+ if ($enforcedOrder !== []) {
+ $changed = true;
+ $prettyOrder = " [\n " . implode(",\n ", $enforcedOrder) . "\n ]";
+ $this->console->labeledVerbose("Updating $section order:\n$prettyOrder");
+ }
+ }
+
  if ($changed) {
  $replacements = array_values($adds);
 
  /** @var Link $userLink */
- foreach ($userLinkMap as $pkg => $userLink) {
+ foreach ($userLinks as $pkg => $userLink) {
  if (in_array($pkg, $removes)) {
  continue;
  } elseif (key_exists($pkg, $changes)) {
@@ -301,6 +309,12 @@ public function resolveLinkSection($section, $originalMageLinks, $targetMageLink
  }
  }
 
+ usort($replacements, $this->buildLinkOrderComparator(
+ $enforcedOrder,
+ array_keys($targetMageLinks),
+ array_keys($userLinks)
+ ));
+
  $newJson = [];
  /** @var Link $link */
  foreach ($replacements as $link) {
@@ -349,37 +363,15 @@ public function resolveNestedArray($field, $originalMageVal, $targetMageVal, $us
  $result = $userVal === null ? [] : $userVal;
 
  if (is_array($originalMageVal) && is_array($targetMageVal) && is_array($userVal)) {
- $originalMageAssociativePart = [];
- $originalMageFlatPart = [];
- foreach ($originalMageVal as $key => $value) {
- if (is_string($key)) {
- $originalMageAssociativePart[$key] = $value;
- } else {
- $originalMageFlatPart[] = $value;
- }
- }
+ $originalMageAssociativePart = array_filter($originalMageVal, 'is_string', ARRAY_FILTER_USE_KEY);
+ $originalMageFlatPart = array_filter($originalMageVal, 'is_int', ARRAY_FILTER_USE_KEY);
 
- $targetMageAssociativePart = [];
- $targetMageFlatPart = [];
- foreach ($targetMageVal as $key => $value) {
- if (is_string($key)) {
- $targetMageAssociativePart[$key] = $value;
- } else {
- $targetMageFlatPart[] = $value;
- }
- }
+ $targetMageAssociativePart = array_filter($targetMageVal, 'is_string', ARRAY_FILTER_USE_KEY);
+ $targetMageFlatPart = array_filter($targetMageVal, 'is_int', ARRAY_FILTER_USE_KEY);
 
- $userAssociativePart = [];
- $userFlatPart = [];
- foreach ($userVal as $key => $value) {
- if (is_string($key)) {
- $userAssociativePart[$key] = $value;
- } else {
- $userFlatPart[] = $value;
- }
- }
+ $userAssociativePart = array_filter($userVal, 'is_string', ARRAY_FILTER_USE_KEY);
 
- $associativeResult = array_filter($result, 'is_string', ARRAY_FILTER_USE_KEY);
+ $associativeResult = $userAssociativePart;
  $mageKeys = array_unique(
  array_merge(array_keys($originalMageAssociativePart), array_keys($targetMageAssociativePart))
  );
@@ -417,7 +409,7 @@ public function resolveNestedArray($field, $originalMageVal, $targetMageVal, $us
  }
  }
 
- $flatResult = array_filter($result, 'is_int', ARRAY_FILTER_USE_KEY);
+ $flatResult = array_filter($userVal, 'is_int', ARRAY_FILTER_USE_KEY);
  $flatAdds = array_diff(array_diff($targetMageFlatPart, $originalMageFlatPart), $flatResult);
  if ($flatAdds !== []) {
  $valChanged = true;
@@ -456,17 +448,132 @@ public function resolveNestedArray($field, $originalMageVal, $targetMageVal, $us
  }
 
  /**
- * Helper function to convert a set of links to an associative array with target package names as keys
+ * Helper function to find the resolution action for a package constraint in the Link sections
  *
- * @param Link[] $links
- * @return array
+ * @param string $section
+ * @param string $pkg
+ * @param Link[] $originalLinkMap
+ * @param Link[] $targetLinkMap
+ * @param Link[] $userLinkMap
+ * @return string|null ADD_VAL|REMOVE_VAL|CHANGE_VAL to adjust the link constraint, null for no change
  */
- protected function linksToMap($links)
+ protected function findLinkResolution($section, $pkg, $originalLinkMap, $targetLinkMap, $userLinkMap)
  {
- $targets = array_map(function ($link) {
- /** @var Link $link */
- return $link->getTarget();
- }, $links);
- return array_combine($targets, $links);
+ $field = "$section:$pkg";
+ $originalConstraint = key_exists($pkg, $originalLinkMap) ? $originalLinkMap[$pkg]->getConstraint() : null;
+ $originalMageVal = ($originalConstraint === null) ? null : $originalConstraint->__toString();
+ $prettyOriginalMageVal = ($originalConstraint === null) ? null : $originalConstraint->getPrettyString();
+ $targetConstraint = key_exists($pkg, $targetLinkMap) ? $targetLinkMap[$pkg]->getConstraint() : null;
+ $targetMageVal = ($targetConstraint === null) ? null : $targetConstraint->__toString();
+ $prettyTargetMageVal = ($targetConstraint === null) ? null : $targetConstraint->getPrettyString();
+ $userConstraint = key_exists($pkg, $userLinkMap) ? $userLinkMap[$pkg]->getConstraint() : null;
+ $userVal = ($userConstraint === null) ? null : $userConstraint->__toString();
+ $prettyUserVal = ($userConstraint === null) ? null : $userConstraint->getPrettyString();
+
+ return $this->findResolution(
+ $field,
+ $originalMageVal,
+ $targetMageVal,
+ $userVal,
+ $prettyOriginalMageVal,
+ $prettyTargetMageVal,
+ $prettyUserVal
+ );
+ }
+
+ /**
+ * Get the order to use for a link section if local and target versions disagree
+ *
+ * @param string $section
+ * @param string[] $originalMageOrder
+ * @param string[] $targetMageOrder
+ * @param string[] $userOrder
+ * @return string[]
+ */
+ protected function getLinkOrderOverride($section, $originalMageOrder, $targetMageOrder, $userOrder)
+ {
+ $overrideOrder = [];
+
+ $conflictTargetOrder = array_values(array_intersect($targetMageOrder, $userOrder));
+ $conflictUserOrder = array_values(array_intersect($userOrder, $targetMageOrder));
+
+ // Check if the user's link order does not match the target section for links that appear in both
+ if ($conflictTargetOrder != $conflictUserOrder) {
+ $conflictOriginalOrder = array_values(array_intersect($originalMageOrder, $targetMageOrder));
+
+ // Check if the user's order is different than the target order because the order has changed between
+ // the original and target Magento versions
+ if ($conflictOriginalOrder !== $conflictUserOrder) {
+ $targetLabel = $this->retriever->getTargetLabel();
+ $userOrderDesc = " [\n " . implode(",\n ", $conflictUserOrder) . "\n ]";
+ $targetOrderDesc = " [\n " . implode(",\n ", $conflictTargetOrder) . "\n ]";
+ $conflictDesc = "$targetLabel is trying to change the existing order of the $section section.\n" .
+ "Local order:\n$userOrderDesc\n$targetLabel order:\n$targetOrderDesc";
+ $shouldOverride = $this->overrideUserValues;
+ if ($this->overrideUserValues) {
+ $this->console->log($conflictDesc);
+ $this->console->log(
+ 'Overriding local order due to --' . MageRootRequireCommand::OVERRIDE_OPT . '.'
+ );
+ } else {
+ $shouldOverride = $this->console->ask(
+ "$conflictDesc\nWould you like to override the local order?"
+ );
+ }
+
+ if (!$shouldOverride) {
+ $this->console->comment("$conflictDesc but it will not be changed. Re-run using " .
+ '--' . MageRootRequireCommand::OVERRIDE_OPT . ' or ' .
+ '--' . MageRootRequireCommand::INTERACTIVE_OPT . ' to override with the Magento order.');
+ $overrideOrder = $conflictUserOrder;
+ } else {
+ $overrideOrder = $conflictTargetOrder;
+ }
+ } else {
+ $overrideOrder = $conflictTargetOrder;
+ }
+ }
+
+ return $overrideOrder;
+ }
+
+ /**
+ * Construct a comparison function to use in sorting an array of links by prioritized order lists
+ *
+ * @param string[] $overrideOrder
+ * @param string[] $targetMageOrder
+ * @param string[] $userOrder
+ * @return \Closure
+ */
+ protected function buildLinkOrderComparator($overrideOrder, $targetMageOrder, $userOrder)
+ {
+ $prioritizedOrderings = [$overrideOrder, $targetMageOrder, $userOrder];
+
+ return function ($link1, $link2) use ($prioritizedOrderings) {
+ /**
+ * @var Link $link1
+ * @var Link $link2
+ */
+ $package1 = $link1->getTarget();
+ $package2 = $link2->getTarget();
+
+ // Check each ordering array to see if it contains both links and if so use their positions to sort
+ // If the ordering array does not contain both links, try the next one
+ foreach ($prioritizedOrderings as $sortOrder) {
+ $index1 = array_search($package1, $sortOrder);
+ $index2 = array_search($package2, $sortOrder);
+ if ($index1 !== false && $index2 !== false) {
+ if ($index1 == $index2) {
+ return 0;
+ } else {
+ return $index1 < $index2 ? -1 : 1;
+ }
+ }
+ }
+
+ // None of the ordering arrays contain both elements, so their relative positions in the sorted array
+ // do not matter
+ return 0;
+ };
  }
 }