diff --git a/CHANGELOG.md b/CHANGELOG.md index d748255..0624111 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## 0.7.3 - 2026-08-09 + +- Coordinate batched component refreshes by revision as well as scope and batch + name. Invalidations for one revision arrive as separate WebSocket messages, so + the microtask merge could not see them all, and each request aborted the one + before it. Only the last component updated. Same-revision requests now run + alongside each other and every frame is applied; only a strictly newer + revision supersedes an in-flight request. Frames already applied at a revision + are not applied twice. + ## 0.7.2 - 2026-08-09 - Render batched component partials as HTML regardless of the request format. diff --git a/Gemfile.lock b/Gemfile.lock index ea1fd79..4c7447d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - solid_objects (0.7.2) + solid_objects (0.7.3) actioncable (>= 8.0) actionpack (>= 8.0) actionview (>= 8.0) @@ -373,7 +373,7 @@ CHECKSUMS rubocop-rails-omakase (1.1.0) sha256=2af73ac8ee5852de2919abbd2618af9c15c19b512c4cfc1f9a5d3b6ef009109d ruby-progressbar (1.13.0) sha256=80fc9c47a9b640d6834e0dc7b3c94c9df37f08cb072b7761e4a71e22cff29b33 securerandom (0.4.1) sha256=cc5193d414a4341b6e225f0cb4446aceca8e50d5e1888743fac16987638ea0b1 - solid_objects (0.7.2) + solid_objects (0.7.3) sqlite3 (2.9.5-aarch64-linux-gnu) sha256=78075b6337d3d182c6d2b4691049ed45cd220826160c9ea18946bf6a1de200dc sqlite3 (2.9.5-aarch64-linux-musl) sha256=18c801185deb4adc01ddb281e8f672a39e3d1729979ca91e39439cd3eac0402d sqlite3 (2.9.5-arm-linux-gnu) sha256=1bdfca0c7d63998c60b0f4a8e3c8df2d33800ccc4abd2d612eddbbbc92a4c48b diff --git a/app/assets/javascripts/solid_objects/component_batch_refresh.js b/app/assets/javascripts/solid_objects/component_batch_refresh.js index 816e599..0cdb26d 100644 --- a/app/assets/javascripts/solid_objects/component_batch_refresh.js +++ b/app/assets/javascripts/solid_objects/component_batch_refresh.js @@ -1,5 +1,7 @@ const pendingBatches = new Map() const activeBatches = new Map() +const appliedRevisions = new Map() +let requestSequence = 0 class SolidObjectsBatchRefreshElement extends HTMLElement { connectedCallback() { @@ -34,18 +36,26 @@ class SolidObjectsBatchRefreshElement extends HTMLElement { pendingBatches.set(key, merged) queueMicrotask(() => { pendingBatches.delete(key) - requestBatch(group, batch, merged.sources) + requestBatch(group, batch, revision, merged.sources) }) this.remove() } } -async function requestBatch(group, batch, sources) { - const previous = activeBatches.get(group) - previous?.abort() - +// Invalidations for one revision arrive in separate WebSocket messages, so the +// microtask merge cannot see them all. Requests are tracked per revision and a +// request is only cancelled by a strictly newer one; same-revision requests run +// alongside each other and every frame is applied. +async function requestBatch(group, batch, revision, sources) { + const parsed = parseRevision(revision) + supersedeOlderRequests(group, parsed) + + // Same-revision requests run concurrently, so each needs its own entry. + // Sharing one key per revision would leave all but the last untracked and + // therefore impossible to supersede. + const key = `${group}:${revision}:${(requestSequence += 1)}` const controller = new AbortController() - activeBatches.set(group, controller) + activeBatches.set(key, { controller, group, revision: parsed }) try { const url = mergedUrl(sources) @@ -68,10 +78,29 @@ async function requestBatch(group, batch, sources) { } catch (error) { if (error.name !== "AbortError") dispatchBatchError(batch, "request_failed") } finally { - if (activeBatches.get(group) === controller) activeBatches.delete(group) + if (activeBatches.get(key)?.controller === controller) activeBatches.delete(key) } } +function supersedeOlderRequests(group, revision) { + if (!revision) return + + activeBatches.forEach((entry, key) => { + if (entry.group !== group) return + if (!olderRevision(entry.revision, revision)) return + + entry.controller.abort() + activeBatches.delete(key) + }) +} + +function olderRevision(candidate, current) { + if (!candidate || !current) return false + + return candidate[0] < current[0] || + (candidate[0] === current[0] && candidate[1] < current[1]) +} + // Every notification for one batch and revision carries the same endpoint and // differs only by which components changed, so the union of their tokens is the // complete set to render. @@ -93,6 +122,13 @@ function applyFrame(frame) { const target = document.getElementById(frame?.target) if (!target || !frame.html) return if (!newerRevision(frame.revision, target.dataset.solidObjectsRevision)) return + // Concurrent same-revision responses can carry the same frame. The target's + // own revision only advances once Turbo applies the stream, so what has + // already been applied is tracked here as well. + const applied = appliedRevisions.get(frame.target) + if (applied && !newerRevision(frame.revision, applied)) return + + appliedRevisions.set(frame.target, frame.revision) const parsed = new DOMParser().parseFromString(frame.html, "text/html") const replacement = parsed.getElementById(frame.target) diff --git a/docs/realtime.md b/docs/realtime.md index 09fe015..5be7673 100644 --- a/docs/realtime.md +++ b/docs/realtime.md @@ -173,8 +173,11 @@ they are while giving the client a documented contract with per-frame revisions. ### What the protocol guarantees Only components whose dependencies changed are requested; the rest are never -named in the batch. Duplicate notifications for the same batch and revision merge -into one request, and a superseded request for the same batch is aborted. Each +named in the batch. Notifications for the same batch and revision that arrive in +one task merge into a single request. Notifications that arrive in separate +WebSocket messages issue their own requests and all of their frames are applied, +because cancelling a same-revision request would drop the components it carried. +Only a strictly newer revision supersedes an in-flight request. Each frame carries its own revision and cannot overwrite a target that already holds a newer one. Authorization is unchanged: every component in the batch passes the same `authorize_query` boundary an individual refresh uses, and the batch name is diff --git a/lib/solid_objects/version.rb b/lib/solid_objects/version.rb index 64a53f7..ffe386a 100644 --- a/lib/solid_objects/version.rb +++ b/lib/solid_objects/version.rb @@ -1,5 +1,5 @@ # rbs_inline: enabled module SolidObjects - VERSION = "0.7.2" + VERSION = "0.7.3" end diff --git a/test/javascript/component_batch_refresh.test.mjs b/test/javascript/component_batch_refresh.test.mjs index e0af153..4ebfe2a 100644 --- a/test/javascript/component_batch_refresh.test.mjs +++ b/test/javascript/component_batch_refresh.test.mjs @@ -3,43 +3,80 @@ import { test, before, beforeEach } from "node:test" import { openDocument, loadModule, nextTick } from "./browser_test_helper.mjs" const ENDPOINT = "/solid_objects/components/batch" +const ORIGIN = "https://example.test/" let requests +let aborted let responder +let recordedErrors let defaultScope -let scopeCount = 0 +let run = 0 before(async () => { const dom = openDocument() - dom.window.fetch = async (url) => { + // Honour the abort signal, otherwise cancelled requests still resolve and no + // test can observe cancellation. + dom.window.fetch = async (url, options = {}) => { requests.push(url.toString()) - return responder(url) + const signal = options.signal + if (signal?.aborted) throw abortError() + + return Promise.race([ + responder(url), + new Promise((_resolve, reject) => { + signal?.addEventListener("abort", () => { + aborted.push(url.toString()) + reject(abortError()) + }) + }) + ]) } globalThis.fetch = dom.window.fetch + // Registered once; re-registering per test would record each error N times. + dom.window.document.addEventListener( + "solid-objects:batch-refresh-error", + (event) => recordedErrors.push(event.detail.reason) + ) await loadModule("component_batch_refresh.js") }) beforeEach(() => { document.body.innerHTML = "" - // Applied frames are appended to documentElement, outside body. document.querySelectorAll("turbo-stream").forEach((stream) => stream.remove()) requests = [] - responder = async () => jsonResponse({ frames: [] }) - defaultScope = scopeNamed(`solid-objects-scope-${(scopeCount += 1)}`) + aborted = [] + recordedErrors = [] + // Scope and target identity are page-lifetime state in the module, so each + // test needs its own names. + run += 1 + defaultScope = scopeNamed(id("scope")) }) +function id(name) { + return `${name}-${run}` +} + +function abortError() { + const error = new Error("aborted") + error.name = "AbortError" + return error +} + function jsonResponse(body, { ok = true, status = 200 } = {}) { - return { - ok, - status, - json: async () => body - } + return { ok, status, json: async () => body } } function frameHtml(target, revision) { return `fresh` } +function scopeNamed(name) { + const scope = document.createElement("div") + scope.id = name + document.body.append(scope) + return scope +} + function addTarget(target, revision) { const frame = document.createElement("turbo-frame") frame.id = target @@ -49,164 +86,246 @@ function addTarget(target, revision) { return frame } -function scopeNamed(id) { - const scope = document.createElement("div") - scope.id = id - document.body.append(scope) - return scope -} - -function notify({ batch, revision, targets, tokens, scope }) { +function notify({ revision, tokens, scope, batch = "playmat", queryRevision = "9" }) { const element = document.createElement("solid-objects-batch-refresh") element.dataset.batch = batch element.dataset.revision = revision - element.dataset.targets = targets.join(" ") + element.dataset.targets = tokens.join(" ") const query = tokens.map((token) => `tokens[]=${token}`).join("&") - element.dataset.source = `${ENDPOINT}?instance_id=1&revision=9&${query}` + element.dataset.source = `${ENDPOINT}?instance_id=1&revision=${queryRevision}&${query}` ;(scope ?? defaultScope).append(element) return element } -test("three components changing together issue one request", async () => { - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) - notify({ batch: "playmat", revision: "1:9", targets: [ "controls" ], tokens: [ "t2" ] }) - notify({ batch: "playmat", revision: "1:9", targets: [ "library" ], tokens: [ "t3" ] }) - await nextTick() +function appliedTargets() { + return [ ...document.querySelectorAll("turbo-stream") ].map( + (stream) => stream.getAttribute("target") + ) +} - assert.equal(requests.length, 1) -}) +function tokensIn(request) { + return new URL(request, ORIGIN).searchParams.getAll("tokens[]") +} -test("the merged request asks for every changed component", async () => { - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) - notify({ batch: "playmat", revision: "1:9", targets: [ "controls" ], tokens: [ "t2" ] }) - await nextTick() +// Answers each request with a frame per requested token, after a delay so that +// responses are genuinely in flight when the next notification arrives. +function respondPerToken({ revision = "1:9", delay = 15 } = {}) { + responder = async (url) => { + await new Promise((resolve) => setTimeout(resolve, delay)) + return jsonResponse({ + frames: tokensIn(url).map((token) => ({ + target: token, + revision, + html: frameHtml(token, revision) + })) + }) + } +} - const url = new URL(requests[0], "https://example.test/") - assert.deepEqual(url.searchParams.getAll("tokens[]").sort(), [ "t1", "t2" ]) -}) +function settle(milliseconds = 80) { + return new Promise((resolve) => setTimeout(resolve, milliseconds)) +} -test("a token requested twice is only sent once", async () => { - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) - await nextTick() +test("three same-revision invalidations in separate tasks all get applied", async () => { + const targets = [ id("player"), id("controls"), id("library") ] + targets.forEach((target) => addTarget(target, "1:8")) + respondPerToken() + + for (const target of targets) { + notify({ revision: "1:9", tokens: [ target ] }) + await nextTick() + } + await settle() - const url = new URL(requests[0], "https://example.test/") - assert.deepEqual(url.searchParams.getAll("tokens[]"), [ "t1" ]) + assert.deepEqual(appliedTargets().sort(), [ ...targets ].sort()) }) -test("separate batches issue separate requests", async () => { - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) - notify({ batch: "sidebar", revision: "1:9", targets: [ "chat" ], tokens: [ "t2" ] }) +test("a same-revision request does not abort another same-revision request", async () => { + respondPerToken({ delay: 20 }) + + notify({ revision: "1:9", tokens: [ id("player") ] }) + await nextTick() + notify({ revision: "1:9", tokens: [ id("controls") ] }) await nextTick() + await settle() assert.equal(requests.length, 2) + assert.deepEqual(recordedErrors, []) }) -test("a later revision issues its own request", async () => { - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) - await nextTick() - notify({ batch: "playmat", revision: "1:10", targets: [ "player" ], tokens: [ "t1" ] }) +test("a newer revision supersedes an in-flight older request", async () => { + const target = id("player") + addTarget(target, "1:8") + responder = async (url) => { + const queryRevision = new URL(url, ORIGIN).searchParams.get("revision") + await new Promise((resolve) => setTimeout(resolve, 25)) + return jsonResponse({ + frames: [ { + target, + revision: `1:${queryRevision}`, + html: frameHtml(target, `1:${queryRevision}`) + } ] + }) + } + + notify({ revision: "1:9", tokens: [ target ], queryRevision: "9" }) await nextTick() + notify({ revision: "1:10", tokens: [ target ], queryRevision: "10" }) + await settle(120) - assert.equal(requests.length, 2) + assert.deepEqual(appliedTargets(), [ target ]) }) -test("applies a newer frame to its target", async () => { - addTarget("player", "1:8") +test("an older frame cannot overwrite a newer target", async () => { + const target = id("player") + addTarget(target, "1:12") responder = async () => jsonResponse({ - frames: [ { target: "player", revision: "1:9", html: frameHtml("player", "1:9") } ] + frames: [ { target, revision: "1:9", html: frameHtml(target, "1:9") } ] }) - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) - await nextTick() - const stream = document.querySelector("turbo-stream[target='player']") - assert.ok(stream, "expected a turbo-stream to be emitted for the target") + notify({ revision: "1:9", tokens: [ target ] }) + await settle(40) + + assert.deepEqual(appliedTargets(), []) }) -test("a stale frame cannot overwrite a newer target", async () => { - addTarget("player", "1:12") - responder = async () => jsonResponse({ - frames: [ { target: "player", revision: "1:9", html: frameHtml("player", "1:9") } ] - }) - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) +test("a duplicate frame is applied only once", async () => { + const target = id("player") + addTarget(target, "1:8") + respondPerToken({ delay: 5 }) + + notify({ revision: "1:9", tokens: [ target ] }) await nextTick() + notify({ revision: "1:9", tokens: [ target ] }) + await settle() - assert.equal(document.querySelector("turbo-stream[target='player']"), null) + assert.deepEqual(appliedTargets(), [ target ]) +}) + +test("notifications merged in one task issue a single request", async () => { + respondPerToken({ delay: 5 }) + + notify({ revision: "1:9", tokens: [ id("player") ] }) + notify({ revision: "1:9", tokens: [ id("controls") ] }) + notify({ revision: "1:9", tokens: [ id("library") ] }) + await settle() + + assert.equal(requests.length, 1) +}) + +test("duplicate tokens are sent only once", async () => { + respondPerToken({ delay: 5 }) + + notify({ revision: "1:9", tokens: [ id("player") ] }) + notify({ revision: "1:9", tokens: [ id("player") ] }) + await settle() + + assert.deepEqual(tokensIn(requests[0]), [ id("player") ]) +}) + +test("different scopes stay isolated", async () => { + const other = scopeNamed(id("other-scope")) + respondPerToken({ delay: 20 }) + + notify({ revision: "1:9", tokens: [ id("player") ] }) + notify({ revision: "1:9", tokens: [ id("chat") ], scope: other }) + await settle() + + assert.equal(requests.length, 2) + assert.deepEqual(recordedErrors, []) +}) + +test("different batch names stay independent", async () => { + respondPerToken({ delay: 20 }) + + notify({ revision: "1:9", tokens: [ id("player") ] }) + notify({ revision: "1:9", tokens: [ id("chat") ], batch: "sidebar" }) + await settle() + + assert.equal(requests.length, 2) }) test("a frame for an absent target is ignored", async () => { responder = async () => jsonResponse({ - frames: [ { target: "missing", revision: "1:9", html: frameHtml("missing", "1:9") } ] + frames: [ { target: id("missing"), revision: "1:9", html: frameHtml(id("missing"), "1:9") } ] }) - notify({ batch: "playmat", revision: "1:9", targets: [ "missing" ], tokens: [ "t1" ] }) - await nextTick() - assert.equal(document.querySelector("turbo-stream"), null) + notify({ revision: "1:9", tokens: [ id("missing") ] }) + await settle(40) + + assert.deepEqual(appliedTargets(), []) }) test("reports a failed response", async () => { - const errors = [] - document.addEventListener("solid-objects:batch-refresh-error", (event) => { - errors.push(event.detail.reason) - }) responder = async () => jsonResponse({}, { ok: false, status: 403 }) - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) - await nextTick() - assert.deepEqual(errors, [ "http_403" ]) + notify({ revision: "1:9", tokens: [ id("player") ] }) + await settle(40) + + assert.deepEqual(recordedErrors, [ "http_403" ]) }) test("reports a malformed response", async () => { - const errors = [] - document.addEventListener("solid-objects:batch-refresh-error", (event) => { - errors.push(event.detail.reason) - }) responder = async () => jsonResponse({ frames: "nope" }) - notify({ batch: "playmat", revision: "1:9", targets: [ "player" ], tokens: [ "t1" ] }) + + notify({ revision: "1:9", tokens: [ id("player") ] }) + await settle(40) + + assert.deepEqual(recordedErrors, [ "invalid_response" ]) +}) + +test("a superseded request reports no error", async () => { + respondPerToken({ delay: 30 }) + + notify({ revision: "1:9", tokens: [ id("player") ] }) await nextTick() + notify({ revision: "1:10", tokens: [ id("player") ], queryRevision: "10" }) + await settle(120) - assert.deepEqual(errors, [ "invalid_response" ]) + assert.deepEqual(recordedErrors, []) }) test("the notification element removes itself", async () => { - const element = notify({ - batch: "playmat", - revision: "1:9", - targets: [ "player" ], - tokens: [ "t1" ] - }) - await nextTick() + const element = notify({ revision: "1:9", tokens: [ id("player") ] }) + await settle(40) assert.equal(element.isConnected, false) }) -test("two scopes sharing a batch name do not merge requests", async () => { - const first = scopeNamed("solid-objects-scope-table-1") - const second = scopeNamed("solid-objects-scope-table-2") - notify({ batch: "playmat", revision: "1:9", targets: [ "a" ], tokens: [ "t1" ], scope: first }) - notify({ batch: "playmat", revision: "1:9", targets: [ "b" ], tokens: [ "t2" ], scope: second }) - await nextTick() - - assert.equal(requests.length, 2) - const tokens = requests.map( - (request) => new URL(request, "https://example.test/").searchParams.getAll("tokens[]") - ) - assert.deepEqual(tokens, [ [ "t1" ], [ "t2" ] ]) +test("a cross-origin source is refused", async () => { + const element = document.createElement("solid-objects-batch-refresh") + element.dataset.batch = "playmat" + element.dataset.revision = "1:9" + element.dataset.targets = id("player") + element.dataset.source = `https://elsewhere.test${ENDPOINT}?tokens[]=t1` + defaultScope.append(element) + await settle(40) + + assert.equal(requests.length, 0) }) -test("one scope does not abort another scope's request", async () => { - const first = scopeNamed("solid-objects-scope-table-3") - const second = scopeNamed("solid-objects-scope-table-4") - const aborted = [] +test("a newer revision supersedes every concurrent older request", async () => { + const targets = [ id("player"), id("controls") ] + targets.forEach((target) => addTarget(target, "1:8")) + respondPerToken({ delay: 40 }) + + notify({ revision: "1:9", tokens: [ targets[0] ] }) + await nextTick() + notify({ revision: "1:9", tokens: [ targets[1] ] }) + await nextTick() responder = async (url) => { await new Promise((resolve) => setTimeout(resolve, 5)) - return jsonResponse({ frames: [] }) + return jsonResponse({ + frames: tokensIn(url).map((token) => ({ + target: token, + revision: "1:10", + html: frameHtml(token, "1:10") + })) + }) } - notify({ batch: "playmat", revision: "1:9", targets: [ "a" ], tokens: [ "t1" ], scope: first }) - await nextTick() - notify({ batch: "playmat", revision: "1:10", targets: [ "b" ], tokens: [ "t2" ], scope: second }) - await new Promise((resolve) => setTimeout(resolve, 20)) + notify({ revision: "1:10", tokens: [ targets[0] ], queryRevision: "10" }) + await settle(150) - assert.equal(requests.length, 2) - assert.deepEqual(aborted, []) + assert.equal(aborted.length, 2, "both older same-revision requests should be superseded") + assert.deepEqual(recordedErrors, []) })