diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 35263a32b..a23cf4974 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -59,6 +59,7 @@ use OCP\User\Events\UserDeletedEvent; use OCP\Util; use Psr\Container\ContainerInterface; +use Psr\Log\LoggerInterface; class Application extends App implements IBootstrap { public const APP_ID = 'activity'; @@ -110,7 +111,8 @@ public function register(IRegistrationContext $context): void { $context->registerService(Data::class, function (ContainerInterface $c) { return new Data( $c->get(IManager::class), - $c->get('ActivityConnectionAdapter') + $c->get('ActivityConnectionAdapter'), + $c->get(LoggerInterface::class), ); }); diff --git a/lib/Data.php b/lib/Data.php index 5143a8454..23be17f10 100755 --- a/lib/Data.php +++ b/lib/Data.php @@ -1,4 +1,6 @@ activityManager = $activityManager; $this->connection = $connection; + $this->logger = $logger; } /** @@ -368,9 +373,16 @@ public function expire($expireDays = 365) { * 'field' => 'value' => `field` = 'value' * 'field' => array('value', 'operator') => `field` operator 'value' */ - public function deleteActivities($conditions) { - $delete = $this->connection->getQueryBuilder(); - $delete->delete('activity'); + public function deleteActivities($conditions): void { + $platform = $this->connection->getDatabasePlatform(); + if($platform instanceof MySQLPlatform) { + $this->logger->debug('Choosing chunked activity delete for MySQL/MariaDB', ['app' => 'activity']); + $this->deleteActivitiesForMySQL($conditions); + return; + } + $this->logger->debug('Choosing regular activity delete', ['app' => 'activity']); + $deleteQuery = $this->connection->getQueryBuilder(); + $deleteQuery->delete('activity'); foreach ($conditions as $column => $comparison) { if (is_array($comparison)) { @@ -381,22 +393,14 @@ public function deleteActivities($conditions) { $value = $comparison; } - $delete->andWhere($delete->expr()->comparison($column, $operation, $delete->createNamedParameter($value))); + $deleteQuery->andWhere($deleteQuery->expr()->comparison($column, $operation, $deleteQuery->createNamedParameter($value))); } - // Add galera safe delete chunking if using mysql - // Stops us hitting wsrep_max_ws_rows when large row counts are deleted - if ($this->connection->getDatabasePlatform() instanceof MySQLPlatform) { - // Then use chunked delete - $max = 100000; - $delete->setMaxResults($max); - do { - $deleted = $delete->executeStatement(); - } while ($deleted === $max); - } else { - // Dont use chunked delete - let the DB handle the large row count natively - $delete->executeStatement(); - } + + + + // Dont use chunked delete - let the DB handle the large row count natively + $deleteQuery->executeStatement(); } public function getById(int $activityId): ?IEvent { @@ -467,4 +471,48 @@ public function getActivitySince(string $user, int $since, bool $byOthers) { return $query->execute()->fetch(); } + + /** + * Add galera safe delete chunking if using mysql + * Stops us hitting wsrep_max_ws_rows when large row counts are deleted + * + * @param array $conditions + * @return void + */ + private function deleteActivitiesForMySQL(array $conditions): void { + $query = $this->connection->getQueryBuilder(); + $query->select('activity_id') + ->from('activity'); + + foreach ($conditions as $column => $comparison) { + if (is_array($comparison)) { + $operation = $comparison[1] ?? '='; + $value = $comparison[0]; + } else { + $operation = '='; + $value = $comparison; + } + $query->where($query->expr()->comparison($column, $operation, $query->createNamedParameter($value))); + } + + $query->setMaxResults(10000); + $result = $query->executeQuery(); + $count = $result->rowCount(); + if($count === 0) { + return; + } + $ids = array_map(static function (array $id) { + return (int)$id[0]; + }, $result->fetchAll(\PDO::FETCH_NUM)); + $result->closeCursor(); + + $deleteQuery = $this->connection->getQueryBuilder(); + $deleteQuery->delete('activity'); + $deleteQuery->where($deleteQuery->expr()->in('activity_id', $deleteQuery->createParameter('ids'), IQueryBuilder::PARAM_INT_ARRAY)); + $deleteQuery->setParameter('ids', $ids, IQueryBuilder::PARAM_INT_ARRAY); + $queryResult = $deleteQuery->executeStatement(); + if($queryResult === 10000) { + $this->deleteActivitiesForMySQL($conditions); + } + } } diff --git a/psalm.xml b/psalm.xml index b8e1039ad..7bebfdf76 100644 --- a/psalm.xml +++ b/psalm.xml @@ -17,6 +17,7 @@ + diff --git a/tests/Controller/APIv1ControllerTest.php b/tests/Controller/APIv1ControllerTest.php index 63f883c76..2babdeb17 100644 --- a/tests/Controller/APIv1ControllerTest.php +++ b/tests/Controller/APIv1ControllerTest.php @@ -41,6 +41,7 @@ use OCP\IRequest; use OCP\IUserSession; use OCP\RichObjectStrings\IValidator; +use Psr\Log\LoggerInterface; /** * Class APIv1Test @@ -106,7 +107,8 @@ protected function tearDown(): void { protected function cleanUp(): void { $data = new Data( $this->createMock(IManager::class), - \OC::$server->getDatabaseConnection() + \OC::$server->getDatabaseConnection(), + $this->createMock(LoggerInterface::class) ); $this->deleteUser($data, 'activity-api-user1'); @@ -220,7 +222,10 @@ public function testGet(string $user, int $start, int $count, array $expected): ->method('getUID') ->willReturn($user); - $data = new Data($activityManager, \OC::$server->getDatabaseConnection()); + $data = new Data($activityManager, + \OC::$server->getDatabaseConnection(), + $this->createMock(LoggerInterface::class) + ); $controller = new APIv1Controller( 'activity', diff --git a/tests/DataDeleteActivitiesTest.php b/tests/DataDeleteActivitiesTest.php index 037b94eb2..7f1cee91a 100644 --- a/tests/DataDeleteActivitiesTest.php +++ b/tests/DataDeleteActivitiesTest.php @@ -32,7 +32,7 @@ use OCP\BackgroundJob\IJobList; use OCP\DB\IPreparedStatement; use OCP\IConfig; -use OCP\IUserSession; +use Psr\Log\LoggerInterface; /** * Class DataDeleteActivitiesTest @@ -74,7 +74,7 @@ protected function setUp(): void { $this->data = new Data( $this->createMock(IManager::class), \OC::$server->getDatabaseConnection(), - $this->createMock(IUserSession::class) + $this->createMock(LoggerInterface::class) ); } diff --git a/tests/DataTest.php b/tests/DataTest.php index 195b2b163..bc8b16f7f 100644 --- a/tests/DataTest.php +++ b/tests/DataTest.php @@ -31,6 +31,7 @@ use OCP\IUserSession; use OCP\Util; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Log\NullLogger; /** * Class DataTest @@ -54,6 +55,9 @@ class DataTest extends TestCase { /** @var IManager */ protected $realActivityManager; + /** @var NullLogger */ + protected $logger; + protected function setUp(): void { parent::setUp(); @@ -61,10 +65,12 @@ protected function setUp(): void { $activityManager = $this->createMock(IManager::class); $this->dbConnection = \OC::$server->get(IDBConnection::class); $this->realActivityManager = \OC::$server->get(IManager::class); + $this->logger = \OC::$server->get(NullLogger::class); $this->data = new Data( $activityManager, - $this->dbConnection + $this->dbConnection, + $this->logger ); }