Added test covering imported short URLs with visits

composer.json

@@ -124,6 +124,7 @@
         ],
         "test:unit": "@php vendor/bin/phpunit --order-by=random --colors=always --coverage-php build/coverage-unit.cov --testdox",
         "test:unit:ci": "@test:unit --coverage-xml=build/coverage-unit/coverage-xml --log-junit=build/coverage-unit/junit.xml",
+        "test:unit:pretty": "@php vendor/bin/phpunit --order-by=random --colors=always --coverage-html build/coverage-unit-html",
         "test:db": "@parallel test:db:sqlite:ci test:db:mysql test:db:maria test:db:postgres test:db:ms",
         "test:db:sqlite": "APP_ENV=test php vendor/bin/phpunit --order-by=random --colors=always --testdox -c phpunit-db.xml",
         "test:db:sqlite:ci": "@test:db:sqlite --coverage-php build/coverage-db.cov --coverage-xml=build/coverage-db/coverage-xml --log-junit=build/coverage-db/junit.xml",
@@ -132,7 +133,6 @@
         "test:db:postgres": "DB_DRIVER=postgres composer test:db:sqlite",
         "test:db:ms": "DB_DRIVER=mssql composer test:db:sqlite",
         "test:api": "bin/test/run-api-tests.sh",
-        "test:unit:pretty": "@php vendor/bin/phpunit --order-by=random --colors=always --coverage-html build/coverage-unit-html",
         "infect:ci:base": "infection --threads=4 --log-verbosity=default --only-covered --skip-initial-tests",
         "infect:ci:unit": "@infect:ci:base --coverage=build/coverage-unit --min-msi=80",
         "infect:ci:db": "@infect:ci:base --coverage=build/coverage-db --min-msi=95 --configuration=infection-db.json",

module/Core/src/Entity/Visit.php

@@ -150,6 +150,15 @@ class Visit extends AbstractEntity implements JsonSerializable
         return $this->type;
     }
 
