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 9d316d9d02..df76a96854 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 @@ -810,9 +810,11 @@ private void readDim(Group.Builder groupBuilder, @Nullable Group refGroup, Eleme String nameInFile = dimElem.getAttributeValue("orgName") != null ? dimElem.getAttributeValue("orgName") : name; - // LOOK this is wrong, groupBuilder may already have the dimension. // see if it already exists - Dimension dim = (refGroup == null) ? null : refGroup.findDimension(nameInFile); + Optional dimFromAgg = groupBuilder.findDimension(nameInFile); + Dimension dim = + (refGroup == null || dimFromAgg.isPresent()) ? dimFromAgg.orElse(null) : refGroup.findDimension(nameInFile); + if (dim == null) { // nope - create it String lengthS = dimElem.getAttributeValue("length"); if (lengthS == null) { diff --git a/cdm/core/src/test/data/ncml/aggregationRenameDim.xml b/cdm/core/src/test/data/ncml/aggregationRenameDim.xml new file mode 100644 index 0000000000..99db871a5a --- /dev/null +++ b/cdm/core/src/test/data/ncml/aggregationRenameDim.xml @@ -0,0 +1,14 @@ + + + + + + + + + + + + + + diff --git a/cdm/core/src/test/data/ncml/aggregationScanRenameDim.xml b/cdm/core/src/test/data/ncml/aggregationScanRenameDim.xml new file mode 100644 index 0000000000..aa1d8a000a --- /dev/null +++ b/cdm/core/src/test/data/ncml/aggregationScanRenameDim.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestNcmlModifyDim.java b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestNcmlModifyDim.java index 4d55ba4d36..0d5591214b 100644 --- a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestNcmlModifyDim.java +++ b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestNcmlModifyDim.java @@ -3,8 +3,6 @@ import static com.google.common.truth.Truth.assertThat; import java.io.IOException; -import org.junit.AfterClass; -import org.junit.BeforeClass; import org.junit.Test; import ucar.ma2.Array; import ucar.ma2.IndexIterator; @@ -14,44 +12,65 @@ import ucar.nc2.ncml.TestNcmlRead; public class TestNcmlModifyDim { - private static final String filename = "file:./" + TestNcmlRead.topDir + "modifyDim.xml"; - private static NetcdfFile ncfile = null; + @Test + public void shouldRenameDim() throws IOException { + final String filename = "file:./" + TestNcmlRead.topDir + "modifyDim.xml"; + + try (NetcdfFile ncfile = NcmlReader.readNcml(filename, null, null).build()) { + assertThat(ncfile.getRootGroup().getDimensions().size()).isEqualTo(3); + + Dimension newDim = ncfile.findDimension("newTime"); + assertThat(newDim).isNotNull(); + assertThat(newDim.isVariableLength()).isFalse(); + assertThat(newDim.isShared()).isTrue(); + assertThat(newDim.isUnlimited()).isTrue(); + + Dimension oldDim = ncfile.findDimension("time"); + assertThat(oldDim).isNull(); + + Variable time = ncfile.findVariable("time"); + assertThat((Object) time).isNotNull(); + assertThat(time.getDimensionsString()).isEqualTo("newTime"); + + Array data = time.read(); + assertThat(data.getRank()).isEqualTo(1); + assertThat(data.getSize()).isEqualTo(4); + assertThat(data.getShape()[0]).isEqualTo(4); + assertThat(data.getElementType()).isEqualTo(int.class); - @BeforeClass - public static void setUp() throws IOException { - ncfile = NcmlReader.readNcml(filename, null, null).build(); + IndexIterator dataIter = data.getIndexIterator(); + assertThat(dataIter.getIntNext()).isEqualTo(6); + assertThat(dataIter.getIntNext()).isEqualTo(18); + } } - @AfterClass - public static void tearDown() throws IOException { - ncfile.close(); + @Test + public void shouldRenameDimInAggregation() throws IOException { + final String filename = "file:./" + TestNcmlRead.topDir + "aggregationRenameDim.xml"; + checkDimIsRenamed(filename); } @Test - public void shouldRenameDim() throws IOException { - assertThat(ncfile.getRootGroup().getDimensions().size()).isEqualTo(3); - - Dimension newDim = ncfile.findDimension("newTime"); - assertThat(newDim).isNotNull(); - assertThat(newDim.isVariableLength()).isFalse(); - assertThat(newDim.isShared()).isTrue(); - assertThat(newDim.isUnlimited()).isTrue(); - - Dimension oldDim = ncfile.findDimension("time"); - assertThat(oldDim).isNull(); - - Variable time = ncfile.findVariable("time"); - assertThat((Object) time).isNotNull(); - assertThat(time.getDimensionsString()).isEqualTo("newTime"); - - Array data = time.read(); - assertThat(data.getRank()).isEqualTo(1); - assertThat(data.getSize()).isEqualTo(4); - assertThat(data.getShape()[0]).isEqualTo(4); - assertThat(data.getElementType()).isEqualTo(int.class); - - IndexIterator dataIter = data.getIndexIterator(); - assertThat(dataIter.getIntNext()).isEqualTo(6); - assertThat(dataIter.getIntNext()).isEqualTo(18); + public void shouldRenameDimInAggregationScan() throws IOException { + final String filename = "file:./" + TestNcmlRead.topDir + "aggregationScanRenameDim.xml"; + checkDimIsRenamed(filename); + } + + private void checkDimIsRenamed(String filename) throws IOException { + try (NetcdfFile ncfile = NcmlReader.readNcml(filename, null, null).build()) { + Dimension newDim = ncfile.findDimension("newTime"); + assertThat(newDim).isNotNull(); + + Dimension oldDim = ncfile.findDimension("time"); + assertThat(oldDim).isNull(); + + Variable newTime = ncfile.findVariable("newTime"); + assertThat((Object) newTime).isNotNull(); + Array data = newTime.read(); + assertThat(data.getSize()).isEqualTo(59); + + Variable time = ncfile.findVariable("time"); + assertThat((Object) time).isNull(); + } } }