Skip to content

Commit

Permalink
Merge pull request #4612 from coralproject/fix/CORL-3166-single-convo…
Browse files Browse the repository at this point in the history
…-cache-prime

[CORL-3166] Prime comment cache when loading single conversation view
  • Loading branch information
tessalt committed May 7, 2024
2 parents fafa513 + 1da4691 commit cb67a39
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 6 deletions.
6 changes: 6 additions & 0 deletions common/lib/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,4 +467,10 @@ export enum ERROR_CODES {
* to a DSA report that is too long
*/
DSA_REPORT_ADDITIONAL_INFO_TOO_LONG = "DSA_REPORT_ADDITIONAL_INFO_TOO_LONG",
/**
* UNABLE_TO_PRIME_CACHED_COMMENTS_FOR_STORY is thrown when DATA_CACHE is enabled and the
* priming of comments for the story in the data caches `commentCache` returns an undefined
* result. This usually means something went very wrong loading from Redis or Mongo.
*/
UNABLE_TO_PRIME_CACHED_COMMENTS_FOR_STORY = "UNABLE_TO_PRIME_CACHED_COMMENTS_FOR_STORY"
}
12 changes: 12 additions & 0 deletions server/src/core/server/data/cache/commentCache.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import RedisClient from "ioredis";
import { waitFor } from "coral-common/common/lib/helpers";
import { CommentCache } from "coral-server/data/cache/commentCache";
import { MongoContext, MongoContextImpl } from "coral-server/data/context";
import { UnableToPrimeCachedCommentsForStory } from "coral-server/errors";
import logger from "coral-server/logger";
import { Comment } from "coral-server/models/comment";
import { createMongoDB } from "coral-server/services/mongodb";
Expand Down Expand Up @@ -84,6 +85,10 @@ it("can load root comments from commentCache", async () => {
story.id,
false
);
if (!primeResult) {
throw new UnableToPrimeCachedCommentsForStory(story.tenantID, story.id);
}

const results = await comments.rootComments(
story.tenantID,
story.id,
Expand Down Expand Up @@ -144,6 +149,10 @@ it("can load replies from commentCache", async () => {
story.id,
false
);
if (!primeResult) {
throw new UnableToPrimeCachedCommentsForStory(story.tenantID, story.id);
}

const rootResults = await comments.rootComments(
story.tenantID,
story.id,
Expand Down Expand Up @@ -211,6 +220,9 @@ it("cache expires appropriately", async () => {
story.id,
false
);
if (!primeResult) {
throw new UnableToPrimeCachedCommentsForStory(story.tenantID, story.id);
}
expect(primeResult.retrievedFrom).toEqual("redis");

let lockExists = await redis.exists(lockKey);
Expand Down
10 changes: 10 additions & 0 deletions server/src/core/server/data/cache/commentCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export interface Filter {
}

export class CommentCache implements IDataCache {
public readonly primedStories: Set<string>;

private disableLocalCaching: boolean;
private expirySeconds: number;

Expand Down Expand Up @@ -55,6 +57,8 @@ export class CommentCache implements IDataCache {

this.commentsByKey = new Map<string, Readonly<Comment>>();
this.membersLookup = new Map<string, string[]>();

this.primedStories = new Set<string>();
}

public async available(tenantID: string): Promise<boolean> {
Expand Down Expand Up @@ -187,6 +191,10 @@ export class CommentCache implements IDataCache {
};
}

public shouldPrimeForStory(tenantID: string, storyID: string) {
return !this.primedStories.has(`${tenantID}:${storyID}`);
}

public async primeCommentsForStory(
tenantID: string,
storyID: string,
Expand Down Expand Up @@ -214,6 +222,8 @@ export class CommentCache implements IDataCache {
commentIDs.add(comment.id);
}

this.primedStories.add(`${tenantID}:${storyID}`);

return {
userIDs: Array.from(userIDs),
commentIDs: Array.from(commentIDs),
Expand Down
9 changes: 9 additions & 0 deletions server/src/core/server/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1064,3 +1064,12 @@ export class InvalidFlairBadgeName extends CoralError {
});
}
}

export class UnableToPrimeCachedCommentsForStory extends CoralError {
constructor(tenantID: string, storyID: string) {
super({
code: ERROR_CODES.UNABLE_TO_PRIME_CACHED_COMMENTS_FOR_STORY,
context: { pub: { tenantID } },
});
}
}
2 changes: 2 additions & 0 deletions server/src/core/server/errors/translations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,6 @@ export const ERROR_TRANSLATIONS: Record<ERROR_CODES, string> = {
INVALID_FLAIR_BADGE_NAME: "error-invalidFlairBadgeName",
DSA_REPORT_LAW_BROKEN_TOO_LONG: "error-dsaReportLawBrokenTooLong",
DSA_REPORT_ADDITIONAL_INFO_TOO_LONG: "error-dsaReportAdditionalInfoTooLong",
UNABLE_TO_PRIME_CACHED_COMMENTS_FOR_STORY:
"error-unableToPrimeCachedCommentsForStory",
};
12 changes: 10 additions & 2 deletions server/src/core/server/graph/loaders/Comments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import DataLoader from "dataloader";
import { defaultTo, isNumber } from "lodash";
import { DateTime } from "luxon";

import { StoryNotFoundError } from "coral-server/errors";
import {
StoryNotFoundError,
UnableToPrimeCachedCommentsForStory,
} from "coral-server/errors";
import GraphContext from "coral-server/graph/context";
import { retrieveManyUserActionPresence } from "coral-server/models/action/comment";
import {
Expand Down Expand Up @@ -369,11 +372,16 @@ export default (ctx: GraphContext) => ({
return connection;
}

const { userIDs } = await ctx.cache.comments.primeCommentsForStory(
const primeResult = await ctx.cache.comments.primeCommentsForStory(
ctx.tenant.id,
storyID,
isArchived
);
if (!primeResult) {
throw new UnableToPrimeCachedCommentsForStory(ctx.tenant.id, storyID);
}

const { userIDs } = primeResult;
await ctx.cache.users.loadUsers(ctx.tenant.id, userIDs);
await ctx.cache.commentActions.primeCommentActions(ctx.tenant.id, story.id);

Expand Down
25 changes: 21 additions & 4 deletions server/src/core/server/graph/resolvers/Comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,29 @@ export const Comment: GQLCommentTypeResolver<comment.Comment> = {
},
statusHistory: ({ id }, input, ctx) =>
ctx.loaders.CommentModerationActions.forComment(input, id),
replies: (c, input, ctx) =>
replies: async (c, input, ctx) => {
// If there is at least one reply, then use the connection loader, otherwise
// return a blank connection.
c.childCount > 0
? ctx.loaders.Comments.forParent(c.storyID, c.id, input)
: createConnection(),
if (c.childCount === 0) {
return createConnection();
}

const cacheAvailable = await ctx.cache.available(ctx.tenant.id);
if (
cacheAvailable &&
ctx.cache.comments.shouldPrimeForStory(ctx.tenant.id, c.storyID)
) {
const story = await ctx.loaders.Stories.find.load({ id: c.storyID });
await ctx.cache.comments.primeCommentsForStory(
ctx.tenant.id,
c.storyID,
!!story?.isArchived
);
}

const result = await ctx.loaders.Comments.forParent(c.storyID, c.id, input);
return result;
},
replyCount: async ({ storyID, childIDs }, input, ctx) => {
// TODO: (wyattjoh) the childCount should be used eventually, but it should be managed with the status so it's only a count of published comments
if (childIDs.length === 0) {
Expand Down
1 change: 1 addition & 0 deletions server/src/core/server/locales/en-US/errors.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,4 @@ error-dataCachingNotAvailable = Data caching is not available at this time.
error-invalidFlairBadgeName = Only letters, numbers, and the special characters - . are permitted in flair badge names.
error-dsaReportLawBrokenTooLong = What law do you believe has been broken for DSA report exceeds maximum length.
error-dsaReportAdditionalInfoTooLong = Additional information for DSA report exceeds maximum length.
error-unableToPrimeCachedCommentsForStory = Unable to prime cached comments for story.

0 comments on commit cb67a39

Please sign in to comment.