Skip to content

Commit 2ff6431

Browse files
committed
fix(SUP-2586): do not return annotations if not asked
1 parent 07bf0cc commit 2ff6431

File tree

2 files changed

+110
-18
lines changed

2 files changed

+110
-18
lines changed

src/kili/adapters/kili_api_gateway/asset/operations_mixin.py

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ def list_assets(
6161
"LLM_STATIC",
6262
"GEOSPATIAL",
6363
}:
64-
fetch_annotations = not (has_labels_url or has_latest_label_url)
64+
fetch_annotations = ("labels.jsonResponse" in fields and not has_labels_url) or (
65+
"latestLabel.jsonResponse" in fields and not has_latest_label_url
66+
)
6567
yield from self.list_assets_split(
6668
filters, fields, options, project_info, fetch_annotations
6769
)
@@ -110,7 +112,11 @@ def list_assets_split(
110112
{inner_annotation_fragment}
111113
}}
112114
"""
113-
static_fragments = {"labels": annotation_fragment, "latestLabel": annotation_fragment}
115+
static_fragments = {}
116+
if "labels.jsonResponse" in fields and "labels.jsonResponseUrl" not in fields:
117+
static_fragments["labels"] = annotation_fragment
118+
if "latestLabel.jsonResponse" in fields and "latestLabel.jsonResponseUrl" not in fields:
119+
static_fragments["latestLabel"] = annotation_fragment
114120

115121
required_fields = {"content", "jsonContent", "resolution.width", "resolution.height"}
116122
fields = list(fields)
@@ -133,24 +139,33 @@ def list_assets_split(
133139
json_interface=project_info["jsonInterface"],
134140
project_input_type=project_info["inputType"],
135141
)
136-
is_requesting_annotations = any("annotations." in element for element in fields)
137-
for asset in assets_gen:
138-
if "latestLabel.jsonResponse" in fields and asset.get("latestLabel"):
139-
converter.patch_label_json_response(
140-
asset, asset["latestLabel"], asset["latestLabel"]["annotations"]
141-
)
142-
if not is_requesting_annotations:
143-
asset["latestLabel"].pop("annotations")
144-
145-
if "labels.jsonResponse" in fields:
146-
for label in asset.get("labels", []):
147-
converter.patch_label_json_response(asset, label, label["annotations"])
148-
if not is_requesting_annotations:
149-
label.pop("annotations")
150-
yield asset
142+
yield from self._reconstruct_json_responses(assets_gen, fields, converter)
151143
else:
152144
yield from assets_gen
153145

146+
def _reconstruct_json_responses(
147+
self,
148+
assets_gen: Generator[dict, None, None],
149+
fields: ListOrTuple[str],
150+
converter: AnnotationsToJsonResponseConverter,
151+
) -> Generator[dict, None, None]:
152+
"""Reconstruct jsonResponse from annotations, then remove annotations from the result."""
153+
is_requesting_annotations = any("annotations." in element for element in fields)
154+
for asset in assets_gen:
155+
if "latestLabel.jsonResponse" in fields and asset.get("latestLabel"):
156+
converter.patch_label_json_response(
157+
asset, asset["latestLabel"], asset["latestLabel"]["annotations"]
158+
)
159+
if not is_requesting_annotations:
160+
asset["latestLabel"].pop("annotations")
161+
162+
if "labels.jsonResponse" in fields:
163+
for label in asset.get("labels", []):
164+
converter.patch_label_json_response(asset, label, label["annotations"])
165+
if not is_requesting_annotations:
166+
label.pop("annotations")
167+
yield asset
168+
154169
def count_assets(self, filters: AssetFilters) -> int:
155170
"""Send a GraphQL request calling countIssues resolver."""
156171
where = asset_where_mapper(filters)

tests/integration/adapters/kili_api_gateway/test_asset.py

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,88 @@
44
from kili.adapters.kili_api_gateway.helpers.queries import QueryOptions
55
from kili.domain.asset import AssetFilters
66
from kili.domain.project import ProjectId
7+
from tests.unit.adapters.kili_api_gateway.label.test_data import test_case_1
8+
9+
10+
def test_given_video_project_only_latest_label_json_response_requested_annotations_not_fetched_for_labels(
11+
graphql_client, http_client
12+
):
13+
"""For VIDEO projects, when only latestLabel.jsonResponse is requested (not labels.jsonResponse).
14+
15+
The annotations fragment should only be injected into latestLabel in the GraphQL query,
16+
not into labels.
17+
"""
18+
captured_queries = []
19+
20+
def mock_graphql_execute(query, *_args, **_kwargs) -> dict:
21+
captured_queries.append(query)
22+
23+
if "countAssetAnnotations" in query:
24+
return {"data": 1}
25+
26+
if "query countAssets" in query:
27+
return {"data": 1}
28+
29+
if "projects(" in query:
30+
return {
31+
"data": [
32+
{
33+
"id": "project_id",
34+
"inputType": "VIDEO",
35+
"jsonInterface": test_case_1.json_interface,
36+
}
37+
]
38+
}
39+
40+
if "query assets" in query:
41+
return {
42+
"data": [
43+
{
44+
"id": "fake_asset_id",
45+
"labels": [{"id": "label_id"}],
46+
"latestLabel": {
47+
"jsonResponse": "{}",
48+
"annotations": test_case_1.annotations,
49+
},
50+
"content": "",
51+
"jsonContent": "",
52+
"resolution": {"width": 100, "height": 100},
53+
}
54+
]
55+
}
56+
57+
return None
58+
59+
graphql_client.execute.side_effect = mock_graphql_execute
60+
61+
asset_operations = AssetOperationMixin()
62+
asset_operations.graphql_client = graphql_client
63+
asset_operations.http_client = http_client
64+
filters = AssetFilters(project_id=ProjectId("project_id"))
65+
fields = ["id", "labels.id", "latestLabel.jsonResponse"]
66+
67+
# when
68+
assets = list(
69+
asset_operations.list_assets(filters, fields, options=QueryOptions(disable_tqdm=None))
70+
)
71+
72+
# then
73+
assert len(assets) == 1
74+
assert assets[0]["id"] == "fake_asset_id"
75+
assert isinstance(assets[0]["latestLabel"]["jsonResponse"], dict)
76+
# annotations were fetched and used to reconstruct jsonResponse, then removed from the result
77+
assert "annotations" not in assets[0]["latestLabel"]
78+
assert "annotations" not in assets[0]["labels"][0]
79+
80+
# then: the GraphQL query only fetches annotations for latestLabel, not for labels
81+
assets_query = next(q for q in captured_queries if "query assets" in q)
82+
assert "annotations" in assets_query # latestLabel has the annotations fragment
83+
assert "labels{ id}" in assets_query # labels block only has id, no annotations
784

885

986
def test_given_a_query_returning_serialized_json_it_parses_json_fields(graphql_client, http_client):
1087
# mocking
11-
def mock_graphql_execute(query, variables, **kwargs):
88+
def mock_graphql_execute(query, variables, **kwargs) -> dict | None:
1289
if "query assets" in query:
1390
label = {"jsonResponse": "{}"}
1491
return {

0 commit comments

Comments
 (0)