From 070ec22425ea6a7b4e4089d16684814695d6bd33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 9 Feb 2024 12:05:27 +0100 Subject: [PATCH 1/7] Process values returned from instance unless function --- src/senaite/app/supermodel/model.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/senaite/app/supermodel/model.py b/src/senaite/app/supermodel/model.py index 8acdb10..704b991 100644 --- a/src/senaite/app/supermodel/model.py +++ b/src/senaite/app/supermodel/model.py @@ -230,7 +230,8 @@ def get(self, name, default=None): instance = self.instance instance_value = getattr(instance, name, _marker) if instance_value is not _marker: - return instance_value + # return the value processed, but only if not a function + return self.process_value(instance_value, safe_call=False) # check if the brain contains this attribute brain = self.brain @@ -265,7 +266,7 @@ def get(self, name, default=None): return value - def process_value(self, value): + def process_value(self, value, safe_call=True): """Process publication value """ # UID -> SuperModel @@ -293,7 +294,7 @@ def process_value(self, value): elif isinstance(value, (dict)): return {k: self.process_value(v) for k, v in value.iteritems()} # Process function - elif safe_callable(value): + elif safe_call and safe_callable(value): return self.process_value(value()) # Always return the unprocessed value last return value From 42c9145af1366bb3cc11876f073376463aa2534e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 9 Feb 2024 12:32:25 +0100 Subject: [PATCH 2/7] Cache the value in the dict if is not a function --- CHANGES.rst | 1 + src/senaite/app/supermodel/model.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 164bbf2..01d5b18 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,7 @@ 2.6.0 (unreleased) ------------------ +- #19 Process values returned from instance except when a function - no changes yet diff --git a/src/senaite/app/supermodel/model.py b/src/senaite/app/supermodel/model.py index 704b991..fb229d7 100644 --- a/src/senaite/app/supermodel/model.py +++ b/src/senaite/app/supermodel/model.py @@ -231,7 +231,11 @@ def get(self, name, default=None): instance_value = getattr(instance, name, _marker) if instance_value is not _marker: # return the value processed, but only if not a function - return self.process_value(instance_value, safe_call=False) + value = self.process_value(instance_value, safe_call=False) + if value != instance_value: + # Store value in the internal data dict + self.data[name] = value + return value # check if the brain contains this attribute brain = self.brain From be39474734b8951bff9cd68fa8935e018ab5b3a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 9 Feb 2024 14:06:52 +0100 Subject: [PATCH 3/7] Better support for value retrieval from DX fields --- src/senaite/app/supermodel/model.py | 97 ++++++++++++++++------------- 1 file changed, 55 insertions(+), 42 deletions(-) diff --git a/src/senaite/app/supermodel/model.py b/src/senaite/app/supermodel/model.py index fb229d7..db09225 100644 --- a/src/senaite/app/supermodel/model.py +++ b/src/senaite/app/supermodel/model.py @@ -206,10 +206,40 @@ def items(self): return list(self.iteritems()) def get_field(self, name, default=None): - accessor = getattr(self.instance, "getField", None) - if accessor is None: + """Returns the instance's field that matches for the given name + """ + try: + fields = api.get_fields(self.instance) + except api.APIError: return default - return accessor(name) + + field = fields.get(name) + if field: + return field + + # DX fields are not CamelCase but snake_case + name = "".join("_" + c.lower() if c.isupper() else c for c in name) + field = fields.get(name) + if field: + return field + + return default + + def get_field_value(self, name, default=None): + """Returns the value for the given name and current instance + """ + # always give priority to getters regardless of type + accessor_name = "get{}".format(name) + accessor = getattr(self.instance, accessor_name, _marker) + if accessor is not _marker: + return accessor() + + # rely on the fields + field = self.get_field(name) + if field: + return field.get(self.instance) + + return default def get(self, name, default=None): # Internal lookup in the data dict @@ -219,48 +249,31 @@ def get(self, name, default=None): if value is not _marker: return self.data[name] - # Field lookup on the instance - field = self.get_field(name) + # Try first to lookup the field value from the instance + value = self.get_field_value(name, default=_marker) - if field is None: + if value is _marker: # expose non-private members of the instance/brain to have access # to e.g. self.absolute_url (function object) or self.review_state - if not name.startswith("_") or not name.startswith("__"): - # check if the instance contains this attribute - instance = self.instance - instance_value = getattr(instance, name, _marker) - if instance_value is not _marker: - # return the value processed, but only if not a function - value = self.process_value(instance_value, safe_call=False) - if value != instance_value: - # Store value in the internal data dict - self.data[name] = value - return value - - # check if the brain contains this attribute - brain = self.brain - # NOTE: we might get no brain here if the object is temporary, - # e.g. during initialization! - if brain: - brain_value = getattr(brain, name, _marker) - if brain_value is not _marker: - return brain_value + if name.startswith("_"): + return default + + # check if the instance contains this attribute + instance = self.instance + instance_value = getattr(instance, name, _marker) + if instance_value is not _marker: + return instance_value + + # check if the brain contains this attribute + brain = self.brain + # NOTE: we might get no brain here if the object is temporary, + # e.g. during initialization! + if brain: + brain_value = getattr(brain, name, _marker) + if brain_value is not _marker: + return brain_value return default - else: - # Retrieve field value by accessor name - accessor = field.getAccessor(self.instance) - accessor_name = accessor.__name__ - - if self.is_temporary(self.instance) is False: - # Metadata lookup by accessor name - value = getattr(self.brain, accessor_name, _marker) - - if value is _marker: - logger.debug("Add metadata column '{}' to the catalog '{}' " - "to increase performance!" - .format(accessor_name, self.catalog.__name__)) - value = accessor() # Process value for publication value = self.process_value(value) @@ -270,7 +283,7 @@ def get(self, name, default=None): return value - def process_value(self, value, safe_call=True): + def process_value(self, value): """Process publication value """ # UID -> SuperModel @@ -298,7 +311,7 @@ def process_value(self, value, safe_call=True): elif isinstance(value, (dict)): return {k: self.process_value(v) for k, v in value.iteritems()} # Process function - elif safe_call and safe_callable(value): + elif safe_callable(value): return self.process_value(value()) # Always return the unprocessed value last return value From 572591534d53c6fee35d4a54f30272eeef415689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 9 Feb 2024 14:10:44 +0100 Subject: [PATCH 4/7] Changelog --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 01d5b18..3c18faf 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,7 @@ 2.6.0 (unreleased) ------------------ -- #19 Process values returned from instance except when a function +- #19 Support for Dexterity's non-CamelCase fieldnames - no changes yet From 01e13d91fbe3fdf7d67c5e0c25a3edcc6331cc7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 9 Feb 2024 14:35:19 +0100 Subject: [PATCH 5/7] Remove snake_case conversion --- src/senaite/app/supermodel/model.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/senaite/app/supermodel/model.py b/src/senaite/app/supermodel/model.py index db09225..7ce31ec 100644 --- a/src/senaite/app/supermodel/model.py +++ b/src/senaite/app/supermodel/model.py @@ -217,12 +217,6 @@ def get_field(self, name, default=None): if field: return field - # DX fields are not CamelCase but snake_case - name = "".join("_" + c.lower() if c.isupper() else c for c in name) - field = fields.get(name) - if field: - return field - return default def get_field_value(self, name, default=None): From 61001edc090f07eb69c14357ad494e2831d57c5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 9 Feb 2024 14:37:33 +0100 Subject: [PATCH 6/7] Changelog --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3c18faf..83afc4e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,7 @@ 2.6.0 (unreleased) ------------------ -- #19 Support for Dexterity's non-CamelCase fieldnames +- #19 Prioritize getters instead of fieldnames on value retrieval from instance - no changes yet From 8105a6d0f1cff932c4321558e0a6f2840d0ab6c2 Mon Sep 17 00:00:00 2001 From: Ramon Bartl Date: Fri, 9 Feb 2024 21:54:27 +0100 Subject: [PATCH 7/7] Removed placeholder --- CHANGES.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 83afc4e..59e0598 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -2,7 +2,6 @@ ------------------ - #19 Prioritize getters instead of fieldnames on value retrieval from instance -- no changes yet 2.5.0 (2024-01-03)