mirrors/shlink
1
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2025-03-14 12:11:19 +03:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #275 from acelaya/feature/doctrine-performance

Browse source 
Feature/doctrine performance
This commit is contained in:
Alejandro Celaya 2018-11-17 09:59:53 +01:00 • committed by GitHub
commit 17fcd637f2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 244 additions and 102 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
CHANGELOG.md
View file

@ -4,6 +4,29 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org).
## [Unreleased]
#### Added
* *Nothing*
#### Changed
* *Nothing*
#### Deprecated
* *Nothing*
#### Removed
* *Nothing*
#### Fixed
* [#271](https://github.com/shlinkio/shlink/issues/271) Fixed memory leak produced when processing high amounts of visits at the same time.
## 1.14.0 - 2018-11-16
#### Added

2
README.md
View file

@ -114,6 +114,8 @@ Those tasks can be performed using shlink's CLI, so it should be easy to schedul
Running this will improve the performance of the `doma.in/abc123/preview` URLs, which return a preview of the site.
*Any of those commands accept the `-q` flag, which makes it not display any output. This is recommended when configuring the commands as cron jobs.*
## Update to new version
When a new Shlink version is available, you don't need to repeat the entire process yourself. Instead, follow these steps:

44
module/CLI/src/Command/Visit/ProcessVisitsCommand.php
View file

@ -6,7 +6,9 @@ namespace Shlinkio\Shlink\CLI\Command\Visit;
use Shlinkio\Shlink\Common\Exception\WrongIpException;
use Shlinkio\Shlink\Common\IpGeolocation\IpLocationResolverInterface;
use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
use Shlinkio\Shlink\Core\Service\VisitServiceInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
@ -53,51 +55,47 @@ class ProcessVisitsCommand extends Command
protected function execute(InputInterface $input, OutputInterface $output): void
{
$io = new SymfonyStyle($input, $output);
$visits = $this->visitService->getUnlocatedVisits();
foreach ($visits as $visit) {
$this->visitService->locateVisits(function (Visit $visit) use ($output) {
if (! $visit->hasRemoteAddr()) {
$io->writeln(
$output->writeln(
sprintf('<comment>%s</comment>', $this->translator->translate('Ignored visit with no IP address')),
OutputInterface::VERBOSITY_VERBOSE
);
continue;
throw new IpCannotBeLocatedException('Ignored visit with no IP address');
}
$ipAddr = $visit->getRemoteAddr();
$io->write(sprintf('%s <fg=blue>%s</>', $this->translator->translate('Processing IP'), $ipAddr));
$output->write(sprintf('%s <fg=blue>%s</>', $this->translator->translate('Processing IP'), $ipAddr));
if ($ipAddr === IpAddress::LOCALHOST) {
$io->writeln(
$output->writeln(
sprintf(' [<comment>%s</comment>]', $this->translator->translate('Ignored localhost address'))
);
continue;
throw new IpCannotBeLocatedException('Ignored localhost address');
}
try {
$result = $this->ipLocationResolver->resolveIpLocation($ipAddr);
$location = new VisitLocation($result);
$visit->setVisitLocation($location);
$this->visitService->saveVisit($visit);
$io->writeln(sprintf(
' [<info>' . $this->translator->translate('Address located at "%s"') . '</info>]',
$location->getCountryName()
));
return $this->ipLocationResolver->resolveIpLocation($ipAddr);
} catch (WrongIpException $e) {
$io->writeln(
$output->writeln(
sprintf(
' [<fg=red>%s</>]',
$this->translator->translate('An error occurred while locating IP. Skipped')
)
);
if ($io->isVerbose()) {
if ($output->isVerbose()) {
$this->getApplication()->renderException($e, $output);
}
}
}
throw new IpCannotBeLocatedException('An error occurred while locating IP', 0, $e);
}
}, function (VisitLocation $location) use ($output) {
$output->writeln(sprintf(
' [<info>' . $this->translator->translate('Address located at "%s"') . '</info>]',
$location->getCountryName()
));
});
$io = new SymfonyStyle($input, $output);
$io->success($this->translator->translate('Finished processing all IPs'));
}
}

2
module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php
View file

@ -81,7 +81,7 @@ class GetVisitsCommandTest extends TestCase
{
$shortCode = 'abc123';
$this->visitsTracker->info($shortCode, Argument::any())->willReturn([
(new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->setVisitLocation(
(new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->locate(
new VisitLocation(['country_name' => 'Spain'])
),
])->shouldBeCalledOnce();

127
module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php
View file

@ -7,16 +7,21 @@ use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\CLI\Command\Visit\ProcessVisitsCommand;
use Shlinkio\Shlink\Common\Exception\WrongIpException;
use Shlinkio\Shlink\Common\IpGeolocation\IpApiLocationResolver;
use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Service\VisitService;
use Symfony\Component\Console\Application;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Tester\CommandTester;
use Throwable;
use Zend\I18n\Translator\Translator;
use function count;
use function array_shift;
class ProcessVisitsCommandTest extends TestCase
{
@ -52,59 +57,107 @@ class ProcessVisitsCommandTest extends TestCase
/**
* @test
*/
public function allReturnedVisitsIpsAreProcessed()
public function allPendingVisitsAreProcessed()
{
$shortUrl = new ShortUrl('');
$visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4'));
$location = new VisitLocation([]);
$visits = [
new Visit($shortUrl, new Visitor('', '', '1.2.3.4')),
new Visit($shortUrl, new Visitor('', '', '4.3.2.1')),
new Visit($shortUrl, new Visitor('', '', '12.34.56.78')),
];
$this->visitService->getUnlocatedVisits()->willReturn($visits)
->shouldBeCalledOnce();
$locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(
function (array $args) use ($visit, $location) {
$firstCallback = array_shift($args);
$firstCallback($visit);
$this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(count($visits));
$this->ipResolver->resolveIpLocation(Argument::any())->willReturn([])
->shouldBeCalledTimes(count($visits));
$secondCallback = array_shift($args);
$secondCallback($location, $visit);
}
);
$resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]);
$this->commandTester->execute([
'command' => 'visit:process',
]);
$output = $this->commandTester->getDisplay();
$this->assertContains('Processing IP 1.2.3.0', $output);
$this->assertContains('Processing IP 4.3.2.0', $output);
$this->assertContains('Processing IP 12.34.56.0', $output);
$locateVisits->shouldHaveBeenCalledOnce();
$resolveIpLocation->shouldHaveBeenCalledOnce();
}
/**
* @test
* @dataProvider provideIgnoredAddresses
*/
public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message)
{
$visit = new Visit(new ShortUrl(''), new Visitor('', '', $address));
$location = new VisitLocation([]);
$locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(
function (array $args) use ($visit, $location) {
$firstCallback = array_shift($args);
$firstCallback($visit);
$secondCallback = array_shift($args);
$secondCallback($location, $visit);
}
);
$resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willReturn([]);
try {
$this->commandTester->execute([
'command' => 'visit:process',
], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
} catch (Throwable $e) {
$output = $this->commandTester->getDisplay();
$this->assertInstanceOf(IpCannotBeLocatedException::class, $e);
$this->assertContains($message, $output);
$locateVisits->shouldHaveBeenCalledOnce();
$resolveIpLocation->shouldNotHaveBeenCalled();
}
}
public function provideIgnoredAddresses(): array
{
return [
['', 'Ignored visit with no IP address'],
[null, 'Ignored visit with no IP address'],
[IpAddress::LOCALHOST, 'Ignored localhost address'],
];
}
/**
* @test
*/
public function localhostAndEmptyAddressIsIgnored()
public function errorWhileLocatingIpIsDisplayed()
{
$shortUrl = new ShortUrl('');
$visit = new Visit(new ShortUrl(''), new Visitor('', '', '1.2.3.4'));
$location = new VisitLocation([]);
$visits = [
new Visit($shortUrl, new Visitor('', '', '1.2.3.4')),
new Visit($shortUrl, new Visitor('', '', '4.3.2.1')),
new Visit($shortUrl, new Visitor('', '', '12.34.56.78')),
new Visit($shortUrl, new Visitor('', '', '127.0.0.1')),
new Visit($shortUrl, new Visitor('', '', '127.0.0.1')),
new Visit($shortUrl, new Visitor('', '', '')),
new Visit($shortUrl, new Visitor('', '', null)),
];
$this->visitService->getUnlocatedVisits()->willReturn($visits)
->shouldBeCalledOnce();
$locateVisits = $this->visitService->locateVisits(Argument::cetera())->will(
function (array $args) use ($visit, $location) {
$firstCallback = array_shift($args);
$firstCallback($visit);
$this->visitService->saveVisit(Argument::any())->shouldBeCalledTimes(count($visits) - 4);
$this->ipResolver->resolveIpLocation(Argument::any())->willReturn([])
->shouldBeCalledTimes(count($visits) - 4);
$secondCallback = array_shift($args);
$secondCallback($location, $visit);
}
);
$resolveIpLocation = $this->ipResolver->resolveIpLocation(Argument::any())->willThrow(WrongIpException::class);
$this->commandTester->execute([
'command' => 'visit:process',
], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
$output = $this->commandTester->getDisplay();
$this->assertContains('Ignored localhost address', $output);
$this->assertContains('Ignored visit with no IP address', $output);
try {
$this->commandTester->execute([
'command' => 'visit:process',
], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]);
} catch (Throwable $e) {
$output = $this->commandTester->getDisplay();
$this->assertInstanceOf(IpCannotBeLocatedException::class, $e);
$this->assertContains('An error occurred while locating IP. Skipped', $output);
$locateVisits->shouldHaveBeenCalledOnce();
$resolveIpLocation->shouldHaveBeenCalledOnce();
}
}
}

2
module/Core/src/Entity/Visit.php
View file

@ -93,7 +93,7 @@ class Visit extends AbstractEntity implements JsonSerializable
return $this->visitLocation;
}
public function setVisitLocation(VisitLocation $visitLocation): self
public function locate(VisitLocation $visitLocation): self
{
$this->visitLocation = $visitLocation;
return $this;

8
module/Core/src/Exception/IpCannotBeLocatedException.php Normal file
View file

@ -0,0 +1,8 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Exception;
class IpCannotBeLocatedException extends RuntimeException
{
}

7
module/Core/src/Repository/VisitRepository.php
View file

@ -10,15 +10,12 @@ use Shlinkio\Shlink\Core\Entity\Visit;
class VisitRepository extends EntityRepository implements VisitRepositoryInterface
{
/**
* @return Visit[]
*/
public function findUnlocatedVisits(): array
public function findUnlocatedVisits(): iterable
{
$qb = $this->createQueryBuilder('v');
$qb->where($qb->expr()->isNull('v.visitLocation'));
return $qb->getQuery()->getResult();
return $qb->getQuery()->iterate();
}
/**

5
module/Core/src/Repository/VisitRepositoryInterface.php
View file

@ -10,10 +10,7 @@ use Shlinkio\Shlink\Core\Entity\Visit;
interface VisitRepositoryInterface extends ObjectRepository
{
/**
* @return Visit[]
*/
public function findUnlocatedVisits(): array;
public function findUnlocatedVisits(): iterable;
/**
* @param ShortUrl|int $shortUrl

34
module/Core/src/Service/VisitService.php
View file

@ -5,6 +5,8 @@ namespace Shlinkio\Shlink\Core\Service;
use Doctrine\ORM\EntityManagerInterface;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
use Shlinkio\Shlink\Core\Repository\VisitRepository;
class VisitService implements VisitServiceInterface
@ -19,22 +21,36 @@ class VisitService implements VisitServiceInterface
$this->em = $em;
}
/**
* @return Visit[]
*/
public function getUnlocatedVisits()
public function locateVisits(callable $getGeolocationData, ?callable $locatedVisit = null): void
{
/** @var VisitRepository $repo */
$repo = $this->em->getRepository(Visit::class);
return $repo->findUnlocatedVisits();
$results = $repo->findUnlocatedVisits();
foreach ($results as [$visit]) {
try {
$locationData = $getGeolocationData($visit);
} catch (IpCannotBeLocatedException $e) {
// Skip if the visit's IP could not be located
continue;
}
$location = new VisitLocation($locationData);
$this->locateVisit($visit, $location, $locatedVisit);
}
}
/**
* @param Visit $visit
*/
public function saveVisit(Visit $visit)
private function locateVisit(Visit $visit, VisitLocation $location, ?callable $locatedVisit): void
{
$visit->locate($location);
$this->em->persist($visit);
$this->em->flush();
if ($locatedVisit !== null) {
$locatedVisit($location, $visit);
}
$this->em->clear();
}
}

12
module/Core/src/Service/VisitServiceInterface.php
View file

@ -3,17 +3,7 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Service;
use Shlinkio\Shlink\Core\Entity\Visit;
interface VisitServiceInterface
{
/**
* @return Visit[]
*/
public function getUnlocatedVisits();
/**
* @param Visit $visit
*/
public function saveVisit(Visit $visit);
public function locateVisits(callable $getGeolocationData, ?callable $locatedVisit = null): void;
}

10
module/Core/test-func/Repository/VisitRepositoryTest.php
View file

@ -45,14 +45,20 @@ class VisitRepositoryTest extends DatabaseTestCase
if ($i % 2 === 0) {
$location = new VisitLocation([]);
$this->getEntityManager()->persist($location);
$visit->setVisitLocation($location);
$visit->locate($location);
}
$this->getEntityManager()->persist($visit);
}
$this->getEntityManager()->flush();
$this->assertCount(3, $this->repo->findUnlocatedVisits());
$resultsCount = 0;
$results = $this->repo->findUnlocatedVisits();
foreach ($results as $value) {
$resultsCount++;
}
$this->assertEquals(3, $resultsCount);
}
/**

70
module/Core/test/Service/VisitServiceTest.php
View file

@ -5,12 +5,18 @@ namespace ShlinkioTest\Shlink\Core\Service;
use Doctrine\ORM\EntityManager;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\Prophecy\ObjectProphecy;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Entity\VisitLocation;
use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Repository\VisitRepository;
use Shlinkio\Shlink\Core\Service\VisitService;
use function array_shift;
use function count;
use function func_get_args;
class VisitServiceTest extends TestCase
{
@ -32,22 +38,68 @@ class VisitServiceTest extends TestCase
/**
* @test
*/
public function saveVisitsPersistsProvidedVisit()
public function locateVisitsIteratesAndLocatesUnlocatedVisits()
{
$visit = new Visit(new ShortUrl(''), Visitor::emptyInstance());
$this->em->persist($visit)->shouldBeCalledOnce();
$this->em->flush()->shouldBeCalledOnce();
$this->visitService->saveVisit($visit);
$unlocatedVisits = [
[new Visit(new ShortUrl('foo'), Visitor::emptyInstance())],
[new Visit(new ShortUrl('bar'), Visitor::emptyInstance())],
];
$repo = $this->prophesize(VisitRepository::class);
$findUnlocatedVisits = $repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
$getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
$persist = $this->em->persist(Argument::type(Visit::class))->will(function () {
});
$flush = $this->em->flush()->will(function () {
});
$clear = $this->em->clear()->will(function () {
});
$this->visitService->locateVisits(function () {
return [];
}, function () {
$args = func_get_args();
$this->assertInstanceOf(VisitLocation::class, array_shift($args));
$this->assertInstanceOf(Visit::class, array_shift($args));
});
$findUnlocatedVisits->shouldHaveBeenCalledOnce();
$getRepo->shouldHaveBeenCalledOnce();
$persist->shouldHaveBeenCalledTimes(count($unlocatedVisits));
$flush->shouldHaveBeenCalledTimes(count($unlocatedVisits));
$clear->shouldHaveBeenCalledTimes(count($unlocatedVisits));
}
/**
* @test
*/
public function getUnlocatedVisitsFallbacksToRepository()
public function visitsWhichCannotBeLocatedAreIgnored()
{
$unlocatedVisits = [
[new Visit(new ShortUrl('foo'), Visitor::emptyInstance())],
];
$repo = $this->prophesize(VisitRepository::class);
$repo->findUnlocatedVisits()->shouldBeCalledOnce();
$this->em->getRepository(Visit::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
$this->visitService->getUnlocatedVisits();
$findUnlocatedVisits = $repo->findUnlocatedVisits()->willReturn($unlocatedVisits);
$getRepo = $this->em->getRepository(Visit::class)->willReturn($repo->reveal());
$persist = $this->em->persist(Argument::type(Visit::class))->will(function () {
});
$flush = $this->em->flush()->will(function () {
});
$clear = $this->em->clear()->will(function () {
});
$this->visitService->locateVisits(function () {
throw new IpCannotBeLocatedException('Cannot be located');
});
$findUnlocatedVisits->shouldHaveBeenCalledOnce();
$getRepo->shouldHaveBeenCalledOnce();
$persist->shouldNotHaveBeenCalled();
$flush->shouldNotHaveBeenCalled();
$clear->shouldNotHaveBeenCalled();
}
}