Write cache to file on CacheStorage object delete

Use a destructor to write out the cache to file when the cache object
object goes out of scope, which will typically be on exit.

This ensures that the CacheStorage class behaviour is fully
encapsulated and doesn't require a caller to be aware to ensure the
cache is saved when finished. Although it may still do so.

Take care to preserve a references to any required modules since python
does not guarantee that any global modules will not be unloaded before
the destructor is called.

Change-Id: I2b066ceff5e23a725132569df85c004cd58b367a

jenkins_jobs/builder.py

@@ -517,6 +517,13 @@ class XmlJob(object):
 
 
 class CacheStorage(object):
+    # ensure each instance of the class has a reference to the required
+    # modules so that they are available to be used when the destructor
+    # is being called since python will not guarantee that it won't have
+    # removed global module references during teardown.
+    _yaml = yaml
+    _logger = logger
+
     def __init__(self, jenkins_url, flush=False):
         cache_dir = self.get_cache_dir()
         # One cache per remote Jenkins URL:
@@ -525,9 +532,9 @@ class CacheStorage(object):
             cache_dir, 'cache-host-jobs-' + host_vary + '.yml')
         if flush or not os.path.isfile(self.cachefilename):
             self.data = {}
-            return
+        else:
-        with file(self.cachefilename, 'r') as yfile:
+            with file(self.cachefilename, 'r') as yfile:
-            self.data = yaml.load(yfile)
+                self.data = yaml.load(yfile)
         logger.debug("Using cache: '{0}'".format(self.cachefilename))
 
     @staticmethod
@@ -544,9 +551,6 @@ class CacheStorage(object):
 
     def set(self, job, md5):
         self.data[job] = md5
-        yfile = file(self.cachefilename, 'w')
-        yaml.dump(self.data, yfile)
-        yfile.close()
 
     def is_cached(self, job):
         if job in self.data:
@@ -558,6 +562,24 @@ class CacheStorage(object):
             return False
         return True
 
+    def save(self):
+        # check we initialized sufficiently in case called via __del__
+        # due to an exception occurring in the __init__
+        if getattr(self, 'data', None) is not None:
+            try:
+                with open(self.cachefilename, 'w') as yfile:
+                    self._yaml.dump(self.data, yfile)
+            except Exception as e:
+                self._logger.error("Failed to write to cache file '%s' on "
+                                   "exit: %s" % (self.cachefilename, e))
+            else:
+                self._logger.info("Cache saved")
+                self._logger.debug("Cache written out to '%s'" %
+                                   self.cachefilename)
+
+    def __del__(self):
+        self.save()
+
 
 class Jenkins(object):
     def __init__(self, url, user, password):

tests/cachestorage/test_cachestorage.py

@@ -0,0 +1,34 @@
+#
+#  - Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import mock
+import testtools
+
+import jenkins_jobs
+
+
+class TestCaseCacheStorage(testtools.TestCase):
+
+    @mock.patch('jenkins_jobs.builder.CacheStorage.get_cache_dir',
+                lambda x: '/bad/file')
+    def test_save_on_exit(self):
+        """
+        Test that the cache is saved on normal object deletion
+        """
+
+        with mock.patch('jenkins_jobs.builder.CacheStorage.save') as save_mock:
+            with mock.patch('os.path.isfile', return_value=False):
+                jenkins_jobs.builder.CacheStorage("dummy")
+            save_mock.assert_called_once_with()