For #28441: refactored to clean up duplication in flag checks

Author: nemoDreamer

shotgun_api3/shotgun.py

@@ -134,7 +134,7 @@ def __init__(self, host, meta):
             self.is_dev = False
 
         self.version = tuple(self.version[:3])
-        self._ensure_json_supported()
+        self.ensure_json_supported()
 
 
     def _ensure_support(self, feature):
@@ -151,25 +151,28 @@ def _ensure_support(self, feature):
                 "%s requires server version %s or higher, "\
                 "server is %s" % (feature['label'], _version_str(feature['version']), _version_str(self.version))
             )
+            return False
+        else:
+            return True
 
 
-    def _ensure_json_supported(self):
+    def ensure_json_supported(self):
         """Wrapper for ensure_support"""
-        self._ensure_support({
+        return self._ensure_support({
             'version': (2, 4, 0),
             'label': 'JSON API'
         })
 
     def ensure_include_archived_projects(self):
         """Wrapper for ensure_support"""
-        self._ensure_support({
+        return self._ensure_support({
             'version': (5, 3, 14),
             'label': 'include_archived_projects parameter'
         })
 
     def ensure_include_template_projects(self):
         """Wrapper for ensure_support"""
-        self._ensure_support({
+        return self._ensure_support({
             'version': (6, 0, 0),
             'label': 'include_template_projects parameter'
         })
@@ -514,7 +517,7 @@ def find_one(self, entity_type, filters, fields=None, order=None,
         :param include_template_projects: Optional, flag to include entities
         belonging to template projects. Default: False
 
-        :returns: Result
+        :returns: dict of requested entity's fields, or None if not found.
         """
 
         results = self.find(entity_type, filters, fields, order,
@@ -577,22 +580,13 @@ def find(self, entity_type, filters, fields=None, order=None,
             raise ShotgunError("Deprecated: Use of filter_operator for find()"
                 " is not valid any more. See the documentation on find()")
 
-        if not include_archived_projects:
-            # This defaults to True on the server (no argument is sent)
-            # So we only need to check the server version if it is False
-            self.server_caps.ensure_include_archived_projects()
-
-        if include_template_projects:
-            # This defaults to False on the server (no argument is sent)
-            # So we only need to check the server version if it is True
-            self.server_caps.ensure_include_template_projects()
-
-
         params = self._construct_read_parameters(entity_type,
                                                  fields,
                                                  filters,
                                                  retired_only,
-                                                 order,
+                                                 order)
+
+        params = self._construct_flag_parameters(params,
                                                  include_archived_projects,
                                                  include_template_projects)
 
@@ -631,31 +625,24 @@ def find(self, entity_type, filters, fields=None, order=None,
         return self._parse_records(records)
 
 
-
     def _construct_read_parameters(self,
                                    entity_type,
                                    fields,
                                    filters,
                                    retired_only,
-                                   order,
-                                   include_archived_projects,
-                                   include_template_projects):
-        params = {}
-        params["type"] = entity_type
-        params["return_fields"] = fields or ["id"]
-        params["filters"] = filters
-        params["return_only"] = (retired_only and 'retired') or "active"
-        params["return_paging_info"] = True
-        params["paging"] = { "entities_per_page": self.config.records_per_page,
-                             "current_page": 1 }
-
-        if include_archived_projects is False:
-            # Defaults to True on the server, so only pass it if it's False
-            params["include_archived_projects"] = False
+                                   order):
 
-        if include_template_projects is True:
-            # Defaults to False on the server, so only pass it if it's True
-            params["include_template_projects"] = True
+        params = {
+            "type": entity_type,
+            "return_fields": fields or ["id"],
+            "filters": filters,
+            "return_only": (retired_only and 'retired') or "active",
+            "return_paging_info": True,
+            "paging": {
+                "entities_per_page": self.config.records_per_page,
+                "current_page": 1
+            }
+        }
 
         if order:
             sort_list = []
@@ -669,8 +656,32 @@ def _construct_read_parameters(self,
                     'direction' : sort['direction']
                 })
             params['sorts'] = sort_list
+
+        return params
+
+
+    def _construct_flag_parameters(self,
+                                   params,
+                                   include_archived_projects,
+                                   include_template_projects):
+
+        if not include_archived_projects:
+            # This defaults to True on the server (no argument is sent)
+            # So we only need to check the server version if it's False
+            self.server_caps.ensure_include_archived_projects()
+            # Only pass it if it's False
+            params["include_archived_projects"] = False
+
+        if include_template_projects:
+            # This defaults to False on the server (no argument is sent)
+            # So we only need to check the server version if it's True
+            self.server_caps.ensure_include_template_projects()
+            # Only pass it if it's True
+            params["include_template_projects"] = True
+
         return params
 
+
     def summarize(self,
                   entity_type,
                   filters,
@@ -695,19 +706,9 @@ def summarize(self,
                   "summaries": summary_fields,
                   "filters": filters}
 
-        if not include_archived_projects:
-            # This defaults to True on the server (no argument is sent)
-            # So we only need to check the server version if it is False
-            self.server_caps.ensure_include_archived_projects()
-            # Only pass it if it's False
-            params["include_archived_projects"] = False
-
-        if include_template_projects:
-            # This defaults to False on the server (no argument is sent)
-            # So we only need to check the server version if it is True
-            self.server_caps.ensure_include_template_projects()
-            # Only pass it if it's True
-            params["include_template_projects"] = True
+        params = self._construct_flag_parameters(params,
+                                                 include_archived_projects,
+                                                 include_template_projects)
 
         if grouping != None:
             params['grouping'] = grouping

tests/test_client.py

@@ -74,18 +74,18 @@ def test_server_version_json(self):
         sc = ServerCapabilities("foo", {"version" : (2,4,0)})
 
         sc.version = (2,3,99)
-        self.assertRaises(api.ShotgunError, sc._ensure_json_supported)
+        self.assertRaises(api.ShotgunError, sc.ensure_json_supported)
         self.assertRaises(api.ShotgunError, ServerCapabilities, "foo",
             {"version" : (2,2,0)})
 
         sc.version = (0,0,0)
-        self.assertRaises(api.ShotgunError, sc._ensure_json_supported)
+        self.assertRaises(api.ShotgunError, sc.ensure_json_supported)
 
         sc.version = (2,4,0)
-        sc._ensure_json_supported()
+        sc.ensure_json_supported()
 
         sc.version = (2,5,0)
-        sc._ensure_json_supported()
+        sc.ensure_json_supported()
 
 
     def test_session_uuid(self):