Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid duplicate files with the same name #4361

Closed
wants to merge 1 commit into from
Closed

Avoid duplicate files with the same name #4361

wants to merge 1 commit into from

Conversation

gvmura
Copy link
Contributor

@gvmura gvmura commented Apr 16, 2017

When it renames the temporary file, it tests if the file is already present. If so, it moves to trash the previous version to avoid duplicate files with the same name (message log: "Ignoring duplicate file name: ... on Google Drive for Google user: ...")

@mention-bot
Copy link

@gvmura, thanks for your PR! By analyzing the history of the files in this pull request, we identified @MTGap, @AdamWill, @icewind1991 and @Xenopathic to be potential reviewers.

@gvmura
Copy link
Contributor Author

gvmura commented Apr 16, 2017

It doesn't handle duplicate files in Google Drive, it tries to avoid them.
You'll watch #4279 issue.

Copy link
Member

@icewind1991 icewind1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine as far as I can tell

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

@rullzer
Copy link
Member

rullzer commented Apr 20, 2017

@gvmura thanks a lot for your pull request!
We require all commits to be signed. Could you do this?

Just run:

git commit --amend -s
git push -f

@gvmura
Copy link
Contributor Author

gvmura commented Apr 20, 2017

UPDATE: I have just tried to sign and commit

@rullzer sorry,
I can't sign the fix. If you want, you could sign it with your name.

This morning I tested owncloud 9.1.5 and I saw that when the system create the secondary file it handle the Google version. No "temporary" file is created in Google.
So my fix isn't necessary. Watch owncloud/core#23517

When it renames the temporary file, it tests if the file is already present. If so, it moves to trash the previous version to avoid duplicate files with the same name (message log: "Ignoring duplicate file name: ... on Google Drive for Google user: ...")

Signed-off-by: gvmura <gvmura@uniss.it>
@codecov
Copy link

codecov bot commented Apr 20, 2017

Codecov Report

Merging #4361 into stable11 will increase coverage by <.01%.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             stable11    #4361      +/-   ##
==============================================
+ Coverage       57.39%   57.39%   +<.01%     
  Complexity      19439    19439              
==============================================
  Files            1214     1214              
  Lines           72565    72563       -2     
  Branches         1237     1237              
==============================================
  Hits            41650    41650              
+ Misses          30915    30913       -2
Impacted Files Coverage Δ Complexity Δ
lib/private/Security/CertificateManager.php 92.78% <0%> (-1.04%) 38% <0%> (ø)
apps/comments/lib/EventHandler.php 100% <0%> (ø) 7% <0%> (ø) ⬇️
lib/private/Server.php 92.74% <0%> (+0.16%) 119% <0%> (ø) ⬇️
lib/private/Files/Storage/DAV.php 7.32% <0%> (+0.23%) 159% <0%> (ø) ⬇️
lib/private/Files/Cache/Propagator.php 96.2% <0%> (+1.26%) 16% <0%> (ø) ⬇️

@rullzer
Copy link
Member

rullzer commented Apr 20, 2017

@gvmura so this is not needed? Because then we can close this...

@MorrisJobke
Copy link
Member

Before merging this keep in mind: this is against stable11

@gvmura
Copy link
Contributor Author

gvmura commented Apr 20, 2017

@rullzer,
it is not needed in owncloud 9.1.5, but nextcloud 11.0.2 uses temporary file and without the fix the problem of duplicate files is present.
I'm going to use nextcloud 12 next month. I have just tested few minutes ago nextcloud 12.0.0.14
[git show: Merge: 810fb7f ec07791]
and the problem is still present.

@MorrisJobke
Copy link
Member

@rullzer,
it is not needed in owncloud 9.1.5, but nextcloud 11.0.2 uses temporary file and without the fix the problem of duplicate files is present.
I'm going to use nextcloud 12 next month. I have just tested few minutes ago nextcloud 12.0.0.14
[git show: Merge: 810fb7f ec07791]
and the problem is still present.

Best would be then to have this PR against master and we will take care of backporting to stable11. Could open this PR against master instead?

@gvmura
Copy link
Contributor Author

gvmura commented Apr 20, 2017

@MorrisJobke Best would be then to have this PR against master and we will take care of backporting to stable11. Could open this PR against master instead?

Ok, tomorrow mornign I'll do it.

@gvmura gvmura closed this Apr 21, 2017
@gvmura gvmura deleted the patch-1 branch April 21, 2017 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants