Merge pull request #1005 from acelaya-forks/feature/fix-string-epoch

Feature/fix string epoch
This commit is contained in:
Alejandro Celaya 2021-02-06 22:23:09 +01:00 committed by GitHub
commit b54350674c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 153 additions and 43 deletions

View File

@ -34,6 +34,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
### Fixed
* [#988](https://github.com/shlinkio/shlink/issues/988) Fixed serving zero-byte static files in apache and apache-compatible web servers.
* [#990](https://github.com/shlinkio/shlink/issues/990) Fixed short URLs not properly composed in REST API endpoints when both custom domain and custom base path are used.
* [#1002](https://github.com/shlinkio/shlink/issues/1002) Fixed weird behavior in which GeoLite2 metadata's `buildEpoch` is parsed as string instead of int.
## [2.5.2] - 2021-01-24

View File

@ -7,18 +7,46 @@ namespace Shlinkio\Shlink\CLI\Exception;
use RuntimeException;
use Throwable;
use function sprintf;
class GeolocationDbUpdateFailedException extends RuntimeException implements ExceptionInterface
{
private bool $olderDbExists;
public static function create(bool $olderDbExists, ?Throwable $prev = null): self
public static function withOlderDb(?Throwable $prev = null): self
{
$e = new self(
'An error occurred while updating geolocation database, and an older version could not be found',
'An error occurred while updating geolocation database, but an older DB is already present.',
0,
$prev,
);
$e->olderDbExists = $olderDbExists;
$e->olderDbExists = true;
return $e;
}
public static function withoutOlderDb(?Throwable $prev = null): self
{
$e = new self(
'An error occurred while updating geolocation database, and an older version could not be found.',
0,
$prev,
);
$e->olderDbExists = false;
return $e;
}
/**
* @param mixed $buildEpoch
*/
public static function withInvalidEpochInOldDb($buildEpoch): self
{
$e = new self(sprintf(
'Build epoch with value "%s" from existing geolocation database, could not be parsed to integer.',
$buildEpoch,
));
$e->olderDbExists = true;
return $e;
}

View File

@ -6,11 +6,14 @@ namespace Shlinkio\Shlink\CLI\Util;
use Cake\Chronos\Chronos;
use GeoIp2\Database\Reader;
use MaxMind\Db\Reader\Metadata;
use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
use Shlinkio\Shlink\IpGeolocation\Exception\RuntimeException;
use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
use Symfony\Component\Lock\LockFactory;
use function is_int;
class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
{
private const LOCK_NAME = 'geolocation-db-update';
@ -52,7 +55,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
}
$meta = $this->geoLiteDbReader->metadata();
if ($this->buildIsTooOld($meta->buildEpoch)) {
if ($this->buildIsTooOld($meta)) {
$this->downloadNewDb(true, $mustBeUpdated, $handleProgress);
}
}
@ -69,14 +72,37 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
try {
$this->dbUpdater->downloadFreshCopy($handleProgress);
} catch (RuntimeException $e) {
throw GeolocationDbUpdateFailedException::create($olderDbExists, $e);
throw $olderDbExists
? GeolocationDbUpdateFailedException::withOlderDb($e)
: GeolocationDbUpdateFailedException::withoutOlderDb($e);
}
}
private function buildIsTooOld(int $buildTimestamp): bool
private function buildIsTooOld(Metadata $meta): bool
{
$buildTimestamp = $this->resolveBuildTimestamp($meta);
$buildDate = Chronos::createFromTimestamp($buildTimestamp);
$now = Chronos::now();
return $now->gt($buildDate->addDays(35));
}
private function resolveBuildTimestamp(Metadata $meta): int
{
// In theory the buildEpoch should be an int, but it has been reported to come as a string.
// See https://github.com/shlinkio/shlink/issues/1002 for context
/** @var int|string $buildEpoch */
$buildEpoch = $meta->buildEpoch;
if (is_int($buildEpoch)) {
return $buildEpoch;
}
$intBuildEpoch = (int) $buildEpoch;
if ($buildEpoch === (string) $intBuildEpoch) {
return $intBuildEpoch;
}
throw GeolocationDbUpdateFailedException::withInvalidEpochInOldDb($buildEpoch);
}
}

View File

@ -217,7 +217,9 @@ class LocateVisitsCommandTest extends TestCase
$mustBeUpdated($olderDbExists);
$handleProgress(100, 50);
throw GeolocationDbUpdateFailedException::create($olderDbExists);
throw $olderDbExists
? GeolocationDbUpdateFailedException::withOlderDb()
: GeolocationDbUpdateFailedException::withoutOlderDb();
},
);

View File

