zlei6/ComfyUI
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI.git synced 2026-08-16 22:46:38 +08:00
Code Issues Packages Projects Releases Wiki Activity

Simplify api docs module and tests

Browse Source 
Cleanup pass over the previous commit. No behaviour change except the
cache header noted below.

app/api_docs.py (111 -> 59 lines):

- Cut the rationale comments. The file was ~32% commentary against 2-6%
  in neighbouring app/ modules, and most of it argued decisions that
  belong in the PR rather than the source, including a TODO explaining
  why an SRI hash could not be computed.
- Inline the spec and bundle URLs into the HTML. The __TOKEN__ replace()
  machinery existed only to hoist two string literals, and then needed a
  comment defending its own existence.
- Drop the hand-rolled <style> block for two inline style attributes.
- Drop the os.path.isfile guard. FileResponse already answers a missing
  file with a 404, so the check was a second stat for the same result.

- Serve the spec with Cache-Control: no-cache rather than no-store.
  Both mean "never use a stale copy", but no-store also forbids storing
  it, so every docs page load re-transferred all 230 KB. no-cache lets
  the browser revalidate against the ETag FileResponse already sets; an
  unchanged spec now costs a 304 instead of a full download.

tests-unit/server_test/test_api_docs.py (11 tests -> 6, 144 -> 70 lines):

- Drop the local copy of server.py's /api prefix loop and the two tests
  that depended on it. They exercised the copy, not the real loop, so
  they would have stayed green through a change to the thing they
  claimed to protect. What actually makes prefixing work is that the
  spec URL is relative, which is now asserted directly.
- Drop the static-catch-all test. It asserted aiohttp's own route
  precedence against a synthetic app, and could not fail if the
  registration call moved after the catch-all in server.py.
- Drop the tautological SPEC_PATH assertion, already covered by fetching
  the spec through the route.
- Loosen the fallback assertions, which pinned the exact quoting and
  inline-handler style of the HTML.

Claude-Session: https://claude.ai/code/session_01BvUveU9ofyGrSz3QxYeecB
This commit is contained in:
Claude
2026-08-14 23:49:26 +00:00
parent 91ec18778a
commit ec820735c7
2 changed files with 50 additions and 176 deletions
Download Patch File Download Diff File Expand all files Collapse all files

106
app/api_docs.py
View File

