From ea6a4f3212d5012f5e825272a749fdf183053aba Mon Sep 17 00:00:00 2001
From: Yves Noirjean <yves.noirjean@id.ethz.ch>
Date: Mon, 2 Jul 2018 10:58:53 +0200
Subject: [PATCH] SSDM-6762: added error log; added -d option to removeref

---
 obis/.gitignore                               |  1 +
 .../integration_tests/integration_tests.py    |  6 +-
 obis/src/python/obis/dm/command_log.py        | 52 ++++++++++++++++++
 obis/src/python/obis/dm/commands/clone.py     |  2 +-
 obis/src/python/obis/dm/commands/move.py      |  3 +
 .../obis/dm/commands/openbis_command.py       |  8 ++-
 obis/src/python/obis/dm/commands/removeref.py | 55 +++++++++++++------
 obis/src/python/obis/dm/data_mgmt.py          | 37 ++++++++++---
 obis/src/python/obis/dm/repository_utils.py   |  2 +-
 obis/src/python/obis/scripts/cli.py           | 49 +++++++++++------
 10 files changed, 167 insertions(+), 48 deletions(-)
 create mode 100644 obis/src/python/obis/dm/command_log.py

diff --git a/obis/.gitignore b/obis/.gitignore
index ca405d3e0d9..fcb2fb420bb 100644
--- a/obis/.gitignore
+++ b/obis/.gitignore
@@ -2,4 +2,5 @@
 **.egg-info
 **__pycache__
 **.cache
+**.pytest_cache
 src/vagrant/initialize/openBIS-installation-standard-technologies-*
diff --git a/obis/src/python/integration_tests/integration_tests.py b/obis/src/python/integration_tests/integration_tests.py
index 8813ad2faaa..4651f7e5277 100644
--- a/obis/src/python/integration_tests/integration_tests.py
+++ b/obis/src/python/integration_tests/integration_tests.py
@@ -2,7 +2,7 @@
 # -*- coding: utf-8 -*-
 
 # can be run on vagrant like this:
-# vagrant ssh obisserver -c 'cd /vagrant_python/OBis/integration_tests && pytest ./integration_tests.py'
+# vagrant ssh obisserver -c 'cd /vagrant_python/integration_tests && pytest ./integration_tests.py'
 
 import json
 import os
@@ -55,7 +55,7 @@ def test_obis(tmpdir):
         with cd('data1'):
             cmd('touch file')
             result = cmd('obis status')
-            assert '?? file' in result
+            assert '? file' in result
             cmd('obis object set id=/DEFAULT/DEFAULT')
             result = cmd('obis commit -m \'commit-message\'')
             settings = get_settings()
@@ -126,7 +126,7 @@ def test_obis(tmpdir):
             result = cmd('obis commit -m \'commit-message\'')
             assert 'Missing configuration settings for [\'object id or collection id\'].' in result
             result = cmd('obis status')
-            assert '?? file' in result
+            assert '? file' in result
             cmd('obis object set id=/DEFAULT/DEFAULT')
             result = cmd('obis commit -m \'commit-message\'')
             settings = get_settings()
