From d1883e9715dff960dc912372b2447f46500ef39a Mon Sep 17 00:00:00 2001 From: korelstar Date: Mon, 18 May 2020 20:21:37 +0200 Subject: [PATCH 1/5] allow max-nc-version to be higher than server git --- tests/nextcloud-version.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/nextcloud-version.php b/tests/nextcloud-version.php index 2886eda99..5446c87c3 100644 --- a/tests/nextcloud-version.php +++ b/tests/nextcloud-version.php @@ -64,11 +64,14 @@ function getNCVersionFromAppInfo($path, $minmax = 'min') { return $v; } -function versionCompare($sv1, $sv2) { +function versionCompare($sv1, $sv2, $type) { $v1 = explode('.', $sv1); $v2 = explode('.', $sv2); $count = min(count($v1), count($v2)); for ($i=0; $i<$count; $i++) { + if ($type == 'max' && $v1[$i] < $v2[$i]) { + return true; + } if ($v1[$i] !== $v2[$i]) { return false; } @@ -82,13 +85,13 @@ function versionCompare($sv1, $sv2) { if ($vComposer === 'dev-master') { $vComposer = getNCVersionFromComposerBranchAlias(__DIR__.'/../vendor/christophwurst/nextcloud/composer.json'); $vAppInfo = getNCVersionFromAppInfo(__DIR__.'/../appinfo/info.xml', 'max'); - echo 'max'; + $type = 'max'; } else { $vAppInfo = getNCVersionFromAppInfo(__DIR__.'/../appinfo/info.xml', 'min'); - echo 'min'; + $type = 'min'; } - echo ': '.$vAppInfo.' (AppInfo) vs. '.$vComposer.' (Composer) => '; - if (versionCompare($vComposer, $vAppInfo)) { + echo $type.': '.$vAppInfo.' (AppInfo) vs. '.$vComposer.' (Composer) => '; + if (versionCompare($vComposer, $vAppInfo, $type)) { echo 'OK'.PHP_EOL; } else { echo 'FAILED'.PHP_EOL; From 2b8bd077a33f7eeb4e6f1b03e17af191f31d5669 Mon Sep 17 00:00:00 2001 From: korelstar Date: Mon, 18 May 2020 20:24:16 +0200 Subject: [PATCH 2/5] increase Nextcloud max-version to 21 --- appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index c7f21ac9b..cf90c212b 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -22,6 +22,6 @@ The Notes app is a distraction free notes taking app for [Nextcloud](https://www https://github.com/nextcloud/notes.git https://raw.githubusercontent.com/nextcloud/screenshots/master/apps/Notes/notes.png - + From c623f4a94dc7b169af1263c3e7073b2a20b5607c Mon Sep 17 00:00:00 2001 From: korelstar Date: Mon, 18 May 2020 20:40:45 +0200 Subject: [PATCH 3/5] run API tests with both Nextcloud min-/max-version --- .github/workflows/test.yml | 22 +++++++++++++++++++--- tests/api/AbstractAPITest.php | 8 ++++---- tests/nextcloud-version.php | 5 +++++ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0bd0ccb6a..e94186487 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,11 +9,23 @@ on: jobs: test-api: runs-on: ubuntu-latest + strategy: + matrix: + version: [min, max] + include: + - version: min + php-version: 7.1 + - version: max + php-version: 7.4 + env: + SERVER_BRANCH: master steps: - name: Checkout uses: actions/checkout@v2 - - name: Set up php + - name: Set up php${{ matrix.php-versions }} uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php-version }} - name: Install Dependencies run: composer install --prefer-dist - name: Prepare MySQL database @@ -22,15 +34,19 @@ jobs: mysql -u root -proot -e "CREATE DATABASE nextcloud;" mysql -u root -proot -e "CREATE USER 'nextcloud'@'localhost' IDENTIFIED BY '';" mysql -u root -proot -e "GRANT ALL ON nextcloud.* TO 'nextcloud'@'localhost';" - - name: Prepare Nextcloud server + - name: Select Nextcloud server branch + run: echo "::set-env name=SERVER_BRANCH::stable$(php tests/nextcloud-version.php --appinfo)" + if: matrix.version == 'min' + - name: Prepare Nextcloud server using ${{ env.SERVER_BRANCH }} working-directory: ../ run: | - git clone https://github.com/nextcloud/server.git --recursive --depth 1 -b master server + git clone https://github.com/nextcloud/server.git --recursive --depth 1 -b ${SERVER_BRANCH} server cp -r notes server/apps/ - name: Setup Nextcloud server working-directory: ../server/ run: | php occ maintenance:install --database-name nextcloud --database-user nextcloud --admin-user admin --admin-pass admin --database mysql --database-pass='' + php occ -V OC_PASS=test php occ user:add --password-from-env --display-name="Test" test OC_PASS=test php occ user:add --password-from-env --display-name="QuotaTest" quotatest php occ user:setting quotatest files quota "0" diff --git a/tests/api/AbstractAPITest.php b/tests/api/AbstractAPITest.php index 473c3af80..4caa9e38c 100644 --- a/tests/api/AbstractAPITest.php +++ b/tests/api/AbstractAPITest.php @@ -82,8 +82,8 @@ protected function checkGetReferenceNotes( } protected function checkReferenceNote( - object $refNote, - object $note, + \stdClass $refNote, + \stdClass $note, string $messagePrefix, array $expectExclude = [] ) : void { @@ -136,8 +136,8 @@ protected function checkReferenceNote( } protected function checkNoteEmpty( - object $refNote, - object $note, + \stdClass $refNote, + \stdClass $note, string $messagePrefix ) : void { $this->assertEquals( diff --git a/tests/nextcloud-version.php b/tests/nextcloud-version.php index 5446c87c3..06c0242ef 100644 --- a/tests/nextcloud-version.php +++ b/tests/nextcloud-version.php @@ -79,6 +79,11 @@ function versionCompare($sv1, $sv2, $type) { return true; } +if (in_array('--appinfo', $argv)) { + echo getNCVersionFromAppInfo(__DIR__.'/../appinfo/info.xml', 'min'); + exit; +} + echo 'Testing Nextcloud version '; try { $vComposer = getNCVersionFromComposer(__DIR__.'/../composer.json'); From a5ccea37de7e72a33e8735b158cc870b270bf35e Mon Sep 17 00:00:00 2001 From: korelstar Date: Mon, 18 May 2020 21:11:20 +0200 Subject: [PATCH 4/5] set strict types --- lib/Application.php | 2 +- lib/Capabilities.php | 2 +- lib/Db/Meta.php | 4 ++-- lib/Db/MetaMapper.php | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Application.php b/lib/Application.php index a92bf5f7b..57fe7c1a7 100644 --- a/lib/Application.php +++ b/lib/Application.php @@ -1,4 +1,4 @@ -setUserId($userId); $meta->setFileId($note->getId()); diff --git a/lib/Db/MetaMapper.php b/lib/Db/MetaMapper.php index 32fdb5b80..e406063cd 100644 --- a/lib/Db/MetaMapper.php +++ b/lib/Db/MetaMapper.php @@ -1,4 +1,4 @@ -db->getQueryBuilder(); $qb->select('*') ->from('*PREFIX*notes_meta') From 8222b594342a25a468bc984dca4bc1c264a408f8 Mon Sep 17 00:00:00 2001 From: korelstar Date: Tue, 19 May 2020 14:24:44 +0200 Subject: [PATCH 5/5] use JSONResponse and remove If-None-Match check --- lib/Controller/Helper.php | 12 ++++++------ lib/Controller/NotesApiController.php | 25 +++++++++++-------------- lib/Controller/NotesController.php | 18 +++++++++--------- 3 files changed, 26 insertions(+), 29 deletions(-) diff --git a/lib/Controller/Helper.php b/lib/Controller/Helper.php index 7eaf1dbe6..f4664381e 100644 --- a/lib/Controller/Helper.php +++ b/lib/Controller/Helper.php @@ -7,7 +7,7 @@ use OCA\Notes\Service\NoteDoesNotExistException; use OCP\AppFramework\Http; -use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\JSONResponse; use OCP\ILogger; class Helper { @@ -23,19 +23,19 @@ public function __construct( $this->appName = $appName; } - public function handleErrorResponse(callable $respond) : DataResponse { + public function handleErrorResponse(callable $respond) : JSONResponse { try { $data = $respond(); - $response = $data instanceof DataResponse ? $data : new DataResponse($data); + $response = $data instanceof JSONResponse ? $data : new JSONResponse($data); } catch (NoteDoesNotExistException $e) { $this->logger->logException($e, [ 'app' => $this->appName ]); - $response = new DataResponse([], Http::STATUS_NOT_FOUND); + $response = new JSONResponse([], Http::STATUS_NOT_FOUND); } catch (InsufficientStorageException $e) { $this->logger->logException($e, [ 'app' => $this->appName ]); - $response = new DataResponse([], Http::STATUS_INSUFFICIENT_STORAGE); + $response = new JSONResponse([], Http::STATUS_INSUFFICIENT_STORAGE); } catch (\Throwable $e) { $this->logger->logException($e, [ 'app' => $this->appName ]); - $response = new DataResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); + $response = new JSONResponse([], Http::STATUS_INTERNAL_SERVER_ERROR); } $response->addHeader('X-Notes-API-Versions', implode(', ', Application::$API_VERSIONS)); return $response; diff --git a/lib/Controller/NotesApiController.php b/lib/Controller/NotesApiController.php index f8e62ef30..d8eaa7ee5 100644 --- a/lib/Controller/NotesApiController.php +++ b/lib/Controller/NotesApiController.php @@ -7,7 +7,7 @@ use OCP\AppFramework\ApiController; use OCP\AppFramework\Http; -use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; use OCP\IUserSession; @@ -47,7 +47,7 @@ private function getUID() : string { * @CORS * @NoCSRFRequired */ - public function index(string $exclude = '', int $pruneBefore = 0) : DataResponse { + public function index(string $exclude = '', int $pruneBefore = 0) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($exclude, $pruneBefore) { $exclude = explode(',', $exclude); $now = new \DateTime(); // this must be before loading notes if there are concurrent changes possible @@ -62,10 +62,7 @@ public function index(string $exclude = '', int $pruneBefore = 0) : DataResponse } }, $notes); $etag = md5(json_encode($notesData)); - if ($this->request->getHeader('If-None-Match') === '"'.$etag.'"') { - return new DataResponse([], Http::STATUS_NOT_MODIFIED); - } - return (new DataResponse($notesData)) + return (new JSONResponse($notesData)) ->setLastModified($now) ->setETag($etag); }); @@ -77,7 +74,7 @@ public function index(string $exclude = '', int $pruneBefore = 0) : DataResponse * @CORS * @NoCSRFRequired */ - public function get(int $id, string $exclude = '') : DataResponse { + public function get(int $id, string $exclude = '') : JSONResponse { return $this->helper->handleErrorResponse(function () use ($id, $exclude) { $exclude = explode(',', $exclude); $note = $this->service->get($this->getUID(), $id); @@ -97,7 +94,7 @@ public function create( string $content = '', int $modified = 0, bool $favorite = false - ) : DataResponse { + ) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($category, $title, $content, $modified, $favorite) { $note = $this->service->create($this->getUID(), $title, $category); try { @@ -128,7 +125,7 @@ public function createAutoTitle( string $content = '', int $modified = 0, bool $favorite = false - ) : DataResponse { + ) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($category, $content, $modified, $favorite) { $title = $this->service->getTitleFromContent($content); return $this->create($category, $title, $content, $modified, $favorite); @@ -147,7 +144,7 @@ public function update( ?string $title = null, ?string $category = null, ?bool $favorite = null - ) : DataResponse { + ) : JSONResponse { return $this->helper->handleErrorResponse(function () use ( $id, $content, @@ -187,7 +184,7 @@ public function updateAutoTitle( ?int $modified = null, ?string $category = null, ?bool $favorite = null - ) : DataResponse { + ) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($id, $content, $modified, $category, $favorite) { if ($content === null) { $note = $this->service->get($this->getUID(), $id); @@ -204,7 +201,7 @@ public function updateAutoTitle( * @CORS * @NoCSRFRequired */ - public function destroy(int $id) : DataResponse { + public function destroy(int $id) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($id) { $this->service->delete($this->getUID(), $id); return []; @@ -215,9 +212,9 @@ public function destroy(int $id) : DataResponse { * @NoAdminRequired * @NoCSRFRequired */ - public function fail() : DataResponse { + public function fail() : JSONResponse { return $this->helper->handleErrorResponse(function () { - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new JSONResponse([], Http::STATUS_BAD_REQUEST); }); } } diff --git a/lib/Controller/NotesController.php b/lib/Controller/NotesController.php index c2be6e733..8bdc80d74 100644 --- a/lib/Controller/NotesController.php +++ b/lib/Controller/NotesController.php @@ -10,7 +10,7 @@ use OCP\IConfig; use OCP\IL10N; use OCP\AppFramework\Http; -use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\JSONResponse; class NotesController extends Controller { @@ -50,7 +50,7 @@ public function __construct( /** * @NoAdminRequired */ - public function index() : DataResponse { + public function index() : JSONResponse { return $this->helper->handleErrorResponse(function () { $settings = $this->settingsService->getAll($this->userId); @@ -94,7 +94,7 @@ public function index() : DataResponse { /** * @NoAdminRequired */ - public function get(int $id) : DataResponse { + public function get(int $id) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($id) { $note = $this->notesService->get($this->userId, $id); @@ -114,7 +114,7 @@ public function get(int $id) : DataResponse { /** * @NoAdminRequired */ - public function create(string $category) : DataResponse { + public function create(string $category) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($category) { $note = $this->notesService->create($this->userId, '', $category); $note->setContent(''); @@ -133,7 +133,7 @@ public function undo( string $category, int $modified, bool $favorite - ) : DataResponse { + ) : JSONResponse { return $this->helper->handleErrorResponse(function () use ( $id, $title, @@ -165,7 +165,7 @@ public function undo( /** * @NoAdminRequired */ - public function update(int $id, string $content, bool $autotitle) : DataResponse { + public function update(int $id, string $content, bool $autotitle) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($id, $content, $autotitle) { $note = $this->notesService->get($this->userId, $id); $note->setContent($content); @@ -188,7 +188,7 @@ public function updateProperty( ?string $title = null, ?string $category = null, ?bool $favorite = null - ) : DataResponse { + ) : JSONResponse { return $this->helper->handleErrorResponse(function () use ( $id, $property, @@ -229,7 +229,7 @@ public function updateProperty( break; default: - return new DataResponse([], Http::STATUS_BAD_REQUEST); + return new JSONResponse([], Http::STATUS_BAD_REQUEST); } return $result; }); @@ -239,7 +239,7 @@ public function updateProperty( /** * @NoAdminRequired */ - public function destroy(int $id) : DataResponse { + public function destroy(int $id) : JSONResponse { return $this->helper->handleErrorResponse(function () use ($id) { $this->notesService->delete($this->userId, $id); return [];