Serialize settings saves with copy-on-write semantics (#27)

* Fix release workflow tag filter and serialize all settings saves

The release workflow tag filter used regex syntax ([0-9]+) but GitHub
Actions expects glob patterns, so it would never match semver tags.
Replace with *.*.*; the manifest/tag equality check remains as strict
validation.

Move the settings save queue from BulkPropertiesSettingTab into the
plugin so all callers (settings tab, bulk-edit modal) share a single
serialized write path, preventing concurrent saves from overwriting
each other.

* Revert release workflow tag filter change

The original [0-9]+.[0-9]+.[0-9]+ pattern is correct. GitHub Actions
filter patterns support + as "one or more of the preceding character",
unlike standard globs. The review finding was based on the incorrect
assumption that + is literal in this context.

* Use copy-on-write semantics for serialized settings saves

updateSetting() on the plugin builds a candidate snapshot and only
assigns it to this.settings after saveData() succeeds. On failure,
in-memory settings remain unchanged, preventing a later unrelated
save from persisting a value that was reported as failed.

Both the settings tab and bulk-edit modal now use this single method
instead of mutating settings before saving.
This commit is contained in:
Gary Ritchie 2026-04-03 09:13:41 -06:00 committed by GitHub
parent a7eacdace0
commit 7e44237185
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 29 additions and 23 deletions

View file

@ -644,9 +644,8 @@ export class BulkEditModal extends Modal {
const selProp = this.plugin.settings.selectionProperty;
const deselect = this.deselectWhenFinished;
this.plugin.settings.lastSelectedProperty = property;
try {
await this.plugin.saveSettings();
await this.plugin.updateSetting("lastSelectedProperty", property);
} catch (err: unknown) {
console.warn("bulk-properties: failed to save lastSelectedProperty:", err);
}

View file

@ -10,6 +10,7 @@ export default class BulkPropertiesPlugin extends Plugin {
settings!: BulkPropertiesSettings;
private statusBarEl: HTMLElement | null = null;
private statusBarTimer: ReturnType<typeof setTimeout> | null = null;
private saveQueue: Promise<void> = Promise.resolve();
override async onload() {
await this.loadSettings();
@ -167,7 +168,21 @@ export default class BulkPropertiesPlugin extends Plugin {
}
}
async saveSettings() {
await this.saveData(this.settings);
/**
* Serializes settings writes through a queue with copy-on-write
* semantics. Builds a candidate snapshot per call; only assigns
* it to `this.settings` after persistence succeeds.
*/
updateSetting<K extends keyof BulkPropertiesSettings>(
key: K,
value: BulkPropertiesSettings[K],
): Promise<void> {
const save = this.saveQueue.then(async () => {
const candidate = {...this.settings, [key]: value};
await this.saveData(candidate);
this.settings = candidate;
});
this.saveQueue = save.then(() => {}, () => {});
return save;
}
}

View file

@ -98,7 +98,6 @@ export const DEFAULT_SETTINGS: BulkPropertiesSettings = {
export class BulkPropertiesSettingTab extends PluginSettingTab {
plugin: BulkPropertiesPlugin;
private saveQueue: Promise<void> = Promise.resolve();
constructor(app: App, plugin: BulkPropertiesPlugin) {
super(app, plugin);
@ -106,28 +105,21 @@ export class BulkPropertiesSettingTab extends PluginSettingTab {
}
/**
* Serializes settings writes through a queue so overlapping
* onChange calls cannot race. Each call snapshots the live
* settings only after all prior saves have committed.
* Updates a single setting key via the plugin's serialized
* copy-on-write save queue.
*/
private updateSetting<K extends keyof BulkPropertiesSettings>(
private async updateSetting<K extends keyof BulkPropertiesSettings>(
key: K,
value: BulkPropertiesSettings[K],
): Promise<boolean> {
const result = this.saveQueue.then(async () => {
const candidate = {...this.plugin.settings, [key]: value};
try {
await this.plugin.saveData(candidate);
this.plugin.settings = candidate;
return true;
} catch (err: unknown) {
console.error("bulk-properties: failed to save settings:", err);
new Notice("Failed to save settings. Check disk space and permissions.");
return false;
}
});
this.saveQueue = result.then(() => {});
return result;
try {
await this.plugin.updateSetting(key, value);
return true;
} catch (err: unknown) {
console.error("bulk-properties: failed to save settings:", err);
new Notice("Failed to save settings. Check disk space and permissions.");
return false;
}
}
display(): void {