Merge pull request #2068 from acelaya-forks/feature/modernize-entities

Feature/modernize entities
shlinkio · Mar 18, 2024 · 457a7a1 · 457a7a1
2 parents 98992c6 + cd38732
commit 457a7a1
Show file tree

Hide file tree

Showing 26 changed files with 199 additions and 280 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,23 @@ 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]
+### Added
+* *Nothing*
+
+### Changed
+* [#2034](https://github.com/shlinkio/shlink/issues/2034) Modernize entities, using constructor property promotion and readonly wherever possible.
+
+### Deprecated
+* *Nothing*
+
+### Removed
+* *Nothing*
+
+### Fixed
+* *Nothing*
+
+
 ## [4.0.3] - 2024-03-15
 ### Added
 * *Nothing*

diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php
@@ -50,11 +50,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         $enabledOnly = $input->getOption('enabled-only');
 
         $rows = array_map(function (ApiKey $apiKey) use ($enabledOnly) {
-            $expiration = $apiKey->getExpirationDate();
+            $expiration = $apiKey->expirationDate;
             $messagePattern = $this->determineMessagePattern($apiKey);
 
             // Set columns for this row
-            $rowData = [sprintf($messagePattern, $apiKey), sprintf($messagePattern, $apiKey->name() ?? '-')];
+            $rowData = [sprintf($messagePattern, $apiKey), sprintf($messagePattern, $apiKey->name ?? '-')];
             if (! $enabledOnly) {
                 $rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey));
             }

diff --git a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php
@@ -44,7 +44,7 @@ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRang
      */
     protected function mapExtraFields(Visit $visit): array
     {
-        $shortUrl = $visit->getShortUrl();
+        $shortUrl = $visit->shortUrl;
         return $shortUrl === null ? [] : ['shortUrl' => $this->shortUrlStringifier->stringify($shortUrl)];
     }
 }
diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php
@@ -229,11 +229,11 @@ private function resolveColumnsMap(InputInterface $input): array
         }
         if ($input->getOption('show-api-key')) {
             $columnsMap['API Key'] = static fn (array $_, ShortUrl $shortUrl): string =>
-                $shortUrl->authorApiKey()?->__toString() ?? '';
+                $shortUrl->authorApiKey?->__toString() ?? '';
         }
         if ($input->getOption('show-api-key-name')) {
             $columnsMap['API Key Name'] = static fn (array $_, ShortUrl $shortUrl): ?string =>
-                $shortUrl->authorApiKey()?->name();
+                $shortUrl->authorApiKey?->name;
         }
 
         return $columnsMap;

diff --git a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php
@@ -44,7 +44,7 @@ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRang
      */
     protected function mapExtraFields(Visit $visit): array
     {
-        $shortUrl = $visit->getShortUrl();
+        $shortUrl = $visit->shortUrl;
         return $shortUrl === null ? [] : ['shortUrl' => $this->shortUrlStringifier->stringify($shortUrl)];
     }
 }
diff --git a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php
@@ -55,8 +55,8 @@ private function resolveRowsAndHeaders(Paginator $paginator): array
 
             $rowData = [
                 ...$visit->jsonSerialize(),
-                'country' => $visit->getVisitLocation()?->getCountryName() ?? 'Unknown',
-                'city' => $visit->getVisitLocation()?->getCityName() ?? 'Unknown',
+                'country' => $visit->getVisitLocation()?->countryName ?? 'Unknown',
+                'city' => $visit->getVisitLocation()?->cityName ?? 'Unknown',
                 ...$extraFields,
             ];
 

diff --git a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php
@@ -40,7 +40,7 @@ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRang
      */
     protected function mapExtraFields(Visit $visit): array
     {
-        $shortUrl = $visit->getShortUrl();
+        $shortUrl = $visit->shortUrl;
         return $shortUrl === null ? [] : ['shortUrl' => $this->shortUrlStringifier->stringify($shortUrl)];
     }
 }
diff --git a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php
@@ -42,6 +42,6 @@ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRang
      */
     protected function mapExtraFields(Visit $visit): array
     {
-        return ['type' => $visit->type()->value];
+        return ['type' => $visit->type->value];
     }
 }
diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php
@@ -132,7 +132,7 @@ protected function lockedExecute(InputInterface $input, OutputInterface $output)
      */
     public function geolocateVisit(Visit $visit): Location
     {
-        $ipAddr = $visit->getRemoteAddr() ?? '?';
+        $ipAddr = $visit->remoteAddr ?? '?';
         $this->io->write(sprintf('Processing IP <fg=blue>%s</>', $ipAddr));
 
         try {
@@ -154,9 +154,9 @@ public function geolocateVisit(Visit $visit): Location
 
     public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void
     {
-        if (! $visitLocation->isEmpty()) {
-            $this->io->writeln(sprintf(' [<info>Address located in "%s"</info>]', $visitLocation->getCountryName()));
-        } elseif ($visit->hasRemoteAddr() && $visit->getRemoteAddr() !== IpAddress::LOCALHOST) {
+        if (! $visitLocation->isEmpty) {
+            $this->io->writeln(sprintf(' [<info>Address located in "%s"</info>]', $visitLocation->countryName));
+        } elseif ($visit->hasRemoteAddr() && $visit->remoteAddr !== IpAddress::LOCALHOST) {
             $this->io->writeln(' <comment>[Could not locate address]</comment>');
         }
     }

diff --git a/module/Core/src/Domain/Entity/Domain.php b/module/Core/src/Domain/Entity/Domain.php
@@ -11,12 +11,12 @@
 
 class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirectConfigInterface
 {
-    private ?string $baseUrlRedirect = null;
-    private ?string $regular404Redirect = null;
-    private ?string $invalidShortUrlRedirect = null;
-
-    private function __construct(public readonly string $authority)
-    {
+    private function __construct(
+        public readonly string $authority,
+        private ?string $baseUrlRedirect = null,
+        private ?string $regular404Redirect = null,
+        private ?string $invalidShortUrlRedirect = null,
+    ) {
     }
 
     public static function withAuthority(string $authority): self

diff --git a/module/Core/src/EventDispatcher/LocateVisit.php b/module/Core/src/EventDispatcher/LocateVisit.php
@@ -55,7 +55,7 @@ private function locateVisit(string $visitId, ?string $originalIpAddress, Visit
         }
 
         $isLocatable = $originalIpAddress !== null || $visit->isLocatable();
-        $addr = $originalIpAddress ?? $visit->getRemoteAddr() ?? '';
+        $addr = $originalIpAddress ?? $visit->remoteAddr ?? '';
 
         try {
             $location = $isLocatable ? $this->ipLocationResolver->resolveIpLocation($addr) : Location::emptyInstance();

diff --git a/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php b/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php
@@ -46,21 +46,21 @@ public function __invoke(VisitLocated $visitLocated): void
 
             $tracker
                 ->setUrl($this->resolveUrlToTrack($visit))
-                ->setCustomTrackingParameter('type', $visit->type()->value)
-                ->setUserAgent($visit->userAgent())
-                ->setUrlReferrer($visit->referer());
+                ->setCustomTrackingParameter('type', $visit->type->value)
+                ->setUserAgent($visit->userAgent)
+                ->setUrlReferrer($visit->referer);
 
             $location = $visit->getVisitLocation();
             if ($location !== null) {
                 $tracker
-                    ->setCity($location->getCityName())
-                    ->setCountry($location->getCountryName())
-                    ->setLatitude($location->getLatitude())
-                    ->setLongitude($location->getLongitude());
+                    ->setCity($location->cityName)
+                    ->setCountry($location->countryName)
+                    ->setLatitude($location->latitude)
+                    ->setLongitude($location->longitude);
             }
 
             // Set not obfuscated IP if possible, as matomo handles obfuscation itself
-            $ip = $visitLocated->originalIpAddress ?? $visit->getRemoteAddr();
+            $ip = $visitLocated->originalIpAddress ?? $visit->remoteAddr;
             if ($ip !== null) {
                 $tracker->setIp($ip);
             }
@@ -79,9 +79,9 @@ public function __invoke(VisitLocated $visitLocated): void
 
     public function resolveUrlToTrack(Visit $visit): string
     {
-        $shortUrl = $visit->getShortUrl();
+        $shortUrl = $visit->shortUrl;
         if ($shortUrl === null) {
-            return $visit->visitedUrl() ?? '';
+            return $visit->visitedUrl ?? '';
         }
 
         return $this->shortUrlStringifier->stringify($shortUrl);

diff --git a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php
@@ -20,7 +20,7 @@ public function __construct(
     public function newVisitUpdate(Visit $visit): Update
     {
         return Update::forTopicAndPayload(Topic::NEW_VISIT->value, [
-            'shortUrl' => $this->shortUrlTransformer->transform($visit->getShortUrl()),
+            'shortUrl' => $this->shortUrlTransformer->transform($visit->shortUrl),
             'visit' => $visit->jsonSerialize(),
         ]);
     }
@@ -34,7 +34,7 @@ public function newOrphanVisitUpdate(Visit $visit): Update
 
     public function newShortUrlVisitUpdate(Visit $visit): Update
     {
-        $shortUrl = $visit->getShortUrl();
+        $shortUrl = $visit->shortUrl;
         $topic = Topic::newShortUrlVisit($shortUrl?->getShortCode());
 
         return Update::forTopicAndPayload($topic, [

diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php
@@ -32,29 +32,30 @@
 
 class ShortUrl extends AbstractEntity
 {
-    private string $longUrl;
-    private string $shortCode;
-    private Chronos $dateCreated;
-    /** @var Collection<int, Visit> & Selectable */
-    private Collection & Selectable $visits;
-    /** @var Collection<int, Tag> */
-    private Collection $tags;
-    private ?Chronos $validSince = null;
-    private ?Chronos $validUntil = null;
-    private ?int $maxVisits = null;
-    private ?Domain $domain = null;
-    private bool $customSlugWasProvided;
-    private int $shortCodeLength;
-    private ?string $importSource = null;
-    private ?string $importOriginalShortCode = null;
-    private ?ApiKey $authorApiKey = null;
-    private ?string $title = null;
-    private bool $titleWasAutoResolved = false;
-    private bool $crawlable = false;
-    private bool $forwardQuery = true;
-
-    private function __construct()
-    {
+    /**
+     * @param Collection<int, Tag> $tags
+     * @param Collection<int, Visit> & Selectable $visits
+     */
+    private function __construct(
+        private string $longUrl,
+        private string $shortCode,
+        private Chronos $dateCreated = new Chronos(),
+        private Collection $tags = new ArrayCollection(),
+        private Collection & Selectable $visits = new ArrayCollection(),
+        private ?Chronos $validSince = null,
+        private ?Chronos $validUntil = null,
+        private ?int $maxVisits = null,
+        private ?Domain $domain = null,
+        private bool $customSlugWasProvided = false,
+        private int $shortCodeLength = 0,
+        public readonly ?ApiKey $authorApiKey = null,
+        private ?string $title = null,
+        private bool $titleWasAutoResolved = false,
+        private bool $crawlable = false,
+        private bool $forwardQuery = true,
+        private ?string $importSource = null,
+        private ?string $importOriginalShortCode = null,
+    ) {
     }
 
     /**
@@ -78,31 +79,29 @@ public static function create(
         ShortUrlCreation $creation,
         ?ShortUrlRelationResolverInterface $relationResolver = null,
     ): self {
-        $instance = new self();
         $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver();
-
-        $instance->longUrl = $creation->getLongUrl();
-        $instance->dateCreated = Chronos::now();
-        $instance->visits = new ArrayCollection();
-        $instance->tags = $relationResolver->resolveTags($creation->tags);
-        $instance->validSince = $creation->validSince;
-        $instance->validUntil = $creation->validUntil;
-        $instance->maxVisits = $creation->maxVisits;
-        $instance->customSlugWasProvided = $creation->hasCustomSlug();
-        $instance->shortCodeLength = $creation->shortCodeLength;
-        $instance->shortCode = sprintf(
-            '%s%s',
-            $creation->pathPrefix ?? '',
-            $creation->customSlug ?? generateRandomShortCode($instance->shortCodeLength, $creation->shortUrlMode),
+        $shortCodeLength = $creation->shortCodeLength;
+
+        return new self(
+            longUrl: $creation->getLongUrl(),
+            shortCode: sprintf(
+                '%s%s',
+                $creation->pathPrefix ?? '',
+                $creation->customSlug ?? generateRandomShortCode($shortCodeLength, $creation->shortUrlMode),
+            ),
+            tags: $relationResolver->resolveTags($creation->tags),
+            validSince: $creation->validSince,
+            validUntil: $creation->validUntil,
+            maxVisits: $creation->maxVisits,
+            domain: $relationResolver->resolveDomain($creation->domain),
+            customSlugWasProvided: $creation->hasCustomSlug(),
+            shortCodeLength: $shortCodeLength,
+            authorApiKey: $creation->apiKey,
+            title: $creation->title,
+            titleWasAutoResolved: $creation->titleWasAutoResolved,
+            crawlable: $creation->crawlable,
+            forwardQuery: $creation->forwardQuery,
         );
-        $instance->domain = $relationResolver->resolveDomain($creation->domain);
-        $instance->authorApiKey = $creation->apiKey;
-        $instance->title = $creation->title;
-        $instance->titleWasAutoResolved = $creation->titleWasAutoResolved;
-        $instance->crawlable = $creation->crawlable;
-        $instance->forwardQuery = $creation->forwardQuery;
-
-        return $instance;
     }
 
     public static function fromImport(
@@ -123,11 +122,11 @@ public static function fromImport(
 
         $instance = self::create(ShortUrlCreation::fromRawData($meta), $relationResolver);
 
-        $instance->importSource = $url->source->value;
-        $instance->importOriginalShortCode = $url->shortCode;
         $instance->validSince = normalizeOptionalDate($url->meta->validSince);
         $instance->validUntil = normalizeOptionalDate($url->meta->validUntil);
         $instance->dateCreated = normalizeDate($url->createdAt);
+        $instance->importSource = $url->source->value;
+        $instance->importOriginalShortCode = $url->shortCode;
 
         return $instance;
     }
@@ -196,11 +195,6 @@ public function getTags(): Collection
         return $this->tags;
     }
 
-    public function authorApiKey(): ?ApiKey
-    {
-        return $this->authorApiKey;
-    }
-
     public function getValidSince(): ?Chronos
     {
         return $this->validSince;