Merge pull request #42209 from nextcloud/bugfix/noid/comments-meta-data

feat(comments): Add a meta data column for comments

apps/dav/tests/unit/Comments/CommentsNodeTest.php

@@ -405,6 +405,11 @@ public function testGetProperties(): void {
 			$ns . 'referenceId' => 'ref',
 			$ns . 'isUnread' => null,
 			$ns . 'reactions' => [],
+			$ns . 'metaData' => [
+				'last_edited_at' => 1702553770,
+				'last_edited_by_id' => 'charly',
+				'last_edited_by_type' => 'user',
+			],
 			$ns . 'expireDate' => new \DateTime('2016-01-12 19:00:00'),
 		];
 
@@ -475,6 +480,10 @@ public function testGetProperties(): void {
 			->method('getReferenceId')
 			->willReturn($expected[$ns . 'referenceId']);
 
+		$this->comment->expects($this->once())
+			->method('getMetaData')
+			->willReturn($expected[$ns . 'metaData']);
+
 		$this->comment->expects($this->once())
 			->method('getExpireDate')
 			->willReturn($expected[$ns . 'expireDate']);
@@ -494,7 +503,7 @@ public function testGetProperties(): void {
 		$properties = $this->node->getProperties(null);
 
 		foreach ($properties as $name => $value) {
-			$this->assertArrayHasKey($name, $expected);
+			$this->assertArrayHasKey($name, $expected, 'Key not found in the list of $expected');
 			$this->assertSame($expected[$name], $value);
 			unset($expected[$name]);
 		}

core/Application.php

@@ -48,10 +48,8 @@
 use OCP\AppFramework\App;
 use OCP\AppFramework\Http\Events\BeforeLoginTemplateRenderedEvent;
 use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
-use OCP\DB\Events\AddMissingColumnsEvent;
 use OCP\DB\Events\AddMissingIndicesEvent;
 use OCP\DB\Events\AddMissingPrimaryKeyEvent;
-use OCP\DB\Types;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\User\Events\BeforeUserDeletedEvent;
 use OCP\User\Events\UserDeletedEvent;
@@ -300,18 +298,6 @@ public function __construct() {
 			);
 		});
 
-		$eventDispatcher->addListener(AddMissingColumnsEvent::class, function (AddMissingColumnsEvent $event) {
-			$event->addMissingColumn(
-				'comments',
-				'reference_id',
-				Types::STRING,
-				[
-					'notnull' => false,
-					'length' => 64,
-				]
-			);
-		});
-
 		$eventDispatcher->addServiceListener(BeforeTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class);
 		$eventDispatcher->addServiceListener(BeforeLoginTemplateRenderedEvent::class, BeforeTemplateRenderedListener::class);
 		$eventDispatcher->addServiceListener(RemoteWipeStarted::class, RemoteWipeActivityListener::class);