@@ -1,111 +1,59 @@
"""Browsable API documentation for the ComfyUI HTTP API.
"""Serves openapi.yaml and renders it as browsable API docs.
Renders the repository's ``openapi.yaml`` with Redoc. Redoc is used rather than
Swagger UI because it has no request-execution feature at all: the local server
is unauthenticated by default, so a docs page that could fire requests would
give one-click access to destructive endpoints such as ``/api/interrupt``,
``/api/free`` and ``DELETE /api/userdata/{file}``.
The viewer bundle is loaded from a CDN, so the page needs outbound network
access to render. Installs without it still get the raw spec from
``/openapi.yaml``; the fallback notice below points there.
Uses Redoc, not Swagger UI: the local server is unauthenticated by default, so
the viewer must not be able to execute requests.
"""
import os
from aiohttp import web
# openapi.yaml lives next to server.py at the repository root. Resolve it from
# __file__ rather than the cwd: ComfyUI is routinely launched from other
# directories and through wrappers, and a relative path would 404 unpredictably.
SPEC_PATH = os.path.join(
os.path.dirname(os.path.dirname(os.path.realpath(__file__))), "openapi.yaml"
)
# Pinned to an exact version rather than a floating tag so a CDN-side release
# cannot change what this page executes.
#
# TODO: add an integrity="sha384-..." attribute. The pinned path is immutable,
# so SRI is worth having; the hash simply could not be computed where this was
# written (no outbound network), and a wrong hash fails the page closed.
REDOC_BUNDLE_URL = "https://cdn.jsdelivr.net/npm/redoc@2.5.0/bundles/redoc.standalone.js"
# The spec URL is deliberately relative. add_routes() re-registers every route
# under an /api prefix, so this page is reachable at both /api-docs and
# /api/api-docs; a relative URL resolves to the sibling spec in either case.
SPEC_URL = "openapi.yaml"
# Substituted with str.replace rather than str.format/f-string so the CSS braces
# below stay literal and a future style edit does not have to double them.
# The spec URL is relative so it resolves from both /api-docs and /api/api-docs.
# The viewer is pinned and loaded from a CDN, so the page needs network access;
# if it fails to load, the fallback below points at the locally served spec.
API_DOCS_HTML = """<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<title>ComfyUI API Reference</title>
<style>
body { margin: 0; padding: 0; font-family: system-ui, sans-serif; }
#fallback {
display: none;
margin: 3rem auto;
max-width: 40rem;
padding: 0 1.5rem;
line-height: 1.6;
color: #333;
}
#fallback code {
background: #f2f2f2;
border-radius: 3px;
padding: 0.1em 0.35em;
}
</style>
</head>
<body>
<redoc spec-url="__SPEC_URL__"></redoc>
<div id="fallback">
<h1>API docs viewer unavailable</h1>
<p>
The documentation viewer is loaded from a CDN and could not be reached.
This is expected on an offline or air-gapped install.
</p>
<p>
The specification itself is served locally and needs no network access:
<a href="__SPEC_URL__">openapi.yaml</a>. Render it with any local viewer,
for example <code>npx @redocly/cli preview-docs openapi.yaml</code>.
</p>
<body style="margin: 0">
<redoc spec-url="openapi.yaml"></redoc>
<div id="fallback" style="display: none; margin: 3rem; line-height: 1.6">
The API docs viewer is loaded from a CDN and could not be reached. The
specification itself is served locally: <a href="openapi.yaml">openapi.yaml</a>
</div>
<script
src="__BUNDLE_URL__"
onerror="document.getElementById('fallback').style.display='block';"
src="https://cdn.jsdelivr.net/npm/redoc@2.5.0/bundles/redoc.standalone.js"
onerror="document.getElementById('fallback').style.display='block'"
></script>
</body>
</html>
""".replace("__SPEC_URL__", SPEC_URL).replace("__BUNDLE_URL__", REDOC_BUNDLE_URL)
"""
def add_api_docs_routes(routes: web.RouteTableDef) -> None:
"""Register the spec and docs-page routes on the given route table.
Registering on PromptServer's route table (rather than on the app directly)
matters twice over: the table is added before the ``web.static('/')``
catch-all that would otherwise shadow these paths, and every route in it is
also re-registered under an ``/api`` prefix.
"""
"""Register /openapi.yaml and /api-docs on the given route table."""
@routes.get("/openapi.yaml")
async def get_openapi_spec(request):
if not os.path.isfile(SPEC_PATH):
return web.Response(status=404, text="openapi.yaml not found")
response = web.FileResponse(SPEC_PATH)
response.headers["Content-Type"] = "application/yaml"
# The cache_control middleware only special-cases js/css/images, so a
# .yaml response falls through untouched. Without this, a user editing
# the spec would keep getting a stale copy from the browser cache.
response.headers["Cache-Control"] = "no-store, must-revalidate"
return response
# The spec is edited in place during development, so never let the
# browser hold a stale copy. no-cache still allows a 304 via the ETag
# FileResponse sets, which matters for a ~230 KB file.
return web.FileResponse(SPEC_PATH, headers={
"Content-Type": "application/yaml",
"Cache-Control": "no-cache",
})
@routes.get("/api-docs")
async def get_api_docs(request):
response = web.Response(text=API_DOCS_HTML, content_type="text/html")
response.headers["Cache-Control"] = "no-store, must-revalidate"
return response
return web.Response(
text=API_DOCS_HTML,
content_type="text/html",
headers={"Cache-Control": "no-cache"},
)

120
tests-unit/server_test/test_api_docs.py
View File

