From afe46aab24bf5b6c9aa34c19041a6aee0bfd4293 Mon Sep 17 00:00:00 2001 From: DanielPerkins7 Date: Tue, 1 Aug 2023 14:53:03 +0000 Subject: [PATCH 1/6] Check if item is a file when using directory queue This fixes issues with the item count being wrong and whole directories being passed on to the sender. --- ssm/message_directory.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ssm/message_directory.py b/ssm/message_directory.py index b7a70c65..667ea8e8 100644 --- a/ssm/message_directory.py +++ b/ssm/message_directory.py @@ -110,7 +110,10 @@ def _get_messages(self, sort_by_mtime=False): for (file_name, _mtime) in sorted(unsorted_messages, key=lambda tup: tup[1]): # Store the sorted file paths in a class element. - sorted_messages.append(file_name) + if os.path.isfile(file_name): + sorted_messages.append(file_name) + else: + continue # Return the sorted list. return sorted_messages From 3fcbb5413f5c3bba956fda6a316bd06c54bef8e1 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Mon, 9 Oct 2023 13:22:05 +0100 Subject: [PATCH 2/6] Revert "Check if item is a file when using directory queue" This reverts commit afe46aab24bf5b6c9aa34c19041a6aee0bfd4293 as the file filtering needs to be implmented earlier in the code. --- ssm/message_directory.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ssm/message_directory.py b/ssm/message_directory.py index 667ea8e8..b7a70c65 100644 --- a/ssm/message_directory.py +++ b/ssm/message_directory.py @@ -110,10 +110,7 @@ def _get_messages(self, sort_by_mtime=False): for (file_name, _mtime) in sorted(unsorted_messages, key=lambda tup: tup[1]): # Store the sorted file paths in a class element. - if os.path.isfile(file_name): - sorted_messages.append(file_name) - else: - continue + sorted_messages.append(file_name) # Return the sorted list. return sorted_messages From 132410a56ce25aba480bb9fb0b101ae4c08ec2a9 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Mon, 9 Oct 2023 13:54:30 +0100 Subject: [PATCH 3/6] Add unit test for dir in mess dir issue --- test/test_message_directory.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/test_message_directory.py b/test/test_message_directory.py index af78bdc7..9e2cdcb2 100644 --- a/test/test_message_directory.py +++ b/test/test_message_directory.py @@ -137,6 +137,15 @@ def test_remove(self): # Check the count method returns the expected value. self.assertEqual(self.message_directory.count(), 0) + def test_dir_in_dir(self): + """Check that directories inside the queue are being ignored.""" + self.longMessage = True # Include normal unittest output before custom message. + + with tempfile.TemporaryFile(dir=self.tmp_dir): + tempfile.mkdtemp(prefix='extra_directory_', dir=self.tmp_dir) + self.assertEqual(self.message_directory.count(), 1, "Expected just one file, " + "but greater result implies that directory is being counted.") + def tearDown(self): """Remove test directory and all contents.""" try: From bc56fd343dbd190dd16d51f6b61e6718346e0429 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Mon, 9 Oct 2023 13:55:47 +0100 Subject: [PATCH 4/6] Filter file list to exclude directories --- ssm/message_directory.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ssm/message_directory.py b/ssm/message_directory.py index b7a70c65..62dad40d 100644 --- a/ssm/message_directory.py +++ b/ssm/message_directory.py @@ -87,8 +87,9 @@ def _get_messages(self, sort_by_mtime=False): """ try: # Get a list of files under self.directory_path - # in an arbitrary order. - file_name_list = os.listdir(self.directory_path) + # in an arbitrary order (ignoring directories). + file_name_list = [file for file in os.listdir(self.directory_path) + if os.path.isfile(os.path.join(self.directory_path, file))] if sort_by_mtime: # Working space to hold the unsorted messages From 1defe699af08710b7e06cef31cf635a854f93457 Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Mon, 9 Oct 2023 13:57:04 +0100 Subject: [PATCH 5/6] Add underscore to temp dir name to make readable tempfile creates files and folders with a random name, but adds the prefix directly before it, so this makes it more readable by separating those parts. --- test/test_message_directory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_message_directory.py b/test/test_message_directory.py index 9e2cdcb2..fae85203 100644 --- a/test/test_message_directory.py +++ b/test/test_message_directory.py @@ -27,7 +27,7 @@ class TestMessageDirectory(unittest.TestCase): def setUp(self): """Create a MessageDirectory class on top of a temporary directory.""" - self.tmp_dir = tempfile.mkdtemp(prefix='message_directory') + self.tmp_dir = tempfile.mkdtemp(prefix='message_directory_') self.message_directory = MessageDirectory(self.tmp_dir) def test_add_and_get(self): From b8a08e63a473e0cf38c48f170fb3e6e98261725c Mon Sep 17 00:00:00 2001 From: Adrian Coveney Date: Mon, 9 Oct 2023 14:46:22 +0100 Subject: [PATCH 6/6] Rewrite test to work on Unix Previous version of test failed in Ubuntu Travis environment, possibly due to the file being kept open in the context manager. --- test/test_message_directory.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/test_message_directory.py b/test/test_message_directory.py index fae85203..2e3d8541 100644 --- a/test/test_message_directory.py +++ b/test/test_message_directory.py @@ -14,6 +14,7 @@ """This module contains test cases for the MessageDirectory class.""" from __future__ import print_function +import os import shutil import tempfile import time @@ -141,10 +142,14 @@ def test_dir_in_dir(self): """Check that directories inside the queue are being ignored.""" self.longMessage = True # Include normal unittest output before custom message. - with tempfile.TemporaryFile(dir=self.tmp_dir): - tempfile.mkdtemp(prefix='extra_directory_', dir=self.tmp_dir) - self.assertEqual(self.message_directory.count(), 1, "Expected just one file, " - "but greater result implies that directory is being counted.") + # Add a single test file (closing it to ensure this works on Unix) + handle, _path = tempfile.mkstemp(dir=self.tmp_dir) + os.close(handle) + # Add a directory (to ignore) + tempfile.mkdtemp(prefix='extra_directory_', dir=self.tmp_dir) + + self.assertEqual(self.message_directory.count(), 1, "Expected just one file, " + "but greater result implies that directory is being counted.") def tearDown(self): """Remove test directory and all contents."""