Skip to content

Commit d2474c9

Browse files
committed
Address review feedback
- Refresh the restorable copy when a project is reopened from Open Recent. `loadProject` moves that path to the front of the recent list, so in "last" mode it becomes the project the next launch resolves to -- but the copy on disk still held whichever project was opened through the picker last, so its `sourcePath` no longer matched and the cold start fell back to the unavailable-project banner. Open A, open B, reopen A from the recent list was enough to hit it. `openRecentProjectFile` now returns the raw text alongside the parsed project so the caller can keep the copy in step without re-reading a URI whose grant may already be gone. - Measure the snapshot size limit in UTF-8 bytes rather than `text.length`. The string is written as UTF-8, so a project of three-byte characters (CJK layer names, accented attribute values in embedded GeoJSON) could be three times the limit and still pass. `exceedsStartupSnapshotLimit` bounds the byte count by the code-unit count first and only encodes when that is inconclusive, so the oversized projects the guard exists to reject are still rejected without allocating a second copy of them on a phone. Covered by a boundary test over ASCII, three-byte, and surrogate-pair text. - State in the capability comment that `mkdir` is already granted by `fs:default` (its `create-app-specific-dirs` set), so the missing `fs:allow-mkdir` next to the other explicit command permissions is deliberate rather than an oversight, and that without the scope entry the copies fail with "forbidden path". - Drop the claim that `MAX_STARTUP_SNAPSHOT_BYTES` "matches" the ceiling on a project fetched by URL. The two share a number today but bound different things -- a download buffered into memory versus a file kept on disk -- and neither has to move when the other does, so the wording now says that instead of implying an invariant nothing enforces.
1 parent 2771f74 commit d2474c9

5 files changed

Lines changed: 90 additions & 7 deletions

File tree

