- Removed the pickling of ZCML actions (the code that wrote

  ``configure.zcml.cache`` next to ``configure.zcml`` files in
  projects).  The code which managed writing and reading of the cache
  file was a source of subtle bugs when users switched between
  imperative (e.g. ``@bfg_view``) registrations and declarative
  registrations (e.g. the ``view`` directive in ZCML) on the same
  project. On a moderately-sized project (535 ZCML actions and 15 ZCML
  files), executing actions read from the pickle was saving us only
  about 200ms (2.5 sec vs 2.7 sec average). On very small projects (1
  ZCML file and 4 actions), startup time was comparable, and sometimes
  even slower when reading from the pickle, and both ways were so fast
  that it really just didn't matter anyway.

3 files changed, 7 insertions, 282 deletions

diff --git a/repoze/bfg/registry.py b/repoze/bfg/registry.py
index 5d856ca19..0cf8e306b 100644
--- a/repoze/bfg/registry.py
+++ b/repoze/bfg/registry.py
@@ -99,7 +99,7 @@ def populateRegistry(registry, filename, package, lock=threading.Lock()):
     try:
         original_getSiteManager.sethook(getSiteManager)
         zope.component.getGlobalSiteManager = registry_manager.get
-        zcml_configure(filename, package=package)
+        zcml_configure(filename, package)
     finally:
         zope.component.getGlobalSiteManager = getGlobalSiteManager
         lock.release()
diff --git a/repoze/bfg/tests/test_zcml.py b/repoze/bfg/tests/test_zcml.py
index 1853483ba..0a1103b26 100644
--- a/repoze/bfg/tests/test_zcml.py
+++ b/repoze/bfg/tests/test_zcml.py
@@ -453,7 +453,7 @@ class TestRoute(unittest.TestCase):
         self.assertEqual(route_discriminator[6], None)
         self.assertEqual(route_args, (route,))
 
-class TestZCMLPickling(unittest.TestCase):
+class TestZCMLConfigure(unittest.TestCase):
     i = 0
     def setUp(self):
         cleanUp()
@@ -469,9 +469,6 @@ class TestZCMLPickling(unittest.TestCase):
         self.i += 1
         self.packagepath = os.path.join(tempdir, modname)
         fixturedir = package_path(package)
-        pckname = os.path.join(fixturedir, 'configure.zcml.cache')
-        if os.path.isfile(pckname):
-            os.remove(pckname)
         shutil.copytree(fixturedir, self.packagepath)
         sys.path.insert(0, tempdir)
         self.module = __import__(modname)
@@ -488,36 +485,10 @@ class TestZCMLPickling(unittest.TestCase):
             shutil.rmtree(self.tempdir)
 
     def test_file_configure(self):
-        import os
-        import cPickle
         from repoze.bfg.zcml import file_configure
-        self.assertEqual(False, file_configure('configure.zcml', self.module))
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        self.failUnless(os.path.exists(picklename))
-        actions = cPickle.load(open(picklename, 'rb'))
+        actions = file_configure('configure.zcml', self.module)
         self.failUnless(actions)
-
-    def test_file_configure_uncacheable_removes_cache(self):
-        import os
-        from repoze.bfg.zcml import file_configure
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        f = open(picklename, 'w')
-        f.write('imhere')
-        self.failUnless(os.path.exists(picklename))
-
-        import repoze.bfg.zcml
-        keep_view = repoze.bfg.zcml.view
-
-        def wrap_view(*arg, **kw):
-            kw['cacheable'] = False
-            return keep_view(*arg, **kw)
-
-        try:
-            repoze.bfg.zcml.view = wrap_view
-            file_configure('configure.zcml', self.module)
-            self.failIf(os.path.exists(picklename)) # should be deleted
-        finally:
-            repoze.bfg.zcml.view = keep_view
+        self.failUnless(isinstance(actions, list))
 
     def test_file_configure_nonexistent_configure_dot_zcml(self):
         import os
@@ -526,168 +497,6 @@ class TestZCMLPickling(unittest.TestCase):
         self.assertRaises(IOError, file_configure, 'configure.zcml',
                           self.module)
 
