From 8b1d3b8e0721e492a36b38a530a81cf97a5cceb9 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Tue, 22 Sep 2015 16:49:08 +0300
Subject: [PATCH 01/12] Add methods to create graph of input thats is related
 to a single resource

---
 solar/solar/interfaces/orm.py | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/solar/solar/interfaces/orm.py b/solar/solar/interfaces/orm.py
index afb06500..c6e6ab05 100644
--- a/solar/solar/interfaces/orm.py
+++ b/solar/solar/interfaces/orm.py
@@ -181,6 +181,18 @@ class DBRelatedField(object):
 
         return ret
 
+    def as_list(self):
+        relations = self.all()
+
+        ret = []
+
+        for rel in relations:
+            ret.append(
+                self.destination_db_class(**rel.end_node.properties)
+            )
+
+        return ret
+
     def sources(self, destination_db_object):
         """
         Reverse of self.as_set, i.e. for given destination_db_object,
@@ -424,6 +436,18 @@ class DBResourceInput(DBObject):
         )
         super(DBResourceInput, self).delete()
 
+    def edges(self):
+        out = db.get_relations(
+                source=self._db_node,
+                type_=base.BaseGraphDB.RELATION_TYPES.input_to_input)
+        incoming = db.get_relations(
+                dest=self._db_node,
+                type_=base.BaseGraphDB.RELATION_TYPES.input_to_input)
+        for r in out + incoming:
+            source = DBResourceInput(**r.start_node.properties)
+            dest = DBResourceInput(**r.end_node.properties)
+            yield source, dest
+
     def check_other_val(self, other_val=None):
         if not other_val:
             return self
@@ -434,7 +458,6 @@ class DBResourceInput(DBObject):
         correct_input = inps[other_val]
         return correct_input.backtrack_value()
 
-
     def backtrack_value_emitter(self, level=None, other_val=None):
         # TODO: this is actually just fetching head element in linked list
         #       so this whole algorithm can be moved to the db backend probably
@@ -597,6 +620,11 @@ class DBResource(DBObject):
             input.delete()
         super(DBResource, self).delete()
 
+    def graph(self):
+        mdg = networkx.MultiDiGraph()
+        for input in self.inputs.as_list():
+            mdg.add_edges_from(input.edges())
+        return mdg
 
 # TODO: remove this
 if __name__ == '__main__':

From fae7db361fd7ea9fe736a5506914225c23744045 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Wed, 23 Sep 2015 11:45:33 +0300
Subject: [PATCH 02/12] Make signals between resources reversable

---
 solar/solar/core/resource/resource.py    | 17 +++++
 solar/solar/interfaces/db/base.py        |  2 +-
 solar/solar/interfaces/orm.py            | 19 ++++++
 solar/solar/system_log/change.py         | 83 +++++++++++++++---------
 solar/solar/system_log/data.py           | 42 ++----------
 solar/solar/system_log/operations.py     | 11 +++-
 solar/solar/test/test_diff_generation.py |  8 ---
 solar/solar/test/test_system_log_api.py  |  2 +-
 8 files changed, 102 insertions(+), 82 deletions(-)

diff --git a/solar/solar/core/resource/resource.py b/solar/solar/core/resource/resource.py
index 31ce3fba..d65fb4c9 100644
--- a/solar/solar/core/resource/resource.py
+++ b/solar/solar/core/resource/resource.py
@@ -149,6 +149,20 @@ class Resource(object):
     def delete(self):
         return self.db_obj.delete()
 
+    @property
+    def connections(self):
+        """
+        Gives you all incoming/outgoing connections for current resource,
+        stored as:
+        [(emitter, emitter_input, receiver, receiver_input), ...]
+        """
+        rst = []
+        for emitter, receiver in self.db_obj.graph().edges():
+            rst.append(
+                [emitter.resource.name, emitter.name,
+                 receiver.resource.name, receiver.name])
+        return rst
+
     def resource_inputs(self):
         return {
             i.name: i for i in self.db_obj.inputs.as_set()
@@ -179,6 +193,9 @@ class Resource(object):
             **self.to_dict()
         )
 
+    def load_commited(self):
+        return orm.DBCommitedState.get_or_create(self.name)
+
 
 def load(name):
     r = orm.DBResource.load(name)
diff --git a/solar/solar/interfaces/db/base.py b/solar/solar/interfaces/db/base.py
index d74fc262..3cdbfbac 100644
--- a/solar/solar/interfaces/db/base.py
+++ b/solar/solar/interfaces/db/base.py
@@ -132,7 +132,7 @@ class BaseGraphDB(object):
     DEFAULT_COLLECTION=COLLECTIONS.resource
     RELATION_TYPES = Enum(
         'RelationTypes',
-        'input_to_input resource_input plan_edge graph_to_node resource_event'
+        'input_to_input resource_input plan_edge graph_to_node resource_event commited'
     )
     DEFAULT_RELATION=RELATION_TYPES.resource_input
 
diff --git a/solar/solar/interfaces/orm.py b/solar/solar/interfaces/orm.py
index c6e6ab05..4c01cde4 100644
--- a/solar/solar/interfaces/orm.py
+++ b/solar/solar/interfaces/orm.py
@@ -566,6 +566,24 @@ class DBEvent(DBObject):
         super(DBEvent, self).delete()
 
 
+class DBCommitedState(DBObject):
+
+    __metaclass__ = DBObjectMeta
+
+    _collection = base.BaseGraphDB.COLLECTIONS.state_data
+
+    id = db_field(schema='str!', is_primary=True)
+    inputs = db_field(schema={}, default_value={})
+    connections = db_field(schema=[], default_value=[])
+
+    @classmethod
+    def get_or_create(cls, name):
+        r = db.get_or_create(
+            name,
+            properties={'id': name},
+            collection=cls._collection)
+        return cls(**r.properties)
+
 
 class DBResource(DBObject):
     __metaclass__ = DBObjectMeta
@@ -626,6 +644,7 @@ class DBResource(DBObject):
             mdg.add_edges_from(input.edges())
         return mdg
 
+
 # TODO: remove this
 if __name__ == '__main__':
     r = DBResource(name=1)
diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index b9d6cc06..f2ba14b9 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -23,6 +23,7 @@ from solar.interfaces.db import get_db
 from solar.system_log import data
 from solar.orchestration import graph
 from solar.events import api as evapi
+from solar.interfaces import orm
 
 db = get_db()
 
@@ -42,57 +43,77 @@ def create_diff(staged, commited):
     return list(dictdiffer.diff(commited, staged))
 
 
-def create_logitem(resource, action, diffed):
+def create_logitem(resource, action, diffed, connections_diffed):
     return data.LogItem(
                 utils.generate_uuid(),
                 resource,
                 '{}.{}'.format(resource, action),
-                diffed)
+                diffed,
+                connections_diffed)
 
 
-def _stage_changes(staged_resources, commited_resources, staged_log):
-
-    union = set(staged_resources.keys()) | set(commited_resources.keys())
-    for res_uid in union:
-        commited_data = commited_resources.get(res_uid, {})
-        staged_data = staged_resources.get(res_uid, {})
-
-        df = create_diff(staged_data, commited_data)
-
-        if df:
-            action = guess_action(commited_data, staged_data)
-            log_item = create_logitem(res_uid, action, df)
-            staged_log.append(log_item)
-    return staged_log
+def create_sorted_diff(staged, commited):
+    staged.sort()
+    commited.sort()
+    return create_diff(staged, commited)
 
 
 def stage_changes():
     log = data.SL()
     log.clean()
-    staged = {r.name: r.args for r in resource.load_all()}
-    commited = data.CD()
-    return _stage_changes(staged, commited, log)
+    resources_map = {r.name: r for r in resource.load_all()}
+    commited_map = {r.id for r in orm.DBCommitedState.load_all()}
+
+    for resource_id in set(resources_map.keys()) | set(commited_map.keys()):
+
+        if resource_id not in resource_map:
+            resource_args = {}
+            resource_connections = []
+        else:
+            resource_args = resource_map[resource_id].args
+            resource_connections = resource_map[resource_id].connections
+
+        if resource_id not in commited_map:
+            commited_args = {}
+            commited_connections = []
+        else:
+            commited_args = commited_map[resource_id].inputs
+            commited_connections = commited_map[resource_id].connections
+
+        inputs_diff = create_diff(resource_args, commited_args)
+        connections_diff = create_sorted_diff(
+            resource_connections, commited_connections)
+
+        # if new connection created it will be reflected in inputs
+        # but using inputs to reverse connections is not possible
+        if inputs_diff:
+            log_item = create_logitem(
+                resource_id,
+                guess_action(commited_connections, resource_connections),
+                inputs_diff,
+                connections_diff)
+            log.append(log_item)
+    return log
 
 
 def send_to_orchestration():
     dg = nx.MultiDiGraph()
-    staged = {r.name: r.args for r in resource.load_all()}
-    commited = data.CD()
+
     events = {}
     changed_nodes = []
 
-    for res_uid in staged.keys():
-        commited_data = commited.get(res_uid, {})
-        staged_data = staged.get(res_uid, {})
+    for resource_obj in resource.load_all():
+        commited_db_obj = resource_obj.load_commited()
+        resource_args = resource_obj.args
 
-        df = create_diff(staged_data, commited_data)
+        df = create_diff(resource_args, commited_db_obj.inputs)
 
         if df:
-            events[res_uid] = evapi.all_events(res_uid)
-            changed_nodes.append(res_uid)
-            action = guess_action(commited_data, staged_data)
+            events[resource_obj.name] = evapi.all_events(resource_obj.name)
+            changed_nodes.append(resource_obj.name)
+            action = guess_action(resource_args, commited_db_obj.inputs)
 
-            state_change = evapi.StateChange(res_uid, action)
+            state_change = evapi.StateChange(resource_obj.name, action)
             state_change.insert(changed_nodes, dg)
 
     evapi.build_edges(dg, events)
@@ -110,13 +131,13 @@ def parameters(res, action, data):
 
 
 def revert_uids(uids):
-    commited = data.CD()
     history = data.CL()
     for uid in uids:
         item = history.get(uid)
         res_db = resource.load(item.res)
+        commited = res_db.load_commited()
         args_to_update = dictdiffer.revert(
-            item.diff, commited.get(item.res, {}))
+            item.diff, commited.inputs)
         res_db.update(args_to_update)
 
 
diff --git a/solar/solar/system_log/data.py b/solar/solar/system_log/data.py
index 226a7a41..192d82ef 100644
--- a/solar/solar/system_log/data.py
+++ b/solar/solar/system_log/data.py
@@ -30,22 +30,20 @@ STATES = Enum('States', 'error inprogress pending success')
 def state_file(name):
     if 'log' in name:
         return Log(name)
-    elif 'data' in name:
-        return Data(name)
 
 
-CD = partial(state_file, 'commited_data')
 SL = partial(state_file, 'stage_log')
 CL = partial(state_file, 'commit_log')
 
 
 class LogItem(object):
 
-    def __init__(self, uid, res, log_action, diff, state=None):
+    def __init__(self, uid, res, log_action, diff, signals_diff, state=None):
         self.uid = uid
         self.res = res
         self.log_action = log_action
         self.diff = diff
+        self.signals_diff = signals_diff
         self.state = state or STATES.pending
 
     def to_yaml(self):
@@ -56,7 +54,8 @@ class LogItem(object):
                 'res': self.res,
                 'log_action': self.log_action,
                 'diff': self.diff,
-                'state': self.state.name}
+                'state': self.state.name,
+                'signals_diff': self.signals_diff}
 
     @classmethod
     def from_dict(cls, **kwargs):
@@ -146,36 +145,3 @@ class Log(object):
 
     def __iter__(self):
         return iter(self.collection())
-
-
-class Data(collections.MutableMapping):
-
-    def __init__(self, path):
-        self.path = path
-        r = db.get(path, collection=db.COLLECTIONS.state_data,
-                   return_empty=True, db_convert=False)
-
-        if r:
-            self.store = r.get('properties', {})
-        else:
-            self.store = {}
-
-    def __getitem__(self, key):
-        return self.store[key]
-
-    def __setitem__(self, key, value):
-        self.store[key] = value
-        db.create(self.path, self.store, collection=db.COLLECTIONS.state_data)
-
-    def __delitem__(self, key):
-        self.store.pop(key)
-        db.create(self.path, self.store, collection=db.COLLECTIONS.state_data)
-
-    def __iter__(self):
-        return iter(self.store)
-
-    def __len__(self):
-        return len(self.store)
-
-    def clean(self):
-        db.create(self.path, {}, collection=db.COLLECTIONS.state_data)
diff --git a/solar/solar/system_log/operations.py b/solar/solar/system_log/operations.py
index e347d237..a31629cc 100644
--- a/solar/solar/system_log/operations.py
+++ b/solar/solar/system_log/operations.py
@@ -14,6 +14,7 @@
 
 from solar.system_log import data
 from dictdiffer import patch
+from solar.interfaces import orm
 
 
 def set_error(log_action, *args, **kwargs):
@@ -29,9 +30,13 @@ def move_to_commited(log_action, *args, **kwargs):
     item = next((i for i in sl if i.log_action == log_action), None)
     sl.pop(item.uid)
     if item:
-        commited = data.CD()
-        staged_data = patch(item.diff, commited.get(item.res, {}))
+        commited = orm.DBCommitedState.get_or_create(item.res)
+        commited.inputs = patch(item.diff, commited.inputs)
+        sorted_connections = sorted(commited.connections)
+        commited.connections = patch(item.signals_diff, sorted_connections)
+        commited.save()
         cl = data.CL()
         item.state = data.STATES.success
         cl.append(item)
-        commited[item.res] = staged_data
+
+
diff --git a/solar/solar/test/test_diff_generation.py b/solar/solar/test/test_diff_generation.py
index 34d81f2b..1df1e67c 100644
--- a/solar/solar/test/test_diff_generation.py
+++ b/solar/solar/test/test_diff_generation.py
@@ -95,11 +95,3 @@ def resources():
               'connections': [['n.1', 'h.1', ['ip', 'ip']]],
               'tags': []}}
     return r
-
-
-def test_stage_changes(resources):
-    commited = {}
-    log = change._stage_changes(resources, commited, [])
-
-    assert len(log) == 3
-    assert {l.res for l in log} == {'n.1', 'r.1', 'h.1'}
diff --git a/solar/solar/test/test_system_log_api.py b/solar/solar/test/test_system_log_api.py
index be36e5a7..13225e43 100644
--- a/solar/solar/test/test_system_log_api.py
+++ b/solar/solar/test/test_system_log_api.py
@@ -35,7 +35,7 @@ def test_revert_update():
 
     log = data.SL()
     logitem =change.create_logitem(
-        res.name, action, change.create_diff(commit, previous))
+        res.name, action, change.create_diff(commit, previous), [])
     log.append(logitem)
     resource_obj.update(commit)
     operations.move_to_commited(logitem.log_action)

From 11e3f03e23cceb2cde4459c4570c932b2cff9189 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Wed, 23 Sep 2015 14:44:24 +0300
Subject: [PATCH 03/12] Add base_path to log_item to revert removal

---
 solar/solar/core/resource/resource.py | 1 +
 solar/solar/system_log/change.py      | 3 ++-
 solar/solar/system_log/data.py        | 7 +++++--
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/solar/solar/core/resource/resource.py b/solar/solar/core/resource/resource.py
index d65fb4c9..ada95cab 100644
--- a/solar/solar/core/resource/resource.py
+++ b/solar/solar/core/resource/resource.py
@@ -53,6 +53,7 @@ class Resource(object):
         else:
             metadata = deepcopy(self._metadata)
 
+        self.base_path = base_path
         self.tags = tags or []
         self.virtual_resource = virtual_resource
 
diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index f2ba14b9..21d994a4 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -91,7 +91,8 @@ def stage_changes():
                 resource_id,
                 guess_action(commited_connections, resource_connections),
                 inputs_diff,
-                connections_diff)
+                connections_diff,
+                base_path=resource_obj.base_path)
             log.append(log_item)
     return log
 
diff --git a/solar/solar/system_log/data.py b/solar/solar/system_log/data.py
index 192d82ef..a5499e1c 100644
--- a/solar/solar/system_log/data.py
+++ b/solar/solar/system_log/data.py
@@ -38,13 +38,15 @@ CL = partial(state_file, 'commit_log')
 
 class LogItem(object):
 
-    def __init__(self, uid, res, log_action, diff, signals_diff, state=None):
+    def __init__(self, uid, res, log_action, diff,
+                 signals_diff, state=None, base_path=None):
         self.uid = uid
         self.res = res
         self.log_action = log_action
         self.diff = diff
         self.signals_diff = signals_diff
         self.state = state or STATES.pending
+        self.base_path = base_path
 
     def to_yaml(self):
         return utils.yaml_dump(self.to_dict())
@@ -55,7 +57,8 @@ class LogItem(object):
                 'log_action': self.log_action,
                 'diff': self.diff,
                 'state': self.state.name,
-                'signals_diff': self.signals_diff}
+                'signals_diff': self.signals_diff,
+                'base_path': self.base_path}
 
     @classmethod
     def from_dict(cls, **kwargs):

From a1d494eaaba1fcb681c74615c97092672bf092c1 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Thu, 24 Sep 2015 12:32:38 +0300
Subject: [PATCH 04/12] Add support for revert of the removal

---
 solar/solar/system_log/change.py        | 36 +++++++++++++++++++------
 solar/solar/system_log/consts.py        | 20 ++++++++++++++
 solar/solar/system_log/data.py          |  9 ++++---
 solar/solar/test/test_system_log_api.py | 33 ++++++++++++++++++++++-
 4 files changed, 85 insertions(+), 13 deletions(-)
 create mode 100644 solar/solar/system_log/consts.py

diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index 21d994a4..a625d6ae 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -24,6 +24,7 @@ from solar.system_log import data
 from solar.orchestration import graph
 from solar.events import api as evapi
 from solar.interfaces import orm
+from .consts import CHANGES
 
 db = get_db()
 
@@ -43,13 +44,15 @@ def create_diff(staged, commited):
     return list(dictdiffer.diff(commited, staged))
 
 
-def create_logitem(resource, action, diffed, connections_diffed):
+def create_logitem(resource, action, diffed, connections_diffed,
+                   base_path=None):
     return data.LogItem(
                 utils.generate_uuid(),
                 resource,
-                '{}.{}'.format(resource, action),
+                action,
                 diffed,
-                connections_diffed)
+                connections_diffed,
+                base_path=base_path)
 
 
 def create_sorted_diff(staged, commited):
@@ -135,13 +138,30 @@ def revert_uids(uids):
     history = data.CL()
     for uid in uids:
         item = history.get(uid)
-        res_db = resource.load(item.res)
-        commited = res_db.load_commited()
-        args_to_update = dictdiffer.revert(
-            item.diff, commited.inputs)
-        res_db.update(args_to_update)
+        if item.action == CHANGES.update.name:
+            _revert_update(item)
+        elif item.action == CHANGES.remove.name:
+            _revert_remove(item)
 
 
+def _revert_remove(logitem):
+    """Resource should be created with all previous connections
+    """
+    commited = orm.DBCommitedState.load(logitem.res)
+    args = dictdiffer.revert(
+        logitem.diff, commited.inputs)
+    resource_obj = resource.Resource(
+        logitem.res, logitem.base_path, args=args)
+
+def _revert_update(logitem):
+    """Revert of update should use update inputs and connections
+    """
+    res_db = resource.load(logitem.res)
+    commited = res_db.load_commited()
+    args_to_update = dictdiffer.revert(
+        logitem.diff, commited.inputs)
+    res_db.update(args_to_update)
+
 def revert(uid):
     return revert_uids([uid])
 
diff --git a/solar/solar/system_log/consts.py b/solar/solar/system_log/consts.py
new file mode 100644
index 00000000..3e24e97e
--- /dev/null
+++ b/solar/solar/system_log/consts.py
@@ -0,0 +1,20 @@
+#    Copyright 2015 Mirantis, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from enum import Enum
+
+CHANGES = Enum(
+        'Changes',
+        'run remove update'
+    )
diff --git a/solar/solar/system_log/data.py b/solar/solar/system_log/data.py
index a5499e1c..1d74a826 100644
--- a/solar/solar/system_log/data.py
+++ b/solar/solar/system_log/data.py
@@ -38,11 +38,12 @@ CL = partial(state_file, 'commit_log')
 
 class LogItem(object):
 
-    def __init__(self, uid, res, log_action, diff,
+    def __init__(self, uid, res, action, diff,
                  signals_diff, state=None, base_path=None):
         self.uid = uid
         self.res = res
-        self.log_action = log_action
+        self.log_action = '{}:{}'.format(res, action)
+        self.action = action
         self.diff = diff
         self.signals_diff = signals_diff
         self.state = state or STATES.pending
@@ -54,11 +55,11 @@ class LogItem(object):
     def to_dict(self):
         return {'uid': self.uid,
                 'res': self.res,
-                'log_action': self.log_action,
                 'diff': self.diff,
                 'state': self.state.name,
                 'signals_diff': self.signals_diff,
-                'base_path': self.base_path}
+                'base_path': self.base_path,
+                'action': self.action}
 
     @classmethod
     def from_dict(cls, **kwargs):
diff --git a/solar/solar/test/test_system_log_api.py b/solar/solar/test/test_system_log_api.py
index 13225e43..dbe00bc6 100644
--- a/solar/solar/test/test_system_log_api.py
+++ b/solar/solar/test/test_system_log_api.py
@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
 
 from pytest import fixture
 from solar.system_log import change
@@ -35,12 +36,42 @@ def test_revert_update():
 
     log = data.SL()
     logitem =change.create_logitem(
-        res.name, action, change.create_diff(commit, previous), [])
+        res.name, action, change.create_diff(commit, previous), [],
+        base_path=res.base_path)
     log.append(logitem)
     resource_obj.update(commit)
     operations.move_to_commited(logitem.log_action)
 
+    assert logitem.diff == [('change', 'a', ('9', '10'))]
     assert resource_obj.args == commit
 
     change.revert(logitem.uid)
     assert resource_obj.args == previous
+
+
+def test_revert_removal():
+    res = orm.DBResource(id='test1', name='test1', base_path='x')
+    res.save()
+    res.add_input('a', 'str', '9')
+    res.delete()
+    commited = orm.DBCommitedState.get_or_create('test1')
+    commited.inputs = {'a': '9'}
+    commited.save()
+
+    logitem =change.create_logitem(
+        res.name, 'remove', change.create_diff({}, {'a': '9'}), [],
+        base_path=res.base_path)
+    log = data.SL()
+    log.append(logitem)
+    operations.move_to_commited(logitem.log_action)
+
+    resources = orm.DBResource.load_all()
+
+    assert resources == []
+    assert logitem.diff == [('remove', '', [('a', '9')])]
+
+    with mock.patch.object(resource, 'read_meta') as mread:
+        mread.return_value = {'input': {'a': {'schema': 'str!'}}}
+        change.revert(logitem.uid)
+    resource_obj = resource.load('test1')
+    assert resource_obj.args == {'a': '9'}

From f436ce84a488aa6a15f27c3ea6e2f5eba47885ae Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Thu, 24 Sep 2015 12:51:52 +0300
Subject: [PATCH 05/12] Add support for revert of create

---
 solar/solar/system_log/change.py        | 15 ++++++++++++---
 solar/solar/test/test_system_log_api.py | 22 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index a625d6ae..eaa920e4 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -142,6 +142,8 @@ def revert_uids(uids):
             _revert_update(item)
         elif item.action == CHANGES.remove.name:
             _revert_remove(item)
+        elif item.action == CHANGES.run.name:
+            _revert_run(item)
 
 
 def _revert_remove(logitem):
@@ -153,14 +155,21 @@ def _revert_remove(logitem):
     resource_obj = resource.Resource(
         logitem.res, logitem.base_path, args=args)
 
+
 def _revert_update(logitem):
     """Revert of update should use update inputs and connections
     """
-    res_db = resource.load(logitem.res)
-    commited = res_db.load_commited()
+    res_obj = resource.load(logitem.res)
+    commited = res_obj.load_commited()
     args_to_update = dictdiffer.revert(
         logitem.diff, commited.inputs)
-    res_db.update(args_to_update)
+    res_obj.update(args_to_update)
+
+
+def _revert_run(logitem):
+    res_obj = resource.load(logitem.res)
+    res_obj.delete()
+
 
 def revert(uid):
     return revert_uids([uid])
diff --git a/solar/solar/test/test_system_log_api.py b/solar/solar/test/test_system_log_api.py
index dbe00bc6..add11613 100644
--- a/solar/solar/test/test_system_log_api.py
+++ b/solar/solar/test/test_system_log_api.py
@@ -75,3 +75,25 @@ def test_revert_removal():
         change.revert(logitem.uid)
     resource_obj = resource.load('test1')
     assert resource_obj.args == {'a': '9'}
+
+
+def test_revert_create():
+    res = orm.DBResource(id='test1', name='test1', base_path='x')
+    res.save()
+    res.add_input('a', 'str', '9')
+
+    logitem =change.create_logitem(
+        res.name, 'run', change.create_diff({'a': '9'}, {}), [],
+        base_path=res.base_path)
+    log = data.SL()
+    log.append(logitem)
+    assert logitem.diff == [('add', '', [('a', '9')])]
+
+    operations.move_to_commited(logitem.log_action)
+    commited = orm.DBCommitedState.load('test1')
+    assert commited.inputs == {'a': '9'}
+
+    change.revert(logitem.uid)
+
+    resources = orm.DBResource.load_all()
+    assert resources == []

From 7c68a0f0a32a509239c6facc52992d98ae87d5c9 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Thu, 24 Sep 2015 15:37:11 +0300
Subject: [PATCH 06/12] Add test for connected resources with revert of
 create/update

---
 solar/solar/core/resource/resource.py   |  1 +
 solar/solar/interfaces/orm.py           |  1 +
 solar/solar/system_log/change.py        | 45 ++++++++++-----
 solar/solar/system_log/data.py          |  3 +
 solar/solar/system_log/operations.py    |  1 +
 solar/solar/test/test_system_log_api.py | 74 +++++++++++++++++++++++++
 6 files changed, 110 insertions(+), 15 deletions(-)

diff --git a/solar/solar/core/resource/resource.py b/solar/solar/core/resource/resource.py
index ada95cab..cc01cfb1 100644
--- a/solar/solar/core/resource/resource.py
+++ b/solar/solar/core/resource/resource.py
@@ -83,6 +83,7 @@ class Resource(object):
     def __init__(self, resource_db):
         self.db_obj = resource_db
         self.name = resource_db.name
+        self.base_path = resource_db.base_path
         # TODO: tags
         self.tags = []
         self.virtual_resource = None
diff --git a/solar/solar/interfaces/orm.py b/solar/solar/interfaces/orm.py
index 4c01cde4..5031f6dc 100644
--- a/solar/solar/interfaces/orm.py
+++ b/solar/solar/interfaces/orm.py
@@ -575,6 +575,7 @@ class DBCommitedState(DBObject):
     id = db_field(schema='str!', is_primary=True)
     inputs = db_field(schema={}, default_value={})
     connections = db_field(schema=[], default_value=[])
+    base_path = db_field('str')
 
     @classmethod
     def get_or_create(cls, name):
diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index eaa920e4..50f499e4 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -33,11 +33,11 @@ def guess_action(from_, to):
     # NOTE(dshulyak) imo the way to solve this - is dsl for orchestration,
     # something where this action will be excplicitly specified
     if not from_:
-        return 'run'
+        return CHANGES.run.name
     elif not to:
-        return 'remove'
+        return CHANGES.remove.name
     else:
-        return 'update'
+        return CHANGES.update.name
 
 
 def create_diff(staged, commited):
@@ -65,16 +65,18 @@ def stage_changes():
     log = data.SL()
     log.clean()
     resources_map = {r.name: r for r in resource.load_all()}
-    commited_map = {r.id for r in orm.DBCommitedState.load_all()}
+    commited_map = {r.id: r for r in orm.DBCommitedState.load_all()}
 
     for resource_id in set(resources_map.keys()) | set(commited_map.keys()):
 
-        if resource_id not in resource_map:
+        if resource_id not in resources_map:
             resource_args = {}
             resource_connections = []
+            base_path = commited_map[resource_id].base_path
         else:
-            resource_args = resource_map[resource_id].args
-            resource_connections = resource_map[resource_id].connections
+            resource_args = resources_map[resource_id].args
+            resource_connections = resources_map[resource_id].connections
+            base_path = resources_map[resource_id].base_path
 
         if resource_id not in commited_map:
             commited_args = {}
@@ -95,7 +97,7 @@ def stage_changes():
                 guess_action(commited_connections, resource_connections),
                 inputs_diff,
                 connections_diff,
-                base_path=resource_obj.base_path)
+                base_path=base_path)
             log.append(log_item)
     return log
 
@@ -150,21 +152,34 @@ def _revert_remove(logitem):
     """Resource should be created with all previous connections
     """
     commited = orm.DBCommitedState.load(logitem.res)
-    args = dictdiffer.revert(
-        logitem.diff, commited.inputs)
-    resource_obj = resource.Resource(
-        logitem.res, logitem.base_path, args=args)
+    args = dictdiffer.revert(logitem.diff, commited.inputs)
+    connections = dictdiffer.revert(logitem.signals_diff, sorted(commited.connections))
+    resource.Resource(logitem.res, logitem.base_path, args=args)
+    for emitter, emitter_input, receiver, receiver_input in connections:
+        emmiter_obj = resource.load(emitter)
+        receiver_obj = resource.load(receiver)
+        signals.connect(emmiter_obj, receiver_obj, {emitter_input: receiver_input})
 
 
 def _revert_update(logitem):
-    """Revert of update should use update inputs and connections
+    """Revert of update should update inputs and connections
     """
     res_obj = resource.load(logitem.res)
     commited = res_obj.load_commited()
