New hook octoprint.filemanager.preprocessor

Allows preprocessing files that are uploaded or otherwise added to the system (e.g. through slicing) before their contents are saved to disk

docs/modules/filemanager.rst

@@ -30,3 +30,10 @@ octoprint.filemanager.storage
 .. automodule:: octoprint.filemanager.storage
    :members: StorageInterface, LocalFileStorage
 
+.. _sec-modules-filemanager-util:
+
+octoprint.filemanager.util
+--------------------------
+
+.. automodule:: octoprint.filemanager.util
+   :members:

docs/plugins/hooks.rst

@@ -183,11 +183,11 @@ octoprint.comm.transport.serial.factory
           return serial_obj
 
    :param MachineCom comm_instance: The :class:`~octoprint.util.comm.MachineCom` instance which triggered the hook.
-   :param port str: The port for which to construct a serial instance. May be ``None`` or ``AUTO`` in which case port
+   :param str port: The port for which to construct a serial instance. May be ``None`` or ``AUTO`` in which case port
        auto detection is to be performed.
-   :param baudrate int: The baudrate for which to construct a serial instance. May be 0 in which case baudrate auto
+   :param int baudrate: The baudrate for which to construct a serial instance. May be 0 in which case baudrate auto
        detection is to be performed.
-   :param read_timeout int: The read timeout to set on the serial port.
+   :param int read_timeout: The read timeout to set on the serial port.
    :return: The constructed serial object ready for use, or ``None`` if the handler could not construct the object.
    :rtype: A serial instance implementing implementing the methods ``readline(...)``, ``write(...)``, ``close()`` and
        optionally ``baudrate`` and ``timeout`` attributes as described above.
@@ -226,4 +226,38 @@ octoprint.filemanager.extension_tree
       in the rest of the system (e.g. handling/preprocessing new machine code file types for printing etc)!
 
    :return: The partial extension tree to merge with the full extension tree.
-   :rtype: dict
+   :rtype: dict
+
+.. _sec-plugins-hook-filemanager-preprocessor:
+
+octoprint.filemanager.preprocessor
+----------------------------------
+
+.. py:function:: hook(path, file_object, links=None, printer_profile=None, allow_overwrite=False)
+
+   Replace the ``file_object`` used for saving added files to storage by calling :func:`~octoprint.filemanager.util.AbstractFileWrapper.save`.
+
+   ``path`` will be the future path of the file on the storage. The file's name is accessible via
+   :attr:`~octoprint.filemanager.util.AbstractFileWrapper.filename`.
+
+   ``file_object`` will be a subclass of :class:`~octoprint.filemanager.util.AbstractFileWrapper`. Handlers may
+   access the raw data of the file via :func:`~octoprint.filemanager.util.AbstractFileWrapper.stream`, e.g.
+   to wrap it further. Handlers which do not wish to handle the `file_object`
+
+   **Example**
+
+   The following plugin example strips all comments from uploaded/generated GCODE files ending on the name postfix ``_strip``.
+
+   .. onlineinclude:: https://raw.githubusercontent.com/OctoPrint/Plugin-Examples/master/strip_all_comments.py
+      :linenos:
+      :tab-width: 4
+      :caption: `strip_all_comments.py <https://github.com/OctoPrint/Plugin-Examples/blob/master/strip_all_comments.py>`_
+
+   :param str path: The path on storage the `file_object` is to be stored
+   :param AbstractFileWrapper file_object: The :class:`~octoprint.filemanager.util.AbstractFileWrapper` instance
+       representing the file object to store.
+   :param dict links: The links that are going to be stored with the file.
+   :param dict printer_profile: The printer profile associated with the file.
+   :param boolean allow_overwrite: Whether to allow overwriting an existing file named the same or not.
+   :return: The `file_object` as passed in or None, or a replaced version to use instead for further processing.
+   :rtype: AbstractFileWrapper or None

src/octoprint/filemanager/__init__.py

@@ -16,6 +16,7 @@ from octoprint.events import eventManager, Events
 from .destinations import FileDestinations
 from .analysis import QueueEntry, AnalysisQueue
 from .storage import LocalFileStorage
+from .util import AbstractFileWrapper, StreamWrapper, DiskFileWrapper
 
 extensions = dict(
 )
@@ -130,11 +131,18 @@ class FileManager(object):
 		self._slicing_progress_callbacks = []
 		self._last_slicing_progress = None
 
-		self._progress_plugins = octoprint.plugin.plugin_manager().get_implementations(octoprint.plugin.ProgressPlugin)
+		self._progress_plugins = []
+		self._preprocessor_hooks = dict()
 
+	def initialize(self):
+		self.reload_plugins()
 		for storage_type, storage_manager in self._storage_managers.items():
 			self._determine_analysis_backlog(storage_type, storage_manager)
 
+	def reload_plugins(self):
+		self._progress_plugins = octoprint.plugin.plugin_manager().get_implementations(octoprint.plugin.ProgressPlugin)
+		self._preprocessor_hooks = octoprint.plugin.plugin_manager().get_hooks("octoprint.filemanager.preprocessor")
+
 	def register_slicingprogress_callback(self, callback):
 		self._slicing_progress_callbacks.append(callback)
 
@@ -185,22 +193,12 @@ class FileManager(object):
 					source_meta = self.get_metadata(source_location, source_path)
 					hash = source_meta["hash"]
 
-					class Wrapper(object):
+					import io
-						def __init__(self, stl_name, temp_path, hash):
-							self.stl_name = stl_name
-							self.temp_path = temp_path
-							self.hash = hash
-
-						def save(self, absolute_dest_path):
-							with open(absolute_dest_path, "w") as d:
-								d.write(";Generated from {stl_name} {hash}\r".format(**vars(self)))
-								with open(tmp_path, "r") as s:
-									import shutil
-									shutil.copyfileobj(s, d)
-
 					links = [("model", dict(name=source_path))]
 					_, stl_name = self.split_path(source_location, source_path)
-					file_obj = Wrapper(stl_name, temp_path, hash)
+					file_obj = StreamWrapper(os.path.basename(dest_path),
+					                         io.BytesIO(u";Generated from {stl_name} {hash}\n".format(**locals()).encode("ascii", "replace")),
+					                         io.FileIO(tmp_path, "rb"))
 
 					printer_profile = self._printer_profile_manager.get(printer_profile_id)
 					self.add_file(dest_location, dest_path, file_obj, links=links, allow_overwrite=True, printer_profile=printer_profile, analysis=_analysis)
@@ -305,6 +303,10 @@ class FileManager(object):
 		if printer_profile is None:
 			printer_profile = self._printer_profile_manager.get_current_or_default()
 
+		for hook in self._preprocessor_hooks.values():
+			hook_file_object = hook(path, file_object, links=links, printer_profile=printer_profile, allow_overwrite=allow_overwrite)
+			if hook_file_object is not None:
+				file_object = hook_file_object
 		file_path = self._storage(destination).add_file(path, file_object, links=links, printer_profile=printer_profile, allow_overwrite=allow_overwrite)
 		absolute_path = self._storage(destination).path_on_disk(file_path)
 

src/octoprint/filemanager/storage.py

@@ -13,8 +13,6 @@ import tempfile
 
 import octoprint.filemanager
 
-from octoprint.util import safe_rename
-
 class StorageInterface(object):
 	"""
 	Interface of storage adapters for OctoPrint.
@@ -1003,7 +1001,8 @@ class LocalFileStorage(StorageInterface):
 				with open(metadata_temporary_path, "w") as f:
 					import yaml
 					yaml.safe_dump(metadata, stream=f, default_flow_style=False, indent="  ", allow_unicode=True)
-				safe_rename(metadata_temporary_path, metadata_path, throw_error=True)
+				import shutil
+				shutil.move(metadata_temporary_path, metadata_path)
 			except:
 				self._logger.exception("Error while writing .metadata.yaml to {path}".format(**locals()))
 			else:

src/octoprint/filemanager/util.py

@@ -0,0 +1,239 @@
+# coding=utf-8
+from __future__ import absolute_import
+
+__author__ = "Gina Häußge <osd@foosel.net>"
+__license__ = 'GNU Affero General Public License http://www.gnu.org/licenses/agpl.html'
+__copyright__ = "Copyright (C) 2015 The OctoPrint Project - Released under terms of the AGPLv3 License"
+
+import io
+
+class AbstractFileWrapper(object):
+	"""
+	Wrapper for file representations to save to storages.
+
+	Arguments:
+	    filename (str): The file's name
+	"""
+
+	def __init__(self, filename):
+		self.filename = filename
+
+	def save(self, path):
+		"""
+		Saves the file's content to the given absolute path.
+
+		Arguments:
+		    path (str): The absolute path to where to save the file
+		"""
+		raise NotImplementedError()
+
+	def stream(self):
+		"""
+		Returns a Python stream object (subclass of io.IOBase) representing the file's contents.
+
+		Returns:
+		    io.IOBase: The file's contents as a stream.
+		"""
+		raise NotImplementedError()
+
+class DiskFileWrapper(AbstractFileWrapper):
+	"""
+	An implementation of :class:`.AbstractFileWrapper` that wraps an actual file on disk. The `save` implementations
+	will either copy the file to the new path (preserving file attributes) or -- if `move` is `True` (the default) --
+	move the file.
+
+	Arguments:
+	    filename (str): The file's name
+	    path (str): The file's absolute path
+	    move (boolean): Whether to move the file upon saving (True, default) or copying.
+	"""
+
+	def __init__(self, filename, path, move=True):
+		AbstractFileWrapper.__init__(self, filename)
+		self.path = path
+		self.move = move
+
+	def save(self, path):
+		import shutil
+
+		if self.move:
+			shutil.move(self.path, path)
+		else:
+			shutil.copy2(self.path, path)
+
+	def stream(self):
+		return io.open(self.path, "rb")
+
+class StreamWrapper(AbstractFileWrapper):
+	"""
+	A wrapper allowing processing of one or more consecutive streams.
+
+	Arguments:
+	    *streams (io.IOBase): One or more streams to process one after another to save to storage.
+	"""
+	def __init__(self, filename, *streams):
+		if not len(streams) > 0:
+			raise ValueError("Need at least one stream to wrap")
+
+		AbstractFileWrapper.__init__(self, filename)
+		self.streams = streams
+
+	def save(self, path):
+		"""
+		Will dump the contents of all streams provided during construction into the target file, in the order they were
+		provided.
+		"""
+		import shutil
+
+		with open(path, "wb") as dest:
+			with self.stream() as source:
+				shutil.copyfileobj(source, dest)
+
+	def stream(self):
+		"""
+		If more than one stream was provided to the constructor, will return a :class:`.MultiStream` wrapping all
+		provided streams in the order they were provided, else the first and only stream is returned directly.
+		"""
+		if len(self.streams) > 1:
+			return MultiStream(*self.streams)
+		else:
+			return self.streams[0]
+
+class MultiStream(io.RawIOBase):
+	"""
+	A stream implementation which when read reads from multiple streams, one after the other, basically concatenating
+	their contents in the order they are provided to the constructor.
+
+	Arguments:
+	    *streams (io.IOBase): One or more streams to concatenate.
+	"""
+	def __init__(self, *streams):
+		io.RawIOBase.__init__(self)
+		self.streams = streams
+		self.current_stream = 0
+
+	def read(self, n=-1):
+		if n == 0:
+			return b''
+
+		if len(self.streams) == 0:
+			return b''
+
+		while self.current_stream < len(self.streams):
+			stream = self.streams[self.current_stream]
+
+			result = stream.read(n)
+			if result is None or len(result) != 0:
+				return result
+			else:
+				self.current_stream += 1
+
+		return b''
+
+	def readinto(self, b):
+		n = len(b)
+		read = self.read(n)
+		b[:len(read)] = read
+		return len(read)
+
+	def close(self):
+		for stream in self.streams:
+			try:
+				stream.close()
+			except:
+				pass
+
+	def readable(self, *args, **kwargs):
+		return True
+
+	def seekable(self, *args, **kwargs):
+		return False
+
+	def writable(self, *args, **kwargs):
+		return False
+
+class LineProcessorStream(io.RawIOBase):
+	"""
+	While reading from this stream the provided `input_stream` is read line by line, calling the (overridable) method
+	:meth:`.process_line` for each read line.
+
+	Sub classes can thus modify the contents of the `input_stream` in line, while it is being read.
+
+	Arguments:
+	    input_stream (io.IOBase): The stream to process on the fly.
+	"""
+
+	def __init__(self, input_stream):
+		io.RawIOBase.__init__(self)
+		self.input_stream = io.BufferedReader(input_stream)
+		self.leftover = None
+
+	def read(self, n=-1):
+		if n == 0:
+			return b''
+
+		result = b''
+		while len(result) < n or n == -1:
+			bytes_left = (n - len(result)) if n != -1 else -1
+			if self.leftover is not None:
+				if bytes_left != -1 and bytes_left < len(self.leftover):
+					result += self.leftover[:bytes_left]
+					self.leftover = self.leftover[bytes_left:]
+					break
+				else:
+					result += self.leftover
+					self.leftover = None
+
+			processed_line = None
+			while processed_line is None:
+				line = self.input_stream.readline()
+				if not line:
+					break
+				processed_line = self.process_line(line)
+
+			if processed_line is None:
+				break
+
+			bytes_left = (n - len(result)) if n != -1 else -1
+			if bytes_left != -1 and bytes_left < len(processed_line):
+				result += processed_line[:bytes_left]
+				self.leftover = processed_line[bytes_left:]
+				break
+			else:
+				result += processed_line
+
+		return result
+
+	def readinto(self, b):
+		n = len(b)
+		read = self.read(n)
+		b[:len(read)] = read
+		return len(read)
+
+	def process_line(self, line):
+		"""
+		Called from the `read` Method of this stream with each line read from `self.input_stream`.
+
+		By returning ``None`` the line will not be returned from the read stream, effectively being stripped from the
+		wrapper `input_stream`.
+
+		Arguments:
+		    line (str): The line as read from `self.input_stream`
+
+		Returns:
+		    str or None: The processed version of the line (might also be multiple lines), or None if the line is to be
+		        stripped from the processed stream.
+		"""
+		return line
+
+	def close(self):
+		self.input_stream.close()
+
+	def readable(self, *args, **kwargs):
+		return True
+
+	def seekable(self, *args, **kwargs):
+		return False
+
+	def writable(self, *args, **kwargs):
+		return False

src/octoprint/plugin/types.py

@@ -64,6 +64,11 @@ class OctoPrintPlugin(Plugin):
 
 	   The :class:`~octoprint.users.SessionManager` instance. Injected by the plugin core system upon initialization of
 	   the implementation.
+
+	.. attribute:: _plugin_lifecycle_manager
+
+	   The :class:`~octoprint.server.LifecycleManager` instance. Injected by the plugin core system upon initialization
+	   of the implementation.
 	"""
 
 	pass

