diff --git a/api/src/database/UserDB.ts b/api/src/database/UserDB.ts index 2cdec6cb..ae8d7e49 100644 --- a/api/src/database/UserDB.ts +++ b/api/src/database/UserDB.ts @@ -50,21 +50,6 @@ export class UserDB { WHERE userID = $1`, [uuid]) .then(extract).then(map(u => u.samlid)).then(one); } - static async getUserByPossibleMentionInCourse(possibleMention: string, courseID: string, params: DBTools = {}) { - const {client = pool} = params; - const courseid = UUIDHelper.toUUID(courseID); - return client.query(` - SELECT * - FROM "UsersView" as u, "CourseRegistration" as cr - WHERE - (u.userID = cr.userID) - AND (courseID = $1) - AND (POSITION(userName in $2) = 1) - ORDER BY (char_length(userName)) DESC - LIMIT 1 - `, [courseid, possibleMention]) - .then(extract).then(one).then(userToAPI); - } /** * diff --git a/api/src/database/dbTester.ts b/api/src/database/dbTester.ts index 206127b4..c99a3f7e 100644 --- a/api/src/database/dbTester.ts +++ b/api/src/database/dbTester.ts @@ -86,7 +86,6 @@ async function DBusersTest() { await promise(UH.getAllStudents(), "getAllStudents"); await promise(UH.getUserByID(uuid0), "getUserById"); await promise(UH.searchUser("CAS"), "searchUser"); - await promise(UH.getUserByPossibleMentionInCourse("Caas rest of comment", uuid0), "UserMention"); await promise(UH.filterUserInCourse({courseID: uuid1}), "searchuserInCourse"); const u1 = await promise(UH.getUserBySamlID("samling_admin"), "samlgetter"); const samlid = await promise(UH.getSamlIDForUserID(u1.ID), "saml by user"); diff --git a/api/src/helpers/MentionsHelper.ts b/api/src/helpers/MentionsHelper.ts index 2bf49a63..1fa1253c 100644 --- a/api/src/helpers/MentionsHelper.ts +++ b/api/src/helpers/MentionsHelper.ts @@ -4,31 +4,16 @@ import {CourseRole} from "../../../models/enums/CourseRoleEnum"; import {containsPermission, PermissionEnum} from "../../../models/enums/PermissionEnum"; import {CourseRegistrationDB} from "../database/CourseRegistrationDB"; -import {NotFoundDatabaseError} from "../database/DatabaseErrors"; import {pgDB} from "../database/HelperDB"; import {MentionsDB} from "../database/MentionsDB"; -import {UserDB} from "../database/UserDB"; +import {MentionsUserCache} from "./MentionsUserCache"; /** Get the parts of the a comment following the @-sign, which starts a mention */ export function getPossibleMentions(commentBody: string) { return commentBody.split("@").slice(1).map(mention => mention.trim()); } -/** - * Make a database call to find a user that matches the first part of a possible mention, - * returning the possbileMention if none is found - */ -async function getUserForPossibleMention(possibleMention: string, courseID: string, client?: pgDB) { - try { - return await UserDB.getUserByPossibleMentionInCourse(possibleMention, courseID, {client}); - } catch (err) { - if (err instanceof NotFoundDatabaseError) { - return possibleMention; - } else { - throw err; - } - } -} +const usersCache = new MentionsUserCache(); /** Find a course role that matches the first part of a possible mention */ function getRoleForPossibleMention(possibleMention: string) { @@ -44,7 +29,7 @@ function getRoleForPossibleMention(possibleMention: string) { export async function getMentions(commentBody: string, courseID: string, client?: pgDB) { const users = await Promise.all( getPossibleMentions(commentBody) - .map(async pm => getUserForPossibleMention(pm, courseID, client)) + .map(async pm => usersCache.getUserForPossibleMention(pm, courseID, client)) ); return users .map(userOrComment => diff --git a/api/src/helpers/MentionsUserCache.ts b/api/src/helpers/MentionsUserCache.ts new file mode 100644 index 00000000..a0d04d50 --- /dev/null +++ b/api/src/helpers/MentionsUserCache.ts @@ -0,0 +1,47 @@ +import {pgDB} from "../database/HelperDB"; +import {User} from "../../../models/api/User"; +import {PrefixLookupTree} from "./PrefixLookupTree"; +import {UserDB} from "../database/UserDB"; + +interface CacheItem { + lookupTree: PrefixLookupTree; + lastUpdated: Date; +} + +/** Caching layer for mention lookups to spare the database. */ +export class MentionsUserCache { + /** Map to store the cache, indexed by courseID. */ + private cache: Map = new Map(); + private expirationMs = 1000 * 60 * 60; // 1 hour + + /** Get the prefix lookup tree associated with a certain course. */ + private async getLookupTree(courseID: string, client?: pgDB) { + // Try to get the lookup tree from the cache. + const item = this.cache.get(courseID); + // If it is not in the cache, or it is expired, + if (item === undefined || item.lastUpdated.getTime() < Date.now() - this.expirationMs) { + // then get the full userlist for this course from the database, + const users = await UserDB.filterUserInCourse({courseID, client}); + // create the lookup tree for it, + const lookupTree = PrefixLookupTree.ofList(users, u => u.name); + // store it in the cache, + this.cache.set(courseID, {lookupTree, lastUpdated: new Date()}); + // and return it. + return lookupTree; + } else { + // If it is in the cache, and it is not expired, return it. + return item.lookupTree; + } + } + + /** Get the user that is mentioned at the start of the possibleMention string. */ + public async getUserForPossibleMention(possibleMention: string, courseID: string, client?: pgDB) { + const lookupTree = await this.getLookupTree(courseID, client); + const user = lookupTree.lookup(possibleMention); + if (user === undefined) { + return possibleMention; + } else { + return user; + } + } +} diff --git a/api/src/helpers/PrefixLookupTree.ts b/api/src/helpers/PrefixLookupTree.ts new file mode 100644 index 00000000..cd70fe05 --- /dev/null +++ b/api/src/helpers/PrefixLookupTree.ts @@ -0,0 +1,110 @@ +/** A prefix lookup tree, used to e.g. find the user that is mentioned in a comment. + * The structure is as follows, e.g. for the user named Cas: + * { "C": { "a": { "s": { undefined: (user Cas) } } } } + * If we have three users, Cas, Caas and Cass, the structure looks like this: + * { "C": { + * "a": { + * "a": { + * "s": { match: (user Caas) } + * }, + * "s": { + * match: (user Cas), + * "s": { match: (user Cass) } + * } + * } + * } + * A lookup is performed by traversing the tree, for example with the comment "@Cas help me!" + * we try to look up the longest name that matches "Cas help me!": first we can match "C", which + * returns a tree with no match value (as there is no user named "C") and a next character "a". + * Our next character is "a", so we choose that next tree, which again has no match value and + * next characters "a" and "s". Our next character is "s", so we choose that tree. This tree has + * a match value (the user Cas), and a next character "s", but our next character is a space. + * Therefore we return the last match value we encountered while traversing the tree, which in + * this case is the match value at the last node we visited. In general, it could happen that we + * encounter more characters that have branches down the tree, but we never reach deep enough to + * reach another matching character. In this case the last match value will be farther up the tree. + */ +export class PrefixLookupTree { + /** The default value to choose if there are no more matching characters. */ + private match: T | undefined; + /** Map of next character to be matched and each corresponding subtree. */ + private next: Map>; + + /** Create an empty tree. Use the ofList static method instead, if you want to create + * a tree from a list of values. + */ + public constructor() { + this.match = undefined; + this.next = new Map(); + } + + /** Build a tree from a list of values. + * @param list The values to insert into the tree. + * @param getKey A function that returns the key for a value. + */ + public static ofList(list: T[], getKey: (value: T) => string): PrefixLookupTree { + const tree = new PrefixLookupTree(); + for (const value of list) { + tree.insert(getKey(value), value); + } + return tree; + } + + /** Insert an item into the tree. + * @param key The key to insert. Note that the key should be unique, existing items are overriden. + * @param value The value to insert. + */ + public insert(key: string, value: T) { + // If the key is the empty string, then we have a match on this subtree. + if (key === "") { + // Thus we set the match value to the given value. + this.match = value; + } else { + // Else, we traverse the tree based on the next character of the text we want to match with. + let next = this.next.get(key[0]); + // If there is no subtree for the next character, + if (next === undefined) { + // we create one + next = new PrefixLookupTree(); + // and store it in the current map of subtrees. + this.next.set(key[0], next); + } + // Then we can recursively add the remainder of the key to this subtree. + next.insert(key.slice(1), value); + } + } + + /** Find the longest prefix stored in the tree that matches the start of the given text. */ + public lookup(text: string): T | undefined { + return this._lookup(text); + } + + /** This is an internal version of lookup, which also takes the current longest match we have already found. */ + private _lookup(text: string, longestMatch?: T): T | undefined { + // If we have no more characters to match, then this subtree is the longest possible match. + if (text === "") { + // If this subtree has a match value, then the text fully matches to this subtree + if (this.match !== undefined) { + // and thus we return the match value. + return this.match; + } else { + // Else, we return the longest match that we have encountered along the way. + return longestMatch; + } + } else { + // If there is a match at the current subtree, then that is the new longest match we have found. + const newLongest = this.match !== undefined ? this.match : longestMatch; + // If there are characters to match, we check if there is a subtree for the next character. + const next = this.next.get(text[0]); + if (next === undefined) { + // If there is no such subtree, then we cannot continue our match and so we return the longest + // match that we encountered up to this point. + return newLongest; + } else { + // If that subtree exists, we recursively call _lookup on that subtree with the remainder of the text, + // propagating the longest match we have found so far. + return next._lookup(text.slice(1), newLongest); + } + } + } +} diff --git a/test/api/helpers/PrefixLookupTree.spec.ts b/test/api/helpers/PrefixLookupTree.spec.ts new file mode 100644 index 00000000..cd4b271a --- /dev/null +++ b/test/api/helpers/PrefixLookupTree.spec.ts @@ -0,0 +1,77 @@ +import "mocha"; +import {expect} from "chai"; + +import {PrefixLookupTree} from "../../../api/src/helpers/PrefixLookupTree"; + +describe("PrefixLookupTree", () => { + it("should return exact match for a single value", () => { + const sut = new PrefixLookupTree(); + sut.insert("test", "value"); + expect(sut.lookup("test")).to.equal("value"); + }); + + it("should return prefix for a single value", () => { + const sut = new PrefixLookupTree(); + sut.insert("test", "value"); + expect(sut.lookup("test string")).to.equal("value"); + }); + + it("should return undefined if single value doesn't fully match", () => { + const sut = new PrefixLookupTree(); + sut.insert("test", "value"); + expect(sut.lookup("tes")).to.be.undefined; + }); + + it("should return the longest exact match of two", () => { + const sut = new PrefixLookupTree(); + sut.insert("test", "value1"); + sut.insert("test string", "value2"); + expect(sut.lookup("test string")).to.equal("value2"); + }); + + it("should return the longest prefix of two", () => { + const sut = new PrefixLookupTree(); + sut.insert("test", "value1"); + sut.insert("test string", "value2"); + expect(sut.lookup("test string test")).to.equal("value2"); + }); + + it("should backtrack if a possible longer match doesn't fully match", () => { + const sut = new PrefixLookupTree(); + sut.insert("test", "value1"); + sut.insert("test string", "value2"); + expect(sut.lookup("test strong")).to.equal("value1"); + }); + + it("should still return the longest prefix when backtracking", () => { + const sut = new PrefixLookupTree(); + sut.insert("t", "value1"); + sut.insert("test", "value2"); + sut.insert("test string", "value3"); + expect(sut.lookup("test strong")).to.equal("value2"); + }); + + it("should return the longest prefix out of three fully distinct", () => { + const sut = new PrefixLookupTree(); + sut.insert("one", "value1"); + sut.insert("two", "value2"); + sut.insert("four", "value4"); + expect(sut.lookup("two and a half")).to.equal("value2"); + }); + + it("should return the longest prefix out of several overlapping", () => { + const sut = new PrefixLookupTree(); + sut.insert("abc", "value1"); + sut.insert("abd", "value2"); + sut.insert("abcd", "value3"); + sut.insert("aa", "value4"); + sut.insert("abcde", "value5"); + + expect(sut.lookup("a a")).to.be.undefined; + expect(sut.lookup("abc def")).to.equal("value1"); + expect(sut.lookup("abdef")).to.equal("value2"); + expect(sut.lookup("abcd e")).to.equal("value3"); + expect(sut.lookup("aa b")).to.equal("value4"); + expect(sut.lookup("abcde f")).to.equal("value5"); + }); +});