-    args_to_update = dictdiffer.revert(
-        logitem.diff, commited.inputs)
+    args_to_update = dictdiffer.revert(logitem.diff, commited.inputs)
     res_obj.update(args_to_update)
 
+    for emitter, _, receiver, _ in commited.connections:
+        emmiter_obj = resource.load(emitter)
+        receiver_obj = resource.load(receiver)
+        signals.disconnect(emmiter_obj, receiver_obj)
+
+    connections = dictdiffer.revert(logitem.signals_diff, sorted(commited.connections))
+    for emitter, emitter_input, receiver, receiver_input in connections:
+        emmiter_obj = resource.load(emitter)
+        receiver_obj = resource.load(receiver)
+        signals.connect(emmiter_obj, receiver_obj, {emitter_input: receiver_input})
+
 
 def _revert_run(logitem):
     res_obj = resource.load(logitem.res)
diff --git a/solar/solar/system_log/data.py b/solar/solar/system_log/data.py
index 1d74a826..ba6c5c79 100644
--- a/solar/solar/system_log/data.py
+++ b/solar/solar/system_log/data.py
@@ -149,3 +149,6 @@ class Log(object):
 
     def __iter__(self):
         return iter(self.collection())
+
+    def __len__(self):
+        return len(list(self.collection()))
diff --git a/solar/solar/system_log/operations.py b/solar/solar/system_log/operations.py
index a31629cc..a4c1cee2 100644
--- a/solar/solar/system_log/operations.py
+++ b/solar/solar/system_log/operations.py
@@ -34,6 +34,7 @@ def move_to_commited(log_action, *args, **kwargs):
         commited.inputs = patch(item.diff, commited.inputs)
         sorted_connections = sorted(commited.connections)
         commited.connections = patch(item.signals_diff, sorted_connections)
+        commited.base_path = item.base_path
         commited.save()
         cl = data.CL()
         item.state = data.STATES.success
diff --git a/solar/solar/test/test_system_log_api.py b/solar/solar/test/test_system_log_api.py
index add11613..7c8dafd1 100644
--- a/solar/solar/test/test_system_log_api.py
+++ b/solar/solar/test/test_system_log_api.py
@@ -18,6 +18,7 @@ from pytest import fixture
 from solar.system_log import change
 from solar.system_log import data
 from solar.system_log import operations
+from solar.core import signals
 from solar.core.resource import resource
 from solar.interfaces import orm
 
@@ -49,6 +50,47 @@ def test_revert_update():
     assert resource_obj.args == previous
 
 
+def test_revert_update_connected():
+    res1 = orm.DBResource(id='test1', name='test1', base_path='x')
+    res1.save()
+    res1.add_input('a', 'str', '9')
+
+    res2 = orm.DBResource(id='test2', name='test2', base_path='x')
+    res2.save()
+    res2.add_input('a', 'str', 0)
+
+    res3 = orm.DBResource(id='test3', name='test3', base_path='x')
+    res3.save()
+    res3.add_input('a', 'str', 0)
+
+    res1 = resource.load('test1')
+    res2 = resource.load('test2')
+    res3 = resource.load('test3')
+    signals.connect(res1, res2)
+    signals.connect(res2, res3)
+
+    staged_log = change.stage_changes()
+    assert len(staged_log) == 3
+    for item in staged_log:
+        operations.move_to_commited(item.log_action)
+    assert len(staged_log) == 0
+
+    signals.disconnect(res1, res2)
+
+    staged_log = change.stage_changes()
+    assert len(staged_log) == 2
+    to_revert = []
+    for item in staged_log:
+        operations.move_to_commited(item.log_action)
+        to_revert.append(item.uid)
+
+    change.revert_uids(reversed(to_revert))
+    staged_log = change.stage_changes()
+    assert len(staged_log) == 2
+    for item in staged_log:
+        assert item.diff == [['change', 'a', [0, '9']]]
+
+
 def test_revert_removal():
     res = orm.DBResource(id='test1', name='test1', base_path='x')
     res.save()
@@ -77,6 +119,38 @@ def test_revert_removal():
     assert resource_obj.args == {'a': '9'}
 
 
+def test_revert_removed_child():
+    res1 = orm.DBResource(id='test1', name='test1', base_path='x')
+    res1.save()
+    res1.add_input('a', 'str', '9')
+
+    res2 = orm.DBResource(id='test2', name='test2', base_path='x')
+    res2.save()
+    res2.add_input('a', 'str', 0)
+
+    res1 = resource.load('test1')
+    res2 = resource.load('test2')
+    signals.connect(res1, res2)
+
+    staged_log = change.stage_changes()
+    assert len(staged_log) == 2
+    for item in staged_log:
+        operations.move_to_commited(item.log_action)
+    res2.delete()
+
+    staged_log = change.stage_changes()
+    assert len(staged_log) == 1
+    logitem = next(staged_log.collection())
+    operations.move_to_commited(logitem.log_action)
+
+    with mock.patch.object(resource, 'read_meta') as mread:
+        mread.return_value = {'input': {'a': {'schema': 'str!'}}}
+        change.revert(logitem.uid)
+
+    res2 = resource.load('test2')
+    assert res2.args == {'a': '9'}
+
+
 def test_revert_create():
     res = orm.DBResource(id='test1', name='test1', base_path='x')
     res.save()

From 24217e913740c4755c434d656ef793e79adfff30 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Fri, 25 Sep 2015 10:13:40 +0300
Subject: [PATCH 07/12] Use staged log in routine for building orchestration
 graph

---
 solar/solar/system_log/change.py | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index 50f499e4..8533f77d 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -61,6 +61,7 @@ def create_sorted_diff(staged, commited):
     return create_diff(staged, commited)
 
 
+
 def stage_changes():
     log = data.SL()
     log.clean()
@@ -94,7 +95,7 @@ def stage_changes():
         if inputs_diff:
             log_item = create_logitem(
                 resource_id,
-                guess_action(commited_connections, resource_connections),
+                guess_action(commited_args, resource_args),
                 inputs_diff,
                 connections_diff,
                 base_path=base_path)
@@ -104,27 +105,19 @@ def stage_changes():
 
 def send_to_orchestration():
     dg = nx.MultiDiGraph()
-
     events = {}
     changed_nodes = []
 
-    for resource_obj in resource.load_all():
-        commited_db_obj = resource_obj.load_commited()
-        resource_args = resource_obj.args
+    for logitem in data.SL():
+        events[logitem.res] = evapi.all_events(logitem.res)
+        changed_nodes.append(logitem.res)
 
-        df = create_diff(resource_args, commited_db_obj.inputs)
-
-        if df:
-            events[resource_obj.name] = evapi.all_events(resource_obj.name)
-            changed_nodes.append(resource_obj.name)
-            action = guess_action(resource_args, commited_db_obj.inputs)
-
-            state_change = evapi.StateChange(resource_obj.name, action)
-            state_change.insert(changed_nodes, dg)
+        state_change = evapi.StateChange(logitem.res, logitem.action)
+        state_change.insert(changed_nodes, dg)
 
     evapi.build_edges(dg, events)
 
-    # what it should be?
+    # what `name` should be?
     dg.graph['name'] = 'system_log'
     return graph.create_plan_from_graph(dg)
 
@@ -146,6 +139,9 @@ def revert_uids(uids):
             _revert_remove(item)
         elif item.action == CHANGES.run.name:
             _revert_run(item)
+        else:
+            log.debug('Action %s for resource %s is a side'
+                      ' effect of another action', item.action, item.res)
 
 
 def _revert_remove(logitem):

From 847d7a246dab4bc3f2587e16d6753b88fea0cf88 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Fri, 25 Sep 2015 10:46:45 +0300
Subject: [PATCH 08/12] Store events separetely from resource

---
 resources/hosts_file/actions/remove.yaml |  5 +++++
 solar/solar/events/api.py                | 14 +++++++-------
 solar/solar/interfaces/db/base.py        |  2 +-
 solar/solar/interfaces/orm.py            | 21 +++++++++++++++++++--
 solar/solar/system_log/data.py           |  2 +-
 solar/solar/system_log/operations.py     |  2 +-
 6 files changed, 34 insertions(+), 12 deletions(-)
 create mode 100644 resources/hosts_file/actions/remove.yaml

diff --git a/resources/hosts_file/actions/remove.yaml b/resources/hosts_file/actions/remove.yaml
new file mode 100644
index 00000000..0c38605e
--- /dev/null
+++ b/resources/hosts_file/actions/remove.yaml
@@ -0,0 +1,5 @@
+- hosts: [{{host}}]
+  sudo: yes
+  tasks:
+    - name: Remove hosts file
+      shell: rm /etc/hosts
diff --git a/solar/solar/events/api.py b/solar/solar/events/api.py
index bb13189c..7471c880 100644
--- a/solar/solar/events/api.py
+++ b/solar/solar/events/api.py
@@ -44,10 +44,10 @@ def add_event(ev):
             break
     else:
         rst.append(ev)
-        resource_db = orm.DBResource.load(ev.parent)
+        resource_events = orm.DBResourceEvents.get_or_create(ev.parent)
         event_db = orm.DBEvent(**ev.to_dict())
         event_db.save()
-        resource_db.events.add(event_db)
+        resource_events.events.add(event_db)
 
 
 def add_dep(parent, dep, actions, state='success'):
@@ -67,21 +67,21 @@ def add_react(parent, dep, actions, state='success'):
 
 
 def add_events(resource, lst):
-    db_resource = orm.DBResource.load(resource)
+    resource_events = orm.DBResourceEvents.get_or_create(resource)
     for ev in lst:
         event_db = orm.DBEvent(**ev.to_dict())
         event_db.save()
-        db_resource.events.add(event_db)
+        resource_events.events.add(event_db)
 
 
 def set_events(resource, lst):
-    db_resource = orm.DBResource.load(resource)
+    resource_events = orm.DBResourceEvents.get_or_create(resource)
     for ev in db_resource.events.as_set():
         ev.delete()
     for ev in lst:
         event_db = orm.DBEvent(**ev.to_dict())
         event_db.save()
-        db_resource.events.add(event_db)
+        resource_events.events.add(event_db)
 
 
 def remove_event(ev):
@@ -90,7 +90,7 @@ def remove_event(ev):
 
 
 def all_events(resource):
-    events = orm.DBResource.load(resource).events.as_set()
+    events = orm.DBResourceEvents.get_or_create(resource).events.as_set()
 
     if not events:
         return []
diff --git a/solar/solar/interfaces/db/base.py b/solar/solar/interfaces/db/base.py
index 3cdbfbac..ddb448ac 100644
--- a/solar/solar/interfaces/db/base.py
+++ b/solar/solar/interfaces/db/base.py
@@ -127,7 +127,7 @@ class BaseGraphDB(object):
 
     COLLECTIONS = Enum(
         'Collections',
-        'input resource state_data state_log plan_node plan_graph events stage_log commit_log'
+        'input resource state_data state_log plan_node plan_graph events stage_log commit_log resource_events'
     )
     DEFAULT_COLLECTION=COLLECTIONS.resource
     RELATION_TYPES = Enum(
diff --git a/solar/solar/interfaces/orm.py b/solar/solar/interfaces/orm.py
index 5031f6dc..0266bb1b 100644
--- a/solar/solar/interfaces/orm.py
+++ b/solar/solar/interfaces/orm.py
@@ -566,6 +566,25 @@ class DBEvent(DBObject):
         super(DBEvent, self).delete()
 
 
+class DBResourceEvents(DBObject):
+
+    __metaclass__ = DBObjectMeta
+
+    _collection = base.BaseGraphDB.COLLECTIONS.resource_events
+
+    id = db_field(schema='str!', is_primary=True)
+    events = db_related_field(base.BaseGraphDB.RELATION_TYPES.resource_event,
+                              DBEvent)
+
+    @classmethod
+    def get_or_create(cls, name):
+        r = db.get_or_create(
+            name,
+            properties={'id': name},
+            collection=cls._collection)
+        return cls(**r.properties)
+
+
 class DBCommitedState(DBObject):
 
     __metaclass__ = DBObjectMeta
@@ -604,8 +623,6 @@ class DBResource(DBObject):
 
     inputs = db_related_field(base.BaseGraphDB.RELATION_TYPES.resource_input,
                               DBResourceInput)
-    events = db_related_field(base.BaseGraphDB.RELATION_TYPES.resource_event,
-                              DBEvent)
 
     def add_input(self, name, schema, value):
         # NOTE: Inputs need to have uuid added because there can be many
diff --git a/solar/solar/system_log/data.py b/solar/solar/system_log/data.py
index ba6c5c79..564f853e 100644
--- a/solar/solar/system_log/data.py
+++ b/solar/solar/system_log/data.py
@@ -42,7 +42,7 @@ class LogItem(object):
                  signals_diff, state=None, base_path=None):
         self.uid = uid
         self.res = res
-        self.log_action = '{}:{}'.format(res, action)
+        self.log_action = '{}.{}'.format(res, action)
         self.action = action
         self.diff = diff
         self.signals_diff = signals_diff
diff --git a/solar/solar/system_log/operations.py b/solar/solar/system_log/operations.py
index a4c1cee2..ebf636d4 100644
--- a/solar/solar/system_log/operations.py
+++ b/solar/solar/system_log/operations.py
@@ -28,8 +28,8 @@ def set_error(log_action, *args, **kwargs):
 def move_to_commited(log_action, *args, **kwargs):
     sl = data.SL()
     item = next((i for i in sl if i.log_action == log_action), None)
-    sl.pop(item.uid)
     if item:
+        sl.pop(item.uid)
         commited = orm.DBCommitedState.get_or_create(item.res)
         commited.inputs = patch(item.diff, commited.inputs)
         sorted_connections = sorted(commited.connections)

From 3b420ffe4f845a53d4aafd85edcd1c87bb89152d Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Fri, 25 Sep 2015 13:07:51 +0300
Subject: [PATCH 09/12] Make removal of staged data only after commit

---
 solar/solar/cli/resource.py                 |  5 ++--
 solar/solar/core/resource/resource.py       | 26 ++++++++++++++++++++-
 solar/solar/events/api.py                   |  2 +-
 solar/solar/interfaces/db/redis_graph_db.py |  3 +--
 solar/solar/interfaces/orm.py               |  1 +
 solar/solar/system_log/change.py            |  2 +-
 solar/solar/system_log/operations.py        | 13 +++++++++++
 solar/solar/test/test_orm.py                |  8 +++----
 solar/solar/test/test_system_log_api.py     |  8 +++++--
 9 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/solar/solar/cli/resource.py b/solar/solar/cli/resource.py
index d4938a69..e4986ef7 100644
--- a/solar/solar/cli/resource.py
+++ b/solar/solar/cli/resource.py
@@ -222,6 +222,7 @@ def get_inputs(path):
 
 @resource.command()
 @click.argument('name')
-def remove(name):
+@click.option('-f', default=False, help='force removal from database')
+def remove(name, f):
     res = sresource.load(name)
