mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-05-30 19:07:25 +08:00
Add clear_preview_id flag + validation to UpdateAsset
Replaces #13744 with a narrower change. Decision after review: drop the dedicated PUT/DELETE /api/assets/{id}/preview endpoints in favor of extending the existing PUT /api/assets/{id} route with a clear_preview_id flag, matching the codebase's existing pattern for clearing operations elsewhere in the project (`clear: true` on /api/queue and /api/history in the cloud sibling). Changes: - openapi.yaml: add `clear_preview_id: boolean` to the UpdateAsset request body, documented as taking precedence over `preview_id` when both are set. - schemas_in.UpdateAssetBody: add the field, extend at-least-one-field validation (clear=true counts, clear=false is a no-op), reject the zero UUID as `preview_id`. - services/asset_management.update_asset_metadata: add `clear_preview_id` param; when true, call set_reference_preview with None to clear the link. Clear takes precedence over set when both provided. - routes.update_asset_route: wire the flag through. Reject self-referential preview_id (preview_id == path id) at 400 SELF_REFERENCE before service call. - Tests: cover clear-via-flag happy path, clear-takes-precedence, schema validation edge cases (zero UUID rejected, clear-only valid, false-only rejected).
This commit is contained in:
parent
a8d2519058
commit
54c92d0462
@ -488,6 +488,17 @@ async def update_asset_route(request: web.Request) -> web.Response:
|
||||
400, "INVALID_JSON", "Request body must be valid JSON."
|
||||
)
|
||||
|
||||
# Self-reference rejection. Cheap pre-check at the handler layer since the
|
||||
# path id isn't visible to the pydantic body validator.
|
||||
clear_preview = bool(body.clear_preview_id)
|
||||
if not clear_preview and body.preview_id == reference_id:
|
||||
return _build_error_response(
|
||||
400,
|
||||
"SELF_REFERENCE",
|
||||
"preview_id cannot reference the asset itself.",
|
||||
{"id": reference_id},
|
||||
)
|
||||
|
||||
try:
|
||||
result = update_asset_metadata(
|
||||
reference_id=reference_id,
|
||||
@ -495,6 +506,7 @@ async def update_asset_route(request: web.Request) -> web.Response:
|
||||
user_metadata=body.user_metadata,
|
||||
owner_id=USER_MANAGER.get_request_user_id(request),
|
||||
preview_id=body.preview_id,
|
||||
clear_preview_id=clear_preview,
|
||||
)
|
||||
payload = _build_asset_response(result)
|
||||
except PermissionError as pe:
|
||||
|
||||
@ -101,18 +101,41 @@ class UpdateAssetBody(BaseModel):
|
||||
name: str | None = None
|
||||
user_metadata: dict[str, Any] | None = None
|
||||
preview_id: str | None = None # references an asset_reference id, not an asset id
|
||||
# clear_preview_id matches the `clear` flag convention used by /api/queue
|
||||
# and /api/history. When true, the preview link is cleared; takes precedence
|
||||
# over preview_id if both are provided.
|
||||
clear_preview_id: bool | None = None
|
||||
|
||||
@model_validator(mode="after")
|
||||
def _validate_at_least_one_field(self):
|
||||
if all(
|
||||
v is None
|
||||
for v in (self.name, self.user_metadata, self.preview_id)
|
||||
# clear_preview_id is only meaningful when true; explicit false is a no-op
|
||||
# and shouldn't satisfy the "at least one field" requirement.
|
||||
if (
|
||||
self.name is None
|
||||
and self.user_metadata is None
|
||||
and self.preview_id is None
|
||||
and not self.clear_preview_id
|
||||
):
|
||||
raise ValueError(
|
||||
"Provide at least one of: name, user_metadata, preview_id."
|
||||
"Provide at least one of: name, user_metadata, preview_id, clear_preview_id."
|
||||
)
|
||||
return self
|
||||
|
||||
@model_validator(mode="after")
|
||||
def _validate_preview_id_shape(self):
|
||||
# Skip preview_id semantic checks when the caller is clearing — the
|
||||
# field is ignored in that case.
|
||||
if self.clear_preview_id:
|
||||
return self
|
||||
if self.preview_id is None:
|
||||
return self
|
||||
# Reject zero UUID — would otherwise become a real DB lookup that
|
||||
# returns 404, leaking that the caller fat-fingered the UUID rather
|
||||
# than that the asset doesn't exist.
|
||||
if self.preview_id == "00000000-0000-0000-0000-000000000000":
|
||||
raise ValueError("preview_id must not be the zero UUID.")
|
||||
return self
|
||||
|
||||
|
||||
class CreateFromHashBody(BaseModel):
|
||||
model_config = ConfigDict(extra="ignore", str_strip_whitespace=True)
|
||||
|
||||
@ -71,6 +71,7 @@ def update_asset_metadata(
|
||||
owner_id: str = "",
|
||||
mime_type: str | None = None,
|
||||
preview_id: str | None = None,
|
||||
clear_preview_id: bool = False,
|
||||
) -> AssetDetailResult:
|
||||
with create_session() as session:
|
||||
ref = get_reference_with_owner_check(session, reference_id, owner_id)
|
||||
@ -114,7 +115,16 @@ def update_asset_metadata(
|
||||
if updated:
|
||||
touched = True
|
||||
|
||||
if preview_id is not None:
|
||||
# Clear takes precedence over set when both are provided — matches the
|
||||
# handler-level flag semantics.
|
||||
if clear_preview_id:
|
||||
set_reference_preview(
|
||||
session,
|
||||
reference_id=reference_id,
|
||||
preview_reference_id=None,
|
||||
)
|
||||
touched = True
|
||||
elif preview_id is not None:
|
||||
set_reference_preview(
|
||||
session,
|
||||
reference_id=reference_id,
|
||||
|
||||
@ -1768,7 +1768,13 @@ paths:
|
||||
preview_id:
|
||||
type: string
|
||||
format: uuid
|
||||
description: ID of the asset to use as the preview
|
||||
description: ID of the asset to use as the preview. Ignored when `clear_preview_id` is true.
|
||||
clear_preview_id:
|
||||
type: boolean
|
||||
description: |
|
||||
When true, clears the preview asset link (sets `preview_id` to null).
|
||||
Takes precedence over `preview_id` if both are provided. Matches the
|
||||
`clear` flag convention used by `/api/queue` and `/api/history`.
|
||||
mime_type:
|
||||
type: string
|
||||
nullable: true
|
||||
|
||||
@ -123,6 +123,65 @@ class TestUpdateAssetMetadata:
|
||||
assert updated_ref.user_metadata["key"] == "value"
|
||||
assert updated_ref.user_metadata["num"] == 42
|
||||
|
||||
def test_sets_preview_via_preview_id(self, mock_create_session, session: Session):
|
||||
asset = _make_asset(session)
|
||||
preview_asset = _make_asset(session, hash_val="blake3:preview")
|
||||
ref = _make_reference(session, asset)
|
||||
preview_ref = _make_reference(session, preview_asset, name="preview.png")
|
||||
ref_id = ref.id
|
||||
preview_ref_id = preview_ref.id
|
||||
session.commit()
|
||||
|
||||
update_asset_metadata(
|
||||
reference_id=ref_id,
|
||||
preview_id=preview_ref_id,
|
||||
)
|
||||
|
||||
session.expire_all()
|
||||
updated_ref = session.get(AssetReference, ref_id)
|
||||
assert updated_ref.preview_id == preview_ref_id
|
||||
|
||||
def test_clear_preview_id_clears_the_link(self, mock_create_session, session: Session):
|
||||
asset = _make_asset(session)
|
||||
preview_asset = _make_asset(session, hash_val="blake3:preview")
|
||||
ref = _make_reference(session, asset)
|
||||
preview_ref = _make_reference(session, preview_asset, name="preview.png")
|
||||
ref.preview_id = preview_ref.id
|
||||
ref_id = ref.id
|
||||
session.commit()
|
||||
|
||||
update_asset_metadata(
|
||||
reference_id=ref_id,
|
||||
clear_preview_id=True,
|
||||
)
|
||||
|
||||
session.expire_all()
|
||||
updated_ref = session.get(AssetReference, ref_id)
|
||||
assert updated_ref.preview_id is None
|
||||
|
||||
def test_clear_preview_id_takes_precedence_over_preview_id(
|
||||
self, mock_create_session, session: Session
|
||||
):
|
||||
asset = _make_asset(session)
|
||||
preview_asset = _make_asset(session, hash_val="blake3:preview")
|
||||
ref = _make_reference(session, asset)
|
||||
preview_ref = _make_reference(session, preview_asset, name="preview.png")
|
||||
ref.preview_id = preview_ref.id
|
||||
ref_id = ref.id
|
||||
preview_ref_id = preview_ref.id
|
||||
session.commit()
|
||||
|
||||
# Both flags set — clear wins. preview_id should not be linked.
|
||||
update_asset_metadata(
|
||||
reference_id=ref_id,
|
||||
preview_id=preview_ref_id,
|
||||
clear_preview_id=True,
|
||||
)
|
||||
|
||||
session.expire_all()
|
||||
updated_ref = session.get(AssetReference, ref_id)
|
||||
assert updated_ref.preview_id is None
|
||||
|
||||
def test_raises_for_nonexistent(self, mock_create_session):
|
||||
with pytest.raises(ValueError, match="not found"):
|
||||
update_asset_metadata(reference_id="nonexistent", name="fail")
|
||||
|
||||
Loading…
Reference in New Issue
Block a user