Split attachment paths (#848)

* 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 <a@b.c>
Co-authored-by: Jan Böhmer <mail@jan-boehmer.de>
This commit is contained in:
Treeed 2025-02-22 17:29:14 +01:00 committed by GitHub
parent ebb977e99f
commit 29f92d9bd3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 561 additions and 371 deletions

View file

@ -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;
}

View file

@ -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

View file

@ -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],
]);
}

View file

@ -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());

View file

@ -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()]);

View file

@ -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());
}
/**

View file

@ -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!');
}
/**

View file

@ -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;
}

View file

@ -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'],