From fdb242d0882d71557bf25f34b9552c9b12eb8c9f Mon Sep 17 00:00:00 2001 From: Guy Sartorelli Date: Thu, 22 Aug 2024 14:32:16 +1200 Subject: [PATCH] API Strong typing for the view layer --- src/File.php | 8 +- src/Folder.php | 2 +- src/Storage/DBFile.php | 176 ++++++++++++++++------------------------- 3 files changed, 69 insertions(+), 117 deletions(-) diff --git a/src/File.php b/src/File.php index 8b3c6483..3d553bca 100644 --- a/src/File.php +++ b/src/File.php @@ -299,10 +299,8 @@ public static function get_shortcodes() * * Use $file->isInDB() to only check for a DB record * Use $file->File->exists() to only check if the asset exists - * - * @return bool */ - public function exists() + public function exists(): bool { return parent::exists() && $this->File->exists(); } @@ -1316,10 +1314,8 @@ public function getVariant() /** * Return a html5 tag of the appropriate for this file (normally img or a) - * - * @return string */ - public function forTemplate() + public function forTemplate(): string { return $this->getTag() ?: ''; } diff --git a/src/Folder.php b/src/Folder.php index e9e1c78d..0fce61bf 100644 --- a/src/Folder.php +++ b/src/Folder.php @@ -40,7 +40,7 @@ class Folder extends File private static $table_name = 'Folder'; - public function exists() + public function exists(): bool { return $this->isInDB(); } diff --git a/src/Storage/DBFile.php b/src/Storage/DBFile.php index d75e61ec..ca8f3bb0 100644 --- a/src/Storage/DBFile.php +++ b/src/Storage/DBFile.php @@ -7,6 +7,7 @@ use SilverStripe\Assets\Thumbnail; use SilverStripe\Control\Director; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Forms\FormField; use SilverStripe\ORM\FieldType\DBComposite; use SilverStripe\ORM\ValidationException; use SilverStripe\ORM\ValidationResult; @@ -28,20 +29,15 @@ class DBFile extends DBComposite implements AssetContainer, Thumbnail * List of allowed file categories. * * {@see File::$app_categories} - * - * @var array */ - protected $allowedCategories = []; + protected array $allowedCategories = []; /** * List of image mime types supported by the image manipulations API * * {@see File::app_categories} for matching extensions. - * - * @config - * @var array */ - private static $supported_images = [ + private static array $supported_images = [ 'image/jpg', 'image/jpeg', 'image/pjpeg', @@ -67,13 +63,32 @@ class DBFile extends DBComposite implements AssetContainer, Thumbnail 'image/webp', ]; + private static array $composite_db = [ + "Hash" => "Varchar(255)", // SHA of the base content + "Filename" => "Varchar(255)", // Path identifier of the base content + "Variant" => "Varchar(255)", // Identifier of the variant to the base, if given + ]; + + private static array $casting = [ + 'URL' => 'Varchar', + 'AbsoluteURL' => 'Varchar', + 'Basename' => 'Varchar', + 'Title' => 'Varchar', + 'MimeType' => 'Varchar', + 'String' => 'Text', + 'Tag' => 'HTMLFragment', + 'getTag' => 'HTMLFragment', + 'Size' => 'Varchar', + 'AttributesHTML' => 'HTMLFragment', + 'getAttributesHTML' => 'HTMLFragment', + ]; + /** * Create a new image manipulation * - * @param string $name * @param array|string $allowed List of allowed file categories (not extensions), as per File::$app_categories */ - public function __construct($name = null, $allowed = []) + public function __construct(string $name = null, array|string $allowed = []) { parent::__construct($name); $this->setAllowedCategories($allowed); @@ -82,65 +97,36 @@ public function __construct($name = null, $allowed = []) /** * Determine if a valid non-empty image exists behind this asset, which is a format * compatible with image manipulations - * - * @return boolean */ - public function getIsImage() + public function getIsImage(): bool { // Check file type $mime = $this->getMimeType(); return $mime && in_array($mime, $this->config()->supported_images ?? []); } - /** - * @return AssetStore - */ - protected function getStore() + protected function getStore(): AssetStore { return Injector::inst()->get(AssetStore::class); } - private static $composite_db = [ - "Hash" => "Varchar(255)", // SHA of the base content - "Filename" => "Varchar(255)", // Path identifier of the base content - "Variant" => "Varchar(255)", // Identifier of the variant to the base, if given - ]; - - private static $casting = [ - 'URL' => 'Varchar', - 'AbsoluteURL' => 'Varchar', - 'Basename' => 'Varchar', - 'Title' => 'Varchar', - 'MimeType' => 'Varchar', - 'String' => 'Text', - 'Tag' => 'HTMLFragment', - 'getTag' => 'HTMLFragment', - 'Size' => 'Varchar', - 'AttributesHTML' => 'HTMLFragment', - 'getAttributesHTML' => 'HTMLFragment', - ]; - - public function scaffoldFormField($title = null, $params = null) + public function scaffoldFormField(?string $title = null, array $params = []): ?FormField { return null; } /** * Return a html5 tag of the appropriate for this file (normally img or a) - * - * @return string */ - public function XML() + public function XML(): string { return $this->getTag() ?: ''; } /** * Return a html5 tag of the appropriate for this file (normally img or a) - * - * @return string */ - public function getTag() + public function getTag(): string { $template = $this->getFrontendTemplate(); if (empty($template)) { @@ -154,7 +140,7 @@ public function getTag() * * @return string Name of template */ - public function getFrontendTemplate() + public function getFrontendTemplate(): string { // Check that path is available $url = $this->getURL(); @@ -173,36 +159,30 @@ public function getFrontendTemplate() /** * Get trailing part of filename - * - * @return string */ - public function getBasename() + public function getBasename(): string { if (!$this->exists()) { - return null; + return ''; } return basename($this->getSourceURL() ?? ''); } /** * Get file extension - * - * @return string */ - public function getExtension() + public function getExtension(): string { if (!$this->exists()) { - return null; + return ''; } return pathinfo($this->Filename ?? '', PATHINFO_EXTENSION); } /** * Alt title for this - * - * @return string */ - public function getTitle() + public function getTitle(): string { // If customised, use the customised title if ($this->failover && ($title = $this->failover->Title)) { @@ -264,17 +244,17 @@ public function getStream() public function getString() { if (!$this->exists()) { - return null; + return ''; } return $this ->getStore() ->getAsString($this->Filename, $this->Hash, $this->Variant); } - public function getURL($grant = true) + public function getURL($grant = true): string { if (!$this->exists()) { - return null; + return ''; } $url = $this->getSourceURL($grant); $this->updateURL($url); @@ -284,20 +264,16 @@ public function getURL($grant = true) /** * Return URL for this image. Alias for getURL() - * - * @return string */ - public function Link() + public function Link(): string { return $this->getURL(); } /** * Return absolute URL for this image. Alias for getAbsoluteURL() - * - * @return string */ - public function AbsoluteLink() + public function AbsoluteLink(): string { return $this->getAbsoluteURL(); } @@ -307,9 +283,8 @@ public function AbsoluteLink() * Note that this will return the url even if the file does not exist. * * @param bool $grant Ensures that the url for any protected assets is granted for the current user. - * @return string */ - public function getSourceURL($grant = true) + public function getSourceURL(bool $grant = true): string { return $this ->getStore() @@ -318,38 +293,36 @@ public function getSourceURL($grant = true) /** * Get the absolute URL to this resource - * - * @return string */ - public function getAbsoluteURL() + public function getAbsoluteURL(): string { if (!$this->exists()) { - return null; + return ''; } return Director::absoluteURL((string) $this->getURL()); } - public function getMetaData() + public function getMetaData(): array { if (!$this->exists()) { - return null; + return []; } return $this ->getStore() ->getMetadata($this->Filename, $this->Hash, $this->Variant); } - public function getMimeType() + public function getMimeType(): string { if (!$this->exists()) { - return null; + return ''; } return $this ->getStore() ->getMimeType($this->Filename, $this->Hash, $this->Variant); } - public function getValue() + public function getValue(): ?array { if (!$this->exists()) { return null; @@ -361,7 +334,7 @@ public function getValue() ]; } - public function getVisibility() + public function getVisibility(): ?string { if (empty($this->Filename)) { return null; @@ -371,7 +344,7 @@ public function getVisibility() ->getVisibility($this->Filename, $this->Hash); } - public function exists() + public function exists(): bool { if (empty($this->Filename)) { return false; @@ -381,27 +354,25 @@ public function exists() ->exists($this->Filename, $this->Hash, $this->Variant); } - public function getFilename() + public function getFilename(): ?string { return $this->getField('Filename'); } - public function getHash() + public function getHash(): ?string { return $this->getField('Hash'); } - public function getVariant() + public function getVariant(): ?string { return $this->getField('Variant'); } /** * Return file size in bytes. - * - * @return int */ - public function getAbsoluteSize() + public function getAbsoluteSize(): int { $metadata = $this->getMetaData(); if (isset($metadata['size'])) { @@ -412,11 +383,8 @@ public function getAbsoluteSize() /** * Customise this object with an "original" record for getting other customised fields - * - * @param AssetContainer $original - * @return $this */ - public function setOriginal($original) + public function setOriginal(AssetContainer $original): static { if ($original instanceof ViewableData) { $this->setFailover($original); @@ -426,21 +394,16 @@ public function setOriginal($original) /** * Get list of allowed file categories - * - * @return array */ - public function getAllowedCategories() + public function getAllowedCategories(): array { return $this->allowedCategories; } /** * Assign allowed categories - * - * @param array|string $categories - * @return $this */ - public function setAllowedCategories($categories) + public function setAllowedCategories(array|string $categories): static { if (is_string($categories)) { $categories = preg_split('/\s*,\s*/', $categories ?? ''); @@ -452,10 +415,8 @@ public function setAllowedCategories($categories) /** * Gets the list of extensions (if limited) for this field. Empty list * means there is no restriction on allowed types. - * - * @return array */ - protected function getAllowedExtensions() + protected function getAllowedExtensions(): array { $categories = $this->getAllowedCategories(); return File::get_category_extensions($categories); @@ -464,11 +425,9 @@ protected function getAllowedExtensions() /** * Validate that this DBFile accepts this filename as valid * - * @param string $filename * @throws ValidationException - * @return bool */ - protected function isValidFilename($filename) + protected function isValidFilename(string $filename): bool { $extension = strtolower(File::get_file_extension($filename) ?? ''); @@ -491,10 +450,9 @@ protected function isValidFilename($filename) /** * Check filename, and raise a ValidationException if invalid * - * @param string $filename * @throws ValidationException */ - protected function assertFilenameValid($filename) + protected function assertFilenameValid(string $filename): void { $result = new ValidationResult(); $this->validate($result, $filename); @@ -507,11 +465,9 @@ protected function assertFilenameValid($filename) /** * Hook to validate this record against a validation result * - * @param ValidationResult $result - * @param string $filename Optional filename to validate. If omitted, the current value is validated. - * @return bool Valid flag + * @param null|string $filename Optional filename to validate. If omitted, the current value is validated. */ - public function validate(ValidationResult $result, $filename = null) + public function validate(ValidationResult $result, ?string $filename = null): bool { if (empty($filename)) { $filename = $this->getFilename(); @@ -529,14 +485,14 @@ public function validate(ValidationResult $result, $filename = null) return false; } - public function setField($field, $value, $markChanged = true) + public function setField(string $fieldName, mixed $value, bool $markChanged = true): static { // Catch filename validation on direct assignment - if ($field === 'Filename' && $value) { + if ($fieldName === 'Filename' && $value) { $this->assertFilenameValid($value); } - return parent::setField($field, $value, $markChanged); + return parent::setField($fieldName, $value, $markChanged); } @@ -545,7 +501,7 @@ public function setField($field, $value, $markChanged = true) * * @return string|false String value, or false if doesn't exist */ - public function getSize() + public function getSize(): string|false { $size = $this->getAbsoluteSize(); if ($size) {