From 2a2cef38feb7f259ea6b5735a6ce3e46812e217d Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 14 Mar 2018 15:25:04 +0100 Subject: [PATCH 1/7] cidict: Rewrite in terms of MutableMapping The UserDict API is not specified when it comes to extending: it is not clear which methods can be safely overridden. The MutableMapping ABC fixes this problem by providing an explicit set of abstract methods, in terms of which the rest of the API is implemented. Switch to using MutableMapping for cidict. --- Lib/ldap/cidict.py | 71 +++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/Lib/ldap/cidict.py b/Lib/ldap/cidict.py index 875d5f5..db57468 100644 --- a/Lib/ldap/cidict.py +++ b/Lib/ldap/cidict.py @@ -5,57 +5,58 @@ See https://www.python-ldap.org/ for details. """ +from collections import MutableMapping import warnings from ldap import __version__ -from ldap.compat import IterableUserDict +class cidict(MutableMapping): + """ + Case-insensitive but case-respecting dictionary. + """ -class cidict(IterableUserDict): - """ - Case-insensitive but case-respecting dictionary. - """ + def __init__(self, default=None): + self._keys = {} + self._data = {} + if default: + self.update(default) + + # MutableMapping abstract methods - def __init__(self,default=None): - self._keys = {} - IterableUserDict.__init__(self,{}) - self.update(default or {}) + def __getitem__(self, key): + return self._data[key.lower()] - def __getitem__(self,key): - return self.data[key.lower()] + def __setitem__(self, key, value): + lower_key = key.lower() + self._keys[lower_key] = key + self._data[lower_key] = value - def __setitem__(self,key,value): - lower_key = key.lower() - self._keys[lower_key] = key - self.data[lower_key] = value + def __delitem__(self, key): + lower_key = key.lower() + del self._keys[lower_key] + del self._data[lower_key] - def __delitem__(self,key): - lower_key = key.lower() - del self._keys[lower_key] - del self.data[lower_key] + def __iter__(self): + return iter(self._keys.values()) - def update(self,dict): - for key, value in dict.items(): - self[key] = value + def __len__(self): + return len(self._keys) - def has_key(self,key): - return key in self + # Specializations for performance - def __contains__(self,key): - return IterableUserDict.__contains__(self, key.lower()) + def __contains__(self, key): + return key.lower() in self._keys - def __iter__(self): - return iter(self.keys()) + def clear(self): + self._keys.clear() + self._data.clear() - def keys(self): - return self._keys.values() + # Backwards compatibility - def items(self): - result = [] - for k in self._keys.values(): - result.append((k,self[k])) - return result + def has_key(self, key): + """Compatibility with python-ldap 2.x""" + return key in self def strlist_minus(a,b): From dada91afdba682f4d755f972b3d94a6e9e08f385 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 14 Mar 2018 15:47:52 +0100 Subject: [PATCH 2/7] cidict: Re-add the `data` attribute --- Lib/ldap/cidict.py | 11 +++++++++++ Tests/t_cidict.py | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/Lib/ldap/cidict.py b/Lib/ldap/cidict.py index db57468..3dcfe48 100644 --- a/Lib/ldap/cidict.py +++ b/Lib/ldap/cidict.py @@ -58,6 +58,17 @@ def has_key(self, key): """Compatibility with python-ldap 2.x""" return key in self + @property + def data(self): + """Compatibility with older IterableUserDict-based implementation""" + warnings.warn( + 'ldap.cidict.cidict.data is an internal attribute; it may be ' + + 'removed at any time', + category=DeprecationWarning, + stacklevel=2, + ) + return self._data + def strlist_minus(a,b): """ diff --git a/Tests/t_cidict.py b/Tests/t_cidict.py index b96a26e..6878617 100644 --- a/Tests/t_cidict.py +++ b/Tests/t_cidict.py @@ -62,6 +62,16 @@ def test_strlist_deprecated(self): strlist_func(["a"], ["b"]) self.assertEqual(len(w), 1) + def test_cidict_data(self): + """test the deprecated data atrtribute""" + d = ldap.cidict.cidict({'A': 1, 'B': 2}) + with warnings.catch_warnings(record=True) as w: + warnings.resetwarnings() + warnings.simplefilter('always', DeprecationWarning) + data = d.data + assert data == {'a': 1, 'b': 2} + self.assertEqual(len(w), 1) + if __name__ == '__main__': unittest.main() From 3e11046243b0b29fa48b84ce74082920796cdd78 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 14 Mar 2018 16:16:05 +0100 Subject: [PATCH 3/7] ldapurl.LDAPUrlExtensions: Rewrite in terms of MutableMapping The UserDict API is not specified when it comes to extending: it is not clear which methods can be safely overridden. The MutableMapping ABC fixes this problem by providing an explicit set of abstract methods, in terms of which the rest of the API is implemented. Switch to using MutableMapping for LDAPUrlExtensions. --- Lib/ldapurl.py | 102 ++++++++++++++++++++++++++----------------------- 1 file changed, 55 insertions(+), 47 deletions(-) diff --git a/Lib/ldapurl.py b/Lib/ldapurl.py index 6de0645..04f78ad 100644 --- a/Lib/ldapurl.py +++ b/Lib/ldapurl.py @@ -16,7 +16,9 @@ 'LDAPUrlExtension','LDAPUrlExtensions','LDAPUrl' ] -from ldap.compat import UserDict, quote, unquote +from collections import MutableMapping + +from ldap.compat import quote, unquote LDAP_SCOPE_BASE = 0 LDAP_SCOPE_ONELEVEL = 1 @@ -130,58 +132,64 @@ def __ne__(self,other): return not self.__eq__(other) -class LDAPUrlExtensions(UserDict): - """ - Models a collection of LDAP URL extensions as - dictionary type - """ - - def __init__(self,default=None): - UserDict.__init__(self) - for k,v in (default or {}).items(): - self[k]=v - - def __setitem__(self,name,value): +class LDAPUrlExtensions(MutableMapping): """ - value - Either LDAPUrlExtension instance, (critical,exvalue) - or string'ed exvalue + Models a collection of LDAP URL extensions as + a mapping type """ - assert isinstance(value,LDAPUrlExtension) - assert name==value.extype - self.data[name] = value - def values(self): - return [ - self[k] - for k in self.keys() - ] - - def __str__(self): - return ','.join(str(v) for v in self.values()) - - def __repr__(self): - return '<%s.%s instance at %s: %s>' % ( - self.__class__.__module__, - self.__class__.__name__, - hex(id(self)), - self.data - ) + def __init__(self, default=None): + self._data = {} + if default is not None: + self.update(default) + + def __setitem__(self, name, value): + """ + value + Either LDAPUrlExtension instance, (critical,exvalue) + or string'ed exvalue + """ + assert isinstance(value, LDAPUrlExtension) + assert name == value.extype + self._data[name] = value + + def __getitem__(self, name): + return self._data[name] + + def __delitem__(self, name): + del self._data[name] + + def __iter__(self): + return iter(self._data) + + def __len__(self): + return len(self._data) + + def __str__(self): + return ','.join(str(v) for v in self.values()) + + def __repr__(self): + return '<%s.%s instance at %s: %s>' % ( + self.__class__.__module__, + self.__class__.__name__, + hex(id(self)), + self._data + ) - def __eq__(self,other): - assert isinstance(other,self.__class__),TypeError( - "other has to be instance of %s" % (self.__class__) - ) - return self.data==other.data + def __eq__(self,other): + assert isinstance(other, self.__class__), TypeError( + "other has to be instance of %s" % (self.__class__) + ) + return self._data == other._data - def parse(self,extListStr): - for extension_str in extListStr.strip().split(','): - if extension_str: - e = LDAPUrlExtension(extension_str) - self[e.extype] = e + def parse(self,extListStr): + for extension_str in extListStr.strip().split(','): + if extension_str: + e = LDAPUrlExtension(extension_str) + self[e.extype] = e - def unparse(self): - return ','.join([ v.unparse() for v in self.values() ]) + def unparse(self): + return ','.join(v.unparse() for v in self.values()) class LDAPUrl(object): From 917f9c34c0860f0fe5b8ce26cbd56fad466d7238 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 27 Mar 2018 15:35:35 +0200 Subject: [PATCH 4/7] ldapurl: Replace asserts with proper checks --- Lib/ldapurl.py | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/Lib/ldapurl.py b/Lib/ldapurl.py index 04f78ad..1edc2fe 100644 --- a/Lib/ldapurl.py +++ b/Lib/ldapurl.py @@ -149,8 +149,13 @@ def __setitem__(self, name, value): Either LDAPUrlExtension instance, (critical,exvalue) or string'ed exvalue """ - assert isinstance(value, LDAPUrlExtension) - assert name == value.extype + if not isinstance(value, LDAPUrlExtension): + raise TypeError("value must be LDAPUrlExtension, not " + + type(value).__name__) + if name != value.extype: + raise ValueError( + "key {!r} does not match extension type {!r}".format( + name, value.extype)) self._data[name] = value def __getitem__(self, name): @@ -177,9 +182,8 @@ def __repr__(self): ) def __eq__(self,other): - assert isinstance(other, self.__class__), TypeError( - "other has to be instance of %s" % (self.__class__) - ) + if not isinstance(other, self.__class__): + return NotImplemented return self._data == other._data def parse(self,extListStr): @@ -374,17 +378,23 @@ def htmlHREF(self,urlPrefix='',hrefText=None,hrefTarget=None): hrefTarget string added as link target attribute """ - assert type(urlPrefix)==StringType, "urlPrefix must be StringType" + if not isinstance(urlPrefix, str): + raise TypeError("urlPrefix must be str, not " + + type(urlPrefix).__name__) if hrefText is None: - hrefText = self.unparse() - assert type(hrefText)==StringType, "hrefText must be StringType" + hrefText = self.unparse() + if not isinstance(hrefText, str): + raise TypeError("hrefText must be str, not " + + type(hrefText).__name__) if hrefTarget is None: - target = '' + target = '' else: - assert type(hrefTarget)==StringType, "hrefTarget must be StringType" - target = ' target="%s"' % hrefTarget + if not isinstance(hrefTarget, str): + raise TypeError("hrefTarget must be str, not " + + type(hrefTarget).__name__) + target = ' target="%s"' % hrefTarget return '%s' % ( - target,urlPrefix,self.unparse(),hrefText + target, urlPrefix, self.unparse(), hrefText ) def __str__(self): From 89f66f8dfa0be86a90dc1684135ab1d2fd04240d Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 27 Mar 2018 15:42:36 +0200 Subject: [PATCH 5/7] ldapurl: Update docstring of LDAPUrlExtensions.__setitem__ --- Lib/ldapurl.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Lib/ldapurl.py b/Lib/ldapurl.py index 1edc2fe..7ed1d8a 100644 --- a/Lib/ldapurl.py +++ b/Lib/ldapurl.py @@ -144,10 +144,12 @@ def __init__(self, default=None): self.update(default) def __setitem__(self, name, value): - """ + """Store an extension + + name + string value - Either LDAPUrlExtension instance, (critical,exvalue) - or string'ed exvalue + LDAPUrlExtension instance, whose extype nust match `name` """ if not isinstance(value, LDAPUrlExtension): raise TypeError("value must be LDAPUrlExtension, not " From 6f2a45d77a82a34d20ba1bd21246d0ef0433faea Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 27 Mar 2018 15:43:30 +0200 Subject: [PATCH 6/7] Lib: Add __slots__ to cidict and LDAPUrlExtensions --- Lib/ldap/cidict.py | 1 + Lib/ldapurl.py | 1 + 2 files changed, 2 insertions(+) diff --git a/Lib/ldap/cidict.py b/Lib/ldap/cidict.py index 3dcfe48..cf13388 100644 --- a/Lib/ldap/cidict.py +++ b/Lib/ldap/cidict.py @@ -15,6 +15,7 @@ class cidict(MutableMapping): """ Case-insensitive but case-respecting dictionary. """ + __slots__ = ('_keys', '_data') def __init__(self, default=None): self._keys = {} diff --git a/Lib/ldapurl.py b/Lib/ldapurl.py index 7ed1d8a..ecfc539 100644 --- a/Lib/ldapurl.py +++ b/Lib/ldapurl.py @@ -137,6 +137,7 @@ class LDAPUrlExtensions(MutableMapping): Models a collection of LDAP URL extensions as a mapping type """ + __slots__ = ('_data', ) def __init__(self, default=None): self._data = {} From b1b3b5a91b252afabf9c3d125ef648907dae5527 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 5 Jun 2020 20:55:07 +0200 Subject: [PATCH 7/7] Import MutableMapping from collections.abc, except on legacy Python --- Lib/ldap/cidict.py | 2 +- Lib/ldap/compat.py | 2 ++ Lib/ldapurl.py | 4 +--- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/ldap/cidict.py b/Lib/ldap/cidict.py index cf13388..48aeacb 100644 --- a/Lib/ldap/cidict.py +++ b/Lib/ldap/cidict.py @@ -5,9 +5,9 @@ See https://www.python-ldap.org/ for details. """ -from collections import MutableMapping import warnings +from ldap.compat import MutableMapping from ldap import __version__ diff --git a/Lib/ldap/compat.py b/Lib/ldap/compat.py index cbfeef5..901457b 100644 --- a/Lib/ldap/compat.py +++ b/Lib/ldap/compat.py @@ -10,6 +10,7 @@ from urllib import unquote as urllib_unquote from urllib import urlopen from urlparse import urlparse + from collections import MutableMapping def unquote(uri): """Specialized unquote that uses UTF-8 for parsing.""" @@ -33,6 +34,7 @@ def unquote(uri): IterableUserDict = UserDict from urllib.parse import quote, quote_plus, unquote, urlparse from urllib.request import urlopen + from collections.abc import MutableMapping def reraise(exc_type, exc_value, exc_traceback): """Re-raise an exception given information from sys.exc_info() diff --git a/Lib/ldapurl.py b/Lib/ldapurl.py index ecfc539..a3dd7ff 100644 --- a/Lib/ldapurl.py +++ b/Lib/ldapurl.py @@ -16,9 +16,7 @@ 'LDAPUrlExtension','LDAPUrlExtensions','LDAPUrl' ] -from collections import MutableMapping - -from ldap.compat import quote, unquote +from ldap.compat import quote, unquote, MutableMapping LDAP_SCOPE_BASE = 0 LDAP_SCOPE_ONELEVEL = 1