-    res.delete()
+    res.remove(force=f)
diff --git a/solar/solar/core/resource/resource.py b/solar/solar/core/resource/resource.py
index cc01cfb1..a290295d 100644
--- a/solar/solar/core/resource/resource.py
+++ b/solar/solar/core/resource/resource.py
@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from enum import Enum
+
 from copy import deepcopy
 from multipledispatch import dispatch
 import os
@@ -40,6 +42,9 @@ def read_meta(base_path):
     return metadata
 
 
+RESOURCE_STATE = Enum('ResourceState', 'created operational removed error updated')
+
+
 class Resource(object):
     _metadata = {}
 
@@ -73,7 +78,7 @@ class Resource(object):
             'meta_inputs': inputs
 
         })
-
+        self.db_obj.state = RESOURCE_STATE.created.name
         self.db_obj.save()
 
         self.create_inputs(args)
@@ -141,6 +146,7 @@ class Resource(object):
     def update(self, args):
         # TODO: disconnect input when it is updated and end_node
         #       for some input_to_input relation
+        self.db_obj.state = RESOURCE_STATE.updated.name
         resource_inputs = self.resource_inputs()
 
         for k, v in args.items():
@@ -151,6 +157,24 @@ class Resource(object):
     def delete(self):
         return self.db_obj.delete()
 
+    def remove(self, force=False):
+        if force:
+            self.delete()
+        else:
+            self.db_obj.state = RESOURCE_STATE.removed.name
+            self.db_obj.save()
+
+    def set_operational(self):
+        self.db_obj.state = RESOURCE_STATE.operational.name
+        self.db_obj.save()
+
+    def set_error(self):
+        self.db_obj.state = RESOURCE_STATE.error.name
+        self.db_obj.save()
+
+    def to_be_removed(self):
+        return self.db_obj.state == RESOURCE_STATE.error.name
+
     @property
     def connections(self):
         """
diff --git a/solar/solar/events/api.py b/solar/solar/events/api.py
index 7471c880..12bae370 100644
--- a/solar/solar/events/api.py
+++ b/solar/solar/events/api.py
@@ -76,7 +76,7 @@ def add_events(resource, lst):
 
 def set_events(resource, lst):
     resource_events = orm.DBResourceEvents.get_or_create(resource)
-    for ev in db_resource.events.as_set():
+    for ev in resource_events.events.as_set():
         ev.delete()
     for ev in lst:
         event_db = orm.DBEvent(**ev.to_dict())
diff --git a/solar/solar/interfaces/db/redis_graph_db.py b/solar/solar/interfaces/db/redis_graph_db.py
index 2e85da66..8b2cf602 100644
--- a/solar/solar/interfaces/db/redis_graph_db.py
+++ b/solar/solar/interfaces/db/redis_graph_db.py
@@ -39,7 +39,7 @@ class RedisGraphDB(BaseGraphDB):
             source_collection = BaseGraphDB.COLLECTIONS.resource
             dest_collection = BaseGraphDB.COLLECTIONS.input
         elif relation_db['type_'] == BaseGraphDB.RELATION_TYPES.resource_event.name:
-            source_collection = BaseGraphDB.COLLECTIONS.resource
+            source_collection = BaseGraphDB.COLLECTIONS.resource_events
             dest_collection = BaseGraphDB.COLLECTIONS.events
 
         source = self.get(relation_db['source'], collection=source_collection)
@@ -146,7 +146,6 @@ class RedisGraphDB(BaseGraphDB):
     def get(self, name, collection=BaseGraphDB.DEFAULT_COLLECTION,
             return_empty=False):
         """Fetch element with given name and collection type."""
-
         try:
             collection_key = self._make_collection_key(collection, name)
             item = self._r.get(collection_key)
diff --git a/solar/solar/interfaces/orm.py b/solar/solar/interfaces/orm.py
index 0266bb1b..b264aa5c 100644
--- a/solar/solar/interfaces/orm.py
+++ b/solar/solar/interfaces/orm.py
@@ -620,6 +620,7 @@ class DBResource(DBObject):
     version = db_field(schema='str')
     tags = db_field(schema=[], default_value=[])
     meta_inputs = db_field(schema={}, default_value={})
+    state = db_field(schema='str')
 
     inputs = db_related_field(base.BaseGraphDB.RELATION_TYPES.resource_input,
                               DBResourceInput)
diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index 8533f77d..8fad02c1 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -70,7 +70,7 @@ def stage_changes():
 
     for resource_id in set(resources_map.keys()) | set(commited_map.keys()):
 
-        if resource_id not in resources_map:
+        if resources_map[resource_id].to_be_removed():
             resource_args = {}
             resource_connections = []
             base_path = commited_map[resource_id].base_path
diff --git a/solar/solar/system_log/operations.py b/solar/solar/system_log/operations.py
index ebf636d4..a5f35510 100644
--- a/solar/solar/system_log/operations.py
+++ b/solar/solar/system_log/operations.py
@@ -15,12 +15,16 @@
 from solar.system_log import data
 from dictdiffer import patch
 from solar.interfaces import orm
+from solar.core.resource import resource
+from .consts import CHANGES
 
 
 def set_error(log_action, *args, **kwargs):
     sl = data.SL()
     item = next((i for i in sl if i.log_action == log_action), None)
     if item:
+        resource_obj = resource.load(item.res)
+        resource.set_error()
         item.state = data.STATES.error
         sl.update(item)
 
@@ -30,6 +34,15 @@ def move_to_commited(log_action, *args, **kwargs):
     item = next((i for i in sl if i.log_action == log_action), None)
     if item:
         sl.pop(item.uid)
+        resource_obj = resource.load(item.res)
+
+        if item.action == CHANGES.remove.name:
+            resource_obj.delete()
+        elif item.action == CHANGES.run.name:
+            resource_obj.set_operational()
+        elif item.action == CHANGES.update.name:
+            resource_obj.set_operational()
+
         commited = orm.DBCommitedState.get_or_create(item.res)
         commited.inputs = patch(item.diff, commited.inputs)
         sorted_connections = sorted(commited.connections)
diff --git a/solar/solar/test/test_orm.py b/solar/solar/test/test_orm.py
index 38df33a6..03cbc517 100644
--- a/solar/solar/test/test_orm.py
+++ b/solar/solar/test/test_orm.py
@@ -431,7 +431,7 @@ input:
 class TestEventORM(BaseResourceTest):
 
     def test_return_emtpy_set(self):
-        r = orm.DBResource(id='test1', name='test1', base_path='x')
+        r = orm.DBResourceEvents(id='test1')
         r.save()
         self.assertEqual(r.events.as_set(), set())
 
@@ -468,11 +468,11 @@ class TestEventORM(BaseResourceTest):
         self.assertEqual(len(orm.DBEvent.load_all()), 2)
 
     def test_removal_of_event(self):
-        r = orm.DBResource(id='n1', name='n1', base_path='x')
+        r = orm.DBResourceEvents(id='test1')
         r.save()
 
         ev = orm.DBEvent(
-            parent='n1',
+            parent='test1',
             parent_action='run',
             state='success',
             child_action='run',
@@ -484,5 +484,5 @@ class TestEventORM(BaseResourceTest):
         self.assertEqual(r.events.as_set(), {ev})
         ev.delete()
 
-        r = orm.DBResource.load('n1')
+        r = orm.DBResourceEvents.load('test1')
         self.assertEqual(r.events.as_set(), set())
diff --git a/solar/solar/test/test_system_log_api.py b/solar/solar/test/test_system_log_api.py
index 7c8dafd1..c40d0d61 100644
--- a/solar/solar/test/test_system_log_api.py
+++ b/solar/solar/test/test_system_log_api.py
@@ -15,6 +15,7 @@
 import mock
 
 from pytest import fixture
+from pytest import mark
 from solar.system_log import change
 from solar.system_log import data
 from solar.system_log import operations
@@ -95,7 +96,7 @@ def test_revert_removal():
     res = orm.DBResource(id='test1', name='test1', base_path='x')
     res.save()
     res.add_input('a', 'str', '9')
-    res.delete()
+
     commited = orm.DBCommitedState.get_or_create('test1')
     commited.inputs = {'a': '9'}
     commited.save()
@@ -105,6 +106,8 @@ def test_revert_removal():
         base_path=res.base_path)
     log = data.SL()
     log.append(logitem)
+    resource_obj = resource.load(res.name)
+    resource_obj.remove()
     operations.move_to_commited(logitem.log_action)
 
     resources = orm.DBResource.load_all()
@@ -119,6 +122,7 @@ def test_revert_removal():
     assert resource_obj.args == {'a': '9'}
 
 
+@mark.xfail(reason='With current approach child will be notice changes after parent is removed')
 def test_revert_removed_child():
     res1 = orm.DBResource(id='test1', name='test1', base_path='x')
     res1.save()
@@ -136,7 +140,7 @@ def test_revert_removed_child():
     assert len(staged_log) == 2
     for item in staged_log:
         operations.move_to_commited(item.log_action)
-    res2.delete()
+    res2.remove()
 
     staged_log = change.stage_changes()
     assert len(staged_log) == 1

From 055f89b7f678af329940a0631651508cd8a59dca Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Fri, 25 Sep 2015 15:52:26 +0300
Subject: [PATCH 10/12] Fix several inconsistencies with data and receiver
 input in case of hash

Add example of usage to hosts_file/README.md
---
 examples/hosts_file/README.md            | 61 +++++++++++++++++++++++-
 resources/hosts_file/actions/remove.yaml |  2 +-
 solar/solar/core/resource/resource.py    | 12 +++--
 solar/solar/interfaces/orm.py            | 14 ++++--
 solar/solar/system_log/change.py         | 28 +++++------
 solar/solar/system_log/data.py           |  3 ++
 solar/solar/system_log/operations.py     | 17 +++----
 solar/solar/test/test_system_log_api.py  | 15 +++---
 8 files changed, 112 insertions(+), 40 deletions(-)

diff --git a/examples/hosts_file/README.md b/examples/hosts_file/README.md
index b3f66ee5..61086897 100644
--- a/examples/hosts_file/README.md
+++ b/examples/hosts_file/README.md
@@ -9,8 +9,8 @@ Then you can continue with standard solar things:
 ```
 solar changes stage -d
 solar changes process
-solar changes run-once last
-watch -n 1 solar changes report last
+solar or run-once last
+watch -n 1 solar or report last
 ```
 
 Wait until all actions have state `SUCCESS`,
@@ -21,3 +21,60 @@ after that check `/etc/hosts` files on both nodes, it will contain entries like:
 10.0.0.4 second1441705178.0
 ```
 
+If you want to try out revert functionality - you can do it in a next way:
+
+After you created all the stuff, print history like this:
+
+`solar ch history`
+
+Output:
+
+```
+log task=hosts_file1.run uid=282fe919-6059-4100-affc-56a2b3992d9d
+log task=hosts_file2.run uid=774f5a49-00f1-4bae-8a77-90d1b2d54164
+log task=node1.run uid=2559f22c-5aa9-4c05-91c6-b70884190a56
+log task=node2.run uid=18f06abe-3e8d-4356-b172-128e1dded0e6
+```
+
+Now you can try to revert creation of hosts_file1
+
+```
+solar ch revert 282fe919-6059-4100-affc-56a2b3992d9d
+solar ch stage
+log task=hosts_file1.remove uid=1fe456c1-a847-4902-88bf-b7f2c5687d40
+solar ch process
+solar or run-once last
+watch -n 1 solar or report last
+```
+
+For now this file will be simply cleaned (more cophisticated task can be added later).
+And you can create revert of your revert, which will lead to created hosts_file1
+resource and /etc/hosts with appropriate content
+
+```
+solar ch revert 282fe919-6059-4100-affc-56a2b3992d9d
+solar ch stage
+log task=hosts_file1.remove uid=1fe456c1-a847-4902-88bf-b7f2c5687d40
+solar ch process
+solar changes run-once last
+watch -n 1 solar changes report last
+```
+
+After this you can revert your result of your previous revert, which will
+create this file with relevant content.
+
+```
+solar ch history -n 1
+log task=hosts_file1.remove uid=1fe456c1-a847-4902-88bf-b7f2c5687d40
+solar ch revert 1fe456c1-a847-4902-88bf-b7f2c5687d40
+solar ch stage
+log task=hosts_file1.run uid=493326b2-989f-4b94-a22c-0bbd0fc5e755
+solar ch process
+solar changes run-once last
+watch -n 1 solar changes report last
+```
+
+
+
+
+
diff --git a/resources/hosts_file/actions/remove.yaml b/resources/hosts_file/actions/remove.yaml
index 0c38605e..d211b58a 100644
--- a/resources/hosts_file/actions/remove.yaml
+++ b/resources/hosts_file/actions/remove.yaml
@@ -2,4 +2,4 @@
   sudo: yes
   tasks:
     - name: Remove hosts file
-      shell: rm /etc/hosts
+      shell: echo '# flushed by ansible' > /etc/hosts
diff --git a/solar/solar/core/resource/resource.py b/solar/solar/core/resource/resource.py
index a290295d..214a03dd 100644
--- a/solar/solar/core/resource/resource.py
+++ b/solar/solar/core/resource/resource.py
@@ -173,7 +173,7 @@ class Resource(object):
         self.db_obj.save()
 
     def to_be_removed(self):
-        return self.db_obj.state == RESOURCE_STATE.error.name
+        return self.db_obj.state == RESOURCE_STATE.removed.name
 
     @property
     def connections(self):
@@ -183,10 +183,16 @@ class Resource(object):
         [(emitter, emitter_input, receiver, receiver_input), ...]
         """
         rst = []