@@ -1,143 +1,69 @@
"""Tests for the OpenAPI spec and API docs routes."""
import os
import pytest
import pytest_asyncio
import yaml
from aiohttp import web
from app.api_docs import SPEC_PATH, SPEC_URL, add_api_docs_routes
from app.api_docs import add_api_docs_routes
pytestmark = pytest.mark.asyncio
def _build_app():
"""Mirror how PromptServer mounts these routes, including the /api prefix.
add_routes() walks the route table and re-registers every RouteDef under an
/api prefix, so both the bare and prefixed forms are served. Reproducing
that here keeps the prefix behaviour covered by tests.
"""
@pytest_asyncio.fixture
async def client(aiohttp_client):
app = web.Application()
routes = web.RouteTableDef()
add_api_docs_routes(routes)
api_routes = web.RouteTableDef()
for route in routes:
if isinstance(route, web.RouteDef):
api_routes.route(route.method, "/api" + route.path)(
route.handler, **route.kwargs
)
app.add_routes(api_routes)
app.add_routes(routes)
return app
return await aiohttp_client(app)
@pytest_asyncio.fixture
async def client(aiohttp_client):
return await aiohttp_client(_build_app())
async def test_spec_path_points_at_the_repo_spec():
"""SPEC_PATH must resolve from __file__, not the cwd."""
assert os.path.isfile(SPEC_PATH)
assert os.path.basename(SPEC_PATH) == "openapi.yaml"
async def test_get_spec_returns_yaml(client):
async def test_get_spec(client):
resp = await client.get("/openapi.yaml")
assert resp.status == 200
assert resp.headers["Content-Type"] == "application/yaml"
assert resp.headers["Cache-Control"] == "no-store, must-revalidate"
async def test_spec_body_is_valid_openapi_3(client):
resp = await client.get("/openapi.yaml")
assert resp.headers["Cache-Control"] == "no-cache"
spec = yaml.safe_load(await resp.text())
assert spec["openapi"].startswith("3.")
assert spec["paths"]
async def test_spec_is_also_served_under_the_api_prefix(client):
"""Documents the prefix duplication so a refactor cannot silently break it."""
resp = await client.get("/api/openapi.yaml")
assert resp.status == 200
assert resp.headers["Content-Type"] == "application/yaml"
async def test_missing_spec_returns_404(client, monkeypatch):
monkeypatch.setattr("app.api_docs.SPEC_PATH", "/nonexistent/openapi.yaml")
resp = await client.get("/openapi.yaml")
assert resp.status == 404
async def test_docs_page_returns_html_referencing_the_spec(client):
async def test_docs_page(client):
resp = await client.get("/api-docs")
assert resp.status == 200
assert resp.content_type == "text/html"
body = await resp.text()
assert SPEC_URL in body
async def test_docs_page_spec_url_is_relative(client):
"""A relative URL resolves correctly from both /api-docs and /api/api-docs."""
assert not SPEC_URL.startswith("/")
resp = await client.get("/api/api-docs")
assert resp.status == 200
async def test_docs_page_cannot_execute_requests(client):
"""The local server is unauthenticated, so the docs UI must not fire requests.
Redoc has no request execution at all. Guard against a swap to Swagger UI,
whose "Try it out" would give one-click access to destructive endpoints.
"""
body = (await (await client.get("/api-docs")).text()).lower()
# Redoc has no request execution; Swagger UI's "Try it out" would give
# one-click access to destructive endpoints on an unauthenticated server.
assert "<redoc" in body
assert "swagger" not in body
assert "openapi.yaml" in body
async def test_docs_page_degrades_when_the_cdn_is_unreachable(client):
"""Offline installs must get the fallback notice, not a blank page.
async def test_docs_page_has_offline_fallback(client):
"""Offline installs should get a notice, not a blank page."""
body = await (await client.get("/api-docs")).text()
assert 'id="fallback"' in body
The viewer bundle is the only part that needs network, so the page carries
an onerror hook that reveals a notice pointing at the locally served spec.
async def test_spec_url_is_relative(client):
"""What makes the page work under both /api-docs and /api/api-docs.
server.py re-registers every route under an /api prefix; a relative spec
URL resolves to the sibling spec from either mount point.
"""
body = await (await client.get("/api-docs")).text()
assert 'onerror=' in body
assert "getElementById('fallback')" in body
assert 'id="fallback"' in body
# The notice has to link the spec, which is served locally.
assert f'<a href="{SPEC_URL}">' in body
assert 'spec-url="openapi.yaml"' in body
async def test_routes_survive_the_static_catch_all(aiohttp_client, tmp_path):
"""web.static('/') is registered last and matches everything.
Registering on PromptServer's route table is what keeps these paths
reachable; this fails if they are ever moved after the catch-all.
"""
(tmp_path / "index.html").write_text("frontend")
(tmp_path / "docs").mkdir()
(tmp_path / "docs" / "node.json").write_text("{}")
app = _build_app()
app.add_routes([web.static("/docs", tmp_path / "docs")])
app.add_routes([web.static("/", tmp_path)])
client = await aiohttp_client(app)
assert (await client.get("/openapi.yaml")).headers["Content-Type"] == (
"application/yaml"
)
assert (await client.get("/api-docs")).content_type == "text/html"
# Embedded node docs and the frontend bundle are untouched.
assert await (await client.get("/docs/node.json")).text() == "{}"
assert await (await client.get("/index.html")).text() == "frontend"
async def test_api_docs_flag_is_off_by_default():
"""Both routes are gated on this flag in PromptServer.add_routes()."""
async def test_flag_is_off_by_default():
"""Serving the API surface of an unauthenticated server is opt-in."""
from comfy.cli_args import parser
assert parser.parse_args([]).enable_api_docs is False