From e656e789586083eca07f3e571d02422b583a5061 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Tue, 8 Sep 2015 00:20:10 +0300 Subject: [PATCH 1/8] Genesis::getHashByPath is no longer used --- lib/genesis.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/genesis.php b/lib/genesis.php index 52e82e8f..a74d3274 100644 --- a/lib/genesis.php +++ b/lib/genesis.php @@ -76,10 +76,6 @@ class Genesis { return $this->hash; } - public static function getHashByPath($path){ - return preg_replace('|([a-zA-Z0-9])*\..*$|', '\1', $path); - } - protected function getDocumentHash($view, $path){ $this->validate($view, $path); $hash = sha1($view->file_get_contents($path)); From 61d2a6cb6fad974b2f181d35d3d1d418e03b742a Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Thu, 17 Sep 2015 19:55:02 +0300 Subject: [PATCH 2/8] Fix test --- tests/bootstrap.php | 2 +- tests/controller/documentcontrollertest.php | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 86bfd7dd..fe1e965c 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -9,5 +9,5 @@ require_once __DIR__.'/../../../lib/base.php'; if(!class_exists('PHPUnit_Framework_TestCase')) { require_once('PHPUnit/Autoload.php'); } - +\OC_App::loadApp('documents'); OC_Hook::clear(); diff --git a/tests/controller/documentcontrollertest.php b/tests/controller/documentcontrollertest.php index cebdb566..cbc17ce3 100644 --- a/tests/controller/documentcontrollertest.php +++ b/tests/controller/documentcontrollertest.php @@ -51,10 +51,6 @@ class DocumentControllerTest extends \PHPUnit_Framework_TestCase { \OC_Util::setupFS(); } - public static function tearDownAfterClass(){ - \OC_User::deleteUser(\OC_User::getUserSession()->getUser()->getUID()); - } - public function testRename(){ $result = array( 'status' => 'error', From a6be42cb266d198a5def8b53bff796b93787aff4 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Thu, 17 Sep 2015 21:33:24 +0300 Subject: [PATCH 3/8] Cleanup file and storage models --- lib/file.php | 77 ++++++++++++++++++++++++------------------------- lib/storage.php | 2 +- 2 files changed, 39 insertions(+), 40 deletions(-) diff --git a/lib/file.php b/lib/file.php index ed4acf99..58542673 100644 --- a/lib/file.php +++ b/lib/file.php @@ -27,19 +27,23 @@ use \OC\Files\View; class File { protected $fileId; protected $owner; - protected $path; protected $sharing; protected $token =''; protected $passwordProtected = false; + protected $ownerView; + protected $ownerViewFiles; + protected $path; + protected $pathFiles; - - public function __construct($fileId, $shareOps = null){ + public function __construct($fileId, $shareOps = null, $token = null){ if (!$fileId){ throw new \Exception('No valid file has been passed'); } $this->fileId = $fileId; $this->sharing = $shareOps; + $this->token = $token; + $this->initViews(); } @@ -52,8 +56,7 @@ class File { throw new \Exception('This file was probably unshared'); } - $file = new File($rootLinkItem['file_source'], $rootLinkItem); - $file->setToken($token); + $file = new File($rootLinkItem['file_source'], $rootLinkItem, $token); if (isset($linkItem['share_with']) && !empty($linkItem['share_with'])){ $file->setPasswordProtected(true); @@ -70,14 +73,6 @@ class File { return $this->fileId; } - public function setOwner($owner){ - $this->owner = $owner; - } - - public function setPath($path){ - $this->path = $path; - } - public function setToken($token){ $this->token = $token; } @@ -136,7 +131,6 @@ class File { public function setPasswordProtected($value){ $this->passwordProtected = $value; } - /** * @@ -144,9 +138,25 @@ class File { * @throws \Exception */ public function getOwnerViewAndPath($useDefaultRoot = false){ - if ($this->isPublicShare()){ + return $useDefaultRoot ? [$this->ownerViewFiles, $this->pathFiles] : [$this->ownerView, $this->path]; + } + + public function getOwner(){ + return $this->owner; + } + + public function getOwnerView($relativeToFiles = false){ + return $relativeToFiles ? $this->ownerViewFiles : $this->ownerView; + } + + public function getPath($relativeToFiles = false){ + return $relativeToFiles ? $this->pathFiles : $this->path; + } + + protected function initViews(){ + if ($this->isPublicShare()) { if (isset($this->sharing['uid_owner'])){ - $owner = $this->sharing['uid_owner']; + $this->owner = $this->sharing['uid_owner']; if (!\OC::$server->getUserManager()->userExists($this->sharing['uid_owner'])) { throw new \Exception('Share owner' . $this->sharing['uid_owner'] . ' does not exist '); } @@ -156,39 +166,28 @@ class File { } else { throw new \Exception($this->fileId . ' is a broken share'); } - $view = new View('/' . $owner . '/files'); } else { - $owner = \OC::$server->getUserSession()->getUser()->getUID(); - $root = '/' . $owner; - if ($useDefaultRoot){ - $root .= '/' . 'files'; - } - $view = new View($root); + $this->owner = \OC::$server->getUserSession()->getUser()->getUID(); } - - $path = $view->getPath($this->fileId); - if (!$path){ + + $this->ownerView = new View('/' . $this->owner); + $this->ownerViewFiles = new View('/' . $this->owner . '/files'); + $this->path = $this->ownerView->getPath($this->fileId); + $this->pathFiles = $this->ownerViewFiles->getPath($this->fileId); + + if (!$this->path || !$this->pathFiles) { throw new \Exception($this->fileId . ' can not be resolved'); } - $this->path = $path; - $this->owner = $owner; - if (!$view->file_exists($this->path)){ + if (!$this->ownerView->file_exists($this->path)) { throw new \Exception($this->path . ' doesn\'t exist'); } - - return array($view, $this->path); - } - - - public function getOwner(){ - if (!$this->owner){ - $this->getOwnerViewAndPath(); + + if (!$this->ownerViewFiles->file_exists($this->pathFiles)) { + throw new \Exception($this->pathFiles . ' doesn\'t exist'); } - return $this->owner; } - protected function getPassword(){ return $this->sharing['share_with']; } diff --git a/lib/storage.php b/lib/storage.php index dc39122f..b3c2ce02 100644 --- a/lib/storage.php +++ b/lib/storage.php @@ -100,7 +100,7 @@ class Storage { public static function getSupportedMimetypes(){ return array_merge( array(self::MIMETYPE_LIBREOFFICE_WORDPROCESSOR), - Filter::getAll() + Filter::getAll() ); } } From b8917435f37b2c076aaeb27c25d5ca2e4e0eee40 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Fri, 18 Sep 2015 00:15:18 +0300 Subject: [PATCH 4/8] Use different entry points for users and guests --- appinfo/routes.php | 10 +- controller/sessioncontroller.php | 152 ++++++++++++++++++------------- js/ServerFactory.js | 2 - js/documents.js | 17 +++- lib/db/session.php | 18 ++-- lib/file.php | 60 ++++++------ lib/genesis.php | 12 +-- 7 files changed, 151 insertions(+), 120 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index c765fea6..511105d0 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -19,10 +19,12 @@ $application->registerRoutes($this, [ ['name' => 'user#disconnectUser', 'url' => 'ajax/user/disconnect', 'verb' => 'POST'], ['name' => 'user#disconnectGuest', 'url' => 'ajax/user/disconnectGuest', 'verb' => 'POST'], //session - ['name' => 'session#joinAsUser', 'url' => 'ajax/session/joinasuser/{fileId}', 'verb' => 'POST'], - ['name' => 'session#joinAsGuest', 'url' => 'ajax/session/joinasguest/{token}', 'verb' => 'POST'], - ['name' => 'session#save', 'url' => 'ajax/session/save', 'verb' => 'POST'], - ['name' => 'session#poll', 'url' => 'ajax/otpoll.php', 'verb' => 'POST'], + ['name' => 'session#join', 'url' => 'session/user/join/{fileId}', 'verb' => 'POST'], + ['name' => 'session#poll', 'url' => 'session/user/poll', 'verb' => 'POST'], + ['name' => 'session#save', 'url' => 'session/user/save', 'verb' => 'POST'], + ['name' => 'session#joinAsGuest', 'url' => 'session/guest/join/{token}', 'verb' => 'POST'], + ['name' => 'session#pollAsGuest', 'url' => 'session/guest/poll/{token}', 'verb' => 'POST'], + ['name' => 'session#saveAsGuest', 'url' => 'session/guest/save/{token}', 'verb' => 'POST'], //documents ['name' => 'document#index', 'url' => 'index', 'verb' => 'GET'], ['name' => 'document#create', 'url' => 'ajax/documents/create', 'verb' => 'POST'], diff --git a/controller/sessioncontroller.php b/controller/sessioncontroller.php index 2c806e17..f8c6b6e7 100644 --- a/controller/sessioncontroller.php +++ b/controller/sessioncontroller.php @@ -16,7 +16,6 @@ use \OCP\IRequest; use \OCP\AppFramework\Http; use \OCP\AppFramework\Http\JSONResponse; - use \OCA\Documents\Db; use \OCA\Documents\File; use \OCA\Documents\Helper; @@ -40,6 +39,7 @@ class SessionController extends Controller{ protected $uid; protected $logger; + protected $shareToken; public function __construct($appName, IRequest $request, $logger, $uid){ parent::__construct($appName, $request); @@ -62,13 +62,11 @@ class SessionController extends Controller{ $response = array_merge( Db\Session::start($uid, $file), - array('status'=>'success') + [ 'status'=>'success' ] ); } catch (\Exception $e){ - $this->logger->warning('Starting a session failed. Reason: ' . $e->getMessage(), array('app' => $this->appName)); - $response = array ( - 'status'=>'error' - ); + $this->logger->warning('Starting a session failed. Reason: ' . $e->getMessage(), ['app' => $this->appName]); + $response = [ 'status'=>'error' ]; } return $response; @@ -76,8 +74,27 @@ class SessionController extends Controller{ /** * @NoAdminRequired + * @PublicPage + */ + public function pollAsGuest($command, $args){ + $this->shareToken = $this->request->getParam('token'); + return $this->poll($command, $args); + } + + /** + * Store the document content to its origin + * @NoAdminRequired + * @PublicPage */ - public function joinAsUser($fileId){ + public function saveAsGuest(){ + $this->shareToken = $this->request->getParam('token'); + return $this->save(); + } + + /** + * @NoAdminRequired + */ + public function join($fileId){ try { $view = \OC\Files\Filesystem::getView(); $path = $view->getPath($fileId); @@ -87,20 +104,18 @@ class SessionController extends Controller{ $response = Db\Session::start($this->uid, $file); } else { $info = $view->getFileInfo($path); - $response = array( + $response = [ 'permissions' => $info['permissions'], 'id' => $fileId - ); + ]; } $response = array_merge( $response, - array('status'=>'success') + [ 'status'=>'success' ] ); } catch (\Exception $e){ - $this->logger->warning('Starting a session failed. Reason: ' . $e->getMessage(), array('app' => $this->appName)); - $response = array ( - 'status'=>'error' - ); + $this->logger->warning('Starting a session failed. Reason: ' . $e->getMessage(), [ 'app' => $this->appName ]); + $response = [ 'status'=>'error' ]; } return $response; @@ -108,41 +123,24 @@ class SessionController extends Controller{ /** * @NoAdminRequired - * @PublicPage */ public function poll($command, $args){ $response = new JSONResponse(); try{ $esId = isset($args['es_id']) ? $args['es_id'] : null; - - $session = new Db\Session(); - $session->load($esId); + $session = $this->loadSession($esId); $memberId = isset($args['member_id']) ? $args['member_id'] : null; - $member = new Db\Member(); - $member->load($memberId); - - if (!$member->getIsGuest()){ - \OCP\JSON::checkLoggedIn(); - } + $member = $this->loadMember($memberId); - try { - new File($session->getFileId()); - } catch (\Exception $e){ - $this->logger->warning('Error. Session no longer exists. ' . $e->getMessage(), array('app' => $this->appName)); - $ex = new BadRequestException(); - $ex->setBody( - implode(',', $this->request->getParams()) - ); - throw $ex; - } + $this->validateSession($session); switch ($command){ case 'sync_ops': $seqHead = (string) isset($args['seq_head']) ? $args['seq_head'] : null; if (!is_null($seqHead)){ - $ops = isset($args['client_ops']) ? $args['client_ops'] : array(); + $ops = isset($args['client_ops']) ? $args['client_ops'] : []; $op = new Db\Op(); $currentHead = $op->getHeadSeq($esId); @@ -177,33 +175,24 @@ class SessionController extends Controller{ } catch (BadRequestException $e){ $response->setStatus(Http::STATUS_BAD_REQUEST); $response->setData( - array('err' => 'bad request:[' . $e->getBody() . ']') + [ 'err' => 'bad request:[' . $e->getBody() . ']' ] ); } return $response; } - /** - * @NoAdminRequired - * @PublicPage * Store the document content to its origin + * @NoAdminRequired */ public function save(){ + $response = new JSONResponse(); try { $esId = $this->request->server['HTTP_WEBODF_SESSION_ID']; - if (!$esId){ - throw new \Exception('Session id can not be empty'); - } + $session = $this->loadSession($esId); $memberId = $this->request->server['HTTP_WEBODF_MEMBER_ID']; - $currentMember = new Db\Member(); - $currentMember->load($memberId); - - //check if member belongs to the session - if ($esId != $currentMember->getEsId()){ - throw new \Exception($memberId . ' does not belong to session ' . $esId); - } + $currentMember = $this->loadMember($memberId, $esId); // Extra info for future usage // $sessionRevision = $this->request->server['HTTP_WEBODF_SESSION_REVISION']; @@ -215,13 +204,6 @@ class SessionController extends Controller{ } $content = stream_get_contents($stream); - $session = new Db\Session(); - $session->load($esId); - - if (!$session->getEsId()){ - throw new \Exception('Session does not exist'); - } - try { if ($currentMember->getIsGuest()){ $file = File::getByShareToken($currentMember->getToken()); @@ -229,7 +211,8 @@ class SessionController extends Controller{ $file = new File($session->getFileId()); } - list($view, $path) = $file->getOwnerViewAndPath(true); + $view = $file->getOwnerView(true); + $path = $file->getPath(true); } catch (\Exception $e){ //File was deleted or unshared. We need to save content as new file anyway //Sorry, but for guests it would be lost :( @@ -256,7 +239,7 @@ class SessionController extends Controller{ $memberCount = count($memberIds) - 1; if ($view->file_exists($path)){ - $currentHash = sha1($view->file_get_contents($path)); + $currentHash = $view->hash('sha1', $path, false); if (!Helper::isVersionsEnabled() && $currentHash !== $session->getGenesisHash()){ // Original file was modified externally. Save to a new one @@ -274,7 +257,7 @@ class SessionController extends Controller{ // Not a last user if ($memberCount>0){ // Update genesis hash to prevent conflicts - $this->logger->debug('Update hash', array('app' => $this->appName)); + $this->logger->debug('Update hash', [ 'app' => $this->appName ]); $session->updateGenesisHash($esId, sha1($data['content'])); } else { // Last user. Kill session data @@ -283,13 +266,56 @@ class SessionController extends Controller{ $view->touch($path); } - $response = array('status'=>'success'); + $response->setData(['status'=>'success']); } catch (\Exception $e){ - $this->logger->warning('Saving failed. Reason:' . $e->getMessage(), array('app' => $this->appName)); - \OC_Response::setStatus(500); - $response = array(); + $response->setStatus(Http::STATUS_INTERNAL_SERVER_ERROR); + $response->setData([]); + $this->logger->warning('Saving failed. Reason:' . $e->getMessage(), [ 'app' => $this->appName ]); } return $response; } + + protected function validateSession($session){ + try { + if (is_null($this->shareToken)) { + new File($session->getFileId()); + } else { + File::getByShareToken($this->shareToken); + } + } catch (\Exception $e){ + $this->logger->warning('Error. Session no longer exists. ' . $e->getMessage(), [ 'app' => $this->appName ]); + $ex = new BadRequestException(); + $ex->setBody( + implode(',', $this->request->getParams()) + ); + throw $ex; + } + } + + protected function loadSession($esId){ + if (!$esId){ + throw new \Exception('Session id can not be empty'); + } + + $session = new Db\Session(); + $session->load($esId); + if (!$session->getEsId()){ + throw new \Exception('Session does not exist'); + } + return $session; + } + + protected function loadMember($memberId, $expectedEsId = null){ + if (!$memberId){ + throw new \Exception('Member id can not be empty'); + } + $member = new Db\Member(); + $member->load($memberId); + //check if member belongs to the session + if (!is_null($expectedEsId) && $expectedEsId !== $member->getEsId()){ + throw new \Exception($memberId . ' does not belong to session ' . $expectedEsId); + } + return $member; + } } diff --git a/js/ServerFactory.js b/js/ServerFactory.js index 8f5c98d2..b3f5066b 100644 --- a/js/ServerFactory.js +++ b/js/ServerFactory.js @@ -40,8 +40,6 @@ define("owncloud/ServerFactory", [ this.createServer = function (args) { var server; args = args || {}; - args.url = OC.filePath('documents', 'ajax', 'otpoll.php'); - args.sessionStateToFileUrl = OC.generateUrl('apps/documents/ajax/session/save'); server = new PullBoxServer(args); server.getGenesisUrl = function(sid) { diff --git a/js/documents.js b/js/documents.js index 0bcf1943..9de2211b 100644 --- a/js/documents.js +++ b/js/documents.js @@ -328,6 +328,13 @@ var documentsMain = { return; } + var pollUrl = documentsMain.isGuest + ? OC.generateUrl('apps/documents/session/guest/poll/{token}', {'token' : $("[name='document']").val()}) + : OC.generateUrl('apps/documents/session/user/poll'), + saveUrl = documentsMain.isGuest + ? OC.generateUrl('apps/documents/session/guest/save/{token}', {'token' : $("[name='document']").val()}) + : OC.generateUrl('apps/documents/session/user/save') + ; documentsMain.canShare = !documentsMain.isGuest && typeof OC.Share !== 'undefined' && response.permissions & OC.PERMISSION_SHARE; require({ }, ["owncloud/ServerFactory", "webodf/editor/Editor"], function (ServerFactory, Editor) { @@ -347,8 +354,10 @@ var documentsMain = { documentsMain.memberId = response.member_id; // TODO: set webodf translation system, by passing a proper function translate(!string):!string in "runtime.setTranslator(translate);" - - documentsMain.webodfServerInstance = serverFactory.createServer(); + documentsMain.webodfServerInstance = serverFactory.createServer({ + url : pollUrl, + sessionStateToFileUrl : saveUrl + }); documentsMain.webodfServerInstance.setToken(oc_requesttoken); documentsMain.webodfEditorInstance = new Editor( { @@ -383,9 +392,9 @@ var documentsMain = { console.log('joining session '+fileId); var url; if (documentsMain.isGuest){ - url = OC.generateUrl('apps/documents/ajax/session/joinasguest/{token}', {token: fileId}); + url = OC.generateUrl('apps/documents/session/guest/join/{token}', {token: fileId}); } else { - url = OC.generateUrl('apps/documents/ajax/session/joinasuser/{file_id}', {file_id: fileId}); + url = OC.generateUrl('apps/documents/session/user/join/{file_id}', {file_id: fileId}); } $.post( url, diff --git a/lib/db/session.php b/lib/db/session.php index cbfe0cb8..31b2ab30 100644 --- a/lib/db/session.php +++ b/lib/db/session.php @@ -49,12 +49,7 @@ class Session extends \OCA\Documents\Db { public static function start($uid, $file){ // Create a directory to store genesis $genesis = new \OCA\Documents\Genesis($file); - - list($ownerView, $path) = $file->getOwnerViewAndPath(); - $mimetype = $ownerView->getMimeType($path); - if (!Filter::isSupportedMimetype($mimetype)){ - throw new \Exception( $path . ' is ' . $mimetype . ' and is not supported by Documents app'); - } + $oldSession = new Session(); $oldSession->loadBy('file_id', $file->getFileId()); @@ -78,14 +73,14 @@ class Session extends \OCA\Documents\Db { ; $memberColor = \OCA\Documents\Helper::getMemberColor($uid); - $member = new \OCA\Documents\Db\Member(array( + $member = new \OCA\Documents\Db\Member([ $sessionData['es_id'], $uid, $memberColor, time(), intval($file->isPublicShare()), $file->getToken() - )); + ]); if (!$member->insert()){ throw new \Exception('Failed to add member into database'); @@ -113,10 +108,9 @@ class Session extends \OCA\Documents\Db { $memberColor, $imageUrl ); - - $sessionData['title'] = basename($path); - $fileInfo = $ownerView->getFileInfo($path); - $sessionData['permissions'] = $fileInfo->getPermissions(); + + $sessionData['title'] = basename($file->getPath()); + $sessionData['permissions'] = $file->getPermissions(); return $sessionData; } diff --git a/lib/file.php b/lib/file.php index 58542673..fb6ae9fb 100644 --- a/lib/file.php +++ b/lib/file.php @@ -28,14 +28,14 @@ class File { protected $fileId; protected $owner; protected $sharing; - protected $token =''; + protected $token; protected $passwordProtected = false; protected $ownerView; protected $ownerViewFiles; protected $path; protected $pathFiles; - public function __construct($fileId, $shareOps = null, $token = null){ + public function __construct($fileId, $shareOps = null, $token = ''){ if (!$fileId){ throw new \Exception('No valid file has been passed'); } @@ -43,6 +43,22 @@ class File { $this->fileId = $fileId; $this->sharing = $shareOps; $this->token = $token; + + if ($this->isPublicShare()) { + if (isset($this->sharing['uid_owner'])){ + $this->owner = $this->sharing['uid_owner']; + if (!\OC::$server->getUserManager()->userExists($this->sharing['uid_owner'])) { + throw new \Exception('Share owner' . $this->sharing['uid_owner'] . ' does not exist '); + } + + \OC_Util::tearDownFS(); + \OC_Util::setupFS($this->sharing['uid_owner']); + } else { + throw new \Exception($this->fileId . ' is a broken share'); + } + } else { + $this->owner = \OC::$server->getUserSession()->getUser()->getUID(); + } $this->initViews(); } @@ -131,15 +147,6 @@ class File { public function setPasswordProtected($value){ $this->passwordProtected = $value; } - - /** - * - * @return string owner of the current file item - * @throws \Exception - */ - public function getOwnerViewAndPath($useDefaultRoot = false){ - return $useDefaultRoot ? [$this->ownerViewFiles, $this->pathFiles] : [$this->ownerView, $this->path]; - } public function getOwner(){ return $this->owner; @@ -153,23 +160,12 @@ class File { return $relativeToFiles ? $this->pathFiles : $this->path; } + public function getPermissions(){ + $fileInfo = $this->ownerView->getFileInfo($this->path); + return $fileInfo->getPermissions(); + } + protected function initViews(){ - if ($this->isPublicShare()) { - if (isset($this->sharing['uid_owner'])){ - $this->owner = $this->sharing['uid_owner']; - if (!\OC::$server->getUserManager()->userExists($this->sharing['uid_owner'])) { - throw new \Exception('Share owner' . $this->sharing['uid_owner'] . ' does not exist '); - } - - \OC_Util::tearDownFS(); - \OC_Util::setupFS($this->sharing['uid_owner']); - } else { - throw new \Exception($this->fileId . ' is a broken share'); - } - } else { - $this->owner = \OC::$server->getUserSession()->getUser()->getUID(); - } - $this->ownerView = new View('/' . $this->owner); $this->ownerViewFiles = new View('/' . $this->owner . '/files'); $this->path = $this->ownerView->getPath($this->fileId); @@ -186,6 +182,16 @@ class File { if (!$this->ownerViewFiles->file_exists($this->pathFiles)) { throw new \Exception($this->pathFiles . ' doesn\'t exist'); } + + if (!$this->ownerView->is_file($this->path)){ + throw new \Exception('Object ' . $this->path . ' is not a file.'); + } + //TODO check if it is a valid odt + + $mimetype = $this->ownerView->getMimeType($this->path); + if (!Filter::isSupportedMimetype($mimetype)){ + throw new \Exception( $this->path . ' is ' . $mimetype . ' and is not supported by Documents app'); + } } protected function getPassword(){ diff --git a/lib/genesis.php b/lib/genesis.php index a74d3274..345e7a78 100644 --- a/lib/genesis.php +++ b/lib/genesis.php @@ -40,7 +40,8 @@ class Genesis { * @param File $file * */ public function __construct(File $file){ - list($view, $path) = $file->getOwnerViewAndPath(); + $view = $file->getOwnerView(); + $path = $file->getPath(); $owner = $file->getOwner(); $this->view = new View('/' . $owner); @@ -48,8 +49,9 @@ class Genesis { if (!$this->view->file_exists(self::DOCUMENTS_DIRNAME)){ $this->view->mkdir(self::DOCUMENTS_DIRNAME ); } + $this->validate($view, $path); - $this->hash = $this->getDocumentHash($view, $path); + $this->hash = $view->hash('sha1', $path, false); $this->path = self::DOCUMENTS_DIRNAME . '/' . $this->hash . '.odt'; if (!$this->view->file_exists($this->path)){ //copy new genesis to /user/documents/{hash}.odt @@ -76,12 +78,6 @@ class Genesis { return $this->hash; } - protected function getDocumentHash($view, $path){ - $this->validate($view, $path); - $hash = sha1($view->file_get_contents($path)); - return $hash; - } - /** * Check if genesis is valid * @param \OC\Files\View $view From 98db623f0717bb2cefc6d874aa1a4dc168b529dc Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Fri, 18 Sep 2015 20:44:38 +0300 Subject: [PATCH 5/8] Remove getL10N --- lib/config.php | 4 ---- lib/db/member.php | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/config.php b/lib/config.php index b4e59067..3d5848bf 100644 --- a/lib/config.php +++ b/lib/config.php @@ -45,10 +45,6 @@ class Config { return true; } - public static function getL10n(){ - return \OCP\Util::getL10N(self::APP_NAME); - } - public static function getConverter(){ return self::getAppValue('converter', 'off'); } diff --git a/lib/db/member.php b/lib/db/member.php index 9d70b843..a0db4dba 100644 --- a/lib/db/member.php +++ b/lib/db/member.php @@ -36,7 +36,7 @@ class Member extends \OCA\Documents\Db{ protected $loadStatement = 'SELECT * FROM `*PREFIX*documents_member` WHERE `member_id`= ?'; public static function getGuestPostfix(){ - return '(' . \OCA\Documents\Config::getL10n()->t('guest') . ')'; + return '(' . \OC::$server->getL10n('documents')->t('guest') . ')'; } From 5c8a7923cbff95c023b3731f8236f2f5b69bd7c5 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Fri, 18 Sep 2015 20:56:07 +0300 Subject: [PATCH 6/8] Kill static logger --- lib/config.php | 6 +++++- lib/converter.php | 12 ++++++++++-- lib/helper.php | 30 ++++++++++-------------------- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/lib/config.php b/lib/config.php index 3d5848bf..6065de87 100644 --- a/lib/config.php +++ b/lib/config.php @@ -37,7 +37,11 @@ class Config { $exists = file_exists($outfile); if (!$exists){ - Helper::warnLog('Conversion test failed. Raw output:' . $result); + \OC::$server->getLogger()->warn( + 'Conversion test failed. Raw output:' . $result, + ['app' => 'documents'] + + ); return false; } else { unlink($outfile); diff --git a/lib/converter.php b/lib/converter.php index 5dbe2007..ccd3d6b3 100644 --- a/lib/converter.php +++ b/lib/converter.php @@ -22,7 +22,11 @@ class Converter { } if (empty($output)){ - Helper::warnLog('Empty conversion output'); + \OC::$server->getLogger()->warn( + 'Empty conversion output', + ['app' => 'documents'] + + ); throw new \RuntimeException('Empty conversion output'); } return $output; @@ -86,7 +90,11 @@ class Converter { curl_setopt_array($ch, $options); $content = curl_exec($ch); if (curl_errno($ch)){ - Helper::debugLog('cURL error' . curl_errno($ch) . ':' . curl_error($ch)); + \OC::$server->getLogger()->debug( + 'cURL error' . curl_errno($ch) . ':' . curl_error($ch), + ['app' => 'documents'] + + ); } curl_close($ch); diff --git a/lib/helper.php b/lib/helper.php index e6ffaee5..2f02be51 100644 --- a/lib/helper.php +++ b/lib/helper.php @@ -54,24 +54,6 @@ class Helper { return '#' . self::convertHSLToRGB($hue, 90, 60); } - /** - * @param string $message - */ - public static function debugLog($message){ - self::log($message, \OCP\Util::DEBUG); - } - - /** - * @param string $message - */ - public static function warnLog($message){ - self::log($message, \OCP\Util::WARN); - } - - public static function log($message, $level){ - \OCP\Util::writeLog(self::APP_ID, $message, $level); - } - /** * @param integer $iH * @param integer $iS @@ -178,8 +160,16 @@ class Helper { } if (empty($cmd)){ - Helper::warnLog('Pure configuration issue. Missing open office binary that is mandatory for conversion.'); - Helper::debugLog('If openoffice or libreoffice is already installed please specify the path to it using preview_libreoffice_path config. Refer to admin manual for details.'); + \OC::$server->getLogger()->warn( + 'Pure configuration issue. Missing open office binary that is mandatory for conversion.', + ['app' => 'documents'] + + ); + \OC::$server->getLogger()->debug( + 'If openoffice or libreoffice is already installed please specify the path to it using preview_libreoffice_path config. Refer to admin manual for details.', + ['app' => 'documents'] + + ); throw new \RuntimeException('Missing open office binary that is mandatory for conversion.'); } From 76ef4e48aa2ad9218e474bcd41a9df7a89118c92 Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Fri, 18 Sep 2015 22:10:27 +0300 Subject: [PATCH 7/8] ByeBye static Config class --- appinfo/app.php | 2 +- appinfo/application.php | 9 +++- controller/settingscontroller.php | 35 ++++++------- lib/appconfig.php | 86 +++++++++++++++++++++++++++++++ lib/config.php | 76 --------------------------- lib/converter.php | 47 ++++++++++++++++- 6 files changed, 157 insertions(+), 98 deletions(-) create mode 100644 lib/appconfig.php delete mode 100644 lib/config.php diff --git a/appinfo/app.php b/appinfo/app.php index 4155214e..66c3b464 100644 --- a/appinfo/app.php +++ b/appinfo/app.php @@ -53,7 +53,7 @@ if (isset($request->server['REQUEST_URI'])) { } } -if (Config::getConverter() !== 'off'){ +if ($c->query('AppConfig')->isConverterEnabled()){ $docFilter = new Office( [ 'read' => diff --git a/appinfo/application.php b/appinfo/application.php index a36f7855..1dda6655 100644 --- a/appinfo/application.php +++ b/appinfo/application.php @@ -17,6 +17,7 @@ use \OCA\Documents\Controller\UserController; use \OCA\Documents\Controller\SessionController; use \OCA\Documents\Controller\DocumentController; use \OCA\Documents\Controller\SettingsController; +use \OCA\Documents\AppConfig; class Application extends App { public function __construct (array $urlParams = array()) { @@ -54,12 +55,18 @@ class Application extends App { return new SettingsController( $c->query('AppName'), $c->query('Request'), - $c->query('CoreConfig'), $c->query('L10N'), + $c->query('AppConfig'), $c->query('UserId') ); }); + $container->registerService('AppConfig', function($c) { + return new AppConfig( + $c->query('CoreConfig') + ); + }); + /** * Core */ diff --git a/controller/settingscontroller.php b/controller/settingscontroller.php index fdd3e097..c9d90a54 100644 --- a/controller/settingscontroller.php +++ b/controller/settingscontroller.php @@ -13,26 +13,25 @@ namespace OCA\Documents\Controller; use \OCP\AppFramework\Controller; use \OCP\IRequest; -use \OCP\IConfig; use \OCP\IL10N; use \OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCA\Documents\AppConfig; use OCA\Documents\Converter; -use OCA\Documents\Config; use OCA\Documents\Filter; class SettingsController extends Controller{ private $userId; - private $settings; private $l10n; + private $appConfig; - public function __construct($appName, IRequest $request, IConfig $settings, IL10N $l10n, $userId){ + public function __construct($appName, IRequest $request, IL10N $l10n, AppConfig $appConfig, $userId){ parent::__construct($appName, $request); $this->userId = $userId; - $this->settings = $settings; $this->l10n = $l10n; + $this->appConfig = $appConfig; } /** @@ -51,9 +50,9 @@ class SettingsController extends Controller{ */ public function personalIndex(){ return new TemplateResponse( - 'documents', + $this->appName, 'personal', - [ 'save_path' => $this->settings->getUserValue($this->userId, 'documents', 'save_path', '/') ], + [ 'save_path' => $this->appConfig->getUserValue($this->userId, 'save_path') ], 'blank' ); } @@ -63,9 +62,9 @@ class SettingsController extends Controller{ */ public function settingsIndex(){ return new TemplateResponse( - 'documents', + $this->appName, 'settings', - [ 'unstable' => $this->settings->getAppValue('documents', 'unstable', 'false') ], + [ 'unstable' => $this->appConfig->getAppValue('unstable') ], 'blank' ); } @@ -75,11 +74,11 @@ class SettingsController extends Controller{ */ public function adminIndex(){ return new TemplateResponse( - 'documents', + $this->appName, 'admin', [ - 'converter' => Config::getConverter(), - 'converter_url' => Config::getConverterUrl(), + 'converter' => $this->appConfig->getAppValue('converter'), + 'converter_url' => $this->appConfig->getAppValue('converter_url'), ], 'blank' ); @@ -98,7 +97,7 @@ class SettingsController extends Controller{ } if ($status){ - $this->settings->setUserValue($this->userId, $this->appName, 'save_path', $savePath); + $this->appConfig->setUserValue($this->userId, 'save_path', $savePath); $response = array( 'status' => 'success', 'data' => array('message'=> $this->l10n->t('Directory saved successfully.')) @@ -116,18 +115,18 @@ class SettingsController extends Controller{ public function setUnstable($unstable){ if (!is_null($unstable)){ - $this->settings->setAppValue($this->appName, 'unstable', $unstable); + $this->appConfig->setAppValue('unstable', $unstable); } return array('status' => 'success'); } public function setConverter($converter, $url){ if (!is_null($converter)){ - $this->settings->setAppValue($this->appName, 'converter', $converter); + $this->appConfig->setAppValue('converter', $converter); } if (!is_null($url)){ - $this->settings->setAppValue($this->appName, 'converter_url', $url); + $this->appConfig->setAppValue('converter_url', $url); } $response = array( @@ -135,7 +134,7 @@ class SettingsController extends Controller{ 'data' => array('message' => (string) $this->l10n->t('Saved')) ); - $currentConverter = $this->settings->getAppValue($this->appName, 'converter', 'off'); + $currentConverter = $this->appConfig->getAppValue('converter'); if ($currentConverter == 'external'){ if (!Converter::checkConnection()){ \OC::$server->getLogger()->warning( @@ -150,7 +149,7 @@ class SettingsController extends Controller{ } } elseif ($currentConverter === 'local') { try { - if (!Config::testConversion()){ + if (!Converter::testConversion()){ $response = array( 'status' => 'error', 'data'=> diff --git a/lib/appconfig.php b/lib/appconfig.php new file mode 100644 index 00000000..1f4463d0 --- /dev/null +++ b/lib/appconfig.php @@ -0,0 +1,86 @@ + 'off', + 'converter_url' => 'http://localhost:16080', + 'unstable' => 'false' + ]; + + private $config; + + public function __construct(IConfig $config) { + $this->config = $config; + } + + /** + * Can we convert anything to odt? + * @return bool + */ + public function isConverterEnabled(){ + return $this->getAppValue('converter') !== 'off'; + } + + /** + * Get a value by key + * @param string $key + * @return string + */ + public function getAppValue($key) { + $defaultValue = null; + if (array_key_exists($key, $this->defaults)){ + $defaultValue = $this->defaults[$key]; + } + return $this->config->getAppValue($this->appName, $key, $defaultValue); + } + + /** + * Set a value by key + * @param string $key + * @param string $value + * @return string + */ + public function setAppValue($key, $value) { + return $this->config->setAppValue($this->appName, $key, $value); + } + + /** + * Get a value by key for a user + * @param string $userId + * @param string $key + * @return string + */ + public function getUserValue($userId, $key) { + $defaultValue = null; + if (array_key_exists($key, $this->defaults)){ + $defaultValue = $this->defaults[$key]; + } + return $this->config->getUserValue($userId, $this->appName, $key, $defaultValue); + } + + /** + * Set a value by key for a user + * @param string $userId + * @param string $key + * @param string $value + * @return string + */ + public function setUserValue($userId, $key, $value) { + return $this->config->setAppValue($userId, $this->appName, $key, $value); + } + } + \ No newline at end of file diff --git a/lib/config.php b/lib/config.php deleted file mode 100644 index 6065de87..00000000 --- a/lib/config.php +++ /dev/null @@ -1,76 +0,0 @@ -getTempManager()->getTemporaryFile(); - $outdir = \OC::$server->getTempManager()->getTemporaryFolder(); - $outfile = $outdir . '/' . basename($infile) . '.' . $targetExtension; - $cmd = Helper::findOpenOffice(); - - $params = ' --headless --convert-to ' . escapeshellarg($targetFilter) . ' --outdir ' - . escapeshellarg($outdir) - . ' --writer '. escapeshellarg($infile) - . ' -env:UserInstallation=file://' - . escapeshellarg(get_temp_dir() . '/owncloud-' . \OC_Util::getInstanceId().'/') . ' 2>&1' - ; - file_put_contents($infile, $input); - - $result = shell_exec($cmd . $params); - $exists = file_exists($outfile); - - if (!$exists){ - \OC::$server->getLogger()->warn( - 'Conversion test failed. Raw output:' . $result, - ['app' => 'documents'] - - ); - return false; - } else { - unlink($outfile); - } - return true; - } - - public static function getConverter(){ - return self::getAppValue('converter', 'off'); - } - - public static function setConverter($value){ - return self::setAppValue('converter', $value); - } - - public static function getConverterUrl(){ - return self::getAppValue('converter_url', 'http://localhost:16080'); - } - - public static function setConverterUrl($value){ - return self::setAppValue('converter_url', $value); - } - - protected static function getAppValue($key, $default){ - return \OC::$server->getConfig()->getAppValue(self::APP_NAME, $key, $default); - } - - protected static function setAppValue($key, $value){ - return \OC::$server->getConfig()->setAppValue(self::APP_NAME, $key, $value); - } - -} diff --git a/lib/converter.php b/lib/converter.php index ccd3d6b3..0ba1a4b1 100644 --- a/lib/converter.php +++ b/lib/converter.php @@ -12,10 +12,47 @@ namespace OCA\Documents; +use OCA\Documents\AppInfo\Application; + class Converter { + const TEST_DOC_PATH = '/assets/test.doc'; + + public static function testConversion(){ + $targetFilter = 'odt:writer8'; + $targetExtension = 'odt'; + $input = file_get_contents(dirname(__DIR__) . self::TEST_DOC_PATH); + $infile = \OC::$server->getTempManager()->getTemporaryFile(); + $outdir = \OC::$server->getTempManager()->getTemporaryFolder(); + $outfile = $outdir . '/' . basename($infile) . '.' . $targetExtension; + $cmd = Helper::findOpenOffice(); + + $params = ' --headless --convert-to ' . escapeshellarg($targetFilter) . ' --outdir ' + . escapeshellarg($outdir) + . ' --writer '. escapeshellarg($infile) + . ' -env:UserInstallation=file://' + . escapeshellarg(\OC::$server->getTempManager()->getTempBaseDir() . '/owncloud-' . \OC_Util::getInstanceId().'/') . ' 2>&1' + ; + file_put_contents($infile, $input); + + $result = shell_exec($cmd . $params); + $exists = file_exists($outfile); + + if (!$exists){ + \OC::$server->getLogger()->warn( + 'Conversion test failed. Raw output:' . $result, + ['app' => 'documents'] + + ); + return false; + } else { + unlink($outfile); + } + return true; + } + public static function convert($input, $targetFilter, $targetExtension){ - if (Config::getConverter() == 'local'){ + if (self::getAppConfig()->getAppValue('converter') === 'local'){ $output = self::convertLocal($input, $targetFilter, $targetExtension); } else { $output = self::convertExternal($input, $targetExtension); @@ -86,7 +123,7 @@ class Converter { CURLOPT_VERBOSE => 1 ); - $ch = curl_init(Config::getConverterUrl() . '?target_format=' . $targetExtension); + $ch = curl_init(self::getAppConfig()->getAppValue('converter_url') . '?target_format=' . $targetExtension); curl_setopt_array($ch, $options); $content = curl_exec($ch); if (curl_errno($ch)){ @@ -100,5 +137,11 @@ class Converter { return $content; } + + protected static function getAppConfig(){ + $app = new Application(); + $c = $app->getContainer(); + return $c->query('AppConfig'); + } } From 488d68de6a97b1fa8eceb7b140a391d571e1f5eb Mon Sep 17 00:00:00 2001 From: Victor Dubiniuk Date: Sat, 19 Sep 2015 00:37:16 +0300 Subject: [PATCH 8/8] A few fixes by scrutinizer --- js/widgets/ocToolbar.js | 2 +- lib/db/op.php | 2 +- lib/downloadresponse.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/js/widgets/ocToolbar.js b/js/widgets/ocToolbar.js index e6432172..c0aa96ab 100644 --- a/js/widgets/ocToolbar.js +++ b/js/widgets/ocToolbar.js @@ -42,7 +42,7 @@ define("owncloud/widgets/ocToolbar", ocClose.startup(); }); return callback(ocToolbar); - }; + } // init makeWidget(function (widget) { return callback(widget); diff --git a/lib/db/op.php b/lib/db/op.php index b8a8ff03..e795da6d 100644 --- a/lib/db/op.php +++ b/lib/db/op.php @@ -78,7 +78,7 @@ class Op extends \OCA\Documents\Db { AND `seq`>? ORDER BY `seq` ASC '); - $result = $query->execute(array($esId, $seq)); + $query->execute(array($esId, $seq)); return $query->fetchAll(); } diff --git a/lib/downloadresponse.php b/lib/downloadresponse.php index a37ee32b..a24ca447 100644 --- a/lib/downloadresponse.php +++ b/lib/downloadresponse.php @@ -32,12 +32,12 @@ class DownloadResponse extends \OCP\AppFramework\Http\Response { $this->view = new View('/' . $user); if (!$this->view->file_exists($path)){ - parent::setStatus(Http::STATUS_NOT_FOUND); + $this->setStatus(Http::STATUS_NOT_FOUND); } } public function render(){ - if (parent::getStatus() === Http::STATUS_NOT_FOUND){ + if ($this->getStatus() === Http::STATUS_NOT_FOUND){ return ''; } $info = $this->view->getFileInfo($this->path);