From 1bd07bd15b1bf238e186b23b7d112e9fe4637737 Mon Sep 17 00:00:00 2001
From: Petr Kalis <petr.kalis@gmail.com>
Date: Mon, 25 Sep 2023 18:01:24 +0200
Subject: [PATCH] OP-6874 - remove trailing underscore in subset name (#5647)

If {layer} placeholder is at the end of subset name template and not used (for example in auto_image where separating it by layer doesn't make any sense) trailing '_' was kept.
This updates cleaning logic and extracts it as it might be similar in regular `image` instance.
---
 openpype/hosts/photoshop/lib.py               | 17 +++++++++++++
 .../plugins/create/create_flatten_image.py    | 14 ++---------
 .../photoshop/plugins/create/create_image.py  |  3 ++-
 .../unit/openpype/hosts/photoshop/test_lib.py | 25 +++++++++++++++++++
 4 files changed, 46 insertions(+), 13 deletions(-)
 create mode 100644 tests/unit/openpype/hosts/photoshop/test_lib.py

diff --git a/openpype/hosts/photoshop/lib.py b/openpype/hosts/photoshop/lib.py
index ae7a33b7b6..9f603a70d2 100644
--- a/openpype/hosts/photoshop/lib.py
+++ b/openpype/hosts/photoshop/lib.py
@@ -1,5 +1,8 @@
+import re
+
 import openpype.hosts.photoshop.api as api
 from openpype.client import get_asset_by_name
+from openpype.lib import prepare_template_data
 from openpype.pipeline import (
     AutoCreator,
     CreatedInstance
@@ -78,3 +81,17 @@ class PSAutoCreator(AutoCreator):
             existing_instance["asset"] = asset_name
             existing_instance["task"] = task_name
             existing_instance["subset"] = subset_name
+
+
+def clean_subset_name(subset_name):
+    """Clean all variants leftover {layer} from subset name."""
+    dynamic_data = prepare_template_data({"layer": "{layer}"})
+    for value in dynamic_data.values():
+        if value in subset_name:
+            subset_name = (subset_name.replace(value, "")
+                                      .replace("__", "_")
+                                      .replace("..", "."))
+    # clean trailing separator as Main_
+    pattern = r'[\W_]+$'
+    replacement = ''
+    return re.sub(pattern, replacement, subset_name)
diff --git a/openpype/hosts/photoshop/plugins/create/create_flatten_image.py b/openpype/hosts/photoshop/plugins/create/create_flatten_image.py
index e4229788bd..afde77fdb4 100644
--- a/openpype/hosts/photoshop/plugins/create/create_flatten_image.py
+++ b/openpype/hosts/photoshop/plugins/create/create_flatten_image.py
@@ -2,7 +2,7 @@ from openpype.pipeline import CreatedInstance
 
 from openpype.lib import BoolDef
 import openpype.hosts.photoshop.api as api
-from openpype.hosts.photoshop.lib import PSAutoCreator
+from openpype.hosts.photoshop.lib import PSAutoCreator, clean_subset_name
 from openpype.pipeline.create import get_subset_name
 from openpype.lib import prepare_template_data
 from openpype.client import get_asset_by_name
@@ -129,14 +129,4 @@ class AutoImageCreator(PSAutoCreator):
             self.family, variant, task_name, asset_doc,
             project_name, host_name, dynamic_data=dynamic_data
         )
-        return self._clean_subset_name(subset_name)
-
-    def _clean_subset_name(self, subset_name):
-        """Clean all variants leftover {layer} from subset name."""
-        dynamic_data = prepare_template_data({"layer": "{layer}"})
-        for value in dynamic_data.values():
-            if value in subset_name:
-                return (subset_name.replace(value, "")
-                        .replace("__", "_")
-                        .replace("..", "."))
-        return subset_name
+        return clean_subset_name(subset_name)
diff --git a/openpype/hosts/photoshop/plugins/create/create_image.py b/openpype/hosts/photoshop/plugins/create/create_image.py
index af20d456e0..4f2e90886a 100644
--- a/openpype/hosts/photoshop/plugins/create/create_image.py
+++ b/openpype/hosts/photoshop/plugins/create/create_image.py
@@ -10,6 +10,7 @@ from openpype.pipeline import (
 from openpype.lib import prepare_template_data
 from openpype.pipeline.create import SUBSET_NAME_ALLOWED_SYMBOLS
 from openpype.hosts.photoshop.api.pipeline import cache_and_get_instances
+from openpype.hosts.photoshop.lib import clean_subset_name
 
 
 class ImageCreator(Creator):
@@ -88,6 +89,7 @@ class ImageCreator(Creator):
 
             layer_fill = prepare_template_data({"layer": layer_name})
             subset_name = subset_name.format(**layer_fill)
+            subset_name = clean_subset_name(subset_name)
 
             if group.long_name:
                 for directory in group.long_name[::-1]:
@@ -184,7 +186,6 @@ class ImageCreator(Creator):
         self.mark_for_review = plugin_settings["mark_for_review"]
         self.enabled = plugin_settings["enabled"]
 
-
     def get_detail_description(self):
         return """Creator for Image instances
 
diff --git a/tests/unit/openpype/hosts/photoshop/test_lib.py b/tests/unit/openpype/hosts/photoshop/test_lib.py
new file mode 100644
index 0000000000..ad4feb42ae
--- /dev/null
+++ b/tests/unit/openpype/hosts/photoshop/test_lib.py
@@ -0,0 +1,25 @@
+import pytest
+
+from openpype.hosts.photoshop.lib import clean_subset_name
+
+"""
+Tests cleanup of unused layer placeholder ({layer}) from subset name.
+Layer differentiation might be desired in subset name, but in some cases it
+might be used (in `auto_image` - only single image without layer diff.,
+single image instance created without toggled use of subset name etc.)
+"""
+
+
+def test_no_layer_placeholder():
+    clean_subset = clean_subset_name("imageMain")
+    assert "imageMain" == clean_subset
+
+
+@pytest.mark.parametrize("subset_name",
+                         ["imageMain{Layer}",
+                          "imageMain_{layer}",  # trailing _
+                          "image{Layer}Main",
+                          "image{LAYER}Main"])
+def test_not_used_layer_placeholder(subset_name):
+    clean_subset = clean_subset_name(subset_name)
+    assert "imageMain" == clean_subset