-        for emitter, receiver in self.db_obj.graph().edges():
+        for emitter, receiver, meta in self.db_obj.graph().edges(data=True):
+            if meta:
+                receiver_input = '{}:{}|{}'.format(receiver.name,
+                    meta['destination_key'], meta['tag'])
+            else:
+                receiver_input = receiver.name
+
             rst.append(
                 [emitter.resource.name, emitter.name,
-                 receiver.resource.name, receiver.name])
+                 receiver.resource.name, receiver_input])
         return rst
 
     def resource_inputs(self):
diff --git a/solar/solar/interfaces/orm.py b/solar/solar/interfaces/orm.py
index b264aa5c..83b27e24 100644
--- a/solar/solar/interfaces/orm.py
+++ b/solar/solar/interfaces/orm.py
@@ -437,16 +437,18 @@ class DBResourceInput(DBObject):
         super(DBResourceInput, self).delete()
 
     def edges(self):
+
         out = db.get_relations(
                 source=self._db_node,
                 type_=base.BaseGraphDB.RELATION_TYPES.input_to_input)
         incoming = db.get_relations(
                 dest=self._db_node,
                 type_=base.BaseGraphDB.RELATION_TYPES.input_to_input)
-        for r in out + incoming:
-            source = DBResourceInput(**r.start_node.properties)
-            dest = DBResourceInput(**r.end_node.properties)
-            yield source, dest
+        for relation in out + incoming:
+            meta = relation.properties
+            source = DBResourceInput(**relation.start_node.properties)
+            dest = DBResourceInput(**relation.end_node.properties)
+            yield source, dest, meta
 
     def check_other_val(self, other_val=None):
         if not other_val:
@@ -594,7 +596,9 @@ class DBCommitedState(DBObject):
     id = db_field(schema='str!', is_primary=True)
     inputs = db_field(schema={}, default_value={})
     connections = db_field(schema=[], default_value=[])
-    base_path = db_field('str')
+    base_path = db_field(schema='str')
+    tags = db_field(schema=[], default_value=[])
+    state = db_field(schema='str', default_value='removed')
 
     @classmethod
     def get_or_create(cls, name):
diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index 8fad02c1..f5f91113 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -25,6 +25,7 @@ from solar.orchestration import graph
 from solar.events import api as evapi
 from solar.interfaces import orm
 from .consts import CHANGES
+from solar.core.resource.resource import RESOURCE_STATE
 
 db = get_db()
 
@@ -65,26 +66,23 @@ def create_sorted_diff(staged, commited):
 def stage_changes():
     log = data.SL()
     log.clean()
-    resources_map = {r.name: r for r in resource.load_all()}
-    commited_map = {r.id: r for r in orm.DBCommitedState.load_all()}
 
-    for resource_id in set(resources_map.keys()) | set(commited_map.keys()):
-
-        if resources_map[resource_id].to_be_removed():
+    for resouce_obj in resource.load_all():
+        commited = resouce_obj.load_commited()
+        base_path = resouce_obj.base_path
+        if resouce_obj.to_be_removed():
             resource_args = {}
             resource_connections = []
-            base_path = commited_map[resource_id].base_path
         else:
-            resource_args = resources_map[resource_id].args
-            resource_connections = resources_map[resource_id].connections
-            base_path = resources_map[resource_id].base_path
+            resource_args = resouce_obj.args
+            resource_connections = resouce_obj.connections
 
-        if resource_id not in commited_map:
+        if commited.state == RESOURCE_STATE.removed.name:
             commited_args = {}
             commited_connections = []
         else:
