From 39545295025ab5f1194ff8ec60101c270a45d814 Mon Sep 17 00:00:00 2001 From: Eu Date: Wed, 8 Jun 2016 16:36:39 -0300 Subject: [PATCH] Fix naive fileserver map diff algorithm Naively comparing sorted dict keys does not guarantee that maps are equal. We must compare mtimes for filenames in case keys are the same to make sure there isnt a modified file. --- salt/fileserver/__init__.py | 8 ++++++-- tests/unit/fileserver/__init__.py | 0 tests/unit/fileserver/map.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 tests/unit/fileserver/__init__.py create mode 100644 tests/unit/fileserver/map.py diff --git a/salt/fileserver/__init__.py b/salt/fileserver/__init__.py index c89abe87c9..7997b4479f 100644 --- a/salt/fileserver/__init__.py +++ b/salt/fileserver/__init__.py @@ -198,11 +198,15 @@ def diff_mtime_map(map1, map2): ''' # check if the mtimes are the same if sorted(map1) != sorted(map2): - #log.debug('diff_mtime_map: the maps are different') return True + # map1 and map2 are guaranteed to have same keys, + # so compare mtimes + for filename, mtime in map1.iteritems(): + if map2[filename] != mtime: + return True + # we made it, that means we have no changes - #log.debug('diff_mtime_map: the maps are the same') return False diff --git a/tests/unit/fileserver/__init__.py b/tests/unit/fileserver/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/unit/fileserver/map.py b/tests/unit/fileserver/map.py new file mode 100644 index 0000000000..577864712f --- /dev/null +++ b/tests/unit/fileserver/map.py @@ -0,0 +1,30 @@ +# -*- coding: utf-8 -*- +''' + :codeauthor: :email:`Joao Mesquita ` +''' + +# Import pytohn libs +from __future__ import absolute_import + +# Import Salt Testing libs +from salttesting import TestCase + +from salt import fileserver + + +class MapDiffTestCase(TestCase): + def test_diff_with_diffent_keys(self): + ''' + Test that different maps are indeed reported different + ''' + map1 = {'file1': 1234} + map2 = {'file2': 1234} + assert fileserver.diff_mtime_map(map1, map2) is True + + def test_diff_with_diffent_values(self): + ''' + Test that different maps are indeed reported different + ''' + map1 = {'file1': 12345} + map2 = {'file1': 1234} + assert fileserver.diff_mtime_map(map1, map2) is True