src/octoprint/server/__init__.py

@@ -591,6 +591,7 @@ class Server():
 		fileManager = octoprint.filemanager.FileManager(analysisQueue, slicingManager, printerProfileManager, initial_storage_managers=storage_managers)
 		printer = Printer(fileManager, analysisQueue, printerProfileManager)
 		appSessionManager = util.flask.AppSessionManager()
+		pluginLifecycleManager = LifecycleManager(pluginManager)
 
 		def octoprint_plugin_inject_factory(name, implementation):
 			if not isinstance(implementation, octoprint.plugin.OctoPrintPlugin):
@@ -603,7 +604,8 @@ class Server():
 				slicing_manager=slicingManager,
 				file_manager=fileManager,
 				printer=printer,
-				app_session_manager=appSessionManager
+				app_session_manager=appSessionManager,
+				plugin_lifecycle_manager=pluginLifecycleManager
 			)
 
 		def settings_plugin_inject_factory(name, implementation):
@@ -620,12 +622,15 @@ class Server():
 		pluginManager.implementation_inject_factories=[octoprint_plugin_inject_factory, settings_plugin_inject_factory]
 		pluginManager.initialize_implementations()
 
-		lifecycleManager = LifecycleManager(self, pluginManager)
 		pluginManager.log_all_plugins()
 
+		# initialize file manager and register it for changes in the registered plugins
+		fileManager.initialize()
+		pluginLifecycleManager.add_callback(["enabled", "disabled"], lambda name, plugin: fileManager.reload_plugins())
+
 		# initialize slicing manager and register it for changes in the registered plugins
 		slicingManager.initialize()
