Compare commits

...

11 Commits

Author SHA1 Message Date
Alejandro Celaya
dc8f5d002d Merge pull request #1215 from shlinkio/develop
Release 2.9.2
2021-10-23 16:52:47 +02:00
Alejandro Celaya
9030e5e6eb Merge pull request #1214 from acelaya-forks/feature/min-task-workers
Feature/min task workers
2021-10-23 16:47:18 +02:00
Alejandro Celaya
2b827baeed Updated changelog 2021-10-23 16:35:38 +02:00
Alejandro Celaya
cc6fa312f0 Ensured minimum amount of task workers provided via config option or env var is 4 2021-10-23 16:32:06 +02:00
Alejandro Celaya
b8eba5b643 Merge pull request #1213 from acelaya-forks/feature/migrations-3.3
Feature/migrations 3.3
2021-10-23 16:18:39 +02:00
Alejandro Celaya
0c3f98cc37 Replaced implicit false in migration by a check on the platform 2021-10-23 16:04:54 +02:00
Alejandro Celaya
cd35770d26 Ensured migrations are not transactional when run in mysql 2021-10-23 16:02:29 +02:00
Alejandro Celaya
bd3a59e9ca Updated to doctrine-migrations 3.3 2021-10-23 15:44:56 +02:00
Alejandro Celaya
ff50d601b3 Merge pull request #1212 from acelaya-forks/feature/wrong-transactionality
Removed transactionality when dispatching async events
2021-10-23 13:49:09 +02:00
Alejandro Celaya
a4fde0f9e6 Changed mechanism to determine if connection to database worked for health endpoint 2021-10-23 13:36:27 +02:00
Alejandro Celaya
c7a621cb31 Removed transactionality when dispatching async events, as they run in different processes with different db connections 2021-10-23 13:22:42 +02:00
42 changed files with 223 additions and 63 deletions

View File

