Skip to content

Commit 991bb61

Browse files
adrianholovatyadrianholovaty
committed
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
1 parent 0ecdad8 commit 991bb61

File tree

6 files changed

+515
-119
lines changed

6 files changed

+515
-119
lines changed

‎django/contrib/admin/views/main.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def __init__(self, request, app_label, module_name):
5757
self.get_modules_and_options(app_label, module_name, request)
5858
self.get_search_parameters(request)
5959
self.get_ordering()
60-
self.query = request.GET.get(SEARCH_VAR,'')
60+
self.query = request.GET.get(SEARCH_VAR, '')
6161
self.get_lookup_params()
6262
self.get_results(request)
6363
self.title = (self.is_popup
@@ -100,13 +100,12 @@ def get_modules_and_options(self, app_label, module_name, request):
100100
def get_search_parameters(self, request):
101101
# Get search parameters from the query string.
102102
try:
103-
self.req_get = request.GET
104103
self.page_num = int(request.GET.get(PAGE_VAR, 0))
105104
except ValueError:
106105
self.page_num = 0
107106
self.show_all = request.GET.has_key(ALL_VAR)
108107
self.is_popup = request.GET.has_key(IS_POPUP_VAR)
109-
self.params = dict(request.GET.copy())
108+
self.params = dict((k, v) for k, v in request.GET.items())
110109
if self.params.has_key(PAGE_VAR):
111110
del self.params[PAGE_VAR]
112111

‎django/core/meta/__init__.py

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1678,7 +1678,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
16781678
param = f.get_default()
16791679
params[f.attname] = param
16801680

1681-
16821681
if change:
16831682
params[opts.pk.attname] = self.obj_key
16841683

@@ -1710,7 +1709,7 @@ def manipulator_save(opts, klass, add, change, self, new_data):
17101709
if change and was_changed:
17111710
self.fields_changed.append(f.verbose_name)
17121711

1713-
expanded_data = DotExpandedDict(new_data.data)
1712+
expanded_data = DotExpandedDict(dict(new_data))
17141713
# Save many-to-one objects. Example: Add the Choice objects for a Poll.
17151714
for related in opts.get_all_related_objects():
17161715
# Create obj_list, which is a DotExpandedDict such as this:
@@ -1724,7 +1723,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
17241723
obj_list.sort(lambda x, y: cmp(int(x[0]), int(y[0])))
17251724
params = {}
17261725

1727-
17281726
# For each related item...
17291727
for _, rel_new_data in obj_list:
17301728

@@ -1769,7 +1767,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
17691767
if param != None:
17701768
params[f.attname] = param
17711769

1772-
17731770
# Related links are a special case, because we have to
17741771
# manually set the "content_type_id" and "object_id" fields.
17751772
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):
17801777
# Create the related item.
17811778
new_rel_obj = related.opts.get_model_module().Klass(**params)
17821779

1783-
1784-
17851780
# If all the core fields were provided (non-empty), save the item.
17861781
if all_cores_given:
17871782
new_rel_obj.save()
@@ -1814,7 +1809,6 @@ def manipulator_save(opts, klass, add, change, self, new_data):
18141809
new_rel_obj.delete()
18151810
self.fields_deleted.append('%s "%s"' % (related.opts.verbose_name, old_rel_obj))
18161811

1817-
18181812
# Save the order, if applicable.
18191813
if change and opts.get_ordered_objects():
18201814
order = new_data['order_'] and map(int, new_data['order_'].split(',')) or []

‎django/utils/datastructures.py

Lines changed: 63 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ def has_key(self, key):
4343
class MultiValueDictKeyError(KeyError):
4444
pass
4545

46-
class MultiValueDict:
46+
class MultiValueDict(dict):
4747
"""
48-
A dictionary-like class customized to deal with multiple values for the same key.
48+
A subclass of dictionary customized to handle multiple values for the same key.
4949
5050
>>> d = MultiValueDict({'name': ['Adrian', 'Simon'], 'position': ['Developer']})
5151
>>> d['name']
@@ -60,35 +60,32 @@ class MultiValueDict:
6060
which returns a list for every key, even though most Web forms submit
6161
single name-value pairs.
6262
"""
63-
def __init__(self, key_to_list_mapping=None):
64-
self.data = key_to_list_mapping or {}
65-
66-
def __repr__(self):
67-
return repr(self.data)
63+
def __init__(self, key_to_list_mapping=()):
64+
dict.__init__(self, key_to_list_mapping)
6865

