fix moving files from external storage to object store trashbin by icewind1991 · Pull Request #21628 · nextcloud/server

icewind1991 · 2020-06-29T16:15:55Z

having the "cache rename" after the "storage move" caused the target
to get the fileid from the source file, without taking care that the object
is stored under the original file id.

By doing the "cache rename" first, we trigger the "update existing file"
logic while moving the file to the object store and the object gets stored for the
correct file id

Signed-off-by: Robin Appelman robin@icewind.nl

kesselb · 2020-06-29T20:10:34Z

cc @dragon988

SimplyCorbett · 2020-06-29T20:22:25Z

I'm not using S3 anymore but the code looks good to me.

kesselb · 2020-07-01T15:39:34Z

- return (int)file_put_contents($this->getSourcePath($path), $stream);
+ $result = file_put_contents($this->getSourcePath($path), $stream);
+ if ($result === false) {
+ throw new GenericFileException("Failed write steam to $path");


Thanks 👍

rullzer

Looks good!

MorrisJobke

Code looks good 👍 I refined one exception message to also include the path so that looking at the logs makes more sense later on.

MorrisJobke · 2020-07-06T21:17:14Z

PHPUnit fails

having the "cache rename" after the "storage move" caused the target to get the fileid from the source file, without taking care that the object is stored under the original file id. By doing the "cache rename" first, we trigger the "update existing file" logic while moving the file to the object store and the object gets stored for the correct file id Signed-off-by: Robin Appelman <robin@icewind.nl>

Signed-off-by: Robin Appelman <robin@icewind.nl>

this remove the ambiguity when writing zero length files Signed-off-by: Robin Appelman <robin@icewind.nl>

Signed-off-by: Robin Appelman <robin@icewind.nl>

icewind1991 · 2020-07-23T19:31:37Z

ci fixed

icewind1991 · 2020-07-23T20:58:08Z

/backport to stable19

icewind1991 · 2020-07-23T20:58:12Z

/backport to stable18

MorrisJobke · 2020-07-30T12:31:52Z

So this breaks deleting files in an object store setup :/

MorrisJobke · 2020-07-30T12:42:17Z

{
"reqId": "8eOJATfs8cPyjKQ5j2DI",
"level": 4,
"time": "2020-07-30T12:30:26+00:00",
"remoteAddr": "127.0.0.1",
"user": "admin",
"app": "webdav",
"method": "DELETE",
"url": "/remote.php/dav/files/admin/Bildschirmfoto%202020-07-29%20um%2017.50.40.png",
"message": {
"Exception": "TypeError",
"Message": "Argument 1 passed to OC\\Files\\Cache\\CacheQueryBuilder::whereFileId() must be of the type int, null given, called in /Users/morris/Projects/nextcloud/server/lib/private/Files/Cache/Cache.php on line 661",
"Code": 0,
"Trace": [
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/Cache/Cache.php",
"line": 661,
"function": "whereFileId",
"class": "OC\\Files\\Cache\\CacheQueryBuilder",
"type": "->",
"args": [
null
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/Cache/Cache.php",
"line": 585,
"function": "moveFromCache",
"class": "OC\\Files\\Cache\\Cache",
"type": "->",
"args": [
{
"__class__": "OC\\Files\\Cache\\Cache"
},
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png",
"files_trashbin/files/Bildschirmfoto 2020-07-29 um 17.50.40.png.d1596112226"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/ObjectStore/ObjectStoreStorage.php",
"line": 347,
"function": "move",
"class": "OC\\Files\\Cache\\Cache",
"type": "->",
"args": [
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png",
"files_trashbin/files/Bildschirmfoto 2020-07-29 um 17.50.40.png.d1596112226"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/Storage/Wrapper/Wrapper.php",
"line": 278,
"function": "rename",
"class": "OC\\Files\\ObjectStore\\ObjectStoreStorage",
"type": "->",
"args": [
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png",
"files_trashbin/files/Bildschirmfoto 2020-07-29 um 17.50.40.png.d1596112226"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/Storage/Wrapper/Availability.php",
"line": 295,
"function": "rename",
"class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
"type": "->",
"args": [
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png",
"files_trashbin/files/Bildschirmfoto 2020-07-29 um 17.50.40.png.d1596112226"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/Storage/Wrapper/Wrapper.php",
"line": 278,
"function": "rename",
"class": "OC\\Files\\Storage\\Wrapper\\Availability",
"type": "->",
"args": [
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png",
"files_trashbin/files/Bildschirmfoto 2020-07-29 um 17.50.40.png.d1596112226"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/Storage/Wrapper/Wrapper.php",
"line": 572,
"function": "rename",
"class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
"type": "->",
"args": [
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png",
"files_trashbin/files/Bildschirmfoto 2020-07-29 um 17.50.40.png.d1596112226"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/apps/files_trashbin/lib/Trashbin.php",
"line": 288,
"function": "moveFromStorage",
"class": "OC\\Files\\Storage\\Wrapper\\Wrapper",
"type": "->",
"args": [
{
"cache": {
"__class__": "OC\\Files\\Cache\\Cache"
},
"scanner": null,
"watcher": null,
"propagator": {
"__class__": "OC\\Files\\Cache\\Propagator"
},
"updater": {
"__class__": "OC\\Files\\Cache\\Updater"
},
"__class__": "OCA\\Files_Trashbin\\Storage"
},
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png",
"files_trashbin/files/Bildschirmfoto 2020-07-29 um 17.50.40.png.d1596112226"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/apps/files_trashbin/lib/Trash/LegacyTrashBackend.php",
"line": 108,
"function": "move2trash",
"class": "OCA\\Files_Trashbin\\Trashbin",
"type": "::",
"args": [
"Bildschirmfoto 2020-07-29 um 17.50.40.png"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/apps/files_trashbin/lib/Trash/TrashManager.php",
"line": 103,
"function": "moveToTrash",
"class": "OCA\\Files_Trashbin\\Trash\\LegacyTrashBackend",
"type": "->",
"args": [
{
"cache": {
"__class__": "OC\\Files\\Cache\\Cache"
},
"scanner": null,
"watcher": null,
"propagator": {
"__class__": "OC\\Files\\Cache\\Propagator"
},
"updater": {
"__class__": "OC\\Files\\Cache\\Updater"
},
"__class__": "OCA\\Files_Trashbin\\Storage"
},
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/apps/files_trashbin/lib/Storage.php",
"line": 192,
"function": "moveToTrash",
"class": "OCA\\Files_Trashbin\\Trash\\TrashManager",
"type": "->",
"args": [
{
"cache": {
"__class__": "OC\\Files\\Cache\\Cache"
},
"scanner": null,
"watcher": null,
"propagator": {
"__class__": "OC\\Files\\Cache\\Propagator"
},
"updater": {
"__class__": "OC\\Files\\Cache\\Updater"
},
"__class__": "OCA\\Files_Trashbin\\Storage"
},
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/apps/files_trashbin/lib/Storage.php",
"line": 99,
"function": "doDelete",
"class": "OCA\\Files_Trashbin\\Storage",
"type": "->",
"args": [
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png",
"unlink"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/View.php",
"line": 1161,
"function": "unlink",
"class": "OCA\\Files_Trashbin\\Storage",
"type": "->",
"args": [
"files/Bildschirmfoto 2020-07-29 um 17.50.40.png"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/lib/private/Files/View.php",
"line": 718,
"function": "basicOperation",
"class": "OC\\Files\\View",
"type": "->",
"args": [
"unlink",
"/Bildschirmfoto 2020-07-29 um 17.50.40.png",
[
"delete"
]
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/apps/dav/lib/Connector/Sabre/File.php",
"line": 455,
"function": "unlink",
"class": "OC\\Files\\View",
"type": "->",
"args": [
"/Bildschirmfoto 2020-07-29 um 17.50.40.png"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/3rdparty/sabre/dav/lib/DAV/Tree.php",
"line": 183,
"function": "delete",
"class": "OCA\\DAV\\Connector\\Sabre\\File",
"type": "->",
"args": []
},
{
"file": "/Users/morris/Projects/nextcloud/server/3rdparty/sabre/dav/lib/DAV/CorePlugin.php",
"line": 295,
"function": "delete",
"class": "Sabre\\DAV\\Tree",
"type": "->",
"args": [
"files/admin/Bildschirmfoto 2020-07-29 um 17.50.40.png"
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/3rdparty/sabre/event/lib/WildcardEmitterTrait.php",
"line": 89,
"function": "httpDelete",
"class": "Sabre\\DAV\\CorePlugin",
"type": "->",
"args": [
{
"__class__": "Sabre\\HTTP\\Request"
},
{
"__class__": "Sabre\\HTTP\\Response"
}
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 474,
"function": "emit",
"class": "Sabre\\DAV\\Server",
"type": "->",
"args": [
"method:DELETE",
[
{
"__class__": "Sabre\\HTTP\\Request"
},
{
"__class__": "Sabre\\HTTP\\Response"
}
]
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 251,
"function": "invokeMethod",
"class": "Sabre\\DAV\\Server",
"type": "->",
"args": [
{
"__class__": "Sabre\\HTTP\\Request"
},
{
"__class__": "Sabre\\HTTP\\Response"
}
]
},
{
"file": "/Users/morris/Projects/nextcloud/server/3rdparty/sabre/dav/lib/DAV/Server.php",
"line": 319,
"function": "start",
"class": "Sabre\\DAV\\Server",
"type": "->",
"args": []
},
{
"file": "/Users/morris/Projects/nextcloud/server/apps/dav/lib/Server.php",
"line": 325,
"function": "exec",
"class": "Sabre\\DAV\\Server",
"type": "->",
"args": []
},
{
"file": "/Users/morris/Projects/nextcloud/server/apps/dav/appinfo/v2/remote.php",
"line": 35,
"function": "exec",
"class": "OCA\\DAV\\Server",
"type": "->",
"args": []
},
{
"file": "/Users/morris/Projects/nextcloud/server/remote.php",
"line": 167,
"args": [
"/Users/morris/Projects/nextcloud/server/apps/dav/appinfo/v2/remote.php"
],
"function": "require_once"
}
],
"File": "/Users/morris/Projects/nextcloud/server/lib/private/Files/Cache/CacheQueryBuilder.php",
"Line": 66,
"CustomMessage": "--"
},
"userAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_5) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.1.1 Safari/605.1.15",
"version": "20.0.0.0"
}

The get() method of the cache returns false:

server/lib/private/Files/Cache/Cache.php

Line 614 in cb05782

$sourceData = $sourceCache->get($sourcePath);

solracsf · 2020-08-01T12:33:53Z

Fix #19171 and #18132

icewind1991 added the 2. developing Work in progress label Jun 29, 2020

rullzer reviewed Jun 30, 2020
View reviewed changes

Comment threadlib/private/Files/Storage/Common.php Outdated

icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 1, 2020

kesselb reviewed Jul 1, 2020
View reviewed changes

Comment threadlib/private/Files/Storage/Common.php

icewind1991 force-pushed the external-to-s3-trashbin-fix branch 2 times, most recently from 9fe6afe to 63efe79Compare July 3, 2020 12:41

rullzer approved these changes Jul 4, 2020
View reviewed changes

MorrisJobke reviewed Jul 6, 2020
View reviewed changes

Comment threadlib/private/Files/Storage/Common.php

MorrisJobke approved these changes Jul 6, 2020
View reviewed changes

MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish feature: external storage feature: object storage feature: trashbin and removed 3. to review Waiting for reviews labels Jul 6, 2020

MorrisJobke added this to the Nextcloud 20 milestone Jul 6, 2020

MorrisJobke added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Jul 6, 2020

icewind1991 added 3 commits July 23, 2020 15:24

rollback cache rename if trashbin move fails
fcad692
Signed-off-by: Robin Appelman <robin@icewind.nl>

use exceptions for error signaling in writeStream
ad7798f
this remove the ambiguity when writing zero length files Signed-off-by: Robin Appelman <robin@icewind.nl>

icewind1991 force-pushed the external-to-s3-trashbin-fix branch from b5170c2 to ad7798fCompare July 23, 2020 13:25

fix renameFromStorage messing with folder mimetype
c8cf2e8
Signed-off-by: Robin Appelman <robin@icewind.nl>

nextcloud deleted a comment from faily-botBotJul 23, 2020

MorrisJobke merged commit 173f8ab into masterJul 23, 2020

MorrisJobke deleted the external-to-s3-trashbin-fix branch July 23, 2020 19:35

backportbot-nextcloudBot added the backport-request label Jul 23, 2020

This was referenced Jul 23, 2020
[stable18] fix moving files from external storage to object store trashbin #21982
Merged
[stable19] fix moving files from external storage to object store trashbin #21983
Merged

backportbot-nextcloudBot removed the backport-request label Jul 23, 2020

MorrisJobke mentioned this pull request Jul 30, 2020
Object store trashbin fixes #22060
Merged

This was referenced Aug 1, 2020
Delete from external storage will cause an error in log and no particular files in trash bin #18132
Closed
Files moved to trash bin from external storage are missing in object store setup #19171
Closed

juliusknorr mentioned this pull request Sep 3, 2020
Empty files (zero byte files) cannot be copied via Nextcloud web interface: // Sabre\DAV\Exception: Error while copying file to target location (copied bytes: 0, expected filesize: -1 ) #16989
Closed

Uh oh!

Uh oh!

Conversation

icewind1991 commented Jun 29, 2020

Uh oh!

kesselb commented Jun 29, 2020

Uh oh!

SimplyCorbett commented Jun 29, 2020

Uh oh!

Uh oh!

kesselbJul 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rullzer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MorrisJobke left a comment

Choose a reason for hiding this comment

Uh oh!

MorrisJobke commented Jul 6, 2020

Uh oh!

icewind1991 commented Jul 23, 2020

Uh oh!

icewind1991 commented Jul 23, 2020

Uh oh!

icewind1991 commented Jul 23, 2020

Uh oh!

MorrisJobke commented Jul 30, 2020

Uh oh!

MorrisJobke commented Jul 30, 2020

Uh oh!

solracsf commented Aug 1, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

solracsf commented Aug 1, 2020•
edited
Loading