@@ -4,6 +4,25 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org).
## [2.9.2] - 2021-10-23
### Added
* *Nothing*
### Changed
* *Nothing*
### Deprecated
* *Nothing*
### Removed
* *Nothing*
### Fixed
* [#1210](https://github.com/shlinkio/shlink/issues/1210) Fixed real time updates not being notified due to an incorrect handling of db transactions on multi-process tasks.
* [#1211](https://github.com/shlinkio/shlink/issues/1211) Fixed `There is no active transaction` error when running migrations in MySQL/Mariadb after updating to doctrine-migrations 3.3.
* [#1197](https://github.com/shlinkio/shlink/issues/1197) Fixed amount of task workers provided via config option or env var not being validated to ensure enough workers to process all parallel tasks.
## [2.9.1] - 2021-10-11
### Added
* *Nothing*

View File

@@ -18,7 +18,7 @@
"akrabat/ip-address-middleware": "^2.0",
"cakephp/chronos": "^2.2",
"cocur/slugify": "^4.0",
"doctrine/migrations": "^3.2",
"doctrine/migrations": "^3.3",
"doctrine/orm": "^2.9",
"endroid/qr-code": "^4.2",
"geoip2/geoip2": "^2.11",
@@ -51,7 +51,7 @@
"shlinkio/shlink-config": "^1.2",
"shlinkio/shlink-event-dispatcher": "^2.1",
"shlinkio/shlink-importer": "^2.3.1",
"shlinkio/shlink-installer": "^6.2",
"shlinkio/shlink-installer": "^6.2.1",
"shlinkio/shlink-ip-geolocation": "^2.0",
"symfony/console": "^5.3",
"symfony/filesystem": "^5.3",
@@ -73,7 +73,7 @@
"phpunit/phpunit": "^9.5",
"roave/security-advisories": "dev-master",
"shlinkio/php-coding-standard": "~2.2.0",
"shlinkio/shlink-test-utils": "^2.2",
"shlinkio/shlink-test-utils": "^2.3",
"symfony/var-dumper": "^5.3",
"veewee/composer-run-parallel": "^1.0"
},

View File

@@ -4,22 +4,28 @@ declare(strict_types=1);
use function Shlinkio\Shlink\Common\env;
return [
use const Shlinkio\Shlink\MIN_TASK_WORKERS;
'mezzio-swoole' => [
// Setting this to true can have unexpected behaviors when running several concurrent slow DB queries
'enable_coroutine' => false,
return (static function () {
$taskWorkers = (int) env('TASK_WORKER_NUM', 16);
'swoole-http-server' => [
'host' => '0.0.0.0',
'port' => (int) env('PORT', 8080),
'process-name' => 'shlink',
return [
'options' => [
'worker_num' => (int) env('WEB_WORKER_NUM', 16),
'task_worker_num' => (int) env('TASK_WORKER_NUM', 16),
'mezzio-swoole' => [
// Setting this to true can have unexpected behaviors when running several concurrent slow DB queries
'enable_coroutine' => false,
'swoole-http-server' => [
'host' => '0.0.0.0',
'port' => (int) env('PORT', 8080),
'process-name' => 'shlink',
'options' => [
'worker_num' => (int) env('WEB_WORKER_NUM', 16),
'task_worker_num' => $taskWorkers < MIN_TASK_WORKERS ? MIN_TASK_WORKERS : $taskWorkers,
],
],
],
],
];
];
})();

View File

@@ -18,3 +18,4 @@ const DEFAULT_QR_CODE_SIZE = 300;
const DEFAULT_QR_CODE_MARGIN = 0;
const DEFAULT_QR_CODE_FORMAT = 'png';
const DEFAULT_QR_CODE_ERROR_CORRECTION = 'l';
const MIN_TASK_WORKERS = 4;

View File

@@ -29,6 +29,6 @@ register_shutdown_function(function () use ($httpClient): void {
);
});
$testHelper->createTestDb();
$testHelper->createTestDb(['bin/cli', 'db:create'], ['bin/cli', 'db:migrate']);
ApiTest\ApiTestCase::setApiClient($httpClient);
ApiTest\ApiTestCase::setSeedFixturesCallback(fn () => $testHelper->seedFixtures($em, $config['data_fixtures'] ?? []));

View File

@@ -39,6 +39,11 @@ class Version20160819142757 extends AbstractMigration
*/
public function down(Schema $schema): void
{
$db = $this->connection->getDatabasePlatform()->getName();
$this->connection->getDatabasePlatform()->getName();
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -73,4 +73,9 @@ class Version20160820191203 extends AbstractMigration
$schema->dropTable('short_urls_in_tags');
$schema->dropTable('tags');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -45,4 +45,9 @@ class Version20171021093246 extends AbstractMigration
$shortUrls->dropColumn('valid_since');
$shortUrls->dropColumn('valid_until');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -42,4 +42,9 @@ class Version20171022064541 extends AbstractMigration
$shortUrls->dropColumn('max_visits');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -39,4 +39,9 @@ final class Version20180801183328 extends AbstractMigration
{
$schema->getTable('short_urls')->getColumn('short_code')->setLength($size);
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -66,4 +66,9 @@ final class Version20180913205455 extends AbstractMigration
{
// Nothing to rollback
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -47,4 +47,9 @@ final class Version20180915110857 extends AbstractMigration
{
// Nothing to run
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -65,4 +65,9 @@ final class Version20181020060559 extends AbstractMigration
{
// No down
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -38,4 +38,9 @@ final class Version20181020065148 extends AbstractMigration
{
// No down
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -34,4 +34,9 @@ final class Version20181110175521 extends AbstractMigration
{
return $schema->getTable('visits')->getColumn('user_agent');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -34,4 +34,9 @@ final class Version20190824075137 extends AbstractMigration
{
return $schema->getTable('visits')->getColumn('referer');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -52,4 +52,9 @@ final class Version20190930165521 extends AbstractMigration
$schema->getTable('short_urls')->dropColumn('domain_id');
$schema->dropTable('domains');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -46,4 +46,9 @@ final class Version20191001201532 extends AbstractMigration
$shortUrls->dropIndex('unique_short_code_plus_domain');
$shortUrls->addUniqueIndex(['short_code']);
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -34,4 +34,9 @@ final class Version20191020074522 extends AbstractMigration
{
return $schema->getTable('short_urls')->getColumn('original_url');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -93,4 +93,9 @@ final class Version20200105165647 extends AbstractMigration
$visitLocations->dropColumn($colName);
}
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -44,4 +44,9 @@ final class Version20200106215144 extends AbstractMigration
]);
}
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -50,4 +50,9 @@ final class Version20200110182849 extends AbstractMigration
{
// No need (and no way) to undo this migration
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -42,4 +42,9 @@ final class Version20200323190014 extends AbstractMigration
$visitLocations->dropColumn('is_empty');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -24,4 +24,9 @@ final class Version20200503170404 extends AbstractMigration
$this->skipIf(! $visits->hasIndex(self::INDEX_NAME));
$visits->dropIndex(self::INDEX_NAME);
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -41,4 +41,9 @@ final class Version20201023090929 extends AbstractMigration
$shortUrls->dropColumn('import_original_short_code');
$shortUrls->dropIndex('unique_imports');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -83,4 +83,9 @@ final class Version20201102113208 extends AbstractMigration
$shortUrls->removeForeignKey('FK_' . self::API_KEY_COLUMN);
$shortUrls->dropColumn(self::API_KEY_COLUMN);
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -49,4 +49,9 @@ final class Version20210102174433 extends AbstractMigration
$schema->getTable(self::TABLE_NAME)->dropIndex('UQ_role_plus_api_key');
$schema->dropTable(self::TABLE_NAME);
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -23,4 +23,9 @@ final class Version20210118153932 extends AbstractMigration
public function down(Schema $schema): void
{
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -33,4 +33,9 @@ final class Version20210202181026 extends AbstractMigration
$shortUrls->dropColumn(self::TITLE);
$shortUrls->dropColumn('title_was_auto_resolved');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -40,4 +40,9 @@ final class Version20210207100807 extends AbstractMigration
$visits->dropColumn('visited_url');
$visits->dropColumn('type');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -34,4 +34,9 @@ final class Version20210306165711 extends AbstractMigration
$apiKeys->dropColumn(self::COLUMN);
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -23,4 +23,9 @@ final class Version20210522051601 extends AbstractMigration
$this->skipIf(! $shortUrls->hasColumn('crawlable'));
$shortUrls->dropColumn('crawlable');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -25,4 +25,9 @@ final class Version20210522124633 extends AbstractMigration
$this->skipIf(! $visits->hasColumn(self::POTENTIAL_BOT_COLUMN));
$visits->dropColumn(self::POTENTIAL_BOT_COLUMN);
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -38,4 +38,9 @@ final class Version20210720143824 extends AbstractMigration
$domainsTable->dropColumn('regular_not_found_redirect');
$domainsTable->dropColumn('invalid_short_url_redirect');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -23,4 +23,9 @@ final class Version20211002072605 extends AbstractMigration
$this->skipIf(! $shortUrls->hasColumn('forward_query'));
$shortUrls->dropColumn('forward_query');
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -18,4 +18,9 @@ final class <className> extends AbstractMigration
{
<down>
}
public function isTransactional(): bool
{
return $this->connection->getDatabasePlatform()->getName() !== 'mysql';
}
}

View File

@@ -67,11 +67,11 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
// In order to create the new database, we have to use a connection where the dbname was not set.
// Otherwise, it will fail to connect and will not be able to create the new database
$schemaManager = $this->noDbNameConn->getSchemaManager();
$schemaManager = $this->noDbNameConn->createSchemaManager();
$databases = $schemaManager->listDatabases();
$shlinkDatabase = $this->regularConn->getDatabase();
if (! contains($databases, $shlinkDatabase)) {
if ($shlinkDatabase !== null && ! contains($databases, $shlinkDatabase)) {
$schemaManager->createDatabase($shlinkDatabase);
}
}
@@ -80,7 +80,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand
{
// If at least one of the shlink tables exist, we will consider the database exists somehow.
// Any inconsistency should be taken care by the migrations
$schemaManager = $this->regularConn->getSchemaManager();
$schemaManager = $this->regularConn->createSchemaManager();
return ! empty($schemaManager->listTableNames());
}
}

View File

@@ -46,10 +46,10 @@ class CreateDatabaseCommandTest extends TestCase
$this->databasePlatform = $this->prophesize(AbstractPlatform::class);
$this->regularConn = $this->prophesize(Connection::class);
$this->regularConn->getSchemaManager()->willReturn($this->schemaManager->reveal());
$this->regularConn->createSchemaManager()->willReturn($this->schemaManager->reveal());
$this->regularConn->getDatabasePlatform()->willReturn($this->databasePlatform->reveal());
$noDbNameConn = $this->prophesize(Connection::class);
$noDbNameConn->getSchemaManager()->willReturn($this->schemaManager->reveal());
$noDbNameConn->createSchemaManager()->willReturn($this->schemaManager->reveal());
$command = new CreateDatabaseCommand(
$locker->reveal(),

View File

@@ -69,11 +69,9 @@ class VisitsTracker implements VisitsTrackerInterface
}
$visit = $createVisit($visitor->normalizeForTrackingOptions($this->options));
$this->em->transactional(function () use ($visit, $visitor): void {
$this->em->persist($visit);
$this->em->flush();
$this->em->persist($visit);
$this->em->flush();
$this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->getRemoteAddress()));
});
$this->eventDispatcher->dispatch(new UrlVisited($visit->getId(), $visitor->getRemoteAddress()));
}
}

View File

@@ -29,10 +29,6 @@ class VisitsTrackerTest extends TestCase
public function setUp(): void
{
$this->em = $this->prophesize(EntityManager::class);
$this->em->transactional(Argument::any())->will(function (array $args) {
[$callback] = $args;
return $callback();
});
$this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
$this->options = new TrackingOptions();
@@ -52,7 +48,6 @@ class VisitsTrackerTest extends TestCase
$this->visitsTracker->{$method}(...$args);
$persist->shouldHaveBeenCalledOnce();
$this->em->transactional(Argument::cetera())->shouldHaveBeenCalledOnce();
$this->em->flush()->shouldHaveBeenCalledOnce();
$this->eventDispatcher->dispatch(Argument::type(UrlVisited::class))->shouldHaveBeenCalled();
}
@@ -68,7 +63,6 @@ class VisitsTrackerTest extends TestCase
$this->visitsTracker->{$method}(...$args);
$this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled();
$this->em->transactional(Argument::cetera())->shouldNotHaveBeenCalled();
$this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
$this->em->flush()->shouldNotHaveBeenCalled();
}
@@ -92,7 +86,6 @@ class VisitsTrackerTest extends TestCase
$this->visitsTracker->{$method}(Visitor::emptyInstance());
$this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled();
$this->em->transactional(Argument::cetera())->shouldNotHaveBeenCalled();
$this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
$this->em->flush()->shouldNotHaveBeenCalled();
}

View File

@@ -32,7 +32,9 @@ class HealthAction extends AbstractRestAction
public function handle(ServerRequestInterface $request): ResponseInterface
{
try {
$connected = $this->em->getConnection()->ping();
$connection = $this->em->getConnection();
$connection->executeQuery($connection->getDatabasePlatform()->getDummySelectSQL());
$connected = true;
} catch (Throwable) {
$connected = false;
}

View File

@@ -5,11 +5,14 @@ declare(strict_types=1);
namespace ShlinkioTest\Shlink\Rest\Action;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Result;
use Doctrine\ORM\EntityManagerInterface;
use Exception;
use Laminas\Diactoros\Response\JsonResponse;
use Laminas\Diactoros\ServerRequest;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Core\Options\AppOptions;
@@ -25,6 +28,11 @@ class HealthActionTest extends TestCase
public function setUp(): void
{
$this->conn = $this->prophesize(Connection::class);
$this->conn->executeQuery(Argument::cetera())->willReturn($this->prophesize(Result::class)->reveal());
$dbPlatform = $this->prophesize(AbstractPlatform::class);
$dbPlatform->getDummySelectSQL()->willReturn('');
$this->conn->getDatabasePlatform()->willReturn($dbPlatform->reveal());
$em = $this->prophesize(EntityManagerInterface::class);
$em->getConnection()->willReturn($this->conn->reveal());
@@ -32,10 +40,8 @@ class HealthActionTest extends TestCase
}
/** @test */
public function passResponseIsReturnedWhenConnectionSucceeds(): void
public function passResponseIsReturnedWhenDummyQuerySucceeds(): void
{
$ping = $this->conn->ping()->willReturn(true);
/** @var JsonResponse $resp */
$resp = $this->action->handle(new ServerRequest());
$payload = $resp->getPayload();
@@ -48,13 +54,13 @@ class HealthActionTest extends TestCase
'project' => 'https://github.com/shlinkio/shlink',
], $payload['links']);
self::assertEquals('application/health+json', $resp->getHeaderLine('Content-type'));
$ping->shouldHaveBeenCalledOnce();
$this->conn->executeQuery(Argument::cetera())->shouldHaveBeenCalledOnce();
}
/** @test */
public function failResponseIsReturnedWhenConnectionFails(): void
public function failResponseIsReturnedWhenDummyQueryThrowsException(): void
{
$ping = $this->conn->ping()->willReturn(false);
$executeQuery = $this->conn->executeQuery(Argument::cetera())->willThrow(Exception::class);
/** @var JsonResponse $resp */
$resp = $this->action->handle(new ServerRequest());
@@ -68,26 +74,6 @@ class HealthActionTest extends TestCase
'project' => 'https://github.com/shlinkio/shlink',
], $payload['links']);
self::assertEquals('application/health+json', $resp->getHeaderLine('Content-type'));
$ping->shouldHaveBeenCalledOnce();
}
/** @test */
public function failResponseIsReturnedWhenConnectionThrowsException(): void
{
$ping = $this->conn->ping()->willThrow(Exception::class);
/** @var JsonResponse $resp */
$resp = $this->action->handle(new ServerRequest());
$payload = $resp->getPayload();
self::assertEquals(503, $resp->getStatusCode());
self::assertEquals('fail', $payload['status']);
self::assertEquals('1.2.3', $payload['version']);
self::assertEquals([
'about' => 'https://shlink.io',
'project' => 'https://github.com/shlinkio/shlink',
], $payload['links']);
self::assertEquals('application/health+json', $resp->getHeaderLine('Content-type'));
$ping->shouldHaveBeenCalledOnce();
$executeQuery->shouldHaveBeenCalledOnce();
}
}