diff --git a/CHANGELOG.md b/CHANGELOG.md index e646df82..7c58d8e6 100644 --- 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 index fab02087..40ae8eef 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -50,11 +50,11 @@ class ListKeysCommand extends Command $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 index 8d2eb8c9..dd3797f8 100644 --- a/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php +++ b/module/CLI/src/Command/Domain/GetDomainVisitsCommand.php @@ -44,7 +44,7 @@ class GetDomainVisitsCommand extends AbstractVisitsListCommand */ 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 index a318e6e4..c4346f14 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -229,11 +229,11 @@ class ListShortUrlsCommand extends Command } 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 index 290a172a..1dfd0ba9 100644 --- a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php +++ b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php @@ -44,7 +44,7 @@ class GetTagVisitsCommand extends AbstractVisitsListCommand */ 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 index bd20a4ae..a999760e 100644 --- a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php +++ b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php @@ -55,8 +55,8 @@ abstract class AbstractVisitsListCommand extends Command $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 index 0dd32f3e..1462620d 100644 --- a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php @@ -40,7 +40,7 @@ class GetNonOrphanVisitsCommand extends AbstractVisitsListCommand */ 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 index 7beae19a..d495db77 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -42,6 +42,6 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand */ 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 index 09e53556..596b287e 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -132,7 +132,7 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat */ public function geolocateVisit(Visit $visit): Location { - $ipAddr = $visit->getRemoteAddr() ?? '?'; + $ipAddr = $visit->remoteAddr ?? '?'; $this->io->write(sprintf('Processing IP %s', $ipAddr)); try { @@ -154,9 +154,9 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat public function onVisitLocated(VisitLocation $visitLocation, Visit $visit): void { - if (! $visitLocation->isEmpty()) { - $this->io->writeln(sprintf(' [Address located in "%s"]', $visitLocation->getCountryName())); - } elseif ($visit->hasRemoteAddr() && $visit->getRemoteAddr() !== IpAddress::LOCALHOST) { + if (! $visitLocation->isEmpty) { + $this->io->writeln(sprintf(' [Address located in "%s"]', $visitLocation->countryName)); + } elseif ($visit->hasRemoteAddr() && $visit->remoteAddr !== IpAddress::LOCALHOST) { $this->io->writeln(' [Could not locate address]'); } } diff --git a/module/Core/src/Domain/Entity/Domain.php b/module/Core/src/Domain/Entity/Domain.php index 4e6ea865..b3d2b734 100644 --- a/module/Core/src/Domain/Entity/Domain.php +++ b/module/Core/src/Domain/Entity/Domain.php @@ -11,12 +11,12 @@ use Shlinkio\Shlink\Core\Config\NotFoundRedirects; 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 index f139c0f5..aa6afed8 100644 --- a/module/Core/src/EventDispatcher/LocateVisit.php +++ b/module/Core/src/EventDispatcher/LocateVisit.php @@ -55,7 +55,7 @@ class LocateVisit } $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 index ad9660cb..be288fd0 100644 --- a/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php +++ b/module/Core/src/EventDispatcher/Matomo/SendVisitToMatomo.php @@ -46,21 +46,21 @@ class SendVisitToMatomo $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 @@ class SendVisitToMatomo 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 index 9acdfd04..06d06c84 100644 --- a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php +++ b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php @@ -20,7 +20,7 @@ final class PublishingUpdatesGenerator implements PublishingUpdatesGeneratorInte 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 @@ final class PublishingUpdatesGenerator implements PublishingUpdatesGeneratorInte 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 index 8a577205..ef703e54 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -32,29 +32,30 @@ use function sprintf; class ShortUrl extends AbstractEntity { - private string $longUrl; - private string $shortCode; - private Chronos $dateCreated; - /** @var Collection & Selectable */ - private Collection & Selectable $visits; - /** @var Collection */ - 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 $tags + * @param Collection & 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 @@ class ShortUrl extends AbstractEntity ShortUrlCreation $creation, ?ShortUrlRelationResolverInterface $relationResolver = null, ): self { - $instance = new self(); $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver(); + $shortCodeLength = $creation->shortCodeLength; - $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), + 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 @@ class ShortUrl extends AbstractEntity $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 @@ class ShortUrl extends AbstractEntity return $this->tags; } - public function authorApiKey(): ?ApiKey - { - return $this->authorApiKey; - } - public function getValidSince(): ?Chronos { return $this->validSince; diff --git a/module/Core/src/Visit/Entity/Visit.php b/module/Core/src/Visit/Entity/Visit.php index 255a55f4..0302f898 100644 --- a/module/Core/src/Visit/Entity/Visit.php +++ b/module/Core/src/Visit/Entity/Visit.php @@ -20,29 +20,22 @@ use function Shlinkio\Shlink\Core\normalizeDate; class Visit extends AbstractEntity implements JsonSerializable { - private string $referer; - private Chronos $date; - private ?string $remoteAddr = null; - private ?string $visitedUrl = null; - private string $userAgent; - private VisitType $type; - private ?ShortUrl $shortUrl; - private ?VisitLocation $visitLocation = null; - private bool $potentialBot; - - private function __construct(?ShortUrl $shortUrl, VisitType $type) - { - $this->shortUrl = $shortUrl; - $this->date = Chronos::now(); - $this->type = $type; + private function __construct( + public readonly ?ShortUrl $shortUrl, + public readonly VisitType $type, + public readonly string $userAgent, + public readonly string $referer, + private readonly bool $potentialBot, + public readonly ?string $remoteAddr = null, + public readonly ?string $visitedUrl = null, + private ?VisitLocation $visitLocation = null, + private Chronos $date = new Chronos(), + ) { } public static function forValidShortUrl(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true): self { - $instance = new self($shortUrl, VisitType::VALID_SHORT_URL); - $instance->hydrateFromVisitor($visitor, $anonymize); - - return $instance; + return self::hydrateFromVisitor($shortUrl, VisitType::VALID_SHORT_URL, $visitor, $anonymize); } public static function fromImport(ShortUrl $shortUrl, ImportedShlinkVisit $importedVisit): self @@ -52,13 +45,10 @@ class Visit extends AbstractEntity implements JsonSerializable public static function fromOrphanImport(ImportedShlinkOrphanVisit $importedVisit): self { - $instance = self::fromImportOrOrphanImport( + return self::fromImportOrOrphanImport( $importedVisit, VisitType::tryFrom($importedVisit->type) ?? VisitType::IMPORTED, ); - $instance->visitedUrl = $importedVisit->visitedUrl; - - return $instance; } private static function fromImportOrOrphanImport( @@ -66,52 +56,52 @@ class Visit extends AbstractEntity implements JsonSerializable VisitType $type, ?ShortUrl $shortUrl = null, ): self { - $instance = new self($shortUrl, $type); - $instance->userAgent = $importedVisit->userAgent; - $instance->potentialBot = isCrawler($instance->userAgent); - $instance->referer = $importedVisit->referer; - $instance->date = normalizeDate($importedVisit->date); - $importedLocation = $importedVisit->location; - $instance->visitLocation = $importedLocation !== null ? VisitLocation::fromImport($importedLocation) : null; - - return $instance; + return new self( + shortUrl: $shortUrl, + type: $type, + userAgent: $importedVisit->userAgent, + referer: $importedVisit->referer, + potentialBot: isCrawler($importedVisit->userAgent), + visitedUrl: $importedVisit instanceof ImportedShlinkOrphanVisit ? $importedVisit->visitedUrl : null, + visitLocation: $importedLocation !== null ? VisitLocation::fromImport($importedLocation) : null, + date: normalizeDate($importedVisit->date), + ); } public static function forBasePath(Visitor $visitor, bool $anonymize = true): self { - $instance = new self(null, VisitType::BASE_URL); - $instance->hydrateFromVisitor($visitor, $anonymize); - - return $instance; + return self::hydrateFromVisitor(null, VisitType::BASE_URL, $visitor, $anonymize); } public static function forInvalidShortUrl(Visitor $visitor, bool $anonymize = true): self { - $instance = new self(null, VisitType::INVALID_SHORT_URL); - $instance->hydrateFromVisitor($visitor, $anonymize); - - return $instance; + return self::hydrateFromVisitor(null, VisitType::INVALID_SHORT_URL, $visitor, $anonymize); } public static function forRegularNotFound(Visitor $visitor, bool $anonymize = true): self { - $instance = new self(null, VisitType::REGULAR_404); - $instance->hydrateFromVisitor($visitor, $anonymize); - - return $instance; + return self::hydrateFromVisitor(null, VisitType::REGULAR_404, $visitor, $anonymize); } - private function hydrateFromVisitor(Visitor $visitor, bool $anonymize = true): void - { - $this->userAgent = $visitor->userAgent; - $this->referer = $visitor->referer; - $this->remoteAddr = $this->processAddress($anonymize, $visitor->remoteAddress); - $this->visitedUrl = $visitor->visitedUrl; - $this->potentialBot = $visitor->isPotentialBot(); + private static function hydrateFromVisitor( + ?ShortUrl $shortUrl, + VisitType $type, + Visitor $visitor, + bool $anonymize, + ): self { + return new self( + shortUrl: $shortUrl, + type: $type, + userAgent: $visitor->userAgent, + referer: $visitor->referer, + potentialBot: $visitor->isPotentialBot(), + remoteAddr: self::processAddress($anonymize, $visitor->remoteAddress), + visitedUrl: $visitor->visitedUrl, + ); } - private function processAddress(bool $anonymize, ?string $address): ?string + private static function processAddress(bool $anonymize, ?string $address): ?string { // Localhost addresses do not need to be anonymized if (! $anonymize || $address === null || $address === IpAddress::LOCALHOST) { @@ -125,21 +115,11 @@ class Visit extends AbstractEntity implements JsonSerializable } } - public function getRemoteAddr(): ?string - { - return $this->remoteAddr; - } - public function hasRemoteAddr(): bool { return ! empty($this->remoteAddr); } - public function getShortUrl(): ?ShortUrl - { - return $this->shortUrl; - } - public function getVisitLocation(): ?VisitLocation { return $this->visitLocation; @@ -161,23 +141,13 @@ class Visit extends AbstractEntity implements JsonSerializable return $this->shortUrl === null; } - public function visitedUrl(): ?string - { - return $this->visitedUrl; - } - - public function type(): VisitType - { - return $this->type; - } - /** * Needed only for ArrayCollections to be able to apply criteria filtering * @internal */ public function getType(): VisitType { - return $this->type(); + return $this->type; } /** @@ -188,16 +158,6 @@ class Visit extends AbstractEntity implements JsonSerializable return $this->date; } - public function userAgent(): string - { - return $this->userAgent; - } - - public function referer(): string - { - return $this->referer; - } - public function jsonSerialize(): array { return [ diff --git a/module/Core/src/Visit/Entity/VisitLocation.php b/module/Core/src/Visit/Entity/VisitLocation.php index 2b3b854e..1f1db686 100644 --- a/module/Core/src/Visit/Entity/VisitLocation.php +++ b/module/Core/src/Visit/Entity/VisitLocation.php @@ -11,89 +11,54 @@ use Shlinkio\Shlink\IpGeolocation\Model\Location; class VisitLocation extends AbstractEntity implements JsonSerializable { - private string $countryCode; - private string $countryName; - private string $regionName; - private string $cityName; - private float $latitude; - private float $longitude; - private string $timezone; - private bool $isEmpty; + public readonly bool $isEmpty; - private function __construct() - { + private function __construct( + public readonly string $countryCode, + public readonly string $countryName, + public readonly string $regionName, + public readonly string $cityName, + public readonly float $latitude, + public readonly float $longitude, + public readonly string $timezone, + ) { + $this->isEmpty = ( + $countryCode === '' && + $countryName === '' && + $regionName === '' && + $cityName === '' && + $latitude === 0.0 && + $longitude === 0.0 && + $timezone === '' + ); } public static function fromGeolocation(Location $location): self { - $instance = new self(); - - $instance->countryCode = $location->countryCode; - $instance->countryName = $location->countryName; - $instance->regionName = $location->regionName; - $instance->cityName = $location->city; - $instance->latitude = $location->latitude; - $instance->longitude = $location->longitude; - $instance->timezone = $location->timeZone; - $instance->computeIsEmpty(); - - return $instance; + return new self( + countryCode: $location->countryCode, + countryName: $location->countryName, + regionName: $location->regionName, + cityName: $location->city, + latitude: $location->latitude, + longitude: $location->longitude, + timezone: $location->timeZone, + ); } public static function fromImport(ImportedShlinkVisitLocation $location): self { - $instance = new self(); - - $instance->countryCode = $location->countryCode; - $instance->countryName = $location->countryName; - $instance->regionName = $location->regionName; - $instance->cityName = $location->cityName; - $instance->latitude = $location->latitude; - $instance->longitude = $location->longitude; - $instance->timezone = $location->timezone; - $instance->computeIsEmpty(); - - return $instance; - } - - private function computeIsEmpty(): void - { - $this->isEmpty = ( - $this->countryCode === '' && - $this->countryName === '' && - $this->regionName === '' && - $this->cityName === '' && - $this->latitude === 0.0 && - $this->longitude === 0.0 && - $this->timezone === '' + return new self( + countryCode: $location->countryCode, + countryName: $location->countryName, + regionName: $location->regionName, + cityName: $location->cityName, + latitude: $location->latitude, + longitude: $location->longitude, + timezone: $location->timezone, ); } - public function getCountryName(): string - { - return $this->countryName; - } - - public function getLatitude(): float - { - return $this->latitude; - } - - public function getLongitude(): float - { - return $this->longitude; - } - - public function getCityName(): string - { - return $this->cityName; - } - - public function isEmpty(): bool - { - return $this->isEmpty; - } - public function jsonSerialize(): array { return [ diff --git a/module/Core/src/Visit/Geolocation/VisitToLocationHelper.php b/module/Core/src/Visit/Geolocation/VisitToLocationHelper.php index 2a261019..9d614a7b 100644 --- a/module/Core/src/Visit/Geolocation/VisitToLocationHelper.php +++ b/module/Core/src/Visit/Geolocation/VisitToLocationHelper.php @@ -26,7 +26,7 @@ class VisitToLocationHelper implements VisitToLocationHelperInterface throw IpCannotBeLocatedException::forEmptyAddress(); } - $ipAddr = $visit->getRemoteAddr() ?? ''; + $ipAddr = $visit->remoteAddr ?? ''; if ($ipAddr === IpAddress::LOCALHOST) { throw IpCannotBeLocatedException::forLocalhost(); } diff --git a/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php b/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php index cf1c8bc9..c4dd6253 100644 --- a/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php +++ b/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php @@ -15,8 +15,8 @@ class OrphanVisitDataTransformer implements DataTransformerInterface public function transform($visit): array // phpcs:ignore { $serializedVisit = $visit->jsonSerialize(); - $serializedVisit['visitedUrl'] = $visit->visitedUrl(); - $serializedVisit['type'] = $visit->type()->value; + $serializedVisit['visitedUrl'] = $visit->visitedUrl; + $serializedVisit['type'] = $visit->type->value; return $serializedVisit; } diff --git a/module/Core/test/EventDispatcher/LocateVisitTest.php b/module/Core/test/EventDispatcher/LocateVisitTest.php index ddadde84..63595a6c 100644 --- a/module/Core/test/EventDispatcher/LocateVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateVisitTest.php @@ -157,7 +157,7 @@ class LocateVisitTest extends TestCase #[Test, DataProvider('provideIpAddresses')] public function locatableVisitsResolveToLocation(Visit $visit, ?string $originalIpAddress): void { - $ipAddr = $originalIpAddress ?? $visit->getRemoteAddr(); + $ipAddr = $originalIpAddress ?? $visit->remoteAddr; $location = new Location('', '', '', '', 0.0, 0.0, ''); $event = new UrlVisited('123', $originalIpAddress); diff --git a/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php b/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php index 94c66623..d821bcbb 100644 --- a/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php +++ b/module/Core/test/EventDispatcher/Matomo/SendVisitToMatomoTest.php @@ -76,13 +76,13 @@ class SendVisitToMatomoTest extends TestCase if ($visit->isOrphan()) { $tracker->expects($this->exactly(2))->method('setCustomTrackingParameter')->willReturnMap([ - ['type', $visit->type()->value, $tracker], + ['type', $visit->type->value, $tracker], ['orphan', 'true', $tracker], ]); } else { $tracker->expects($this->once())->method('setCustomTrackingParameter')->with( 'type', - $visit->type()->value, + $visit->type->value, )->willReturn($tracker); } diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index 545c5b47..75faa25e 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -93,8 +93,8 @@ class PublishingUpdatesGeneratorTest extends TestCase 'visitLocation' => null, 'date' => $orphanVisit->getDate()->toAtomString(), 'potentialBot' => false, - 'visitedUrl' => $orphanVisit->visitedUrl(), - 'type' => $orphanVisit->type()->value, + 'visitedUrl' => $orphanVisit->visitedUrl, + 'type' => $orphanVisit->type->value, ], ], $update->payload); } diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index 7c8a17d1..a1816563 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -244,7 +244,7 @@ class ImportedLinksProcessorTest extends TestCase $this->em->expects($this->once())->method('persist')->willReturnCallback( static fn (Visit $visit) => Assert::assertSame( $foundShortUrl ?? $originalShortUrl, - $visit->getShortUrl(), + $visit->shortUrl, ), ); diff --git a/module/Core/test/Visit/Entity/VisitLocationTest.php b/module/Core/test/Visit/Entity/VisitLocationTest.php index 46400f70..5f5c458d 100644 --- a/module/Core/test/Visit/Entity/VisitLocationTest.php +++ b/module/Core/test/Visit/Entity/VisitLocationTest.php @@ -18,7 +18,7 @@ class VisitLocationTest extends TestCase $payload = new Location(...$args); $location = VisitLocation::fromGeolocation($payload); - self::assertEquals($isEmpty, $location->isEmpty()); + self::assertEquals($isEmpty, $location->isEmpty); } public static function provideArgs(): iterable diff --git a/module/Core/test/Visit/Entity/VisitTest.php b/module/Core/test/Visit/Entity/VisitTest.php index 5eb88527..3fea2882 100644 --- a/module/Core/test/Visit/Entity/VisitTest.php +++ b/module/Core/test/Visit/Entity/VisitTest.php @@ -49,7 +49,7 @@ class VisitTest extends TestCase $anonymize, ); - self::assertEquals($expectedAddress, $visit->getRemoteAddr()); + self::assertEquals($expectedAddress, $visit->remoteAddr); } public static function provideAddresses(): iterable diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index 9ad3fcf4..46548dcf 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -17,21 +17,17 @@ use Shlinkio\Shlink\Rest\ApiKey\Role; class ApiKey extends AbstractEntity { - private string $key; - private ?Chronos $expirationDate = null; - private bool $enabled; - /** @var Collection */ - private Collection $roles; - private ?string $name = null; - /** + * @param Collection $roles * @throws Exception */ - private function __construct(string $key) - { - $this->key = $key; - $this->enabled = true; - $this->roles = new ArrayCollection(); + private function __construct( + private string $key, + public readonly ?string $name = null, + public readonly ?Chronos $expirationDate = null, + private bool $enabled = true, + private Collection $roles = new ArrayCollection(), + ) { } /** @@ -47,10 +43,7 @@ class ApiKey extends AbstractEntity */ public static function fromMeta(ApiKeyMeta $meta): self { - $apiKey = new self($meta->key); - $apiKey->name = $meta->name; - $apiKey->expirationDate = $meta->expirationDate; - + $apiKey = new self($meta->key, $meta->name, $meta->expirationDate); foreach ($meta->roleDefinitions as $roleDefinition) { $apiKey->registerRole($roleDefinition); } @@ -58,21 +51,11 @@ class ApiKey extends AbstractEntity return $apiKey; } - public function getExpirationDate(): ?Chronos - { - return $this->expirationDate; - } - public function isExpired(): bool { return $this->expirationDate !== null && $this->expirationDate->lessThan(Chronos::now()); } - public function name(): ?string - { - return $this->name; - } - public function isEnabled(): bool { return $this->enabled; diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index f45e6ca5..45364070 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -44,8 +44,8 @@ class ApiKeyServiceTest extends TestCase ApiKeyMeta::fromParams(name: $name, expirationDate: $date, roleDefinitions: $roles), ); - self::assertEquals($date, $key->getExpirationDate()); - self::assertEquals($name, $key->name()); + self::assertEquals($date, $key->expirationDate); + self::assertEquals($name, $key->name); foreach ($roles as $roleDefinition) { self::assertTrue($key->hasRole($roleDefinition->role)); }