diff --git a/obis/src/python/obis/dm/command_log.py b/obis/src/python/obis/dm/command_log.py
new file mode 100644
index 00000000000..a8ad203d00b
--- /dev/null
+++ b/obis/src/python/obis/dm/command_log.py
@@ -0,0 +1,52 @@
+from datetime import datetime
+import os
+
+
+class CommandLog(object):
+
+    def __init__(self):
+        self.folder_path = os.path.join(os.path.expanduser('~'), ".obis", "log")
+        self.file_paths = []
+        self.logs = []
+        self.most_recent_command = None
+
+
+    def any_log_exists(self):
+        if os.path.exists(self.folder_path) == False:
+            return False
+        return len(os.listdir(self.folder_path)) != 0
+
+
+    def log(self, command, message):
+        # log first message only when second one is done
+        # error on first step does not need recovery
+        self.logs.append((command, message))
+        if len(self.logs) == 1:
+            return
+        elif len(self.logs) == 2:
+            first_command, first_message = self.logs[0]
+            self._log(first_command, first_message)
+        self._log(command, message)
+
+
+    def log_error(self, error):
+        if self.most_recent_command is not None:
+            self._log(self.most_recent_command, error)
+
+
+    def _log(self, command, message):
+        self.most_recent_command = command
+        if os.path.exists(self.folder_path) == False:
+            os.makedirs(self.folder_path)
+        file_path = os.path.join(self.folder_path, command + ".log")
+        self.file_paths.append(file_path)
+        timestamp = datetime.now().strftime("%H:%M:%S")
+        with open(file_path, "a") as file:
+            file.write(timestamp + ": " + message + "\n")
+
+
+    def success(self):
+        for file_path in self.file_paths:
+            if os.path.exists(file_path):
+                os.remove(file_path)
+        os.rmdir(self.folder_path)
diff --git a/obis/src/python/obis/dm/commands/clone.py b/obis/src/python/obis/dm/commands/clone.py
index 286956ed4d7..e22aec7e2fe 100644
--- a/obis/src/python/obis/dm/commands/clone.py
+++ b/obis/src/python/obis/dm/commands/clone.py
@@ -20,8 +20,8 @@ class Clone(OpenbisCommand):
         self.data_set_id = data_set_id
         self.ssh_user = ssh_user
         self.content_copy_index = content_copy_index
-        self.load_global_config(dm)
         self.skip_integrity_check = skip_integrity_check
+        self.load_global_config(dm)
         super(Clone, self).__init__(dm)
 
 
diff --git a/obis/src/python/obis/dm/commands/move.py b/obis/src/python/obis/dm/commands/move.py
index 7d3cdeb0bcf..060f3ef830d 100644
--- a/obis/src/python/obis/dm/commands/move.py
+++ b/obis/src/python/obis/dm/commands/move.py
@@ -26,11 +26,13 @@ class Move(OpenbisCommand):
         super(Move, self).__init__(dm)
 
     def run(self):
+        self.log("cloning repository...")
         clone = Clone(self.data_mgmt, self.data_set_id, self.ssh_user, self.content_copy_index, self.skip_integrity_check)
         result = clone.run()
         if result.failure():
             return result
 
+        self.log("removing old content copy from openBIS...")
         self.openbis.delete_content_copy(self.data_set_id, clone.content_copy)
 
         host = clone.content_copy['externalDms']['address'].split(':')[0]
@@ -42,6 +44,7 @@ class Move(OpenbisCommand):
             return CommandResult(returncode=0, output="Since the integrit check was skipped, please make sure the data was " +
                                                         "copied correctly and delete the old copy manually {}.".format(old_repository_location))
 
+        self.log("deleting old repository at {}:{}...".format(host, path))
         delete_repository(self.ssh_user, host, path)
 
         return CommandResult(returncode=0, output="")
diff --git a/obis/src/python/obis/dm/commands/openbis_command.py b/obis/src/python/obis/dm/commands/openbis_command.py
index 5b6f94a10de..734282e8ae6 100644
--- a/obis/src/python/obis/dm/commands/openbis_command.py
+++ b/obis/src/python/obis/dm/commands/openbis_command.py
@@ -99,6 +99,12 @@ class OpenbisCommand(object):
     def set_openbis_url(self, value):
         self.config_dict['config']['openbis_url'] = value
 
+
+    def log(self, message):
+        command = type(self).__name__
+        self.data_mgmt.log.log(command, message)
+
+
     def prepare_run(self):
         result = self.check_configuration()
         if result.failure():
@@ -206,7 +212,7 @@ class OpenbisCommand(object):
 class ContentCopySelector(object):
 
 
-    def __init__(self, data_set, content_copy_index, get_index=False):
+    def __init__(self, data_set, content_copy_index=None, get_index=False):
         self.data_set = data_set
         self.content_copy_index = content_copy_index
         self.get_index = get_index
diff --git a/obis/src/python/obis/dm/commands/removeref.py b/obis/src/python/obis/dm/commands/removeref.py
index dd5edadfab1..ffc7481de63 100644
--- a/obis/src/python/obis/dm/commands/removeref.py
+++ b/obis/src/python/obis/dm/commands/removeref.py
@@ -1,7 +1,7 @@
 import json
 import os
-from .openbis_command import OpenbisCommand
-from ..command_result import CommandResult
+from .openbis_command import OpenbisCommand, ContentCopySelector
+from ..command_result import CommandResult, CommandException
 from ..utils import complete_openbis_config
 
 
@@ -11,34 +11,57 @@ class Removeref(OpenbisCommand):
     a new content copy to openBIS.
     """
 
-    def __init__(self, dm):
+    def __init__(self, dm, data_set_id=None):
+        self._data_set_id = data_set_id
+        self.load_global_config(dm)
         super(Removeref, self).__init__(dm)
 
 
     def run(self):
+
+        if self._data_set_id is None:
+            self._remove_content_copies_from_repository()
+        else:
+            self._remove_content_copies_from_data_set()
+
+
+        return CommandResult(returncode=0, output="")
+
+
+    def _remove_content_copies_from_repository(self):
         result = self.check_obis_repository()
         if result.failure():
-            return result
-
-        data_set = self.openbis.get_dataset(self.data_set_id()).data
+            raise CommandException(result)
 
-        if data_set['linkedData'] is None:
-            return CommandResult(returncode=-1, output="Data set has no linked data: " + self.data_set_id())
-        if data_set['linkedData']['contentCopies'] is None:
-            return CommandResult(returncode=-1, output="Data set has no content copies: " + self.data_set_id())
+        data_set = self.openbis.get_dataset(self.data_set_id())
+        self._validate_data_set(data_set)
 
-        content_copies = data_set['linkedData']['contentCopies']
-        matching_content_copies = list(filter(lambda cc: 
+        content_copies = data_set.data['linkedData']['contentCopies']
+        content_copies_to_delete = list(filter(lambda cc: 
                 cc['externalDms']['code'] == self.external_dms_id() and cc['path'] == self.path()
             , content_copies))
 
-        if len(matching_content_copies) == 0:
-            return CommandResult(returncode=-1, output="Matching content copy not fount in data set: " + self.data_set_id())
+        if len(content_copies_to_delete) == 0:
+            raise CommandException(CommandResult(returncode=-1, output="Matching content copy not fount in data set: " + self.data_set_id()))
 
-        for content_copy in matching_content_copies:
+        for content_copy in content_copies_to_delete:
             self.openbis.delete_content_copy(self.data_set_id(), content_copy)
 
-        return CommandResult(returncode=0, output="")
+
+    def _remove_content_copies_from_data_set(self):
+        data_set = self.openbis.get_dataset(self._data_set_id)
+        self._validate_data_set(data_set)
+        selector = ContentCopySelector(data_set)
+        content_copy_to_delete = selector.select()
+        self.openbis.delete_content_copy(self._data_set_id, content_copy_to_delete)
+
+
+    def _validate_data_set(self, data_set):
+        if data_set.data['linkedData'] is None:
+            raise CommandException(CommandResult(returncode=-1, output="Data set has no linked data: " + self.data_set_id()))
+
+        if data_set.data['linkedData']['contentCopies'] is None:
+            raise CommandException(CommandResult(returncode=-1, output="Data set has no content copies: " + self.data_set_id()))
 
 
     def check_obis_repository(self):
diff --git a/obis/src/python/obis/dm/data_mgmt.py b/obis/src/python/obis/dm/data_mgmt.py
index 7b7654514e8..c5ec2315d74 100644
--- a/obis/src/python/obis/dm/data_mgmt.py
+++ b/obis/src/python/obis/dm/data_mgmt.py
@@ -24,6 +24,7 @@ from .commands.clone import Clone
 from .commands.move import Move
 from .commands.openbis_sync import OpenbisSync
 from .commands.download import Download
+from .command_log import CommandLog
 from .command_result import CommandResult
 from .command_result import CommandException
 from .git import GitWrapper
@@ -35,7 +36,7 @@ from ..scripts import cli
 
 
 # noinspection PyPep8Naming
-def DataMgmt(echo_func=None, settings_resolver=None, openbis_config={}, git_config={}, openbis=None, debug=False):
+def DataMgmt(echo_func=None, settings_resolver=None, openbis_config={}, git_config={}, openbis=None, log=None, debug=False):
     """Factory method for DataMgmt instances"""
 
     echo_func = echo_func if echo_func is not None else default_echo
@@ -43,7 +44,7 @@ def DataMgmt(echo_func=None, settings_resolver=None, openbis_config={}, git_conf
     complete_git_config(git_config)
     git_wrapper = GitWrapper(**git_config)
     if not git_wrapper.can_run():
-        return NoGitDataMgmt(settings_resolver, None, git_wrapper, openbis)
+        return NoGitDataMgmt(settings_resolver, None, git_wrapper, openbis, log)
 
     if settings_resolver is None:
         settings_resolver = dm_config.SettingsResolver()
@@ -52,7 +53,7 @@ def DataMgmt(echo_func=None, settings_resolver=None, openbis_config={}, git_conf
             settings_resolver.set_resolver_location_roots('data_set', result.output)
     complete_openbis_config(openbis_config, settings_resolver)
 
-    return GitDataMgmt(settings_resolver, openbis_config, git_wrapper, openbis, debug)
+    return GitDataMgmt(settings_resolver, openbis_config, git_wrapper, openbis, log, debug)
 
 
 class AbstractDataMgmt(metaclass=abc.ABCMeta):
@@ -61,17 +62,18 @@ class AbstractDataMgmt(metaclass=abc.ABCMeta):
     All operations throw an exepction if they fail.
     """
 
-    def __init__(self, settings_resolver, openbis_config, git_wrapper, openbis, debug=False):
+    def __init__(self, settings_resolver, openbis_config, git_wrapper, openbis, log, debug=False):
         self.settings_resolver = settings_resolver
         self.openbis_config = openbis_config
         self.git_wrapper = git_wrapper
         self.openbis = openbis
+        self.log = log
         self.debug = debug
 
     def error_raise(self, command, reason):
         """Raise an exception."""
         message = "'{}' failed. {}".format(command, reason)
-        raise ValueError(reason)
+        raise ValueError(message)
 
     @abc.abstractmethod
     def init_data(self, path, desc=None, create=True):
@@ -150,8 +152,9 @@ class AbstractDataMgmt(metaclass=abc.ABCMeta):
         return
 
     @abc.abstractmethod
-    def removeref(self):
+    def removeref(self, data_set_id=None):
         """Remove the current folder / repository from openBIS.
+        :param data_set_id: Id of the data from which a reference should be removed.
         :return: A CommandResult.
         """
         return
@@ -194,7 +197,7 @@ class NoGitDataMgmt(AbstractDataMgmt):
     def addref(self):
         self.error_raise("addref", "No git command found.")
 
-    def removeref(self):
+    def removeref(self, data_set_id=None):
         self.error_raise("removeref", "No git command found.")
 
     def download(self, data_set_id, content_copy_index, file, skip_integrity_check):
@@ -206,6 +209,21 @@ def restore_signal_handler(data_mgmt):
     sys.exit(0)
 
 
+def with_log(f):
+    def f_with_log(self, *args):
+        try:
+            result = f(self, *args)
+        except Exception as e:
+            self.log.log_error(str(e))
+            raise e
+        if result.failure() ==  False:
+            self.log.success()
+        else:
+            self.log.log_error(result.output)
+        return result
+    return f_with_log
+
+
 def with_restore(f):
     def f_with_restore(self, *args):
         self.set_restorepoint()
@@ -378,6 +396,7 @@ class GitDataMgmt(AbstractDataMgmt):
         cmd = Clone(self, data_set_id, ssh_user, content_copy_index, skip_integrity_check)
         return cmd.run()
 
+    @with_log
     def move(self, data_set_id, ssh_user, content_copy_index, skip_integrity_check):
         cmd = Move(self, data_set_id, ssh_user, content_copy_index, skip_integrity_check)
         return cmd.run()
@@ -386,8 +405,8 @@ class GitDataMgmt(AbstractDataMgmt):
         cmd = Addref(self)
         return cmd.run()
 
-    def removeref(self):
-        cmd = Removeref(self)
+    def removeref(self, data_set_id=None):
+        cmd = Removeref(self, data_set_id=data_set_id)
         return cmd.run()
 
     def download(self, data_set_id, content_copy_index, file, skip_integrity_check):
diff --git a/obis/src/python/obis/dm/repository_utils.py b/obis/src/python/obis/dm/repository_utils.py
index 41c4d36afa5..7b4ee025797 100644
--- a/obis/src/python/obis/dm/repository_utils.py
+++ b/obis/src/python/obis/dm/repository_utils.py
@@ -5,7 +5,7 @@ from .utils import run_shell
 
 
 def copy_repository(ssh_user, host, path):
-        # abort if local folder already exists
+    # abort if local folder already exists
     repository_folder = path.split('/')[-1]
     if os.path.exists(repository_folder):
         return CommandResult(returncode=-1, output="Folder for repository to clone already exists: " + repository_folder)
diff --git a/obis/src/python/obis/scripts/cli.py b/obis/src/python/obis/scripts/cli.py
index c0d95abc0da..9966e476451 100644
--- a/obis/src/python/obis/scripts/cli.py
+++ b/obis/src/python/obis/scripts/cli.py
@@ -11,6 +11,7 @@ Created by Chandrasekhar Ramakrishnan on 2017-01-27.
 Copyright (c) 2017 Chandrasekhar Ramakrishnan. All rights reserved.
 """
 import json
+import sys
 from datetime import datetime
 
 import click
@@ -19,6 +20,7 @@ from .. import dm
 from ..dm.command_result import CommandResult
 from ..dm.command_result import CommandException
 from ..dm.utils import cd
+from ..dm.command_log import CommandLog
 
 
 def click_echo(message):
@@ -44,12 +46,16 @@ def add_params(params):
     return _add_params
 
 
-def shared_data_mgmt(context={}):
+def shared_data_mgmt(context={}, halt_on_error_log=True):
     git_config = {'find_git': True}
     openbis_config = {}
     if context.get('verify_certificates') is not None:
         openbis_config['verify_certificates'] = context['verify_certificates']
-    return dm.DataMgmt(openbis_config=openbis_config, git_config=git_config, debug=context['debug'])
+    log = CommandLog()
+    if halt_on_error_log and log.any_log_exists():
+        click_echo("Error: A previous command did not finish. Please check the log ({}) and remove it when you want to continue using obis".format(log.folder_path))
+        sys.exit(-1)
+    return dm.DataMgmt(openbis_config=openbis_config, git_config=git_config, log=log, debug=context['debug'])
 
 
 def check_result(command, result):
@@ -311,7 +317,7 @@ def settings(ctx, is_global):
 @settings.command('get')
 @click.pass_context
 def settings_get(ctx):
-    data_mgmt = shared_data_mgmt(ctx.obj)
+    data_mgmt = shared_data_mgmt(ctx.obj, halt_on_error_log=False)
     settings = data_mgmt.settings_resolver.config_dict()
     settings_str = json.dumps(settings, indent=4, sort_keys=True)
     click.echo("{}".format(settings_str))
@@ -326,7 +332,7 @@ def repository(ctx, is_global):
     """ Get/set settings related to the repository.
     """
     ctx.obj['is_global'] = is_global
-    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj)
+    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj, halt_on_error_log=False)
     ctx.obj['resolver'] = ctx.obj['data_mgmt'].settings_resolver.repository
 
 
@@ -363,7 +369,7 @@ def data_set(ctx, is_global, is_data_set_property):
     """
     ctx.obj['is_global'] = is_global
     ctx.obj['is_data_set_property'] = is_data_set_property
-    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj)
+    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj, halt_on_error_log=False)
     ctx.obj['resolver'] = ctx.obj['data_mgmt'].settings_resolver.data_set
 
 
@@ -398,7 +404,7 @@ def object(ctx, is_global):
     """ Get/set settings related to the object.
     """
     ctx.obj['is_global'] = is_global
-    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj)
+    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj, halt_on_error_log=False)
     ctx.obj['resolver'] = ctx.obj['data_mgmt'].settings_resolver.object
 
 
@@ -433,7 +439,7 @@ def collection(ctx, is_global):
     """ Get/set settings related to the collection.
     """
     ctx.obj['is_global'] = is_global
-    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj)
+    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj, halt_on_error_log=False)
     ctx.obj['resolver'] = ctx.obj['data_mgmt'].settings_resolver.collection
 
 
@@ -468,7 +474,7 @@ def config(ctx, is_global):
     """ Get/set configurations.
     """
     ctx.obj['is_global'] = is_global
-    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj)
+    ctx.obj['data_mgmt'] = shared_data_mgmt(ctx.obj, halt_on_error_log=False)
     ctx.obj['resolver'] = ctx.obj['data_mgmt'].settings_resolver.config
 
 
@@ -643,27 +649,36 @@ def addref(ctx, repository):
 # removeref
 
 _removeref_params = [
+    click.option('-d', '--data_set_id', help='Remove ref by data set id, in case the repository is not available anymore.'),
     click.argument('repository', type=click.Path(exists=True, file_okay=False), required=False),
 ]
 
-def _repository_removeref(ctx):
+def _repository_removeref(ctx, data_set_id=None):
     data_mgmt = shared_data_mgmt(ctx.obj)
-    return check_result("addref", run(ctx, data_mgmt.removeref))
+    return check_result("removeref", run(ctx, lambda: data_mgmt.removeref(data_set_id=data_set_id)))
 
 @repository.command("removeref", short_help="Remove the reference to the given repository from openBIS.")
 @click.pass_context
 @add_params(_removeref_params)
-def repository_removeref(ctx, repository):
-    if repository is None:
-        return _repository_removeref(ctx)
-    with cd(repository):
-        return _repository_removeref(ctx)
+def repository_removeref(ctx, data_set_id, repository):
+    if data_set_id is None:
+        if repository is None:
+            return _repository_removeref(ctx)
+        with cd(repository):
+            return _repository_removeref(ctx)
+    else:
+        if repository is not None:
+            print(repository)
+            click_echo("Only provide the data_set id OR the repository.")
+            return -1
+        return _repository_removeref(ctx, data_set_id=data_set_id)
+
 
 @cli.command(short_help="Remove the reference to the given repository from openBIS.")
 @click.pass_context
 @add_params(_removeref_params)
-def removeref(ctx, repository):
-    ctx.invoke(repository_removeref, repository=repository)
+def removeref(ctx, data_set_id, repository):
+    ctx.invoke(repository_removeref, data_set_id=data_set_id, repository=repository)
 
 
 # data set commands: download / clone
-- 
GitLab