diff --git a/CHANGELOG.md b/CHANGELOG.md index ace9527e..4b284af2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ ## CHANGELOG +### 1.9.1 + +**Bugs:** + +* [154: When filtering by searchTerm, sizes of every result page has an unexpected behavior](https://github.com/shlinkio/shlink/issues/154) +* [157: Background commands executed by installation process do not respect the used php binary](https://github.com/shlinkio/shlink/issues/157) + +**Enhancements:** + +* [155: Improve the pagination object returned in lists, including more meaningful properties](https://github.com/shlinkio/shlink/issues/155) + ### 1.9.0 **Features** diff --git a/docs/swagger/definitions/Pagination.json b/docs/swagger/definitions/Pagination.json index e9b731ef..18aab934 100644 --- a/docs/swagger/definitions/Pagination.json +++ b/docs/swagger/definitions/Pagination.json @@ -3,11 +3,23 @@ "properties": { "currentPage": { "type": "integer", - "description": "The number of current page being displayed." + "description": "The number of current page." }, "pagesCount": { "type": "integer", - "description": "The total number of pages that can be displayed." + "description": "The total number of pages that can be obtained." + }, + "itemsPerPage": { + "type": "integer", + "description": "The number of items for every page." + }, + "itemsInCurrentPage": { + "type": "integer", + "description": "The number of items in current page (could be smaller than itemsPerPage)." + }, + "totalItems": { + "type": "integer", + "description": "The total number of items among all pages." } } } diff --git a/docs/swagger/paths/v1_short-codes.json b/docs/swagger/paths/v1_short-codes.json index b29bd182..9d52f798 100644 --- a/docs/swagger/paths/v1_short-codes.json +++ b/docs/swagger/paths/v1_short-codes.json @@ -116,7 +116,10 @@ ], "pagination": { "currentPage": 5, - "pagesCount": 12 + "pagesCount": 12, + "itemsPerPage": 10, + "itemsInCurrentPage": 10, + "totalItems": 115 } } } diff --git a/module/CLI/src/Command/Install/InstallCommand.php b/module/CLI/src/Command/Install/InstallCommand.php index 7c2ef2e2..68afa422 100644 --- a/module/CLI/src/Command/Install/InstallCommand.php +++ b/module/CLI/src/Command/Install/InstallCommand.php @@ -18,6 +18,7 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use Symfony\Component\Filesystem\Exception\IOException; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\Process\PhpExecutableFinder; use Zend\Config\Writer\WriterInterface; class InstallCommand extends Command @@ -48,6 +49,14 @@ class InstallCommand extends Command * @var bool */ private $isUpdate; + /** + * @var PhpExecutableFinder + */ + private $phpFinder; + /** + * @var string|bool + */ + private $phpBinary; /** * InstallCommand constructor. @@ -55,22 +64,25 @@ class InstallCommand extends Command * @param Filesystem $filesystem * @param ConfigCustomizerManagerInterface $configCustomizers * @param bool $isUpdate + * @param PhpExecutableFinder|null $phpFinder * @throws LogicException */ public function __construct( WriterInterface $configWriter, Filesystem $filesystem, ConfigCustomizerManagerInterface $configCustomizers, - $isUpdate = false + bool $isUpdate = false, + PhpExecutableFinder $phpFinder = null ) { parent::__construct(); $this->configWriter = $configWriter; $this->isUpdate = $isUpdate; $this->filesystem = $filesystem; $this->configCustomizers = $configCustomizers; + $this->phpFinder = $phpFinder ?: new PhpExecutableFinder(); } - public function configure() + protected function configure(): void { $this ->setName('shlink:install') @@ -84,7 +96,7 @@ class InstallCommand extends Command * @throws ContainerExceptionInterface * @throws NotFoundExceptionInterface */ - public function execute(InputInterface $input, OutputInterface $output) + protected function execute(InputInterface $input, OutputInterface $output): void { $this->io = new SymfonyStyle($input, $output); @@ -133,8 +145,8 @@ class InstallCommand extends Command // If current command is not update, generate database if (! $this->isUpdate) { $this->io->write('Initializing database...'); - if (! $this->runCommand( - 'php vendor/doctrine/orm/bin/doctrine.php orm:schema-tool:create', + if (! $this->runPhpCommand( + 'vendor/doctrine/orm/bin/doctrine.php orm:schema-tool:create', 'Error generating database.', $output )) { @@ -144,8 +156,8 @@ class InstallCommand extends Command // Run database migrations $this->io->write('Updating database...'); - if (! $this->runCommand( - 'php vendor/doctrine/migrations/bin/doctrine-migrations.php migrations:migrate', + if (! $this->runPhpCommand( + 'vendor/doctrine/migrations/bin/doctrine-migrations.php migrations:migrate', 'Error updating database.', $output )) { @@ -154,8 +166,8 @@ class InstallCommand extends Command // Generate proxies $this->io->write('Generating proxies...'); - if (! $this->runCommand( - 'php vendor/doctrine/orm/bin/doctrine.php orm:generate-proxies', + if (! $this->runPhpCommand( + 'vendor/doctrine/orm/bin/doctrine.php orm:generate-proxies', 'Error generating proxies.', $output )) { @@ -213,23 +225,27 @@ class InstallCommand extends Command * @throws LogicException * @throws InvalidArgumentException */ - private function runCommand($command, $errorMessage, OutputInterface $output): bool + private function runPhpCommand($command, $errorMessage, OutputInterface $output): bool { if ($this->processHelper === null) { $this->processHelper = $this->getHelper('process'); } - $process = $this->processHelper->run($output, $command); + if ($this->phpBinary === null) { + $this->phpBinary = $this->phpFinder->find(false) ?: 'php'; + } + + $this->io->writeln('Running "' . sprintf('%s %s', $this->phpBinary, $command) . '"'); + $process = $this->processHelper->run($output, sprintf('%s %s', $this->phpBinary, $command)); if ($process->isSuccessful()) { $this->io->writeln(' Success!'); return true; } - if ($this->io->isVerbose()) { - return false; + if (! $this->io->isVerbose()) { + $this->io->error($errorMessage . ' Run this command with -vvv to see specific error info.'); } - $this->io->error($errorMessage . ' Run this command with -vvv to see specific error info.'); return false; } } diff --git a/module/CLI/test/Command/Install/InstallCommandTest.php b/module/CLI/test/Command/Install/InstallCommandTest.php index a4c49f9e..556f628b 100644 --- a/module/CLI/test/Command/Install/InstallCommandTest.php +++ b/module/CLI/test/Command/Install/InstallCommandTest.php @@ -15,6 +15,7 @@ use Symfony\Component\Console\Helper\ProcessHelper; use Symfony\Component\Console\Tester\CommandTester; use Symfony\Component\Filesystem\Exception\IOException; use Symfony\Component\Filesystem\Filesystem; +use Symfony\Component\Process\PhpExecutableFinder; use Symfony\Component\Process\Process; use Zend\Config\Writer\WriterInterface; @@ -55,6 +56,9 @@ class InstallCommandTest extends TestCase $configCustomizers = $this->prophesize(ConfigCustomizerManagerInterface::class); $configCustomizers->get(Argument::cetera())->willReturn($configCustomizer->reveal()); + $finder = $this->prophesize(PhpExecutableFinder::class); + $finder->find(false)->willReturn('php'); + $app = new Application(); $helperSet = $app->getHelperSet(); $helperSet->set($processHelper->reveal()); @@ -62,7 +66,9 @@ class InstallCommandTest extends TestCase $this->command = new InstallCommand( $this->configWriter->reveal(), $this->filesystem->reveal(), - $configCustomizers->reveal() + $configCustomizers->reveal(), + false, + $finder->reveal() ); $app->add($this->command); diff --git a/module/Common/src/Paginator/Adapter/PaginableRepositoryAdapter.php b/module/Common/src/Paginator/Adapter/PaginableRepositoryAdapter.php index b57bb6e8..c83fa5a7 100644 --- a/module/Common/src/Paginator/Adapter/PaginableRepositoryAdapter.php +++ b/module/Common/src/Paginator/Adapter/PaginableRepositoryAdapter.php @@ -8,7 +8,7 @@ use Zend\Paginator\Adapter\AdapterInterface; class PaginableRepositoryAdapter implements AdapterInterface { - const ITEMS_PER_PAGE = 10; + public const ITEMS_PER_PAGE = 10; /** * @var PaginableRepositoryInterface @@ -34,7 +34,7 @@ class PaginableRepositoryAdapter implements AdapterInterface $orderBy = null ) { $this->paginableRepository = $paginableRepository; - $this->searchTerm = $searchTerm !== null ? trim(strip_tags($searchTerm)) : null; + $this->searchTerm = $searchTerm !== null ? \trim(\strip_tags($searchTerm)) : null; $this->orderBy = $orderBy; $this->tags = $tags; } @@ -46,7 +46,7 @@ class PaginableRepositoryAdapter implements AdapterInterface * @param int $itemCountPerPage Number of items per page * @return array */ - public function getItems($offset, $itemCountPerPage) + public function getItems($offset, $itemCountPerPage): array { return $this->paginableRepository->findList( $itemCountPerPage, @@ -66,7 +66,7 @@ class PaginableRepositoryAdapter implements AdapterInterface * The return value is cast to an integer. * @since 5.1.0 */ - public function count() + public function count(): int { return $this->paginableRepository->countList($this->searchTerm, $this->tags); } diff --git a/module/Common/src/Paginator/Util/PaginatorUtilsTrait.php b/module/Common/src/Paginator/Util/PaginatorUtilsTrait.php index 167de806..40f51d90 100644 --- a/module/Common/src/Paginator/Util/PaginatorUtilsTrait.php +++ b/module/Common/src/Paginator/Util/PaginatorUtilsTrait.php @@ -8,13 +8,16 @@ use Zend\Stdlib\ArrayUtils; trait PaginatorUtilsTrait { - protected function serializePaginator(Paginator $paginator) + protected function serializePaginator(Paginator $paginator): array { return [ 'data' => ArrayUtils::iteratorToArray($paginator->getCurrentItems()), 'pagination' => [ 'currentPage' => $paginator->getCurrentPageNumber(), 'pagesCount' => $paginator->count(), + 'itemsPerPage' => $paginator->getItemCountPerPage(), + 'itemsInCurrentPage' => $paginator->getCurrentItemCount(), + 'totalItems' => $paginator->getTotalItemCount(), ], ]; } @@ -25,7 +28,7 @@ trait PaginatorUtilsTrait * @param Paginator $paginator * @return bool */ - protected function isLastPage(Paginator $paginator) + protected function isLastPage(Paginator $paginator): bool { return $paginator->getCurrentPageNumber() >= $paginator->count(); } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index 6a1d1545..a959f818 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -25,7 +25,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI $orderBy = null ): array { $qb = $this->createListQueryBuilder($searchTerm, $tags); - $qb->select('s'); + $qb->select('DISTINCT s'); // Set limit and offset if ($limit !== null) { @@ -47,17 +47,17 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI protected function processOrderByForList(QueryBuilder $qb, $orderBy) { - $fieldName = is_array($orderBy) ? key($orderBy) : $orderBy; - $order = is_array($orderBy) ? $orderBy[$fieldName] : 'ASC'; + $fieldName = \is_array($orderBy) ? \key($orderBy) : $orderBy; + $order = \is_array($orderBy) ? $orderBy[$fieldName] : 'ASC'; - if (in_array($fieldName, ['visits', 'visitsCount', 'visitCount'], true)) { + if (\in_array($fieldName, ['visits', 'visitsCount', 'visitCount'], true)) { $qb->addSelect('COUNT(v) AS totalVisits') ->leftJoin('s.visits', 'v') ->groupBy('s') ->orderBy('totalVisits', $order); - return array_column($qb->getQuery()->getResult(), 0); - } elseif (in_array($fieldName, ['originalUrl', 'shortCode', 'dateCreated'], true)) { + return \array_column($qb->getQuery()->getResult(), 0); + } elseif (\in_array($fieldName, ['originalUrl', 'shortCode', 'dateCreated'], true)) { $qb->orderBy('s.' . $fieldName, $order); } @@ -74,7 +74,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI public function countList(string $searchTerm = null, array $tags = []): int { $qb = $this->createListQueryBuilder($searchTerm, $tags); - $qb->select('COUNT(s)'); + $qb->select('COUNT(DISTINCT s)'); return (int) $qb->getQuery()->getSingleScalarResult(); } @@ -92,7 +92,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI // Apply search term to every searchable field if not empty if (! empty($searchTerm)) { - $qb->join('s.tags', 't'); + $qb->leftJoin('s.tags', 't'); $conditions = [ $qb->expr()->like('s.originalUrl', ':searchPattern'), @@ -102,8 +102,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI // Unpack and apply search conditions $qb->andWhere($qb->expr()->orX(...$conditions)); - $searchTerm = '%' . $searchTerm . '%'; - $qb->setParameter('searchPattern', $searchTerm); + $qb->setParameter('searchPattern', '%' . $searchTerm . '%'); } // Filter by tags if provided @@ -119,7 +118,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI * @param string $shortCode * @return ShortUrl|null */ - public function findOneByShortCode(string $shortCode) + public function findOneByShortCode(string $shortCode): ?ShortUrl { $now = new \DateTimeImmutable();