From e12d9cd50b1f78b738f2edc20bdc5f3cdb1c4d29 Mon Sep 17 00:00:00 2001
From: Tomas Peterka <tomas.peterka@nexedi.com>
Date: Wed, 3 Jan 2018 18:31:20 +0100
Subject: [PATCH] [hal_json_style] Improve search-value resolution (bug
20171215-133705C)
- introduce External Function for Acquisition resolution to get over permissions
- improve recursive TALES expression support
- closely follow value resolution from ListBox
- remove external method for default value resolution
---
.../extension.erp5.HalStyle.py | 11 +-
...ld_getDefaultValue.xml => Base_aqBase.xml} | 4 +-
.../erp5_hal_json_style/Base_aqSelf.xml | 28 +++++
.../ERP5Document_getHateoas.py | 103 ++++++++----------
.../erp5_ui_test/FooModule_listAsContext.py | 16 +++
.../erp5_ui_test/FooModule_listAsContext.xml | 62 +++++++++++
.../testListboxValueAsContext.xml | 58 ++++++++++
.../testListboxValueAsContext.zpt | 56 ++++++++++
8 files changed, 278 insertions(+), 60 deletions(-)
rename bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/{Field_getDefaultValue.xml => Base_aqBase.xml} (83%)
create mode 100644 bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_aqSelf.xml
create mode 100644 bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/FooModule_listAsContext.py
create mode 100644 bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/FooModule_listAsContext.xml
create mode 100644 bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_listbox_zuite/testListboxValueAsContext.xml
create mode 100644 bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_listbox_zuite/testListboxValueAsContext.zpt
diff --git a/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.py b/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.py
index 333016a197..55fcceb03f 100644
--- a/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.py
+++ b/bt5/erp5_hal_json_style/ExtensionTemplateItem/portal_components/extension.erp5.HalStyle.py
@@ -1,10 +1,15 @@
+from Acquisition import aq_self, aq_base
+
+def Base_aqSelf(self):
+ return aq_self(self)
+
+def Base_aqBase(self):
+ return aq_base(self)
+
def Field_getSubFieldKeyDict(self, field, field_id, key=None):
"""XXX"""
return field.generate_subfield_key(field_id, key=key)
-def Field_getDefaultValue(self, field, key, value, REQUEST):
- return field._get_default(key, value, REQUEST)
-
def WorkflowTool_listActionParameterList(self):
action_list = self.listActions()
info = self._getOAI(None)
diff --git a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Field_getDefaultValue.xml b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_aqBase.xml
similarity index 83%
rename from bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Field_getDefaultValue.xml
rename to bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_aqBase.xml
index 9268b3b28f..eb2ec46e42 100644
--- a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Field_getDefaultValue.xml
+++ b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_aqBase.xml
@@ -8,7 +8,7 @@
<dictionary>
<item>
<key> <string>_function</string> </key>
- <value> <string>Field_getDefaultValue</string> </value>
+ <value> <string>Base_aqBase</string> </value>
</item>
<item>
<key> <string>_module</string> </key>
@@ -16,7 +16,7 @@
</item>
<item>
<key> <string>id</string> </key>
- <value> <string>Field_getDefaultValue</string> </value>
+ <value> <string>Base_aqBase</string> </value>
</item>
<item>
<key> <string>title</string> </key>
diff --git a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_aqSelf.xml b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_aqSelf.xml
new file mode 100644
index 0000000000..3388b51894
--- /dev/null
+++ b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_aqSelf.xml
@@ -0,0 +1,28 @@
+<?xml version="1.0"?>
+<ZopeData>
+ <record id="1" aka="AAAAAAAAAAE=">
+ <pickle>
+ <global name="ExternalMethod" module="Products.ExternalMethod.ExternalMethod"/>
+ </pickle>
+ <pickle>
+ <dictionary>
+ <item>
+ <key> <string>_function</string> </key>
+ <value> <string>Base_aqSelf</string> </value>
+ </item>
+ <item>
+ <key> <string>_module</string> </key>
+ <value> <string>HalStyle</string> </value>
+ </item>
+ <item>
+ <key> <string>id</string> </key>
+ <value> <string>Base_aqSelf</string> </value>
+ </item>
+ <item>
+ <key> <string>title</string> </key>
+ <value> <string></string> </value>
+ </item>
+ </dictionary>
+ </pickle>
+ </record>
+</ZopeData>
diff --git a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
index 54b4447a98..797b7bea1b 100644
--- a/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
+++ b/bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
@@ -117,7 +117,7 @@ def selectKwargsForCallable(func, initial_kwargs, kwargs_dict):
def getUidAndAccessorForAnything(search_result, result_index, traversed_document):
- """Return unique ID, unique URL, getter and hasser for any combination of `search_result` and `index`.
+ """Return unique ID, unique URL, getter for any combination of `search_result` and `index`.
You want to use this method when you need a unique reference to random object in iterable (for example
result of list_method or stat_method). This will give you UID and URL for identification within JIO and
@@ -126,9 +126,7 @@ def getUidAndAccessorForAnything(search_result, result_index, traversed_document
Usage::
for i, random_object in enumerate(unknown_iterable):
- uid, url, getter, hasser = object_ids_and_access(random_object, i)
- if hasser(random_object, "linkable"):
- result[uid] = {'url': portal.abolute_url() + url}
+ uid, url, getter = object_ids_and_access(random_object, i)
value = getter(random_object, "value")
"""
if hasattr(search_result, "getObject"):
@@ -138,22 +136,14 @@ def getUidAndAccessorForAnything(search_result, result_index, traversed_document
contents_relative_url = getRealRelativeUrl(search_result)
# get property in secure way from documents
search_property_getter = getProtectedProperty
- def search_property_hasser (doc, attr):
- """Brains cannot access Properties - they use permissioned getters."""
- try:
- return doc.hasProperty(attr)
- except (AttributeError, Unauthorized) as e:
- log('Cannot state ownership of property "{}" on {!s} because of "{!s}"'.format(
- attr, doc, e))
- return False
- elif hasattr(search_result, "aq_self"):
+ elif hasattr(search_result, 'hasProperty') and hasattr(search_result, 'getId'):
# Zope products have at least ID thus we work with that
contents_uid = search_result.uid
# either we got a document with relativeUrl or we got product and use ID
contents_relative_url = getRealRelativeUrl(search_result) or search_result.getId()
# documents and products have the same way of accessing properties
search_property_getter = getProtectedProperty
- search_property_hasser = lambda doc, attr: doc.hasProperty(attr)
+ # search_property_hasser = lambda doc, attr: doc.hasProperty(attr)
else:
# In case of reports the `search_result` can be list of
# PythonScripts.standard._Object - a reimplementation of plain dictionary
@@ -167,13 +157,12 @@ def getUidAndAccessorForAnything(search_result, result_index, traversed_document
contents_relative_url = "{}/{}".format(traversed_document.getRelativeUrl(), contents_uid)
# property getter must be simple __getattr__ implementation
search_property_getter = lambda obj, attr: getattr(obj, attr, None)
- search_property_hasser = lambda obj, attr: hasattr(obj, attr)
- return contents_uid, contents_relative_url, search_property_getter, search_property_hasser
+ return contents_uid, contents_relative_url, search_property_getter
-def getAttrFromAnything(search_result, select, search_property_getter, search_property_hasser, kwargs):
- """Given `search_result` extract value named `select` using helper getter/hasser.
+def getAttrFromAnything(search_result, select, search_property_getter, kwargs):
+ """Given `search_result` extract value named `select` using helper getter.
:param search_result: any dict-like object (usually dict or Brain or Document)
:param select: field name (can represent actual attributes, Properties or even Scripts)
@@ -201,43 +190,46 @@ def getAttrFromAnything(search_result, select, search_property_getter, search_pr
# or obvious getter (starts with "get" or Capital letter - Script)
accessor_name = select
- # 1. resolve attribute on a raw object (all wrappers removed) using
- # lowest-level secure getattr method given object type
- raw_search_result = search_result
- if hasattr(search_result, 'aq_base'):
- raw_search_result = search_result.aq_base
- # BUT! only if there is no accessor (because that is the prefered way)
- if search_property_hasser(raw_search_result, select) and not hasattr(raw_search_result, accessor_name):
- contents_value = search_property_getter(raw_search_result, select)
+ # Following value resolution is copied from product/ERP5Form/ListBox.py#L2223
+ # 1. resolve attribute on unwrapped object using getter
+ try:
+ unwrapped_search_result = search_result.Base_aqSelf()
+ if contents_value is None and unwrapped_search_result is not None:
+ try:
+ if getattr(unwrapped_search_result, select, None) is not None:
+ # I know this looks suspicious but product/ERP5Form/ListBox.py#L2224
+ contents_value = getattr(search_result, select)
+ except Unauthorized:
+ pass
+ except AttributeError:
+ pass # search_result is not a Document
# 2. use the fact that wrappers (brain or acquisition wrapper) use
# permissioned getters
- unwrapped_search_result = search_result
- if hasattr(search_result, 'aq_self'):
- unwrapped_search_result = search_result.aq_self
+ try:
+ raw_search_result = search_result.Base_aqBase()
+ if contents_value is None and raw_search_result is not None:
+ try:
+ if getattr(raw_search_result, accessor_name, None) is not None:
+ contents_value = getattr(search_result, accessor_name, None)
+ except Unauthorized:
+ pass
+ except AttributeError:
+ pass # search_result is not a Document
+ # Following part resolves values on other objects than Documents
+ # Prefer getter (accessor) than raw property name
if contents_value is None:
- # again we check on a unwrapped object to avoid acquisition resolution
- # which would certainly find something which we don't want
try:
- if hasattr(raw_search_result, accessor_name) and callable(getattr(search_result, accessor_name)):
- # test on raw object but get the actual accessor using wrapper and acquisition
- # do not call it here - it will be done later in generic call part
- contents_value = getattr(search_result, accessor_name)
- except (AttributeError, KeyError, Unauthorized) as error:
- log("Could not evaluate {} nor {} on {} with error {!s}".format(
- select, accessor_name, search_result, error), level=100) # WARNING
-
- if contents_value is None and search_property_hasser(search_result, select):
- # maybe it is just a attribute
- contents_value = search_property_getter(search_result, select)
+ contents_value = getattr(search_result, accessor_name)
+ except (AttributeError, Unauthorized):
+ pass
if contents_value is None:
try:
- contents_value = getattr(search_result, select, None)
- except (Unauthorized, AttributeError, KeyError) as error:
- log("Cannot resolve {} on {!s} because {!s}".format(
- select, raw_search_result, error), level=100)
+ contents_value = search_property_getter(search_result, select)
+ except (AttributeError, Unauthorized):
+ pass
if callable(contents_value):
has_mandatory_param = False
@@ -1430,7 +1422,7 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
# we can render fields which need 'here' to be set to currently rendered document
#REQUEST.set('here', search_result)
contents_item = {}
- contents_uid, contents_relative_url, property_getter, property_hasser = \
+ contents_uid, contents_relative_url, property_getter = \
getUidAndAccessorForAnything(search_result, result_index, traversed_document)
# _links.self.href is mandatory for JIO so it can create reference to the
@@ -1453,20 +1445,21 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
'value': contents_uid
}
+ # if default value is given by evaluating Tales expression then we only
+ # put "cell" to request (expected by tales) and let the field evaluate
+ REQUEST.set('cell', search_result)
for select in select_list:
default_field_value = None
# every `select` can have a template field or be just a exotic getter for a value
if editable_field_dict.has_key(select):
# cell has a Form Field template thus render it using the field
# fields are nice because they are standard
- REQUEST.set('cell', search_result)
- # if default value is given by evaluating Tales expression then we only
- # put "cell" to request (expected by tales) and let the field evaluate
- if (not editable_field_dict[select].get_tales("default") and # no tales
- (not hasattr(editable_field_dict[select], "get_recursive_tales") or # no recursive tales
+ if (not editable_field_dict[select].tales.get("default", "") and # no tales
+ (editable_field_dict[select].meta_type != 'ProxyField' or
+ not hasattr(editable_field_dict[select], "get_recursive_tales") or # no recursive tales
editable_field_dict[select].get_recursive_tales("default") == "")):
# if there is no tales expr (or is empty) we extract the value from search result
- default_field_value = getAttrFromAnything(search_result, select, property_getter, property_hasser, {})
+ default_field_value = getAttrFromAnything(search_result, select, property_getter, {})
contents_item[select] = renderField(
traversed_document,
@@ -1475,13 +1468,13 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
value=default_field_value,
key='field_%s_%s' % (editable_field_dict[select].id, contents_uid))
- REQUEST.other.pop('cell', None)
else:
# most of the complicated magic happens here - we need to resolve field names
# given search_result. This name can unfortunately mean almost anything from
# a key name to Python Script with variable number of input parameters.
- contents_item[select] = getAttrFromAnything(search_result, select, property_getter, property_hasser, {'brain': search_result})
+ contents_item[select] = getAttrFromAnything(search_result, select, property_getter, {'brain': search_result})
# endfor select
+ REQUEST.other.pop('cell', None)
contents_list.append(contents_item)
result_dict['_embedded']['contents'] = contents_list
diff --git a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/FooModule_listAsContext.py b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/FooModule_listAsContext.py
new file mode 100644
index 0000000000..fa842b1e03
--- /dev/null
+++ b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/FooModule_listAsContext.py
@@ -0,0 +1,16 @@
+"""Test for bug 20171215-133705C
+
+acContext creates a temporary copy of Document with changed Properties.
+In related test we ensure that property is resolved before getter because
+that is how it works in the old UI.
+"""
+foo_list = list(context.contentValues())
+foo_context_list = [foo.asContext(state="Couscous") for foo in foo_list]
+
+"""
+for foo, foo_context in zip(foo_list, foo_context_list):
+ context.log("Foo.state {!s}, Foo.getState() {!s}, Foo.asContext().state {!s}, Foo.asContext().getState() {!s}".format(
+ foo.getProperty('state'), foo.getState(), getattr(foo_context, 'state'), foo_context.getState()))
+"""
+
+return foo_context_list
diff --git a/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/FooModule_listAsContext.xml b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/FooModule_listAsContext.xml
new file mode 100644
index 0000000000..d6510e616a
--- /dev/null
+++ b/bt5/erp5_ui_test/SkinTemplateItem/portal_skins/erp5_ui_test/FooModule_listAsContext.xml
@@ -0,0 +1,62 @@
+<?xml version="1.0"?>
+<ZopeData>
+ <record id="1" aka="AAAAAAAAAAE=">
+ <pickle>
+ <global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
+ </pickle>
+ <pickle>
+ <dictionary>
+ <item>
+ <key> <string>Script_magic</string> </key>
+ <value> <int>3</int> </value>
+ </item>
+ <item>
+ <key> <string>_bind_names</string> </key>
+ <value>
+ <object>
+ <klass>
+ <global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
+ </klass>
+ <tuple/>
+ <state>
+ <dictionary>
+ <item>
+ <key> <string>_asgns</string> </key>
+ <value>
+ <dictionary>
+ <item>
+ <key> <string>name_container</string> </key>
+ <value> <string>container</string> </value>
+ </item>
+ <item>
+ <key> <string>name_context</string> </key>
+ <value> <string>context</string> </value>
+ </item>
+ <item>
+ <key> <string>name_m_self</string> </key>
+ <value> <string>script</string> </value>
+ </item>
+ <item>
+ <key> <string>name_subpath</string> </key>
+ <value> <string>traverse_subpath</string> </value>
+ </item>
+ </dictionary>
+ </value>
+ </item>
+ </dictionary>
+ </state>
+ </object>
+ </value>
+ </item>
+ <item>
+ <key> <string>_params</string> </key>
+ <value> <string>**kwargs</string> </value>
+ </item>
+ <item>
+ <key> <string>id</string> </key>
+ <value> <string>FooModule_listAsContext</string> </value>
+ </item>
+ </dictionary>
+ </pickle>
+ </record>
+</ZopeData>
diff --git a/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_listbox_zuite/testListboxValueAsContext.xml b/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_listbox_zuite/testListboxValueAsContext.xml
new file mode 100644
index 0000000000..96de392a93
--- /dev/null
+++ b/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_listbox_zuite/testListboxValueAsContext.xml
@@ -0,0 +1,58 @@
+<?xml version="1.0"?>
+<ZopeData>
+ <record id="1" aka="AAAAAAAAAAE=">
+ <pickle>
+ <global name="ZopePageTemplate" module="Products.PageTemplates.ZopePageTemplate"/>
+ </pickle>
+ <pickle>
+ <dictionary>
+ <item>
+ <key> <string>_bind_names</string> </key>
+ <value>
+ <object>
+ <klass>
+ <global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
+ </klass>
+ <tuple/>
+ <state>
+ <dictionary>
+ <item>
+ <key> <string>_asgns</string> </key>
+ <value>
+ <dictionary>
+ <item>
+ <key> <string>name_subpath</string> </key>
+ <value> <string>traverse_subpath</string> </value>
+ </item>
+ </dictionary>
+ </value>
+ </item>
+ </dictionary>
+ </state>
+ </object>
+ </value>
+ </item>
+ <item>
+ <key> <string>content_type</string> </key>
+ <value> <string>text/html</string> </value>
+ </item>
+ <item>
+ <key> <string>expand</string> </key>
+ <value> <int>0</int> </value>
+ </item>
+ <item>
+ <key> <string>id</string> </key>
+ <value> <string>testListboxValueAsContext</string> </value>
+ </item>
+ <item>
+ <key> <string>output_encoding</string> </key>
+ <value> <string>utf-8</string> </value>
+ </item>
+ <item>
+ <key> <string>title</string> </key>
+ <value> <unicode></unicode> </value>
+ </item>
+ </dictionary>
+ </pickle>
+ </record>
+</ZopeData>
diff --git a/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_listbox_zuite/testListboxValueAsContext.zpt b/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_listbox_zuite/testListboxValueAsContext.zpt
new file mode 100644
index 0000000000..7e8968f8d2
--- /dev/null
+++ b/bt5/erp5_web_renderjs_ui_test/PathTemplateItem/portal_tests/renderjs_ui_listbox_zuite/testListboxValueAsContext.zpt
@@ -0,0 +1,56 @@
+<html xmlns:tal="http://xml.zope.org/namespaces/tal"
+ xmlns:metal="http://xml.zope.org/namespaces/metal">
+<head>
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
+<title>Test ListBox propetry resolution before getter</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">Bug 20171215-133705C - property should be resolved before getter if different and not empty</td></tr>
+</thead><tbody>
+
+<tr><td>store</td>
+ <td>https://softinst81338.host.vifib.net/erp5</td>
+ <td>base_url</td></tr>
+
+
+<tal:block metal:use-macro="here/PTZuite_CommonTemplate/macros/init" />
+<tr><td>open</td>
+ <td>${base_url}/foo_module/ListBoxZuite_reset</td><td></td></tr>
+<tr><td>assertTextPresent</td>
+ <td>Reset Successfully.</td><td></td></tr>
+
+<tr><td>open</td>
+ <td>${base_url}/foo_module/FooModule_createObjects?start:int=1&num:int=2&create_line:int=0</td><td></td></tr>
+<tr><td>assertTextPresent</td>
+ <td>Created Successfully.</td><td></td></tr>
+
+<tr><td>open</td>
+ <td>${base_url}/foo_module/FooModule_viewFooList/listbox/ListBox_setPropertyList?field_list_method=FooModule_listAsContext&field_columns=id%7CID%0Astate%7CState</td>
+ <td></td></tr>
+<tr><td>assertTextPresent</td>
+ <td>Set Successfully.</td><td></td></tr>
+
+<tal:block metal:use-macro="here/Zuite_CommonTemplate/macros/wait_for_activities" />
+
+<!-- Shortcut for full renderjs url -->
+<tr><td>store</td>
+ <td>${base_url}/web_site_module/renderjs_runner</td>
+ <td>renderjs_url</td></tr>
+<tr><td>open</td>
+ <td>${renderjs_url}/#/foo_module</td><td></td></tr>
+
+<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/wait_for_listbox_loaded" />
+
+<!-- Test for value "Couscous" (which is obviously the correct one) instead of "current"
+ Why? Please read description in the bug 20171215-133705C. Custom list_method returns
+ a list of [Foo.asContext(state="Couscous")] and getState seems to refer to original value.
+-->
+<tr><td>assertText</td>
+ <td>//div[@data-gadget-scope="field_listbox"]//table/tbody/tr[1]/td[2]/a</td>
+ <td>Couscous</td></tr>
+
+</tbody></table>
+</body>
+</html>
\ No newline at end of file
--
2.30.9