diff --git a/migrations/Version20250220215048.php b/migrations/Version20250220215048.php new file mode 100644 index 00000000..82e132de --- /dev/null +++ b/migrations/Version20250220215048.php @@ -0,0 +1,87 @@ +addSql('ALTER TABLE attachments ADD internal_path VARCHAR(255) DEFAULT NULL, ADD external_path VARCHAR(255) DEFAULT NULL'); + + //Copy the data from path to external_path and remove the path column + $this->addSql('UPDATE attachments SET external_path=path'); + $this->addSql('ALTER TABLE attachments DROP path'); + + + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%MEDIA#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%BASE#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%SECURE#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%FOOTPRINTS#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%FOOTPRINTS3D#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET external_path=NULL WHERE internal_path IS NOT NULL'); + } + + public function mySQLDown(Schema $schema): void + { + $this->addSql('UPDATE attachments SET external_path=internal_path WHERE internal_path IS NOT NULL'); + $this->addSql('ALTER TABLE attachments DROP internal_path'); + $this->addSql('ALTER TABLE attachments RENAME COLUMN external_path TO path'); + } + + public function postgreSQLUp(Schema $schema): void + { + //We can use the same SQL for PostgreSQL as for MySQL + $this->mySQLUp($schema); + } + + public function postgreSQLDown(Schema $schema): void + { + //We can use the same SQL for PostgreSQL as for MySQL + $this->mySQLDown($schema); + } + + public function sqLiteUp(Schema $schema): void + { + $this->addSql('CREATE TEMPORARY TABLE __temp__attachments AS SELECT id, type_id, original_filename, show_in_table, name, last_modified, datetime_added, class_name, element_id, path FROM attachments'); + $this->addSql('DROP TABLE attachments'); + $this->addSql('CREATE TABLE attachments (id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, type_id INTEGER NOT NULL, original_filename VARCHAR(255) DEFAULT NULL, show_in_table BOOLEAN NOT NULL, name VARCHAR(255) NOT NULL, last_modified DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, datetime_added DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL, class_name VARCHAR(255) NOT NULL, element_id INTEGER NOT NULL, internal_path VARCHAR(255) DEFAULT NULL, external_path VARCHAR(255) DEFAULT NULL, CONSTRAINT FK_47C4FAD6C54C8C93 FOREIGN KEY (type_id) REFERENCES attachment_types (id) ON UPDATE NO ACTION ON DELETE NO ACTION NOT DEFERRABLE INITIALLY IMMEDIATE)'); + $this->addSql('INSERT INTO attachments (id, type_id, original_filename, show_in_table, name, last_modified, datetime_added, class_name, element_id, external_path) SELECT id, type_id, original_filename, show_in_table, name, last_modified, datetime_added, class_name, element_id, path FROM __temp__attachments'); + $this->addSql('DROP TABLE __temp__attachments'); + $this->addSql('CREATE INDEX attachment_element_idx ON attachments (class_name, element_id)'); + $this->addSql('CREATE INDEX attachment_name_idx ON attachments (name)'); + $this->addSql('CREATE INDEX attachments_idx_class_name_id ON attachments (class_name, id)'); + $this->addSql('CREATE INDEX attachments_idx_id_element_id_class_name ON attachments (id, element_id, class_name)'); + $this->addSql('CREATE INDEX IDX_47C4FAD6C54C8C93 ON attachments (type_id)'); + $this->addSql('CREATE INDEX IDX_47C4FAD61F1F2A24 ON attachments (element_id)'); + + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%MEDIA#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%BASE#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%SECURE#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%FOOTPRINTS#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%FOOTPRINTS3D#%%\' ESCAPE \'#\''); + $this->addSql('UPDATE attachments SET external_path=NULL WHERE internal_path IS NOT NULL'); + } + + public function sqLiteDown(Schema $schema): void + { + //Reuse the MySQL down migration: + $this->mySQLDown($schema); + } + + +} diff --git a/src/Controller/AttachmentFileController.php b/src/Controller/AttachmentFileController.php index 936d27c5..d8bd8d87 100644 --- a/src/Controller/AttachmentFileController.php +++ b/src/Controller/AttachmentFileController.php @@ -51,15 +51,15 @@ class AttachmentFileController extends AbstractController $this->denyAccessUnlessGranted('show_private', $attachment); } - if ($attachment->isExternal()) { - throw $this->createNotFoundException('The file for this attachment is external and can not stored locally!'); + if (!$attachment->hasInternal()) { + throw $this->createNotFoundException('The file for this attachment is external and not stored locally!'); } - if (!$helper->isFileExisting($attachment)) { + if (!$helper->isInternalFileExisting($attachment)) { throw $this->createNotFoundException('The file associated with the attachment is not existing!'); } - $file_path = $helper->toAbsoluteFilePath($attachment); + $file_path = $helper->toAbsoluteInternalFilePath($attachment); $response = new BinaryFileResponse($file_path); //Set header content disposition, so that the file will be downloaded @@ -80,15 +80,15 @@ class AttachmentFileController extends AbstractController $this->denyAccessUnlessGranted('show_private', $attachment); } - if ($attachment->isExternal()) { - throw $this->createNotFoundException('The file for this attachment is external and can not stored locally!'); + if (!$attachment->hasInternal()) { + throw $this->createNotFoundException('The file for this attachment is external and not stored locally!'); } - if (!$helper->isFileExisting($attachment)) { + if (!$helper->isInternalFileExisting($attachment)) { throw $this->createNotFoundException('The file associated with the attachment is not existing!'); } - $file_path = $helper->toAbsoluteFilePath($attachment); + $file_path = $helper->toAbsoluteInternalFilePath($attachment); $response = new BinaryFileResponse($file_path); //Set header content disposition, so that the file will be downloaded diff --git a/src/DataFixtures/PartFixtures.php b/src/DataFixtures/PartFixtures.php index 0c8ea36d..a60d037d 100644 --- a/src/DataFixtures/PartFixtures.php +++ b/src/DataFixtures/PartFixtures.php @@ -131,7 +131,7 @@ class PartFixtures extends Fixture implements DependentFixtureInterface $attachment = new PartAttachment(); $attachment->setName('Test2'); - $attachment->setPath('invalid'); + $attachment->setInternalPath('invalid'); $attachment->setShowInTable(true); $attachment->setAttachmentType($manager->find(AttachmentType::class, 1)); $part->addAttachment($attachment); diff --git a/src/DataTables/AttachmentDataTable.php b/src/DataTables/AttachmentDataTable.php index 0d6c5b53..7973bf8a 100644 --- a/src/DataTables/AttachmentDataTable.php +++ b/src/DataTables/AttachmentDataTable.php @@ -50,8 +50,8 @@ final class AttachmentDataTable implements DataTableTypeInterface { $dataTable->add('dont_matter', RowClassColumn::class, [ 'render' => function ($value, Attachment $context): string { - //Mark attachments with missing files yellow - if(!$this->attachmentHelper->isFileExisting($context)){ + //Mark attachments yellow which have an internal file linked that doesn't exist + if($context->hasInternal() && !$this->attachmentHelper->isInternalFileExisting($context)){ return 'table-warning'; } @@ -64,8 +64,8 @@ final class AttachmentDataTable implements DataTableTypeInterface 'className' => 'no-colvis', 'render' => function ($value, Attachment $context): string { if ($context->isPicture() - && !$context->isExternal() - && $this->attachmentHelper->isFileExisting($context)) { + && $this->attachmentHelper->isInternalFileExisting($context)) { + $title = htmlspecialchars($context->getName()); if ($context->getFilename()) { $title .= ' ('.htmlspecialchars($context->getFilename()).')'; @@ -93,26 +93,6 @@ final class AttachmentDataTable implements DataTableTypeInterface $dataTable->add('name', TextColumn::class, [ 'label' => 'attachment.edit.name', 'orderField' => 'NATSORT(attachment.name)', - 'render' => function ($value, Attachment $context) { - //Link to external source - if ($context->isExternal()) { - return sprintf( - '%s', - htmlspecialchars((string) $context->getURL()), - htmlspecialchars($value) - ); - } - - if ($this->attachmentHelper->isFileExisting($context)) { - return sprintf( - '%s', - $this->entityURLGenerator->viewURL($context), - htmlspecialchars($value) - ); - } - - return $value; - }, ]); $dataTable->add('attachment_type', TextColumn::class, [ @@ -136,25 +116,57 @@ final class AttachmentDataTable implements DataTableTypeInterface ), ]); - $dataTable->add('filename', TextColumn::class, [ - 'label' => $this->translator->trans('attachment.table.filename'), + $dataTable->add('internal_link', TextColumn::class, [ + 'label' => 'attachment.table.internal_file', 'propertyPath' => 'filename', + 'render' => function ($value, Attachment $context) { + if ($this->attachmentHelper->isInternalFileExisting($context)) { + return sprintf( + '%s', + $this->entityURLGenerator->viewURL($context), + htmlspecialchars($value) + ); + } + + return $value; + } + ]); + + $dataTable->add('external_link', TextColumn::class, [ + 'label' => 'attachment.table.external_link', + 'propertyPath' => 'host', + 'render' => function ($value, Attachment $context) { + if ($context->hasExternal()) { + return sprintf( + '%s', + htmlspecialchars((string) $context->getExternalPath()), + htmlspecialchars($value) + ); + } + + return $value; + } ]); $dataTable->add('filesize', TextColumn::class, [ 'label' => $this->translator->trans('attachment.table.filesize'), 'render' => function ($value, Attachment $context) { - if ($context->isExternal()) { + if (!$context->hasInternal()) { return sprintf( ' %s ', - $this->translator->trans('attachment.external') + $this->translator->trans('attachment.external_only') ); } - if ($this->attachmentHelper->isFileExisting($context)) { - return $this->attachmentHelper->getHumanFileSize($context); + if ($this->attachmentHelper->isInternalFileExisting($context)) { + return sprintf( + ' + %s + ', + $this->attachmentHelper->getHumanFileSize($context) + ); } return sprintf( diff --git a/src/Entity/Attachments/Attachment.php b/src/Entity/Attachments/Attachment.php index 30d9e257..3c8d890d 100644 --- a/src/Entity/Attachments/Attachment.php +++ b/src/Entity/Attachments/Attachment.php @@ -78,11 +78,16 @@ use LogicException; denormalizationContext: ['groups' => ['attachment:write', 'attachment:write:standalone', 'api:basic:write'], 'openapi_definition_name' => 'Write'], processor: HandleAttachmentsUploadsProcessor::class, )] -#[DocumentedAPIProperty(schemaName: 'Attachment-Read', property: 'media_url', type: 'string', nullable: true, - description: 'The URL to the file, where the attachment file can be downloaded. This can be an internal or external URL.', - example: '/media/part/2/bc547-6508afa5a79c8.pdf')] -#[DocumentedAPIProperty(schemaName: 'Attachment-Read', property: 'thumbnail_url', type: 'string', nullable: true, - description: 'The URL to a thumbnail version of this file. This only exists for internal picture attachments.')] +//This property is added by the denormalizer in order to resolve the placeholder +#[DocumentedAPIProperty( + schemaName: 'Attachment-Read', property: 'internal_path', type: 'string', nullable: false, + description: 'The URL to the internally saved copy of the file, if one exists', + example: '/media/part/2/bc547-6508afa5a79c8.pdf' +)] +#[DocumentedAPIProperty( + schemaName: 'Attachment-Read', property: 'thumbnail_url', type: 'string', nullable: true, + description: 'The URL to a thumbnail version of this file. This only exists for internal picture attachments.' +)] #[ApiFilter(LikeFilter::class, properties: ["name"])] #[ApiFilter(EntityFilter::class, properties: ["attachment_type"])] #[ApiFilter(DateFilter::class, strategy: DateFilterInterface::EXCLUDE_NULL)] @@ -119,10 +124,6 @@ abstract class Attachment extends AbstractNamedDBElement */ final public const MODEL_EXTS = ['x3d']; - /** - * When the path begins with one of the placeholders. - */ - final public const INTERNAL_PLACEHOLDER = ['%BASE%', '%MEDIA%', '%SECURE%']; /** * @var array placeholders for attachments which using built in files @@ -152,10 +153,21 @@ abstract class Attachment extends AbstractNamedDBElement protected ?string $original_filename = null; /** - * @var string The path to the file relative to a placeholder path like %MEDIA% + * @var string|null If a copy of the file is stored internally, the path to the file relative to a placeholder + * path like %MEDIA% */ - #[ORM\Column(name: 'path', type: Types::STRING)] - protected string $path = ''; + #[ORM\Column(type: Types::STRING, nullable: true)] + protected ?string $internal_path = null; + + + /** + * @var string|null The path to the external source if the file is stored externally or was downloaded from an + * external source. Null if there is no external source. + */ + #[ORM\Column(type: Types::STRING, nullable: true)] + #[Groups(['attachment:read'])] + #[ApiProperty(example: 'http://example.com/image.jpg')] + protected ?string $external_path = null; /** * @var string the name of this element @@ -237,7 +249,7 @@ abstract class Attachment extends AbstractNamedDBElement /** * Check if this attachment is a picture (analyse the file's extension). - * If the link is external, it is assumed that this is true. + * If the link is only external and doesn't contain an extension, it is assumed that this is true. * * @return bool * true if the file extension is a picture extension * * otherwise false @@ -245,54 +257,67 @@ abstract class Attachment extends AbstractNamedDBElement #[Groups(['attachment:read'])] public function isPicture(): bool { - if ($this->isExternal()) { + if($this->hasInternal()){ + + $extension = pathinfo($this->getInternalPath(), PATHINFO_EXTENSION); + + return in_array(strtolower($extension), static::PICTURE_EXTS, true); + + } + if ($this->hasExternal()) { //Check if we can extract a file extension from the URL - $extension = pathinfo(parse_url($this->path, PHP_URL_PATH) ?? '', PATHINFO_EXTENSION); + $extension = pathinfo(parse_url($this->getExternalPath(), PHP_URL_PATH) ?? '', PATHINFO_EXTENSION); //If no extension is found or it is known picture extension, we assume that this is a picture extension return $extension === '' || in_array(strtolower($extension), static::PICTURE_EXTS, true); } - - $extension = pathinfo($this->getPath(), PATHINFO_EXTENSION); - - return in_array(strtolower($extension), static::PICTURE_EXTS, true); + //File doesn't have an internal, nor an external copy. This shouldn't happen, but it certainly isn't a picture... + return false; } /** * Check if this attachment is a 3D model and therefore can be directly shown to user. - * If the attachment is external, false is returned (3D Models must be internal). + * If no internal copy exists, false is returned (3D Models must be internal). */ #[Groups(['attachment:read'])] #[SerializedName('3d_model')] public function is3DModel(): bool { //We just assume that 3D Models are internally saved, otherwise we get problems loading them. - if ($this->isExternal()) { + if (!$this->hasInternal()) { return false; } - $extension = pathinfo($this->getPath(), PATHINFO_EXTENSION); + $extension = pathinfo($this->getInternalPath(), PATHINFO_EXTENSION); return in_array(strtolower($extension), static::MODEL_EXTS, true); } /** - * Checks if the attachment file is externally saved (the database saves an URL). + * Checks if this attachment has a path to an external file * - * @return bool true, if the file is saved externally + * @return bool true, if there is a path to an external file + * @phpstan-assert-if-true non-empty-string $this->external_path + * @phpstan-assert-if-true non-empty-string $this->getExternalPath()) */ #[Groups(['attachment:read'])] - public function isExternal(): bool + public function hasExternal(): bool { - //When path is empty, this attachment can not be external - if ($this->path === '') { - return false; - } + return $this->external_path !== null && $this->external_path !== ''; + } - //After the %PLACEHOLDER% comes a slash, so we can check if we have a placeholder via explode - $tmp = explode('/', $this->path); - - return !in_array($tmp[0], array_merge(static::INTERNAL_PLACEHOLDER, static::BUILTIN_PLACEHOLDER), true); + /** + * Checks if this attachment has a path to an internal file. + * Does not check if the file exists. + * + * @return bool true, if there is a path to an internal file + * @phpstan-assert-if-true non-empty-string $this->internal_path + * @phpstan-assert-if-true non-empty-string $this->getInternalPath()) + */ + #[Groups(['attachment:read'])] + public function hasInternal(): bool + { + return $this->internal_path !== null && $this->internal_path !== ''; } /** @@ -305,8 +330,12 @@ abstract class Attachment extends AbstractNamedDBElement #[SerializedName('private')] public function isSecure(): bool { + if ($this->internal_path === null) { + return false; + } + //After the %PLACEHOLDER% comes a slash, so we can check if we have a placeholder via explode - $tmp = explode('/', $this->path); + $tmp = explode('/', $this->internal_path); return '%SECURE%' === $tmp[0]; } @@ -320,7 +349,11 @@ abstract class Attachment extends AbstractNamedDBElement #[Groups(['attachment:read'])] public function isBuiltIn(): bool { - return static::checkIfBuiltin($this->path); + if ($this->internal_path === null) { + return false; + } + + return static::checkIfBuiltin($this->internal_path); } /******************************************************************************** @@ -332,13 +365,13 @@ abstract class Attachment extends AbstractNamedDBElement /** * Returns the extension of the file referenced via the attachment. * For a path like %BASE/path/foo.bar, bar will be returned. - * If this attachment is external null is returned. + * If this attachment is only external null is returned. * * @return string|null the file extension in lower case */ public function getExtension(): ?string { - if ($this->isExternal()) { + if (!$this->hasInternal()) { return null; } @@ -346,7 +379,7 @@ abstract class Attachment extends AbstractNamedDBElement return strtolower(pathinfo($this->original_filename, PATHINFO_EXTENSION)); } - return strtolower(pathinfo($this->getPath(), PATHINFO_EXTENSION)); + return strtolower(pathinfo($this->getInternalPath(), PATHINFO_EXTENSION)); } /** @@ -361,52 +394,54 @@ abstract class Attachment extends AbstractNamedDBElement } /** - * The URL to the external file, or the path to the built-in file. + * The URL to the external file, or the path to the built-in file, but not paths to uploaded files. * Returns null, if the file is not external (and not builtin). + * The output of this function is such, that no changes occur when it is fed back into setURL(). + * Required for the Attachment form field. */ - #[Groups(['attachment:read'])] - #[SerializedName('url')] public function getURL(): ?string { - if (!$this->isExternal() && !$this->isBuiltIn()) { - return null; + if($this->hasExternal()){ + return $this->getExternalPath(); } - - return $this->path; + if($this->isBuiltIn()){ + return $this->getInternalPath(); + } + return null; } /** * Returns the hostname where the external file is stored. - * Returns null, if the file is not external. + * Returns null, if there is no external path. */ public function getHost(): ?string { - if (!$this->isExternal()) { + if (!$this->hasExternal()) { return null; } - return parse_url((string) $this->getURL(), PHP_URL_HOST); + return parse_url($this->getExternalPath(), PHP_URL_HOST); } - /** - * Get the filepath, relative to %BASE%. - * - * @return string A string like %BASE/path/foo.bar - */ - public function getPath(): string + public function getInternalPath(): ?string { - return $this->path; + return $this->internal_path; + } + + public function getExternalPath(): ?string + { + return $this->external_path; } /** * Returns the filename of the attachment. * For a path like %BASE/path/foo.bar, foo.bar will be returned. * - * If the path is a URL (can be checked via isExternal()), null will be returned. + * If there is no internal copy of the file, null will be returned. */ public function getFilename(): ?string { - if ($this->isExternal()) { + if (!$this->hasInternal()) { return null; } @@ -415,7 +450,7 @@ abstract class Attachment extends AbstractNamedDBElement return $this->original_filename; } - return pathinfo($this->getPath(), PATHINFO_BASENAME); + return pathinfo($this->getInternalPath(), PATHINFO_BASENAME); } /** @@ -488,15 +523,12 @@ abstract class Attachment extends AbstractNamedDBElement } /** - * Sets the filepath (with relative placeholder) for this attachment. - * - * @param string $path the new filepath of the attachment - * - * @return Attachment + * Sets the path to a file hosted internally. If you set this path to a file that was not downloaded from the + * external source in external_path, make sure to reset external_path. */ - public function setPath(string $path): self + public function setInternalPath(?string $internal_path): self { - $this->path = $path; + $this->internal_path = $internal_path; return $this; } @@ -512,34 +544,60 @@ abstract class Attachment extends AbstractNamedDBElement } /** - * Sets the url associated with this attachment. - * If the url is empty nothing is changed, to not override the file path. - * - * @return Attachment + * Sets up the paths using a user provided string which might contain an external path or a builtin path. Allows + * resetting the external path if an internal path exists. Resets any other paths if a (nonempty) new path is set. */ #[Groups(['attachment:write'])] #[SerializedName('url')] + #[ApiProperty(description: 'Set the path of the attachment here. + Provide either an external URL, a path to a builtin file (like %FOOTPRINTS%/Active/ICs/IC_DFS.png) or an empty + string if the attachment has an internal file associated and you\'d like to reset the external source. + If you set a new (nonempty) file path any associated internal file will be removed!')] public function setURL(?string $url): self { - //Do nothing if the URL is empty - if ($url === null || $url === '') { + //Don't allow the user to set an empty external path if the internal path is empty already + if (($url === null || $url === "") && !$this->hasInternal()) { return $this; } - $url = trim($url); - //Escape spaces in URL - $url = str_replace(' ', '%20', $url); - - //Only set if the URL is not empty - if ($url !== '') { - if (str_contains($url, '%BASE%') || str_contains($url, '%MEDIA%')) { - throw new InvalidArgumentException('You can not reference internal files via the url field! But nice try!'); - } - - $this->path = $url; - //Reset internal filename - $this->original_filename = null; + //The URL field can also contain the special builtin internal paths, so we need to distinguish here + if ($this::checkIfBuiltin($url)) { + $this->setInternalPath($url); + //make sure the external path isn't still pointing to something unrelated + $this->setExternalPath(null); + } else { + $this->setExternalPath($url); } + return $this; + } + + + /** + * Sets the path to a file hosted on an external server. Setting the external path to a (nonempty) value different + * from the the old one _clears_ the internal path, so that the external path reflects where any associated internal + * file came from. + */ + public function setExternalPath(?string $external_path): self + { + //If we only clear the external path, don't reset the internal path, since that could be confusing + if($external_path === null || $external_path === '') { + $this->external_path = null; + return $this; + } + + $external_path = trim($external_path); + //Escape spaces in URL + $external_path = str_replace(' ', '%20', $external_path); + + if($this->external_path === $external_path) { + //Nothing changed, nothing to do + return $this; + } + + $this->external_path = $external_path; + $this->internal_path = null; + //Reset internal filename + $this->original_filename = null; return $this; } @@ -551,12 +609,17 @@ abstract class Attachment extends AbstractNamedDBElement /** * Checks if the given path is a path to a builtin resource. * - * @param string $path The path that should be checked + * @param string|null $path The path that should be checked * * @return bool true if the path is pointing to a builtin resource */ - public static function checkIfBuiltin(string $path): bool + public static function checkIfBuiltin(?string $path): bool { + //An empty path can't be a builtin + if ($path === null) { + return false; + } + //After the %PLACEHOLDER% comes a slash, so we can check if we have a placeholder via explode $tmp = explode('/', $path); //Builtins must have a %PLACEHOLDER% construction diff --git a/src/EntityListeners/AttachmentDeleteListener.php b/src/EntityListeners/AttachmentDeleteListener.php index e9df5972..1f39b2d0 100644 --- a/src/EntityListeners/AttachmentDeleteListener.php +++ b/src/EntityListeners/AttachmentDeleteListener.php @@ -52,8 +52,8 @@ class AttachmentDeleteListener #[PreUpdate] public function preUpdateHandler(Attachment $attachment, PreUpdateEventArgs $event): void { - if ($event->hasChangedField('path')) { - $old_path = $event->getOldValue('path'); + if ($event->hasChangedField('internal_path')) { + $old_path = $event->getOldValue('internal_path'); //Dont delete file if the attachment uses a builtin ressource: if (Attachment::checkIfBuiltin($old_path)) { diff --git a/src/Repository/AttachmentRepository.php b/src/Repository/AttachmentRepository.php index 865443d2..0a6b1db2 100644 --- a/src/Repository/AttachmentRepository.php +++ b/src/Repository/AttachmentRepository.php @@ -58,7 +58,7 @@ class AttachmentRepository extends DBElementRepository { $qb = $this->createQueryBuilder('attachment'); $qb->select('COUNT(attachment)') - ->where('attachment.path LIKE :like ESCAPE \'#\''); + ->where('attachment.internal_path LIKE :like ESCAPE \'#\''); $qb->setParameter('like', '#%SECURE#%%'); $query = $qb->getQuery(); @@ -66,7 +66,7 @@ class AttachmentRepository extends DBElementRepository } /** - * Gets the count of all external attachments (attachments only containing a URL). + * Gets the count of all external attachments (attachments containing an external path). * * @throws NoResultException * @throws NonUniqueResultException @@ -75,17 +75,15 @@ class AttachmentRepository extends DBElementRepository { $qb = $this->createQueryBuilder('attachment'); $qb->select('COUNT(attachment)') - ->where('ILIKE(attachment.path, :http) = TRUE') - ->orWhere('ILIKE(attachment.path, :https) = TRUE'); - $qb->setParameter('http', 'http://%'); - $qb->setParameter('https', 'https://%'); + ->andWhere('attaachment.internal_path IS NULL') + ->where('attachment.external_path IS NOT NULL'); $query = $qb->getQuery(); return (int) $query->getSingleScalarResult(); } /** - * Gets the count of all attachments where a user uploaded a file. + * Gets the count of all attachments where a user uploaded a file or a file was downloaded from an external source. * * @throws NoResultException * @throws NonUniqueResultException @@ -94,9 +92,9 @@ class AttachmentRepository extends DBElementRepository { $qb = $this->createQueryBuilder('attachment'); $qb->select('COUNT(attachment)') - ->where('attachment.path LIKE :base ESCAPE \'#\'') - ->orWhere('attachment.path LIKE :media ESCAPE \'#\'') - ->orWhere('attachment.path LIKE :secure ESCAPE \'#\''); + ->where('attachment.internal_path LIKE :base ESCAPE \'#\'') + ->orWhere('attachment.internal_path LIKE :media ESCAPE \'#\'') + ->orWhere('attachment.internal_path LIKE :secure ESCAPE \'#\''); $qb->setParameter('secure', '#%SECURE#%%'); $qb->setParameter('base', '#%BASE#%%'); $qb->setParameter('media', '#%MEDIA#%%'); diff --git a/src/Serializer/AttachmentNormalizer.php b/src/Serializer/AttachmentNormalizer.php index 8a37b3c0..bd791d04 100644 --- a/src/Serializer/AttachmentNormalizer.php +++ b/src/Serializer/AttachmentNormalizer.php @@ -52,11 +52,15 @@ class AttachmentNormalizer implements NormalizerInterface, NormalizerAwareInterf $context[self::ALREADY_CALLED] = true; $data = $this->normalizer->normalize($object, $format, $context); + $data['internal_path'] = $this->attachmentURLGenerator->getInternalViewURL($object); - $data['media_url'] = $this->attachmentURLGenerator->getViewURL($object); //Add thumbnail url if the attachment is a picture $data['thumbnail_url'] = $object->isPicture() ? $this->attachmentURLGenerator->getThumbnailURL($object) : null; + //For backwards compatibility reasons + //Deprecated: Use internal_path and external_path instead + $data['media_url'] = $data['internal_path'] ?? $object->getExternalPath(); + return $data; } diff --git a/src/Services/Attachments/AttachmentManager.php b/src/Services/Attachments/AttachmentManager.php index 4429179e..1075141b 100644 --- a/src/Services/Attachments/AttachmentManager.php +++ b/src/Services/Attachments/AttachmentManager.php @@ -44,35 +44,31 @@ class AttachmentManager * * @param Attachment $attachment The attachment for which the file should be generated * - * @return SplFileInfo|null The fileinfo for the attachment file. Null, if the attachment is external or has + * @return SplFileInfo|null The fileinfo for the attachment file. Null, if the attachment is only external or has * invalid file. */ public function attachmentToFile(Attachment $attachment): ?SplFileInfo { - if ($attachment->isExternal() || !$this->isFileExisting($attachment)) { + if (!$this->isInternalFileExisting($attachment)) { return null; } - return new SplFileInfo($this->toAbsoluteFilePath($attachment)); + return new SplFileInfo($this->toAbsoluteInternalFilePath($attachment)); } /** - * Returns the absolute filepath of the attachment. Null is returned, if the attachment is externally saved, - * or is not existing. + * Returns the absolute filepath to the internal copy of the attachment. Null is returned, if the attachment is + * only externally saved, or is not existing. * * @param Attachment $attachment The attachment for which the filepath should be determined */ - public function toAbsoluteFilePath(Attachment $attachment): ?string + public function toAbsoluteInternalFilePath(Attachment $attachment): ?string { - if ($attachment->getPath() === '') { + if (!$attachment->hasInternal()){ return null; } - if ($attachment->isExternal()) { - return null; - } - - $path = $this->pathResolver->placeholderToRealPath($attachment->getPath()); + $path = $this->pathResolver->placeholderToRealPath($attachment->getInternalPath()); //realpath does not work with null as argument if (null === $path) { @@ -89,8 +85,8 @@ class AttachmentManager } /** - * Checks if the file in this attachement is existing. This works for files on the HDD, and for URLs - * (it's not checked if the ressource behind the URL is really existing, so for every external attachment true is returned). + * Checks if the file in this attachment is existing. This works for files on the HDD, and for URLs + * (it's not checked if the resource behind the URL is really existing, so for every external attachment true is returned). * * @param Attachment $attachment The attachment for which the existence should be checked * @@ -98,15 +94,23 @@ class AttachmentManager */ public function isFileExisting(Attachment $attachment): bool { - if ($attachment->getPath() === '') { - return false; - } - - if ($attachment->isExternal()) { + if($attachment->hasExternal()){ return true; } + return $this->isInternalFileExisting($attachment); + } - $absolute_path = $this->toAbsoluteFilePath($attachment); + /** + * Checks if the internal file in this attachment is existing. Returns false if the attachment doesn't have an + * internal file. + * + * @param Attachment $attachment The attachment for which the existence should be checked + * + * @return bool true if the file is existing + */ + public function isInternalFileExisting(Attachment $attachment): bool + { + $absolute_path = $this->toAbsoluteInternalFilePath($attachment); if (null === $absolute_path) { return false; @@ -117,21 +121,17 @@ class AttachmentManager /** * Returns the filesize of the attachments in bytes. - * For external attachments or not existing attachments, null is returned. + * For purely external attachments or inexistent attachments, null is returned. * * @param Attachment $attachment the filesize for which the filesize should be calculated */ public function getFileSize(Attachment $attachment): ?int { - if ($attachment->isExternal()) { + if (!$this->isInternalFileExisting($attachment)) { return null; } - if (!$this->isFileExisting($attachment)) { - return null; - } - - $tmp = filesize($this->toAbsoluteFilePath($attachment)); + $tmp = filesize($this->toAbsoluteInternalFilePath($attachment)); return false !== $tmp ? $tmp : null; } diff --git a/src/Services/Attachments/AttachmentPathResolver.php b/src/Services/Attachments/AttachmentPathResolver.php index e3e7a3ca..1b52c89b 100644 --- a/src/Services/Attachments/AttachmentPathResolver.php +++ b/src/Services/Attachments/AttachmentPathResolver.php @@ -115,12 +115,16 @@ class AttachmentPathResolver * Converts an relative placeholder filepath (with %MEDIA% or older %BASE%) to an absolute filepath on disk. * The directory separator is always /. Relative pathes are not realy possible (.. is striped). * - * @param string $placeholder_path the filepath with placeholder for which the real path should be determined + * @param string|null $placeholder_path the filepath with placeholder for which the real path should be determined * * @return string|null The absolute real path of the file, or null if the placeholder path is invalid */ - public function placeholderToRealPath(string $placeholder_path): ?string + public function placeholderToRealPath(?string $placeholder_path): ?string { + if (null === $placeholder_path) { + return null; + } + //The new attachments use %MEDIA% as placeholders, which is the directory set in media_directory //Older path entries are given via %BASE% which was the project root diff --git a/src/Services/Attachments/AttachmentReverseSearch.php b/src/Services/Attachments/AttachmentReverseSearch.php index 5f4f86de..e05192d0 100644 --- a/src/Services/Attachments/AttachmentReverseSearch.php +++ b/src/Services/Attachments/AttachmentReverseSearch.php @@ -55,7 +55,7 @@ class AttachmentReverseSearch $repo = $this->em->getRepository(Attachment::class); return $repo->findBy([ - 'path' => [$relative_path_new, $relative_path_old], + 'internal_path' => [$relative_path_new, $relative_path_old], ]); } diff --git a/src/Services/Attachments/AttachmentSubmitHandler.php b/src/Services/Attachments/AttachmentSubmitHandler.php index d9b2a380..fc54dd2f 100644 --- a/src/Services/Attachments/AttachmentSubmitHandler.php +++ b/src/Services/Attachments/AttachmentSubmitHandler.php @@ -207,7 +207,7 @@ class AttachmentSubmitHandler if ($file instanceof UploadedFile) { $this->upload($attachment, $file, $secure_attachment); - } elseif ($upload->downloadUrl && $attachment->isExternal()) { + } elseif ($upload->downloadUrl && $attachment->hasExternal()) { $this->downloadURL($attachment, $secure_attachment); } @@ -244,12 +244,12 @@ class AttachmentSubmitHandler protected function renameBlacklistedExtensions(Attachment $attachment): Attachment { //We can not do anything on builtins or external ressources - if ($attachment->isBuiltIn() || $attachment->isExternal()) { + if ($attachment->isBuiltIn() || !$attachment->hasInternal()) { return $attachment; } //Determine the old filepath - $old_path = $this->pathResolver->placeholderToRealPath($attachment->getPath()); + $old_path = $this->pathResolver->placeholderToRealPath($attachment->getInternalPath()); if ($old_path === null || $old_path === '' || !file_exists($old_path)) { return $attachment; } @@ -267,7 +267,7 @@ class AttachmentSubmitHandler $fs->rename($old_path, $new_path); //Update the attachment - $attachment->setPath($this->pathResolver->realPathToPlaceholder($new_path)); + $attachment->setInternalPath($this->pathResolver->realPathToPlaceholder($new_path)); } @@ -275,17 +275,17 @@ class AttachmentSubmitHandler } /** - * Move the given attachment to secure location (or back to public folder) if needed. + * Move the internal copy of the given attachment to a secure location (or back to public folder) if needed. * * @param Attachment $attachment the attachment for which the file should be moved * @param bool $secure_location this value determines, if the attachment is moved to the secure or public folder * - * @return Attachment The attachment with the updated filepath + * @return Attachment The attachment with the updated internal filepath */ protected function moveFile(Attachment $attachment, bool $secure_location): Attachment { //We can not do anything on builtins or external ressources - if ($attachment->isBuiltIn() || $attachment->isExternal()) { + if ($attachment->isBuiltIn() || !$attachment->hasInternal()) { return $attachment; } @@ -295,7 +295,7 @@ class AttachmentSubmitHandler } //Determine the old filepath - $old_path = $this->pathResolver->placeholderToRealPath($attachment->getPath()); + $old_path = $this->pathResolver->placeholderToRealPath($attachment->getInternalPath()); if (!file_exists($old_path)) { return $attachment; } @@ -319,7 +319,7 @@ class AttachmentSubmitHandler //Save info to attachment entity $new_path = $this->pathResolver->realPathToPlaceholder($new_path); - $attachment->setPath($new_path); + $attachment->setInternalPath($new_path); return $attachment; } @@ -329,7 +329,7 @@ class AttachmentSubmitHandler * * @param bool $secureAttachment True if the file should be moved to the secure attachment storage * - * @return Attachment The attachment with the new filepath + * @return Attachment The attachment with the downloaded copy */ protected function downloadURL(Attachment $attachment, bool $secureAttachment): Attachment { @@ -338,7 +338,7 @@ class AttachmentSubmitHandler throw new RuntimeException('Download of attachments is not allowed!'); } - $url = $attachment->getURL(); + $url = $attachment->getExternalPath(); $fs = new Filesystem(); $attachment_folder = $this->generateAttachmentPath($attachment, $secureAttachment); @@ -399,7 +399,7 @@ class AttachmentSubmitHandler //Make our file path relative to %BASE% $new_path = $this->pathResolver->realPathToPlaceholder($new_path); //Save the path to the attachment - $attachment->setPath($new_path); + $attachment->setInternalPath($new_path); } catch (TransportExceptionInterface) { throw new AttachmentDownloadException('Transport error!'); } @@ -427,7 +427,9 @@ class AttachmentSubmitHandler //Make our file path relative to %BASE% $file_path = $this->pathResolver->realPathToPlaceholder($file_path); //Save the path to the attachment - $attachment->setPath($file_path); + $attachment->setInternalPath($file_path); + //reset any external paths the attachment might have had + $attachment->setExternalPath(null); //And save original filename $attachment->setFilename($file->getClientOriginalName()); diff --git a/src/Services/Attachments/AttachmentURLGenerator.php b/src/Services/Attachments/AttachmentURLGenerator.php index d28a8d65..c22cefe4 100644 --- a/src/Services/Attachments/AttachmentURLGenerator.php +++ b/src/Services/Attachments/AttachmentURLGenerator.php @@ -92,9 +92,9 @@ class AttachmentURLGenerator * Returns a URL under which the attachment file can be viewed. * @return string|null The URL or null if the attachment file is not existing */ - public function getViewURL(Attachment $attachment): ?string + public function getInternalViewURL(Attachment $attachment): ?string { - $absolute_path = $this->attachmentHelper->toAbsoluteFilePath($attachment); + $absolute_path = $this->attachmentHelper->toAbsoluteInternalFilePath($attachment); if (null === $absolute_path) { return null; } @@ -111,6 +111,7 @@ class AttachmentURLGenerator /** * Returns a URL to a thumbnail of the attachment file. + * For external files the original URL is returned. * @return string|null The URL or null if the attachment file is not existing */ public function getThumbnailURL(Attachment $attachment, string $filter_name = 'thumbnail_sm'): ?string @@ -119,11 +120,14 @@ class AttachmentURLGenerator throw new InvalidArgumentException('Thumbnail creation only works for picture attachments!'); } - if ($attachment->isExternal() && ($attachment->getURL() !== null && $attachment->getURL() !== '')) { - return $attachment->getURL(); + if (!$attachment->hasInternal()){ + if($attachment->hasExternal()) { + return $attachment->getExternalPath(); + } + return null; } - $absolute_path = $this->attachmentHelper->toAbsoluteFilePath($attachment); + $absolute_path = $this->attachmentHelper->toAbsoluteInternalFilePath($attachment); if (null === $absolute_path) { return null; } @@ -137,7 +141,7 @@ class AttachmentURLGenerator //GD can not work with SVG, so serve it directly... //We can not use getExtension here, because it uses the original filename and not the real extension //Instead we use the logic, which is also used to determine if the attachment is a picture - $extension = pathinfo(parse_url($attachment->getPath(), PHP_URL_PATH) ?? '', PATHINFO_EXTENSION); + $extension = pathinfo(parse_url($attachment->getInternalPath(), PHP_URL_PATH) ?? '', PATHINFO_EXTENSION); if ('svg' === $extension) { return $this->assets->getUrl($asset_path); } @@ -157,7 +161,7 @@ class AttachmentURLGenerator /** * Returns a download link to the file associated with the attachment. */ - public function getDownloadURL(Attachment $attachment): string + public function getInternalDownloadURL(Attachment $attachment): string { //Redirect always to download controller, which sets the correct headers for downloading: return $this->urlGenerator->generate('attachment_download', ['id' => $attachment->getID()]); diff --git a/src/Services/EntityMergers/Mergers/EntityMergerHelperTrait.php b/src/Services/EntityMergers/Mergers/EntityMergerHelperTrait.php index a5c9a5fa..64c952a9 100644 --- a/src/Services/EntityMergers/Mergers/EntityMergerHelperTrait.php +++ b/src/Services/EntityMergers/Mergers/EntityMergerHelperTrait.php @@ -247,7 +247,8 @@ trait EntityMergerHelperTrait { return $this->mergeCollections($target, $other, 'attachments', fn(Attachment $t, Attachment $o): bool => $t->getName() === $o->getName() && $t->getAttachmentType() === $o->getAttachmentType() - && $t->getPath() === $o->getPath()); + && $t->getExternalPath() === $o->getExternalPath() + && $t->getInternalPath() === $o->getInternalPath()); } /** diff --git a/src/Services/EntityURLGenerator.php b/src/Services/EntityURLGenerator.php index 5718daec..c66b5fd9 100644 --- a/src/Services/EntityURLGenerator.php +++ b/src/Services/EntityURLGenerator.php @@ -156,25 +156,32 @@ class EntityURLGenerator public function viewURL(Attachment $entity): string { - if ($entity->isExternal()) { //For external attachments, return the link to external path - return $entity->getURL() ?? throw new \RuntimeException('External attachment has no URL!'); + if ($entity->hasInternal()) { + return $this->attachmentURLGenerator->getInternalViewURL($entity); } - //return $this->urlGenerator->generate('attachment_view', ['id' => $entity->getID()]); - return $this->attachmentURLGenerator->getViewURL($entity) ?? ''; + + if($entity->hasExternal()) { + return $entity->getExternalPath(); + } + + throw new \RuntimeException('Attachment has no internal nor external path!'); } public function downloadURL($entity): string { - if ($entity instanceof Attachment) { - if ($entity->isExternal()) { //For external attachments, return the link to external path - return $entity->getURL() ?? throw new \RuntimeException('External attachment has no URL!'); - } - - return $this->attachmentURLGenerator->getDownloadURL($entity); + if (!($entity instanceof Attachment)) { + throw new EntityNotSupportedException(sprintf('The given entity is not supported yet! Passed class type: %s', $entity::class)); } - //Otherwise throw an error - throw new EntityNotSupportedException(sprintf('The given entity is not supported yet! Passed class type: %s', $entity::class)); + if ($entity->hasInternal()) { + return $this->attachmentURLGenerator->getInternalDownloadURL($entity); + } + + if($entity->hasExternal()) { + return $entity->getExternalPath(); + } + + throw new \RuntimeException('Attachment has not internal or external path!'); } /** diff --git a/src/Services/ImportExportSystem/PartKeeprImporter/PKImportHelperTrait.php b/src/Services/ImportExportSystem/PartKeeprImporter/PKImportHelperTrait.php index 5489fc8c..1e4cd3ba 100644 --- a/src/Services/ImportExportSystem/PartKeeprImporter/PKImportHelperTrait.php +++ b/src/Services/ImportExportSystem/PartKeeprImporter/PKImportHelperTrait.php @@ -105,7 +105,7 @@ trait PKImportHelperTrait //Next comes the filename plus extension $path .= '/'.$attachment_row['filename'].'.'.$attachment_row['extension']; - $attachment->setPath($path); + $attachment->setInternalPath($path); return $attachment; } diff --git a/src/Services/LabelSystem/SandboxedTwigFactory.php b/src/Services/LabelSystem/SandboxedTwigFactory.php index cdf0594f..d6ea6968 100644 --- a/src/Services/LabelSystem/SandboxedTwigFactory.php +++ b/src/Services/LabelSystem/SandboxedTwigFactory.php @@ -122,8 +122,8 @@ final class SandboxedTwigFactory 'getFullPath', 'getPathArray', 'getSubelements', 'getChildren', 'isNotSelectable', ], AbstractCompany::class => ['getAddress', 'getPhoneNumber', 'getFaxNumber', 'getEmailAddress', 'getWebsite', 'getAutoProductUrl'], AttachmentContainingDBElement::class => ['getAttachments', 'getMasterPictureAttachment'], - Attachment::class => ['isPicture', 'is3DModel', 'isExternal', 'isSecure', 'isBuiltIn', 'getExtension', - 'getElement', 'getURL', 'getHost', 'getFilename', 'getAttachmentType', 'getShowInTable', ], + Attachment::class => ['isPicture', 'is3DModel', 'hasExternal', 'hasInternal', 'isSecure', 'isBuiltIn', 'getExtension', + 'getElement', 'getExternalPath', 'getHost', 'getFilename', 'getAttachmentType', 'getShowInTable'], AbstractParameter::class => ['getFormattedValue', 'getGroup', 'getSymbol', 'getValueMin', 'getValueMax', 'getValueTypical', 'getUnit', 'getValueText', ], MeasurementUnit::class => ['getUnit', 'isInteger', 'useSIPrefix'], diff --git a/templates/parts/edit/edit_form_styles.html.twig b/templates/parts/edit/edit_form_styles.html.twig index cf9a40b9..062122de 100644 --- a/templates/parts/edit/edit_form_styles.html.twig +++ b/templates/parts/edit/edit_form_styles.html.twig @@ -152,35 +152,32 @@ {% set attach = form.vars.value %} + {# @var \App\Entity\Attachments\Attachment attach #} {% if attach is not null %} - {% if attachment_manager.fileExisting(attach) %} - {% if not attach.external %} -

