Skip to content
This repository has been archived by the owner on Sep 21, 2023. It is now read-only.

Commit

Permalink
Make file matching more strict (#70)
Browse files Browse the repository at this point in the history
* Make file matching more strict. Do it directly on filenames and wrap each regex with ^ and $

* Only output "For more info" link once per file group

* Set maximum number of steps for the upload progress bar
  • Loading branch information
viktigpetterr authored Feb 9, 2022
1 parent 24eb648 commit 73f3508
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 42 deletions.
46 changes: 23 additions & 23 deletions src/Command/FindAndUploadFilesCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -251,38 +251,37 @@ protected function execute(InputInterface $input, OutputInterface $output): int
$this->setProgressBarStyle($progressBar);

$dependencyFileFormats = DependencyFileFormat::make($dependencyFileFormats);

$numberOfMatchedFiles = 0;
// Find lock files
$lockFileRegexes = \array_merge(...\array_map(fn ($format) => $format->getLockFileRegexes(true), $dependencyFileFormats));
$lockFileFinder = clone $finder;
$lockFileFinder->name($lockFileRegexes);
$lockFileRegexes = \array_merge(...\array_map(fn ($format) => $format->getLockFileRegexes(), $dependencyFileFormats));
$lockFiles = [];
foreach ($lockFileFinder as $file) {
$lockFiles[$file->getPathname()] = $file;
foreach ($finder as $file) {
if (Utility::pregMatchInArray($file->getFilename(), $lockFileRegexes)) {
$lockFiles[$file->getPathname()] = $file;
++$numberOfMatchedFiles;
}
}

/** Find dependency files and create FileGroups(@see FileGroup) */
$dependencyFileRegexes = \array_merge(\array_map(fn ($format) => $format->getRegex(), \array_values($dependencyFileFormats)));
$dependencyFileFinder = clone $finder;
$dependencyFileFinder->name($dependencyFileRegexes);
$fileGroups = [];
foreach ($dependencyFileFinder as $file) {
$dependencyFileFormat = DependencyFileFormat::findFormatByFileName($dependencyFileFormats, $file->getFilename());
$fileGroup = new FileGroup($file, $dependencyFileFormat);
$lockFileRegexes = $dependencyFileFormat->getLockFileRegexes();
// Find matching lock file
foreach ($lockFileRegexes as $lockFileRegex) {
foreach ($lockFiles as $key => $lockFile) {
$quotedLockfilePath = \preg_quote($file->getPath(), '/');
$lockFilePattern = "/$quotedLockfilePath\/$lockFileRegex/";
if (\preg_match($lockFilePattern, $key) === 1) {
$fileGroup->addLockFile($lockFile);
unset($lockFiles[$key]);
break;
foreach ($finder as $file) {
if (($dependencyFileFormat = DependencyFileFormat::findFormatByFileName($dependencyFileFormats, $file->getFilename())) !== null) {
$fileGroup = new FileGroup($file, $dependencyFileFormat);
$lockFileRegexes = $dependencyFileFormat->getLockFileRegexes();
// Find matching lock file
foreach ($lockFileRegexes as $lockFileRegex) {
foreach ($lockFiles as $key => $lockFile) {
$quotedLockfilePath = \preg_quote($file->getPath(), '/');
if (\preg_match("/$quotedLockfilePath\/$lockFileRegex/", $key) === 1) {
$fileGroup->addLockFile($lockFile);
unset($lockFiles[$key]);
break;
}
}
}
$fileGroups[] = $fileGroup;
++$numberOfMatchedFiles;
}
$fileGroups[] = $fileGroup;
}

// Create FileGroups from leftover lock files.
Expand All @@ -293,6 +292,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
}

// Upload FileGroups
$progressBar->setMaxSteps($numberOfMatchedFiles);
foreach ($fileGroups as $fileGroup) {
foreach ($fileGroup->getFiles() as $file) {
try {
Expand Down
21 changes: 10 additions & 11 deletions src/Model/DependencyFileFormat.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@ public static function make(array $formats): array
/**
* @param array<string|int, self> $dependencyFileFormats
*/
public static function findFormatByFileName(array $dependencyFileFormats, string $filename): ?self
public static function findFormatByFileName(array $dependencyFileFormats, string $filename, bool $lockFile = false): ?self
{
foreach ($dependencyFileFormats as $dependencyFileFormat) {
$regexes = $dependencyFileFormat->getRegexes();
if ($lockFile) {
$regexes = $dependencyFileFormat->getLockFileRegexes();
} else {
$regexes = [$dependencyFileFormat->getRegex()];
}
if (Utility::pregMatchInArray($filename, $regexes)) {
return $dependencyFileFormat;
}
Expand All @@ -57,9 +61,9 @@ public function __construct(
/**
* @return string[]
*/
public function getLockFileRegexes(bool $quote = false): array
public function getLockFileRegexes(): array
{
return \array_map(fn ($lockFileRegex) => $quote ? "/$lockFileRegex/" : $lockFileRegex, $this->lockFiles);
return \array_map(fn ($lockFileRegex) => $lockFileRegex, $this->lockFiles);
}

/**
Expand All @@ -72,20 +76,15 @@ public function setLockFilesRegexes(array $lockFiles): void

public function getRegex(): string
{
return "/$this->regex/";
}

public function isLockFileFormat(): bool
{
return empty($this->lockFiles);
return $this->regex;
}

/**
* @return string[] returns format and lock file regexes
*/
public function getRegexes(): array
{
return \array_merge([$this->regex], $this->getLockFileRegexes());
return \array_merge([$this->getRegex()], $this->getLockFileRegexes());
}

public function setRegex(string $regex): void
Expand Down
5 changes: 2 additions & 3 deletions src/Model/FileGroup.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public function addLockFile(SplFileInfo $lockFile): void

/**
* Returns `true` if the dependency file has a lock file. Otherwise `false`.
* If the FileGroup is a lockFileGroup then `true` is always returned.
*/
public function isComplete(): bool
{
Expand All @@ -71,7 +70,7 @@ public function ioPrint(SymfonyStyle $io, string $searchDirectory): void
foreach ($this->lockFiles as $lockFile) {
$lockFileName = \str_replace($searchDirectory, '', $lockFile->getPathname());
$lockFileName = Utility::normaliseRelativePath($lockFileName);
$io->write(" * <fg=green;>$lockFileName</>");
$io->writeln(" * <fg=green;>$lockFileName</>");
}
} else {
$io->text('* <fg=yellow;options=bold>Missing related dependency file(s)!</>');
Expand All @@ -81,8 +80,8 @@ public function ioPrint(SymfonyStyle $io, string $searchDirectory): void
foreach ($this->dependencyFileFormat->getLockFileRegexes() as $lockFileRegex) {
$lockFileName = stripslashes($lockFileRegex);
$io->text("\t* <fg=green;options=bold>$lockFileName</>");
$io->writeln(' For more info: <fg=blue;options=bold>https://debricked.com/docs/language-support</>');
}
$io->writeln(' For more info: <fg=blue;options=bold>https://debricked.com/docs/language-support</>');
}
}
$io->newLine(2);
Expand Down
2 changes: 1 addition & 1 deletion src/Utility/Utility.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static function pregMatchInArray(string $stringToMatch, array $arrayOfReg
return \array_reduce(
$arrayOfRegexes,
function ($matchExists, $regex) use ($stringToMatch) {
return $matchExists || \preg_match('/^'.$regex.'$/', $stringToMatch);
return $matchExists || \preg_match("/^$regex$/", $stringToMatch);
},
false);
}
Expand Down
9 changes: 5 additions & 4 deletions tests/Command/FindAndUploadFilesCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,10 @@ private function setUpReal(): void
$this->commandTester = new CommandTester($this->command);
}

public const FORMATS_JSON_STRING = <<<'EOD'
[{"regex":"advenica_format\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"apk\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"apt\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"axis_packages\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"((?!WORKSPACE|BUILD)).*(?:\\.bazel)","documentationUrl":null,"lockFileRegexes":[]},{"regex":"((?!WORKSPACE|BUILD)).*(?:\\.bzl)","documentationUrl":null,"lockFileRegexes":[]},{"regex":".*_install\\.json","documentationUrl":null,"lockFileRegexes":[]},{"regex":"WORKSPACE\\.bazel","documentationUrl":null,"lockFileRegexes":[]},{"regex":"WORKSPACE\\.bzl","documentationUrl":null,"lockFileRegexes":[]},{"regex":"WORKSPACE","documentationUrl":null,"lockFileRegexes":[]},{"regex":"bitbake_installed_packages\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"\\.manifest","documentationUrl":null,"lockFileRegexes":[]},{"regex":"debian_descriptions\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"mix\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"NaNaNa.batman","documentationUrl":null,"lockFileRegexes":[]},{"regex":"flatpak\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"go\\.sum","documentationUrl":null,"lockFileRegexes":["go\\.mod"]},{"regex":"build\\.gradle","documentationUrl":null,"lockFileRegexes":["\\.debricked-gradle-dependencies\\.txt"]},{"regex":"build\\.gradle\\.kts","documentationUrl":null,"lockFileRegexes":["\\.debricked-gradle-dependencies\\.txt"]},{"regex":"pom\\.xml","documentationUrl":null,"lockFileRegexes":["\\.debricked-maven-dependencies\\.tgf"]},{"regex":"bower\\.json","documentationUrl":null,"lockFileRegexes":[]},{"regex":"package\\.json","documentationUrl":null,"lockFileRegexes":["package-lock\\.json","yarn\\.lock"]},{"regex":"npm-shrinkwrap\\.json","documentationUrl":null,"lockFileRegexes":[]},{"regex":".*(?:\\.csproj)","documentationUrl":null,"lockFileRegexes":["packages\\.lock\\.json"]},{"regex":"packages\\.config","documentationUrl":null,"lockFileRegexes":[]},{"regex":"pacman\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"paket\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"composer\\.json","documentationUrl":null,"lockFileRegexes":["composer\\.lock"]},{"regex":"requirements.*(?:\\.txt)","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Pipfile","documentationUrl":null,"lockFileRegexes":["Pipfile\\.lock"]},{"regex":"manifest_rev_revisions\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"manifest_revisions\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"rootfs_manifest\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"rpm\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"snap\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"\\.debricked-call-graph","documentationUrl":null,"lockFileRegexes":[]},{"regex":"\\.debricked-wfp-fingerprints\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Gopkg\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Gemfile\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Cargo\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Podfile\\.lock","documentationUrl":null,"lockFileRegexes":[]}]
EOD;

private function setUpMocks(bool $expectAccessToken = false): void
{
$ciUploadId = null;
Expand Down Expand Up @@ -645,10 +649,7 @@ private function setUpMocks(bool $expectAccessToken = false): void
} elseif (\strpos($url, '/api/1.0/open/finishes/dependencies/files/uploads') !== false) {
return new MockResponse('', ['http_code' => 204]);
} elseif (\strpos($url, '/api/1.0/open/files/supported-formats') !== false) {
return new MockResponse(<<<'EOD'
[{"regex":"advenica_format\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"apk\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"apt\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"axis_packages\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"((?!WORKSPACE|BUILD)).*(?:\\.bazel)","documentationUrl":null,"lockFileRegexes":[]},{"regex":"((?!WORKSPACE|BUILD)).*(?:\\.bzl)","documentationUrl":null,"lockFileRegexes":[]},{"regex":".*_install\\.json","documentationUrl":null,"lockFileRegexes":[]},{"regex":"WORKSPACE\\.bazel","documentationUrl":null,"lockFileRegexes":[]},{"regex":"WORKSPACE\\.bzl","documentationUrl":null,"lockFileRegexes":[]},{"regex":"WORKSPACE","documentationUrl":null,"lockFileRegexes":[]},{"regex":"bitbake_installed_packages\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"\\.manifest","documentationUrl":null,"lockFileRegexes":[]},{"regex":"debian_descriptions\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"mix\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"NaNaNa.batman","documentationUrl":null,"lockFileRegexes":[]},{"regex":"flatpak\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"go\\.sum","documentationUrl":null,"lockFileRegexes":["go\\.mod"]},{"regex":"build\\.gradle","documentationUrl":null,"lockFileRegexes":["\\.debricked-gradle-dependencies\\.txt"]},{"regex":"build\\.gradle\\.kts","documentationUrl":null,"lockFileRegexes":["\\.debricked-gradle-dependencies\\.txt"]},{"regex":"pom\\.xml","documentationUrl":null,"lockFileRegexes":["\\.debricked-maven-dependencies\\.tgf"]},{"regex":"bower\\.json","documentationUrl":null,"lockFileRegexes":[]},{"regex":"package\\.json","documentationUrl":null,"lockFileRegexes":["package-lock\\.json","yarn\\.lock"]},{"regex":"npm-shrinkwrap\\.json","documentationUrl":null,"lockFileRegexes":[]},{"regex":".*(?:\\.csproj)","documentationUrl":null,"lockFileRegexes":["packages\\.lock\\.json"]},{"regex":"packages\\.config","documentationUrl":null,"lockFileRegexes":[]},{"regex":"pacman\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"paket\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"composer\\.json","documentationUrl":null,"lockFileRegexes":["composer\\.lock"]},{"regex":"requirements.*(?:\\.txt)","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Pipfile","documentationUrl":null,"lockFileRegexes":["Pipfile\\.lock"]},{"regex":"manifest_rev_revisions\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"manifest_revisions\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"rootfs_manifest\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"rpm\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"snap\\.list","documentationUrl":null,"lockFileRegexes":[]},{"regex":"\\.debricked-call-graph","documentationUrl":null,"lockFileRegexes":[]},{"regex":"\\.debricked-wfp-fingerprints\\.txt","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Gopkg\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Gemfile\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Cargo\\.lock","documentationUrl":null,"lockFileRegexes":[]},{"regex":"Podfile\\.lock","documentationUrl":null,"lockFileRegexes":[]}]
EOD
);
return new MockResponse(self::FORMATS_JSON_STRING);
} else {
return new MockResponse('', ['http_code' => 404]);
}
Expand Down
40 changes: 40 additions & 0 deletions tests/Model/DependencyFileFormatTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

namespace App\Tests\Model;

use App\Model\DependencyFileFormat;
use App\Tests\Command\FindAndUploadFilesCommandTest;
use PHPUnit\Framework\TestCase;

class DependencyFileFormatTest extends TestCase
{
public function testMake(): void
{
$this->assertIsArray(DependencyFileFormat::make(self::getFormats()));
}

public function testFindFormatByFileName(): void
{
$formats = DependencyFileFormat::make(self::getFormats());
$this->assertNull(DependencyFileFormat::findFormatByFileName($formats, 'test'));
// Test valid file
$fileName = 'package.json';
$format = DependencyFileFormat::findFormatByFileName($formats, $fileName);
$this->assertInstanceOf(DependencyFileFormat::class, $format);
$this->assertMatchesRegularExpression("/^{$format->getRegex()}$/", $fileName);
// Test match Lock file
$this->assertNull(DependencyFileFormat::findFormatByFileName($formats, $fileName, true));
$lockFileName = 'yarn.lock';
$format = DependencyFileFormat::findFormatByFileName($formats, $lockFileName, true);
$this->assertInstanceOf(DependencyFileFormat::class, $format);
$this->assertMatchesRegularExpression("/^{$format->getRegex()}$/", $fileName);

// Test semi-valid file
$this->assertNull(DependencyFileFormat::findFormatByFileName($formats, 'dev-package.json'));
}

private static function getFormats(): array
{
return \json_decode(FindAndUploadFilesCommandTest::FORMATS_JSON_STRING, true);
}
}
21 changes: 21 additions & 0 deletions tests/Utility/UtilityTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace App\Tests\Utility;

use App\Utility\Utility;
use PHPUnit\Framework\TestCase;

class UtilityTest extends TestCase
{
public function testPregMatchInArray(): void
{
// Test exact match
$this->assertTrue(Utility::pregMatchInArray('test', ['no-match', 'test']));
// Test no match with prefix
$this->assertFalse(Utility::pregMatchInArray('dev-test', ['no-match', 'test']));
// Test no match with postfix
$this->assertFalse(Utility::pregMatchInArray('test-dev', ['no-match', 'test']));
// Test no match
$this->assertFalse(Utility::pregMatchInArray('test', ['no-match']));
}
}

0 comments on commit 73f3508

Please sign in to comment.