Merge pull request #387 from acelaya/feature/fix-check-exists

Feature/fix check exists
This commit is contained in:
Alejandro Celaya 2019-03-30 08:04:44 +01:00 committed by GitHub
commit 20c3bde036
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 29 deletions

View file

@ -4,16 +4,17 @@ 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).
## [Unreleased]
## 1.16.3 - 2019-03-30
#### Added
* [#153](https://github.com/shlinkio/shlink/issues/153) Updated to [doctrine/migrations](https://github.com/doctrine/migrations) version 2.0.0
* [#376](https://github.com/shlinkio/shlink/issues/376) Allowed `visit:update-db` command to not return an error exit code even if download fails, by passing the `-i` flag.
* *Nothing*
#### Changed
* *Nothing*
* [#153](https://github.com/shlinkio/shlink/issues/153) Updated to [doctrine/migrations](https://github.com/doctrine/migrations) version 2.0.0
* [#376](https://github.com/shlinkio/shlink/issues/376) Allowed `visit:update-db` command to not return an error exit code even if download fails, by passing the `-i` flag.
* [#341](https://github.com/shlinkio/shlink/issues/341) Improved database tests so that they are executed against all supported database engines.
#### Deprecated
@ -25,7 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this
#### Fixed
* *Nothing*
* [#382](https://github.com/shlinkio/shlink/issues/382) Fixed existing short URLs not properly checked when providing the `findIfExists` flag.
## 1.16.2 - 2019-03-05

View file

@ -112,32 +112,39 @@ class UrlShortener implements UrlShortenerInterface
if ($meta->hasCustomSlug()) {
$criteria['shortCode'] = $meta->getCustomSlug();
}
/** @var ShortUrl|null $shortUrl */
$shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy($criteria);
if ($shortUrl === null) {
/** @var ShortUrl[] $shortUrls */
$shortUrls = $this->em->getRepository(ShortUrl::class)->findBy($criteria);
if (empty($shortUrls)) {
return null;
}
if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) {
return null;
}
if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) {
return null;
}
if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($shortUrl->getValidUntil())) {
return null;
}
// Iterate short URLs until one that matches is found, or return null otherwise
return array_reduce($shortUrls, function (?ShortUrl $found, ShortUrl $shortUrl) use ($tags, $meta) {
if ($found) {
return $found;
}
$shortUrlTags = invoke($shortUrl->getTags(), '__toString');
$hasAllTags = count($shortUrlTags) === count($tags) && array_reduce(
$tags,
function (bool $hasAllTags, string $tag) use ($shortUrlTags) {
return $hasAllTags && contains($shortUrlTags, $tag);
},
true
);
if ($meta->hasMaxVisits() && $meta->getMaxVisits() !== $shortUrl->getMaxVisits()) {
return null;
}
if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($shortUrl->getValidSince())) {
return null;
}
if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($shortUrl->getValidUntil())) {
return null;
}
return $hasAllTags ? $shortUrl : null;
$shortUrlTags = invoke($shortUrl->getTags(), '__toString');
$hasAllTags = count($shortUrlTags) === count($tags) && array_reduce(
$tags,
function (bool $hasAllTags, string $tag) use ($shortUrlTags) {
return $hasAllTags && contains($shortUrlTags, $tag);
},
true
);
return $hasAllTags ? $shortUrl : null;
});
}
private function checkUrlExists(string $url): void

View file

@ -27,6 +27,8 @@ use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
use Shlinkio\Shlink\Core\Service\UrlShortener;
use Zend\Diactoros\Uri;
use function array_map;
class UrlShortenerTest extends TestCase
{
/** @var UrlShortener */
@ -121,7 +123,7 @@ class UrlShortenerTest extends TestCase
{
$repo = $this->prophesize(ShortUrlRepository::class);
$countBySlug = $repo->count(['shortCode' => 'custom-slug'])->willReturn(1);
$repo->findOneBy(Argument::cetera())->willReturn(null);
$repo->findBy(Argument::cetera())->willReturn([]);
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$countBySlug->shouldBeCalledOnce();
@ -146,20 +148,23 @@ class UrlShortenerTest extends TestCase
?ShortUrl $expected
): void {
$repo = $this->prophesize(ShortUrlRepository::class);
$findExisting = $repo->findOneBy(Argument::any())->willReturn($expected);
$findExisting = $repo->findBy(Argument::any())->willReturn($expected !== null ? [$expected] : []);
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta);
$this->assertSame($expected, $result);
$findExisting->shouldHaveBeenCalledOnce();
$getRepo->shouldHaveBeenCalledOnce();
if ($expected) {
$this->assertSame($expected, $result);
}
}
public function provideExistingShortUrls(): iterable
{
$url = 'http://foo.com';
yield [$url, [], ShortUrlMeta::createFromRawData(['findIfExists' => true]), null];
yield [$url, [], ShortUrlMeta::createFromRawData(['findIfExists' => true]), new ShortUrl($url)];
yield [$url, [], ShortUrlMeta::createFromRawData(
['findIfExists' => true, 'customSlug' => 'foo']
@ -203,6 +208,37 @@ class UrlShortenerTest extends TestCase
];
}
/** @test */
public function properExistingShortUrlIsReturnedWhenMultipleMatch(): void
{
$url = 'http://foo.com';
$tags = ['baz', 'foo', 'bar'];
$meta = ShortUrlMeta::createFromRawData([
'findIfExists' => true,
'validUntil' => Chronos::parse('2017-01-01'),
'maxVisits' => 4,
]);
$tagsCollection = new ArrayCollection(array_map(function (string $tag) {
return new Tag($tag);
}, $tags));
$expected = (new ShortUrl($url, $meta))->setTags($tagsCollection);
$repo = $this->prophesize(ShortUrlRepository::class);
$findExisting = $repo->findBy(Argument::any())->willReturn([
new ShortUrl($url),
new ShortUrl($url, $meta),
$expected,
(new ShortUrl($url))->setTags($tagsCollection),
]);
$getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal());
$result = $this->urlShortener->urlToShortCode(new Uri($url), $tags, $meta);
$this->assertSame($expected, $result);
$findExisting->shouldHaveBeenCalledOnce();
$getRepo->shouldHaveBeenCalledOnce();
}
/** @test */
public function shortCodeIsProperlyParsed(): void
{