From 991bb6124206a503545f5963e5048af45b1751eb Mon Sep 17 00:00:00 2001 From: Adrian Holovaty Date: Wed, 30 Nov 2005 04:08:46 +0000 Subject: [PATCH] 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 --- django/contrib/admin/views/main.py | 5 +- django/core/meta/__init__.py | 8 +- django/utils/datastructures.py | 112 +++++---- django/utils/httpwrappers.py | 87 ++++--- docs/request_response.txt | 64 ++++-- tests/othertests/httpwrappers.py | 358 +++++++++++++++++++++++++++++ 6 files changed, 515 insertions(+), 119 deletions(-) create mode 100644 tests/othertests/httpwrappers.py diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 0cb9dccb26..8a13433613 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -57,7 +57,7 @@ class ChangeList(object): 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 @@ class ChangeList(object): 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] diff --git a/django/core/meta/__init__.py b/django/core/meta/__init__.py index eb83545897..8b760dced7 100644 --- a/django/core/meta/__init__.py +++ b/django/core/meta/__init__.py @@ -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 [] 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) + """ + 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 __setitem__(self, key, value): - self.data[key] = [value] + def _setitem_list(self, key, value): + dict.__setitem__(self, key, [value]) + __setitem__ = _setitem_list - def __len__(self): - return len(self.data) - - def __contains__(self, key): - return self.data.has_key(key) - - 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): + def setdefault(self, key, default=None): + if key not in self: + self[key] = default + return self[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" - try: - self.data[key].append(item) - except KeyError: - self.data[key] = [item] - - def has_key(self, key): - return self.data.has_key(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 appendlist(self, key, value): + self._assert_mutable() + MultiValueDict.appendlist(self, key, value) + + def update(self, other_dict): + self._assert_mutable() + MultiValueDict.update(self, other_dict) + + def pop(self, key): + self._assert_mutable() + return MultiValueDict.pop(self, key) + + 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" - cp = MultiValueDict.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 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 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 keys(self): - self.assert_synchronized() - return self._keys - 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) diff --git a/docs/request_response.txt b/docs/request_response.txt index e0b7a01805..dea1cdb975 100644 --- a/docs/request_response.txt +++ b/docs/request_response.txt @@ -140,45 +140,67 @@ multiple values for the same key. That means you can't change attributes of ``request.POST`` and ``request.GET`` directly. -``QueryDict`` implements the following standard dictionary methods: - - * ``__repr__()`` +``QueryDict`` implements the all standard dictionary methods, because it's a +subclass of dictionary. Exceptions are outlined here: * ``__getitem__(key)`` -- Returns the value for the given key. If the key has more than one value, ``__getitem__()`` returns the last value. * ``__setitem__(key, value)`` -- Sets the given key to ``[value]`` - (a Python list whose single element is ``value``). + (a Python list whose single element is ``value``). Note that this, as + other dictionary functions that have side effects, can only be called on + an immutable ``QueryDict`` (one that was created via ``copy()``). - * ``__contains__(key)`` -- **New in Django development version.*** Returns - ``True`` if the given key exists. This lets you do, e.g., + * ``__contains__(key)`` -- **New in Django development version.** Returns + ``True`` if the given key is set. This lets you do, e.g., ``if "foo" in request.GET``. - * ``__len__()`` - * ``get(key, default)`` -- Uses the same logic as ``__getitem__()`` above, with a hook for returning a default value if the key doesn't exist. * ``has_key(key)`` + * ``setdefault(key, default)`` -- Just like the standard dictionary + ``setdefault()`` method, except it uses ``__setitem__`` internally. + + * ``update(other_dict)`` -- Takes either a ``QueryDict`` or standard + dictionary. Just like the standard dictionary ``update()`` method, except + it *appends* to the current dictionary items rather than replacing them. + For example:: + + >>> q = QueryDict('a=1') + >>> q = q.copy() # to make it mutable + >>> q.update({'a': '2'}) + >>> q.getlist('a') + ['1', '2'] + >>> q['a'] # returns the last + ['2'] + * ``items()`` -- Just like the standard dictionary ``items()`` method, - except this retains the order for values of duplicate keys, if any. For - example, if the original query string was ``"a=1&b=2&b=3"``, ``items()`` - will return ``[("a", ["1"]), ("b", ["2", "3"])]``, where the order of - ``["2", "3"]`` is guaranteed, but the order of ``a`` vs. ``b`` isn't. + except this uses the same last-value logic as ``__getitem()__``. For + example:: - * ``keys()`` + >>> q = QueryDict('a=1&a=2&a=3') + >>> q.items() + [('a', '3')] - * ``update(other_dict)`` + * ``values()`` -- Just like the standard dictionary ``values()`` method, + except this uses the same last-value logic as ``__getitem()__``. For + example:: -In addition, it has the following methods: + >>> q = QueryDict('a=1&a=2&a=3') + >>> q.values() + ['3'] + +In addition, ``QueryDict`` has the following methods: * ``copy()`` -- Returns a copy of the object, using ``copy.deepcopy()`` from the Python standard library. The copy will be mutable -- that is, you can change its values. * ``getlist(key)`` -- Returns the data with the requested key, as a Python - list. Returns an empty list if the key doesn't exist. + list. Returns an empty list if the key doesn't exist. It's guaranteed to + return a list of some sort. * ``setlist(key, list_)`` -- Sets the given key to ``list_`` (unlike ``__setitem__()``). @@ -186,6 +208,16 @@ In addition, it has the following methods: * ``appendlist(key, item)`` -- Appends an item to the internal list associated with key. + * ``setlistdefault(key, default_list)`` -- Just like ``setdefault``, except + it takes a list of values instead of a single value. + + * ``lists()`` -- Like ``items()``, except it includes all values, as a list, + for each member of the dictionary. For example:: + + >>> q = QueryDict('a=1&a=2&a=3') + >>> q.lists() + [('a', ['1', '2', '3'])] + * ``urlencode()`` -- Returns a string of the data in query-string format. Example: ``"a=2&b=3&b=5"``. diff --git a/tests/othertests/httpwrappers.py b/tests/othertests/httpwrappers.py new file mode 100644 index 0000000000..0564ee04f0 --- /dev/null +++ b/tests/othertests/httpwrappers.py @@ -0,0 +1,358 @@ +""" +################### +# Empty QueryDict # +################### + +>>> q = QueryDict('') + +>>> q['foo'] +Traceback (most recent call last): +... +MultiValueDictKeyError: "Key 'foo' not found in MultiValueDict {}" + +>>> q['something'] = 'bar' +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.get('foo', 'default') +'default' + +>>> q.getlist('foo') +[] + +>>> q.setlist('foo', ['bar', 'baz']) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.appendlist('foo', ['bar']) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.has_key('foo') +False + +>>> q.items() +[] + +>>> q.lists() +[] + +>>> q.keys() +[] + +>>> q.values() +[] + +>>> len(q) +0 + +>>> q.update({'foo': 'bar'}) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.pop('foo') +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.popitem() +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.clear() +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.setdefault('foo', 'bar') +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.urlencode() +'' + +################################### +# Mutable copy of empty QueryDict # +################################### + +>>> q = q.copy() + +>>> q['foo'] +Traceback (most recent call last): +... +MultiValueDictKeyError: "Key 'foo' not found in MultiValueDict {}" + +>>> q['name'] = 'john' + +>>> q['name'] +'john' + +>>> q.get('foo', 'default') +'default' + +>>> q.get('name', 'default') +'john' + +>>> q.getlist('name') +['john'] + +>>> q.getlist('foo') +[] + +>>> q.setlist('foo', ['bar', 'baz']) + +>>> q.get('foo', 'default') +'baz' + +>>> q.getlist('foo') +['bar', 'baz'] + +>>> q.appendlist('foo', 'another') + +>>> q.getlist('foo') +['bar', 'baz', 'another'] + +>>> q['foo'] +'another' + +>>> q.has_key('foo') +True + +>>> q.items() +[('foo', 'another'), ('name', 'john')] + +>>> q.lists() +[('foo', ['bar', 'baz', 'another']), ('name', ['john'])] + +>>> q.keys() +['foo', 'name'] + +>>> q.values() +['another', 'john'] + +>>> len(q) +2 + +>>> q.update({'foo': 'hello'}) + +# Displays last value +>>> q['foo'] +'hello' + +>>> q.get('foo', 'not available') +'hello' + +>>> q.getlist('foo') +['bar', 'baz', 'another', 'hello'] + +>>> q.pop('foo') +['bar', 'baz', 'another', 'hello'] + +>>> q.get('foo', 'not there') +'not there' + +>>> q.setdefault('foo', 'bar') +'bar' + +>>> q['foo'] +'bar' + +>>> q.getlist('foo') +['bar'] + +>>> q.urlencode() +'foo=bar&name=john' + +>>> q.clear() + +>>> len(q) +0 + +##################################### +# QueryDict with one key/value pair # +##################################### + +>>> q = QueryDict('foo=bar') + +>>> q['foo'] +'bar' + +>>> q['bar'] +Traceback (most recent call last): +... +MultiValueDictKeyError: "Key 'bar' not found in MultiValueDict {'foo': ['bar']}" + +>>> q['something'] = 'bar' +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.get('foo', 'default') +'bar' + +>>> q.get('bar', 'default') +'default' + +>>> q.getlist('foo') +['bar'] + +>>> q.getlist('bar') +[] + +>>> q.setlist('foo', ['bar', 'baz']) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.appendlist('foo', ['bar']) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.has_key('foo') +True + +>>> q.has_key('bar') +False + +>>> q.items() +[('foo', 'bar')] + +>>> q.lists() +[('foo', ['bar'])] + +>>> q.keys() +['foo'] + +>>> q.values() +['bar'] + +>>> len(q) +1 + +>>> q.update({'foo': 'bar'}) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.pop('foo') +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.popitem() +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.clear() +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.setdefault('foo', 'bar') +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.urlencode() +'foo=bar' + +##################################################### +# QueryDict with two key/value pairs with same keys # +##################################################### + +>>> q = QueryDict('vote=yes&vote=no') + +>>> q['vote'] +'no' + +>>> q['something'] = 'bar' +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.get('vote', 'default') +'no' + +>>> q.get('foo', 'default') +'default' + +>>> q.getlist('vote') +['yes', 'no'] + +>>> q.getlist('foo') +[] + +>>> q.setlist('foo', ['bar', 'baz']) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.appendlist('foo', ['bar']) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.has_key('vote') +True + +>>> q.has_key('foo') +False + +>>> q.items() +[('vote', 'no')] + +>>> q.lists() +[('vote', ['yes', 'no'])] + +>>> q.keys() +['vote'] + +>>> q.values() +['no'] + +>>> len(q) +1 + +>>> q.update({'foo': 'bar'}) +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.pop('foo') +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.popitem() +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.clear() +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.setdefault('foo', 'bar') +Traceback (most recent call last): +... +AttributeError: This QueryDict instance is immutable + +>>> q.urlencode() +'vote=yes&vote=no' + +""" + +from django.utils.httpwrappers import QueryDict + +if __name__ == "__main__": + import doctest + doctest.testmod()