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

copyCollection creates orphaned documents when target collection name is the same as source #99

Open
caseydawsonjordan opened this issue Dec 12, 2013 · 6 comments
Assignees
Labels
enhancement new features, suggestions, etc. needs documentation Signals issues or PRs that will require an update to the documentation repo
Milestone

Comments

@caseydawsonjordan
Copy link
Contributor

To reproduce:

1.) Create a collection with documents in it.
2.) Copy that collection to a seperate location
3.) Copy it back over the original
4.) Take a backup and see all the original files are now in the lost_and_found

IHMO the correct solution to this is to throw an exception when the target collection exists to prevent data loss. The same should be applied to moveCollection, which will simply delete the target if it exists. Without an exception being thrown this creates a situation for data loss to occur very easily without warning.

@dizzzz
Copy link
Member

dizzzz commented Dec 12, 2013

It is all about WebDAV? That is: by calling the WebDAV methods? Or is it in General?

@shabanovd
Copy link
Member

This behavior of eXist's core. I think that exception is normal, or better to say, expecting reaction.

@adamretter @wolfgangmm , what do you think?

@dizzzz
Copy link
Member

dizzzz commented Dec 16, 2013

I have been thinking a bit more on this: For me, this kind of protection should be on application level, not on the 'core'. I do not want this protection out of the box, my move command....... should just be moving data.

@caseydawsonjordan
Copy link
Contributor Author

Dannes I have a very hard time understanding why that would be preferable.

1.) This isn't how any other system operates, especially unix. eXist is
suppose to mimic unix as closely as possible...no?
2.) By leaving this logic at the application level this creates a very real
possibility that developers unintentionally create a way for end users to
very easily lose data. (This is already a fact with several eXist
extensions).
3.) Why not have this protection built in, and then have the application
layer override it if they want?...This would give the same exact amount of
control, but would eliminate the possibility of unintentional data loss....

On Mon, Dec 16, 2013 at 2:45 PM, Dannes Wessels notifications@github.comwrote:

I have been thinking a bit more on this: For me, this kind of protection
should be on application level, not on the 'core'. I do not want this
protection out of the box, my move command....... should just be moving
data.


Reply to this email directly or view it on GitHubhttps://github.com//issues/99#issuecomment-30693203
.

Casey Jordan
easyDITA a product of Jorsek LLC
"CaseyDJordan" on LinkedIn, Twitter & Facebook
(585) 348 7399
easydita.com

This message is intended only for the use of the Addressee(s) and may
contain information that is privileged, confidential, and/or exempt from
disclosure under applicable law. If you are not the intended recipient,
please be advised that any disclosure copying, distribution, or use of
the information contained herein is prohibited. If you have received
this communication in error, please destroy all copies of the message,
whether in electronic or hard copy format, as well as attachments, and
immediately contact the sender by replying to this e-mail or by phone.
Thank you.

@adamretter
Copy link
Contributor

Casey I agree with you. This has to be recognised as a bug in the core, of course we should never be intentionally orphaning files!

The only question is whether to either:

a) Throw an Exception at (3) if the collection already exists
b) Just overwrite the collection at (3) if the collection already exists
c) Something else...

My preference here would be to avoid exceptions and do (c) i.e. just one-way merge the source to the destination during the copy, this is exactly what happens on a UNIX system. e.g. given the filesystem:

/1/A/hello.txt
/2/A/goodbye.txt

and given the command: "cp -r /1/A /2"

Then, /2/A ends up with both hello.txt and goodbye.txt files, any files with the same name that existed in both /1/A and /2/A would have been overwritten in /2/A by the copy.

I meant to add that, if you need protection it is up to you to check whether the collection/resource exists before doing the copy/move, just like in UNIX.

@caseydawsonjordan
Copy link
Contributor Author

Adam,

I see the appeal to this approach. It's certainly better than the
alternative. Still makes me nervous though ;) I'd prefer to have a flag
which needed to be set (or two sets of methods) so that it was very clear
to the developer what the function was doing at a quick glance. It would be
verbose, and a bit ugly but super safe.

It's somewhat like the age old practice of creating a new alias for 'mv'
and 'cp' that does some extra checking to prevent accidentally issuing a
command you would regret when working too late some night...

On Mon, Dec 16, 2013 at 3:11 PM, Adam Retter notifications@github.comwrote:

