Delete reactions when their parent message is deleted

This commit is contained in:
Scott Nonnenberg 2021-10-15 15:54:31 -07:00 committed by GitHub
parent e6ca3872d1
commit efde909484
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 185 additions and 8 deletions

View File

@ -221,6 +221,7 @@ const dataInterface: ClientInterface = {
markReactionAsRead,
removeReactionFromConversation,
addReaction,
_getAllReactions,
getMessageBySender,
getMessageById,
@ -1258,6 +1259,10 @@ async function addReaction(reactionObj: ReactionType) {
return channels.addReaction(reactionObj);
}
async function _getAllReactions() {
return channels._getAllReactions();
}
function handleMessageJSON(messages: Array<MessageTypeUnhydrated>) {
return messages.map(message => JSON.parse(message.json));
}

View File

@ -389,6 +389,7 @@ export type DataInterface = {
targetTimestamp: number;
}) => Promise<void>;
addReaction: (reactionObj: ReactionType) => Promise<void>;
_getAllReactions: () => Promise<Array<ReactionType>>;
getUnprocessedCount: () => Promise<number>;
getAllUnprocessed: () => Promise<Array<UnprocessedType>>;

View File

@ -203,6 +203,7 @@ const dataInterface: ServerInterface = {
markReactionAsRead,
addReaction,
removeReactionFromConversation,
_getAllReactions,
getMessageBySender,
getMessageById,
getMessagesById,
@ -2530,6 +2531,71 @@ function updateToSchemaVersion41(currentVersion: number, db: Database) {
logger.info('updateToSchemaVersion41: success!');
}
function updateToSchemaVersion42(currentVersion: number, db: Database) {
if (currentVersion >= 42) {
return;
}
db.transaction(() => {
// First, recreate messages table delete trigger with reaction support
db.exec(`
DROP TRIGGER messages_on_delete;
CREATE TRIGGER messages_on_delete AFTER DELETE ON messages BEGIN
DELETE FROM messages_fts WHERE rowid = old.rowid;
DELETE FROM sendLogPayloads WHERE id IN (
SELECT payloadId FROM sendLogMessageIds
WHERE messageId = old.id
);
DELETE FROM reactions WHERE rowid IN (
SELECT rowid FROM reactions
WHERE messageId = old.id
);
END;
`);
// Then, delete previously-orphaned reactions
// Note: we use `pluck` here to fetch only the first column of
// returned row.
const messageIdList: Array<string> = db
.prepare('SELECT id FROM messages ORDER BY id ASC;')
.pluck()
.all();
const allReactions: Array<{
rowid: number;
messageId: string;
}> = db.prepare('SELECT rowid, messageId FROM reactions;').all();
const messageIds = new Set(messageIdList);
const reactionsToDelete: Array<number> = [];
allReactions.forEach(reaction => {
if (!messageIds.has(reaction.messageId)) {
reactionsToDelete.push(reaction.rowid);
}
});
function deleteReactions(rowids: Array<number>) {
db.prepare<ArrayQuery>(
`
DELETE FROM reactions
WHERE rowid IN ( ${rowids.map(() => '?').join(', ')} );
`
).run(rowids);
}
if (reactionsToDelete.length > 0) {
logger.info(`Deleting ${reactionsToDelete.length} orphaned reactions`);
batchMultiVarQuery(reactionsToDelete, deleteReactions, db);
}
db.pragma('user_version = 42');
})();
logger.info('updateToSchemaVersion42: success!');
}
export const SCHEMA_VERSIONS = [
updateToSchemaVersion1,
updateToSchemaVersion2,
@ -2572,6 +2638,7 @@ export const SCHEMA_VERSIONS = [
updateToSchemaVersion39,
updateToSchemaVersion40,
updateToSchemaVersion41,
updateToSchemaVersion42,
];
export function updateSchema(db: Database) {
@ -2809,19 +2876,22 @@ function getInstance(): Database {
function batchMultiVarQuery<ValueT>(
values: Array<ValueT>,
query: (batch: Array<ValueT>) => void
query: (batch: Array<ValueT>) => void,
providedDatabase?: Database
): [];
function batchMultiVarQuery<ValueT, ResultT>(
values: Array<ValueT>,
query: (batch: Array<ValueT>) => Array<ResultT>
query: (batch: Array<ValueT>) => Array<ResultT>,
providedDatabase?: Database
): Array<ResultT>;
function batchMultiVarQuery<ValueT, ResultT>(
values: Array<ValueT>,
query:
| ((batch: Array<ValueT>) => void)
| ((batch: Array<ValueT>) => Array<ResultT>)
| ((batch: Array<ValueT>) => Array<ResultT>),
providedDatabase?: Database
): Array<ResultT> {
const db = getInstance();
const db = providedDatabase || getInstance();
if (values.length > MAX_VARIABLE_COUNT) {
const result: Array<ResultT> = [];
db.transaction(() => {
@ -4608,6 +4678,11 @@ async function removeReactionFromConversation({
});
}
async function _getAllReactions(): Promise<Array<ReactionType>> {
const db = getInstance();
return db.prepare<EmptyQuery>('SELECT * from reactions;').all();
}
async function getOlderMessagesByConversation(
conversationId: string,
{

View File

@ -7,10 +7,6 @@ import { v4 as generateGuid } from 'uuid';
import { SCHEMA_VERSIONS } from '../sql/Server';
const THEIR_UUID = generateGuid();
const THEIR_CONVO = generateGuid();
const ANOTHER_CONVO = generateGuid();
const THIRD_CONVO = generateGuid();
const OUR_UUID = generateGuid();
describe('SQL migrations test', () => {
@ -87,6 +83,11 @@ describe('SQL migrations test', () => {
});
describe('updateToSchemaVersion41', () => {
const THEIR_UUID = generateGuid();
const THEIR_CONVO = generateGuid();
const ANOTHER_CONVO = generateGuid();
const THIRD_CONVO = generateGuid();
it('clears sessions and keys if UUID is not available', () => {
updateToVersion(40);
@ -519,4 +520,99 @@ describe('SQL migrations test', () => {
);
});
});
describe('updateToSchemaVersion42', () => {
const MESSAGE_ID_1 = generateGuid();
const MESSAGE_ID_2 = generateGuid();
const MESSAGE_ID_3 = generateGuid();
const MESSAGE_ID_4 = generateGuid();
const CONVERSATION_ID = generateGuid();
it('deletes orphaned reactions', () => {
updateToVersion(41);
db.exec(
`
INSERT INTO messages
(id, conversationId, body)
VALUES
('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'message number 1'),
('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'message number 2');
INSERT INTO reactions (messageId, conversationId) VALUES
('${MESSAGE_ID_1}', '${CONVERSATION_ID}'),
('${MESSAGE_ID_2}', '${CONVERSATION_ID}'),
('${MESSAGE_ID_3}', '${CONVERSATION_ID}'),
('${MESSAGE_ID_4}', '${CONVERSATION_ID}');
`
);
const reactionCount = db
.prepare('SELECT COUNT(*) FROM reactions;')
.pluck();
const messageCount = db.prepare('SELECT COUNT(*) FROM messages;').pluck();
assert.strictEqual(reactionCount.get(), 4);
assert.strictEqual(messageCount.get(), 2);
updateToVersion(42);
assert.strictEqual(reactionCount.get(), 2);
assert.strictEqual(messageCount.get(), 2);
const reactionMessageIds = db
.prepare('SELECT messageId FROM reactions;')
.pluck()
.all();
assert.sameDeepMembers(reactionMessageIds, [MESSAGE_ID_1, MESSAGE_ID_2]);
});
it('new message delete trigger deletes reactions as well', () => {
updateToVersion(41);
db.exec(
`
INSERT INTO messages
(id, conversationId, body)
VALUES
('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'message number 1'),
('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'message number 2'),
('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'message number 3');
INSERT INTO reactions (messageId, conversationId) VALUES
('${MESSAGE_ID_1}', '${CONVERSATION_ID}'),
('${MESSAGE_ID_2}', '${CONVERSATION_ID}'),
('${MESSAGE_ID_3}', '${CONVERSATION_ID}');
`
);
const reactionCount = db
.prepare('SELECT COUNT(*) FROM reactions;')
.pluck();
const messageCount = db.prepare('SELECT COUNT(*) FROM messages;').pluck();
assert.strictEqual(reactionCount.get(), 3);
assert.strictEqual(messageCount.get(), 3);
updateToVersion(42);
assert.strictEqual(reactionCount.get(), 3);
assert.strictEqual(messageCount.get(), 3);
db.exec(
`
DELETE FROM messages WHERE id = '${MESSAGE_ID_1}';
`
);
assert.strictEqual(reactionCount.get(), 2);
assert.strictEqual(messageCount.get(), 2);
const reactionMessageIds = db
.prepare('SELECT messageId FROM reactions;')
.pluck()
.all();
assert.sameDeepMembers(reactionMessageIds, [MESSAGE_ID_2, MESSAGE_ID_3]);
});
});
});