Improve network usage for small text changes #166

Merged
schmelczer merged 9 commits from asch/compact-reconcile into main 2025-11-16 22:10:22 +00:00
19 changed files with 446 additions and 55 deletions
Showing only changes of commit 8a1f27589e - Show all commits
Andras Schmelczer 2025-11-16 20:19:22 +00:00

View file

@ -33,12 +33,7 @@ export default [
"@typescript-eslint/class-methods-use-this": "off", "@typescript-eslint/class-methods-use-this": "off",
"@typescript-eslint/consistent-return": "off", "@typescript-eslint/consistent-return": "off",
"@typescript-eslint/no-unsafe-argument": "off", "@typescript-eslint/no-unsafe-argument": "off",
"@typescript-eslint/max-params": [ "@typescript-eslint/max-params": "off",
"error",
{
max: 6
}
],
"@typescript-eslint/no-magic-numbers": "off", "@typescript-eslint/no-magic-numbers": "off",
"@typescript-eslint/prefer-readonly-parameter-types": "off", "@typescript-eslint/prefer-readonly-parameter-types": "off",
"@typescript-eslint/naming-convention": "off", "@typescript-eslint/naming-convention": "off",

View file

@ -1583,7 +1583,9 @@
} }
}, },
"node_modules/brace-expansion": { "node_modules/brace-expansion": {
"version": "1.1.11", "version": "1.1.12",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.12.tgz",
"integrity": "sha512-9T9UjW3r0UW5c1Q7GTwllptXwhvYmEzFhzMfZ9H7FQWt+uZePjZPjBP/W1ZEyZ1twGWom5/56TF4lPcqjnDHcg==",
"dev": true, "dev": true,
"license": "MIT", "license": "MIT",
"dependencies": { "dependencies": {
@ -2742,7 +2744,9 @@
} }
}, },
"node_modules/js-yaml": { "node_modules/js-yaml": {
"version": "4.1.0", "version": "4.1.1",
"resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.1.tgz",
"integrity": "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA==",
"dev": true, "dev": true,
"license": "MIT", "license": "MIT",
"dependencies": { "dependencies": {
@ -3487,6 +3491,7 @@
"version": "0.5.0", "version": "0.5.0",
"resolved": "https://registry.npmjs.org/reconcile-text/-/reconcile-text-0.5.0.tgz", "resolved": "https://registry.npmjs.org/reconcile-text/-/reconcile-text-0.5.0.tgz",
"integrity": "sha512-zki3lqw9Oxdhm9ZvDN17VyYoL1Isc8BEL07ILVDE2yGfNEI7thrkczoNCUr+hkFU2rzZtfxECTG0b7p61AJ6wg==", "integrity": "sha512-zki3lqw9Oxdhm9ZvDN17VyYoL1Isc8BEL07ILVDE2yGfNEI7thrkczoNCUr+hkFU2rzZtfxECTG0b7p61AJ6wg==",
"dev": true,
"license": "MIT" "license": "MIT"
}, },
"node_modules/regex-parser": { "node_modules/regex-parser": {
@ -4687,7 +4692,7 @@
"byte-base64": "^1.1.0", "byte-base64": "^1.1.0",
"minimatch": "^10.0.1", "minimatch": "^10.0.1",
"p-queue": "^8.1.0", "p-queue": "^8.1.0",
"reconcile-text": "^0.5.0", "reconcile-text": "^0.7.1",
"uuid": "^13.0.0" "uuid": "^13.0.0"
}, },
"devDependencies": { "devDependencies": {
@ -4703,7 +4708,9 @@
} }
}, },
"sync-client/node_modules/brace-expansion": { "sync-client/node_modules/brace-expansion": {
"version": "2.0.1", "version": "2.0.2",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.2.tgz",
"integrity": "sha512-Jt0vHyM+jmUBqojB7E1NIYadt0vI0Qxjxd2TErW94wDz+E2LAm5vKMXXwg6ZZBTHPuUlDgQHKXvjGBdfcF1ZDQ==",
"license": "MIT", "license": "MIT",
"dependencies": { "dependencies": {
"balanced-match": "^1.0.0" "balanced-match": "^1.0.0"
@ -4722,6 +4729,12 @@
"url": "https://github.com/sponsors/isaacs" "url": "https://github.com/sponsors/isaacs"
} }
}, },
"sync-client/node_modules/reconcile-text": {
"version": "0.7.1",
"resolved": "https://registry.npmjs.org/reconcile-text/-/reconcile-text-0.7.1.tgz",
"integrity": "sha512-khedcYvAKs7ELKh5Z8mz2vyomMY5TqznV1dB+k/7qUAX9cheMNN5/EPJVQYZepOMunYbnQitvhFJX3kD4IMcNw==",
"license": "MIT"
},
"test-client": { "test-client": {
"version": "0.9.2", "version": "0.9.2",
"bin": { "bin": {

View file

@ -16,7 +16,7 @@
"byte-base64": "^1.1.0", "byte-base64": "^1.1.0",
"minimatch": "^10.0.1", "minimatch": "^10.0.1",
"p-queue": "^8.1.0", "p-queue": "^8.1.0",
"reconcile-text": "^0.5.0", "reconcile-text": "^0.7.1",
"uuid": "^13.0.0" "uuid": "^13.0.0"
schmelczer commented 2025-11-16 20:43:05 +00:00 (Migrated from github.com)
Pick up https://github.com/schmelczer/reconcile/pull/21 & https://github.com/schmelczer/reconcile/pull/24
}, },
"devDependencies": { "devDependencies": {

View file

@ -3,8 +3,9 @@ import type { FileSystemOperations } from "./filesystem-operations";
import type { Database, RelativePath } from "../persistence/database"; import type { Database, RelativePath } from "../persistence/database";
import { SafeFileSystemOperations } from "./safe-filesystem-operations"; import { SafeFileSystemOperations } from "./safe-filesystem-operations";
import type { TextWithCursors } from "reconcile-text"; import type { TextWithCursors } from "reconcile-text";
import { isBinary, reconcile } from "reconcile-text"; import { reconcile } from "reconcile-text";
import { isFileTypeMergable } from "../utils/is-file-type-mergable"; import { isFileTypeMergable } from "../utils/is-file-type-mergable";
import { isBinary } from "../utils/is-binary";
export class FileOperations { export class FileOperations {
private static readonly PARENTHESES_REGEX = / \((\d+)\)$/; private static readonly PARENTHESES_REGEX = / \((\d+)\)$/;

View file

@ -16,6 +16,7 @@ import type { DocumentVersion } from "./types/DocumentVersion";
import type { FetchLatestDocumentsResponse } from "./types/FetchLatestDocumentsResponse"; import type { FetchLatestDocumentsResponse } from "./types/FetchLatestDocumentsResponse";
import type { PingResponse } from "./types/PingResponse"; import type { PingResponse } from "./types/PingResponse";
import type { DeleteDocumentVersion } from "./types/DeleteDocumentVersion"; import type { DeleteDocumentVersion } from "./types/DeleteDocumentVersion";
import type { UpdateTextDocumentVersion } from "./types/UpdateTextDocumentVersion";
export interface CheckConnectionResult { export interface CheckConnectionResult {
isSuccessful: boolean; isSuccessful: boolean;
@ -102,7 +103,64 @@ export class SyncService {
}); });
} }
public async put({ public async putText({
parentVersionId,
documentId,
relativePath,
content
}: {
parentVersionId: VaultUpdateId;
documentId: DocumentId;
relativePath: RelativePath;
content: (number | string | bigint)[];
}): Promise<DocumentUpdateResponse> {
return this.withRetries(async () => {
this.logger.debug(
`Updating text document ${documentId} with parent version ${parentVersionId} and relative path ${relativePath}`
);
const request: UpdateTextDocumentVersion = {
parentVersionId,
relativePath,
content: content.map((c) => {
if (typeof c === "bigint") {
return Number(c);
}
return c;
})
};
const response = await this.client(
this.getUrl(`/documents/${documentId}/text`),
{
method: "PUT",
body: JSON.stringify(request),
headers: this.getDefaultHeaders({ type: "json" })
}
);
const result: SerializedError | DocumentUpdateResponse =
(await response.json()) as // eslint-disable-line @typescript-eslint/no-unsafe-type-assertion
| SerializedError
| DocumentUpdateResponse;
if ("errorType" in result) {
throw new Error(
`Failed to update document: ${SyncService.formatError(result)}`
);
}
this.logger.debug(
`Updated document ${JSON.stringify(result)} with id ${
result.documentId
}}`
);
return result;
});
}
public async putBinary({
parentVersionId, parentVersionId,
documentId, documentId,
relativePath, relativePath,
@ -115,7 +173,7 @@ export class SyncService {
}): Promise<DocumentUpdateResponse> { }): Promise<DocumentUpdateResponse> {
return this.withRetries(async () => { return this.withRetries(async () => {
this.logger.debug( this.logger.debug(
`Updating document ${documentId} with parent version ${parentVersionId} and relative path ${relativePath}` `Updating binary document ${documentId} with parent version ${parentVersionId} and relative path ${relativePath}`
); );
const formData = new FormData(); const formData = new FormData();
formData.append("parent_version_id", parentVersionId.toString()); formData.append("parent_version_id", parentVersionId.toString());
@ -126,7 +184,7 @@ export class SyncService {
); );
const response = await this.client( const response = await this.client(
this.getUrl(`/documents/${documentId}`), this.getUrl(`/documents/${documentId}/binary`),
{ {
method: "PUT", method: "PUT",
body: formData, body: formData,
@ -171,10 +229,7 @@ export class SyncService {
{ {
method: "DELETE", method: "DELETE",
body: JSON.stringify(request), body: JSON.stringify(request),
headers: { headers: this.getDefaultHeaders({ type: "json" })
"Content-Type": "application/json",
...this.getDefaultHeaders()
}
} }
); );
@ -297,11 +352,21 @@ export class SyncService {
return `${safeRemoteUri}/vaults/${vaultName}${path}`; return `${safeRemoteUri}/vaults/${vaultName}${path}`;
} }
private getDefaultHeaders(): Record<string, string> { private getDefaultHeaders(
return { { type }: { type?: "json" | "form" } = { type: undefined }
): Record<string, string> {
const headers: Record<string, string> = {
"device-id": this.deviceId, "device-id": this.deviceId,
authorization: `Bearer ${this.settings.getSettings().token}` authorization: `Bearer ${this.settings.getSettings().token}`
}; };
if (type === "json") {
headers["Content-Type"] = "application/json";
} else if (type === "form") {
headers["Content-Type"] = "multipart/form-data";
}
return headers;
} }
private async withRetries<T>(fn: () => Promise<T>): Promise<T> { private async withRetries<T>(fn: () => Promise<T>): Promise<T> {

View file

@ -0,0 +1,7 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export interface UpdateBinaryDocumentVersion {
parent_version_id: bigint;
relative_path: string;
content: number[];
}

View file

@ -0,0 +1,7 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export interface UpdateTextDocumentVersion {
parentVersionId: number;
relativePath: string;
content: (number | string)[];
}

View file

@ -21,13 +21,13 @@ import { CursorTracker } from "./sync-operations/cursor-tracker";
import type { CursorSpan } from "./services/types/CursorSpan"; import type { CursorSpan } from "./services/types/CursorSpan";
import type { MaybeOutdatedClientCursors } from "./types/maybe-outdated-client-cursors"; import type { MaybeOutdatedClientCursors } from "./types/maybe-outdated-client-cursors";
import { FileChangeNotifier } from "./sync-operations/file-change-notifier"; import { FileChangeNotifier } from "./sync-operations/file-change-notifier";
import { FixedSizeDocumentCache } from "./utils/fix-sized-cache";
export class SyncClient { export class SyncClient {
private static readonly MINIMUM_SAVE_INTERVAL_MS = 1000; private static readonly MINIMUM_SAVE_INTERVAL_MS = 1000;
private hasStartedOfflineSync = false; private hasStartedOfflineSync = false;
private hasFinishedOfflineSync = false; private hasFinishedOfflineSync = false;
// eslint-disable-next-line @typescript-eslint/max-params
private constructor( private constructor(
private readonly history: SyncHistory, private readonly history: SyncHistory,
private readonly settings: Settings, private readonly settings: Settings,
@ -135,13 +135,15 @@ export class SyncClient {
nativeLineEndings nativeLineEndings
); );
const contentCache = new FixedSizeDocumentCache(1024 * 1024 * 2); // 2 MB cache
const unrestrictedSyncer = new UnrestrictedSyncer( const unrestrictedSyncer = new UnrestrictedSyncer(
logger, logger,
database, database,
settings, settings,
syncService, syncService,
fileOperations, fileOperations,
history history,
contentCache
); );
const syncer = new Syncer( const syncer = new Syncer(

View file

@ -4,6 +4,7 @@ import type {
RelativePath RelativePath
} from "../persistence/database"; } from "../persistence/database";
import { diff } from "reconcile-text";
import type { SyncService } from "../services/sync-service"; import type { SyncService } from "../services/sync-service";
import type { Logger } from "../tracing/logger"; import type { Logger } from "../tracing/logger";
import type { import type {
@ -27,6 +28,9 @@ import { globsToRegexes } from "../utils/globs-to-regexes";
import type { DocumentVersion } from "../services/types/DocumentVersion"; import type { DocumentVersion } from "../services/types/DocumentVersion";
import type { DocumentUpdateResponse } from "../services/types/DocumentUpdateResponse"; import type { DocumentUpdateResponse } from "../services/types/DocumentUpdateResponse";
import type { DocumentVersionWithoutContent } from "../services/types/DocumentVersionWithoutContent"; import type { DocumentVersionWithoutContent } from "../services/types/DocumentVersionWithoutContent";
import type { FixedSizeDocumentCache } from "../utils/fix-sized-cache";
import { isFileTypeMergable } from "../utils/is-file-type-mergable";
import { isBinary } from "../utils/is-binary";
export class UnrestrictedSyncer { export class UnrestrictedSyncer {
private ignorePatterns: RegExp[]; private ignorePatterns: RegExp[];
@ -37,7 +41,8 @@ export class UnrestrictedSyncer {
private readonly settings: Settings, private readonly settings: Settings,
private readonly syncService: SyncService, private readonly syncService: SyncService,
private readonly operations: FileOperations, private readonly operations: FileOperations,
private readonly history: SyncHistory private readonly history: SyncHistory,
private readonly contentCache: FixedSizeDocumentCache
) { ) {
this.ignorePatterns = globsToRegexes( this.ignorePatterns = globsToRegexes(
this.settings.getSettings().ignorePatterns, this.settings.getSettings().ignorePatterns,
@ -87,8 +92,12 @@ export class UnrestrictedSyncer {
}, },
document document
); );
this.database.addSeenUpdateId(response.vaultUpdateId); this.database.addSeenUpdateId(response.vaultUpdateId);
this.updateCache(
response.vaultUpdateId,
contentBytes,
response.relativePath
);
this.history.addHistoryEntry({ this.history.addHistoryEntry({
status: SyncStatus.SUCCESS, status: SyncStatus.SUCCESS,
@ -178,9 +187,29 @@ export class UnrestrictedSyncer {
undefined; undefined;
if (areThereLocalChanges) { if (areThereLocalChanges) {
response = await this.syncService.put({ const isText =
!isBinary(contentBytes) &&
isFileTypeMergable(document.relativePath);
const cachedVersion = this.contentCache.get(
document.metadata.parentVersionId
);
response =
isText && cachedVersion !== undefined
? await this.syncService.putText({
documentId: document.documentId, documentId: document.documentId,
parentVersionId: document.metadata.parentVersionId, parentVersionId:
document.metadata.parentVersionId,
relativePath: document.relativePath,
content: diff(
new TextDecoder().decode(cachedVersion),
new TextDecoder().decode(contentBytes)
)
})
: await this.syncService.putBinary({
documentId: document.documentId,
parentVersionId:
document.metadata.parentVersionId,
relativePath: document.relativePath, relativePath: document.relativePath,
contentBytes contentBytes
}); });
@ -274,12 +303,16 @@ export class UnrestrictedSyncer {
}, },
document document
); );
await this.operations.write( await this.operations.write(
actualPath, actualPath,
contentBytes, contentBytes,
responseBytes responseBytes
); );
this.updateCache(
response.vaultUpdateId,
contentBytes,
actualPath
);
if (!force) { if (!force) {
this.history.addHistoryEntry({ this.history.addHistoryEntry({
@ -297,6 +330,11 @@ export class UnrestrictedSyncer {
}, },
document document
); );
this.updateCache(
response.vaultUpdateId,
contentBytes,
actualPath
);
} }
this.database.addSeenUpdateId(response.vaultUpdateId); this.database.addSeenUpdateId(response.vaultUpdateId);
@ -423,6 +461,11 @@ export class UnrestrictedSyncer {
remoteVersion.relativePath, remoteVersion.relativePath,
contentBytes contentBytes
); );
this.updateCache(
remoteVersion.vaultUpdateId,
contentBytes,
remoteVersion.relativePath
);
resolve(); resolve();
this.database.removeDocumentPromise(promise); this.database.removeDocumentPromise(promise);
@ -513,4 +556,14 @@ export class UnrestrictedSyncer {
}; };
} }
} }
private updateCache(
updateId: number,
contentBytes: Uint8Array,
filePath: RelativePath
): void {
if (isFileTypeMergable(filePath) && !isBinary(contentBytes)) {
this.contentCache.put(updateId, contentBytes);
}
}
} }

View file

@ -0,0 +1,62 @@
import { describe, it } from "node:test";
import assert from "node:assert";
import { FixedSizeDocumentCache } from "./fix-sized-cache";
describe("fixedSizeDocumentCache", () => {
it("happyPath", async () => {
const cache = new FixedSizeDocumentCache(4);
const doc1 = new Uint8Array([1, 2]);
const doc2 = new Uint8Array([3, 4]);
const doc3 = new Uint8Array([5, 6]);
cache.put(1, doc1);
assert.equal(cache.get(1), doc1);
cache.put(2, doc2);
assert.equal(cache.get(1), doc1);
assert.equal(cache.get(2), doc2);
cache.put(3, doc3);
assert.equal(cache.get(1), undefined);
assert.equal(cache.get(2), doc2);
assert.equal(cache.get(3), doc3);
});
it("updateExistingEntry", async () => {
const cache = new FixedSizeDocumentCache(4);
const doc1_v1 = new Uint8Array([1, 2]);
const doc1_v2 = new Uint8Array([3, 4]);
const doc2 = new Uint8Array([5, 6]);
cache.put(1, doc1_v1);
assert.equal(cache.get(1), doc1_v1);
cache.put(2, doc2);
assert.equal(cache.get(1), doc1_v1);
assert.equal(cache.get(2), doc2);
cache.put(1, doc1_v2); // Update doc1
assert.equal(cache.get(1), doc1_v2);
assert.equal(cache.get(2), doc2);
});
it("evictOldestEntry", async () => {
const cache = new FixedSizeDocumentCache(4);
const doc1 = new Uint8Array([1, 2]);
const doc2 = new Uint8Array([3, 4]);
const doc3 = new Uint8Array([5, 6]);
cache.put(1, doc1);
cache.put(2, doc2);
assert.equal(cache.get(2), doc2);
assert.equal(cache.get(1), doc1);
cache.put(3, doc3);
assert.equal(cache.get(1), doc1);
assert.equal(cache.get(2), undefined);
assert.equal(cache.get(3), doc3);
});
it("tooLargeEntry", async () => {
const cache = new FixedSizeDocumentCache(2);
const doc1 = new Uint8Array([1, 2, 3]);
cache.put(1, doc1);
assert.equal(cache.get(1), undefined);
});
});

View file

@ -0,0 +1,57 @@
// Implements an in-memory fixed-size cache for document contents,
copilot-pull-request-reviewer[bot] commented 2025-11-16 21:25:22 +00:00 (Migrated from github.com)

Filtering the entire usage order array on every get operation is O(n). Consider using a doubly-linked list or Map-based approach to track order in O(1) time. With frequent reads, this could become a significant performance bottleneck.

Filtering the entire usage order array on every `get` operation is O(n). Consider using a doubly-linked list or Map-based approach to track order in O(1) time. With frequent reads, this could become a significant performance bottleneck.
copilot-pull-request-reviewer[bot] commented 2025-11-16 21:25:22 +00:00 (Migrated from github.com)

Filtering the entire usage order array on every put operation when updating existing entries is O(n). This compounds the performance issue noted in the get method. Consider using a doubly-linked list or Map-based approach.

Filtering the entire usage order array on every `put` operation when updating existing entries is O(n). This compounds the performance issue noted in the `get` method. Consider using a doubly-linked list or Map-based approach.
import type { VaultUpdateId } from "../persistence/database";
// evicting the least recently used documents when the size limit is exceeded.
export class FixedSizeDocumentCache {
private readonly maxSizeInBytes: number;
private currentSizeInBytes: number;
private readonly cache: Map<VaultUpdateId, Uint8Array>;
private usageOrder: VaultUpdateId[];
public constructor(maxSizeInBytes: number) {
this.maxSizeInBytes = maxSizeInBytes;
this.currentSizeInBytes = 0;
this.cache = new Map();
this.usageOrder = [];
}
public get(updateId: VaultUpdateId): Uint8Array | undefined {
const entry = this.cache.get(updateId);
if (entry) {
this.usageOrder = this.usageOrder.filter((id) => id !== updateId);
this.usageOrder.push(updateId);
return entry;
}
return undefined;
}
public put(updateId: VaultUpdateId, content: Uint8Array): void {
if (content.byteLength > this.maxSizeInBytes) {
// Document is too large to fit in the cache
return;
}
// If the document is already in the cache, update it
const existingEntry = this.cache.get(updateId);
if (existingEntry != null) {
this.currentSizeInBytes -= existingEntry.byteLength;
this.cache.delete(updateId);
this.usageOrder = this.usageOrder.filter((id) => id !== updateId);
}
this.cache.set(updateId, content);
this.usageOrder.push(updateId);
this.currentSizeInBytes += content.byteLength;
// Evict least recently used documents if over size limit
while (
this.currentSizeInBytes > this.maxSizeInBytes &&
this.usageOrder.length > 0
) {
const lruUpdateId = this.usageOrder.shift()!; // eslint-disable-line @typescript-eslint/no-non-null-assertion
const lruEntry = this.cache.get(lruUpdateId)!; // eslint-disable-line @typescript-eslint/no-non-null-assertion
this.cache.delete(lruUpdateId);
this.currentSizeInBytes -= lruEntry.byteLength;
}
}
}

View file

@ -0,0 +1,16 @@
// Text is unlikely to contain null bytes, so we can use that to distinguish binary files.
export function isBinary(content: Uint8Array): boolean {
for (const byte of content) {
if (byte === 0) {
return true;
}
}
try {
new TextDecoder("utf-8", { fatal: true }).decode(content);
} catch {
return true;
}
return false;
}

View file

@ -1680,9 +1680,12 @@ dependencies = [
[[package]] [[package]]
name = "reconcile-text" name = "reconcile-text"
version = "0.5.0" version = "0.7.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c8d690c19b0bf6574cd3591d10f20df5aa52d2af95b8dcaacbc86893292ac8c5" checksum = "913440a3c2b90cd3ed3e967660f2bb624b71e8059b9fc86960a5f91bd1e2e353"
dependencies = [
"serde",
]
[[package]] [[package]]
name = "redox_syscall" name = "redox_syscall"

View file

@ -35,7 +35,7 @@ bimap = "0.6.3"
ts-rs = { version = "10.1", features = ["uuid-impl", "chrono-impl"] } ts-rs = { version = "10.1", features = ["uuid-impl", "chrono-impl"] }
serde_with = "3.15.1" serde_with = "3.15.1"
base64 = "0.22.1" base64 = "0.22.1"
reconcile-text = "0.5.0" reconcile-text = { version = "0.7.1", features = ["serde"] }
[profile.release] [profile.release]
codegen-units = 1 codegen-units = 1

View file

@ -117,8 +117,12 @@ fn get_authed_routes(app_state: AppState) -> Router<AppState> {
get(fetch_latest_document_version::fetch_latest_document_version), get(fetch_latest_document_version::fetch_latest_document_version),
) )
.route( .route(
"/vaults/:vault_id/documents/:document_id", "/vaults/:vault_id/documents/:document_id/binary",
put(update_document::update_document), put(update_document::update_binary),
)
.route(
"/vaults/:vault_id/documents/:document_id/text",
put(update_document::update_text),
) )
.route( .route(
"/vaults/:vault_id/documents/:document_id/versions/:version_id", "/vaults/:vault_id/documents/:document_id/versions/:version_id",

View file

@ -1,5 +1,6 @@
use axum::body::Bytes; use axum::body::Bytes;
use axum_typed_multipart::{FieldData, TryFromMultipart}; use axum_typed_multipart::{FieldData, TryFromMultipart};
use reconcile_text::NumberOrString;
use serde::{self, Deserialize}; use serde::{self, Deserialize};
use ts_rs::TS; use ts_rs::TS;
@ -22,7 +23,7 @@ pub struct CreateDocumentVersion {
#[derive(TS, Debug, TryFromMultipart)] #[derive(TS, Debug, TryFromMultipart)]
#[ts(export)] #[ts(export)]
pub struct UpdateDocumentVersion { pub struct UpdateBinaryDocumentVersion {
pub parent_version_id: VaultUpdateId, pub parent_version_id: VaultUpdateId,
pub relative_path: String, pub relative_path: String,
@ -31,6 +32,19 @@ pub struct UpdateDocumentVersion {
pub content: FieldData<Bytes>, pub content: FieldData<Bytes>,
} }
#[derive(TS, Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
#[ts(export)]
pub struct UpdateTextDocumentVersion {
#[ts(as = "i32")]
pub parent_version_id: VaultUpdateId,
pub relative_path: String,
#[ts(type = "Array<number | string>")]
pub content: Vec<NumberOrString>,
}
#[derive(TS, Debug, Deserialize)] #[derive(TS, Debug, Deserialize)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
#[ts(export)] #[ts(export)]

View file

@ -6,23 +6,25 @@ use axum::{
use axum_extra::TypedHeader; use axum_extra::TypedHeader;
use axum_typed_multipart::TypedMultipart; use axum_typed_multipart::TypedMultipart;
use log::info; use log::info;
use reconcile_text::{BuiltinTokenizer, is_binary, reconcile}; use reconcile_text::{BuiltinTokenizer, EditedText, reconcile};
use serde::Deserialize; use serde::Deserialize;
use super::{ use super::{
device_id_header::DeviceIdHeader, requests::UpdateDocumentVersion, device_id_header::DeviceIdHeader, requests::UpdateTextDocumentVersion,
responses::DocumentUpdateResponse, responses::DocumentUpdateResponse,
}; };
use crate::{ use crate::{
app_state::{ app_state::{
AppState, AppState,
database::models::{DocumentId, StoredDocumentVersion, VaultId}, database::models::{DocumentId, StoredDocumentVersion, VaultId, VaultUpdateId},
}, },
config::user_config::User, config::user_config::User,
errors::{SyncServerError, not_found_error, server_error}, errors::{SyncServerError, not_found_error, server_error},
server::requests::UpdateBinaryDocumentVersion,
utils::{ utils::{
dedup_paths::dedup_paths, is_file_type_mergable::is_file_type_mergable, dedup_paths::dedup_paths, is_binary::is_binary,
normalize::normalize, sanitize_path::sanitize_path, is_file_type_mergable::is_file_type_mergable, normalize::normalize,
sanitize_path::sanitize_path,
}, },
}; };
@ -30,13 +32,11 @@ use crate::{
pub struct UpdateDocumentPathParams { pub struct UpdateDocumentPathParams {
#[serde(deserialize_with = "normalize")] #[serde(deserialize_with = "normalize")]
vault_id: VaultId, vault_id: VaultId,
document_id: DocumentId, document_id: DocumentId,
} }
#[axum::debug_handler] #[axum::debug_handler]
#[allow(clippy::too_many_lines)] pub async fn update_binary(
pub async fn update_document(
Path(UpdateDocumentPathParams { Path(UpdateDocumentPathParams {
vault_id, vault_id,
document_id, document_id,
@ -44,25 +44,92 @@ pub async fn update_document(
Extension(user): Extension<User>, Extension(user): Extension<User>,
TypedHeader(device_id): TypedHeader<DeviceIdHeader>, TypedHeader(device_id): TypedHeader<DeviceIdHeader>,
State(state): State<AppState>, State(state): State<AppState>,
TypedMultipart(request): TypedMultipart<UpdateDocumentVersion>, TypedMultipart(request): TypedMultipart<UpdateBinaryDocumentVersion>,
) -> Result<Json<DocumentUpdateResponse>, SyncServerError> { ) -> Result<Json<DocumentUpdateResponse>, SyncServerError> {
// No need for a transaction as document versions are immutable let parent_document = get_parent_document(&state, &vault_id, request.parent_version_id).await?;
let parent_document = state let content = request.content.contents.to_vec();
update_document(
parent_document,
vault_id,
document_id,
user,
device_id,
state,
&request.relative_path,
content,
)
.await
}
#[axum::debug_handler]
#[allow(clippy::too_many_lines)]
pub async fn update_text(
Path(UpdateDocumentPathParams {
vault_id,
document_id,
}): Path<UpdateDocumentPathParams>,
Extension(user): Extension<User>,
TypedHeader(device_id): TypedHeader<DeviceIdHeader>,
State(state): State<AppState>,
Json(request): Json<UpdateTextDocumentVersion>,
) -> Result<Json<DocumentUpdateResponse>, SyncServerError> {
let parent_document = get_parent_document(&state, &vault_id, request.parent_version_id).await?;
let edited_text = EditedText::from_diff(
str::from_utf8(&parent_document.content)
.expect("parent must be valid UTF-8 because it's a text document"),
request.content,
&*BuiltinTokenizer::Word,
);
let content = edited_text.apply().text().into_bytes();
update_document(
parent_document,
vault_id,
document_id,
user,
device_id,
state,
&request.relative_path,
content,
)
.await
}
async fn get_parent_document(
state: &AppState,
vault_id: &VaultId,
parent_version_id: VaultUpdateId,
) -> Result<StoredDocumentVersion, SyncServerError> {
state
.database .database
.get_document_version(&vault_id, request.parent_version_id, None) .get_document_version(vault_id, parent_version_id, None)
.await .await
.map_err(server_error)? .map_err(server_error)?
.map_or_else( .map_or_else(
|| { || {
Err(not_found_error(anyhow!( Err(not_found_error(anyhow!(
"Parent version with id `{}` not found", "Parent version with id `{parent_version_id}` not found"
request.parent_version_id
))) )))
}, },
Ok, Ok,
)?; )
}
let sanitized_relative_path = sanitize_path(&request.relative_path); #[allow(clippy::too_many_lines, clippy::too_many_arguments)]
async fn update_document(
parent_document: StoredDocumentVersion,
vault_id: VaultId,
document_id: DocumentId,
user: User,
device_id: DeviceIdHeader,
state: AppState,
relative_path: &str,
content: Vec<u8>,
) -> Result<Json<DocumentUpdateResponse>, SyncServerError> {
let sanitized_relative_path = sanitize_path(relative_path);
let mut transaction = state let mut transaction = state
.database .database
@ -102,8 +169,6 @@ pub async fn update_document(
))); )));
} }
let content = request.content.contents.to_vec();
// Return the latest version if the content and path are the same as the latest // Return the latest version if the content and path are the same as the latest
// version // version
if content == latest_version.content && sanitized_relative_path == latest_version.relative_path if content == latest_version.content && sanitized_relative_path == latest_version.relative_path

View file

@ -1,4 +1,5 @@
pub mod dedup_paths; pub mod dedup_paths;
pub mod is_binary;
pub mod is_file_type_mergable; pub mod is_file_type_mergable;
pub mod normalize; pub mod normalize;
pub mod rotating_file_writer; pub mod rotating_file_writer;

View file

@ -0,0 +1,26 @@
/// Heuristically determine if the given data is a binary or a text file's
/// content.
///
/// Only text inputs can be reconciled using the crate's functions.
#[must_use]
pub fn is_binary(data: &[u8]) -> bool {
if data.contains(&0) {
// Even though the NUL character is valid in UTF-8, it's highly suspicious in
// human-readable text.
return true;
}
copilot-pull-request-reviewer[bot] commented 2025-11-16 21:25:22 +00:00 (Migrated from github.com)

The contains method performs a linear search through the entire byte slice. For large files, this could be inefficient. Consider using memchr::memchr(0, data).is_some() which uses SIMD optimizations, or at minimum check only a sample of the data (e.g., first few KB) since NUL bytes in text files typically appear early if present.

    if memchr::memchr(0, data).is_some() {
        // Even though the NUL character is valid in UTF-8, it's highly suspicious in
        // human-readable text.
        return true;
    }
use memchr;
The `contains` method performs a linear search through the entire byte slice. For large files, this could be inefficient. Consider using `memchr::memchr(0, data).is_some()` which uses SIMD optimizations, or at minimum check only a sample of the data (e.g., first few KB) since NUL bytes in text files typically appear early if present. ```suggestion if memchr::memchr(0, data).is_some() { // Even though the NUL character is valid in UTF-8, it's highly suspicious in // human-readable text. return true; } use memchr; ```
schmelczer commented 2025-11-16 21:27:52 +00:00 (Migrated from github.com)

Don't want to add another dep

Don't want to add another dep
std::str::from_utf8(data).is_err()
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_is_binary() {
assert!(is_binary(&[0, 159, 146, 150]));
assert!(is_binary(&[0, 12]));
assert!(!is_binary(b"hello"));
}
}