Skip to content

Commit cf3109e

Browse files
juliusknorrjuliusknorr
committed
fix: Delete inactive sessions in one query
Signed-off-by: Julius Härtl <[email protected]>
1 parent 55f5a07 commit cf3109e

6 files changed

Lines changed: 151 additions & 18 deletions

File tree

‎appinfo/info.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
- **💾 Open format:** Files are saved as [Markdown](https://en.wikipedia.org/wiki/Markdown), so you can edit them from any other text app too.
1212
- **✊ Strong foundation:** We use [🐈 tiptap](https://tiptap.scrumpy.io) which is based on [🦉 ProseMirror](https://prosemirror.net) – huge thanks to them!
1313
]]></description>
14-
<version>3.9.0</version>
14+
<version>3.9.1</version>
1515
<licence>agpl</licence>
1616
<author mail="[email protected]">Julius Härtl</author>
1717
<namespace>Text</namespace>

‎composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
'OCA\\Text\\Migration\\Version030201Date20201116123153' => $baseDir . '/../lib/Migration/Version030201Date20201116123153.php',
5151
'OCA\\Text\\Migration\\Version030501Date20220202101853' => $baseDir . '/../lib/Migration/Version030501Date20220202101853.php',
5252
'OCA\\Text\\Migration\\Version030701Date20230207131313' => $baseDir . '/../lib/Migration/Version030701Date20230207131313.php',
53+
'OCA\\Text\\Migration\\Version030901Date20230615085512' => $baseDir . '/../lib/Migration/Version030901Date20230615085512.php',
5354
'OCA\\Text\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php',
5455
'OCA\\Text\\Service\\ApiService' => $baseDir . '/../lib/Service/ApiService.php',
5556
'OCA\\Text\\Service\\AttachmentService' => $baseDir . '/../lib/Service/AttachmentService.php',

‎composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ class ComposerStaticInitText
6565
'OCA\\Text\\Migration\\Version030201Date20201116123153' => __DIR__ . '/..' . '/../lib/Migration/Version030201Date20201116123153.php',
6666
'OCA\\Text\\Migration\\Version030501Date20220202101853' => __DIR__ . '/..' . '/../lib/Migration/Version030501Date20220202101853.php',
6767
'OCA\\Text\\Migration\\Version030701Date20230207131313' => __DIR__ . '/..' . '/../lib/Migration/Version030701Date20230207131313.php',
68+
'OCA\\Text\\Migration\\Version030901Date20230615085512' => __DIR__ . '/..' . '/../lib/Migration/Version030901Date20230615085512.php',
6869
'OCA\\Text\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php',
6970
'OCA\\Text\\Service\\ApiService' => __DIR__ . '/..' . '/../lib/Service/ApiService.php',
7071
'OCA\\Text\\Service\\AttachmentService' => __DIR__ . '/..' . '/../lib/Service/AttachmentService.php',

‎lib/Db/SessionMapper.php‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,30 +99,29 @@ public function findAllInactive() {
9999
return $this->findEntities($qb);
100100
}
101101

102-
public function deleteInactiveWithoutSteps(?int $documentId = null) {
103-
$qb = $this->db->getQueryBuilder();
104-
$qb->select('session_id')
105-
->from('text_steps');
102+
public function deleteInactiveWithoutSteps(?int $documentId = null): int {
103+
$selectSubQuery = $this->db->getQueryBuilder();
104+
$selectSubQuery->select('s.id')
105+
->from('text_sessions', 's')
106+
->leftJoin('s', 'text_steps', 'st', $selectSubQuery->expr()->eq('st.session_id', 's.id'))
107+
->where($selectSubQuery->expr()->lt('last_contact', $selectSubQuery->createParameter('lastContact')))
108+
->andWhere($selectSubQuery->expr()->isNull('st.id'));
106109
if ($documentId !== null) {
107-
$qb->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId)));
110+
$selectSubQuery->andWhere($selectSubQuery->expr()->eq('s.document_id', $selectSubQuery->createParameter('documentId')));
108111
}
109-
$result = $qb
110-
->groupBy('session_id')
111-
->executeQuery();
112-
$activeSessions = $result->fetchAll(\PDO::FETCH_COLUMN);
113-
$result->closeCursor();
114112

115113
$qb = $this->db->getQueryBuilder();
116-
$qb->delete($this->getTableName());
117-
$qb->where($qb->expr()->lt('last_contact', $qb->createNamedParameter(time() - SessionService::SESSION_VALID_TIME)));
118-
if ($documentId !== null) {
119-
$qb->andWhere($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId)));
120-
}
121-
$qb->andWhere($qb->expr()->notIn('id', $qb->createNamedParameter($activeSessions, IQueryBuilder::PARAM_INT_ARRAY)));
114+
$qb->delete($this->getTableName())
115+
->where($qb->expr()->in('id', $qb->createFunction('(' . $selectSubQuery->getSQL() . ')')));
116+
$qb->setParameters([
117+
'lastContact' => time() - SessionService::SESSION_VALID_TIME,
118+
'documentId' => $documentId,
119+
]);
120+
122121
return $qb->executeStatement();
123122
}
124123

