refactor(UTILITIES): enforce srp on spec class

mac_maker/jobs/bases.py

@@ -7,12 +7,10 @@
 from ..ansible_controller.inventory import InventoryFile
 from ..ansible_controller.runner import AnsibleRunner
 from ..utilities.password import SUDO
-from ..utilities.spec import JobSpec
+from ..utilities.precheck import PrecheckExtractor, TypePrecheckFileData
+from ..utilities.spec import JobSpecExtractor
 from ..utilities.state import TypeState
-from ..utilities.validation.precheck import (
-  PrecheckConfigValidator,
-  TypePrecheckFileData,
-)
+from ..utilities.validation.precheck import PrecheckConfigValidator
 
 
 class SimpleJobBase(abc.ABC):
@@ -28,7 +26,8 @@ class ProvisionerJobBase(abc.ABC):
   """Job base class for the Mac Maker."""
 
   def __init__(self) -> None:
-    self.jobspec = JobSpec()
+    self.jobspec_extractor = JobSpecExtractor()
+    self.precheck_extractor = PrecheckExtractor()
 
   @abc.abstractmethod
   def get_precheck_content(self) -> TypePrecheckFileData:
@@ -52,6 +51,7 @@ def precheck(self) -> None:
       for violation in results['violations']:
         click.echo(violation)
       sys.exit(1)
+
     click.echo(precheck_data['notes'])
 
   def provision(self) -> None:

mac_maker/jobs/filesystem.py

@@ -1,9 +1,12 @@
 """A provisioning job for a spec file and profile on the local file system."""
 
+from typing import Optional, cast
+
 import click
 from .. import config
+from ..utilities.precheck import TypePrecheckFileData
+from ..utilities.spec import TypeSpecFileData
 from ..utilities.state import TypeState
-from ..utilities.validation.precheck import TypePrecheckFileData
 from . import bases
 
 
@@ -14,20 +17,31 @@ class FileSystemJob(bases.ProvisionerJobBase):
   """
 
   spec_file_location: str
+  job_spec_data: Optional[TypeSpecFileData]
 
   def __init__(self, spec_file_location: str):
     super().__init__()
     self.spec_file_location = spec_file_location
+    self.job_spec_data = None
+
+  def _extract_precheck_data(self) -> None:
+    """Extract Precheck data from a loaded Job Spec file."""
+    if not self.job_spec_data:
+      self.job_spec_data = self.jobspec_extractor.get_job_spec_data(
+          self.spec_file_location
+      )
 
   def get_precheck_content(self) -> TypePrecheckFileData:
     """Read the precheck data using a spec file.
 
     :returns: The precheck file data.
     """
 
-    precheck_data = self.jobspec.extract_precheck_from_job_spec(
-        self.spec_file_location
+    self._extract_precheck_data()
+    precheck_data = self.precheck_extractor.get_precheck_data(
+        cast(TypeSpecFileData, self.job_spec_data)
     )
+
     return precheck_data
 
   def get_state(self) -> TypeState:
@@ -36,9 +50,8 @@ def get_state(self) -> TypeState:
     :returns: The created state object.
     """
 
-    job_spec = self.jobspec.read_job_spec_from_filesystem(
-        self.spec_file_location
-    )
+    self._extract_precheck_data()
     click.echo(config.ANSIBLE_JOB_SPEC_READ_MESSAGE)
-    click.echo(job_spec['spec_file_location'])
-    return job_spec['spec_file_content']
+    job_spec_data = cast(TypeSpecFileData, self.job_spec_data)
+    click.echo(job_spec_data['spec_file_location'])
+    return job_spec_data['spec_file_content']

mac_maker/jobs/github.py

@@ -5,9 +5,9 @@
 import click
 from .. import config
 from ..utilities.github import GithubRepository
+from ..utilities.precheck import TypePrecheckFileData
 from ..utilities.spec import TypeSpecFileData
 from ..utilities.state import TypeState
