Create a Workbench class and use it everywhere.

Some references to Workbench.dir look ugly, I'm happy to
hear suggestions there.

mediagoblin/process_media/__init__.py

@@ -32,8 +32,8 @@ def process_media_initial(media_id):
         {'_id': ObjectId(media_id)})
 
     queued_filepath = entry['queued_media_file']
-    queued_filename = mgg.workbench_manager.localized_file(
+    queued_filename = workbench.localized_file(
-        workbench, mgg.queue_store, queued_filepath,
+        mgg.queue_store, queued_filepath,
         'source')
 
     queued_file = file(queued_filename, 'r')

mediagoblin/tests/test_workbench.py

@@ -30,24 +30,31 @@ class TestWorkbench(object):
 
     def test_create_workbench(self):
         workbench = self.workbench_manager.create_workbench()
-        assert os.path.isdir(workbench)
+        assert os.path.isdir(workbench.dir)
-        assert workbench.startswith(self.workbench_manager.base_workbench_dir)
+        assert workbench.dir.startswith(self.workbench_manager.base_workbench_dir)
+
+    def test_joinpath(self):
+        this_workbench = self.workbench_manager.create_workbench()
+        tmpname = this_workbench.joinpath('temp.txt')
+        assert tmpname == os.path.join(this_workbench.dir, 'temp.txt')
+        self.workbench_manager.destroy_workbench(this_workbench)
 
     def test_destroy_workbench(self):
         # kill a workbench
         this_workbench = self.workbench_manager.create_workbench()
-        tmpfile = file(os.path.join(this_workbench, 'temp.txt'), 'w')
+        tmpfile_name = this_workbench.joinpath('temp.txt')
+        tmpfile = file(tmpfile_name, 'w')
         with tmpfile:
             tmpfile.write('lollerskates')
 
-        assert os.path.exists(os.path.join(this_workbench, 'temp.txt'))
+        assert os.path.exists(tmpfile_name)
 
         self.workbench_manager.destroy_workbench(this_workbench)
-        assert not os.path.exists(os.path.join(this_workbench, 'temp.txt'))
+        assert not os.path.exists(tmpfile_name)
-        assert not os.path.exists(this_workbench)
+        assert not os.path.exists(this_workbench.dir)
 
         # make sure we can't kill other stuff though
-        dont_kill_this = tempfile.mkdtemp()
+        dont_kill_this = workbench.Workbench(tempfile.mkdtemp())
 
         assert_raises(
             workbench.WorkbenchOutsideScope,
@@ -65,8 +72,7 @@ class TestWorkbench(object):
             our_file.write('Our file')
 
         # with a local file storage
-        filename = self.workbench_manager.localized_file(
+        filename = this_workbench.localized_file(this_storage, filepath)
-            this_workbench, this_storage, filepath)
         assert filename == os.path.join(
             tmpdir, 'dir1/dir2/ourfile.txt')
 
@@ -80,17 +86,17 @@ class TestWorkbench(object):
         filename = self.workbench_manager.localized_file(
             this_workbench, this_storage, filepath)
         assert filename == os.path.join(
-            this_workbench, 'ourfile.txt')
+            this_workbench.dir, 'ourfile.txt')
         
         # fake remote file storage, filename_if_copying set
         filename = self.workbench_manager.localized_file(
             this_workbench, this_storage, filepath, 'thisfile')
         assert filename == os.path.join(
-            this_workbench, 'thisfile.txt')
+            this_workbench.dir, 'thisfile.txt')
 
         # fake remote file storage, filename_if_copying set,
         # keep_extension_if_copying set to false
         filename = self.workbench_manager.localized_file(
             this_workbench, this_storage, filepath, 'thisfile.text', False)
         assert filename == os.path.join(
-            this_workbench, 'thisfile.text')
+            this_workbench.dir, 'thisfile.text')

mediagoblin/workbench.py

@@ -36,41 +36,24 @@ class WorkbenchOutsideScope(Exception):
 # Actual workbench stuff
 # ----------------------
 
-class WorkbenchManager(object):
+class Workbench(object):
     """
-    A system for generating and destroying workbenches.
+    Represent the directory for the workbench
-
-    Workbenches are actually just subdirectories of a temporary storage space
-    for during the processing stage.
     """
+    def __init__(self, dir):
+        self.dir = dir
 
-    def __init__(self, base_workbench_dir):
+    def __unicode__(self):
-        self.base_workbench_dir = os.path.abspath(base_workbench_dir)
+        return unicode(self.dir)
-        if not os.path.exists(self.base_workbench_dir):
+    def __str__(self):
-            os.makedirs(self.base_workbench_dir)
+        return str(self.dir)
+    def __repr__(self):
+        return repr(self.dir)
 
-    def create_workbench(self):
+    def joinpath(self, *args):
-        """
+        return os.path.join(self.dir, *args)
-        Create and return the path to a new workbench (directory).
-        """
-        return tempfile.mkdtemp(dir=self.base_workbench_dir)
 
-    def destroy_workbench(self, workbench):
+    def localized_file(self, storage, filepath,
-        """
-        Destroy this workbench!  Deletes the directory and all its contents!
-
-        Makes sure the workbench actually belongs to this manager though.
-        """
-        # just in case
-        workbench = os.path.abspath(workbench)
-
-        if not workbench.startswith(self.base_workbench_dir):
-            raise WorkbenchOutsideScope(
-                "Can't destroy workbench outside the base workbench dir")
-
-        shutil.rmtree(workbench)
-
-    def localized_file(self, workbench, storage, filepath,
                        filename_if_copying=None,
                        keep_extension_if_copying=True):
         """
@@ -126,10 +109,48 @@ class WorkbenchManager(object):
                     dest_filename = filename_if_copying
 
             full_dest_filename = os.path.join(
-                workbench, dest_filename)
+                self.dir, dest_filename)
 
             # copy it over
             storage.copy_locally(
                 filepath, full_dest_filename)
 
             return full_dest_filename
+
+
+class WorkbenchManager(object):
+    """
+    A system for generating and destroying workbenches.
+
+    Workbenches are actually just subdirectories of a temporary storage space
+    for during the processing stage.
+    """
+
+    def __init__(self, base_workbench_dir):
+        self.base_workbench_dir = os.path.abspath(base_workbench_dir)
+        if not os.path.exists(self.base_workbench_dir):
+            os.makedirs(self.base_workbench_dir)
+        
+    def create_workbench(self):
+        """
+        Create and return the path to a new workbench (directory).
+        """
+        return Workbench(tempfile.mkdtemp(dir=self.base_workbench_dir))
+
+    def destroy_workbench(self, workbench):
+        """
+        Destroy this workbench!  Deletes the directory and all its contents!
+
+        Makes sure the workbench actually belongs to this manager though.
+        """
+        # just in case
+        workbench = os.path.abspath(workbench.dir)
+
+        if not workbench.startswith(self.base_workbench_dir):
+            raise WorkbenchOutsideScope(
+                "Can't destroy workbench outside the base workbench dir")
+
+        shutil.rmtree(workbench)
+
+    def localized_file(self, workbench, *args, **kwargs):
+        return workbench.localized_file(*args, **kwargs)