fix duplicated widgets key and values generation by calling update mo…

…re then once.

See CHANGES.txt for more infos.

CHANGES.txt

@@ -5,8 +5,18 @@ CHANGES
 Version 2.0.0 (unreleased)
 --------------------------
 
+- Bug: FieldWidgets update where appending keys and values within each
+  update call. Now the util.Manager uses a UniqueOrderedKeys implementation
+  which will ensure that we can't add duplicted manager keys. The implementation
+  also ensures that we can't override the UniqueOrderedKeys instace with a new
+  list by using a decorator. If this UniqueOrderedKeys implementation doesn't
+  fit for all use cases, we should probably use a customized UserList
+  implementation. Now we can call widgets.update() more then one time without
+  any side effect.
+
 - Bug: ButtonActions update where appending keys and values within each
-  update call. 
+  update call. Now we can call actions.update() more then one time without
+  any side effect.
 
 - Bug: The CollectionSequenceDataConverter no longer throws a
   "TypeError: 'NoneType' object is not iterable" when passed the value

src/z3c/form/button.py

@@ -254,7 +254,7 @@ def update(self):
         prefix = util.expandPrefix(self.form.prefix)
         prefix += util.expandPrefix(self.form.buttons.prefix)
         # Walk through each field, making an action out of it.
-        orderedNames = []
+        uniqueOrderedKeys = []
         for name, button in self.form.buttons.items():
             # Step 1: Only create an action for the button, if the condition is
             #         fulfilled.
@@ -286,13 +286,14 @@ def update(self):
             buttonAction.update()
             zope.event.notify(AfterWidgetUpdateEvent(buttonAction))
             # Step 7: Add the widget to the manager
-            orderedNames.append(name)
+            uniqueOrderedKeys.append(name)
             if newButton:
-                # allways keep the order given from button items
-                self._data_keys = orderedNames
                 self._data_values.append(buttonAction)
                 self._data[name] = buttonAction
                 zope.location.locate(buttonAction, self, name)
+            # allways ensure that we add all keys and keep the order given from
+            # button items
+            self._data_keys = uniqueOrderedKeys
 
 
 class ButtonActionHandler(action.ActionHandlerBase):

src/z3c/form/button.txt

@@ -93,6 +93,9 @@ provided interface, he is also responsible for remove existing actions.
 As you can see we still will get the old button action if we only call update:
 
   >>> actions.update()
+  >>> actions.keys()
+  ['apply', 'cancel']
+
   >>> actions['apply']
   <ButtonAction 'form.buttons.apply' u'Apply'>
 
@@ -128,7 +131,6 @@ Again, remove the old button action befor we call update:
   This button factory creates a button only once.
 
   >>> actions.update()
-
   >>> actions['apply'].css
   'happy'
 

src/z3c/form/field.py

@@ -223,6 +223,7 @@ def update(self):
         prefix = util.expandPrefix(self.form.prefix)
         prefix += util.expandPrefix(self.prefix)
         # Walk through each field, making a widget out of it.
+        uniqueOrderedKeys = []
         for field in self.form.fields.values():
             # Step 0. Determine whether the context should be ignored.
             ignoreContext = self.ignoreContext
@@ -242,14 +243,19 @@ def update(self):
                 if not dm.canWrite():
                     mode = interfaces.DISPLAY_MODE
             # Step 2: Get the widget for the given field.
-            factory = field.widgetFactory.get(mode)
-            if factory is not None:
+            shortName = field.__name__
+            newWidget = True
+            if shortName in self._data:
+                # reuse existing widget
+                widget = self._data[shortName]
+                newWidget = False
+            elif field.widgetFactory.get(mode) is not None:
+                factory = field.widgetFactory.get(mode)
                 widget = factory(field.field, self.request)
             else:
                 widget = zope.component.getMultiAdapter(
                     (field.field, self.request), interfaces.IFieldWidget)
             # Step 3: Set the prefix for the widget
-            shortName = field.__name__
             widget.name = prefix + shortName
             widget.id = (prefix + shortName).replace('.', '-')
             # Step 4: Set the context
@@ -269,10 +275,14 @@ def update(self):
             widget.update()
             zope.event.notify(AfterWidgetUpdateEvent(widget))
             # Step 9: Add the widget to the manager
-            self._data_keys.append(shortName)
-            self._data_values.append(widget)
-            self._data[shortName] = widget
-            zope.location.locate(widget, self, shortName)
+            uniqueOrderedKeys.append(shortName)
+            if newWidget:
+                self._data_values.append(widget)
+                self._data[shortName] = widget
+                zope.location.locate(widget, self, shortName)
+            # allways ensure that we add all keys and keep the order given from
+            # button items
+            self._data_keys = uniqueOrderedKeys
 
     def extract(self):
         """See interfaces.IWidgets"""

src/z3c/form/field.txt

@@ -417,28 +417,12 @@ in a particular order:
   >>> manager.keys()
   ['id', 'lastName', 'firstName']
 
-
-
-ISSUE: duplicated key, values if update ge called more then once. 
-NOTE: I'll fix this today.
-
-  >>> keys = list(manager.keys())
-  >>> values = list(manager.values())
-  >>> data = dict(manager._data)
+As you can see, if we call update twice, we still get the same amount and
+order of keys:
 
   >>> manager.update()
   >>> manager.keys()
-  ['id', 'lastName', 'firstName', 'id', 'lastName', 'firstName']
-
-set values back to manager since the test will get otherwise messup
-
-  >>> manager._data_keys = keys
-  >>> manager._data_values = values
-  >>> manager._data = data
-
-END ISSUE
-
-
+  ['id', 'lastName', 'firstName']
 
 Let's make sure that all enumerable mapping functions work correctly:
 

src/z3c/form/util.py

@@ -117,29 +117,62 @@ def extractFileName(form, id, cleanup=True, allowEmptyPostfix=False):
     return widget.filename
 
 
+class UniqueOrderedKeys(object):
+    """Ensures that we only ue unique keys in a list.
+    
+    This is usefull since we use the keys and values list only as ordered
+    keys and values addition for our data dict.
+    
+    Note, this list is only used for Manager keys and not for values since we
+    can't really compare values if we will get new instances of widgets or
+    actions.
+    """
+
+    def __init__(self, values=[]):
+        self.data = []
+        # ensure that we not intialize a list with duplicated key values
+        [self.data.append(value) for value in values]
+
+    def append(self, value):
+        if value in self.data:
+            raise ValueError(value)
+        self.data.append(value)
+
+
 class Manager(object):
     """Non-persistent IManager implementation."""
     zope.interface.implements(interfaces.IManager)
 
     def __init__(self, *args, **kw):
-        self._data_keys = []
+        self.__data_keys = UniqueOrderedKeys()
         self._data_values = []
         self._data = {}
 
+    @apply
+    def _data_keys():
+        """Use a special ordered list which will check for duplicated keys.""" 
+        def get(self):
+            return self.__data_keys
+        def set(self, values):
+            if isinstance(values, UniqueOrderedKeys):
+                self.__data_keys = values
+            else:
+                self.__data_keys = UniqueOrderedKeys(values)
+        return property(get, set)
+
     def __len__(self):
         return len(self._data_values)
 
     def __iter__(self):
-        return iter(self._data_keys)
+        return iter(self._data_keys.data)
 
     def __getitem__(self, name):
         return self._data[name]
 
     def __delitem__(self, name):
-        if name not in self._data_keys:
+        if name not in self._data_keys.data:
             raise KeyError(name)
-
-        del self._data_keys[self._data_keys.index(name)]
+        del self._data_keys.data[self._data_keys.data.index(name)]
         value = self._data[name]
         del self._data_values[self._data_values.index(value)]
         del self._data[name]
@@ -148,13 +181,13 @@ def get(self, name, default=None):
         return self._data.get(name, default)
 
     def keys(self):
-        return self._data_keys
+        return self._data_keys.data
 
     def values(self):
         return self._data_values
 
     def items(self):
-        return [(i, self._data[i]) for i in self._data_keys]
+        return [(i, self._data[i]) for i in self._data_keys.data]
 
     def __contains__(self, name):
         return bool(self.get(name))