Skip to content

Commit 89aa11e

Browse files
committed
fix(assets): remove unused delete_content param from deleteAsset
The delete_content query param on DELETE /api/assets/{id} was introduced in #12125 and had its default flipped to false in #12621. In practice no client sends it: the frontend issues a bare DELETE /assets/{id}, so every real caller already gets the default soft-delete (the reference is hidden, content preserved). The only thing that set delete_content=true was this repo's own test teardown. Remove the param from the route and the OpenAPI spec so the contract matches what clients actually use (and lines up with the cloud surface). The route now always soft-deletes. The underlying delete_asset_reference helper keeps its delete_content_if_orphan option, so orphan reclamation remains available internally for a future GC path — it's just no longer exposed on the public endpoint. Tests that used delete_content=true for hard cleanup now soft-delete; test_delete_upon_reference_count asserts content preservation instead of orphan removal.
1 parent 84e0692 commit 89aa11e

5 files changed

Lines changed: 18 additions & 21 deletions

File tree

app/assets/api/routes.py

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -533,18 +533,14 @@ async def update_asset_route(request: web.Request) -> web.Response:
533533
@_require_assets_feature_enabled
534534
async def delete_asset_route(request: web.Request) -> web.Response:
535535
reference_id = str(uuid.UUID(request.match_info["id"]))
536-
delete_content_param = request.query.get("delete_content")
537-
delete_content = (
538-
False
539-
if delete_content_param is None
540-
else delete_content_param.lower() not in {"0", "false", "no"}
541-
)
542536

543537
try:
538+
# Deleting an asset is a soft delete of the reference; the underlying
539+
# content is preserved (it may be shared with other references).
544540
deleted = delete_asset_reference(
545541
reference_id=reference_id,
546542
owner_id=USER_MANAGER.get_request_user_id(request),
547-
delete_content_if_orphan=delete_content,
543+
delete_content_if_orphan=False,
548544
)
549545
except Exception:
550546
logging.exception(

tests-unit/assets_test/conftest.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ def create(name: str, tags: list[str], meta: dict, data: bytes) -> dict:
212212

213213
for aid in created:
214214
with contextlib.suppress(Exception):
215-
http.delete(f"{api_base}/api/assets/{aid}?delete_content=true", timeout=30)
215+
http.delete(f"{api_base}/api/assets/{aid}", timeout=30)
216216

217217

218218
@pytest.fixture
@@ -260,4 +260,4 @@ def autoclean_unit_test_assets(http: requests.Session, api_base: str):
260260
break
261261
for aid in ids:
262262
with contextlib.suppress(Exception):
263-
http.delete(f"{api_base}/api/assets/{aid}?delete_content=true", timeout=30)
263+
http.delete(f"{api_base}/api/assets/{aid}", timeout=30)

tests-unit/assets_test/test_crud.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ def test_get_and_delete_asset(http: requests.Session, api_base: str, seeded_asse
4545
assert "user_metadata" in detail
4646
assert "filename" in detail["user_metadata"]
4747

48-
# DELETE (hard delete to also remove underlying asset and file)
49-
rd = http.delete(f"{api_base}/api/assets/{aid}?delete_content=true", timeout=120)
48+
# DELETE (soft delete; the reference is hidden, content is preserved)
49+
rd = http.delete(f"{api_base}/api/assets/{aid}", timeout=120)
5050
assert rd.status_code == 204
5151

5252
# GET again -> 404
@@ -60,7 +60,7 @@ def test_soft_delete_hides_from_get(http: requests.Session, api_base: str, seede
6060
aid = seeded_asset["id"]
6161
asset_hash = seeded_asset["asset_hash"]
6262

63-
# Soft-delete (default, no delete_content param)
63+
# Soft-delete (delete is always a soft delete)
6464
rd = http.delete(f"{api_base}/api/assets/{aid}", timeout=120)
6565
assert rd.status_code == 204
6666

@@ -81,8 +81,7 @@ def test_soft_delete_hides_from_get(http: requests.Session, api_base: str, seede
8181
ids = [a["id"] for a in rl.json().get("assets", [])]
8282
assert aid not in ids
8383

84-
# Clean up: hard-delete the soft-deleted reference and orphaned asset
85-
http.delete(f"{api_base}/api/assets/{aid}?delete_content=true", timeout=120)
84+
# The reference is already soft-deleted; content is preserved by design.
8685

8786

8887
def test_delete_upon_reference_count(
@@ -119,16 +118,18 @@ def test_delete_upon_reference_count(
119118
rh2 = http.head(f"{api_base}/api/assets/hash/{src_hash}", timeout=120)
120119
assert rh2.status_code == 200 # asset identity preserved (soft delete)
121120

122-
# Re-associate via from-hash, then hard-delete -> orphan content removed
121+
# Re-associate via from-hash (reuses the preserved content), then
122+
# soft-delete -> content is still preserved (delete is always soft).
123123
r3 = http.post(f"{api_base}/api/assets/from-hash", json=payload, timeout=120)
124124
assert r3.status_code == 201, r3.json()
125+
assert r3.json()["created_new"] is False # content survived the soft deletes
125126
aid3 = r3.json()["id"]
126127

127-
rd3 = http.delete(f"{api_base}/api/assets/{aid3}?delete_content=true", timeout=120)
128+
rd3 = http.delete(f"{api_base}/api/assets/{aid3}", timeout=120)
128129
assert rd3.status_code == 204
129130

130131
rh3 = http.head(f"{api_base}/api/assets/hash/{src_hash}", timeout=120)
131-
assert rh3.status_code == 404 # orphan content removed
132+
assert rh3.status_code == 200 # content preserved (soft delete)
132133

133134

134135
def test_update_asset_fields(http: requests.Session, api_base: str, seeded_asset: dict):
@@ -249,7 +250,7 @@ def test_concurrent_delete_same_asset_info_single_204(
249250

250251
# Hit the same endpoint N times in parallel.
251252
n_tests = 4
252-
url = f"{api_base}/api/assets/{aid}?delete_content=false"
253+
url = f"{api_base}/api/assets/{aid}"
253254

254255
def _do_delete(delete_url):
255256
with requests.Session() as s:

tests-unit/assets_test/test_downloads.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ def test_download_missing_file_returns_404(
117117
assert body["error"]["code"] == "FILE_NOT_FOUND"
118118
finally:
119119
# We created asset without the "unit-tests" tag(see `autoclean_unit_test_assets`), we need to clear it manually.
120-
dr = http.delete(f"{api_base}/api/assets/{aid}?delete_content=true", timeout=120)
120+
dr = http.delete(f"{api_base}/api/assets/{aid}", timeout=120)
121121
dr.content
122122

123123

tests-unit/assets_test/test_tags_api.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ def test_tags_empty_usage(http: requests.Session, api_base: str, asset_factory,
6969
used_names = [t["name"] for t in body2["tags"]]
7070
assert custom_tag in used_names
7171

72-
# Hard-delete the asset so the tag usage drops to zero
73-
rd = http.delete(f"{api_base}/api/assets/{_asset['id']}?delete_content=true", timeout=120)
72+
# Delete the asset reference so the tag usage drops to zero
73+
rd = http.delete(f"{api_base}/api/assets/{_asset['id']}", timeout=120)
7474
assert rd.status_code == 204
7575

7676
# Now the custom tag must not be returned when include_zero=false

0 commit comments

Comments
 (0)