-
+ {% if not attach.hasInternal() and attach.external %} +
- {{ attach.filename }} + {% trans %}attachment.external_only{% endtrans %} -
- +
+ {% elseif attachment_manager.isInternalFileExisting(attach) %} +
+
+ {{ attach.filename|u.truncate(25, ' ...') }} +
+
+
{{ attachment_manager.humanFileSize(attach) }} - -
- {% else %} -

-
- - {% trans %}attachment.external{% endtrans %} - -
- {% endif %} + + {% if attach.secure %} -
+
{% trans %}attachment.secure{% endtrans %} -
+ {% endif %} {% if attach.secure and not is_granted('show_private', attach) %} @@ -190,16 +187,21 @@ {% trans %}attachment.preview.alt{% endtrans %} {% else %} - {% trans %}attachment.view{% endtrans %} + {% trans %}attachment.view_local{% endtrans %} {% endif %} {% else %} -

-
+
{% trans %}attachment.file_not_found{% endtrans %} -
+ + {% endif %} + {% if attach.external %} +
+ {% trans %}attachment.view_external{% endtrans %} +
{% endif %} {% endif %} diff --git a/templates/parts/info/_attachments_info.html.twig b/templates/parts/info/_attachments_info.html.twig index 995c69eb..4f7c0455 100644 --- a/templates/parts/info/_attachments_info.html.twig +++ b/templates/parts/info/_attachments_info.html.twig @@ -24,18 +24,16 @@ {{ attachment.name }} {{ attachment.attachmentType.fullPath }} - {% if attachment.external %} - {{ attachment.host }} - {% else %} + {% if attachment.hasInternal() %} {{ attachment.filename }} {% endif %} - {% if attachment.external %} + {% if not attachment.hasInternal() %} - {% trans %}attachment.external{% endtrans %} + {% trans %}attachment.external_only{% endtrans %} - {% elseif attachment_manager.fileExisting(attachment) %} + {% elseif attachment_manager.internalFileExisting(attachment) %} {{ attachment_manager.humanFileSize(attachment) }} @@ -58,14 +56,19 @@
- + + + + - + diff --git a/tests/API/Endpoints/AttachmentsEndpointTest.php b/tests/API/Endpoints/AttachmentsEndpointTest.php index 8084db5c..8f4d7e77 100644 --- a/tests/API/Endpoints/AttachmentsEndpointTest.php +++ b/tests/API/Endpoints/AttachmentsEndpointTest.php @@ -91,7 +91,7 @@ class AttachmentsEndpointTest extends AuthenticatedApiTestCase //Attachment must be set (not null) $array = json_decode($response->getContent(), true); - self::assertNotNull($array['media_url']); + self::assertNotNull($array['internal_path']); //Attachment must be private self::assertJsonContains([ diff --git a/tests/Entity/Attachments/AttachmentTest.php b/tests/Entity/Attachments/AttachmentTest.php index a2179e53..bac28fd4 100644 --- a/tests/Entity/Attachments/AttachmentTest.php +++ b/tests/Entity/Attachments/AttachmentTest.php @@ -59,14 +59,15 @@ class AttachmentTest extends TestCase $this->assertNull($attachment->getAttachmentType()); $this->assertFalse($attachment->isPicture()); - $this->assertFalse($attachment->isExternal()); + $this->assertFalse($attachment->hasExternal()); + $this->assertFalse($attachment->hasInternal()); $this->assertFalse($attachment->isSecure()); $this->assertFalse($attachment->isBuiltIn()); $this->assertFalse($attachment->is3DModel()); $this->assertFalse($attachment->getShowInTable()); - $this->assertEmpty($attachment->getPath()); + $this->assertEmpty($attachment->getInternalPath()); + $this->assertEmpty($attachment->getExternalPath()); $this->assertEmpty($attachment->getName()); - $this->assertEmpty($attachment->getURL()); $this->assertEmpty($attachment->getExtension()); $this->assertNull($attachment->getElement()); $this->assertEmpty($attachment->getFilename()); @@ -119,82 +120,63 @@ class AttachmentTest extends TestCase $attachment->setElement($element); } - public function externalDataProvider(): \Iterator - { - yield ['', false]; - yield ['%MEDIA%/foo/bar.txt', false]; - yield ['%BASE%/foo/bar.jpg', false]; - yield ['%FOOTPRINTS%/foo/bar.jpg', false]; - yield ['%FOOTPRINTS3D%/foo/bar.jpg', false]; - yield ['%SECURE%/test.txt', false]; - yield ['%test%/foo/bar.ghp', true]; - yield ['foo%MEDIA%/foo.jpg', true]; - yield ['foo%MEDIA%/%BASE%foo.jpg', true]; - } - /** - * @dataProvider externalDataProvider - */ - public function testIsExternal($path, $expected): void + public static function extensionDataProvider(): \Iterator { - $attachment = new PartAttachment(); - $this->setProtectedProperty($attachment, 'path', $path); - $this->assertSame($expected, $attachment->isExternal()); - } - - public function extensionDataProvider(): \Iterator - { - yield ['%MEDIA%/foo/bar.txt', null, 'txt']; - yield ['%MEDIA%/foo/bar.JPeg', null, 'jpeg']; - yield ['%MEDIA%/foo/bar.JPeg', 'test.txt', 'txt']; - yield ['%MEDIA%/foo/bar', null, '']; - yield ['%MEDIA%/foo.bar', 'bar', '']; - yield ['http://google.de', null, null]; - yield ['https://foo.bar', null, null]; - yield ['https://foo.bar/test.jpeg', null, null]; - yield ['test', null, null]; - yield ['test.txt', null, null]; + yield ['%MEDIA%/foo/bar.txt', 'http://google.de', null, 'txt']; + yield ['%MEDIA%/foo/bar.JPeg', 'https://foo.bar', null, 'jpeg']; + yield ['%MEDIA%/foo/bar.JPeg', null, 'test.txt', 'txt']; + yield ['%MEDIA%/foo/bar', 'https://foo.bar/test.jpeg', null, '']; + yield ['%MEDIA%/foo.bar', 'test.txt', 'bar', '']; + yield [null, 'http://google.de', null, null]; + yield [null, 'https://foo.bar', null, null]; + yield [null, ',https://foo.bar/test.jpeg', null, null]; + yield [null, 'test', null, null]; + yield [null, 'test.txt', null, null]; } /** * @dataProvider extensionDataProvider */ - public function testGetExtension($path, $originalFilename, $expected): void + public function testGetExtension(?string $internal_path, ?string $external_path, ?string $originalFilename, ?string $expected): void { $attachment = new PartAttachment(); - $this->setProtectedProperty($attachment, 'path', $path); + $this->setProtectedProperty($attachment, 'internal_path', $internal_path); + $this->setProtectedProperty($attachment, 'external_path', $external_path); $this->setProtectedProperty($attachment, 'original_filename', $originalFilename); $this->assertSame($expected, $attachment->getExtension()); } - public function pictureDataProvider(): \Iterator + public static function pictureDataProvider(): \Iterator { - yield ['%MEDIA%/foo/bar.txt', false]; - yield ['https://test.de/picture.jpeg', true]; - yield ['https://test.de/picture.png?test=fdsj&width=34', true]; - yield ['https://invalid.invalid/file.txt', false]; - yield ['http://infsf.inda/file.zip?test', false]; - yield ['https://test.de', true]; - yield ['https://invalid.com/invalid/pic', true]; - yield ['%MEDIA%/foo/bar.jpeg', true]; - yield ['%MEDIA%/foo/bar.webp', true]; - yield ['%MEDIA%/foo', false]; - yield ['%SECURE%/foo.txt/test', false]; + yield [null, '%MEDIA%/foo/bar.txt', false]; + yield [null, 'https://test.de/picture.jpeg', true]; + yield [null, 'https://test.de/picture.png?test=fdsj&width=34', true]; + yield [null, 'https://invalid.invalid/file.txt', false]; + yield [null, 'http://infsf.inda/file.zip?test', false]; + yield [null, 'https://test.de', true]; + yield [null, 'https://invalid.com/invalid/pic', true]; + yield ['%MEDIA%/foo/bar.jpeg', 'https://invalid.invalid/file.txt', true]; + yield ['%MEDIA%/foo/bar.webp', '', true]; + yield ['%MEDIA%/foo', '', false]; + yield ['%SECURE%/foo.txt/test', 'https://test.de/picture.jpeg', false]; } /** * @dataProvider pictureDataProvider */ - public function testIsPicture($path, $expected): void + public function testIsPicture(?string $internal_path, ?string $external_path, bool $expected): void { $attachment = new PartAttachment(); - $this->setProtectedProperty($attachment, 'path', $path); + $this->setProtectedProperty($attachment, 'internal_path', $internal_path); + $this->setProtectedProperty($attachment, 'external_path', $external_path); $this->assertSame($expected, $attachment->isPicture()); } - public function builtinDataProvider(): \Iterator + public static function builtinDataProvider(): \Iterator { yield ['', false]; + yield [null, false]; yield ['%MEDIA%/foo/bar.txt', false]; yield ['%BASE%/foo/bar.txt', false]; yield ['/', false]; @@ -205,14 +187,14 @@ class AttachmentTest extends TestCase /** * @dataProvider builtinDataProvider */ - public function testIsBuiltIn($path, $expected): void + public function testIsBuiltIn(?string $path, $expected): void { $attachment = new PartAttachment(); - $this->setProtectedProperty($attachment, 'path', $path); + $this->setProtectedProperty($attachment, 'internal_path', $path); $this->assertSame($expected, $attachment->isBuiltIn()); } - public function hostDataProvider(): \Iterator + public static function hostDataProvider(): \Iterator { yield ['%MEDIA%/foo/bar.txt', null]; yield ['https://www.google.de/test.txt', 'www.google.de']; @@ -222,55 +204,60 @@ class AttachmentTest extends TestCase /** * @dataProvider hostDataProvider */ - public function testGetHost($path, $expected): void + public function testGetHost(?string $path, ?string $expected): void { $attachment = new PartAttachment(); - $this->setProtectedProperty($attachment, 'path', $path); + $this->setProtectedProperty($attachment, 'external_path', $path); $this->assertSame($expected, $attachment->getHost()); } - public function filenameProvider(): \Iterator + public static function filenameProvider(): \Iterator { - yield ['%MEDIA%/foo/bar.txt', null, 'bar.txt']; - yield ['%MEDIA%/foo/bar.JPeg', 'test.txt', 'test.txt']; - yield ['https://www.google.de/test.txt', null, null]; + yield ['%MEDIA%/foo/bar.txt', 'https://www.google.de/test.txt', null, 'bar.txt']; + yield ['%MEDIA%/foo/bar.JPeg', 'https://www.google.de/foo.txt', 'test.txt', 'test.txt']; + yield ['', 'https://www.google.de/test.txt', null, null]; + yield [null, 'https://www.google.de/test.txt', null, null]; } /** * @dataProvider filenameProvider */ - public function testGetFilename($path, $original_filename, $expected): void + public function testGetFilename(?string $internal_path, ?string $external_path, ?string $original_filename, ?string $expected): void { $attachment = new PartAttachment(); - $this->setProtectedProperty($attachment, 'path', $path); + $this->setProtectedProperty($attachment, 'internal_path', $internal_path); + $this->setProtectedProperty($attachment, 'external_path', $external_path); $this->setProtectedProperty($attachment, 'original_filename', $original_filename); $this->assertSame($expected, $attachment->getFilename()); } - public function testSetURL(): void + public function testSetExternalPath(): void { $attachment = new PartAttachment(); //Set URL - $attachment->setURL('https://google.de'); - $this->assertSame('https://google.de', $attachment->getURL()); + $attachment->setExternalPath('https://google.de'); + $this->assertSame('https://google.de', $attachment->getExternalPath()); - //Ensure that an empty url does not overwrite the existing one - $attachment->setPath('%MEDIA%/foo/bar.txt'); - $attachment->setURL(' '); - $this->assertSame('%MEDIA%/foo/bar.txt', $attachment->getPath()); + //Ensure that changing the external path does reset the internal one + $attachment->setInternalPath('%MEDIA%/foo/bar.txt'); + $attachment->setExternalPath('https://example.de'); + $this->assertSame(null, $attachment->getInternalPath()); + + //Ensure that setting the same value to the external path again doesn't reset the internal one + $attachment->setExternalPath('https://google.de'); + $attachment->setInternalPath('%MEDIA%/foo/bar.txt'); + $attachment->setExternalPath('https://google.de'); + $this->assertSame('%MEDIA%/foo/bar.txt', $attachment->getInternalPath()); + + //Ensure that resetting the external path doesn't reset the internal one + $attachment->setInternalPath('%MEDIA%/foo/bar.txt'); + $attachment->setExternalPath(''); + $this->assertSame('%MEDIA%/foo/bar.txt', $attachment->getInternalPath()); //Ensure that spaces get replaced by %20 - $attachment->setURL('https://google.de/test file.txt'); - $this->assertSame('https://google.de/test%20file.txt', $attachment->getURL()); - } - - public function testSetURLForbiddenURL(): void - { - $attachment = new PartAttachment(); - - $this->expectException(InvalidArgumentException::class); - $attachment->setURL('%MEDIA%/foo/bar.txt'); + $attachment->setExternalPath('https://example.de/test file.txt'); + $this->assertSame('https://example.de/test%20file.txt', $attachment->getExternalPath()); } public function testIsURL(): void diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index 77dd22da..41388de3 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -242,7 +242,7 @@ part.info.timetravel_hint - This is how the part appeared before %timestamp%. <i>Please note that this feature is experimental, so the info may not be correct.</i> + Please note that this feature is experimental, so the info may not be correct.]]> @@ -731,10 +731,10 @@ user.edit.tfa.disable_tfa_message - This will disable <b>all active two-factor authentication methods of the user</b> and delete the <b>backup codes</b>! -<br> -The user will have to set up all two-factor authentication methods again and print new backup codes! <br><br> -<b>Only do this if you are absolutely sure about the identity of the user (seeking help), otherwise the account could be compromised by an attacker!</b> + all active two-factor authentication methods of the user and delete the backup codes! +
+The user will have to set up all two-factor authentication methods again and print new backup codes!

