Refactor method attachment.py#get_unique_filename

Author: gayanW

launchable/commands/record/attachment.py

@@ -67,7 +67,7 @@ def attachment(
                             continue
 
                         file_name = normalize_filename(zip_info.filename)
-                        file_name = get_unique_filename(file_name)
+                        file_name = get_unique_filename(file_name, used_filenames)
                         status = post_attachment(
                             client, session, file_content, file_name)
                         summary_rows.append([file_name, status])
@@ -94,7 +94,7 @@ def attachment(
                             continue
 
                         file_name = normalize_filename(tar_info.name)
-                        file_name = get_unique_filename(file_name)
+                        file_name = get_unique_filename(file_name, used_filenames)
                         status = post_attachment(
                             client, session, file_content, file_name)
                         summary_rows.append([file_name, status])
@@ -109,7 +109,7 @@ def attachment(
                         continue
 
                     file_name = normalize_filename(a)
-                    file_name = get_unique_filename(file_name)
+                    file_name = get_unique_filename(file_name, used_filenames)
                     status = post_attachment(client, session, file_content, file_name)
                     summary_rows.append([file_name, status])
 
@@ -121,25 +121,37 @@ def attachment(
 
 def get_unique_filename(filepath: str, used_filenames: Set[str]) -> str:
     """
-    Get a unique filename by extracting the basename and appending .1, .2, etc. if needed.
-    Format: file.log, file.1.log, file.2.log
-    Adds the final name to used_filenames set.
+    Get a unique filename by extracting the basename and prepending parent folder if needed.
+    Strategy:
+    1. First occurrence: use basename (e.g., app.log)
+    2. Duplicate: prepend parent folder (e.g., nested-app.log)
+    3. Still duplicate: append .1, .2, etc. (e.g., nested-app.1.log)
     """
-    basename = os.path.basename(filepath)
+    filename = os.path.basename(filepath)
 
     # If basename is not used, return it
-    if basename not in used_filenames:
-        used_filenames.add(basename)
-        return basename
-
-    # Otherwise find the next available numbered version
-    name, ext = os.path.splitext(basename)
+    if filename not in used_filenames:
+        used_filenames.add(filename)
+        return filename
+
+    # Try prepending the parent directory name
+    parent_dir = os.path.basename(os.path.dirname(filepath))
+    if parent_dir:  # Has a parent directory
+        name, ext = os.path.splitext(filename)
+        filename = f"{parent_dir}-{name}{ext}"
+
+        if filename not in used_filenames:
+            used_filenames.add(filename)
+            return filename
+
+    # If still duplicate, append numbers
+    name, ext = os.path.splitext(filename)
     counter = 1
     while True:
-        new_name = f"{name}.{counter}{ext}"
-        if new_name not in used_filenames:
-            used_filenames.add(new_name)
-            return new_name
+        filename = f"{name}.{counter}{ext}"
+        if filename not in used_filenames:
+            used_filenames.add(filename)
+            return filename
         counter += 1
 
 

tests/commands/record/test_attachment.py

@@ -93,15 +93,15 @@ def test_attachment_zip_file(self):
                 responses.POST,
                 "{}/intake/organizations/{}/workspaces/{}/builds/{}/test_sessions/{}/attachment".format(
                     get_base_url(), self.organization, self.workspace, self.build_name, self.session_id),
-                match=[responses.matchers.header_matcher({"Content-Disposition": 'attachment;filename="nested/debug.log"'})],
+                match=[responses.matchers.header_matcher({"Content-Disposition": 'attachment;filename="debug.log"'})],
                 status=200)
 
             expect = """
-| File             | Status                           |
-|------------------|----------------------------------|
-| app.log          | ⚠ Failed to record               |
-| nested/debug.log | ✓ Recorded successfully          |
-| binary.dat       | ⚠ Skipped: not a valid text file |
+| File       | Status                           |
+|------------|----------------------------------|
+| app.log    | ⚠ Failed to record               |
+| debug.log  | ✓ Recorded successfully          |
+| binary.dat | ⚠ Skipped: not a valid text file |
 """
 
             result = self.cli("record", "attachment", "--session", self.session, zip_path)
@@ -150,15 +150,15 @@ def test_attachment_with_include_filter(self):
                 responses.POST,
                 "{}/intake/organizations/{}/workspaces/{}/builds/{}/test_sessions/{}/attachment".format(
                     get_base_url(), self.organization, self.workspace, self.build_name, self.session_id),
-                match=[responses.matchers.header_matcher({"Content-Disposition": 'attachment;filename="nested/debug.log"'})],
+                match=[responses.matchers.header_matcher({"Content-Disposition": 'attachment;filename="debug.log"'})],
                 status=200)
 
             result = self.cli("record", "attachment", "--session", self.session, "--include", "*.log", zip_path)
 
-            expect = """| File             | Status                  |
-|------------------|-------------------------|
-| app.log          | ✓ Recorded successfully |
-| nested/debug.log | ✓ Recorded successfully |
+            expect = """| File      | Status                  |
+|-----------|-------------------------|
+| app.log   | ✓ Recorded successfully |
+| debug.log | ✓ Recorded successfully |
 """
             self.assertEqual(expect, result.output)
 
@@ -196,15 +196,15 @@ def test_attachment_with_identical_file_names(self):
                 responses.POST,
                 "{}/intake/organizations/{}/workspaces/{}/builds/{}/test_sessions/{}/attachment".format(
                     get_base_url(), self.organization, self.workspace, self.build_name, self.session_id),
-                match=[responses.matchers.header_matcher({"Content-Disposition": 'attachment;filename="app.1.log"'})],
+                match=[responses.matchers.header_matcher({"Content-Disposition": 'attachment;filename="nested-app.log"'})],
                 status=200)
 
             result = self.cli("record", "attachment", "--session", self.session, zip_path)
 
             expect = """| File           | Status                  |
 |----------------|-------------------------|
 | app.log        | ✓ Recorded successfully |
-| nested/app.log | ✓ Recorded successfully |
+| nested-app.log | ✓ Recorded successfully |
 """
             self.assertEqual(expect, result.output)
 
@@ -228,7 +228,7 @@ def test_attachment_with_whitespace_in_filename(self):
                 "{}/intake/organizations/{}/workspaces/{}/builds/{}/test_sessions/{}/attachment".format(
                     get_base_url(), self.organization, self.workspace, self.build_name, self.session_id),
                 match=[responses.matchers.header_matcher(
-                    {"Content-Disposition": 'attachment;filename="{}"'.format(file_path.replace(' ', '-'))}
+                    {"Content-Disposition": 'attachment;filename="{}"'.format("app-log-file.txt")}
                 )],
                 status=200)