Review improvements on NanoID PR: validation, caching, and duplicate detection

- Cache customAlphabet generator to avoid recreating on every call
- Add input validation: NanoID length (4-128), alphabet (min 2 unique chars), single-char separators
- Add UID duplicate detection via in-memory Set + filePath→uid Map (O(1) lookups)
- Build cache once on plugin load, keep in sync on create/delete/external edits
- Listen to metadataCache.on('changed') to detect external frontmatter modifications
- Rebuild cache when uidKey setting changes
- Use path map for reliable cache cleanup on file delete
- Fix escaped apostrophes in JSDoc comments
- Revert unrelated quote-style changes

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Netajam 2026-03-24 20:09:18 +01:00
parent 781a449690
commit 8e4644cfe5
3 changed files with 127 additions and 40 deletions

View file

@ -9,6 +9,8 @@ import * as uidUtils from './uidUtils';
export default class UIDGenerator extends Plugin {
settings: UIDGeneratorSettings;
uidCache: Set<string> = new Set();
uidPathMap: Map<string, string> = new Map(); // filePath → uid
async onload() {
await this.loadSettings();
@ -26,11 +28,46 @@ export default class UIDGenerator extends Plugin {
// could be implemented but adds complexity (managing timers per file path).
const debouncedFileHandler = debounce(commands.handleAutoGenerateUid.bind(null, this), 500, true);
this.app.workspace.onLayoutReady(() => {
this.buildUidCache();
this.registerEvent(this.app.vault.on('create', (file) => {
if (file instanceof TFile && file.extension === 'md') {
// Sync cache for files dropped/synced into vault that already have a UID
const existingUid = uidUtils.getUIDFromFile(this, file);
if (existingUid) {
this.uidCache.add(existingUid);
this.uidPathMap.set(file.path, existingUid);
}
debouncedFileHandler(file);
}
}));
this.registerEvent(this.app.vault.on('delete', (file) => {
if (file instanceof TFile && file.extension === 'md') {
// Use path map since metadata cache may already be cleared
const uid = this.uidPathMap.get(file.path);
if (uid) {
this.uidCache.delete(uid);
this.uidPathMap.delete(file.path);
}
}
}));
// Sync cache when frontmatter is edited externally or by another plugin
this.registerEvent(this.app.metadataCache.on('changed', (file) => {
if (file instanceof TFile && file.extension === 'md') {
const oldUid = this.uidPathMap.get(file.path);
const newUid = uidUtils.getUIDFromFile(this, file);
if (oldUid !== newUid) {
if (oldUid) {
this.uidCache.delete(oldUid);
}
if (newUid) {
this.uidCache.add(newUid);
this.uidPathMap.set(file.path, newUid);
} else {
this.uidPathMap.delete(file.path);
}
}
}
}));
this.registerEvent(this.app.workspace.on('file-open', (file) => {
if (file instanceof TFile && file.extension === 'md') {
const activeLeaf = this.app.workspace.getActiveViewOfType(MarkdownView);
@ -187,6 +224,9 @@ export default class UIDGenerator extends Plugin {
if (!Array.isArray(this.settings.autoGenerationExclusions)) {
this.settings.autoGenerationExclusions = [];
}
if (!Array.isArray(this.settings.nanoidSeparators)) {
this.settings.nanoidSeparators = [];
}
this.settings.copyFormatString = this.settings.copyFormatString || DEFAULT_SETTINGS.copyFormatString;
this.settings.copyFormatStringMissingUid = this.settings.copyFormatStringMissingUid || DEFAULT_SETTINGS.copyFormatStringMissingUid;
}
@ -196,6 +236,20 @@ export default class UIDGenerator extends Plugin {
}
// --- UID Cache ---
buildUidCache(): void {
this.uidCache.clear();
this.uidPathMap.clear();
const files = this.app.vault.getMarkdownFiles();
for (const file of files) {
const uid = uidUtils.getUIDFromFile(this, file);
if (uid) {
this.uidCache.add(uid);
this.uidPathMap.set(file.path, uid);
}
}
}
// --- Public method needed by Settings Tab ---
async clearUIDsInFolder(folderPath: string): Promise<void> {
await commands.handleClearUIDsInFolder(this, folderPath);

View file

@ -59,13 +59,17 @@ export class UIDSettingTab extends PluginSettingTab {
.setValue(this.plugin.settings.uidKey)
.onChange(async (value) => {
const cleanedValue = value.trim().replace(/\s+/g, '');
this.plugin.settings.uidKey = cleanedValue || DEFAULT_SETTINGS.uidKey;
const newKey = cleanedValue || DEFAULT_SETTINGS.uidKey;
const keyChanged = newKey !== this.plugin.settings.uidKey;
this.plugin.settings.uidKey = newKey;
if (value !== this.plugin.settings.uidKey) {
// Update input value if it was cleaned (e.g., spaces removed)
text.setValue(this.plugin.settings.uidKey);
}
await this.plugin.saveSettings();
this.display(); // Re-render to potentially update descriptions elsewhere
if (keyChanged) {
this.plugin.buildUidCache();
}
this.display();
}));
// --- UID Generator Type ---
@ -87,12 +91,12 @@ export class UIDSettingTab extends PluginSettingTab {
if (this.plugin.settings.uidGenerator === 'nanoid') {
new Setting(containerEl)
.setName('NanoID length')
.setDesc('Length of the generated ID (excluding injected characters).')
.setDesc('Length of the generated ID (excluding separators). Min 4, max 128.')
.addText(text => text
.setValue(String(this.plugin.settings.nanoidLength))
.onChange(async (value) => {
const num = parseInt(value);
if (!isNaN(num) && num > 0) {
if (!isNaN(num) && num >= 4 && num <= 128) {
this.plugin.settings.nanoidLength = num;
await this.plugin.saveSettings();
}
@ -100,11 +104,12 @@ export class UIDSettingTab extends PluginSettingTab {
new Setting(containerEl)
.setName('NanoID alphabet')
.setDesc('Custom characters to use for ID generation.')
.setDesc('Characters used for ID generation. Must have at least 2 unique characters.')
.addTextArea(text => text
.setValue(this.plugin.settings.nanoidAlphabet)
.onChange(async (value) => {
if (value.length > 0) {
const unique = new Set(value);
if (unique.size >= 2) {
this.plugin.settings.nanoidAlphabet = value;
await this.plugin.saveSettings();
}
@ -131,12 +136,13 @@ export class UIDSettingTab extends PluginSettingTab {
}));
new Setting(groupContainer)
.setName(`Inject character`)
.setDesc('Character to inject (e.g., "+").')
.setName('Inject character')
.setDesc('Single character to inject (e.g., "-").')
.addText(text => text
.setValue(separator.char)
.onChange(async (value) => {
this.plugin.settings.nanoidSeparators[index].char = value;
const char = value.slice(0, 1);
this.plugin.settings.nanoidSeparators[index].char = char;
await this.plugin.saveSettings();
}));
@ -161,7 +167,7 @@ export class UIDSettingTab extends PluginSettingTab {
.setButtonText('Add Group')
.setCta()
.onClick(async () => {
this.plugin.settings.nanoidSeparators.push({ char: '', position: -2 });
this.plugin.settings.nanoidSeparators.push({ char: '-', position: -2 });
await this.plugin.saveSettings();
this.display();
}));
@ -309,7 +315,7 @@ export class UIDSettingTab extends PluginSettingTab {
const uidKey = this.plugin.settings.uidKey;
// Basic validation for the folder path input
if (!folderPath || folderPath.trim() === '') {
new Notice('Please specify a folder path in the \'Folder to clear uids from\' setting above first.');
new Notice("Please specify a folder path in the 'Folder to clear uids from' setting above first.");
return; // Prevent proceeding without a path
}

View file

@ -3,38 +3,61 @@ import { v4 as uuidv4 } from 'uuid';
import { customAlphabet } from 'nanoid';
import UIDGenerator from './main';
// Cache the nanoid generator to avoid recreating on every call
let cachedNanoid: (() => string) | null = null;
let cachedAlphabet = '';
let cachedLength = 0;
function getNanoidGenerator(alphabet: string, length: number): () => string {
if (cachedNanoid && cachedAlphabet === alphabet && cachedLength === length) {
return cachedNanoid;
}
cachedAlphabet = alphabet;
cachedLength = length;
cachedNanoid = customAlphabet(alphabet, length);
return cachedNanoid;
}
/**
* Generates a unique ID using the UUID v4 standard.
* Generates a unique ID using UUID v4 or NanoID based on settings.
* @param plugin The UIDGenerator plugin instance (for settings).
*/
const MAX_COLLISION_RETRIES = 10;
export function generateUID(plugin: UIDGenerator): string {
for (let attempt = 0; attempt < MAX_COLLISION_RETRIES; attempt++) {
const id = generateRawUID(plugin);
if (!plugin.uidCache.has(id)) {
return id;
}
console.warn(`[UIDGenerator] Collision detected for "${id}", retrying (${attempt + 1}/${MAX_COLLISION_RETRIES})`);
}
// Fallback: return the last generated ID anyway (extremely unlikely to reach here)
const fallback = generateRawUID(plugin);
console.error(`[UIDGenerator] Failed to generate unique ID after ${MAX_COLLISION_RETRIES} attempts. Using potentially duplicate ID.`);
return fallback;
}
function generateRawUID(plugin: UIDGenerator): string {
if (plugin.settings.uidGenerator === 'nanoid') {
const alphabet = plugin.settings.nanoidAlphabet;
const length = plugin.settings.nanoidLength;
const nanoid = customAlphabet(alphabet, length);
const { nanoidAlphabet, nanoidLength, nanoidSeparators } = plugin.settings;
const nanoid = getNanoidGenerator(nanoidAlphabet, nanoidLength);
let id = nanoid();
const separators = plugin.settings.nanoidSeparators;
if (separators && separators.length > 0) {
// Create a mutable array of insertions, normalizing positions
const insertions = separators
.filter(s => s.char && s.char.length > 0) // Only process non-empty chars
if (nanoidSeparators && nanoidSeparators.length > 0) {
const insertions = nanoidSeparators
.filter(s => s.char && s.char.length === 1)
.map(s => {
let actualPosition = s.position;
// Normalize negative positions relative to the *initial* nanoid length
if (actualPosition < 0) {
actualPosition = length + actualPosition; // e.g., length=21, pos=-2 -> 19
let pos = s.position;
if (pos < 0) {
pos = nanoidLength + pos;
}
// Ensure position is within bounds [0, length]
actualPosition = Math.max(0, Math.min(length, actualPosition));
return { char: s.char, position: actualPosition };
pos = Math.max(0, Math.min(nanoidLength, pos));
return { char: s.char, position: pos };
})
.sort((a, b) => b.position - a.position); // Sort descending by position
.sort((a, b) => b.position - a.position);
// Apply insertions from right to left to avoid index shifting issues
for (const insertion of insertions) {
const { char, position } = insertion;
for (const { char, position } of insertions) {
id = id.slice(0, position) + char + id.slice(position);
}
}
@ -59,7 +82,7 @@ export function getUIDFromFile(plugin: UIDGenerator, file: TFile | null): string
return null;
}
const uidValue = frontmatter[plugin.settings.uidKey];
// Ensure it\'s treated as a string, even if stored as number in YAML
// Ensure it's treated as a string, even if stored as number in YAML
return typeof uidValue === 'string' || typeof uidValue === 'number' ? String(uidValue) : null;
} catch (error) {
console.error(`[UIDGenerator] Error reading metadata for ${file.path}:`, error);
@ -68,7 +91,7 @@ export function getUIDFromFile(plugin: UIDGenerator, file: TFile | null): string
}
/**
* Sets or updates the UID property in a file\'s frontmatter.
* Sets or updates the UID property in a file's frontmatter.
* @param plugin The UIDGenerator plugin instance.
* @param file The TFile to modify.
* @param uid The UID string to set.
@ -103,7 +126,7 @@ export async function setUID(plugin: UIDGenerator, file: TFile, uid: string, ove
frontmatter[key] = uid;
uidWasSetOrOverwritten = true;
} else {
// If overwrite is true but value is the same, we didn\'t *really* change it
// If overwrite is true but value is the same, we didn't *really* change it
uidWasSetOrOverwritten = false;
}
@ -115,8 +138,8 @@ export async function setUID(plugin: UIDGenerator, file: TFile, uid: string, ove
});
if (uidWasSetOrOverwritten) {
// Determine action based on initial state and overwrite flag
const action = initialUidExists && overwrite ? 'Overwrote' : 'Set';
plugin.uidCache.add(uid);
plugin.uidPathMap.set(file.path, uid);
}
return uidWasSetOrOverwritten;
@ -128,7 +151,7 @@ export async function setUID(plugin: UIDGenerator, file: TFile, uid: string, ove
}
/**
* Removes the UID property from a file\'s frontmatter, respecting the custom key.
* Removes the UID property from a file's frontmatter, respecting the custom key.
* @param plugin The UIDGenerator plugin instance.
* @param file The TFile to modify.
* @returns Promise<boolean> - True if a UID was found and removed, false otherwise.
@ -136,15 +159,19 @@ export async function setUID(plugin: UIDGenerator, file: TFile, uid: string, ove
export async function removeUID(plugin: UIDGenerator, file: TFile): Promise<boolean> {
if (!file || !(file instanceof TFile)) return false;
let uidWasPresent = false;
let removedUid: string | null = null;
const key = plugin.settings.uidKey;
try {
await plugin.app.fileManager.processFrontMatter(file, (frontmatter) => {
if (frontmatter.hasOwnProperty(key)) {
removedUid = String(frontmatter[key]);
uidWasPresent = true;
delete frontmatter[key];
}
});
if (uidWasPresent) {
if (uidWasPresent && removedUid) {
plugin.uidCache.delete(removedUid);
plugin.uidPathMap.delete(file.path);
}
return uidWasPresent;
} catch (error) {