6966
def __getitem__(self, key):
70-
"Returns the data value for this key; raises KeyError if not found"
71-
if self.data.has_key(key):
72-
try:
73-
return self.data[key][-1] # in case of duplicates, use last value ([-1])
74-
except IndexError:
75-
return []
76-
raise MultiValueDictKeyError, "Key '%s' not found in MultiValueDict %s" % (key, self.data)
77-
78-
def __setitem__(self, key, value):
79-
self.data[key] = [value]
80-
81-
def __len__(self):
82-
return len(self.data)
67+
"""
68+
Returns the last data value for this key, or [] if it's an empty list;
69+
raises KeyError if not found.
70+
"""
71+
try:
72+
list_ = dict.__getitem__(self, key)
73+
except KeyError:
74+
raise MultiValueDictKeyError, "Key %r not found in MultiValueDict %r" % (key, self)
75+
try:
76+
return list_[-1]
77+
except IndexError:
78+
return []
8379

84-
def __contains__(self, key):
85-
return self.data.has_key(key)
80+
def _setitem_list(self, key, value):
81+
dict.__setitem__(self, key, [value])
82+
__setitem__ = _setitem_list
8683

87-
def get(self, key, default):
84+
def get(self, key, default=None):
8885
"Returns the default value if the requested data doesn't exist"
8986
try:
9087
val = self[key]
91-
except (KeyError, IndexError):
88+
except KeyError:
9289
return default
9390
if val == []:
9491
return default
@@ -97,47 +94,64 @@ def get(self, key, default):
9794
def getlist(self, key):
9895
"Returns an empty list if the requested data doesn't exist"
9996
try:
100-
return self.data[key]
97+
return dict.__getitem__(self, key)
10198
except KeyError:
10299
return []
103100

104101
def setlist(self, key, list_):
105-
self.data[key] = list_
102+
dict.__setitem__(self, key, list_)
106103

107-
def appendlist(self, key, item):
108-
"Appends an item to the internal list associated with key"
109-
try:
110-
self.data[key].append(item)
111-
except KeyError:
112-
self.data[key] = [item]
104+
def setdefault(self, key, default=None):
105+
if key not in self:
106+
self[key] = default
107+
return self[key]
113108

114-
def has_key(self, key):
115-
return self.data.has_key(key)
109+
def setlistdefault(self, key, default_list=()):
110+
if key not in self:
111+
self.setlist(key, default_list)
112+
return self.getlist(key)
113+
114+
def appendlist(self, key, value):
115+
"Appends an item to the internal list associated with key"
116+
self.setlistdefault(key, [])
117+
dict.__setitem__(self, key, self.getlist(key) + [value])
116118

117119
def items(self):
118-
# we don't just return self.data.items() here, because we want to use
119-
# self.__getitem__() to access the values as *strings*, not lists
120-
return [(key, self[key]) for key in self.data.keys()]
120+
"""
121+
Returns a list of (key, value) pairs, where value is the last item in
122+
the list associated with the key.
123+
"""
124+
return [(key, self[key]) for key in self.keys()]
121125

122-
def keys(self):
123-
return self.data.keys()
126+
def lists(self):
127+
"Returns a list of (key, list) pairs."
128+
return dict.items(self)
124129

125-
def update(self, other_dict):
126-
if isinstance(other_dict, MultiValueDict):
127-
for key, value_list in other_dict.data.items():
128-
self.data.setdefault(key, []).extend(value_list)
129-
elif type(other_dict) == type({}):
130-
for key, value in other_dict.items():
131-
self.data.setdefault(key, []).append(value)
132-
else:
133-
raise ValueError, "MultiValueDict.update() takes either a MultiValueDict or dictionary"
130+
def values(self):
131+
"Returns a list of the last value on every key list."
132+
return [self[key] for key in self.keys()]
134133

135134
def copy(self):
136-
"Returns a copy of this object"
135+
"Returns a copy of this object."
137136
import copy
137+
# Our custom __setitem__ must be disabled for copying machinery.
138+
MultiValueDict.__setitem__ = dict.__setitem__
138139
cp = copy.deepcopy(self)
140+
MultiValueDict.__setitem__ = MultiValueDict._setitem_list
139141
return cp
140142

