Compare commits

..
Author SHA1 Message Date
Vito Sansevero 6342d7235d test(collections): change sources only after the page's selected-file read is answered
The details panel reads its selected file as soon as the page lists. When the
test's source change landed before that read was answered, the page found the
file again on its own (409 -> rescope -> relist), so the card under the old
numbering the test was about to click was gone. Every test in the spec now
waits for that read before changing sources.

Closes #484
2026-10-29 10:47:23 -07:00
4 changed files with 29 additions and 206 deletions
+13 -26
View File
@@ -40,7 +40,6 @@ from aiohttp import web
from PIL import Image
from .prompts import PromptRoutesMixin
from .revalidation import revalidated_response
from .pairs import PairRoutesMixin
from .library import LibraryRoutesMixin
from .vocabulary import VocabularyRoutesMixin
@@ -654,39 +653,35 @@ class PromptManagerAPI(
@routes.get("/prompt_manager/workspace")
async def serve_workspace_ui(request: web.Request) -> web.Response:
return self._serve_html_page(request, "workspace.html", "Workspace")
return self._serve_html_page("workspace.html", "Workspace")
@routes.get("/prompt_manager/web")
async def serve_web_ui(request):
return self._serve_html_page(request, "index.html", "Web UI", cache=False)
return self._serve_html_page("index.html", "Web UI", cache=False)
@routes.get("/prompt_manager/gallery.html")
async def serve_gallery_ui(request):
return self._serve_html_page(request, "metadata.html", "Gallery")
return self._serve_html_page("metadata.html", "Gallery")
@routes.get("/prompt_manager/admin")
async def serve_admin_ui(request):
return self._serve_html_page(request, "admin.html", "Admin UI")
return self._serve_html_page("admin.html", "Admin UI")
@routes.get("/prompt_manager/gallery")
async def serve_gallery_admin_ui(request):
return self._serve_html_page(request, "gallery.html", "Gallery")
return self._serve_html_page("gallery.html", "Gallery")
# ── Static file serving ───────────────────────────────────────
@routes.get("/prompt_manager/lib/{filepath:.*}")
async def serve_lib_static(request):
"""Serve static library files (JS, CSS) from web/lib directory."""
return self._serve_static_file(
request, "lib", request.match_info.get("filepath")
)
return self._serve_static_file("lib", request.match_info.get("filepath"))
@routes.get("/prompt_manager/js/{filepath:.*}")
async def serve_js_static(request):
"""Serve static JavaScript files from web/js directory."""
return self._serve_static_file(
request, "js", request.match_info.get("filepath")
)
return self._serve_static_file("js", request.match_info.get("filepath"))
# ── Register domain-specific routes from mixins ───────────────
@@ -731,11 +726,8 @@ class PromptManagerAPI(
self.logger.info("All routes registered with decorator pattern")
def _serve_static_file(self, request, subdir, filepath):
"""Serve ``web/<subdir>/<filepath>``; 403 outside it, 404 when missing.
A served file is revalidated on every reuse (#478, ``revalidation``).
"""
def _serve_static_file(self, subdir, filepath):
"""Serve ``web/<subdir>/<filepath>``; 403 outside it, 404 when missing."""
root = os.path.join(_get_project_root(), "web", subdir)
exact = STATIC_EXACT_FILES.get((subdir, filepath))
# An exact file passes the same containment check, its extension only.
@@ -750,13 +742,10 @@ class PromptManagerAPI(
return web.Response(text="Not Found", status=404)
with open(file_path, "rb") as f:
content = f.read()
return revalidated_response(request, content, content_type)
return web.Response(body=content, content_type=content_type)
def _serve_html_page(self, request, filename, title, cache=True):
"""Serve ``web/<filename>``; 404 when missing, 500 when unreadable.
A served page is revalidated on every reuse (#478, ``revalidation``).
"""
def _serve_html_page(self, filename, title, cache=True):
"""Serve ``web/<filename>``; 404 when missing, 500 when unreadable."""
html_path = os.path.join(_get_project_root(), "web", filename)
try:
if cache and html_path in self._html_cache:
@@ -773,9 +762,7 @@ class PromptManagerAPI(
content_type="text/html",
status=404,
)
return revalidated_response(
request, content.encode("utf-8"), "text/html", charset="utf-8"
)
return web.Response(text=content, content_type="text/html", charset="utf-8")
except Exception:
self.logger.exception(f"Failed to load {title}")
return web.Response(
-47
View File
@@ -1,47 +0,0 @@
"""Revalidated responses for the pages and static files PromptManager serves.
``Cache-Control: no-cache`` lets the browser keep a copy but makes it ask the
server before every reuse, so a PromptManager update reaches the browser on a
normal reload (#478). The ETag lets an unchanged file answer 304 without a
body, which keeps that reload cheap.
The tag hashes the bytes rather than the file's mtime and size: an update can
keep both (an archive extract, a same-length edit within one clock tick), and
the handlers read the whole file anyway. It is weak because the gzip
middleware may send the same content in a different encoding.
"""
import hashlib
from aiohttp import web
REVALIDATE = "no-cache"
def entity_tag(body: bytes) -> str:
"""Weak ETag for ``body``."""
return f'W/"{hashlib.sha256(body).hexdigest()[:32]}"'
def etag_matches(if_none_match, etag: str) -> bool:
"""True when an If-None-Match header names ``etag`` (weak comparison)."""
if not if_none_match:
return False
if if_none_match.strip() == "*":
return True
opaque = etag.removeprefix("W/")
return any(
candidate.strip().removeprefix("W/") == opaque
for candidate in if_none_match.split(",")
)
def revalidated_response(request, body: bytes, content_type: str, charset=None):
"""200 with ``body``, or 304 when the request already holds this version."""
etag = entity_tag(body)
headers = {"Cache-Control": REVALIDATE, "ETag": etag}
if etag_matches(request.headers.get("If-None-Match"), etag):
return web.Response(status=304, headers=headers)
return web.Response(
body=body, content_type=content_type, charset=charset, headers=headers
)
+16 -7
View File
@@ -37,6 +37,15 @@ test.afterEach(async ({ page, request, browserName }) => {
await expect(await request.post('/__test__/tagging/reset', { data: {} })).toBeOK();
});
async function openSettled(page, url) {
// The page reads its selected file's details as soon as it lists. A source change before that read is answered
// makes the page find the file again on its own, so the test's old-numbering locators are gone before they are
// used (#484): every test changes sources only after the read was answered.
const inspected = page.waitForResponse(item => new URL(item.url()).pathname.startsWith('/prompt_manager/images/inspect/') &&
item.request().method() === 'GET');
await page.goto(url);
expect((await inspected).status()).toBe(200);
}
async function openTag(page, path = FIRST, root = 0) {
const observed = answered(page, '/image-annotations/observe');
await tagButton(page, path, root).click();
@@ -53,7 +62,7 @@ async function savedTags(request, path, root) {
}
test('renumbered sources under an open Tag dialog: it finds the file again, keeps the draft and Generate works', async ({ page, request }) => {
await page.goto(images);
await openSettled(page, images);
await expect(card(page, FIRST)).toHaveClass(/\bselected\b/);
expect((await openTag(page)).status()).toBe(200);
await manual(page).fill('kept_draft');
@@ -91,7 +100,7 @@ test('renumbered sources under an open Tag dialog: it finds the file again, keep
});
test('the inspector finds a renumbered file again; checked files from the old numbering are cleared', async ({ page, request }) => {
await page.goto(images);
await openSettled(page, images);
await expect(card(page, FIRST)).toHaveClass(/\bselected\b/);
await card(page, FIRST).getByRole('checkbox', { name: /^Select / }).check();
await expect(page.locator('.bulkbar')).toBeVisible();
@@ -113,7 +122,7 @@ test('the inspector finds a renumbered file again; checked files from the old nu
});
test('a scope that cannot be mapped: Generate and Save are visibly disabled, the reason and Refresh sit beside them', async ({ page }) => {
await page.goto(images);
await openSettled(page, images);
await expect(card(page, FIRST)).toBeVisible();
await expect(await page.request.post('/__test__/scope-key/rotate')).toBeOK();
const rescoped = answered(page, '/images/rescope');
@@ -157,7 +166,7 @@ test('a scope that cannot be mapped: Generate and Save are visibly disabled, the
});
test('an unmappable inspector read keeps its visible Refresh, and Refresh recovers the panel', async ({ page }) => {
await page.goto(images);
await openSettled(page, images);
await expect(card(page, FIRST)).toHaveClass(/\bselected\b/);
await expect(await page.request.post('/__test__/scope-key/rotate')).toBeOK();
const rescoped = answered(page, '/images/rescope');
@@ -173,7 +182,7 @@ test('an unmappable inspector read keeps its visible Refresh, and Refresh recove
test('a Load more cursor the server cannot verify offers Refresh instead of a dead Retry', async ({ page, request }) => {
await expect(await request.post('/__test__/tagging/reset', { data: { extra_images: 60 } })).toBeOK();
await page.goto(images);
await openSettled(page, images);
await expect(cards(page).first()).toBeVisible();
await expect(page.locator('#gallery-continuation')).toContainText('more load as you scroll');
await expect(await request.post('/__test__/scope-key/rotate')).toBeOK();
@@ -191,7 +200,7 @@ test('a Load more cursor the server cannot verify offers Refresh instead of a de
});
test('a viewer whose media source was renumbered closes onto the file, kept selected', async ({ page, request }) => {
await page.goto(images);
await openSettled(page, images);
await expect(card(page, FIRST)).toHaveClass(/\bselected\b/);
await expect(await request.post('/__test__/sources/reverse')).toBeOK();
const rescoped = answered(page, '/images/rescope');
@@ -217,7 +226,7 @@ test('a collection page finds a renumbered member again and stays on its members
await expect(added).toBeOK();
const collections = (await (await request.get('/prompt_manager/collections')).json()).collections;
const { id } = collections.find(item => item.name === name);
await page.goto(`/prompt_manager/workspace#/collection/${id}`);
await openSettled(page, `/prompt_manager/workspace#/collection/${id}`);
await expect(cards(page)).toHaveCount(1);
await expect(card(page, FIRST)).toHaveClass(/\bselected\b/);
await expect(await request.post('/__test__/sources/reverse')).toBeOK();
-126
View File
@@ -684,132 +684,6 @@ class TestStaticRoutes(CoreRoutesTestCase):
self.assertEqual(resp.status, 403)
class TestRevalidatedCaching(CoreRoutesTestCase):
"""Every served page and static file is revalidated on reload (#478).
``Cache-Control: no-cache`` makes the browser ask before it reuses a copy,
and the ETag lets an unchanged file answer 304 without a body, so an update
reaches the browser on a normal reload and an unchanged reload stays cheap.
"""
STATIC_PATHS = (
"/prompt_manager/js/workspace/index.mjs",
"/prompt_manager/js/admin.js",
"/prompt_manager/js/inspection-semantics.json",
"/prompt_manager/lib/viewerjs/viewer.min.css",
)
async def _assert_revalidated(self, path):
resp = await self.client.request("GET", path)
self.assertEqual(resp.status, 200, path)
self.assertEqual(resp.headers.get("Cache-Control"), "no-cache", path)
etag = resp.headers.get("ETag")
self.assertTrue(etag, path)
body = await resp.read()
self.assertTrue(body, path)
again = await self.client.request("GET", path, headers={"If-None-Match": etag})
self.assertEqual(again.status, 304, path)
self.assertEqual(again.headers.get("ETag"), etag, path)
self.assertEqual(again.headers.get("Cache-Control"), "no-cache", path)
self.assertEqual(await again.read(), b"", path)
stale = await self.client.request(
"GET", path, headers={"If-None-Match": 'W/"stale"'}
)
self.assertEqual(stale.status, 200, path)
self.assertEqual(await stale.read(), body, path)
return etag
async def test_static_files_are_revalidated(self):
for path in self.STATIC_PATHS:
await self._assert_revalidated(path)
async def test_pages_are_revalidated(self):
for path in UI_PATHS:
await self._assert_revalidated(path)
async def test_etag_matches_in_a_list_and_as_wildcard(self):
path = "/prompt_manager/js/workspace/index.mjs"
etag = (await self.client.request("GET", path)).headers["ETag"]
for header in (f'W/"other", {etag}', etag.removeprefix("W/"), "*"):
resp = await self.client.request(
"GET", path, headers={"If-None-Match": header}
)
self.assertEqual(resp.status, 304, header)
async def test_changed_file_gets_a_new_etag_and_its_new_body(self):
web_dir = self._fake_root()
module = web_dir / "js" / "m.mjs"
module.write_text("export const v = 1;")
path = "/prompt_manager/js/m.mjs"
first = await self.client.request("GET", path)
etag = first.headers["ETag"]
# Same size and, on coarse clocks, the same mtime: only the bytes differ.
module.write_text("export const v = 2;")
resp = await self.client.request("GET", path, headers={"If-None-Match": etag})
self.assertEqual(resp.status, 200)
self.assertNotEqual(resp.headers["ETag"], etag)
self.assertEqual(await resp.text(), "export const v = 2;")
async def test_gzipped_and_plain_bodies_share_a_weak_etag(self):
"""The gzip middleware changes the bytes, so the tag must be weak."""
resp = await self.client.request("GET", "/prompt_manager/js/admin.js")
self.assertTrue(resp.headers["ETag"].startswith('W/"'))
async def test_errors_carry_no_validator(self):
resp = await self.client.request("GET", "/prompt_manager/js/nope.js")
self.assertEqual(resp.status, 404)
self.assertNotIn("ETag", resp.headers)
@web.middleware
async def _comfyui_cache_control(request, handler):
"""ComfyUI's own middleware (middleware/cache_middleware.py), JS/CSS branch.
It only fills a missing Cache-Control, and ``.mjs`` is not ``.js``: that is
why the workspace modules went out with no header at all before #478.
"""
response = await handler(request)
if request.path.endswith(".js") or request.path.endswith(".css"):
response.headers.setdefault("Cache-Control", "no-store")
return response
class TestRevalidatedCachingBehindMiddlewares(CoreRoutesTestCase):
"""The host's middlewares keep the header and the 304 intact."""
async def get_application(self):
app = await super().get_application()
app.middlewares.append(_comfyui_cache_control)
app.middlewares.append(api_module._gzip_middleware)
return app
async def test_gzipped_files_keep_no_cache_and_answer_304(self):
for path in (
"/prompt_manager/js/workspace/index.mjs",
"/prompt_manager/js/admin.js",
"/prompt_manager/lib/viewerjs/viewer.min.css",
"/prompt_manager/workspace",
):
resp = await self.client.request(
"GET", path, headers={"Accept-Encoding": "gzip"}
)
self.assertEqual(resp.status, 200, path)
self.assertEqual(resp.headers.get("Content-Encoding"), "gzip", path)
self.assertEqual(resp.headers["Cache-Control"], "no-cache", path)
again = await self.client.request(
"GET",
path,
headers={
"Accept-Encoding": "gzip",
"If-None-Match": resp.headers["ETag"],
},
)
self.assertEqual(again.status, 304, path)
self.assertEqual(again.headers["Cache-Control"], "no-cache", path)
# ── instance helpers ──────────────────────────────────────────────────