From 0a90ea80b824428d3700edcfa3e275653ac568f8 Mon Sep 17 00:00:00 2001 From: Benedikt Ziemons Date: Sat, 18 Sep 2021 19:28:10 +0200 Subject: [PATCH] Update database tables for matrix data backend and move API to /api/v1/ticket --- appinfo/routes.php | 13 +-- composer.json | 3 +- composer.lock | 6 +- lib/Controller/Errors.php | 4 +- lib/Controller/TicketController.php | 77 ------------------ lib/Db/MatrixTicket.php | 27 +++++++ lib/Db/MatrixUser.php | 19 +++++ lib/Db/Ticket.php | 27 ------- lib/Db/TicketMapper.php | 36 ++++++--- lib/Db/UserMapper.php | 31 +++++++ .../Version000000Date20181013124731.php | 48 ----------- .../Version000000Date20210918151800.php | 80 +++++++++++++++++++ lib/Service/MatrixService.php | 5 +- lib/Service/TicketService.php | 13 +-- ...st.php => MatrixTicketIntegrationTest.php} | 15 ++-- .../Integration/MatrixUserIntegrationTest.php | 49 ++++++++++++ .../Unit/Controller/NoteApiControllerTest.php | 50 +++++++++++- tests/Unit/Controller/NoteControllerTest.php | 54 ------------- tests/Unit/Controller/PageControllerTest.php | 4 +- tests/Unit/Service/NoteServiceTest.php | 18 ++--- tests/bootstrap.php | 2 +- 21 files changed, 320 insertions(+), 261 deletions(-) delete mode 100644 lib/Controller/TicketController.php create mode 100644 lib/Db/MatrixTicket.php create mode 100644 lib/Db/MatrixUser.php delete mode 100644 lib/Db/Ticket.php create mode 100644 lib/Db/UserMapper.php delete mode 100644 lib/Migration/Version000000Date20181013124731.php create mode 100644 lib/Migration/Version000000Date20210918151800.php rename tests/Integration/{NoteIntegrationTest.php => MatrixTicketIntegrationTest.php} (82%) create mode 100644 tests/Integration/MatrixUserIntegrationTest.php delete mode 100644 tests/Unit/Controller/NoteControllerTest.php diff --git a/appinfo/routes.php b/appinfo/routes.php index 8fadb63..7c3e44c 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -2,12 +2,15 @@ return [ 'resources' => [ - 'note' => ['url' => '/notes'], - 'note_api' => ['url' => '/api/0.1/notes'] + 'ticket_api' => ['url' => '/api/v1/ticket'] ], 'routes' => [ ['name' => 'page#index', 'url' => '/', 'verb' => 'GET'], - ['name' => 'note_api#preflighted_cors', 'url' => '/api/0.1/{path}', - 'verb' => 'OPTIONS', 'requirements' => ['path' => '.+']] - ] + [ + 'name' => 'ticket_api#preflighted_cors', + 'url' => '/api/v1/{path}', + 'verb' => 'OPTIONS', + 'requirements' => ['path' => '.+'], + ], + ], ]; diff --git a/composer.json b/composer.json index d7ce82e..6ba5149 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,8 @@ } ], "require": { - "aryess/php-matrix-sdk": "dev-feature/guzzle7-update" + "aryess/php-matrix-sdk": "dev-feature/guzzle7-update", + "ext-json": "*" }, "require-dev": { "phpunit/phpunit": "^8.5", diff --git a/composer.lock b/composer.lock index 55e2480..9eb05ab 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "50de7fbcac58915f4bc01c0cf85e68ae", + "content-hash": "4f8aeff7f36c2b13932751ca0c8eb5cd", "packages": [ { "name": "aryess/php-matrix-sdk", @@ -4344,7 +4344,9 @@ }, "prefer-stable": false, "prefer-lowest": false, - "platform": [], + "platform": { + "ext-json": "*" + }, "platform-dev": [], "platform-overrides": { "php": "7.2.5" diff --git a/lib/Controller/Errors.php b/lib/Controller/Errors.php index d1f8aff..d118e8c 100644 --- a/lib/Controller/Errors.php +++ b/lib/Controller/Errors.php @@ -3,12 +3,10 @@ namespace OCA\UPschooling\Controller; use Closure; - +use OCA\UPschooling\Service\NoteNotFound; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; -use OCA\UPschooling\Service\NoteNotFound; - trait Errors { protected function handleNotFound(Closure $callback): DataResponse { try { diff --git a/lib/Controller/TicketController.php b/lib/Controller/TicketController.php deleted file mode 100644 index 96b391e..0000000 --- a/lib/Controller/TicketController.php +++ /dev/null @@ -1,77 +0,0 @@ -service = $service; - $this->matrix = $matrix; - $this->userId = $userId; - } - - /** - * @NoAdminRequired - */ - public function index(): DataResponse { - return new DataResponse($this->service->findAll($this->userId)); - } - - /** - * @NoAdminRequired - */ - public function show(int $id): DataResponse { - return $this->handleNotFound(function () use ($id) { - return $this->service->find($id, $this->userId); - }); - } - - /** - * @NoAdminRequired - */ - public function create(string $title, string $content): DataResponse { - return new DataResponse($this->service->create($title, $content, - $this->userId)); - } - - /** - * @NoAdminRequired - */ - public function update(int $id, string $title, - string $content): DataResponse { - return $this->handleNotFound(function () use ($id, $title, $content) { - return $this->service->update($id, $title, $content, $this->userId); - }); - } - - /** - * @NoAdminRequired - */ - public function destroy(int $id): DataResponse { - return $this->handleNotFound(function () use ($id) { - return $this->service->delete($id, $this->userId); - }); - } -} diff --git a/lib/Db/MatrixTicket.php b/lib/Db/MatrixTicket.php new file mode 100644 index 0000000..d4869f7 --- /dev/null +++ b/lib/Db/MatrixTicket.php @@ -0,0 +1,27 @@ + $this->id, + 'matrixRoom' => $this->matrixRoom, + 'matrixAssistedUser' => $this->matrixAssistedUser, + 'matrixHelperUser' => $this->matrixHelperUser, + 'status' => $this->status, + 'version' => $this->version, + ]; + } +} diff --git a/lib/Db/MatrixUser.php b/lib/Db/MatrixUser.php new file mode 100644 index 0000000..5af1881 --- /dev/null +++ b/lib/Db/MatrixUser.php @@ -0,0 +1,19 @@ + $this->id, + 'userId' => $this->userId, + 'matrixUser' => $this->matrixUser, + ]; + } +} diff --git a/lib/Db/Ticket.php b/lib/Db/Ticket.php deleted file mode 100644 index 04c0943..0000000 --- a/lib/Db/Ticket.php +++ /dev/null @@ -1,27 +0,0 @@ - $this->id, - 'title' => $this->title, - 'description' => $this->description, - 'helperId' => $this->helperId, - 'dueDate' => $this->dueDate, - 'status' => $this->status, - ]; - } -} diff --git a/lib/Db/TicketMapper.php b/lib/Db/TicketMapper.php index 1f985b2..d55d96f 100644 --- a/lib/Db/TicketMapper.php +++ b/lib/Db/TicketMapper.php @@ -8,25 +8,34 @@ use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; -class TicketMapper extends QBMapper { - public function __construct(IDBConnection $db) { - parent::__construct($db, 'upschooling', Ticket::class); +class TicketMapper extends QBMapper +{ + public function __construct(IDBConnection $db) + { + parent::__construct($db, 'upschooling_tickets', MatrixTicket::class); } /** * @param int $id * @param string $userId - * @return Entity|Ticket + * @return Entity|MatrixTicket * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException * @throws DoesNotExistException */ - public function find(int $id, string $userId): Ticket { + public function find(int $id, string $userId): MatrixTicket + { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select('*') - ->from('upschooling') + ->from('upschooling_tickets', 't') ->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))); + ->innerJoin( + 't', + 'upschooling_users', + 'u', + $qb->expr()->in('u.matrix_user', ['t.matrix_assisted_user', 't.matrix_helper_user']) + ) + ->andWhere($qb->expr()->eq('u.user_id', $qb->createNamedParameter($userId))); return $this->findEntity($qb); } @@ -34,12 +43,19 @@ class TicketMapper extends QBMapper { * @param string $userId * @return array */ - public function findAll(string $userId): array { + public function findAll(string $userId): array + { /* @var $qb IQueryBuilder */ $qb = $this->db->getQueryBuilder(); $qb->select('*') - ->from('upschooling') - ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))); + ->from('upschooling_users', 'u') + ->where($qb->expr()->eq('u.user_id', $qb->createNamedParameter($userId))) + ->innerJoin( + 'u', + 'upschooling_tickets', + 't', + $qb->expr()->in('u.matrix_user', ['t.matrix_assisted_user', 't.matrix_helper_user']) + ); return $this->findEntities($qb); } } diff --git a/lib/Db/UserMapper.php b/lib/Db/UserMapper.php new file mode 100644 index 0000000..03e2816 --- /dev/null +++ b/lib/Db/UserMapper.php @@ -0,0 +1,31 @@ +db->getQueryBuilder(); + $qb->select('*') + ->from('upschooling_users') + ->where($qb->expr()->eq('user_id', $qb->createNamedParameter($userId))); + return $this->findEntity($qb); + } +} diff --git a/lib/Migration/Version000000Date20181013124731.php b/lib/Migration/Version000000Date20181013124731.php deleted file mode 100644 index bf78eb7..0000000 --- a/lib/Migration/Version000000Date20181013124731.php +++ /dev/null @@ -1,48 +0,0 @@ -hasTable('upschooling')) { - $table = $schema->createTable('upschooling'); - $table->addColumn('id', 'integer', [ - 'autoincrement' => true, - 'notnull' => true, - ]); - $table->addColumn('title', 'string', [ - 'notnull' => true, - 'length' => 200 - ]); - $table->addColumn('user_id', 'string', [ - 'notnull' => true, - 'length' => 200, - ]); - $table->addColumn('content', 'text', [ - 'notnull' => true, - 'default' => '' - ]); - - $table->setPrimaryKey(['id']); - $table->addIndex(['user_id'], 'upschooling_user_id_index'); - } - return $schema; - } -} diff --git a/lib/Migration/Version000000Date20210918151800.php b/lib/Migration/Version000000Date20210918151800.php new file mode 100644 index 0000000..4270fd9 --- /dev/null +++ b/lib/Migration/Version000000Date20210918151800.php @@ -0,0 +1,80 @@ +hasTable('upschooling_tickets')) { + $table = $schema->createTable('upschooling_tickets'); + $table->addColumn('id', 'integer', [ + 'autoincrement' => true, + 'notnull' => true, + ]); + $table->addColumn('matrix_room', 'string', [ + 'notnull' => true, + 'length' => 63, + ]); + $table->addColumn('matrix_assisted_user', 'string', [ + 'notnull' => true, + 'length' => 63, + ]); + $table->addColumn('matrix_helper_user', 'string', [ + 'notnull' => false, + 'length' => 63, + ]); + // could be optimized by having a "localUser" with foreign key to users.id, + // but that would create consistency issues + + $table->addColumn('status', 'string', [ + 'notnull' => true, + 'length' => 100, + ]); + $table->addColumn('version', 'integer', [ + 'notnull' => true, + ]); + + $table->setPrimaryKey(['id']); + $table->addUniqueConstraint(['matrix_room'], 'upschooling_mx_room_id_uniq'); + $table->addIndex(['matrix_room'], 'upschooling_mx_room_id_idx'); + } + + if (!$schema->hasTable('upschooling_users')) { + $table = $schema->createTable('upschooling_users'); + $table->addColumn('id', 'integer', [ + 'autoincrement' => true, + 'notnull' => true, + ]); + $table->addColumn('user_id', 'string', [ + 'notnull' => true, + 'length' => 100, + ]); + $table->addColumn('matrix_user', 'string', [ + 'notnull' => true, + 'length' => 63, + ]); + + $table->setPrimaryKey(['id']); + $table->addUniqueConstraint(['user_id'], 'upschooling_mx_user_nc_uniq'); + $table->addUniqueConstraint(['matrix_user'], 'upschooling_mx_user_mx_uniq'); + $table->addForeignKeyConstraint('users', ['user_id'], ['id'], [], 'upschooling_mx_user_nc_fk'); + } + return $schema; + } +} diff --git a/lib/Service/MatrixService.php b/lib/Service/MatrixService.php index 66aea08..795daa0 100644 --- a/lib/Service/MatrixService.php +++ b/lib/Service/MatrixService.php @@ -2,12 +2,11 @@ namespace OCA\UPschooling\Service; -use Aryess\PhpMatrixSdk\Exceptions\MatrixRequestException; -use \Psr\Log\LoggerInterface; - use Aryess\PhpMatrixSdk\Exceptions\MatrixException; +use Aryess\PhpMatrixSdk\Exceptions\MatrixRequestException; use Aryess\PhpMatrixSdk\MatrixClient; use Aryess\PhpMatrixSdk\Room; +use Psr\Log\LoggerInterface; class MatrixService { diff --git a/lib/Service/TicketService.php b/lib/Service/TicketService.php index b2966f3..14751f5 100644 --- a/lib/Service/TicketService.php +++ b/lib/Service/TicketService.php @@ -3,19 +3,20 @@ namespace OCA\UPschooling\Service; use Exception; - +use OCA\UPschooling\Db\MatrixTicket; +use OCA\UPschooling\Db\TicketMapper; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use OCA\UPschooling\Db\Ticket; -use OCA\UPschooling\Db\TicketMapper; - class TicketService { /** @var TicketMapper */ private $mapper; - public function __construct(TicketMapper $mapper) { + private $matrix; + + public function __construct(MatrixService $matrix, TicketMapper $mapper) { + $this->matrix = $matrix; $this->mapper = $mapper; } @@ -46,7 +47,7 @@ class TicketService { } public function create($title, $content, $userId) { - $note = new Ticket(); + $note = new MatrixTicket(); $note->setTitle($title); $note->setContent($content); $note->setUserId($userId); diff --git a/tests/Integration/NoteIntegrationTest.php b/tests/Integration/MatrixTicketIntegrationTest.php similarity index 82% rename from tests/Integration/NoteIntegrationTest.php rename to tests/Integration/MatrixTicketIntegrationTest.php index 1a73d07..90a19ea 100644 --- a/tests/Integration/NoteIntegrationTest.php +++ b/tests/Integration/MatrixTicketIntegrationTest.php @@ -2,16 +2,15 @@ namespace OCA\UPschooling\Tests\Integration\Controller; +use OCA\UPschooling\Controller\TicketApiController; +use OCA\UPschooling\Db\MatrixTicket; +use OCA\UPschooling\Db\TicketMapper; use OCP\AppFramework\App; use OCP\IRequest; use PHPUnit\Framework\TestCase; -use OCA\UPschooling\Db\Ticket; -use OCA\UPschooling\Db\TicketMapper; -use OCA\UPschooling\Controller\TicketController; - -class NoteIntegrationTest extends TestCase { +class MatrixTicketIntegrationTest extends TestCase { private $controller; private $mapper; private $userId = 'john'; @@ -30,13 +29,13 @@ class NoteIntegrationTest extends TestCase { return $this->createMock(IRequest::class); }); - $this->controller = $container->query(TicketController::class); + $this->controller = $container->query(TicketApiController::class); $this->mapper = $container->query(TicketMapper::class); } public function testUpdate() { // create a new note that should be updated - $note = new Ticket(); + $note = new MatrixTicket(); $note->setTitle('old_title'); $note->setContent('old_content'); $note->setUserId($this->userId); @@ -44,7 +43,7 @@ class NoteIntegrationTest extends TestCase { $id = $this->mapper->insert($note)->getId(); // fromRow does not set the fields as updated - $updatedNote = Ticket::fromRow([ + $updatedNote = MatrixTicket::fromRow([ 'id' => $id, 'user_id' => $this->userId ]); diff --git a/tests/Integration/MatrixUserIntegrationTest.php b/tests/Integration/MatrixUserIntegrationTest.php new file mode 100644 index 0000000..e58e71b --- /dev/null +++ b/tests/Integration/MatrixUserIntegrationTest.php @@ -0,0 +1,49 @@ +getContainer(); + + // only replace the user id + $container->registerService('userId', function () { + return $this->userId; + }); + + // we do not care about the request but the controller needs it + $container->registerService(IRequest::class, function () { + return $this->createMock(IRequest::class); + }); + + $this->mapper = $container->query(UserMapper::class); + } + + public function testUpdate() { + // create a new user + $user = new MatrixUser(); + $user->setMatrixUser($this->matrixUserId); + $user->setUserId($this->userId); + + $this->mapper->insert($user); + + // test that user is in database + $result = $this->mapper->find($this->userId); + $this->assertEquals($user, $result->getData()); + + // clean up + $this->mapper->delete($result->getData()); + } +} diff --git a/tests/Unit/Controller/NoteApiControllerTest.php b/tests/Unit/Controller/NoteApiControllerTest.php index 83318ce..3378322 100644 --- a/tests/Unit/Controller/NoteApiControllerTest.php +++ b/tests/Unit/Controller/NoteApiControllerTest.php @@ -3,10 +3,54 @@ namespace OCA\UPschooling\Tests\Unit\Controller; use OCA\UPschooling\Controller\TicketApiController; +use OCA\UPschooling\Service\NoteNotFound; +use OCA\UPschooling\Service\TicketService; +use OCP\AppFramework\Http; +use OCP\IRequest; +use PHPUnit\Framework\TestCase; -class NoteApiControllerTest extends NoteControllerTest { - public function setUp(): void { - parent::setUp(); +class NoteApiControllerTest extends TestCase +{ + protected $controller; + protected $service; + protected $userId = 'john'; + protected $request; + + public function setUp(): void + { + $this->request = $this->getMockBuilder(IRequest::class)->getMock(); + $this->service = $this->getMockBuilder(TicketService::class) + ->disableOriginalConstructor() + ->getMock(); $this->controller = new TicketApiController($this->request, $this->service, $this->userId); } + + public function testUpdate() + { + $note = 'just check if this value is returned correctly'; + $this->service->expects($this->once()) + ->method('update') + ->with($this->equalTo(3), + $this->equalTo('title'), + $this->equalTo('content'), + $this->equalTo($this->userId)) + ->will($this->returnValue($note)); + + $result = $this->controller->update(3, 'title', 'content'); + + $this->assertEquals($note, $result->getData()); + } + + + public function testUpdateNotFound() + { + // test the correct status code if no note is found + $this->service->expects($this->once()) + ->method('update') + ->will($this->throwException(new NoteNotFound())); + + $result = $this->controller->update(3, 'title', 'content'); + + $this->assertEquals(Http::STATUS_NOT_FOUND, $result->getStatus()); + } } diff --git a/tests/Unit/Controller/NoteControllerTest.php b/tests/Unit/Controller/NoteControllerTest.php deleted file mode 100644 index bf4950a..0000000 --- a/tests/Unit/Controller/NoteControllerTest.php +++ /dev/null @@ -1,54 +0,0 @@ -request = $this->getMockBuilder(IRequest::class)->getMock(); - $this->service = $this->getMockBuilder(TicketService::class) - ->disableOriginalConstructor() - ->getMock(); - $this->controller = new TicketController($this->request, $this->service, $this->userId); - } - - public function testUpdate() { - $note = 'just check if this value is returned correctly'; - $this->service->expects($this->once()) - ->method('update') - ->with($this->equalTo(3), - $this->equalTo('title'), - $this->equalTo('content'), - $this->equalTo($this->userId)) - ->will($this->returnValue($note)); - - $result = $this->controller->update(3, 'title', 'content'); - - $this->assertEquals($note, $result->getData()); - } - - - public function testUpdateNotFound() { - // test the correct status code if no note is found - $this->service->expects($this->once()) - ->method('update') - ->will($this->throwException(new NoteNotFound())); - - $result = $this->controller->update(3, 'title', 'content'); - - $this->assertEquals(Http::STATUS_NOT_FOUND, $result->getStatus()); - } -} diff --git a/tests/Unit/Controller/PageControllerTest.php b/tests/Unit/Controller/PageControllerTest.php index 258fada..c8615cb 100644 --- a/tests/Unit/Controller/PageControllerTest.php +++ b/tests/Unit/Controller/PageControllerTest.php @@ -2,9 +2,8 @@ namespace OCA\UPschooling\Controller; -use PHPUnit\Framework\TestCase; - use OCP\AppFramework\Http\TemplateResponse; +use PHPUnit\Framework\TestCase; class PageControllerTest extends TestCase { private $controller; @@ -14,7 +13,6 @@ class PageControllerTest extends TestCase { $this->controller = new PageController($request); } - public function testIndex() { $result = $this->controller->index(); diff --git a/tests/Unit/Service/NoteServiceTest.php b/tests/Unit/Service/NoteServiceTest.php index 51af8c5..342ceff 100644 --- a/tests/Unit/Service/NoteServiceTest.php +++ b/tests/Unit/Service/NoteServiceTest.php @@ -1,15 +1,13 @@ 3, 'title' => 'yo', 'content' => 'nope' @@ -36,7 +34,7 @@ class NoteServiceTest extends TestCase { ->will($this->returnValue($note)); // the note when updated - $updatedNote = Ticket::fromRow(['id' => 3]); + $updatedNote = MatrixTicket::fromRow(['id' => 3]); $updatedNote->setTitle('title'); $updatedNote->setContent('content'); $this->mapper->expects($this->once()) diff --git a/tests/bootstrap.php b/tests/bootstrap.php index c0ceffa..03184e3 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,3 +1,3 @@