Skip to content

Commit

Permalink
short-links: Add comment links support to EventService (notifications)
Browse files Browse the repository at this point in the history 
This change adds parsing of short comment links (e.g. `/user/9bac13#ad2b`)
to EventService a.k.a Notifications. That is the only place we need
special support. Other relevant server parts (backlinks table, reindex
script, realtime events) only need the post_id anyway (for now), so they
work with such links already, not caring about the comment_id part.
  • Loading branch information
@clbn
clbn committed Aug 15, 2023
1 parent 55ae56c commit abf46f7
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 15 deletions.
23 changes: 23 additions & 0 deletions app/support/DbAdapter/comments.js
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import validator from 'validator';
import pgFormat from 'pg-format';

import { Comment } from '../../models';
import { toTSVector } from '../search/to-tsvector';
Expand Down Expand Up @@ -43,6 +44,28 @@ const commentsTrait = (superClass) =>
});
}

async getCommentLongIds(shortIds) {
if (shortIds.length === 0) {
return [];
}

const values = shortIds
.map((l) =>
l.replace(/^(.+)#(..)(.+)$/, (m, p1, p2, p3) =>
pgFormat(`(%L, %L, %L::int)`, p1, p2, +`0x${p3}`),
),
)
.join(',');

return await this.database.getCol(`
SELECT c.uid
FROM (VALUES ${values}) AS links (post_short_id, comment_prefix, comment_num)
JOIN post_short_ids AS psi ON links.post_short_id = psi.short_id
JOIN comments AS c ON psi.long_id = c.post_id
WHERE c.uid::text ^@ links.comment_prefix AND c.seq_number = links.comment_num
`);
}

