From a996f7b6fb5d7439c5b280949684b4212aeb923b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gina=20H=C3=A4u=C3=9Fge?= Date: Fri, 17 Apr 2015 14:45:58 +0200 Subject: [PATCH] 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 | 7 + docs/plugins/hooks.rst | 42 ++++- src/octoprint/filemanager/__init__.py | 32 ++-- src/octoprint/filemanager/storage.py | 5 +- src/octoprint/filemanager/util.py | 239 ++++++++++++++++++++++++++ src/octoprint/plugin/types.py | 5 + src/octoprint/server/__init__.py | 39 +++-- src/octoprint/server/api/files.py | 36 +++- src/octoprint/server/util/watchdog.py | 14 +- src/octoprint/util/__init__.py | 44 ----- tests/filemanager/test_filemanager.py | 18 +- 11 files changed, 376 insertions(+), 105 deletions(-) create mode 100644 src/octoprint/filemanager/util.py diff --git a/docs/modules/filemanager.rst b/docs/modules/filemanager.rst index aaa7beeb..691db309 100644 --- a/docs/modules/filemanager.rst +++ b/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: \ No newline at end of file diff --git a/docs/plugins/hooks.rst b/docs/plugins/hooks.rst index 735ec793..7298d4a3 100644 --- a/docs/plugins/hooks.rst +++ b/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 \ No newline at end of file + :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 `_ + + :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 \ No newline at end of file diff --git a/src/octoprint/filemanager/__init__.py b/src/octoprint/filemanager/__init__.py index 04961254..bc8903b7 100644 --- a/src/octoprint/filemanager/__init__.py +++ b/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): - 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) - + import io 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) diff --git a/src/octoprint/filemanager/storage.py b/src/octoprint/filemanager/storage.py index 5dc8695f..bd81df0e 100644 --- a/src/octoprint/filemanager/storage.py +++ b/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: diff --git a/src/octoprint/filemanager/util.py b/src/octoprint/filemanager/util.py new file mode 100644 index 00000000..105987f3 --- /dev/null +++ b/src/octoprint/filemanager/util.py @@ -0,0 +1,239 @@ +# coding=utf-8 +from __future__ import absolute_import + +__author__ = "Gina Häußge " +__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 diff --git a/src/octoprint/plugin/types.py b/src/octoprint/plugin/types.py index 0f88e45e..d49571e7 100644 --- a/src/octoprint/plugin/types.py +++ b/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 diff --git a/src/octoprint/server/__init__.py b/src/octoprint/server/__init__.py index 4cffbef0..a3409eea 100644 --- a/src/octoprint/server/__init__.py +++ b/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) - lifecycleManager.add_callback("disabled", template_disabled) + pluginLifecycleManager.add_callback("enabled", template_enabled) + 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): - self._server = server + def __init__(self, plugin_manager): 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_unloaded = on_plugin_event_factory("unloaded", "Unloaded plugin {name}: {plugin}") - self._plugin_manager.on_plugin_activated = on_plugin_event_factory("activated", "Activated plugin {name}: {plugin}") - self._plugin_manager.on_plugin_deactivated = on_plugin_event_factory("deactivated", "Deactivated plugin {name}: {plugin}") - self._plugin_manager.on_plugin_enabled = on_plugin_event_factory("enabled", "Enabled plugin {name}: {plugin}") - self._plugin_manager.on_plugin_disabled = on_plugin_event_factory("disabled", "Disabled 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") + self._plugin_manager.on_plugin_activated = on_plugin_event_factory("activated") + self._plugin_manager.on_plugin_deactivated = on_plugin_event_factory("deactivated") + self._plugin_manager.on_plugin_enabled = on_plugin_event_factory("enabled") + 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]: diff --git a/src/octoprint/server/api/files.py b/src/octoprint/server/api/files.py index e20dba4e..209ece50 100644 --- a/src/octoprint/server/api/files.py +++ b/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 = util.Object() - upload.filename = request.values[input_upload_name] - upload.save = lambda new_path: shutil.move(request.values[input_upload_path], new_path) + upload = octoprint.filemanager.util.DiskFileWrapper(request.values[input_upload_name], request.values[input_upload_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 `_ + 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 diff --git a/src/octoprint/server/util/watchdog.py b/src/octoprint/server/util/watchdog.py index 0ea409b4..ae9e33c3 100644 --- a/src/octoprint/server/util/watchdog.py +++ b/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): - - 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) + import os + file_wrapper = octoprint.filemanager.util.DiskFileWrapper(os.path.basename(path), path) # determine current job currentFilename = None diff --git a/src/octoprint/util/__init__.py b/src/octoprint/util/__init__.py index cb47af81..b2d2a6e7 100644 --- a/src/octoprint/util/__init__.py +++ b/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. diff --git a/tests/filemanager/test_filemanager.py b/tests/filemanager/test_filemanager.py index d517f6c0..110df373 100644 --- a/tests/filemanager/test_filemanager.py +++ b/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 - mocked_shutil.assert_called_once_with(mock.ANY, mock.ANY) + # assert that shutil was asked to copy the concatenated multistream + 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")