Dne 21.12.2011 14:32, Jan Cholasta napsal(a):
Dne 2.12.2011 21:26, Alexander Bokovoy napsal(a):
On Fri, 02 Dec 2011, Jan Cholasta wrote:
I don't like the idea of introducing a new class every time we need a
ReadOnly attribute to be writable. There's quite a few places in the
code where we do object.__setattr__ on ReadOnly objects. IMO the right
thing to do would be to add means of whitelisting ReadOnly attributes
that need to stay writable after locking.
You can move those _select_ca(), _select_any_master(),
_host_has_service() to CaCache as they seem to not depend on anything
in class ca but rather use global api.env.
This way you will get is a fairly simple CaCache class reusable both
in ca and ra classes.
Honza
What is the status of this patch?
rob
It fixes the issue and I wouldn't mind leaving it as it is.
Alexander?
I still don't like it. There is nothing in CA that really requires
enabling writting to ReadOnly after locking. ReadOnly is a fundamental
promise of our API and breaking it should be possible only for cases
where any other approach will be ineffective. This particular case is
rather poor implementation of CA/RA classes that could be solved in a
simpler way.
Sometimes you need to hold promises. ;)
Updated and rebased the patch.
Added a class for creating property-like attributes that cache the
return value of a function call.
Fixed some more unit test issues that popped up since I first made the
patch.
Note that some tests are still failing, most of the failures seem to be
related to user private groups (see attached log).
Honza
Fixed cachedproperty so that the return value is cached per-instance
instead of per-class.
Updated patch attached.
Honza
--
Jan Cholasta
>From f2711cf5b85be3cf285936365a80b6f8cdfaaf9e Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 1 Nov 2011 08:58:05 -0400
Subject: [PATCH] Fix attempted write to attribute of read-only object.
Add new class "cachedproperty" for creating property-like attributes
that cache the return value of a method call.
Also fix few issues in the unit tests to enable them to succeed.
ticket 1959
---
ipalib/dn.py | 2 +-
ipalib/errors.py | 6 +++---
ipalib/util.py | 34 ++++++++++++++++++++++++++++++++++
ipaserver/install/ldapupdate.py | 4 ++--
ipaserver/ipaldap.py | 2 +-
ipaserver/plugins/dogtag.py | 8 ++------
ipaserver/plugins/ldap2.py | 2 +-
tests/test_ipalib/test_plugable.py | 2 +-
tests/test_ipaserver/test_ldap.py | 10 +++++++++-
9 files changed, 54 insertions(+), 16 deletions(-)
diff --git a/ipalib/dn.py b/ipalib/dn.py
index dc3119d..6f2f7de 100644
--- a/ipalib/dn.py
+++ b/ipalib/dn.py
@@ -1092,7 +1092,7 @@ class DN(object):
return rdns
elif isinstance(value, (tuple, list)):
if len(value) != 2:
- raise ValueError("tuple or list must be 2-valued, not \"%s\"" % (rdn))
+ raise ValueError("tuple or list must be 2-valued, not \"%s\"" % (value))
rdn = RDN(value, first_key_match=self.first_key_match)
return rdn
else:
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 5b63488..f115f0c 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -448,10 +448,10 @@ class RefererError(PublicError):
For example:
- >>> raise RefererError()
+ >>> raise RefererError(referer='referer')
Traceback (most recent call last):
...
- RefererError: Missing or invalid HTTP Referer
+ RefererError: Missing or invalid HTTP Referer, referer
"""
errno = 911
@@ -1537,7 +1537,7 @@ class DependentEntry(ExecutionError):
>>> raise DependentEntry(label=u'SELinux User Map', key=u'test', dependent=u'test1')
Traceback (most recent call last):
...
- DependentEntry: Not registered yet
+ DependentEntry: test cannot be deleted because SELinux User Map test1 requires it
"""
diff --git a/ipalib/util.py b/ipalib/util.py
index ffa2759..d575329 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -27,6 +27,7 @@ import time
import socket
import re
from types import NoneType
+from weakref import WeakKeyDictionary
from ipalib import errors
from ipalib.text import _
@@ -272,3 +273,36 @@ def validate_hostname(hostname):
if not all(regex_name.match(part) for part in hostname.split(".")):
raise ValueError(_('hostname parts may only include letters, numbers, and - ' \
'(which is not allowed as the last character)'))
+
+class cachedproperty(object):
+ """
+ A property-like attribute that caches the return value of a method call.
+
+ When the attribute is first read, the method is called and its return
+ value is saved and returned. On subsequent reads, the saved value is
+ returned.
+
+ Typical usage:
+ class C(object):
+ @cachedproperty
+ def attr(self):
+ return 'value'
+ """
+ __slots__ = ('getter', 'store')
+
+ def __init__(self, getter):
+ self.getter = getter
+ self.store = WeakKeyDictionary()
+
+ def __get__(self, obj, cls):
+ if obj is None:
+ return None
+ if obj not in self.store:
+ self.store[obj] = self.getter(obj)
+ return self.store[obj]
+
+ def __set__(self, obj, value):
+ raise AttributeError("can't set attribute")
+
+ def __delete__(self, obj):
+ raise AttributeError("can't delete attribute")
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 91d3d83..8fbfeaf 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -700,7 +700,7 @@ class LDAPUpdate:
self.conn.deleteEntry(dn)
self.modified = True
except errors.NotFound, e:
- root_logger.info("%s did not exist:%s", (dn, e))
+ root_logger.info("%s did not exist:%s", dn, e)
self.modified = True
except errors.DatabaseError, e:
root_logger.error("Delete failed: %s", e)
@@ -717,7 +717,7 @@ class LDAPUpdate:
self.conn.deleteEntry(dn)
self.modified = True
except errors.NotFound, e:
- root_logger.info("%s did not exist:%s", (dn, e))
+ root_logger.info("%s did not exist:%s", dn, e)
self.modified = True
except errors.DatabaseError, e:
root_logger.error("Delete failed: %s", e)
diff --git a/ipaserver/ipaldap.py b/ipaserver/ipaldap.py
index ae36dd2..4dca604 100644
--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -314,7 +314,7 @@ class IPAdmin(IPAEntryLDAPObject):
raise errors.DuplicateEntry()
except ldap.CONSTRAINT_VIOLATION, e:
# This error gets thrown by the uniqueness plugin
- if info == 'Another entry with the same attribute value already exists':
+ if info.startswith('Another entry with the same attribute value already exists'):
raise errors.DuplicateEntry()
else:
raise errors.DatabaseError(desc=desc,info=info)
diff --git a/ipaserver/plugins/dogtag.py b/ipaserver/plugins/dogtag.py
index 23d06ab..39eeeb4 100644
--- a/ipaserver/plugins/dogtag.py
+++ b/ipaserver/plugins/dogtag.py
@@ -1200,6 +1200,7 @@ import os, random, ldap
from ipaserver.plugins import rabase
from ipalib.errors import NetworkError, CertificateOperationError
from ipalib.constants import TYPE_ERROR
+from ipalib.util import cachedproperty
from ipapython import dogtag
from ipalib import _
@@ -1218,7 +1219,6 @@ class ra(rabase.rabase):
self.ipa_key_size = "2048"
self.ipa_certificate_nickname = "ipaCert"
self.ca_certificate_nickname = "caCert"
- self.ca_host = None
try:
f = open(self.pwd_file, "r")
self.password = f.readline().strip()
@@ -1283,6 +1283,7 @@ class ra(rabase.rabase):
return host
else:
return api.env.ca_host
+ ca_host = cachedproperty(_select_ca)
def _request(self, url, port, **kw):
"""
@@ -1293,8 +1294,6 @@ class ra(rabase.rabase):
Perform an HTTP request.
"""
- if self.ca_host == None:
- self.ca_host = self._select_ca()
return dogtag.http_request(self.ca_host, port, url, **kw)
def _sslget(self, url, port, **kw):
@@ -1306,9 +1305,6 @@ class ra(rabase.rabase):
Perform an HTTPS request
"""
-
- if self.ca_host == None:
- self.ca_host = self._select_ca()
return dogtag.https_request(self.ca_host, port, url, self.sec_dir, self.password, self.ipa_certificate_nickname, **kw)
def get_parse_result_xml(self, xml_text, parse_func):
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 4bfc849..0698034 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -210,7 +210,7 @@ def _handle_errors(e, **kw):
raise errors.DuplicateEntry()
except _ldap.CONSTRAINT_VIOLATION:
# This error gets thrown by the uniqueness plugin
- if info == 'Another entry with the same attribute value already exists':
+ if info.startswith('Another entry with the same attribute value already exists'):
raise errors.DuplicateEntry()
else:
raise errors.DatabaseError(desc=desc, info=info)
diff --git a/tests/test_ipalib/test_plugable.py b/tests/test_ipalib/test_plugable.py
index 23b7330..3355e05 100644
--- a/tests/test_ipalib/test_plugable.py
+++ b/tests/test_ipalib/test_plugable.py
@@ -247,7 +247,7 @@ class test_Plugin(ClassChecker):
info = 'whatever'
e = raises(StandardError, check)
assert str(e) == \
- "check.info attribute ('whatever') conflicts with Plugin logger"
+ "info is already bound to tests.test_ipalib.test_plugable.check()"
def test_set_api(self):
"""
diff --git a/tests/test_ipaserver/test_ldap.py b/tests/test_ipaserver/test_ldap.py
index abfd1be..1bbd94f 100644
--- a/tests/test_ipaserver/test_ldap.py
+++ b/tests/test_ipaserver/test_ldap.py
@@ -112,7 +112,15 @@ class test_ldap(object):
myapi.register(service)
myapi.register(service_show)
myapi.finalize()
- myapi.Backend.ldap2.connect(bind_dn="cn=Directory Manager", bind_pw='password')
+
+ pwfile = api.env.dot_ipa + os.sep + ".dmpw"
+ if ipautil.file_exists(pwfile):
+ fp = open(pwfile, "r")
+ dm_password = fp.read().rstrip()
+ fp.close()
+ else:
+ raise nose.SkipTest("No directory manager password in %s" % pwfile)
+ myapi.Backend.ldap2.connect(bind_dn="cn=Directory Manager", bind_pw=dm_password)
result = myapi.Command['service_show']('ldap/%s@%s' % (api.env.host, api.env.realm,))
entry_attrs = result['result']
--
1.7.6.4
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
