Fixed system tags DAV and API and docs

diff --git a/apps/dav/lib/systemtag/systemtagnode.php b/apps/dav/lib/systemtag/systemtagnode.php
index f7228108b3d3e4f0a0b9acbdc5349b741251ed6f..7ab4a8a14f442e3cbcda2cb2d412de61323e254e 100644 (file)
--- a/apps/dav/lib/systemtag/systemtagnode.php
+++ b/apps/dav/lib/systemtag/systemtagnode.php
@@ -30,6 +30,9 @@ use OCP\SystemTag\ISystemTagManager;
 use OCP\SystemTag\TagNotFoundException;
 use OCP\SystemTag\TagAlreadyExistsException;
 
+/**
+ * DAV node representing a system tag, with the name being the tag id.
+ */
 class SystemTagNode implements \Sabre\DAV\INode {
 
        /**

diff --git a/apps/dav/lib/systemtag/systemtagplugin.php b/apps/dav/lib/systemtag/systemtagplugin.php
index 692b7e970169bd9c5c68a368acacdd3eed12871f..51db0632549cfc78d57331b21b9b557e8960b5e8 100644 (file)
--- a/apps/dav/lib/systemtag/systemtagplugin.php
+++ b/apps/dav/lib/systemtag/systemtagplugin.php
@@ -33,6 +33,13 @@ use OCP\SystemTag\TagAlreadyExistsException;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
 
+/**
+ * Sabre plugin to handle system tags:
+ *
+ * - makes it possible to create new tags with POST operation
+ * - get/set Webdav properties for tags
+ *
+ */
 class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
 
        // namespace
@@ -86,7 +93,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
        }
 
        /**
-        * We intercept this to handle POST requests on calendars.
+        * POST operation on system tag collections
         *
         * @param RequestInterface $request request object
         * @param ResponseInterface $response response object
@@ -130,18 +137,17 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
        /**
         * Creates a new tag
         *
-        * @param string $data
+        * @param string $data JSON encoded string containing the properties of the tag to create
         * @param string $contentType content type of the data
         * @return ISystemTag newly created system tag
         *
         * @throws BadRequest if a field was missing
-        * @throws Conflict
+        * @throws Conflict if a tag with the same properties already exists
         * @throws UnsupportedMediaType if the content type is not supported
         */
        private function createTag($data, $contentType = 'application/json') {
                if ($contentType === 'application/json') {
                        $data = json_decode($data, true);
-                       // TODO: application/x-www-form-urlencoded ?
                } else {
                        throw new UnsupportedMediaType();
                }
@@ -164,7 +170,7 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin {
                try {
                        return $this->tagManager->createTag($tagName, $userVisible, $userAssignable);
                } catch (TagAlreadyExistsException $e) {
-                       throw new Conflict('Tag already exists');
+                       throw new Conflict('Tag already exists', 0, $e);
                }
        }
 

diff --git a/apps/dav/lib/systemtag/systemtagsbyidcollection.php b/apps/dav/lib/systemtag/systemtagsbyidcollection.php
index 0164b9b0b3d269dc07c8c7ebe4c07602452fbd57..e7b7b6d0acc387a7bc3a5c8f272b7ab288c5782b 100644 (file)
--- a/apps/dav/lib/systemtag/systemtagsbyidcollection.php
+++ b/apps/dav/lib/systemtag/systemtagsbyidcollection.php
@@ -56,7 +56,7 @@ class SystemTagsByIdCollection implements ICollection {
 
        function getChild($name) {
                try {
-                       $tags = $this->tagManager->getTagsById($name);
+                       $tags = $this->tagManager->getTagsByIds([$name]);
                        return $this->makeNode(current($tags));
                } catch (\InvalidArgumentException $e) {
                        throw new BadRequest('Invalid tag id', 0, $e);
@@ -66,7 +66,6 @@ class SystemTagsByIdCollection implements ICollection {
        }
 
        function getChildren() {
-               // TODO: set visibility filter based on principal/permissions ?
                $tags = $this->tagManager->getAllTags(true);
                return array_map(function($tag) {
                        return $this->makeNode($tag);
@@ -75,7 +74,7 @@ class SystemTagsByIdCollection implements ICollection {
 
        function childExists($name) {
                try {
-                       $this->tagManager->getTagsById($name);
+                       $this->tagManager->getTagsByIds([$name]);
                        return true;
                } catch (\InvalidArgumentException $e) {
                        throw new BadRequest('Invalid tag id', 0, $e);

diff --git a/apps/dav/lib/systemtag/systemtagsobjectmappingcollection.php b/apps/dav/lib/systemtag/systemtagsobjectmappingcollection.php
index e81994e0bd63b83487f1781530a2c07e2a16c2dc..89e8620614b88a98a6c924c5a6e5e1d79aa5c701 100644 (file)
--- a/apps/dav/lib/systemtag/systemtagsobjectmappingcollection.php
+++ b/apps/dav/lib/systemtag/systemtagsobjectmappingcollection.php
@@ -86,8 +86,8 @@ class SystemTagsObjectMappingCollection implements ICollection {
 
        function getChild($tagId) {
                try {
-                       if ($this->tagMapper->haveTag($this->objectId, $this->objectType, $tagId, true)) {
-                               $tag = $this->tagManager->getTagsById($tagId);
+                       if ($this->tagMapper->haveTag([$this->objectId], $this->objectType, $tagId, true)) {
+                               $tag = $this->tagManager->getTagsByIds([$tagId]);
                                return $this->makeNode(current($tag));
                        }
                        throw new NotFound('Tag with id ' . $tagId . ' not present for object ' . $this->objectId);
@@ -99,11 +99,11 @@ class SystemTagsObjectMappingCollection implements ICollection {
        }
 
        function getChildren() {
-               $tagIds = current($this->tagMapper->getTagIdsForObjects($this->objectId, $this->objectType));
+               $tagIds = current($this->tagMapper->getTagIdsForObjects([$this->objectId], $this->objectType));
                if (empty($tagIds)) {
                        return [];
                }
-               $tags = $this->tagManager->getTagsById($tagIds);
+               $tags = $this->tagManager->getTagsByIds($tagIds);
                return array_values(array_map(function($tag) {
                        return $this->makeNode($tag);
                }, $tags));
@@ -111,7 +111,7 @@ class SystemTagsObjectMappingCollection implements ICollection {
 
        function childExists($tagId) {
                try {
-                       return ($this->tagMapper->haveTag($this->objectId, $this->objectType, $tagId, true));
+                       return ($this->tagMapper->haveTag([$this->objectId], $this->objectType, $tagId, true));
                } catch (\InvalidArgumentException $e) {
                        throw new BadRequest('Invalid tag id', 0, $e);
                } catch (TagNotFoundException $e) {
@@ -141,7 +141,8 @@ class SystemTagsObjectMappingCollection implements ICollection {
        }
 
        /**
-        * Create a sabre node for the given system tag
+        * Create a sabre node for the mapping of the 
+        * given system tag to the collection's object
         *
         * @param ISystemTag $tag
         *

diff --git a/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php b/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php
index 8dee85ccd449aebc88cfcacd16f3127f1c751402..e544073613f3b099d75890a313f941ba1b9ca6d5 100644 (file)
--- a/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php
+++ b/apps/dav/lib/systemtag/systemtagsobjecttypecollection.php
@@ -62,7 +62,7 @@ class SystemTagsObjectTypeCollection implements ICollection {
        }
 
        function createFile($name, $data = null) {
-               throw new Forbidden('Permission denied to create collections');
+               throw new Forbidden('Permission denied to create nodes');
        }
 
        function createDirectory($name) {

diff --git a/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php b/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php
index fdaaf2cd009cb5135a61a450f35c2482c6f4bc86..104ce366034046f642c2bf01e701953a284d8106 100644 (file)
--- a/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php
+++ b/apps/dav/tests/unit/systemtag/systemtagsbyidcollection.php
@@ -51,8 +51,8 @@ class SystemTagsByIdCollection extends \Test\TestCase {
                $tag = new SystemTag(123, 'Test', true, false);
 
                $this->tagManager->expects($this->once())
-                       ->method('getTagsById')
-                       ->with('123')
+                       ->method('getTagsByIds')
+                       ->with(['123'])
                        ->will($this->returnValue([$tag]));
 
                $childNode = $this->node->getChild('123');
@@ -67,8 +67,8 @@ class SystemTagsByIdCollection extends \Test\TestCase {
         */
        public function testGetChildInvalidName() {
                $this->tagManager->expects($this->once())
-                       ->method('getTagsById')
-                       ->with('invalid')
+                       ->method('getTagsByIds')
+                       ->with(['invalid'])
                        ->will($this->throwException(new \InvalidArgumentException()));
 
                $this->node->getChild('invalid');
@@ -79,8 +79,8 @@ class SystemTagsByIdCollection extends \Test\TestCase {
         */
        public function testGetChildNotFound() {
                $this->tagManager->expects($this->once())
-                       ->method('getTagsById')
-                       ->with('444')
+                       ->method('getTagsByIds')
+                       ->with(['444'])
                        ->will($this->throwException(new TagNotFoundException()));
 
                $this->node->getChild('444');
@@ -117,8 +117,8 @@ class SystemTagsByIdCollection extends \Test\TestCase {
                $tag = new SystemTag(123, 'One', true, false);
 
                $this->tagManager->expects($this->once())
-                       ->method('getTagsById')
-                       ->with('123')
+                       ->method('getTagsByIds')
+                       ->with(['123'])
                        ->will($this->returnValue([$tag]));
 
                $this->assertTrue($this->node->childExists('123'));
@@ -126,8 +126,8 @@ class SystemTagsByIdCollection extends \Test\TestCase {
 
        public function testChildExistsNotFound() {
                $this->tagManager->expects($this->once())
-                       ->method('getTagsById')
-                       ->with('123')
+                       ->method('getTagsByIds')
+                       ->with(['123'])
                        ->will($this->throwException(new TagNotFoundException()));
 
                $this->assertFalse($this->node->childExists('123'));
@@ -138,8 +138,8 @@ class SystemTagsByIdCollection extends \Test\TestCase {
         */
        public function testChildExistsBadRequest() {
                $this->tagManager->expects($this->once())
-                       ->method('getTagsById')
-                       ->with('invalid')
+                       ->method('getTagsByIds')
+                       ->with(['invalid'])
                        ->will($this->throwException(new \InvalidArgumentException()));
 
                $this->node->childExists('invalid');

diff --git a/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php b/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php
index 1a9ffa6f4aed16e6db2c18dc478681a51742f47e..6e15bb78e7c02bd513639ac3a74311afc74dfbcc 100644 (file)
--- a/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php
+++ b/apps/dav/tests/unit/systemtag/systemtagsobjectmappingcollection.php
@@ -76,13 +76,13 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 
                $this->tagMapper->expects($this->once())
                        ->method('haveTag')
-                       ->with(111, 'files', '555', true)
+                       ->with([111], 'files', '555', true)
                        ->will($this->returnValue(true));
 
                $this->tagManager->expects($this->once())
-                       ->method('getTagsById')
-                       ->with('555')
-                       ->will($this->returnValue([$tag]));
+                       ->method('getTagsByIds')
+                       ->with(['555'])
+                       ->will($this->returnValue(['555' => $tag]));
 
                $childNode = $this->node->getChild('555');
 
@@ -96,7 +96,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
        public function testGetChildRelationNotFound() {
                $this->tagMapper->expects($this->once())
                        ->method('haveTag')
-                       ->with(111, 'files', '777')
+                       ->with([111], 'files', '777')
                        ->will($this->returnValue(false));
 
                $this->node->getChild('777');
@@ -108,7 +108,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
        public function testGetChildInvalidId() {
                $this->tagMapper->expects($this->once())
                        ->method('haveTag')
-                       ->with(111, 'files', 'badid')
+                       ->with([111], 'files', 'badid')
                        ->will($this->throwException(new \InvalidArgumentException()));
 
                $this->node->getChild('badid');
@@ -120,7 +120,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
        public function testGetChildTagDoesNotExist() {
                $this->tagMapper->expects($this->once())
                        ->method('haveTag')
-                       ->with(111, 'files', '777')
+                       ->with([111], 'files', '777')
                        ->will($this->throwException(new TagNotFoundException()));
 
                $this->node->getChild('777');
@@ -132,11 +132,11 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
 
                $this->tagMapper->expects($this->once())
                        ->method('getTagIdsForObjects')
-                       ->with(111, 'files')
+                       ->with([111], 'files')
                        ->will($this->returnValue(['111' => ['555', '556']]));
 
                $this->tagManager->expects($this->once())
-                       ->method('getTagsById')
+                       ->method('getTagsByIds')
                        ->with(['555', '556'])
                        ->will($this->returnValue(['555' => $tag1, '666' => $tag2]));
 
@@ -159,7 +159,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
        public function testChildExists() {
                $this->tagMapper->expects($this->once())
                        ->method('haveTag')
-                       ->with(111, 'files', '555')
+                       ->with([111], 'files', '555')
                        ->will($this->returnValue(true));
 
                $this->assertTrue($this->node->childExists('555'));
@@ -168,7 +168,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
        public function testChildExistsNotFound() {
                $this->tagMapper->expects($this->once())
                        ->method('haveTag')
-                       ->with(111, 'files', '555')
+                       ->with([111], 'files', '555')
                        ->will($this->returnValue(false));
 
                $this->assertFalse($this->node->childExists('555'));
@@ -177,7 +177,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
        public function testChildExistsTagNotFound() {
                $this->tagMapper->expects($this->once())
                        ->method('haveTag')
-                       ->with(111, 'files', '555')
+                       ->with([111], 'files', '555')
                        ->will($this->throwException(new TagNotFoundException()));
 
                $this->assertFalse($this->node->childExists('555'));
@@ -189,7 +189,7 @@ class SystemTagsObjectMappingCollection extends \Test\TestCase {
        public function testChildExistsInvalidId() {
                $this->tagMapper->expects($this->once())
                        ->method('haveTag')
-                       ->with(111, 'files', '555')
+                       ->with([111], 'files', '555')
                        ->will($this->throwException(new \InvalidArgumentException()));
 
                $this->node->childExists('555');

diff --git a/lib/private/systemtag/systemtagmanager.php b/lib/private/systemtag/systemtagmanager.php
index 8caf10d69da0fca001b9fd887b7d1d05753dfca7..7f239dc84cf5a20e905f870f56c152ac2bfb3e88 100644 (file)
--- a/lib/private/systemtag/systemtagmanager.php
+++ b/lib/private/systemtag/systemtagmanager.php
@@ -63,7 +63,7 @@ class SystemTagManager implements ISystemTagManager {
        /**
         * {@inheritdoc}
         */
-       public function getTagsById($tagIds) {
+       public function getTagsByIds($tagIds) {
                if (!is_array($tagIds)) {
                        $tagIds = [$tagIds];
                }
@@ -242,7 +242,7 @@ class SystemTagManager implements ISystemTagManager {
 
                $tagNotFoundException = null;
                try {
-                       $this->getTagsById($tagIds);
+                       $this->getTagsByIds($tagIds);
                } catch (TagNotFoundException $e) {
                        $tagNotFoundException = $e;
                }

diff --git a/lib/private/systemtag/systemtagobjectmapper.php b/lib/private/systemtag/systemtagobjectmapper.php
index bb64a35456f6be9523664417f30b3e3c24f524b7..988fa66d77eb0b83cc36d97545c8d38e4ef421c5 100644 (file)
--- a/lib/private/systemtag/systemtagobjectmapper.php
+++ b/lib/private/systemtag/systemtagobjectmapper.php
@@ -213,7 +213,7 @@ class SystemTagObjectMapper implements ISystemTagObjectMapper {
         * @throws \OCP\SystemTag\TagNotFoundException if at least one tag did not exist
         */
        private function assertTagsExist($tagIds) {
-               $tags = $this->tagManager->getTagsById($tagIds);
+               $tags = $this->tagManager->getTagsByIds($tagIds);
                if (count($tags) !== count($tagIds)) {
                        // at least one tag missing, bail out
                        $foundTagIds = array_map(

diff --git a/lib/public/systemtag/isystemtagmanager.php b/lib/public/systemtag/isystemtagmanager.php
index 4e3b263e56c97a5c644598276accd7859f440925..6e8fed36dce52b0378add426ebff5c2ceb5c18ea 100644 (file)
--- a/lib/public/systemtag/isystemtagmanager.php
+++ b/lib/public/systemtag/isystemtagmanager.php
@@ -41,7 +41,7 @@ interface ISystemTagManager {
         *
         * @since 9.0.0
         */
-       public function getTagsById($tagIds);
+       public function getTagsByIds($tagIds);
 
        /**
         * Returns the tag object matching the given attributes.

diff --git a/tests/lib/systemtag/systemtagmanagertest.php b/tests/lib/systemtag/systemtagmanagertest.php
index 8498b85519f4c8fb2fd552c0e561b1000484e2f9..97c072f33f634df318373c2cf9ea95de59aa6f31 100644 (file)
--- a/tests/lib/systemtag/systemtagmanagertest.php
+++ b/tests/lib/systemtag/systemtagmanagertest.php
@@ -250,7 +250,7 @@ class SystemTagManagerTest extends TestCase {
                $tag1 = $this->tagManager->createTag('one', true, false);
                $tag2 = $this->tagManager->createTag('two', false, true);
 
-               $tagList = $this->tagManager->getTagsById([$tag1->getId(), $tag2->getId()]);
+               $tagList = $this->tagManager->getTagsByIds([$tag1->getId(), $tag2->getId()]);
 
                $this->assertCount(2, $tagList);
 
@@ -270,7 +270,7 @@ class SystemTagManagerTest extends TestCase {
         */
        public function testGetNonExistingTagsById() {
                $tag1 = $this->tagManager->createTag('one', true, false);
-               $this->tagManager->getTagsById([$tag1->getId(), 100, 101]);
+               $this->tagManager->getTagsByIds([$tag1->getId(), 100, 101]);
        }
 
        /**
@@ -278,7 +278,7 @@ class SystemTagManagerTest extends TestCase {
         */
        public function testGetInvalidTagIdFormat() {
                $tag1 = $this->tagManager->createTag('one', true, false);
-               $this->tagManager->getTagsById([$tag1->getId() . 'suffix']);
+               $this->tagManager->getTagsByIds([$tag1->getId() . 'suffix']);
        }
 
        public function updateTagProvider() {

diff --git a/tests/lib/systemtag/systemtagobjectmappertest.php b/tests/lib/systemtag/systemtagobjectmappertest.php
index 43d0b8c696067060905b126c7989ddb454944776..4ea80c216edcdb09313432cfc43d7b50c81e2621 100644 (file)
--- a/tests/lib/systemtag/systemtagobjectmappertest.php
+++ b/tests/lib/systemtag/systemtagobjectmappertest.php
@@ -74,7 +74,7 @@ class SystemTagObjectMapperTest extends TestCase {
                $this->tag3 = new SystemTag(3, 'testtag3', false, false);
 
                $this->tagManager->expects($this->any())
-                       ->method('getTagsById')
+                       ->method('getTagsByIds')
                        ->will($this->returnCallback(function($tagIds) {
                                $result = [];
                                if (in_array(1, $tagIds)) {