From df89e7126929f638a347d0f0af50113ccde638c9 Mon Sep 17 00:00:00 2001
From: Sean Arms <67096+lesserwhirls@users.noreply.github.com>
Date: Fri, 3 Jan 2025 15:45:28 -0700
Subject: [PATCH] Fix NcmlReader.mergeNcml with groups

Ensure reference group is passed into readGroup when using NcmlReader.mergeNcml,
 and make sure the reference group information is taken into account by the readGroup code.

Fixes Unidata/netcdf-java#1403
---
 .../ucar/nc2/internal/ncml/NcmlReader.java    |   9 ++-
 .../src/test/data/ncml/modifyNestedGroups.xml |  16 +++++
 .../src/test/data/testModifyNestedGroups.nc4  | Bin 0 -> 8616 bytes
 .../nc2/internal/ncml/TestNcmlReader.java     |  56 ++++++++++++++++--
 4 files changed, 73 insertions(+), 8 deletions(-)
 create mode 100644 cdm/core/src/test/data/ncml/modifyNestedGroups.xml
 create mode 100644 cdm/core/src/test/data/testModifyNestedGroups.nc4

diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java
index df76a96854..cb55b68a26 100644
--- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java
+++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java
@@ -288,7 +288,7 @@ public static NetcdfDataset.Builder mergeNcml(NetcdfFile ref, @Nullable Element
 
     if (ncmlElem != null) {
       NcmlReader reader = new NcmlReader();
-      reader.readGroup(targetDS, null, null, ncmlElem);
+      reader.readGroup(targetDS, null, ref.getRootGroup(), ncmlElem);
     }
 
     setEnhanceMode(targetDS, ncmlElem, null);
@@ -574,9 +574,12 @@ private Group.Builder readGroup(NetcdfDataset.Builder builder, @Nullable Group.B
     Group refGroup = null;
 
     if (parent == null) {
-      refGroup = this.refFile == null ? null : this.refFile.getRootGroup();
+      if (this.refFile != null) {
+        refGroup = this.refFile.getRootGroup();
+      } else if (refParent != null) {
+        refGroup = refParent;
+      }
       groupBuilder = builder.rootGroup;
-
     } else {
       String name = groupElem.getAttributeValue("name");
       if (name == null) {
diff --git a/cdm/core/src/test/data/ncml/modifyNestedGroups.xml b/cdm/core/src/test/data/ncml/modifyNestedGroups.xml
new file mode 100644
index 0000000000..5e23c2a646
--- /dev/null
+++ b/cdm/core/src/test/data/ncml/modifyNestedGroups.xml
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<netcdf xmlns="http://www.unidata.ucar.edu/namespaces/netcdf/ncml-2.2"
+  location="nc/example.nc">
+	<group name="outer_group">
+	  <attribute name="new_outer_attr" value="new_outer_attr_value" />
+  	<variable name="lat">
+	  	<attribute name="new_lat_attr" value="new_lat_attr_value" />
+	  </variable>
+	  <group name="inner_group">
+  		<attribute name="new_inner_attr" value="new_inner_attr_value" />
+	  	<variable name="lon">
+		  	<attribute name="new_lon_attr" value="new_lon_attr_value" />
+	  	</variable>
+  	</group>
+	</group>
+</netcdf>
diff --git a/cdm/core/src/test/data/testModifyNestedGroups.nc4 b/cdm/core/src/test/data/testModifyNestedGroups.nc4
new file mode 100644
index 0000000000000000000000000000000000000000..3685d9538b9e9c84ce007757bd3cbdd7eac09648
GIT binary patch
literal 8616
zcmeHMO-vI(6n?u~uqlh;4<LqU@#i9hfWasSNNTB6fnul!^<XV_DVvmCvfZjN#{YwO
zRTC49Hxo}@G@7Uvd+|bydf?zqFD4qJiD#ql%?~aFlpo{4%rv`i=FOX#Z@%|tHhG;$
z_Ux^WY>4RLFv#*llzUBF(<VE%Omq$=dxmuLp}v{v3|(}|A+u#f2aQZBH_}@R(VI$@
zTk<V0llR=x_!q8%ni{H<*YjpcXrWS8Kv;~4MURM%%6h73A+`|KLcTp_y^zQ%gcZQI
zTW_xYE%>(MNci`~y0wi!+cPwnO@eIm4PFo=92UST>!|VzzEQ~fZzWNI&41f?vU?VM
zxnsG&Y$@623twHqSKKQW4l--@k<x$&tivDh?PYx*8Z-^aFZA%`!k}wd$R3N71~o(o
zG0Lk*5h$;ZT`mvClYM4CDGjLNE9DFBaJG<H>Qc1)`~9C@F=DFkYZ`(F22lE6a$=Z-
zwFV(_Zlh2UA&t>YZLFNHsw1K66tWt#BB=pwC%l&CHFSMf168nyiKN#n=JmQ6BXa|{
zYZA4Yl;t1Kjda9a*UQ;X*0+i?P|ZayO<*<?N&}r1CT#KkSUL@aiabM`?b$bwNTv4;
zrZVYxtUm$fCej-SpAAUG1`^DkwnO%GO4zIh8m&ZdII9HLV~^RnnXqsK6|<KJBp57E
zN{;Op7aSxkFhr0Lgxb${udtYy8GZb2SZYX6Bqp~$d%#2DW*{V3jAHM!T!)Op>gXos
zY=9`#hoSx)T8Plt&O_zvneZA^;?d4Gkwg}_fPdnB#JB|S<Apny_i0B&Jzg@4b3Q|{
z9OqXPXTte};-)`&j{67#KH`kaLyDX)*hl=51M&GN(`s1$iHC^LMW4^$_4~no`jHNT
zL_7aCM1C#U=I~_{T<5I#N_@X~<BrrXRQARP`^mNJ8bQJ8DCHR$ab4<VF+-9aVU(k8
z$t$9ic2W*!Ju(v~3V?YX+c_)mSynOQxSl_XZ8^`cai?k6G%50g$#GV;=raTHI<P9(
zQgy|EVn8vV7*Gr-1{4E|0mXn~Krx^gPz)#re#Zc19b|T^*u*Va9v-XapK?g9v$z}*
z%abf8hs3f1B8NmtkL%5c#)Ekief}(Lf~QYc@@!}(6+hs)<Cf>!n17`->SlZh-wDer
z+OE^Jqs3u~+OCe)&ek0*qq&itUG1&y9jzUy{OzfOLx}|ChGlkH%56BqT;-CWcDgT+
z2%FZaief-9pcqgLC<YV*{}}^Y_uwCl_%!14whS<Z&l`MRmQNl1{5*#LyM#h*-FmX+
YN%Zm9Bkg<}{X-Jk4be&C-SCR^PxvU=p#T5?

literal 0
HcmV?d00001

diff --git a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestNcmlReader.java b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestNcmlReader.java
index 00f307cacd..3509a10ff3 100644
--- a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestNcmlReader.java
+++ b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestNcmlReader.java
@@ -10,6 +10,8 @@
 import org.jdom2.input.SAXBuilder;
 import org.junit.Test;
 import ucar.ma2.Array;
+import ucar.nc2.Attribute;
+import ucar.nc2.Group;
 import ucar.nc2.NetcdfFile;
 import ucar.nc2.Variable;
 import ucar.nc2.dataset.NetcdfDataset;
@@ -25,8 +27,9 @@ public class TestNcmlReader {
   public void shouldMergeNcml() throws IOException, JDOMException {
     final String filename = TestDir.cdmLocalTestDataDir + "example1.nc";
 
-    try (NetcdfFile netcdfFile = NetcdfDatasets.openFile(filename, null)) {
-      final NetcdfDataset netcdfDataset = NcmlReader.mergeNcml(netcdfFile, getNcmlElement("modifyVars.xml")).build();
+    try (NetcdfFile netcdfFile = NetcdfDatasets.openFile(filename, null);
+        final NetcdfDataset netcdfDataset =
+            NcmlReader.mergeNcml(netcdfFile, getNcmlElement("modifyVars.xml")).build();) {
 
       final Variable ncmlVariable = netcdfDataset.findVariable("deltaLat");
       assertThat((Object) ncmlVariable).isInstanceOf(VariableDS.class);
@@ -40,9 +43,9 @@ public void shouldMergeNcml() throws IOException, JDOMException {
   public void shouldMergeNcmlWithEnhancements() throws IOException, JDOMException {
     final String filename = TestDir.cdmLocalTestDataDir + "example1.nc";
 
-    try (NetcdfFile netcdfFile = NetcdfDatasets.openFile(filename, null)) {
-      final NetcdfDataset netcdfDataset =
-          NcmlReader.mergeNcml(netcdfFile, getNcmlElement("enhance/testStandardizer.ncml")).build();
+    try (NetcdfFile netcdfFile = NetcdfDatasets.openFile(filename, null);
+        final NetcdfDataset netcdfDataset =
+            NcmlReader.mergeNcml(netcdfFile, getNcmlElement("enhance/testStandardizer.ncml")).build();) {
 
       final Variable ncmlVariable = netcdfDataset.findVariable("doublevar");
       assertThat((Object) ncmlVariable).isNotNull();
@@ -51,6 +54,49 @@ public void shouldMergeNcmlWithEnhancements() throws IOException, JDOMException
     }
   }
 
+  @Test
+  public void mergeWithExistingGroups() throws IOException, JDOMException {
+    final String filename = TestDir.cdmLocalTestDataDir + "testModifyNestedGroups.nc4";
+
+    try (NetcdfFile netcdfFile = NetcdfDatasets.openFile(filename, null);
+        final NetcdfDataset netcdfDataset =
+            NcmlReader.mergeNcml(netcdfFile, getNcmlElement("modifyNestedGroups.xml")).build()) {
+
+      final Group ncmlOuterGroup = netcdfDataset.findGroup("/outer_group");
+      assertThat(ncmlOuterGroup).isNotNull();
+
+      final Attribute ncmlOuterAttribute = ncmlOuterGroup.findAttribute("new_outer_attr");
+      assertThat(ncmlOuterAttribute).isNotNull();
+      assertThat(ncmlOuterAttribute.getStringValue()).isNotNull();
+      assertThat(ncmlOuterAttribute.getStringValue()).isEqualTo("new_outer_attr_value");
+
+      final Variable ncmlOuterVar = ncmlOuterGroup.findVariableLocal("lat");
+      assertThat(ncmlOuterVar != null).isTrue();
+      assertThat(ncmlOuterVar.attributes().hasAttribute("new_lat_attr")).isTrue();
+      assertThat(ncmlOuterVar.attributes().findAttribute("new_lat_attr")).isNotNull();
+      assertThat(ncmlOuterVar.attributes().findAttribute("new_lat_attr").getStringValue()).isNotNull();
+      assertThat(ncmlOuterVar.attributes().findAttribute("new_lat_attr").getStringValue())
+          .isEqualTo("new_lat_attr_value");
+      // <attribute name="new_lat_attr" value="new_lat_attr_value" />
+
+      final Group ncmlInnerGroup = netcdfDataset.findGroup("/outer_group/inner_group");
+      assertThat(ncmlInnerGroup).isNotNull();
+
+      final Attribute ncmlInnerAttribute = ncmlInnerGroup.findAttribute("new_inner_attr");
+      assertThat(ncmlInnerAttribute).isNotNull();
+      assertThat(ncmlInnerAttribute.getStringValue()).isNotNull();
+      assertThat(ncmlInnerAttribute.getStringValue()).isEqualTo("new_inner_attr_value");
+
+      final Variable ncmlInnerVar = ncmlInnerGroup.findVariableLocal("lon");
+      assertThat(ncmlInnerVar != null).isTrue();
+      assertThat(ncmlInnerVar.attributes().hasAttribute("new_lon_attr")).isTrue();
+      assertThat(ncmlInnerVar.attributes().findAttribute("new_lon_attr")).isNotNull();
+      assertThat(ncmlInnerVar.attributes().findAttribute("new_lon_attr").getStringValue()).isNotNull();
+      assertThat(ncmlInnerVar.attributes().findAttribute("new_lon_attr").getStringValue())
+          .isEqualTo("new_lon_attr_value");
+    }
+  }
+
   private static Element getNcmlElement(String filename) throws IOException, JDOMException {
     final String ncml = TestNcmlRead.topDir + filename;