Fixed #736 -- Changed behavior of QueryDict items() to be more consistent, fixed mutability holes, gave MultiValueDict many more dictionary methods and added unit tests. Thanks, Kieran Holland. This is slightly backwards-incompatible if you happened to rely on the behavior of QueryDict.items(), which is highly unlikely.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@1504 bcc190cf-cafb-0310-a4f2-bffc1f526a37

2 files changed, 106 insertions, 93 deletions

diff --git a/django/utils/datastructures.py b/django/utils/datastructures.py
index 2779e2ae00..81e4b30f6e 100644
--- a/django/utils/datastructures.py
+++ b/django/utils/datastructures.py
@@ -43,9 +43,9 @@ class MergeDict:
 class MultiValueDictKeyError(KeyError):
     pass
 
-class MultiValueDict:
+class MultiValueDict(dict):
     """
-    A dictionary-like class customized to deal with multiple values for the same key.
+    A subclass of dictionary customized to handle multiple values for the same key.
 
     >>> d = MultiValueDict({'name': ['Adrian', 'Simon'], 'position': ['Developer']})
     >>> d['name']
@@ -60,35 +60,32 @@ class MultiValueDict:
     which returns a list for every key, even though most Web forms submit
     single name-value pairs.
     """
-    def __init__(self, key_to_list_mapping=None):
-        self.data = key_to_list_mapping or {}
-
-    def __repr__(self):
-        return repr(self.data)
+    def __init__(self, key_to_list_mapping=()):
+        dict.__init__(self, key_to_list_mapping)
 
     def __getitem__(self, key):
-        "Returns the data value for this key; raises KeyError if not found"
-        if self.data.has_key(key):
-            try:
-                return self.data[key][-1] # in case of duplicates, use last value ([-1])
-            except IndexError:
-                return []
-        raise MultiValueDictKeyError, "Key '%s' not found in MultiValueDict %s" % (key, self.data)
-
-    def __setitem__(self, key, value):
-        self.data[key] = [value]
-
-    def __len__(self):
-        return len(self.data)
+        """
+        Returns the last data value for this key, or [] if it's an empty list;
+        raises KeyError if not found.
+        """
+        try:
+            list_ = dict.__getitem__(self, key)
+        except KeyError:
+            raise MultiValueDictKeyError, "Key %r not found in MultiValueDict %r" % (key, self)
+        try:
+            return list_[-1]
+        except IndexError:
+            return []
 
-    def __contains__(self, key):
-        return self.data.has_key(key)
+    def _setitem_list(self, key, value):
+        dict.__setitem__(self, key, [value])
+    __setitem__ = _setitem_list
 
-    def get(self, key, default):
+    def get(self, key, default=None):
         "Returns the default value if the requested data doesn't exist"
         try:
             val = self[key]
-        except (KeyError, IndexError):
+        except KeyError:
             return default
         if val == []:
             return default
@@ -97,47 +94,64 @@ class MultiValueDict:
     def getlist(self, key):
         "Returns an empty list if the requested data doesn't exist"
         try:
-            return self.data[key]
+            return dict.__getitem__(self, key)
         except KeyError:
             return []
 
     def setlist(self, key, list_):
-        self.data[key] = list_
+        dict.__setitem__(self, key, list_)
 
-    def appendlist(self, key, item):
-        "Appends an item to the internal list associated with key"
-        try:
-            self.data[key].append(item)
-        except KeyError:
-            self.data[key] = [item]
+    def setdefault(self, key, default=None):
+        if key not in self:
+            self[key] = default
+        return self[key]
 
-    def has_key(self, key):
-        return self.data.has_key(key)
+    def setlistdefault(self, key, default_list=()):
+        if key not in self:
+            self.setlist(key, default_list)
+        return self.getlist(key)
+
+    def appendlist(self, key, value):
+        "Appends an item to the internal list associated with key"
+        self.setlistdefault(key, [])
+        dict.__setitem__(self, key, self.getlist(key) + [value])
 
     def items(self):
-        # we don't just return self.data.items() here, because we want to use
-        # self.__getitem__() to access the values as *strings*, not lists
-        return [(key, self[key]) for key in self.data.keys()]
+        """
+        Returns a list of (key, value) pairs, where value is the last item in
+        the list associated with the key.
+        """
+        return [(key, self[key]) for key in self.keys()]
 
-    def keys(self):
-        return self.data.keys()
+    def lists(self):
+        "Returns a list of (key, list) pairs."
+        return dict.items(self)
 
-    def update(self, other_dict):
-        if isinstance(other_dict, MultiValueDict):
-            for key, value_list in other_dict.data.items():
-                self.data.setdefault(key, []).extend(value_list)
-        elif type(other_dict) == type({}):
-            for key, value in other_dict.items():
-                self.data.setdefault(key, []).append(value)
-        else:
-            raise ValueError, "MultiValueDict.update() takes either a MultiValueDict or dictionary"
+    def values(self):
+        "Returns a list of the last value on every key list."
+        return [self[key] for key in self.keys()]
 
     def copy(self):