143+
def update(self, other_dict):
144+
"update() extends rather than replaces existing key lists."
145+
if isinstance(other_dict, MultiValueDict):
146+
for key, value_list in other_dict.lists():
147+
self.setlistdefault(key, []).extend(value_list)
148+
else:
149+
try:
150+
for key, value in other_dict.items():
151+
self.setlistdefault(key, []).append(value)
152+
except TypeError:
153+
raise ValueError, "MultiValueDict.update() takes either a MultiValueDict or dictionary"
154+
141155
class DotExpandedDict(dict):
142156
"""
143157
A special dictionary constructor that takes a dictionary in which the keys

‎django/utils/httpwrappers.py

Lines changed: 43 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -67,63 +67,62 @@ class QueryDict(MultiValueDict):
6767
"""A specialized MultiValueDict that takes a query string when initialized.
6868
This is immutable unless you create a copy of it."""
6969
def __init__(self, query_string):
70-
if not query_string:
71-
self.data = {}
72-
self._keys = []
73-
else:
74-
self.data = {}
75-
self._keys = []
76-
for name, value in parse_qsl(query_string, True): # keep_blank_values=True
77-
if name in self.data:
78-
self.data[name].append(value)
79-
else:
80-
self.data[name] = [value]
81-
if name not in self._keys:
82-
self._keys.append(name)
70+
MultiValueDict.__init__(self)
71+
self._mutable = True
72+
for key, value in parse_qsl(query_string, True): # keep_blank_values=True
73+
self.appendlist(key, value)
8374
self._mutable = False
8475

85-
def __setitem__(self, key, value):
76+
def _assert_mutable(self):
8677
if not self._mutable:
8778
raise AttributeError, "This QueryDict instance is immutable"
88-
else:
89-
self.data[key] = [value]
90-
if not key in self._keys:
91-
self._keys.append(key)
79+
80+
def _setitem_if_mutable(self, key, value):
81+
self._assert_mutable()
82+
MultiValueDict.__setitem__(self, key, value)
83+
__setitem__ = _setitem_if_mutable
9284

9385
def setlist(self, key, list_):
94-
if not self._mutable:
95-
raise AttributeError, "This QueryDict instance is immutable"
96-
else:
97-
self.data[key] = list_
98-
if not key in self._keys:
99-
self._keys.append(key)
86+
self._assert_mutable()
87+
MultiValueDict.setlist(self, key, list_)
10088

101-
def copy(self):
102-
"Returns a mutable copy of this object"
103-
cp = MultiValueDict.copy(self)
104-
cp._mutable = True
105-
return cp
89+
def appendlist(self, key, value):
90+
self._assert_mutable()
91+
MultiValueDict.appendlist(self, key, value)
10692

107-
def assert_synchronized(self):
108-
assert(len(self._keys) == len(self.data.keys())), \
109-
"QueryDict data structure is out of sync: %s %s" % (str(self._keys), str(self.data))
93+
def update(self, other_dict):
94+
self._assert_mutable()
95+
MultiValueDict.update(self, other_dict)
11096

111-
def items(self):
112-
"Respect order preserved by self._keys"
113-
self.assert_synchronized()
114-
items = []
115-
for key in self._keys:
116-
if key in self.data:
117-
items.append((key, self.data[key][0]))
118-
return items
97+
def pop(self, key):
98+
self._assert_mutable()
99+
return MultiValueDict.pop(self, key)
119100

120-
def keys(self):
121-
self.assert_synchronized()
122-
return self._keys
101+
def popitem(self):
102+
self._assert_mutable()
103+
return MultiValueDict.popitem(self)
104+
105+
def clear(self):
106+
self._assert_mutable()
107+
MultiValueDict.clear(self)
108+
109+
def setdefault(self, *args):
110+
self._assert_mutable()
111+
return MultiValueDict.setdefault(self, *args)
112+
113+
def copy(self):
114+
"Returns a mutable copy of this object."
115+
import copy
116+
# Our custom __setitem__ must be disabled for copying machinery.
117+
QueryDict.__setitem__ = dict.__setitem__
118+
cp = copy.deepcopy(self)
119+
QueryDict.__setitem__ = QueryDict._setitem_if_mutable
120+
cp._mutable = True
121+
return cp
123122

124123
def urlencode(self):
125124
output = []
126-
for k, list_ in self.data.items():
125+
for k, list_ in self.lists():
127126
output.extend([urlencode({k: v}) for v in list_])
128127
return '&'.join(output)
129128

0 commit comments

Comments
 (0)