-    def test_file_configure_pickling_error(self):
-        import os
-        from repoze.bfg.zcml import file_configure
-        def dumpfail(actions, f, num):
-            raise IOError
-        self.assertEqual(False,
-                      file_configure('configure.zcml', self.module, dumpfail))
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        self.failIf(os.path.exists(picklename))
-
-    def test_zcml_configure_writes_pickle_when_none_exists(self):
-        import os
-        import cPickle
-        from repoze.bfg.zcml import zcml_configure
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        self.failUnless(os.path.exists(picklename))
-        actions = cPickle.load(open(picklename, 'rb'))
-        self.failUnless(actions)
-
-    def test_zcml_configure_uses_file_configure_with_bad_pickle1(self):
-        import os
-        import cPickle
-        from repoze.bfg.zcml import zcml_configure
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        f = open(picklename, 'wb')
-        cPickle.dump((), f)
-        f.close()
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-
-    def test_zcml_configure_uses_file_configure_with_bad_pickle2(self):
-        import os
-        from repoze.bfg.zcml import zcml_configure
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        f = open(picklename, 'wb')
-        f.write('garbage')
-        f.close()
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-
-    def test_zcml_configure_uses_file_configure_with_outofdate_pickle1(self):
-        import os
-        import cPickle
-        import time
-        from repoze.bfg.zcml import zcml_configure
-        basename = os.path.join(self.packagepath, 'configure.zcml')
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-        self.failUnless(os.path.exists(picklename))
-        actions = cPickle.load(open(picklename, 'rb'))
-        self.failUnless(actions)
-        os.utime(basename, (-1, time.time() + 100))
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-
-    def test_zcml_configure_uses_file_configure_with_outofdate_pickle2(self):
-        import os
-        import cPickle
-        import time
-        from repoze.bfg.zcml import zcml_configure
-        basename = os.path.join(self.packagepath, 'another.zcml')
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-        self.failUnless(os.path.exists(picklename))
-        actions = cPickle.load(open(picklename, 'rb'))
-        self.failUnless(actions)
-        os.utime(basename, (-1, time.time() + 100))
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-
-    def test_zcml_configure_uses_file_configure_with_missing_dependent(self):
-        import os
-        import cPickle
-        from repoze.bfg.zcml import zcml_configure
-        from zope.configuration.xmlconfig import ZopeXMLConfigurationError
-        basename = os.path.join(self.packagepath, 'another.zcml')
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-        self.failUnless(os.path.exists(picklename))
-        actions = cPickle.load(open(picklename, 'rb'))
-        self.failUnless(actions)
-        os.remove(basename)
-        self.assertRaises(ZopeXMLConfigurationError, zcml_configure,
-                          'configure.zcml', self.module)
-
-    def test_zcml_configure_uses_file_configure_with_bad_version(self):
-        import os
-        from repoze.bfg.zcml import zcml_configure
-        from repoze.bfg.zcml import PVERSION
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        f = open(picklename, 'wb')
-        import cPickle
-        data = (PVERSION+1, 0, [])
-        cPickle.dump(data, open(picklename, 'wb'))
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-
-    def test_zcml_configure_uses_file_configure_with_bad_time(self):
-        import os
-        from repoze.bfg.zcml import zcml_configure
-        from repoze.bfg.zcml import PVERSION
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        f = open(picklename, 'wb')
-        import cPickle
-        data = (PVERSION, None, [])
-        cPickle.dump(data, open(picklename, 'wb'))
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-
-    def test_zcml_configure_uses_file_configure_with_bad_actions(self):
-        import os
-        from repoze.bfg.zcml import zcml_configure
-        from repoze.bfg.zcml import PVERSION
-        import time
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        f = open(picklename, 'wb')
-        import cPickle
-        data = (PVERSION, time.time()+500, None)
-        cPickle.dump(data, open(picklename, 'wb'))
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-
-    def test_zcml_configure_uses_file_configure_with_bad_actions2(self):
-        import cPickle
-        import os
-        import time
-        from repoze.bfg.zcml import zcml_configure
-        from repoze.bfg.zcml import PVERSION
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        f = open(picklename, 'wb')
-        actions = [(None, None, None, None, None)]
-        data = (PVERSION, time.time()+500, actions)
-        cPickle.dump(data, open(picklename, 'wb'))
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-
-    def test_zcml_configure_uses_good_pickle(self):
-        import os
-        import cPickle
-        import time
-        from repoze.bfg.zcml import zcml_configure
-        from repoze.bfg.zcml import PVERSION
-        basename = os.path.join(self.packagepath, 'another.zcml')
-        picklename = os.path.join(self.packagepath, 'configure.zcml.cache')
-        self.assertEqual(False, zcml_configure('configure.zcml', self.module))
-        self.failUnless(os.path.exists(picklename))
-        actions = cPickle.load(open(picklename, 'rb'))
-        self.failUnless(actions)
-        actions = (PVERSION, time.time()+100, actions[2])
-        cPickle.dump(actions, open(picklename, 'wb'))
-        self.assertEqual(True, zcml_configure('configure.zcml', self.module))
-
-class TestRemove(unittest.TestCase):
-    def _callFUT(self, name, os):
-        from repoze.bfg.zcml import remove
-        return remove(name, os)
-
-    def test_fail(self):
-        class FakeOS:
-            def remove(self, name):
-                raise IOError('foo')
-        self.assertEqual(self._callFUT('name', FakeOS()), False)
-
-    def test_succeed(self):
-        class FakeOS:
-            def remove(self, name):
-                pass
-        self.assertEqual(self._callFUT('name', FakeOS()), True)
-
 class TestBFGViewFunctionGrokker(unittest.TestCase):
     def setUp(self):
         cleanUp()
diff --git a/repoze/bfg/zcml.py b/repoze/bfg/zcml.py
index 16e6d7012..7ea6eae8d 100644
--- a/repoze/bfg/zcml.py
+++ b/repoze/bfg/zcml.py
@@ -1,7 +1,3 @@
-import cPickle
-import os
-from os.path import realpath
-import time
 import types
 
 from zope.configuration import xmlconfig
@@ -28,8 +24,6 @@ from repoze.bfg.interfaces import IRoutesContext
 from repoze.bfg.interfaces import IViewPermission
 from repoze.bfg.interfaces import IView
 
-from repoze.bfg.path import package_path
-
 from repoze.bfg.security import ViewPermissionFactory
 
 import martian
@@ -132,93 +126,15 @@ class IViewDirective(Interface):
 
 PVERSION = 1
 
-def pickle_name(name, package):
-    path = package_path(package)
-    basename = os.path.join(path, name)
-    return os.path.join(path, basename + '.cache')
-
-def zcml_configure(name, package, load=cPickle.load):
-    """ Execute pickled zcml actions or fall back to parsing from file
-    """
-    pckname = pickle_name(name, package)
-
-    if not (os.path.isfile(pckname) or os.path.islink(pckname)):
-        return file_configure(name, package)
-
-    try:
-        vers, ptime, actions = load(open(pckname, 'rb'))
-    except (IOError, cPickle.UnpicklingError, EOFError, TypeError, ValueError,
-            AttributeError, NameError, ImportError):
-        return file_configure(name, package)
-
-    if vers != PVERSION:
-        return file_configure(name, package)
-
-    try:
-        ptime = int(ptime)
-    except:
-        return file_configure(name, package)
-
-    if not hasattr(actions, '__iter__'):
-        return file_configure(name, package)
-
-    files = set()
-    for action in actions:
-        try:
-            fileset = action[4]
-            files.update(fileset)
-        except (TypeError, IndexError):
-            return file_configure(name, package)
-
-    for file in files:
-        if not(os.path.isfile(file) or os.path.islink(file)):
-            return file_configure(name, package)
-
-        mtime = os.stat(realpath(file)).st_mtime
-        
-        if  mtime >= ptime:
-            return file_configure(name, package)
-
-    context = zope.configuration.config.ConfigurationMachine()
-    xmlconfig.registerCommonDirectives(context)
-    context.actions = actions
-    context.cached_execution = True
-    context.execute_actions()
-    return True
-
-def remove(name, os=os): # os parameterized for unit tests
-    try:
-        os.remove(name)
-        return True
-    except:
-        pass
-    return False
-
-def file_configure(name, package, dump=cPickle.dump):
+def zcml_configure(name, package):
     context = zope.configuration.config.ConfigurationMachine()
     xmlconfig.registerCommonDirectives(context)
     context.package = package
-
     xmlconfig.include(context, name, package)
     context.execute_actions(clear=False)
+    return context.actions
 
-    actions = context.actions
-    pckname = pickle_name(name, package)
-
-    for action in actions:
-        
-        discriminator = action[0]
-        if discriminator and Uncacheable in discriminator:
-            remove(pckname)
-            return False
-
-    try:
-        data = (PVERSION, time.time(), actions)
-        dump(data, open(pckname, 'wb'), -1)
-    except (OSError, IOError, TypeError, cPickle.PickleError):
-        remove(pckname)
-
-    return False
+file_configure = zcml_configure # backwards compat (>0.8.1)
 
 def exclude(name):
     if name.startswith('.'):