-        "Returns a copy of this object"
+        "Returns a copy of this object."
         import copy
+        # Our custom __setitem__ must be disabled for copying machinery.
+        MultiValueDict.__setitem__ = dict.__setitem__
         cp = copy.deepcopy(self)
+        MultiValueDict.__setitem__ = MultiValueDict._setitem_list
         return cp
 
+    def update(self, other_dict):
+        "update() extends rather than replaces existing key lists."
+        if isinstance(other_dict, MultiValueDict):
+            for key, value_list in other_dict.lists():
+                self.setlistdefault(key, []).extend(value_list)
+        else:
+            try:
+                for key, value in other_dict.items():
+                    self.setlistdefault(key, []).append(value)
+            except TypeError:
+                raise ValueError, "MultiValueDict.update() takes either a MultiValueDict or dictionary"
+
 class DotExpandedDict(dict):
     """
     A special dictionary constructor that takes a dictionary in which the keys
diff --git a/django/utils/httpwrappers.py b/django/utils/httpwrappers.py
index 305ef51607..32f2bfb8f7 100644
--- a/django/utils/httpwrappers.py
+++ b/django/utils/httpwrappers.py
@@ -67,63 +67,62 @@ class QueryDict(MultiValueDict):
     """A specialized MultiValueDict that takes a query string when initialized.
     This is immutable unless you create a copy of it."""
     def __init__(self, query_string):
-        if not query_string:
-            self.data = {}
-            self._keys = []
-        else:
-            self.data = {}
-            self._keys = []
-            for name, value in parse_qsl(query_string, True): # keep_blank_values=True
-                if name in self.data:
-                    self.data[name].append(value)
-                else:
-                    self.data[name] = [value]
-                if name not in self._keys:
-                    self._keys.append(name)
+        MultiValueDict.__init__(self)
+        self._mutable = True
+        for key, value in parse_qsl(query_string, True): # keep_blank_values=True
+            self.appendlist(key, value)
         self._mutable = False
 
-    def __setitem__(self, key, value):
+    def _assert_mutable(self):
         if not self._mutable:
             raise AttributeError, "This QueryDict instance is immutable"
-        else:
-            self.data[key] = [value]
-            if not key in self._keys:
-                self._keys.append(key)
+
+    def _setitem_if_mutable(self, key, value):
+        self._assert_mutable()
+        MultiValueDict.__setitem__(self, key, value)
+    __setitem__ = _setitem_if_mutable
 
     def setlist(self, key, list_):
-        if not self._mutable:
-            raise AttributeError, "This QueryDict instance is immutable"
-        else:
-            self.data[key] = list_
-            if not key in self._keys:
-                self._keys.append(key)
+        self._assert_mutable()
+        MultiValueDict.setlist(self, key, list_)
 
-    def copy(self):
-        "Returns a mutable copy of this object"
-        cp = MultiValueDict.copy(self)
-        cp._mutable = True
-        return cp
+    def appendlist(self, key, value):
+        self._assert_mutable()
+        MultiValueDict.appendlist(self, key, value)
 
-    def assert_synchronized(self):
-        assert(len(self._keys) == len(self.data.keys())), \
-            "QueryDict data structure is out of sync: %s %s" % (str(self._keys), str(self.data))
+    def update(self, other_dict):
+        self._assert_mutable()
+        MultiValueDict.update(self, other_dict)
 
-    def items(self):
-        "Respect order preserved by self._keys"
-        self.assert_synchronized()
-        items = []
-        for key in self._keys:
-            if key in self.data:
-                items.append((key, self.data[key][0]))
-        return items
+    def pop(self, key):
+        self._assert_mutable()
+        return MultiValueDict.pop(self, key)
 
-    def keys(self):
-        self.assert_synchronized()
-        return self._keys
+    def popitem(self):
+        self._assert_mutable()
+        return MultiValueDict.popitem(self)
+
+    def clear(self):
+        self._assert_mutable()
+        MultiValueDict.clear(self)
+
+    def setdefault(self, *args):
+        self._assert_mutable()
+        return MultiValueDict.setdefault(self, *args)
+
+    def copy(self):
+        "Returns a mutable copy of this object."
+        import copy
+        # Our custom __setitem__ must be disabled for copying machinery.
+        QueryDict.__setitem__ = dict.__setitem__
+        cp = copy.deepcopy(self)
+        QueryDict.__setitem__ = QueryDict._setitem_if_mutable
+        cp._mutable = True
+        return cp
 
     def urlencode(self):
         output = []
-        for k, list_ in self.data.items():
+        for k, list_ in self.lists():
             output.extend([urlencode({k: v}) for v in list_])
         return '&'.join(output)