To use field id and argument id were not enough to generate unique key.

So I added field's oid in addition.


git-svn-id: https://svn.erp5.org/repos/public/erp5/trunk@18747 20353a03-c40f-0410-a6d1-a30d3c3de9de

product/ERP5Form/Form.py

@@ -123,7 +123,9 @@ class TALESValue(StaticValue):
       # Proxyfield stores the "real" field in the request. Look if the
       # corresponding field exists in request, and use it as field in the
       # TALES context 
-      field = REQUEST.get('field__proxyfield_%s_%s' % (field.id, id), field)
+      field = REQUEST.get(
+        'field__proxyfield_%s_%s_%s' % (field.id, field._p_oid, id),
+        field)
 
     kw['field'] = field
 
@@ -259,7 +261,9 @@ def getFieldValue(self, field, id, **kw):
 def get_value(self, id, **kw):
   REQUEST = get_request()
   if REQUEST is not None:
-    field = REQUEST.get('field__proxyfield_%s_%s' % (self.id, id), self)
+    field = REQUEST.get(
+      'field__proxyfield_%s_%s_%s' % (self.id, self._p_oid, id),
+      self)
   else:
     field = self
 

product/ERP5Form/ProxyField.py

@@ -594,8 +594,12 @@ class ProxyField(ZMIField):
     field = self
     proxy_field = self.getTemplateField()
     if proxy_field is not None and REQUEST is not None:
-      field = REQUEST.get('field__proxyfield_%s_%s' % (self.id, id), self)
-      REQUEST.set('field__proxyfield_%s_%s' % (proxy_field.id, id), field)
+      field = REQUEST.get(
+        'field__proxyfield_%s_%s_%s' % (self.id, self._p_oid, id),
+        self)
+      REQUEST.set(
+        'field__proxyfield_%s_%s_%s' % (proxy_field.id, proxy_field._p_oid, id),
+        field)
 
     # Don't use cache if field is not stored in zodb, or if target field is
     # defined by a TALES

[Tomáš Peterka]

Hi @yusei , this change is producing invalid string keys into REQUEST (e.g. "field__proxyfield_my_checkbox_^@^@^@^@^@^X^K_enabled"). I suppose that _p_oid can contain binary data.

What is failing is then ZPublisher.HttpRequest.py:1439 keys.sort() where keys cannot be decoded using ascii codec thus cannot be sorted.

[Tomáš Peterka]

Do you remember @yusei why this change was necessary? Is it still the case? It constantly breaks HttpRequest in case TALES value using plain here/Script format for the reason I mentioned in 1132ce50 (comment 58517) . Why do we even save fields into the REQUEST? That seems like a hack. (@romain might be interested)

[Yusei Tahara]

Hi. As far as I remember, there may be more than one proxy field that has the same id. So, a pair of proxy_field.id and id was not enough. I think that a pair of oid and id was enough in reality. And I did not think about sort. Since oid is a struct packed value(See https://github.com/zopefoundation/ZODB/blob/c75a197deebc3a65aa521bda11bdaa83dadd8b48/src/ZODB/FileStorage/FileStorage.py#L1416), use struct.unpack(">Q", _p_oid)[0], you get int value.

And why we save fields into the REQUEST? Hmm, I do not remember exactly, it is surely a hack, I think it was important for performance.

[Yusei Tahara]

I think that we should be able to use getUid() instead. I will try it in for_testrunner_1 branch.

[Yusei Tahara]

Unfortunately , to replace _p_oid with getUid() was not enough, it did not work. I can investigate it more once I finished my current tasks.

[Tomáš Peterka]

Ok, until then I need to keep replacing all TALES expressions such as "here/Script" by "python: here.Script()" to avoid Publisher

[Tomáš Peterka]

Can we get rid of saving the field into REQUEST?

[Tomáš Peterka]

It seems that _p_oid is actually packed with ">8s8sQQHQ" https://github.com/zopefoundation/ZODB/blob/c75a197deebc3a65aa521bda11bdaa83dadd8b48/src/ZODB/FileStorage/format.py#L115 and this would be too hack-ish to unpack and use.