Commit 9ec4ba8d authored by Julien Muchembled's avatar Julien Muchembled

HBTreeFolder2: review object retrieval and deletion

_getOb and similar methods are reimplemented in a faster and safer way.
It now checks it is only used to return leafs.

Similarly, _delOb now refuses to delete trees at the root.

__getattr__ wrongly returned wrapped results (__of__).
parent 81222025
...@@ -142,6 +142,10 @@ ContainerAssertions[HBTreeObjectValues] = 1 ...@@ -142,6 +142,10 @@ ContainerAssertions[HBTreeObjectValues] = 1
class HBTreeFolder2Base (Persistent): class HBTreeFolder2Base (Persistent):
"""Base for BTree-based folders. """Base for BTree-based folders.
BUG: Due to wrong design, we can't store 2 objects <A> and <A>-<B>
where <A> does not contain '-'. We detect conflicts at the
root level using 'type(ob) is OOBTree'
""" """
security = ClassSecurityInfo() security = ClassSecurityInfo()
...@@ -264,47 +268,38 @@ class HBTreeFolder2Base (Persistent): ...@@ -264,47 +268,38 @@ class HBTreeFolder2Base (Persistent):
return 0 return 0
def hashId(self, id): def hashId(self, id):
"""Return a tuple of ids return id.split(H_SEPARATOR)
"""
# XXX: why tolerate non-string ids ? def _htree_get(self, id):
id_list = str(id).split(H_SEPARATOR) # We use '-' as the separator by default id_list = self.hashId(id)
if len(id_list) > 1: if len(id_list) == 1:
return tuple(id_list) ob = self._htree[id]
if type(ob) is OOBTree:
raise KeyError
else: else:
return [id,] ob = self._htree[id_list.pop(0)]
if type(ob) is not OOBTree:
# try: # We then try int hashing raise KeyError
# id_int = int(id) id_list[-1] = id
# except ValueError: for sub_id in id_list:
# return id_list ob = ob[sub_id]
# result = [] return ob
# while id_int:
# result.append(id_int % MAX_OBJECT_PER_LEVEL)
# id_int = id_int / MAX_OBJECT_PER_LEVEL
# result.reverse()
# return tuple(result)
def _getOb(self, id, default=_marker): def _getOb(self, id, default=_marker):
"""Return the named object from the folder
""" """
Return the named object from the folder. try:
""" return self._htree_get(id).__of__(self)
htree = self._htree except KeyError:
ob = htree
id_list = self.hashId(id)
for sub_id in id_list[0:-1]:
if default is _marker: if default is _marker:
ob = ob[sub_id] raise KeyError(id)
else: return default
ob = ob.get(sub_id, _marker)
if ob is _marker: def __getitem__(self, id):
return default try:
if default is _marker: return self._htree_get(id).__of__(self)
ob = ob[id] except KeyError:
else: raise KeyError(id)
ob = ob.get(id, _marker)
if ob is _marker:
return default
return ob.__of__(self)
def _setOb(self, id, object): def _setOb(self, id, object):
"""Store the named object in the folder. """Store the named object in the folder.
...@@ -340,9 +335,12 @@ class HBTreeFolder2Base (Persistent): ...@@ -340,9 +335,12 @@ class HBTreeFolder2Base (Persistent):
"""Remove the named object from the folder. """Remove the named object from the folder.
""" """
htree = self._htree htree = self._htree
id_list = self.hashId(id) for sub_id in self.hashId(id)[:-1]:
for sub_id in id_list[0:-1]: htree = htree.get(sub_id)
htree = htree[sub_id] if type(htree) is not OOBTree:
raise KeyError(id)
if type(htree[id]) is OOBTree:
raise KeyError(id)
del htree[id] del htree[id]
self._count.change(-1) self._count.change(-1)
...@@ -413,15 +411,9 @@ class HBTreeFolder2Base (Persistent): ...@@ -413,15 +411,9 @@ class HBTreeFolder2Base (Persistent):
def has_key(self, id): def has_key(self, id):
"""Indicates whether the folder has an item by ID. """Indicates whether the folder has an item by ID.
""" """
htree = self._htree try:
id_list = self.hashId(id) self._htree_get(id)
for sub_id in id_list[0:-1]: except KeyError:
if not isinstance(htree, OOBTree):
return 0
if not htree.has_key(sub_id):
return 0
htree = htree[sub_id]
if not htree.has_key(id):
return 0 return 0
return 1 return 1
...@@ -610,8 +602,10 @@ class HBTreeFolder2Base (Persistent): ...@@ -610,8 +602,10 @@ class HBTreeFolder2Base (Persistent):
security.declareProtected(access_contents_information, 'get') security.declareProtected(access_contents_information, 'get')
def get(self, name, default=None): def get(self, name, default=None):
return self._getOb(name, default) try:
return self._htree_get(name).__of__(self)
except KeyError:
return default
# Utility for generating unique IDs. # Utility for generating unique IDs.
...@@ -644,10 +638,10 @@ class HBTreeFolder2Base (Persistent): ...@@ -644,10 +638,10 @@ class HBTreeFolder2Base (Persistent):
# to subitems, and __bobo_traverse__ hooks don't work with # to subitems, and __bobo_traverse__ hooks don't work with
# restrictedTraverse() unless __getattr__() is also present. # restrictedTraverse() unless __getattr__() is also present.
# Oh well. # Oh well.
res = self._getOb(name, None) try:
if res is None: return self._htree_get(name)
raise AttributeError, name except KeyError:
return res raise AttributeError(name)
InitializeClass(HBTreeFolder2Base) InitializeClass(HBTreeFolder2Base)
......
...@@ -39,6 +39,21 @@ class HBTreeFolder2Tests(ERP5TypeTestCase): ...@@ -39,6 +39,21 @@ class HBTreeFolder2Tests(ERP5TypeTestCase):
self.f._setOb(ff.id, ff) self.f._setOb(ff.id, ff)
self.ff = self.f._getOb(ff.id) self.ff = self.f._getOb(ff.id)
def testKey(self):
f = self.f
ff = f.item
ok = "a", "b-a", "b-b", "c-a-b", "c-a-d"
for id in ok:
f._setOb(id, ff)
f._getOb(id)
for id in "a-a", "b", "c-a":
self.assertRaises(KeyError, f._getOb, id)
self.assertRaises(KeyError, f._delOb, id)
self.assertEqual(len(f), 1 + len(ok))
for id in ok:
f._delOb(id)
self.assertEqual(len(f), 1)
def testAdded(self): def testAdded(self):
self.assertEqual(self.ff.id, 'item') self.assertEqual(self.ff.id, 'item')
...@@ -99,11 +114,12 @@ class HBTreeFolder2Tests(ERP5TypeTestCase): ...@@ -99,11 +114,12 @@ class HBTreeFolder2Tests(ERP5TypeTestCase):
def testWrapped(self): def testWrapped(self):
# Verify that the folder returns wrapped versions of objects. # Verify that the folder returns wrapped versions of objects.
base = self.getBase(self.f._getOb('item')) base = self.f.item
self.assert_(self.f._getOb('item') is not base) for x in (self.f._getOb('item'),
self.assert_(self.f['item'] is not base) self.f['item'],
self.assert_(self.f.get('item') is not base) self.f.get('item')):
self.assert_(self.getBase(self.f._getOb('item')) is base) self.assertIsNot(base, x)
self.assertIs(base, self.getBase(x))
def testGenerateId(self): def testGenerateId(self):
ids = {} ids = {}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment