Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle old ghost items #114

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions axiom/item.py
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,8 @@ def checkpoint(self):
# don't issue duplicate SQL and crap; we were created, then
# destroyed immediately.
return
self.store.executeSQL(self._baseDeleteSQL(self.store), [self.storeID])
self.store.executeSQL(self._baseDeleteSQL(self.store),
[self.storeID])
# re-using OIDs plays havoc with the cache, and with other things
# as well. We need to make sure that we leave a placeholder row at
# the end of the table.
Expand All @@ -770,16 +771,17 @@ def checkpoint(self):
self.store.executeSchemaSQL(_schema.CHANGE_TYPE,
[-1, self.storeID])

# Can't do this any more:
# self.store.executeSchemaSQL(_schema.DELETE_OBJECT, [self.storeID])
# I want to do this to reclaim the space, but I can't because
# the ID allocation algorithm depends on the fact that old
# objects aren't recycled:

# self.store.executeSchemaSQL(_schema.DELETE_OBJECT,
# [self.storeID])

# TODO: need to measure the performance impact of this, then do
# it to make sure things are in fact deleted:
# self.store.executeSchemaSQL(_schema.APP_VACUUM)

else:
assert self.__legacy__

# we're done...
if self.store.autocommit:
self.committed()
Expand Down
11 changes: 11 additions & 0 deletions axiom/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -2302,6 +2302,17 @@ def getItemByID(self, storeID, default=_noItem, autoUpgrade=True):
# for the moment we're going to assume no inheritance
attrs = self.querySQL(T._baseSelectSQL(self), [storeID])
if len(attrs) == 0:
# there was an oid allocation row with a type set to a valid
# type, but no accompanying data row.

# there are cases where items will be deleted but their oid
# allocation rows won't be. normally this should set the type
# to -1 but upgraders or old bugs may have left the database
# in a state where it's still set.

# do cleanup if we're not in a read-only transaction (haha
# there is no such thing as a read-only transaction)
self.executeSchemaSQL(_schema.CHANGE_TYPE, [-1, storeID])
if default is _noItem:
raise errors.ItemNotFound(
'No results for known-to-be-good object')
Expand Down
22 changes: 22 additions & 0 deletions axiom/test/test_item.py
Original file line number Diff line number Diff line change
Expand Up @@ -656,3 +656,25 @@ class TI3(Item):
[(TestInterface2, 20),
(TestInterface, 0)])
self.assertTrue(TestInterface.implementedBy(TI3))


def test_fixDangling(self):
st = store.Store()
i = itemtest.PlainItem(store=st)
oldStoreID = i.storeID
self.assertEquals(st.getItemByID(oldStoreID, default=1234),
i)

# this magic keyword argument actually simulates an old bug, but rather
# than carry around a legacy database, we'll just call an internal API,
# because this is actually used by upgraders as well (e.g. a subtly
# buggy upgrader could leave the DB in the same state as this old bug)

i.deleteFromStore(deleteObject=False)

self.assertEquals(st.getItemByID(oldStoreID+100, default=1234),
1234)

self.assertEquals(st.getItemByID(oldStoreID, default=1234),
1234)