Fixed system tags DAV and API and docs

7 files changed, 49 insertions, 40 deletions

diff --git a/apps/dav/lib/systemtag/systemtagnode.php b/apps/dav/lib/systemtag/systemtagnode.php
index f7228108b3d..7ab4a8a14f4 100644
--- 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 692b7e97016..51db0632549 100644
--- 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 0164b9b0b3d..e7b7b6d0acc 100644
--- 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 e81994e0bd6..89e8620614b 100644
--- 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 8dee85ccd44..e544073613f 100644
--- 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 fdaaf2cd009..104ce366034 100644
--- 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 1a9ffa6f4ae..6e15bb78e7c 100644
--- 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');