From 6ff177594d24b927408300a2ad402dceb8e9abb7 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Fri, 27 Jan 2023 12:05:45 -0700 Subject: [PATCH] Fix EnumTypedef name problem (#1091) * ckp * Fix EnumTypedef name problem ## Description of Changes re: Issue https://github.com/Unidata/netcdf-java/issues/126 The code for handling enum types in Group.java is incorrect. When creating a new enum type, it is appropriate to search only the current group for a conflicting name and this is what the current code does. But when an enum typed variable searches for the matching enum type, it must search not only the current group but all parent groups and the current code does not do that. The fix consists of two similar parts. 1. Modify Group.findEnumeration to take an extra boolean parameter to control if search is this group only or to search up the group's parents. 2. Modify Group.Builder.findEnumTypedef to act like \#1 but to search the sequence of parent Group.Builder instances. As a consequence, this PR modifies a number of other files to adapt to the modified signatures. * 1. Remove unused import in H5HeaderNew 2. Add overloaded functions to Group.java to restore access to original versions of findEnumTypedef and findEnumeration. * Force re-test * It turns out that I missed the error in the code in H5headerNew that attempts to match an enum typed variable with the proper enumeration type declaration. The problem and fix (as described by a comment in H5headerNew) is a bit of a hack. It should be fixed in H5Object.read(). Unfortunately, information is not being passed down so that the proper fix can be applied early in the HDF5->CDM translation. Fixing this would affect a lot of function signatures. Also modified TestEnumTypedef.java to test two cases: 1. the actual enum type def is in the same group as the variable that uses it. 2. the actual enum type def is in a parent group of the variable that uses it ## Misc. Other Changes * Suppress printing of _NCProperties to simplify text-based comparison testing. * ## Addendum 2 Sigh! Apparently NetcdfFile.java defaulted to using H5iosp instead of H5iospNew. This meant that many of my changes were being bypassed. So, modify NetcdfFile to default to H5iospNew. * Undo change to NetcdfFile.java' * test4 * NCProperties fix * ## Additional modifications * NetcdfFile.java: convert to use H5iospNew (needed by TestH5iosp.java) * H5headerNew.java: provide get function for accessing the btree (needed by TestDataBTree.java) * H5iospNew.java: make getRandomAccessFile() method public (needed by tests) * CompareNetcdf2.java: Add a constructor to specify if attribute name comparison should ignore case or not. It turns out that some tests require case sensitive name matching. Specifically TestCoordSysCompare.java and TestN3iospCompare.java * Apply Spotless * remove debugging --------- Co-authored-by: haileyajohnson --- .../java/ucar/nc2/iosp/bufr/Construct2.java | 2 +- .../src/main/java/ucar/nc2/Attribute.java | 2 +- cdm/core/src/main/java/ucar/nc2/Group.java | 77 +++- .../src/main/java/ucar/nc2/NetcdfFile.java | 4 +- cdm/core/src/main/java/ucar/nc2/Variable.java | 2 +- .../nc2/internal/iosp/hdf5/H5headerNew.java | 71 ++- .../nc2/internal/iosp/hdf5/H5iospNew.java | 2 +- .../nc2/internal/iosp/hdf5/H5objects.java | 2 +- .../java/ucar/nc2/iosp/hdf5/H5header.java | 4 +- .../main/java/ucar/nc2/ncml/NcMLReader.java | 4 +- .../java/ucar/nc2/util/CompareNetcdf2.java | 13 +- cdm/core/src/test/data/hdf5/test_enum_2.nc4 | Bin 0 -> 1428 bytes .../java/ucar/nc2/TestSpecialAttributes.java | 2 +- .../ucar/nc2/dataset/TestCoordSysCompare.java | 2 +- .../ucar/nc2/iosp/hdf5/TestDataBTree.java | 6 +- .../ucar/nc2/iosp/hdf5/TestEnumTypedef.java | 42 +- .../java/ucar/nc2/iosp/hdf5/TestH5iosp.java | 7 +- .../nc2/iosp/netcdf3/TestN3iospCompare.java | 2 +- .../test/java/dap4/test/TestDAP4Client.java | 406 ++++++++++++++++++ .../src/test/java/dap4/test/TestHyrax.java | 1 - .../java/ucar/nc2/jni/netcdf/Nc4Iosp.java | 2 +- 21 files changed, 591 insertions(+), 62 deletions(-) create mode 100644 cdm/core/src/test/data/hdf5/test_enum_2.nc4 create mode 100644 dap4/d4tests/src/test/java/dap4/test/TestDAP4Client.java diff --git a/bufr/src/main/java/ucar/nc2/iosp/bufr/Construct2.java b/bufr/src/main/java/ucar/nc2/iosp/bufr/Construct2.java index 3932a6fbe5..cb768431d0 100644 --- a/bufr/src/main/java/ucar/nc2/iosp/bufr/Construct2.java +++ b/bufr/src/main/java/ucar/nc2/iosp/bufr/Construct2.java @@ -299,7 +299,7 @@ else if (nbytes == 4) Group g = struct.getParentGroupOrRoot(); if (g == null) log.warn("Struct parent group is null."); - EnumTypedef enumTypedef = g.findEnumeration(ct.getName()); + EnumTypedef enumTypedef = g.findEnumeration(ct.getName(), false); if (enumTypedef == null) { enumTypedef = new EnumTypedef(ct.getName(), ct.getMap()); g.addEnumeration(enumTypedef); diff --git a/cdm/core/src/main/java/ucar/nc2/Attribute.java b/cdm/core/src/main/java/ucar/nc2/Attribute.java index b8dca41f7a..b8ab59955d 100644 --- a/cdm/core/src/main/java/ucar/nc2/Attribute.java +++ b/cdm/core/src/main/java/ucar/nc2/Attribute.java @@ -38,7 +38,7 @@ public class Attribute extends CDMNode { /** @deprecated move to jni.Nc4Iosp */ @Deprecated - static final String[] SPECIALS = + static public final String[] SPECIALS = {CDM.NCPROPERTIES, CDM.ISNETCDF4, CDM.SUPERBLOCKVERSION, CDM.DAP4_LITTLE_ENDIAN, CDM.EDU_UCAR_PREFIX}; /** diff --git a/cdm/core/src/main/java/ucar/nc2/Group.java b/cdm/core/src/main/java/ucar/nc2/Group.java index e73edd8277..8ebb113285 100755 --- a/cdm/core/src/main/java/ucar/nc2/Group.java +++ b/cdm/core/src/main/java/ucar/nc2/Group.java @@ -2,8 +2,10 @@ * Copyright (c) 1998-2018 University Corporation for Atmospheric Research/Unidata * See LICENSE for license information. */ + package ucar.nc2; +import static java.util.Optional.ofNullable; import static ucar.nc2.NetcdfFiles.reservedFullName; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -295,7 +297,7 @@ public Dimension findDimensionLocal(String shortName) { /** The attributes contained by this Group. */ public AttributeContainer attributes() { - return attributes; + return AttributeContainer.filter(attributes, Attribute.SPECIALS); } /** Find the attribute by name, return null if not exist */ @@ -374,20 +376,29 @@ public boolean removeAttributeIgnoreCase(String attName) { //////////////////////////////////////////////////////////////////////// - /** Find a Enumeration in this or a parent Group, using its short name. */ + /** Find a Enumeration in this Group, using its short name. */ @Nullable public EnumTypedef findEnumeration(String name) { + // Keep the old behavior + return findEnumeration(name, false); + } + + /** Find a Enumeration in this or optionally the parent Groups, using its short name. */ + @Nullable + public EnumTypedef findEnumeration(String name, boolean searchup) { if (name == null) return null; // name = NetcdfFile.makeNameUnescaped(name); - for (EnumTypedef d : enumTypedefs) { + // search this group's EnumTypedefs + for (EnumTypedef d : this.getEnumTypedefs()) { if (name.equals(d.getShortName())) return d; } - Group parent = getParentGroup(); - if (parent != null) - return parent.findEnumeration(name); - + if (searchup) { + Group parent = getParentGroup(); + if (parent != null) + return parent.findEnumeration(name, searchup); + } return null; } @@ -1114,6 +1125,52 @@ public Group.Builder commonParent(Group.Builder other) { return other; } + public Optional findSimilarEnumTypedef(EnumTypedef template, boolean searchup, boolean anyname) { + Optional ed = Optional.empty(); + assert (template != null); + // search this group builders's EnumTypedefs but with constraint on name + ed = this.enumTypedefs.stream() + .filter(e -> (anyname || !template.getShortName().equals(e.getShortName())) && e.equalsMapOnly(template)) + .findFirst(); + if (ed.isPresent()) + return ed; + // Optionally search parents + if (searchup) { + Group.Builder gb = getParentGroup(); + if (gb != null) + ed = gb.findSimilarEnumTypedef(template, searchup, anyname); + if (ed.isPresent()) + return ed; + } + return Optional.empty(); + } + + /** Find a Enumeration in this Group Builder, using its short name. */ + public Optional findEnumTypedef(String name) { + // Keep the old behavior + return findEnumTypedef(name, false); + } + + /** Find a Enumeration in this or a parent Group Builder, using its short name. */ + public Optional findEnumTypedef(String name, boolean searchup) { + if (name == null) + return Optional.empty(); + // name = NetcdfFile.makeNameUnescaped(name); + // search this group builders's EnumTypedefs + Optional ed = this.enumTypedefs.stream().filter(e -> e.shortName.equals(name)).findFirst(); + if (ed.isPresent()) + return ed; + // Optionally search parents + if (searchup) { + Group.Builder gb = getParentGroup(); + if (gb != null) + ed = gb.findEnumTypedef(name, searchup); + if (ed.isPresent()) + return ed; + } + return Optional.empty(); + } + public Builder addEnumTypedef(EnumTypedef typedef) { Preconditions.checkNotNull(typedef); enumTypedefs.add(typedef); @@ -1131,7 +1188,7 @@ public Builder addEnumTypedefs(Collection typedefs) { * Return new or existing. */ public EnumTypedef findOrAddEnumTypedef(String name, Map map) { - Optional opt = findEnumTypedef(name); + Optional opt = findEnumTypedef(name, false); // Find in this group if (opt.isPresent()) { return opt.get(); } else { @@ -1141,10 +1198,6 @@ public EnumTypedef findOrAddEnumTypedef(String name, Map map) { } } - public Optional findEnumTypedef(String name) { - return this.enumTypedefs.stream().filter(e -> e.shortName.equals(name)).findFirst(); - } - /** Add a Variable, throw error if one of the same name if it exists. */ public Builder addVariable(Variable.Builder variable) { Preconditions.checkNotNull(variable); diff --git a/cdm/core/src/main/java/ucar/nc2/NetcdfFile.java b/cdm/core/src/main/java/ucar/nc2/NetcdfFile.java index 9c7f7e3dfb..645328cff4 100644 --- a/cdm/core/src/main/java/ucar/nc2/NetcdfFile.java +++ b/cdm/core/src/main/java/ucar/nc2/NetcdfFile.java @@ -41,12 +41,12 @@ import ucar.ma2.InvalidRangeException; import ucar.ma2.Section; import ucar.ma2.StructureDataIterator; +import ucar.nc2.internal.iosp.hdf5.H5iospNew; import ucar.nc2.internal.iosp.netcdf3.N3headerNew; import ucar.nc2.iosp.AbstractIOServiceProvider; import ucar.nc2.iosp.IOServiceProvider; import ucar.nc2.iosp.IospHelper; import ucar.nc2.iosp.hdf5.H5header; -import ucar.nc2.iosp.hdf5.H5iosp; import ucar.nc2.iosp.netcdf3.N3header; import ucar.nc2.iosp.netcdf3.N3iosp; import ucar.nc2.iosp.netcdf3.SPFactory; @@ -830,7 +830,7 @@ private static IOServiceProvider getIosp(RandomAccessFile raf) throws IOExceptio return SPFactory.getServiceProvider(); } else if (H5header.isValidFile(raf)) { - return new H5iosp(); + return new H5iospNew(); } else { diff --git a/cdm/core/src/main/java/ucar/nc2/Variable.java b/cdm/core/src/main/java/ucar/nc2/Variable.java index 19d6b98060..a94219af21 100644 --- a/cdm/core/src/main/java/ucar/nc2/Variable.java +++ b/cdm/core/src/main/java/ucar/nc2/Variable.java @@ -1830,7 +1830,7 @@ protected Variable(Builder builder, Group parentGroup) { this.cache = builder.cache; if (this.dataType.isEnum()) { - this.enumTypedef = this.group.findEnumeration(builder.enumTypeName); + this.enumTypedef = this.group.findEnumeration(builder.enumTypeName, true); if (this.enumTypedef == null) { throw new IllegalStateException( String.format("EnumTypedef '%s' does not exist in a parent Group", builder.enumTypeName)); diff --git a/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5headerNew.java b/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5headerNew.java index 765721f8f1..faa7fb8fa6 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5headerNew.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5headerNew.java @@ -17,13 +17,8 @@ import java.nio.ShortBuffer; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; -import java.util.Formatter; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.TimeZone; +import java.util.*; + import ucar.ma2.Array; import ucar.ma2.ArrayChar; import ucar.ma2.ArrayObject; @@ -44,7 +39,6 @@ import ucar.nc2.Structure; import ucar.nc2.Variable; import ucar.nc2.constants.CDM; -import ucar.nc2.filter.Filter; import ucar.nc2.internal.iosp.hdf4.HdfEos; import ucar.nc2.internal.iosp.hdf4.HdfHeaderIF; import ucar.nc2.internal.iosp.hdf5.H5objects.DataObject; @@ -498,7 +492,7 @@ private boolean makeNetcdfGroup(Group.Builder parentGroup, H5Group h5group) thro } if (facadeNested.dobj.mdt.map != null) { - EnumTypedef enumTypedef = parentGroup.findEnumTypedef(facadeNested.name).orElse(null); + EnumTypedef enumTypedef = parentGroup.findEnumTypedef(facadeNested.name, true).orElse(null); if (enumTypedef == null) { DataType basetype; switch (facadeNested.dobj.mdt.byteSize) { @@ -554,7 +548,7 @@ private boolean makeNetcdfGroup(Group.Builder parentGroup, H5Group h5group) thro } // This code apparently addresses the possibility of an anonymous enum LOOK ?? if (enumTypeName.isEmpty()) { - EnumTypedef enumTypedef = parentGroup.findEnumTypedef(facadeNested.name).orElse(null); + EnumTypedef enumTypedef = parentGroup.findEnumTypedef(facadeNested.name, true).get(); if (enumTypedef == null) { enumTypedef = new EnumTypedef(facadeNested.name, facadeNested.dobj.mdt.map); parentGroup.addEnumTypedef(enumTypedef); @@ -1553,14 +1547,51 @@ private boolean makeVariableShapeAndType(Group.Builder parent, Variable.Builder // set the enumTypedef if (dt.isEnum()) { - // TODO Not sure why, but there may be both a user type and a "local" mdt enum. May need to do a value match? - EnumTypedef enumTypedef = parent.findEnumTypedef(mdt.enumTypeName).orElse(null); - if (enumTypedef == null) { // if shared object, wont have a name, shared version gets added later - EnumTypedef local = new EnumTypedef(mdt.enumTypeName, mdt.map); - enumTypedef = parent.enumTypedefs.stream().filter((e) -> e.equalsMapOnly(local)).findFirst().orElse(local); - parent.addEnumTypedef(enumTypedef); - } - v.setEnumTypeName(enumTypedef.getShortName()); + // dmh: HDF5, at least as used by netcdf-4, defines an enumeration type multiple times. + // One is when the enum type is defined, and then for every variable or field typed by that + // enum type, another enum type is created but with the name of the variable/field instead of + // the name of the actual type. This appears to be because H5header(New) does not pass + // down enough information for the HDF5->CDM translator to recognize the second case. + // The solution provided is to see if the following conditions hold when an enum typed + // variable is encountered: + // 1. The enum name is the same as the variable name. + // 2. There is no enum of that name in the current group. + // 3. There is another enum type in some containing group + // that is structurally identical to the EnumTypedef -- but with a + // different name -- associated with the variable. This is in contrast + // to testing for name equality. + // + // If all three conditions are true, then use the enum found + // in condition 3 as the enum type for the variable. + // Note that provision is made only for a shortname rather than a + // fully qualified name, which is incorrect, but too hard to change. + // + // If condition 1 if false, then it is ok to create a new EnumTypedef. + // Else if condition 2 is false then it is an error because the existence + // of such an enum is illegal netcdf-4 because duplicate name. + // Else if condition 3 is false, then it is an error because the + // variable's enum type is not defined. + + EnumTypedef actualEnumTypedef = null; // The final chosen EnumTypedef + // Create the variables EnumTypedef on speculation + EnumTypedef template = new EnumTypedef(mdt.enumTypeName, mdt.map); + if (template.getShortName().equals(v.shortName)) { // Condition 1 + Optional candidate = parent.findEnumTypedef(mdt.enumTypeName, false); + if (!candidate.isPresent()) { // Condition 2 + candidate = parent.findSimilarEnumTypedef(template, true, false); + if (candidate.isPresent()) // Condition 3; use correct EnumTypedef + actualEnumTypedef = candidate.get(); + else { // !Condition 3 + log.warn("EnumTypedef is missing for variable: {}", v.shortName); + throw new IllegalStateException("EnumTypedef is missing for variable: " + v.shortName); + } + } else { // !Condition 2 + log.warn("Duplicate name: EnumTypedef and Variable: {}", v.shortName); + throw new IllegalStateException("Duplicate name: EnumTypedef and Variable: " + v.shortName); + } + } else // ! Condition 1; use template + actualEnumTypedef = template; + v.setEnumTypeName(actualEnumTypedef.getShortName()); } return true; @@ -1610,6 +1641,10 @@ public class Vinfo { boolean useFillValue; byte[] fillValue; + public DataBTree getBtree() { + return btree; + } + public String getCompression() { if (mfp == null) return null; diff --git a/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5iospNew.java b/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5iospNew.java index e881ee9103..771737de0e 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5iospNew.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5iospNew.java @@ -97,7 +97,7 @@ public String getFileTypeDescription() { return "Hierarchical Data Format, version 5"; } - RandomAccessFile getRandomAccessFile() { + public RandomAccessFile getRandomAccessFile() { return raf; } diff --git a/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5objects.java b/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5objects.java index 110885a280..23794555d0 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5objects.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/iosp/hdf5/H5objects.java @@ -1774,7 +1774,7 @@ String getName() { } int getId() { - return (int) this.properties.get(Filters.Keys.ID); + return ((Short) this.properties.get(Filters.Keys.ID)).intValue(); } Map getProperties() { diff --git a/cdm/core/src/main/java/ucar/nc2/iosp/hdf5/H5header.java b/cdm/core/src/main/java/ucar/nc2/iosp/hdf5/H5header.java index 3db0bd88c1..429b9d1b41 100644 --- a/cdm/core/src/main/java/ucar/nc2/iosp/hdf5/H5header.java +++ b/cdm/core/src/main/java/ucar/nc2/iosp/hdf5/H5header.java @@ -489,7 +489,7 @@ private boolean makeNetcdfGroup(ucar.nc2.Group ncGroup, H5Group h5group) throws } if (facadeNested.dobj.mdt.map != null) { - EnumTypedef enumTypedef = ncGroup.findEnumeration(facadeNested.name); + EnumTypedef enumTypedef = ncGroup.findEnumeration(facadeNested.name, false); if (enumTypedef == null) { DataType basetype; switch (facadeNested.dobj.mdt.byteSize) { @@ -547,7 +547,7 @@ private boolean makeNetcdfGroup(ucar.nc2.Group ncGroup, H5Group h5group) throws // This code apparently addresses the possibility of an anonymous enum LOOK ?? String ename = enumTypedef.getShortName(); if (ename == null || ename.isEmpty()) { - enumTypedef = ncGroup.findEnumeration(facadeNested.name); + enumTypedef = ncGroup.findEnumeration(facadeNested.name, false); if (enumTypedef == null) { enumTypedef = new EnumTypedef(facadeNested.name, facadeNested.dobj.mdt.map); ncGroup.addEnumeration(enumTypedef); diff --git a/cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java b/cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java index 189a8a2abd..c757a94a1f 100644 --- a/cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java +++ b/cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java @@ -996,7 +996,7 @@ private void readVariable(NetcdfDataset ds, Group g, Group refg, Element varElem if (dtype.isEnum()) { String typedefS = varElem.getAttributeValue("typedef"); if (typedefS != null) - typedef = g.findEnumeration(typedefS); + typedef = g.findEnumeration(typedefS, true); } String shape = varElem.getAttributeValue("shape"); @@ -1207,7 +1207,7 @@ private Variable readVariableNew(NetcdfDataset ds, Group g, Structure parentS, E if (dtype.isEnum()) { String typedefS = varElem.getAttributeValue("typedef"); if (typedefS != null) - typedef = g.findEnumeration(typedefS); + typedef = g.findEnumeration(typedefS, true); } String shape = varElem.getAttributeValue("shape"); diff --git a/cdm/core/src/main/java/ucar/nc2/util/CompareNetcdf2.java b/cdm/core/src/main/java/ucar/nc2/util/CompareNetcdf2.java index f71c84a0bc..5de0fa63f4 100644 --- a/cdm/core/src/main/java/ucar/nc2/util/CompareNetcdf2.java +++ b/cdm/core/src/main/java/ucar/nc2/util/CompareNetcdf2.java @@ -165,6 +165,7 @@ public static boolean checkContains(String what, List container, ListLBw#ADF$x>E|tyu6H)7_~ZoK2QVQWOe%k%At) z=qK^iqX$2Rhl;QuUOn}q?@eatvQj)sU&x#H=Qs1-y!Vo~gZ_HEb-e{dK>$8Qcnh(iq90rHdLsjO5#SnfIcg`Egf4U3Lms-#JjXnn;odP_TMIo~n zm>~E3M)N|j)S7O^gN^2Gio$&2MD3xEbuF-!5 zm}ogp^=k})fnM1GfH!sc|Ibg7<9Ae;Yd7&z!5*_SZ`iy_*)rc%g6%!Bvorhh{+lUR z`!Z)Z6mBf=xFedN^Exn%CCZ+2&gjllzd@=IgDbRp?1TM_kdf%$RSbU+j=@Fzm{uTJ bmr6ZLRVB!Jek;XHg--kK>M=dp4rlxi9>7yV literal 0 HcmV?d00001 diff --git a/cdm/core/src/test/java/ucar/nc2/TestSpecialAttributes.java b/cdm/core/src/test/java/ucar/nc2/TestSpecialAttributes.java index d888260dba..bc94e28e8f 100644 --- a/cdm/core/src/test/java/ucar/nc2/TestSpecialAttributes.java +++ b/cdm/core/src/test/java/ucar/nc2/TestSpecialAttributes.java @@ -21,7 +21,7 @@ public class TestSpecialAttributes { @Test public void testReadAll() throws IOException { NetcdfFile ncfile = TestDir.openFileLocal("testSpecialAttributes.nc4"); - // Iterate over all top-level attributes and see if it is special + // Iterate over all top-level visible attributes and see if it is special for (Attribute a : ncfile.getRootGroup().getAttributes()) { Assert.assertFalse("Attribute iteration found special attribute: " + a.getShortName(), Attribute.isspecial(a)); } diff --git a/cdm/core/src/test/java/ucar/nc2/dataset/TestCoordSysCompare.java b/cdm/core/src/test/java/ucar/nc2/dataset/TestCoordSysCompare.java index 650bda95bf..c15198d257 100644 --- a/cdm/core/src/test/java/ucar/nc2/dataset/TestCoordSysCompare.java +++ b/cdm/core/src/test/java/ucar/nc2/dataset/TestCoordSysCompare.java @@ -52,7 +52,7 @@ public void compareCoordSysBuilders() throws IOException { try (NetcdfDataset org = NetcdfDataset.openDataset(fileLocation)) { try (NetcdfDataset withBuilder = NetcdfDatasets.openDataset(fileLocation)) { Formatter f = new Formatter(); - CompareNetcdf2 compare = new CompareNetcdf2(f, false, false, true); + CompareNetcdf2 compare = new CompareNetcdf2(f, false, false, true, false); boolean ok = compare.compare(org, withBuilder, new CoordsObjFilter()); System.out.printf("%s %s%n", ok ? "OK" : "NOT OK", f); System.out.printf("org = %s%n", org.getRootGroup().findAttributeString(_Coordinate._CoordSysBuilder, "")); diff --git a/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestDataBTree.java b/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestDataBTree.java index a6b018dac2..0964b9446c 100644 --- a/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestDataBTree.java +++ b/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestDataBTree.java @@ -11,7 +11,7 @@ import org.slf4j.LoggerFactory; import ucar.nc2.NetcdfFile; import ucar.nc2.Variable; -import ucar.nc2.iosp.hdf5.H5header.Vinfo; +import ucar.nc2.internal.iosp.hdf5.H5headerNew; import ucar.unidata.util.test.TestDir; public class TestDataBTree { @@ -34,7 +34,7 @@ public void close() throws Exception { public void shouldReleaseRafs() throws IOException { final Variable variable = netcdfFile.findVariable("data"); assertThat((Object) variable).isNotNull(); - final DataBTree bTree = ((Vinfo) variable.getSPobject()).btree; + final DataBTree bTree = ((H5headerNew.Vinfo) variable.getSPobject()).getBtree(); assertThat(bTree.getRandomAccessFile()).isNotNull(); netcdfFile.release(); @@ -45,7 +45,7 @@ public void shouldReleaseRafs() throws IOException { public void shouldCloseRafs() throws IOException { final Variable variable = netcdfFile.findVariable("data"); assertThat((Object) variable).isNotNull(); - final DataBTree bTree = ((Vinfo) variable.getSPobject()).btree; + final DataBTree bTree = ((H5headerNew.Vinfo) variable.getSPobject()).getBtree(); assertThat(bTree.getRandomAccessFile()).isNotNull(); netcdfFile.close(); diff --git a/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestEnumTypedef.java b/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestEnumTypedef.java index da45ee3a65..dee04157e5 100644 --- a/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestEnumTypedef.java +++ b/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestEnumTypedef.java @@ -3,29 +3,53 @@ import static com.google.common.truth.Truth.assertThat; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.lang.invoke.MethodHandles; import ucar.ma2.DataType; -import ucar.nc2.EnumTypedef; -import ucar.nc2.NetcdfFile; -import ucar.nc2.NetcdfFiles; -import ucar.nc2.Variable; +import ucar.nc2.*; import ucar.unidata.util.test.TestDir; /** Test handling of enums in hdf5 / netcdf 4 files. */ +// see Issue #126 public class TestEnumTypedef { + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + // Test case where enum type is in same group to the variable using it. @Test - public void problem() throws Exception { - try (NetcdfFile ncfile = NetcdfFiles.open(TestDir.cdmLocalTestDataDir + "hdf5/test_atomic_types.nc")) { - Variable primaryCloud = ncfile.findVariable("primary_cloud"); + public void test1() throws Exception { + String s = TestDir.cdmLocalTestDataDir + "hdf5/test_atomic_types.nc"; + logger.info("TestEnumTypedef on {}%n", s); + try (NetcdfFile ncfile = NetcdfFiles.open(s)) { + Variable primaryCloud = ncfile.findVariable("/primary_cloud"); assertThat((Object) primaryCloud).isNotNull(); assertThat(primaryCloud.getDataType().isEnum()); assertThat(primaryCloud.getDataType()).isEqualTo(DataType.ENUM1); assertThat(primaryCloud.getEnumTypedef()).isNotNull(); EnumTypedef typedef = primaryCloud.getEnumTypedef(); assertThat(typedef).isNotNull(); - // TODO disable this until we have a fix see Issue #126 - // assertThat(typedef.getShortName()).isEqualTo("cloud_class_t"); + logger.info("TestEnumTypedef typedef name {}%n", typedef.getShortName()); + assertThat(typedef.getShortName()).isEqualTo("cloud_class_t"); } } + // Test case where enum type is in a parent group to the variable using it. + @Test + public void test2() throws Exception { + String s = TestDir.cdmLocalTestDataDir + "hdf5/test_enum_2.nc4"; + logger.info("TestEnumTypedef on {}%n", s); + try (NetcdfFile ncfile = NetcdfFiles.open(s)) { + Group h = ncfile.findGroup("/h"); + Variable primaryCloud = h.findVariableLocal("primary_cloud"); + assertThat((Object) primaryCloud).isNotNull(); + assertThat(primaryCloud.getDataType().isEnum()); + assertThat(primaryCloud.getDataType()).isEqualTo(DataType.ENUM1); + assertThat(primaryCloud.getEnumTypedef()).isNotNull(); + EnumTypedef typedef = primaryCloud.getEnumTypedef(); + assertThat(typedef).isNotNull(); + logger.info("TestEnumTypedef typedef name {}%n", typedef.getShortName()); + assertThat(typedef.getShortName()).isEqualTo("cloud_class_t"); + } + } } + diff --git a/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestH5iosp.java b/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestH5iosp.java index fdc07ae719..d056de3a7b 100644 --- a/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestH5iosp.java +++ b/cdm/core/src/test/java/ucar/nc2/iosp/hdf5/TestH5iosp.java @@ -10,6 +10,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import ucar.nc2.NetcdfFile; +import ucar.nc2.internal.iosp.hdf5.H5iospNew; import ucar.nc2.iosp.IOServiceProvider; import ucar.unidata.util.test.TestDir; @@ -18,14 +19,14 @@ public class TestH5iosp { private static final String TEST_FILE = TestDir.cdmLocalTestDataDir + "hdf5/structmetadata_eos.h5"; private NetcdfFile netcdfFile; - private H5iosp h5iosp; + private H5iospNew h5iosp; @Before public void open() throws Exception { netcdfFile = NetcdfFile.open(TEST_FILE); final IOServiceProvider iosp = netcdfFile.getIosp(); - assertThat(iosp).isInstanceOf(H5iosp.class); - h5iosp = (H5iosp) iosp; + assertThat(iosp).isInstanceOf(H5iospNew.class); + h5iosp = (H5iospNew) iosp; assertThat(h5iosp).isNotNull(); } diff --git a/cdm/core/src/test/java/ucar/nc2/iosp/netcdf3/TestN3iospCompare.java b/cdm/core/src/test/java/ucar/nc2/iosp/netcdf3/TestN3iospCompare.java index 2af94e4d8a..d98c42610f 100644 --- a/cdm/core/src/test/java/ucar/nc2/iosp/netcdf3/TestN3iospCompare.java +++ b/cdm/core/src/test/java/ucar/nc2/iosp/netcdf3/TestN3iospCompare.java @@ -52,7 +52,7 @@ public void compareWithBuilder() SPFactory.setServiceProvider("ucar.nc2.internal.iosp.netcdf3.N3iospNew"); try (NetcdfFile withBuilder = NetcdfFiles.open(filename)) { Formatter f = new Formatter(); - CompareNetcdf2 compare = new CompareNetcdf2(f, false, false, true); + CompareNetcdf2 compare = new CompareNetcdf2(f, false, false, true, false); if (!compare.compare(org, withBuilder)) { System.out.printf("Compare %s%n%s%n", filename, f); fail(); diff --git a/dap4/d4tests/src/test/java/dap4/test/TestDAP4Client.java b/dap4/d4tests/src/test/java/dap4/test/TestDAP4Client.java new file mode 100644 index 0000000000..8ddca4ec20 --- /dev/null +++ b/dap4/d4tests/src/test/java/dap4/test/TestDAP4Client.java @@ -0,0 +1,406 @@ +package dap4.test; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import ucar.httpservices.HTTPException; +import ucar.httpservices.HTTPFactory; +import ucar.httpservices.HTTPMethod; +import ucar.nc2.dataset.NetcdfDataset; +import ucar.unidata.util.test.UnitTestCommon; +import java.io.IOException; +import java.io.StringWriter; +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.List; + +/** + * Test OpenDap Server at the NetcdfDataset level + */ +public class TestDAP4Client extends DapTestCommon { + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + static final boolean DEBUG = false; + + static final boolean BROKEN = false; // on/off known broken tests + + static final boolean BUILDBASELINE = false; + + static final boolean NCDUMP = true; // Use NcDumpW instead of NCPrint + + static final String EXTENSION = (NCDUMP ? "ncdump" : "dmp"); + + static final String TESTEXTENSION = "dmr"; + + // Mnemonic + static final boolean HEADERONLY = false; + + ////////////////////////////////////////////////// + // Constants + + static final String SERVLETPATH = "d4ts/testfiles"; + + static final String DATADIR = "src/test/data"; // relative to dap4 root + static final String BASELINEDIR = "TestDAP4Client/baseline"; + + // Define the names of the xfail tests + static final String[] XFAIL_TESTS = {}; + + static boolean isXfailTest(String t) { + for (String s : XFAIL_TESTS) { + if (s.equals(t)) + return true; + } + return false; + } + + ////////////////////////////////////////////////// + // Type Declarations + + static class Server { + public static String SERVLET = "dts"; + public String ip; + public String port; + + public Server(String ip, String port) { + this.ip = ip; + this.port = port; + } + + public String getURL() { + StringBuilder buf = new StringBuilder(); + buf.append("http://"); + buf.append(this.ip); + if (port != null) { + buf.append(":"); + buf.append(this.port); + } + return buf.toString(); + } + + // Return a URL for testing if server is up/down + public String getTestURL() { + StringBuilder baseurl = new StringBuilder().append(getURL()); + baseurl.append("/"); + baseurl.append(SERVLET); + return baseurl.toString(); + } + + } + + static class ClientTest { + static String root = null; + static String server = null; + static String servlet = null; + static int counter = 0; + + boolean checksumming = true; + boolean xfail = false; + boolean headeronly = false; + + String title; + String dataset; // path minus the server url part. + String baselinepath; + String constraint; + int id; + + ClientTest(String dataset) { + this(0, dataset, null); + } + + ClientTest(int id, String datasetpath, String constraint) { + if (constraint == null) + constraint = ""; + // Break off the final file set name + int index = datasetpath.lastIndexOf('/'); + this.dataset = datasetpath.substring(index + 1, datasetpath.length()); + this.title = this.dataset; + this.id = id; + this.constraint = (constraint.length() == 0 ? null : constraint); + this.baselinepath = root + "/" + BASELINEDIR + "/" + dataset; + if (this.constraint != null) + this.baselinepath += ("." + String.valueOf(this.id)); + } + + public ClientTest nochecksum() { + this.checksumming = false; + return this; + } + + public ClientTest xfail() { + this.xfail = true; + return this; + } + + public ClientTest headeronly() { + this.headeronly = true; + return this; + } + + String makeurl() { + String url = this.server + "/" + this.servlet + "/" + this.dataset; + if (constraint != null) + url += ("?" + constraint); + url += "#dap4"; + return url; + } + + public String toString() { + return dataset; + } + } + + ////////////////////////////////////////////////// + // Class variables + + // Order is important; testing reachability is in the order listed + static List SERVERS; + + static { + SERVERS = new ArrayList<>(); + SERVERS.add(new Server("149.165.169.123", "8080")); + SERVERS.add(new Server("remotetest.unidata.ucar.edu", null)); + } + + ////////////////////////////////////////////////// + // Instance variables + + // Test cases + + List alltestcases = new ArrayList(); + List chosentests = new ArrayList(); + + String datasetpath = null; + + Server server = null; + + ////////////////////////////////////////////////// + + @Before + public void setup() throws Exception { + // Find the server to use + this.server = null; + for (Server svc : SERVERS) { + String url = svc.getTestURL(); + try (HTTPMethod method = HTTPFactory.Get(url)) { + try { + int code = method.execute(); + if (code == 200) { + this.server = svc; + System.out.println("Using server url " + url); + break; + } + } catch (HTTPException e) { + this.server = null; + } + } + } + if (this.server == null) + throw new Exception("Cannot locate server"); + defineAllTestcases(this.getResourceDir(), SERVLETPATH, this.server.getURL()); + chooseTestcases(); + if (BUILDBASELINE) + prop_baseline = true; + } + + ////////////////////////////////////////////////// + // Define test cases + + void chooseTestcases() { + if (false) { + chosentests = locate("test_atomic_types.nc"); + prop_baseline = true; + } else { + for (ClientTest tc : alltestcases) { + chosentests.add(tc); + } + } + } + + boolean defineAllTestcases(String root, String servlet, String server) { + boolean what = HEADERONLY; + ClientTest.root = root; + ClientTest.server = server; + ClientTest.servlet = servlet; + alltestcases.add(new ClientTest("test_atomic_array.nc")); + alltestcases.add(new ClientTest("test_atomic_types.nc")); + alltestcases.add(new ClientTest("test_enum.nc")); + alltestcases.add(new ClientTest("test_enum_2.nc")); + alltestcases.add(new ClientTest("test_enum_array.nc")); + alltestcases.add(new ClientTest("test_enum1.nc")); + alltestcases.add(new ClientTest("test_fill.nc")); + alltestcases.add(new ClientTest("test_groups1.nc")); + if (BROKEN) + alltestcases.add(new ClientTest("test_misc1.nc")); // 0 size unlimited + if (BROKEN) + alltestcases.add(new ClientTest("test_one_var.nc")); // 0 size unlimited + alltestcases.add(new ClientTest("test_one_vararray.nc")); + alltestcases.add(new ClientTest("test_opaque.nc")); + alltestcases.add(new ClientTest("test_opaque_array.nc")); + alltestcases.add(new ClientTest("test_struct_array.nc")); + alltestcases.add(new ClientTest("test_struct_nested.nc")); + alltestcases.add(new ClientTest("test_struct_nested3.nc")); + alltestcases.add(new ClientTest("test_struct_type.nc")); + alltestcases.add(new ClientTest("test_struct1.nc")); + alltestcases.add(new ClientTest("test_test.nc")); + if (BROKEN) + alltestcases.add(new ClientTest("test_unlim.nc")); // ? + if (BROKEN) + alltestcases.add(new ClientTest("test_unlim1.nc")); // ? + if (BROKEN) + alltestcases.add(new ClientTest("test_utf8.nc")); // ? + alltestcases.add(new ClientTest("test_vlen1.nc")); + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen2.nc")); // non scalar vlen + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen3.nc")); // non scalar vlen + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen4.nc")); // non scalar vlen + alltestcases.add(new ClientTest("test_vlen5.nc")); + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen6.nc")); // non-scalar vlen + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen7.nc")); // non-scalar vlen + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen8.nc")); // non-scalar vlen + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen9.nc")); // non-scalar + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen10.nc")); // non-scalar + if (BROKEN) + alltestcases.add(new ClientTest("test_vlen11.nc")); // unknown failure + if (BROKEN) + alltestcases.add(new ClientTest("test_zerodim.nc")); // non-scalar seq + alltestcases.add(new ClientTest("tst_fills.nc")); + for (ClientTest test : alltestcases) { + if (what == HEADERONLY) + test.headeronly(); + } + return true; + } + + ////////////////////////////////////////////////// + // Junit test method + + @Test + public void testDAP4Client() throws Exception { + boolean pass = true; + for (ClientTest testcase : chosentests) { + if (!doOneTest(testcase)) + pass = false; + } + Assert.assertTrue("*** Fail: TestDAP4Client", pass); + } + + ////////////////////////////////////////////////// + // Primary test method + + boolean doOneTest(ClientTest testcase) throws Exception { + boolean pass = true; + System.out.println("Testcase: " + testcase.dataset); + String url = testcase.makeurl(); + NetcdfDataset ncfile = null; + try { + ncfile = openDataset(url); + } catch (Exception e) { + System.err.println(testcase.xfail ? "XFail" : "Fail"); + e.printStackTrace(); + return testcase.xfail; + } + String usethisname = UnitTestCommon.extractDatasetname(url, null); + String metadata = (NCDUMP ? ncdumpmetadata(ncfile, usethisname) : null); + if (prop_visual) { + visual(testcase.title + ".dmr", metadata); + } + + String data = null; + if (!testcase.headeronly) { + data = (NCDUMP ? ncdumpdata(ncfile, usethisname) : null); + if (prop_visual) { + visual(testcase.title + ".dap", data); + } + } + + String testoutput = (testcase.headeronly ? metadata : (NCDUMP ? data : metadata + data)); + + String baselinefile = testcase.baselinepath + "." + EXTENSION; + + if (prop_baseline) + writefile(baselinefile, testoutput); + + if (prop_diff) { // compare with baseline + // Read the baseline file(s) + String baselinecontent = readfile(baselinefile); + System.out.println("Comparison: vs " + baselinefile); + pass = pass && same(getTitle(), baselinecontent, testoutput); + System.out.println(pass ? "Pass" : "Fail"); + } + return pass; + } + + + String ncdumpmetadata(NetcdfDataset ncfile, String datasetname) throws Exception { + StringWriter sw = new StringWriter(); + + StringBuilder args = new StringBuilder("-strict"); + if (datasetname != null) { + args.append(" -datasetname "); + args.append(datasetname); + } + // Print the meta-databuffer using these args to NcdumpW + try { + if (!ucar.nc2.NCdumpW.print(ncfile, args.toString(), sw, null)) + throw new Exception("NcdumpW failed"); + } catch (IOException ioe) { + throw new Exception("NcdumpW failed", ioe); + } + sw.close(); + return sw.toString(); + } + + String ncdumpdata(NetcdfDataset ncfile, String datasetname) throws Exception { + StringWriter sw = new StringWriter(); + + StringBuilder args = new StringBuilder("-strict -vall"); + if (datasetname != null) { + args.append(" -datasetname "); + args.append(datasetname); + } + + // Dump the databuffer + sw = new StringWriter(); + try { + if (!ucar.nc2.NCdumpW.print(ncfile, args.toString(), sw, null)) + throw new Exception("NCdumpW failed"); + } catch (IOException ioe) { + ioe.printStackTrace(); + throw new Exception("NCdumpW failed", ioe); + } + sw.close(); + return sw.toString(); + } + + ////////////////////////////////////////////////// + // Utility methods + + + // Locate the test cases with given prefix + List locate(String prefix) { + List results = new ArrayList(); + for (ClientTest ct : this.alltestcases) { + if (!ct.dataset.startsWith(prefix)) + continue; + results.add(ct); + } + return results; + } + + static boolean report(String msg) { + System.err.println(msg); + return false; + } + +} // class TestDAP4Client + diff --git a/dap4/d4tests/src/test/java/dap4/test/TestHyrax.java b/dap4/d4tests/src/test/java/dap4/test/TestHyrax.java index c4b678aa8f..3dfa0006ce 100644 --- a/dap4/d4tests/src/test/java/dap4/test/TestHyrax.java +++ b/dap4/d4tests/src/test/java/dap4/test/TestHyrax.java @@ -21,7 +21,6 @@ /** * Test OpenDap Server at the NetcdfDataset level */ - @Ignore public class TestHyrax extends DapTestCommon { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); diff --git a/netcdf4/src/main/java/ucar/nc2/jni/netcdf/Nc4Iosp.java b/netcdf4/src/main/java/ucar/nc2/jni/netcdf/Nc4Iosp.java index af411373e0..2aa52af3b0 100755 --- a/netcdf4/src/main/java/ucar/nc2/jni/netcdf/Nc4Iosp.java +++ b/netcdf4/src/main/java/ucar/nc2/jni/netcdf/Nc4Iosp.java @@ -1072,7 +1072,7 @@ private Variable makeVariable(Group g, Structure parent, String vname, int typei } if (dtype.isEnum()) { - EnumTypedef enumTypedef = g.findEnumeration(utype.name); + EnumTypedef enumTypedef = g.findEnumeration(utype.name, true); v.setEnumTypedef(enumTypedef); } else if (dtype == DataType.OPAQUE) { // TODO whats the problem with knowing the size?? Needed to read properly??