Skip to content

Commit ba6d3cf

Browse files
mejo-mejo-
authored
Merge pull request #7307 from nextcloud/backport/7298/stable31
[stable31] Fix: Replace attachment file IDs when copying markdown files
2 parents 6c1ffe1 + 7ab3666 commit ba6d3cf

3 files changed

Lines changed: 106 additions & 4 deletions

File tree

‎lib/Listeners/NodeCopiedListener.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,10 @@ public function handle(Event $event): void {
3636
&& $target instanceof File
3737
&& $target->getMimeType() === 'text/markdown'
3838
) {
39-
$this->attachmentService->copyAttachments($source, $target);
39+
$fileIdMapping = $this->attachmentService->copyAttachments($source, $target);
4040
$target->unlock(ILockingProvider::LOCK_SHARED);
4141
AttachmentService::replaceAttachmentFolderId($source, $target);
42+
AttachmentService::replaceAttachmentFileIds($target, $fileIdMapping);
4243
$target->lock(ILockingProvider::LOCK_SHARED);
4344
}
4445
}

‎lib/Service/AttachmentService.php‎

100644100755
Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -643,27 +643,34 @@ public function deleteAttachments(File $source): void {
643643
* @param File $source
644644
* @param File $target
645645
*
646+
* @return array file id translation map
646647
* @throws InvalidPathException
647648
* @throws NoUserException
648649
* @throws NotFoundException
649650
* @throws NotPermittedException
650651
* @throws LockedException
651652
*/
652-
public function copyAttachments(File $source, File $target): void {
653+
public function copyAttachments(File $source, File $target): array {
653654
try {
654655
$sourceAttachmentDir = $this->getAttachmentDirectoryForFile($source);
655656
} catch (NotFoundException $e) {
656657
// silently return if no attachment dir was found for source file
657-
return;
658+
return [];
658659
}
659660
// create a new attachment dir next to the new file
660661
$targetAttachmentDir = $this->getAttachmentDirectoryForFile($target, true);
661662
// copy the attachment files
663+
$fileIdMapping = [];
662664
foreach ($sourceAttachmentDir->getDirectoryListing() as $sourceAttachment) {
663665
if ($sourceAttachment instanceof File) {
664-
$targetAttachmentDir->newFile($sourceAttachment->getName(), $sourceAttachment->getContent());
666+
$newFile = $targetAttachmentDir->newFile($sourceAttachment->getName(), $sourceAttachment->getContent());
667+
$fileIdMapping[] = [
668+
$sourceAttachment->getId(),
669+
$newFile->getId()
670+
];
665671
}
666672
}
673+
return $fileIdMapping;
667674
}
668675

669676
public static function replaceAttachmentFolderId(File $source, File $target): void {
@@ -685,4 +692,18 @@ public static function replaceAttachmentFolderId(File $source, File $target): vo
685692
$target->putContent($content);
686693
}
687694
}
695+
696+
public static function replaceAttachmentFileIds(File $target, array $fileIdMapping): void {
697+
$patterns = [];
698+
$replacements = [];
699+
foreach ($fileIdMapping as $mapping) {
700+
$patterns[] = '/(\[(?:\\\]|[^]])+\]\(\s*\S+\/f\/)' . $mapping[0] . '(\s*)(\(preview\)\s*)?\)/';
701+
// Replace `[title](URL/f/sourceId (preview))` with `[title](URL/f/targetId (preview))`
702+
$replacements[] = '${1}' . $mapping[1] . '${2}${3})';
703+
}
704+
$content = preg_replace($patterns, $replacements, $target->getContent());
705+
if ($content !== null) {
706+
$target->putContent($content);
707+
}
708+
}
688709
}

‎tests/unit/Service/AttachmentServiceTest.php‎

