From 29f92d9bd366690a795cb8e7547fffeb1b2656bb Mon Sep 17 00:00:00 2001
From: Treeed <21248276+Treeed@users.noreply.github.com>
Date: Sat, 22 Feb 2025 17:29:14 +0100
Subject: [PATCH] Split attachment paths (#848)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* fixed attachment statistics for sqlite
* Split attachment path into internal and external path, so the external source URL can be retained after a file is downloaded
* Make internal and external path for attachments nullable, to make clear that they have no internal or external path
* Added migrations for nullable columns for postgres and mysql
* Added migration for nullable internal and external pathes for sqlite
* Added translations
* Fixed upload error
* Restrict length of filename badge in attachment edit view
* Improved margins with badges in attachment edit
* Added a link to view external version from attachment edit
* Let media_url stay in API attachments responses for backward compatibility
---------
Co-authored-by: jona
Co-authored-by: Jan Böhmer
---
migrations/Version20250220215048.php | 87 +++++++
src/Controller/AttachmentFileController.php | 16 +-
src/DataFixtures/PartFixtures.php | 2 +-
src/DataTables/AttachmentDataTable.php | 72 +++---
src/Entity/Attachments/Attachment.php | 235 +++++++++++-------
.../AttachmentDeleteListener.php | 4 +-
src/Repository/AttachmentRepository.php | 18 +-
src/Serializer/AttachmentNormalizer.php | 6 +-
.../Attachments/AttachmentManager.php | 54 ++--
.../Attachments/AttachmentPathResolver.php | 8 +-
.../Attachments/AttachmentReverseSearch.php | 2 +-
.../Attachments/AttachmentSubmitHandler.php | 28 ++-
.../Attachments/AttachmentURLGenerator.php | 18 +-
.../Mergers/EntityMergerHelperTrait.php | 3 +-
src/Services/EntityURLGenerator.php | 31 ++-
.../PartKeeprImporter/PKImportHelperTrait.php | 2 +-
.../LabelSystem/SandboxedTwigFactory.php | 4 +-
.../parts/edit/edit_form_styles.html.twig | 48 ++--
.../parts/info/_attachments_info.html.twig | 27 +-
.../API/Endpoints/AttachmentsEndpointTest.php | 2 +-
tests/Entity/Attachments/AttachmentTest.php | 149 +++++------
translations/messages.en.xlf | 116 +++++----
22 files changed, 561 insertions(+), 371 deletions(-)
create mode 100644 migrations/Version20250220215048.php
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 %}
+
- {% 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:52Part-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:67Part-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
+
+