+Only do this if you are absolutely sure about the identity of the user (seeking help), otherwise the account could be compromised by an attacker!]]>
@@ -780,18 +780,10 @@ The user will have to set up all two-factor authentication methods again and pri Delete - - - Part-DB1\templates\AdminPages\_attachments.html.twig:41 - Part-DB1\templates\Parts\edit\_attachments.html.twig:38 - Part-DB1\templates\Parts\info\_attachments_info.html.twig:35 - Part-DB1\src\DataTables\AttachmentDataTable.php:159 - Part-DB1\templates\Parts\edit\_attachments.html.twig:38 - Part-DB1\src\DataTables\AttachmentDataTable.php:159 - + - attachment.external - External + attachment.external_only + External only @@ -806,7 +798,7 @@ The user will have to set up all two-factor authentication methods again and pri Attachment thumbnail - + Part-DB1\templates\AdminPages\_attachments.html.twig:52 Part-DB1\templates\Parts\edit\_attachments.html.twig:50 @@ -816,8 +808,8 @@ The user will have to set up all two-factor authentication methods again and pri Part-DB1\templates\Parts\info\_attachments_info.html.twig:45 - attachment.view - View + attachment.view_local + View Local Copy @@ -893,9 +885,9 @@ The user will have to set up all two-factor authentication methods again and pri entity.delete.message - This can not be undone! -<br> -Sub elements will be moved upwards. + +Sub elements will be moved upwards.]]> @@ -1449,7 +1441,7 @@ Sub elements will be moved upwards. homepage.github.text - Source, downloads, bug reports, to-do-list etc. can be found on <a href="%href%" class="link-external" target="_blank">GitHub project page</a> + GitHub project page]]> @@ -1471,7 +1463,7 @@ Sub elements will be moved upwards. homepage.help.text - Help and tips can be found in Wiki the <a href="%href%" class="link-external" target="_blank">GitHub page</a> + GitHub page]]> @@ -1713,7 +1705,7 @@ Sub elements will be moved upwards. email.pw_reset.fallback - If this does not work for you, go to <a href="%url%">%url%</a> and enter the following info + %url% and enter the following info]]> @@ -1743,7 +1735,7 @@ Sub elements will be moved upwards. email.pw_reset.valid_unit %date% - The reset token will be valid until <i>%date%</i>. + %date%.]]> @@ -2119,14 +2111,14 @@ Sub elements will be moved upwards. Preview picture - + Part-DB1\templates\Parts\info\_attachments_info.html.twig:67 Part-DB1\templates\Parts\info\_attachments_info.html.twig:50 - attachment.download - Download + attachment.download_local + Download Local Copy @@ -3586,8 +3578,8 @@ Sub elements will be moved upwards. tfa_google.disable.confirm_message - If you disable the Authenticator App, all backup codes will be deleted, so you may need to reprint them.<br> -Also note that without two-factor authentication, your account is no longer as well protected against attackers! + +Also note that without two-factor authentication, your account is no longer as well protected against attackers!]]> @@ -3607,7 +3599,7 @@ Also note that without two-factor authentication, your account is no longer as w tfa_google.step.download - Download an authenticator app (e.g. <a class="link-external" target="_blank" href="https://play.google.com/store/apps/details?id=com.google.android.apps.authenticator2">Google Authenticator</a> oder <a class="link-external" target="_blank" href="https://play.google.com/store/apps/details?id=org.fedorahosted.freeotp">FreeOTP Authenticator</a>) + Google Authenticator oder FreeOTP Authenticator)]]> @@ -3849,8 +3841,8 @@ Also note that without two-factor authentication, your account is no longer as w tfa_trustedDevices.explanation - When checking the second factor, the current computer can be marked as trustworthy, so no more two-factor checks on this computer are needed. -If you have done this incorrectly or if a computer is no longer trusted, you can reset the status of <i>all </i>computers here. + all computers here.]]> @@ -5321,7 +5313,7 @@ If you have done this incorrectly or if a computer is no longer trusted, you can label_options.lines_mode.help - If you select Twig here, the content field is interpreted as Twig template. See <a href="https://twig.symfony.com/doc/3.x/templates.html">Twig documentation</a> and <a href="https://docs.part-db.de/usage/labels.html#twig-mode">Wiki</a> for more information. + Twig documentation and Wiki for more information.]]> @@ -9396,25 +9388,25 @@ Element 3 filter.parameter_value_constraint.operator.< - Typ. Value < + filter.parameter_value_constraint.operator.> - Typ. Value > + ]]> filter.parameter_value_constraint.operator.<= - Typ. Value <= + filter.parameter_value_constraint.operator.>= - Typ. Value >= + =]]> @@ -9522,7 +9514,7 @@ Element 3 parts_list.search.searching_for - Searching parts with keyword <b>%keyword%</b> + %keyword%]]> @@ -10182,13 +10174,13 @@ Element 3 project.builds.number_of_builds_possible - You have enough stocked to build <b>%max_builds%</b> builds of this project. + %max_builds% builds of this project.]]> project.builds.check_project_status - The current project status is <b>"%project_status%"</b>. You should check if you really want to build the project with this status! + "%project_status%". You should check if you really want to build the project with this status!]]> @@ -10290,7 +10282,7 @@ Element 3 entity.select.add_hint - Use -> to create nested structures, e.g. "Node 1->Node 1.1" + to create nested structures, e.g. "Node 1->Node 1.1"]]> @@ -10314,13 +10306,13 @@ Element 3 homepage.first_steps.introduction - Your database is still empty. You might want to read the <a href="%url%">documentation</a> or start to creating the following data structures: + documentation or start to creating the following data structures:]]> homepage.first_steps.create_part - Or you can directly <a href="%url%">create a new part</a>. + create a new part.]]> @@ -10332,7 +10324,7 @@ Element 3 homepage.forum.text - For questions about Part-DB use the <a href="%href%" class="link-external" target="_blank">discussion forum</a> + discussion forum]]> @@ -10986,7 +10978,7 @@ Element 3 parts.import.help_documentation - See the <a href="%link%">documentation</a> for more information on the file format. + documentation for more information on the file format.]]> @@ -11166,7 +11158,7 @@ Element 3 part.filter.lessThanDesired - In stock less than desired (total amount < min. amount) + @@ -11978,13 +11970,13 @@ Please note, that you can not impersonate a disabled user. If you try you will g part.merge.confirm.title - Do you really want to merge <b>%other%</b> into <b>%target%</b>? + %other% into %target%?]]> part.merge.confirm.message - <b>%other%</b> will be deleted, and the part will be saved with the shown information. + %other% will be deleted, and the part will be saved with the shown information.]]> @@ -12329,5 +12321,29 @@ Please note, that you can not impersonate a disabled user. If you try you will g There are no entities to export! + + + attachment.table.internal_file + Internal file + + + + + attachment.table.external_link + External link + + + + + attachment.view_external.view_at + View at %host% + + + + + attachment.view_external + View external version + +