Skip to content

Commit

Permalink
Merge pull request #31087 from owncloud/android-file-create-event-sta…
Browse files Browse the repository at this point in the history 
…ble10

[stable10] During chunking process the after file creation not triggered
  • Loading branch information
Vincent Petry authored Apr 17, 2018
2 parents 1efd6a3 + b9b0aef commit 67a710a
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 5 deletions.
40 changes: 35 additions & 5 deletions apps/dav/lib/Connector/Sabre/File.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,6 @@ public function __construct($view, $info, $shareManager = null, Request $request
* @return string|null
*/
public function put($data) {
$path = $this->fileView->getAbsolutePath($this->path);
$beforeEvent = new GenericEvent(null, ['path' => $path]);
\OC::$server->getEventDispatcher()->dispatch('file.beforecreate', $beforeEvent);
try {
$exists = $this->fileView->file_exists($this->path);
if ($this->info && $exists && !$this->info->isUpdateable()) {
Expand All @@ -135,6 +132,16 @@ public function put($data) {
}
}

$newFile = false;
$path = $this->fileView->getAbsolutePath($this->path);
$beforeEvent = new GenericEvent(null, ['path' => $path]);
if (!$this->fileView->file_exists($this->path)) {
\OC::$server->getEventDispatcher()->dispatch('file.beforecreate', $beforeEvent);
$newFile = true;
} else {
\OC::$server->getEventDispatcher()->dispatch('file.beforeupdate', $beforeEvent);
}

list($partStorage) = $this->fileView->resolvePath($this->path);
$needsPartFile = $this->needsPartFile($partStorage) && (strlen($this->path) > 1);

Expand Down Expand Up @@ -265,7 +272,11 @@ public function put($data) {
}

$afterEvent = new GenericEvent(null, ['path' => $path]);
\OC::$server->getEventDispatcher()->dispatch('file.aftercreate', $afterEvent);
if ($newFile === true) {
\OC::$server->getEventDispatcher()->dispatch('file.aftercreate', $afterEvent);
} else {
\OC::$server->getEventDispatcher()->dispatch('file.afterupdate', $afterEvent);
}
return '"' . $this->info->getEtag() . '"';
}

Expand Down Expand Up @@ -471,6 +482,16 @@ private function createFileChunked($data) {
$partFile = null;

$targetPath = $path . '/' . $info['name'];
$absPath = $this->fileView->getAbsolutePath($targetPath);
$beforeEvent = new GenericEvent(null, ['path' => $absPath]);
$newFile = false;
if (!$this->fileView->file_exists($targetPath)) {
\OC::$server->getEventDispatcher()->dispatch('file.beforecreate', $beforeEvent);
$newFile = true;
} else {
\OC::$server->getEventDispatcher()->dispatch('file.beforeupdate', $beforeEvent);
}

/** @var \OC\Files\Storage\Storage $targetStorage */
list($targetStorage, $targetInternalPath) = $this->fileView->resolvePath($targetPath);

Expand Down Expand Up @@ -553,7 +574,16 @@ private function createFileChunked($data) {

$this->fileView->unlockFile($targetPath, ILockingProvider::LOCK_SHARED);

return $info->getEtag();
$etag = $info->getEtag();
if ($etag !== null) {
$afterEvent = new GenericEvent(null, ['path' => $absPath]);
if ($newFile === true) {
\OC::$server->getEventDispatcher()->dispatch('file.aftercreate', $afterEvent);
} else {
\OC::$server->getEventDispatcher()->dispatch('file.afterupdate', $afterEvent);
}
}
return $etag;
} catch (\Exception $e) {
if ($partFile !== null) {
$targetStorage->unlink($targetInternalPath);
Expand Down
102 changes: 102 additions & 0 deletions apps/dav/tests/unit/Connector/Sabre/FileTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,9 +526,43 @@ public function testChunkedPutLegalMtime($requestMtime, $resultMtime) {

$_SERVER['HTTP_OC_CHUNKED'] = true;
$file = 'foo.txt';

$calledBeforeCreateFile = [];
\OC::$server->getEventDispatcher()->addListener('file.beforecreate',
function (GenericEvent $event) use (&$calledBeforeCreateFile) {
$calledBeforeCreateFile[] = 'file.beforecreate';
$calledBeforeCreateFile[] = $event;
});
$calledAfterCreateFile = [];
\OC::$server->getEventDispatcher()->addListener('file.aftercreate',
function (GenericEvent $event) use (&$calledAfterCreateFile) {
$calledAfterCreateFile[] = 'file.aftercreate';
$calledAfterCreateFile[] = $event;
});
$calledAfterUpdateFile = [];
\OC::$server->getEventDispatcher()->addListener('file.afterupdate',
function (GenericEvent $event) use (&$calledAfterUpdateFile) {
$calledAfterUpdateFile[] = 'file.afterupdate';
$calledAfterUpdateFile[] = $event;
});
$this->doPut($file.'-chunking-12345-2-0', null, $request);
$this->doPut($file.'-chunking-12345-2-1', null, $request);
$this->assertEquals($resultMtime, $this->getFileInfos($file)['mtime']);
$this->assertInstanceOf(GenericEvent::class, $calledAfterCreateFile[1]);
$this->assertInstanceOf(GenericEvent::class, $calledBeforeCreateFile[1]);
$this->assertEquals('file.aftercreate', $calledAfterCreateFile[0]);
$this->assertEquals('file.beforecreate', $calledBeforeCreateFile[0]);
$this->assertEquals('file.afterupdate', $calledAfterUpdateFile[0]);
$this->assertArrayHasKey('path', $calledAfterCreateFile[1]);
$this->assertArrayHasKey('path', $calledBeforeCreateFile[1]);
//Internally it even tries to call mkdir to create cache dir So lets test what ever
// is there in the arrays. We will test all the indices.
$this->assertEquals('/'.$this->user.'/cache', $calledBeforeCreateFile[1]->getArgument('path'));
$this->assertEquals('/'.$this->user.'/files/'.$file, $calledBeforeCreateFile[3]->getArgument('path'));
$this->assertEquals('/'.$this->user.'/cache', $calledAfterCreateFile[1]->getArgument('path'));
//The indices 1 and 3 have part files.
$this->assertNotFalse(strpos($calledAfterUpdateFile[1]->getArgument('path'), '/'.$this->user.'/cache/'. $file.'-chunking-12345-0'));
$this->assertNotFalse(strpos($calledAfterUpdateFile[3]->getArgument('path'), '/'.$this->user.'/cache/'. $file.'-chunking-12345-1'));
}

/**
Expand Down Expand Up @@ -1081,6 +1115,74 @@ protected function assertHookCall($callData, $signal, $hookPath) {
);
}

/**
* Testing update of file when put method is called repeatedly on same file.
* This test also verifies the hooks being called.
*/
public function testUpdateFileWithPut() {
$view = new View('/' . $this->user . '/files/');

$path = 'test-update.txt';
$info = new FileInfo(
'/' . $this->user . '/files/' . $path,
$this->getMockStorage(),
null,
['permissions' => Constants::PERMISSION_ALL],
null
);

$file = new File($view, $info);

$calledBeforeCreate = [];
\OC::$server->getEventDispatcher()->addListener('file.beforecreate',
function (GenericEvent $event) use (&$calledBeforeCreate) {
$calledBeforeCreate[] = 'file.beforecreate';
$calledBeforeCreate[] = $event;
});
$calledAfterCreate = [];
\OC::$server->getEventDispatcher()->addListener('file.aftercreate',
function (GenericEvent $event) use (&$calledAfterCreate) {
$calledAfterCreate[] = 'file.aftercreate';
$calledAfterCreate[] = $event;
});
$view->lockFile($path, ILockingProvider::LOCK_SHARED);
$file->put($this->getStream('hello'));
$view->unlockFile($path, ILockingProvider::LOCK_SHARED);

$this->assertInstanceOf(GenericEvent::class, $calledBeforeCreate[1]);
$this->assertInstanceOf(GenericEvent::class, $calledAfterCreate[1]);
$this->assertEquals('file.beforecreate', $calledBeforeCreate[0]);
$this->assertEquals('file.aftercreate', $calledAfterCreate[0]);
$this->assertArrayHasKey('path', $calledBeforeCreate[1]);
$this->assertEquals('/'.$this->user.'/files//test-update.txt', $calledBeforeCreate[1]->getArgument('path'));
$this->assertArrayHasKey('path', $calledAfterCreate[1]);
$this->assertEquals('/'.$this->user.'/files//test-update.txt', $calledAfterCreate[1]->getArgument('path'));

$calledBeforeUpdate = [];
\OC::$server->getEventDispatcher()->addListener('file.beforeupdate',
function (GenericEvent $event) use (&$calledBeforeUpdate) {
$calledBeforeUpdate[] = 'file.beforeupdate';
$calledBeforeUpdate[] = $event;
});
$calledAfterUpdte = [];
\OC::$server->getEventDispatcher()->addListener('file.afterupdate',
function (GenericEvent $event) use (&$calledAfterUpdte) {
$calledAfterUpdte[] = 'file.afterupdate';
$calledAfterUpdte[] = $event;
});
$view->lockFile($path, ILockingProvider::LOCK_SHARED);
$file->put($this->getStream('world'));
$view->unlockFile($path, ILockingProvider::LOCK_SHARED);
$this->assertInstanceOf(GenericEvent::class, $calledBeforeUpdate[1]);
$this->assertInstanceOf(GenericEvent::class, $calledAfterUpdte[1]);
$this->assertEquals('file.beforeupdate', $calledBeforeUpdate[0]);
$this->assertEquals('file.afterupdate', $calledAfterUpdte[0]);
$this->assertArrayHasKey('path', $calledBeforeUpdate[1]);
$this->assertEquals('/'.$this->user.'/files//'.$path, $calledBeforeUpdate[1]->getArgument('path'));
$this->assertArrayHasKey('path', $calledAfterUpdte[1]);
$this->assertEquals('/'.$this->user.'/files//'.$path, $calledAfterUpdte[1]->getArgument('path'));
}

/**
* Test whether locks are set before and after the operation
*/
Expand Down

0 comments on commit 67a710a

Please sign in to comment.