Merge pull request #421 from acelaya/bugfix/db-reader-proxy

Bugfix/db reader proxy
This commit is contained in:
Alejandro Celaya 2019-07-23 22:27:40 +02:00 committed by GitHub
commit 22630c7656
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 119 additions and 40 deletions

View file

@ -47,7 +47,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
#### Fixed
* *Nothing*
* [#416](https://github.com/shlinkio/shlink/issues/416) Fixed error thrown when trying to locate visits after the GeoLite2 DB is downloaded for the first time.
## 1.17.0 - 2019-05-13

View file

@ -115,15 +115,18 @@
"test:ci": [
"@test:unit:ci",
"@test:db",
"@test:db:mysql",
"@test:db:postgres",
"@test:api"
],
"test:unit": "phpdbg -qrr vendor/bin/phpunit --order-by=random --coverage-php build/coverage-unit.cov --testdox",
"test:unit:ci": "phpdbg -qrr vendor/bin/phpunit --order-by=random --coverage-php build/coverage-unit.cov --coverage-clover=build/clover.xml --coverage-xml=build/coverage-xml --log-junit=build/phpunit.junit.xml --testdox",
"test:db": "APP_ENV=test phpdbg -qrr vendor/bin/phpunit --order-by=random -c phpunit-db.xml --coverage-php build/coverage-db.cov --testdox",
"test:db:mysql": "DB_DRIVER=mysql composer test:db",
"test:db:postgres": "DB_DRIVER=postgres composer test:db",
"test:db": [
"@test:db:sqlite",
"@test:db:mysql",
"@test:db:postgres"
],
"test:db:sqlite": "APP_ENV=test phpdbg -qrr vendor/bin/phpunit --order-by=random -c phpunit-db.xml --coverage-php build/coverage-db.cov --testdox",
"test:db:mysql": "DB_DRIVER=mysql composer test:db:sqlite",
"test:db:postgres": "DB_DRIVER=postgres composer test:db:sqlite",
"test:api": "bin/test/run-api-tests.sh",
"test:pretty": [
@ -150,7 +153,8 @@
"test:ci": "<fg=blue;options=bold>Runs all test suites, generating all needed reports and logs for CI envs</>",
"test:unit": "<fg=blue;options=bold>Runs unit test suites</>",
"test:unit:ci": "<fg=blue;options=bold>Runs unit test suites, generating all needed reports and logs for CI envs</>",
"test:db": "<fg=blue;options=bold>Runs database test suites on a SQLite database</>",
"test:db": "<fg=blue;options=bold>Runs database test suites on a SQLite, MySQL and PostgreSQL</>",
"test:db:sqlite": "<fg=blue;options=bold>Runs database test suites on a SQLite database</>",
"test:db:mysql": "<fg=blue;options=bold>Runs database test suites on a MySQL database</>",
"test:db:postgres": "<fg=blue;options=bold>Runs database test suites on a PostgreSQL database</>",
"test:api": "<fg=blue;options=bold>Runs API test suites</>",

View file

@ -1,16 +1,40 @@
<?php
declare(strict_types=1);
use Monolog\Handler\StreamHandler;
use Monolog\Logger;
return [
$isSwoole = extension_loaded('swoole');
'logger' => [
'handlers' => [
'shlink_rotating_handler' => [
'level' => Logger::DEBUG,
],
// For swoole, send logs to standard output
$logger = $isSwoole ? [
'handlers' => [
'shlink_rotating_handler' => [
'level' => Logger::EMERGENCY, // This basically disables regular file logs
],
'shlink_stdout_handler' => [
'class' => StreamHandler::class,
'level' => Logger::DEBUG,
'stream' => 'php://stdout',
'formatter' => 'dashed',
],
],
'loggers' => [
'Shlink' => [
'handlers' => ['shlink_stdout_handler'],
],
],
] : [
'handlers' => [
'shlink_rotating_handler' => [
'level' => Logger::DEBUG,
],
],
];
return [
'logger' => $logger,
];

View file

@ -84,7 +84,9 @@ WORKDIR /home/shlink
# Expose swoole port
EXPOSE 8080
CMD /usr/local/bin/composer update && \
CMD \
# Install dependencies if the vendor dir does not exist
if [[ ! -d "./vendor" ]]; then /usr/local/bin/composer install ; fi && \
# When restarting the container, swoole might think it is already in execution
# This forces the app to be started every second until the exit code is 0
until php ./vendor/bin/zend-expressive-swoole start; do sleep 1 ; done

View file

@ -3,6 +3,7 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\CLI\Command\Visit;
use Exception;
use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
use Shlinkio\Shlink\CLI\Util\ExitCodes;
use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdaterInterface;
@ -78,8 +79,8 @@ class LocateVisitsCommand extends Command
$this->visitService->locateUnlocatedVisits(
[$this, 'getGeolocationDataForVisit'],
function (VisitLocation $location) use ($output) {
if (! $location->isEmpty()) {
static function (VisitLocation $location) use ($output) {
if (!$location->isEmpty()) {
$output->writeln(
sprintf(' [<info>Address located at "%s"</info>]', $location->getCountryName())
);
@ -88,9 +89,16 @@ class LocateVisitsCommand extends Command
);
$this->io->success('Finished processing all IPs');
return ExitCodes::EXIT_SUCCESS;
} catch (Exception $e) {
$this->io->error($e->getMessage());
if ($this->io->isVerbose()) {
$this->getApplication()->renderException($e, $this->io);
}
return ExitCodes::EXIT_FAILURE;
} finally {
$lock->release();
return ExitCodes::EXIT_SUCCESS;
}
}

View file

@ -5,11 +5,11 @@ namespace Shlinkio\Shlink\CLI\Util;
use Cake\Chronos\Chronos;
use GeoIp2\Database\Reader;
use InvalidArgumentException;
use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
use Shlinkio\Shlink\Common\Exception\RuntimeException;
use Shlinkio\Shlink\Common\IpGeolocation\GeoLite2\DbUpdaterInterface;
use Symfony\Component\Lock\Factory as Locker;
use Throwable;
class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
{
@ -32,39 +32,41 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
/**
* @throws GeolocationDbUpdateFailedException
*/
public function checkDbUpdate(callable $mustBeUpdated = null, callable $handleProgress = null): void
public function checkDbUpdate(?callable $mustBeUpdated = null, ?callable $handleProgress = null): void
{
$lock = $this->locker->createLock(self::LOCK_NAME);
$lock->acquire(true); // Block until lock is released
try {
$meta = $this->geoLiteDbReader->metadata();
if ($this->buildIsTooOld($meta->__get('buildEpoch'))) {
$this->downloadNewDb(true, $mustBeUpdated, $handleProgress);
}
} catch (InvalidArgumentException $e) {
// This is the exception thrown by the reader when the database file does not exist
$this->downloadNewDb(false, $mustBeUpdated, $handleProgress);
$this->downloadIfNeeded($mustBeUpdated, $handleProgress);
} catch (Throwable $e) {
throw $e;
} finally {
$lock->release();
}
}
private function buildIsTooOld(int $buildTimestamp): bool
/**
* @throws GeolocationDbUpdateFailedException
*/
private function downloadIfNeeded(?callable $mustBeUpdated, ?callable $handleProgress): void
{
$buildDate = Chronos::createFromTimestamp($buildTimestamp);
$now = Chronos::now();
return $now->gt($buildDate->addDays(35));
if (! $this->dbUpdater->databaseFileExists()) {
$this->downloadNewDb(false, $mustBeUpdated, $handleProgress);
return;
}
$meta = $this->geoLiteDbReader->metadata();
if ($this->buildIsTooOld($meta->__get('buildEpoch'))) {
$this->downloadNewDb(true, $mustBeUpdated, $handleProgress);
}
}
/**
* @throws GeolocationDbUpdateFailedException
*/
private function downloadNewDb(
bool $olderDbExists,
callable $mustBeUpdated = null,
callable $handleProgress = null
): void {
private function downloadNewDb(bool $olderDbExists, ?callable $mustBeUpdated, ?callable $handleProgress): void
{
if ($mustBeUpdated !== null) {
$mustBeUpdated($olderDbExists);
}
@ -75,4 +77,11 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
throw GeolocationDbUpdateFailedException::create($olderDbExists, $e);
}
}
private function buildIsTooOld(int $buildTimestamp): bool
{
$buildDate = Chronos::createFromTimestamp($buildTimestamp);
$now = Chronos::now();
return $now->gt($buildDate->addDays(35));
}
}

View file

@ -10,5 +10,5 @@ interface GeolocationDbUpdaterInterface
/**
* @throws GeolocationDbUpdateFailedException
*/
public function checkDbUpdate(callable $mustBeUpdated = null, callable $handleProgress = null): void;
public function checkDbUpdate(?callable $mustBeUpdated = null, ?callable $handleProgress = null): void;
}

View file

@ -5,7 +5,6 @@ namespace ShlinkioTest\Shlink\CLI\Util;
use Cake\Chronos\Chronos;
use GeoIp2\Database\Reader;
use InvalidArgumentException;
use MaxMind\Db\Reader\Metadata;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
@ -58,8 +57,10 @@ class GeolocationDbUpdaterTest extends TestCase
$mustBeUpdated = function () {
$this->assertTrue(true);
};
$getMeta = $this->geoLiteDbReader->metadata()->willThrow(InvalidArgumentException::class);
$prev = new RuntimeException('');
$fileExists = $this->dbUpdater->databaseFileExists()->willReturn(false);
$getMeta = $this->geoLiteDbReader->metadata();
$download = $this->dbUpdater->downloadFreshCopy(null)->willThrow($prev);
try {
@ -72,7 +73,8 @@ class GeolocationDbUpdaterTest extends TestCase
$this->assertFalse($e->olderDbExists());
}
$getMeta->shouldHaveBeenCalledOnce();
$fileExists->shouldHaveBeenCalledOnce();
$getMeta->shouldNotHaveBeenCalled();
$download->shouldHaveBeenCalledOnce();
}
@ -82,6 +84,7 @@ 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' => '',
@ -106,6 +109,7 @@ class GeolocationDbUpdaterTest extends TestCase
$this->assertTrue($e->olderDbExists());
}
$fileExists->shouldHaveBeenCalledOnce();
$getMeta->shouldHaveBeenCalledOnce();
$download->shouldHaveBeenCalledOnce();
}
@ -124,6 +128,7 @@ class GeolocationDbUpdaterTest extends TestCase
*/
public function databaseIsNotUpdatedIfItIsYoungerThanOneWeek(int $days): void
{
$fileExists = $this->dbUpdater->databaseFileExists()->willReturn(true);
$getMeta = $this->geoLiteDbReader->metadata()->willReturn(new Metadata([
'binary_format_major_version' => '',
'binary_format_minor_version' => '',
@ -140,6 +145,7 @@ class GeolocationDbUpdaterTest extends TestCase
$this->geolocationDbUpdater->checkDbUpdate();
$fileExists->shouldHaveBeenCalledOnce();
$getMeta->shouldHaveBeenCalledOnce();
$download->shouldNotHaveBeenCalled();
}

View file

@ -98,4 +98,9 @@ class DbUpdater implements DbUpdaterInterface
// Ignore any error produced when trying to delete temp files
}
}
public function databaseFileExists(): bool
{
return $this->filesystem->exists($this->options->getDbLocation());
}
}

View file

@ -7,6 +7,8 @@ use Shlinkio\Shlink\Common\Exception\RuntimeException;
interface DbUpdaterInterface
{
public function databaseFileExists(): bool;
/**
* @throws RuntimeException
*/

View file

@ -32,7 +32,7 @@ class DbUpdaterTest extends TestCase
$this->filesystem = $this->prophesize(Filesystem::class);
$this->options = new GeoLite2Options([
'temp_dir' => __DIR__ . '/../../../test-resources',
'db_location' => '',
'db_location' => 'db_location',
'download_from' => '',
]);
@ -110,4 +110,23 @@ class DbUpdaterTest extends TestCase
$copy->shouldHaveBeenCalledOnce();
$remove->shouldHaveBeenCalledOnce();
}
/**
* @test
* @dataProvider provideExists
*/
public function databaseFileExistsChecksIfTheFilesExistsInTheFilesystem(bool $expected): void
{
$exists = $this->filesystem->exists('db_location')->willReturn($expected);
$result = $this->dbUpdater->databaseFileExists();
$this->assertEquals($expected, $result);
$exists->shouldHaveBeenCalledOnce();
}
public function provideExists(): iterable
{
return [[true], [false]];
}
}