-		lifecycleManager.add_callback(["enabled", "disabled"], lambda name, plugin: slicingManager.reload_slicers())
+		pluginLifecycleManager.add_callback(["enabled", "disabled"], lambda name, plugin: slicingManager.reload_slicers())
 
 		# setup jinja2
 		self._setup_jinja2()
@@ -637,8 +642,8 @@ class Server():
 			if plugin.implementation is None or not isinstance(plugin.implementation, octoprint.plugin.TemplatePlugin):
 				return
 			self._unregister_additional_template_plugin(plugin.implementation)
-		lifecycleManager.add_callback("enabled", template_enabled)
+		pluginLifecycleManager.add_callback("enabled", template_enabled)
-		lifecycleManager.add_callback("disabled", template_disabled)
+		pluginLifecycleManager.add_callback("disabled", template_disabled)
 
 		# configure timelapse
 		octoprint.timelapse.configureTimelapse()
@@ -696,7 +701,7 @@ class Server():
 			if plugin.implementation is None or not isinstance(plugin.implementation, octoprint.plugin.BlueprintPlugin):
 				return
 			self._register_blueprint_plugin(plugin.implementation)
-		lifecycleManager.add_callback(["enabled"], blueprint_enabled)
+		pluginLifecycleManager.add_callback(["enabled"], blueprint_enabled)
 
 		## Tornado initialization starts here
 
@@ -742,7 +747,7 @@ class Server():
 			if implementation is None:
 				return
 			implementation.on_startup(self._host, self._port)
-		lifecycleManager.add_callback("enabled", call_on_startup)
+		pluginLifecycleManager.add_callback("enabled", call_on_startup)
 
 		# prepare our after startup function
 		def on_after_startup():
@@ -762,7 +767,7 @@ class Server():
 					if implementation is None:
 						return
 					implementation.on_after_startup()
-				lifecycleManager.add_callback("enabled", call_on_after_startup)
+				pluginLifecycleManager.add_callback("enabled", call_on_after_startup)
 
 			import threading
 			threading.Thread(target=work).start()
@@ -929,25 +934,23 @@ class Server():
 			self._logger.debug("Registered API of plugin {name} under URL prefix {url_prefix}".format(name=name, url_prefix=url_prefix))
 
 class LifecycleManager(object):
-	def __init__(self, server, plugin_manager):
+	def __init__(self, plugin_manager):
-		self._server = server
 		self._plugin_manager = plugin_manager
 
 		self._plugin_lifecycle_callbacks = defaultdict(list)
 		self._logger = logging.getLogger(__name__)
 
-		def on_plugin_event_factory(lifecycle_event, text):
+		def on_plugin_event_factory(lifecycle_event):
 			def on_plugin_event(name, plugin):
 				self.on_plugin_event(lifecycle_event, name, plugin)
-				self._logger.debug(text.format(**locals()))
 			return on_plugin_event
 
-		self._plugin_manager.on_plugin_loaded = on_plugin_event_factory("loaded", "Loaded plugin {name}: {plugin}")
+		self._plugin_manager.on_plugin_loaded = on_plugin_event_factory("loaded")
-		self._plugin_manager.on_plugin_unloaded = on_plugin_event_factory("unloaded", "Unloaded plugin {name}: {plugin}")
+		self._plugin_manager.on_plugin_unloaded = on_plugin_event_factory("unloaded")
-		self._plugin_manager.on_plugin_activated = on_plugin_event_factory("activated", "Activated plugin {name}: {plugin}")
+		self._plugin_manager.on_plugin_activated = on_plugin_event_factory("activated")
-		self._plugin_manager.on_plugin_deactivated = on_plugin_event_factory("deactivated", "Deactivated plugin {name}: {plugin}")
+		self._plugin_manager.on_plugin_deactivated = on_plugin_event_factory("deactivated")
-		self._plugin_manager.on_plugin_enabled = on_plugin_event_factory("enabled", "Enabled plugin {name}: {plugin}")
+		self._plugin_manager.on_plugin_enabled = on_plugin_event_factory("enabled")
-		self._plugin_manager.on_plugin_disabled = on_plugin_event_factory("disabled", "Disabled plugin {name}: {plugin}")
+		self._plugin_manager.on_plugin_disabled = on_plugin_event_factory("disabled")
 
 	def on_plugin_event(self, event, name, plugin):
 		for lifecycle_callback in self._plugin_lifecycle_callbacks[event]:

src/octoprint/server/api/files.py

@@ -15,6 +15,7 @@ from octoprint.server.util.flask import restricted_access, get_json_command_from
 from octoprint.server.api import api
 from octoprint.events import Events
 import octoprint.filemanager
+import octoprint.filemanager.util
 import octoprint.slicing
 
 
@@ -133,12 +134,9 @@ def uploadGcodeFile(target):
 	input_upload_name = input_name + "." + settings().get(["server", "uploads", "nameSuffix"])
 	input_upload_path = input_name + "." + settings().get(["server", "uploads", "pathSuffix"])
 	if input_upload_name in request.values and input_upload_path in request.values:
-		import shutil
+		upload = octoprint.filemanager.util.DiskFileWrapper(request.values[input_upload_name], request.values[input_upload_path])
-		upload = util.Object()
-		upload.filename = request.values[input_upload_name]
-		upload.save = lambda new_path: shutil.move(request.values[input_upload_path], new_path)
 	elif input_name in request.files:
-		upload = request.files[input_name]
+		upload = WerkzeugFileWrapper(request.files[input_name])
 	else:
 		return make_response("No file included", 400)
 
@@ -456,3 +454,31 @@ def _getCurrentFile():
 	else:
 		return None, None
 
+
+class WerkzeugFileWrapper(octoprint.filemanager.util.AbstractFileWrapper):
+	"""
+	A wrapper around a Werkzeug ``FileStorage`` object.
+
+	Arguments:
+	    file_obj (werkzeug.datastructures.FileStorage): The Werkzeug ``FileStorage`` instance to wrap.
+
+	.. seealso::
+
+	   `werkzeug.datastructures.FileStorage <http://werkzeug.pocoo.org/docs/0.10/datastructures/#werkzeug.datastructures.FileStorage>`_
+	        The documentation of Werkzeug's ``FileStorage`` class.
+	"""
+	def __init__(self, file_obj):
+		octoprint.filemanager.util.AbstractFileWrapper.__init__(self, file_obj.filename)
+		self.file_obj = file_obj
+
+	def save(self, path):
+		"""
+		Delegates to ``werkzeug.datastructures.FileStorage.save``
+		"""
+		self.file_obj.save(path)
+
+	def stream(self):
+		"""
+		Returns ``werkzeug.datastructures.FileStorage.stream``
+		"""
+		return self.file_obj.stream

src/octoprint/server/util/watchdog.py

@@ -9,6 +9,7 @@ import logging
 import watchdog.events
 
 import octoprint.filemanager
+import octoprint.filemanager.util
 import octoprint.util
 
 
@@ -26,17 +27,8 @@ class GcodeWatchdogHandler(watchdog.events.PatternMatchingEventHandler):
 		self._printer = printer
 
 	def _upload(self, path):
-		class WatchdogFileWrapper(object):
+		import os
-
+		file_wrapper = octoprint.filemanager.util.DiskFileWrapper(os.path.basename(path), path)
-			def __init__(self, path):
-				import os
-				self._path = path
-				self.filename = os.path.basename(self._path)
-
-			def save(self, target):
-				octoprint.util.safe_rename(self._path, target)
-
-		file_wrapper = WatchdogFileWrapper(path)
 
 		# determine current job
 		currentFilename = None

src/octoprint/util/__init__.py

