Extract tempdir handling to a module which class extend for use

Purely for better organization of he code. Prior uses are maintained with
deprecation.

CHANGELOG.md

@@ -4,6 +4,11 @@
   all known cases, rather than just when the #close method is called. The #close
   method can be used to cleanup early. [326](https://github.com/roo-rb/roo/issues/326)
 
+### Deprecations
+- Roo::Base::TEMP_PREFIX should be accessed via Roo::TEMP_PREFIX
+- The private Roo::Base#make_tempdir is now available at the class level in
+  classes that use tempdirs, added via Roo::Tempdir
+
 ## [2.4.0] 2016-05-14
 ### Fixed  
 - Fixed opening spreadsheets with charts [315](https://github.com/roo-rb/roo/pull/315)

lib/roo.rb

@@ -9,6 +9,8 @@ module Roo
   autoload :Excelx,       'roo/excelx'
   autoload :CSV,          'roo/csv'
 
+  TEMP_PREFIX = 'roo_'.freeze
+
   CLASS_FOR_EXTENSION = {
     ods: Roo::OpenOffice,
     xlsx: Roo::Excelx,

lib/roo/base.rb

@@ -9,7 +9,6 @@
 class Roo::Base
   include Enumerable
 
-  TEMP_PREFIX = 'roo_'.freeze
   MAX_ROW_COL = 999_999.freeze
   MIN_ROW_COL = 0.freeze
 
@@ -18,27 +17,9 @@ class Roo::Base
   # sets the line with attribute names (default: 1)
   attr_accessor :header_line
 
-  class << self
-    attr_reader :tempdirs
-
-    def record_tempdir(object, path)
-      @tempdirs ||= {}
-      if tempdirs[object.object_id]
-        tempdirs[object.object_id] << path
-      else
-        tempdirs[object.object_id] = [path]
-        ObjectSpace.define_finalizer(object, method(:finalize))
-      end
-    end
-
-    def finalize(object_id)
-      if tempdirs && (dirs_to_remove = tempdirs[object_id])
-        tempdirs[object_id] = nil
-        dirs_to_remove.each do |dir|
-          ::FileUtils.remove_entry(dir)
-        end
-      end
-    end
+  def self.TEMP_PREFIX
+    warn '[DEPRECATION] please access TEMP_PREFIX via Roo::TEMP_PREFIX'
+    Roo::TEMP_PREFIX
   end
 
   def initialize(filename, options = {}, _file_warning = :error, _tmpdir = nil)
@@ -55,13 +36,12 @@ def initialize(filename, options = {}, _file_warning = :error, _tmpdir = nil)
     @last_column = {}
 
     @header_line = 1
-  rescue
-    self.class.finalize(object_id)
-    raise
   end
 
   def close
-    self.class.finalize(object_id)
+    if self.class.respond_to?(:finalize_tempdirs)
+      self.class.finalize_tempdirs(object_id)
+    end
     nil
   end
 
@@ -564,17 +544,15 @@ def find_basename(filename)
   end
 
   def make_tmpdir(prefix = nil, root = nil, &block)
+    warn '[DEPRECATION] extend Roo::Tempdir and use its .make_tempdir instead'
     prefix = "#{TEMP_PREFIX}#{prefix}"
     root ||= ENV['ROO_TMP']
 
     if block_given?
       # folder is deleted at end of block
       ::Dir.mktmpdir(prefix, root, &block)
     else
-      # folder is cleaned up in .finalize
-      ::Dir.mktmpdir(prefix, root).tap do |tmpdir|
-        self.class.record_tempdir(self, tmpdir)
-      end
+      self.class.make_tempdir(self, prefix, root)
     end
   end
 

lib/roo/csv.rb

@@ -56,7 +56,7 @@ def celltype_class(value)
 
   def each_row(options, &block)
     if uri?(filename)
-      make_tmpdir do |tmpdir|
+      ::Dir.mktmpdir(Roo::TEMP_PREFIX, ENV['ROO_TMP']) do |tmpdir|
         tmp_filename = download_uri(filename, tmpdir)
         CSV.foreach(tmp_filename, options, &block)
       end

lib/roo/excelx.rb

@@ -1,11 +1,14 @@
 require 'nokogiri'
 require 'zip/filesystem'
 require 'roo/link'
+require 'roo/tempdir'
 require 'roo/utils'
 require 'forwardable'
 
 module Roo
   class Excelx < Roo::Base
+    extend Roo::Tempdir
+
     require 'set'
     extend Forwardable
 
@@ -42,7 +45,7 @@ def initialize(filename_or_stream, options = {})
         basename = find_basename(filename_or_stream)
       end
 
-      @tmpdir = make_tmpdir(basename, options[:tmpdir_root])
+      @tmpdir = self.class.make_tempdir(self, basename, options[:tmpdir_root])
       @shared = Shared.new(@tmpdir)
       @filename = local_filename(filename_or_stream, @tmpdir, packed)
       process_zipfile(@filename || filename_or_stream)
@@ -65,7 +68,7 @@ def initialize(filename_or_stream, options = {})
 
       super
     rescue
-      self.class.finalize(object_id)
+      self.class.finalize_tempdirs(object_id)
       raise
     end
 

lib/roo/open_office.rb

@@ -3,10 +3,13 @@
 require 'cgi'
 require 'zip/filesystem'
 require 'roo/font'
+require 'roo/tempdir'
 require 'base64'
 
 module Roo
   class OpenOffice < Roo::Base
+    extend Roo::Tempdir
+
     ERROR_MISSING_CONTENT_XML = 'file missing required content.xml'.freeze
     XPATH_FIND_TABLE_STYLES   = "//*[local-name()='automatic-styles']".freeze
     XPATH_LOCAL_NAME_TABLE    = "//*[local-name()='table']".freeze
@@ -19,7 +22,7 @@ def initialize(filename, options = {})
 
       @only_visible_sheets = options[:only_visible_sheets]
       file_type_check(filename, '.ods', 'an Roo::OpenOffice', file_warning, packed)
-      @tmpdir   = make_tmpdir(find_basename(filename), options[:tmpdir_root])
+      @tmpdir   = self.class.make_tempdir(self, find_basename(filename), options[:tmpdir_root])
       @filename = local_filename(filename, @tmpdir, packed)
       # TODO: @cells_read[:default] = false
       open_oo_file(options)
@@ -38,7 +41,7 @@ def initialize(filename, options = {})
         end
       end.compact
     rescue
-      self.class.finalize(object_id)
+      self.class.finalize_tempdirs(object_id)
       raise
     end
 

lib/roo/tempdir.rb

@@ -0,0 +1,26 @@
+module Roo
+  module Tempdir
+    def finalize_tempdirs(object_id)
+      if @tempdirs && (dirs_to_remove = @tempdirs[object_id])
+        @tempdirs[object_id] = nil
+        dirs_to_remove.each do |dir|
+          ::FileUtils.remove_entry(dir)
+        end
+      end
+    end
+
+    def make_tempdir(object, prefix, root)
+      root ||= ENV['ROO_TMP']
+      # folder is cleaned up in .finalize_tempdirs
+      ::Dir.mktmpdir("#{Roo::TEMP_PREFIX}#{prefix}", root).tap do |tmpdir|
+        @tempdirs ||= {}
+        if @tempdirs[object.object_id]
+          @tempdirs[object.object_id] << tmpdir
+        else
+          @tempdirs[object.object_id] = [tmpdir]
+          ObjectSpace.define_finalizer(object, method(:finalize_tempdirs))
+        end
+      end
+    end
+  end
+end