Fix ImportedLinksProcessorTest

shlinkio · Dec 22, 2024 · 2807b9c · 2807b9c
1 parent 2f39aff
commit 2807b9c
Show file tree

Hide file tree

Showing 11 changed files with 80 additions and 16 deletions.
diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php
@@ -74,7 +74,7 @@ public function deleteIsRetriedWhenThresholdIsReachedAndQuestionIsAccepted(
         $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode);
         $this->service->expects($this->exactly($expectedDeleteCalls))->method('deleteByShortCode')->with(
             $identifier,
-            $this->isType('bool'),
+            $this->isBool(),
         )->willReturnCallback(function ($_, bool $ignoreThreshold) use ($shortCode): void {
             if (!$ignoreThreshold) {
                 throw Exception\DeleteShortUrlException::fromVisitsThreshold(

diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php
@@ -47,7 +47,7 @@ protected function setUp(): void
 
         $locker = $this->createMock(Lock\LockFactory::class);
         $this->lock = $this->createMock(Lock\SharedLockInterface::class);
-        $locker->method('createLock')->with($this->isType('string'), 600.0, false)->willReturn($this->lock);
+        $locker->method('createLock')->with($this->isString(), 600.0, false)->willReturn($this->lock);
 
         $command = new LocateVisitsCommand($this->visitService, $this->visitToLocation, $locker);
 

diff --git a/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php b/module/CLI/test/RedirectRule/RedirectRuleHandlerTest.php
@@ -77,7 +77,7 @@ public function rulesAreDisplayedWhenRulesListIsEmpty(
         $this->io->expects($this->once())->method('choice')->willReturn($action->value);
         $this->io->expects($this->never())->method('newLine');
         $this->io->expects($this->never())->method('text');
-        $this->io->expects($this->once())->method('table')->with($this->isType('array'), [
+        $this->io->expects($this->once())->method('table')->with($this->isArray(), [
             ['1', $comment($this->cond1->toHumanFriendly()), 'https://example.com/one'],
             [
                 '2',

diff --git a/module/Core/src/Importer/ShortUrlImporting.php b/module/Core/src/Importer/ShortUrlImporting.php
@@ -14,6 +14,7 @@
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectRule;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit;
 
+use function count;
 use function Shlinkio\Shlink\Core\ArrayUtils\map;
 use function Shlinkio\Shlink\Core\normalizeDate;
 use function sprintf;
@@ -69,6 +70,10 @@ public function importRedirectRules(
         EntityManagerInterface $em,
         ShortUrlRedirectRuleServiceInterface $redirectRuleService,
     ): void {
+        if ($this->isNew && count($rules) === 0) {
+            return;
+        }
+
         $shortUrl = $this->resolveShortUrl($em);
         $redirectRules = map(
             $rules,

diff --git a/module/Core/src/RedirectRule/Entity/RedirectCondition.php b/module/Core/src/RedirectRule/Entity/RedirectCondition.php
@@ -24,7 +24,7 @@
 class RedirectCondition extends AbstractEntity implements JsonSerializable
 {
     private function __construct(
-        private readonly RedirectConditionType $type,
+        public readonly RedirectConditionType $type,
         private readonly string $matchValue,
         private readonly string|null $matchKey = null,
     ) {

diff --git a/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php b/module/Core/test/Geolocation/GeolocationDbUpdaterTest.php
@@ -291,7 +291,7 @@ public static function provideTrackingOptions(): iterable
     private function geolocationDbUpdater(TrackingOptions|null $options = null): GeolocationDbUpdater
     {
         $locker = $this->createMock(Lock\LockFactory::class);
-        $locker->method('createLock')->with($this->isType('string'))->willReturn($this->lock);
+        $locker->method('createLock')->with($this->isString())->willReturn($this->lock);
 
         return new GeolocationDbUpdater($this->dbUpdater, $locker, $options ?? new TrackingOptions(), $this->em, 3);
     }

diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php
@@ -14,6 +14,9 @@
 use PHPUnit\Framework\TestCase;
 use RuntimeException;
 use Shlinkio\Shlink\Core\Importer\ImportedLinksProcessor;
+use Shlinkio\Shlink\Core\Model\DeviceType;
+use Shlinkio\Shlink\Core\RedirectRule\Entity\ShortUrlRedirectRule;
+use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType;
 use Shlinkio\Shlink\Core\RedirectRule\ShortUrlRedirectRuleServiceInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortCodeUniquenessHelperInterface;
@@ -24,6 +27,8 @@
 use Shlinkio\Shlink\Core\Visit\Model\Visitor;
 use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkOrphanVisit;
+use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition;
+use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectRule;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit;
 use Shlinkio\Shlink\Importer\Model\ImportResult;
@@ -71,10 +76,31 @@ protected function setUp(): void
     #[Test]
     public function newUrlsWithNoErrorsAreAllPersisted(): void
     {
+        $now = Chronos::now();
         $urls = [
-            new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null),
-            new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', 'foo'),
-            new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null),
+            new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], $now, null, 'foo', null),
+            new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], $now, null, 'bar', 'foo'),
+            new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], $now, null, 'baz', null, redirectRules: [
+                new ImportedShlinkRedirectRule(
+                    longUrl: 'https://example.com/android',
+                    conditions: [
+                        new ImportedShlinkRedirectCondition(
+                            RedirectConditionType::DEVICE->value,
+                            DeviceType::ANDROID->value,
+                        ),
+                    ],
+                ),
+                new ImportedShlinkRedirectRule(
+                    longUrl: 'https://example.com/spain',
+                    conditions: [
+                        new ImportedShlinkRedirectCondition(
+                            RedirectConditionType::GEOLOCATION_COUNTRY_CODE->value,
+                            'ES',
+                        ),
+                        new ImportedShlinkRedirectCondition(RedirectConditionType::LANGUAGE->value, 'es-ES'),
+                    ],
+                ),
+            ]),
         ];
         $expectedCalls = count($urls);
 
@@ -86,7 +112,19 @@ public function newUrlsWithNoErrorsAreAllPersisted(): void
         $this->em->expects($this->exactly($expectedCalls))->method('persist')->with(
             $this->isInstanceOf(ShortUrl::class),
         );
-        $this->io->expects($this->exactly($expectedCalls))->method('text')->with($this->isType('string'));
+        $this->io->expects($this->exactly($expectedCalls))->method('text')->with($this->isString());
+        $this->redirectRuleService->expects($this->once())->method('saveRulesForShortUrl')->with(
+            $this->isInstanceOf(ShortUrl::class),
+            $this->callback(function (array $rules): bool {
+                Assert::assertCount(2, $rules);
+                Assert::assertInstanceOf(ShortUrlRedirectRule::class, $rules[0]);
+                Assert::assertInstanceOf(ShortUrlRedirectRule::class, $rules[1]);
+                Assert::assertCount(1, $rules[0]->mapConditions(fn ($c) => $c));
+                Assert::assertCount(2, $rules[1]->mapConditions(fn ($c) => $c));
+
+                return true;
+            }),
+        );
 
         $this->processor->process($this->io, ImportResult::withShortUrls($urls), $this->buildParams());
     }
@@ -243,7 +281,8 @@ public function visitsArePersistedWithProperShortUrl(ShortUrl $originalShortUrl,
         if (!$originalShortUrl->getId()) {
             $this->em->expects($this->never())->method('find');
         } else {
-            $this->em->expects($this->exactly(2))->method('find')->willReturn($foundShortUrl);
+            // 3 times: Initial short URL checking, before creating redirect rules, before creating visits
+            $this->em->expects($this->exactly(3))->method('find')->willReturn($foundShortUrl);
         }
         $this->em->expects($this->once())->method('persist')->willReturnCallback(
             static fn (Visit $visit)  => Assert::assertSame(

diff --git a/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php b/module/Core/test/RedirectRule/Entity/RedirectConditionTest.php
@@ -9,6 +9,8 @@
 use PHPUnit\Framework\TestCase;
 use Shlinkio\Shlink\Core\Model\DeviceType;
 use Shlinkio\Shlink\Core\RedirectRule\Entity\RedirectCondition;
+use Shlinkio\Shlink\Core\RedirectRule\Model\RedirectConditionType;
+use Shlinkio\Shlink\Importer\Model\ImportedShlinkRedirectCondition;
 use Shlinkio\Shlink\IpGeolocation\Model\Location;
 
 use const Shlinkio\Shlink\IP_ADDRESS_REQUEST_ATTRIBUTE;
@@ -133,4 +135,22 @@ public static function provideVisitsWithCity(): iterable
         yield 'matching location' => [new Location(city: 'Madrid'), 'Madrid', true];
         yield 'matching case-insensitive' => [new Location(city: 'Los Angeles'), 'los angeles', true];
     }
+
+    #[Test]
+    #[TestWith(['invalid', null])]
+    #[TestWith([RedirectConditionType::DEVICE->value, RedirectConditionType::DEVICE])]
+    #[TestWith([RedirectConditionType::LANGUAGE->value, RedirectConditionType::LANGUAGE])]
+    #[TestWith([RedirectConditionType::QUERY_PARAM->value, RedirectConditionType::QUERY_PARAM])]
+    #[TestWith([RedirectConditionType::IP_ADDRESS->value, RedirectConditionType::IP_ADDRESS])]
+    #[TestWith(
+        [RedirectConditionType::GEOLOCATION_COUNTRY_CODE->value, RedirectConditionType::GEOLOCATION_COUNTRY_CODE],
+    )]
+    #[TestWith([RedirectConditionType::GEOLOCATION_CITY_NAME->value, RedirectConditionType::GEOLOCATION_CITY_NAME])]
+    public function canBeCreatedFromImport(string $type, RedirectConditionType|null $expectedType): void
+    {
+        $condition = RedirectCondition::fromImport(
+            new ImportedShlinkRedirectCondition($type, DeviceType::ANDROID->value, ''),
+        );
+        self::assertEquals($expectedType, $condition?->type);
+    }
 }
diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php
@@ -78,7 +78,7 @@ public function findsAndPersistsTagsWrappedIntoCollection(array $tags, array $ex
 
         $tagRepo = $this->createMock(TagRepository::class);
         $tagRepo->expects($this->exactly($expectedLookedOutTags))->method('findOneBy')->with(
-            $this->isType('array'),
+            $this->isArray(),
         )->willReturnCallback(function (array $criteria): Tag|null {
             ['name' => $name] = $criteria;
             return $name === 'foo' ? new Tag($name) : null;
@@ -115,7 +115,7 @@ public function returnsEmptyCollectionWhenProvidingEmptyListOfTags(): void
     public function newDomainsAreMemoizedUntilStateIsCleared(): void
     {
         $repo = $this->createMock(DomainRepository::class);
-        $repo->expects($this->exactly(3))->method('findOneBy')->with($this->isType('array'))->willReturn(null);
+        $repo->expects($this->exactly(3))->method('findOneBy')->with($this->isArray())->willReturn(null);
         $this->em->method('getRepository')->with(Domain::class)->willReturn($repo);
 
         $authority = 'foo.com';
@@ -134,7 +134,7 @@ public function newDomainsAreMemoizedUntilStateIsCleared(): void
     public function newTagsAreMemoizedUntilStateIsCleared(): void
     {
         $tagRepo = $this->createMock(TagRepository::class);
-        $tagRepo->expects($this->exactly(6))->method('findOneBy')->with($this->isType('array'))->willReturn(null);
+        $tagRepo->expects($this->exactly(6))->method('findOneBy')->with($this->isArray())->willReturn(null);
         $this->em->method('getRepository')->with(Tag::class)->willReturn($tagRepo);
 
         $tags = ['foo', 'bar'];

diff --git a/module/Core/test/ShortUrl/UrlShortenerTest.php b/module/Core/test/ShortUrl/UrlShortenerTest.php
@@ -38,7 +38,7 @@ protected function setUp(): void
         // FIXME Should use the interface, but it doe snot define wrapInTransaction explicitly
         $this->em = $this->createMock(EntityManager::class);
         $this->em->method('persist')->willReturnCallback(fn (ShortUrl $shortUrl) => $shortUrl->setId('10'));
-        $this->em->method('wrapInTransaction')->with($this->isType('callable'))->willReturnCallback(
+        $this->em->method('wrapInTransaction')->with($this->isCallable())->willReturnCallback(
             fn (callable $callback) => $callback(),
         );
 

diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php
@@ -45,7 +45,7 @@ protected function setUp(): void
     public function apiKeyIsProperlyCreated(Chronos|null $date, string|null $name, array $roles): void
     {
         $this->repo->expects($this->once())->method('nameExists')->with(
-            ! empty($name) ? $name : $this->isType('string'),
+            ! empty($name) ? $name : $this->isString(),
         )->willReturn(false);
         $this->em->expects($this->once())->method('persist')->with($this->isInstanceOf(ApiKey::class));
 
@@ -83,7 +83,7 @@ public function autoGeneratedNameIsRegeneratedIfAlreadyExists(): void
     {
         $callCount = 0;
         $this->repo->expects($this->exactly(3))->method('nameExists')->with(
-            $this->isType('string'),
+            $this->isString(),
         )->willReturnCallback(function () use (&$callCount): bool {
             $callCount++;
             return $callCount < 3;