125-
public function deleteByDocumentId($documentId) {
124+
public function deleteByDocumentId($documentId): int {
126125
$qb = $this->db->getQueryBuilder();
127126
$qb->delete($this->getTableName())
128127
->where($qb->expr()->eq('document_id', $qb->createNamedParameter($documentId)));

‎lib/Migration/Version030901Date20230615085512.php‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OCA\Text\Migration;
6+
7+
use Closure;
8+
use OCP\DB\ISchemaWrapper;
9+
use OCP\Migration\IOutput;
10+
use OCP\Migration\SimpleMigrationStep;
11+
12+
class Version030901Date20230615085512 extends SimpleMigrationStep {
13+
14+
/**
15+
* @param IOutput $output
16+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
17+
* @param array $options
18+
* @return null|ISchemaWrapper
19+
*/
20+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
21+
/** @var ISchemaWrapper $schema */
22+
$schema = $schemaClosure();
23+
24+
$table = $schema->getTable('text_steps');
25+
if (!$table->hasIndex('ts_session')) {
26+
$table->addIndex(['session_id'], 'ts_session');
27+
return $schema;
28+
}
29+
30+
return null;
31+
}
32+
}

‎tests/unit/Db/SessionMapperTest.php‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
<?php
2+
3+
namespace OCA\Text\Db;
4+
5+
/**
6+
* @group DB
7+
*/
8+
class SessionMapperTest extends \Test\TestCase {
9+
10+
private SessionMapper $sessionMapper;
11+
private StepMapper $stepMapper;
12+
13+
public function setUp(): void {
14+
parent::setUp();
15+
$this->sessionMapper = \OCP\Server::get(SessionMapper::class);
16+
$this->stepMapper = \OCP\Server::get(StepMapper::class);
17+
18+
}
19+
20+
public function testDeleteInactiveWithoutSteps() {
21+
$this->sessionMapper->deleteByDocumentId(1);
22+
$this->sessionMapper->deleteByDocumentId(2);
23+
$this->sessionMapper->insert(Session::fromParams([
24+
'userId' => 'admin',
25+
'documentId' => 1,
26+
'lastContact' => 1337,
27+
'token' => uniqid(),
28+
'color' => '00ff00',
29+
]));
30+
$this->sessionMapper->insert(Session::fromParams([
31+
'userId' => 'admin',
32+
'documentId' => 2,
33+
'lastContact' => 1337,
34+
'token' => uniqid(),
35+
'color' => '00ff00',
36+
]));
37+
$this->sessionMapper->deleteInactiveWithoutSteps(1);
38+
self::assertCount(0, $this->sessionMapper->findAll(1));
39+
self::assertCount(1, $this->sessionMapper->findAll(2));
40+
$this->sessionMapper->deleteInactiveWithoutSteps();
41+
self::assertCount(0, $this->sessionMapper->findAll(2));
42+
}
43+
44+
public function testDeleteInactiveWithoutStepsKeep() {
45+
$this->stepMapper->deleteAll(1);
46+
$this->sessionMapper->deleteByDocumentId(1);
47+
$this->sessionMapper->deleteByDocumentId(2);
48+
49+
$s1 = $this->sessionMapper->insert(Session::fromParams([
50+
'userId' => 'admin',
51+
'documentId' => 1,
52+
'lastContact' => 1337,
53+
'token' => uniqid(),
54+
'color' => '00ff00',
55+
]));
56+
$s2 = $this->sessionMapper->insert(Session::fromParams([
57+
'userId' => 'admin',
58+
'documentId' => 2,
59+
'lastContact' => 1337,
60+
'token' => uniqid(),
61+
'color' => '00ff00',
62+
]));
63+
$this->stepMapper->insert(Step::fromParams([
64+
'sessionId' => $s1->getId(),
65+
'documentId' => 1,
66+
'data' => 'YJSDATA',
67+
'version' => 1,
68+
]));
69+
self::assertCount(1, $this->sessionMapper->findAll(1));
70+
71+
$this->sessionMapper->deleteInactiveWithoutSteps(1);
72+
self::assertCount(1, $this->sessionMapper->findAll(1));
73+
self::assertCount(1, $this->sessionMapper->findAll(2));
74+
75+
$this->sessionMapper->deleteInactiveWithoutSteps();
76+
self::assertCount(1, $this->sessionMapper->findAll(1));
77+
self::assertCount(0, $this->sessionMapper->findAll(2));
78+
}
79+
80+
public function testDeleteInactiveWithoutSteps1010() {
81+
$this->stepMapper->deleteAll(1);
82+
$this->sessionMapper->deleteByDocumentId(1);
83+
84+
for ($i = 0;$i < 1010;$i++) {
85+
$this->sessionMapper->insert(Session::fromParams([
86+
'userId' => 'admin',
87+
'documentId' => 1,
88+
'lastContact' => 1337,
89+
'token' => uniqid(),
90+
'color' => '00ff00',
91+
]));
92+
}
93+
94+
self::assertCount(1010, $this->sessionMapper->findAll(1));
95+
96+
$this->sessionMapper->deleteInactiveWithoutSteps(1);
97+
self::assertCount(0, $this->sessionMapper->findAll(1));
98+
}
99+
100+
}

0 commit comments

Comments
 (0)