`moveFile` operation type has missing optional `options` parameter
Hello, first of all thanks for this great webdav client, it is really solid!
I have a minor issue though; it seems the types for some of the operations (i.e. moveFile [1]) are not supporting optional options parameter even though the implementation supports [2] it and documentation [3] says they do.
I am creating this issue very specific to moveFile because passing a header for Overwrite flag is part of the standard [4], and by default it overwrites (using Nextcloud), which is not safe in my use-case (My current and probably only workaround is ignoring ts check at the moment).
So, as the possible solutions:
1 - types.ts can be updated to reflect the implementation.
2 - Overwrite parameter can be supported natively by moveFile operation as an optional parameter.
I'd be more than happy to implement both or one of them if you could tell me your preference (and the target version v4 or v5).
Thanks!
[1] - https://github.com/perry-mitchell/webdav-client/blob/f4403c449b5cf648e678a64306ba35d27f49053b/source/types.ts#L233 [2] - https://github.com/perry-mitchell/webdav-client/blob/f4403c449b5cf648e678a64306ba35d27f49053b/source/factory.ts#L96 [3] - https://github.com/perry-mitchell/webdav-client/blob/master/README.md#movefile [4] - http://www.webdav.org/specs/rfc4918.html#rfc.section.9.9.3
Thanks for the detailed issue! I'd actually not realised that such an option wasn't available on moveFile. I thought it had already been implemented.
I'd accept a PR for this, though I'd probably get to this at some stage. What would the default be though? Need to test current functionality without it specified and it's safer that the default mirrors current functionality i think.
Thanks for really quick response!
I understand that the goal of this package is not to follow any standard but the one I am checking rfc4918 clearly explains the default behavior for move and copy operations:
If the overwrite header is not included in a COPY or MOVE request, then the resource MUST treat the request as if it has an overwrite header of value "T".
That means, lack of overwrite behavior does overwrite the target resource if exists. I also tested against Nextcloud, which aligns with this. Though, I am not sure if there are any other standards which contradict with this, or any implementation that doesn't follow.
My suggestion would be:
- Have an optional boolean parameter
overwriteformoveFileandcopyFile(why not also fix for copying..) implementations. - a. If the
overwriteparameter is not provided, do not passOverwriteheader at all. - If the
overwriteparameter is provided astrue, then passTtoOverwriteheader. - If the
overwriteparameter is provided asfalse, then passFtoOverwriteheader. - Update
types.tsto support newoverwrite, and existingoptionsparameters formoveFileandcopyFileoperations.
I think this is the safest approach for backward compatibility without following any standard strictly.
Alternatively, step 2. b. would be, passing also T if the overwrite parameter is not provided but there is a risk of breaking some current functionality. Equally, might be less confusing in the perspective of API design..
Let me know what do you think about it.
Hi @ahaltindis - I'm sorry for the long delay. I completely agree with your suggestion.. this sounds like the best way to implement Overwrite.