upload new files in objectstore to a .part path first
This prevent the object store and cache from getting out of sync when an objectstore silently fails or the php process get's killed during the upload without giving us the chance to cleanup Signed-off-by: Robin Appelman <robin@icewind.nl>
This commit is contained in:
parent
6c9f2644cf
commit
d6bf5d4384
|
@ -436,7 +436,10 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common {
|
||||||
$stat['mimetype'] = $mimetype;
|
$stat['mimetype'] = $mimetype;
|
||||||
$stat['etag'] = $this->getETag($path);
|
$stat['etag'] = $this->getETag($path);
|
||||||
|
|
||||||
$fileId = $this->getCache()->put($path, $stat);
|
$exists = $this->getCache()->inCache($path);
|
||||||
|
$uploadPath = $exists ? $path : $path . '.part';
|
||||||
|
$fileId = $this->getCache()->put($uploadPath, $stat);
|
||||||
|
$urn = $this->getURN($fileId);
|
||||||
try {
|
try {
|
||||||
//upload to object storage
|
//upload to object storage
|
||||||
if ($size === null) {
|
if ($size === null) {
|
||||||
|
@ -446,22 +449,31 @@ class ObjectStoreStorage extends \OC\Files\Storage\Common {
|
||||||
]);
|
]);
|
||||||
$size = $writtenSize;
|
$size = $writtenSize;
|
||||||
});
|
});
|
||||||
$this->objectStore->writeObject($this->getURN($fileId), $countStream);
|
$this->objectStore->writeObject($urn, $countStream);
|
||||||
if (is_resource($countStream)) {
|
if (is_resource($countStream)) {
|
||||||
fclose($countStream);
|
fclose($countStream);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
$this->objectStore->writeObject($this->getURN($fileId), $stream);
|
$this->objectStore->writeObject($urn, $stream);
|
||||||
}
|
}
|
||||||
} catch (\Exception $ex) {
|
} catch (\Exception $ex) {
|
||||||
$this->getCache()->remove($path);
|
$this->getCache()->remove($uploadPath);
|
||||||
$this->logger->logException($ex, [
|
$this->logger->logException($ex, [
|
||||||
'app' => 'objectstore',
|
'app' => 'objectstore',
|
||||||
'message' => 'Could not create object ' . $this->getURN($fileId) . ' for ' . $path,
|
'message' => 'Could not create object ' . $urn . ' for ' . $path,
|
||||||
]);
|
]);
|
||||||
throw $ex; // make this bubble up
|
throw $ex; // make this bubble up
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!$exists) {
|
||||||
|
if ($this->objectStore->objectExists($urn)) {
|
||||||
|
$this->getCache()->move($uploadPath, $path);
|
||||||
|
} else {
|
||||||
|
$this->getCache()->remove($uploadPath);
|
||||||
|
throw new \Exception("Object not found after writing (urn: $urn, path: $path)", 404);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return $size;
|
return $size;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -0,0 +1,53 @@
|
||||||
|
<?php declare(strict_types=1);
|
||||||
|
/**
|
||||||
|
* @copyright Copyright (c) 2018 Robin Appelman <robin@icewind.nl>
|
||||||
|
*
|
||||||
|
* @license GNU AGPL version 3 or any later version
|
||||||
|
*
|
||||||
|
* This program is free software: you can redistribute it and/or modify
|
||||||
|
* it under the terms of the GNU Affero General Public License as
|
||||||
|
* published by the Free Software Foundation, either version 3 of the
|
||||||
|
* License, or (at your option) any later version.
|
||||||
|
*
|
||||||
|
* This program is distributed in the hope that it will be useful,
|
||||||
|
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||||
|
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||||
|
* GNU Affero General Public License for more details.
|
||||||
|
*
|
||||||
|
* You should have received a copy of the GNU Affero General Public License
|
||||||
|
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
*
|
||||||
|
*/
|
||||||
|
|
||||||
|
namespace Test\Files\ObjectStore;
|
||||||
|
|
||||||
|
use OCP\Files\ObjectStore\IObjectStore;
|
||||||
|
|
||||||
|
class FailWriteObjectStore implements IObjectStore {
|
||||||
|
private $objectStore;
|
||||||
|
|
||||||
|
public function __construct(IObjectStore $objectStore) {
|
||||||
|
$this->objectStore = $objectStore;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getStorageId() {
|
||||||
|
return $this->objectStore->getStorageId();
|
||||||
|
}
|
||||||
|
|
||||||
|
public function readObject($urn) {
|
||||||
|
return $this->objectStore->readObject($urn);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function writeObject($urn, $stream) {
|
||||||
|
// emulate a failed write that didn't throw an error
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function deleteObject($urn) {
|
||||||
|
$this->objectStore->deleteObject($urn);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function objectExists($urn) {
|
||||||
|
return $this->objectStore->objectExists($urn);
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,38 @@
|
||||||
|
<?php declare(strict_types=1);
|
||||||
|
/**
|
||||||
|
* @copyright Copyright (c) 2018 Robin Appelman <robin@icewind.nl>
|
||||||
|
*
|
||||||
|
* @license GNU AGPL version 3 or any later version
|
||||||
|
*
|
||||||
|
* This program is free software: you can redistribute it and/or modify
|
||||||
|
* it under the terms of the GNU Affero General Public License as
|
||||||
|
* published by the Free Software Foundation, either version 3 of the
|
||||||
|
* License, or (at your option) any later version.
|
||||||
|
*
|
||||||
|
* This program is distributed in the hope that it will be useful,
|
||||||
|
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||||
|
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||||
|
* GNU Affero General Public License for more details.
|
||||||
|
*
|
||||||
|
* You should have received a copy of the GNU Affero General Public License
|
||||||
|
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||||
|
*
|
||||||
|
*/
|
||||||
|
|
||||||
|
namespace Test\Files\ObjectStore;
|
||||||
|
|
||||||
|
use OC\Files\ObjectStore\ObjectStoreStorage;
|
||||||
|
use OCP\Files\ObjectStore\IObjectStore;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Allow overwriting the object store instance for test purposes
|
||||||
|
*/
|
||||||
|
class ObjectStoreStorageOverwrite extends ObjectStoreStorage {
|
||||||
|
public function setObjectStore(IObjectStore $objectStore) {
|
||||||
|
$this->objectStore = $objectStore;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getObjectStore(): IObjectStore {
|
||||||
|
return $this->objectStore;
|
||||||
|
}
|
||||||
|
}
|
|
@ -30,6 +30,8 @@ use Test\Files\Storage\Storage;
|
||||||
* @group DB
|
* @group DB
|
||||||
*/
|
*/
|
||||||
class ObjectStoreStorageTest extends Storage {
|
class ObjectStoreStorageTest extends Storage {
|
||||||
|
/** @var ObjectStoreStorageOverwrite */
|
||||||
|
protected $instance;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @var IObjectStore
|
* @var IObjectStore
|
||||||
|
@ -42,7 +44,7 @@ class ObjectStoreStorageTest extends Storage {
|
||||||
$baseStorage = new Temporary();
|
$baseStorage = new Temporary();
|
||||||
$this->objectStorage = new StorageObjectStore($baseStorage);
|
$this->objectStorage = new StorageObjectStore($baseStorage);
|
||||||
$config['objectstore'] = $this->objectStorage;
|
$config['objectstore'] = $this->objectStorage;
|
||||||
$this->instance = new ObjectStoreStorage($config);
|
$this->instance = new ObjectStoreStorageOverwrite($config);
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function tearDown() {
|
protected function tearDown() {
|
||||||
|
@ -166,4 +168,17 @@ class ObjectStoreStorageTest extends Storage {
|
||||||
$targetId = $this->instance->getCache()->getId('target');
|
$targetId = $this->instance->getCache()->getId('target');
|
||||||
$this->assertSame($sourceId, $targetId, 'fileid must be stable on move or shares will break');
|
$this->assertSame($sourceId, $targetId, 'fileid must be stable on move or shares will break');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function testWriteObjectSilentFailure() {
|
||||||
|
$objectStore = $this->instance->getObjectStore();
|
||||||
|
$this->instance->setObjectStore(new FailWriteObjectStore($objectStore));
|
||||||
|
|
||||||
|
try {
|
||||||
|
$this->instance->file_put_contents('test.txt', 'foo');
|
||||||
|
$this->fail('expected exception');
|
||||||
|
} catch (\Exception $e) {
|
||||||
|
$this->assertStringStartsWith('Object not found after writing', $e->getMessage());
|
||||||
|
}
|
||||||
|
$this->assertFalse($this->instance->file_exists('test.txt'));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue