feat: expanded regexp generation, locale support, caching (#2869)

Co-authored-by: Kim Gardner <kgardnr@gmail.com>

src/core/server/services/comments/pipeline/phases/wordList.ts

@@ -1,18 +1,24 @@
-import {
-  GQLCOMMENT_FLAG_REASON,
-  GQLCOMMENT_STATUS,
-} from "coral-server/graph/schema/__generated__/types";
 import { ACTION_TYPE } from "coral-server/models/action/comment";
 import {
   IntermediateModerationPhase,
   IntermediatePhaseResult,
 } from "coral-server/services/comments/pipeline";
-import { containsMatchingPhraseMemoized } from "coral-server/services/comments/pipeline/wordList";
+
+import {
+  GQLCOMMENT_FLAG_REASON,
+  GQLCOMMENT_STATUS,
+} from "coral-server/graph/schema/__generated__/types";
+
+import { WordList } from "../wordList";
+
+// Create a new wordlist instance to use.
+const list = new WordList();
 
 // This phase checks the comment against the wordList.
 export const wordList: IntermediateModerationPhase = ({
   tenant,
   comment,
+  htmlStripped,
 }): IntermediatePhaseResult | void => {
   // If there isn't a body, there can't be a bad word!
   if (!comment.body) {
@@ -23,7 +29,7 @@ export const wordList: IntermediateModerationPhase = ({
   // has pre-mod enabled or not. If the comment was rejected based on the
   // wordList, then reject it, otherwise if the moderation setting is
   // premod, set it to `premod`.
-  if (containsMatchingPhraseMemoized(tenant.wordList.banned, comment.body)) {
+  if (list.test(tenant, "banned", htmlStripped)) {
     // Add the flag related to Trust to the comment.
     return {
       status: GQLCOMMENT_STATUS.REJECTED,
@@ -43,7 +49,7 @@ export const wordList: IntermediateModerationPhase = ({
 
   // If the wordList has matched the suspect word filter and we haven't disabled
   // auto-flagging suspect words, then we should flag the comment!
-  if (containsMatchingPhraseMemoized(tenant.wordList.suspect, comment.body)) {
+  if (list.test(tenant, "suspect", htmlStripped)) {
     return {
       actions: [
         {

src/core/server/services/comments/pipeline/wordList.spec.ts

@@ -1,66 +1,82 @@
 import {
-  containsMatchingPhrase,
-  containsMatchingPhraseMemoized,
+  Options,
+  WordList,
 } from "coral-server/services/comments/pipeline/wordList";
 
-const phrases = [
-  "cookies",
-  "how to do bad things",
-  "how to do really bad things",
-  "s h i t",
-  "$hit",
-  "p**ch",
-  "p*ch",
-];
+describe("en-US", () => {
+  const list = new WordList();
+  const options: Options = {
+    id: "tenant_1",
+    locale: "en-US",
+    wordList: {
+      banned: [
+        "cookies",
+        "how to do bad things",
+        "how to do really bad things",
+        "s h i t",
+        "$hit",
+        "p**ch",
+        "p*ch",
+        "banned",
+        "ban",
+      ],
+      suspect: [],
+    },
+  };
 
-describe("containsMatchingPhrase", () => {
-  it("does match on a word in the list", () => {
-    [
-      "how to do really bad things",
-      "what is cookies",
-      "cookies",
-      "COOKIES.",
-      "how to do bad things",
-      "How To do bad things!",
-      "This stuff is $hit!",
-      "That's a p**ch!",
-    ].forEach(word => {
-      expect(containsMatchingPhrase(phrases, word)).toEqual(true);
+  describe("containsMatchingPhrase", () => {
+    it("does match on a word in the list", () => {
+      [
+        "how to do really bad things",
+        "what is cookies",
+        "cookies",
+        "COOKIES.",
+        "how to do bad things",
+        "How To do bad things!",
+        "How.To.do.bad.things!",
+        "This stuff is $hit!",
+        "This is a test.\nTo see if cookies are found, in the second line.",
+        "That's a p**ch!",
+        "Banned words should be detected",
+      ].forEach(word => {
+        expect(list.test(options, "banned", word)).toEqual(true);
+      });
+    });
+
+    it("does not match on a word not in the list", () => {
+      [
+        "how to",
+        "cookie",
+        "how to be a great person?",
+        "how to not do really bad things?",
+        "i have $100 dollars.",
+        "I have bad $ hit lling",
+        "That's a p***ch!",
+        "When bann is spelt wrong, it won't be caught.",
+      ].forEach(word => {
+        expect(list.test(options, "banned", word)).toEqual(false);
+      });
+    });
+
+    it("allows an empty list", () => {
+      expect(list.test(options, "banned", "test")).toEqual(false);
     });
   });
 
-  it("does not match on a word not in the list", () => {
-    [
-      "how to",
-      "cookie",
-      "how to be a great person?",
-      "how to not do really bad things?",
-      "i have $100 dollars.",
-      "I have bad $ hit lling",
-      "That's a p***ch!",
-    ].forEach(word => {
-      expect(containsMatchingPhrase(phrases, word)).toEqual(false);
-    });
-  });
-
-  it("allows an empty list", () => {
-    expect(containsMatchingPhrase([], "test")).toEqual(false);
-  });
-});
-
-describe("containsMatchingPhraseMemoized", () => {
-  it("return true for all cases after memoizing the first result", () => {
-    [
-      "cookies 1",
-      "cookies 2",
-      "cookies 4",
-      "cookies 5",
-      "this is for cookies 6",
-      "this is for cookies 7",
-      "this is for cookies 8",
-      "this is for cookies 9",
-    ].forEach(word => {
-      expect(containsMatchingPhraseMemoized(phrases, word)).toEqual(true);
+  describe("containsMatchingPhraseMemoized", () => {
+    it("return true for all cases after memoizing the first result", () => {
+      [
+        "cookies 1",
+        "cookies 2",
+        "cookies 4",
+        "cookies 5",
+        "this is for cookies 6",
+        "this is for cookies 7",
+        "this is for cookies 8",
+        "this is for cookies 9",
+      ].forEach(word => {
+        expect(list.test(options, "banned", word)).toEqual(true);
+      });
     });
   });
 });

src/core/server/services/comments/pipeline/wordList.ts

@@ -1,37 +1,100 @@
-import { memoize } from "lodash";
+import ms from "ms";
+import now from "performance-now";
 
-// Replace `memoize.Cache`.
-memoize.Cache = WeakMap;
+import { LanguageCode } from "coral-common/helpers";
+import { createWordListRegExp } from "coral-common/utils";
+import logger from "coral-server/logger";
+import { Tenant } from "coral-server/models/tenant";
 
-/**
- * Escape string for special regular expression characters.
- */
-export function escapeRegExp(str: string) {
-  return str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); // $& means the whole matched string
+interface Lists {
+  banned: RegExp | false;
+  suspect: RegExp | false;
 }
 
-/**
- * Generate a regular expression that catches the `phrases`.
- */
-export function generateRegExp(phrases: string[]) {
-  const inner = phrases
-    .map(phrase =>
-      phrase
-        .split(/\s+/)
-        .map(word => escapeRegExp(word))
-        .join('[\\s"?!.]+')
-    )
-    .join("|");
-  return new RegExp(`(^|[^\\w])(${inner})(?=[^\\w]|$)`, "miu");
+export type Options = Pick<Tenant, "id" | "locale" | "wordList">;
+
+export class WordList {
+  private readonly cache = new WeakMap<Options, Lists>();
+
+  private generate(locale: LanguageCode, list: string[]) {
+    // If a word list has no entries, then we can make a simple tester.
+    if (list.length === 0) {
+      return false;
+    }
+
+    return createWordListRegExp(locale, list);
+  }
+
+  /**
+   * create will create the List's.
+   *
+   * @param options options used to generate Lists
+   */
+  private create(options: Options): Lists {
+    return {
+      banned: this.generate(options.locale, options.wordList.banned),
+      suspect: this.generate(options.locale, options.wordList.suspect),
+    };
+  }
+
+  /**
+   * lists will create/return a cached set of testers for the provided word
+   * lists.
+   *
+   * @param options the options object that is also used as the cache key
+   */
+  private lists(options: Options, cache: boolean): Lists {
+    // If the request isn't supposed to use the cache, then just return a new
+    // one.
+    if (!cache) {
+      return this.create(options);
+    }
+
+    // As this is supposed to be cached, try to get it from the cache, or create
+    // it.
+    let lists = this.cache.get(options);
+    if (!lists) {
+      const startedAt = now();
+      lists = this.create(options);
+      logger.info(
+        { tenantID: options.id, took: ms(now() - startedAt) },
+        "regenerated word list cache"
+      );
+
+      this.cache.set(options, lists);
+    }
+
+    return lists;
+  }
+
+  /**
+   * test will test the string against the selected list. The generated lists
+   * are cached and re-used on subsequent calls.
+   *
+   * @param options the options object that is also used as the cache key
+   * @param listName the list to test against
+   * @param testString the string to test to see if they match anything on the
+   *                   list
+   * @param cache when true, will re-use the cached testers based on the lists
+   */
+  public test(
+    options: Options,
+    listName: keyof Lists,
+    testString: string,
+    cache = true
+  ): boolean {
+    const list = this.lists(options, cache)[listName];
+    if (!list) {
+      return false;
+    }
+
+    const startedAt = now();
+    const result = list.test(testString);
+    logger.debug(
+      { tenantID: options.id, took: ms(now() - startedAt) },
+      "word list phrase test complete"
+    );
+
+    return result;
+  }
 }
-
-export const generateRegExpMemoized = memoize(generateRegExp);
-
-export const containsMatchingPhrase = (phrases: string[], testString: string) =>
-  phrases.length > 0 ? generateRegExp(phrases).test(testString) : false;
-
-export const containsMatchingPhraseMemoized = (
-  phrases: string[],
-  testString: string
-) =>
-  phrases.length > 0 ? generateRegExpMemoized(phrases).test(testString) : false;

src/core/server/services/tenant/index.ts

@@ -1,5 +1,5 @@
 import { Redis } from "ioredis";
-import { isUndefined } from "lodash";
+import { isUndefined, lowerCase, uniqBy } from "lodash";
 import { DateTime } from "luxon";
 import { Db } from "mongodb";
 import { URL } from "url";
@@ -45,11 +45,11 @@ function cleanWordList(
   list: GQLSettingsWordListInput
 ): GQLSettingsWordListInput {
   if (list.banned) {
-    list.banned = list.banned.filter(Boolean);
+    list.banned = uniqBy(list.banned.filter(Boolean), lowerCase) as string[];
   }
 
   if (list.suspect) {
-    list.suspect = list.suspect.filter(Boolean);
+    list.suspect = uniqBy(list.suspect.filter(Boolean), lowerCase) as string[];
   }
 
   return list;