Merge pull request #112 from Netflix/merge-on-last-updated-time

Merge on last updated time

stethoscope/api/devices.py

@@ -42,6 +42,20 @@ def merge_practices(*args, **kwargs):
   return practices
 
 
+def merge_practices_by_last_updated_time(*args):
+  """Merge two or more dictionaries, preferring values in decreasing order of `last_updated` value.
+
+  Treats practices with no `last_updated` value as the unix epoch time.
+  """
+  practices = dict()
+  for practice in set(itertools.chain.from_iterable(arg.keys() for arg in args)):
+    practices[practice] = max(
+        (arg[practice] for arg in args if practice in arg),
+        key=lambda _practice: _practice.get('last_updated', arrow.get(0)),
+    )
+  return practices
+
+
 def merge_identifiers(identifier_sets):
   canonical = identifier_sets[0]
   for identifier_set in identifier_sets[1:]:
@@ -148,7 +162,7 @@ def merge_device_group(entries):
     device['_raw'] = entries
 
   device['sources'] = [entry['source'] for entry in entries]
-  device['practices'] = merge_practices(*six.moves.map(copy.deepcopy,
+  device['practices'] = merge_practices_by_last_updated_time(*six.moves.map(copy.deepcopy,
     (entry.get('practices', {}) for entry in entries)))
   device['identifiers'] = merge_identifiers(list(six.moves.map(copy.deepcopy,
     (entry.get('identifiers', {}) for entry in entries))))

tests/test_device_merging.py

@@ -2,6 +2,9 @@
 
 from __future__ import absolute_import, print_function, unicode_literals
 
+import copy
+import unittest
+
 import arrow
 import pytest
 
@@ -15,6 +18,10 @@
 ZERODMAC = '00:00:00:00:00:00'
 
 
+def _copy_then_apply(fn, *values):
+  return fn(*(copy.deepcopy(val) for val in values))
+
+
 def test_compare_identifiers_by_serial():
   this = {'serial': '0xDECAFBAD'}
   other = {'serial': '0xDECAFBAD'}
@@ -155,27 +162,27 @@ def test_merge_identifiers():
 def test_merge_device_group():
   alpha = {
     'practices': {
-      'foo': {'status': 'unknown'},
-      'bar': {'status': 'warn'},
+      'foo': {'status': 'unknown', 'last_updated': arrow.get(1)},
+      'bar': {'status': 'warn', 'last_updated': arrow.get(5)},
     },
     'source': 'alpha',
     'last_sync': arrow.get(2),
   }
   bravo = {
     'practices': {
-      'foo': {'status': 'warn'},
-      'bar': {'status': 'nudge'},
+      'foo': {'status': 'warn', 'last_updated': arrow.get(5)},
+      'bar': {'status': 'nudge', 'last_updated': arrow.get(1)},
     },
     'source': 'bravo',
     'last_sync': arrow.get(1),
   }
 
-  merged = stethoscope.api.devices.merge_device_group([alpha, bravo])
+  merged = _copy_then_apply(stethoscope.api.devices.merge_device_group, [alpha, bravo])
   assert merged == {
     'sources': ['alpha', 'bravo'],
     'practices': {
-      'foo': {'status': 'warn'},
-      'bar': {'status': 'warn'},
+      'foo': bravo['practices']['foo'],
+      'bar': alpha['practices']['bar'],
     },
     'identifiers': {},
     'last_sync': arrow.get(2),
@@ -188,62 +195,105 @@ def test_merge_device_group():
     'source': 'charlie',
     '_raw': 'the raw data'
   }
-  merged = stethoscope.api.devices.merge_device_group([alpha, bravo, charlie])
+  merged = _copy_then_apply(stethoscope.api.devices.merge_device_group, [alpha, bravo, charlie])
   assert merged == {
     'sources': ['alpha', 'bravo', 'charlie'],
     'practices': {
-      'foo': {'status': 'warn'},
-      'bar': {'status': 'warn'},
-      'baz': {'status': 'nudge'},
+      'foo': bravo['practices']['foo'],
+      'bar': alpha['practices']['bar'],
+      'baz': charlie['practices']['baz'],
     },
     'identifiers': {},
     '_raw': [alpha, bravo, charlie],
     'last_sync': arrow.get(2),
   }
 
 
-def test_merge_practices():
-  unknown = {'practice': {'status': 'unknown'}}
-  nudge = {'practice': {'status': 'nudge'}}
-  warn = {'practice': {'status': 'warn'}}
-  assert stethoscope.api.devices.merge_practices(unknown, nudge) == \
-      {'practice': {'status': 'nudge'}}
-  assert stethoscope.api.devices.merge_practices({}, nudge) == \
-      {'practice': {'status': 'nudge'}}
-  assert stethoscope.api.devices.merge_practices(unknown, nudge, warn) == \
-      {'practice': {'status': 'warn'}}
+class MergePracticesByStatusOrder(unittest.TestCase):
 
+  def merge(self, *values):
+    return _copy_then_apply(stethoscope.api.devices.merge_practices, *values)
 
-def test_merge_practices_raises_on_extra_kwargs():
-  with pytest.raises(TypeError) as excinfo:
-    stethoscope.api.devices.merge_practices(foo='bar', baz='qux')
-  assert str(excinfo.value) in (
-      "merge_practices() got unexpected keyword argument(s) 'foo', 'baz'",
-      "merge_practices() got unexpected keyword argument(s) 'baz', 'foo'"
-  )
+  def test_merge_practices(self):
+    unknown = {'practice': {'status': 'unknown'}}
+    nudge = {'practice': {'status': 'nudge'}}
+    warn = {'practice': {'status': 'warn'}}
+    assert self.merge(unknown, nudge) == \
+        {'practice': {'status': 'nudge'}}
+    assert self.merge({}, nudge) == \
+        {'practice': {'status': 'nudge'}}
+    assert self.merge(unknown, nudge, warn) == \
+        {'practice': {'status': 'warn'}}
 
+  def test_merge_multiple_practices(self):
+    alpha = {
+      'foo': {'status': 'unknown'},
+      'bar': {'status': 'warn'},
+    }
+    bravo = {
+      'foo': {'status': 'warn'},
+      'bar': {'status': 'nudge'},
+      'baz': {'status': 'unknown'},
+    }
+    assert self.merge(alpha, bravo) == {
+      'foo': {'status': 'warn'},
+      'bar': {'status': 'warn'},
+      'baz': {'status': 'unknown'},
+    }
 
-def test_merge_multiple_practices():
-  alpha = {
-    'foo': {'status': 'unknown'},
-    'bar': {'status': 'warn'},
-  }
-  bravo = {
-    'foo': {'status': 'warn'},
-    'bar': {'status': 'nudge'},
-    'baz': {'status': 'unknown'},
-  }
-  assert stethoscope.api.devices.merge_practices(alpha, bravo) == {
-    'foo': {'status': 'warn'},
-    'bar': {'status': 'warn'},
-    'baz': {'status': 'unknown'},
-  }
+    charlie = {
+      'baz': {'status': 'nudge'},
+    }
+    assert self.merge(alpha, bravo, charlie) == {
+      'foo': {'status': 'warn'},
+      'bar': {'status': 'warn'},
+      'baz': {'status': 'nudge'},
+    }
 
-  charlie = {
-    'baz': {'status': 'nudge'},
-  }
-  assert stethoscope.api.devices.merge_practices(alpha, bravo, charlie) == {
-    'foo': {'status': 'warn'},
-    'bar': {'status': 'warn'},
-    'baz': {'status': 'nudge'},
-  }
+  def test_merge_practices_raises_on_extra_kwargs(self):
+    with pytest.raises(TypeError) as excinfo:
+      stethoscope.api.devices.merge_practices(foo='bar', baz='qux')
+    assert str(excinfo.value) in (
+        "merge_practices() got unexpected keyword argument(s) 'foo', 'baz'",
+        "merge_practices() got unexpected keyword argument(s) 'baz', 'foo'"
+    )
+
+
+class MergePracticesByLastUpdatedTime(unittest.TestCase):
+
+  def merge(self, *values):
+    return _copy_then_apply(stethoscope.api.devices.merge_practices_by_last_updated_time, *values)
+
+  def test_merge_practices_by_last_updated_time(self):
+    unknown = {'practice': {'status': 'unknown', 'last_updated': arrow.get(1)}}
+    nudge = {'practice': {'status': 'nudge', 'last_updated': arrow.get(2)}}
+    warn = {'practice': {'status': 'warn', 'last_updated': arrow.get(3)}}
+
+    assert self.merge(unknown, nudge) == nudge
+    assert self.merge({}, nudge) == nudge
+    assert self.merge(unknown, nudge, warn) == warn
+
+  def test_merge_multiple_practices(self):
+    alpha = {
+      'foo': {'status': 'unknown', 'last_updated': arrow.get(8)},
+      'bar': {'status': 'warn', 'last_updated': arrow.get(10)},
+    }
+    bravo = {
+      'foo': {'status': 'warn', 'last_updated': arrow.get(10)},
+      'bar': {'status': 'nudge', 'last_updated': arrow.get(5)},
+      'baz': {'status': 'unknown'},
+    }
+    assert self.merge(alpha, bravo) == {
+      'foo': bravo['foo'],
+      'bar': alpha['bar'],
+      'baz': bravo['baz'],
+    }
+
+    charlie = {
+      'baz': {'status': 'nudge', 'last_updated': arrow.get(1)},
+    }
+    assert self.merge(alpha, bravo, charlie) == {
+      'foo': bravo['foo'],
+      'bar': alpha['bar'],
+      'baz': charlie['baz'],
+    }