divmod-dev team mailing list archive

[Tristan Seligmann <mithrandi@xxxxxxxxxxxxx>]

Tristan Seligmann has proposed merging lp:~divmod-dev/divmod.org/829872-createnew-type-check into lp:divmod.org with lp:~divmod-dev/divmod.org/829869-explicit-type-dropping as a prerequisite.

Requested reviews:
  Divmod-dev (divmod-dev)
Related bugs:
  Bug #829872 in Divmod Axiom: "SubStore.createNew is too easy to misuse"
  https://bugs.launchpad.net/divmod-axiom/+bug/829872

For more details, see:
https://code.launchpad.net/~divmod-dev/divmod.org/829872-createnew-type-check/+merge/72277
-- 
https://code.launchpad.net/~divmod-dev/divmod.org/829872-createnew-type-check/+merge/72277
Your team Divmod-dev is requested to review the proposed merge of lp:~divmod-dev/divmod.org/829872-createnew-type-check into lp:divmod.org.

=== modified file 'Axiom/axiom/substore.py'
--- Axiom/axiom/substore.py	2007-11-15 19:58:09 +0000
+++ Axiom/axiom/substore.py	2011-08-20 02:05:24 +0000
@@ -26,6 +26,9 @@
         """
         Create a new SubStore, allocating a new file space for it.
         """
+        if isinstance(pathSegments, basestring):
+            raise ValueError(
+                'Received %s instead of a sequence' % (pathSegments,))
         if store.dbdir is None:
             self = cls(store=store, storepath=None)
         else:
@@ -37,11 +40,13 @@
 
     createNew = classmethod(createNew)
 
+
     def close(self):
         self.substore.close()
         del self.substore._openSubStore
         del self.substore
 
+
     def open(self, debug=False):
         if hasattr(self, 'substore'):
             return self.substore
@@ -51,6 +56,7 @@
                                    # store is alive!
             return s
 
+
     def createStore(self, debug):
         """
         Create the actual Store this Substore represents.
@@ -73,6 +79,7 @@
                          idInParent=self.storeID,
                          debug=debug)
 
+
     def __conform__(self, interface):
         """
         I adapt my store object to whatever interface I am adapted to.  This
@@ -84,6 +91,7 @@
         ifa = interface(self.open(debug=self.store.debug), None)
         return ifa
 
+
     def indirect(self, interface):
         """
         Like __conform__, I adapt my store to whatever interface I am asked to
@@ -94,6 +102,7 @@
         return interface(self)
 
 
+
 class SubStoreStartupService(Item, service.Service):
     """
     This class no longer exists.  It is here simply to trigger an upgrade which

=== modified file 'Axiom/axiom/test/test_batch.py'
--- Axiom/axiom/test/test_batch.py	2011-08-20 02:05:24 +0000
+++ Axiom/axiom/test/test_batch.py	2011-08-20 02:05:24 +0000
@@ -537,7 +537,7 @@
         """
         dbdir = filepath.FilePath(self.mktemp())
         s = store.Store(dbdir)
-        ss = substore.SubStore.createNew(s, 'substore')
+        ss = substore.SubStore.createNew(s, ['substore'])
         bs = iaxiom.IBatchService(ss)
         self.failUnless(iaxiom.IBatchService.providedBy(bs))
 
@@ -559,12 +559,15 @@
         """
         dbdir = filepath.FilePath(self.mktemp())
         s = store.Store(dbdir)
-        ss = substore.SubStore.createNew(s, 'substore')
+        ss = substore.SubStore.createNew(s, ['substore'])
         service.IService(s).startService()
-        d = iaxiom.IBatchService(ss).call(BatchCallTestItem(store=ss.open()).callIt)
+        d = iaxiom.IBatchService(ss).call(
+            BatchCallTestItem(store=ss.open()).callIt)
         ss.close()
         def called(ign):
-            self.failUnless(ss.open().findUnique(BatchCallTestItem).called, "Was not called")
+            self.assertTrue(
+                ss.open().findUnique(BatchCallTestItem).called,
+                "Was not called")
             return service.IService(s).stopService()
         return d.addCallback(called)
 

=== modified file 'Axiom/axiom/test/test_substore.py'
--- Axiom/axiom/test/test_substore.py	2008-05-06 13:33:30 +0000
+++ Axiom/axiom/test/test_substore.py	2011-08-20 02:05:24 +0000
@@ -12,15 +12,14 @@
 
 
 class SubStored(Item):
-
     schemaVersion = 1
     typeName = 'substoredthing'
     a = text()
     b = bytes()
 
 
+
 class YouCantStartThis(Item, Service):
-
     parent = inmemory()
     running = inmemory()
     name = inmemory()
@@ -30,8 +29,9 @@
     def startService(self):
         self.started = True
 
+
+
 class YouShouldStartThis(Item, Service):
-
     parent = inmemory()
     running = inmemory()
     name = inmemory()
@@ -42,11 +42,11 @@
         self.started = True
 
 
+
 class SubStoreTest(unittest.TestCase):
     """
     Test on-disk creation of substores.
     """
-
     def testOneThing(self):
         """
         Ensure that items can be inserted into substores and
@@ -72,6 +72,7 @@
         self.assertEquals(reopenssd.a, u'hello world')
         self.assertEquals(reopenssd.b, 'what, its text')
 
+
     def test_oneThingMemory(self):
         """
         Ensure that items put into in-memory substores are retrievable.
@@ -113,6 +114,7 @@
         self.assertEquals(item.a, u'hello world')
         self.assertEquals(item.b, 'what, its text')
 
+
     def test_memorySubstoreFile(self):
         """
         In-memory substores whose stores have file directories should be able
@@ -127,6 +129,17 @@
         f.close()
         self.assertEqual(open(f.finalpath.path).read(), "yay")
 
+
+    def test_createNewStringPath(self):
+        """
+        Passing a string instead of a sequence of strings to
+        L{SubStore.createNew} results in an exception.
+        """
+        s = Store()
+        self.assertRaises(ValueError, SubStore.createNew, s, 'notasequence')
+
+
+
 class SubStoreStartupSemantics(unittest.TestCase):
     """
     These tests verify that interactions between store and substore services
@@ -134,10 +147,10 @@
     behavior.  Read the code if you are interested in how to get startup
     notifications from substore items.
     """
-
     def setUp(self):
         """
-        Set up the tests by creating a store and a substore and opening them both.
+        Set up the tests by creating a store and a substore and opening them
+        both.
         """
         self.topdb = topdb = Store(filepath.FilePath(self.mktemp()))
         self.ssitem = ssitem = SubStore.createNew(
@@ -147,8 +160,8 @@
 
     def testDontStartNormally(self):
         """
-        Substores' services are not supposed to be started when their parent stores
-        are.
+        Substores' services are not supposed to be started when their parent
+        stores are.
         """
         ss = self.ss
         ycst = YouCantStartThis(store=ss)
@@ -183,4 +196,3 @@
         """
         if self.serviceStarted:
             return IService(self.topdb).stopService()
-