Emit `hash` alongside `asset_hash` on all Asset responses (#13739) · code/app-python-comfyui@6de7fc0 · GitHub
Skip to content

Commit 6de7fc0

Browse files
mattmilleraimillermediaguill
authored
Emit hash alongside asset_hash on all Asset responses (Comfy-Org#13739)
* Emit `hash` alongside `asset_hash` on all Asset responses Add a `hash` field to the Asset response schema that carries the same value as the existing `asset_hash` field. Both fields are now populated in _build_asset_response, so every Asset-returning endpoint (GET, POST, PUT) includes both. No existing fields are removed. Tests updated to assert both fields. Co-authored-by: Matt Miller <MillerMedia@users.noreply.github.com> * Tighten hash field tests and DRY response builder - Extract assert_hash_fields_consistent() helper that verifies presence parity and value equality, replacing body.get()-based assertions that treated missing keys and explicit nulls identically. - Conftest seeded_asset fixture and seed-asset list assertions now check key absence directly, so a regression that surfaces null fields would be caught (validates exclude_none behavior). - DRY duplicate hash expression in _build_asset_response. - Add list-endpoint coverage asserting hash is present and consistent on populated assets. - Add schema-level test asserting AssetCreated inherits the hash field from Asset, guarding against future inheritance drift. --------- Co-authored-by: Matt Miller <MillerMedia@users.noreply.github.com> Co-authored-by: guill <jacob.e.segal@gmail.com>
1 parent a4141a0 commit 6de7fc0

8 files changed

Lines changed: 67 additions & 3 deletions

File tree

‎app/assets/api/routes.py‎

Lines changed: 3 additions & 1 deletion

‎app/assets/api/schemas_out.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ class Asset(BaseModel):
1010

1111
id: str
1212
name: str
13+
hash: str | None = None
1314
asset_hash: str | None = None
1415
size: int | None = None
1516
mime_type: str | None = None

‎tests-unit/assets_test/conftest.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,8 @@ def seeded_asset(request: pytest.FixtureRequest, http: requests.Session, api_bas
236236
r = http.post(api_base + "/api/assets", files=files, data=form_data, timeout=120)
237237
body = r.json()
238238
assert r.status_code == 201, body
239+
from helpers import assert_hash_fields_consistent
240+
assert_hash_fields_consistent(body)
239241
return body
240242

241243

‎tests-unit/assets_test/helpers.py‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,26 @@ def trigger_sync_seed_assets(session: requests.Session, base_url: str) -> None:
2626

2727
def get_asset_filename(asset_hash: str, extension: str) -> str:
2828
return asset_hash.removeprefix("blake3:") + extension
29+
30+
31+
def assert_hash_fields_consistent(body: dict, expected_hash: str | None = None) -> None:
32+
"""Assert hash and asset_hash invariants on an Asset response.
33+
34+
Both must be present or both absent (so a regression that drops only one
35+
is caught). When present, they must equal each other and, if expected_hash
36+
is provided, must equal that value.
37+
"""
38+
hash_present = "hash" in body
39+
asset_hash_present = "asset_hash" in body
40+
assert hash_present == asset_hash_present, (
41+
f"hash and asset_hash must both be present or both absent: "
42+
f"hash present={hash_present}, asset_hash present={asset_hash_present}"
43+
)
44+
if hash_present:
45+
h = body["hash"]
46+
ah = body["asset_hash"]
47+
assert h == ah, f"hash and asset_hash must match: hash={h!r}, asset_hash={ah!r}"
48+
if expected_hash is not None:
49+
assert h == expected_hash, (
50+
f"hash must equal expected: got {h!r}, expected {expected_hash!r}"
51+
)

‎tests-unit/assets_test/test_assets_missing_sync.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ def test_seed_asset_removed_when_file_is_deleted(
4040
# there should be exactly one with that name
4141
matches = [a for a in body1.get("assets", []) if a.get("name") == name]
4242
assert matches
43-
assert matches[0].get("asset_hash") is None
43+
# Seed assets have no hash; exclude_none drops both keys from the response
44+
assert "asset_hash" not in matches[0]
45+
assert "hash" not in matches[0]
4446
asset_info_id = matches[0]["id"]
4547

4648
# Remove the underlying file and sync again

‎tests-unit/assets_test/test_crud.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ def test_create_from_hash_success(
2121
b1 = r1.json()
2222
assert r1.status_code == 201, b1
2323
assert b1["asset_hash"] == h
24+
assert b1["hash"] == h
25+
assert b1["hash"] == b1["asset_hash"]
2426
assert b1["created_new"] is False
2527
aid = b1["id"]
2628

@@ -39,6 +41,7 @@ def test_get_and_delete_asset(http: requests.Session, api_base: str, seeded_asse
3941
detail = rg.json()
4042
assert rg.status_code == 200, detail
4143
assert detail["id"] == aid
44+
assert detail["hash"] == detail["asset_hash"]
4245
assert "user_metadata" in detail
4346
assert "filename" in detail["user_metadata"]
4447

@@ -97,6 +100,7 @@ def test_delete_upon_reference_count(
97100
copy = r2.json()
98101
assert r2.status_code == 201, copy
99102
assert copy["asset_hash"] == src_hash
103+
assert copy["hash"] == src_hash
100104
assert copy["created_new"] is False
101105

102106
# Soft-delete original reference (default) -> asset identity must remain
@@ -139,6 +143,7 @@ def test_update_asset_fields(http: requests.Session, api_base: str, seeded_asset
139143
body = ru.json()
140144
assert ru.status_code == 200, body
141145
assert body["name"] == payload["name"]
146+
assert body["hash"] == body["asset_hash"]
142147
assert body["tags"] == original_tags # tags unchanged
143148
assert body["user_metadata"]["purpose"] == "updated"
144149
# filename should still be present and normalized by server
@@ -289,7 +294,9 @@ def test_metadata_filename_is_set_for_seed_asset_without_hash(
289294
assert r1.status_code == 200, body
290295
matches = [a for a in body.get("assets", []) if a.get("name") == name]
291296
assert matches, "Seed asset should be visible after sync"
292-
assert matches[0].get("asset_hash") is None # still a seed
297+
# Seed assets have no hash; exclude_none drops both keys from the response
298+
assert "asset_hash" not in matches[0]
299+
assert "hash" not in matches[0]
293300
aid = matches[0]["id"]
294301

295302
r2 = http.get(f"{api_base}/api/assets/{aid}", timeout=120)

‎tests-unit/assets_test/test_list_filter.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
import pytest
55
import requests
6+
from helpers import assert_hash_fields_consistent
67

78

89
def test_list_assets_paging_and_sort(http: requests.Session, api_base: str, asset_factory, make_asset_bytes):
@@ -26,6 +27,10 @@ def test_list_assets_paging_and_sort(http: requests.Session, api_base: str, asse
2627
got1 = [a["name"] for a in b1["assets"]]
2728
assert got1 == sorted(names)[:2]
2829
assert b1["has_more"] is True
30+
# Populated assets in list responses must carry both `hash` and `asset_hash` consistently
31+
for asset in b1["assets"]:
32+
assert_hash_fields_consistent(asset)
33+
assert "hash" in asset, "populated asset must emit hash on list endpoint"
2934

3035
r2 = http.get(
3136
api_base + "/api/assets",

‎tests-unit/assets_test/test_uploads.py‎

Lines changed: 22 additions & 0 deletions

0 commit comments

Comments
 (0)