From 689ab78b3dfb94b374905ead0f77ab258346a8a1 Mon Sep 17 00:00:00 2001 From: Steffen Siebert Date: Wed, 29 Aug 2018 09:54:48 +0200 Subject: [PATCH] Invalidate cache if linter path or arguments change (#19) --- gitlint/linters.py | 5 ++-- gitlint/utils.py | 35 ++++++++++++++++++++++------ test/unittest/test_utils.py | 46 ++++++++++++++++++++++++------------- 3 files changed, 61 insertions(+), 25 deletions(-) diff --git a/gitlint/linters.py b/gitlint/linters.py index 7b1cae1..38041bf 100644 --- a/gitlint/linters.py +++ b/gitlint/linters.py @@ -73,7 +73,8 @@ def lint_command(name, program, arguments, filter_regex, filename, lines): Returns: dict: a dict with the extracted info from the message. """ - output = utils.get_output_from_cache(name, filename) + linter_hash = utils.calculate_hash(program, arguments) + output = utils.get_output_from_cache(name, linter_hash, filename) if output is None: call_arguments = [program] + arguments + [filename] @@ -91,7 +92,7 @@ def lint_command(name, program, arguments, filter_regex, filename, lines): } } output = output.decode('utf-8') - utils.save_output_in_cache(name, filename, output) + utils.save_output_in_cache(name, linter_hash, filename, output) output_lines = output.split(os.linesep) diff --git a/gitlint/utils.py b/gitlint/utils.py index 71c74f7..753f321 100644 --- a/gitlint/utils.py +++ b/gitlint/utils.py @@ -13,6 +13,7 @@ # limitations under the License. """Common function used across modules.""" +import hashlib import io import os import re @@ -72,16 +73,34 @@ def _open_for_write(filename): return io.open(filename, 'w') -def _get_cache_filename(name, filename): - """Returns the cache location for filename and linter name.""" +def _get_cache_filename(name, linter_hash, filename): + """Returns the cache location for filename and linter name and hash.""" filename = os.path.abspath(filename)[1:] + linter_dir = "%s.%s" % (name, linter_hash) home_folder = os.path.expanduser('~') base_cache_dir = os.path.join(home_folder, '.git-lint', 'cache') - return os.path.join(base_cache_dir, name, filename) + return os.path.join(base_cache_dir, linter_dir, filename) -def get_output_from_cache(name, filename): +def calculate_hash(program, arguments): + """Calculate sha256 hash as hex string over program and arguments. + + Args: + program: string: lint program. + arguments: list[string]: extra arguments for the program. + + Returns: string: a string with the calculated sha256 hex value. + """ + algorithm = hashlib.sha256() + algorithm.update(program) + for argument in arguments: + algorithm.update("|") + algorithm.update(argument) + return algorithm.hexdigest() + + +def get_output_from_cache(name, linter_hash, filename): """Returns the output from the cache if still valid. It checks that the cache file is defined and that its modification time is @@ -89,12 +108,13 @@ def get_output_from_cache(name, filename): Args: name: string: name of the linter. + linter_hash: string: hash representing linter binary and arguments. filename: string: path of the filename for which we are retrieving the output. Returns: a string with the output, if it is still valid, or None otherwise. """ - cache_filename = _get_cache_filename(name, filename) + cache_filename = _get_cache_filename(name, linter_hash, filename) if (os.path.exists(cache_filename) and os.path.getmtime(filename) < os.path.getmtime(cache_filename)): with io.open(cache_filename) as f: @@ -103,14 +123,15 @@ def get_output_from_cache(name, filename): return None -def save_output_in_cache(name, filename, output): +def save_output_in_cache(name, linter_hash, filename, output): """Saves output in the cache location. Args: name: string: name of the linter. + linter_hash: string: hash representing linter binary and arguments. filename: string: path of the filename for which we are saving the output. output: string: full output (not yet filetered) of the lint command. """ - cache_filename = _get_cache_filename(name, filename) + cache_filename = _get_cache_filename(name, linter_hash, filename) with _open_for_write(cache_filename) as f: f.write(output) diff --git a/test/unittest/test_utils.py b/test/unittest/test_utils.py index 4aa44cd..e6961a8 100644 --- a/test/unittest/test_utils.py +++ b/test/unittest/test_utils.py @@ -91,60 +91,74 @@ def test_open_for_write(self): def test_get_cache_filename(self): self.fs.create_dir('/abspath') os.chdir('/abspath') + dummy_hash = utils.calculate_hash("some_tool", []) with mock.patch('os.path.expanduser', return_value='/home/user'): self.assertEqual( - '/home/user/.git-lint/cache/linter1/abspath/bar/file.txt', - utils._get_cache_filename('linter1', 'bar/file.txt')) + '/home/user/.git-lint/cache/linter1.%s/abspath/bar/file.txt' % + dummy_hash, + utils._get_cache_filename('linter1', dummy_hash, + 'bar/file.txt')) self.assertEqual( - '/home/user/.git-lint/cache/linter2/abspath/file.txt', - utils._get_cache_filename('linter2', 'file.txt')) + '/home/user/.git-lint/cache/linter2.%s/abspath/file.txt' % + dummy_hash, + utils._get_cache_filename('linter2', dummy_hash, 'file.txt')) self.assertEqual( - '/home/user/.git-lint/cache/linter3/bar/file.txt', - utils._get_cache_filename('linter3', '/bar/file.txt')) + '/home/user/.git-lint/cache/linter3.%s/bar/file.txt' % + dummy_hash, + utils._get_cache_filename('linter3', dummy_hash, + '/bar/file.txt')) @unittest.skipUnless(sys.version_info >= (3, 5), 'pyfakefs does not support pathlib2. See' 'https://github.com/jmcgeheeiv/pyfakefs/issues/408') def test_save_output_in_cache(self): + dummy_hash = utils.calculate_hash("some_tool", []) output = 'Some content' with mock.patch( 'gitlint.utils._get_cache_filename', - return_value='/cache/filename.txt'): - utils.save_output_in_cache('linter', 'filename', output) + return_value='/cache/linter.%s/filename.txt' % dummy_hash): + utils.save_output_in_cache('linter', dummy_hash, 'filename', + output) - with open(utils._get_cache_filename('linter', 'filename')) as f: + with open( + utils._get_cache_filename('linter', dummy_hash, + 'filename')) as f: self.assertEqual(output, f.read()) def test_get_output_from_cache_no_cache(self): - cache_filename = '/cache/filename.txt' + dummy_hash = utils.calculate_hash("some_tool", []) + cache_filename = '/cache/linter.%s/filename.txt' % dummy_hash with mock.patch( 'gitlint.utils._get_cache_filename', return_value=cache_filename): self.assertIsNone( - utils.get_output_from_cache('linter', 'filename')) + utils.get_output_from_cache('linter', dummy_hash, 'filename')) def test_get_output_from_cache_cache_is_expired(self): - cache_filename = '/cache/filename.txt' + dummy_hash = utils.calculate_hash("some_tool", []) + cache_filename = '/cache/linter.%s/filename.txt' % dummy_hash self.fs.create_file(cache_filename) self.fs.create_file('filename') with mock.patch( 'gitlint.utils._get_cache_filename', return_value=cache_filename): self.assertIsNone( - utils.get_output_from_cache('linter', 'filename')) + utils.get_output_from_cache('linter', dummy_hash, 'filename')) def test_get_output_from_cache_cache_is_valid(self): - cache_filename = '/cache/filename.txt' + dummy_hash = utils.calculate_hash("some_tool", []) + cache_filename = '/cache/linter.%s/filename.txt' % dummy_hash content = 'some_content' self.fs.create_file('filename') self.fs.create_file(cache_filename, contents=content) with mock.patch( 'gitlint.utils._get_cache_filename', return_value=cache_filename): - self.assertEqual(content, - utils.get_output_from_cache('linter', 'filename')) + self.assertEqual( + content, + utils.get_output_from_cache('linter', dummy_hash, 'filename')) def test_which_absolute_path(self): filename = '/foo/bar.sh'