core/Migrations/Version29000Date20231213104850.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2023 Joas Schilling <[email protected]>
+ *
+ * @author Joas Schilling <[email protected]>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Core\Migrations;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version29000Date20231213104850 extends SimpleMigrationStep {
+	/**
+	 * @param IOutput $output
+	 * @param Closure(): ISchemaWrapper $schemaClosure
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$table = $schema->getTable('comments');
+		$modified = false;
+
+		if (!$table->hasColumn('reference_id')) {
+			$modified = true;
+			$table->addColumn('reference_id', Types::STRING, [
+				'notnull' => false,
+				'length' => 64,
+			]);
+		}
+
+		if (!$table->hasColumn('meta_data')) {
+			$modified = true;
+			$table->addColumn('meta_data', Types::TEXT, [
+				'notnull' => false,
+				'default' => '',
+			]);
+		}
+
+		return $modified ? $schema : null;
+	}
+}

lib/composer/composer/autoload_classmap.php

@@ -1230,6 +1230,7 @@
     'OC\\Core\\Migrations\\Version28000Date20230906104802' => $baseDir . '/core/Migrations/Version28000Date20230906104802.php',
     'OC\\Core\\Migrations\\Version28000Date20231004103301' => $baseDir . '/core/Migrations/Version28000Date20231004103301.php',
     'OC\\Core\\Migrations\\Version28000Date20231103104802' => $baseDir . '/core/Migrations/Version28000Date20231103104802.php',
+    'OC\\Core\\Migrations\\Version29000Date20231213104850' => $baseDir . '/core/Migrations/Version29000Date20231213104850.php',
     'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',
     'OC\\Core\\Service\\LoginFlowV2Service' => $baseDir . '/core/Service/LoginFlowV2Service.php',
     'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php',

lib/composer/composer/autoload_static.php

@@ -1263,6 +1263,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Core\\Migrations\\Version28000Date20230906104802' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20230906104802.php',
         'OC\\Core\\Migrations\\Version28000Date20231004103301' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231004103301.php',
         'OC\\Core\\Migrations\\Version28000Date20231103104802' => __DIR__ . '/../../..' . '/core/Migrations/Version28000Date20231103104802.php',
+        'OC\\Core\\Migrations\\Version29000Date20231213104850' => __DIR__ . '/../../..' . '/core/Migrations/Version29000Date20231213104850.php',
         'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',
         'OC\\Core\\Service\\LoginFlowV2Service' => __DIR__ . '/../../..' . '/core/Service/LoginFlowV2Service.php',
         'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php',

lib/private/Comments/Comment.php

@@ -42,6 +42,7 @@ class Comment implements IComment {
 		'objectType' => '',
 		'objectId' => '',
 		'referenceId' => null,
+		'metaData' => null,
 		'creationDT' => null,
 		'latestChildDT' => null,
 		'reactions' => null,
@@ -400,6 +401,34 @@ public function setReferenceId(?string $referenceId): IComment {
 		return $this;
 	}
 
+	/**
+	 * @inheritDoc
+	 */
+	public function getMetaData(): ?array {
+		if ($this->data['metaData'] === null) {
+			return null;
+		}
+
+		try {
+			$metaData = json_decode($this->data['metaData'], true, flags: JSON_THROW_ON_ERROR);
+		} catch (\JsonException $e) {
+			return null;
+		}
+		return is_array($metaData) ? $metaData : null;
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	public function setMetaData(?array $metaData):  IComment {
+		if ($metaData === null) {
+			$this->data['metaData'] = null;
+		} else {
+			$this->data['metaData'] = json_encode($metaData, JSON_THROW_ON_ERROR);
+		}
+		return $this;
+	}
+
 	/**
 	 * @inheritDoc
 	 */

lib/private/Comments/Manager.php

@@ -29,7 +29,6 @@
 namespace OC\Comments;
 
 use Doctrine\DBAL\Exception\DriverException;
-use Doctrine\DBAL\Exception\InvalidFieldNameException;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Comments\CommentsEvent;
 use OCP\Comments\IComment;
@@ -97,7 +96,8 @@ protected function normalizeDatabaseData(array $data): array {
 			$data['expire_date'] = new \DateTime($data['expire_date']);
 		}
 		$data['children_count'] = (int)$data['children_count'];
-		$data['reference_id'] = $data['reference_id'] ?? null;
+		$data['reference_id'] = $data['reference_id'];
+		$data['meta_data'] = json_decode($data['meta_data'], true);
 		if ($this->supportReactions()) {
 			if ($data['reactions'] !== null) {
 				$list = json_decode($data['reactions'], true);
@@ -1150,22 +1150,6 @@ public function save(IComment $comment) {
 	 * @return bool
 	 */
 	protected function insert(IComment $comment): bool {
-		try {
-			$result = $this->insertQuery($comment, true);
-		} catch (InvalidFieldNameException $e) {
-			// The reference id field was only added in Nextcloud 19.
-			// In order to not cause too long waiting times on the update,
-			// it was decided to only add it lazy, as it is also not a critical
-			// feature, but only helps to have a better experience while commenting.
-			// So in case the reference_id field is missing,
-			// we simply save the comment without that field.
-			$result = $this->insertQuery($comment, false);
-		}
-
-		return $result;
-	}
-
-	protected function insertQuery(IComment $comment, bool $tryWritingReferenceId): bool {
 		$qb = $this->dbConn->getQueryBuilder();
 
 		$values = [
@@ -1181,12 +1165,10 @@ protected function insertQuery(IComment $comment, bool $tryWritingReferenceId):
 			'object_type' => $qb->createNamedParameter($comment->getObjectType()),
 			'object_id' => $qb->createNamedParameter($comment->getObjectId()),
 			'expire_date' => $qb->createNamedParameter($comment->getExpireDate(), 'datetime'),
+			'reference_id' => $qb->createNamedParameter($comment->getReferenceId()),
+			'meta_data' => $qb->createNamedParameter(json_encode($comment->getMetaData())),
 		];
 
-		if ($tryWritingReferenceId) {
-			$values['reference_id'] = $qb->createNamedParameter($comment->getReferenceId());
-		}
-
 		$affectedRows = $qb->insert('comments')
 			->values($values)
 			->execute();
@@ -1289,12 +1271,7 @@ protected function update(IComment $comment) {
 		$this->sendEvent(CommentsEvent::EVENT_PRE_UPDATE, $this->get($comment->getId()));
 		$this->uncache($comment->getId());
 
-		try {
-			$result = $this->updateQuery($comment, true);
-		} catch (InvalidFieldNameException $e) {
-			// See function insert() for explanation
-			$result = $this->updateQuery($comment, false);
-		}
+		$result = $this->updateQuery($comment);
 
 		if ($comment->getVerb() === 'reaction_deleted') {
 			$this->deleteReaction($comment);
@@ -1305,7 +1282,7 @@ protected function update(IComment $comment) {
 		return $result;
 	}
 
-	protected function updateQuery(IComment $comment, bool $tryWritingReferenceId): bool {
+	protected function updateQuery(IComment $comment): bool {
 		$qb = $this->dbConn->getQueryBuilder();
 		$qb
 			->update('comments')
@@ -1320,14 +1297,11 @@ protected function updateQuery(IComment $comment, bool $tryWritingReferenceId):
 			->set('latest_child_timestamp', $qb->createNamedParameter($comment->getLatestChildDateTime(), 'datetime'))
 			->set('object_type', $qb->createNamedParameter($comment->getObjectType()))
 			->set('object_id', $qb->createNamedParameter($comment->getObjectId()))
-			->set('expire_date', $qb->createNamedParameter($comment->getExpireDate(), 'datetime'));
-
-		if ($tryWritingReferenceId) {
-			$qb->set('reference_id', $qb->createNamedParameter($comment->getReferenceId()));
-		}
-
-		$affectedRows = $qb->where($qb->expr()->eq('id', $qb->createNamedParameter($comment->getId())))
-			->execute();
+			->set('expire_date', $qb->createNamedParameter($comment->getExpireDate(), 'datetime'))
+			->set('reference_id', $qb->createNamedParameter($comment->getReferenceId()))
+			->set('meta_data', $qb->createNamedParameter(json_encode($comment->getMetaData())))
+			->where($qb->expr()->eq('id', $qb->createNamedParameter($comment->getId())));
+		$affectedRows = $qb->executeStatement();
 
 		if ($affectedRows === 0) {
 			throw new NotFoundException('Comment to update does ceased to exist');

lib/public/Comments/IComment.php

@@ -279,6 +279,25 @@ public function getReferenceId(): ?string;
 	 */
 	public function setReferenceId(?string $referenceId): IComment;
 
+	/**
+	 * Returns the metadata of the comment
+	 *
+	 * @return array|null
+	 * @since 29.0.0
+	 */
+	public function getMetaData(): ?array;
+
+	/**
+	 * Sets (overwrites) the metadata of the comment
+	 * Data as a json encoded array
+	 *
+	 * @param array|null $metaData
+	 * @return IComment
+	 * @throws \JsonException When the metadata can not be converted to a json encoded string
+	 * @since 29.0.0
+	 */
+	public function setMetaData(?array $metaData): IComment;
+
 	/**
 	 * Returns the reactions array if exists
 	 *

tests/lib/Comments/CommentTest.php

@@ -23,6 +23,8 @@ public function testSettersValidInput() {
 		$creationDT = new \DateTime();
 		$latestChildDT = new \DateTime('yesterday');
 		$object = ['type' => 'files', 'id' => 'file64'];
+		$referenceId = sha1('referenceId');
+		$metaData = ['last_edit_actor_id' => 'admin'];
 
 		$comment
 			->setId($id)
@@ -34,7 +36,9 @@ public function testSettersValidInput() {
 			->setActor($actor['type'], $actor['id'])
 			->setCreationDateTime($creationDT)
 			->setLatestChildDateTime($latestChildDT)
-			->setObject($object['type'], $object['id']);
+			->setObject($object['type'], $object['id'])
+			->setReferenceId($referenceId)
+			->setMetaData($metaData);
 
 		$this->assertSame($id, $comment->getId());
 		$this->assertSame($parentId, $comment->getParentId());
@@ -48,6 +52,8 @@ public function testSettersValidInput() {
 		$this->assertSame($latestChildDT, $comment->getLatestChildDateTime());
 		$this->assertSame($object['type'], $comment->getObjectType());
 		$this->assertSame($object['id'], $comment->getObjectId());
+		$this->assertSame($referenceId, $comment->getReferenceId());
+		$this->assertSame($metaData, $comment->getMetaData());
 	}
 
 

tests/lib/Comments/ManagerTest.php

@@ -65,6 +65,8 @@ protected function addDatabaseEntry($parentId, $topmostParentId, $creationDT = n
 				'object_type' => $qb->createNamedParameter('files'),
 				'object_id' => $qb->createNamedParameter($objectId),
 				'expire_date' => $qb->createNamedParameter($expireDate, 'datetime'),
+				'reference_id' => $qb->createNamedParameter('referenceId'),
+				'meta_data' => $qb->createNamedParameter(json_encode(['last_edit_actor_id' => 'admin'])),
 			])
 			->execute();
 
@@ -119,6 +121,8 @@ public function testGetComment() {
 				'latest_child_timestamp' => $qb->createNamedParameter($latestChildDT, 'datetime'),
 				'object_type' => $qb->createNamedParameter('files'),
 				'object_id' => $qb->createNamedParameter('file64'),
+				'reference_id' => $qb->createNamedParameter('referenceId'),
+				'meta_data' => $qb->createNamedParameter(json_encode(['last_edit_actor_id' => 'admin'])),
 			])
 			->execute();
 
@@ -138,6 +142,8 @@ public function testGetComment() {
 		$this->assertSame($comment->getObjectId(), 'file64');
 		$this->assertEquals($comment->getCreationDateTime()->getTimestamp(), $creationDT->getTimestamp());
 		$this->assertEquals($comment->getLatestChildDateTime(), $latestChildDT);
+		$this->assertEquals($comment->getReferenceId(), 'referenceId');
+		$this->assertEquals($comment->getMetaData(), ['last_edit_actor_id' => 'admin']);
 	}
 
 

version.php

@@ -30,7 +30,7 @@
 // between betas, final and RCs. This is _not_ the public version number. Reset minor/patch level
 // when updating major/minor version number.
 
-$OC_Version = [29, 0, 0, 2];
+$OC_Version = [29, 0, 0, 3];
 
 // The human-readable string
 $OC_VersionString = '29.0.0 dev';