100644100755
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,84 @@ public function testReplaceAttachmentFolderIdNoReplace(string $sourceContent): v
160160
AttachmentService::replaceAttachmentFolderId($source, $target);
161161
$this->assertEquals($sourceContent, $replacedContent);
162162
}
163+
164+
// Replacement expected
165+
public static function contentReplaceAttachmentFileIdProvider(): array {
166+
return [
167+
['[image.png](https://localhost:8000/f/1)', '[image.png](https://localhost:8000/f/2)'],
168+
['[image.png](https://localhost:8000/f/1(preview))', '[image.png](https://localhost:8000/f/2(preview))'],
169+
['[image.png](https://localhost:8000/f/1 (preview))', '[image.png](https://localhost:8000/f/2 (preview))'],
170+
// Link in title
171+
['[https://localhost:8000/f/1](https://localhost:8000/f/1)', '[https://localhost:8000/f/1](https://localhost:8000/f/2)'],
172+
['[https://localhost:8000/f/1](https://localhost:8000/f/1 (preview))', '[https://localhost:8000/f/1](https://localhost:8000/f/2 (preview))'],
173+
// Spaces surrounding link URL
174+
['[image.png]( https://localhost:8000/f/1 )', '[image.png]( https://localhost:8000/f/2 )'],
175+
['[image.png]( https://localhost:8000/f/1 (preview))', '[image.png]( https://localhost:8000/f/2 (preview))'],
176+
['[image.png]( https://localhost:8000/f/1 (preview) )', '[image.png]( https://localhost:8000/f/2 (preview) )'],
177+
// Escaped square brackets in title
178+
['[title \[#1\]](https://localhost:8000/f/1)', '[title \[#1\]](https://localhost:8000/f/2)'],
179+
['[title \[#1\]](https://localhost:8000/f/1 (preview))', '[title \[#1\]](https://localhost:8000/f/2 (preview))'],
180+
// Spaces in title
181+
['[title with space](https://localhost:8000/f/1)', '[title with space](https://localhost:8000/f/2)'],
182+
['[title with space](https://localhost:8000/f/1 (preview))', '[title with space](https://localhost:8000/f/2 (preview))'],
183+
// Several links in a row
184+
['Some text\n\n[image.png](https://localhost:8000/f/1 (preview))\n\nMore text. [file.tar.gz](https://localhost:8000/f/3) ...', 'Some text\n\n[image.png](https://localhost:8000/f/2 (preview))\n\nMore text. [file.tar.gz](https://localhost:8000/f/4) ...'],
185+
];
186+
}
187+
188+
/**
189+
* @dataProvider contentReplaceAttachmentFileIdProvider
190+
*/
191+
public function testReplaceAttachmentFileId(string $sourceContent, string $targetContent): void {
192+
$target = $this->createMock(File::class);
193+
$target->method('getId')->willReturn(2);
194+
$target->method('getContent')->willReturn($sourceContent);
195+
$replacedContent = '';
196+
$target->method('putContent')->willReturnCallback(function (string $content) use (&$replacedContent) {
197+
$replacedContent = $content;
198+
});
199+
200+
$fileIdMapping = [
201+
[1, 2],
202+
[3, 4],
203+
];
204+
AttachmentService::replaceAttachmentFileIds($target, $fileIdMapping);
205+
$this->assertEquals($targetContent, $replacedContent);
206+
}
207+
208+
// No replacement expected
209+
public static function contentReplaceAttachmentFileIdNoReplaceProvider(): array {
210+
return [
211+
// Empty title
212+
[ '[](https://localhost:8000/f/1)' ],
213+
// Different url
214+
[ '[title](https://localhost:8000/f/1/a/asdf.png)' ],
215+
// Wrong fileId #1
216+
[ '[image.png](https://localhost:8000/f/112)' ],
217+
// Wrong fileId #2
218+
[ '[image.png](https://localhost:8000/f/12)' ],
219+
// Normal brackets around title
220+
[ '(image.png)(https://localhost:8000/f/1)' ],
221+
// Square brackets in title
222+
['[title [#1]](https://localhost:8000/f/1)' ],
223+
// Space between brackets
224+
['[title] (https://localhost:8000/f/1)' ],
225+
];
226+
}
227+
228+
/**
229+
* @dataProvider contentReplaceAttachmentFileIdNoReplaceProvider
230+
*/
231+
public function testReplaceAttachmentFileIdNoReplace(string $sourceContent): void {
232+
$target = $this->createMock(File::class);
233+
$target->method('getId')->willReturn(2);
234+
$target->method('getContent')->willReturn($sourceContent);
235+
$replacedContent = '';
236+
$target->method('putContent')->willReturnCallback(function (string $content) use (&$replacedContent) {
237+
$replacedContent = $content;
238+
});
239+
240+
AttachmentService::replaceAttachmentFileIds($target, [[1, 2]]);
241+
$this->assertEquals($sourceContent, $replacedContent);
242+
}
163243
}

0 commit comments

Comments
 (0)