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.

lib/private/Files/Storage/Common.php

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

lib/private/Files/Storage/Local.php

-		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 👍

lib/private/Files/Storage/Common.php

rullzer

Looks good!

lib/private/Files/Storage/Common.php

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

lib/private/Files/Storage/Common.php Outdated Show resolved Hide resolved

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

lib/private/Files/Storage/Common.php Show resolved Hide resolved

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

rullzer approved these changes Jul 4, 2020

View reviewed changes

MorrisJobke reviewed Jul 6, 2020

View reviewed changes

lib/private/Files/Storage/Common.php Show resolved Hide resolved

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 ad7798f Compare 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-bot bot Jul 23, 2020

MorrisJobke merged commit 173f8ab into master Jul 23, 2020

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

backportbot-nextcloud bot 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-nextcloud bot 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!

Conversation

icewind1991 commented Jun 29, 2020

Uh oh!

kesselb commented Jun 29, 2020

Uh oh!

SimplyCorbett commented Jun 29, 2020

Uh oh!

Uh oh!

kesselb Jul 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