From 4d811c397177b1710ac3cff61e3df903e1482d75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Wed, 4 Dec 2024 21:55:33 +0100 Subject: [PATCH 01/19] Speed improvements --- pynetbox/core/app.py | 14 +++- pynetbox/core/response.py | 172 ++++++++++++++++---------------------- 2 files changed, 82 insertions(+), 104 deletions(-) diff --git a/pynetbox/core/app.py b/pynetbox/core/app.py index 10cc644..b173afe 100644 --- a/pynetbox/core/app.py +++ b/pynetbox/core/app.py @@ -41,6 +41,7 @@ def __init__(self, api, name): self.api = api self.name = name self._setmodel() + self._cached_endpoints = {} models = { "dcim": dcim, @@ -63,7 +64,11 @@ def __setstate__(self, d): self._setmodel() def __getattr__(self, name): - return Endpoint(self.api, self, name, model=self.model) + if name not in self._cached_endpoints: + self._cached_endpoints[name] = Endpoint( + self.api, self, name, model=self.model + ) + return self._cached_endpoints[name] def config(self): """Returns config response from app @@ -103,6 +108,7 @@ class PluginsApp: def __init__(self, api): self.api = api + self._cached_apps = {} def __getstate__(self): return self.__dict__ @@ -111,7 +117,11 @@ def __setstate__(self, d): self.__dict__.update(d) def __getattr__(self, name): - return App(self.api, "plugins/{}".format(name.replace("_", "-"))) + if name not in self._cached_apps: + self._cached_apps[name] = App( + self.api, "plugins/{}".format(name.replace("_", "-")) + ) + return self._cached_apps[name] def installed_plugins(self): """Returns raw response with installed plugins diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index fd8cfae..41c3299 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -14,9 +14,8 @@ limitations under the License. """ -import copy +import marshal from collections import OrderedDict -from urllib.parse import urlsplit import pynetbox.core.app from pynetbox.core.query import Request @@ -26,35 +25,20 @@ LIST_AS_SET = ("tags", "tagged_vlans") -def get_return(lookup, return_fields=None): - """Returns simple representations for items passed to lookup. - - Used to return a "simple" representation of objects and collections - sent to it via lookup. Otherwise, we look to see if - lookup is a "choices" field (dict with only 'id' and 'value') - or a nested_return. Finally, we check if it's a Record, if - so simply return a string. Order is important due to nested_return - being self-referential. - - :arg list,optional return_fields: A list of fields to reference when - calling values on lookup. +def get_return(record): + """ + Used to return a "simple" representation of objects and collections. """ + return_fields = ["id", "value"] - for i in return_fields or ["id", "value", "nested_return"]: - if isinstance(lookup, dict) and lookup.get(i): - return lookup[i] - else: - if hasattr(lookup, i): - # check if this is a "choices" field record - # from a NetBox 2.7 server. - if sorted(dict(lookup)) == sorted(["id", "value", "label"]): - return getattr(lookup, "value") - return getattr(lookup, i) - - if isinstance(lookup, Record): - return str(lookup) + if not isinstance(record, Record): + raise ValueError + + for i in return_fields: + if value := getattr(record, i, None): + return value else: - return lookup + return str(record) def flatten_custom(custom_dict): @@ -277,7 +261,6 @@ class Record: def __init__(self, values, api, endpoint): self.has_details = False - self._full_cache = [] self._init_cache = [] self.api = api self.default_ret = Record @@ -308,16 +291,16 @@ def __getattr__(self, k): raise AttributeError('object has no attribute "{}"'.format(k)) def __iter__(self): - for i in dict(self._init_cache): - cur_attr = getattr(self, i) + for k, _ in self._init_cache: + cur_attr = getattr(self, k) if isinstance(cur_attr, Record): - yield i, dict(cur_attr) + yield k, dict(cur_attr) elif isinstance(cur_attr, list) and all( isinstance(i, Record) for i in cur_attr ): - yield i, [dict(x) for x in cur_attr] + yield k, [dict(x) for x in cur_attr] else: - yield i, cur_attr + yield k, cur_attr def __getitem__(self, k): return dict(self)[k] @@ -353,10 +336,6 @@ def __eq__(self, other): return self.__key__() == other.__key__() return NotImplemented - def _add_cache(self, item): - key, value = item - self._init_cache.append((key, get_return(value))) - def _parse_values(self, values): """Parses values init arg. @@ -364,81 +343,70 @@ def _parse_values(self, values): values within. """ - def generic_list_parser(key_name, list_item): + def dict_parser(key_name, value): + # We keep must keep some specific fields as dictionaries + if key_name not in ["custom_fields", "local_context_data"]: + lookup = getattr(self.__class__, key_name, None) + if lookup is None or not issubclass(lookup, JsonField): + # If we have a custom model field, use it, otherwise use a default Record model + args = [value, self.api, self.endpoint] + value = lookup(*args) if lookup else self.default_ret(*args) + return value, get_return(value) + return value, marshal.loads(marshal.dumps(value)) + + def list_item_parser(list_item): from pynetbox.models.mapper import CONTENT_TYPE_MAPPER - if ( - isinstance(list_item, dict) - and "object_type" in list_item - and "object" in list_item - ): - lookup = list_item["object_type"] - model = None - model = CONTENT_TYPE_MAPPER.get(lookup) - if model: - return model(list_item["object"], self.api, self.endpoint) - + lookup = list_item["object_type"] + if model := CONTENT_TYPE_MAPPER.get(lookup, None): + return model(list_item["object"], self.api, self.endpoint) return list_item - def list_parser(key_name, list_item): - if isinstance(list_item, dict): - lookup = getattr(self.__class__, key_name, None) - if not isinstance(lookup, list): - # This is *list_parser*, so if the custom model field is not - # a list (or is not defined), just return the default model - return self.default_ret(list_item, self.api, self.endpoint) - else: - model = lookup[0] - return model(list_item, self.api, self.endpoint) + def list_parser(key_name, value): + if not value: + return value, [] - return list_item + if key_name in ["constraints"]: + return value, marshal.loads(marshal.dumps(value)) - for k, v in values.items(): - if isinstance(v, dict): - lookup = getattr(self.__class__, k, None) - if k in ["custom_fields", "local_context_data"] or hasattr( - lookup, "_json_field" - ): - self._add_cache((k, copy.deepcopy(v))) - setattr(self, k, v) - continue - if lookup: - v = lookup(v, self.api, self.endpoint) + sample_item = value[0] + if isinstance(sample_item, dict): + if "object_type" in sample_item and "object" in sample_item: + value = [list_item_parser(item) for item in value] else: - v = self.default_ret(v, self.api, self.endpoint) - self._add_cache((k, v)) - - elif isinstance(v, list): - # check if GFK - if len(v) and isinstance(v[0], dict) and "object_type" in v[0]: - v = [generic_list_parser(k, i) for i in v] - to_cache = list(v) - elif k == "constraints": - # Permissions constraints can be either dict or list - to_cache = copy.deepcopy(v) - else: - v = [list_parser(k, i) for i in v] - to_cache = list(v) - self._add_cache((k, to_cache)) - - else: - self._add_cache((k, v)) - setattr(self, k, v) + lookup = getattr(self.__class__, key_name, None) + if not isinstance(lookup, list): + # This is *list_parser*, so if the custom model field is not + # a list (or is not defined), just return the default model + value = [ + self.default_ret(i, self.api, self.endpoint) for i in value + ] + else: + model = lookup[0] + value = [model(i, self.api, self.endpoint) for i in value] + return value, [*value] + + def parse_value(key_name, value): + if not isinstance(value, (dict, list)): + to_cache = value + elif isinstance(value, dict): + value, to_cache = dict_parser(key_name, value) + elif isinstance(value, list): + value, to_cache = list_parser(key_name, value) + setattr(self, key_name, value) + return to_cache + + self._init_cache = [(k, parse_value(k, v)) for k, v in values.items()] def _endpoint_from_url(self, url): - url_path = urlsplit(url).path - base_url_path_parts = urlsplit(self.api.base_url).path.split("/") - if len(base_url_path_parts) > 2: - # There are some extra directories in the path, remove them from url - extra_path = "/".join(base_url_path_parts[:-1]) - url_path = url_path[len(extra_path) :] + url_path = url.replace(self.api.base_url, "") split_url_path = url_path.split("/") - if split_url_path[2] == "plugins": - app = "plugins/{}".format(split_url_path[3]) - name = split_url_path[4] - else: + if split_url_path[1] == "plugins": app, name = split_url_path[2:4] - return getattr(pynetbox.core.app.App(self.api, app), name) + return getattr(getattr(getattr(self.api, "plugins"), app), name) + else: + app, name = split_url_path[1:3] + return getattr(getattr(self.api, app), name) def full_details(self): """Queries the hyperlinked endpoint if 'url' is defined. From b2900a0f6fdee8f3f425917f6a68ba126eb9c7d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:52:19 +0100 Subject: [PATCH 02/19] Fix tests --- pynetbox/core/response.py | 14 ++++++------- tests/unit/test_response.py | 40 +++++++++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 41c3299..c96f4ad 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -399,13 +399,13 @@ def parse_value(key_name, value): self._init_cache = [(k, parse_value(k, v)) for k, v in values.items()] def _endpoint_from_url(self, url): - url_path = url.replace(self.api.base_url, "") - split_url_path = url_path.split("/") - if split_url_path[1] == "plugins": - app, name = split_url_path[2:4] - return getattr(getattr(getattr(self.api, "plugins"), app), name) + url_path = url.replace(self.api.base_url, "").split("/") + is_plugin = url_path and url_path[1] == "plugins" + start = 2 if is_plugin else 1 + app, name = [i.replace("-", "_") for i in url_path[start : start + 2]] + if is_plugin: + return getattr(getattr(self.api.plugins, app), name) else: - app, name = split_url_path[1:3] return getattr(getattr(self.api, app), name) def full_details(self): @@ -569,7 +569,7 @@ def delete(self): """Deletes an existing object. :returns: True if DELETE operation was successful. - :example: + :examples: >>> x = nb.dcim.devices.get(name='test1-a3-tor1b') >>> x.delete() diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py index 1b4beb5..d973b7c 100644 --- a/tests/unit/test_response.py +++ b/tests/unit/test_response.py @@ -219,11 +219,14 @@ def test_compare(self): self.assertEqual(test1, test2) def test_nested_write(self): - app = Mock() - app.token = "abc123" - app.base_url = "http://localhost:8080/api" + api = Mock() + api.token = "abc123" + api.base_url = "http://localhost:8080/api" endpoint = Mock() endpoint.name = "test-endpoint" + endpoint.url = "http://localhost:8080/api/test-app/test-endpoint/" + api.test_app = Mock() + api.test_app.test_endpoint = endpoint test = Record( { "id": 123, @@ -234,22 +237,25 @@ def test_nested_write(self): "url": "http://localhost:8080/api/test-app/test-endpoint/321/", }, }, - app, + api, endpoint, ) test.child.name = "test321" test.child.save() self.assertEqual( - app.http_session.patch.call_args[0][0], + api.http_session.patch.call_args[0][0], "http://localhost:8080/api/test-app/test-endpoint/321/", ) def test_nested_write_with_directory_in_base_url(self): - app = Mock() - app.token = "abc123" - app.base_url = "http://localhost:8080/testing/api" + api = Mock() + api.token = "abc123" + api.base_url = "http://localhost:8080/testing/api" endpoint = Mock() endpoint.name = "test-endpoint" + endpoint.url = "http://localhost:8080/testing/api/test-app/test-endpoint/" + api.test_app = Mock() + api.test_app.test_endpoint = endpoint test = Record( { "id": 123, @@ -260,19 +266,22 @@ def test_nested_write_with_directory_in_base_url(self): "url": "http://localhost:8080/testing/api/test-app/test-endpoint/321/", }, }, - app, + api, endpoint, ) test.child.name = "test321" test.child.save() self.assertEqual( - app.http_session.patch.call_args[0][0], + api.http_session.patch.call_args[0][0], "http://localhost:8080/testing/api/test-app/test-endpoint/321/", ) def test_endpoint_from_url(self): api = Mock() api.base_url = "http://localhost:8080/api" + api.test_app = Mock() + api.test_app.test_endpoint = Mock() + api.test_app.test_endpoint.name = "test-endpoint" test = Record( { "id": 123, @@ -288,6 +297,9 @@ def test_endpoint_from_url(self): def test_endpoint_from_url_with_directory_in_base_url(self): api = Mock() api.base_url = "http://localhost:8080/testing/api" + api.test_app = Mock() + api.test_app.test_endpoint = Mock() + api.test_app.test_endpoint.name = "test-endpoint" test = Record( { "id": 123, @@ -303,6 +315,10 @@ def test_endpoint_from_url_with_directory_in_base_url(self): def test_endpoint_from_url_with_plugins(self): api = Mock() api.base_url = "http://localhost:8080/api" + api.plugins = Mock() + api.plugins.test_app = Mock() + api.plugins.test_app.test_endpoint = Mock() + api.plugins.test_app.test_endpoint.name = "test-endpoint" test = Record( { "id": 123, @@ -318,6 +334,10 @@ def test_endpoint_from_url_with_plugins(self): def test_endpoint_from_url_with_plugins_and_directory_in_base_url(self): api = Mock() api.base_url = "http://localhost:8080/testing/api" + api.plugins = Mock() + api.plugins.test_app = Mock() + api.plugins.test_app.test_endpoint = Mock() + api.plugins.test_app.test_endpoint.name = "test-endpoint" test = Record( { "id": 123, From 0f9e41b2cad75ce29f6ffc0adebf5d10ffefb7fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Thu, 5 Dec 2024 12:54:13 +0100 Subject: [PATCH 03/19] Compute endpoint on demand --- pynetbox/core/response.py | 43 +++++++++++++++++++------------------ tests/unit/test_response.py | 8 +++---- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index c96f4ad..b329cdd 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -264,11 +264,8 @@ def __init__(self, values, api, endpoint): self._init_cache = [] self.api = api self.default_ret = Record - self.endpoint = ( - self._endpoint_from_url(values["url"]) - if values and "url" in values and values["url"] - else endpoint - ) + self.url = values.get("url", None) if values else None + self._endpoint = endpoint if values: self._parse_values(values) @@ -336,6 +333,22 @@ def __eq__(self, other): return self.__key__() == other.__key__() return NotImplemented + @property + def endpoint(self): + if self._endpoint is None: + self._endpoint = self._endpoint_from_url() + return self._endpoint + + def _endpoint_from_url(self): + url_path = self.url.replace(self.api.base_url, "").split("/") + is_plugin = url_path and url_path[1] == "plugins" + start = 2 if is_plugin else 1 + app, name = [i.replace("-", "_") for i in url_path[start : start + 2]] + if is_plugin: + return getattr(getattr(self.api.plugins, app), name) + else: + return getattr(getattr(self.api, app), name) + def _parse_values(self, values): """Parses values init arg. @@ -349,7 +362,7 @@ def dict_parser(key_name, value): lookup = getattr(self.__class__, key_name, None) if lookup is None or not issubclass(lookup, JsonField): # If we have a custom model field, use it, otherwise use a default Record model - args = [value, self.api, self.endpoint] + args = [value, self.api, None] value = lookup(*args) if lookup else self.default_ret(*args) return value, get_return(value) return value, marshal.loads(marshal.dumps(value)) @@ -359,7 +372,7 @@ def list_item_parser(list_item): lookup = list_item["object_type"] if model := CONTENT_TYPE_MAPPER.get(lookup, None): - return model(list_item["object"], self.api, self.endpoint) + return model(list_item["object"], self.api, None) return list_item def list_parser(key_name, value): @@ -378,12 +391,10 @@ def list_parser(key_name, value): if not isinstance(lookup, list): # This is *list_parser*, so if the custom model field is not # a list (or is not defined), just return the default model - value = [ - self.default_ret(i, self.api, self.endpoint) for i in value - ] + value = [self.default_ret(i, self.api, None) for i in value] else: model = lookup[0] - value = [model(i, self.api, self.endpoint) for i in value] + value = [model(i, self.api, None) for i in value] return value, [*value] def parse_value(key_name, value): @@ -398,16 +409,6 @@ def parse_value(key_name, value): self._init_cache = [(k, parse_value(k, v)) for k, v in values.items()] - def _endpoint_from_url(self, url): - url_path = url.replace(self.api.base_url, "").split("/") - is_plugin = url_path and url_path[1] == "plugins" - start = 2 if is_plugin else 1 - app, name = [i.replace("-", "_") for i in url_path[start : start + 2]] - if is_plugin: - return getattr(getattr(self.api.plugins, app), name) - else: - return getattr(getattr(self.api, app), name) - def full_details(self): """Queries the hyperlinked endpoint if 'url' is defined. diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py index d973b7c..ebcc8d7 100644 --- a/tests/unit/test_response.py +++ b/tests/unit/test_response.py @@ -291,7 +291,7 @@ def test_endpoint_from_url(self): api, None, ) - ret = test._endpoint_from_url(test.url) + ret = test._endpoint_from_url() self.assertEqual(ret.name, "test-endpoint") def test_endpoint_from_url_with_directory_in_base_url(self): @@ -309,7 +309,7 @@ def test_endpoint_from_url_with_directory_in_base_url(self): api, None, ) - ret = test._endpoint_from_url(test.url) + ret = test._endpoint_from_url() self.assertEqual(ret.name, "test-endpoint") def test_endpoint_from_url_with_plugins(self): @@ -328,7 +328,7 @@ def test_endpoint_from_url_with_plugins(self): api, None, ) - ret = test._endpoint_from_url(test.url) + ret = test._endpoint_from_url() self.assertEqual(ret.name, "test-endpoint") def test_endpoint_from_url_with_plugins_and_directory_in_base_url(self): @@ -347,7 +347,7 @@ def test_endpoint_from_url_with_plugins_and_directory_in_base_url(self): api, None, ) - ret = test._endpoint_from_url(test.url) + ret = test._endpoint_from_url() self.assertEqual(ret.name, "test-endpoint") def test_serialize_tag_list_order(self): From 1f61ece629956a7374c010a2d416e1c6fa5e9f80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Thu, 5 Dec 2024 13:27:28 +0100 Subject: [PATCH 04/19] Simplifies get_return --- pynetbox/core/response.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index b329cdd..9068fd5 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -17,7 +17,6 @@ import marshal from collections import OrderedDict -import pynetbox.core.app from pynetbox.core.query import Request from pynetbox.core.util import Hashabledict @@ -29,16 +28,7 @@ def get_return(record): """ Used to return a "simple" representation of objects and collections. """ - return_fields = ["id", "value"] - - if not isinstance(record, Record): - raise ValueError - - for i in return_fields: - if value := getattr(record, i, None): - return value - else: - return str(record) + return getattr(record, "id", None) or getattr(record, "value", None) or str(record) def flatten_custom(custom_dict): From 3df1baba952e1c578ac43a50862e3f8929917b3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Thu, 5 Dec 2024 13:50:09 +0100 Subject: [PATCH 05/19] Optimize parse_value type checking --- pynetbox/core/response.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 9068fd5..31f75dd 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -388,12 +388,12 @@ def list_parser(key_name, value): return value, [*value] def parse_value(key_name, value): - if not isinstance(value, (dict, list)): - to_cache = value - elif isinstance(value, dict): + if isinstance(value, dict): value, to_cache = dict_parser(key_name, value) elif isinstance(value, list): value, to_cache = list_parser(key_name, value) + else: + to_cache = value setattr(self, key_name, value) return to_cache From a9cc782dede514ec7c9bb0a1aff4d1d9acf7586f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Thu, 5 Dec 2024 15:03:34 +0100 Subject: [PATCH 06/19] Improve list parser --- pynetbox/core/response.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 31f75dd..6cf04d5 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -378,13 +378,10 @@ def list_parser(key_name, value): value = [list_item_parser(item) for item in value] else: lookup = getattr(self.__class__, key_name, None) - if not isinstance(lookup, list): - # This is *list_parser*, so if the custom model field is not - # a list (or is not defined), just return the default model - value = [self.default_ret(i, self.api, None) for i in value] - else: - model = lookup[0] - value = [model(i, self.api, None) for i in value] + # This is *list_parser*, so if the custom model field is not + # a list (or is not defined), just return the default model + model = lookup[0] if isinstance(lookup, list) else self.default_ret + value = [model(i, self.api, None) for i in value] return value, [*value] def parse_value(key_name, value): From cb089b42cce4bbf0a2bc0ccb28e4c7c1a602c71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Fri, 6 Dec 2024 08:31:42 +0100 Subject: [PATCH 07/19] Cache nested records for RecordSet --- pynetbox/core/endpoint.py | 6 +- pynetbox/core/response.py | 109 ++++++++++++++++++++++----- tests/fixtures/users/permission.json | 1 + tests/unit/test_response.py | 2 + 4 files changed, 96 insertions(+), 22 deletions(-) diff --git a/pynetbox/core/endpoint.py b/pynetbox/core/endpoint.py index dfd989e..52b670a 100644 --- a/pynetbox/core/endpoint.py +++ b/pynetbox/core/endpoint.py @@ -53,6 +53,7 @@ def __init__(self, api, app, name, model=None): endpoint=self.name, ) self._choices = None + self._cache = None def _lookup_ret_obj(self, name, model): """Loads unique Response objects. @@ -636,8 +637,9 @@ def count(self, *args, **kwargs): if any(i in RESERVED_KWARGS for i in kwargs): raise ValueError( - "A reserved {} kwarg was passed. Please remove it " - "try again.".format(RESERVED_KWARGS) + "A reserved {} kwarg was passed. Please remove it " "try again.".format( + RESERVED_KWARGS + ) ) ret = Request( diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 6cf04d5..170d046 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -24,11 +24,19 @@ LIST_AS_SET = ("tags", "tagged_vlans") -def get_return(record): +def get_foreign_key(record): """ - Used to return a "simple" representation of objects and collections. + Get the foreign key for Record objects and dictionaries. """ - return getattr(record, "id", None) or getattr(record, "value", None) or str(record) + if isinstance(record, Record): + gfk = getattr(record, "id", None) or getattr(record, "value", None) + elif isinstance(record, dict): + gfk = record.get("id", None) or record.get("value", None) + + if gfk is None: + raise ValueError("Unable to find a foreign key for the record.") + + return gfk def flatten_custom(custom_dict): @@ -54,6 +62,30 @@ class JsonField: _json_field = True +class CachedRecordRegistry: + """ + A cache for Record objects. + """ + + def __init__(self, api): + self.api = api + self._cache = {} + + def get(self, object_type, key): + """ + Retrieves a record from the cache + """ + return self._cache.get(object_type, {}).get(key, None) + + def set(self, object_type, key, value): + """ + Stores a record in the cache + """ + if object_type not in self._cache: + self._cache[object_type] = {} + self._cache[object_type][key] = value + + class RecordSet: """Iterator containing Record objects. @@ -88,18 +120,36 @@ def __init__(self, endpoint, request, **kwargs): self.request = request self.response = self.request.get() self._response_cache = [] + self._init_endpoint_cache() def __iter__(self): return self def __next__(self): - if self._response_cache: + try: + if self._response_cache: + return self.endpoint.return_obj( + self._response_cache.pop(), + self.endpoint.api, + self.endpoint, + ) return self.endpoint.return_obj( - self._response_cache.pop(), self.endpoint.api, self.endpoint + next(self.response), + self.endpoint.api, + self.endpoint, ) - return self.endpoint.return_obj( - next(self.response), self.endpoint.api, self.endpoint - ) + except StopIteration: + self._clear_endpoint_cache() + raise + + # def __del__(self): + # self._clear_endpoint_cache() + + def _init_endpoint_cache(self): + self.endpoint._cache = CachedRecordRegistry(self.endpoint.api) + + def _clear_endpoint_cache(self): + self.endpoint._cache = None def __len__(self): try: @@ -339,6 +389,20 @@ def _endpoint_from_url(self): else: return getattr(getattr(self.api, app), name) + def _get_or_init(self, object_type, key, value, model): + """ + Returns a record from the endpoint cache if it exists, otherwise + initializes a new record, store it in the cache, and return it. + """ + if self._endpoint and self._endpoint._cache: + if cached := self._endpoint._cache.get(object_type, key): + return cached + model = model or Record + record = model(value, self.api, None) + if self._endpoint and self._endpoint._cache: + self._endpoint._cache.set(object_type, key, record) + return record + def _parse_values(self, values): """Parses values init arg. @@ -346,23 +410,25 @@ def _parse_values(self, values): values within. """ + non_record_fields = ["custom_fields", "local_context_data"] + def dict_parser(key_name, value): - # We keep must keep some specific fields as dictionaries - if key_name not in ["custom_fields", "local_context_data"]: + if key_name not in non_record_fields: lookup = getattr(self.__class__, key_name, None) if lookup is None or not issubclass(lookup, JsonField): - # If we have a custom model field, use it, otherwise use a default Record model - args = [value, self.api, None] - value = lookup(*args) if lookup else self.default_ret(*args) - return value, get_return(value) + fkey = get_foreign_key(value) + value = self._get_or_init(key_name, fkey, value, lookup) + return value, fkey return value, marshal.loads(marshal.dumps(value)) - def list_item_parser(list_item): + def generic_list_item_parser(list_item): from pynetbox.models.mapper import CONTENT_TYPE_MAPPER lookup = list_item["object_type"] if model := CONTENT_TYPE_MAPPER.get(lookup, None): - return model(list_item["object"], self.api, None) + fkey = get_foreign_key(list_item["object"]) + value = self._get_or_init(lookup, fkey, list_item["object"], model) + return value return list_item def list_parser(key_name, value): @@ -375,13 +441,16 @@ def list_parser(key_name, value): sample_item = value[0] if isinstance(sample_item, dict): if "object_type" in sample_item and "object" in sample_item: - value = [list_item_parser(item) for item in value] + value = [generic_list_item_parser(item) for item in value] else: lookup = getattr(self.__class__, key_name, None) # This is *list_parser*, so if the custom model field is not # a list (or is not defined), just return the default model model = lookup[0] if isinstance(lookup, list) else self.default_ret - value = [model(i, self.api, None) for i in value] + value = [ + self._get_or_init(key_name, get_foreign_key(i), i, model) + for i in value + ] return value, [*value] def parse_value(key_name, value): @@ -435,7 +504,7 @@ def serialize(self, nested=False, init=False): :returns: dict. """ if nested: - return get_return(self) + return get_foreign_key(self) if init: init_vals = dict(self._init_cache) @@ -557,7 +626,7 @@ def delete(self): """Deletes an existing object. :returns: True if DELETE operation was successful. - :examples: + :example: >>> x = nb.dcim.devices.get(name='test1-a3-tor1b') >>> x.delete() diff --git a/tests/fixtures/users/permission.json b/tests/fixtures/users/permission.json index b33f7cb..5c65b07 100644 --- a/tests/fixtures/users/permission.json +++ b/tests/fixtures/users/permission.json @@ -3,6 +3,7 @@ "name": "permission1", "users": [ { + "id": 1, "username": "user1" } ], diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py index ebcc8d7..81f587e 100644 --- a/tests/unit/test_response.py +++ b/tests/unit/test_response.py @@ -225,6 +225,7 @@ def test_nested_write(self): endpoint = Mock() endpoint.name = "test-endpoint" endpoint.url = "http://localhost:8080/api/test-app/test-endpoint/" + endpoint._cache = None api.test_app = Mock() api.test_app.test_endpoint = endpoint test = Record( @@ -254,6 +255,7 @@ def test_nested_write_with_directory_in_base_url(self): endpoint = Mock() endpoint.name = "test-endpoint" endpoint.url = "http://localhost:8080/testing/api/test-app/test-endpoint/" + endpoint._cache = None api.test_app = Mock() api.test_app.test_endpoint = endpoint test = Record( From cd6679d689cfe18a445626ff692d603ca7b3c10d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Fri, 6 Dec 2024 08:41:55 +0100 Subject: [PATCH 08/19] Fix linting --- pynetbox/core/endpoint.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pynetbox/core/endpoint.py b/pynetbox/core/endpoint.py index 52b670a..5b8a939 100644 --- a/pynetbox/core/endpoint.py +++ b/pynetbox/core/endpoint.py @@ -637,9 +637,8 @@ def count(self, *args, **kwargs): if any(i in RESERVED_KWARGS for i in kwargs): raise ValueError( - "A reserved {} kwarg was passed. Please remove it " "try again.".format( - RESERVED_KWARGS - ) + "A reserved {} kwarg was passed. Please remove it " + "try again.".format(RESERVED_KWARGS) ) ret = Request( From 7a3cc864ad3d61ebd6d6f3d3b8fbf211b78a333d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:07:45 +0100 Subject: [PATCH 09/19] Fix linter check --- pynetbox/core/endpoint.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pynetbox/core/endpoint.py b/pynetbox/core/endpoint.py index 5b8a939..b5295d9 100644 --- a/pynetbox/core/endpoint.py +++ b/pynetbox/core/endpoint.py @@ -637,8 +637,8 @@ def count(self, *args, **kwargs): if any(i in RESERVED_KWARGS for i in kwargs): raise ValueError( - "A reserved {} kwarg was passed. Please remove it " - "try again.".format(RESERVED_KWARGS) + "A reserved kwarg was passed ({}). Please remove it " + "and try again.".format(RESERVED_KWARGS) ) ret = Request( From 2053db88d30b3d1937b1acd3410cc1f3fc2743d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:24:22 +0100 Subject: [PATCH 10/19] Prefer use of URL as caching key --- pynetbox/core/response.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 170d046..0c0e3b8 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -70,6 +70,8 @@ class CachedRecordRegistry: def __init__(self, api): self.api = api self._cache = {} + self._hit = 0 + self._miss = 0 def get(self, object_type, key): """ @@ -142,8 +144,8 @@ def __next__(self): self._clear_endpoint_cache() raise - # def __del__(self): - # self._clear_endpoint_cache() + def __del__(self): + self._clear_endpoint_cache() def _init_endpoint_cache(self): self.endpoint._cache = CachedRecordRegistry(self.endpoint.api) @@ -389,17 +391,22 @@ def _endpoint_from_url(self): else: return getattr(getattr(self.api, app), name) - def _get_or_init(self, object_type, key, value, model): + def _get_or_init(self, object_type, value, model): """ Returns a record from the endpoint cache if it exists, otherwise initializes a new record, store it in the cache, and return it. """ - if self._endpoint and self._endpoint._cache: + key = ( + value.get("url", None) or value.get("id", None) or value.get("value", None) + ) + if key and self._endpoint and self._endpoint._cache: if cached := self._endpoint._cache.get(object_type, key): + self._endpoint._cache._hit += 1 return cached model = model or Record record = model(value, self.api, None) - if self._endpoint and self._endpoint._cache: + if key and self._endpoint and self._endpoint._cache: + self._endpoint._cache._miss += 1 self._endpoint._cache.set(object_type, key, record) return record @@ -417,7 +424,7 @@ def dict_parser(key_name, value): lookup = getattr(self.__class__, key_name, None) if lookup is None or not issubclass(lookup, JsonField): fkey = get_foreign_key(value) - value = self._get_or_init(key_name, fkey, value, lookup) + value = self._get_or_init(key_name, value, lookup) return value, fkey return value, marshal.loads(marshal.dumps(value)) @@ -426,8 +433,7 @@ def generic_list_item_parser(list_item): lookup = list_item["object_type"] if model := CONTENT_TYPE_MAPPER.get(lookup, None): - fkey = get_foreign_key(list_item["object"]) - value = self._get_or_init(lookup, fkey, list_item["object"], model) + value = self._get_or_init(lookup, list_item["object"], model) return value return list_item @@ -447,10 +453,7 @@ def list_parser(key_name, value): # This is *list_parser*, so if the custom model field is not # a list (or is not defined), just return the default model model = lookup[0] if isinstance(lookup, list) else self.default_ret - value = [ - self._get_or_init(key_name, get_foreign_key(i), i, model) - for i in value - ] + value = [self._get_or_init(key_name, i, model) for i in value] return value, [*value] def parse_value(key_name, value): From 140c6f7f7b49f9163f5e32affc48cdbeec8d1608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:43:27 +0100 Subject: [PATCH 11/19] Fix caching --- pynetbox/core/response.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 0c0e3b8..48c93ff 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -144,9 +144,6 @@ def __next__(self): self._clear_endpoint_cache() raise - def __del__(self): - self._clear_endpoint_cache() - def _init_endpoint_cache(self): self.endpoint._cache = CachedRecordRegistry(self.endpoint.api) From 323255bd356338f920e070c3ab482681d6493c12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Fri, 6 Dec 2024 15:52:33 +0100 Subject: [PATCH 12/19] Adds fallback in case of no foreign key --- pynetbox/core/response.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 48c93ff..09828ba 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -32,10 +32,6 @@ def get_foreign_key(record): gfk = getattr(record, "id", None) or getattr(record, "value", None) elif isinstance(record, dict): gfk = record.get("id", None) or record.get("value", None) - - if gfk is None: - raise ValueError("Unable to find a foreign key for the record.") - return gfk @@ -416,14 +412,23 @@ def _parse_values(self, values): non_record_fields = ["custom_fields", "local_context_data"] + def deep_copy(value): + return marshal.loads(marshal.dumps(value)) + def dict_parser(key_name, value): - if key_name not in non_record_fields: - lookup = getattr(self.__class__, key_name, None) - if lookup is None or not issubclass(lookup, JsonField): - fkey = get_foreign_key(value) - value = self._get_or_init(key_name, value, lookup) - return value, fkey - return value, marshal.loads(marshal.dumps(value)) + if key_name in non_record_fields: + return value, deep_copy(value) + + lookup = getattr(self.__class__, key_name, None) + + if lookup and issubclass(lookup, JsonField): + return value, deep_copy(value) + + if fkey := get_foreign_key(value): + value = self._get_or_init(key_name, value, lookup) + return value, fkey + + return value, deep_copy(value) def generic_list_item_parser(list_item): from pynetbox.models.mapper import CONTENT_TYPE_MAPPER @@ -439,7 +444,7 @@ def list_parser(key_name, value): return value, [] if key_name in ["constraints"]: - return value, marshal.loads(marshal.dumps(value)) + return value, deep_copy(value) sample_item = value[0] if isinstance(sample_item, dict): From f39102af336140dc5366f37f93681bd6e6ee67c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Sat, 7 Dec 2024 14:51:24 +0100 Subject: [PATCH 13/19] Improves readability --- pynetbox/core/response.py | 45 +++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 09828ba..43c0ad6 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -410,13 +410,13 @@ def _parse_values(self, values): values within. """ - non_record_fields = ["custom_fields", "local_context_data"] + non_record_dict_fields = ["custom_fields", "local_context_data"] def deep_copy(value): return marshal.loads(marshal.dumps(value)) def dict_parser(key_name, value): - if key_name in non_record_fields: + if key_name in non_record_dict_fields: return value, deep_copy(value) lookup = getattr(self.__class__, key_name, None) @@ -430,32 +430,41 @@ def dict_parser(key_name, value): return value, deep_copy(value) - def generic_list_item_parser(list_item): + def mixed_list_parser(value): from pynetbox.models.mapper import CONTENT_TYPE_MAPPER - lookup = list_item["object_type"] - if model := CONTENT_TYPE_MAPPER.get(lookup, None): - value = self._get_or_init(lookup, list_item["object"], model) - return value - return list_item + parsed_list = [] + for item in value: + lookup = item["object_type"] + if model := CONTENT_TYPE_MAPPER.get(lookup, None): + item = self._get_or_init(lookup, item["object"], model) + parsed_list.append(item) + return parsed_list + + def uniform_list_parser(key_name, value): + lookup = getattr(self.__class__, key_name, None) + model = lookup[0] if isinstance(lookup, list) else self.default_ret + value = [self._get_or_init(key_name, i, model) for i in value] + return value def list_parser(key_name, value): if not value: - return value, [] + return [], [] if key_name in ["constraints"]: return value, deep_copy(value) sample_item = value[0] - if isinstance(sample_item, dict): - if "object_type" in sample_item and "object" in sample_item: - value = [generic_list_item_parser(item) for item in value] - else: - lookup = getattr(self.__class__, key_name, None) - # This is *list_parser*, so if the custom model field is not - # a list (or is not defined), just return the default model - model = lookup[0] if isinstance(lookup, list) else self.default_ret - value = [self._get_or_init(key_name, i, model) for i in value] + if not isinstance(sample_item, dict): + return value, [*value] + + is_mixed_list = "object_type" in sample_item and "object" in sample_item + value = ( + mixed_list_parser(value) + if is_mixed_list + else uniform_list_parser(key_name, value) + ) + return value, [*value] def parse_value(key_name, value): From f6c403e4209980dbb2e94fc8b06612a139b429eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Sun, 8 Dec 2024 00:23:05 +0100 Subject: [PATCH 14/19] Use dict parser for uniform lists --- pynetbox/core/response.py | 32 ++++++++++++++------------------ pynetbox/models/dcim.py | 1 + 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 43c0ad6..8d4659c 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -28,10 +28,10 @@ def get_foreign_key(record): """ Get the foreign key for Record objects and dictionaries. """ - if isinstance(record, Record): - gfk = getattr(record, "id", None) or getattr(record, "value", None) - elif isinstance(record, dict): + if isinstance(record, dict): gfk = record.get("id", None) or record.get("value", None) + elif isinstance(record, Record): + gfk = getattr(record, "id", None) or getattr(record, "value", None) return gfk @@ -415,17 +415,18 @@ def _parse_values(self, values): def deep_copy(value): return marshal.loads(marshal.dumps(value)) - def dict_parser(key_name, value): + def dict_parser(key_name, value, model=None): if key_name in non_record_dict_fields: return value, deep_copy(value) - lookup = getattr(self.__class__, key_name, None) + if model is None: + model = getattr(self.__class__, key_name, None) - if lookup and issubclass(lookup, JsonField): + if model and issubclass(model, JsonField): return value, deep_copy(value) if fkey := get_foreign_key(value): - value = self._get_or_init(key_name, value, lookup) + value = self._get_or_init(key_name, value, model) return value, fkey return value, deep_copy(value) @@ -441,12 +442,6 @@ def mixed_list_parser(value): parsed_list.append(item) return parsed_list - def uniform_list_parser(key_name, value): - lookup = getattr(self.__class__, key_name, None) - model = lookup[0] if isinstance(lookup, list) else self.default_ret - value = [self._get_or_init(key_name, i, model) for i in value] - return value - def list_parser(key_name, value): if not value: return [], [] @@ -459,11 +454,12 @@ def list_parser(key_name, value): return value, [*value] is_mixed_list = "object_type" in sample_item and "object" in sample_item - value = ( - mixed_list_parser(value) - if is_mixed_list - else uniform_list_parser(key_name, value) - ) + if is_mixed_list: + value = mixed_list_parser(value) + else: + lookup = getattr(self.__class__, key_name, None) + model = lookup[0] if isinstance(lookup, list) else self.default_ret + value = [dict_parser(key_name, i, model=model)[0] for i in value] return value, [*value] diff --git a/pynetbox/models/dcim.py b/pynetbox/models/dcim.py index 02533c0..67bcd85 100644 --- a/pynetbox/models/dcim.py +++ b/pynetbox/models/dcim.py @@ -151,6 +151,7 @@ def __str__(self): class Interfaces(TraceableRecord): + device = Devices interface_connection = InterfaceConnection From d3b1a8dc23ddbe9994c4ea5e60c61731965ebcd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Mon, 9 Dec 2024 13:51:43 +0100 Subject: [PATCH 15/19] Add new class for nested value dictionary --- pynetbox/core/response.py | 96 ++++++++++++++++++++-------- tests/fixtures/users/permission.json | 3 +- tests/unit/test_response.py | 7 +- 3 files changed, 77 insertions(+), 29 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 8d4659c..60488da 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -201,7 +201,56 @@ def delete(self): return self.endpoint.delete(self) -class Record: +class BaseRecord: + def __init__(self): + self._init_cache = [] + + def __getitem__(self, k): + return dict(self)[k] + + def __repr__(self): + return str(self) + + def __getstate__(self): + return self.__dict__ + + def __setstate__(self, d): + self.__dict__.update(d) + + +class ValueRecord(BaseRecord): + def __init__(self, values, *args, **kwargs): + super().__init__() + if values: + self._parse_values(values) + + def __iter__(self): + for k, _ in self._init_cache: + cur_attr = getattr(self, k) + yield k, cur_attr + + def __repr__(self): + return getattr(self, "label", "") + + @property + def _key(self): + return getattr(self, "value") + + def __eq__(self, other): + if isinstance(other, ValueRecord): + return self._foreign_key == other._foreign_key + return NotImplemented + + def _parse_values(self, values): + for k, v in values.items(): + self._init_cache.append((k, v)) + setattr(self, k, v) + + def serialize(self, nested=False): + return self._key if nested else dict(self) + + +class Record(BaseRecord): """Create Python objects from NetBox API responses. Creates an object from a NetBox response passed as ``values``. @@ -292,11 +341,9 @@ class Record: """ - url = None - def __init__(self, values, api, endpoint): self.has_details = False - self._init_cache = [] + super().__init__() self.api = api self.default_ret = Record self.url = values.get("url", None) if values else None @@ -334,9 +381,6 @@ def __iter__(self): else: yield k, cur_attr - def __getitem__(self, k): - return dict(self)[k] - def __str__(self): return ( getattr(self, "name", None) @@ -345,15 +389,6 @@ def __str__(self): or "" ) - def __repr__(self): - return str(self) - - def __getstate__(self): - return self.__dict__ - - def __setstate__(self, d): - self.__dict__.update(d) - def __key__(self): if hasattr(self, "id"): return (self.endpoint.name, self.id) @@ -384,19 +419,15 @@ def _endpoint_from_url(self): else: return getattr(getattr(self.api, app), name) - def _get_or_init(self, object_type, value, model): + def _get_or_init(self, object_type, key, value, model): """ Returns a record from the endpoint cache if it exists, otherwise initializes a new record, store it in the cache, and return it. """ - key = ( - value.get("url", None) or value.get("id", None) or value.get("value", None) - ) if key and self._endpoint and self._endpoint._cache: if cached := self._endpoint._cache.get(object_type, key): self._endpoint._cache._hit += 1 return cached - model = model or Record record = model(value, self.api, None) if key and self._endpoint and self._endpoint._cache: self._endpoint._cache._miss += 1 @@ -425,9 +456,18 @@ def dict_parser(key_name, value, model=None): if model and issubclass(model, JsonField): return value, deep_copy(value) - if fkey := get_foreign_key(value): - value = self._get_or_init(key_name, value, model) - return value, fkey + if id := value.get("id", None): + # if model or "url" in value: + if url := value.get("url", None): + model = model or Record + value = self._get_or_init(key_name, url, value, model) + return value, id + + if record_value := value.get("value", None): + # if set(value.keys()) == {"value", "label"}: + # value = ValueRecord(values) + value = self._get_or_init(key_name, record_value, value, ValueRecord) + return value, record_value return value, deep_copy(value) @@ -438,7 +478,9 @@ def mixed_list_parser(value): for item in value: lookup = item["object_type"] if model := CONTENT_TYPE_MAPPER.get(lookup, None): - item = self._get_or_init(lookup, item["object"], model) + item = self._get_or_init( + lookup, item["object"]["url"], item["object"], model + ) parsed_list.append(item) return parsed_list @@ -514,7 +556,7 @@ def serialize(self, nested=False, init=False): :returns: dict. """ if nested: - return get_foreign_key(self) + return getattr(self, "id") if init: init_vals = dict(self._init_cache) @@ -525,7 +567,7 @@ def serialize(self, nested=False, init=False): if i == "custom_fields": ret[i] = flatten_custom(current_val) else: - if isinstance(current_val, Record): + if isinstance(current_val, BaseRecord): current_val = getattr(current_val, "serialize")(nested=True) if isinstance(current_val, list): diff --git a/tests/fixtures/users/permission.json b/tests/fixtures/users/permission.json index 5c65b07..d807d9f 100644 --- a/tests/fixtures/users/permission.json +++ b/tests/fixtures/users/permission.json @@ -4,7 +4,8 @@ "users": [ { "id": 1, - "username": "user1" + "username": "user1", + "url": "http://localhost:8000/api/users/users/1/" } ], "constraints": [ diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py index 81f587e..7cad4bd 100644 --- a/tests/unit/test_response.py +++ b/tests/unit/test_response.py @@ -26,8 +26,13 @@ def test_attribute_access(self): test_values = { "id": 123, "units": 12, - "nested_dict": {"id": 222, "name": "bar"}, + "nested_dict": { + "id": 222, + "name": "bar", + "url": "http://localhost:8000/api/test-app/test-endpoint/222/", + }, "int_list": [123, 321, 231], + "url": "http://localhost:8000/api/test-app/test-endpoint/123/", } test_obj = Record(test_values, None, None) self.assertEqual(test_obj.id, 123) From 5851e624bf2d3d8d17d8da0b9d181cec41905c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Mon, 9 Dec 2024 14:40:36 +0100 Subject: [PATCH 16/19] Fix dict serialization of Record --- pynetbox/core/response.py | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 60488da..db15cb9 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -24,17 +24,6 @@ LIST_AS_SET = ("tags", "tagged_vlans") -def get_foreign_key(record): - """ - Get the foreign key for Record objects and dictionaries. - """ - if isinstance(record, dict): - gfk = record.get("id", None) or record.get("value", None) - elif isinstance(record, Record): - gfk = getattr(record, "id", None) or getattr(record, "value", None) - return gfk - - def flatten_custom(custom_dict): ret = {} @@ -372,10 +361,10 @@ def __getattr__(self, k): def __iter__(self): for k, _ in self._init_cache: cur_attr = getattr(self, k) - if isinstance(cur_attr, Record): + if isinstance(cur_attr, BaseRecord): yield k, dict(cur_attr) elif isinstance(cur_attr, list) and all( - isinstance(i, Record) for i in cur_attr + isinstance(i, BaseRecord) for i in cur_attr ): yield k, [dict(x) for x in cur_attr] else: @@ -456,16 +445,12 @@ def dict_parser(key_name, value, model=None): if model and issubclass(model, JsonField): return value, deep_copy(value) - if id := value.get("id", None): - # if model or "url" in value: - if url := value.get("url", None): - model = model or Record - value = self._get_or_init(key_name, url, value, model) - return value, id + if (id := value.get("id", None)) and (url := value.get("url", None)): + model = model or Record + value = self._get_or_init(key_name, url, value, model) + return value, id if record_value := value.get("value", None): - # if set(value.keys()) == {"value", "label"}: - # value = ValueRecord(values) value = self._get_or_init(key_name, record_value, value, ValueRecord) return value, record_value @@ -572,7 +557,7 @@ def serialize(self, nested=False, init=False): if isinstance(current_val, list): current_val = [ - v.id if isinstance(v, Record) else v for v in current_val + v.id if isinstance(v, BaseRecord) else v for v in current_val ] if i in LIST_AS_SET and ( all([isinstance(v, str) for v in current_val]) From 1133cac2ac7af483656270a7e6403014a4f3de74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Mon, 9 Dec 2024 21:10:35 +0100 Subject: [PATCH 17/19] Adds generic list object --- pynetbox/core/response.py | 51 ++++++++++++++++++++++++++++++++------- pynetbox/models/mapper.py | 2 ++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index fd8cfae..160792e 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -313,7 +313,7 @@ def __iter__(self): if isinstance(cur_attr, Record): yield i, dict(cur_attr) elif isinstance(cur_attr, list) and all( - isinstance(i, Record) for i in cur_attr + isinstance(i, (Record, GenericListObject)) for i in cur_attr ): yield i, [dict(x) for x in cur_attr] else: @@ -373,10 +373,9 @@ def generic_list_parser(key_name, list_item): and "object" in list_item ): lookup = list_item["object_type"] - model = None - model = CONTENT_TYPE_MAPPER.get(lookup) - if model: - return model(list_item["object"], self.api, self.endpoint) + if model := CONTENT_TYPE_MAPPER.get(lookup, None): + record = model(list_item["object"], self.api, self.endpoint) + return GenericListObject(record) return list_item @@ -412,7 +411,7 @@ def list_parser(key_name, list_item): # check if GFK if len(v) and isinstance(v[0], dict) and "object_type" in v[0]: v = [generic_list_parser(k, i) for i in v] - to_cache = list(v) + to_cache = [i.serialize() for i in v] elif k == "constraints": # Permissions constraints can be either dict or list to_cache = copy.deepcopy(v) @@ -470,6 +469,7 @@ def serialize(self, nested=False, init=False): If an attribute's value is a ``Record`` type it's replaced with the ``id`` field of that object. + .. note:: Using this to get a dictionary representation of the record @@ -485,6 +485,7 @@ def serialize(self, nested=False, init=False): init_vals = dict(self._init_cache) ret = {} + for i in dict(self): current_val = getattr(self, i) if not init else init_vals.get(i) if i == "custom_fields": @@ -494,15 +495,21 @@ def serialize(self, nested=False, init=False): current_val = getattr(current_val, "serialize")(nested=True) if isinstance(current_val, list): - current_val = [ - v.id if isinstance(v, Record) else v for v in current_val - ] + serialized_list = [] + for v in current_val: + if isinstance(v, GenericListObject): + v = v.serialize() + elif isinstance(v, Record): + v = v.id + serialized_list.append(v) + current_val = serialized_list if i in LIST_AS_SET and ( all([isinstance(v, str) for v in current_val]) or all([isinstance(v, int) for v in current_val]) ): current_val = list(OrderedDict.fromkeys(current_val)) ret[i] = current_val + return ret def _diff(self): @@ -615,3 +622,29 @@ def delete(self): http_session=self.api.http_session, ) return True if req.delete() else False + + +class GenericListObject: + def __init__(self, record): + from pynetbox.models.mapper import TYPE_CONTENT_MAPPER + + self.object = record + self.object_id = record.id + self.object_type = TYPE_CONTENT_MAPPER.get(record.__class__) + + def __repr__(self): + return str(self.object) + + def serialize(self): + ret = {k: getattr(self, k) for k in ["object_id", "object_type"]} + return ret + + def __getattr__(self, k): + return getattr(self.object, k) + + def __iter__(self): + for i in dir(self): + cur_attr = getattr(self, i) + if isinstance(cur_attr, Record): + cur_attr = dict(cur_attr) + yield i, cur_attr diff --git a/pynetbox/models/mapper.py b/pynetbox/models/mapper.py index 1dcfc99..9a468ee 100644 --- a/pynetbox/models/mapper.py +++ b/pynetbox/models/mapper.py @@ -109,3 +109,5 @@ "wireless.WirelessLANGroup": None, "wireless.wirelesslink": None, } + +TYPE_CONTENT_MAPPER = {v: k for k, v in CONTENT_TYPE_MAPPER.items() if v is not None} From 108a080c927d55471ee490ed11511934eccc53fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Mon, 9 Dec 2024 21:52:42 +0100 Subject: [PATCH 18/19] Fix iteration on GenericListObject --- pynetbox/core/response.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index 160792e..68f65a1 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -643,7 +643,7 @@ def __getattr__(self, k): return getattr(self.object, k) def __iter__(self): - for i in dir(self): + for i in ["object_id", "object_type", "object"]: cur_attr = getattr(self, i) if isinstance(cur_attr, Record): cur_attr = dict(cur_attr) From eaa75ad56445f9f8997ee666e23e6bf232f94d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Flagothier?= <29896153+srfwx@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:23:46 +0100 Subject: [PATCH 19/19] Moved CachedRecordRegistry --- pynetbox/core/endpoint.py | 13 ++++----- pynetbox/core/response.py | 55 +++++++------------------------------ tests/unit/test_response.py | 8 ++++-- 3 files changed, 22 insertions(+), 54 deletions(-) diff --git a/pynetbox/core/endpoint.py b/pynetbox/core/endpoint.py index cd9c19e..3e42a81 100644 --- a/pynetbox/core/endpoint.py +++ b/pynetbox/core/endpoint.py @@ -14,8 +14,6 @@ limitations under the License. """ -from collections import OrderedDict - from pynetbox.core.query import Request, RequestError from pynetbox.core.response import Record, RecordSet @@ -36,10 +34,13 @@ def get(self, object_type, key): """ Retrieves a record from the cache """ - object_cache = self._cache.get(object_type) - if object_cache is None: + if not (object_cache := self._cache.get(object_type)): return None - return object_cache.get(key, None) + if object := object_cache.get(key, None): + self._hit += 1 + return object + self._miss += 1 + return None def set(self, object_type, key, value): """ @@ -83,8 +84,6 @@ def __init__(self, api, app, name, model=None): endpoint=self.name, ) self._choices = None - self._attribute_type_map = {} - self._attribute_endpoint_map = {} self._init_cache() def _init_cache(self): diff --git a/pynetbox/core/response.py b/pynetbox/core/response.py index c1cb4fc..2262f50 100644 --- a/pynetbox/core/response.py +++ b/pynetbox/core/response.py @@ -47,32 +47,6 @@ class JsonField: _json_field = True -class CachedRecordRegistry: - """ - A cache for Record objects. - """ - - def __init__(self, api): - self.api = api - self._cache = {} - self._hit = 0 - self._miss = 0 - - def get(self, object_type, key): - """ - Retrieves a record from the cache - """ - return self._cache.get(object_type, {}).get(key, None) - - def set(self, object_type, key, value): - """ - Stores a record in the cache - """ - if object_type not in self._cache: - self._cache[object_type] = {} - self._cache[object_type][key] = value - - class RecordSet: """Iterator containing Record objects. @@ -107,7 +81,6 @@ def __init__(self, endpoint, request, **kwargs): self.request = request self.response = self.request.get() self._response_cache = [] - self._init_endpoint_cache() def __iter__(self): return self @@ -126,15 +99,9 @@ def __next__(self): self.endpoint, ) except StopIteration: - self._clear_endpoint_cache() + self.endpoint._init_cache() raise - def _init_endpoint_cache(self): - self.endpoint._cache = CachedRecordRegistry(self.endpoint.api) - - def _clear_endpoint_cache(self): - self.endpoint._cache = None - def __len__(self): try: return self.request.count @@ -413,13 +380,11 @@ def _get_or_init(self, object_type, key, value, model): Returns a record from the endpoint cache if it exists, otherwise initializes a new record, store it in the cache, and return it. """ - if key and self._endpoint and self._endpoint._cache: + if self._endpoint: if cached := self._endpoint._cache.get(object_type, key): - self._endpoint._cache._hit += 1 return cached record = model(value, self.api, None) - if key and self._endpoint and self._endpoint._cache: - self._endpoint._cache._miss += 1 + if self._endpoint: self._endpoint._cache.set(object_type, key, record) return record @@ -456,15 +421,15 @@ def dict_parser(key_name, value, model=None): return value, deep_copy(value) - def mixed_list_parser(value): + def generic_list_parser(value): from pynetbox.models.mapper import CONTENT_TYPE_MAPPER parsed_list = [] for item in value: - lookup = item["object_type"] - if model := CONTENT_TYPE_MAPPER.get(lookup, None): + object_type = item["object_type"] + if model := CONTENT_TYPE_MAPPER.get(object_type, None): item = self._get_or_init( - lookup, item["object"]["url"], item["object"], model + object_type, item["object"]["url"], item["object"], model ) parsed_list.append(GenericListObject(item)) return parsed_list @@ -480,9 +445,9 @@ def list_parser(key_name, value): if not isinstance(sample_item, dict): return value, [*value] - is_mixed_list = "object_type" in sample_item and "object" in sample_item - if is_mixed_list: - value = mixed_list_parser(value) + is_generic_list = "object_type" in sample_item and "object" in sample_item + if is_generic_list: + value = generic_list_parser(value) else: lookup = getattr(self.__class__, key_name, None) model = lookup[0] if isinstance(lookup, list) else self.default_ret diff --git a/tests/unit/test_response.py b/tests/unit/test_response.py index 7cad4bd..e27bd12 100644 --- a/tests/unit/test_response.py +++ b/tests/unit/test_response.py @@ -230,7 +230,8 @@ def test_nested_write(self): endpoint = Mock() endpoint.name = "test-endpoint" endpoint.url = "http://localhost:8080/api/test-app/test-endpoint/" - endpoint._cache = None + endpoint._cache = Mock() + endpoint._cache.get = Mock(return_value=None) api.test_app = Mock() api.test_app.test_endpoint = endpoint test = Record( @@ -248,6 +249,8 @@ def test_nested_write(self): ) test.child.name = "test321" test.child.save() + print(api) + print(api.http_session) self.assertEqual( api.http_session.patch.call_args[0][0], "http://localhost:8080/api/test-app/test-endpoint/321/", @@ -260,7 +263,8 @@ def test_nested_write_with_directory_in_base_url(self): endpoint = Mock() endpoint.name = "test-endpoint" endpoint.url = "http://localhost:8080/testing/api/test-app/test-endpoint/" - endpoint._cache = None + endpoint._cache = Mock() + endpoint._cache.get = Mock(return_value=None) api.test_app = Mock() api.test_app.test_endpoint = endpoint test = Record(