async getCommentById(id) {
if (!validator.isUUID(id)) {
return null;
Expand Down
1 change: 1 addition & 0 deletions app/support/DbAdapter/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ export class DbAdapter {
unlikePost(postId: UUID, userId: UUID): Promise<boolean>;

// Comments
getCommentLongIds(shortIds: string[]): Promise<UUID[]>;
getCommentById(id: UUID): Promise<Comment | null>;
getCommentsByIds(ids: UUID[]): Promise<Comment[]>;
getCommentsByIntIds(ids: number[]): Promise<Comment[]>;
Expand Down
20 changes: 14 additions & 6 deletions app/support/EventService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
T_EVENT_TYPE,
} from './EventTypes';
import { Nullable, UUID } from './types';
import { extractShortIds, extractUUIDs } from './backlinks';
import { extractHashedShortIds, extractShortIds, extractUUIDs } from './backlinks';

type OnPostFeedsChangedParams = {
addedFeeds?: Timeline[];
Expand Down Expand Up @@ -715,15 +715,23 @@ async function getDirectEvents(post: Post, authorId: Nullable<UUID>, eventType:
}

async function processBacklinks(srcEntity: Post | Comment, prevBody = '') {
// Long links with UUIDs, both post and post+comment
const prevUUIDs = extractUUIDs(prevBody);
const newUUIDs = extractUUIDs(srcEntity.body);
const uuids = difference(newUUIDs, prevUUIDs);

const prevShortIds = extractShortIds(prevBody);
const newShortIds = extractShortIds(srcEntity.body);
const shortIds = difference(newShortIds, prevShortIds);
const moreUUIDs = await dbAdapter.getPostLongIds(shortIds);
uuids.push(...moreUUIDs);
// Short post links (e.g. `/user/5168a0`)
const postShortIds = difference(extractShortIds(srcEntity.body), extractShortIds(prevBody));
const morePostUUIDs = await dbAdapter.getPostLongIds(postShortIds);
uuids.push(...morePostUUIDs);

// Short comment links (e.g. `/user/9bac13#ad2b`)
const commentShortIds = difference(
extractHashedShortIds(srcEntity.body),
extractHashedShortIds(prevBody),
);
const moreCommentUUIDs = await dbAdapter.getCommentLongIds(commentShortIds);
uuids.push(...moreCommentUUIDs);

const [mentionedPosts, mentionedComments] = await Promise.all([
dbAdapter.getPostsByIds(uuids),
Expand Down
5 changes: 5 additions & 0 deletions app/support/backlinks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ export function extractShortIds(text: string | null): string[] {
return text ? [...text.matchAll(shortLinkRe)].map((m) => m[1]).filter(onlyUnique) : [];
}

export const hashedShortLinkRe = /\/[A-Za-z0-9-]{3,35}\/([0-9a-f]{6,10}#[0-9a-f]{3,6})/gi;
export function extractHashedShortIds(text: string | null): string[] {
return text ? [...text.matchAll(hashedShortLinkRe)].map((m) => m[1]).filter(onlyUnique) : [];
}

function onlyUnique<T>(value: T, index: number, arr: T[]) {
return arr.indexOf(value) === index;
}
Expand Down
86 changes: 77 additions & 9 deletions test/integration/support/events/backlinks.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-env node, mocha */
/* global $pg_database */
import expect from 'unexpected';
import { sortBy } from 'lodash';

import { User, Post, Group, dbAdapter, Comment } from '../../../../app/models';
import cleanDB from '../../../dbCleaner';
Expand All @@ -15,7 +16,7 @@ describe('EventService', () => {
let /** @type {User} */ luna, /** @type {User} */ mars, /** @type {User} */ venus;
let /** @type {Group} */ dubhe;
let /** @type {Post} */ lunaPost, /** @type {Post} */ marsPost, /** @type {Post} */ venusPost;
let marsPostShortId;
let marsPostShortId, venusPostShortId;

before(async () => {
luna = new User({ username: 'luna', password: 'pw' });
Expand All @@ -32,6 +33,7 @@ describe('EventService', () => {
);

marsPostShortId = await marsPost.getShortId();
venusPostShortId = await venusPost.getShortId();
});

// Clean events before each test
Expand Down Expand Up @@ -285,15 +287,31 @@ describe('EventService', () => {
await expectBacklinkEvents(luna, []);
});

it('should create backlink_in_post event for mentioned comment author (short link)', async () => {
post = await createPost(
luna,
`Mentioning /venus/${venusPostShortId}#${marsComment.shortId}`,
);
await expectBacklinkEvents(mars, [await backlinkInPostEvent(post, marsComment)]);
await expectBacklinkEvents(venus, [await backlinkInPostEvent(post, venusPost)]);
await expectBacklinkEvents(luna, []);
});

it('should not create backlink_in_post event for mentioned comment author', async () => {
post = await createPost(luna, `Mentioning ${lunaComment.id}`);
await expectBacklinkEvents(luna, []);
});

it('should create backlink_in_post event for each mentioned comment', async () => {
post = await createPost(luna, `Mentioning ${marsComment.id} ${venusComment.id}`);
post = await createPost(
luna,
`Mentioning /venus/${venusPostShortId}#${marsComment.shortId} ${venusComment.id}`,
);
await expectBacklinkEvents(mars, [await backlinkInPostEvent(post, marsComment)]);
await expectBacklinkEvents(venus, [await backlinkInPostEvent(post, venusComment)]);
await expectBacklinkEvents(venus, [
await backlinkInPostEvent(post, venusPost),
await backlinkInPostEvent(post, venusComment),
]);
await expectBacklinkEvents(luna, []);
});

Expand Down Expand Up @@ -351,6 +369,17 @@ describe('EventService', () => {
await expectBacklinkEvents(mars, [await backlinkInPostEvent(post, marsComment)]);
});

it('should create backlink_in_post when post without mention updates with mention (short link)', async () => {
post = await createAndUpdatePost(
luna,
'Post without mentions',
`Mentioning /venus/${venusPostShortId}#${marsComment.shortId}`,
);

await expectBacklinkEvents(mars, [await backlinkInPostEvent(post, marsComment)]);
await expectBacklinkEvents(venus, [await backlinkInPostEvent(post, venusPost)]);
});

it('should not remove backlink_in_post when mention disappears from the post', async () => {
post = await createAndUpdatePost(
luna,
Expand All @@ -363,11 +392,14 @@ describe('EventService', () => {
it('should create additional backlink_in_post when a new mention appears in the post', async () => {
post = await createAndUpdatePost(
luna,
`Mentioning ${marsComment.id}`,
`Mentioning ${venusComment.id}`,
`Mentioning /venus/${venusPostShortId}#${marsComment.shortId}`,
);
await expectBacklinkEvents(mars, [await backlinkInPostEvent(post, marsComment)]);
await expectBacklinkEvents(venus, [await backlinkInPostEvent(post, venusComment)]);
await expectBacklinkEvents(venus, [
await backlinkInPostEvent(post, venusPost),
await backlinkInPostEvent(post, venusComment),
]);
});
});
});
Expand All @@ -382,6 +414,17 @@ describe('EventService', () => {
await expectBacklinkEvents(luna, []);
});

it('should create backlink_in_comment event for mentioned comment author (short link)', async () => {
comment = await createComment(
luna,
venusPost,
`Mentioning /venus/${venusPostShortId}#${marsComment.shortId}`,
);
await expectBacklinkEvents(mars, [await backlinkInCommentEvent(comment, marsComment)]);
await expectBacklinkEvents(venus, [await backlinkInCommentEvent(comment, venusPost)]);
await expectBacklinkEvents(luna, []);
});

it('should not create backlink_in_comment event for mentioned comment author', async () => {
comment = await createComment(luna, venusPost, `Mentioning ${lunaComment.id}`);
await expectBacklinkEvents(luna, []);
Expand All @@ -391,10 +434,13 @@ describe('EventService', () => {
comment = await createComment(
luna,
venusPost,
`Mentioning ${marsComment.id} ${venusComment.id}`,
`Mentioning /venus/${venusPostShortId}#${marsComment.shortId} ${venusComment.id}`,
);
await expectBacklinkEvents(mars, [await backlinkInCommentEvent(comment, marsComment)]);
await expectBacklinkEvents(venus, [await backlinkInCommentEvent(comment, venusComment)]);
await expectBacklinkEvents(venus, [
await backlinkInCommentEvent(comment, venusPost),
await backlinkInCommentEvent(comment, venusComment),
]);
await expectBacklinkEvents(luna, []);
});

Expand Down Expand Up @@ -449,6 +495,17 @@ describe('EventService', () => {
await expectBacklinkEvents(mars, [await backlinkInCommentEvent(comment, marsComment)]);
});

it('should create backlink_in_post when comment without mention updates with mention (short link)', async () => {
comment = await createComment(
luna,
venusPost,
'Post without mentions',
`Mentioning /venus/${venusPostShortId}#${marsComment.shortId}`,
);

await expectBacklinkEvents(mars, [await backlinkInCommentEvent(comment, marsComment)]);
});

it('should not remove backlink_in_post when mention disappears from the comment', async () => {
comment = await createComment(
luna,
Expand All @@ -463,11 +520,12 @@ describe('EventService', () => {
comment = await createComment(
luna,
venusPost,
`Mentioning ${marsComment.id}`,
`Mentioning ${venusComment.id}`,
`Mentioning /venus/${venusPostShortId}#${marsComment.shortId}`,
);
await expectBacklinkEvents(mars, [await backlinkInCommentEvent(comment, marsComment)]);
await expectBacklinkEvents(venus, [
await backlinkInCommentEvent(comment, venusPost),
await backlinkInCommentEvent(comment, venusComment),
]);
});
Expand Down Expand Up @@ -504,7 +562,17 @@ async function expectBacklinkEvents(user, shape) {
'backlink_in_post',
'backlink_in_comment',
]);
expect(events, 'to satisfy', shape);

// If array `shape` has more than one element, it's prone to race conditions
// (as `events` may go in random order, not matching the order in `shape`).
// So we need to sort both to make the comparison deterministic.
if (shape.length > 1) {
const events2 = sortBy(events, Object.keys(shape[0]));
const shape2 = sortBy(shape, Object.keys(shape[0]));
expect(events2, 'to satisfy', shape2);
} else {
expect(events, 'to satisfy', shape);
}
}

/**
Expand Down
39 changes: 39 additions & 0 deletions test/unit/support/backlinks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import expect from 'unexpected';
import {
extractUUIDs,
extractShortIds,
extractHashedShortIds,
notifyBacklinkedLater,
notifyBacklinkedNow,
} from '../../../app/support/backlinks';
Expand Down Expand Up @@ -79,6 +80,44 @@ describe('Backlinks parser', () => {
});
});
});

describe('extractHashedShortIds', () => {
const cases = [
{ text: 'abc', result: [] },
{
text: 'abc /venus/f482e5#ad2b',
result: ['f482e5#ad2b'],
},
{
text: 'abc /venus/f482e5#ad2b /venus/f482e5#ad2b',
result: ['f482e5#ad2b'],
},
{
text: 'abc /venus/f482e5#ad2b /venus/f482e5#bf9',
result: ['f482e5#ad2b', 'f482e5#bf9'],
},
{
text: 'abc /venus/f482e5#ad2b /venus/f4g2e5#ad2b',
// _______________________________^ (invalid hexadecimal)
result: ['f482e5#ad2b'],
},
{
text: 'abc /venus/f482e5#ad2b hello mars/4a39b8#055',
// _________________________________^ (no starting slash)
result: ['f482e5#ad2b'],
},
{
text: 'abc /venus/f482e5#ad2b /group-for-very-secret-meetings/4a39b8#0a5',
result: ['f482e5#ad2b', '4a39b8#0a5'],
},
];

cases.forEach(({ text, result }) => {
it(`should extract ${result.length} shortId(s) from "${text}"`, () => {
expect(extractHashedShortIds(text), 'to equal', result);
});
});
});
});

describe('Backlinks notifier', () => {
Expand Down

0 comments on commit abf46f7

Please sign in to comment.