From 52606616c4b7dac5f57d9e863543274c014bfdec Mon Sep 17 00:00:00 2001 From: Cohee <18619528+Cohee1207@users.noreply.github.com> Date: Fri, 29 Nov 2024 01:06:10 +0200 Subject: [PATCH 1/5] Add 100MB limit to parsed characters cache --- src/endpoints/characters.js | 8 +- src/util.js | 179 ++++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 2 deletions(-) diff --git a/src/endpoints/characters.js b/src/endpoints/characters.js index f2bd478e8..1a209e710 100644 --- a/src/endpoints/characters.js +++ b/src/endpoints/characters.js @@ -14,7 +14,7 @@ import jimp from 'jimp'; import { AVATAR_WIDTH, AVATAR_HEIGHT } from '../constants.js'; import { jsonParser, urlencodedParser } from '../express-common.js'; -import { deepMerge, humanizedISO8601DateTime, tryParse, extractFileFromZipBuffer } from '../util.js'; +import { deepMerge, humanizedISO8601DateTime, tryParse, extractFileFromZipBuffer, LimitedMap } from '../util.js'; import { TavernCardValidator } from '../validator/TavernCardValidator.js'; import { parse, write } from '../character-card-parser.js'; import { readWorldInfoFile } from './worldinfo.js'; @@ -23,7 +23,8 @@ import { importRisuSprites } from './sprites.js'; const defaultAvatarPath = './public/img/ai4.png'; // KV-store for parsed character data -const characterDataCache = new Map(); +// 100 MB limit. Would take roughly 3000 characters to reach this limit +const characterDataCache = new LimitedMap(1024 * 1024 * 100); // Some Android devices require tighter memory management const isAndroid = process.platform === 'android'; @@ -58,6 +59,9 @@ async function writeCharacterData(inputFile, data, outputFile, request, crop = u try { // Reset the cache for (const key of characterDataCache.keys()) { + if (Buffer.isBuffer(inputFile)) { + break; + } if (key.startsWith(inputFile)) { characterDataCache.delete(key); break; diff --git a/src/util.js b/src/util.js index 7cebdc031..d03fb6a9c 100644 --- a/src/util.js +++ b/src/util.js @@ -670,3 +670,182 @@ export function isValidUrl(url) { return false; } } + +/** + * LimitedMap class that limits the memory usage of string values. + */ +export class LimitedMap { + /** + * Creates an instance of LimitedMap. + * @param {number} maxMemoryInBytes - The maximum allowed memory in bytes for string values. + */ + constructor(maxMemoryInBytes) { + if (typeof maxMemoryInBytes !== 'number' || maxMemoryInBytes <= 0) { + throw new Error('maxMemoryInBytes must be a positive number'); + } + this.maxMemory = maxMemoryInBytes; + this.currentMemory = 0; + this.map = new Map(); + this.queue = []; + } + + /** + * Estimates the memory usage of a string in bytes. + * Assumes each character occupies 2 bytes (UTF-16). + * @param {string} str + * @returns {number} + */ + static estimateStringSize(str) { + return str.length * 2; + } + + /** + * Adds or updates a key-value pair in the map. + * If adding the new value exceeds the memory limit, evicts oldest entries. + * @param {string} key + * @param {string} value + */ + set(key, value) { + if (typeof key !== 'string' || typeof value !== 'string') { + return; + } + + const newValueSize = LimitedMap.estimateStringSize(value); + + // If the new value itself exceeds the max memory, reject it + if (newValueSize > this.maxMemory) { + return; + } + + // Check if the key already exists to adjust memory accordingly + if (this.map.has(key)) { + const oldValue = this.map.get(key); + const oldValueSize = LimitedMap.estimateStringSize(oldValue); + this.currentMemory -= oldValueSize; + // Remove the key from its current position in the queue + const index = this.queue.indexOf(key); + if (index > -1) { + this.queue.splice(index, 1); + } + } + + // Evict oldest entries until there's enough space + while (this.currentMemory + newValueSize > this.maxMemory && this.queue.length > 0) { + const oldestKey = this.queue.shift(); + const oldestValue = this.map.get(oldestKey); + const oldestValueSize = LimitedMap.estimateStringSize(oldestValue); + this.map.delete(oldestKey); + this.currentMemory -= oldestValueSize; + } + + // After eviction, check again if there's enough space + if (this.currentMemory + newValueSize > this.maxMemory) { + return; + } + + // Add the new key-value pair + this.map.set(key, value); + this.queue.push(key); + this.currentMemory += newValueSize; + } + + /** + * Retrieves the value associated with the given key. + * @param {string} key + * @returns {string | undefined} + */ + get(key) { + return this.map.get(key); + } + + /** + * Checks if the map contains the given key. + * @param {string} key + * @returns {boolean} + */ + has(key) { + return this.map.has(key); + } + + /** + * Deletes the key-value pair associated with the given key. + * @param {string} key + * @returns {boolean} - Returns true if the key was found and deleted, else false. + */ + delete(key) { + if (!this.map.has(key)) { + return false; + } + const value = this.map.get(key); + const valueSize = LimitedMap.estimateStringSize(value); + this.map.delete(key); + this.currentMemory -= valueSize; + + // Remove the key from the queue + const index = this.queue.indexOf(key); + if (index > -1) { + this.queue.splice(index, 1); + } + + return true; + } + + /** + * Clears all entries from the map. + */ + clear() { + this.map.clear(); + this.queue = []; + this.currentMemory = 0; + } + + /** + * Returns the number of key-value pairs in the map. + * @returns {number} + */ + size() { + return this.map.size; + } + + /** + * Returns the current memory usage in bytes. + * @returns {number} + */ + totalMemory() { + return this.currentMemory; + } + + /** + * Returns an iterator over the keys in the map. + * @returns {IterableIterator} + */ + keys() { + return this.map.keys(); + } + + /** + * Returns an iterator over the values in the map. + * @returns {IterableIterator} + */ + values() { + return this.map.values(); + } + + /** + * Iterates over the map in insertion order. + * @param {Function} callback - Function to execute for each element. + */ + forEach(callback) { + this.map.forEach((value, key) => { + callback(value, key, this); + }); + } + + /** + * Makes the LimitedMap iterable. + * @returns {Iterator} - Iterator over [key, value] pairs. + */ + [Symbol.iterator]() { + return this.map[Symbol.iterator](); + } +} From c873362d0140208857d1857e637916d00d1c4d9a Mon Sep 17 00:00:00 2001 From: Cohee <18619528+Cohee1207@users.noreply.github.com> Date: Fri, 29 Nov 2024 01:28:10 +0200 Subject: [PATCH 2/5] Safer estimation of possibly undefined values --- src/util.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util.js b/src/util.js index d03fb6a9c..8a1334259 100644 --- a/src/util.js +++ b/src/util.js @@ -696,7 +696,7 @@ export class LimitedMap { * @returns {number} */ static estimateStringSize(str) { - return str.length * 2; + return str ? str.length * 2 : 0; } /** From e8004b5b56c20b4395c478e58aaf34ce91d266c3 Mon Sep 17 00:00:00 2001 From: ceruleandeep Date: Fri, 29 Nov 2024 11:52:57 +1100 Subject: [PATCH 3/5] Wire up id= parameter for /qr-context* Parameter is in the named arguments but was not handled in the handler. Added `args.id !== undefined ? Number(args.id) : args.label` etc, as used elsewhere. --- .../quick-reply/src/SlashCommandHandler.js | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/public/scripts/extensions/quick-reply/src/SlashCommandHandler.js b/public/scripts/extensions/quick-reply/src/SlashCommandHandler.js index f6b339cdf..15f7524a4 100644 --- a/public/scripts/extensions/quick-reply/src/SlashCommandHandler.js +++ b/public/scripts/extensions/quick-reply/src/SlashCommandHandler.js @@ -419,30 +419,35 @@ export class SlashCommandHandler { namedArgumentList: [ SlashCommandNamedArgument.fromProps({ name: 'set', - description: 'QR set name', + description: 'Name of QR set to add the context menu to', typeList: [ARGUMENT_TYPE.STRING], isRequired: true, enumProvider: localEnumProviders.qrSets, }), SlashCommandNamedArgument.fromProps({ name: 'label', - description: 'Quick Reply label', + description: 'Label of Quick Reply to add the context menu to', typeList: [ARGUMENT_TYPE.STRING], enumProvider: localEnumProviders.qrEntries, }), SlashCommandNamedArgument.fromProps({ name: 'id', - description: 'numeric ID of the QR, e.g., id=42', + description: 'Numeric ID of Quick Reply to add the context menu to, e.g. id=42', typeList: [ARGUMENT_TYPE.NUMBER], enumProvider: localEnumProviders.qrIds, }), new SlashCommandNamedArgument( - 'chain', 'boolean', [ARGUMENT_TYPE.BOOLEAN], false, false, 'false', + 'chain', + 'If true, button QR is sent together with (before) the clicked QR from the context menu', + [ARGUMENT_TYPE.BOOLEAN], + false, + false, + 'false', ), ], unnamedArgumentList: [ SlashCommandArgument.fromProps({ - description: 'QR set name', + description: 'Name of QR set to add as a context menu', typeList: [ARGUMENT_TYPE.STRING], isRequired: true, enumProvider: localEnumProviders.qrSets, @@ -450,13 +455,16 @@ export class SlashCommandHandler { ], helpString: `
- Add context menu preset to a QR. + Add a context menu preset to a QR. +
+
+ If id and label are both provided, id will be used.
Example:
  • - 
    /qr-contextadd set=MyPreset label=MyButton chain=true MyOtherPreset
    + 
    /qr-contextadd set=MyQRSetWithTheButton label=MyButton chain=true MyQRSetWithContextItems
