9.9 KiB
9.9 KiB
Edge Case Hunter — Story 4.2 Code Review
You are an Edge Case Hunter. You receive the diff below AND read access to the project. Walk every branching path and boundary condition. Report ONLY unhandled edge cases.
Instructions
- Review the diff against the existing project codebase.
- Walk all: conditional branches, type transitions, null/undefined checks, boundary values, error paths, async flows, DOM states, mock/fake surfaces, API integration points.
- Report ONLY issues where an edge case is unhandled or handled incorrectly — not general critique.
- Each finding: one-line title, file + line, the edge condition, and why it matters.
- If every edge case is properly handled, say "No unhandled edge cases found."
Project Access
You have read access to the full project at: /home/morr/work/foundryvtt/video-view-manager/
Key files to cross-reference:
src/contracts/privacy-settings.js— existing contract (read the full file for context around the diff)src/core/PlayerPrivacyManager.js— existing manager (read the full file for context around the diff)src/core/PortraitFallbackHandler.js— new file (shown in full below)src/foundry/FoundryAdapter.js— adapter interfacesrc/ui/RoleRenderer.js— eventual integration targetsrc/ui/player/PlayerPrivacyPanel.js— eventual UI targettests/helpers/foundryAdapterMock.js— existing mock patternstests/helpers/playerPrivacyManagerMock.js— new mock helper
Diff
=== MODIFIED FILES ===
diff --git a/src/contracts/privacy-settings.js b/src/contracts/privacy-settings.js
index b838445..f618241 100644
--- a/src/contracts/privacy-settings.js
+++ b/src/contracts/privacy-settings.js
@@ -5,15 +5,34 @@
* their on-screen presence. Settings are stored as user flags on the user document.
*
* Storage key: game.user.setFlag('video-view-manager', key, value)
- * Shape: { reactionCamEnabled: boolean, hpReactiveCamStylingEnabled: boolean }
+ * Shape: { reactionCamEnabled: boolean, hpReactiveCamStylingEnabled: boolean, customPortraitFallback: string | null }
*
* @module contracts/privacy-settings
*/
+export const MAX_PORTRAIT_SIZE = 5 * 1024 * 1024;
+
+export const VALID_PORTRAIT_FORMATS = Object.freeze([
+ "image/png",
+ "image/jpeg",
+ "image/webp",
+ "image/gif",
+]);
+
/**
* @typedef {Object} PrivacySettings
* @property {boolean} reactionCamEnabled
* @property {boolean} hpReactiveCamStylingEnabled
+ * @property {string|null} customPortraitFallback
*/
export const PRIVACY_SETTINGS_VERSION = 1;
@@ -25,6 +44,7 @@ export const PRIVACY_SETTINGS_VERSION = 1;
export const PRIVACY_SETTINGS_DEFAULT = {
reactionCamEnabled: false,
hpReactiveCamStylingEnabled: false,
+ customPortraitFallback: null,
};
@@ -34,6 +54,7 @@ export const PRIVACY_SETTING_KEYS = Object.freeze([
"reactionCamEnabled",
"hpReactiveCamStylingEnabled",
+ "customPortraitFallback",
]);
@@ -45,6 +66,49 @@ export const FEATURE_NAME_MAP = Object.freeze({
hpReactiveCamStyling: "hpReactiveCamStylingEnabled",
});
+export function validatePortraitDataURL(dataURL) {
+ if (dataURL === null || dataURL === undefined) {
+ return dataURL;
+ }
+ if (typeof dataURL !== "string") {
+ throw new TypeError(`Invalid DataURL: expected string, got ${typeof dataURL}`);
+ }
+ if (dataURL === "") {
+ return dataURL;
+ }
+
+ if (!dataURL.startsWith("data:")) {
+ throw new TypeError("Invalid DataURL format: must start with 'data:'");
+ }
+
+ const mimeMatch = dataURL.match(/^data:(image\/[a-zA-Z0-9+\-.]+|video\/[a-zA-Z0-9+\-.]+)/);
+ if (!mimeMatch) {
+ throw new TypeError("Invalid DataURL format: missing or invalid MIME type");
+ }
+
+ const mimeType = mimeMatch[1].toLowerCase();
+
+ if (!VALID_PORTRAIT_FORMATS.includes(mimeType)) {
+ throw new TypeError(
+ `Unsupported portrait format: ${mimeType}. Supported: ${VALID_PORTRAIT_FORMATS.join(", ")}`
+ );
+ }
+
+ return dataURL;
+}
+
/**
* Creates a new PrivacySettings object with defaults.
*/
@@ -63,6 +127,7 @@ export function createPrivacySettings(overrides = {}) {
/**
* Validates a PrivacySettings DTO. Throws TypeError on any violation.
+ * Backward compatible: accepts settings without customPortraitFallback key.
*/
@@ -75,23 +140,50 @@ export function isValidPrivacySettings(data) {
throw new TypeError("PrivacySettings: must be an object");
}
const obj = /** @type {Record<string, unknown>} */ (data);
- const { reactionCamEnabled, hpReactiveCamStylingEnabled, ...rest } = obj;
- if (Object.keys(rest).length > 0) {
+
+ const knownKeys = new Set(PRIVACY_SETTING_KEYS);
+ const unknownKeys = Object.keys(obj).filter((k) => !knownKeys.has(k));
+ if (unknownKeys.length > 0) {
throw new TypeError(
- `PrivacySettings: unknown keys: ${Object.keys(rest).join(", ")}`
+ `PrivacySettings: unknown keys: ${unknownKeys.join(", ")}`
);
}
- if (typeof reactionCamEnabled !== "boolean") {
- throw new TypeError(
- `PrivacySettings: reactionCamEnabled must be a boolean, got ${typeof reactionCamEnabled}`
- );
+
+ if ("reactionCamEnabled" in obj) {
+ if (typeof obj.reactionCamEnabled !== "boolean") {
+ throw new TypeError(
+ `PrivacySettings: reactionCamEnabled must be a boolean, got ${typeof obj.reactionCamEnabled}`
+ );
+ }
}
- if (typeof hpReactiveCamStylingEnabled !== "boolean") {
- throw new TypeError(
- `PrivacySettings: hpReactiveCamStylingEnabled must be a boolean, got ${typeof hpReactiveCamStylingEnabled}`
- );
+ if ("hpReactiveCamStylingEnabled" in obj) {
+ if (typeof obj.hpReactiveCamStylingEnabled !== "boolean") {
+ throw new TypeError(
+ `PrivacySettings: hpReactiveCamStylingEnabled must be a boolean, got ${typeof obj.hpReactiveCamStylingEnabled}`
+ );
+ }
}
- return /** @type {PrivacySettings} */ (data);
+ if ("customPortraitFallback" in obj) {
+ if (obj.customPortraitFallback !== null && typeof obj.customPortraitFallback !== "string") {
+ throw new TypeError(
+ `PrivacySettings: customPortraitFallback must be a string or null, got ${typeof obj.customPortraitFallback}`
+ );
+ }
+ if (typeof obj.customPortraitFallback === "string" && obj.customPortraitFallback !== "") {
+ try {
+ validatePortraitDataURL(obj.customPortraitFallback);
+ } catch (e) {
+ throw new TypeError(
+ `PrivacySettings: customPortraitFallback ${e.message}`
+ );
+ }
+ }
+ }
+
+ return /** @type {PrivacySettings} */ (obj);
}
diff --git a/src/core/PlayerPrivacyManager.js b/src/core/PlayerPrivacyManager.js
index 117afa2..9347edd 100644
--- a/src/core/PlayerPrivacyManager.js
+++ b/src/core/PlayerPrivacyManager.js
@@ -15,9 +15,11 @@ import {
PRIVACY_SETTINGS_DEFAULT,
PRIVACY_SETTING_KEYS,
FEATURE_NAME_MAP,
+ MAX_PORTRAIT_SIZE,
validateSettingKey,
validateSettingValue,
validateFeatureName,
+ validatePortraitDataURL,
} from "../contracts/privacy-settings.js";
@@ -118,12 +120,19 @@ export class PlayerPrivacyManager {
* @param {string} key
* @param {boolean} value
*/
async setSetting(userId, key, value) {
+ if (key === "customPortraitFallback") {
+ throw new TypeError(
+ "PlayerPrivacyManager: customPortraitFallback must use setPortraitFallback() method"
+ );
+ }
+
// Validate key
validateSettingKey(key);
@@ -245,6 +254,149 @@ export class PlayerPrivacyManager {
}
}
+ async setPortraitFallback(userId, dataURL) {
+ validatePortraitDataURL(dataURL);
+
+ const user = this._adapter.users.get(userId);
+ if (!user) {
+ throw new TypeError(`PlayerPrivacyManager: User '${userId}' not found`);
+ }
+
+ if (typeof user.setFlag !== "function") {
+ throw new TypeError(`PlayerPrivacyManager: User '${userId}' does not support setFlag`);
+ }
+
+ const previousValue = this.getPortraitFallback(userId);
+
+ await user.setFlag("video-view-manager", "customPortraitFallback", dataURL);
+
+ this._notifyPortraitChange(userId, dataURL, previousValue);
+ }
+
+ getPortraitFallback(userId) {
+ const user = this._adapter.users.get(userId);
+
+ if (!user || typeof user.getFlag !== "function") {
+ return null;
+ }
+
+ const dataURL = user.getFlag("video-view-manager", "customPortraitFallback");
+
+ if (dataURL !== null && dataURL !== undefined) {
+ try {
+ if (typeof dataURL === "string" && dataURL !== "") {
+ validatePortraitDataURL(dataURL);
+ }
+ return dataURL ?? null;
+ } catch (e) {
+ console.warn(`[ScryingPool] PlayerPrivacyManager: Invalid stored portrait DataURL for user '${userId}': ${e.message}`);
+ return null;
+ }
+ }
+
+ return null;
+ }
+
+ getPortraitFallbackDataURL(userId) {
+ return this.getPortraitFallback(userId);
+ }
+
+ async removePortraitFallback(userId) {
+ const user = this._adapter.users.get(userId);
+ if (!user) {
+ throw new TypeError(`PlayerPrivacyManager: User '${userId}' not found`);
+ }
+
+ if (typeof user.unsetFlag !== "function") {
+ throw new TypeError(`PlayerPrivacyManager: User '${userId}' does not support unsetFlag`);
+ }
+
+ const previousValue = this.getPortraitFallback(userId);
+
+ await user.unsetFlag("video-view-manager", "customPortraitFallback");
+
+ this._notifyPortraitChange(userId, null, previousValue);
+ }
+
+ _notifyPortraitChange(userId, newValue, previousValue) {
+ for (const callback of this._subscribers) {
+ try {
+ callback(userId, "customPortraitFallback", newValue, previousValue);
+ } catch (err) {
+ console.error(`[ScryingPool] PlayerPrivacyManager portrait subscriber error:`, err);
+ }
+ }
+ }
+
/**
* Cleans up internal state.
teardown() { ... }
=== NEW FILES ===
src/core/PortraitFallbackHandler.js (249 lines — full file available at that path)
tests/helpers/playerPrivacyManagerMock.js (50 lines — full file available at that path)