Zarr rename: detect conflict between group and array within the same …

…group
OSGeo · May 8, 2023 · a127e62 · a127e62
1 parent c81d12f
commit a127e62
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 14 deletions.
diff --git a/autotest/gdrivers/zarr_driver.py b/autotest/gdrivers/zarr_driver.py
@@ -4067,6 +4067,10 @@ def test():
         with pytest.raises(Exception):
             group.Rename("other_group")
 
+        # Existing array name (group and array names share the same namespace)
+        with pytest.raises(Exception):
+            subgroup.Rename("ar")
+
         # Rename group and test effects
         group.Rename("group_renamed")
         assert group.GetName() == "group_renamed"
@@ -4261,6 +4265,7 @@ def test():
         )
         rg = ds.GetRootGroup()
         group = rg.CreateGroup("group")
+        group.CreateGroup("subgroup")
 
         dim = group.CreateDimension(
             "dim0", "unspecified type", "unspecified direction", 2
@@ -4283,6 +4288,10 @@ def test():
         with pytest.raises(Exception):
             ar.Rename("other_ar")
 
+        # Existing subgroup name (group and array names share the same namespace)
+        with pytest.raises(Exception):
+            ar.Rename("subgroup")
+
         # Rename array and test effects
         ar.Rename("ar_renamed")
 

diff --git a/frmts/zarr/zarr.h b/frmts/zarr/zarr.h
@@ -428,6 +428,10 @@ class ZarrGroupBase CPL_NON_FINAL : public GDALGroup
 
     bool Rename(const std::string &osNewName) override;
 
+    //! Returns false in case of error
+    bool
+    CheckArrayOrGroupWithSameNameDoesNotExist(const std::string &osName) const;
+
     void ParentRenamed(const std::string &osNewParentFullName) override;
 
     void NotifyArrayRenamed(const std::string &osOldName,

diff --git a/frmts/zarr/zarr_array.cpp b/frmts/zarr/zarr_array.cpp
@@ -2560,14 +2560,8 @@ bool ZarrArray::Rename(const std::string &osNewName)
     auto poParent = m_poGroupWeak.lock();
     if (poParent)
     {
-        const auto arrayNames = poParent->GetMDArrayNames();
-        if (std::find(arrayNames.begin(), arrayNames.end(), osNewName) !=
-            arrayNames.end())
-        {
-            CPLError(CE_Failure, CPLE_AppDefined,
-                     "An array with same name already exists");
+        if (!poParent->CheckArrayOrGroupWithSameNameDoesNotExist(osNewName))
             return false;
-        }
     }
 
     const std::string osRootDirectoryName(

diff --git a/frmts/zarr/zarr_group.cpp b/frmts/zarr/zarr_group.cpp
@@ -271,6 +271,34 @@ bool ZarrGroupBase::IsValidObjectName(const std::string &osName)
              STARTS_WITH(osName.c_str(), ".z"));
 }
 
+/************************************************************************/
+/*                 CheckArrayOrGroupWithSameNameDoesNotExist()          */
+/************************************************************************/
+
+bool ZarrGroupBase::CheckArrayOrGroupWithSameNameDoesNotExist(
+    const std::string &osName) const
+{
+    const auto groupNames = GetGroupNames();
+    if (std::find(groupNames.begin(), groupNames.end(), osName) !=
+        groupNames.end())
+    {
+        CPLError(CE_Failure, CPLE_AppDefined,
+                 "A group with same name already exists");
+        return false;
+    }
+
+    const auto arrayNames = GetMDArrayNames();
+    if (std::find(arrayNames.begin(), arrayNames.end(), osName) !=
+        arrayNames.end())
+    {
+        CPLError(CE_Failure, CPLE_AppDefined,
+                 "An array with same name already exists");
+        return false;
+    }
+
+    return true;
+}
+
 /************************************************************************/
 /*                              Rename()                                */
 /************************************************************************/
@@ -297,14 +325,8 @@ bool ZarrGroupBase::Rename(const std::string &osNewName)
     auto pParent = std::dynamic_pointer_cast<ZarrGroupBase>(m_poParent.lock());
     if (pParent)
     {
-        const auto groupNames = pParent->GetGroupNames();
-        if (std::find(groupNames.begin(), groupNames.end(), osNewName) !=
-            groupNames.end())
-        {
-            CPLError(CE_Failure, CPLE_AppDefined,
-                     "A group with same name already exists");
+        if (!pParent->CheckArrayOrGroupWithSameNameDoesNotExist(osNewName))
             return false;
-        }
     }
 
     std::string osNewDirectoryName(m_osDirectoryName);