From 720dc4e93d83d738861c614745f514cc347ef1f9 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 11 Mar 2020 12:29:52 +0100 Subject: [PATCH 1/6] Add optional column oc_comments.reference_id Signed-off-by: Joas Schilling --- .../lib/Controller/CheckSetupController.php | 11 ++ .../Controller/CheckSetupControllerTest.php | 1 + core/Application.php | 18 +++ core/Command/Db/AddMissingColumns.php | 105 ++++++++++++++++++ core/Command/Db/AddMissingIndices.php | 2 +- .../Version13000Date20170718121200.php | 4 + core/js/setupchecks.js | 15 +++ core/js/tests/specs/setupchecksSpec.js | 15 +++ core/register_command.php | 1 + lib/private/DB/MissingColumnInformation.php | 43 +++++++ lib/public/IDBConnection.php | 2 + 11 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 core/Command/Db/AddMissingColumns.php create mode 100644 lib/private/DB/MissingColumnInformation.php diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php index 04711cf5308ad..6f3bb539eff56 100644 --- a/apps/settings/lib/Controller/CheckSetupController.php +++ b/apps/settings/lib/Controller/CheckSetupController.php @@ -46,6 +46,7 @@ use OC; use OC\AppFramework\Http; use OC\DB\Connection; +use OC\DB\MissingColumnInformation; use OC\DB\MissingIndexInformation; use OC\DB\SchemaWrapper; use OC\IntegrityCheck\Checker; @@ -445,6 +446,15 @@ protected function hasMissingIndexes(): array { return $indexInfo->getListOfMissingIndexes(); } + protected function hasMissingColumns(): array { + $indexInfo = new MissingColumnInformation(); + // Dispatch event so apps can also hint for pending index updates if needed + $event = new GenericEvent($indexInfo); + $this->dispatcher->dispatch(IDBConnection::CHECK_MISSING_COLUMNS_EVENT, $event); + + return $indexInfo->getListOfMissingColumns(); + } + protected function isSqliteUsed() { return strpos($this->config->getSystemValue('dbtype'), 'sqlite') !== false; } @@ -693,6 +703,7 @@ public function check() { 'isSettimelimitAvailable' => $this->isSettimelimitAvailable(), 'hasFreeTypeSupport' => $this->hasFreeTypeSupport(), 'missingIndexes' => $this->hasMissingIndexes(), + 'missingColumns' => $this->hasMissingColumns(), 'isSqliteUsed' => $this->isSqliteUsed(), 'databaseConversionDocumentation' => $this->urlGenerator->linkToDocs('admin-db-conversion'), 'isPHPMailerUsed' => $this->isPHPMailerUsed(), diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php index 5e59bfe353aff..c15f3b8f23ac5 100644 --- a/apps/settings/tests/Controller/CheckSetupControllerTest.php +++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php @@ -583,6 +583,7 @@ public function testCheck() { 'isSqliteUsed' => false, 'databaseConversionDocumentation' => 'http://docs.example.org/server/go.php?to=admin-db-conversion', 'missingIndexes' => [], + 'missingColumns' => [], 'isPHPMailerUsed' => false, 'mailSettingsDocumentation' => 'https://server/index.php/settings/admin', 'isMemoryLimitSufficient' => true, diff --git a/core/Application.php b/core/Application.php index c2da7f8d19467..e4fc33b5402ce 100644 --- a/core/Application.php +++ b/core/Application.php @@ -39,6 +39,7 @@ use OC\Authentication\Listeners\UserDeletedStoreCleanupListener; use OC\Authentication\Notifications\Notifier as AuthenticationNotifier; use OC\Core\Notification\RemoveLinkSharesNotifier; +use OC\DB\MissingColumnInformation; use OC\DB\MissingIndexInformation; use OC\DB\SchemaWrapper; use OCP\AppFramework\App; @@ -167,6 +168,23 @@ function (GenericEvent $event) use ($container) { } ); + $eventDispatcher->addListener(IDBConnection::CHECK_MISSING_COLUMNS_EVENT, + function (GenericEvent $event) use ($container) { + /** @var MissingColumnInformation $subject */ + $subject = $event->getSubject(); + + $schema = new SchemaWrapper($container->query(IDBConnection::class)); + + if ($schema->hasTable('comments')) { + $table = $schema->getTable('comments'); + + if (!$table->hasColumn('reference_id')) { + $subject->addHintForMissingColumn($table->getName(), 'reference_id'); + } + } + } + ); + $eventDispatcher->addServiceListener(RemoteWipeStarted::class, RemoteWipeActivityListener::class); $eventDispatcher->addServiceListener(RemoteWipeStarted::class, RemoteWipeNotificationsListener::class); $eventDispatcher->addServiceListener(RemoteWipeStarted::class, RemoteWipeEmailListener::class); diff --git a/core/Command/Db/AddMissingColumns.php b/core/Command/Db/AddMissingColumns.php new file mode 100644 index 0000000000000..4672770c6af33 --- /dev/null +++ b/core/Command/Db/AddMissingColumns.php @@ -0,0 +1,105 @@ + + * + * @author Joas Schilling + * + * @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 . + * + */ + +namespace OC\Core\Command\Db; + +use OC\DB\SchemaWrapper; +use OCP\IDBConnection; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; +use Symfony\Component\EventDispatcher\GenericEvent; + +/** + * Class AddMissingColumns + * + * if you added a new lazy column to the database, this is the right place to add + * your update routine for existing instances + * + * @package OC\Core\Command\Db + */ +class AddMissingColumns extends Command { + + /** @var IDBConnection */ + private $connection; + + /** @var EventDispatcherInterface */ + private $dispatcher; + + public function __construct(IDBConnection $connection, EventDispatcherInterface $dispatcher) { + parent::__construct(); + + $this->connection = $connection; + $this->dispatcher = $dispatcher; + } + + protected function configure() { + $this + ->setName('db:add-missing-columns') + ->setDescription('Add missing optional columns to the database tables'); + } + + protected function execute(InputInterface $input, OutputInterface $output) { + $this->addCoreColumns($output); + + // Dispatch event so apps can also update columns if needed + $event = new GenericEvent($output); + $this->dispatcher->dispatch(IDBConnection::ADD_MISSING_COLUMNS_EVENT, $event); + } + + /** + * add missing indices to the share table + * + * @param OutputInterface $output + * @throws \Doctrine\DBAL\Schema\SchemaException + */ + private function addCoreColumns(OutputInterface $output) { + + $output->writeln('Check columns of the comments table.'); + + $schema = new SchemaWrapper($this->connection); + $updated = false; + + if ($schema->hasTable('comments')) { + $table = $schema->getTable('comments'); + if (!$table->hasColumn('reference_id')) { + $output->writeln('Adding additional reference_id column to the comments table, this can take some time...'); + $table->addColumn('reference_id', 'string', [ + 'notnull' => false, + 'length' => 64, + ]); + $this->connection->migrateToSchema($schema->getWrappedSchema()); + $updated = true; + $output->writeln('Comments table updated successfully.'); + } + } + + if (!$updated) { + $output->writeln('Done.'); + } + } +} diff --git a/core/Command/Db/AddMissingIndices.php b/core/Command/Db/AddMissingIndices.php index 0152df2173755..c4006a2d7fa72 100644 --- a/core/Command/Db/AddMissingIndices.php +++ b/core/Command/Db/AddMissingIndices.php @@ -43,7 +43,7 @@ * Class AddMissingIndices * * if you added any new indices to the database, this is the right place to add - * it your update routine for existing instances + * your update routine for existing instances * * @package OC\Core\Command\Db */ diff --git a/core/Migrations/Version13000Date20170718121200.php b/core/Migrations/Version13000Date20170718121200.php index e9f376a7e4248..757697e7e90a5 100644 --- a/core/Migrations/Version13000Date20170718121200.php +++ b/core/Migrations/Version13000Date20170718121200.php @@ -769,6 +769,10 @@ public function changeSchema(IOutput $output, \Closure $schemaClosure, array $op 'length' => 64, 'default' => '', ]); + $table->addColumn('reference_id', 'string', [ + 'notnull' => false, + 'length' => 64, + ]); $table->setPrimaryKey(['id']); $table->addIndex(['parent_id'], 'comments_parent_id_index'); $table->addIndex(['topmost_parent_id'], 'comments_topmost_parent_id_idx'); diff --git a/core/js/setupchecks.js b/core/js/setupchecks.js index 12d1104a6327b..2e94c82486ceb 100644 --- a/core/js/setupchecks.js +++ b/core/js/setupchecks.js @@ -357,6 +357,21 @@ type: OC.SetupChecks.MESSAGE_TYPE_INFO }) } + if (data.missingColumns.length > 0) { + var listOfMissingColumns = ""; + data.missingColumns.forEach(function(element){ + listOfMissingColumns += "
  • "; + listOfMissingColumns += t('core', 'Missing optional column "{columnName}" in table "{tableName}".', element); + listOfMissingColumns += "
    • "; + }); + messages.push({ + msg: t( + 'core', + 'The database is missing some optional columns. Due to the fact that adding columns on big tables could take some time they were not added automatically when they can be optional. By running "occ db:add-missing-columns" those missing columns could be added manually while the instance keeps running. Once the columns are added some features might improve responsiveness or usability.' + ) + "
      " + listOfMissingColumns + "
    ", + type: OC.SetupChecks.MESSAGE_TYPE_INFO + }) + } if (data.recommendedPHPModules.length > 0) { var listOfRecommendedPHPModules = ""; data.recommendedPHPModules.forEach(function(element){ diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 5e93cbf7bdf86..891b5fc84c896 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -240,6 +240,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -293,6 +294,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -347,6 +349,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -399,6 +402,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -449,6 +453,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -499,6 +504,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -551,6 +557,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -601,6 +608,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: false, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -651,6 +659,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -722,6 +731,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -773,6 +783,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -824,6 +835,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -875,6 +887,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: false, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -925,6 +938,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 @@ -1026,6 +1040,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 diff --git a/core/register_command.php b/core/register_command.php index efa3146c49223..e355d1429c148 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -99,6 +99,7 @@ $application->add(new OC\Core\Command\Db\ConvertMysqlToMB4(\OC::$server->getConfig(), \OC::$server->getDatabaseConnection(), \OC::$server->getURLGenerator(), \OC::$server->getLogger())); $application->add(new OC\Core\Command\Db\ConvertFilecacheBigInt(\OC::$server->getDatabaseConnection())); $application->add(new OC\Core\Command\Db\AddMissingIndices(\OC::$server->getDatabaseConnection(), \OC::$server->getEventDispatcher())); + $application->add(new OC\Core\Command\Db\AddMissingColumns(\OC::$server->getDatabaseConnection(), \OC::$server->getEventDispatcher())); $application->add(new OC\Core\Command\Db\Migrations\StatusCommand(\OC::$server->getDatabaseConnection())); $application->add(new OC\Core\Command\Db\Migrations\MigrateCommand(\OC::$server->getDatabaseConnection())); $application->add(new OC\Core\Command\Db\Migrations\GenerateCommand(\OC::$server->getDatabaseConnection(), \OC::$server->getAppManager())); diff --git a/lib/private/DB/MissingColumnInformation.php b/lib/private/DB/MissingColumnInformation.php new file mode 100644 index 0000000000000..bcf585848fd90 --- /dev/null +++ b/lib/private/DB/MissingColumnInformation.php @@ -0,0 +1,43 @@ + + * + * @author Joas Schilling + * + * @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 . + * + */ + +namespace OC\DB; + +class MissingColumnInformation { + + private $listOfMissingColumns = []; + + public function addHintForMissingColumn(string $tableName, string $columnName): void { + $this->listOfMissingColumns[] = [ + 'tableName' => $tableName, + 'columnName' => $columnName, + ]; + } + + public function getListOfMissingColumns(): array { + return $this->listOfMissingColumns; + } +} diff --git a/lib/public/IDBConnection.php b/lib/public/IDBConnection.php index 30010bb4cefa5..eabd07e9a0375 100644 --- a/lib/public/IDBConnection.php +++ b/lib/public/IDBConnection.php @@ -50,6 +50,8 @@ interface IDBConnection { const ADD_MISSING_INDEXES_EVENT = self::class . '::ADD_MISSING_INDEXES'; const CHECK_MISSING_INDEXES_EVENT = self::class . '::CHECK_MISSING_INDEXES'; + const ADD_MISSING_COLUMNS_EVENT = self::class . '::ADD_MISSING_COLUMNS'; + const CHECK_MISSING_COLUMNS_EVENT = self::class . '::CHECK_MISSING_COLUMNS'; /** * Gets the QueryBuilder for the connection. From a20e81f0f34389ef9c9e1a4df3ec2b514968b19f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 11 Mar 2020 12:11:01 +0100 Subject: [PATCH 2/6] Allow to set and get the reference id Signed-off-by: Joas Schilling --- lib/private/Comments/Comment.php | 31 +++++++++++++++++++++++++++++++ lib/private/Comments/Manager.php | 1 + lib/public/Comments/IComment.php | 17 +++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php index 71687c7a60920..3b4a523b8848c 100644 --- a/lib/private/Comments/Comment.php +++ b/lib/private/Comments/Comment.php @@ -42,6 +42,7 @@ class Comment implements IComment { 'actorId' => '', 'objectType' => '', 'objectId' => '', + 'referenceId' => null, 'creationDT' => null, 'latestChildDT' => null, ]; @@ -395,6 +396,36 @@ public function setObject($objectType, $objectId) { return $this; } + /** + * returns the reference id of the comment + * + * @return string|null + * @since 19.0.0 + */ + public function getReferenceId(): ?string { + return $this->data['referenceId']; + } + + /** + * sets (overwrites) the reference id of the comment + * + * @param string $referenceId e.g. sha256 hash sum + * @return IComment + * @since 19.0.0 + */ + public function setReferenceId(?string $referenceId): IComment { + if ($referenceId === null) { + $this->data['referenceId'] = $referenceId; + } else { + $referenceId = trim($referenceId); + if ($referenceId === '') { + throw new \InvalidArgumentException('Non empty string expected.'); + } + $this->data['referenceId'] = $referenceId; + } + return $this; + } + /** * sets the comment data based on an array with keys as taken from the * database. diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index f1c7224359774..aad7cad020095 100644 --- a/lib/private/Comments/Manager.php +++ b/lib/private/Comments/Manager.php @@ -96,6 +96,7 @@ protected function normalizeDatabaseData(array $data) { $data['latest_child_timestamp'] = new \DateTime($data['latest_child_timestamp']); } $data['children_count'] = (int)$data['children_count']; + $data['reference_id'] = $data['reference_id'] ?? null; return $data; } diff --git a/lib/public/Comments/IComment.php b/lib/public/Comments/IComment.php index aac68a4036ba1..b98a015a30e49 100644 --- a/lib/public/Comments/IComment.php +++ b/lib/public/Comments/IComment.php @@ -263,4 +263,21 @@ public function getObjectId(); */ public function setObject($objectType, $objectId); + /** + * returns the reference id of the comment + * + * @return string|null + * @since 19.0.0 + */ + public function getReferenceId(): ?string; + + /** + * sets (overwrites) the reference id of the comment + * + * @param string|null $referenceId e.g. sha256 hash sum + * @return IComment + * @since 19.0.0 + */ + public function setReferenceId(?string $referenceId): IComment; + } From 60d9384e5ec7e8d6f7ffa1c3ecc180ffaac710a3 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 11 Mar 2020 12:12:07 +0100 Subject: [PATCH 3/6] Optionally write the reference id into the database Signed-off-by: Joas Schilling --- lib/private/Comments/Manager.php | 83 +++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 22 deletions(-) diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index aad7cad020095..d04f3f965b348 100644 --- a/lib/private/Comments/Manager.php +++ b/lib/private/Comments/Manager.php @@ -29,6 +29,7 @@ namespace OC\Comments; use Doctrine\DBAL\Exception\DriverException; +use Doctrine\DBAL\Exception\InvalidFieldNameException; use OCP\Comments\CommentsEvent; use OCP\Comments\IComment; use OCP\Comments\ICommentsEventHandler; @@ -745,23 +746,46 @@ public function save(IComment $comment) { * @param IComment $comment * @return bool */ - protected function insert(IComment &$comment) { + 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(); - $affectedRows = $qb - ->insert('comments') - ->values([ - 'parent_id' => $qb->createNamedParameter($comment->getParentId()), - 'topmost_parent_id' => $qb->createNamedParameter($comment->getTopmostParentId()), - 'children_count' => $qb->createNamedParameter($comment->getChildrenCount()), - 'actor_type' => $qb->createNamedParameter($comment->getActorType()), - 'actor_id' => $qb->createNamedParameter($comment->getActorId()), - 'message' => $qb->createNamedParameter($comment->getMessage()), - 'verb' => $qb->createNamedParameter($comment->getVerb()), - 'creation_timestamp' => $qb->createNamedParameter($comment->getCreationDateTime(), 'datetime'), - 'latest_child_timestamp' => $qb->createNamedParameter($comment->getLatestChildDateTime(), 'datetime'), - 'object_type' => $qb->createNamedParameter($comment->getObjectType()), - 'object_id' => $qb->createNamedParameter($comment->getObjectId()), - ]) + + $values = [ + 'parent_id' => $qb->createNamedParameter($comment->getParentId()), + 'topmost_parent_id' => $qb->createNamedParameter($comment->getTopmostParentId()), + 'children_count' => $qb->createNamedParameter($comment->getChildrenCount()), + 'actor_type' => $qb->createNamedParameter($comment->getActorType()), + 'actor_id' => $qb->createNamedParameter($comment->getActorId()), + 'message' => $qb->createNamedParameter($comment->getMessage()), + 'verb' => $qb->createNamedParameter($comment->getVerb()), + 'creation_timestamp' => $qb->createNamedParameter($comment->getCreationDateTime(), 'datetime'), + 'latest_child_timestamp' => $qb->createNamedParameter($comment->getLatestChildDateTime(), 'datetime'), + 'object_type' => $qb->createNamedParameter($comment->getObjectType()), + 'object_id' => $qb->createNamedParameter($comment->getObjectId()), + ]; + + if ($tryWritingReferenceId) { + $values['reference_id'] = $qb->createNamedParameter($comment->getReferenceId()); + } + + $affectedRows = $qb->insert('comments') + ->values($values) ->execute(); if ($affectedRows > 0) { @@ -786,8 +810,21 @@ 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); + } + + $this->sendEvent(CommentsEvent::EVENT_UPDATE, $comment); + + return $result; + } + + protected function updateQuery(IComment $comment, bool $tryWritingReferenceId): bool { $qb = $this->dbConn->getQueryBuilder(); - $affectedRows = $qb + $qb ->update('comments') ->set('parent_id', $qb->createNamedParameter($comment->getParentId())) ->set('topmost_parent_id', $qb->createNamedParameter($comment->getTopmostParentId())) @@ -799,17 +836,19 @@ protected function update(IComment $comment) { ->set('creation_timestamp', $qb->createNamedParameter($comment->getCreationDateTime(), 'datetime')) ->set('latest_child_timestamp', $qb->createNamedParameter($comment->getLatestChildDateTime(), 'datetime')) ->set('object_type', $qb->createNamedParameter($comment->getObjectType())) - ->set('object_id', $qb->createNamedParameter($comment->getObjectId())) - ->where($qb->expr()->eq('id', $qb->createParameter('id'))) - ->setParameter('id', $comment->getId()) + ->set('object_id', $qb->createNamedParameter($comment->getObjectId())); + + if ($tryWritingReferenceId) { + $qb->set('reference_id', $qb->createNamedParameter($comment->getReferenceId())); + } + + $affectedRows = $qb->where($qb->expr()->eq('id', $qb->createNamedParameter($comment->getId()))) ->execute(); if ($affectedRows === 0) { throw new NotFoundException('Comment to update does ceased to exist'); } - $this->sendEvent(CommentsEvent::EVENT_UPDATE, $comment); - return $affectedRows > 0; } From b974dd5b733db8b0bbf20a9cccbb99715992c545 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 1 Apr 2020 13:20:01 +0200 Subject: [PATCH 4/6] Update new test as well Signed-off-by: Joas Schilling --- core/js/tests/specs/setupchecksSpec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/core/js/tests/specs/setupchecksSpec.js b/core/js/tests/specs/setupchecksSpec.js index 891b5fc84c896..f4c81c6bf78ed 100644 --- a/core/js/tests/specs/setupchecksSpec.js +++ b/core/js/tests/specs/setupchecksSpec.js @@ -989,6 +989,7 @@ describe('OC.SetupChecks tests', function() { isSettimelimitAvailable: true, hasFreeTypeSupport: true, missingIndexes: [], + missingColumns: [], cronErrors: [], cronInfo: { diffInSeconds: 0 From 81455479bc53e7703da6e6f990de7ffd797888f8 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 2 Apr 2020 09:59:44 +0200 Subject: [PATCH 5/6] Fix unit tests Signed-off-by: Joas Schilling --- .../tests/unit/Comments/CommentsNodeTest.php | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/apps/dav/tests/unit/Comments/CommentsNodeTest.php b/apps/dav/tests/unit/Comments/CommentsNodeTest.php index 9d0e56041d03a..4f6d96aa7f274 100644 --- a/apps/dav/tests/unit/Comments/CommentsNodeTest.php +++ b/apps/dav/tests/unit/Comments/CommentsNodeTest.php @@ -107,7 +107,7 @@ public function testDelete() { $this->node->delete(); } - + public function testDeleteForbidden() { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); @@ -149,7 +149,7 @@ public function testGetName() { $this->assertSame($this->node->getName(), $id); } - + public function testSetName() { $this->expectException(\Sabre\DAV\Exception\MethodNotAllowed::class); @@ -194,7 +194,7 @@ public function testUpdateComment() { $this->assertTrue($this->node->updateComment($msg)); } - + public function testUpdateCommentLogException() { $this->expectException(\Exception::class); $this->expectExceptionMessage('buh!'); @@ -235,7 +235,7 @@ public function testUpdateCommentLogException() { $this->node->updateComment($msg); } - + public function testUpdateCommentMessageTooLongException() { $this->expectException(\Sabre\DAV\Exception\BadRequest::class); $this->expectExceptionMessage('Message exceeds allowed character limit of'); @@ -274,7 +274,7 @@ public function testUpdateCommentMessageTooLongException() { $this->node->updateComment('foo'); } - + public function testUpdateForbiddenByUser() { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); @@ -309,7 +309,7 @@ public function testUpdateForbiddenByUser() { $this->node->updateComment($msg); } - + public function testUpdateForbiddenByType() { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); @@ -339,7 +339,7 @@ public function testUpdateForbiddenByType() { $this->node->updateComment($msg); } - + public function testUpdateForbiddenByNotLoggedIn() { $this->expectException(\Sabre\DAV\Exception\Forbidden::class); @@ -402,6 +402,7 @@ public function testGetProperties() { $ns . 'latestChildDateTime' => new \DateTime('2016-01-12 18:48:00'), $ns . 'objectType' => 'files', $ns . 'objectId' => '1848', + $ns . 'referenceId' => 'ref', $ns . 'isUnread' => null, ]; @@ -468,6 +469,10 @@ public function testGetProperties() { ->method('getObjectId') ->willReturn($expected[$ns . 'objectId']); + $this->comment->expects($this->once()) + ->method('getReferenceId') + ->willReturn($expected[$ns . 'referenceId']); + $user = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); @@ -483,7 +488,7 @@ public function testGetProperties() { $properties = $this->node->getProperties(null); foreach($properties as $name => $value) { - $this->assertTrue(array_key_exists($name, $expected)); + $this->assertArrayHasKey($name, $expected); $this->assertSame($expected[$name], $value); unset($expected[$name]); } From 1f5ba56235349ad811329b80cc1a2338dc8c79c7 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 2 Apr 2020 10:02:05 +0200 Subject: [PATCH 6/6] Update autoloader Signed-off-by: Joas Schilling --- lib/composer/composer/autoload_classmap.php | 2 ++ lib/composer/composer/autoload_static.php | 2 ++ 2 files changed, 4 insertions(+) diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 388c7906eb81c..f1cbfbf43935e 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -729,6 +729,7 @@ 'OC\\Core\\Command\\Config\\System\\DeleteConfig' => $baseDir . '/core/Command/Config/System/DeleteConfig.php', 'OC\\Core\\Command\\Config\\System\\GetConfig' => $baseDir . '/core/Command/Config/System/GetConfig.php', 'OC\\Core\\Command\\Config\\System\\SetConfig' => $baseDir . '/core/Command/Config/System/SetConfig.php', + 'OC\\Core\\Command\\Db\\AddMissingColumns' => $baseDir . '/core/Command/Db/AddMissingColumns.php', 'OC\\Core\\Command\\Db\\AddMissingIndices' => $baseDir . '/core/Command/Db/AddMissingIndices.php', 'OC\\Core\\Command\\Db\\ConvertFilecacheBigInt' => $baseDir . '/core/Command/Db/ConvertFilecacheBigInt.php', 'OC\\Core\\Command\\Db\\ConvertMysqlToMB4' => $baseDir . '/core/Command/Db/ConvertMysqlToMB4.php', @@ -862,6 +863,7 @@ 'OC\\DB\\MigrationException' => $baseDir . '/lib/private/DB/MigrationException.php', 'OC\\DB\\MigrationService' => $baseDir . '/lib/private/DB/MigrationService.php', 'OC\\DB\\Migrator' => $baseDir . '/lib/private/DB/Migrator.php', + 'OC\\DB\\MissingColumnInformation' => $baseDir . '/lib/private/DB/MissingColumnInformation.php', 'OC\\DB\\MissingIndexInformation' => $baseDir . '/lib/private/DB/MissingIndexInformation.php', 'OC\\DB\\MySQLMigrator' => $baseDir . '/lib/private/DB/MySQLMigrator.php', 'OC\\DB\\MySqlTools' => $baseDir . '/lib/private/DB/MySqlTools.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index cfc6d9842dfa7..ebad89663843b 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -758,6 +758,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Core\\Command\\Config\\System\\DeleteConfig' => __DIR__ . '/../../..' . '/core/Command/Config/System/DeleteConfig.php', 'OC\\Core\\Command\\Config\\System\\GetConfig' => __DIR__ . '/../../..' . '/core/Command/Config/System/GetConfig.php', 'OC\\Core\\Command\\Config\\System\\SetConfig' => __DIR__ . '/../../..' . '/core/Command/Config/System/SetConfig.php', + 'OC\\Core\\Command\\Db\\AddMissingColumns' => __DIR__ . '/../../..' . '/core/Command/Db/AddMissingColumns.php', 'OC\\Core\\Command\\Db\\AddMissingIndices' => __DIR__ . '/../../..' . '/core/Command/Db/AddMissingIndices.php', 'OC\\Core\\Command\\Db\\ConvertFilecacheBigInt' => __DIR__ . '/../../..' . '/core/Command/Db/ConvertFilecacheBigInt.php', 'OC\\Core\\Command\\Db\\ConvertMysqlToMB4' => __DIR__ . '/../../..' . '/core/Command/Db/ConvertMysqlToMB4.php', @@ -891,6 +892,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\DB\\MigrationException' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationException.php', 'OC\\DB\\MigrationService' => __DIR__ . '/../../..' . '/lib/private/DB/MigrationService.php', 'OC\\DB\\Migrator' => __DIR__ . '/../../..' . '/lib/private/DB/Migrator.php', + 'OC\\DB\\MissingColumnInformation' => __DIR__ . '/../../..' . '/lib/private/DB/MissingColumnInformation.php', 'OC\\DB\\MissingIndexInformation' => __DIR__ . '/../../..' . '/lib/private/DB/MissingIndexInformation.php', 'OC\\DB\\MySQLMigrator' => __DIR__ . '/../../..' . '/lib/private/DB/MySQLMigrator.php', 'OC\\DB\\MySqlTools' => __DIR__ . '/../../..' . '/lib/private/DB/MySqlTools.php',