Skip to content

Commit 01038ff

Browse files
dartcafedartcafe
authored
Merge pull request #4187 from nextcloud/enh/janitor-cron-purge-orphaned-votes
add janitor job: remove orphaned votes
2 parents c02520f + 8c95671 commit 01038ff

12 files changed

Lines changed: 107 additions & 185 deletions

File tree

‎lib/AppInfo/Application.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
use OCA\Polls\Event\ShareLockedEvent;
4343
use OCA\Polls\Event\ShareRegistrationEvent;
4444
use OCA\Polls\Event\ShareTypeChangedEvent;
45-
use OCA\Polls\Event\VoteDeletedOrphanedEvent;
4645
use OCA\Polls\Event\VoteEvent;
4746
use OCA\Polls\Event\VoteSetEvent;
4847
use OCA\Polls\Listener\CommentListener;
@@ -127,7 +126,6 @@ public function register(IRegistrationContext $context): void {
127126

128127
$context->registerEventListener(VoteEvent::class, VoteListener::class);
129128
$context->registerEventListener(VoteSetEvent::class, VoteListener::class);
130-
$context->registerEventListener(VoteDeletedOrphanedEvent::class, VoteListener::class);
131129
$context->registerEventListener(UserDeletedEvent::class, UserDeletedListener::class);
132130
$context->registerEventListener(GroupDeletedEvent::class, GroupDeletedListener::class);
133131

‎lib/Command/Db/removeOrphanedVotes.php‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* SPDX-FileCopyrightText: 2021 Nextcloud contributors
6+
* SPDX-License-Identifier: AGPL-3.0-or-later
7+
*/
8+
9+
namespace OCA\Polls\Command\Db;
10+
11+
use Doctrine\DBAL\Schema\Schema;
12+
use OCA\Polls\Command\Command;
13+
use OCA\Polls\Db\IndexManager;
14+
use OCP\IDBConnection;
15+
16+
/**
17+
* @psalm-api
18+
*/
19+
class removeOrphanedVotes extends Command {
20+
protected string $name = parent::NAME_PREFIX . 'index:create';
21+
protected string $description = 'Add all indices and foreign key constraints';
22+
protected array $operationHints = [
23+
'Adds indices and foreing key constraints.',
24+
'NO data migration will be executed, so make sure you have a backup of your database.',
25+
];
26+
27+
public function __construct(
28+
private IndexManager $indexManager,
29+
private IDBConnection $connection,
30+
private Schema $schema,
31+
) {
32+
parent::__construct();
33+
}
34+
35+
protected function runCommands(): int {
36+
// create indices and constraints
37+
// secure, that the schema is updated to the current status
38+
$this->schema = $this->connection->createSchema();
39+
$this->indexManager->setSchema($this->schema);
40+
$this->addForeignKeyConstraints();
41+
$this->addIndices();
42+
$this->connection->migrateToSchema($this->schema);
43+
44+
return 0;
45+
}
46+
47+
/**
48+
* add an on delete fk contraint to all tables referencing the main polls table
49+
*/
50+
private function addForeignKeyConstraints(): void {
51+
$this->printComment('Add foreign key constraints');
52+
$messages = $this->indexManager->createForeignKeyConstraints();
53+
$this->printInfo($messages, ' - ');
54+
}
55+
56+
/**
57+
* Create index for $table
58+
*/
59+
private function addIndices(): void {
60+
$this->printComment('Add indices');
61+
$messages = $this->indexManager->createIndices();
62+
$this->printInfo($messages, ' - ');
63+
}
64+
}

‎lib/Controller/PollApiController.php‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ public function get(int $pollId): DataResponse {
6767
'poll' => $this->pollService->get($pollId),
6868
'options' => $this->optionService->list($pollId),
6969
'votes' => $this->voteService->list($pollId),
70-
'orphaned' => count($this->voteService->getOprhanedVotes($pollId)),
7170
'comments' => $this->commentService->list($pollId),
7271
'shares' => $this->shareService->list($pollId),
7372
'subscribed' => $this->subscriptionService->get($pollId),
@@ -196,20 +195,6 @@ public function getParticipantsEmailAddresses(int $pollId): DataResponse {
196195
return $this->response(fn () => ['addresses' => $this->pollService->getParticipantsEmailAddresses($pollId)]);
197196
}
198197

199-
/**
200-
* Delete orphaned votes from pollId
201-
* @param int $pollId poll id
202-
*/
203-
#[CORS]
204-
#[NoAdminRequired]
205-
#[NoCSRFRequired]
206-
#[ApiRoute(verb: 'DELETE', url: '/api/v1.0/poll/{pollId}/votes/orphaned/all', requirements: ['apiVersion' => '(v2)'])]
207-
public function deleteOrphaned(int $pollId): DataResponse {
208-
return $this->response(fn () => [
209-
'deleted' => $this->voteService->deleteOrphanedVotes($pollId)
210-
]);
211-
}
212-
213198
#[CORS]
214199
#[NoAdminRequired]
215200
#[NoCSRFRequired]

‎lib/Controller/PollController.php‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,6 @@ private function getFullPoll(int $pollId, bool $withTimings = false): array {
121121
$votes = $this->voteService->list($pollId);
122122
$timerMicro['votes'] = microtime(true);
123123

124-
$orphaned = $this->voteService->getOprhanedVotes($pollId);
125-
$timerMicro['orphaned'] = microtime(true);
126-
127124
$comments = $this->commentService->list($pollId);
128125
$timerMicro['comments'] = microtime(true);
129126

@@ -137,8 +134,7 @@ private function getFullPoll(int $pollId, bool $withTimings = false): array {
137134
$diffMicro['poll'] = $timerMicro['poll'] - $timerMicro['start'];
138135
$diffMicro['options'] = $timerMicro['options'] - $timerMicro['poll'];
139136
$diffMicro['votes'] = $timerMicro['votes'] - $timerMicro['options'];
140-
$diffMicro['orphaned'] = $timerMicro['orphaned'] - $timerMicro['votes'];
141-
$diffMicro['comments'] = $timerMicro['comments'] - $timerMicro['orphaned'];
137+
$diffMicro['comments'] = $timerMicro['comments'] - $timerMicro['votes'];
142138
$diffMicro['shares'] = $timerMicro['shares'] - $timerMicro['comments'];
143139
$diffMicro['subscribed'] = $timerMicro['subscribed'] - $timerMicro['shares'];
144140

@@ -147,7 +143,6 @@ private function getFullPoll(int $pollId, bool $withTimings = false): array {
147143
'poll' => $poll,
148144
'options' => $options,
149145
'votes' => $votes,
150-
'orphaned' => count($orphaned),
151146
'comments' => $comments,
152147
'shares' => $shares,
153148
'subscribed' => $subscribed,
@@ -158,7 +153,6 @@ private function getFullPoll(int $pollId, bool $withTimings = false): array {
158153
'poll' => $poll,
159154
'options' => $options,
160155
'votes' => $votes,
161-
'orphaned' => count($orphaned),
162156
'comments' => $comments,
163157
'shares' => $shares,
164158
'subscribed' => $subscribed,
@@ -329,17 +323,4 @@ public function getParticipantsEmailAddresses(int $pollId): JSONResponse {
329323
return $this->response(fn () => $this->pollService->getParticipantsEmailAddresses($pollId));
330324
}
331325

332-
/**
333-
* Delete orphaned votes
334-
* @param int $pollId poll id
335-
*/
336-
#[NoAdminRequired]
337-
#[OpenAPI(OpenAPI::SCOPE_IGNORE)]
338-
#[FrontpageRoute(verb: 'DELETE', url: '/poll/{pollId}/votes/orphaned/all')]
339-
public function deleteOrphaned(int $pollId): JSONResponse {
340-
return $this->response(fn () => [
341-
'deleted' => $this->voteService->deleteOrphanedVotes($pollId)
342-
]);
343-
}
344-
345326
}

‎lib/Cron/JanitorCron.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OCA\Polls\Db\PollMapper;
1717
use OCA\Polls\Db\ShareMapper;
1818
use OCA\Polls\Db\TableManager;
19+
use OCA\Polls\Db\VoteMapper;
1920
use OCA\Polls\Db\WatchMapper;
2021
use OCA\Polls\Helper\Container;
2122
use OCA\Polls\Model\Settings\AppSettings;
@@ -39,6 +40,7 @@ public function __construct(
3940
private OptionMapper $optionMapper,
4041
private PollMapper $pollMapper,
4142
private ShareMapper $shareMapper,
43+
private VoteMapper $voteMapper,
4244
private WatchMapper $watchMapper,
4345
private TableManager $tableManager,
4446
) {
@@ -65,11 +67,14 @@ protected function run($argument) {
6567
// delete entries older than 1 day
6668
$this->watchMapper->deleteOldEntries(time() - 86400);
6769

68-
// purge entries virtually deleted more than 12 hour ago
70+
// purge entries virtually deleted more than 12 hours ago
6971
$deleted['comments'] = $this->commentMapper->purgeDeletedComments(time() - 4320);
7072
$deleted['options'] = $this->optionMapper->purgeDeletedOptions(time() - 4320);
7173
$deleted['shares'] = $this->shareMapper->purgeDeletedShares(time() - 4320);
7274

75+
// purge orphaned votes; Votes without any corresponding option
76+
$deleted['orphaned votes'] = $this->voteMapper->removeOrphanedVotes();
77+
7378
// delete polls after defined days after archiving date
7479
$autoDeleteOffset = $this->appSettings->getAutoDeleteOffsetDays();
7580
if ($this->appSettings->getAutoDeleteEnabled() && $autoDeleteOffset > 0) {

‎lib/Db/VoteMapper.php‎

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OCP\AppFramework\Db\Entity;
1414
use OCP\DB\QueryBuilder\IQueryBuilder;
1515
use OCP\IDBConnection;
16+
use PDO;
1617
use Psr\Log\LoggerInterface;
1718

1819
/**
@@ -155,25 +156,43 @@ public function findOrphanedByPollandUser(int $pollId, string $userId): array {
155156
}
156157

157158
/**
158-
* @throws \OCP\AppFramework\Db\DoesNotExistException if not found
159-
* @return Vote[]
160-
* @psalm-return array<array-key, Vote>
159+
* Remove all votes that no more belong to any existing option
160+
*
161+
* @return int Number of deleted votes
161162
*/
162-
public function findOrphanedByPoll(int $pollId): array {
163+
public function removeOrphanedVotes(): int {
163164
$qb = $this->db->getQueryBuilder();
164165

165-
$qb->select(self::TABLE . '.*')
166-
->where($qb->expr()->isNotNull(self::TABLE . '.poll_id'))
167-
->from($this->getTableName(), self::TABLE)
168-
->groupBy(self::TABLE . '.id');
166+
// get ids of votes that have no option
167+
$qb->select('votes.id');
168+
$qb->from($this->getTableName(), 'votes');
169+
$qb->leftJoin(
170+
'votes',
171+
Option::TABLE,
172+
'options',
173+
'votes.poll_id = options.poll_id AND votes.vote_option_text = options.poll_option_text'
174+
);
175+
$qb->where('options.poll_id IS NULL');
169176

170-
$optionAlias = $this->joinOption($qb, self::TABLE);
177+
// get the ids as array
178+
$idsToDelete = $qb->executeQuery()->fetchAll(PDO::FETCH_COLUMN);
171179

172-
$qb->andWhere($qb->expr()->isNull($optionAlias . '.id'));
173-
$qb->andWhere($qb->expr()->eq(self::TABLE . '.poll_id', $qb->createNamedParameter($pollId, IQueryBuilder::PARAM_INT)));
174-
return $this->findEntities($qb);
175-
}
180+
if (empty($idsToDelete)) {
181+
return 0;
182+
}
183+
184+
// delete all votes contained in the id array
185+
$delete = $this->db->getQueryBuilder()
186+
->delete($this->getTableName())
187+
->where('id IN (:ids)')
188+
->setParameter('ids', $idsToDelete, IQueryBuilder::PARAM_INT_ARRAY);
176189

190+
$this->logger->debug('Removing orphaned votes', [
191+
'ids' => $idsToDelete,
192+
]);
193+
194+
return $delete->executeStatement();
195+
}
177196
/**
178197
* Build the enhanced query with joined tables
179198
*/

‎lib/Event/VoteDeletedOrphanedEvent.php‎

Lines changed: 0 additions & 24 deletions
This file was deleted.

‎lib/Service/VoteService.php‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use OCA\Polls\Db\PollMapper;
1515
use OCA\Polls\Db\Vote;
1616
use OCA\Polls\Db\VoteMapper;
17-
use OCA\Polls\Event\VoteDeletedOrphanedEvent;
1817
use OCA\Polls\Event\VoteSetEvent;
1918
use OCA\Polls\Exceptions\ForbiddenException;
2019
use OCA\Polls\Exceptions\NotFoundException;
@@ -133,43 +132,6 @@ public function set(Option|int $optionOrOptionIdoptionId, string $setTo): ?Vote
133132
return $this->vote;
134133
}
135134

136-
/**
137-
* Get all votes of a poll, which are not assigned to an option
138-
*
139-
* @param int $pollId poll id of the poll the votes get deleted from
140-
* @return Vote[]
141-
*/
142-
public function getOprhanedVotes(int $pollId): array {
143-
try {
144-
$this->pollMapper->get($pollId, true, withRoles: true)
145-
->request(Poll::PERMISSION_POLL_EDIT);
146-
return $this->voteMapper->findOrphanedByPoll($pollId);
147-
} catch (ForbiddenException $e) {
148-
return [];
149-
}
150-
}
151-
152-
/**
153-
* Delete all votes of a poll, which are not assigned to an option
154-
*
155-
* @param int $pollId poll id of the poll the votes get deleted from
156-
* @return Vote[]
157-
*/
158-
public function deleteOrphanedVotes(int $pollId): array {
159-
$this->pollMapper->get($pollId, withRoles: true)
160-
->request(Poll::PERMISSION_VOTE_FOREIGN_CHANGE);
161-
162-
// delete all votes of the poll, which are not assigned to an option
163-
$votes = $this->voteMapper->findOrphanedByPoll($pollId);
164-
foreach ($votes as $vote) {
165-
$this->voteMapper->delete($vote);
166-
// TODO: rework notification methods
167-
// keep this dispatch as reminder
168-
// $this->eventDispatcher->dispatchTyped(new VoteDeletedOrphanedEvent($this->vote, false));
169-
}
170-
return $votes;
171-
}
172-
173135
/**
174136
* Remove user from poll
175137
*

‎src/Api/modules/api.types.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ export type FullPollResponse = {
99
poll: Poll
1010
options: Option[]
1111
votes: Vote[]
12-
orphaned: number
1312
comments: Comment[]
1413
shares: Share[]
1514
subscribed: boolean

‎src/Api/modules/polls.ts‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import { Poll, PollConfiguration, PollType } from '../../stores/poll.ts'
66
import { AxiosResponse } from '@nextcloud/axios'
77
import { httpInstance, createCancelTokenHandler } from './HttpApi.js'
8-
import { ApiEmailAdressList, Vote } from '../../Types/index.ts'
8+
import { ApiEmailAdressList } from '../../Types/index.ts'
99
import { PollGroup } from '../../stores/pollGroups.types.ts'
1010
import { FullPollResponse } from './api.types.ts'
1111

@@ -145,19 +145,6 @@ const polls = {
145145
})
146146
},
147147

148-
removeOrphanedVotes(
149-
pollId: number,
150-
): Promise<AxiosResponse<{ deleted: Vote[] }>> {
151-
return httpInstance.request({
152-
method: 'DELETE',
153-
url: `poll/${pollId}/votes/orphaned/all`,
154-
cancelToken:
155-
cancelTokenHandlerObject[
156-
this.removeOrphanedVotes.name
157-
].handleRequestCancellation().token,
158-
})
159-
},
160-
161148
closePoll(pollId: number): Promise<AxiosResponse<{ poll: Poll }>> {
162149
return httpInstance.request({
163150
method: 'PUT',

0 commit comments

Comments
 (0)