Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print a warning when manually running visit:download-db with no license #1926

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
* [#1055](https://github.com/shlinkio/shlink/issues/1055) Update OAS definition to v3.1.
* [#1885](https://github.com/shlinkio/shlink/issues/1885) Update to chronos 3.0.
* [#1896](https://github.com/shlinkio/shlink/issues/1896) Requests to health endpoint are no longer logged.
* [#1877](https://github.com/shlinkio/shlink/issues/1877) Print a warning when manually running `visit:download-db` command and a GeoLite2 license was not provided.

### Deprecated
* [#1783](https://github.com/shlinkio/shlink/issues/1783) Deprecated support for openswoole. RoadRunner is the best replacement, with the same capabilities, but much easier and convenient to install and manage.
Expand Down
37 changes: 24 additions & 13 deletions module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException;
use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface;
use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult;
use Shlinkio\Shlink\CLI\Util\ExitCode;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Helper\ProgressBar;
Expand Down Expand Up @@ -41,14 +42,19 @@
$io = new SymfonyStyle($input, $output);

try {
$this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) use ($io): void {
$result = $this->dbUpdater->checkDbUpdate(function (bool $olderDbExists) use ($io): void {
$io->text(sprintf('<fg=blue>%s GeoLite2 db file...</>', $olderDbExists ? 'Updating' : 'Downloading'));
$this->progressBar = new ProgressBar($io);
}, function (int $total, int $downloaded): void {
$this->progressBar?->setMaxSteps($total);
$this->progressBar?->setProgress($downloaded);
});

if ($result === GeolocationResult::LICENSE_MISSING) {
$io->warning('It was not possible to download GeoLite2 db, because a license was not provided.');
return ExitCode::EXIT_WARNING;
}

if ($this->progressBar === null) {
$io->info('GeoLite2 db file is up to date.');
} else {
Expand All @@ -58,21 +64,26 @@

return ExitCode::EXIT_SUCCESS;
} catch (GeolocationDbUpdateFailedException $e) {
$olderDbExists = $e->olderDbExists();
return $this->processGeoLiteUpdateError($e, $io);
}
}

if ($olderDbExists) {
$io->warning(
'GeoLite2 db file update failed. Visits will continue to be located with the old version.',
);
} else {
$io->error('GeoLite2 db file download failed. It will not be possible to locate visits.');
}
private function processGeoLiteUpdateError(GeolocationDbUpdateFailedException $e, SymfonyStyle $io): int
{
$olderDbExists = $e->olderDbExists();

if ($io->isVerbose()) {
$this->getApplication()?->renderThrowable($e, $io);
}
if ($olderDbExists) {
$io->warning(
'GeoLite2 db file update failed. Visits will continue to be located with the old version.',
);
} else {
$io->error('GeoLite2 db file download failed. It will not be possible to locate visits.');
}

return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE;
if ($io->isVerbose()) {
$this->getApplication()?->renderThrowable($e, $io);

Check warning on line 84 in module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php

View check run for this annotation

Codecov / codecov/patch

module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php#L84

Added line #L84 was not covered by tests
}

return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE;
}
}
3 changes: 1 addition & 2 deletions module/CLI/src/GeoLite/GeolocationDbUpdater.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ private function downloadNewDb(
$this->dbUpdater->downloadFreshCopy($this->wrapHandleProgressCallback($handleProgress, $olderDbExists));
return $olderDbExists ? GeolocationResult::DB_UPDATED : GeolocationResult::DB_CREATED;
} catch (MissingLicenseException) {
// If there's no license key, just ignore the error
return GeolocationResult::CHECK_SKIPPED;
return GeolocationResult::LICENSE_MISSING;
} catch (DbUpdateException | WrongIpException $e) {
throw $olderDbExists
? GeolocationDbUpdateFailedException::withOlderDb($e)
Expand Down
1 change: 1 addition & 0 deletions module/CLI/src/GeoLite/GeolocationResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
enum GeolocationResult
{
case CHECK_SKIPPED;
case LICENSE_MISSING;
case DB_CREATED;
case DB_UPDATED;
case DB_IS_UP_TO_DATE;
Expand Down
15 changes: 15 additions & 0 deletions module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,21 @@ public static function provideFailureParams(): iterable
];
}

#[Test]
public function warningIsPrintedWhenLicenseIsMissing(): void
{
$this->dbUpdater->expects($this->once())->method('checkDbUpdate')->withAnyParameters()->willReturn(
GeolocationResult::LICENSE_MISSING,
);

$this->commandTester->execute([]);
$output = $this->commandTester->getDisplay();
$exitCode = $this->commandTester->getStatusCode();

self::assertStringContainsString('[WARNING] It was not possible to download GeoLite2 db', $output);
self::assertSame(ExitCode::EXIT_WARNING, $exitCode);
}

#[Test, DataProvider('provideSuccessParams')]
public function printsExpectedMessageWhenNoErrorOccurs(callable $checkUpdateBehavior, string $expectedMessage): void
{
Expand Down
16 changes: 16 additions & 0 deletions module/CLI/test/GeoLite/GeolocationDbUpdaterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult;
use Shlinkio\Shlink\Core\Options\TrackingOptions;
use Shlinkio\Shlink\IpGeolocation\Exception\DbUpdateException;
use Shlinkio\Shlink\IpGeolocation\Exception\MissingLicenseException;
use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdaterInterface;
use Symfony\Component\Lock;
use Throwable;
Expand All @@ -37,6 +38,21 @@ protected function setUp(): void
$this->lock->method('acquire')->with($this->isTrue())->willReturn(true);
}

#[Test]
public function properResultIsReturnedWhenLicenseIsMissing(): void
{
$mustBeUpdated = fn () => self::assertTrue(true);

$this->dbUpdater->expects($this->once())->method('databaseFileExists')->willReturn(false);
$this->dbUpdater->expects($this->once())->method('downloadFreshCopy')->willThrowException(
new MissingLicenseException(''),
);
$this->geoLiteDbReader->expects($this->never())->method('metadata');

$result = $this->geolocationDbUpdater()->checkDbUpdate($mustBeUpdated);
self::assertEquals(GeolocationResult::LICENSE_MISSING, $result);
}

#[Test]
public function exceptionIsThrownWhenOlderDbDoesNotExistAndDownloadFails(): void
{
Expand Down
Loading