@@ -470,27 +478,27 @@ export class SlashCommandHandler { namedArgumentList: [ SlashCommandNamedArgument.fromProps({ name: 'set', - description: 'QR set name', + description: 'Name of QR set to remove the context menu from', typeList: [ARGUMENT_TYPE.STRING], isRequired: true, enumProvider: localEnumProviders.qrSets, }), SlashCommandNamedArgument.fromProps({ name: 'label', - description: 'Quick Reply label', + description: 'Label of Quick Reply to remove the context menu from', typeList: [ARGUMENT_TYPE.STRING], enumProvider: localEnumProviders.qrEntries, }), SlashCommandNamedArgument.fromProps({ name: 'id', - description: 'numeric ID of the QR, e.g., id=42', + description: 'Numeric ID of Quick Reply to remove the context menu from, e.g. id=42', typeList: [ARGUMENT_TYPE.NUMBER], enumProvider: localEnumProviders.qrIds, }), ], unnamedArgumentList: [ SlashCommandArgument.fromProps({ - description: 'QR set name', + description: 'Name of QR set to remove', typeList: [ARGUMENT_TYPE.STRING], isRequired: true, enumProvider: localEnumProviders.qrSets, @@ -500,6 +508,9 @@ export class SlashCommandHandler {
Remove context menu preset from a QR.
+
+ If id and label are both provided, id will be used. +
Example:
    @@ -541,6 +552,9 @@ export class SlashCommandHandler {
    Remove all context menu presets from a QR.
    +
    + If id and a label are both provided, id will be used. +
    Example:
      @@ -908,12 +922,11 @@ export class SlashCommandHandler { } } - createContextItem(args, name) { try { this.api.createContextItem( args.set, - args.label, + args.id !== undefined ? Number(args.id) : args.label, name, isTrueBoolean(args.chain), ); @@ -923,14 +936,14 @@ export class SlashCommandHandler { } deleteContextItem(args, name) { try { - this.api.deleteContextItem(args.set, args.label, name); + this.api.deleteContextItem(args.set, args.id !== undefined ? Number(args.id) : args.label, name); } catch (ex) { toastr.error(ex.message); } } clearContextMenu(args, label) { try { - this.api.clearContextMenu(args.set, args.label ?? label); + this.api.clearContextMenu(args.set, args.id !== undefined ? Number(args.id) : args.label ?? label); } catch (ex) { toastr.error(ex.message); } From 3a1a955164427cfbae63c4ad97845e500dba676b Mon Sep 17 00:00:00 2001 From: ceruleandeep Date: Fri, 29 Nov 2024 17:20:11 +1100 Subject: [PATCH 4/5] Make QR context menu display options more consistent with QR bar Use QR title as tooltip if set on the QR Add qr--hidden class to "invisible" context items to allow hiding with CSS Add title and isHidden props to MenuItem Remove domIcon and domLabel props: not needed for ctx menu rendering; isForceExpanded: unimplemented --- .../quick-reply/src/ui/ctx/ContextMenu.js | 6 ++- .../quick-reply/src/ui/ctx/MenuHeader.js | 2 +- .../quick-reply/src/ui/ctx/MenuItem.js | 46 ++++++++++++++++--- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/public/scripts/extensions/quick-reply/src/ui/ctx/ContextMenu.js b/public/scripts/extensions/quick-reply/src/ui/ctx/ContextMenu.js index 48ac3fbae..3741d2869 100644 --- a/public/scripts/extensions/quick-reply/src/ui/ctx/ContextMenu.js +++ b/public/scripts/extensions/quick-reply/src/ui/ctx/ContextMenu.js @@ -19,7 +19,7 @@ export class ContextMenu { this.itemList = this.build(qr).children; this.itemList.forEach(item => { item.onExpand = () => { - this.itemList.filter(it => it != item) + this.itemList.filter(it => it !== item) .forEach(it => it.collapse()); }; }); @@ -36,7 +36,9 @@ export class ContextMenu { icon: qr.icon, showLabel: qr.showLabel, label: qr.label, + title: qr.title, message: (chainedMessage && qr.message ? `${chainedMessage} | ` : '') + qr.message, + isHidden: qr.isHidden, children: [], }; qr.contextList.forEach((cl) => { @@ -51,7 +53,9 @@ export class ContextMenu { subTree.icon, subTree.showLabel, subTree.label, + subTree.title, subTree.message, + subTree.isHidden, (evt) => { evt.stopPropagation(); const finalQr = Object.assign(new QuickReply(), subQr); diff --git a/public/scripts/extensions/quick-reply/src/ui/ctx/MenuHeader.js b/public/scripts/extensions/quick-reply/src/ui/ctx/MenuHeader.js index 2fe6f7a68..c4c10148c 100644 --- a/public/scripts/extensions/quick-reply/src/ui/ctx/MenuHeader.js +++ b/public/scripts/extensions/quick-reply/src/ui/ctx/MenuHeader.js @@ -2,7 +2,7 @@ import { MenuItem } from './MenuItem.js'; export class MenuHeader extends MenuItem { constructor(/**@type {String}*/label) { - super(null, null, label, null, null); + super(null, null, label, null, null, false, null, []); } diff --git a/public/scripts/extensions/quick-reply/src/ui/ctx/MenuItem.js b/public/scripts/extensions/quick-reply/src/ui/ctx/MenuItem.js index 2b4daba99..f6a5db069 100644 --- a/public/scripts/extensions/quick-reply/src/ui/ctx/MenuItem.js +++ b/public/scripts/extensions/quick-reply/src/ui/ctx/MenuItem.js @@ -4,11 +4,12 @@ export class MenuItem { /**@type {string}*/ icon; /**@type {boolean}*/ showLabel; /**@type {string}*/ label; + /**@type {string}*/ title; /**@type {object}*/ value; + /**@type {boolean}*/ isHidden = false; /**@type {function}*/ callback; /**@type {MenuItem[]}*/ childList = []; /**@type {SubMenu}*/ subMenu; - /**@type {boolean}*/ isForceExpanded = false; /**@type {HTMLElement}*/ root; @@ -19,35 +20,67 @@ export class MenuItem { /** * - * @param {string} icon - * @param {boolean} showLabel + * @param {?string} icon + * @param {?boolean} showLabel * @param {string} label + * @param {?string} title Tooltip * @param {object} value + * @param {boolean} isHidden QR is Invisible (auto-execute only) * @param {function} callback * @param {MenuItem[]} children */ - constructor(icon, showLabel, label, value, callback, children = []) { + constructor(icon, showLabel, label, title, value, isHidden, callback, children = []) { this.icon = icon; this.showLabel = showLabel; this.label = label; + this.title = title; this.value = value; + this.isHidden = isHidden; this.callback = callback; this.childList = children; } + /** + * Renders the MenuItem + * + * A .qr--hidden class is added to: + * - the item if it is "Invisible (auto-execute only)" + * - the icon if no icon is set + * - the label if an icon is set and showLabel is false + * + * There is no .qr--hidden class defined in default CSS, since having items + * that are invisible on the QR bar but visible in the context menu, + * or icon-only on the QR bar but labelled in the context menu, is a valid use case. + * + * To hide optional labels when icons are present, add this user CSS: + * .ctx-menu .ctx-item .qr--button-label.qr--hidden {display: none;} + * To hide icons when no icon is present (removes unwanted padding): + * .ctx-menu .ctx-item .qr--button-icon.qr--hidden {display: none;} + * To hide items that are set "invisible": + * .ctx-menu .ctx-item.qr--hidden {display: none;} + * To target submenus only, use .ctx-menu .ctx-sub-menu .qr--hidden {display: none;} + * + * @returns {HTMLElement} + */ render() { if (!this.root) { const item = document.createElement('li'); { this.root = item; item.classList.add('list-group-item'); item.classList.add('ctx-item'); - item.title = this.value; + + // if this item is Invisible, add the hidden class + if (this.isHidden) item.classList.add('qr--hidden'); + + // if a title/tooltip is set, add it, otherwise use the QR content + // same as for the main QR list + item.title = this.title || this.value; + if (this.callback) { item.addEventListener('click', (evt) => this.callback(evt, this)); } const icon = document.createElement('div'); { - this.domIcon = icon; icon.classList.add('qr--button-icon'); icon.classList.add('fa-solid'); if (!this.icon) icon.classList.add('qr--hidden'); @@ -55,7 +88,6 @@ export class MenuItem { item.append(icon); } const lbl = document.createElement('div'); { - this.domLabel = lbl; lbl.classList.add('qr--button-label'); if (this.icon && !this.showLabel) lbl.classList.add('qr--hidden'); lbl.textContent = this.label; From 095d19cda73742ee23daedb3af8c7b55615b4789 Mon Sep 17 00:00:00 2001 From: Cohee <18619528+Cohee1207@users.noreply.github.com> Date: Fri, 29 Nov 2024 12:33:48 +0000 Subject: [PATCH 5/5] Rename LimitedMap => MemoryLimitedMap --- src/endpoints/characters.js | 4 ++-- src/util.js | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/endpoints/characters.js b/src/endpoints/characters.js index 1a209e710..b3be9d6f4 100644 --- a/src/endpoints/characters.js +++ b/src/endpoints/characters.js @@ -14,7 +14,7 @@ import jimp from 'jimp'; import { AVATAR_WIDTH, AVATAR_HEIGHT } from '../constants.js'; import { jsonParser, urlencodedParser } from '../express-common.js'; -import { deepMerge, humanizedISO8601DateTime, tryParse, extractFileFromZipBuffer, LimitedMap } from '../util.js'; +import { deepMerge, humanizedISO8601DateTime, tryParse, extractFileFromZipBuffer, MemoryLimitedMap } from '../util.js'; import { TavernCardValidator } from '../validator/TavernCardValidator.js'; import { parse, write } from '../character-card-parser.js'; import { readWorldInfoFile } from './worldinfo.js'; @@ -24,7 +24,7 @@ const defaultAvatarPath = './public/img/ai4.png'; // KV-store for parsed character data // 100 MB limit. Would take roughly 3000 characters to reach this limit -const characterDataCache = new LimitedMap(1024 * 1024 * 100); +const characterDataCache = new MemoryLimitedMap(1024 * 1024 * 100); // Some Android devices require tighter memory management const isAndroid = process.platform === 'android'; diff --git a/src/util.js b/src/util.js index 8a1334259..623626d3d 100644 --- a/src/util.js +++ b/src/util.js @@ -672,11 +672,11 @@ export function isValidUrl(url) { } /** - * LimitedMap class that limits the memory usage of string values. + * MemoryLimitedMap class that limits the memory usage of string values. */ -export class LimitedMap { +export class MemoryLimitedMap { /** - * Creates an instance of LimitedMap. + * Creates an instance of MemoryLimitedMap. * @param {number} maxMemoryInBytes - The maximum allowed memory in bytes for string values. */ constructor(maxMemoryInBytes) { @@ -710,7 +710,7 @@ export class LimitedMap { return; } - const newValueSize = LimitedMap.estimateStringSize(value); + const newValueSize = MemoryLimitedMap.estimateStringSize(value); // If the new value itself exceeds the max memory, reject it if (newValueSize > this.maxMemory) { @@ -720,7 +720,7 @@ export class LimitedMap { // Check if the key already exists to adjust memory accordingly if (this.map.has(key)) { const oldValue = this.map.get(key); - const oldValueSize = LimitedMap.estimateStringSize(oldValue); + const oldValueSize = MemoryLimitedMap.estimateStringSize(oldValue); this.currentMemory -= oldValueSize; // Remove the key from its current position in the queue const index = this.queue.indexOf(key); @@ -733,7 +733,7 @@ export class LimitedMap { while (this.currentMemory + newValueSize > this.maxMemory && this.queue.length > 0) { const oldestKey = this.queue.shift(); const oldestValue = this.map.get(oldestKey); - const oldestValueSize = LimitedMap.estimateStringSize(oldestValue); + const oldestValueSize = MemoryLimitedMap.estimateStringSize(oldestValue); this.map.delete(oldestKey); this.currentMemory -= oldestValueSize; } @@ -777,7 +777,7 @@ export class LimitedMap { return false; } const value = this.map.get(key); - const valueSize = LimitedMap.estimateStringSize(value); + const valueSize = MemoryLimitedMap.estimateStringSize(value); this.map.delete(key); this.currentMemory -= valueSize; @@ -842,7 +842,7 @@ export class LimitedMap { } /** - * Makes the LimitedMap iterable. + * Makes the MemoryLimitedMap iterable. * @returns {Iterator} - Iterator over [key, value] pairs. */ [Symbol.iterator]() {