+    /**
+     * Needed only for ArrayCollections to be able to apply criteria filtering
+     * @internal
+     */
+    public function getType(): string
+    {
+        return $this->type();
+    }
+
     public function jsonSerialize(): array
     {
         return [

module/Core/src/Importer/ImportedLinksProcessor.php

@@ -49,9 +49,7 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface
 
         /** @var ImportedShlinkUrl $importedUrl */
         foreach ($iterable as $importedUrl) {
-            $longUrl = $importedUrl->longUrl();
+            $skipOnShortCodeConflict = static function () use ($io, $importedUrl): bool {
-
-            $generateNewIfDuplicated = static function () use ($io, $importedUrl): bool {
                 $action = $io->choice(sprintf(
                     'Failed to import URL "%s" because its short-code "%s" is already in use. Do you want to generate '
                     . 'a new one or skip it?',
@@ -59,10 +57,11 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface
                     $importedUrl->shortCode(),
                 ), ['Generate new short-code', 'Skip'], 1);
 
-                return $action !== 'Skip';
+                return $action === 'Skip';
             };
-            [$shortUrl, $isNew] = $this->getOrCreateShortUrl($importedUrl, $importShortCodes, $generateNewIfDuplicated);
+            [$shortUrl, $isNew] = $this->getOrCreateShortUrl($importedUrl, $importShortCodes, $skipOnShortCodeConflict);
 
+            $longUrl = $importedUrl->longUrl();
             if ($shortUrl === null) {
                 $io->text(sprintf('%s: <fg=red>Error</>', $longUrl));
                 continue;
@@ -93,15 +92,15 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface
     private function getOrCreateShortUrl(
         ImportedShlinkUrl $importedUrl,
         bool $importShortCodes,
-        callable $generateNewIfDuplicated
+        callable $skipOnShortCodeConflict
     ): array {
-        $existingShortUrl = $this->shortUrlRepo->findOneByImportedUrl($importedUrl);
+        $alreadyImportedShortUrl = $this->shortUrlRepo->findOneByImportedUrl($importedUrl);
-        if ($existingShortUrl !== null) {
+        if ($alreadyImportedShortUrl !== null) {
-            return [$existingShortUrl, false];
+            return [$alreadyImportedShortUrl, false];
         }
 
         $shortUrl = ShortUrl::fromImport($importedUrl, $importShortCodes, $this->relationResolver);
-        if (! $this->handleShortCodeUniqueness($shortUrl, $importShortCodes, $generateNewIfDuplicated)) {
+        if (! $this->handleShortCodeUniqueness($shortUrl, $importShortCodes, $skipOnShortCodeConflict)) {
             return [null, false];
         }
 
@@ -112,13 +111,13 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface
     private function handleShortCodeUniqueness(
         ShortUrl $shortUrl,
         bool $importShortCodes,
-        callable $generateNewIfDuplicated
+        callable $skipOnShortCodeConflict
     ): bool {
         if ($this->shortCodeHelper->ensureShortCodeUniqueness($shortUrl, $importShortCodes)) {
             return true;
         }
 
-        if (! $generateNewIfDuplicated()) {
+        if ($skipOnShortCodeConflict()) {
             return false;
         }
 

module/Core/test/Importer/ImportedLinksProcessorTest.php

@@ -5,18 +5,21 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Core\Importer;
 
 use Cake\Chronos\Chronos;
+use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\ORM\EntityManagerInterface;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Importer\ImportedLinksProcessor;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
 use Shlinkio\Shlink\Core\Service\ShortUrl\ShortCodeHelperInterface;
 use Shlinkio\Shlink\Core\ShortUrl\Resolver\SimpleShortUrlRelationResolver;
 use Shlinkio\Shlink\Core\Util\DoctrineBatchHelperInterface;
 use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl;
+use Shlinkio\Shlink\Importer\Model\ImportedShlinkVisit;
 use Symfony\Component\Console\Style\StyleInterface;
 
 use function count;
@@ -86,7 +89,6 @@ class ImportedLinksProcessorTest extends TestCase
             new ImportedShlinkUrl('', 'baz2', [], Chronos::now(), null, 'baz2', null),
             new ImportedShlinkUrl('', 'baz3', [], Chronos::now(), null, 'baz3', null),
         ];
-        $contains = fn (string $needle) => fn (string $text) => str_contains($text, $needle);
 
         $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->will(
             function (array $args): ?ShortUrl {
@@ -104,8 +106,8 @@ class ImportedLinksProcessorTest extends TestCase
         $importedUrlExists->shouldHaveBeenCalledTimes(count($urls));
         $ensureUniqueness->shouldHaveBeenCalledTimes(2);
         $persist->shouldHaveBeenCalledTimes(2);
-        $this->io->text(Argument::that($contains('Skipped')))->shouldHaveBeenCalledTimes(3);
+        $this->io->text(Argument::containingString('Skipped'))->shouldHaveBeenCalledTimes(3);
-        $this->io->text(Argument::that($contains('Imported')))->shouldHaveBeenCalledTimes(2);
+        $this->io->text(Argument::containingString('Imported'))->shouldHaveBeenCalledTimes(2);
     }
 
     /** @test */
@@ -118,7 +120,6 @@ class ImportedLinksProcessorTest extends TestCase
             new ImportedShlinkUrl('', 'baz2', [], Chronos::now(), null, 'baz2', null),
             new ImportedShlinkUrl('', 'baz3', [], Chronos::now(), null, 'baz3', 'bar'),
         ];
-        $contains = fn (string $needle) => fn (string $text) => str_contains($text, $needle);
 
         $importedUrlExists = $this->repo->findOneByImportedUrl(Argument::cetera())->willReturn(null);
         $failingEnsureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(
@@ -144,7 +145,70 @@ class ImportedLinksProcessorTest extends TestCase
         $successEnsureUniqueness->shouldHaveBeenCalledTimes(2);
         $choice->shouldHaveBeenCalledTimes(5);
         $persist->shouldHaveBeenCalledTimes(2);
-        $this->io->text(Argument::that($contains('Skipped')))->shouldHaveBeenCalledTimes(3);
+        $this->io->text(Argument::containingString('Error'))->shouldHaveBeenCalledTimes(3);
-        $this->io->text(Argument::that($contains('Imported')))->shouldHaveBeenCalledTimes(2);
+        $this->io->text(Argument::containingString('Imported'))->shouldHaveBeenCalledTimes(2);
+    }
+
+    /**
+     * @test
+     * @dataProvider provideUrlsWithVisits
+     */
+    public function properAmountOfVisitsIsImported(
+        ImportedShlinkUrl $importedUrl,
+        string $expectedOutput,
+        int $amountOfPersistedVisits,
+        ?ShortUrl $foundShortUrl
+    ): void {
+        $findExisting = $this->repo->findOneByImportedUrl(Argument::cetera())->willReturn($foundShortUrl);
+        $ensureUniqueness = $this->shortCodeHelper->ensureShortCodeUniqueness(Argument::cetera())->willReturn(true);
+        $persistUrl = $this->em->persist(Argument::type(ShortUrl::class));
+        $persistVisits = $this->em->persist(Argument::type(Visit::class));
+
+        $this->processor->process($this->io->reveal(), [$importedUrl], ['import_short_codes' => true]);
+
+        $findExisting->shouldHaveBeenCalledOnce();
+        $ensureUniqueness->shouldHaveBeenCalledTimes($foundShortUrl === null ? 1 : 0);
+        $persistUrl->shouldHaveBeenCalledTimes($foundShortUrl === null ? 1 : 0);
+        $persistVisits->shouldHaveBeenCalledTimes($amountOfPersistedVisits);
+        $this->io->text(Argument::containingString($expectedOutput))->shouldHaveBeenCalledOnce();
+    }
+
+    public function provideUrlsWithVisits(): iterable
+    {
+        $now = Chronos::now();
+        $createImportedUrl = fn (array $visits) => new ImportedShlinkUrl('', 's', [], $now, null, 's', null, $visits);
+
+        yield 'new short URL' => [$createImportedUrl([
+            new ImportedShlinkVisit('', '', $now, null),
+            new ImportedShlinkVisit('', '', $now, null),
+            new ImportedShlinkVisit('', '', $now, null),
+            new ImportedShlinkVisit('', '', $now, null),
+            new ImportedShlinkVisit('', '', $now, null),
+        ]), '<info>Imported</info> with <info>5</info> visits', 5, null];
+        yield 'existing short URL without previous imported visits' => [
+            $createImportedUrl([
+                new ImportedShlinkVisit('', '', $now, null),
+                new ImportedShlinkVisit('', '', $now, null),
+                new ImportedShlinkVisit('', '', $now->addDays(3), null),
+                new ImportedShlinkVisit('', '', $now->addDays(3), null),
+            ]),
+            '<comment>Skipped</comment>. Imported <info>4</info> visits',
+            4,
+            ShortUrl::createEmpty(),
+        ];
+        yield 'existing short URL with previous imported visits' => [
+            $createImportedUrl([
+                new ImportedShlinkVisit('', '', $now, null),
+                new ImportedShlinkVisit('', '', $now, null),
+                new ImportedShlinkVisit('', '', $now, null),
+                new ImportedShlinkVisit('', '', $now->addDays(3), null),
+                new ImportedShlinkVisit('', '', $now->addDays(3), null),
+            ]),
+            '<comment>Skipped</comment>. Imported <info>2</info> visits',
+            2,
+            ShortUrl::createEmpty()->setVisits(new ArrayCollection([
+                Visit::fromImport(ShortUrl::createEmpty(), new ImportedShlinkVisit('', '', $now, null)),
+            ])),
+        ];
     }
 }