@ -14,26 +14,54 @@ class GeolocationDbUpdateFailedExceptionTest extends TestCase
{
/**
* @test
* @dataProvider provideCreateArgs
* @dataProvider providePrev
*/
public function createBuildsException(bool $olderDbExists, ?Throwable $prev): void
public function withOlderDbBuildsException(?Throwable $prev): void
{
$e = GeolocationDbUpdateFailedException::create($olderDbExists, $prev);
$e = GeolocationDbUpdateFailedException::withOlderDb($prev);
self::assertEquals($olderDbExists, $e->olderDbExists());
self::assertTrue($e->olderDbExists());
self::assertEquals(
'An error occurred while updating geolocation database, and an older version could not be found',
'An error occurred while updating geolocation database, but an older DB is already present.',
$e->getMessage(),
);
self::assertEquals(0, $e->getCode());
self::assertEquals($prev, $e->getPrevious());
}
public function provideCreateArgs(): iterable
/**
* @test
* @dataProvider providePrev
*/
public function withoutOlderDbBuildsException(?Throwable $prev): void
{
yield 'older DB and no prev' => [true, null];
yield 'older DB and prev' => [true, new RuntimeException('prev')];
yield 'no older DB and no prev' => [false, null];
yield 'no older DB and prev' => [false, new Exception('prev')];
$e = GeolocationDbUpdateFailedException::withoutOlderDb($prev);
self::assertFalse($e->olderDbExists());
self::assertEquals(
'An error occurred while updating geolocation database, and an older version could not be found.',
$e->getMessage(),
);
self::assertEquals(0, $e->getCode());
self::assertEquals($prev, $e->getPrevious());
}
public function providePrev(): iterable
{
yield 'no prev' => [null];
yield 'RuntimeException' => [new RuntimeException('prev')];
yield 'Exception' => [new Exception('prev')];
}
/** @test */
public function withInvalidEpochInOldDbBuildsException(): void
{
$e = GeolocationDbUpdateFailedException::withInvalidEpochInOldDb('foobar');
self::assertTrue($e->olderDbExists());
self::assertEquals(
'Build epoch with value "foobar" from existing geolocation database, could not be parsed to integer.',
$e->getMessage(),
);
}
}

View File

@ -80,17 +80,9 @@ class GeolocationDbUpdaterTest extends TestCase
public function exceptionIsThrownWhenOlderDbIsTooOldAndDownloadFails(int $days): void
{
$fileExists = $this->dbUpdater->databaseFileExists()->willReturn(true);
$getMeta = $this->geoLiteDbReader->metadata()->willReturn(new Metadata([
'binary_format_major_version' => '',
'binary_format_minor_version' => '',
'build_epoch' => Chronos::now()->subDays($days)->getTimestamp(),
'database_type' => '',
'languages' => '',
'description' => '',
'ip_version' => '',
'node_count' => 1,
'record_size' => 4,
]));
$getMeta = $this->geoLiteDbReader->metadata()->willReturn($this->buildMetaWithBuildEpoch(
Chronos::now()->subDays($days)->getTimestamp(),
));
$prev = new RuntimeException('');
$download = $this->dbUpdater->downloadFreshCopy(null)->willThrow($prev);
@ -120,21 +112,12 @@ class GeolocationDbUpdaterTest extends TestCase
/**
* @test
* @dataProvider provideSmallDays
* @param string|int $buildEpoch
*/
public function databaseIsNotUpdatedIfItIsYoungerThanOneWeek(int $days): void
public function databaseIsNotUpdatedIfItIsYoungerThanOneWeek($buildEpoch): void
{
$fileExists = $this->dbUpdater->databaseFileExists()->willReturn(true);
$getMeta = $this->geoLiteDbReader->metadata()->willReturn(new Metadata([
'binary_format_major_version' => '',
'binary_format_minor_version' => '',
'build_epoch' => Chronos::now()->subDays($days)->getTimestamp(),
'database_type' => '',
'languages' => '',
'description' => '',
'ip_version' => '',
'node_count' => 1,
'record_size' => 4,
]));
$getMeta = $this->geoLiteDbReader->metadata()->willReturn($this->buildMetaWithBuildEpoch($buildEpoch));
$download = $this->dbUpdater->downloadFreshCopy(null)->will(function (): void {
});
@ -147,6 +130,48 @@ class GeolocationDbUpdaterTest extends TestCase
public function provideSmallDays(): iterable
{
return map(range(0, 34), fn (int $days) => [$days]);
$generateParamsWithTimestamp = static function (int $days) {
$timestamp = Chronos::now()->subDays($days)->getTimestamp();
return [$days % 2 === 0 ? $timestamp : (string) $timestamp];
};
return map(range(0, 34), $generateParamsWithTimestamp);
}
/** @test */
public function exceptionIsThrownWhenCheckingExistingDatabaseWithInvalidBuildEpoch(): void
{
$fileExists = $this->dbUpdater->databaseFileExists()->willReturn(true);
$getMeta = $this->geoLiteDbReader->metadata()->willReturn($this->buildMetaWithBuildEpoch('invalid'));
$download = $this->dbUpdater->downloadFreshCopy(null)->will(function (): void {
});
$this->expectException(GeolocationDbUpdateFailedException::class);
$this->expectExceptionMessage(
'Build epoch with value "invalid" from existing geolocation database, could not be parsed to integer.',
);
$fileExists->shouldBeCalledOnce();
$getMeta->shouldBeCalledOnce();
$download->shouldNotBeCalled();
$this->geolocationDbUpdater->checkDbUpdate();
}
/**
* @param string|int $buildEpoch
*/
private function buildMetaWithBuildEpoch($buildEpoch): Metadata
{
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,
]);
}
}

View File

@ -169,7 +169,7 @@ class LocateShortUrlVisitTest extends TestCase
/** @test */
public function errorWhenUpdatingGeoLiteWithExistingCopyLogsWarning(): void
{
$e = GeolocationDbUpdateFailedException::create(true);
$e = GeolocationDbUpdateFailedException::withOlderDb();
$ipAddr = '1.2.3.0';
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr));
$location = new Location('', '', '', '', 0.0, 0.0, '');
@ -200,7 +200,7 @@ class LocateShortUrlVisitTest extends TestCase
/** @test */
public function errorWhenDownloadingGeoLiteCancelsLocation(): void
{
$e = GeolocationDbUpdateFailedException::create(false);
$e = GeolocationDbUpdateFailedException::withoutOlderDb();
$ipAddr = '1.2.3.0';
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr));
$location = new Location('', '', '', '', 0.0, 0.0, '');