-from ..utilities.validation.precheck import TypePrecheckFileData
 from ..utilities.workspace import WorkSpace
 from . import bases
 
@@ -42,8 +42,9 @@ def _download_repository(self) -> None:
     self.workspace = WorkSpace()
     self.workspace.add_repository(repo, self.branch_name)
     repo.download_zip_bundle_profile(self.workspace.root, self.branch_name)
-    self.loaded_spec_file_data = self.jobspec.read_job_spec_from_workspace(
-        self.workspace
+    self.workspace.add_spec_file()
+    self.loaded_spec_file_data = self.jobspec_extractor.get_job_spec_data(
+        str(self.workspace.spec_file)
     )
 
   def get_precheck_content(self) -> TypePrecheckFileData:
@@ -53,8 +54,8 @@ def get_precheck_content(self) -> TypePrecheckFileData:
     """
 
     self._download_repository()
-    precheck_data = self.jobspec.extract_precheck_from_job_spec(
-        str(self.loaded_spec_file_data['spec_file_location'])
+    precheck_data = self.precheck_extractor.get_precheck_data(
+        self.loaded_spec_file_data
     )
     return precheck_data
 

mac_maker/jobs/tests/test_bases.py

@@ -3,8 +3,8 @@
 from typing import cast
 from unittest import TestCase, mock
 
-from ...utilities import spec, state
-from ...utilities.validation import precheck
+from ...utilities import precheck, spec, state
+from ...utilities.validation import precheck as precheck_validation
 from .. import bases as bases_module
 
 BASES_MODULE = bases_module.__name__
@@ -15,10 +15,10 @@ class MockConcreteJob(bases_module.ProvisionerJobBase):
 
   def __init__(self) -> None:
     super().__init__()
-    self.mock_precheck_content = cast(spec.TypePrecheckFileData, {})
+    self.mock_precheck_content = cast(precheck.TypePrecheckFileData, {})
     self.mock_state = cast(state.TypeState, {})
 
-  def get_precheck_content(self) -> spec.TypePrecheckFileData:
+  def get_precheck_content(self) -> precheck.TypePrecheckFileData:
     return self.mock_precheck_content
 
   def get_state(self) -> state.TypeState:
@@ -33,8 +33,12 @@ def setUp(self) -> None:
 
   def test_init(self) -> None:
     self.assertIsInstance(
-        self.concrete_job.jobspec,
-        spec.JobSpec,
+        self.concrete_job.jobspec_extractor,
+        spec.JobSpecExtractor,
+    )
+    self.assertIsInstance(
+        self.concrete_job.precheck_extractor,
+        precheck.PrecheckExtractor,
     )
 
 
@@ -71,9 +75,11 @@ def test_precheck_config_invalid(
       m_env: mock.Mock,
   ) -> None:
     instance = m_env.return_value
-    instance.validate_config.side_effect = precheck.ValidationError("Boom!")
+    instance.validate_config.side_effect = precheck_validation.ValidationError(
+        "Boom!"
+    )
 
-    with self.assertRaises(precheck.ValidationError):
+    with self.assertRaises(precheck_validation.ValidationError):
       self.concrete_job.precheck()
 
   def test_precheck_environment(self, _: mock.Mock, m_env: mock.Mock) -> None:

mac_maker/jobs/tests/test_filesystem_job.py

@@ -20,43 +20,45 @@ def setUp(self) -> None:
     self.job = jobs_module.FileSystemJob(self.mock_path)
 
   def test_init(self) -> None:
-    self.assertIsInstance(
-        self.job.jobspec,
-        spec.JobSpec,
-    )
     self.assertEqual(self.job.spec_file_location, self.mock_path)
+    self.assertIsNone(self.job.job_spec_data)
 
 
-@mock.patch(JOBS_BASES + ".JobSpec.extract_precheck_from_job_spec")
+@mock.patch(JOBS_BASES + ".JobSpecExtractor.get_job_spec_data")
+@mock.patch(JOBS_BASES + ".PrecheckExtractor.get_precheck_data")
 class TestFileSystemJobGetPrecheck(TestCase):
   """Test the FileSystemJob class get_precheck_content method."""
 
   def setUp(self) -> None:
     self.mock_path = "/mock/path"
     self.job = jobs_module.FileSystemJob(self.mock_path)
+    self.mock_data1 = {
+        "a": "b"
+    }
+    self.mock_data2 = {
+        "b": "c"
+    }
 
   def test_get_precheck_content_return_value(
-      self, m_extract: mock.Mock
+      self, m_precheck: mock.Mock, m_job: mock.Mock
   ) -> None:
-    mock_data = {"a", "b"}
-    m_extract.return_value = mock_data
+    m_job.return_value = self.mock_data1
+    m_precheck.return_value = self.mock_data2
 
     results = self.job.get_precheck_content()
 
-    self.assertEqual(results, mock_data)
-
-  def test_get_precheck_content_call(self, m_extract: mock.Mock) -> None:
-
-    mock_data = {"a", "b"}
-    m_extract.return_value = mock_data
+    self.assertEqual(self.job.job_spec_data, self.mock_data1)
+    self.assertEqual(results, self.mock_data2)
 
+  def test_get_precheck_content_call(
+      self, _: mock.Mock, m_job: mock.Mock
+  ) -> None:
     self.job.get_precheck_content()
-
-    m_extract.assert_called_once_with(self.mock_path)
+    m_job.assert_called_once_with(self.mock_path)
 
 
 @mock.patch(JOBS_MODULE + ".click.echo")
-@mock.patch(JOBS_BASES + ".JobSpec.read_job_spec_from_filesystem")
+@mock.patch(JOBS_BASES + ".JobSpecExtractor.get_job_spec_data")
 class TestFileSystemJobGetStateCase(TestCase):
   """Test the FileSystemJob class get_state method."""
 
@@ -70,28 +72,34 @@ def setUp(self) -> None:
         }
     )
 
-  def test_get_state_return_value(
-      self, m_create: mock.Mock, _: mock.Mock
-  ) -> None:
-    m_create.return_value = self.mock_spec_content
+  def test_get_state_return_value(self, m_job: mock.Mock, _: mock.Mock) -> None:
+    m_job.return_value = self.mock_spec_content
 
     results = self.jobs.get_state()
 
     self.assertEqual(results, self.mock_spec_content['spec_file_content'])
 
-  def test_get_state_echo(self, m_create: mock.Mock, m_echo: mock.Mock) -> None:
-    m_create.return_value = self.mock_spec_content
+  def test_get_state_echo(self, m_job: mock.Mock, m_echo: mock.Mock) -> None:
+    m_job.return_value = self.mock_spec_content
 
     self.jobs.get_state()
 
     m_echo.assert_any_call(config.ANSIBLE_JOB_SPEC_READ_MESSAGE)
     m_echo.assert_any_call(self.mock_spec_content['spec_file_location'])
 
-  def test_get_state_call(self, m_create: mock.Mock, __: mock.Mock) -> None:
-    m_create.return_value = self.mock_spec_content
+  def test_get_state_call(self, m_job: mock.Mock, __: mock.Mock) -> None:
+    m_job.return_value = self.mock_spec_content
 
     self.jobs.get_state()
 
-    m_create.assert_called_once_with(
-        self.mock_spec_content['spec_file_location']
-    )
+    m_job.assert_called_once_with(self.mock_spec_content['spec_file_location'])
+
+  def test_get_state_spec_not_extracted_twice(
+      self, m_job: mock.Mock, __: mock.Mock
+  ) -> None:
+    m_job.return_value = self.mock_spec_content
+
+    self.jobs.get_state()
+    self.jobs.get_state()
+
+    m_job.assert_called_once_with(self.mock_spec_content['spec_file_location'])