Make the Bringer better testable

org_fedora_oscap/content_discovery.py

@@ -7,7 +7,7 @@
 from typing import List
 
 from pyanaconda.core import constants
-from pyanaconda.threading import threadMgr
+from pyanaconda.threading import threadMgr, AnacondaThread
 from pykickstart.errors import KickstartValueError
 
 from org_fedora_oscap import data_fetch, utils
@@ -31,7 +31,6 @@ def paths_are_equivalent(p1, p2):
 
 
 def path_is_present_among_paths(path, paths):
-    absolute_path = os.path.abspath(path)
     for second_path in paths:
         if paths_are_equivalent(path, second_path):
             return True
@@ -43,6 +42,7 @@ class ContentBringer:
 
     def __init__(self, what_if_fail):
         self._valid_content_uri = ""
+        self.ca_certs_path = ""
         self.dest_file_name = ""
 
         self.activity_lock = threading.Lock()
@@ -84,25 +84,27 @@ def fetch_content(self, content_uri, ca_certs_path=""):
         """
         try:
             self.content_uri = content_uri
+            self.ca_certs_path = ca_certs_path
         except Exception as exc:
             self.what_if_fail(exc)
         shutil.rmtree(self.CONTENT_DOWNLOAD_LOCATION, ignore_errors=True)
         self.CONTENT_DOWNLOAD_LOCATION.mkdir(parents=True, exist_ok=True)
-        fetching_thread_name = self._fetch_files(ca_certs_path)
+        fetching_thread_name = self._fetch_files()
         return fetching_thread_name
 
-    def _fetch_files(self, ca_certs_path):
+    def _fetch_files(self):
         with self.activity_lock:
             if self.now_fetching_or_processing:
-                msg = "OSCAP Addon: Strange, it seems that we are already " \
-                    "fetching something."
-                log.warn(msg)
+                msg = _(
+                    f"Attempting to fetch '{self.content_uri}, "
+                    "but the previous fetch is still in progress")
+                log.warn(f"OSCAP Addon: {msg}")
                 return
             self.now_fetching_or_processing = True
 
         fetching_thread_name = None
         try:
-            fetching_thread_name = self._start_actual_fetch(ca_certs_path)
+            fetching_thread_name = self._start_actual_fetch()
         except Exception as exc:
             with self.activity_lock:
                 self.now_fetching_or_processing = False
@@ -111,24 +113,31 @@ def _fetch_files(self, ca_certs_path):
         # We are not finished yet with the fetch
         return fetching_thread_name
 
-    def _start_actual_fetch(self, ca_certs_path):
-        fetching_thread_name = None
+    def _start_actual_fetch(self):
+        fetching_thread_name = common.THREAD_FETCH_DATA
 
         scheme = self.content_uri.split("://")[0]
         if is_network(scheme):
-            fetching_thread_name = data_fetch.wait_and_fetch_net_data(
-                self.content_uri,
-                self.dest_file_name,
-                ca_certs_path
-            )
-        else:  # invalid schemes are handled down the road
-            fetching_thread_name = data_fetch.fetch_local_data(
-                self.content_uri,
-                self.dest_file_name,
-            )
+            try:
+                data_fetch.wait_for_network()
+            except common.OSCAPaddonNetworkError as exc:
+                msg = _(f"Network connection needed to fetch data. {exc}")
+                raise common.OSCAPaddonNetworkError(msg)
+
+        fetch_data_thread = AnacondaThread(
+            name=fetching_thread_name,
+            target=self.fetch_operation,
+            args=(self.dest_file_name,),
+            fatal=False)
+
+        threadMgr.add(fetch_data_thread)
+
         return fetching_thread_name
 
-    def finish_content_fetch(self, fetching_thread_name, fingerprint):
+    def fetch_operation(self, out_file):
+        return data_fetch.fetch_data(self.content_uri, out_file, self.ca_certs_path)
+
+    def finish_content_fetch(self, fetching_thread_name, fingerprint=""):
         try:
             self._finish_actual_fetch(fetching_thread_name)
             if fingerprint:

org_fedora_oscap/data_fetch.py

@@ -75,57 +75,19 @@ class FetchError(DataFetchError):
     pass
 
 
-def fetch_local_data(url, out_file):
-    """
-    Function that fetches data locally.
-
-    :see: org_fedora_oscap.data_fetch.fetch_data
-    :return: the name of the thread running fetch_data
-    :rtype: str
-
-    """
-    fetch_data_thread = AnacondaThread(name=common.THREAD_FETCH_DATA,
-                                       target=fetch_data,
-                                       args=(url, out_file, None),
-                                       fatal=False)
-
-    # register and run the thread
-    threadMgr.add(fetch_data_thread)
-
-    return common.THREAD_FETCH_DATA
-
-
-def wait_and_fetch_net_data(url, out_file, ca_certs_path=None):
-    """
-    Function that waits for network connection and starts a thread that fetches
-    data over network.
-
-    :see: org_fedora_oscap.data_fetch.fetch_data
-    :return: the name of the thread running fetch_data
-    :rtype: str
-
-    """
-
+def wait_for_network(timeout=None):
     # get thread that tries to establish a network connection
     nm_conn_thread = threadMgr.get(constants.THREAD_WAIT_FOR_CONNECTING_NM)
     if nm_conn_thread:
         # NM still connecting, wait for it to finish
-        nm_conn_thread.join()
+        nm_conn_thread.join(timeout)
 
     network_proxy = NETWORK.get_proxy()
     if not network_proxy.Connected:
-        raise common.OSCAPaddonNetworkError(_("Network connection needed to fetch data."))
-
-    log.info(f"Fetching data from {url}")
-    fetch_data_thread = AnacondaThread(name=common.THREAD_FETCH_DATA,
-                                       target=fetch_data,
-                                       args=(url, out_file, ca_certs_path),
-                                       fatal=False)
-
-    # register and run the thread
-    threadMgr.add(fetch_data_thread)
-
-    return common.THREAD_FETCH_DATA
+        msg = _("Failed to connect.")
+        if timeout is not None:
+            msg += " " + _(f"Reached the timeout of {timeout} s.")
+        raise common.OSCAPaddonNetworkError(msg)
 
 
 def can_fetch_from(url):

tests/test_content_discovery.py

@@ -1,9 +1,15 @@
 import os
+import time
+import pathlib
+import hashlib
 
 import pytest
 
 import org_fedora_oscap.content_discovery as tested_module
 from org_fedora_oscap import content_handling
+from org_fedora_oscap import utils
+
+import test_data_fetch
 
 
 @pytest.fixture
@@ -68,3 +74,28 @@ def test_path_presence_detection():
 
     for path in list_of_paths_not_in_list:
         assert not tested_module.path_is_present_among_paths(path, list_of_paths)
+
+
+class SlowBringer(tested_module.ContentBringer):
+    def fetch_operation(self, out_file):
+        time.sleep(1)
+        super().fetch_operation(out_file)
+
+
+def if_problem_raise_exception(exc):
+    raise(exc)
+
+
+def test_bringer_blocks_double_download_and_finishes_the_first(tmp_path):
+    source_path = pathlib.Path(__file__).absolute()
+    source_fingerprint = utils.get_file_fingerprint(str(source_path), hashlib.sha512())
+
+    dest_path = tmp_path / "dest"
+    uri = f"file://{source_path}"
+
+    bringer = SlowBringer(if_problem_raise_exception)
+    thread_name = bringer.fetch_content(uri)
+    second_thread_name = bringer.fetch_content(uri)
+    assert second_thread_name is None
+
+    bringer.finish_content_fetch(thread_name, source_fingerprint)