-            commited_args = commited_map[resource_id].inputs
-            commited_connections = commited_map[resource_id].connections
+            commited_args = commited.inputs
+            commited_connections = commited.connections
 
         inputs_diff = create_diff(resource_args, commited_args)
         connections_diff = create_sorted_diff(
@@ -94,7 +92,7 @@ def stage_changes():
         # but using inputs to reverse connections is not possible
         if inputs_diff:
             log_item = create_logitem(
-                resource_id,
+                resouce_obj.name,
                 guess_action(commited_args, resource_args),
                 inputs_diff,
                 connections_diff,
@@ -150,7 +148,7 @@ def _revert_remove(logitem):
     commited = orm.DBCommitedState.load(logitem.res)
     args = dictdiffer.revert(logitem.diff, commited.inputs)
     connections = dictdiffer.revert(logitem.signals_diff, sorted(commited.connections))
-    resource.Resource(logitem.res, logitem.base_path, args=args)
+    resource.Resource(logitem.res, logitem.base_path, args=args, tags=commited.tags)
     for emitter, emitter_input, receiver, receiver_input in connections:
         emmiter_obj = resource.load(emitter)
         receiver_obj = resource.load(receiver)
@@ -179,7 +177,7 @@ def _revert_update(logitem):
 
 def _revert_run(logitem):
     res_obj = resource.load(logitem.res)
-    res_obj.delete()
+    res_obj.remove()
 
 
 def revert(uid):
diff --git a/solar/solar/system_log/data.py b/solar/solar/system_log/data.py
index 564f853e..40551ef8 100644
--- a/solar/solar/system_log/data.py
+++ b/solar/solar/system_log/data.py
@@ -91,6 +91,9 @@ def details(diff):
         elif type_ == 'change':
             rst.append('-+ {}: {} >> {}'.format(
                 unwrap_change_val(val), change[0], change[1]))
+        elif type_ == 'remove':
+            for key, val in change:
+                rst.append('-- {}: {}'.format(key ,val))
     return rst
 
 
diff --git a/solar/solar/system_log/operations.py b/solar/solar/system_log/operations.py
index a5f35510..4a692881 100644
--- a/solar/solar/system_log/operations.py
+++ b/solar/solar/system_log/operations.py
@@ -35,19 +35,20 @@ def move_to_commited(log_action, *args, **kwargs):
     if item:
         sl.pop(item.uid)
         resource_obj = resource.load(item.res)
+        commited = orm.DBCommitedState.get_or_create(item.res)
 
         if item.action == CHANGES.remove.name:
             resource_obj.delete()
-        elif item.action == CHANGES.run.name:
-            resource_obj.set_operational()
-        elif item.action == CHANGES.update.name:
+            commited.state = resource.RESOURCE_STATE.removed.name
+        else:
             resource_obj.set_operational()
+            commited.state = resource.RESOURCE_STATE.operational.name
+            commited.inputs = patch(item.diff, commited.inputs)
+            commited.tags = resource_obj.tags
+            sorted_connections = sorted(commited.connections)
+            commited.connections = patch(item.signals_diff, sorted_connections)
+            commited.base_path = item.base_path
 
-        commited = orm.DBCommitedState.get_or_create(item.res)
-        commited.inputs = patch(item.diff, commited.inputs)
-        sorted_connections = sorted(commited.connections)
-        commited.connections = patch(item.signals_diff, sorted_connections)
-        commited.base_path = item.base_path
         commited.save()
         cl = data.CL()
         item.state = data.STATES.success
diff --git a/solar/solar/test/test_system_log_api.py b/solar/solar/test/test_system_log_api.py
index c40d0d61..e18ce5e0 100644
--- a/solar/solar/test/test_system_log_api.py
+++ b/solar/solar/test/test_system_log_api.py
@@ -160,18 +160,21 @@ def test_revert_create():
     res.save()
     res.add_input('a', 'str', '9')
 
-    logitem =change.create_logitem(
-        res.name, 'run', change.create_diff({'a': '9'}, {}), [],
-        base_path=res.base_path)
-    log = data.SL()
-    log.append(logitem)
-    assert logitem.diff == [('add', '', [('a', '9')])]
+    staged_log = change.stage_changes()
+    assert len(staged_log) == 1
+    logitem = next(staged_log.collection())
 
     operations.move_to_commited(logitem.log_action)
+    assert logitem.diff == [['add', '', [['a', '9']]]]
+
     commited = orm.DBCommitedState.load('test1')
     assert commited.inputs == {'a': '9'}
 
     change.revert(logitem.uid)
 
+    staged_log = change.stage_changes()
+    assert len(staged_log) == 1
+    for item in staged_log:
+        operations.move_to_commited(item.log_action)
     resources = orm.DBResource.load_all()
     assert resources == []

From c7edbd466c7c0ea36764a039eae1776792c8c0e0 Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Mon, 28 Sep 2015 09:57:31 +0300
Subject: [PATCH 11/12] Make connections for transports and location optional

For testing you dont need to create this things all the time, because
tests doesnt depend on them. And it only leads to unnecessary complexity
in asserts
---
 solar/solar/core/signals.py             | 7 ++++++-
 solar/solar/test/test_system_log_api.py | 6 ++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/solar/solar/core/signals.py b/solar/solar/core/signals.py
index 4cef5dde..5250eeb2 100644
--- a/solar/solar/core/signals.py
+++ b/solar/solar/core/signals.py
@@ -85,7 +85,12 @@ def location_and_transports(emitter, receiver, orig_mapping):
     inps_receiver = receiver.args
     # XXX: should be somehow parametrized (input attribute?)
     for single in ('transports_id', 'location_id'):
-        _single(single, inps_emitter[single], inps_receiver[single])
+        if single in inps_emitter and inps_receiver:
+            _single(single, inps_emitter[single], inps_receiver[single])
+        else:
+            log.warning('Unable to create connection for %s with'
+                        ' emitter %s, receiver %s',
+                        single, emitter.name, receiver.name)
     return
 
 
diff --git a/solar/solar/test/test_system_log_api.py b/solar/solar/test/test_system_log_api.py
index e18ce5e0..f6bf0be2 100644
--- a/solar/solar/test/test_system_log_api.py
+++ b/solar/solar/test/test_system_log_api.py
@@ -96,9 +96,11 @@ def test_revert_removal():
     res = orm.DBResource(id='test1', name='test1', base_path='x')
     res.save()
     res.add_input('a', 'str', '9')
+    res.add_input('location_id', 'str', '1')
+    res.add_input('transports_id', 'str', '1')
 
     commited = orm.DBCommitedState.get_or_create('test1')
-    commited.inputs = {'a': '9'}
+    commited.inputs = {'a': '9', 'location_id': '1', 'transports_id': '1'}
     commited.save()
 
     logitem =change.create_logitem(
@@ -119,7 +121,7 @@ def test_revert_removal():
         mread.return_value = {'input': {'a': {'schema': 'str!'}}}
         change.revert(logitem.uid)
     resource_obj = resource.load('test1')
-    assert resource_obj.args == {'a': '9'}
+    assert resource_obj.args == {'a': '9', 'location_id': '1', 'transports_id': '1'}
 
 
 @mark.xfail(reason='With current approach child will be notice changes after parent is removed')

From 6268b534ae81e7cf033510b3855a395cc8bbdcaf Mon Sep 17 00:00:00 2001
From: Dmitry Shulyak <dshulyak@mirantis.com>
Date: Mon, 28 Sep 2015 16:52:54 +0300
Subject: [PATCH 12/12] Raise error if provided UID is not found in history

---
 solar/solar/cli/system_log.py           |  7 +++++--
 solar/solar/system_log/change.py        | 12 ++++++++++++
 solar/solar/test/test_system_log_api.py |  2 +-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/solar/solar/cli/system_log.py b/solar/solar/cli/system_log.py
index a21cb65c..fa7172f9 100644
--- a/solar/solar/cli/system_log.py
+++ b/solar/solar/cli/system_log.py
@@ -16,6 +16,7 @@ import sys
 
 import click
 
+from solar import errors
 from solar.core import testing
 from solar.core import resource
 from solar.system_log import change
@@ -96,8 +97,10 @@ def history(n, d, s):
 @changes.command()
 @click.argument('uid')
 def revert(uid):
-    change.revert(uid)
-
+    try:
+        change.revert(uid)
+    except errors.SolarError as er:
+        raise click.BadParameter(str(er))
 
 @changes.command()
 @click.option('--name', default=None)
diff --git a/solar/solar/system_log/change.py b/solar/solar/system_log/change.py
index f5f91113..0acf1eee 100644
--- a/solar/solar/system_log/change.py
+++ b/solar/solar/system_log/change.py
@@ -26,6 +26,7 @@ from solar.events import api as evapi
 from solar.interfaces import orm
 from .consts import CHANGES
 from solar.core.resource.resource import RESOURCE_STATE
+from solar.errors import CannotFindID
 
 db = get_db()
 
@@ -128,9 +129,20 @@ def parameters(res, action, data):
 
 
 def revert_uids(uids):
+    """
+    :param uids: iterable not generator
+    """
     history = data.CL()
+    not_valid = []
+    for uid in uids:
+        if history.get(uid) is None:
+            not_valid.append(uid)
+    if not_valid:
+        raise CannotFindID('UIDS: {} not in history.'.format(not_valid))
+
     for uid in uids:
         item = history.get(uid)
+
         if item.action == CHANGES.update.name:
             _revert_update(item)
         elif item.action == CHANGES.remove.name:
diff --git a/solar/solar/test/test_system_log_api.py b/solar/solar/test/test_system_log_api.py
index f6bf0be2..a467060c 100644
--- a/solar/solar/test/test_system_log_api.py
+++ b/solar/solar/test/test_system_log_api.py
@@ -85,7 +85,7 @@ def test_revert_update_connected():
         operations.move_to_commited(item.log_action)
         to_revert.append(item.uid)
 
-    change.revert_uids(reversed(to_revert))
+    change.revert_uids(sorted(to_revert, reverse=True))
     staged_log = change.stage_changes()
     assert len(staged_log) == 2
     for item in staged_log: