Skip to content

Commit

Permalink
Merge pull request #35 from mediact/feature/PD-731
Browse files Browse the repository at this point in the history
1.1.0 - Package requirements resolver

When DependencyGuard is expecting resolved dependents, it is expecting package links of the "require" type. Leveraging Composer to determine the dependents will result in a list with all types of package link, which causes DependencyGuard to make assumptions using a wrong dependency graph.

This problem is solved by adding a new violation filter based on package requirements as defined in the Composer Locker.

The package requirements filter will determine the package dependents based on the installed non-dev packages inside the locker and use the dependents to determine what package caused the violation of another package violation.

Resolving the locker and filtering violations based on the resulting graph have been split up in separate classes, to keep proper separation of concerns.

As a direct result, the functional test, verifying what packages are and are not dependent on a given package, has become easier to implement.
  • Loading branch information
marcelmediact authored Jan 11, 2019
2 parents 49ab1be + 0335417 commit 57515c4
Show file tree
Hide file tree
Showing 16 changed files with 5,515 additions and 16 deletions.
2 changes: 2 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
"minimum-stability": "stable",
"require": {
"php": "^7.1",
"ext-SPL": "^7.1",
"ext-json": "^1.5",
"composer-plugin-api": "^1.1",
"composer/composer": "^1.6",
"nikic/php-parser": "^3.0 | ^4.0",
Expand Down
1 change: 1 addition & 0 deletions grumphp.dist.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ imports:
- resource: vendor/mediact/testing-suite/config/default/grumphp.yml

parameters:
process_timeout: 300
tasks:
composer:
strict: true
Expand Down
5 changes: 5 additions & 0 deletions phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@
<testsuites>
<testsuite name="default">
<directory>tests/Regression</directory>
<directory>tests/Functional</directory>
<directory>tests</directory>
</testsuite>
<testsuite name="coverage">
<directory>tests</directory>
<exclude>tests/Regression</exclude>
<exclude>tests/Functional</exclude>
</testsuite>
<testsuite name="regression">
<directory>tests/Regression</directory>
</testsuite>
<testsuite name="functional">
<directory>tests/Functional</directory>
</testsuite>
</testsuites>
<filter>
<whitelist processUncoveredFilesFromWhitelist="true">
Expand Down
150 changes: 150 additions & 0 deletions src/Composer/Locker/PackageRequirementsResolver.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
<?php
/**
* Copyright MediaCT. All rights reserved.
* https://www.mediact.nl
*/

namespace Mediact\DependencyGuard\Composer\Locker;

use Composer\Package\Locker;
use Composer\Package\PackageInterface;
use SplObjectStorage;

class PackageRequirementsResolver implements PackageRequirementsResolverInterface
{
/** @var SplObjectStorage|PackageInterface[][][] */
private $resolvedLockers;

/**
* Constructor.
*
* @param SplObjectStorage|null $resolvedLockers
*/
public function __construct(SplObjectStorage $resolvedLockers = null)
{
$this->resolvedLockers = $resolvedLockers ?? new SplObjectStorage();
}

/**
* Resolve the dependents graph for the given Composer locker.
*
* @param Locker $locker
*
* @return array|PackageInterface[][]
*/
public function resolve(Locker $locker): array
{
if (!$this->resolvedLockers->contains($locker)) {
$lockedPackages = $locker->getLockedRepository()->getPackages();

$this->resolvedLockers->attach(
$locker,
array_map(
function (array $packages) use ($lockedPackages) : array {
return array_reduce(
$lockedPackages,
function (
array $carry,
PackageInterface $package
) use ($packages) : array {
if (in_array(
$package->getName(),
$packages,
true
)) {
$carry[] = $package;
}

return $carry;
},
[]
);
},
$this->resolveGraph(
...$locker->getLockData()['packages'] ?? []
)
)
);
}

return $this->resolvedLockers->offsetGet($locker);
}

/**
* Get the dependent packages for the given package, using the given locker.
*
* @param string $package
* @param Locker $locker
*
* @return array|PackageInterface[]
*/
public function getDependents(string $package, Locker $locker): array
{
return $this->resolve($locker)[$package] ?? [];
}

/**
* Resolve the dependents graph for the given packages.
*
* @param array ...$packages
*
* @return string[]
*/
private function resolveGraph(array ...$packages): array
{
$graph = array_reduce(
$packages,
function (array $carry, array $package): array {
foreach (array_keys($package['require'] ?? []) as $link) {
if (!preg_match('/^[^\/]+\/[^\/]+$/', $link)) {
// Most likely a platform requirement.
// E.g.: php
// E.g.: ext-openssl
continue;
}

if (!array_key_exists($link, $carry)) {
$carry[$link] = [];
}

if (!in_array($package['name'], $carry[$link], true)) {
$carry[$link][] = $package['name'];
}
}

return $carry;
},
[]
);

// While the graph keeps receiving updates, keep on resolving.
for ($previousGraph = []; $graph !== $previousGraph;) {
// Do not update the previous graph before the current iteration
// has started.
$previousGraph = $graph;

// For each package and its dependents in the graph ...
foreach ($graph as $package => $dependents) {
// ... Update the dependents of the package with grandparents.
$graph[$package] = array_reduce(
// Determine grandparents by looking up the parents of the
// available dependents.
$dependents,
function (array $carry, string $parent) use ($graph): array {
foreach ($graph[$parent] ?? [] as $grandparent) {
if (!in_array($grandparent, $carry, true)) {
$carry[] = $grandparent;
}
}

return $carry;
},
// Start out with the current list of dependents.
$dependents
);
}
}

return $graph;
}
}
32 changes: 32 additions & 0 deletions src/Composer/Locker/PackageRequirementsResolverInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php
/**
* Copyright MediaCT. All rights reserved.
* https://www.mediact.nl
*/

namespace Mediact\DependencyGuard\Composer\Locker;

use Composer\Package\Locker;
use Composer\Package\PackageInterface;

interface PackageRequirementsResolverInterface
{
/**
* Resolve the dependents graph for the given Composer locker.
*
* @param Locker $locker
*
* @return array|PackageInterface[][]
*/
public function resolve(Locker $locker): array;

/**
* Get the dependent packages for the given package, using the given locker.
*
* @param string $package
* @param Locker $locker
*
* @return array|PackageInterface[]
*/
public function getDependents(string $package, Locker $locker): array;
}
4 changes: 4 additions & 0 deletions src/Composer/Repository/DependentsResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
use Composer\Repository\CompositeRepository;
use Composer\Repository\RepositoryInterface;

/**
* @deprecated The conceptual difference between dependents in Composer and
* DependencyGuard is too great to rely on the output of a dependents resolver.
*/
class DependentsResolver implements DependentsResolverInterface
{
/** @var CompositeRepository */
Expand Down
4 changes: 4 additions & 0 deletions src/Composer/Repository/DependentsResolverInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@

namespace Mediact\DependencyGuard\Composer\Repository;

/**
* @deprecated The conceptual difference between dependents in Composer and
* DependencyGuard is too great to rely on the output of a dependents resolver.
*/
interface DependentsResolverInterface
{
/**
Expand Down
4 changes: 4 additions & 0 deletions src/Violation/Filter/DependencyFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
use Mediact\DependencyGuard\Violation\Violation;
use Mediact\DependencyGuard\Violation\ViolationInterface;

/**
* @deprecated The conceptual difference between dependents in Composer and
* DependencyGuard is too great to rely on the output of a dependents resolver.
*/
class DependencyFilter implements ViolationFilterInterface
{
/** @var ViolationFilterInterface */
Expand Down
82 changes: 82 additions & 0 deletions src/Violation/Filter/PackageRequirementsFilter.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php
/**
* Copyright MediaCT. All rights reserved.
* https://www.mediact.nl
*/

namespace Mediact\DependencyGuard\Violation\Filter;

use Composer\Package\Locker;
use Composer\Package\PackageInterface;
use Mediact\DependencyGuard\Candidate\Candidate;
use Mediact\DependencyGuard\Composer\Locker\PackageRequirementsResolver;
use Mediact\DependencyGuard\Composer\Locker\PackageRequirementsResolverInterface;
use Mediact\DependencyGuard\Violation\Violation;
use Mediact\DependencyGuard\Violation\ViolationInterface;

class PackageRequirementsFilter implements ViolationFilterInterface
{
/** @var Locker */
private $locker;

/** @var ViolationFilterInterface */
private $filter;

/** @var PackageRequirementsResolverInterface */
private $resolver;

/**
* Constructor.
*
* @param Locker $locker
* @param ViolationFilterInterface $filter
* @param PackageRequirementsResolverInterface|null $resolver
*/
public function __construct(
Locker $locker,
ViolationFilterInterface $filter,
PackageRequirementsResolverInterface $resolver = null
) {
$this->locker = $locker;
$this->filter = $filter;
$this->resolver = $resolver ?? new PackageRequirementsResolver();
}

/**
* Filter violations.
*
* @param ViolationInterface $violation
*
* @return bool
*/
public function __invoke(ViolationInterface $violation): bool
{
return array_reduce(
array_map(
function (
PackageInterface $package
) use ($violation): ViolationInterface {
return new Violation(
sprintf(
'Package "%s" provides violating package "%s".',
$package->getName(),
$violation->getPackage()->getName()
),
new Candidate(
$package,
$violation->getSymbols()
)
);
},
$this->resolver->getDependents(
$violation->getPackage()->getName(),
$this->locker
)
),
function (bool $carry, ViolationInterface $violation): bool {
return $carry || $this->filter->__invoke($violation);
},
false
);
}
}
6 changes: 2 additions & 4 deletions src/Violation/Filter/ViolationFilterFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@ public function create(Composer $composer): ViolationFilterInterface

return new ViolationFilterChain(
$chain,
new DependencyFilter(
$composer
->getRepositoryManager()
->getLocalRepository(),
new PackageRequirementsFilter(
$composer->getLocker(),
$chain
)
);
Expand Down
Loading

0 comments on commit 57515c4

Please sign in to comment.