mirror of
https://github.com/vrtmrz/obsidian-livesync.git
synced 2026-11-03 14:58:18 +00:00
Cancel stale conflict resolutions when queueing checks
This commit is contained in:
1 parent
17d1e3f50a
commit
8844a88399
4 files changed
+108
-3
No files matched your search
@@ -18,6 +18,7 @@ export interface ConflictCheckingDependencies {
|
|||||||
"getOptionalConflictCheckMethod" | "queueCheckFor" | "resolve" | "resolveByNewest"
|
"getOptionalConflictCheckMethod" | "queueCheckFor" | "resolve" | "resolveByNewest"
|
||||||
>;
|
>;
|
||||||
readonly conflictProcessQueueCount: ReactiveSource<number>;
|
readonly conflictProcessQueueCount: ReactiveSource<number>;
|
||||||
|
readonly cancelActiveResolution: (filename: FilePathWithPrefix) => void;
|
||||||
readonly currentSettings: () => ConflictCheckingSettings;
|
readonly currentSettings: () => ConflictCheckingSettings;
|
||||||
readonly vault: Pick<IVaultService, "getActiveFilePath">;
|
readonly vault: Pick<IVaultService, "getActiveFilePath">;
|
||||||
readonly log: LogFunction;
|
readonly log: LogFunction;
|
||||||
@@ -69,6 +70,10 @@ export function createConflictCheckingHandlers(dependencies: ConflictCheckingDep
|
|||||||
// The conflict should be resolved by the newer entry.
|
// The conflict should be resolved by the newer entry.
|
||||||
await dependencies.conflict.resolveByNewest(file);
|
await dependencies.conflict.resolveByNewest(file);
|
||||||
} else {
|
} else {
|
||||||
|
// The next batch waits for active checks to settle. Cancel the
|
||||||
|
// older same-file resolution now so its dialogue cannot hold the
|
||||||
|
// replacement behind that wait.
|
||||||
|
dependencies.cancelActiveResolution(file);
|
||||||
conflictQueue.enqueue(file);
|
conflictQueue.enqueue(file);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -695,6 +695,86 @@ describe("conflict resolution serviceFeature", () => {
|
|||||||
expect(harness.tryAutoMerge).toHaveBeenCalledWith(path, true);
|
expect(harness.tryAutoMerge).toHaveBeenCalledWith(path, true);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("replaces an active same-file dialogue when a later check enters the queue", async () => {
|
||||||
|
const path = "queued-dialogue-replacement.md" as FilePathWithPrefix;
|
||||||
|
const harness = createHarness({ settings: { syncAfterMerge: false } });
|
||||||
|
harness.tryAutoMerge.mockResolvedValue({
|
||||||
|
rightRev: "2-conflict",
|
||||||
|
leftLeaf: leaf("2-current", "Current\n", 2),
|
||||||
|
rightLeaf: leaf("2-conflict", "Conflict\n", 3),
|
||||||
|
});
|
||||||
|
let finishFirst: (() => void) | undefined;
|
||||||
|
const resolveByUserInteraction = vi
|
||||||
|
.fn()
|
||||||
|
.mockImplementationOnce(
|
||||||
|
() =>
|
||||||
|
new Promise<boolean>((resolve) => {
|
||||||
|
finishFirst = () => resolve(false);
|
||||||
|
})
|
||||||
|
)
|
||||||
|
.mockResolvedValue(false);
|
||||||
|
harness.conflict.resolveByUserInteraction.addHandler(resolveByUserInteraction);
|
||||||
|
const subscription = new AbortController();
|
||||||
|
harness.context.events.onEvent(
|
||||||
|
EVENT_CONFLICT_CANCELLED,
|
||||||
|
(filename) => {
|
||||||
|
if (filename === path) finishFirst?.();
|
||||||
|
},
|
||||||
|
{ signal: subscription.signal }
|
||||||
|
);
|
||||||
|
|
||||||
|
try {
|
||||||
|
await harness.conflict.queueCheckFor(path);
|
||||||
|
await vi.waitFor(() => expect(resolveByUserInteraction).toHaveBeenCalledOnce());
|
||||||
|
|
||||||
|
await harness.conflict.queueCheckFor(path);
|
||||||
|
|
||||||
|
await vi.waitFor(() => expect(resolveByUserInteraction).toHaveBeenCalledTimes(2));
|
||||||
|
await expect(harness.conflict.ensureAllProcessed()).resolves.toBe(true);
|
||||||
|
expect(harness.conflict.conflictProcessQueueCount.value).toBe(0);
|
||||||
|
} finally {
|
||||||
|
subscription.abort();
|
||||||
|
finishFirst?.();
|
||||||
|
await harness.conflict.ensureAllProcessed();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not open an outdated dialogue when a later check is queued during inspection", async () => {
|
||||||
|
const path = "queued-during-inspection.md" as FilePathWithPrefix;
|
||||||
|
const harness = createHarness();
|
||||||
|
const manualConflict = {
|
||||||
|
rightRev: "2-conflict",
|
||||||
|
leftLeaf: leaf("2-current", "Current\n", 2),
|
||||||
|
rightLeaf: leaf("2-conflict", "Conflict\n", 3),
|
||||||
|
};
|
||||||
|
let finishInspection!: (result: typeof manualConflict) => void;
|
||||||
|
harness.tryAutoMerge
|
||||||
|
.mockImplementationOnce(
|
||||||
|
() =>
|
||||||
|
new Promise<typeof manualConflict>((resolve) => {
|
||||||
|
finishInspection = resolve;
|
||||||
|
})
|
||||||
|
)
|
||||||
|
.mockResolvedValueOnce({ ok: NOT_CONFLICTED });
|
||||||
|
const resolveByUserInteraction = vi.fn(async () => false);
|
||||||
|
harness.conflict.resolveByUserInteraction.addHandler(resolveByUserInteraction);
|
||||||
|
|
||||||
|
try {
|
||||||
|
await harness.conflict.queueCheckFor(path);
|
||||||
|
await vi.waitFor(() => expect(harness.tryAutoMerge).toHaveBeenCalledOnce());
|
||||||
|
|
||||||
|
await harness.conflict.queueCheckFor(path);
|
||||||
|
finishInspection(manualConflict);
|
||||||
|
await expect(harness.conflict.ensureAllProcessed()).resolves.toBe(true);
|
||||||
|
|
||||||
|
expect(resolveByUserInteraction).not.toHaveBeenCalled();
|
||||||
|
expect(harness.tryAutoMerge).toHaveBeenCalledTimes(2);
|
||||||
|
} finally {
|
||||||
|
finishInspection?.(manualConflict);
|
||||||
|
await harness.conflict.ensureAllProcessed();
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it("limits concurrent conflict checks and reports queued and active work", async () => {
|
it("limits concurrent conflict checks and reports queued and active work", async () => {
|
||||||
const paths = Array.from({ length: 11 }, (_, index) => `concurrent-${index}.md` as FilePathWithPrefix);
|
const paths = Array.from({ length: 11 }, (_, index) => `concurrent-${index}.md` as FilePathWithPrefix);
|
||||||
const harness = createHarness();
|
const harness = createHarness();
|
||||||
@@ -929,8 +1009,10 @@ describe("conflict resolution serviceFeature", () => {
|
|||||||
it("resolves an identical pair, emits cancellation, and rechecks after merging", async () => {
|
it("resolves an identical pair, emits cancellation, and rechecks after merging", async () => {
|
||||||
const path = "independently-created.md" as FilePathWithPrefix;
|
const path = "independently-created.md" as FilePathWithPrefix;
|
||||||
const harness = createHarness({ settings: { syncAfterMerge: false } });
|
const harness = createHarness({ settings: { syncAfterMerge: false } });
|
||||||
const cancelled: FilePathWithPrefix[] = [];
|
const cancelled: Array<{ filename: FilePathWithPrefix; checks: number }> = [];
|
||||||
harness.context.events.onEvent(EVENT_CONFLICT_CANCELLED, (filename) => cancelled.push(filename));
|
harness.context.events.onEvent(EVENT_CONFLICT_CANCELLED, (filename) => {
|
||||||
|
cancelled.push({ filename, checks: harness.tryAutoMerge.mock.calls.length });
|
||||||
|
});
|
||||||
const leftLeaf = leaf("1-left", "Same content\n", 1000);
|
const leftLeaf = leaf("1-left", "Same content\n", 1000);
|
||||||
const rightLeaf = leaf("1-right", "Same content\n", 2000);
|
const rightLeaf = leaf("1-right", "Same content\n", 2000);
|
||||||
harness.tryAutoMerge
|
harness.tryAutoMerge
|
||||||
@@ -947,7 +1029,15 @@ describe("conflict resolution serviceFeature", () => {
|
|||||||
|
|
||||||
expect(harness.fileHandler.deleteRevisionFromDB).toHaveBeenCalledWith(path, "1-left");
|
expect(harness.fileHandler.deleteRevisionFromDB).toHaveBeenCalledWith(path, "1-left");
|
||||||
expect(harness.fileHandler.dbToStorage).toHaveBeenCalledWith(path, path, true);
|
expect(harness.fileHandler.dbToStorage).toHaveBeenCalledWith(path, path, true);
|
||||||
expect(cancelled).toEqual([path, path]);
|
// A queued recheck may also cancel its finishing resolver. Keep both
|
||||||
|
// merge and final-inspection notifications covered without fixing the count.
|
||||||
|
expect(cancelled).toEqual(
|
||||||
|
expect.arrayContaining([
|
||||||
|
{ filename: path, checks: 1 },
|
||||||
|
{ filename: path, checks: 2 },
|
||||||
|
])
|
||||||
|
);
|
||||||
|
expect(cancelled.every(({ filename }) => filename === path)).toBe(true);
|
||||||
expect(harness.tryAutoMerge).toHaveBeenCalledTimes(2);
|
expect(harness.tryAutoMerge).toHaveBeenCalledTimes(2);
|
||||||
expect(AUTO_MERGED).toBeDefined();
|
expect(AUTO_MERGED).toBeDefined();
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -34,6 +34,7 @@ export const useConflictResolutionFeature = createServiceFeature((host: Conflict
|
|||||||
const checking = createConflictCheckingHandlers({
|
const checking = createConflictCheckingHandlers({
|
||||||
conflict: services.conflict,
|
conflict: services.conflict,
|
||||||
conflictProcessQueueCount: services.conflict.conflictProcessQueueCount,
|
conflictProcessQueueCount: services.conflict.conflictProcessQueueCount,
|
||||||
|
cancelActiveResolution: operations.cancelActiveResolution,
|
||||||
currentSettings: () => services.setting.currentSettings(),
|
currentSettings: () => services.setting.currentSettings(),
|
||||||
vault: services.vault,
|
vault: services.vault,
|
||||||
log,
|
log,
|
||||||
|
|||||||
@@ -54,6 +54,7 @@ export interface ConflictResolutionOperationsDependencies {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export interface ConflictResolutionOperations {
|
export interface ConflictResolutionOperations {
|
||||||
|
readonly cancelActiveResolution: (filename: FilePathWithPrefix) => void;
|
||||||
readonly resolveByDeletingRevision: (
|
readonly resolveByDeletingRevision: (
|
||||||
path: FilePathWithPrefix,
|
path: FilePathWithPrefix,
|
||||||
deleteRevision: string,
|
deleteRevision: string,
|
||||||
@@ -72,6 +73,13 @@ export function createConflictResolutionOperations(
|
|||||||
const latestResolveRequestByFilename = new Map<FilePathWithPrefix, number>();
|
const latestResolveRequestByFilename = new Map<FilePathWithPrefix, number>();
|
||||||
let nextResolveRequestId = 0;
|
let nextResolveRequestId = 0;
|
||||||
|
|
||||||
|
const cancelActiveResolution = (filename: FilePathWithPrefix): void => {
|
||||||
|
latestResolveRequestByFilename.delete(filename);
|
||||||
|
if (isLockAcquired(`conflict-resolve:${filename}`)) {
|
||||||
|
dependencies.events.emitEvent(EVENT_CONFLICT_CANCELLED, filename);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
const resolveByDeletingRevision = async (
|
const resolveByDeletingRevision = async (
|
||||||
path: FilePathWithPrefix,
|
path: FilePathWithPrefix,
|
||||||
deleteRevision: string,
|
deleteRevision: string,
|
||||||
@@ -299,6 +307,7 @@ export function createConflictResolutionOperations(
|
|||||||
};
|
};
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
cancelActiveResolution,
|
||||||
resolveByDeletingRevision,
|
resolveByDeletingRevision,
|
||||||
checkConflictAndPerformAutoMerge,
|
checkConflictAndPerformAutoMerge,
|
||||||
resolve,
|
resolve,
|
||||||
|
|||||||
Reference in new issue
Block a user