Problem/Motivation

$destination will always be FALSE at the time FileExistsException is thrown in FileSystem::prepareDestination().

https://git.drupalcode.org/project/drupal/-/blob/902684ebd0252ccabf4dbe9...

Steps to reproduce

$file_system = \Drupal::service('file_system');

file_put_contents('public://test.txt', 'test');
mkdir('public://test2.txt');

$file_system->copy('public://test.txt', 'public://test2.txt', 2);

Proposed resolution

Retain the original destination for output in exception.
Adjust tests to ensure path is included in validation.

Alternative:
Remove path from message

Remaining tasks

User interface changes

Exception will now include a PATH.

Introduced terminology

None

API changes

None Expected

Data model changes

None Expected

Release notes snippet

CommentFileSizeAuthor
#2 3615576.patch1.87 KBshank115

Issue fork drupal-3615576

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

cmlara created an issue. See original summary.

shank115’s picture

shank115 commented
StatusFileSize
new 3615576.patch1.87 KB

Check if the attached patch fixes the issue.

kieran.cott changed the visibility of the branch 3615576-fileexistsexception-thrown-in to hidden.

kieran.cott opened merge request !16737

kieran.cott’s picture

kieran.cott
Location Manila 🇵🇭
commented
Status: Active » Needs review

I have not been able to reproduce this issue using the steps in the issue summary since copying into an empty directory succeeds. However, the issue can be reproduced with:

 $file_system = \Drupal::service('file_system');
 $uri = 'public://test.txt';
 file_put_contents($uri, 'test');
 $file_system->copy($uri, $uri, \Drupal\Core\File\FileExists::Error);

@shank115's patch fixes the issue for me by by retaining $destination before getDestinationFilename() replaces it with FALSE, and the associated test checks the corrected path.

I've implemented the patch as an MR for review.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Thanks @kieran.cott MR looks fine but wanted to note this was tagged novice and looking at your post history believe you can avoid this now! Congrats! No problem here just noting for future tickets.

kieran.cott’s picture

kieran.cott
Location Manila 🇵🇭
commented

Thanks @smustgrave, will avoid Novice issues going forward. :)

smustgrave’s picture

smustgrave commented

Let me amend my previous statement if a novice issue has sat for 2-3 weeks then it's fair game.

  • godotislate committed 6b67f192 on 11.x 
    fix: #3615576 FileExistsException thrown in FileSystem::...

  • godotislate committed cfcf382a on main 
    fix: #3615576 FileExistsException thrown in FileSystem::...
godotislate’s picture

godotislate
he/him
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed cfcf382 to main and 6b67f19 to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.