From 92fe7465a29113bfbc86e885be7549da79279a10 Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Tue, 26 Mar 2019 09:59:43 -0700 Subject: [PATCH 1/8] add value kwarg to filter for specific query parameters to remove, fixes #70 --- hyperlink/_url.py | 9 ++++++--- hyperlink/test/test_url.py | 7 +++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/hyperlink/_url.py b/hyperlink/_url.py index c2cf2310..df44e81d 100644 --- a/hyperlink/_url.py +++ b/hyperlink/_url.py @@ -1560,7 +1560,7 @@ def get(self, name): """ return [value for (key, value) in self.query if name == key] - def remove(self, name): + def remove(self, name, value=_UNSET): """Make a new :class:`URL` instance with all occurrences of the query parameter *name* removed. No exception is raised if the parameter is not already set. @@ -1572,8 +1572,11 @@ def remove(self, name): URL: A new :class:`URL` instance with the parameter removed. """ - return self.replace(query=((k, v) for (k, v) in self.query - if k != name)) + if value is _UNSET: + nq = [(k, v) for (k, v) in self.query if k != name] + else: + nq = [(k, v) for (k, v) in self.query if not (k == name and v == value)] + return self.replace(query=nq) EncodedURL = URL # An alias better describing what the URL really is diff --git a/hyperlink/test/test_url.py b/hyperlink/test/test_url.py index b522c35a..7819e121 100644 --- a/hyperlink/test/test_url.py +++ b/hyperlink/test/test_url.py @@ -535,6 +535,13 @@ def test_queryRemove(self): URL.from_text(u"https://example.com/a/b/?bar=2") ) + self.assertEqual( + url.remove(name=u"foo", value=u"1"), + URL.from_text(u"https://example.com/a/b/?bar=2&foo=3") + ) + + + def test_parseEqualSignInParamValue(self): """ Every C{=}-sign after the first in a query parameter is simply included From d694c2f73f088ea0aa78c683fa8faa02699dedbc Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Tue, 26 Mar 2019 11:11:15 -0700 Subject: [PATCH 2/8] update URL.remove() docstring --- hyperlink/_url.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hyperlink/_url.py b/hyperlink/_url.py index df44e81d..0d126944 100644 --- a/hyperlink/_url.py +++ b/hyperlink/_url.py @@ -1562,15 +1562,18 @@ def get(self, name): def remove(self, name, value=_UNSET): """Make a new :class:`URL` instance with all occurrences of the query - parameter *name* removed. No exception is raised if the + parameter *name* removed, or, if *value* is set, parameters + matching *name* and *value*. No exception is raised if the parameter is not already set. Args: name (unicode): The name of the query parameter to remove. + value (unicode): Optional value to additionally filter + on. Setting this removes query parameters which match + both name and value. Returns: URL: A new :class:`URL` instance with the parameter removed. - """ if value is _UNSET: nq = [(k, v) for (k, v) in self.query if k != name] From 3354b9bdb597ad9bfd2d529fa03bd46fd6501d10 Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Thu, 28 Mar 2019 23:16:46 -0700 Subject: [PATCH 3/8] add limit param to URL.remove() --- hyperlink/_url.py | 17 +++++++++++++---- hyperlink/test/test_url.py | 8 ++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/hyperlink/_url.py b/hyperlink/_url.py index 0d126944..44d7489f 100644 --- a/hyperlink/_url.py +++ b/hyperlink/_url.py @@ -1560,7 +1560,7 @@ def get(self, name): """ return [value for (key, value) in self.query if name == key] - def remove(self, name, value=_UNSET): + def remove(self, name, value=_UNSET, limit=None): """Make a new :class:`URL` instance with all occurrences of the query parameter *name* removed, or, if *value* is set, parameters matching *name* and *value*. No exception is raised if the @@ -1575,10 +1575,19 @@ def remove(self, name, value=_UNSET): Returns: URL: A new :class:`URL` instance with the parameter removed. """ - if value is _UNSET: - nq = [(k, v) for (k, v) in self.query if k != name] + if limit is None: + if value is _UNSET: + nq = [(k, v) for (k, v) in self.query if k != name] + else: + nq = [(k, v) for (k, v) in self.query if not (k == name and v == value)] else: - nq = [(k, v) for (k, v) in self.query if not (k == name and v == value)] + nq, removed_count = [], 0 + for k, v in self.query: + if k != name and (value is _UNSET or v != value) or removed_count >= limit: + nq.append((k, v)) + else: + removed_count += 1 + return self.replace(query=nq) diff --git a/hyperlink/test/test_url.py b/hyperlink/test/test_url.py index 7819e121..8884d714 100644 --- a/hyperlink/test/test_url.py +++ b/hyperlink/test/test_url.py @@ -540,7 +540,15 @@ def test_queryRemove(self): URL.from_text(u"https://example.com/a/b/?bar=2&foo=3") ) + self.assertEqual( + url.remove(name=u"foo", limit=1), + URL.from_text(u"https://example.com/a/b/?bar=2&foo=3") + ) + self.assertEqual( + url.remove(name=u"foo", value=u"1", limit=0), + URL.from_text(u"https://example.com/a/b/?foo=1&bar=2&foo=3") + ) def test_parseEqualSignInParamValue(self): """ From 38b582981d0418db5e3091861335ae4df2c2341d Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Thu, 28 Mar 2019 23:25:15 -0700 Subject: [PATCH 4/8] replication of new remove() logic to DecodedURL --- hyperlink/_url.py | 18 +++++++++++++++--- hyperlink/test/test_decoded_url.py | 6 ++++++ hyperlink/test/test_url.py | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/hyperlink/_url.py b/hyperlink/_url.py index 44d7489f..575f3f99 100644 --- a/hyperlink/_url.py +++ b/hyperlink/_url.py @@ -1802,10 +1802,22 @@ def set(self, name, value=None): q[idx:idx] = [(name, value)] return self.replace(query=q) - def remove(self, name): + def remove(self, name, value=_UNSET, limit=None): "Return a new DecodedURL with query parameter *name* removed." - return self.replace(query=((k, v) for (k, v) in self.query - if k != name)) + if limit is None: + if value is _UNSET: + nq = [(k, v) for (k, v) in self.query if k != name] + else: + nq = [(k, v) for (k, v) in self.query if not (k == name and v == value)] + else: + nq, removed_count = [], 0 + for k, v in self.query: + if k != name and (value is _UNSET or v != value) or removed_count >= limit: + nq.append((k, v)) + else: + removed_count += 1 + + return self.replace(query=nq) def __repr__(self): cn = self.__class__.__name__ diff --git a/hyperlink/test/test_decoded_url.py b/hyperlink/test/test_decoded_url.py index 348460d1..3ac7f45a 100644 --- a/hyperlink/test/test_decoded_url.py +++ b/hyperlink/test/test_decoded_url.py @@ -88,6 +88,12 @@ def test_query_manipulation(self): assert durl.set('arg', 'd').get('arg') == ['d'] + durl = DecodedURL.from_text(u"https://example.com/a/b/?foo=1&bar=2&foo=3") + assert durl.remove("foo") == DecodedURL.from_text("https://example.com/a/b/?bar=2") + assert durl.remove("foo", value="1") == DecodedURL.from_text("https://example.com/a/b/?bar=2&foo=3") + assert durl.remove("foo", limit=1) == DecodedURL.from_text("https://example.com/a/b/?bar=2&foo=3") + assert durl.remove("foo", value="1", limit=0) == DecodedURL.from_text("https://example.com/a/b/?foo=1&bar=2&foo=3") + def test_equality_and_hashability(self): durl = DecodedURL.from_text(TOTAL_URL) durl2 = DecodedURL.from_text(TOTAL_URL) diff --git a/hyperlink/test/test_url.py b/hyperlink/test/test_url.py index 8884d714..d0a11f32 100644 --- a/hyperlink/test/test_url.py +++ b/hyperlink/test/test_url.py @@ -527,7 +527,7 @@ def test_querySet(self): def test_queryRemove(self): """ - L{URL.remove} removes all instances of a query parameter. + L{URL.remove} removes instances of a query parameter. """ url = URL.from_text(u"https://example.com/a/b/?foo=1&bar=2&foo=3") self.assertEqual( From d3299608648cd10aa1ad3bc93046c96c0bf63c1d Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Thu, 28 Mar 2019 23:27:58 -0700 Subject: [PATCH 5/8] add limit description to .remove() docstrings --- hyperlink/_url.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hyperlink/_url.py b/hyperlink/_url.py index 575f3f99..65854d21 100644 --- a/hyperlink/_url.py +++ b/hyperlink/_url.py @@ -1571,6 +1571,7 @@ def remove(self, name, value=_UNSET, limit=None): value (unicode): Optional value to additionally filter on. Setting this removes query parameters which match both name and value. + limit (int): Optional maximum number of parameters to remove. Returns: URL: A new :class:`URL` instance with the parameter removed. @@ -1803,7 +1804,11 @@ def set(self, name, value=None): return self.replace(query=q) def remove(self, name, value=_UNSET, limit=None): - "Return a new DecodedURL with query parameter *name* removed." + """Return a new DecodedURL with query parameter *name* removed. + + Optionally also filter for *value*, as well as cap the number + of parameters removed with *limit*. + """ if limit is None: if value is _UNSET: nq = [(k, v) for (k, v) in self.query if k != name] From ad189ed371bee89b66ecfca679458eff6e475555 Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Thu, 28 Mar 2019 23:31:33 -0700 Subject: [PATCH 6/8] use more exotic query parameters in DecodedURL's tests --- hyperlink/test/test_decoded_url.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hyperlink/test/test_decoded_url.py b/hyperlink/test/test_decoded_url.py index 3ac7f45a..4e6f8b97 100644 --- a/hyperlink/test/test_decoded_url.py +++ b/hyperlink/test/test_decoded_url.py @@ -88,11 +88,11 @@ def test_query_manipulation(self): assert durl.set('arg', 'd').get('arg') == ['d'] - durl = DecodedURL.from_text(u"https://example.com/a/b/?foo=1&bar=2&foo=3") - assert durl.remove("foo") == DecodedURL.from_text("https://example.com/a/b/?bar=2") - assert durl.remove("foo", value="1") == DecodedURL.from_text("https://example.com/a/b/?bar=2&foo=3") - assert durl.remove("foo", limit=1) == DecodedURL.from_text("https://example.com/a/b/?bar=2&foo=3") - assert durl.remove("foo", value="1", limit=0) == DecodedURL.from_text("https://example.com/a/b/?foo=1&bar=2&foo=3") + durl = DecodedURL.from_text(u"https://example.com/a/b/?fóó=1&bar=2&fóó=3") + assert durl.remove("fóó") == DecodedURL.from_text("https://example.com/a/b/?bar=2") + assert durl.remove("fóó", value="1") == DecodedURL.from_text("https://example.com/a/b/?bar=2&fóó=3") + assert durl.remove("fóó", limit=1) == DecodedURL.from_text("https://example.com/a/b/?bar=2&fóó=3") + assert durl.remove("fóó", value="1", limit=0) == DecodedURL.from_text("https://example.com/a/b/?fóó=1&bar=2&fóó=3") def test_equality_and_hashability(self): durl = DecodedURL.from_text(TOTAL_URL) From db10d3dc09d5a88ae71723edaf58bb6b7d891720 Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Tue, 2 Apr 2019 09:44:07 -0700 Subject: [PATCH 7/8] rearrange .remove() logic to be more readable --- hyperlink/_url.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/hyperlink/_url.py b/hyperlink/_url.py index ed239e7f..e461b15f 100644 --- a/hyperlink/_url.py +++ b/hyperlink/_url.py @@ -1608,11 +1608,12 @@ def remove(self, name, value=_UNSET, limit=None): nq = [(k, v) for (k, v) in self.query if not (k == name and v == value)] else: nq, removed_count = [], 0 + for k, v in self.query: - if k != name and (value is _UNSET or v != value) or removed_count >= limit: - nq.append((k, v)) + if k == name and (value is _UNSET or v == value) and removed_count < limit: + removed_count += 1 # drop it else: - removed_count += 1 + nq.append((k, v)) # keep it return self.replace(query=nq) @@ -1842,10 +1843,10 @@ def remove(self, name, value=_UNSET, limit=None): else: nq, removed_count = [], 0 for k, v in self.query: - if k != name and (value is _UNSET or v != value) or removed_count >= limit: - nq.append((k, v)) + if k == name and (value is _UNSET or v == value) and removed_count < limit: + removed_count += 1 # drop it else: - removed_count += 1 + nq.append((k, v)) # keep it return self.replace(query=nq) From dced0dd5bbdcfc813ead1e95ff306ae767141f8d Mon Sep 17 00:00:00 2001 From: Mahmoud Hashemi Date: Sun, 7 Apr 2019 23:24:05 -0700 Subject: [PATCH 8/8] update docstring wording --- hyperlink/_url.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hyperlink/_url.py b/hyperlink/_url.py index e461b15f..50e8535e 100644 --- a/hyperlink/_url.py +++ b/hyperlink/_url.py @@ -1586,7 +1586,7 @@ def get(self, name): return [value for (key, value) in self.query if name == key] def remove(self, name, value=_UNSET, limit=None): - """Make a new :class:`URL` instance with all occurrences of the query + """Make a new :class:`URL` instance with occurrences of the query parameter *name* removed, or, if *value* is set, parameters matching *name* and *value*. No exception is raised if the parameter is not already set.