Permalink
Browse files

Fixed #736 -- Changed behavior of QueryDict items() to be more consis…

…tent, 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
  • Loading branch information...
1 parent 0ecdad8 commit 991bb6124206a503545f5963e5048af45b1751eb @adrianholovaty adrianholovaty committed Nov 30, 2005
@@ -57,7 +57,7 @@ def __init__(self, request, app_label, module_name):
self.get_modules_and_options(app_label, module_name, request)
self.get_search_parameters(request)
self.get_ordering()
- self.query = request.GET.get(SEARCH_VAR,'')
+ self.query = request.GET.get(SEARCH_VAR, '')
self.get_lookup_params()
self.get_results(request)
self.title = (self.is_popup
@@ -100,13 +100,12 @@ def get_modules_and_options(self, app_label, module_name, request):
def get_search_parameters(self, request):
# Get search parameters from the query string.
try:
- self.req_get = request.GET
self.page_num = int(request.GET.get(PAGE_VAR, 0))
except ValueError:
self.page_num = 0
self.show_all = request.GET.has_key(ALL_VAR)
self.is_popup = request.GET.has_key(IS_POPUP_VAR)
- self.params = dict(request.GET.copy())
+ self.params = dict((k, v) for k, v in request.GET.items())
if self.params.has_key(PAGE_VAR):
del self.params[PAGE_VAR]
@@ -1678,7 +1678,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
param = f.get_default()
params[f.attname] = param
-
if change:
params[opts.pk.attname] = self.obj_key
@@ -1710,7 +1709,7 @@ def manipulator_save(opts, klass, add, change, self, new_data):
if change and was_changed:
self.fields_changed.append(f.verbose_name)
- expanded_data = DotExpandedDict(new_data.data)
+ expanded_data = DotExpandedDict(dict(new_data))
# Save many-to-one objects. Example: Add the Choice objects for a Poll.
for related in opts.get_all_related_objects():
# Create obj_list, which is a DotExpandedDict such as this:
@@ -1724,7 +1723,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
obj_list.sort(lambda x, y: cmp(int(x[0]), int(y[0])))
params = {}
-
# For each related item...
for _, rel_new_data in obj_list:
@@ -1769,7 +1767,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
if param != None:
params[f.attname] = param
-
# Related links are a special case, because we have to
# manually set the "content_type_id" and "object_id" fields.
if opts.has_related_links and related.opts.module_name == 'relatedlinks':
@@ -1780,8 +1777,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
# Create the related item.
new_rel_obj = related.opts.get_model_module().Klass(**params)
-
-
# If all the core fields were provided (non-empty), save the item.
if all_cores_given:
new_rel_obj.save()
@@ -1814,7 +1809,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
new_rel_obj.delete()
self.fields_deleted.append('%s "%s"' % (related.opts.verbose_name, old_rel_obj))
-
# Save the order, if applicable.
if change and opts.get_ordered_objects():
order = new_data['order_'] and map(int, new_data['order_'].split(',')) or []
@@ -43,9 +43,9 @@ def has_key(self, key):
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 @@ def get(self, key, default):
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
@@ -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)
Oops, something went wrong.

0 comments on commit 991bb61

Please sign in to comment.