From 9114ff55dc07bd675c251b31d5edd69f97d81c19 Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Thu, 19 Dec 2024 08:52:08 -0500 Subject: [PATCH 01/10] Include the "in" and "not_in" for payload optimization --- shotgun_api3/shotgun.py | 9 ++------- tests/test_unit.py | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 87f68d3e9..58f621323 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -4482,16 +4482,11 @@ def _translate_filters_simple(sg_filter): if ( SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION and condition["path"] != "id" - and condition["relation"] in ["is", "is_not"] + and condition["relation"] in ["is", "is_not", "in", "not_in"] and isinstance(values[0], dict) ): try: - values = [ - { - "type": values[0]["type"], - "id": values[0]["id"], - } - ] + values = [{"type": v["type"], "id": v["id"]} for v in values] except KeyError: pass diff --git a/tests/test_unit.py b/tests/test_unit.py index 096ca9327..896d2a52d 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -434,7 +434,7 @@ def test_related_object(self): result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) - def test_related_object_entity_optimization(self): + def test_related_object_entity_optimization_is(self): filters = [ [ "project", @@ -462,6 +462,41 @@ def test_related_object_entity_optimization(self): result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) + def test_related_object_entity_optimization_in(self): + filters = [ + [ + "project", + "in", + [ + {"foo1": "foo1", "bar1": "bar1", "id": 999, "baz1": "baz1", "type": "Anything"}, + {"foo2": "foo2", "bar2": "bar2", "id": 998, "baz2": "baz2", "type": "Anything"} + ], + ], + ] + expected = { + "logical_operator": "and", + "conditions": [ + { + "path": "project", + "relation": "in", + "values": [ + { + "id": 999, + "type": "Anything", + }, + { + "id": 998, + "type": "Anything", + } + ], + } + ], + } + os.environ["SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION"] = "1" + api.Shotgun("http://server_path", "script_name", "api_key", connect=False) + result = api.shotgun._translate_filters(filters, "all") + self.assertEqual(result, expected) + class TestCerts(unittest.TestCase): # A dummy bad url provided by Amazon From 594a079d643e0960ac6316381b3dbb2a81fbbbcf Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Thu, 19 Dec 2024 15:33:48 -0500 Subject: [PATCH 02/10] Use a decorator to mock environmental variable --- tests/test_unit.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/test_unit.py b/tests/test_unit.py index 896d2a52d..3f2e7593d 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -12,6 +12,7 @@ import os import unittest +from unittest import mock from .mock import patch import shotgun_api3 as api from shotgun_api3.shotgun import _is_mimetypes_broken @@ -434,6 +435,7 @@ def test_related_object(self): result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) + @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) def test_related_object_entity_optimization_is(self): filters = [ [ @@ -457,11 +459,11 @@ def test_related_object_entity_optimization_is(self): } ], } - os.environ["SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION"] = "1" api.Shotgun("http://server_path", "script_name", "api_key", connect=False) result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) + @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) def test_related_object_entity_optimization_in(self): filters = [ [ @@ -492,7 +494,6 @@ def test_related_object_entity_optimization_in(self): } ], } - os.environ["SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION"] = "1" api.Shotgun("http://server_path", "script_name", "api_key", connect=False) result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) From e0e1ecd9b59946ecb1599ee5e93574bf97ab9960 Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Fri, 20 Dec 2024 08:26:43 -0500 Subject: [PATCH 03/10] Add payload optimization for update action --- shotgun_api3/shotgun.py | 34 ++++++++++++++++++++++++++-------- tests/test_unit.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 58f621323..a20f46cc3 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -1132,6 +1132,31 @@ def _add_project_param(self, params, project_entity): params["project"] = project_entity return params + + def _translate_update_params( + self, entity_type, entity_id, data, multi_entity_update_modes + ): + global SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION + + def optimize_value(field_value): + if isinstance(field_value, dict) and SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION: + return {"type": field_value["type"], "id": field_value["id"]} + return field_value + + def optimize_field(field_dict): + return {k: optimize_value(v) for k, v in field_dict.items()} + + full_fields = self._dict_to_list( + data, + extra_data=self._dict_to_extra_data( + multi_entity_update_modes, "multi_entity_update_mode" + ), + ) + return { + "type": entity_type, + "id": entity_id, + "fields": [optimize_field(field_dict) for field_dict in full_fields], + } def summarize(self, entity_type, @@ -1463,14 +1488,7 @@ def update(self, entity_type, entity_id, data, multi_entity_update_modes=None): upload_filmstrip_image = data.pop("filmstrip_image") if data: - params = { - "type": entity_type, - "id": entity_id, - "fields": self._dict_to_list( - data, - extra_data=self._dict_to_extra_data( - multi_entity_update_modes, "multi_entity_update_mode")) - } + params = self._translate_update_params(entity_type, entity_id, data, multi_entity_update_modes) record = self._call_rpc("update", params) result = self._parse_records(record)[0] else: diff --git a/tests/test_unit.py b/tests/test_unit.py index 3f2e7593d..014ca1af4 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -498,6 +498,45 @@ def test_related_object_entity_optimization_in(self): result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) + @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) + def test_related_object_update_entity(self): + entity_type = "Anything" + entity_id = 999 + multi_entity_update_modes = {"project": "set", "name": "set"} + data = { + "name": "test", + "project": { + "foo1": "foo1", + "bar1": "bar1", + "id": 999, + "baz1": "baz1", + "type": "Anything", + }, + } + expected = { + "id": 999, + "type": "Anything", + "fields": [ + { + "field_name": "name", + "value": "test", + "multi_entity_update_mode": "set", + }, + { + "field_name": "project", + "multi_entity_update_mode": "set", + "value": { + # Entity is optimized with type/id fields. + "id": 999, + "type": "Anything", + }, + }, + ], + } + sg = api.Shotgun("http://server_path", "script_name", "api_key", connect=False) + result = sg._translate_update_params(entity_type, entity_id, data, multi_entity_update_modes) + self.assertEqual(result, expected) + class TestCerts(unittest.TestCase): # A dummy bad url provided by Amazon From 8027f0b6039ac383c49f48a006dc690c29009e9c Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Fri, 20 Dec 2024 11:13:21 -0500 Subject: [PATCH 04/10] Change conditional order --- shotgun_api3/shotgun.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index a20f46cc3..7ab6f39bf 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -1139,7 +1139,7 @@ def _translate_update_params( global SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION def optimize_value(field_value): - if isinstance(field_value, dict) and SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION: + if SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION and isinstance(field_value, dict): return {"type": field_value["type"], "id": field_value["id"]} return field_value From 02f67b8239c031a3a47ce487c5037297c3e51bba Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Mon, 23 Dec 2024 14:04:46 -0500 Subject: [PATCH 05/10] Extract function to be reused. Improve testing. --- shotgun_api3/shotgun.py | 25 +++++++------ tests/test_unit.py | 79 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 13 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 7ab6f39bf..78c9f3c40 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -1138,13 +1138,10 @@ def _translate_update_params( ): global SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION - def optimize_value(field_value): - if SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION and isinstance(field_value, dict): - return {"type": field_value["type"], "id": field_value["id"]} - return field_value - def optimize_field(field_dict): - return {k: optimize_value(v) for k, v in field_dict.items()} + if SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION: + return {k: _get_type_and_id_from_value(v) for k, v in field_dict.items()} + return field_dict full_fields = self._dict_to_list( data, @@ -4503,10 +4500,7 @@ def _translate_filters_simple(sg_filter): and condition["relation"] in ["is", "is_not", "in", "not_in"] and isinstance(values[0], dict) ): - try: - values = [{"type": v["type"], "id": v["id"]} for v in values] - except KeyError: - pass + values = [_get_type_and_id_from_value(v) for v in values] condition["values"] = values @@ -4518,3 +4512,14 @@ def _version_str(version): Convert a tuple of int's to a '.' separated str. """ return ".".join(map(str, version)) + + +def _get_type_and_id_from_value(value_dict): + """ + For an entity dictionary, returns a new dictionary with only the type and id keys. + If ant of these keys are not present, the original dictionary is returned. + """ + try: + return {"type": value_dict["type"], "id": value_dict["id"]} + except (KeyError, TypeError): + return value_dict diff --git a/tests/test_unit.py b/tests/test_unit.py index 014ca1af4..435295023 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -463,6 +463,28 @@ def test_related_object_entity_optimization_is(self): result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) + # Now test a non-related object. The expected result should not be optimized. + filters = [ + [ + "something", + "is", + {"foo": "foo", "bar": "bar"}, + ], + ] + expected = { + "logical_operator": "and", + "conditions": [ + { + "path": "something", + "relation": "is", + "values": [{'bar': 'bar', 'foo': 'foo'}], + } + ], + } + api.Shotgun("http://server_path", "script_name", "api_key", connect=False) + result = api.shotgun._translate_filters(filters, "all") + self.assertEqual(result, expected) + @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) def test_related_object_entity_optimization_in(self): filters = [ @@ -471,7 +493,8 @@ def test_related_object_entity_optimization_in(self): "in", [ {"foo1": "foo1", "bar1": "bar1", "id": 999, "baz1": "baz1", "type": "Anything"}, - {"foo2": "foo2", "bar2": "bar2", "id": 998, "baz2": "baz2", "type": "Anything"} + {"foo2": "foo2", "bar2": "bar2", "id": 998, "baz2": "baz2", "type": "Anything"}, + {"foo3": "foo3", "bar3": "bar3"}, ], ], ] @@ -489,6 +512,10 @@ def test_related_object_entity_optimization_in(self): { "id": 998, "type": "Anything", + }, + { + "foo3": "foo3", + "bar3": "bar3", } ], } @@ -498,13 +525,51 @@ def test_related_object_entity_optimization_in(self): result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) - @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) def test_related_object_update_entity(self): entity_type = "Anything" entity_id = 999 - multi_entity_update_modes = {"project": "set", "name": "set"} + multi_entity_update_modes = {"link": "set", "name": "set"} data = { "name": "test", + "link": { + "name": "test", + "url": "http://test.com", + }, + } + expected = { + "id": 999, + "type": "Anything", + "fields": [ + { + "field_name": "name", + "value": "test", + "multi_entity_update_mode": "set", + }, + { + "field_name": "link", + "value": { + "name": "test", + "url": "http://test.com", + }, + "multi_entity_update_mode": "set", + }, + ], + } + sg = api.Shotgun("http://server_path", "script_name", "api_key", connect=False) + result = sg._translate_update_params(entity_type, entity_id, data, multi_entity_update_modes) + self.assertEqual(result, expected) + + @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) + def test_related_object_update_optimization_entity(self): + entity_type = "Anything" + entity_id = 999 + multi_entity_update_modes = {"project": "set", "link": "set", "name": "set"} + data = { + "name": "test", + "link": { + "name": "test", + "url": "http://test.com", + }, "project": { "foo1": "foo1", "bar1": "bar1", @@ -522,6 +587,14 @@ def test_related_object_update_entity(self): "value": "test", "multi_entity_update_mode": "set", }, + { + "field_name": "link", + "value": { + "name": "test", + "url": "http://test.com", + }, + "multi_entity_update_mode": "set", + }, { "field_name": "project", "multi_entity_update_mode": "set", From a62351e08406ed0770335a65ee79339f0a4fee92 Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Mon, 23 Dec 2024 14:11:38 -0500 Subject: [PATCH 06/10] Fix typo --- shotgun_api3/shotgun.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 78c9f3c40..73a02b9a8 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -4517,7 +4517,7 @@ def _version_str(version): def _get_type_and_id_from_value(value_dict): """ For an entity dictionary, returns a new dictionary with only the type and id keys. - If ant of these keys are not present, the original dictionary is returned. + If any of these keys are not present, the original dictionary is returned. """ try: return {"type": value_dict["type"], "id": value_dict["id"]} From 5c9dfa76ac02d90edb7b45c60f5a8a07dec76455 Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Tue, 28 Jan 2025 15:06:06 -0500 Subject: [PATCH 07/10] Fix test --- tests/test_unit.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_unit.py b/tests/test_unit.py index d8f0b31ff..789b83dc6 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -436,8 +436,6 @@ def test_related_object(self): result = api.shotgun._translate_filters(filters, "all") self.assertEqual(result, expected) - @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) - def test_related_object_entity_optimization_is(self): @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) def test_related_object_entity_optimization_is(self): filters = [ From 2faa5a2569b0009553139c433e3e17144e9049e0 Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Tue, 28 Jan 2025 15:51:09 -0500 Subject: [PATCH 08/10] Support multi entity, Add tests --- shotgun_api3/shotgun.py | 11 ++++++++--- tests/test_unit.py | 43 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 73a02b9a8..1392623db 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -4514,12 +4514,17 @@ def _version_str(version): return ".".join(map(str, version)) -def _get_type_and_id_from_value(value_dict): +def _get_type_and_id_from_value(value): """ For an entity dictionary, returns a new dictionary with only the type and id keys. If any of these keys are not present, the original dictionary is returned. """ try: - return {"type": value_dict["type"], "id": value_dict["id"]} + if isinstance(value, dict): + return {"type": value["type"], "id": value["id"]} + elif isinstance(value, list): + return [{"type": v["type"], "id": v["id"]} for v in value] + else: + return value except (KeyError, TypeError): - return value_dict + return value diff --git a/tests/test_unit.py b/tests/test_unit.py index 789b83dc6..b3321882a 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -291,6 +291,8 @@ def test_py_version(self, mock_sys): class TestFilters(unittest.TestCase): + maxDiff = None + def test_empty(self): expected = { "logical_operator": "and", @@ -574,9 +576,9 @@ def test_related_object_update_optimization_entity(self): "project": { "foo1": "foo1", "bar1": "bar1", - "id": 999, + "id": 888, "baz1": "baz1", - "type": "Anything", + "type": "Project", }, } expected = { @@ -601,8 +603,8 @@ def test_related_object_update_optimization_entity(self): "multi_entity_update_mode": "set", "value": { # Entity is optimized with type/id fields. - "id": 999, - "type": "Anything", + "id": 888, + "type": "Project", }, }, ], @@ -611,6 +613,39 @@ def test_related_object_update_optimization_entity(self): result = sg._translate_update_params(entity_type, entity_id, data, multi_entity_update_modes) self.assertEqual(result, expected) + @mock.patch.dict(os.environ, {"SHOTGUN_API_ENABLE_ENTITY_OPTIMIZATION": "1"}) + def test_related_object_update_optimization_entity_multi(self): + entity_type = "Asset" + entity_id = 6626 + data = { + "sg_status_list": "ip", + "project": {"id": 70, "type": "Project", "name": "disposable name 70"}, + "sg_vvv": [ + {"id": 6441, "type": "Asset", "name": "disposable name 6441"}, + {"id": 6440, "type": "Asset"}, + ], + "sg_class": {"id": 1, "type": "CustomEntity53", "name": "disposable name 1"}, + } + expected = { + "type": "Asset", + "id": 6626, + "fields": [ + {"field_name": "sg_status_list", "value": "ip"}, + {"field_name": "project", "value": {"type": "Project", "id": 70}}, + { + "field_name": "sg_vvv", + "value": [ + {"id": 6441, "type": "Asset"}, + {"id": 6440, "type": "Asset"}, + ], + }, + {"field_name": "sg_class", "value": {"type": "CustomEntity53", "id": 1}}, + ], + } + sg = api.Shotgun("http://server_path", "script_name", "api_key", connect=False) + result = sg._translate_update_params(entity_type, entity_id, data, None) + self.assertEqual(result, expected) + class TestCerts(unittest.TestCase): # A dummy bad url provided by Amazon From 5536ef3dd0ee00b0b1ba73ec2a13b9ff6228ef28 Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio Date: Wed, 29 Jan 2025 09:15:31 -0500 Subject: [PATCH 09/10] Apply CR feedback --- shotgun_api3/shotgun.py | 9 +++++---- tests/test_unit.py | 1 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 1392623db..d8bd966b6 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -4523,8 +4523,9 @@ def _get_type_and_id_from_value(value): if isinstance(value, dict): return {"type": value["type"], "id": value["id"]} elif isinstance(value, list): - return [{"type": v["type"], "id": v["id"]} for v in value] - else: - return value + return [{"type": v["type"], "id": v["id"]} for v in value] except (KeyError, TypeError): - return value + LOG.debug(f"Could not optimize entity value {value}") + pass + + return value diff --git a/tests/test_unit.py b/tests/test_unit.py index b3321882a..c8144d51b 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -13,7 +13,6 @@ import os import unittest from unittest import mock -from unittest import mock from .mock import patch import shotgun_api3 as api from shotgun_api3.shotgun import _is_mimetypes_broken From b37fa34c0e5b1dc891f7840adf0b5ef061d98e12 Mon Sep 17 00:00:00 2001 From: Carlos Villavicencio <123113322+carlos-villavicencio-adsk@users.noreply.github.com> Date: Wed, 29 Jan 2025 14:09:01 -0500 Subject: [PATCH 10/10] Update shotgun_api3/shotgun.py Co-authored-by: Julien Langlois <16244608+julien-lang@users.noreply.github.com> --- shotgun_api3/shotgun.py | 1 - 1 file changed, 1 deletion(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index d8bd966b6..d3f2cfba1 100644 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -4526,6 +4526,5 @@ def _get_type_and_id_from_value(value): return [{"type": v["type"], "id": v["id"]} for v in value] except (KeyError, TypeError): LOG.debug(f"Could not optimize entity value {value}") - pass return value