Casey I agree with you. This has to be recognised as a bug in the core, of
course we should never be intentionally orphaning files!

The only question is whether to either:

a) Throw an Exception at (3) if the collection already exists
b) Just overwrite the collection at (3) if the collection already exists

My preference here would be to avoid exceptions and just one-way merge the
source to the destination during the copy, this is exactly what happens on
a UNIX system. e.g. given the filesystem:

/1/A/hello.txt
/2/A/goodbye.txt

and given the command: "cp -r /1/A /2"

Then, /2/A ends up with both hello.txt and goodbye.txt files, any files
with the same name that existed in both /1/A and /2/A would have been
overwritten in /2/A by the copy.


Reply to this email directly or view it on GitHubhttps://github.com//issues/99#issuecomment-30695738
.

Casey Jordan
easyDITA a product of Jorsek LLC
"CaseyDJordan" on LinkedIn, Twitter & Facebook
(585) 348 7399
easydita.com

This message is intended only for the use of the Addressee(s) and may
contain information that is privileged, confidential, and/or exempt from
disclosure under applicable law. If you are not the intended recipient,
please be advised that any disclosure copying, distribution, or use of
the information contained herein is prohibited. If you have received
this communication in error, please destroy all copies of the message,
whether in electronic or hard copy format, as well as attachments, and
immediately contact the sender by replying to this e-mail or by phone.
Thank you.

@dizzzz dizzzz added this to the eXist-2.3 milestone Aug 6, 2014
@adamretter adamretter modified the milestones: eXist-3.0, eXist-2.3 Apr 1, 2015
@shabanovd shabanovd self-assigned this Apr 8, 2015
@zwobit zwobit modified the milestones: eXist-3.0, eXist 3.1 May 30, 2015
@shabanovd shabanovd modified the milestones: eXist-3.2.0, eXist-3.1.0 Mar 8, 2017
@adamretter adamretter modified the milestones: eXist-3.3.0, eXist-3.2.0 May 3, 2017
@adamretter adamretter modified the milestones: eXist-3.4.0, eXist-3.3.0 Jul 3, 2017
@adamretter adamretter modified the milestones: eXist-3.4.0, eXist-3.5.0 Jul 27, 2017
@adamretter adamretter modified the milestones: eXist-3.5.0, eXist-3.6.0 Sep 28, 2017
@adamretter adamretter modified the milestones: eXist-3.6.0, eXist-3.7.0 Nov 27, 2017
@adamretter adamretter modified the milestones: eXist-3.7.0, eXist-4.0.1 Feb 14, 2018
@adamretter adamretter modified the milestones: eXist-4.0.1, 4.1.1 Apr 18, 2018
@adamretter adamretter modified the milestones: eXist-4.1.1, eXist-4.2.1 Jun 6, 2018
@adamretter adamretter modified the milestones: eXist-4.2.1, eXist-4.2.2 Jun 14, 2018
@adamretter adamretter modified the milestones: eXist-4.2.2, eXist-4.3.1 Jul 7, 2018
@adamretter adamretter modified the milestones: eXist-4.3.1, eXist-4.3.2 Jul 24, 2018
@adamretter adamretter modified the milestones: eXist-4.3.2, eXist-4.4.1 Sep 21, 2018
@duncdrum duncdrum modified the milestones: eXist-4.4.1, eXist-4.5.1 Nov 30, 2018
@duncdrum duncdrum added the needs documentation Signals issues or PRs that will require an update to the documentation repo label Jan 8, 2019
@joewiz joewiz modified the milestones: eXist-4.5.1, eXist-4.6.2 Apr 6, 2019
@joewiz joewiz modified the milestones: eXist-4.6.2, eXist 4.7.1 May 24, 2019
@adamretter adamretter modified the milestones: eXist-4.7.1, eXist-4.7.2 Aug 14, 2019
@adamretter adamretter modified the milestones: eXist 4.8.1, eXist-4.10.1 Feb 14, 2022
@adamretter adamretter modified the milestones: eXist-4.10.1, eXist-4.11.1 Jan 11, 2023
@adamretter adamretter modified the milestones: eXist-4.11.1, eXist-4.11.3 Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new features, suggestions, etc. needs documentation Signals issues or PRs that will require an update to the documentation repo
Projects
None yet
Development

No branches or pull requests

7 participants