Skip to content

Commit 29f92d9

Browse files
Treeedjonajbtronics
authored
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 <[email protected]> Co-authored-by: Jan Böhmer <[email protected]>
1 parent ebb977e commit 29f92d9

File tree

22 files changed

+557
-367
lines changed

22 files changed

+557
-367
lines changed

‎migrations/Version20250220215048.php‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace DoctrineMigrations;
6+
7+
use App\Migration\AbstractMultiPlatformMigration;
8+
use Doctrine\DBAL\Schema\Schema;
9+
use Doctrine\Migrations\AbstractMigration;
10+
11+
/**
12+
* Auto-generated Migration: Please modify to your needs!
13+
*/
14+
final class Version20250220215048 extends AbstractMultiPlatformMigration
15+
{
16+
public function getDescription(): string
17+
{
18+
return 'Split $path property for attachments into $internal_path and $external_path';
19+
}
20+
21+
public function mySQLUp(Schema $schema): void
22+
{
23+
//Create the new columns as nullable (that is easier modifying them)
24+
$this->addSql('ALTER TABLE attachments ADD internal_path VARCHAR(255) DEFAULT NULL, ADD external_path VARCHAR(255) DEFAULT NULL');
25+
26+
//Copy the data from path to external_path and remove the path column
27+
$this->addSql('UPDATE attachments SET external_path=path');
28+
$this->addSql('ALTER TABLE attachments DROP path');
29+
30+
31+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%MEDIA#%%\' ESCAPE \'#\'');
32+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%BASE#%%\' ESCAPE \'#\'');
33+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%SECURE#%%\' ESCAPE \'#\'');
34+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%FOOTPRINTS#%%\' ESCAPE \'#\'');
35+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%FOOTPRINTS3D#%%\' ESCAPE \'#\'');
36+
$this->addSql('UPDATE attachments SET external_path=NULL WHERE internal_path IS NOT NULL');
37+
}
38+
39+
public function mySQLDown(Schema $schema): void
40+
{
41+
$this->addSql('UPDATE attachments SET external_path=internal_path WHERE internal_path IS NOT NULL');
42+
$this->addSql('ALTER TABLE attachments DROP internal_path');
43+
$this->addSql('ALTER TABLE attachments RENAME COLUMN external_path TO path');
44+
}
45+
46+
public function postgreSQLUp(Schema $schema): void
47+
{
48+
//We can use the same SQL for PostgreSQL as for MySQL
49+
$this->mySQLUp($schema);
50+
}
51+
52+
public function postgreSQLDown(Schema $schema): void
53+
{
54+
//We can use the same SQL for PostgreSQL as for MySQL
55+
$this->mySQLDown($schema);
56+
}
57+
58+
public function sqLiteUp(Schema $schema): void
59+
{
60+
$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');
61+
$this->addSql('DROP TABLE attachments');
62+
$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)');
63+
$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');
64+
$this->addSql('DROP TABLE __temp__attachments');
65+
$this->addSql('CREATE INDEX attachment_element_idx ON attachments (class_name, element_id)');
66+
$this->addSql('CREATE INDEX attachment_name_idx ON attachments (name)');
67+
$this->addSql('CREATE INDEX attachments_idx_class_name_id ON attachments (class_name, id)');
68+
$this->addSql('CREATE INDEX attachments_idx_id_element_id_class_name ON attachments (id, element_id, class_name)');
69+
$this->addSql('CREATE INDEX IDX_47C4FAD6C54C8C93 ON attachments (type_id)');
70+
$this->addSql('CREATE INDEX IDX_47C4FAD61F1F2A24 ON attachments (element_id)');
71+
72+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%MEDIA#%%\' ESCAPE \'#\'');
73+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%BASE#%%\' ESCAPE \'#\'');
74+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%SECURE#%%\' ESCAPE \'#\'');
75+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%FOOTPRINTS#%%\' ESCAPE \'#\'');
76+
$this->addSql('UPDATE attachments SET internal_path=external_path WHERE external_path LIKE \'#%FOOTPRINTS3D#%%\' ESCAPE \'#\'');
77+
$this->addSql('UPDATE attachments SET external_path=NULL WHERE internal_path IS NOT NULL');
78+
}
79+
80+
public function sqLiteDown(Schema $schema): void
81+
{
82+
//Reuse the MySQL down migration:
83+
$this->mySQLDown($schema);
84+
}
85+
86+
87+
}

‎src/Controller/AttachmentFileController.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,15 @@ public function download(Attachment $attachment, AttachmentManager $helper): Bin
5151
$this->denyAccessUnlessGranted('show_private', $attachment);
5252
}
5353

54-
if ($attachment->isExternal()) {
55-
throw $this->createNotFoundException('The file for this attachment is external and can not stored locally!');
54+
if (!$attachment->hasInternal()) {
55+
throw $this->createNotFoundException('The file for this attachment is external and not stored locally!');
5656
}
5757

58-
if (!$helper->isFileExisting($attachment)) {
58+
if (!$helper->isInternalFileExisting($attachment)) {
5959
throw $this->createNotFoundException('The file associated with the attachment is not existing!');
6060
}
6161

62-
$file_path = $helper->toAbsoluteFilePath($attachment);
62+
$file_path = $helper->toAbsoluteInternalFilePath($attachment);
6363
$response = new BinaryFileResponse($file_path);
6464

6565
//Set header content disposition, so that the file will be downloaded
@@ -80,15 +80,15 @@ public function view(Attachment $attachment, AttachmentManager $helper): BinaryF
8080
$this->denyAccessUnlessGranted('show_private', $attachment);
8181
}
8282

83-
if ($attachment->isExternal()) {
84-
throw $this->createNotFoundException('The file for this attachment is external and can not stored locally!');
83+
if (!$attachment->hasInternal()) {
84+
throw $this->createNotFoundException('The file for this attachment is external and not stored locally!');
8585
}
8686

87-
if (!$helper->isFileExisting($attachment)) {
87+
if (!$helper->isInternalFileExisting($attachment)) {
8888
throw $this->createNotFoundException('The file associated with the attachment is not existing!');
8989
}
9090

91-
$file_path = $helper->toAbsoluteFilePath($attachment);
91+
$file_path = $helper->toAbsoluteInternalFilePath($attachment);
9292
$response = new BinaryFileResponse($file_path);
9393

9494
//Set header content disposition, so that the file will be downloaded

‎src/DataFixtures/PartFixtures.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ public function load(ObjectManager $manager): void
131131

132132
$attachment = new PartAttachment();
133133
$attachment->setName('Test2');
134-
$attachment->setPath('invalid');
134+
$attachment->setInternalPath('invalid');
135135
$attachment->setShowInTable(true);
136136
$attachment->setAttachmentType($manager->find(AttachmentType::class, 1));
137137
$part->addAttachment($attachment);

‎src/DataTables/AttachmentDataTable.php‎

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ public function configure(DataTable $dataTable, array $options): void
5050
{
5151
$dataTable->add('dont_matter', RowClassColumn::class, [
5252
'render' => function ($value, Attachment $context): string {
53-
//Mark attachments with missing files yellow
54-
if(!$this->attachmentHelper->isFileExisting($context)){
53+
//Mark attachments yellow which have an internal file linked that doesn't exist
54+
if($context->hasInternal() && !$this->attachmentHelper->isInternalFileExisting($context)){
5555
return 'table-warning';
5656
}
5757

@@ -64,8 +64,8 @@ public function configure(DataTable $dataTable, array $options): void
6464
'className' => 'no-colvis',
6565
'render' => function ($value, Attachment $context): string {
6666
if ($context->isPicture()
67-
&& !$context->isExternal()
68-
&& $this->attachmentHelper->isFileExisting($context)) {
67+
&& $this->attachmentHelper->isInternalFileExisting($context)) {
68+
6969
$title = htmlspecialchars($context->getName());
7070
if ($context->getFilename()) {
7171
$title .= ' ('.htmlspecialchars($context->getFilename()).')';
@@ -93,26 +93,6 @@ public function configure(DataTable $dataTable, array $options): void
9393
$dataTable->add('name', TextColumn::class, [
9494
'label' => 'attachment.edit.name',
9595
'orderField' => 'NATSORT(attachment.name)',
96-
'render' => function ($value, Attachment $context) {
97-
//Link to external source
98-
if ($context->isExternal()) {
99-
return sprintf(
100-
'<a href="%s" class="link-external">%s</a>',
101-
htmlspecialchars((string) $context->getURL()),
102-
htmlspecialchars($value)
103-
);
104-
}
105-
106-
if ($this->attachmentHelper->isFileExisting($context)) {
107-
return sprintf(
108-
'<a href="%s" target="_blank" data-no-ajax>%s</a>',
109-
$this->entityURLGenerator->viewURL($context),
110-
htmlspecialchars($value)
111-
);
112-
}
113-
114-
return $value;
115-
},
11696
]);
11797

11898
$dataTable->add('attachment_type', TextColumn::class, [
@@ -136,25 +116,57 @@ public function configure(DataTable $dataTable, array $options): void
136116
),
137117
]);
138118

139-
$dataTable->add('filename', TextColumn::class, [
140-
'label' => $this->translator->trans('attachment.table.filename'),
119+
$dataTable->add('internal_link', TextColumn::class, [
120+
'label' => 'attachment.table.internal_file',
141121
'propertyPath' => 'filename',
122+
'render' => function ($value, Attachment $context) {
123+
if ($this->attachmentHelper->isInternalFileExisting($context)) {
124+
return sprintf(
125+
'<a href="%s" target="_blank" data-no-ajax>%s</a>',
126+
$this->entityURLGenerator->viewURL($context),
127+
htmlspecialchars($value)
128+
);
129+
}
130+
131+
return $value;
132+
}
133+
]);
134+
135+
$dataTable->add('external_link', TextColumn::class, [
136+
'label' => 'attachment.table.external_link',
137+
'propertyPath' => 'host',
138+
'render' => function ($value, Attachment $context) {
139+
if ($context->hasExternal()) {
140+
return sprintf(
141+
'<a href="%s" class="link-external">%s</a>',
142+
htmlspecialchars((string) $context->getExternalPath()),
143+
htmlspecialchars($value)
144+
);
145+
}
146+
147+
return $value;
148+
}
142149
]);
143150

144151
$dataTable->add('filesize', TextColumn::class, [
145152
'label' => $this->translator->trans('attachment.table.filesize'),
146153
'render' => function ($value, Attachment $context) {
147-
if ($context->isExternal()) {
154+
if (!$context->hasInternal()) {
148155
return sprintf(
149156
'<span class="badge bg-primary">
150157
<i class="fas fa-globe fa-fw"></i>%s
151158
</span>',
152-
$this->translator->trans('attachment.external')
159+
$this->translator->trans('attachment.external_only')
153160
);
154161
}
155162

156-
if ($this->attachmentHelper->isFileExisting($context)) {
157-
return $this->attachmentHelper->getHumanFileSize($context);
163+
if ($this->attachmentHelper->isInternalFileExisting($context)) {
164+
return sprintf(
165+
'<span class="badge bg-secondary">
166+
<i class="fas fa-hdd fa-fw"></i> %s
167+
</span>',
168+
$this->attachmentHelper->getHumanFileSize($context)
169+
);
158170
}
159171

160172
return sprintf(

0 commit comments

Comments
 (0)