From 0d1b86cabccb0c545ef51efe704cd683df61afe9 Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Fri, 20 Sep 2024 13:46:37 -0400
Subject: [PATCH 01/11] Split Copier into Walker and Copier steps

---
 src/ansible_creator/subcommands/init.py |  15 +-
 src/ansible_creator/utils.py            | 320 ++++++++++++++++--------
 tests/units/test_utils.py               |  15 +-
 3 files changed, 242 insertions(+), 108 deletions(-)

diff --git a/src/ansible_creator/subcommands/init.py b/src/ansible_creator/subcommands/init.py
index 093cdecb..8bb128af 100644
--- a/src/ansible_creator/subcommands/init.py
+++ b/src/ansible_creator/subcommands/init.py
@@ -11,7 +11,7 @@
 from ansible_creator.exceptions import CreatorError
 from ansible_creator.templar import Templar
 from ansible_creator.types import TemplateData
-from ansible_creator.utils import Copier
+from ansible_creator.utils import Copier, Walker
 
 
 if TYPE_CHECKING:
@@ -125,14 +125,21 @@ def _scaffold(self) -> None:
             dev_file_name=self.unique_name_in_devfile(),
         )
 
-        copier = Copier(
-            resources=[f"{self._project}_project", *self.common_resources],
+        walker = Walker(
+            resources=(f"{self._project}_project", *self.common_resources),
             resource_id=f"{self._project}_project",
             dest=self._init_path,
             output=self.output,
             templar=self._templar,
             template_data=template_data,
         )
-        copier.copy_containers()
+        paths = walker.collect_paths()
+
+        copier = Copier(
+            output=self.output,
+            templar=self._templar,
+            template_data=template_data,
+        )
+        copier.copy_containers(paths)
 
         self.output.note(f"{self._project} project created at {self._init_path}")
diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index 1d5d6773..84160275 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -16,7 +16,6 @@
 
 
 if TYPE_CHECKING:
-
     from ansible_creator.compat import Traversable
     from ansible_creator.output import Output
     from ansible_creator.templar import Templar
@@ -65,8 +64,97 @@ def expand_path(path: str) -> Path:
 
 
 @dataclass
-class Copier:
-    """Configuration for the Copier class.
+class DestinationFile:
+    """Container to hold information about a file to be copied.
+
+    Attributes:
+        destination_path: The path the file will be written to.
+        original_path: The path of the original copy.
+    """
+
+    destination_path: Path
+    original_path: Traversable
+
+    def __str__(self) -> str:
+        """Supports str() on DestinationFile.
+
+        Returns:
+            A string representation of the destination path.
+        """
+        return str(self.destination_path)
+
+    @property
+    def conflicts(self) -> bool:
+        """Check for file conflicts.
+
+        Returns:
+            True if a conflict exists on the destination else False.
+        """
+        if not self.exists():
+            return False
+
+        return not (self.dest_is_dir() and self.is_dir())
+
+    @property
+    def needs_templating(self) -> bool:
+        """Check if templating is required.
+
+        Returns:
+            True if the file needs to be templated else False.
+        """
+        return self.original_path.name.endswith(".j2")
+
+    def exists(self) -> bool:
+        """Check if a file exists at the destination.
+
+        Returns:
+            True if the destination path exists else False.
+        """
+        return self.destination_path.exists()
+
+    def dest_is_dir(self) -> bool:
+        """Check if the destination path is an existing directory.
+
+        Returns:
+            True if the destination path exists and is a directory.
+        """
+        return self.destination_path.exists() and self.destination_path.is_dir()
+
+    def dest_is_file(self) -> bool:
+        """Check if the destination path is an existing file.
+
+        Returns:
+            True if the destination path exists and is a file.
+        """
+        return self.destination_path.exists() and self.destination_path.is_file()
+
+    def is_dir(self) -> bool:
+        """Check if the source path is a directory.
+
+        Returns:
+            True if the source path is a directory.
+        """
+        return self.original_path.is_dir()
+
+    def is_file(self) -> bool:
+        """Check if the source path is a file.
+
+        Returns:
+            True if the source path is a file.
+        """
+        return self.original_path.is_file()
+
+    def remove_existing(self) -> None:
+        """Remove existing files or directories at destination path."""
+        if self.dest_is_file():
+            self.destination_path.unlink()
+        elif self.dest_is_dir():
+            self.destination_path.rmdir()
+
+
+@dataclass
+class Walker:
+    """Configuration for the Walker class.
 
     Attributes:
         resources: List of resource containers to copy.
@@ -74,144 +162,118 @@ class Copier:
         dest: The destination path to copy resources to.
         output: An instance of the Output class.
         template_data: A dictionary containing the original data to render templates with.
-        index: Index of the current resource being copied.
         resource_root: Root path for the resources.
         templar: An instance of the Templar class.
     """
 
-    resources: list[str]
+    resources: tuple[str, ...]
     resource_id: str
     dest: Path
     output: Output
     template_data: TemplateData
-    index: int = 0
     resource_root: str = "ansible_creator.resources"
     templar: Templar | None = None
 
-    @property
-    def resource(self: Copier) -> str:
-        """Return the current resource being copied."""
-        return self.resources[self.index]
-
-    def _recursive_copy(
-        self: Copier,
+    def _recursive_walk(
+        self,
         root: Traversable,
+        resource: str,
         template_data: TemplateData,
-    ) -> None:
-        """Recursively traverses a resource container and copies content to destination.
+    ) -> list[DestinationFile]:
+        """Recursively traverses a resource container looking for content to copy.
 
         Args:
             root: A traversable object representing root of the container to copy.
+            resource: The resource being scanned.
             template_data: A dictionary containing current data to render templates with.
+
+        Returns:
+            A list of paths to be written to.
         """
         self.output.debug(msg=f"current root set to {root}")
 
+        file_list = []
         for obj in root.iterdir():
-            self.each_obj(obj, template_data)
+            file_list.extend(
+                self.each_obj(
+                    obj,
+                    resource=resource,
+                    template_data=template_data,
+                ),
+            )
+        return file_list
 
-    def each_obj(self, obj: Traversable, template_data: TemplateData) -> None:
+    def each_obj(
+        self,
+        obj: Traversable,
+        resource: str,
+        template_data: TemplateData,
+    ) -> list[DestinationFile]:
         """Recursively traverses a resource container and copies content to destination.
 
         Args:
             obj: A traversable object representing the root of the container to copy.
+            resource: The resource to consult for path names.
             template_data: A dictionary containing current data to render templates with.
+
+        Returns:
+            A list of paths.
         """
         # resource names may have a . but directories use / in the path
         dest_name = str(obj).split(
-            self.resource.replace(".", "/") + "/",
+            resource.replace(".", "/") + "/",
             maxsplit=1,
         )[-1]
-        dest_path = self.dest / dest_name
-
         # replace placeholders in destination path with real values
         for key, val in PATH_REPLACERS.items():
-            if key in str(dest_path) and template_data:
-                str_dest_path = str(dest_path)
+            if key in dest_name:
                 repl_val = getattr(template_data, val)
-                dest_path = Path(str_dest_path.replace(key, repl_val))
-
-        if obj.is_dir():
-            if obj.name in SKIP_DIRS:
-                return
-            self._recursive_copy_dir(obj=obj, dest_path=dest_path, template_data=template_data)
-
-        elif obj.is_file():
-            if obj.name.split(".")[-1] in SKIP_FILES_TYPES or obj.name == "__meta__.yml":
-                return
-            self._copy_file(
-                obj=obj,
-                dest_path=dest_path,
-                template_data=template_data,
-            )
+                dest_name = dest_name.replace(key, repl_val)
+        dest_name = dest_name.removesuffix(".j2")
 
-    def _copy_file(
-        self,
-        obj: Traversable,
-        dest_path: Path,
-        template_data: TemplateData,
-    ) -> None:
-        """Copy a file to destination.
+        dest_path = DestinationFile(self.dest / dest_name, obj)
+        self.output.debug(f"Working on {dest_path}")
 
-        Args:
-            obj: A traversable object representing the file to copy.
-            dest_path: The destination path to copy the file to.
-            template_data: A dictionary containing current data to render templates with.
-        """
-        # remove .j2 suffix at destination
-        needs_templating = False
-        if dest_path.suffix == ".j2":
-            dest_path = dest_path.with_suffix("")
-            needs_templating = True
-        dest_file = Path(self.dest) / dest_path
-        self.output.debug(msg=f"dest file is {dest_file}")
-
-        content = obj.read_text(encoding="utf-8")
-        # only render as templates if both of these are provided,
-        # and original file suffix was j2
-        if self.templar and template_data and needs_templating:
-            content = self.templar.render_from_content(
-                template=content,
-                data=template_data,
-            )
-        with dest_file.open("w", encoding="utf-8") as df_handle:
-            df_handle.write(content)
+        if dest_path.conflicts:
+            if dest_path.dest_is_dir():
+                self.output.warning(f"{dest_path} already exists and is a directory!")
+            elif dest_path.is_dir():
+                self.output.warning(f"{dest_path} already exists and is a file!")
+            else:
+                self.output.warning(f"{dest_path} will be overwritten!")
 
-    def _recursive_copy_dir(
-        self,
-        obj: Traversable,
-        dest_path: Path,
-        template_data: TemplateData,
-    ) -> None:
-        """Recursively copy directories to destination.
+        if dest_path.is_dir() and obj.name not in SKIP_DIRS:
+            return [
+                dest_path,
+                *self._recursive_walk(root=obj, resource=resource, template_data=template_data),
+            ]
 
-        Args:
-            obj: A traversable object representing the directory to copy.
-            dest_path: The destination path to copy the directory to.
-            template_data: A dictionary containing current data to render templates with.
-        """
-        dest_path.mkdir(parents=True, exist_ok=True)
+        if (
+            obj.is_file()
+            and obj.name.split(".")[-1] not in SKIP_FILES_TYPES
+            and obj.name != "__meta__.yml"
+        ):
+            return [dest_path]
 
-        # recursively copy the directory
-        self._recursive_copy(
-            root=obj,
-            template_data=template_data,
-        )
+        return []
 
-    def _per_container(self: Copier) -> None:
-        """Copy files and directories from a possibly nested source to a destination.
+    def _per_container(self, resource: str) -> list[DestinationFile]:
+        """Generate a list of all paths that will be written to for a particular resource.
 
-        :param copier_config: Configuration for the Copier class.
+        Args:
+            resource: The resource to search through.
 
-        :raises CreatorError: if allow_overwrite is not a list.
+        Returns:
+            A list of paths to be written to.
         """
-        msg = f"starting recursive copy with source container '{self.resource}'"
+        msg = f"starting recursive walk with source container '{resource}'"
         self.output.debug(msg)
 
         # Cast the template data to not pollute the original
         template_data = copy.deepcopy(self.template_data)
 
         # Collect and template any resource specific variables
-        meta_file = impl_resources.files(f"{self.resource_root}.{self.resource}") / "__meta__.yml"
+        meta_file = impl_resources.files(f"{self.resource_root}.{resource}") / "__meta__.yml"
         try:
             with meta_file.open("r", encoding="utf-8") as meta_fileh:
                 self.output.debug(
@@ -235,18 +297,76 @@ def _per_container(self: Copier) -> None:
             else:
                 setattr(template_data, key, value["value"])
 
-        self._recursive_copy(
-            root=impl_resources.files(f"{self.resource_root}.{self.resource}"),
-            template_data=template_data,
+        return self._recursive_walk(
+            impl_resources.files(f"{self.resource_root}.{resource}"),
+            resource,
+            template_data,
         )
 
-    def copy_containers(
-        self: Copier,
+    def collect_paths(self) -> list[DestinationFile]:
+        """Determine paths that will be written to.
+
+        Returns:
+            A list of paths to be written to.
+        """
+        file_list = []
+        for resource in self.resources:
+            file_list.extend(self._per_container(resource))
+
+        return file_list
+
+
+@dataclass
+class Copier:
+    """Configuration for the Copier class.
+
+    Attributes:
+        output: An instance of the Output class.
+        template_data: A dictionary containing the original data to render templates with.
+        templar: An instance of the Templar class.
+    """
+
+    output: Output
+    template_data: TemplateData
+    templar: Templar | None = None
+
+    def _copy_file(
+        self,
+        dest_path: DestinationFile,
+        template_data: TemplateData,
     ) -> None:
+        """Copy a file to destination.
+
+        Args:
+            dest_path: The destination path to copy the file to.
+            template_data: A dictionary containing current data to render templates with.
+        """
+        # remove .j2 suffix at destination
+        self.output.debug(msg=f"dest file is {dest_path}")
+
+        content = dest_path.original_path.read_text(encoding="utf-8")
+        # only render as templates if both of these are provided,
+        # and original file suffix was j2
+        if self.templar and template_data and dest_path.needs_templating:
+            content = self.templar.render_from_content(
+                template=content,
+                data=template_data,
+            )
+        with dest_path.destination_path.open("w", encoding="utf-8") as df_handle:
+            df_handle.write(content)
+
+    def copy_containers(self: Copier, paths: list[DestinationFile]) -> None:
         """Copy multiple containers to destination.
 
-        :param copier_config: Configuration for the Copier class.
+        Args:
+            paths: A list of paths to create in the destination.
         """
-        for i in range(len(self.resources)):
-            self.index = i
-            self._per_container()
+        for path in paths:
+            if path.conflicts:
+                path.remove_existing()
+
+            if path.is_dir():
+                path.destination_path.mkdir(parents=True, exist_ok=True)
+
+            elif path.is_file():
+                self._copy_file(path, self.template_data)
diff --git a/tests/units/test_utils.py b/tests/units/test_utils.py
index 786b7b2b..486f00f0 100644
--- a/tests/units/test_utils.py
+++ b/tests/units/test_utils.py
@@ -6,7 +6,7 @@
 from typing import TYPE_CHECKING
 
 from ansible_creator.types import TemplateData
-from ansible_creator.utils import Copier, expand_path
+from ansible_creator.utils import Copier, Walker, expand_path
 
 
 if TYPE_CHECKING:
@@ -35,13 +35,20 @@ def test_skip_dirs(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, output: Outp
         output: Output class object.
     """
     monkeypatch.setattr("ansible_creator.utils.SKIP_DIRS", ["docker"])
-    copier = Copier(
-        resources=["common.devcontainer"],
+
+    walker = Walker(
+        resources=("common.devcontainer",),
         resource_id="common.devcontainer",
         dest=tmp_path,
         output=output,
         template_data=TemplateData(),
     )
-    copier.copy_containers()
+    paths = walker.collect_paths()
+
+    copier = Copier(
+        output=output,
+        template_data=TemplateData(),
+    )
+    copier.copy_containers(paths)
     assert (tmp_path / ".devcontainer" / "podman").exists()
     assert not (tmp_path / ".devcontainer" / "docker").exists()

From 736784dc0ac38ca1f100251e6105b4f4a029d30e Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Fri, 20 Sep 2024 15:34:18 -0400
Subject: [PATCH 02/11] Strip a lot back from DestinationFile

---
 src/ansible_creator/utils.py | 82 +++++++++---------------------------
 1 file changed, 21 insertions(+), 61 deletions(-)

diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index 84160275..8d882ef6 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -68,12 +68,12 @@ class DestinationFile:
     """Container to hold information about a file to be copied.
 
     Attributes:
-        destination_path: The path the file will be written to.
-        original_path: The path of the original copy.
+        source: The path of the original copy.
+        dest: The path the file will be written to.
     """
 
-    destination_path: Path
-    original_path: Traversable
+    source: Traversable
+    dest: Path
 
     def __str__(self) -> str:
         """Supports str() on DestinationFile.
@@ -81,7 +81,7 @@ def __str__(self) -> str:
         Returns:
             A string representation of the destination path.
         """
-        return str(self.destination_path)
+        return str(self.dest)
 
     @property
     def conflicts(self) -> bool:
@@ -90,10 +90,10 @@ def conflicts(self) -> bool:
         Returns:
             True if a conflict exists on the destination else False.
         """
-        if not self.exists():
+        if not self.dest.exists():
             return False
 
-        return not (self.dest_is_dir() and self.is_dir())
+        return not (self.dest.is_dir() and self.source.is_dir())
 
     @property
     def needs_templating(self) -> bool:
@@ -102,54 +102,14 @@ def needs_templating(self) -> bool:
         Returns:
             True if the file needs to be templated else False.
         """
-        return self.original_path.name.endswith(".j2")
-
-    def exists(self) -> bool:
-        """Check if a file exists at the destination.
-
-        Returns:
-            True if the destination path exists else False.
-        """
-        return self.destination_path.exists()
-
-    def dest_is_dir(self) -> bool:
-        """Check if the destination path is an existing directory.
-
-        Returns:
-            True if the destination path exists and is a directory.
-        """
-        return self.destination_path.exists() and self.destination_path.is_dir()
-
-    def dest_is_file(self) -> bool:
-        """Check if the destination path is an existing file.
-
-        Returns:
-            True if the destination path exists and is a file.
-        """
-        return self.destination_path.exists() and self.destination_path.is_file()
-
-    def is_dir(self) -> bool:
-        """Check if the source path is a directory.
-
-        Returns:
-            True if the source path is a directory.
-        """
-        return self.original_path.is_dir()
-
-    def is_file(self) -> bool:
-        """Check if the source path is a file.
-
-        Returns:
-            True if the source path is a file.
-        """
-        return self.original_path.is_file()
+        return self.source.name.endswith(".j2")
 
     def remove_existing(self) -> None:
         """Remove existing files or directories at destination path."""
-        if self.dest_is_file():
-            self.destination_path.unlink()
-        elif self.dest_is_dir():
-            self.destination_path.rmdir()
+        if self.dest.is_file():
+            self.dest.unlink()
+        elif self.dest.is_dir():
+            self.dest.rmdir()
 
 
 @dataclass
@@ -231,18 +191,18 @@ def each_obj(
                 dest_name = dest_name.replace(key, repl_val)
         dest_name = dest_name.removesuffix(".j2")
 
-        dest_path = DestinationFile(self.dest / dest_name, obj)
+        dest_path = DestinationFile(dest=self.dest / dest_name, source=obj)
         self.output.debug(f"Working on {dest_path}")
 
         if dest_path.conflicts:
-            if dest_path.dest_is_dir():
+            if dest_path.dest.is_dir():
                 self.output.warning(f"{dest_path} already exists and is a directory!")
-            elif dest_path.is_dir():
+            elif obj.is_dir():
                 self.output.warning(f"{dest_path} already exists and is a file!")
             else:
                 self.output.warning(f"{dest_path} will be overwritten!")
 
-        if dest_path.is_dir() and obj.name not in SKIP_DIRS:
+        if obj.is_dir() and obj.name not in SKIP_DIRS:
             return [
                 dest_path,
                 *self._recursive_walk(root=obj, resource=resource, template_data=template_data),
@@ -344,7 +304,7 @@ def _copy_file(
         # remove .j2 suffix at destination
         self.output.debug(msg=f"dest file is {dest_path}")
 
-        content = dest_path.original_path.read_text(encoding="utf-8")
+        content = dest_path.source.read_text(encoding="utf-8")
         # only render as templates if both of these are provided,
         # and original file suffix was j2
         if self.templar and template_data and dest_path.needs_templating:
@@ -352,7 +312,7 @@ def _copy_file(
                 template=content,
                 data=template_data,
             )
-        with dest_path.destination_path.open("w", encoding="utf-8") as df_handle:
+        with dest_path.dest.open("w", encoding="utf-8") as df_handle:
             df_handle.write(content)
 
     def copy_containers(self: Copier, paths: list[DestinationFile]) -> None:
@@ -365,8 +325,8 @@ def copy_containers(self: Copier, paths: list[DestinationFile]) -> None:
             if path.conflicts:
                 path.remove_existing()
 
-            if path.is_dir():
-                path.destination_path.mkdir(parents=True, exist_ok=True)
+            if path.source.is_dir():
+                path.dest.mkdir(parents=True, exist_ok=True)
 
-            elif path.is_file():
+            elif path.source.is_file():
                 self._copy_file(path, self.template_data)

From 49eb3e7ff2beab105827214e5905b0b66c0759cd Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Mon, 23 Sep 2024 10:07:59 -0400
Subject: [PATCH 03/11] Carry (potentially modified) template_data with src and
 dest

---
 src/ansible_creator/subcommands/init.py |  1 -
 src/ansible_creator/utils.py            | 18 ++++++++++--------
 tests/units/test_utils.py               |  1 -
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/ansible_creator/subcommands/init.py b/src/ansible_creator/subcommands/init.py
index 8bb128af..2fd07c54 100644
--- a/src/ansible_creator/subcommands/init.py
+++ b/src/ansible_creator/subcommands/init.py
@@ -138,7 +138,6 @@ def _scaffold(self) -> None:
         copier = Copier(
             output=self.output,
             templar=self._templar,
-            template_data=template_data,
         )
         copier.copy_containers(paths)
 
diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index 8d882ef6..145a2330 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -70,10 +70,12 @@ class DestinationFile:
     Attributes:
         source: The path of the original copy.
         dest: The path the file will be written to.
+        template_data: A dictionary containing the customized data to render templates with.
     """
 
     source: Traversable
     dest: Path
+    template_data: TemplateData
 
     def __str__(self) -> str:
         """Supports str() on DestinationFile.
@@ -191,7 +193,11 @@ def each_obj(
                 dest_name = dest_name.replace(key, repl_val)
         dest_name = dest_name.removesuffix(".j2")
 
-        dest_path = DestinationFile(dest=self.dest / dest_name, source=obj)
+        dest_path = DestinationFile(
+            dest=self.dest / dest_name,
+            source=obj,
+            template_data=template_data,
+        )
         self.output.debug(f"Working on {dest_path}")
 
         if dest_path.conflicts:
@@ -282,24 +288,20 @@ class Copier:
 
     Attributes:
         output: An instance of the Output class.
-        template_data: A dictionary containing the original data to render templates with.
         templar: An instance of the Templar class.
     """
 
     output: Output
-    template_data: TemplateData
     templar: Templar | None = None
 
     def _copy_file(
         self,
         dest_path: DestinationFile,
-        template_data: TemplateData,
     ) -> None:
         """Copy a file to destination.
 
         Args:
             dest_path: The destination path to copy the file to.
-            template_data: A dictionary containing current data to render templates with.
         """
         # remove .j2 suffix at destination
         self.output.debug(msg=f"dest file is {dest_path}")
@@ -307,10 +309,10 @@ def _copy_file(
         content = dest_path.source.read_text(encoding="utf-8")
         # only render as templates if both of these are provided,
         # and original file suffix was j2
-        if self.templar and template_data and dest_path.needs_templating:
+        if self.templar and dest_path.template_data and dest_path.needs_templating:
             content = self.templar.render_from_content(
                 template=content,
-                data=template_data,
+                data=dest_path.template_data,
             )
         with dest_path.dest.open("w", encoding="utf-8") as df_handle:
             df_handle.write(content)
@@ -329,4 +331,4 @@ def copy_containers(self: Copier, paths: list[DestinationFile]) -> None:
                 path.dest.mkdir(parents=True, exist_ok=True)
 
             elif path.source.is_file():
-                self._copy_file(path, self.template_data)
+                self._copy_file(path)
diff --git a/tests/units/test_utils.py b/tests/units/test_utils.py
index 486f00f0..2f5b8959 100644
--- a/tests/units/test_utils.py
+++ b/tests/units/test_utils.py
@@ -47,7 +47,6 @@ def test_skip_dirs(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, output: Outp
 
     copier = Copier(
         output=output,
-        template_data=TemplateData(),
     )
     copier.copy_containers(paths)
     assert (tmp_path / ".devcontainer" / "podman").exists()

From 95ba126036c5865ae4fd523c5aad9cfa7486fa6a Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Mon, 23 Sep 2024 11:33:32 -0400
Subject: [PATCH 04/11] Clear up per-file debug lines

---
 src/ansible_creator/utils.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index 145a2330..dd354141 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -198,7 +198,7 @@ def each_obj(
             source=obj,
             template_data=template_data,
         )
-        self.output.debug(f"Working on {dest_path}")
+        self.output.debug(f"Looking at {dest_path}")
 
         if dest_path.conflicts:
             if dest_path.dest.is_dir():
@@ -304,7 +304,7 @@ def _copy_file(
             dest_path: The destination path to copy the file to.
         """
         # remove .j2 suffix at destination
-        self.output.debug(msg=f"dest file is {dest_path}")
+        self.output.debug(msg=f"Writing to {dest_path}")
 
         content = dest_path.source.read_text(encoding="utf-8")
         # only render as templates if both of these are provided,

From e9af343c0c839fb7596469daab1e72b865cd1803 Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Mon, 23 Sep 2024 12:09:14 -0400
Subject: [PATCH 05/11] Add test for Copier overwriting files

---
 src/ansible_creator/utils.py |  3 ++-
 tests/units/test_utils.py    | 43 ++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index dd354141..ee318f0d 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -4,6 +4,7 @@
 
 import copy
 import os
+import shutil
 
 from dataclasses import dataclass
 from importlib import resources as impl_resources
@@ -111,7 +112,7 @@ def remove_existing(self) -> None:
         if self.dest.is_file():
             self.dest.unlink()
         elif self.dest.is_dir():
-            self.dest.rmdir()
+            shutil.rmtree(self.dest)
 
 
 @dataclass
diff --git a/tests/units/test_utils.py b/tests/units/test_utils.py
index 2f5b8959..19a325ad 100644
--- a/tests/units/test_utils.py
+++ b/tests/units/test_utils.py
@@ -2,6 +2,8 @@
 
 from __future__ import annotations
 
+import shutil
+
 from pathlib import Path
 from typing import TYPE_CHECKING
 
@@ -51,3 +53,44 @@ def test_skip_dirs(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, output: Outp
     copier.copy_containers(paths)
     assert (tmp_path / ".devcontainer" / "podman").exists()
     assert not (tmp_path / ".devcontainer" / "docker").exists()
+
+
+def test_overwrite(tmp_path: Path, output: Output) -> None:
+    """Test Copier overwriting existing files.
+
+    Args:
+        tmp_path: Temporary directory path.
+        output: Output class object.
+    """
+    walker = Walker(
+        resources=("common.devcontainer",),
+        resource_id="common.devcontainer",
+        dest=tmp_path,
+        output=output,
+        template_data=TemplateData(),
+    )
+    paths = walker.collect_paths()
+
+    copier = Copier(
+        output=output,
+    )
+    copier.copy_containers(paths)
+
+    # Replace podman with a file
+    podman = tmp_path / ".devcontainer" / "podman"
+    shutil.rmtree(podman)
+    podman.write_text("This is an error")
+    # Replace docker devcontainer with a directory
+    docker = tmp_path / ".devcontainer" / "docker" / "devcontainer.json"
+    docker.unlink()
+    docker.mkdir()
+
+    # Re-walk directory to generate warnings, but not make changes
+    paths = walker.collect_paths()
+    assert podman.is_file()
+    assert docker.is_dir()
+
+    # Re-copy to overwrite structure
+    copier.copy_containers(paths)
+    assert podman.is_dir()
+    assert docker.is_file()

From 42d5409750a993da4396249cbbcdd976a3acf0ca Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Mon, 23 Sep 2024 12:25:34 -0400
Subject: [PATCH 06/11] Be a little more explicit about what those test paths
 should be

---
 tests/units/test_utils.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/units/test_utils.py b/tests/units/test_utils.py
index 19a325ad..9162fc14 100644
--- a/tests/units/test_utils.py
+++ b/tests/units/test_utils.py
@@ -77,20 +77,20 @@ def test_overwrite(tmp_path: Path, output: Output) -> None:
     copier.copy_containers(paths)
 
     # Replace podman with a file
-    podman = tmp_path / ".devcontainer" / "podman"
-    shutil.rmtree(podman)
-    podman.write_text("This is an error")
+    podman_dir = tmp_path / ".devcontainer" / "podman"
+    shutil.rmtree(podman_dir)
+    podman_dir.write_text("This is an error")
     # Replace docker devcontainer with a directory
-    docker = tmp_path / ".devcontainer" / "docker" / "devcontainer.json"
-    docker.unlink()
-    docker.mkdir()
+    docker_file = tmp_path / ".devcontainer" / "docker" / "devcontainer.json"
+    docker_file.unlink()
+    docker_file.mkdir()
 
     # Re-walk directory to generate warnings, but not make changes
     paths = walker.collect_paths()
-    assert podman.is_file()
-    assert docker.is_dir()
+    assert podman_dir.is_file()
+    assert docker_file.is_dir()
 
     # Re-copy to overwrite structure
     copier.copy_containers(paths)
-    assert podman.is_dir()
-    assert docker.is_file()
+    assert podman_dir.is_dir()
+    assert docker_file.is_file()

From 33d790bf64c86d1ed03a1bc8a1b2686310f84df5 Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Wed, 25 Sep 2024 14:56:05 -0400
Subject: [PATCH 07/11] Also take templating away from Copier's
 responsibilities

---
 src/ansible_creator/subcommands/init.py |  1 -
 src/ansible_creator/utils.py            | 90 +++++++++++++++----------
 tests/units/test_utils.py               | 14 +++-
 3 files changed, 67 insertions(+), 38 deletions(-)

diff --git a/src/ansible_creator/subcommands/init.py b/src/ansible_creator/subcommands/init.py
index 2fd07c54..066634fd 100644
--- a/src/ansible_creator/subcommands/init.py
+++ b/src/ansible_creator/subcommands/init.py
@@ -137,7 +137,6 @@ def _scaffold(self) -> None:
 
         copier = Copier(
             output=self.output,
-            templar=self._templar,
         )
         copier.copy_containers(paths)
 
diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index ee318f0d..bdbf3a46 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -71,12 +71,12 @@ class DestinationFile:
     Attributes:
         source: The path of the original copy.
         dest: The path the file will be written to.
-        template_data: A dictionary containing the customized data to render templates with.
+        content: The templated content to be written to dest.
     """
 
     source: Traversable
     dest: Path
-    template_data: TemplateData
+    content: str = ""
 
     def __str__(self) -> str:
         """Supports str() on DestinationFile.
@@ -87,25 +87,62 @@ def __str__(self) -> str:
         return str(self.dest)
 
     @property
-    def conflicts(self) -> bool:
+    def conflict(self) -> str:
         """Check for file conflicts.
 
         Returns:
-            True if a conflict exists on the destination else False.
+            String describing the file conflict, if any.
         """
         if not self.dest.exists():
-            return False
+            return ""
+
+        if self.source.is_file():
+            if self.dest.is_file():
+                dest_content = self.dest.read_text("utf8")
+                if self.content != dest_content:
+                    return f"{self.dest} will be overwritten!"
+            else:
+                return f"{self.dest} already exists and is a directory!"
 
-        return not (self.dest.is_dir() and self.source.is_dir())
+        if self.source.is_dir() and not self.dest.is_dir():
+            return f"{self.dest} already exists and is a file!"
+
+        return ""
 
     @property
-    def needs_templating(self) -> bool:
-        """Check if templating is required.
+    def needs_write(self) -> bool:
+        """Check if file needs to be written to.
 
         Returns:
-            True if the file needs to be templated else False.
+            True if dest differs from source else False.
+        """
+        # Skip files in SKIP_FILES_TYPES and __meta__.yaml
+        if self.source.is_file() and (
+            self.source.name.split(".")[-1] in SKIP_FILES_TYPES
+            or self.source.name == "__meta__.yml"
+        ):
+            return False
+
+        if not self.dest.exists():
+            return True
+        return bool(self.conflict)
+
+    def set_content(self, template_data: TemplateData, templar: Templar | None) -> None:
+        """Set expected content from source file, templated by templar if necessary.
+
+        Args:
+            template_data: A dictionary containing current data to render templates with.
+            templar: An instance of the Templar class.
         """
-        return self.source.name.endswith(".j2")
+        content = self.source.read_text(encoding="utf-8")
+        # only render as templates if both of these are provided,
+        # and original file suffix was j2
+        if templar and template_data and self.source.name.endswith("j2"):
+            content = templar.render_from_content(
+                template=content,
+                data=template_data,
+            )
+        self.content = content
 
     def remove_existing(self) -> None:
         """Remove existing files or directories at destination path."""
@@ -197,29 +234,22 @@ def each_obj(
         dest_path = DestinationFile(
             dest=self.dest / dest_name,
             source=obj,
-            template_data=template_data,
         )
         self.output.debug(f"Looking at {dest_path}")
 
-        if dest_path.conflicts:
-            if dest_path.dest.is_dir():
-                self.output.warning(f"{dest_path} already exists and is a directory!")
-            elif obj.is_dir():
-                self.output.warning(f"{dest_path} already exists and is a file!")
-            else:
-                self.output.warning(f"{dest_path} will be overwritten!")
+        if obj.is_file():
+            dest_path.set_content(template_data, self.templar)
 
+        conflict_msg = dest_path.conflict
+        if dest_path.needs_write and conflict_msg:
+            self.output.warning(conflict_msg)
         if obj.is_dir() and obj.name not in SKIP_DIRS:
             return [
                 dest_path,
                 *self._recursive_walk(root=obj, resource=resource, template_data=template_data),
             ]
 
-        if (
-            obj.is_file()
-            and obj.name.split(".")[-1] not in SKIP_FILES_TYPES
-            and obj.name != "__meta__.yml"
-        ):
+        if obj.is_file():
             return [dest_path]
 
         return []
@@ -289,11 +319,9 @@ class Copier:
 
     Attributes:
         output: An instance of the Output class.
-        templar: An instance of the Templar class.
     """
 
     output: Output
-    templar: Templar | None = None
 
     def _copy_file(
         self,
@@ -307,16 +335,8 @@ def _copy_file(
         # remove .j2 suffix at destination
         self.output.debug(msg=f"Writing to {dest_path}")
 
-        content = dest_path.source.read_text(encoding="utf-8")
-        # only render as templates if both of these are provided,
-        # and original file suffix was j2
-        if self.templar and dest_path.template_data and dest_path.needs_templating:
-            content = self.templar.render_from_content(
-                template=content,
-                data=dest_path.template_data,
-            )
         with dest_path.dest.open("w", encoding="utf-8") as df_handle:
-            df_handle.write(content)
+            df_handle.write(dest_path.content)
 
     def copy_containers(self: Copier, paths: list[DestinationFile]) -> None:
         """Copy multiple containers to destination.
@@ -325,7 +345,7 @@ def copy_containers(self: Copier, paths: list[DestinationFile]) -> None:
             paths: A list of paths to create in the destination.
         """
         for path in paths:
-            if path.conflicts:
+            if path.conflict:
                 path.remove_existing()
 
             if path.source.is_dir():
diff --git a/tests/units/test_utils.py b/tests/units/test_utils.py
index 9162fc14..80709393 100644
--- a/tests/units/test_utils.py
+++ b/tests/units/test_utils.py
@@ -71,26 +71,36 @@ def test_overwrite(tmp_path: Path, output: Output) -> None:
     )
     paths = walker.collect_paths()
 
+    # We will be manipulating these paths later
+    base_file = tmp_path / ".devcontainer" / "devcontainer.json"
+    podman_dir = tmp_path / ".devcontainer" / "podman"
+    docker_file = tmp_path / ".devcontainer" / "docker" / "devcontainer.json"
+
     copier = Copier(
         output=output,
     )
     copier.copy_containers(paths)
+    base_contents = base_file.read_text()
+    assert podman_dir.is_dir()
+    assert docker_file.is_file()
 
+    # Rewrite devcontainer.json
+    base_file.write_text("This is not what a devcontainer file looks like.")
     # Replace podman with a file
-    podman_dir = tmp_path / ".devcontainer" / "podman"
     shutil.rmtree(podman_dir)
     podman_dir.write_text("This is an error")
     # Replace docker devcontainer with a directory
-    docker_file = tmp_path / ".devcontainer" / "docker" / "devcontainer.json"
     docker_file.unlink()
     docker_file.mkdir()
 
     # Re-walk directory to generate warnings, but not make changes
     paths = walker.collect_paths()
+    assert base_file.read_text() != base_contents
     assert podman_dir.is_file()
     assert docker_file.is_dir()
 
     # Re-copy to overwrite structure
     copier.copy_containers(paths)
+    assert base_file.read_text() == base_contents
     assert podman_dir.is_dir()
     assert docker_file.is_file()

From e63832a026f8a82099fa0e4bc69041cc8ed9a2c0 Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Wed, 25 Sep 2024 15:06:10 -0400
Subject: [PATCH 08/11] Walker now only returns path that need changes to be
 applied.

---
 src/ansible_creator/utils.py | 28 ++++++++++++++++------------
 tests/units/test_utils.py    | 27 +++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index bdbf3a46..9367d571 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -240,17 +240,22 @@ def each_obj(
         if obj.is_file():
             dest_path.set_content(template_data, self.templar)
 
-        conflict_msg = dest_path.conflict
-        if dest_path.needs_write and conflict_msg:
-            self.output.warning(conflict_msg)
-        if obj.is_dir() and obj.name not in SKIP_DIRS:
-            return [
-                dest_path,
-                *self._recursive_walk(root=obj, resource=resource, template_data=template_data),
-            ]
+        if dest_path.needs_write:
+            # Warn on conflict
+            conflict_msg = dest_path.conflict
+            if conflict_msg:
+                self.output.warning(conflict_msg)
+
+            if obj.is_dir() and obj.name not in SKIP_DIRS:
+                return [
+                    dest_path,
+                    *self._recursive_walk(root=obj, resource=resource, template_data=template_data),
+                ]
+            if obj.is_file():
+                return [dest_path]
 
-        if obj.is_file():
-            return [dest_path]
+        if obj.is_dir() and obj.name not in SKIP_DIRS:
+            return self._recursive_walk(root=obj, resource=resource, template_data=template_data)
 
         return []
 
@@ -345,8 +350,7 @@ def copy_containers(self: Copier, paths: list[DestinationFile]) -> None:
             paths: A list of paths to create in the destination.
         """
         for path in paths:
-            if path.conflict:
-                path.remove_existing()
+            path.remove_existing()
 
             if path.source.is_dir():
                 path.dest.mkdir(parents=True, exist_ok=True)
diff --git a/tests/units/test_utils.py b/tests/units/test_utils.py
index 80709393..1502af10 100644
--- a/tests/units/test_utils.py
+++ b/tests/units/test_utils.py
@@ -104,3 +104,30 @@ def test_overwrite(tmp_path: Path, output: Output) -> None:
     assert base_file.read_text() == base_contents
     assert podman_dir.is_dir()
     assert docker_file.is_file()
+
+
+def test_skip_repeats(tmp_path: Path, output: Output) -> None:
+    """Test Copier skipping existing files.
+
+    Args:
+        tmp_path: Temporary directory path.
+        output: Output class object.
+    """
+    walker = Walker(
+        resources=("common.devcontainer",),
+        resource_id="common.devcontainer",
+        dest=tmp_path,
+        output=output,
+        template_data=TemplateData(),
+    )
+    paths = walker.collect_paths()
+    assert paths
+
+    copier = Copier(
+        output=output,
+    )
+    copier.copy_containers(paths)
+
+    # Re-walk directory to generate new path list
+    paths = walker.collect_paths()
+    assert not paths

From 01c949b58ed46c8c416e4199fa1e786738bdf2b6 Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Fri, 27 Sep 2024 11:41:46 -0400
Subject: [PATCH 09/11] Cache DestinationFile properties and provide container
 class

---
 src/ansible_creator/utils.py | 49 +++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 15 deletions(-)

diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index 9367d571..e8db59e1 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -7,6 +7,7 @@
 import shutil
 
 from dataclasses import dataclass
+from functools import cached_property
 from importlib import resources as impl_resources
 from pathlib import Path
 from typing import TYPE_CHECKING
@@ -86,7 +87,7 @@ def __str__(self) -> str:
         """
         return str(self.dest)
 
-    @property
+    @cached_property
     def conflict(self) -> str:
         """Check for file conflicts.
 
@@ -109,7 +110,7 @@ def conflict(self) -> str:
 
         return ""
 
-    @property
+    @cached_property
     def needs_write(self) -> bool:
         """Check if file needs to be written to.
 
@@ -152,6 +153,18 @@ def remove_existing(self) -> None:
             shutil.rmtree(self.dest)
 
 
+class FileList(list[DestinationFile]):
+    """list subclass holding DestinationFiles with convenience methods."""
+
+    def has_conflicts(self) -> bool:
+        """Check if any files have conflicts in the destination.
+
+        Returns:
+            True if there are any conflicts else False.
+        """
+        return any(path.conflict for path in self)
+
+
 @dataclass
 class Walker:
     """Configuration for the Walker class.
@@ -179,7 +192,7 @@ def _recursive_walk(
         root: Traversable,
         resource: str,
         template_data: TemplateData,
-    ) -> list[DestinationFile]:
+    ) -> FileList:
         """Recursively traverses a resource container looking for content to copy.
 
         Args:
@@ -192,7 +205,7 @@ def _recursive_walk(
         """
         self.output.debug(msg=f"current root set to {root}")
 
-        file_list = []
+        file_list = FileList()
         for obj in root.iterdir():
             file_list.extend(
                 self.each_obj(
@@ -208,7 +221,7 @@ def each_obj(
         obj: Traversable,
         resource: str,
         template_data: TemplateData,
-    ) -> list[DestinationFile]:
+    ) -> FileList:
         """Recursively traverses a resource container and copies content to destination.
 
         Args:
@@ -247,19 +260,25 @@ def each_obj(
                 self.output.warning(conflict_msg)
 
             if obj.is_dir() and obj.name not in SKIP_DIRS:
-                return [
-                    dest_path,
-                    *self._recursive_walk(root=obj, resource=resource, template_data=template_data),
-                ]
+                return FileList(
+                    [
+                        dest_path,
+                        *self._recursive_walk(
+                            root=obj,
+                            resource=resource,
+                            template_data=template_data,
+                        ),
+                    ],
+                )
             if obj.is_file():
-                return [dest_path]
+                return FileList([dest_path])
 
         if obj.is_dir() and obj.name not in SKIP_DIRS:
             return self._recursive_walk(root=obj, resource=resource, template_data=template_data)
 
-        return []
+        return FileList()
 
-    def _per_container(self, resource: str) -> list[DestinationFile]:
+    def _per_container(self, resource: str) -> FileList:
         """Generate a list of all paths that will be written to for a particular resource.
 
         Args:
@@ -305,13 +324,13 @@ def _per_container(self, resource: str) -> list[DestinationFile]:
             template_data,
         )
 
-    def collect_paths(self) -> list[DestinationFile]:
+    def collect_paths(self) -> FileList:
         """Determine paths that will be written to.
 
         Returns:
             A list of paths to be written to.
         """
-        file_list = []
+        file_list = FileList()
         for resource in self.resources:
             file_list.extend(self._per_container(resource))
 
@@ -343,7 +362,7 @@ def _copy_file(
         with dest_path.dest.open("w", encoding="utf-8") as df_handle:
             df_handle.write(dest_path.content)
 
-    def copy_containers(self: Copier, paths: list[DestinationFile]) -> None:
+    def copy_containers(self: Copier, paths: FileList) -> None:
         """Copy multiple containers to destination.
 
         Args:

From c2450bc6e3c65d2edb6b6532bfc2542e9709f983 Mon Sep 17 00:00:00 2001
From: Kate Case <kcase@redhat.com>
Date: Fri, 27 Sep 2024 11:46:24 -0400
Subject: [PATCH 10/11] Test has_conflicts in FileList

---
 tests/units/test_utils.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/units/test_utils.py b/tests/units/test_utils.py
index 1502af10..d639b83e 100644
--- a/tests/units/test_utils.py
+++ b/tests/units/test_utils.py
@@ -98,6 +98,7 @@ def test_overwrite(tmp_path: Path, output: Output) -> None:
     assert base_file.read_text() != base_contents
     assert podman_dir.is_file()
     assert docker_file.is_dir()
+    assert paths.has_conflicts()
 
     # Re-copy to overwrite structure
     copier.copy_containers(paths)

From 7a02542f0a65a08c994b3627872d19a7611be64a Mon Sep 17 00:00:00 2001
From: Kate Case <this.is@katherineca.se>
Date: Fri, 27 Sep 2024 11:48:43 -0400
Subject: [PATCH 11/11] Update src/ansible_creator/utils.py

Co-authored-by: Bradley A. Thornton <bthornto@redhat.com>
---
 src/ansible_creator/utils.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ansible_creator/utils.py b/src/ansible_creator/utils.py
index e8db59e1..7ce4017f 100644
--- a/src/ansible_creator/utils.py
+++ b/src/ansible_creator/utils.py
@@ -154,7 +154,7 @@ def remove_existing(self) -> None:
 
 
 class FileList(list[DestinationFile]):
-    """list subclass holding DestinationFiles with convenience methods."""
+    """A list subclass holding DestinationFiles with convenience methods."""
 
     def has_conflicts(self) -> bool:
         """Check if any files have conflicts in the destination.