This commit is contained in:
Andras Schmelczer 2025-08-23 19:25:52 +01:00
parent fc40d3eccf
commit 491a601ad2
No known key found for this signature in database
GPG key ID: FC8F2C3D3D1A718C
9 changed files with 101 additions and 77 deletions

View file

@ -30,32 +30,5 @@ jobs:
sqlx database create --database-url sqlite://db.sqlite3
sqlx migrate run --source src/app_state/database/migrations --database-url sqlite://db.sqlite3
- name: Lint sync-server
run: |
cd sync-server
cargo clippy --all-targets --all-features
cargo fmt --all -- --check
cargo machete
- name: Test sync-server
run: |
cd sync-server
cargo test --verbose
- name: Lint frontend
run: |
cd frontend
npm ci
npm run build
npm run lint
if [[ $(git status --porcelain) ]]; then
git status --porcelain
echo "Failing CI because the working directory is not clean after linting"
exit 1
fi
- name: Test frontend
run: |
cd frontend
npm ci
npm run test
- name: Lint & test
run: scripts/check.sh

View file

@ -15,7 +15,8 @@ import { MarkdownView } from "obsidian";
import { StateEffect } from "@codemirror/state";
import { getRandomColor } from "src/utils/get-random-color";
import { reconcileWithHistory, SpanWithHistory } from "reconcile-text";
import type { SpanWithHistory } from "reconcile-text";
import { reconcileWithHistory } from "reconcile-text";
function findWhereToMoveCursor(
cursor: number,
@ -39,8 +40,6 @@ function findWhereToMoveCursor(
const forceUpdate = StateEffect.define();
export class RemoteCursorsPluginValue implements PluginValue {
public decorations: DecorationSet = RangeSet.of([]);
private static cursors: {
name: string;
path: string;
@ -49,6 +48,8 @@ export class RemoteCursorsPluginValue implements PluginValue {
isOutdated: boolean;
}[] = [];
public decorations: DecorationSet = RangeSet.of([]);
public static setCursors(
clients: MaybeOutdatedClientCursors[],
app: App
@ -101,7 +102,7 @@ export class RemoteCursorsPluginValue implements PluginValue {
const original = update.startState.doc.toString();
const edited = update.state.doc.toString();
let updatedPositions: number[] = [];
const updatedPositions: number[] = [];
const reconciled = reconcileWithHistory(
original,
{

View file

@ -31,14 +31,17 @@ export class SafeFileSystemOperations implements FileSystemOperations {
this.logger.debug(`Reading file '${path}'`);
return this.safeOperation(
path,
async () => this.locks.withLock(path, () => this.fs.read(path)),
async () =>
this.locks.withLock(path, async () => this.fs.read(path)),
"read"
);
}
public async write(path: RelativePath, content: Uint8Array): Promise<void> {
this.logger.debug(`Writing to file '${path}'`);
return this.locks.withLock(path, () => this.fs.write(path, content));
return this.locks.withLock(path, async () =>
this.fs.write(path, content)
);
}
public async atomicUpdateText(
@ -49,7 +52,7 @@ export class SafeFileSystemOperations implements FileSystemOperations {
return this.safeOperation(
path,
async () =>
this.locks.withLock(path, () =>
this.locks.withLock(path, async () =>
this.fs.atomicUpdateText(path, updater)
),
"atomicUpdateText"
@ -61,19 +64,23 @@ export class SafeFileSystemOperations implements FileSystemOperations {
return this.safeOperation(
path,
async () =>
this.locks.withLock(path, () => this.fs.getFileSize(path)),
this.locks.withLock(path, async () =>
this.fs.getFileSize(path)
),
"getFileSize"
);
}
public async exists(path: RelativePath): Promise<boolean> {
this.logger.debug(`Checking if file '${path}' exists`);
return this.locks.withLock(path, () => this.fs.exists(path));
return this.locks.withLock(path, async () => this.fs.exists(path));
}
public async createDirectory(path: RelativePath): Promise<void> {
this.logger.debug(`Creating directory '${path}'`);
return this.locks.withLock(path, () => this.fs.createDirectory(path));
return this.locks.withLock(path, async () =>
this.fs.createDirectory(path)
);
}
public async delete(path: RelativePath): Promise<void> {
@ -89,7 +96,7 @@ export class SafeFileSystemOperations implements FileSystemOperations {
return this.safeOperation(
oldPath,
async () =>
this.locks.withLock([oldPath, newPath], () =>
this.locks.withLock([oldPath, newPath], async () =>
this.fs.rename(oldPath, newPath)
),
"rename"

View file

@ -61,7 +61,7 @@ export class CursorTracker {
}
);
this.fileChangeNotifier.addFileChangeListener(async (relativePath) => {
this.fileChangeNotifier.addFileChangeListener(async (relativePath) =>
this.updateLock.withLock(async () => {
for (const clientCursor of this.knownRemoteCursors) {
if (
@ -74,8 +74,8 @@ export class CursorTracker {
await this.getDocumentsUpToDateness(clientCursor);
}
}
});
});
})
);
}
/// Update the local cursors for the given documents.

View file

@ -15,9 +15,11 @@ export function createPromise<T = unknown>(): [
let reject: undefined | ((error: unknown) => unknown) = undefined;
const creationPromise = new Promise<T>(
(resolve_, reject_) => (
(resolve = resolve_ as ResolveFunction<T>), (reject = reject_)
)
(resolve_, reject_) =>
(
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
(resolve = resolve_ as ResolveFunction<T>), (reject = reject_)
)
);
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion

View file

@ -29,7 +29,7 @@ describe("withLock", () => {
let executionCount = 0;
const result = await locks.withLock(testPath, async () => {
executionCount++;
await new Promise(resolve => setTimeout(resolve, 10));
await new Promise((resolve) => setTimeout(resolve, 10));
return "async-success";
});
@ -54,14 +54,14 @@ describe("withLock", () => {
// Start two concurrent operations with keys in different orders
const promise1 = locks.withLock([testPath2, testPath], async () => {
executionOrder.push("operation1-start");
await new Promise(resolve => setTimeout(resolve, 50));
await new Promise((resolve) => setTimeout(resolve, 50));
executionOrder.push("operation1-end");
return "result1";
});
const promise2 = locks.withLock([testPath, testPath2], async () => {
executionOrder.push("operation2-start");
await new Promise(resolve => setTimeout(resolve, 50));
await new Promise((resolve) => setTimeout(resolve, 50));
executionOrder.push("operation2-end");
return "result2";
});
@ -84,14 +84,14 @@ describe("withLock", () => {
const promise1 = locks.withLock(testPath, async () => {
executionOrder.push("operation1-start");
await new Promise(resolve => setTimeout(resolve, 50));
await new Promise((resolve) => setTimeout(resolve, 50));
executionOrder.push("operation1-end");
return "result1";
});
const promise2 = locks.withLock(testPath, async () => {
executionOrder.push("operation2-start");
await new Promise(resolve => setTimeout(resolve, 30));
await new Promise((resolve) => setTimeout(resolve, 30));
executionOrder.push("operation2-end");
return "result2";
});
@ -113,14 +113,14 @@ describe("withLock", () => {
const promise1 = locks.withLock(testPath, async () => {
executionOrder.push("operation1-start");
await new Promise(resolve => setTimeout(resolve, 50));
await new Promise((resolve) => setTimeout(resolve, 50));
executionOrder.push("operation1-end");
return "result1";
});
const promise2 = locks.withLock(testPath2, async () => {
executionOrder.push("operation2-start");
await new Promise(resolve => setTimeout(resolve, 30));
await new Promise((resolve) => setTimeout(resolve, 30));
executionOrder.push("operation2-end");
return "result2";
});
@ -136,26 +136,36 @@ describe("withLock", () => {
test("should release locks even if function throws", async () => {
const error = new Error("test error");
await expect(locks.withLock(testPath, () => {
throw error;
})).rejects.toThrow("test error");
await expect(
locks.withLock(testPath, () => {
throw error;
})
).rejects.toThrow("test error");
// Lock should be released, allowing another operation
const result = await locks.withLock(testPath, () => "success-after-error");
const result = await locks.withLock(
testPath,
() => "success-after-error"
);
expect(result).toBe("success-after-error");
});
test("should release locks even if async function throws", async () => {
const error = new Error("async test error");
await expect(locks.withLock(testPath, async () => {
await new Promise(resolve => setTimeout(resolve, 10));
throw error;
})).rejects.toThrow("async test error");
await expect(
locks.withLock(testPath, async () => {
await new Promise((resolve) => setTimeout(resolve, 10));
throw error;
})
).rejects.toThrow("async test error");
// Lock should be released, allowing another operation
const result = await locks.withLock(testPath, () => "success-after-async-error");
const result = await locks.withLock(
testPath,
() => "success-after-async-error"
);
expect(result).toBe("success-after-async-error");
});
@ -170,30 +180,34 @@ describe("withLock", () => {
// Start first operation that holds the lock
const firstPromise = locks.withLock(testPath, async () => {
executionOrder.push("first-start");
await new Promise(resolve => setTimeout(resolve, 100));
await new Promise((resolve) => setTimeout(resolve, 100));
executionOrder.push("first-end");
return "first";
});
// Small delay to ensure first operation starts
await new Promise(resolve => setTimeout(resolve, 10));
await new Promise((resolve) => setTimeout(resolve, 10));
// Queue second and third operations
const secondPromise = locks.withLock(testPath, async () => {
executionOrder.push("second-start");
await new Promise(resolve => setTimeout(resolve, 30));
await new Promise((resolve) => setTimeout(resolve, 30));
executionOrder.push("second-end");
return "second";
});
const thirdPromise = locks.withLock(testPath, async () => {
executionOrder.push("third-start");
await new Promise(resolve => setTimeout(resolve, 20));
await new Promise((resolve) => setTimeout(resolve, 20));
executionOrder.push("third-end");
return "third";
});
const [first, second, third] = await Promise.all([firstPromise, secondPromise, thirdPromise]);
const [first, second, third] = await Promise.all([
firstPromise,
secondPromise,
thirdPromise
]);
expect(first).toBe("first");
expect(second).toBe("second");
@ -207,4 +221,4 @@ describe("withLock", () => {
"third-end"
]);
});
});
});

View file

@ -17,16 +17,16 @@ export class Locks<T> {
/**
* Executes a function while holding exclusive locks on one or more keys.
*
*
* This method ensures that the provided function runs with exclusive access to the
* specified key(s). Multiple keys are sorted to prevent deadlocks when different
* operations request the same keys in different orders.
*
*
* @template R The return type of the function to execute
* @param keyOrKeys A single key or array of keys to lock during function execution
* @param fn The function to execute while holding the lock(s). Can be sync or async.
* @returns A Promise that resolves to the return value of the executed function
*
*
* @example
* ```typescript
* // Lock a single key
@ -34,14 +34,14 @@ export class Locks<T> {
* // Critical section - only one operation can access 'file1' at a time
* return processFile('file1');
* });
*
*
* // Lock multiple keys (prevents deadlocks through consistent ordering)
* await locks.withLock(['file1', 'file2'], async () => {
* // Critical section - exclusive access to both files
* await moveFile('file1', 'file2');
* });
* ```
*
*
* @throws Any error thrown by the provided function will be propagated after locks are released
*/
public async withLock<R>(
@ -49,7 +49,7 @@ export class Locks<T> {
fn: () => R | Promise<R>
): Promise<R> {
const keys = Array.isArray(keyOrKeys) ? keyOrKeys : [keyOrKeys];
keys.sort(); // Ensure consistent order to prevent deadlocks
keys.sort((a, b) => String(a).localeCompare(String(b))); // Ensure consistent order to prevent deadlocks
await Promise.all(keys.map(async (key) => this.waitForLock(key)));

View file

@ -37,7 +37,7 @@ export class MockClient implements FileSystemOperations {
fs: this,
persistence: {
load: async () => this.data,
save: async (data) => (this.data = data)
save: async (data) => void (this.data = data)
},
fetch: fetchImplementation,
webSocket: webSocketImplementation

27
scripts/check.sh Executable file
View file

@ -0,0 +1,27 @@
#!/usr/bin/env bash
set -e
echo "Running checks in sync-server"
cd sync-server
cargo clippy --all-targets --all-features
cargo fmt --all -- --check
cargo machete
cargo test --verbose
echo "Running checks in frontend"
cd ../frontend
npm ci
npm run build
npm run lint
if [[ $(git status --porcelain) ]]; then
git status --porcelain
echo "Failing CI because the working directory is not clean after linting"
exit 1
fi
npm run test
echo "Finished"
cd ..