Fix loadBefore.

There are 2 possibilities when an object is not found:
- the object doesn't exist at all
- the object exists, but nothing matches serial criterion

git-svn-id: https://svn.erp5.org/repos/neo/trunk@2256 71dcc9de-d417-0410-9af5-da40c76e7ee4

neo/client/Storage.py

@@ -19,6 +19,7 @@ from ZODB import BaseStorage, ConflictResolution, POSException
 
 from neo.client.app import Application
 from neo.client.exception import NEOStorageNotFoundError
+from neo.client.exception import NEOStorageDoesNotExistError
 
 def check_read_only(func):
     def wrapped(self, *args, **kw):
@@ -88,6 +89,8 @@ class Storage(BaseStorage.BaseStorage,
     def loadBefore(self, oid, tid):
         try:
             return self.app.loadBefore(oid=oid, tid=tid)
+        except NEOStorageDoesNotExistError:
+            raise POSException.POSKeyError(oid)
         except NEOStorageNotFoundError:
             return None
 

neo/client/app.py

@@ -464,6 +464,7 @@ class Application(object):
                 technical problem
             NEOStorageNotFoundError
                 object exists but no data satisfies given parameters
+            NEOStorageDoesNotExistError
                 object doesn't exist
         """
         # TODO:

neo/client/exception.py

@@ -25,3 +25,11 @@ class NEOStorageError(POSException.StorageError):
 
 class NEOStorageNotFoundError(NEOStorageError):
     pass
+
+class NEOStorageDoesNotExistError(NEOStorageNotFoundError):
+    """
+    This error is a refinement of NEOStorageNotFoundError: this means
+    that some object was not found, but also that it does not exist at all.
+    """
+    pass
+

neo/client/handlers/storage.py

@@ -23,6 +23,7 @@ from neo.client.handlers import BaseHandler, AnswerBaseHandler
 from neo.protocol import NodeTypes, ProtocolError, LockState
 from neo.util import dump
 from neo.client.exception import NEOStorageError, NEOStorageNotFoundError
+from neo.client.exception import NEOStorageDoesNotExistError
 
 class StorageEventHandler(BaseHandler):
 
@@ -116,6 +117,9 @@ class StorageAnswersHandler(AnswerBaseHandler):
         # - asking for history
         raise NEOStorageNotFoundError(message)
 
+    def oidDoesNotExist(self, conn, message):
+        raise NEOStorageDoesNotExistError(message)
+
     def tidNotFound(self, conn, message):
         # This can happen when requiring txn informations
         raise NEOStorageNotFoundError(message)

neo/handler.py

@@ -362,6 +362,9 @@ class EventHandler(object):
     def oidNotFound(self, conn, message):
         raise UnexpectedPacketError
 
+    def oidDoesNotExist(self, conn, message):
+        raise UnexpectedPacketError
+
     def tidNotFound(self, conn, message):
         raise UnexpectedPacketError
 
@@ -466,6 +469,7 @@ class EventHandler(object):
         d[ErrorCodes.ACK] = self.ack
         d[ErrorCodes.NOT_READY] = self.notReady
         d[ErrorCodes.OID_NOT_FOUND] = self.oidNotFound
+        d[ErrorCodes.OID_DOES_NOT_EXIST] = self.oidDoesNotExist
         d[ErrorCodes.TID_NOT_FOUND] = self.tidNotFound
         d[ErrorCodes.PROTOCOL_ERROR] = self.protocolError
         d[ErrorCodes.BROKEN_NODE] = self.brokenNodeDisallowedError

neo/protocol.py

@@ -41,6 +41,7 @@ class ErrorCodes(Enum):
     ACK = Enum.Item(0)
     NOT_READY = Enum.Item(1)
     OID_NOT_FOUND = Enum.Item(2)
+    OID_DOES_NOT_EXIST = Enum.Item(6)
     TID_NOT_FOUND = Enum.Item(3)
     PROTOCOL_ERROR = Enum.Item(4)
     BROKEN_NODE = Enum.Item(5)
@@ -1853,6 +1854,7 @@ class ErrorRegistry(dict):
     ProtocolError = register_error(ErrorCodes.PROTOCOL_ERROR)
     TidNotFound = register_error(ErrorCodes.TID_NOT_FOUND)
     OidNotFound = register_error(ErrorCodes.OID_NOT_FOUND)
+    OidDoesNotExist = register_error(ErrorCodes.OID_DOES_NOT_EXIST)
     NotReady = register_error(ErrorCodes.NOT_READY)
     Broken = register_error(ErrorCodes.BROKEN_NODE)
 

neo/storage/database/mysqldb.py

@@ -339,7 +339,13 @@ class MySQLDatabaseManager(DatabaseManager):
         if before_tid is not None:
             before_tid = u64(before_tid)
         result = self._getObject(oid, tid, before_tid)
-        if result is not None:
+        if result is None:
+            # See if object exists at all
+            result = self._getObject(oid)
+            if result is not None:
+                # Object exists
+                result = False
+        else:
             serial, next_serial, compression, checksum, data, data_serial = \
                 result
             if data is None and resolve_data:

neo/storage/handlers/__init__.py

@@ -85,14 +85,17 @@ class BaseClientAndStorageOperationHandler(EventHandler):
             app.queueEvent(self.askObject, conn, oid, serial, tid)
             return
         o = self._askObject(oid, serial, tid)
-        if o is not None:
+        if o is None:
+            logging.debug('oid = %s does not exist', dump(oid))
+            p = Errors.OidDoesNotExist(dump(oid))
+        elif o is False:
+            logging.debug('oid = %s not found', dump(oid))
+            p = Errors.OidNotFound(dump(oid))
+        else:
             serial, next_serial, compression, checksum, data, data_serial = o
             logging.debug('oid = %s, serial = %s, next_serial = %s',
                           dump(oid), dump(serial), dump(next_serial))
             p = Packets.AnswerObject(oid, serial, next_serial,
                 compression, checksum, data, data_serial)
-        else:
-            logging.debug('oid = %s not found', dump(oid))
-            p = Errors.OidNotFound('%s does not exist' % dump(oid))
         conn.answer(p)
 

neo/tests/client/testClientApp.py

@@ -22,6 +22,7 @@ from ZODB.POSException import StorageTransactionError, UndoError, ConflictError
 from neo.tests import NeoTestBase
 from neo.client.app import Application
 from neo.client.exception import NEOStorageError, NEOStorageNotFoundError
+from neo.client.exception import NEOStorageDoesNotExistError
 from neo.protocol import Packet, Packets, Errors, INVALID_TID, INVALID_SERIAL
 from neo.util import makeChecksum
 
@@ -317,9 +318,9 @@ class ClientApplicationTests(NeoTestBase):
         oid = self.makeOID()
         tid1 = self.makeTID(1)
         tid2 = self.makeTID(2)
-        # object not found in NEO -> NEOStorageNotFoundError
+        # object not found in NEO -> NEOStorageDoesNotExistError
         self.assertTrue(oid not in mq)
-        packet = Errors.OidNotFound('')
+        packet = Errors.OidDoesNotExist('')
         packet.setId(0)
         cell = Mock({ 'getUUID': '\x00' * 16})
         conn = Mock({
@@ -328,7 +329,7 @@ class ClientApplicationTests(NeoTestBase):
         })
         app.pt = Mock({ 'getCellListForOID': [cell, ], })
         app.cp = Mock({ 'getConnForCell' : conn})
-        self.assertRaises(NEOStorageNotFoundError, app.loadBefore, oid, tid2)
+        self.assertRaises(NEOStorageDoesNotExistError, app.loadBefore, oid, tid2)
         self.checkAskObject(conn)
         # no visible version -> NEOStorageNotFoundError
         an_object = (1, oid, INVALID_SERIAL, None, 0, 0, '', None)

neo/tests/client/testStorageHandler.py

@@ -22,6 +22,7 @@ from neo.protocol import NodeTypes, LockState
 from neo.client.handlers.storage import StorageBootstrapHandler, \
        StorageAnswersHandler
 from neo.client.exception import NEOStorageError, NEOStorageNotFoundError
+from neo.client.exception import NEOStorageDoesNotExistError
 from ZODB.POSException import ConflictError
 
 MARKER = []
@@ -217,6 +218,11 @@ class StorageAnswerHandlerTests(NeoTestBase):
         conn = self.getConnection()
         self.assertRaises(NEOStorageNotFoundError, self.handler.oidNotFound,
             conn, 'message')
+
+    def test_oidDoesNotExist(self):
+        conn = self.getConnection()
+        self.assertRaises(NEOStorageDoesNotExistError,
+            self.handler.oidDoesNotExist, conn, 'message')
         
     def test_tidNotFound(self):
         conn = self.getConnection()

neo/tests/storage/testStorageMySQLdb.py

@@ -255,35 +255,45 @@ class StorageMySQSLdbTests(NeoTestBase):
     def test_getObject(self):
         oid1, = self.getOIDs(1)
         tid1, tid2 = self.getTIDs(2)
+        FOUND_BUT_NOT_VISIBLE = False
+        OBJECT_T1_NO_NEXT = (tid1, None, 1, 0, '', None)
+        OBJECT_T1_NEXT = (tid1, tid2, 1, 0, '', None)
+        OBJECT_T2 = (tid2, None, 1, 0, '', None)
         txn1, objs1 = self.getTransaction([oid1])
         txn2, objs2 = self.getTransaction([oid1])
         # non-present
+        self.assertEqual(self.db.getObject(oid1), None)
         self.assertEqual(self.db.getObject(oid1, tid1), None)
         self.assertEqual(self.db.getObject(oid1, before_tid=tid1), None)
         # one non-commited version
         self.db.storeTransaction(tid1, objs1, txn1)
+        self.assertEqual(self.db.getObject(oid1), None)
         self.assertEqual(self.db.getObject(oid1, tid1), None)
+        self.assertEqual(self.db.getObject(oid1, before_tid=tid1), None)
         # one commited version
         self.db.finishTransaction(tid1)
-        result = self.db.getObject(oid1, tid1)
-        self.assertEqual(result, (tid1, None, 1, 0, '', None))
-        self.assertEqual(self.db.getObject(oid1, before_tid=tid1), None)
+        self.assertEqual(self.db.getObject(oid1), OBJECT_T1_NO_NEXT)
+        self.assertEqual(self.db.getObject(oid1, tid1), OBJECT_T1_NO_NEXT)
+        self.assertEqual(self.db.getObject(oid1, before_tid=tid1),
+            FOUND_BUT_NOT_VISIBLE)
         # two version available, one non-commited
         self.db.storeTransaction(tid2, objs2, txn2)
-        result = self.db.getObject(oid1, tid1)
-        self.assertEqual(result, (tid1, None, 1, 0, '', None))
-        self.assertEqual(self.db.getObject(oid1, before_tid=tid1), None)
+        self.assertEqual(self.db.getObject(oid1), OBJECT_T1_NO_NEXT)
+        self.assertEqual(self.db.getObject(oid1, tid1), OBJECT_T1_NO_NEXT)
+        self.assertEqual(self.db.getObject(oid1, before_tid=tid1),
+            FOUND_BUT_NOT_VISIBLE)
+        self.assertEqual(self.db.getObject(oid1, tid2), FOUND_BUT_NOT_VISIBLE)
+        self.assertEqual(self.db.getObject(oid1, before_tid=tid2),
+            OBJECT_T1_NO_NEXT)
         # two commited versions
         self.db.finishTransaction(tid2)
-        result = self.db.getObject(oid1, tid1)
-        self.assertEqual(result, (tid1, None, 1, 0, '', None))
-        result = self.db.getObject(oid1, tid2)
-        self.assertEqual(result, (tid2, None, 1, 0, '', None))
-        result = self.db.getObject(oid1, before_tid=tid2)
-        self.assertEqual(result, (tid1, tid2, 1, 0, '', None))
-        # no tid specified, return the last version
-        result = self.db.getObject(oid1)
-        self.assertEqual(result, (tid2, None, 1, 0, '', None))
+        self.assertEqual(self.db.getObject(oid1), OBJECT_T2)
+        self.assertEqual(self.db.getObject(oid1, tid1), OBJECT_T1_NO_NEXT)
+        self.assertEqual(self.db.getObject(oid1, before_tid=tid1),
+            FOUND_BUT_NOT_VISIBLE)
+        self.assertEqual(self.db.getObject(oid1, tid2), OBJECT_T2)
+        self.assertEqual(self.db.getObject(oid1, before_tid=tid2),
+            OBJECT_T1_NEXT)
 
     def test_setPartitionTable(self):
         ptid = self.getPTID(1)