OilfieldToolsNet/shlink
4
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2024-12-17 21:00:41 -06:00
Code Issues Packages Projects Releases Wiki Activity

Fix GeolocationDbUpdater test

Browse Source
This commit is contained in:
Alejandro Celaya 2024-12-16 19:50:06 +01:00
parent e715a0fb6f
commit 509ef668e6
4 changed files with 161 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
CHANGELOG.md
View File

@ -15,13 +15,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
This option effectively replaces the old `REDIRECT_APPEND_EXTRA_PATH` option, which is now deprecated and will be removed in Shlink 5.0.0
### Changed
* * [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4
* [#2281](https://github.com/shlinkio/shlink/issues/2281) Update docker image to PHP 8.4
* [#2124](https://github.com/shlinkio/shlink/issues/2124) Improve how Shlink decides if a GeoLite db file needs to be downloaded, and reduces the chances for API limits to be reached.
Now Shlink tracks all download attempts, and knows which of them failed and succeeded. This lets it know when was the last error or success, how many consecutive errors have happened, etc.
It also tracks now the reason for a download to be attempted, and the error that happened when one fails.
### Deprecated
* *Nothing*
### Removed
* * [#2247](https://github.com/shlinkio/shlink/issues/2247) Drop support for PHP 8.2
* [#2247](https://github.com/shlinkio/shlink/issues/2247) Drop support for PHP 8.2
### Fixed
* *Nothing*

15
module/Core/src/Geolocation/Entity/GeolocationDbUpdate.php
View File

@ -6,14 +6,15 @@ namespace Shlinkio\Shlink\Core\Geolocation\Entity;
use Cake\Chronos\Chronos;
use Shlinkio\Shlink\Common\Entity\AbstractEntity;
use Shlinkio\Shlink\Core\Exception\RuntimeException;
use function stat;
class GeolocationDbUpdate extends AbstractEntity
{
private function __construct(
public readonly string $reason,
private readonly string $filesystemId,
private readonly string $reason,
private GeolocationDbUpdateStatus $status = GeolocationDbUpdateStatus::IN_PROGRESS,
private readonly Chronos $dateCreated = new Chronos(),
private Chronos $dateUpdated = new Chronos(),
@ -21,32 +22,34 @@ class GeolocationDbUpdate extends AbstractEntity
) {
}
public static function withReason(string $reason, string|null $filesystemId = null): self
public static function withReason(string $reason): self
{
return new self($reason, $filesystemId ?? self::currentFilesystemId());
return new self($reason, self::currentFilesystemId());
}
public static function currentFilesystemId(): string
{
$system = stat(__FILE__);
if (! $system) {
// TODO Throw error
throw new RuntimeException('It was not possible to resolve filesystem ID via stat function');
}
return (string) $system['dev'];
}
public function finishSuccessfully(): void
public function finishSuccessfully(): self
{
$this->dateUpdated = Chronos::now();
$this->status = GeolocationDbUpdateStatus::SUCCESS;
return $this;
}
public function finishWithError(string $error): void
public function finishWithError(string $error): self
{
$this->error = $error;
$this->dateUpdated = Chronos::now();
$this->status = GeolocationDbUpdateStatus::ERROR;
return $this;
}
/**

4
module/Core/src/Geolocation/GeolocationDbUpdater.php
View File

@ -95,9 +95,9 @@ readonly class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
// - Most recent attempt is older than 30 days (and implicitly, successful)
$reasonMatch = match (true) {
$mostRecentDownload === null => [false, 'No download attempts tracked for this instance'],
$this->dbUpdater->databaseFileExists() => [false, 'Geolocation db file does not exist'],
! $this->dbUpdater->databaseFileExists() => [false, 'Geolocation db file does not exist'],
$lastAttemptIsError => [true, 'Max consecutive errors not reached'],
$mostRecentDownload->isOlderThan(days: 30) => [true, 'Last successful attempt'],
$mostRecentDownload->isOlderThan(days: 30) => [true, 'Last successful attempt is old enough'],
default => null,
};
if ($reasonMatch !== null) {

202
module/Core/test/Geolocation/GeolocationDbUpdaterTest.php
View File

@ -6,14 +6,16 @@ namespace ShlinkioTest\Shlink\Core\Geolocation;
use Cake\Chronos\Chronos;
use Closure;
use GeoIp2\Database\Reader;
use MaxMind\Db\Reader\Metadata;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\EntityRepository;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Test;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
use RuntimeException;
use Shlinkio\Shlink\Core\Config\Options\TrackingOptions;
use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException;
use Shlinkio\Shlink\Core\Geolocation\Entity\GeolocationDbUpdate;
use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdater;
use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface;
use Shlinkio\Shlink\Core\Geolocation\GeolocationResult;
@ -29,17 +31,24 @@ use function range;
class GeolocationDbUpdaterTest extends TestCase
{
private MockObject & DbUpdaterInterface $dbUpdater;
private MockObject & Reader $geoLiteDbReader;
private MockObject & Lock\LockInterface $lock;
private MockObject & EntityManagerInterface $em;
/** @var MockObject&EntityRepository<GeolocationDbUpdate> */
private MockObject & EntityRepository $repo;
/** @var GeolocationDownloadProgressHandlerInterface&object{beforeDownloadCalled: bool, handleProgressCalled: bool} */
private GeolocationDownloadProgressHandlerInterface $progressHandler;
protected function setUp(): void
{
$this->dbUpdater = $this->createMock(DbUpdaterInterface::class);
$this->geoLiteDbReader = $this->createMock(Reader::class);
$this->lock = $this->createMock(Lock\SharedLockInterface::class);
$this->lock->method('acquire')->with($this->isTrue())->willReturn(true);
$this->em = $this->createMock(EntityManagerInterface::class);
$this->repo = $this->createMock(EntityRepository::class);
$this->em->method('getRepository')->willReturn($this->repo);
$this->progressHandler = new class implements GeolocationDownloadProgressHandlerInterface {
public function __construct(
public bool $beforeDownloadCalled = false,
@ -59,6 +68,32 @@ class GeolocationDbUpdaterTest extends TestCase
};
}
#[Test]
public function properResultIsReturnedIfMostRecentUpdateIsInProgress(): void
{
$this->repo->expects($this->once())->method('findBy')->willReturn([GeolocationDbUpdate::withReason('')]);
$this->dbUpdater->expects($this->never())->method('databaseFileExists');
$result = $this->geolocationDbUpdater()->checkDbUpdate();
self::assertEquals(GeolocationResult::UPDATE_IN_PROGRESS, $result);
}
#[Test]
public function properResultIsReturnedIfMaxConsecutiveErrorsAreReached(): void
{
$this->repo->expects($this->once())->method('findBy')->willReturn([
GeolocationDbUpdate::withReason('')->finishWithError(''),
GeolocationDbUpdate::withReason('')->finishWithError(''),
GeolocationDbUpdate::withReason('')->finishWithError(''),
]);
$this->dbUpdater->expects($this->never())->method('databaseFileExists');
$result = $this->geolocationDbUpdater()->checkDbUpdate();
self::assertEquals(GeolocationResult::MAX_ERRORS_REACHED, $result);
}
#[Test]
public function properResultIsReturnedWhenLicenseIsMissing(): void
{
@ -66,7 +101,9 @@ class GeolocationDbUpdaterTest extends TestCase
$this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->willThrowException(
new MissingLicenseException(''),
);
$this->geoLiteDbReader->expects($this->never())->method('metadata');
$this->repo->expects($this->once())->method('findBy')->willReturn([
GeolocationDbUpdate::withReason('')->finishSuccessfully(),
]);
$result = $this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler);
@ -74,16 +111,19 @@ class GeolocationDbUpdaterTest extends TestCase
self::assertEquals(GeolocationResult::LICENSE_MISSING, $result);
}
#[Test]
public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void
#[Test, DataProvider('provideDbDoesNotExist')]
public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(Closure $setUp): void
{
$prev = new DbUpdateException('');
$this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false);
$expectedReason = $setUp($this);
$this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with(
$this->isInstanceOf(Closure::class),
)->willThrowException($prev);
$this->geoLiteDbReader->expects($this->never())->method('metadata');
$this->em->expects($this->once())->method('persist')->with($this->callback(
fn (GeolocationDbUpdate $newUpdate): bool => $newUpdate->reason === $expectedReason,
));
try {
$this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler);
@ -96,17 +136,31 @@ class GeolocationDbUpdaterTest extends TestCase
}
}
public static function provideDbDoesNotExist(): iterable
{
yield 'file does not exist' => [function (self $test): string {
$test->repo->expects($test->once())->method('findBy')->willReturn([
GeolocationDbUpdate::withReason('')->finishSuccessfully(),
]);
$test->dbUpdater->expects($test->once())->method('databaseFileExists')->willReturn(false);
return 'Geolocation db file does not exist';
}];
yield 'no attempts' => [function (self $test): string {
$test->repo->expects($test->once())->method('findBy')->willReturn([]);
$test->dbUpdater->expects($test->never())->method('databaseFileExists');
return 'No download attempts tracked for this instance';
}];
}
#[Test, DataProvider('provideBigDays')]
public function exceptionIsThrownWhenOlderDbIsTooOldAndDownloadFails(int $days): void
public function exceptionIsThrownWhenOlderDbIsOldEnoughAndDownloadFails(int $days): void
{
$prev = new DbUpdateException('');
$this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true);
$this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with(
$this->isInstanceOf(Closure::class),
)->willThrowException($prev);
$this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn(
$this->buildMetaWithBuildEpoch(Chronos::now()->subDays($days)->getTimestamp()),
);
$this->repo->expects($this->once())->method('findBy')->willReturn([self::createFinishedOldUpdate($days)]);
try {
$this->geolocationDbUpdater()->checkDbUpdate();
@ -120,74 +174,109 @@ class GeolocationDbUpdaterTest extends TestCase
public static function provideBigDays(): iterable
{
yield [36];
yield [31];
yield [50];
yield [75];
yield [100];
}
#[Test, DataProvider('provideSmallDays')]
public function databaseIsNotUpdatedIfItIsNewEnough(string|int $buildEpoch): void
#[Test]
public function exceptionIsThrownWhenUnknownErrorHappens(): void
{
$this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->with(
$this->isInstanceOf(Closure::class),
)->willThrowException(new RuntimeException('An error occurred'));
$newUpdate = null;
$this->em->expects($this->once())->method('persist')->with($this->callback(
function (GeolocationDbUpdate $u) use (&$newUpdate): bool {
$newUpdate = $u;
return true;
},
));
try {
$this->geolocationDbUpdater()->checkDbUpdate($this->progressHandler);
self::fail();
} catch (Throwable) {
}
self::assertTrue($this->progressHandler->beforeDownloadCalled);
self::assertNotNull($newUpdate);
self::assertTrue($newUpdate->isError());
}
#[Test, DataProvider('provideNotAldEnoughDays')]
public function databaseIsNotUpdatedIfItIsNewEnough(int $days): void
{
$this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true);
$this->dbUpdater->expects($this->never())->method('downloadFreshCopy');
$this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn(
$this->buildMetaWithBuildEpoch($buildEpoch),
);
$this->repo->expects($this->once())->method('findBy')->willReturn([self::createFinishedOldUpdate($days)]);
$result = $this->geolocationDbUpdater()->checkDbUpdate();
self::assertEquals(GeolocationResult::DB_IS_UP_TO_DATE, $result);
}
public static function provideSmallDays(): iterable
public static function provideNotAldEnoughDays(): iterable
{
$generateParamsWithTimestamp = static function (int $days) {
$timestamp = Chronos::now()->subDays($days)->getTimestamp();
return [$days % 2 === 0 ? $timestamp : (string) $timestamp];
};
return array_map($generateParamsWithTimestamp, range(0, 34));
return array_map(static fn (int $value) => [$value], range(0, 29));
}
#[Test]
public function exceptionIsThrownWhenCheckingExistingDatabaseWithInvalidBuildEpoch(): void
{
$this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(true);
$this->dbUpdater->expects($this->never())->method('downloadFreshCopy');
$this->geoLiteDbReader->expects($this->once())->method('metadata')->with()->willReturn(
$this->buildMetaWithBuildEpoch('invalid'),
);
#[Test, DataProvider('provideUpdatesThatWillDownload')]
public function properResultIsReturnedWhenDownloadSucceeds(
array $updates,
GeolocationResult $expectedResult,
string $expectedReason,
): void {
$this->repo->expects($this->once())->method('findBy')->willReturn($updates);
$this->dbUpdater->method('databaseFileExists')->willReturn(true);
$this->dbUpdater->expects($this->once())->method('downloadFreshCopy');
$this->em->expects($this->once())->method('persist')->with($this->callback(
fn (GeolocationDbUpdate $newUpdate): bool => $newUpdate->reason === $expectedReason,
));
$this->expectException(GeolocationDbUpdateFailedException::class);
$this->expectExceptionMessage(
'Build epoch with value "invalid" from existing geolocation database, could not be parsed to integer.',
);
$result = $this->geolocationDbUpdater()->checkDbUpdate();
$this->geolocationDbUpdater()->checkDbUpdate();
self::assertEquals($expectedResult, $result);
}
private function buildMetaWithBuildEpoch(string|int $buildEpoch): Metadata
public static function provideUpdatesThatWillDownload(): iterable
{
return new Metadata([
'binary_format_major_version' => '',
'binary_format_minor_version' => '',
'build_epoch' => $buildEpoch,
'database_type' => '',
'languages' => '',
'description' => '',
'ip_version' => '',
'node_count' => 1,
'record_size' => 4,
]);
yield 'no updates' => [[], GeolocationResult::DB_CREATED, 'No download attempts tracked for this instance'];
yield 'old successful update' => [
[self::createFinishedOldUpdate(days: 31)],
GeolocationResult::DB_UPDATED,
'Last successful attempt is old enough',
];
yield 'not enough errors' => [
[self::createFinishedOldUpdate(days: 3, successful: false)],
GeolocationResult::DB_UPDATED,
'Max consecutive errors not reached',
];
}
public static function createFinishedOldUpdate(int $days, bool $successful = true): GeolocationDbUpdate
{
Chronos::setTestNow(Chronos::now()->subDays($days));
$update = GeolocationDbUpdate::withReason('');
if ($successful) {
$update->finishSuccessfully();
} else {
$update->finishWithError('');
}
Chronos::setTestNow();
return $update;
}
#[Test, DataProvider('provideTrackingOptions')]
public function downloadDbIsSkippedIfTrackingIsDisabled(TrackingOptions $options): void
{
$result = $this->geolocationDbUpdater($options)->checkDbUpdate();
$this->dbUpdater->expects($this->never())->method('databaseFileExists');
$this->geoLiteDbReader->expects($this->never())->method('metadata');
$this->em->expects($this->never())->method('getRepository');
$result = $this->geolocationDbUpdater($options)->checkDbUpdate();
self::assertEquals(GeolocationResult::CHECK_SKIPPED, $result);
}
@ -204,11 +293,6 @@ class GeolocationDbUpdaterTest extends TestCase
$locker = $this->createMock(Lock\LockFactory::class);
$locker->method('createLock')->with($this->isType('string'))->willReturn($this->lock);
return new GeolocationDbUpdater(
$this->dbUpdater,
fn () => $this->geoLiteDbReader,
$locker,
$options ?? new TrackingOptions(),
);
return new GeolocationDbUpdater($this->dbUpdater, $locker, $options ?? new TrackingOptions(), $this->em, 3);
}
}