apps/geolibre-desktop/src-tauri/capabilities/default.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"allow": [{ "path": "$TEMP/geolibre-gdb-*.geojson" }]
2626
},
2727
{
28-
"comment": "Restorable copies of the startup project (GeoLibre#1948). On Android a project picked from device storage is a content:// URI whose read grant dies with the process, so the startup restore has nothing to reopen; GeoLibre keeps its own copy here instead. This must be `fs:scope` rather than a path on the read/write permissions above: an fs permission's scope applies only to the commands that permission grants, so `fs:default`'s app-directory scope reaches `read_text_file` but not `mkdir` or `write_text_file` (the app's other writes are to paths a dialog just added to the runtime scope). Two entries because the directory itself is the mkdir target and its files are the read/write targets. See lib/startup-project-snapshot.ts.",
28+
"comment": "Restorable copies of the startup project (GeoLibre#1948). On Android a project picked from device storage is a content:// URI whose read grant dies with the process, so the startup restore has nothing to reopen; GeoLibre keeps its own copy here instead. Only a scope entry is needed, no new command permission: `mkdir` is already granted by `fs:default` (its `create-app-specific-dirs` set), and `read_text_file`/`write_text_file` by the entries above. It must be `fs:scope` rather than a path on those entries because an fs permission's scope applies only to the commands that permission grants, so `fs:default`'s app-directory scope reaches `read_text_file` but not `mkdir` or `write_text_file` (the app's other writes are to paths a dialog just added to the runtime scope) — without this the copies fail with \"forbidden path\". Two entries because the directory itself is the mkdir target and its files are the read/write targets. See lib/startup-project-snapshot.ts.",
2929
"identifier": "fs:scope",
3030
"allow": [
3131
{ "path": "$APPLOCALDATA/startup-projects" },

apps/geolibre-desktop/src/hooks/useProjectFileActions.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,13 @@ export function useProjectFileActions(mapControllerRef: MapControllerRef) {
701701
const project = await resolveProjectXyzLayers(result.project, controller.signal);
702702
if (controller.signal.aborted) return null;
703703
loadProject(project, result.path);
704+
// `loadProject` moves this path to the front of the recent list, so in
705+
// "last" mode it is now the project the next launch will reopen. Without
706+
// this, reopening an older project from the recent list would leave the
707+
// copy on disk holding whichever project was opened through the picker
708+
// last, and the next cold start would find no copy matching the path it
709+
// resolves (GeoLibre#1948 review).
710+
rememberStartupProjectSnapshot(result.path, result.text);
704711
return null;
705712
} catch (error) {
706713
if (controller.signal.aborted) return null;

apps/geolibre-desktop/src/lib/startup-project-snapshot.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,12 @@ export const STARTUP_SNAPSHOT_DIR = "startup-projects";
5959
* Above this the copy is skipped and the restore keeps today's behaviour (the
6060
* "startup project is unavailable" banner). A project with embedded vector data
6161
* can run to hundreds of megabytes, and silently doubling that on a phone's
62-
* internal storage would be a worse bug than the one being fixed. Matches the
63-
* ceiling `openRecentProjectFile` already applies to a project fetched by URL.
62+
* internal storage would be a worse bug than the one being fixed.
63+
*
64+
* Its own limit, deliberately: `openRecentProjectFile` happens to cap a project
65+
* fetched by URL at the same number, but that one bounds a download buffered
66+
* into memory and this one bounds a file kept on disk. Neither has to move when
67+
* the other does.
6468
*/
6569
export const MAX_STARTUP_SNAPSHOT_BYTES = 25 * 1024 * 1024;
6670

@@ -83,6 +87,28 @@ export function startupSnapshotFile(slot: StartupSnapshotSlot): string {
8387
return `${slot}.geolibre.json`;
8488
}
8589

90+
/**
91+
* Whether a project is too large to keep a copy of, measured as the UTF-8 bytes
92+
* the file would actually occupy.
93+
*
94+
* `text.length` counts UTF-16 code units, so a project full of non-ASCII (CJK
95+
* layer names, accented attribute values in embedded GeoJSON) can be up to three
96+
* times the size of the string that passed the check. The bounds below settle
97+
* most projects without encoding anything: the byte count is never below the
98+
* code-unit count and never above three times it, and encoding is a full second
99+
* copy of the text -- which for the very projects this guard exists to reject
100+
* would mean allocating hundreds of megabytes on a phone just to confirm they
101+
* are too big.
102+
*
103+
* @param text - The serialized project.
104+
* @returns True when the copy would exceed {@link MAX_STARTUP_SNAPSHOT_BYTES}.
105+
*/
106+
export function exceedsStartupSnapshotLimit(text: string): boolean {
107+
if (text.length > MAX_STARTUP_SNAPSHOT_BYTES) return true;
108+
if (text.length * 3 <= MAX_STARTUP_SNAPSHOT_BYTES) return false;
109+
return new TextEncoder().encode(text).byteLength > MAX_STARTUP_SNAPSHOT_BYTES;
110+
}
111+
86112
function defaultStorage(): SnapshotStorage | null {
87113
try {
88114
return typeof window === "undefined" ? null : window.localStorage;
@@ -188,9 +214,9 @@ export async function writeStartupSnapshot(
188214
): Promise<StartupSnapshotSlot | null> {
189215
const slot = startupSnapshotSlot(path, settings);
190216
if (!slot) return null;
191-
if (text.length > MAX_STARTUP_SNAPSHOT_BYTES) {
217+
if (exceedsStartupSnapshotLimit(text)) {
192218
console.warn(
193-
`Startup project is too large to keep a restorable copy (${text.length} bytes).`,
219+
`Startup project is too large to keep a restorable copy (over ${MAX_STARTUP_SNAPSHOT_BYTES} bytes).`,
194220
path,
195221
);
196222
return null;

apps/geolibre-desktop/src/lib/tauri-io.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2850,12 +2850,23 @@ function isFileMissingError(error: unknown): boolean {
28502850
);
28512851
}
28522852

2853+
/**
2854+
* Reopen a project from a remembered path, URL, or Android content URI.
2855+
*
2856+
* @param path - The remembered location.
2857+
* @param signal - Abort signal for the URL branch's fetch.
2858+
* @returns The parsed project, the path it came from, and the raw text -- which
2859+
* callers hand to {@link saveStartupProjectSnapshot} so reopening from Open
2860+
* Recent keeps the restorable copy pointing at the project that is now the
2861+
* most recent one.
2862+
*/
28532863
export async function openRecentProjectFile(
28542864
path: string,
28552865
signal?: AbortSignal,
28562866
): Promise<{
28572867
project: GeoLibreProject;
28582868
path: string;
2869+
text: string;
28592870
}> {
28602871
if (isHttpUrl(path)) {
28612872
const response = await fetch(path, {
@@ -2884,7 +2895,8 @@ export async function openRecentProjectFile(
28842895
);
28852896
}
28862897

2887-
return { project: parseProject(await response.text()), path };
2898+
const body = await response.text();
2899+
return { project: parseProject(body), path, text: body };
28882900
}
28892901

28902902
if (!isTauri()) {
@@ -2914,7 +2926,7 @@ export async function openRecentProjectFile(
29142926
text = snapshot;
29152927
}
29162928

2917-
return { project: parseProject(text), path };
2929+
return { project: parseProject(text), path, text };
29182930
}
29192931

29202932
export async function saveProjectFile(

tests/startup-project-snapshot.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, it } from "node:test";
44
import type { StartupSettings } from "../apps/geolibre-desktop/src/hooks/useDesktopSettings";
55
import { STARTUP_SNAPSHOTS_STORAGE_KEY } from "../apps/geolibre-desktop/src/lib/storage-keys";
66
import {
7+
exceedsStartupSnapshotLimit,
78
MAX_STARTUP_SNAPSHOT_BYTES,
89
readStartupSnapshot,
910
readStartupSnapshotIndex,
@@ -92,6 +93,27 @@ describe("startupSnapshotSlot", () => {
9293
});
9394
});
9495

96+
describe("exceedsStartupSnapshotLimit", () => {
97+
it("measures the bytes the file will hold, not the code units of the string", () => {
98+
// The file is written as UTF-8, so a project of three-byte characters is
99+
// over the limit at a third of the string length that ASCII would need.
100+
const thirdOfLimit = Math.floor(MAX_STARTUP_SNAPSHOT_BYTES / 3);
101+
assert.equal(exceedsStartupSnapshotLimit("a".repeat(thirdOfLimit)), false);
102+
assert.equal(exceedsStartupSnapshotLimit("\u20ac".repeat(thirdOfLimit + 1)), true);
103+
// A surrogate pair is four bytes over two code units, so it costs two bytes
104+
// per code unit rather than three: this many is 20 MB, still under.
105+
assert.equal(
106+
exceedsStartupSnapshotLimit("\u{1f600}".repeat(Math.floor(MAX_STARTUP_SNAPSHOT_BYTES / 5))),
107+
false,
108+
);
109+
});
110+
111+
it("accepts and rejects at the ASCII boundary", () => {
112+
assert.equal(exceedsStartupSnapshotLimit("a".repeat(MAX_STARTUP_SNAPSHOT_BYTES)), false);
113+
assert.equal(exceedsStartupSnapshotLimit("a".repeat(MAX_STARTUP_SNAPSHOT_BYTES + 1)), true);
114+
});
115+
});
116+
95117
describe("writeStartupSnapshot", () => {
96118
const warnings: unknown[][] = [];
97119
const originalWarn = console.warn;
@@ -177,6 +199,22 @@ describe("writeStartupSnapshot", () => {
177199
assert.equal(warnings.length, 1);
178200
});
179201

202+
it("skips one that is only too large once encoded as UTF-8", async () => {
203+
const storage = makeStorage();
204+
const { io, writes } = makeIo();
205+
// Well under the limit counted as code units, well over it as bytes.
206+
const slot = await writeStartupSnapshot(
207+
CONTENT_URI,
208+
"\u20ac".repeat(Math.floor(MAX_STARTUP_SNAPSHOT_BYTES / 2)),
209+
settings({ mode: "last" }),
210+
io,
211+
{ storage },
212+
);
213+
assert.equal(slot, null);
214+
assert.equal(writes.length, 0);
215+
assert.equal(warnings.length, 1);
216+
});
217+
180218
it("swallows a write failure so it cannot fail the save that triggered it", async () => {
181219
const storage = makeStorage();
182220
const { io } = makeIo({ writeError: new Error("No space left on device") });

0 commit comments

Comments
 (0)