From 583fad78db103dc80c4c4d1ad4329f5f46f8b4b6 Mon Sep 17 00:00:00 2001
From: Jiri Podivin <jpodivin@redhat.com>
Date: Wed, 3 Mar 2021 16:20:53 +0100
Subject: [PATCH] Simplification of the log path handling by the ValidationLog
 class

Redundant checks were removed an branching reduced.
Previously de-facto function _get_content now behaves as
a method should and works with attributes.

Several string processsing snippets were factored out
and are now separate helper methods.

Added check for abs path.

Changed related test to provide absolute path.
Added new test case for absolute path checking.

Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Change-Id: I46df7b4befbef36a74b9138961650806c457fc13
---
 validations_libs/tests/test_validation_log.py |  9 +++-
 validations_libs/validation_logs.py           | 54 ++++++++++++-------
 2 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/validations_libs/tests/test_validation_log.py b/validations_libs/tests/test_validation_log.py
index 419e3f4f..4600c7c5 100644
--- a/validations_libs/tests/test_validation_log.py
+++ b/validations_libs/tests/test_validation_log.py
@@ -258,7 +258,14 @@ class TestValidationLog(TestCase):
         self.assertRaises(
             IOError,
             ValidationLog,
-            logfile='non-existing.yaml'
+            logfile='/tmp/fakelogs/non-existing.yaml'
+        )
+
+    def test_log_not_abs_path(self):
+        self.assertRaises(
+            ValueError,
+            ValidationLog,
+            logfile='fake.yaml'
         )
 
     @mock.patch('json.load')
diff --git a/validations_libs/validation_logs.py b/validations_libs/validation_logs.py
index 135d75b2..eeba03fe 100644
--- a/validations_libs/validation_logs.py
+++ b/validations_libs/validation_logs.py
@@ -41,34 +41,36 @@ class ValidationLog(object):
         :param log_path: The absolute path of the logs directory
         :type log_path: ``string``
         :param extension: The file extension (Default to 'json')
-        :type extension: ````
+        :type extension: ``string``
         """
         # Set properties
         self.uuid = uuid
         self.validation_id = validation_id
-        self.log_path = log_path
+        self.abs_log_path = log_path
         self.extension = extension
         self.content = {}
         self.name = None
         self.datetime = None
 
-        if not logfile and (not uuid or not validation_id):
+        # Get full path and content raise exception if it's impossible
+        if logfile:
+            if os.path.isabs(logfile):
+                self.abs_log_path = logfile
+            else:
+                raise ValueError(
+                'logfile must be absolute path, but is: {}'.format(logfile)
+            )
+        elif uuid and validation_id:
+            self.abs_log_path = self.get_log_path()
+        else:
             raise Exception(
                 'When not using logfile argument, the uuid and '
                 'validation_id have to be set'
             )
 
-        # Get full path and content
-        if logfile:
-            full_path = logfile
-        else:
-            if uuid and validation_id:
-                full_path = self.get_log_path()
-
-        if full_path:
-            self.content = self._get_content(full_path)
-            self.name = os.path.splitext(os.path.basename(full_path))[0]
-            self.datetime = self.name.rsplit('_', 1)[-1]
+        self.content = self._get_content()
+        self.name = self._get_name()
+        self.datetime = self._get_time()
 
         # if we have a log file then extract uuid, validation_id and timestamp
         if logfile:
@@ -79,24 +81,38 @@ class ValidationLog(object):
                 logging.warning('Wrong log file format, it should be formed '
                                 'such as {uuid}_{validation-id}_{timestamp}')
 
-    def _get_content(self, file):
+    def _get_content(self):
         try:
-            with open(file, 'r') as log_file:
+            with open(self.abs_log_path, 'r') as log_file:
                 return json.load(log_file)
         except IOError:
-            msg = "log file: {} not found".format(file)
+            msg = "log file: {} not found".format(self.abs_log_path)
             raise IOError(msg)
         except ValueError:
-            msg = "bad json format for {}".format(file)
+            msg = "bad json format for {}".format(self.abs_log_path)
             raise ValueError(msg)
 
     def get_log_path(self):
         """Return full path of a validation log"""
         # We return occurence 0, because it should be a uniq file name:
-        return glob.glob("{}/{}_{}_*.{}".format(self.log_path,
+        return glob.glob("{}/{}_{}_*.{}".format(self.abs_log_path,
                                                 self.uuid, self.validation_id,
                                                 self.extension))[0]
 
+    def _get_name(self):
+        """Return name of the log file under the self.full_path
+
+        :rtype: ``string``
+        """
+        return os.path.splitext(os.path.basename(self.abs_log_path))[0]
+
+    def _get_time(self):
+        """Return time component of the log file name
+
+        :rtype: ``string``
+        """
+        return self.name.rsplit('_', 1)[-1]
+
     def is_valid_format(self):
         """Return True if the log file is a valid validation format