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 #1403
Unidata · Jan 3, 2025 · df89e71 · df89e71
1 parent 5e5b339
commit df89e71
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 8 deletions.
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
@@ -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
@@ -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
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
@@ -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;