Change indexing strategy for votes and options (#2777)

* check mysql instead of mariaDB * change index strategy * 4.1.4
nextcloud · Feb 24, 2023 · a073c67 · a073c67
1 parent dd0e450
commit a073c67
Show file tree

Hide file tree

Showing 13 changed files with 179 additions and 17 deletions.
diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml
@@ -66,7 +66,7 @@ jobs:
 
     services:
       mysql:
-        image: mariadb:10.5
+        image: mysql:8.0
         ports:
           - 4444:3306/tcp
         env:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,12 +1,14 @@
 # Changelog
 All notable changes to this project will be documented in this file.
-## [4.1.3] - 2023-02-23
+## [4.1.4] - 2023-02-23
 ### Fix
- - Fix infinite updates call, if no polling type for watches were set (avoid server spamming)
- - Fix migrations and repair steps
+ - Fix infinite updates call, if no polling type for watches were set (avoid server spamming) (v4.1.3)
+ - Fix migrations and repair steps (v4.1.3)
+ - Fix MySQL error 1071 Specified key was too long;
  ### changes
- - Change default of life update mechanism to manual updates instead of long polling
-
+ - Change default of life update mechanism to manual updates instead of long polling (v4.1.3)
+ - Added Nextcloud 26
+
 ## [4.1.2] - 2023-01-23
 ### Fix
  - Invitations are not send out if poll has no description (fix 2)

diff --git a/appinfo/info.xml b/appinfo/info.xml
@@ -4,7 +4,7 @@
     <name>Polls</name>
     <summary>A polls app, similar to Doodle/Dudle with the possibility to restrict access.</summary>
     <description>A polls app, similar to Doodle/Dudle with the possibility to restrict access (members, certain groups/users, hidden and public).</description>
-    <version>4.1.3</version>
+    <version>4.1.4</version>
     <licence>agpl</licence>
     <author>Vinzenz Rosenkranz</author>
     <author>René Gieling</author>
@@ -57,6 +57,7 @@
             <step>OCA\Polls\Migration\RepairSteps\RemoveIndices</step>
             <step>OCA\Polls\Migration\RepairSteps\CreateTables</step>
             <step>OCA\Polls\Migration\RepairSteps\DeleteInvalidRecords</step>
+            <step>OCA\Polls\Migration\RepairSteps\UpdateHashes</step>
         </pre-migration>
         <post-migration>
             <step>OCA\Polls\Migration\RepairSteps\DropOrphanedTables</step>

diff --git a/lib/Command/Db/Rebuild.php b/lib/Command/Db/Rebuild.php
@@ -72,8 +72,12 @@ protected function runCommands(): int {
 		$this->createOrUpdateSchema();
 		$this->tableManager->migrate();
 
+		// validate and fix/create current table layout
+		$this->printComment('Step 4. set hashes for votes and options');
+		$this->migrateOptionsToHash();
+
 		// recreate indices and constraints
-		$this->printComment('Step 4. Recreate indices and foreign key constraints');
+		$this->printComment('Step 5. Recreate indices and foreign key constraints');
 		// secure, that the schema is updated to the current status
 		$this->indexManager->refreshSchema();
 		$this->addForeignKeyConstraints();
@@ -132,6 +136,17 @@ private function createOrUpdateSchema(): void {
 		}
 	}
 
+	/**
+	 * Add or update hash for votes and options
+	 */
+	private function migrateOptionsToHash(): void {
+		$this->printComment('- add or update hashes');
+		$messages = $this->tableManager->migrateOptionsToHash();
+		foreach ($messages as $message) {
+			$this->printInfo(' - ' . $message);
+		}
+	}
+
 	private function removeObsoleteColumns(): void {
 		$this->printComment('- Drop orphaned columns');
 		$messages = $this->tableManager->removeObsoleteColumns();

diff --git a/lib/Db/Option.php b/lib/Db/Option.php
@@ -47,6 +47,8 @@
  * @method void setPollId(integer $value)
  * @method string getPollOptionText()
  * @method void setPollOptionText(string $value)
+ * @method string getPollOptionHash()
+ * @method void setPollOptionHash(string $value)
  * @method int getReleased()
  * @method void setReleased(int $value)
  * @method int getTimestamp()
@@ -67,6 +69,9 @@ class Option extends EntityWithUser implements JsonSerializable {
 	/** @var string $pollOptionText */
 	protected $pollOptionText = '';
 
+	/** @var string $pollOptionHash */
+	protected $pollOptionHash = '';
+
 	/** @var int $timestamp */
 	protected $timestamp = 0;
 

diff --git a/lib/Db/OptionMapper.php b/lib/Db/OptionMapper.php
@@ -24,6 +24,7 @@
 
 namespace OCA\Polls\Db;
 
+use OCP\AppFramework\Db\Entity;
 use OCP\AppFramework\Db\QBMapper;
 use OCP\DB\Exception;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -40,6 +41,28 @@ public function __construct(IDBConnection $db) {
 		parent::__construct($db, self::TABLE, Option::class);
 	}
 
+	public function update(Entity $entity): Entity {
+		$entity->setPollOptionHash(hash('md5', $entity->getPollId() . $entity->getPollOptionText() . $entity->getTimestamp()));
+		return parent::update($entity);
+	}
+
+	public function insert(Entity $entity): Entity {
+		$entity->setPollOptionHash(hash('md5', $entity->getPollId() . $entity->getPollOptionText() . $entity->getTimestamp()));
+		return parent::insert($entity);
+	}
+
+	/**
+	 * @throws \OCP\AppFramework\Db\DoesNotExistException if not found
+	 * @return Option[]
+	 * @psalm-return array<array-key, Option>
+	 */
+	public function getAll(): array {
+		$qb = $this->db->getQueryBuilder();
+
+		$qb->select('*')->from($this->getTableName());
+		return $this->findEntities($qb);
+	}
+
 	/**
 	 * @throws \OCP\AppFramework\Db\DoesNotExistException if not found
 	 */

diff --git a/lib/Db/TableManager.php b/lib/Db/TableManager.php
@@ -252,7 +252,7 @@ public function removeOrphaned(): void {
 	public function deleteDuplicates(): array {
 		$messages = [];
 
-		if ($this->schema->hasTable(Poll::TABLE)) {
+		if ($this->schema->hasTable($this->dbPrefix . Poll::TABLE)) {
 			$this->removeOrphaned();
 
 			$count[LogMapper::TABLE] = $this->logMapper->removeDuplicates();
@@ -290,9 +290,9 @@ public function removeObsoleteMigrations(): array {
 		}
 		return $messages;
 	}
-	public function fixVotes() {
-		if ($this->schema->hasTable(OptionMapper::TABLE)) {
-			$table = $this->schema->getTable(OptionMapper::TABLE);
+	public function fixVotes(): void {
+		if ($this->schema->hasTable($this->dbPrefix . OptionMapper::TABLE)) {
+			$table = $this->schema->getTable($this->dbPrefix . OptionMapper::TABLE);
 			if ($table->hasColumn('duration')) {
 				$foundOptions = $this->optionMapper->findOptionsWithDuration();
 				foreach ($foundOptions as $option) {
@@ -306,4 +306,37 @@ public function fixVotes() {
 			}
 		}
 	}
+
+	public function migrateOptionsToHash(): array {
+		$messages = [];
+
+		if ($this->schema->hasTable($this->dbPrefix . OptionMapper::TABLE)) {
+			$table = $this->schema->getTable($this->dbPrefix . OptionMapper::TABLE);
+			$count = 0;
+			if ($table->hasColumn('poll_option_hash')) {
+				foreach ($this->optionMapper->getAll() as $option) {
+					$option->setPollOptionHash(hash('md5', $option->getPollId() . $option->getPollOptionText() . $option->getTimestamp()));
+
+					$this->optionMapper->update($option);
+					$count++;
+				}
+			}
+			$messages[] = 'Updated ' . $count . ' option hashes';
+		}
+
+
+		if ($this->schema->hasTable($this->dbPrefix . VoteMapper::TABLE)) {
+			$table = $this->schema->getTable($this->dbPrefix . VoteMapper::TABLE);
+			$count = 0;
+			if ($table->hasColumn('vote_option_hash')) {
+				foreach ($this->voteMapper->getAll() as $vote) {
+					$vote->setVoteOptionHash(hash('md5', $vote->getPollId() . $vote->getUserId() . $vote->getVoteOptionText()));
+					$this->voteMapper->update($vote);
+					$count++;
+				}
+			}
+			$messages[] = 'Updated ' . $count . ' vote hashes';
+		}
+		return $messages;
+	}
 }
diff --git a/lib/Db/Vote.php b/lib/Db/Vote.php
@@ -36,6 +36,8 @@
  * @method void setVoteOptionId(integer $value)
  * @method string getVoteOptionText()
  * @method void setVoteOptionText(string $value)
+ * @method string getVoteOptionHash()
+ * @method void setVoteOptionHash(string $value)
  * @method string getVoteAnswer()
  * @method void setVoteAnswer(string $value)
  */
@@ -54,6 +56,9 @@ class Vote extends EntityWithUser implements JsonSerializable {
 	/** @var string $voteOptionText */
 	protected $voteOptionText = '';
 
+	/** @var string $voteOptionHash */
+	protected $voteOptionHash = '';
+
 	/** @var string $voteAnswer */
 	protected $voteAnswer = '';
 

diff --git a/lib/Db/VoteMapper.php b/lib/Db/VoteMapper.php
@@ -24,6 +24,7 @@
 
 namespace OCA\Polls\Db;
 
+use OCP\AppFramework\Db\Entity;
 use OCP\AppFramework\Db\QBMapper;
 use OCP\DB\Exception;
 use OCP\DB\QueryBuilder\IQueryBuilder;
@@ -40,6 +41,29 @@ public function __construct(IDBConnection $db) {
 		parent::__construct($db, self::TABLE, Vote::class);
 	}
 
+	public function update(Entity $entity): Entity {
+		$entity->setVoteOptionHash(hash('md5', $entity->getPollId() . $entity->getUserId() . $entity->getVoteOptionText()));
+		return parent::update($entity);
+	}
+
+	public function insert(Entity $entity): Entity {
+		$entity->setVoteOptionHash(hash('md5', $entity->getPollId() . $entity->getUserId() . $entity->getVoteOptionText()));
+		return parent::insert($entity);
+	}
+
+	/**
+	 * @throws \OCP\AppFramework\Db\DoesNotExistException if not found
+	 * @return Vote[]
+	 * @psalm-return array<array-key, Vote>
+	 */
+	public function getAll(): array {
+		$qb = $this->db->getQueryBuilder();
+
+		$qb->select('*')->from($this->getTableName());
+		return $this->findEntities($qb);
+	}
+
+
 	/**
 	 * @throws \OCP\AppFramework\Db\DoesNotExistException if not found
 	 * @return Vote[]

diff --git a/lib/Migration/RepairSteps/UpdateHashes.php b/lib/Migration/RepairSteps/UpdateHashes.php
@@ -0,0 +1,52 @@
+<?php
+/**
+ * @copyright Copyright (c) 2021 René Gieling <[email protected]>
+ *
+ * @author René Gieling <[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 OCA\Polls\Migration\RepairSteps;
+
+use OCA\Polls\Db\TableManager;
+use OCP\Migration\IRepairStep;
+use OCP\Migration\IOutput;
+
+class UpdateHashes implements IRepairStep {
+	/** @var TableManager */
+	private $tableManager;
+
+	public function __construct(
+		TableManager $tableManager
+	) {
+		$this->tableManager = $tableManager;
+	}
+
+	public function getName() {
+		return 'Polls - Create hashes vor votes and options';
+	}
+
+	public function run(IOutput $output): void {
+		// Add hashes to votes and options
+		$messages = $this->tableManager->migrateOptionsToHash();
+		foreach ($messages as $message) {
+			$output->info($message);
+		}
+	}
+}
diff --git a/lib/Migration/TableSchema.php b/lib/Migration/TableSchema.php
@@ -55,11 +55,11 @@ abstract class TableSchema {
 	];
 
 	public const UNIQUE_INDICES = [
-		Option::TABLE => ['name' => 'UNIQ_options', 'unique' => true, 'columns' => ['poll_id', 'poll_option_text', 'timestamp']],
+		Option::TABLE => ['name' => 'UNIQ_options', 'unique' => true, 'columns' => ['poll_id', 'poll_option_hash', 'timestamp']],
 		Log::TABLE => ['name' => 'UNIQ_unprocessed', 'unique' => true, 'columns' => ['processed', 'poll_id', 'user_id', 'message_id']],
 		Subscription::TABLE => ['name' => 'UNIQ_subscription', 'unique' => true, 'columns' => ['poll_id', 'user_id']],
 		Share::TABLE => ['name' => 'UNIQ_shares', 'unique' => true, 'columns' => ['poll_id', 'user_id']],
-		Vote::TABLE => ['name' => 'UNIQ_votes', 'unique' => true, 'columns' => ['poll_id', 'user_id', 'vote_option_text']],
+		Vote::TABLE => ['name' => 'UNIQ_votes', 'unique' => true, 'columns' => ['poll_id', 'user_id', 'vote_option_hash']],
 		Preferences::TABLE => ['name' => 'UNIQ_preferences', 'unique' => true, 'columns' => ['user_id']],
 		Watch::TABLE => ['name' => 'UNIQ_watch', 'unique' => true, 'columns' => ['poll_id', 'table', 'session_id']],
 	];
@@ -172,6 +172,7 @@ abstract class TableSchema {
 			'id' => ['type' => Types::BIGINT, 'options' => ['autoincrement' => true, 'notnull' => true, 'length' => 20]],
 			'poll_id' => ['type' => Types::BIGINT, 'options' => ['notnull' => true, 'default' => 0, 'length' => 20]],
 			'poll_option_text' => ['type' => Types::STRING, 'options' => ['notnull' => false, 'default' => '', 'length' => 1024]],
+			'poll_option_hash' => ['type' => Types::STRING, 'options' => ['notnull' => false, 'default' => '', 'length' => 256]],
 			'timestamp' => ['type' => Types::BIGINT, 'options' => ['notnull' => true, 'default' => 0, 'length' => 20]],
 			'duration' => ['type' => Types::BIGINT, 'options' => ['notnull' => true, 'default' => 0, 'length' => 20]],
 			'order' => ['type' => Types::BIGINT, 'options' => ['notnull' => true, 'default' => 0, 'length' => 20]],
@@ -185,6 +186,7 @@ abstract class TableSchema {
 			'user_id' => ['type' => Types::STRING, 'options' => ['notnull' => false, 'default' => '', 'length' => 256]],
 			'vote_option_id' => ['type' => Types::BIGINT, 'options' => ['notnull' => true, 'default' => 0, 'length' => 20]],
 			'vote_option_text' => ['type' => Types::STRING, 'options' => ['notnull' => false, 'default' => '', 'length' => 1024]],
+			'vote_option_hash' => ['type' => Types::STRING, 'options' => ['notnull' => false, 'default' => '', 'length' => 256]],
 			'vote_answer' => ['type' => Types::STRING, 'options' => ['notnull' => false, 'default' => '', 'length' => 64]],
 		],
 		Comment::TABLE => [

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
 	"name": "polls",
 	"description": "Polls app for nextcloud",
-	"version": "4.1.3",
+	"version": "4.1.4",
 	"authors": [
 		{
 			"name": "Vinzenz Rosenkranz",