Merge pull request #2362 from nextcloud/fix/deduplicate-migration

fix(Migration): Merge per-user-database-duplicate bookmarks before hashing URLs

Author: marcelklehr

.github/workflows/floccus.yml

@@ -130,8 +130,6 @@ jobs:
           MYSQL_PASSWORD: ${{env.MYSQL_PASSWORD}}
           MYSQL_HOST: mysql
           NEXTCLOUD_TRUSTED_DOMAINS: nextcloud
-        volumes:
-          - /home/runner/actions-runner/_work/bookmarks/bookmarks/apps:/var/www/html/apps
         options: --name nextcloud
       mysql:
         image: mariadb:10.5 # see https://github.com/nextcloud/docker/issues/1536
@@ -170,7 +168,7 @@ jobs:
       - name: Enable bookmarks app
         shell: bash
         run: |
-          sudo cp -R ${{env.APP_NAME}} apps/
+          docker cp ${{env.APP_NAME}} nextcloud:/var/www/html/apps/
           NEXT_WAIT_TIME=0
           until [ $NEXT_WAIT_TIME -eq 25 ] || docker exec --user www-data nextcloud php occ app:enable ${{ env.APP_NAME }}; do
               sleep $(( NEXT_WAIT_TIME++ ))

appinfo/info.xml

@@ -23,7 +23,7 @@ Requirements:
    - mbstring: *
  - when using MySQL, use at least v8.0
 ]]></description>
-	<version>16.1.0</version>
+	<version>16.1.1-dev.1</version>
 	<licence>agpl</licence>
 	<author mail="mklehr@gmx.net" homepage="https://marcelklehr.de">Marcel Klehr</author>
 	<author mail="blizzz@arthur-schiwon.de" homepage="https://www.arthur-schiwon.de">Arthur Schiwon</author>

lib/Db/BookmarkMapper.php

@@ -859,6 +859,7 @@ public function update(Entity $entity): Bookmark {
 		if ($entity->isWebLink()) {
 			$entity->setUrl($this->urlNormalizer->normalize($entity->getUrl()));
 		}
+		$entity->setUrlHash(hash('xxh128', $entity->getUrl()));
 		$entity->setLastmodified(time());
 		parent::update($entity);
 		$this->eventDispatcher->dispatchTyped(new ManipulateEvent('bookmark', $entity->getId()));
@@ -883,19 +884,25 @@ public function insert(Entity $entity): Bookmark {
 		if ($entity->isWebLink()) {
 			$entity->setUrl($this->urlNormalizer->normalize($entity->getUrl()));
 		}
-		$entity->setUrlHash(hash('xxh128', $entity->getUrl()));
+		$entityToInsert = new Bookmark();
+		foreach ($entity->getUpdatedFields() as $field => $_value) {
+			$value = $entity->{'get' . ucfirst($field)}();
+			$entityToInsert->{'set' . ucfirst($field)}($value);
+		}
 
-		if ($entity->getAdded() === null) {
-			$entity->setAdded(time());
+		$entityToInsert->setUrlHash(hash('xxh128', $entity->getUrl()));
+
+		if ($entityToInsert->getAdded() === null) {
+			$entityToInsert->setAdded(time());
 		}
-		$entity->setLastmodified(time());
-		$entity->setLastPreview(0);
-		$entity->setClickcount(0);
+		$entityToInsert->setLastmodified(time());
+		$entityToInsert->setLastPreview(0);
+		$entityToInsert->setClickcount(0);
 
 		try {
-			parent::insert($entity);
-			$this->eventDispatcher->dispatchTyped(new InsertEvent('bookmark', $entity->getId()));
-			return $entity;
+			parent::insert($entityToInsert);
+			$this->eventDispatcher->dispatchTyped(new InsertEvent('bookmark', $entityToInsert->getId()));
+			return $entityToInsert;
 		} catch (Exception $e) {
 			if ($e->getReason() === Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) {
 				if ($this->db->inTransaction()) {
@@ -919,12 +926,15 @@ public function insert(Entity $entity): Bookmark {
 	public function insertOrUpdate(Entity $entity): Bookmark {
 		try {
 			$newEntity = $this->insert($entity);
-		} catch (AlreadyExistsError $e) {
+		} catch (AlreadyExistsError) {
 			try {
 				$bookmark = $this->findByUrl($entity->getUserId(), $entity->getUrl());
-				$entity->setId($bookmark->getId());
-				$newEntity = $this->update($entity);
-			} catch (DoesNotExistException $e) {
+				foreach ($entity->getUpdatedFields() as $field => $_value) {
+					$value = $entity->{'get' . ucfirst($field)}();
+					$bookmark->{'set' . ucfirst($field)}($value);
+				}
+				$newEntity = $this->update($bookmark);
+			} catch (DoesNotExistException) {
 				$newEntity = $this->insert($entity);
 			}
 		}

lib/Migration/Version016002000Date20260201124723.php

@@ -48,35 +48,5 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
 	}
 
 	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
-		$countQb = $this->db->getQueryBuilder();
-		$countQb->select($countQb->func()->count('id'))->from('bookmarks')->where($countQb->expr()->isNull('url_hash'));
-		$result = $countQb->executeQuery();
-		$count = $result->fetchOne();
-		$output->info('Hashing URLs of n=' . $count . ' bookmarks');
-		$output->startProgress($count);
-
-		$qb = $this->db->getQueryBuilder();
-		$qb->select('id', 'url')->from('bookmarks')->where($qb->expr()->isNull('url_hash'));
-		$result = $qb->executeQuery();
-		$setQb = $this->db->getQueryBuilder();
-		$setQb->update('bookmarks')
-			->set('url_hash', $qb->createParameter('url_hash'))
-			->where($qb->expr()->eq('id', $qb->createParameter('id')));
-		$i = 1;
-		$this->db->beginTransaction();
-		while ($row = $result->fetch()) {
-			if ($i++ % 1000 === 0) {
-				$this->db->commit();
-				$this->db->beginTransaction();
-			}
-			$setQb->setParameter('url_hash', hash('xxh128', $row['url']));
-			$setQb->setParameter('id', $row['id']);
-			$setQb->executeStatement();
-			$output->advance();
-		}
-		$this->db->commit();
-		$output->finishProgress();
-		$result->closeCursor();
 	}
-
 }