From cc0ad0010be2dc01cca4e718f7331829b1f818ed Mon Sep 17 00:00:00 2001 From: YidaWeng <84164537+YidaWeng@users.noreply.github.com> Date: Wed, 28 Oct 2026 12:35:05 +0800 Subject: [PATCH] feat(#417): read a file's raw bytes in bounded windows fs.read is text only: a file with NUL or invalid UTF-8 is refused as Binary, and nothing else in the SDK returns a file's bytes, so a binary file cannot be read back after fs.write. This adds the byte read the issue proposes; the issue's owner decision is still pending, so this is the proposed shape, not a settled one. - fs.read_bytes(ref, path, offset = 0, length?) answers { kind, snapshot, root, path, bytes, size, hasMore }. It reads one window at the same immutable snapshot as fs.read (Branch, Commit or Layer). `length` defaults to and is capped at the 48 KiB text window, so a whole-file read never holds a large file in memory; a larger file is paged by advancing offset by the returned byte count. - An offset past the end answers empty bytes with hasMore: false. A directory is refused IsDirectory, a symlink NotFile, a missing path NoSuchPath, and a negative or unsafe offset/length InvalidInput. - The engine reads through layerfs-content's existing read_range over the call's read caches, so it is the same object walk as blame's byte windows; telemetry is `sdk.bytes`. No storage, binding or control-record change: the binding already returns SdkValue::Bytes as a Node Buffer. - sdk/fs surface: the @layerfs/agent wrapper, the `layerfs fs read_bytes` command (--offset, --length), the CLI's JSON bytes output (already { base64 }), and the agent/cli docs tables. Tests: added engine/crates/layerfs-api/tests/sdk_read_bytes.rs (a 48 KiB+1000-byte file read in two windows, cap of a longer request, offset past EOF, zero length; directory/symlink/missing/negative refusals). The sdk_method_golden transcript gains six fs.read_bytes lines. The JS fixture gains SDK and CLI cases; binary_write.mjs checks a binary round-trip, paging, past-EOF and refusals; packaging.mjs counts fs methods 14 -> 15. No tests deleted. Checks (macOS): cargo +1.85.1 test -p layerfs-api --test sdk_read_bytes --test sdk_method_golden (3 passed); clippy --locked --all-targets -D warnings; cargo fmt --all -- --check; test --locked --all-targets --no-run; tools/check_product_boundary.py PASS (959 files); tools tests 155 passed; node api/agent/test/calls.mjs PASS (41 cases); node --test api/cli/test/cli.test.mjs (77 passed); packaging surface and lifecycle pass. The focused Rust tests ran with rusqlite's bundled SQLite enabled locally (the host SQLite is 3.54.0, outside the overlay profile); the manifest is unchanged in this commit. Not run here: api/agent/test/binary_write.mjs and the Linux FUSE targets, which need the real native binding and /dev/fuse on Linux. Production LOC: 102763 -> 102848 (delta +85) core 38395 -> 38395 (0), engine 61709 -> 61783 (+74), api 2659 -> 2670 (+11); reference absent (retired R9c; last counted 65,417 at tree 498dd1917812). Method: python3 tools/production_loc.py --root on `git archive` of the parent (582bc786f) and of the staged tree 9ae38d1f41. --- api/agent/src/fs/index.js | 1 + api/agent/src/fs/read_bytes.js | 4 + api/agent/test/binary_write.mjs | 21 +++- api/agent/test/fixture/cases.mjs | 4 + api/agent/test/fixture/fake-native.cjs | 1 + api/agent/test/packaging.mjs | 4 +- api/cli/src/fs/index.js | 3 +- api/cli/src/fs/read_bytes.js | 8 ++ api/docs/agent.md | 22 ++-- core/docs/architecture/82-sdk-call-channel.md | 15 ++- engine/crates/layerfs-api/src/base/args.rs | 11 ++ .../crates/layerfs-api/src/base/dispatch.rs | 1 + engine/crates/layerfs-api/src/fs/mod.rs | 2 + .../crates/layerfs-api/src/fs/read_bytes.rs | 21 ++++ .../layerfs-api/tests/golden/sdk_methods.txt | 6 + .../layerfs-api/tests/sdk_method_golden.rs | 37 ++++++ .../layerfs-api/tests/sdk_read_bytes.rs | 116 ++++++++++++++++++ .../layerfs-store-service/src/read/reads.rs | 49 ++++++++ 18 files changed, 309 insertions(+), 17 deletions(-) create mode 100644 api/agent/src/fs/read_bytes.js create mode 100644 api/cli/src/fs/read_bytes.js create mode 100644 engine/crates/layerfs-api/src/fs/read_bytes.rs create mode 100644 engine/crates/layerfs-api/tests/sdk_read_bytes.rs diff --git a/api/agent/src/fs/index.js b/api/agent/src/fs/index.js index 0909b56589..fd0986c24b 100644 --- a/api/agent/src/fs/index.js +++ b/api/agent/src/fs/index.js @@ -1,5 +1,6 @@ // The `fs` group: reads at a saved snapshot and one-Commit Branch writes. export { read } from "./read.js"; +export { read_bytes } from "./read_bytes.js"; export { list } from "./list.js"; export { stat } from "./stat.js"; export { grep } from "./grep.js"; diff --git a/api/agent/src/fs/read_bytes.js b/api/agent/src/fs/read_bytes.js new file mode 100644 index 0000000000..bdff10f8dc --- /dev/null +++ b/api/agent/src/fs/read_bytes.js @@ -0,0 +1,4 @@ +// fs.read_bytes: one bounded window of a file's raw bytes at a saved snapshot. +export function read_bytes(client, ref, path, offset = 0, length) { + return client.call("fs.read_bytes", { branch: ref, path, offset, length }); +} diff --git a/api/agent/test/binary_write.mjs b/api/agent/test/binary_write.mjs index 9c1cd93569..dfc4ae161f 100644 --- a/api/agent/test/binary_write.mjs +++ b/api/agent/test/binary_write.mjs @@ -1,8 +1,9 @@ // Binary write bodies (#413): fs.write and fs.apply take a Buffer, any typed // array, a DataView or an ArrayBuffer as the bytes they view, and refuse -// shared, detached and non-binary bodies before the call. Reads stay text -// only. Real native binding, Disposable Store, no Docker or daemon; Linux, -// with LAYERFS_STORE_PATH on a local volume (the E2E launcher's): +// shared, detached and non-binary bodies before the call. fs.read stays text +// only; fs.read_bytes (#417) returns the same bytes back. Real native binding, +// Disposable Store, no Docker or daemon; Linux, with LAYERFS_STORE_PATH on a +// local volume (the E2E launcher's): // node api/agent/test/binary_write.mjs import assert from "node:assert/strict"; import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; @@ -39,10 +40,22 @@ try { assert.equal(await text(path), "ABCD", path); } - // Bytes that are not text are stored whole and read back as Binary. + // Bytes that are not text are stored whole, read back as Binary by fs.read, + // and read back exactly by fs.read_bytes. await layerfs.fs.write("feat", "/blob", Buffer.from([0, 0xff, 0xfe, 1])); assert.equal(await size("/blob"), 4); await assert.rejects(layerfs.fs.read("feat", "/blob"), { code: "Binary" }); + const whole = await layerfs.fs.read_bytes("feat", "/blob"); + assert.ok(Buffer.isBuffer(whole.bytes)); + assert.deepEqual([...whole.bytes], [0, 0xff, 0xfe, 1]); + assert.deepEqual([whole.size, whole.hasMore], [4, false]); + const middle = await layerfs.fs.read_bytes("feat", "/blob", 1, 2); + assert.deepEqual([...middle.bytes], [0xff, 0xfe]); + assert.deepEqual([middle.size, middle.hasMore], [4, true]); + const past = await layerfs.fs.read_bytes("feat", "/blob", 9); + assert.deepEqual([[...past.bytes], past.size, past.hasMore], [[], 4, false]); + await assert.rejects(layerfs.fs.read_bytes("feat", "/"), { code: "IsDirectory" }); + await assert.rejects(layerfs.fs.read_bytes("feat", "/missing"), { code: "NoSuchPath" }); // fs.apply: `bytes` in any binary form, and binary `content` sent as bytes. const applied = await layerfs.fs.apply("feat", [ diff --git a/api/agent/test/fixture/cases.mjs b/api/agent/test/fixture/cases.mjs index ec70729391..535987631d 100644 --- a/api/agent/test/fixture/cases.mjs +++ b/api/agent/test/fixture/cases.mjs @@ -69,6 +69,8 @@ export const CASES = [ op: "branch.publish", sent: { branch: "cart" }, result: { kind: "Added", source: { branch: "feature", commit: "pub1" }, publishedLayer: { id: "lay1" } } }, { name: "fs read", argv: ["fs", "read", "main", "/a.txt", "--start-line=3", "--end-line", "9"], sdk: (lf) => lf.fs.read("main", "/a.txt", 3, 9), op: "fs.read", sent: { branch: "main", path: "/a.txt", startLine: 3, endLine: 9 }, result: { text: "b\n", nextLine: null } }, + { name: "fs read_bytes", argv: ["fs", "read_bytes", "main", "/a.bin", "--offset", "2", "--length", "3"], sdk: (lf) => lf.fs.read_bytes("main", "/a.bin", 2, 3), + op: "fs.read_bytes", sent: { branch: "main", path: "/a.bin", offset: 2, length: 3 }, result: { bytes: { base64: "AP8B" }, size: 5, hasMore: true } }, { name: "fs list", argv: ["fs", "list", "main"], sdk: (lf) => lf.fs.list("main"), op: "fs.list", sent: { branch: "main", path: "/", pageSize: 50, pagePos: 0 }, result: page }, { name: "fs stat", argv: ["fs", "stat", "main", "/a"], sdk: (lf) => lf.fs.stat("main", "/a"), @@ -118,6 +120,8 @@ export const CASES = [ // the first window, and refusals: the binding's, then one the SDK makes itself { name: "fs read (first window)", argv: ["fs", "read", "main", "/a"], sdk: (lf) => lf.fs.read("main", "/a"), op: "fs.read", sent: { branch: "main", path: "/a", startLine: 1 }, result: { text: "a\n", nextLine: 2 } }, + { name: "fs read_bytes (first window)", argv: ["fs", "read_bytes", "main", "/a.bin"], sdk: (lf) => lf.fs.read_bytes("main", "/a.bin"), + op: "fs.read_bytes", sent: { branch: "main", path: "/a.bin", offset: 0 }, result: { bytes: { base64: "AP8B" }, size: 5, hasMore: true } }, { name: "fs read (missing)", argv: ["fs", "read", "main", "/missing"], sdk: (lf) => lf.fs.read("main", "/missing"), op: "fs.read", sent: { path: "/missing" }, refused: { code: "NoSuchPath", message: "NoSuchPath: /missing does not exist", detail: null } }, { name: "branch merge (refused)", argv: ["branch", "merge", "a", "b", "ab"], sdk: (lf) => lf.branch.merge("a", "b", "ab"), refuse: ["branch.merge"], diff --git a/api/agent/test/fixture/fake-native.cjs b/api/agent/test/fixture/fake-native.cjs index 19dbe47be4..f84a9f0523 100644 --- a/api/agent/test/fixture/fake-native.cjs +++ b/api/agent/test/fixture/fake-native.cjs @@ -50,6 +50,7 @@ function answer(op, args) { case "branch.merge": return { value: { kind: "Merged", branch: args.name, head: "m3rge", root: "r00t" } }; case "branch.publish": return { value: { kind: "Added", source: { branch: "feature", commit: "pub1" }, publishedLayer: { id: "lay1" } } }; case "fs.read": return { value: args.startLine === 1 ? { text: "a\n", nextLine: 2 } : { text: "b\n", nextLine: null } }; + case "fs.read_bytes": return { value: { bytes: Buffer.from([0, 255, 1]), size: 5, hasMore: true } }; case "fs.stat": return { value: { type: "file", size: 5, mode: 420 } }; case "fs.rm": return { value: { kind: "UpToDate", head: "h3ad" } }; case "workspace.mount": return { value: { kind: "Mounted", sandbox: args.sandbox, container: `layerfs-website-${args.sandbox}`, path: "/workspaces/w1", image: "fake-image", branch: args.branch, base: "b4se", baseLayer: "lay0", root: "r00t" } }; diff --git a/api/agent/test/packaging.mjs b/api/agent/test/packaging.mjs index 496a616b95..75ccfb3761 100644 --- a/api/agent/test/packaging.mjs +++ b/api/agent/test/packaging.mjs @@ -31,11 +31,11 @@ if (selected === "surface") { assert.equal(shop.projectName, "website"); assert.equal(shop.options.home, "/private/configured-home"); assert.deepEqual(shop.options.deploy, { docker: "/configured/docker.sock", image: "configured-image", volume: "configured-volume", daemon: "/configured/daemon" }); - // five groups: project 2, branch 6, fs 14, sandbox 3, workspace 4 + // five groups: project 2, branch 6, fs 15, sandbox 3, workspace 4 const methods = (group) => Object.keys(shop[group]).sort(); assert.deepEqual(methods("project"), ["list_branches", "list_sandboxes"]); assert.deepEqual(methods("branch"), ["diff_branches", "diff_commits", "fork", "list_commits", "merge", "publish"]); - assert.equal(methods("fs").length, 14); + assert.equal(methods("fs").length, 15); assert.deepEqual(methods("sandbox"), ["close", "create", "list_workspaces"]); assert.deepEqual(methods("workspace"), ["commit", "exec", "mount", "unmount"]); assert.equal(shop.inspect, undefined); diff --git a/api/cli/src/fs/index.js b/api/cli/src/fs/index.js index 553fe5e696..303f2817ce 100644 --- a/api/cli/src/fs/index.js +++ b/api/cli/src/fs/index.js @@ -1,5 +1,6 @@ // The `fs` commands, in help order: reads at a saved snapshot and one-Commit Branch writes. import read from "./read.js"; +import read_bytes from "./read_bytes.js"; import list from "./list.js"; import stat from "./stat.js"; import grep from "./grep.js"; @@ -14,4 +15,4 @@ import rm from "./rm.js"; import cp from "./cp.js"; import mv from "./mv.js"; -export default { read, list, stat, grep, glob, diff_files, blame, write, edit, apply, mkdir, rm, cp, mv }; +export default { read, read_bytes, list, stat, grep, glob, diff_files, blame, write, edit, apply, mkdir, rm, cp, mv }; diff --git a/api/cli/src/fs/read_bytes.js b/api/cli/src/fs/read_bytes.js new file mode 100644 index 0000000000..a76cf812ee --- /dev/null +++ b/api/cli/src/fs/read_bytes.js @@ -0,0 +1,8 @@ +// layerfs fs read_bytes: fs.read_bytes(ref, path, offset?, length?). + +export default { + args: ["ref", "path"], + options: { offset: "int", length: "int" }, + summary: "a file's raw bytes, a bounded window at a time", + run: (lf, a, o) => lf.fs.read_bytes(a.ref, a.path, o.offset, o.length), +}; diff --git a/api/docs/agent.md b/api/docs/agent.md index 330fcab914..45d08eb834 100644 --- a/api/docs/agent.md +++ b/api/docs/agent.md @@ -269,7 +269,7 @@ trace line covers the command's run. | --- | --- | | `LayerFS` (2) | `LayerFS.open(name)`, `LayerFS.init(name, folder, timeout?)`: static, each returns a ready client | | project (2) | `list_branches(pageSize?, pagePos?)`, `list_sandboxes(pageSize?, pagePos?)` | -| fs reads (7) | `read(ref, path, startLine?, endLine?)`, `list(ref, path?, pageSize?, pagePos?)`, `stat(ref, path)`, `grep(ref, pattern, path?, pageSize?, pagePos?)`, `glob(ref, pattern, path?, pageSize?, pagePos?)`, `diff_files(leftRef, leftPath, rightRef, rightPath)`, `blame(ref, path, fromLine?)` | +| fs reads (8) | `read(ref, path, startLine?, endLine?)`, `read_bytes(ref, path, offset?, length?)`, `list(ref, path?, pageSize?, pagePos?)`, `stat(ref, path)`, `grep(ref, pattern, path?, pageSize?, pagePos?)`, `glob(ref, pattern, path?, pageSize?, pagePos?)`, `diff_files(leftRef, leftPath, rightRef, rightPath)`, `blame(ref, path, fromLine?)` | | fs writes (7) | `write(branch, path, content, message? \| { message?, mode? })`, `edit(branch, path, oldText, newText, message?)`, `apply(branch, changes, message?)`, `mkdir(branch, path, message? \| { message?, mode? })`, `rm(branch, path, message?)`, `cp(branch, from, to, message?)`, `mv(branch, from, to, message?)` | | branch (6) | `fork(name, fromRef?)`, `list_commits(branch, pageSize?, pagePos?)`, `diff_commits(from, to, pageSize?, pagePos?, { cursor?, hunks? }?)`, `diff_branches(from, to, pageSize?, pagePos?, { cursor?, hunks? }?)`, `merge(a, b, name, message?, units?)`, `publish(branch, commit?)` | | sandbox (3) | `create({ image }?)` → `{ sandbox, image }`, `list_workspaces(sandbox, pageSize?, pagePos?)`, `close(sandbox)` | @@ -341,9 +341,14 @@ binary `content` is sent as `bytes`. Bytes in a `SharedArrayBuffer` or a detached `ArrayBuffer` are refused as InvalidInput, and so is anything else, such as an array of numbers. The bytes are copied once the call starts, after the client's Store is open, so do not change them until the call settles. -Reads are text only: `fs.read` refuses a file that is not UTF-8 text as -`Binary`, and `fs.stat` reports its size. `node api/agent/test/binary_write.mjs` -checks these forms and refusals on Linux. +`fs.read` is text only: it refuses a file that is not UTF-8 text as `Binary`, +and `fs.stat` reports its size. `fs.read_bytes(ref, path, offset?, length?)` +returns up to `length` raw bytes from `offset` (default 0), capped at one 48 KiB +window, with the file's `size` and `hasMore`; a larger file is paged by +advancing `offset` by the returned byte count. It refuses a directory +`IsDirectory`, a symlink `NotFile` and a missing path `NoSuchPath`; an offset +past the end answers empty `bytes` with `hasMore: false`. +`node api/agent/test/binary_write.mjs` checks these forms and refusals on Linux. ### Pages and reads @@ -360,9 +365,11 @@ component-by-component byte order, then line number for grep. Commit history is newest first along first parents. Every call resolves each live ref once; separate calls against live heads can shift numeric positions. -Read/blame use 1-based inclusive line positions, no byte offsets or byte-read -method. Responses fit the authenticated control frame: at most 2000 lines and -48 KiB of text/value payload, below the selected 256 KiB ceiling. LF separates +Read/blame use 1-based inclusive line positions. `fs.read_bytes` is the byte +read: it takes a byte `offset` and `length`, capped at one 48 KiB window, and +answers the file's `size` and `hasMore`. Responses fit the authenticated +control frame: at most 2000 lines and 48 KiB of text/value payload, below the +selected 256 KiB ceiling. LF separates lines; CRLF text is preserved, and a final LF creates no phantom line. Read reports `startLine`, `endLine`, `truncated`, `lineTruncated` and `nextLine`. Huge lines return a UTF-8 prefix, with no within-line continuation; the remainder is explicitly @@ -502,6 +509,7 @@ table differs, and when a family's files stop matching the SDK's. | `branch.merge` | `layerfs branch merge [-m ] [--units ]` | | `branch.publish` | `layerfs branch publish []` | | `fs.read` | `layerfs fs read [--start-line ] [--end-line ]` | +| `fs.read_bytes` | `layerfs fs read_bytes [--offset ] [--length ]` | | `fs.list` | `layerfs fs list [] [--page-size ] [--page-pos ]` | | `fs.stat` | `layerfs fs stat ` | | `fs.grep` | `layerfs fs grep [] [--page-size ] [--page-pos ]` | diff --git a/core/docs/architecture/82-sdk-call-channel.md b/core/docs/architecture/82-sdk-call-channel.md index f0090fe08e..9c48d751e8 100644 --- a/core/docs/architecture/82-sdk-call-channel.md +++ b/core/docs/architecture/82-sdk-call-channel.md @@ -4,7 +4,7 @@ > #352 and #373 source updates below. Correctness receipts and the component > counter screen are distinct from this architecture. -The JavaScript SDK has 27 positional methods in four groups (29 before #373 +The JavaScript SDK has 28 positional methods in four groups (29 before #373 replaced the `sandbox` group with `workspace`; see the #373 update below). The [SDK guide](../../../api/docs/agent.md) lists signatures, environment configuration, read/page limits and compatibility. [SPEC](../issues/327/SPEC.md) describes the @@ -68,6 +68,13 @@ read caches (then `sdk/cached.rs`, now store-service `read/cached.rs`) and resol the walker memoizes directory pages and the inode table's upper pages. Results are byte-identical to the earlier calls; paging still counts positions. +Issue #417 source update, 2026-10-28: `fs.read_bytes` reads one bounded raw +byte window through `layerfs-content`'s `read_range`, at the same immutable +snapshot and path lookup as `fs.read`. `length` defaults to and is capped at the +48 KiB text window; an offset past the end returns no bytes and `hasMore: +false`. No storage, binding or control-record change is needed: the binding +already returns `SdkValue::Bytes` as a Buffer. + Diff omits a same-identity directory only when its metadata is also unchanged. Directory metadata changes occupy a returned item and therefore exactly one pagination position. This corrects the earlier mismatch where the scan counted @@ -85,8 +92,10 @@ retains the original failure and custody. There is no resident paths draft or separate canonical encoder. Main is read-only except through publish. Immutable reads use 1-based inclusive lines, at most 2,000 lines/48 KiB reply -payload. Huge-line UTF-8 prefixes, omitted remainder and nextLine are explicit; -there is no byte-read method. Grep streams literal matching. Blame follows +payload. Huge-line UTF-8 prefixes, omitted remainder and nextLine are explicit. +`fs.read_bytes` is the byte read: at most 48 KiB of a regular file's raw bytes +from a byte offset, with the file's size and hasMore; a directory, symlink or +missing path is refused. Grep streams literal matching. Blame follows immutable first parents without retaining the entire chain, skips unchanged content IDs and uses bounded LCS with exact common edges. In coarse comparisons, a line is inherited from the common suffix only when it starts at a diff --git a/engine/crates/layerfs-api/src/base/args.rs b/engine/crates/layerfs-api/src/base/args.rs index 365224e7b9..14bbcdab1c 100644 --- a/engine/crates/layerfs-api/src/base/args.rs +++ b/engine/crates/layerfs-api/src/base/args.rs @@ -49,6 +49,17 @@ pub fn line(args: &SdkValue, name: &str, default: u64) -> Result Result { + match args.get(name) { + None | Some(SdkValue::Null) => Ok(default), + Some(SdkValue::Int(n)) if (0..=9_007_199_254_740_991).contains(n) => Ok(*n as u64), + _ => Err(invalid(format!( + "{name} must be a non-negative safe integer" + ))), + } +} + /// A tree difference's page: offset or `cursor` path, and whether `hunks`. pub fn diff_page(args: &SdkValue) -> Result { let (limit, pos) = paging(args)?; diff --git a/engine/crates/layerfs-api/src/base/dispatch.rs b/engine/crates/layerfs-api/src/base/dispatch.rs index 0db081d124..4105bb4e5b 100644 --- a/engine/crates/layerfs-api/src/base/dispatch.rs +++ b/engine/crates/layerfs-api/src/base/dispatch.rs @@ -27,6 +27,7 @@ pub fn call(service: &StoreService, call: &SdkCall) -> Answer { "branch.diff_commits" => branch::diff_commits(service, args), "branch.diff_branches" => branch::diff_branches(service, args), "fs.read" => fs::read(service, args), + "fs.read_bytes" => fs::read_bytes(service, args), "fs.list" => fs::list(service, args), "fs.stat" => fs::stat(service, args), "fs.grep" => fs::grep(service, args), diff --git a/engine/crates/layerfs-api/src/fs/mod.rs b/engine/crates/layerfs-api/src/fs/mod.rs index 876b4dc1bd..503479df97 100644 --- a/engine/crates/layerfs-api/src/fs/mod.rs +++ b/engine/crates/layerfs-api/src/fs/mod.rs @@ -10,6 +10,7 @@ mod list; mod mkdir; mod mv; mod read; +mod read_bytes; mod rm; mod stat; mod write; @@ -24,6 +25,7 @@ pub use list::list; pub use mkdir::mkdir; pub use mv::mv; pub use read::read; +pub use read_bytes::read_bytes; pub use rm::rm; pub use stat::stat; pub use write::write; diff --git a/engine/crates/layerfs-api/src/fs/read_bytes.rs b/engine/crates/layerfs-api/src/fs/read_bytes.rs new file mode 100644 index 0000000000..8d401f40d9 --- /dev/null +++ b/engine/crates/layerfs-api/src/fs/read_bytes.rs @@ -0,0 +1,21 @@ +//! `fs.read_bytes`. +use crate::base::args::{count, path, snapshot, Answer}; +use layerfs_bridge::control::SdkValue; +use layerfs_store_service::{ + read::{reads, window}, + StoreService, +}; + +/// `read_bytes(path, { branch, commit?, offset?, length? })`: one bounded +/// window of a file's raw bytes at a Branch, Commit or Layer. +pub fn read_bytes(service: &StoreService, args: &SdkValue) -> Answer { + let (reader, snap) = snapshot(service, args)?; + let path = path(args, "path")?; + reads::read_bytes( + &reader, + &snap, + &path, + count(args, "offset", 0), + count(args, "length", window::BYTES as u64), + ) +} diff --git a/engine/crates/layerfs-api/tests/golden/sdk_methods.txt b/engine/crates/layerfs-api/tests/golden/sdk_methods.txt index 4f7db82495..ff298eeaa5 100644 --- a/engine/crates/layerfs-api/tests/golden/sdk_methods.txt +++ b/engine/crates/layerfs-api/tests/golden/sdk_methods.txt @@ -34,6 +34,12 @@ fs.read -> refused NoSuchPath "/nope does not exist at feat@" {path: "/nope fs.read -> refused NoSuchBranch "Branch ghost does not exist" null fs.read -> refused InvalidInput "missing text argument `path`" null fs.read -> refused Binary "text contains NUL or invalid UTF-8" null +fs.read_bytes -> ok {kind: "ok", snapshot: "main@", root: "", path: "/blob.bin", bytes: bytes"[0, 159, 146, 150, 0, 1]", size: 6, hasMore: false} +fs.read_bytes -> ok {kind: "ok", snapshot: "main@", root: "", path: "/blob.bin", bytes: bytes"[146, 150]", size: 6, hasMore: true} +fs.read_bytes -> ok {kind: "ok", snapshot: "main@", root: "", path: "/blob.bin", bytes: bytes"[]", size: 6, hasMore: false} +fs.read_bytes -> refused IsDirectory "expected a regular file" null +fs.read_bytes -> refused InvalidInput "offset must be a non-negative safe integer" null +fs.read_bytes -> refused NoSuchPath "/nope does not exist at main@" {path: "/nope"} fs.list -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/", items: [{name: "a.txt", path: "/a.txt", type: "file", inode: 2}, {name: "applied-dir", path: "/applied-dir", type: "dir", inode: 201}, {name: "applied.txt", path: "/applied.txt", type: "file", inode: 200}, {name: "blob.bin", path: "/blob.bin", type: "file", inode: 3}, {name: "dir", path: "/dir", type: "dir", inode: 4}, {name: "made", path: "/made", type: "dir", inode: 72}, {name: "moved.txt", path: "/moved.txt", type: "file", inode: 137}, {name: "new.txt", path: "/new.txt", type: "file", inode: 8}], hasMore: false, nextPagePos: null} fs.list -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/dir", items: [{name: "b.txt", path: "/dir/b.txt", type: "file", inode: 5}], hasMore: true, nextPagePos: 1} fs.list -> refused NotDirectory "expected a directory" null diff --git a/engine/crates/layerfs-api/tests/sdk_method_golden.rs b/engine/crates/layerfs-api/tests/sdk_method_golden.rs index 590ec9f7c1..694584caa2 100644 --- a/engine/crates/layerfs-api/tests/sdk_method_golden.rs +++ b/engine/crates/layerfs-api/tests/sdk_method_golden.rs @@ -329,6 +329,43 @@ fn public_methods_answer_as_recorded() { "fs.read", vec![("branch", t("main")), ("path", t("/blob.bin"))], ); + s.call( + "fs.read_bytes", + vec![("branch", t("main")), ("path", t("/blob.bin"))], + ); + s.call( + "fs.read_bytes", + vec![ + ("branch", t("main")), + ("path", t("/blob.bin")), + ("offset", SdkValue::Int(2)), + ("length", SdkValue::Int(2)), + ], + ); + s.call( + "fs.read_bytes", + vec![ + ("branch", t("main")), + ("path", t("/blob.bin")), + ("offset", SdkValue::Int(6)), + ], + ); + s.call( + "fs.read_bytes", + vec![("branch", t("main")), ("path", t("/dir"))], + ); + s.call( + "fs.read_bytes", + vec![ + ("branch", t("main")), + ("path", t("/blob.bin")), + ("offset", SdkValue::Int(-1)), + ], + ); + s.call( + "fs.read_bytes", + vec![("branch", t("main")), ("path", t("/nope"))], + ); s.call("fs.list", vec![("branch", t("feat"))]); s.call( "fs.list", diff --git a/engine/crates/layerfs-api/tests/sdk_read_bytes.rs b/engine/crates/layerfs-api/tests/sdk_read_bytes.rs new file mode 100644 index 0000000000..17d614094a --- /dev/null +++ b/engine/crates/layerfs-api/tests/sdk_read_bytes.rs @@ -0,0 +1,116 @@ +//! `fs.read_bytes` (#417): a file's raw bytes at a saved snapshot, one +//! bounded window at a time, through the public method table. +#[path = "support/installed_store.rs"] +#[allow(dead_code)] +mod installed; +#[path = "support/sdk.rs"] +#[allow(dead_code)] +mod support; + +use layerfs_bridge::control::{SdkRefusal, SdkValue}; +use layerfs_store_service::read::window; +use support::Fixture; + +fn field<'a>(value: &'a SdkValue, name: &str) -> &'a SdkValue { + value.get(name).unwrap_or_else(|| panic!("missing {name}")) +} + +fn bytes(value: &SdkValue) -> &[u8] { + match field(value, "bytes") { + SdkValue::Bytes(bytes) => bytes, + other => panic!("bytes: {other:?}"), + } +} + +fn integer(value: &SdkValue, name: &str) -> i64 { + match field(value, name) { + SdkValue::Int(n) => *n, + other => panic!("{name}: {other:?}"), + } +} + +fn boolean(value: &SdkValue, name: &str) -> bool { + match field(value, name) { + SdkValue::Bool(b) => *b, + other => panic!("{name}: {other:?}"), + } +} + +fn call( + fixture: &Fixture, + path: &str, + offset: Option, + length: Option, +) -> Result { + let mut fields = vec![ + ("branch", SdkValue::text("main")), + ("path", SdkValue::text(path)), + ]; + fields.extend(offset.map(|n| ("offset", SdkValue::Int(n)))); + fields.extend(length.map(|n| ("length", SdkValue::Int(n)))); + fixture.try_call("fs.read_bytes", fields) +} + +#[test] +fn a_file_reads_back_in_bounded_windows() { + let content: Vec = (0..window::BYTES + 1000).map(|n| (n % 251) as u8).collect(); + let fixture = Fixture::new("sdk-read-bytes", |source| { + std::fs::write(source.join("big.bin"), &content).unwrap(); + 1 + }); + let read = |offset, length| call(&fixture, "/big.bin", offset, length).unwrap(); + + let first = read(None, None); + assert_eq!(bytes(&first), &content[..window::BYTES]); + assert_eq!(integer(&first, "size"), content.len() as i64); + assert!(boolean(&first, "hasMore")); + + let rest = read(Some(window::BYTES as i64), None); + assert_eq!(bytes(&rest), &content[window::BYTES..]); + assert_eq!(integer(&rest, "size"), content.len() as i64); + assert!(!boolean(&rest, "hasMore")); + + // A longer request is capped at one window. + let capped = read(Some(0), Some(1_000_000)); + assert_eq!(bytes(&capped), &content[..window::BYTES]); + assert!(boolean(&capped, "hasMore")); + + // An offset past the end names no bytes and no continuation. + let past = read(Some(content.len() as i64 + 5), None); + assert!(bytes(&past).is_empty()); + assert_eq!(integer(&past, "size"), content.len() as i64); + assert!(!boolean(&past, "hasMore")); + + let empty = read(Some(0), Some(0)); + assert!(bytes(&empty).is_empty()); + assert!(boolean(&empty, "hasMore")); +} + +#[test] +fn a_directory_a_symlink_and_a_missing_path_are_refused() { + let fixture = Fixture::new("sdk-read-bytes-refusals", |source| { + std::fs::write(source.join("file"), b"x").unwrap(); + std::os::unix::fs::symlink("file", source.join("link")).unwrap(); + 2 + }); + assert_eq!( + call(&fixture, "/", None, None).unwrap_err().code, + "IsDirectory" + ); + assert_eq!( + call(&fixture, "/link", None, None).unwrap_err().code, + "NotFile" + ); + assert_eq!( + call(&fixture, "/nope", None, None).unwrap_err().code, + "NoSuchPath" + ); + assert_eq!( + call(&fixture, "/file", Some(-1), None).unwrap_err().code, + "InvalidInput" + ); + assert_eq!( + call(&fixture, "/file", None, Some(-1)).unwrap_err().code, + "InvalidInput" + ); +} diff --git a/engine/crates/layerfs-store-service/src/read/reads.rs b/engine/crates/layerfs-store-service/src/read/reads.rs index 26ca567023..d1cc2a575d 100644 --- a/engine/crates/layerfs-store-service/src/read/reads.rs +++ b/engine/crates/layerfs-store-service/src/read/reads.rs @@ -76,6 +76,55 @@ pub fn read( answer(fields) } +/// One bounded window of the file at `path` as raw bytes, from `offset` for +/// at most `length` bytes. `length` is capped at [`window::BYTES`], and an +/// offset past the end returns no bytes with `hasMore: false`. +pub fn read_bytes( + reader: &StorePorts, + snap: &Snap, + path: &str, + offset: Arg, + length: Arg, +) -> Answer { + let cached = Cached::new(reader)?; + let entry = existing(&Memo::new(&cached), snap, path)?; + if entry.value.kind != InodeKind::RegularFile { + return Err(args::refuse( + if entry.value.kind == InodeKind::Directory { + "IsDirectory" + } else { + "NotFile" + }, + "expected a regular file", + SdkValue::Null, + )); + } + let offset = offset?; + let length = length?.min(window::BYTES as u64); + let size = size(reader, &entry)?; + let end = offset.saturating_add(length).min(size); + let mut bytes = Vec::new(); + if offset < end { + crate::telemetry::timed("sdk.bytes", |timing| { + layerfs_content::file::read_range( + &cached, + entry.value.content_root, + offset..end, + &mut bytes, + timing.child("content.read"), + ) + }) + .map_err(args::content)?; + } + let mut fields = header(snap, path); + fields.extend([ + ("bytes", SdkValue::Bytes(bytes)), + ("size", SdkValue::Int(size as i64)), + ("hasMore", SdkValue::Bool(end < size)), + ]); + answer(fields) +} + /// The type, inode, size and permission mode of the entry at `path`. An /// inline mode costs no object read. pub fn stat(reader: &StorePorts, snap: &Snap, path: &str) -> Answer {