From 686db5d45253a721b60fb1e445aa4dff0fcf2e87 Mon Sep 17 00:00:00 2001 From: Josh Imhoff Date: Fri, 18 May 2012 00:48:59 -0400 Subject: [PATCH 1/5] Defined keys(), values(), and items() in order to support dict-like client interface. Now, we throw exceptions if __getitem__() or __setitem__() is used when multiple domains are in jar. --- requests/cookies.py | 58 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/requests/cookies.py b/requests/cookies.py index 92a24ccb..ffaaa4b7 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -156,13 +156,59 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): self.set_cookie(c) return c + def keys(): + keys = [] + for cookie in iter(self): + keys.append(cookie.name) + return keys + + def values(): + values = [] + for cookie in iter(self): + values.append(cookie.values) + return values + + def items(): + items = [] + for cookie in iter(self): + items.append((cookies.name, cookie.values)) + return items + + def list_domains(self): + domains = [] + for cookie in iter(self): + if cookie.domain not in domains: + domains.append(cookie.domain) + return domains + + def list_paths(self): + paths = [] + for cookie in iter(self): + if cookie.path not in paths: + paths.append(cookie.path) + return paths + + def get_dict(self, domain, path): + dictionary = {} + for cookie in iter(self): + # TODO double check this logic + if (domain == None or cookie.domain == domain) and (path == None + or cookie.path == path): + dictionary[cookie.name] = cookie.value + return dictionary + def __getitem__(self, name): + if self._multipleDomainsOrPaths(): + raise KeyError('Multiple domains/paths in jar. Use .get instead.') return self._find(name) def __setitem__(self, name, value): + if self._multipleDomainsOrPaths(): + raise KeyError('Multiple domains/paths in jar. Use .set instead.') self.set(name, value) def __delitem__(self, name): + # consider testing for multiple domains remove_cookie_by_name(self, name) def _find(self, name, domain=None, path=None): @@ -174,6 +220,18 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): raise KeyError('name=%r, domain=%r, path=%r' % (name, domain, path)) + def _multipleDomainsOrPaths(self): + domains = [] + paths = [] + for cookie in iter(self): + if cookie.domain is not None and cookie.domain in domains: + return True + domains.append(cookie.domain) + if cookie.path is not None and cookie.path in paths: + return True + paths.append(cookie.path) + return False # there is only one domains and one path in jar + def __getstate__(self): state = self.__dict__.copy() # remove the unpickleable RLock object From b7d0925d6b2c37e8ebe5ac4cdfffce08ffae7dde Mon Sep 17 00:00:00 2001 From: Josh Imhoff Date: Mon, 21 May 2012 17:59:38 -0400 Subject: [PATCH 2/5] Docstrings and bug fixes in cookies.py --- requests/cookies.py | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/requests/cookies.py b/requests/cookies.py index ffaaa4b7..56fb8c24 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -137,13 +137,21 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): Unlike a regular CookieJar, this class is pickleable. """ + # TODO test, test, test + # TODO should get and set require domain and/or path? def get(self, name, default=None, domain=None, path=None): + """Dict-like get() that also supports optional domain and path args in + order to resolve naming collisions from using one cookie jar over + multiple domains. Caution: operation is O(n), not O(1).""" try: return self._find(name, domain, path) except KeyError: return default def set(self, name, value, **kwargs): + """Dict-like set() that also supports optional domain and path args in + order to resolve naming collisions from using one cookie jar over + multiple domains.""" # support client code that unsets cookies by assignment of a None value: if value is None: remove_cookie_by_name(self, name, domain=kwargs.get('domain'), path=kwargs.get('path')) @@ -157,24 +165,32 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): return c def keys(): + """Dict-like keys() that returns a list of names of cookies from the jar. + See values() and items().""" keys = [] for cookie in iter(self): keys.append(cookie.name) return keys def values(): + """Dict-like values() that returns a list of values of cookies from the jar. + See keys() and items().""" values = [] for cookie in iter(self): values.append(cookie.values) return values def items(): + """Dict-like items() that returns a list of name-value tuples from the jar. + See keys() and values(). Allows client-code to call "dict(RequestsCookieJar) + and get a vanilla python dict of key value pairs.""" items = [] for cookie in iter(self): items.append((cookies.name, cookie.values)) return items def list_domains(self): + """Utility method to list all the domains in the jar.""" domains = [] for cookie in iter(self): if cookie.domain not in domains: @@ -182,36 +198,48 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): return domains def list_paths(self): + """Utility method to list all the paths in the jar.""" paths = [] for cookie in iter(self): if cookie.path not in paths: paths.append(cookie.path) return paths - def get_dict(self, domain, path): + def get_dict(self, domain=None, path=None): + """Takes as an argument an optional domain and path and returns a plain old + Python dict of name-value pairs of cookies that meet the requirements.""" dictionary = {} for cookie in iter(self): - # TODO double check this logic if (domain == None or cookie.domain == domain) and (path == None or cookie.path == path): dictionary[cookie.name] = cookie.value return dictionary def __getitem__(self, name): + """Dict-like __getitem__() for compatibility with client code. Throws exception + if there are multiple domains or paths in the jar. In that case, use the more + explicit get() method instead. Caution: operation is O(n), not O(1).""" if self._multipleDomainsOrPaths(): raise KeyError('Multiple domains/paths in jar. Use .get instead.') return self._find(name) def __setitem__(self, name, value): + """Dict-like __setitem__ for compatibility with client code. Throws exception + if there are multiple domains or paths in the jar. In that case, use the more + explicit set() method instead.""" + # TODO is this check nesc? if self._multipleDomainsOrPaths(): raise KeyError('Multiple domains/paths in jar. Use .set instead.') self.set(name, value) def __delitem__(self, name): - # consider testing for multiple domains + """Deletes a cookie given a name. Wraps cookielib.CookieJar's remove_cookie_by_name().""" remove_cookie_by_name(self, name) def _find(self, name, domain=None, path=None): + """Requests uses this method internally to find cookies. Takes as args name + and optional domain and path. Returns a cookie.value. Throws KeyError + if cookie is not found.""" for cookie in iter(self): if cookie.name == name: if domain is None or cookie.domain == domain: @@ -221,6 +249,9 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): raise KeyError('name=%r, domain=%r, path=%r' % (name, domain, path)) def _multipleDomainsOrPaths(self): + """Returns True if there are multiple domains or paths in the jar. + Returns False otherwise.""" + # TODO: possible bug -- always multiple paths, right? domains = [] paths = [] for cookie in iter(self): @@ -232,6 +263,7 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): paths.append(cookie.path) return False # there is only one domains and one path in jar + # TODO docstrings def __getstate__(self): state = self.__dict__.copy() # remove the unpickleable RLock object @@ -244,7 +276,7 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): self._cookies_lock = threading.RLock() def copy(self): - """We're probably better off forbidding this.""" + """This is not currently implemented. Calling copy() will throw an exception.""" raise NotImplementedError def create_cookie(name, value, **kwargs): From 9f26d0ced3d7a360b1bb392a5e8257409378bac5 Mon Sep 17 00:00:00 2001 From: Josh Imhoff Date: Mon, 21 May 2012 18:21:12 -0400 Subject: [PATCH 3/5] In cookies.py, CookieConflictError is now thrown if there is more than one cookie with same name. --- requests/cookies.py | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/requests/cookies.py b/requests/cookies.py index 56fb8c24..bc789f1a 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -120,6 +120,10 @@ def remove_cookie_by_name(cookiejar, name, domain=None, path=None): for domain, path, name in clearables: cookiejar.clear(domain, path, name) +class CookieConflictError(RuntimeError): + """There are two cookies that meet the criteria specified in the cookie jar. + Use .get and .set and include domain and path args in order to be more specific.""" + class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): """Compatibility class; is a cookielib.CookieJar, but exposes a dict interface. @@ -138,7 +142,6 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): """ # TODO test, test, test - # TODO should get and set require domain and/or path? def get(self, name, default=None, domain=None, path=None): """Dict-like get() that also supports optional domain and path args in order to resolve naming collisions from using one cookie jar over @@ -217,19 +220,14 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): def __getitem__(self, name): """Dict-like __getitem__() for compatibility with client code. Throws exception - if there are multiple domains or paths in the jar. In that case, use the more + if there are more than one cookie with name. In that case, use the more explicit get() method instead. Caution: operation is O(n), not O(1).""" - if self._multipleDomainsOrPaths(): - raise KeyError('Multiple domains/paths in jar. Use .get instead.') return self._find(name) def __setitem__(self, name, value): """Dict-like __setitem__ for compatibility with client code. Throws exception - if there are multiple domains or paths in the jar. In that case, use the more + if there is already a cookie of that name in the jar. In that case, use the more explicit set() method instead.""" - # TODO is this check nesc? - if self._multipleDomainsOrPaths(): - raise KeyError('Multiple domains/paths in jar. Use .set instead.') self.set(name, value) def __delitem__(self, name): @@ -239,29 +237,30 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): def _find(self, name, domain=None, path=None): """Requests uses this method internally to find cookies. Takes as args name and optional domain and path. Returns a cookie.value. Throws KeyError - if cookie is not found.""" + if cookie is not found and CookieConflictError if there are multiple cookies + that match name and optionally domain and path.""" + toReturn = None for cookie in iter(self): if cookie.name == name: if domain is None or cookie.domain == domain: if path is None or cookie.path == path: - return cookie.value + if toReturn != None: # if there are multiple cookies that meet passed in criteria + raise CookieConflictError('There are multiple cookies with name, %r' % (name)) + toReturn = cookie.value # we will eventually return this as long as no cookie conflict + if toReturn: + return toReturn raise KeyError('name=%r, domain=%r, path=%r' % (name, domain, path)) - def _multipleDomainsOrPaths(self): - """Returns True if there are multiple domains or paths in the jar. + def _multipleDomains(self): + """Returns True if there are multiple domains in the jar. Returns False otherwise.""" - # TODO: possible bug -- always multiple paths, right? domains = [] - paths = [] for cookie in iter(self): if cookie.domain is not None and cookie.domain in domains: return True domains.append(cookie.domain) - if cookie.path is not None and cookie.path in paths: - return True - paths.append(cookie.path) - return False # there is only one domains and one path in jar + return False # there is only one domain in jar # TODO docstrings def __getstate__(self): @@ -276,7 +275,7 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): self._cookies_lock = threading.RLock() def copy(self): - """This is not currently implemented. Calling copy() will throw an exception.""" + """This is not implemented. Calling this will throw an exception.""" raise NotImplementedError def create_cookie(name, value, **kwargs): From 96cd8e9ca0b6953794b9b3244b244dd34f975544 Mon Sep 17 00:00:00 2001 From: Josh Imhoff Date: Mon, 21 May 2012 20:14:12 -0400 Subject: [PATCH 4/5] Wrote tests for new CookieJar functionality and made them pass. --- requests/cookies.py | 61 +++++++++++++++++++++++++++---------------- tests/test_cookies.py | 33 +++++++++++++++++++++++ 2 files changed, 71 insertions(+), 23 deletions(-) diff --git a/requests/cookies.py b/requests/cookies.py index bc789f1a..4eab64c9 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -141,13 +141,12 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): Unlike a regular CookieJar, this class is pickleable. """ - # TODO test, test, test def get(self, name, default=None, domain=None, path=None): """Dict-like get() that also supports optional domain and path args in order to resolve naming collisions from using one cookie jar over multiple domains. Caution: operation is O(n), not O(1).""" try: - return self._find(name, domain, path) + return self._find_no_duplicates(name, domain, path) except KeyError: return default @@ -167,7 +166,7 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): self.set_cookie(c) return c - def keys(): + def keys(self): """Dict-like keys() that returns a list of names of cookies from the jar. See values() and items().""" keys = [] @@ -175,21 +174,21 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): keys.append(cookie.name) return keys - def values(): + def values(self): """Dict-like values() that returns a list of values of cookies from the jar. See keys() and items().""" values = [] for cookie in iter(self): - values.append(cookie.values) + values.append(cookie.value) return values - def items(): + def items(self): """Dict-like items() that returns a list of name-value tuples from the jar. See keys() and values(). Allows client-code to call "dict(RequestsCookieJar) and get a vanilla python dict of key value pairs.""" items = [] for cookie in iter(self): - items.append((cookies.name, cookie.values)) + items.append((cookie.name, cookie.value)) return items def list_domains(self): @@ -208,11 +207,23 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): paths.append(cookie.path) return paths + def multiple_domains(self): + """Returns True if there are multiple domains in the jar. + Returns False otherwise.""" + domains = [] + for cookie in iter(self): + if cookie.domain is not None and cookie.domain in domains: + return True + domains.append(cookie.domain) + return False # there is only one domain in jar + def get_dict(self, domain=None, path=None): """Takes as an argument an optional domain and path and returns a plain old Python dict of name-value pairs of cookies that meet the requirements.""" dictionary = {} for cookie in iter(self): + print path + print cookie.path if (domain == None or cookie.domain == domain) and (path == None or cookie.path == path): dictionary[cookie.name] = cookie.value @@ -222,7 +233,7 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): """Dict-like __getitem__() for compatibility with client code. Throws exception if there are more than one cookie with name. In that case, use the more explicit get() method instead. Caution: operation is O(n), not O(1).""" - return self._find(name) + return self._find_no_duplicates(name) def __setitem__(self, name, value): """Dict-like __setitem__ for compatibility with client code. Throws exception @@ -235,10 +246,23 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): remove_cookie_by_name(self, name) def _find(self, name, domain=None, path=None): - """Requests uses this method internally to find cookies. Takes as args name - and optional domain and path. Returns a cookie.value. Throws KeyError - if cookie is not found and CookieConflictError if there are multiple cookies - that match name and optionally domain and path.""" + """Requests uses this method internally to get cookie values. Takes as args name + and optional domain and path. Returns a cookie.value. If there are conflicting cookies, + _find arbitrarily chooses one. See _find_no_duplicates if you want an exception thrown + if there are conflicting cookies.""" + for cookie in iter(self): + if cookie.name == name: + if domain is None or cookie.domain == domain: + if path is None or cookie.path == path: + return cookie.value + + raise KeyError('name=%r, domain=%r, path=%r' % (name, domain, path)) + + def _find_no_duplicates(self, name, domain=None, path=None): + """__get_item__ and get call _find_no_duplicates -- never used in Requests internally. + Takes as args name and optional domain and path. Returns a cookie.value. + Throws KeyError if cookie is not found and CookieConflictError if there are + multiple cookies that match name and optionally domain and path.""" toReturn = None for cookie in iter(self): if cookie.name == name: @@ -252,24 +276,15 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): return toReturn raise KeyError('name=%r, domain=%r, path=%r' % (name, domain, path)) - def _multipleDomains(self): - """Returns True if there are multiple domains in the jar. - Returns False otherwise.""" - domains = [] - for cookie in iter(self): - if cookie.domain is not None and cookie.domain in domains: - return True - domains.append(cookie.domain) - return False # there is only one domain in jar - - # TODO docstrings def __getstate__(self): + """Unlike a normal CookieJar, this class is pickleable.""" state = self.__dict__.copy() # remove the unpickleable RLock object state.pop('_cookies_lock') return state def __setstate__(self, state): + """Unlike a normal CookieJar, this class is pickleable.""" self.__dict__.update(state) if '_cookies_lock' not in self.__dict__: self._cookies_lock = threading.RLock() diff --git a/tests/test_cookies.py b/tests/test_cookies.py index 9cb97ea6..bdc9172a 100755 --- a/tests/test_cookies.py +++ b/tests/test_cookies.py @@ -24,6 +24,7 @@ class CookieTests(TestBaseMixin, unittest.TestCase): # test deprecated dictionary interface self.assertEqual(r.cookies['myname'], 'myvalue') + self.assertEqual(r.cookies.get('myname'), 'myvalue') # test CookieJar interface jar = r.cookies self.assertEqual(len(jar), 1) @@ -122,6 +123,38 @@ class CookieTests(TestBaseMixin, unittest.TestCase): r = s.get(httpbin('cookies')) self.assertEqual(json.loads(r.text)['cookies'], {}) + def test_jar_utility_functions(self): + """Test utility functions such as list_domains, list_paths, multiple_domains.""" + r = requests.get("http://github.com") + c = r.cookies + # github should send us cookies + self.assertTrue(len(c) >= 1) + self.assertEqual(len(c), len(r.cookies.keys())) + self.assertEqual(len(c), len(r.cookies.values())) + self.assertEqual(len(c), len(r.cookies.items())) + + # domain and path utility functions + domain = r.cookies.list_domains()[0] + path = r.cookies.list_paths()[0] + self.assertEqual(dict(r.cookies), r.cookies.get_dict(domain=domain, path=path)) + self.assertEqual(len(r.cookies.list_domains()), 1) + self.assertEqual(len(r.cookies.list_paths()), 1) + self.assertFalse(r.cookies.multiple_domains()) + + def test_convert_jar_to_dict(self): + """Test that keys, values, and items are defined and that we can convert + cookie jars to plain old Python dicts.""" + r = requests.get(httpbin('cookies', 'set', 'myname', 'myvalue')) + + # test keys, values, and items + self.assertEqual(r.cookies.keys(), ['myname']) + self.assertEqual(r.cookies.values(), ['myvalue']) + self.assertEqual(r.cookies.items(), [('myname','myvalue')]) + + # test if we can convert jar to dict + dictOfCookies = dict(r.cookies) + self.assertEqual(dictOfCookies, {'myname':'myvalue'}) + self.assertEqual(dictOfCookies, r.cookies.get_dict()) class LWPCookieJarTest(TestBaseMixin, unittest.TestCase): """Check store/load of cookies to FileCookieJar's, specifically LWPCookieJar's.""" From 0eb0b89431d467197b361e4eab3c3399ff07b22e Mon Sep 17 00:00:00 2001 From: Josh Imhoff Date: Mon, 21 May 2012 20:27:47 -0400 Subject: [PATCH 5/5] Removed print statements left over from debugging. --- requests/cookies.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/requests/cookies.py b/requests/cookies.py index 4eab64c9..85726b06 100644 --- a/requests/cookies.py +++ b/requests/cookies.py @@ -222,8 +222,6 @@ class RequestsCookieJar(cookielib.CookieJar, collections.MutableMapping): Python dict of name-value pairs of cookies that meet the requirements.""" dictionary = {} for cookie in iter(self): - print path - print cookie.path if (domain == None or cookie.domain == domain) and (path == None or cookie.path == path): dictionary[cookie.name] = cookie.value