@@ -335,50 +335,6 @@ def find_collision_free_name(filename, extension, existing_filenames, max_power=
 	raise ValueError("Can't create a collision free filename")
 
 
-def safe_rename(old, new, throw_error=False):
-	"""
-	Safely renames a file.
-
-	On Windows this is achieved by first creating a backup file of the new file (if it
-	already exists), thus moving it, then renaming the old into the new file and finally removing the backup. If
-	anything goes wrong during those steps, the backup (if already there) will be renamed to its old name and thus
-	the operation hopefully result in a no-op.
-
-	On other operating systems :func:`shutil.move` will be used instead.
-
-	Arguments:
-	    old (string): The path to the old file to be renamed.
-	    new (string): The path to the new file to be created/replaced.
-	    throw_error (boolean): Whether to throw an error upon errors during the renaming procedure (True) or not
-	        (False, default).
-
-	Raises:
-	    OSError: One of the renaming steps on windows failed and ``throw_error`` was True
-	"""
-
-	if sys.platform == "win32":
-		fh, backup = tempfile.mkstemp()
-		os.close(fh)
-
-		try:
-			if os.path.exists(new):
-				silent_remove(backup)
-				os.rename(new, backup)
-			os.rename(old, new)
-			os.remove(backup)
-		except OSError as e:
-			# if anything went wrong, try to rename the backup file to its original name
-			logger.error("Could not perform safe rename, trying to revert")
-			if os.path.exists(backup):
-				silent_remove(new)
-				os.rename(backup, new)
-			if throw_error:
-				raise e
-	else:
-		# on anything else than windows it's ooooh so much easier...
-		shutil.move(old, new)
-
-
 def silent_remove(file):
 	"""
 	Silently removes a file. Does not raise an error if the file doesn't exist.

tests/filemanager/test_filemanager.py

@@ -6,10 +6,12 @@ __license__ = 'GNU Affero General Public License http://www.gnu.org/licenses/agp
 __copyright__ = "Copyright (C) 2014 The OctoPrint Project - Released under terms of the AGPLv3 License"
 
 
+import io
 import unittest
 import mock
 
 import octoprint.filemanager
+import octoprint.filemanager.util
 
 class FileManagerTest(unittest.TestCase):
 
@@ -108,11 +110,12 @@ class FileManagerTest(unittest.TestCase):
 		self.local_storage.get_metadata.assert_called_once_with("test.file")
 
 	@mock.patch("__builtin__.open", new_callable=mock.mock_open)
+	@mock.patch("io.FileIO")
 	@mock.patch("shutil.copyfileobj")
 	@mock.patch("os.remove")
 	@mock.patch("tempfile.NamedTemporaryFile")
 	@mock.patch("time.time", side_effect=[1411979916.422, 1411979932.116])
-	def test_slice(self, mocked_time, mocked_tempfile, mocked_os, mocked_shutil, mocked_open):
+	def test_slice(self, mocked_time, mocked_tempfile, mocked_os, mocked_shutil, mocked_fileio, mocked_open):
 		callback = mock.MagicMock()
 		callback_args = ("one", "two", "three")
 
@@ -184,12 +187,17 @@ class FileManagerTest(unittest.TestCase):
 		self.local_storage.add_file.assert_called_once_with("dest.file", mock.ANY, printer_profile=expected_printer_profile, allow_overwrite=True, links=expected_links)
 
 		# assert that the generated gcode was manipulated as required
-		expected_open_calls = [mock.call("prefix/dest.file", "w"), mock.call("tmp.file", "r")]
+		expected_open_calls = [mock.call("prefix/dest.file", "wb")]
 		self.assertEquals(mocked_open.call_args_list, expected_open_calls)
-		mocked_open.return_value.write.assert_called_once_with(";Generated from source.file aabbccddeeff\r")
+		#mocked_open.return_value.write.assert_called_once_with(";Generated from source.file aabbccddeeff\r")
 
-		# assert that the contents of tmp.file where copied to dest.file
+		# assert that shutil was asked to copy the concatenated multistream
-		mocked_shutil.assert_called_once_with(mock.ANY, mock.ANY)
+		self.assertEquals(1, len(mocked_shutil.call_args_list))
+		shutil_call_args = mocked_shutil.call_args_list[0]
+		self.assertTrue(isinstance(shutil_call_args[0][0], octoprint.filemanager.util.MultiStream))
+		multi_stream = shutil_call_args[0][0]
+		self.assertEquals(2, len(multi_stream.streams))
+		self.assertTrue(isinstance(multi_stream.streams[0], io.BytesIO))
 
 		# assert that the temporary file was deleted
 		mocked_os.assert_called_once_with("tmp.file")