From 964a6a27282d19ac84f1949a7cee97da7b4f4556 Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Fri, 22 Oct 2021 15:47:34 -0400 Subject: [PATCH 1/4] Check in failing tests --- server/dive_utils/models.py | 14 ++++++++++++++ .../tests/integration/test_dataset_operations.py | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/server/dive_utils/models.py b/server/dive_utils/models.py index 0fd636fd5..13d5a53f4 100644 --- a/server/dive_utils/models.py +++ b/server/dive_utils/models.py @@ -1,6 +1,7 @@ from typing import Any, Dict, List, Optional, Tuple, Union from pydantic import BaseModel, Field, validator +from pydantic.class_validators import root_validator from typing_extensions import Literal @@ -95,6 +96,19 @@ class MetadataMutable(BaseModel): confidenceFilters: Optional[Dict[str, float]] attributes: Optional[Dict[str, Attribute]] + # @root_validator + # def validate_at_least_some_field(cls, value): + # assert any( + # [ + # key in value + # for key in [ + # 'customTypeStyling', + # 'confidenceFilters', + # 'attributes', + # ] + # ] + # ), 'At least one configuration value must be set' + class GirderMetadataStatic(MetadataMutable): # Required diff --git a/server/tests/integration/test_dataset_operations.py b/server/tests/integration/test_dataset_operations.py index 56f7c3a92..c7fe6e70c 100644 --- a/server/tests/integration/test_dataset_operations.py +++ b/server/tests/integration/test_dataset_operations.py @@ -79,6 +79,20 @@ def test_set_configuration(user: dict): assert new_meta['confidenceFilters']['another'] == 0.6 +@pytest.mark.integration +@pytest.mark.parametrize("user", users.values()) +@pytest.mark.run(order=5) +def test_upload_json_detections(user: dict): + client = getClient(user['login']) + privateFolder = getTestFolder(client) + for dataset in client.listFolder(privateFolder['_id']): + client.uploadFileToFolder(dataset['_id'], '../testutils/28-tracks.json') + old_tracks = client.get(f'dive_annotation?folderId={dataset["_id"]}') + client.post(f'dive_rpc/postprocess/{dataset["_id"]}', data={"skipJobs": True}) + new_tracks = client.get(f'dive_annotation?folderId={dataset["_id"]}') + assert new_tracks.keys() != old_tracks.keys(), "Tracks should have updated" + + @pytest.mark.integration @pytest.mark.parametrize("user", users.values()) @pytest.mark.run(order=5) From af42fb92023c225d8ce7beb1d1554b68d33d8ba4 Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Fri, 22 Oct 2021 16:26:27 -0400 Subject: [PATCH 2/4] Add better validation for configuration upload file --- server/dive_utils/models.py | 26 ++++++++++--------- .../integration/test_dataset_download.py | 22 ++++++++++++++++ .../integration/test_dataset_operations.py | 14 ---------- testutils/tracks.json | 1 + 4 files changed, 37 insertions(+), 26 deletions(-) create mode 100644 testutils/tracks.json diff --git a/server/dive_utils/models.py b/server/dive_utils/models.py index 13d5a53f4..ef9ba964b 100644 --- a/server/dive_utils/models.py +++ b/server/dive_utils/models.py @@ -96,18 +96,20 @@ class MetadataMutable(BaseModel): confidenceFilters: Optional[Dict[str, float]] attributes: Optional[Dict[str, Attribute]] - # @root_validator - # def validate_at_least_some_field(cls, value): - # assert any( - # [ - # key in value - # for key in [ - # 'customTypeStyling', - # 'confidenceFilters', - # 'attributes', - # ] - # ] - # ), 'At least one configuration value must be set' + @root_validator + @classmethod + def validate_at_least_some_field(cls, value): + assert any( + [ + value.get(key, None) + for key in [ + 'customTypeStyling', + 'confidenceFilters', + 'attributes', + ] + ] + ), 'At least one configuration value must be set' + return value class GirderMetadataStatic(MetadataMutable): diff --git a/server/tests/integration/test_dataset_download.py b/server/tests/integration/test_dataset_download.py index 6758642b1..545e5e81c 100644 --- a/server/tests/integration/test_dataset_download.py +++ b/server/tests/integration/test_dataset_download.py @@ -36,3 +36,25 @@ def test_download_csv(user: dict): if not row.startswith('#'): track_set.add(row.split(',')[0]) assert len(track_set) == expected[0]['trackCount'] + + +@pytest.mark.integration +@pytest.mark.parametrize("user", users.values()) +@pytest.mark.run(order=7) +def test_upload_json_detections(user: dict): + """ + Upload new annotations, and verify that the existing annotations change. + This test cleans up after itself, and does not change the state of the system + for future tests + """ + client = getClient(user['login']) + privateFolder = getTestFolder(client) + for dataset in client.listFolder(privateFolder['_id']): + old_tracks = client.get(f'dive_annotation?folderId={dataset["_id"]}') + newfile = client.uploadFileToFolder(dataset['_id'], '../testutils/tracks.json') + client.post(f'dive_rpc/postprocess/{dataset["_id"]}', data={"skipJobs": True}) + new_tracks = client.get(f'dive_annotation?folderId={dataset["_id"]}') + assert '999999' not in old_tracks, "Tracks should have updated" + assert '999999' in new_tracks, "Should have one track, 999999" + assert len(new_tracks.keys()) == 1, "Should have a single track" + client.delete(f'item/{newfile["itemId"]}') # Remove the uploaded detections diff --git a/server/tests/integration/test_dataset_operations.py b/server/tests/integration/test_dataset_operations.py index c7fe6e70c..56f7c3a92 100644 --- a/server/tests/integration/test_dataset_operations.py +++ b/server/tests/integration/test_dataset_operations.py @@ -79,20 +79,6 @@ def test_set_configuration(user: dict): assert new_meta['confidenceFilters']['another'] == 0.6 -@pytest.mark.integration -@pytest.mark.parametrize("user", users.values()) -@pytest.mark.run(order=5) -def test_upload_json_detections(user: dict): - client = getClient(user['login']) - privateFolder = getTestFolder(client) - for dataset in client.listFolder(privateFolder['_id']): - client.uploadFileToFolder(dataset['_id'], '../testutils/28-tracks.json') - old_tracks = client.get(f'dive_annotation?folderId={dataset["_id"]}') - client.post(f'dive_rpc/postprocess/{dataset["_id"]}', data={"skipJobs": True}) - new_tracks = client.get(f'dive_annotation?folderId={dataset["_id"]}') - assert new_tracks.keys() != old_tracks.keys(), "Tracks should have updated" - - @pytest.mark.integration @pytest.mark.parametrize("user", users.values()) @pytest.mark.run(order=5) diff --git a/testutils/tracks.json b/testutils/tracks.json new file mode 100644 index 000000000..eb1de7e4c --- /dev/null +++ b/testutils/tracks.json @@ -0,0 +1 @@ +{"999999": {"begin": 0, "end": 0, "trackId": 0, "features": [{"frame": 0, "bounds": [670, 183, 968, 433], "interpolate": false, "keyframe": true}], "confidencePairs": [["motion_signature", 0.996269]], "attributes": {}}} From 23a18bab732c868d6f3742041fc065837eff8f0e Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Mon, 1 Nov 2021 10:21:28 -0400 Subject: [PATCH 3/4] Fix json upload --- server/dive_server/crud.py | 9 ++++----- server/dive_utils/models.py | 16 ++++++++-------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/server/dive_server/crud.py b/server/dive_server/crud.py index c9beef6fe..6c69a22e5 100644 --- a/server/dive_server/crud.py +++ b/server/dive_server/crud.py @@ -139,12 +139,11 @@ def get_data_by_type( raise RestException('No array-type json objects are supported') if kwcoco.is_coco_json(data_dict): as_type = FileType.COCO_JSON + elif models.MetadataMutable.is_dive_configuration(data_dict): + data_dict = models.MetadataMutable(**data_dict).dict(exclude_none=True) + as_type = FileType.DIVE_CONF else: - try: - data_dict = models.MetadataMutable(**data_dict).dict(exclude_none=True) - as_type = FileType.DIVE_CONF - except pydantic.ValidationError: - as_type = FileType.DIVE_JSON + as_type = FileType.DIVE_JSON else: raise RestException('Got file of unknown and unusable type') diff --git a/server/dive_utils/models.py b/server/dive_utils/models.py index ef9ba964b..ac940901a 100644 --- a/server/dive_utils/models.py +++ b/server/dive_utils/models.py @@ -1,7 +1,6 @@ from typing import Any, Dict, List, Optional, Tuple, Union from pydantic import BaseModel, Field, validator -from pydantic.class_validators import root_validator from typing_extensions import Literal @@ -96,20 +95,21 @@ class MetadataMutable(BaseModel): confidenceFilters: Optional[Dict[str, float]] attributes: Optional[Dict[str, Attribute]] - @root_validator - @classmethod - def validate_at_least_some_field(cls, value): - assert any( + @staticmethod + def is_dive_configuration(value: dict): + """ + Check if value is a configuration file if at lease one of the config options is populated + """ + return any( [ - value.get(key, None) + value.get(key, False) for key in [ 'customTypeStyling', 'confidenceFilters', 'attributes', ] ] - ), 'At least one configuration value must be set' - return value + ) class GirderMetadataStatic(MetadataMutable): From 1c3833482ae31ad22f31f1b3c45788a523683e4f Mon Sep 17 00:00:00 2001 From: Brandon Davis Date: Tue, 2 Nov 2021 09:52:04 -0400 Subject: [PATCH 4/4] Change to using pydantic schema to get model keys --- server/dive_utils/models.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/server/dive_utils/models.py b/server/dive_utils/models.py index ac940901a..32b189987 100644 --- a/server/dive_utils/models.py +++ b/server/dive_utils/models.py @@ -100,16 +100,13 @@ def is_dive_configuration(value: dict): """ Check if value is a configuration file if at lease one of the config options is populated """ - return any( - [ - value.get(key, False) - for key in [ - 'customTypeStyling', - 'confidenceFilters', - 'attributes', - ] - ] - ) + keys = list(MetadataMutable.schema()['properties'].keys()) + + # Remove version: its appearance is not enough to indicate that + # the value is actually a configuration object. + keys.remove("version") + + return any([value.get(key, False) for key in keys]) class GirderMetadataStatic(MetadataMutable):