From 338726902cf8e582d880c7de872211501c2b7311 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Fri, 27 Oct 2023 15:11:07 -0600 Subject: [PATCH 01/36] Add test to reproduce issue with reading aggregation variable that is renamed in ncml --- .../ncml/TestRenameVariableInAggregation.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 cdm/core/src/test/java/ucar/nc2/internal/ncml/TestRenameVariableInAggregation.java diff --git a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestRenameVariableInAggregation.java b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestRenameVariableInAggregation.java new file mode 100644 index 0000000000..24a248856e --- /dev/null +++ b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestRenameVariableInAggregation.java @@ -0,0 +1,67 @@ +package ucar.nc2.internal.ncml; + +import static com.google.common.truth.Truth.assertThat; + +import java.io.IOException; +import java.io.StringReader; +import org.junit.Test; +import ucar.ma2.Array; +import ucar.ma2.InvalidRangeException; +import ucar.ma2.Section; +import ucar.nc2.NetcdfFile; +import ucar.nc2.Variable; + +public class TestRenameVariableInAggregation { + private static final double TOLERANCE = 1.0e-6; + + @Test + public void shouldRenameVariableUsedInAggregation() throws IOException, InvalidRangeException { + final String ncml = "\n" // leavit + + "\n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + ""; // leavit + + checkNcmlDataset(ncml); + } + + @Test + public void shouldRenameVariableUsedInAggregationWithScan() throws IOException, InvalidRangeException { + final String ncml = "\n" // leavit + + "\n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + ""; // leavit + + checkNcmlDataset(ncml); + } + + private static void checkNcmlDataset(String ncml) throws InvalidRangeException, IOException { + try (NetcdfFile ncfile = NcmlReader.readNcml(new StringReader(ncml), null, null).build()) { + final Variable oldVariable = ncfile.findVariable("T"); + assertThat((Object) oldVariable).isNull(); + final Variable newVariable = ncfile.findVariable("temperature"); + assertThat((Object) newVariable).isNotNull(); + + final Array partialArray = newVariable.read(new Section("0:2, 0:0, 0:0")); + assertThat(partialArray.getSize()).isEqualTo(3); + assertThat(partialArray.getDouble(0)).isWithin(TOLERANCE).of(0.0); + assertThat(partialArray.getDouble(1)).isWithin(TOLERANCE).of(100.0); + assertThat(partialArray.getDouble(2)).isWithin(TOLERANCE).of(200.0); + + final Array array = newVariable.read(); + assertThat(array.getSize()).isEqualTo(36); + assertThat(array.getDouble(0)).isWithin(TOLERANCE).of(0.0); + assertThat(array.getDouble(12)).isWithin(TOLERANCE).of(100.0); + assertThat(array.getDouble(24)).isWithin(TOLERANCE).of(200.0); + } + } +} From 31301e031cf2f935fe86a65c0d60b81e4b3ef4ea Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Mon, 30 Oct 2023 11:13:11 -0600 Subject: [PATCH 02/36] Be more consistent in error handling in aggregation variable reading --- .../src/main/java/ucar/nc2/internal/ncml/AggDataset.java | 4 ++++ .../main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java | 6 ++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDataset.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDataset.java index 92557b33b5..53ba1f1d84 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDataset.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDataset.java @@ -145,6 +145,10 @@ protected Array read(Variable mainv, CancelTask cancelTask) throws IOException { return null; Variable v = findVariable(ncd, mainv); + if (v == null) { + Aggregation.logger.error("AggDataset can't find " + mainv.getFullName() + " in " + ncd.getLocation()); + throw new IllegalArgumentException("Variable '" + mainv.getFullName() + "' does not exist in aggregation."); + } if (debugRead) System.out.printf("Agg.read %s from %s in %s%n", mainv.getNameAndDimensions(), v.getNameAndDimensions(), getLocation()); diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java index 8508dea591..697374bc94 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java @@ -285,10 +285,8 @@ protected Array read(Variable mainv, CancelTask cancelTask, List section) Variable v = findVariable(ncd, mainv); if (v == null) { - Aggregation.logger.error("AggOuterDimension cant find " + mainv.getFullName() + " in " + ncd.getLocation() - + "; return all zeroes!!!"); - return Array.factory(mainv.getDataType(), new Section(section).getShape()); // all zeros LOOK need missing - // value + Aggregation.logger.error("AggOuterDimension can't find " + mainv.getFullName() + " in " + ncd.getLocation()); + throw new IllegalArgumentException("Variable '" + mainv.getFullName() + "' does not exist in aggregation."); } if (Aggregation.debugRead) { From cd168566727085b0d807185b4a2bbeb288f424f8 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Mon, 30 Oct 2023 13:58:21 -0600 Subject: [PATCH 03/36] Add original name to existing ncml variable that may be renamed --- cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 23fb0d5a06..104f7fa07a 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 @@ -989,7 +989,7 @@ private Optional readVariableExisting(Group.Builder groupBuilder, .orElseThrow(() -> new IllegalStateException("Cant find variable " + nameInFile)); } } - vb.setName(name).setDataType(dtype); + vb.setOriginalName(nameInFile).setName(name).setDataType(dtype); if (typedefS != null) { vb.setEnumTypeName(typedefS); } From cdd73b09cca5a85e902c11cce99940d2f1428fe0 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Fri, 3 Nov 2023 20:30:57 -0600 Subject: [PATCH 04/36] ## Description of Changes re: Issue The DAP4 code was improperly handling the use of the "dap4:" protocol in a url. This PR fixes it by detecting that protocol and doing two actions: 1. Convert the protocol to "https:". 2. Add the fragment "#dap4" to end of the url. Note that in \#1, this should really be https, but test.opendap.org still uses http; one hopes that other servers are setup to redirect http: to https: ## PR Checklist - [X] Link to any issues that the PR addresses - [X] Add labels - [X] Open as a [draft PR](https://github.blog/2019-02-14-introducing-draft-pull-requests/) until ready for review - [X] Make sure GitHub tests pass - [X] Mark PR as "Ready for Review" --- dap4/build.gradle | 2 -- dap4/src/main/java/dap4/core/util/XURI.java | 13 +++++++++++++ .../java/dap4/dap4lib/cdm/nc2/DapNetcdfFile.java | 7 +++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/dap4/build.gradle b/dap4/build.gradle index 287a259f88..5956343144 100644 --- a/dap4/build.gradle +++ b/dap4/build.gradle @@ -32,11 +32,9 @@ test { include 'dap4/test/TestParserCE.class' include 'dap4/test/TestRaw.class' include 'dap4/test/TestDap4Url.class' -if(false) { // disable selected dap4 tests for now include 'dap4/test/TestRemote.class' include 'dap4/test/TestConstraints.class' include 'dap4/test/TestHyrax.class' -} dependsOn('farmBeforeIntegrationTest') finalizedBy('farmAfterIntegrationTest') diff --git a/dap4/src/main/java/dap4/core/util/XURI.java b/dap4/src/main/java/dap4/core/util/XURI.java index 02fde56bd1..74a0a55129 100644 --- a/dap4/src/main/java/dap4/core/util/XURI.java +++ b/dap4/src/main/java/dap4/core/util/XURI.java @@ -265,6 +265,19 @@ public void insertQueryField(String key, String newval) { rebuildQuery(); } + /** + * Allow fragment fields to be inserted + * + * @param key + * @param newval + * @return previous value or this value if key not set + */ + public void insertFragmentField(String key, String newval) { + // Watch out in case there is no query + this.fragfields = insertAmpField(key, newval, parent.getFragment()); + rebuildQuery(); + } + /** * Allow queryfields to be removed * diff --git a/dap4/src/main/java/dap4/dap4lib/cdm/nc2/DapNetcdfFile.java b/dap4/src/main/java/dap4/dap4lib/cdm/nc2/DapNetcdfFile.java index 22c9d7c954..ecd04674ce 100644 --- a/dap4/src/main/java/dap4/dap4lib/cdm/nc2/DapNetcdfFile.java +++ b/dap4/src/main/java/dap4/dap4lib/cdm/nc2/DapNetcdfFile.java @@ -128,6 +128,13 @@ public DapNetcdfFile(String location, CancelTask cancelTask) throws IOException } catch (URISyntaxException use) { throw new IOException(use); } + // We need to convert the protocol to #dap4 + if ("dap4".equalsIgnoreCase(xuri.getScheme())) { + xuri.setScheme("https"); // Note that this should be https, but + // test.opendap.org still uses http; one + // hopes that other servers are setup to + // redirect http: to https: + } this.dsplocation = xuri.assemble(XURI.URLQUERY); cancel = (cancelTask == null ? nullcancel : cancelTask); From 240da2b18d2c9081837767ce7fc3b54aa34b8351 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Sat, 8 Jul 2023 13:22:39 -0600 Subject: [PATCH 05/36] ## Description of Changes The DAP4 specification says how to properly handle checksums, but this was improperly implemented in netcdf-java. This PR makes some changes to handle checksums. WARNING: The merging of this PR needs to be synchronized with the following netcdf-java PR: So after both PRs are reviewed and approved, both need to be merged one right after the other. Note also that their are still unresolved ambiguities about checksums in the DAP4 specification that need resolution. See (here)[https://github.com/OPENDAP/dap4-specification/discussions/6] for example. So at the moment, accessing non-thredds servers via DAP4 requires special treatment. ## PR Checklist - [X] Link to any issues that the PR addresses - [] Add labels - [X] Open as a [draft PR](https://github.blog/2019-02-14-introducing-draft-pull-requests/) until ready for review - [X] Make sure GitHub tests pass - [X] Make sure Jenkins tests pass - [ ] Mark PR as "Ready for Review" ## DAP4 Changes 1. Properly compute and apply checksums in the DAP and DMR responses. 2. Fix tests that require the _DAP4_Checksum_CRC32 attribute. ## Non-DAP4 Changes The only controversial change is that to ucar.ma2.Index, about line 573. The change is as follows: ```` int prefixrank = (hasvlen ? rank: rank - 1); to int prefixrank = (hasvlen ? rank - 1 : rank); ```` The original line is clearly an error under the assumption that "*" dimensions are always the last dimension for an array. I was worried about this causing follow-on failures, but not surprisingly because vlen is so screwed up, no follow-on failures have appeared. --- cdm/core/src/main/java/ucar/ma2/Index.java | 4 ++-- dap4/src/main/java/dap4/core/ce/CEConstraint.java | 2 +- dap4/src/main/java/dap4/core/dmr/DMRPrinter.java | 4 +--- .../baselineremote/test_atomic_array.nc.ncdump | 8 ++++++++ .../baselineremote/test_atomic_types.nc.ncdump | 15 +++++++++++++++ .../baselineremote/test_enum_1.nc.ncdump | 1 + .../baselineremote/test_enum_2.nc.ncdump | 1 + .../baselineremote/test_enum_3.nc.ncdump | 1 + .../baselineremote/test_enum_array.nc.ncdump | 1 + .../resources/baselineremote/test_fill.nc.ncdump | 3 +++ .../baselineremote/test_fill_2.nc.ncdump | 4 ++++ .../baselineremote/test_groups1.nc.ncdump | 4 ++++ .../resources/baselineremote/test_misc1.nc.ncdump | 3 +++ .../baselineremote/test_one_var.nc.ncdump | 1 + .../baselineremote/test_one_vararray.nc.ncdump | 1 + .../baselineremote/test_opaque.nc.ncdump | 1 + .../baselineremote/test_opaque_array.nc.ncdump | 1 + .../baselineremote/test_struct1.nc.ncdump | 3 +++ .../baselineremote/test_struct_array.nc.ncdump | 3 +++ .../baselineremote/test_struct_nested.nc.ncdump | 7 +++++++ .../baselineremote/test_struct_nested3.nc.ncdump | 4 ++++ .../baselineremote/test_struct_type.nc.ncdump | 3 +++ .../resources/baselineremote/test_test.nc.ncdump | 1 + .../resources/baselineremote/test_unlim.nc.ncdump | 4 ++++ .../baselineremote/test_unlim1.nc.ncdump | 4 ++++ .../resources/baselineremote/test_utf8.nc.ncdump | 1 + .../resources/baselineremote/test_vlen1.nc.ncdump | 2 ++ .../baselineremote/test_vlen11.nc.ncdump | 2 ++ .../baselineremote/test_zerodim.nc.ncdump | 3 +++ 29 files changed, 86 insertions(+), 6 deletions(-) diff --git a/cdm/core/src/main/java/ucar/ma2/Index.java b/cdm/core/src/main/java/ucar/ma2/Index.java index 11f6ebb0a6..0efe0856d8 100644 --- a/cdm/core/src/main/java/ucar/ma2/Index.java +++ b/cdm/core/src/main/java/ucar/ma2/Index.java @@ -570,10 +570,10 @@ public Index set(int[] index) { throw new ArrayIndexOutOfBoundsException(); if (rank == 0) return this; - int prefixrank = (hasvlen ? rank : rank - 1); + int prefixrank = (hasvlen ? rank - 1 : rank); System.arraycopy(index, 0, current, 0, prefixrank); if (hasvlen) - current[prefixrank] = -1; + current[rank] = -1; // ?? return this; } diff --git a/dap4/src/main/java/dap4/core/ce/CEConstraint.java b/dap4/src/main/java/dap4/core/ce/CEConstraint.java index d17a188e26..224a089df1 100644 --- a/dap4/src/main/java/dap4/core/ce/CEConstraint.java +++ b/dap4/src/main/java/dap4/core/ce/CEConstraint.java @@ -334,7 +334,7 @@ public List getConstrainedSlices(DapVariable var) throws DapException { Segment seg = findSegment(var); if (seg != null) slices = seg.slices; - if (slices == null) + if (slices == null || slices.size() == 0) slices = Universal.universalSlices(var); return slices; } diff --git a/dap4/src/main/java/dap4/core/dmr/DMRPrinter.java b/dap4/src/main/java/dap4/core/dmr/DMRPrinter.java index 2e4acad3d2..be0d681dee 100644 --- a/dap4/src/main/java/dap4/core/dmr/DMRPrinter.java +++ b/dap4/src/main/java/dap4/core/dmr/DMRPrinter.java @@ -94,7 +94,6 @@ public static String printAsString(DapDataset dmr) { // Following extracted from context protected ByteOrder order = null; protected Map localchecksummap = null; - protected Map remotechecksummap = null; protected EnumSet controls = EnumSet.noneOf(Controls.class); @@ -120,8 +119,7 @@ public DMRPrinter(DapDataset dmr, CEConstraint ce, PrintWriter writer, ResponseF this.format = (format == null ? ResponseFormat.XML : format); this.cxt = (cxt == null ? new DapContext() : cxt); this.order = (ByteOrder) this.cxt.get(DapConstants.DAP4ENDIANTAG); - this.localchecksummap = (Map) this.cxt.get("localchecksummap"); - this.remotechecksummap = (Map) this.cxt.get("remotechecksummap"); + this.localchecksummap = (Map) this.cxt.get("checksummap"); } ////////////////////////////////////////////////// diff --git a/dap4/src/test/data/resources/baselineremote/test_atomic_array.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_atomic_array.nc.ncdump index 448052ed2e..c50f3f6367 100644 --- a/dap4/src/test/data/resources/baselineremote/test_atomic_array.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_atomic_array.nc.ncdump @@ -10,21 +10,29 @@ netcdf test_atomic_array { d5 = 5; variables: ubyte vu8(d2, d3); + vu8:_DAP4_Checksum_CRC32 = -735091578; short v16(d4); + v16:_DAP4_Checksum_CRC32 = -1835461610; uint vu32(d2, d3); + vu32:_DAP4_Checksum_CRC32 = 207024370; double vd(d2); + vd:_DAP4_Checksum_CRC32 = 2081016750; char vc(d2); + vc:_DAP4_Checksum_CRC32 = 1672337415; string vs(d2, d2); + vs:_DAP4_Checksum_CRC32 = 1731162308; opaque vo(d1, d2); + vo:_DAP4_Checksum_CRC32 = 660035085; enum cloud_class_t primary_cloud(d5); string primary_cloud:_FillValue = "Missing"; + primary_cloud:_DAP4_Checksum_CRC32 = -1249222022; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_atomic_types.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_atomic_types.nc.ncdump index b68d5b3fa1..7e39588cff 100644 --- a/dap4/src/test/data/resources/baselineremote/test_atomic_types.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_atomic_types.nc.ncdump @@ -4,36 +4,51 @@ netcdf test_atomic_types { variables: byte v8; + v8:_DAP4_Checksum_CRC32 = 1069182125; ubyte vu8; + vu8:_DAP4_Checksum_CRC32 = -16777216; short v16; + v16:_DAP4_Checksum_CRC32 = -1402891809; ushort vu16; + vu16:_DAP4_Checksum_CRC32 = -65536; int v32; + v32:_DAP4_Checksum_CRC32 = 306674911; uint vu32; + vu32:_DAP4_Checksum_CRC32 = -1; long v64; + v64:_DAP4_Checksum_CRC32 = -855876548; ulong vu64; + vu64:_DAP4_Checksum_CRC32 = 558161692; float vf; + vf:_DAP4_Checksum_CRC32 = -1943399579; double vd; + vd:_DAP4_Checksum_CRC32 = -222639246; char vc; + vc:_DAP4_Checksum_CRC32 = -1528910307; string vs; + vs:_DAP4_Checksum_CRC32 = 915515092; opaque vo; + vo:_DAP4_Checksum_CRC32 = -766649635; enum cloud_class_t primary_cloud; string primary_cloud:_FillValue = "Missing"; + primary_cloud:_DAP4_Checksum_CRC32 = 1007455905; enum cloud_class_t secondary_cloud; string secondary_cloud:_FillValue = "Missing"; + secondary_cloud:_DAP4_Checksum_CRC32 = 314082080; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_enum_1.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_enum_1.nc.ncdump index abcb007674..4b7e2e01aa 100644 --- a/dap4/src/test/data/resources/baselineremote/test_enum_1.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_enum_1.nc.ncdump @@ -5,6 +5,7 @@ netcdf test_enum_1 { variables: enum cloud_class_t primary_cloud; string primary_cloud:_FillValue = "Missing"; + primary_cloud:_DAP4_Checksum_CRC32 = 1007455905; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_enum_2.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_enum_2.nc.ncdump index 651cc35de6..da63517043 100644 --- a/dap4/src/test/data/resources/baselineremote/test_enum_2.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_enum_2.nc.ncdump @@ -6,6 +6,7 @@ netcdf test_enum_2 { variables: enum cloud_class_t primary_cloud; string primary_cloud:_FillValue = "Missing"; + primary_cloud:_DAP4_Checksum_CRC32 = 1007455905; } diff --git a/dap4/src/test/data/resources/baselineremote/test_enum_3.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_enum_3.nc.ncdump index d550189ba8..2c7082e493 100644 --- a/dap4/src/test/data/resources/baselineremote/test_enum_3.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_enum_3.nc.ncdump @@ -5,6 +5,7 @@ netcdf test_enum_3 { group: h { variables: enum cloud_class_t primary_cloud; + primary_cloud:_DAP4_Checksum_CRC32 = -1526341861; } diff --git a/dap4/src/test/data/resources/baselineremote/test_enum_array.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_enum_array.nc.ncdump index b11536aadb..9a5c876bd2 100644 --- a/dap4/src/test/data/resources/baselineremote/test_enum_array.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_enum_array.nc.ncdump @@ -7,6 +7,7 @@ netcdf test_enum_array { variables: enum cloud_class_t primary_cloud(d5); string primary_cloud:_FillValue = "Missing"; + primary_cloud:_DAP4_Checksum_CRC32 = -1249222022; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_fill.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_fill.nc.ncdump index 1a740740b1..d782d2d2ef 100644 --- a/dap4/src/test/data/resources/baselineremote/test_fill.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_fill.nc.ncdump @@ -1,11 +1,14 @@ netcdf test_fill { variables: ubyte uv8; + uv8:_DAP4_Checksum_CRC32 = 1874795921; short v16; + v16:_DAP4_Checksum_CRC32 = -921460762; int uv32; uv32:_FillValue = 17; + uv32:_DAP4_Checksum_CRC32 = -2076724431; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_fill_2.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_fill_2.nc.ncdump index 70d7dd28ae..340a090c61 100644 --- a/dap4/src/test/data/resources/baselineremote/test_fill_2.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_fill_2.nc.ncdump @@ -7,15 +7,19 @@ netcdf test_fill_2 { variables: enum cloud_class_t enumvar(d2); string enumvar:_FillValue = "Missing"; + enumvar:_DAP4_Checksum_CRC32 = -1286267696; int uv32(d2); uv32:_FillValue = 17; + uv32:_DAP4_Checksum_CRC32 = 1019008870; ubyte uv8(d2); uv8:_FillValue = 120B; + uv8:_DAP4_Checksum_CRC32 = 196807244; short v16(d2); v16:_FillValue = -37S; + v16:_DAP4_Checksum_CRC32 = 1719923210; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_groups1.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_groups1.nc.ncdump index e8a21f1643..b068dcf906 100644 --- a/dap4/src/test/data/resources/baselineremote/test_groups1.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_groups1.nc.ncdump @@ -9,8 +9,10 @@ netcdf test_groups1 { dim3 = 7; variables: int v1(dim1); + v1:_DAP4_Checksum_CRC32 = 488449836; float v2(dim2); + v2:_DAP4_Checksum_CRC32 = 266808643; } @@ -19,8 +21,10 @@ netcdf test_groups1 { dim3 = 7; variables: int v1(dim1); + v1:_DAP4_Checksum_CRC32 = -1447780101; float v3(dim3); + v3:_DAP4_Checksum_CRC32 = 703718162; } diff --git a/dap4/src/test/data/resources/baselineremote/test_misc1.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_misc1.nc.ncdump index e7494b8a76..9d1499fc49 100644 --- a/dap4/src/test/data/resources/baselineremote/test_misc1.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_misc1.nc.ncdump @@ -7,12 +7,15 @@ netcdf test_misc1 { variables: float var(unlim); var:_ChunkSizes = 1024; + var:_DAP4_Checksum_CRC32 = -1062079627; float lon(lon); + lon:_DAP4_Checksum_CRC32 = -947018072; string lon:units = "degrees_east"; string lon:_CoordinateAxisType = "Lon"; float lat(lat); + lat:_DAP4_Checksum_CRC32 = -1521140044; string lat:units = "degrees_north"; string lat:_CoordinateAxisType = "Lat"; diff --git a/dap4/src/test/data/resources/baselineremote/test_one_var.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_one_var.nc.ncdump index dc9ffc41ce..db831e0144 100644 --- a/dap4/src/test/data/resources/baselineremote/test_one_var.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_one_var.nc.ncdump @@ -1,6 +1,7 @@ netcdf test_one_var { variables: int t; + t:_DAP4_Checksum_CRC32 = -907939866; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_one_vararray.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_one_vararray.nc.ncdump index 6f4c930944..b9cb4381e0 100644 --- a/dap4/src/test/data/resources/baselineremote/test_one_vararray.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_one_vararray.nc.ncdump @@ -3,6 +3,7 @@ netcdf test_one_vararray { d2 = 2; variables: int t(d2); + t:_DAP4_Checksum_CRC32 = 1121956304; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_opaque.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_opaque.nc.ncdump index 60e1ee1b7b..305505cfbd 100644 --- a/dap4/src/test/data/resources/baselineremote/test_opaque.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_opaque.nc.ncdump @@ -1,6 +1,7 @@ netcdf test_opaque { variables: opaque vo1; + vo1:_DAP4_Checksum_CRC32 = -766649635; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_opaque_array.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_opaque_array.nc.ncdump index 07790cf395..59b3c48c23 100644 --- a/dap4/src/test/data/resources/baselineremote/test_opaque_array.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_opaque_array.nc.ncdump @@ -3,6 +3,7 @@ netcdf test_opaque_array { d2 = 2; variables: opaque vo2(d2, d2); + vo2:_DAP4_Checksum_CRC32 = -1856496422; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_struct1.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_struct1.nc.ncdump index b647e4efc8..67f9dc4c52 100644 --- a/dap4/src/test/data/resources/baselineremote/test_struct1.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_struct1.nc.ncdump @@ -3,10 +3,13 @@ netcdf test_struct1 { Structure { int x; + x:_DAP4_Checksum_CRC32 = 0; string x:_CoordinateAxisType = "GeoX"; int y; + y:_DAP4_Checksum_CRC32 = 0; string y:_CoordinateAxisType = "GeoY"; } s; + s:_DAP4_Checksum_CRC32 = -812672911; // global attributes: diff --git a/dap4/src/test/data/resources/baselineremote/test_struct_array.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_struct_array.nc.ncdump index 5d64ae742d..50c13b5e49 100644 --- a/dap4/src/test/data/resources/baselineremote/test_struct_array.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_struct_array.nc.ncdump @@ -6,10 +6,13 @@ netcdf test_struct_array { Structure { int x; + x:_DAP4_Checksum_CRC32 = 0; string x:_CoordinateAxisType = "GeoX"; int y; + y:_DAP4_Checksum_CRC32 = 0; string y:_CoordinateAxisType = "GeoY"; } s(dx, dy); + s:_DAP4_Checksum_CRC32 = 788761034; // global attributes: diff --git a/dap4/src/test/data/resources/baselineremote/test_struct_nested.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_struct_nested.nc.ncdump index 416611fbbc..f903015485 100644 --- a/dap4/src/test/data/resources/baselineremote/test_struct_nested.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_struct_nested.nc.ncdump @@ -5,18 +5,25 @@ netcdf test_struct_nested { Structure { int x; + x:_DAP4_Checksum_CRC32 = 0; string x:_CoordinateAxisType = "GeoX"; int y; + y:_DAP4_Checksum_CRC32 = 0; string y:_CoordinateAxisType = "GeoY"; } field1; + field1:_DAP4_Checksum_CRC32 = 0; Structure { int x; + x:_DAP4_Checksum_CRC32 = 0; int y; + y:_DAP4_Checksum_CRC32 = 0; } field2; + field2:_DAP4_Checksum_CRC32 = 0; } x; + x:_DAP4_Checksum_CRC32 = -542685669; // global attributes: diff --git a/dap4/src/test/data/resources/baselineremote/test_struct_nested3.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_struct_nested3.nc.ncdump index 626573d139..8e5a9d5c9f 100644 --- a/dap4/src/test/data/resources/baselineremote/test_struct_nested3.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_struct_nested3.nc.ncdump @@ -7,11 +7,15 @@ netcdf test_struct_nested3 { Structure { int field1; + field1:_DAP4_Checksum_CRC32 = 0; } field2; + field2:_DAP4_Checksum_CRC32 = 0; } field3; + field3:_DAP4_Checksum_CRC32 = 0; } x; + x:_DAP4_Checksum_CRC32 = -907939866; // global attributes: diff --git a/dap4/src/test/data/resources/baselineremote/test_struct_type.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_struct_type.nc.ncdump index dc6deedce7..d5a67f4268 100644 --- a/dap4/src/test/data/resources/baselineremote/test_struct_type.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_struct_type.nc.ncdump @@ -3,10 +3,13 @@ netcdf test_struct_type { Structure { int x; + x:_DAP4_Checksum_CRC32 = 0; string x:_CoordinateAxisType = "GeoX"; int y; + y:_DAP4_Checksum_CRC32 = 0; string y:_CoordinateAxisType = "GeoY"; } s; + s:_DAP4_Checksum_CRC32 = -812672911; // global attributes: diff --git a/dap4/src/test/data/resources/baselineremote/test_test.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_test.nc.ncdump index aa94dd1cc3..fdd4fac109 100644 --- a/dap4/src/test/data/resources/baselineremote/test_test.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_test.nc.ncdump @@ -4,6 +4,7 @@ netcdf test_test { variables: enum enum_t v1; + v1:_DAP4_Checksum_CRC32 = -1526341861; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_unlim.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_unlim.nc.ncdump index ff378e3fac..1d8b772f04 100644 --- a/dap4/src/test/data/resources/baselineremote/test_unlim.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_unlim.nc.ncdump @@ -5,21 +5,25 @@ netcdf test_unlim { lat = 3; variables: float lon(lon); + lon:_DAP4_Checksum_CRC32 = 27072276; string lon:units = "degrees_east"; string lon:_CoordinateAxisType = "Lon"; float pr(time, lat, lon); pr:_ChunkSizes = 1, 3, 2; + pr:_DAP4_Checksum_CRC32 = 826391215; string pr:standard_name = "air_pressure_at_sea_level"; string pr:units = "hPa"; string pr:_CoordinateAxisType = "Pressure"; double time(time); time:_ChunkSizes = 512; + time:_DAP4_Checksum_CRC32 = -1907298714; string time:units = "seconds since 2009-01-01"; string time:_CoordinateAxisType = "Time"; float lat(lat); + lat:_DAP4_Checksum_CRC32 = 431075914; string lat:units = "degrees_north"; string lat:_CoordinateAxisType = "Lat"; diff --git a/dap4/src/test/data/resources/baselineremote/test_unlim1.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_unlim1.nc.ncdump index 496abbabdc..fded68ca09 100644 --- a/dap4/src/test/data/resources/baselineremote/test_unlim1.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_unlim1.nc.ncdump @@ -5,21 +5,25 @@ netcdf test_unlim1 { time = 2; variables: float lon(lon); + lon:_DAP4_Checksum_CRC32 = 27072276; string lon:units = "degrees_east"; string lon:_CoordinateAxisType = "Lon"; float pr(time, lat, lon); pr:_ChunkSizes = 1, 3, 2; + pr:_DAP4_Checksum_CRC32 = 826391215; string pr:standard_name = "air_pressure_at_sea_level"; string pr:units = "hPa"; string pr:_CoordinateAxisType = "Pressure"; double time(time); time:_ChunkSizes = 512; + time:_DAP4_Checksum_CRC32 = -1907298714; string time:units = "seconds since 2009-01-01"; string time:_CoordinateAxisType = "Time"; float lat(lat); + lat:_DAP4_Checksum_CRC32 = 431075914; string lat:units = "degrees_north"; string lat:_CoordinateAxisType = "Lat"; diff --git a/dap4/src/test/data/resources/baselineremote/test_utf8.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_utf8.nc.ncdump index 067d42687d..7d88163dbf 100644 --- a/dap4/src/test/data/resources/baselineremote/test_utf8.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_utf8.nc.ncdump @@ -3,6 +3,7 @@ netcdf test_utf8 { d2 = 2; variables: string vs(d2); + vs:_DAP4_Checksum_CRC32 = -52179672; // global attributes: :_DAP4_Little_Endian = 1B; diff --git a/dap4/src/test/data/resources/baselineremote/test_vlen1.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_vlen1.nc.ncdump index 7a0f561084..7cf82539f9 100644 --- a/dap4/src/test/data/resources/baselineremote/test_vlen1.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_vlen1.nc.ncdump @@ -3,9 +3,11 @@ netcdf test_vlen1 { Sequence { int x; + x:_DAP4_Checksum_CRC32 = 0; string x:_CoordinateAxisType = "GeoX"; } x(*); x:_FillValue = 0; + x:_DAP4_Checksum_CRC32 = -972676669; // global attributes: diff --git a/dap4/src/test/data/resources/baselineremote/test_vlen11.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_vlen11.nc.ncdump index 8474f2c1a2..3716a262a8 100644 --- a/dap4/src/test/data/resources/baselineremote/test_vlen11.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_vlen11.nc.ncdump @@ -3,8 +3,10 @@ netcdf test_vlen11 { Sequence { int v; + v:_DAP4_Checksum_CRC32 = 0; } v(*); v:_FillValue = 0; + v:_DAP4_Checksum_CRC32 = -1061524850; // global attributes: diff --git a/dap4/src/test/data/resources/baselineremote/test_zerodim.nc.ncdump b/dap4/src/test/data/resources/baselineremote/test_zerodim.nc.ncdump index 3e0a65caae..e25d61f5f3 100644 --- a/dap4/src/test/data/resources/baselineremote/test_zerodim.nc.ncdump +++ b/dap4/src/test/data/resources/baselineremote/test_zerodim.nc.ncdump @@ -7,12 +7,15 @@ netcdf test_zerodim { variables: float var(unlim); var:_ChunkSizes = 1024; + var:_DAP4_Checksum_CRC32 = -1062079627; float lon(lon); + lon:_DAP4_Checksum_CRC32 = -947018072; string lon:units = "degrees_east"; string lon:_CoordinateAxisType = "Lon"; float lat(lat); + lat:_DAP4_Checksum_CRC32 = -1521140044; string lat:units = "degrees_north"; string lat:_CoordinateAxisType = "Lat"; From 0d0831a81188250e428eb879ec0662f9bffbd895 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Mon, 6 Nov 2023 13:16:34 -0700 Subject: [PATCH 06/36] Convert List.size()==0 to List.isEmpty() --- dap4/src/main/java/dap4/core/ce/CEConstraint.java | 2 +- dap4/src/main/java/dap4/core/util/SliceConstraint.java | 2 +- dap4/src/main/java/dap4/dap4lib/cdm/CDMUtil.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dap4/src/main/java/dap4/core/ce/CEConstraint.java b/dap4/src/main/java/dap4/core/ce/CEConstraint.java index 224a089df1..3ae2d581d6 100644 --- a/dap4/src/main/java/dap4/core/ce/CEConstraint.java +++ b/dap4/src/main/java/dap4/core/ce/CEConstraint.java @@ -334,7 +334,7 @@ public List getConstrainedSlices(DapVariable var) throws DapException { Segment seg = findSegment(var); if (seg != null) slices = seg.slices; - if (slices == null || slices.size() == 0) + if (slices == null || slices.isEmpty() == 0) slices = Universal.universalSlices(var); return slices; } diff --git a/dap4/src/main/java/dap4/core/util/SliceConstraint.java b/dap4/src/main/java/dap4/core/util/SliceConstraint.java index 2512165df2..30e9452ec3 100644 --- a/dap4/src/main/java/dap4/core/util/SliceConstraint.java +++ b/dap4/src/main/java/dap4/core/util/SliceConstraint.java @@ -63,7 +63,7 @@ public String toString() { } protected void add(List slices) throws DapException { - if (slices == null || slices.size() == 0) + if (slices == null || slices.isEmpty()) throw new DapException("Null slice set"); if (this.slicesets.size() == this.rank) throw new DapException("Sliceset overflow"); diff --git a/dap4/src/main/java/dap4/dap4lib/cdm/CDMUtil.java b/dap4/src/main/java/dap4/dap4lib/cdm/CDMUtil.java index 7b964fb8a9..9bd5e9d5c7 100644 --- a/dap4/src/main/java/dap4/dap4lib/cdm/CDMUtil.java +++ b/dap4/src/main/java/dap4/dap4lib/cdm/CDMUtil.java @@ -179,7 +179,7 @@ public static NetcdfFile unwrapfile(NetcdfFile file) { } public static boolean hasVLEN(List ranges) { - if (ranges == null || ranges.size() == 0) + if (ranges == null || ranges.isEmpty()) return false; return ranges.get(ranges.size() - 1) == Range.VLEN; } @@ -213,7 +213,7 @@ public static boolean containsVLEN(List dimset) { * @return effective shape */ public static int[] computeEffectiveShape(List dimset) { - if (dimset == null || dimset.size() == 0) + if (dimset == null || dimset.isEmpty()) return new int[0]; int effectiverank = dimset.size(); int[] shape = new int[effectiverank]; From 442b0fb6f333599391d14593eeae033e8fda4018 Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Mon, 6 Nov 2023 13:31:09 -0700 Subject: [PATCH 07/36] spotify --- dap4/src/main/java/dap4/core/ce/CEConstraint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dap4/src/main/java/dap4/core/ce/CEConstraint.java b/dap4/src/main/java/dap4/core/ce/CEConstraint.java index 3ae2d581d6..703def6799 100644 --- a/dap4/src/main/java/dap4/core/ce/CEConstraint.java +++ b/dap4/src/main/java/dap4/core/ce/CEConstraint.java @@ -334,7 +334,7 @@ public List getConstrainedSlices(DapVariable var) throws DapException { Segment seg = findSegment(var); if (seg != null) slices = seg.slices; - if (slices == null || slices.isEmpty() == 0) + if (slices == null || slices.isEmpty()) slices = Universal.universalSlices(var); return slices; } From 736567ae0e024e62aae8da2ab62e0d57ea63204f Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Fri, 27 Oct 2023 10:36:24 -0600 Subject: [PATCH 08/36] Add test that feature collection best time in strictly monotonically increasing --- .../grib/coord/TestDiscontiguousInterval.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java b/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java index ab83386ced..3e749b91b5 100644 --- a/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java +++ b/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java @@ -1,10 +1,16 @@ package ucar.nc2.grib.coord; +import static com.google.common.truth.Truth.assertThat; + import org.junit.Assert; import org.junit.Test; import org.junit.experimental.categories.Category; +import thredds.featurecollection.FeatureCollectionConfig; +import thredds.featurecollection.FeatureCollectionType; +import thredds.inventory.CollectionUpdateType; import ucar.ma2.Array; import ucar.nc2.*; +import ucar.nc2.grib.collection.GribCdmIndex; import ucar.unidata.util.test.TestDir; import ucar.unidata.util.test.category.NeedsCdmUnitTest; @@ -103,10 +109,30 @@ public void testTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing_Data checkTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing(testfile, varName); } + @Test + @Category(NeedsCdmUnitTest.class) + public void testFeatureColectionBest_isStrictlyMonotonicallyIncreasing() throws IOException { + final String spec = + TestDir.cdmUnitTestDir + "/gribCollections/gfs_conus80/20231027/GFS_CONUS_80km_#yyyyMMdd_HHmm#\\.grib1$"; + final FeatureCollectionConfig config = new FeatureCollectionConfig("testFeatureCollectionBest", "path", + FeatureCollectionType.GRIB1, spec, null, null, null, "file", null); + final boolean changed = GribCdmIndex.updateGribCollection(config, CollectionUpdateType.always, null); + assertThat(changed).isTrue(); + final String topLevelIndex = GribCdmIndex.getTopIndexFileFromConfig(config).getAbsolutePath(); + + final String varName = "Total_precipitation_surface_Mixed_intervals_Accumulation"; + checkTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing(topLevelIndex, varName, "Best/"); + } + private void checkTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing(String testfile, String varName) throws IOException { + checkTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing(testfile, varName, ""); + } + + private void checkTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing(String testfile, String varName, + String groupName) throws IOException { try (NetcdfFile nc = NetcdfFiles.open(testfile)) { - Variable dataVar = nc.findVariable(varName); + Variable dataVar = nc.findVariable(groupName + varName); Assert.assertNotNull(dataVar); Dimension timeDim = null; @@ -116,7 +142,7 @@ private void checkTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing(St break; } } - Variable timeCoordVar = nc.findVariable(timeDim.getShortName()); + Variable timeCoordVar = nc.findVariable(groupName + timeDim.getShortName()); Assert.assertNotNull(timeCoordVar); Attribute att = timeCoordVar.findAttribute("bounds"); From 6d7d6e9f6d750524674388281645eee5938e0b50 Mon Sep 17 00:00:00 2001 From: Ethan Davis Date: Tue, 7 Nov 2023 15:26:06 -0700 Subject: [PATCH 09/36] Fix time coords from intervals for feature collection Best time case. --- .../nc2/grib/collection/GribIospBuilder.java | 21 +++++++------------ .../grib/coord/TestDiscontiguousInterval.java | 2 +- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/grib/src/main/java/ucar/nc2/grib/collection/GribIospBuilder.java b/grib/src/main/java/ucar/nc2/grib/collection/GribIospBuilder.java index 929e8d31f1..b61a3173c6 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/GribIospBuilder.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/GribIospBuilder.java @@ -552,13 +552,12 @@ private void makeTimeCoordinate1D(Group.Builder g, CoordinateTimeIntv coordTime) v.addAttribute(new Attribute(CDM.LONG_NAME, Grib.GRIB_VALID_TIME)); v.addAttribute(new Attribute(CF.CALENDAR, Calendar.proleptic_gregorian.toString())); - double[] data = new double[ntimes]; - int count = 0; - - for (TimeCoordIntvValue tinv : coordTime.getTimeIntervals()) { - data[count++] = tinv.getCoordValue(); - } - v.setCachedData(Array.factory(DataType.DOUBLE, new int[] {ntimes}, data), false); + double[] timeCoordValues = new double[ntimes]; + double[] timeCoordBoundsValues = new double[ntimes * 2]; + int count = GribTimeCoordIntervalUtils.generateTimeCoordValuesFromTimeCoordIntervals(coordTime.getTimeIntervals(), + timeCoordValues, timeCoordBoundsValues, 0, coordTime.getTimeUnit().getValue(), 0); + assert (count == ntimes); + v.setCachedData(Array.factory(DataType.DOUBLE, new int[] {ntimes}, timeCoordValues), false); // bounds String bounds_name = tcName + "_bounds"; @@ -569,13 +568,7 @@ private void makeTimeCoordinate1D(Group.Builder g, CoordinateTimeIntv coordTime) bounds.addAttribute(new Attribute(CDM.UNITS, units)); bounds.addAttribute(new Attribute(CDM.LONG_NAME, "bounds for " + tcName)); - data = new double[ntimes * 2]; - count = 0; - for (TimeCoordIntvValue tinv : coordTime.getTimeIntervals()) { - data[count++] = tinv.getBounds1(); - data[count++] = tinv.getBounds2(); - } - bounds.setCachedData(Array.factory(DataType.DOUBLE, new int[] {ntimes, 2}, data), false); + bounds.setCachedData(Array.factory(DataType.DOUBLE, new int[] {ntimes, 2}, timeCoordBoundsValues), false); makeTimeAuxReference(g, tcName, units, coordTime); } diff --git a/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java b/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java index 3e749b91b5..456418f865 100644 --- a/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java +++ b/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java @@ -111,7 +111,7 @@ public void testTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing_Data @Test @Category(NeedsCdmUnitTest.class) - public void testFeatureColectionBest_isStrictlyMonotonicallyIncreasing() throws IOException { + public void testFeatureCollectionBest_isStrictlyMonotonicallyIncreasing() throws IOException { final String spec = TestDir.cdmUnitTestDir + "/gribCollections/gfs_conus80/20231027/GFS_CONUS_80km_#yyyyMMdd_HHmm#\\.grib1$"; final FeatureCollectionConfig config = new FeatureCollectionConfig("testFeatureCollectionBest", "path", From bc9b968c3a2fde7456e1ac23b2ebe30d043cf63d Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Mon, 13 Nov 2023 09:51:55 -0700 Subject: [PATCH 10/36] Fix test data location --- .../java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java b/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java index 456418f865..b7ba843598 100644 --- a/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java +++ b/grib/src/test/java/ucar/nc2/grib/coord/TestDiscontiguousInterval.java @@ -113,7 +113,7 @@ public void testTimeCoord2D_fromIntervals_isStrictlyMonotonicallyIncreasing_Data @Category(NeedsCdmUnitTest.class) public void testFeatureCollectionBest_isStrictlyMonotonicallyIncreasing() throws IOException { final String spec = - TestDir.cdmUnitTestDir + "/gribCollections/gfs_conus80/20231027/GFS_CONUS_80km_#yyyyMMdd_HHmm#\\.grib1$"; + TestDir.cdmUnitTestDir + "/gribCollections/nonMonotonicTime/GFS_CONUS_80km_#yyyyMMdd_HHmm#\\.grib1$"; final FeatureCollectionConfig config = new FeatureCollectionConfig("testFeatureCollectionBest", "path", FeatureCollectionType.GRIB1, spec, null, null, null, "file", null); final boolean changed = GribCdmIndex.updateGribCollection(config, CollectionUpdateType.always, null); From 837ffb6dc516d0185f2d3b88e63a0f319947706a Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 14 Nov 2023 10:23:14 -0700 Subject: [PATCH 11/36] Deprecate class using deprecated NetcdfFile constructors --- cdm/core/src/main/java/ucar/nc2/NetcdfFileSubclass.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cdm/core/src/main/java/ucar/nc2/NetcdfFileSubclass.java b/cdm/core/src/main/java/ucar/nc2/NetcdfFileSubclass.java index a3448a246f..95a3afa0ff 100644 --- a/cdm/core/src/main/java/ucar/nc2/NetcdfFileSubclass.java +++ b/cdm/core/src/main/java/ucar/nc2/NetcdfFileSubclass.java @@ -16,7 +16,9 @@ * * @author caron * @since 10/29/2014 + * @deprecated Use NetcdfFile.builder(). Remove in v6 */ +@Deprecated public class NetcdfFileSubclass extends NetcdfFile { public NetcdfFileSubclass() {} From 797d9a818e31f8abb57020c5be719b6dda1c53ac Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 14 Nov 2023 10:25:42 -0700 Subject: [PATCH 12/36] Do not use deprecated method of constructing NetcdfFile and NetcdfDataset in grib code --- .../src/main/java/ucar/nc2/NetcdfFiles.java | 2 +- .../nc2/grib/collection/Grib1Collection.java | 22 ++++++++++++------- .../nc2/grib/collection/Grib1Partition.java | 12 ++++++---- .../nc2/grib/collection/Grib2Collection.java | 22 ++++++++++++------- .../nc2/grib/collection/Grib2Partition.java | 14 +++++++----- 5 files changed, 46 insertions(+), 26 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/NetcdfFiles.java b/cdm/core/src/main/java/ucar/nc2/NetcdfFiles.java index 390a2b8c09..24c6131c04 100644 --- a/cdm/core/src/main/java/ucar/nc2/NetcdfFiles.java +++ b/cdm/core/src/main/java/ucar/nc2/NetcdfFiles.java @@ -805,7 +805,7 @@ private static IOServiceProvider getIosp(ucar.unidata.io.RandomAccessFile raf) t return null; } - private static NetcdfFile build(IOServiceProvider spi, ucar.unidata.io.RandomAccessFile raf, String location, + public static NetcdfFile build(IOServiceProvider spi, ucar.unidata.io.RandomAccessFile raf, String location, ucar.nc2.util.CancelTask cancelTask) throws IOException { NetcdfFile.Builder builder = NetcdfFile.builder().setIosp((AbstractIOServiceProvider) spi).setLocation(location); diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib1Collection.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib1Collection.java index 3ddf97c0b9..a17aeddadb 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib1Collection.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib1Collection.java @@ -6,6 +6,7 @@ package ucar.nc2.grib.collection; import javax.annotation.Nullable; +import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.grib.coord.CoordinateTimeAbstract; import ucar.nc2.*; import ucar.nc2.constants.DataFormatType; @@ -22,6 +23,7 @@ import ucar.nc2.grib.grib1.tables.Grib1Customizer; import java.io.IOException; import java.util.Formatter; +import ucar.unidata.io.RandomAccessFile; /** * Grib1-specific subclass of GribCollection. @@ -41,8 +43,9 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group FeatureCollectionConfig gribConfig, Formatter errlog, org.slf4j.Logger logger) throws IOException { if (filename == null) { Grib1Iosp iosp = new Grib1Iosp(group, ds.getType()); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - return new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); } else { MFile wantFile = findMFileByName(filename); @@ -53,8 +56,9 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group return null; Grib1Iosp iosp = new Grib1Iosp(gc); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - return new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); } return null; } @@ -66,8 +70,9 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St FeatureCollectionConfig gribConfig, Formatter errlog, org.slf4j.Logger logger) throws IOException { if (filename == null) { Grib1Iosp iosp = new Grib1Iosp(group, ds.getType()); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation() + "#" + group.getId(), null); - NetcdfDataset ncd = new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation() + "#" + group.getId(), null); + NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); return new ucar.nc2.dt.grid.GridDataset(ncd); // LOOK - replace with custom GridDataset?? } else { @@ -79,8 +84,9 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St return null; Grib1Iosp iosp = new Grib1Iosp(gc); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - NetcdfDataset ncd = new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); return new ucar.nc2.dt.grid.GridDataset(ncd); // LOOK - replace with custom GridDataset?? } return null; diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib1Partition.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib1Partition.java index c294764189..ddd548a313 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib1Partition.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib1Partition.java @@ -10,11 +10,13 @@ import thredds.featurecollection.FeatureCollectionConfig; import ucar.nc2.constants.CDM; import ucar.nc2.dataset.NetcdfDataset; +import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.ft2.coverage.CoverageCollection; import ucar.nc2.grib.GribUtils; import ucar.nc2.grib.coverage.GribCoverageDataset; import java.io.IOException; import java.util.Formatter; +import ucar.unidata.io.RandomAccessFile; /** * PartitionCollection for Grib1. @@ -33,8 +35,9 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group FeatureCollectionConfig config, Formatter errlog, org.slf4j.Logger logger) throws IOException { ucar.nc2.grib.collection.Grib1Iosp iosp = new ucar.nc2.grib.collection.Grib1Iosp(group, ds.getType()); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - return new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); } @Override @@ -42,8 +45,9 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St FeatureCollectionConfig config, Formatter errlog, org.slf4j.Logger logger) throws IOException { ucar.nc2.grib.collection.Grib1Iosp iosp = new ucar.nc2.grib.collection.Grib1Iosp(group, ds.getType()); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - NetcdfDataset ncd = new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); return new ucar.nc2.dt.grid.GridDataset(ncd); } diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib2Collection.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib2Collection.java index 50b0bd21c0..2d17f4f42e 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib2Collection.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib2Collection.java @@ -6,6 +6,7 @@ package ucar.nc2.grib.collection; import javax.annotation.Nullable; +import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.grib.coord.CoordinateTimeAbstract; import ucar.ma2.Array; import ucar.nc2.*; @@ -20,6 +21,7 @@ import ucar.nc2.grib.GribTables; import ucar.nc2.grib.coverage.GribCoverageDataset; import ucar.nc2.grib.grib2.table.Grib2Tables; +import ucar.unidata.io.RandomAccessFile; import ucar.unidata.util.StringUtil2; import java.io.IOException; import java.util.Formatter; @@ -43,8 +45,9 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group if (filename == null) { Grib2Iosp iosp = new Grib2Iosp(group, ds.getType()); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - return new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); } else { MFile wantFile = findMFileByName(filename); @@ -55,8 +58,9 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group return null; Grib2Iosp iosp = new Grib2Iosp(gc); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - return new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); } return null; } @@ -69,8 +73,9 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St if (filename == null) { Grib2Iosp iosp = new Grib2Iosp(group, ds.getType()); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation() + "#" + group.getId(), null); - NetcdfDataset ncd = new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation() + "#" + group.getId(), null); + NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); return new ucar.nc2.dt.grid.GridDataset(ncd); // LOOK - replace with custom GridDataset?? } else { @@ -82,8 +87,9 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St return null; Grib2Iosp iosp = new Grib2Iosp(gc); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - NetcdfDataset ncd = new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); return new ucar.nc2.dt.grid.GridDataset(ncd); // LOOK - replace with custom GridDataset?? } return null; diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib2Partition.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib2Partition.java index 16e68aa704..1f7073b59a 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib2Partition.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib2Partition.java @@ -12,12 +12,14 @@ import ucar.nc2.Attribute; import ucar.nc2.AttributeContainer; import ucar.nc2.NetcdfFile; -import ucar.nc2.NetcdfFileSubclass; +import ucar.nc2.NetcdfFiles; import ucar.nc2.constants.CDM; import ucar.nc2.constants.DataFormatType; import ucar.nc2.dataset.NetcdfDataset; +import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.ft2.coverage.CoverageCollection; import ucar.nc2.grib.coverage.GribCoverageDataset; +import ucar.unidata.io.RandomAccessFile; /** * PartitionCollection for Grib2. @@ -36,8 +38,9 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group FeatureCollectionConfig config, Formatter errlog, org.slf4j.Logger logger) throws IOException { ucar.nc2.grib.collection.Grib2Iosp iosp = new ucar.nc2.grib.collection.Grib2Iosp(group, ds.getType()); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - return new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); } @Override @@ -45,8 +48,9 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St FeatureCollectionConfig config, Formatter errlog, org.slf4j.Logger logger) throws IOException { ucar.nc2.grib.collection.Grib2Iosp iosp = new ucar.nc2.grib.collection.Grib2Iosp(group, ds.getType()); - NetcdfFile ncfile = new NetcdfFileSubclass(iosp, null, getLocation(), null); - NetcdfDataset ncd = new NetcdfDataset(ncfile); + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); + NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); return new ucar.nc2.dt.grid.GridDataset(ncd); } From fec51d0e858a78468219857fe59d7f3fc0073e9c Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 14 Nov 2023 15:01:50 -0700 Subject: [PATCH 13/36] Refactor building NetcdfDataset code to helper methog to avoid duplication --- .../nc2/grib/collection/Grib1Collection.java | 18 ++++-------------- .../nc2/grib/collection/Grib1Partition.java | 10 ++-------- .../nc2/grib/collection/Grib2Collection.java | 18 ++++-------------- .../nc2/grib/collection/Grib2Partition.java | 12 ++---------- .../collection/GribCollectionImmutable.java | 12 ++++++++++++ 5 files changed, 24 insertions(+), 46 deletions(-) diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib1Collection.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib1Collection.java index a17aeddadb..4b53f9984f 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib1Collection.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib1Collection.java @@ -6,7 +6,6 @@ package ucar.nc2.grib.collection; import javax.annotation.Nullable; -import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.grib.coord.CoordinateTimeAbstract; import ucar.nc2.*; import ucar.nc2.constants.DataFormatType; @@ -23,7 +22,6 @@ import ucar.nc2.grib.grib1.tables.Grib1Customizer; import java.io.IOException; import java.util.Formatter; -import ucar.unidata.io.RandomAccessFile; /** * Grib1-specific subclass of GribCollection. @@ -43,9 +41,7 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group FeatureCollectionConfig gribConfig, Formatter errlog, org.slf4j.Logger logger) throws IOException { if (filename == null) { Grib1Iosp iosp = new Grib1Iosp(group, ds.getType()); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + return buildNetcdfDataset(iosp, getLocation()); } else { MFile wantFile = findMFileByName(filename); @@ -56,9 +52,7 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group return null; Grib1Iosp iosp = new Grib1Iosp(gc); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + return buildNetcdfDataset(iosp, getLocation()); } return null; } @@ -70,9 +64,7 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St FeatureCollectionConfig gribConfig, Formatter errlog, org.slf4j.Logger logger) throws IOException { if (filename == null) { Grib1Iosp iosp = new Grib1Iosp(group, ds.getType()); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation() + "#" + group.getId(), null); - NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + NetcdfDataset ncd = buildNetcdfDataset(iosp, getLocation() + "#" + group.getId()); return new ucar.nc2.dt.grid.GridDataset(ncd); // LOOK - replace with custom GridDataset?? } else { @@ -84,9 +76,7 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St return null; Grib1Iosp iosp = new Grib1Iosp(gc); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + NetcdfDataset ncd = buildNetcdfDataset(iosp, getLocation()); return new ucar.nc2.dt.grid.GridDataset(ncd); // LOOK - replace with custom GridDataset?? } return null; diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib1Partition.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib1Partition.java index ddd548a313..61481d8cb0 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib1Partition.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib1Partition.java @@ -10,13 +10,11 @@ import thredds.featurecollection.FeatureCollectionConfig; import ucar.nc2.constants.CDM; import ucar.nc2.dataset.NetcdfDataset; -import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.ft2.coverage.CoverageCollection; import ucar.nc2.grib.GribUtils; import ucar.nc2.grib.coverage.GribCoverageDataset; import java.io.IOException; import java.util.Formatter; -import ucar.unidata.io.RandomAccessFile; /** * PartitionCollection for Grib1. @@ -35,9 +33,7 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group FeatureCollectionConfig config, Formatter errlog, org.slf4j.Logger logger) throws IOException { ucar.nc2.grib.collection.Grib1Iosp iosp = new ucar.nc2.grib.collection.Grib1Iosp(group, ds.getType()); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + return buildNetcdfDataset(iosp, getLocation()); } @Override @@ -45,9 +41,7 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St FeatureCollectionConfig config, Formatter errlog, org.slf4j.Logger logger) throws IOException { ucar.nc2.grib.collection.Grib1Iosp iosp = new ucar.nc2.grib.collection.Grib1Iosp(group, ds.getType()); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + NetcdfDataset ncd = buildNetcdfDataset(iosp, getLocation()); return new ucar.nc2.dt.grid.GridDataset(ncd); } diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib2Collection.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib2Collection.java index 2d17f4f42e..0430891b54 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib2Collection.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib2Collection.java @@ -6,7 +6,6 @@ package ucar.nc2.grib.collection; import javax.annotation.Nullable; -import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.grib.coord.CoordinateTimeAbstract; import ucar.ma2.Array; import ucar.nc2.*; @@ -21,7 +20,6 @@ import ucar.nc2.grib.GribTables; import ucar.nc2.grib.coverage.GribCoverageDataset; import ucar.nc2.grib.grib2.table.Grib2Tables; -import ucar.unidata.io.RandomAccessFile; import ucar.unidata.util.StringUtil2; import java.io.IOException; import java.util.Formatter; @@ -45,9 +43,7 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group if (filename == null) { Grib2Iosp iosp = new Grib2Iosp(group, ds.getType()); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + return buildNetcdfDataset(iosp, getLocation()); } else { MFile wantFile = findMFileByName(filename); @@ -58,9 +54,7 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group return null; Grib2Iosp iosp = new Grib2Iosp(gc); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + return buildNetcdfDataset(iosp, getLocation()); } return null; } @@ -73,9 +67,7 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St if (filename == null) { Grib2Iosp iosp = new Grib2Iosp(group, ds.getType()); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation() + "#" + group.getId(), null); - NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + NetcdfDataset ncd = buildNetcdfDataset(iosp, getLocation() + "#" + group.getId()); return new ucar.nc2.dt.grid.GridDataset(ncd); // LOOK - replace with custom GridDataset?? } else { @@ -87,9 +79,7 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St return null; Grib2Iosp iosp = new Grib2Iosp(gc); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + NetcdfDataset ncd = buildNetcdfDataset(iosp, getLocation()); return new ucar.nc2.dt.grid.GridDataset(ncd); // LOOK - replace with custom GridDataset?? } return null; diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib2Partition.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib2Partition.java index 1f7073b59a..6ab3716d76 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib2Partition.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib2Partition.java @@ -11,15 +11,11 @@ import thredds.featurecollection.FeatureCollectionConfig; import ucar.nc2.Attribute; import ucar.nc2.AttributeContainer; -import ucar.nc2.NetcdfFile; -import ucar.nc2.NetcdfFiles; import ucar.nc2.constants.CDM; import ucar.nc2.constants.DataFormatType; import ucar.nc2.dataset.NetcdfDataset; -import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.ft2.coverage.CoverageCollection; import ucar.nc2.grib.coverage.GribCoverageDataset; -import ucar.unidata.io.RandomAccessFile; /** * PartitionCollection for Grib2. @@ -38,9 +34,7 @@ public ucar.nc2.dataset.NetcdfDataset getNetcdfDataset(Dataset ds, GroupGC group FeatureCollectionConfig config, Formatter errlog, org.slf4j.Logger logger) throws IOException { ucar.nc2.grib.collection.Grib2Iosp iosp = new ucar.nc2.grib.collection.Grib2Iosp(group, ds.getType()); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + return buildNetcdfDataset(iosp, getLocation()); } @Override @@ -48,9 +42,7 @@ public ucar.nc2.dt.grid.GridDataset getGridDataset(Dataset ds, GroupGC group, St FeatureCollectionConfig config, Formatter errlog, org.slf4j.Logger logger) throws IOException { ucar.nc2.grib.collection.Grib2Iosp iosp = new ucar.nc2.grib.collection.Grib2Iosp(group, ds.getType()); - RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); - NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, getLocation(), null); - NetcdfDataset ncd = NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + NetcdfDataset ncd = buildNetcdfDataset(iosp, getLocation()); return new ucar.nc2.dt.grid.GridDataset(ncd); } diff --git a/grib/src/main/java/ucar/nc2/grib/collection/GribCollectionImmutable.java b/grib/src/main/java/ucar/nc2/grib/collection/GribCollectionImmutable.java index f1790e43cd..53dc60f9b5 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/GribCollectionImmutable.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/GribCollectionImmutable.java @@ -14,9 +14,13 @@ import ucar.nc2.Attribute; import ucar.nc2.AttributeContainer; import ucar.nc2.AttributeContainerMutable; +import ucar.nc2.NetcdfFile; +import ucar.nc2.NetcdfFiles; import ucar.nc2.constants.CDM; import ucar.nc2.constants.CF; import ucar.nc2.constants.FeatureType; +import ucar.nc2.dataset.NetcdfDataset; +import ucar.nc2.dataset.NetcdfDatasets; import ucar.nc2.ft2.coverage.CoverageCollection; import ucar.nc2.ft2.coverage.SubsetParams; import ucar.nc2.grib.*; @@ -31,6 +35,7 @@ import ucar.nc2.grib.grib1.Grib1Variable; import ucar.nc2.grib.grib1.tables.Grib1Customizer; import ucar.nc2.grib.grib2.table.Grib2Tables; +import ucar.nc2.iosp.AbstractIOServiceProvider; import ucar.nc2.time.CalendarDate; import ucar.nc2.time.CalendarDateRange; import ucar.nc2.util.cache.FileCacheIF; @@ -1113,6 +1118,13 @@ String getDataRafFilename(int fileno) { return mfile.getPath(); } + protected static NetcdfDataset buildNetcdfDataset(AbstractIOServiceProvider iosp, String location) + throws IOException { + RandomAccessFile raf = (RandomAccessFile) iosp.sendIospMessage(NetcdfFile.IOSP_MESSAGE_RANDOM_ACCESS_FILE); + NetcdfFile ncfile = NetcdfFiles.build(iosp, raf, location, null); + return NetcdfDatasets.enhance(ncfile, NetcdfDataset.getDefaultEnhanceMode(), null); + } + /////////////////////// // stuff needed by InvDatasetFcGrib From a80b27d3652750957438c1c2c0571767ba0e8b39 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 21 Nov 2023 13:37:35 -0600 Subject: [PATCH 14/36] Update visad mcidas library --- netcdf-java-platform/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netcdf-java-platform/build.gradle b/netcdf-java-platform/build.gradle index a9279f6357..ecfffe1993 100644 --- a/netcdf-java-platform/build.gradle +++ b/netcdf-java-platform/build.gradle @@ -44,7 +44,7 @@ dependencies { api 'org.jsoup:jsoup:1.11.2' // HTML scraper used in GRIB // cdm-mcidas (GEMPAK and McIDAS IOSPs) - api 'edu.wisc.ssec:visad-mcidas-slim-ucar-ns:20200507-2' + api 'edu.wisc.ssec:visad-mcidas-slim-ucar-ns:20231121' // cdm-vis5d (vis5d IOSP) api 'edu.wisc.ssec:visad:2.0-20130124' From f90162655957898833ba48adf81d345a20d8de2e Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 21 Nov 2023 13:58:31 -0600 Subject: [PATCH 15/36] Add NeedsExternalResource category to test using thredds-dev server --- .../src/test/java/ucar/nc2/util/net/TestHTTPSession.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java b/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java index 7ab6a08845..ca32464fed 100755 --- a/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java +++ b/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java @@ -41,6 +41,7 @@ import org.apache.http.message.BasicHeader; import org.junit.Assert; import org.junit.Test; +import org.junit.experimental.categories.Category; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import ucar.httpservices.HTTPFactory; @@ -49,7 +50,9 @@ import ucar.unidata.util.test.UnitTestCommon; import java.lang.invoke.MethodHandles; import java.util.List; +import ucar.unidata.util.test.category.NeedsExternalResource; +@Category(NeedsExternalResource.class) public class TestHTTPSession extends UnitTestCommon { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); From bddee541ec88d91c7193256df830628030af4cf8 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 21 Nov 2023 15:07:34 -0600 Subject: [PATCH 16/36] Remove unused code --- .../java/ucar/nc2/util/net/TestHTTPSession.java | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java b/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java index ca32464fed..ba584974b8 100755 --- a/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java +++ b/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java @@ -38,7 +38,6 @@ import org.apache.http.auth.UsernamePasswordCredentials; import org.apache.http.client.config.RequestConfig; import org.apache.http.impl.client.BasicCredentialsProvider; -import org.apache.http.message.BasicHeader; import org.junit.Assert; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -56,27 +55,12 @@ public class TestHTTPSession extends UnitTestCommon { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - ////////////////////////////////////////////////// - // Constants - - // static final String TESTURL1 = "http://" + TestDir.dap2TestServer + "/dts/test.01.dds"; static final String TESTURL1 = "https://thredds-dev.unidata.ucar.edu"; static final String GLOBALAGENT = "TestUserAgent123global"; static final String SESSIONAGENT = "TestUserAgent123session"; static final String USER = "dmh"; static final String PWD = "FakDennisPassword"; - ////////////////////////////////////////////////// - // Define the test sets - - int passcount = 0; - int xfailcount = 0; - int failcount = 0; - boolean verbose = true; - boolean pass = false; - - String datadir = null; - String threddsroot = null; public TestHTTPSession() { super(); From 4bcd9bccbb65c1404dec99e37843f2fca38aada8 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 21 Nov 2023 15:26:32 -0600 Subject: [PATCH 17/36] Use google truth asserts in test --- .../ucar/nc2/util/net/TestHTTPSession.java | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java b/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java index ba584974b8..06e337b054 100755 --- a/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java +++ b/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java @@ -32,13 +32,14 @@ package ucar.nc2.util.net; +import static com.google.common.truth.Truth.assertWithMessage; + import org.apache.http.Header; import org.apache.http.auth.AuthScope; import org.apache.http.auth.Credentials; import org.apache.http.auth.UsernamePasswordCredentials; import org.apache.http.client.config.RequestConfig; import org.apache.http.impl.client.BasicCredentialsProvider; -import org.junit.Assert; import org.junit.Test; import org.junit.experimental.categories.Category; import org.slf4j.Logger; @@ -84,18 +85,18 @@ public void testAgent() throws Exception { // Use special interface to access the request // Look for the user agent header agents = session.getDebugRequestInterceptor().getHeaders(HTTPSession.HEADER_USERAGENT); - Assert.assertFalse("User-Agent Header not found", agents.size() == 0); + assertWithMessage("User-Agent Header not found").that(agents.size()).isNotEqualTo(0); // It is possible to see multiple same headers, so verify that they have same value String agentvalue = null; for (Header h : agents) { - Assert.assertTrue("Bad Agent Header", h.getName().equals("User-Agent")); + assertWithMessage("Bad Agent Header").that(h.getName()).isEqualTo("User-Agent"); if (agentvalue == null) agentvalue = h.getValue(); else - Assert.assertTrue("Bad Agent Value", h.getValue().equals(agentvalue)); + assertWithMessage("Bad Agent Value").that(h.getValue()).isEqualTo(agentvalue); } - Assert.assertTrue(String.format("User-Agent mismatch: expected %s found:%s", GLOBALAGENT, agentvalue), - GLOBALAGENT.equals(agentvalue)); + assertWithMessage(String.format("User-Agent mismatch: expected %s found:%s", GLOBALAGENT, agentvalue)) + .that(GLOBALAGENT).isEqualTo(agentvalue); logger.debug("*** Pass: set global agent"); // method.close(); @@ -106,17 +107,17 @@ public void testAgent() throws Exception { method.execute(); // Use special interface to access the request agents = session.getDebugRequestInterceptor().getHeaders(HTTPSession.HEADER_USERAGENT); - Assert.assertFalse("User-Agent Header not found", agents.size() == 0); + assertWithMessage("User-Agent Header not found").that(agents.size()).isNotEqualTo(0); agentvalue = null; for (Header h : agents) { - Assert.assertTrue("Bad Agent Header", h.getName().equals("User-Agent")); + assertWithMessage("Bad Agent Header").that(h.getName()).isEqualTo("User-Agent"); if (agentvalue == null) agentvalue = h.getValue(); else - Assert.assertTrue("Bad Agent Value", h.getValue().equals(agentvalue)); + assertWithMessage("Bad Agent Value").that(h.getValue()).isEqualTo(agentvalue); } - Assert.assertTrue(String.format("User-Agent mismatch: expected %s found:%s", SESSIONAGENT, agentvalue), - SESSIONAGENT.equals(agentvalue)); + assertWithMessage(String.format("User-Agent mismatch: expected %s found:%s", SESSIONAGENT, agentvalue)) + .that(SESSIONAGENT).isEqualTo(agentvalue); logger.debug("*** Pass: set session agent"); method.close(); } @@ -144,22 +145,22 @@ public void testConfigure() throws Exception { boolean b = dbgcfg.isCircularRedirectsAllowed(); logger.debug("Test: Circular Redirects"); - Assert.assertTrue("*** Fail: Circular Redirects", b); + assertWithMessage("*** Fail: Circular Redirects").that(b).isTrue(); logger.debug("*** Pass: Circular Redirects"); logger.debug("Test: Max Redirects"); int n = dbgcfg.getMaxRedirects(); - Assert.assertTrue("*** Fail: Max Redirects", n == 111); + assertWithMessage("*** Fail: Max Redirects").that(n).isEqualTo(111); logger.debug("*** Pass: Max Redirects"); logger.debug("Test: SO Timeout"); n = dbgcfg.getSocketTimeout(); - Assert.assertTrue("*** Fail: SO Timeout", n == 17777); + assertWithMessage("*** Fail: SO Timeout").that(n).isEqualTo(17777); logger.debug("*** Pass: SO Timeout"); logger.debug("Test: Connection Timeout"); n = dbgcfg.getConnectTimeout(); - Assert.assertTrue("*** Fail: Connection Timeout", n == 37777); + assertWithMessage("*** Fail: Connection Timeout").that(n).isEqualTo(37777); logger.debug("*** Pass: SO Timeout"); method.close(); } From 3ad2772db5177477ab85765cfe918e1dbdc883bf Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Mon, 27 Nov 2023 10:34:50 -0700 Subject: [PATCH 18/36] Update tests to use thredds-test instead of thredds-dev --- .../test/java/ucar/nc2/ft/coverage/TestCoverageHorizSubset.java | 2 +- .../src/test/java/ucar/nc2/util/net/TestHTTPSession.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cdm-test/src/test/java/ucar/nc2/ft/coverage/TestCoverageHorizSubset.java b/cdm-test/src/test/java/ucar/nc2/ft/coverage/TestCoverageHorizSubset.java index c6d0f07eb2..35684d4ba0 100644 --- a/cdm-test/src/test/java/ucar/nc2/ft/coverage/TestCoverageHorizSubset.java +++ b/cdm-test/src/test/java/ucar/nc2/ft/coverage/TestCoverageHorizSubset.java @@ -133,7 +133,7 @@ public void testLongitudeSubset() throws Exception { @Category(NeedsExternalResource.class) public void testCdmRemoteSubset() throws Exception { String filename = - "cdmremote:https://thredds-dev.unidata.ucar.edu/thredds/cdmremote/grib/NCEP/NAM/CONUS_40km/conduit/best"; + "cdmremote:https://thredds-test.unidata.ucar.edu/thredds/cdmremote/grib/NCEP/NAM/CONUS_40km/conduit/best"; System.out.printf("open %s%n", filename); try (FeatureDatasetCoverage cc = CoverageDatasetFactory.open(filename)) { diff --git a/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java b/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java index 06e337b054..18f7d3aa6c 100755 --- a/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java +++ b/httpservices/src/test/java/ucar/nc2/util/net/TestHTTPSession.java @@ -56,7 +56,7 @@ public class TestHTTPSession extends UnitTestCommon { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - static final String TESTURL1 = "https://thredds-dev.unidata.ucar.edu"; + static final String TESTURL1 = "https://thredds-test.unidata.ucar.edu"; static final String GLOBALAGENT = "TestUserAgent123global"; static final String SESSIONAGENT = "TestUserAgent123session"; From 3090d5941e4a1c9218d5a24fea8c5ec9ccd185bf Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Fri, 18 Aug 2023 09:35:14 -0700 Subject: [PATCH 19/36] fix scale offset applied twice fix scale offset applied twice remove extra calls to convert --- .../java/ucar/nc2/dataset/CoordinateAxis1D.java | 1 + .../ucar/nc2/dataset/CoordinateAxis1DTime.java | 4 ++-- .../main/java/ucar/nc2/dataset/VariableDS.java | 12 ++++-------- .../main/java/ucar/nc2/filter/ScaleOffset.java | 3 +++ .../ft2/coverage/adapter/DtCoverageDataset.java | 17 +++++++++++------ 5 files changed, 21 insertions(+), 16 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1D.java b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1D.java index e4244e892e..da8cae0767 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1D.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1D.java @@ -866,6 +866,7 @@ protected void readValues() { } coords = (double[]) data.get1DJavaArray(DataType.DOUBLE); + this.wasRead = true; // IndexIterator iter = data.getIndexIterator(); // while (iter.hasNext()) // coords[count++] = iter.getDoubleNext(); diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java index 37447e4919..c0264220b9 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java @@ -207,11 +207,11 @@ protected void readValues() { // the axis was created by this classes factory. if (this.orgDataType != null && !this.orgDataType.isNumeric()) { this.coords = cdates.stream().mapToDouble(cdate -> (double) cdate.getDifferenceInMsecs(cdates.get(0))).toArray(); - // make sure parent methods do not try to read from the orgVar again - this.wasRead = true; } else { super.readValues(); } + // make sure parent methods do not try to read from the orgVar again + this.wasRead = true; } //////////////////////////////////////////////////////////////////////// diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java index 4be9026221..d787233b2c 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java @@ -439,11 +439,9 @@ protected Array _read() throws IOException { // check if already cached - caching in VariableDS only done explicitly by app if (hasCachedData()) - result = super._read(); + return super._read(); else - result = proxyReader.reallyRead(this, null); - - return convert(result); + return proxyReader.reallyRead(this, null); } // section of regular Variable @@ -455,11 +453,9 @@ protected Array _read(Section section) throws IOException, InvalidRangeException Array result; if (hasCachedData()) - result = super._read(section); + return super._read(section); else - result = proxyReader.reallyRead(this, section, null); - - return convert(result); + return proxyReader.reallyRead(this, section, null); } // do not call directly diff --git a/cdm/core/src/main/java/ucar/nc2/filter/ScaleOffset.java b/cdm/core/src/main/java/ucar/nc2/filter/ScaleOffset.java index 90202ecb7b..268ce83cc8 100644 --- a/cdm/core/src/main/java/ucar/nc2/filter/ScaleOffset.java +++ b/cdm/core/src/main/java/ucar/nc2/filter/ScaleOffset.java @@ -78,13 +78,16 @@ public static ScaleOffset createFromVariable(VariableDS var) { if (scaleAtt != null && !scaleAtt.isString()) { scaleType = FilterHelpers.getAttributeDataType(scaleAtt, signedness); scale = 1 / (var.convertUnsigned(scaleAtt.getNumericValue(), scaleType).doubleValue()); + var.remove(scaleAtt); } Attribute offsetAtt = var.findAttribute(CDM.ADD_OFFSET); if (offsetAtt != null && !offsetAtt.isString()) { offsetType = FilterHelpers.getAttributeDataType(offsetAtt, signedness); offset = var.convertUnsigned(offsetAtt.getNumericValue(), offsetType).doubleValue(); + var.remove(offsetAtt); } + if (scale != DEFAULT_SCALE || offset != DEFAULT_OFFSET) { DataType scaledOffsetType = FilterHelpers.largestOf(var.getUnsignedConversionType(), scaleType, offsetType).withSignedness(signedness); diff --git a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java index 99d54ef239..c2e7597ee7 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java +++ b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java @@ -116,19 +116,24 @@ public DtCoverageDataset(NetcdfDataset ncd, Formatter parseInfo) throws IOExcept this.ncd = ncd; Set enhance = ncd.getEnhanceMode(); - if (enhance == null || enhance.isEmpty()) + if (enhance == null || enhance.isEmpty()) { enhance = NetcdfDataset.getDefaultEnhanceMode(); + } ncd = NetcdfDatasets.enhance(ncd, enhance, null); - DtCoverageCSBuilder facc = DtCoverageCSBuilder.classify(ncd, parseInfo); - if (facc != null) - this.coverageType = facc.type; + // sort by largest size first + List csList = new ArrayList<>(ncd.getCoordinateSystems()); + csList.sort((o1, o2) -> o2.getCoordinateAxes().size() - o1.getCoordinateAxes().size()); Map csHash = new HashMap<>(); - for (CoordinateSystem cs : ncd.getCoordinateSystems()) { + for (CoordinateSystem cs : csList) { DtCoverageCSBuilder fac = new DtCoverageCSBuilder(ncd, cs, parseInfo); - if (fac.type == null) + if (fac.type == null) { continue; + } + if (this.coverageType == null) { + this.coverageType = fac.type; + } DtCoverageCS ccs = fac.makeCoordSys(); if (ccs == null) continue; From e2000c7e1334397226797a2fc4be10b547af8503 Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Tue, 10 Oct 2023 15:22:32 -0700 Subject: [PATCH 20/36] fix failing tests - convert after reading with Variable class --- cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java index d787233b2c..c14d9aab75 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java @@ -435,11 +435,9 @@ public void setCaching(boolean caching) { @Override protected Array _read() throws IOException { - Array result; - // check if already cached - caching in VariableDS only done explicitly by app if (hasCachedData()) - return super._read(); + return convert(super._read()); else return proxyReader.reallyRead(this, null); } @@ -451,9 +449,8 @@ protected Array _read(Section section) throws IOException, InvalidRangeException if ((null == section) || section.computeSize() == getSize()) return _read(); - Array result; if (hasCachedData()) - return super._read(section); + return convert(super._read(section)); else return proxyReader.reallyRead(this, section, null); } From cb748d1874a34ee1fac24e80333c6a972f8b5b9e Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Tue, 10 Oct 2023 15:32:25 -0700 Subject: [PATCH 21/36] fix docs test --- .../java/tests/cdmdatasets/TestNetcdfDatasetTutorial.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/docs/src/test/java/tests/cdmdatasets/TestNetcdfDatasetTutorial.java b/docs/src/test/java/tests/cdmdatasets/TestNetcdfDatasetTutorial.java index acfdb1d848..ac25b310a7 100644 --- a/docs/src/test/java/tests/cdmdatasets/TestNetcdfDatasetTutorial.java +++ b/docs/src/test/java/tests/cdmdatasets/TestNetcdfDatasetTutorial.java @@ -61,13 +61,8 @@ public void testUnpackDataTutorial() throws IOException { assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.FLOAT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); - double scale_factor = scaledvar.attributes().findAttributeDouble("scale_factor", 1.0); - assertThat(scaledvar.attributes().hasAttribute("add_offset")).isTrue(); - double add_offset = scaledvar.attributes().findAttributeDouble("add_offset", 1.0); - double unpacked_data = - NetcdfDatasetTutorial.unpackData(scaledvar.readScalarShort(), scale_factor, add_offset); + NetcdfDatasetTutorial.unpackData(scaledvar.readScalarShort(), 1.0, 0.0); assertThat(unpacked_data).isNotNaN(); } } From b71c567b28d6374a6d6eab9d9cad6406dc2b68be Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Tue, 17 Oct 2023 14:04:26 -0700 Subject: [PATCH 22/36] add wasConverted flag --- .../java/ucar/nc2/dataset/VariableDS.java | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java index c14d9aab75..a31a2046a2 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java @@ -277,6 +277,7 @@ Array convert(Array data, Set enhancements) { if (enhancements.contains(Enhance.ApplyNormalizer) && normalizer != null) { data = normalizer.convert(data); } + this.wasConverted = true; return data; } } @@ -435,24 +436,33 @@ public void setCaching(boolean caching) { @Override protected Array _read() throws IOException { + Array result; // check if already cached - caching in VariableDS only done explicitly by app - if (hasCachedData()) - return convert(super._read()); - else - return proxyReader.reallyRead(this, null); + if (hasCachedData()) { + result = super._read(); + } else { + result = proxyReader.reallyRead(this, null); + } + + return wasConverted ? result : convert(result); } // section of regular Variable @Override protected Array _read(Section section) throws IOException, InvalidRangeException { // really a full read - if ((null == section) || section.computeSize() == getSize()) + if ((null == section) || section.computeSize() == getSize()) { return _read(); + } - if (hasCachedData()) - return convert(super._read(section)); - else - return proxyReader.reallyRead(this, section, null); + Array result; + if (hasCachedData()) { + result = super._read(section); + } else { + result = proxyReader.reallyRead(this, section, null); + } + + return wasConverted ? result : convert(result); } // do not call directly @@ -811,6 +821,8 @@ public Array convert(Array in, boolean convertUnsigned, boolean applyScaleOffset private boolean hasFillValue = false; private double fillValue = Double.MAX_VALUE; + protected boolean wasConverted = false; + protected VariableDS(Builder builder, Group parentGroup) { super(builder, parentGroup); From 2d5a422a9e9ebca03c430bc4e6922eca0aafe8fc Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Fri, 17 Nov 2023 13:04:43 -0800 Subject: [PATCH 23/36] remove extra calls to read time axes --- .../java/ucar/nc2/dataset/CoordinateAxis1DTime.java | 7 +++++++ .../main/java/ucar/nc2/dataset/NetcdfDataset.java | 13 +++++++++++-- .../main/java/ucar/nc2/dataset/NetcdfDatasets.java | 1 + .../src/main/java/ucar/nc2/dataset/VariableDS.java | 3 +++ .../nc2/ft2/coverage/adapter/DtCoverageAdapter.java | 2 +- .../ft2/coverage/adapter/DtCoverageCSBuilder.java | 2 ++ .../nc2/ft2/coverage/adapter/DtCoverageDataset.java | 11 +++-------- .../nc2/internal/dataset/CoordinatesHelper.java | 9 +++++++-- .../main/java/ucar/nc2/util/cache/FileCache.java | 4 ++++ .../java/ucar/nc2/internal/ncml/TestEnhance.java | 9 +++------ .../src/test/java/ucar/nc2/ncml/TestEnhance.java | 9 +++------ 11 files changed, 45 insertions(+), 25 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java index c0264220b9..15bd234d5e 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java @@ -212,6 +212,11 @@ protected void readValues() { } // make sure parent methods do not try to read from the orgVar again this.wasRead = true; + if (orgVar instanceof CoordinateAxis1D) { + CoordinateAxis1D orgAxis = (CoordinateAxis1D) orgVar; + this.wasRead = orgAxis.wasRead; + this.coords = orgAxis.coords; + } } //////////////////////////////////////////////////////////////////////// @@ -338,6 +343,8 @@ private CoordinateAxis1DTime(NetcdfDataset ncd, VariableDS org, Formatter errMes List result = new ArrayList<>(ncoords); Array data = org.read(); + coords = (double[]) data.get1DJavaArray(DataType.DOUBLE); + this.wasRead = true; int count = 0; IndexIterator ii = data.getIndexIterator(); diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java index 17e2a1593b..111b39443c 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java @@ -932,6 +932,10 @@ public ImmutableList getCoordinateAxes() { return ImmutableList.copyOf(coordAxes); } + public void setCoordinateAxes(List axes) { + this.coordAxes = axes; + } + /** * Clear Coordinate System metadata, to allow them to be redone * @@ -1344,12 +1348,16 @@ public void addCoordinateTransform(CoordinateTransform ct) { public CoordinateAxis addCoordinateAxis(VariableDS v) { if (v == null) return null; + + final List coordCopy = new ArrayList<>(coordAxes); + CoordinateAxis oldVar = findCoordinateAxis(v.getFullName()); if (oldVar != null) - coordAxes.remove(oldVar); + coordCopy.remove(oldVar); CoordinateAxis ca = (v instanceof CoordinateAxis) ? (CoordinateAxis) v : CoordinateAxis.factory(this, v); - coordAxes.add(ca); + coordCopy.add(ca); + this.coordAxes = coordCopy; if (v.isMemberOfStructure()) { Structure parentOrg = v.getParentStructure(); // gotta be careful to get the wrapping parent @@ -1705,6 +1713,7 @@ public void replaceCoordinateAxis(Group.Builder group, CoordinateAxis.Builder ax axis.setParentGroupBuilder(group); } + public T setOrgFile(NetcdfFile orgFile) { this.orgFile = orgFile; return self(); diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDatasets.java b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDatasets.java index 588d6abada..e7c14c445b 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDatasets.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDatasets.java @@ -13,6 +13,7 @@ import ucar.nc2.internal.dataset.DatasetEnhancer; import ucar.nc2.internal.ncml.NcmlReader; import ucar.nc2.util.CancelTask; +import ucar.nc2.util.CompareNetcdf2; import ucar.nc2.util.cache.FileCache; import ucar.nc2.util.cache.FileCacheIF; import ucar.nc2.util.cache.FileFactory; diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java index a31a2046a2..cfa3775292 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java @@ -907,6 +907,9 @@ protected Builder addLocalFieldsToBuilder(Builder> build builder.setOriginalVariable(this.orgVar).setOriginalDataType(this.orgDataType).setOriginalName(this.orgName) .setOriginalFileTypeId(this.orgFileTypeId).setEnhanceMode(this.enhanceMode).setUnits(this.enhanceProxy.units) .setDesc(this.enhanceProxy.desc); + if (this.coordSysNames != null) { + this.coordSysNames.stream().forEach(s -> builder.addCoordinateSystemName(s)); + } return (VariableDS.Builder) super.addLocalFieldsToBuilder(builder); } diff --git a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java index 53cdddc99f..525930f9c7 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java +++ b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java @@ -155,7 +155,7 @@ private static CoverageCoordAxis makeCoordAxisFromDimension(Dimension dim) { private static ucar.nc2.util.Optional makeCoordAxis(FeatureType ftype, ucar.nc2.dataset.CoordinateAxis dtCoordAxis, DtCoverageAdapter reader) { - String name = dtCoordAxis.getFullName(); + String name = dtCoordAxis.getFullName(); DataType dataType = dtCoordAxis.getDataType(); AxisType axisType = dtCoordAxis.getAxisType(); String units = dtCoordAxis.getUnitsString(); diff --git a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java index 3085b313b8..15bc2d4169 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java +++ b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java @@ -5,6 +5,7 @@ package ucar.nc2.ft2.coverage.adapter; import com.google.common.collect.Lists; +import thredds.client.catalog.builder.DatasetBuilder; import ucar.nc2.Dimension; import ucar.nc2.constants.AxisType; import ucar.nc2.constants.FeatureType; @@ -179,6 +180,7 @@ public static String describe(NetcdfDataset ds, CoordinateSystem cs, Formatter e ////////////////////////////////////////////////////////////// // time + boolean axesChanged = false; // if any time axes are rebuilt by the factory, we need to update the netcdfDataset CoordinateAxis rt = cs.findAxis(AxisType.RunTime); if (rt != null) { if (!rt.isScalar() && !(rt instanceof CoordinateAxis1D)) { // A runtime axis must be scalar or one-dimensional diff --git a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java index c2e7597ee7..75ca0eb97f 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java +++ b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java @@ -13,11 +13,8 @@ import java.util.List; import java.util.Map; import java.util.Set; -import ucar.nc2.Attribute; -import ucar.nc2.Dimension; -import ucar.nc2.NetcdfFile; -import ucar.nc2.Variable; -import ucar.nc2.VariableSimpleIF; + +import ucar.nc2.*; import ucar.nc2.constants.CDM; import ucar.nc2.constants.FeatureType; import ucar.nc2.dataset.CoordinateAxis; @@ -113,13 +110,11 @@ public DtCoverageDataset(NetcdfDataset ncd) throws IOException { * @throws java.io.IOException on read error */ public DtCoverageDataset(NetcdfDataset ncd, Formatter parseInfo) throws IOException { - this.ncd = ncd; - Set enhance = ncd.getEnhanceMode(); if (enhance == null || enhance.isEmpty()) { enhance = NetcdfDataset.getDefaultEnhanceMode(); } - ncd = NetcdfDatasets.enhance(ncd, enhance, null); + this.ncd = NetcdfDatasets.enhance(ncd, enhance, null); // sort by largest size first List csList = new ArrayList<>(ncd.getCoordinateSystems()); diff --git a/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordinatesHelper.java b/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordinatesHelper.java index 66bede5fab..00161910e9 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordinatesHelper.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordinatesHelper.java @@ -53,10 +53,15 @@ public List getCoordTransforms() { return coordTransforms; } + // make another constructor that takes a list of axes? private CoordinatesHelper(Builder builder, NetcdfDataset ncd) { List axes = new ArrayList<>(); - addAxes(ncd.getRootGroup(), axes); - this.coordAxes = ImmutableList.copyOf(axes); +// if (builder.coordAxes == null || builder.coordAxes.isEmpty()) { + addAxes(ncd.getRootGroup(), axes); +// } else { +// axes = builder.coordAxes.stream().map(ct -> ct.build(ncd.getRootGroup())).filter(Objects::nonNull).collect(Collectors.toList()); +// } + coordAxes = ImmutableList.copyOf(axes); coordTransforms = builder.coordTransforms.stream().map(ct -> ct.build(ncd)).filter(Objects::nonNull).collect(Collectors.toList()); diff --git a/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java b/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java index 7352c3e8f8..bf489f6b81 100755 --- a/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java +++ b/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java @@ -390,6 +390,10 @@ private FileCacheable acquireCacheOnly(Object hashKey) { return want.ncfile; } + public void replace (Object oldHashKey, Object newHasKey){ + + } + // LOOK should you remove the entire CacheElement ? private void remove(CacheElement.CacheFile want) { want.remove(); diff --git a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java index c5cdbe207a..5f85cc4d42 100644 --- a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java +++ b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java @@ -53,8 +53,7 @@ public void testStandaloneNoEnhanceDataset() throws IOException { Variable scaledvar = ncfile.findVariable("scaledvar"); assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.SHORT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); - assertThat(scaledvar.attributes().findAttributeDouble("scale_factor", 1.0)).isEqualTo(2.0); + assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isFalse(); assertThat(scaledvar.readScalarShort()).isEqualTo(1); } } @@ -72,8 +71,7 @@ public void testStandaloneEnhance() throws IOException { Variable scaledvar = ncfile.findVariable("scaledvar"); assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.FLOAT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); - assertThat(scaledvar.attributes().findAttributeDouble("scale_factor", 1.0)).isEqualTo(2.0); + assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isFalse(); assertThat(scaledvar.readScalarFloat()).isEqualTo(12.0f); } } @@ -91,8 +89,7 @@ public void testStandaloneEnhanceDataset() throws IOException { Variable scaledvar = ncfile.findVariable("scaledvar"); assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.FLOAT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); - assertThat(scaledvar.attributes().findAttributeDouble("scale_factor", 1.0)).isEqualTo(2.0); + assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isFalse(); assertThat(scaledvar.readScalarFloat()).isEqualTo(12.0f); } } diff --git a/cdm/core/src/test/java/ucar/nc2/ncml/TestEnhance.java b/cdm/core/src/test/java/ucar/nc2/ncml/TestEnhance.java index 4d60a7987d..db1eafc08f 100644 --- a/cdm/core/src/test/java/ucar/nc2/ncml/TestEnhance.java +++ b/cdm/core/src/test/java/ucar/nc2/ncml/TestEnhance.java @@ -68,8 +68,7 @@ public void testStandaloneEnhance() throws IOException { Variable scaledvar = ncfile.findVariable("scaledvar"); assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.FLOAT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); - assertThat(scaledvar.attributes().findAttributeDouble("scale_factor", 1.0)).isEqualTo(2.0); + assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isFalse(); assertThat(scaledvar.readScalarFloat()).isEqualTo(12.0f); } } @@ -87,8 +86,7 @@ public void testStandaloneEnhanceDataset() throws IOException { Variable scaledvar = ncfile.findVariable("scaledvar"); assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.FLOAT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); - assertThat(scaledvar.attributes().findAttributeDouble("scale_factor", 1.0)).isEqualTo(2.0); + assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isFalse(); assertThat(scaledvar.readScalarFloat()).isEqualTo(12.0f); } } @@ -106,8 +104,7 @@ public void testStandaloneDoubleEnhanceDataset() throws IOException { Variable scaledvar = ncfile.findVariable("scaledvar"); assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.FLOAT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); - assertThat(scaledvar.attributes().findAttributeDouble("scale_factor", 1.0)).isEqualTo(2.0); + assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isFalse(); assertThat(scaledvar.readScalarFloat()).isEqualTo(12.0f); } } From be10e25722493da0e203fe2e0f29c01b61d2a30f Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Fri, 17 Nov 2023 13:12:56 -0800 Subject: [PATCH 24/36] clean up --- cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java | 5 ----- .../src/main/java/ucar/nc2/dataset/NetcdfDatasets.java | 1 - .../ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java | 2 +- .../ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java | 2 -- .../java/ucar/nc2/internal/dataset/CoordinatesHelper.java | 7 +------ cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java | 4 ---- 6 files changed, 2 insertions(+), 19 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java index 111b39443c..12f1258970 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java @@ -932,10 +932,6 @@ public ImmutableList getCoordinateAxes() { return ImmutableList.copyOf(coordAxes); } - public void setCoordinateAxes(List axes) { - this.coordAxes = axes; - } - /** * Clear Coordinate System metadata, to allow them to be redone * @@ -1713,7 +1709,6 @@ public void replaceCoordinateAxis(Group.Builder group, CoordinateAxis.Builder ax axis.setParentGroupBuilder(group); } - public T setOrgFile(NetcdfFile orgFile) { this.orgFile = orgFile; return self(); diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDatasets.java b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDatasets.java index e7c14c445b..588d6abada 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDatasets.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDatasets.java @@ -13,7 +13,6 @@ import ucar.nc2.internal.dataset.DatasetEnhancer; import ucar.nc2.internal.ncml.NcmlReader; import ucar.nc2.util.CancelTask; -import ucar.nc2.util.CompareNetcdf2; import ucar.nc2.util.cache.FileCache; import ucar.nc2.util.cache.FileCacheIF; import ucar.nc2.util.cache.FileFactory; diff --git a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java index 525930f9c7..53cdddc99f 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java +++ b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageAdapter.java @@ -155,7 +155,7 @@ private static CoverageCoordAxis makeCoordAxisFromDimension(Dimension dim) { private static ucar.nc2.util.Optional makeCoordAxis(FeatureType ftype, ucar.nc2.dataset.CoordinateAxis dtCoordAxis, DtCoverageAdapter reader) { - String name = dtCoordAxis.getFullName(); + String name = dtCoordAxis.getFullName(); DataType dataType = dtCoordAxis.getDataType(); AxisType axisType = dtCoordAxis.getAxisType(); String units = dtCoordAxis.getUnitsString(); diff --git a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java index 15bc2d4169..3085b313b8 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java +++ b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageCSBuilder.java @@ -5,7 +5,6 @@ package ucar.nc2.ft2.coverage.adapter; import com.google.common.collect.Lists; -import thredds.client.catalog.builder.DatasetBuilder; import ucar.nc2.Dimension; import ucar.nc2.constants.AxisType; import ucar.nc2.constants.FeatureType; @@ -180,7 +179,6 @@ public static String describe(NetcdfDataset ds, CoordinateSystem cs, Formatter e ////////////////////////////////////////////////////////////// // time - boolean axesChanged = false; // if any time axes are rebuilt by the factory, we need to update the netcdfDataset CoordinateAxis rt = cs.findAxis(AxisType.RunTime); if (rt != null) { if (!rt.isScalar() && !(rt instanceof CoordinateAxis1D)) { // A runtime axis must be scalar or one-dimensional diff --git a/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordinatesHelper.java b/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordinatesHelper.java index 00161910e9..6b859282d8 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordinatesHelper.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordinatesHelper.java @@ -53,14 +53,9 @@ public List getCoordTransforms() { return coordTransforms; } - // make another constructor that takes a list of axes? private CoordinatesHelper(Builder builder, NetcdfDataset ncd) { List axes = new ArrayList<>(); -// if (builder.coordAxes == null || builder.coordAxes.isEmpty()) { - addAxes(ncd.getRootGroup(), axes); -// } else { -// axes = builder.coordAxes.stream().map(ct -> ct.build(ncd.getRootGroup())).filter(Objects::nonNull).collect(Collectors.toList()); -// } + addAxes(ncd.getRootGroup(), axes); coordAxes = ImmutableList.copyOf(axes); coordTransforms = diff --git a/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java b/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java index bf489f6b81..7352c3e8f8 100755 --- a/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java +++ b/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java @@ -390,10 +390,6 @@ private FileCacheable acquireCacheOnly(Object hashKey) { return want.ncfile; } - public void replace (Object oldHashKey, Object newHasKey){ - - } - // LOOK should you remove the entire CacheElement ? private void remove(CacheElement.CacheFile want) { want.remove(); From c4bb430620fd12fe17d224b276ed20d2736a08e1 Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Mon, 27 Nov 2023 14:33:56 -0800 Subject: [PATCH 25/36] use wasRead in coordinateAcix1DTime factory --- .../ucar/nc2/dataset/CoordinateAxis1DTime.java | 16 +++++++++------- .../main/java/ucar/nc2/dataset/VariableDS.java | 8 ++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java index 15bd234d5e..0b601c6cc8 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java @@ -342,14 +342,17 @@ private CoordinateAxis1DTime(NetcdfDataset ncd, VariableDS org, Formatter errMes int ncoords = (int) org.getSize(); List result = new ArrayList<>(ncoords); - Array data = org.read(); - coords = (double[]) data.get1DJavaArray(DataType.DOUBLE); + if(org instanceof CoordinateAxis1D) { + coords = ((CoordinateAxis1D)org).getCoordValues(); + } else { + Array data = org.read(); + coords = (double[]) data.get1DJavaArray(DataType.DOUBLE); + } this.wasRead = true; int count = 0; - IndexIterator ii = data.getIndexIterator(); for (int i = 0; i < ncoords; i++) { - double val = ii.getDoubleNext(); + double val = coords[i]; if (Double.isNaN(val)) continue; // WTF ?? result.add(helper.makeCalendarDateFromOffset(val)); @@ -362,12 +365,11 @@ private CoordinateAxis1DTime(NetcdfDataset ncd, VariableDS org, Formatter errMes setDimension(0, localDim); // set the shortened values - Array shortData = Array.factory(data.getDataType(), new int[] {count}); + Array shortData = Array.factory(org.getDataType(), new int[] {count}); Index ima = shortData.getIndex(); int count2 = 0; - ii = data.getIndexIterator(); for (int i = 0; i < ncoords; i++) { - double val = ii.getDoubleNext(); + double val = coords[i]; if (Double.isNaN(val)) continue; shortData.setDouble(ima.set0(count2), val); diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java index cfa3775292..88c659bd98 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java @@ -277,7 +277,6 @@ Array convert(Array data, Set enhancements) { if (enhancements.contains(Enhance.ApplyNormalizer) && normalizer != null) { data = normalizer.convert(data); } - this.wasConverted = true; return data; } } @@ -444,7 +443,7 @@ protected Array _read() throws IOException { result = proxyReader.reallyRead(this, null); } - return wasConverted ? result : convert(result); + return convert(result); } // section of regular Variable @@ -461,8 +460,7 @@ protected Array _read(Section section) throws IOException, InvalidRangeException } else { result = proxyReader.reallyRead(this, section, null); } - - return wasConverted ? result : convert(result); + return convert(result); } // do not call directly @@ -821,8 +819,6 @@ public Array convert(Array in, boolean convertUnsigned, boolean applyScaleOffset private boolean hasFillValue = false; private double fillValue = Double.MAX_VALUE; - protected boolean wasConverted = false; - protected VariableDS(Builder builder, Group parentGroup) { super(builder, parentGroup); From 62e8bfab5aa70e0353d45e27f261fbc6d5943c10 Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Mon, 27 Nov 2023 14:35:35 -0800 Subject: [PATCH 26/36] spotless --- .../java/ucar/nc2/dataset/CoordinateAxis1DTime.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java index 0b601c6cc8..233d9b6b86 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java @@ -342,12 +342,12 @@ private CoordinateAxis1DTime(NetcdfDataset ncd, VariableDS org, Formatter errMes int ncoords = (int) org.getSize(); List result = new ArrayList<>(ncoords); - if(org instanceof CoordinateAxis1D) { - coords = ((CoordinateAxis1D)org).getCoordValues(); - } else { - Array data = org.read(); - coords = (double[]) data.get1DJavaArray(DataType.DOUBLE); - } + if (org instanceof CoordinateAxis1D) { + coords = ((CoordinateAxis1D) org).getCoordValues(); + } else { + Array data = org.read(); + coords = (double[]) data.get1DJavaArray(DataType.DOUBLE); + } this.wasRead = true; int count = 0; From 9a57f50ea782800b36fd96b9bd915aef0fe4d1c6 Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Mon, 27 Nov 2023 15:41:48 -0800 Subject: [PATCH 27/36] fix failing tests and persist scale/offset attributes via dataset builders --- .../main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java | 5 ----- cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java | 9 ++++++++- .../ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java | 5 ++--- .../test/java/ucar/nc2/internal/ncml/TestEnhance.java | 5 ++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java index 233d9b6b86..b723fdd592 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java @@ -212,11 +212,6 @@ protected void readValues() { } // make sure parent methods do not try to read from the orgVar again this.wasRead = true; - if (orgVar instanceof CoordinateAxis1D) { - CoordinateAxis1D orgAxis = (CoordinateAxis1D) orgVar; - this.wasRead = orgAxis.wasRead; - this.coords = orgAxis.coords; - } } //////////////////////////////////////////////////////////////////////// diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java index 88c659bd98..b552f2f63d 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java @@ -841,6 +841,7 @@ protected VariableDS(Builder builder, Group parentGroup) { this.orgFileTypeId = builder.orgFileTypeId; this.enhanceProxy = new EnhancementsImpl(this, builder.units, builder.getDescription()); + this.scaleOffset = builder.scaleOffset; createEnhancements(); @@ -859,7 +860,9 @@ private void createEnhancements() { this.dataType = unsignedConversion != null ? unsignedConversion.getOutType() : dataType; } if (this.enhanceMode.contains(Enhance.ApplyScaleOffset) && (dataType.isNumeric() || dataType == DataType.CHAR)) { - this.scaleOffset = ScaleOffset.createFromVariable(this); + if (this.scaleOffset == null) { + this.scaleOffset = ScaleOffset.createFromVariable(this); + } this.dataType = scaleOffset != null ? scaleOffset.getScaledOffsetType() : this.dataType; } Attribute standardizerAtt = findAttribute(CDM.STANDARDIZE); @@ -903,6 +906,7 @@ protected Builder addLocalFieldsToBuilder(Builder> build builder.setOriginalVariable(this.orgVar).setOriginalDataType(this.orgDataType).setOriginalName(this.orgName) .setOriginalFileTypeId(this.orgFileTypeId).setEnhanceMode(this.enhanceMode).setUnits(this.enhanceProxy.units) .setDesc(this.enhanceProxy.desc); + builder.scaleOffset = this.scaleOffset; if (this.coordSysNames != null) { this.coordSysNames.stream().forEach(s -> builder.addCoordinateSystemName(s)); } @@ -948,6 +952,8 @@ public static abstract class Builder> extends Variable.Buil private boolean fillValueIsMissing = NetcdfDataset.fillValueIsMissing; private boolean missingDataIsMissing = NetcdfDataset.missingDataIsMissing; + private ScaleOffset scaleOffset; + private boolean built; protected abstract T self(); @@ -1044,6 +1050,7 @@ public T copyFrom(VariableDS.Builder builder) { setFillValueIsMissing(builder.fillValueIsMissing); setInvalidDataIsMissing(builder.invalidDataIsMissing); setMissingDataIsMissing(builder.missingDataIsMissing); + this.scaleOffset = builder.scaleOffset; this.orgVar = builder.orgVar; this.orgDataType = builder.orgDataType; this.orgFileTypeId = builder.orgFileTypeId; diff --git a/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java b/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java index cb2faddb43..4e791a0972 100644 --- a/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java +++ b/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java @@ -8,6 +8,7 @@ import static java.lang.Float.NaN; import org.junit.Assert; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -85,7 +86,6 @@ public void testWrite() throws Exception { Array readEnhanced; - // read the packed form, enhance using scale/offset, compare to original try (NetcdfDataset ncd = NetcdfDatasets.openDataset(filename)) { VariableDS vs = (VariableDS) ncd.findVariable("packed"); vs.removeEnhancement(Enhance.ConvertMissing); @@ -133,7 +133,6 @@ private void doSubset(String filename) throws IOException, InvalidRangeException } } - // Asserts that "scale_factor" is applied to "_FillValue". // This test demonstrated the bug in https://github.com/Unidata/thredds/issues/1065. @Test @@ -141,7 +140,7 @@ public void testScaledFillValue() throws URISyntaxException, IOException { File testResource = new File(getClass().getResource("testScaledFillValue.ncml").toURI()); // LOOK removeEnhancement does not work in new - try (NetcdfDataset ncd = NetcdfDataset.openDataset(testResource.getAbsolutePath(), true, null)) { + try (NetcdfDataset ncd = NetcdfDatasets.openDataset(testResource.getAbsolutePath(), true, null)) { VariableDS fooVar = (VariableDS) ncd.findVariable("foo"); double expectedFillValue = .99999; diff --git a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java index 5f85cc4d42..41585fd5d6 100644 --- a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java +++ b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java @@ -53,7 +53,8 @@ public void testStandaloneNoEnhanceDataset() throws IOException { Variable scaledvar = ncfile.findVariable("scaledvar"); assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.SHORT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isFalse(); + assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); + assertThat(scaledvar.attributes().findAttributeDouble("scale_factor", 1.0)).isEqualTo(2.0); assertThat(scaledvar.readScalarShort()).isEqualTo(1); } } @@ -107,8 +108,6 @@ public void testStandaloneDoubleEnhanceDataset() throws IOException { Variable scaledvar = ncfile.findVariable("scaledvar"); assertThat((Object) scaledvar).isNotNull(); assertThat(scaledvar.getDataType()).isEqualTo(DataType.FLOAT); - assertThat(scaledvar.attributes().hasAttribute("scale_factor")).isTrue(); - assertThat(scaledvar.attributes().findAttributeDouble("scale_factor", 1.0)).isEqualTo(2.0); assertThat(scaledvar.readScalarFloat()).isEqualTo(12.0f); } } From 0095a36a0299e5add0d57a29177620b60e93b1ea Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Tue, 28 Nov 2023 12:05:30 -0800 Subject: [PATCH 28/36] cleanup --- .../src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java | 4 ++-- .../java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java index b723fdd592..1852cf943b 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/CoordinateAxis1DTime.java @@ -207,11 +207,11 @@ protected void readValues() { // the axis was created by this classes factory. if (this.orgDataType != null && !this.orgDataType.isNumeric()) { this.coords = cdates.stream().mapToDouble(cdate -> (double) cdate.getDifferenceInMsecs(cdates.get(0))).toArray(); + // make sure parent methods do not try to read from the orgVar again + this.wasRead = true; } else { super.readValues(); } - // make sure parent methods do not try to read from the orgVar again - this.wasRead = true; } //////////////////////////////////////////////////////////////////////// diff --git a/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java b/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java index 4e791a0972..8d75ca0880 100644 --- a/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java +++ b/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java @@ -8,7 +8,6 @@ import static java.lang.Float.NaN; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -85,7 +84,7 @@ public void testWrite() throws Exception { } Array readEnhanced; - + // read the packed form, enhance using scale/offset, compare to original try (NetcdfDataset ncd = NetcdfDatasets.openDataset(filename)) { VariableDS vs = (VariableDS) ncd.findVariable("packed"); vs.removeEnhancement(Enhance.ConvertMissing); From eb41b6923ad510e07727b9b104165162f57399b9 Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Fri, 1 Dec 2023 11:10:40 -0500 Subject: [PATCH 29/36] upgrade logback --- netcdf-java-platform/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netcdf-java-platform/build.gradle b/netcdf-java-platform/build.gradle index ecfffe1993..300811adb4 100644 --- a/netcdf-java-platform/build.gradle +++ b/netcdf-java-platform/build.gradle @@ -76,7 +76,7 @@ dependencies { // netcdf-java logging api "org.slf4j:slf4j-api:${depVersion.slf4j}" - runtime 'ch.qos.logback:logback-classic:1.2.9' + runtime 'ch.qos.logback:logback-classic:1.4.13' // legacy gradle module // todo: remove with legacy in 6 From aa419e0be5e44ed455292505eaeaca5144862a9f Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Mon, 4 Dec 2023 08:43:32 -0800 Subject: [PATCH 30/36] upgrade grpc --- gradle/any/shared-mvn-coords.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/any/shared-mvn-coords.gradle b/gradle/any/shared-mvn-coords.gradle index 0a066b1d75..1545d5cda1 100644 --- a/gradle/any/shared-mvn-coords.gradle +++ b/gradle/any/shared-mvn-coords.gradle @@ -23,5 +23,5 @@ ext { depVersion = [:] depVersion.slf4j = '1.7.28' depVersion.protobuf = '3.23.0' - depVersion.grpc = '1.57.2' + depVersion.grpc = '1.59.1' } From 00fbda184efee4c864f0e4e25991d4c412787e49 Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Tue, 5 Dec 2023 12:45:22 -0800 Subject: [PATCH 31/36] fix failing tests in the tds --- .../ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java index 75ca0eb97f..26e5aecc8c 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java +++ b/cdm/core/src/main/java/ucar/nc2/ft2/coverage/adapter/DtCoverageDataset.java @@ -109,12 +109,12 @@ public DtCoverageDataset(NetcdfDataset ncd) throws IOException { * @param parseInfo put parse info here, may be null * @throws java.io.IOException on read error */ - public DtCoverageDataset(NetcdfDataset ncd, Formatter parseInfo) throws IOException { - Set enhance = ncd.getEnhanceMode(); + public DtCoverageDataset(NetcdfDataset ds, Formatter parseInfo) throws IOException { + Set enhance = ds.getEnhanceMode(); if (enhance == null || enhance.isEmpty()) { enhance = NetcdfDataset.getDefaultEnhanceMode(); } - this.ncd = NetcdfDatasets.enhance(ncd, enhance, null); + this.ncd = NetcdfDatasets.enhance(ds, enhance, null); // sort by largest size first List csList = new ArrayList<>(ncd.getCoordinateSystems()); From f671aca33652e6903812231d5fa986a13d60ecdb Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 5 Dec 2023 14:26:19 -0700 Subject: [PATCH 32/36] Add test for cache cleaning up deleted files --- .../TestRandomAccessFileCacheCleanup.java | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 cdm/core/src/test/java/ucar/nc2/util/cache/TestRandomAccessFileCacheCleanup.java diff --git a/cdm/core/src/test/java/ucar/nc2/util/cache/TestRandomAccessFileCacheCleanup.java b/cdm/core/src/test/java/ucar/nc2/util/cache/TestRandomAccessFileCacheCleanup.java new file mode 100644 index 0000000000..ce1f2ba2a6 --- /dev/null +++ b/cdm/core/src/test/java/ucar/nc2/util/cache/TestRandomAccessFileCacheCleanup.java @@ -0,0 +1,43 @@ +package ucar.nc2.util.cache; + +import static com.google.common.truth.Truth.assertThat; + +import java.io.File; +import java.io.IOException; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import ucar.unidata.io.RandomAccessFile; + +public class TestRandomAccessFileCacheCleanup { + + @ClassRule + public static final TemporaryFolder tempFolder = new TemporaryFolder(); + + private static FileCache cache; + + @BeforeClass + public static void enableCache() { + cache = new FileCache("RandomAccessFile", 0, 1, 1, 0); + RandomAccessFile.setGlobalFileCache(cache); + } + + @AfterClass + public static void shutdownCache() { + RandomAccessFile.setGlobalFileCache(null); + } + + @Test + public void shouldReleaseDeletedFileDuringCleanup() throws IOException { + final File toDelete = tempFolder.newFile(); + RandomAccessFile.acquire(toDelete.getAbsolutePath()); + RandomAccessFile.acquire(tempFolder.newFile().getAbsolutePath()); + assertThat(cache.showCache().size()).isEqualTo(2); + + assertThat(toDelete.delete()).isTrue(); + cache.cleanup(1); + assertThat(cache.showCache().size()).isEqualTo(1); + } +} From 3b42c9aa370f86e98dfb9b1030b02879790448a8 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Tue, 5 Dec 2023 16:31:27 -0700 Subject: [PATCH 33/36] Remove deleted files from cache when performing a cleanup --- .../main/java/ucar/nc2/util/cache/FileCache.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java b/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java index 7352c3e8f8..adfa3a71fa 100755 --- a/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java +++ b/cdm/core/src/main/java/ucar/nc2/util/cache/FileCache.java @@ -4,6 +4,8 @@ */ package ucar.nc2.util.cache; +import java.nio.file.Files; +import java.nio.file.Paths; import ucar.nc2.dataset.DatasetUrl; import ucar.nc2.time.CalendarDate; import ucar.nc2.time.CalendarDateFormatter; @@ -697,13 +699,11 @@ public int compareTo(Tracker o) { synchronized void cleanup(int maxElements) { try { - /* - * int size = counter.get(); - * int fsize = files.size(); - * if (debug && (size != fsize)) { - * log.warn("FileCache " + name + " counter " + size + " doesnt match files().size=" + fsize); - * } - */ + for (CacheElement.CacheFile cacheFile : files.values()) { + if (!Files.exists(Paths.get(cacheFile.ncfile.getLocation()))) { + remove(cacheFile); + } + } int size = files.size(); if (size <= minElements) From 1705c65c4427e3d01dc05b31734e1b420f3bea44 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Mon, 11 Dec 2023 13:36:52 -0700 Subject: [PATCH 34/36] Rewrite groovy test as a junit test --- .../ucar/nc2/iosp/netcdf3/N3iospSpec.groovy | 64 ------------------- .../ucar/nc2/iosp/netcdf3/TestN3iosp.java | 56 ++++++++++++++++ 2 files changed, 56 insertions(+), 64 deletions(-) delete mode 100644 cdm/core/src/test/groovy/ucar/nc2/iosp/netcdf3/N3iospSpec.groovy create mode 100644 cdm/core/src/test/java/ucar/nc2/iosp/netcdf3/TestN3iosp.java diff --git a/cdm/core/src/test/groovy/ucar/nc2/iosp/netcdf3/N3iospSpec.groovy b/cdm/core/src/test/groovy/ucar/nc2/iosp/netcdf3/N3iospSpec.groovy deleted file mode 100644 index 253af402e8..0000000000 --- a/cdm/core/src/test/groovy/ucar/nc2/iosp/netcdf3/N3iospSpec.groovy +++ /dev/null @@ -1,64 +0,0 @@ -package ucar.nc2.iosp.netcdf3 - -import org.slf4j.Logger -import org.slf4j.LoggerFactory -import spock.lang.Specification - -/** - * @author cwardgar - * @since 2015/09/16 - */ -class N3iospSpec extends Specification { - private static final Logger logger = LoggerFactory.getLogger(N3iospSpec) - - def "test invalid NetCDF object names: null or empty"() { - expect: "null names are invalid" - !N3iosp.isValidNetcdfObjectName(null) - - when: - N3iosp.makeValidNetcdfObjectName(null) - then: - thrown(NullPointerException) - - expect: "empty names are invalid" - !N3iosp.isValidNetcdfObjectName("") - - when: - N3iosp.makeValidNetcdfObjectName("") - then: - IllegalArgumentException e = thrown() - e.message == "Illegal NetCDF object name: ''" - } - - def "test invalid NetCDF object names: first char"() { - expect: "names with first chars not in ([a-zA-Z0-9_]|{UTF8}) are invalid" - !N3iosp.isValidNetcdfObjectName(" blah") - N3iosp.makeValidNetcdfObjectName(" blah") == "blah" - - !N3iosp.isValidNetcdfObjectName("\n/blah") - N3iosp.makeValidNetcdfObjectName("\n/blah") == "blah" - - !N3iosp.isValidNetcdfObjectName("\u001F\u007F blah") // Unit separator and DEL - N3iosp.makeValidNetcdfObjectName("\u001F\u007F blah") == "blah" - } - - def "test invalid NetCDF object names: remaining chars"() { - expect: "names with remaining chars not in ([^\\x00-\\x1F\\x7F/]|{UTF8})* are invalid" - !N3iosp.isValidNetcdfObjectName("1\u000F2\u007F3/4") - N3iosp.makeValidNetcdfObjectName("1\u000F2\u007F3/4") == "1234" - - and: "names may not have trailing spaces" - !N3iosp.isValidNetcdfObjectName("foo ") - N3iosp.makeValidNetcdfObjectName("foo ") == "foo" - } - - def "test valid NetCDF object names"() { - expect: "valid names have syntax: ([a-zA-Z0-9_]|{UTF8})([^\\x00-\\x1F\\x7F/]|{UTF8})*" - N3iosp.isValidNetcdfObjectName("_KfS9Jn_s9__") - N3iosp.makeValidNetcdfObjectName("_KfS9Jn_s9__") == "_KfS9Jn_s9__" - - and: "unicode characters greater than 0x7F can appear anywhere" - N3iosp.isValidNetcdfObjectName("\u0123\u1234\u2345\u3456") - N3iosp.makeValidNetcdfObjectName("\u0123\u1234\u2345\u3456") == "\u0123\u1234\u2345\u3456" - } -} diff --git a/cdm/core/src/test/java/ucar/nc2/iosp/netcdf3/TestN3iosp.java b/cdm/core/src/test/java/ucar/nc2/iosp/netcdf3/TestN3iosp.java new file mode 100644 index 0000000000..4ed330775e --- /dev/null +++ b/cdm/core/src/test/java/ucar/nc2/iosp/netcdf3/TestN3iosp.java @@ -0,0 +1,56 @@ +package ucar.nc2.iosp.netcdf3; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import org.junit.Test; + +public class TestN3iosp { + + @Test + public void shouldReturnInvalidForNullOrEmptyNames() { + assertThat(N3iosp.isValidNetcdfObjectName(null)).isFalse(); + assertThrows(NullPointerException.class, () -> N3iosp.makeValidNetcdfObjectName(null)); + + assertThat(N3iosp.isValidNetcdfObjectName("")).isFalse(); + IllegalArgumentException e = + assertThrows(IllegalArgumentException.class, () -> N3iosp.makeValidNetcdfObjectName("")); + assertThat(e.getMessage()).isEqualTo("Illegal NetCDF object name: ''"); + } + + @Test + public void shouldReturnInvalidForBadFirstCharacters() { + // names with first chars not in ([a-zA-Z0-9_]|{UTF8}) are invalid + assertThat(N3iosp.isValidNetcdfObjectName(" blah")).isFalse(); + assertThat(N3iosp.makeValidNetcdfObjectName(" blah")).isEqualTo("blah"); + + assertThat(N3iosp.isValidNetcdfObjectName("\n/blah")).isFalse(); + assertThat(N3iosp.makeValidNetcdfObjectName("\n/blah")).isEqualTo("blah"); + + // Unit separator and DEL + assertThat(N3iosp.isValidNetcdfObjectName("\u001F\u007F blah")).isFalse(); + assertThat(N3iosp.makeValidNetcdfObjectName("\u001F\u007F blah")).isEqualTo("blah"); + } + + @Test + public void shouldReturnInvalidForBadCharacters() { + // names with remaining chars not in ([^\x00-\x1F\x7F/]|{UTF8})* are invalid + assertThat(N3iosp.isValidNetcdfObjectName("1\u000F2\u007F3/4")).isFalse(); + assertThat(N3iosp.makeValidNetcdfObjectName("1\u000F2\u007F3/4")).isEqualTo("1234"); + + // names may not have trailing spaces + assertThat(N3iosp.isValidNetcdfObjectName("foo ")).isFalse(); + assertThat(N3iosp.makeValidNetcdfObjectName("foo ")).isEqualTo("foo"); + } + + @Test + public void shouldAcceptValidNames() { + // valid names have syntax: ([a-zA-Z0-9_]|{UTF8})([^\x00-\x1F\x7F/]|{UTF8})* + assertThat(N3iosp.isValidNetcdfObjectName("_KfS9Jn_s9__")).isTrue(); + assertThat(N3iosp.makeValidNetcdfObjectName("_KfS9Jn_s9__")).isEqualTo("_KfS9Jn_s9__"); + + // unicode characters greater than 0x7F can appear anywhere + assertThat(N3iosp.isValidNetcdfObjectName("\u0123\u1234\u2345\u3456")).isTrue(); + assertThat(N3iosp.makeValidNetcdfObjectName("\u0123\u1234\u2345\u3456")).isEqualTo("\u0123\u1234\u2345\u3456"); + } +} From da5eb18d00d941f18ca7dfb1f990219e6857d4f7 Mon Sep 17 00:00:00 2001 From: haileyajohnson Date: Tue, 12 Dec 2023 08:31:32 -0800 Subject: [PATCH 35/36] upgrade logback and grpc --- gradle/any/shared-mvn-coords.gradle | 2 +- netcdf-java-platform/build.gradle | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gradle/any/shared-mvn-coords.gradle b/gradle/any/shared-mvn-coords.gradle index 1545d5cda1..71c2c4f1a4 100644 --- a/gradle/any/shared-mvn-coords.gradle +++ b/gradle/any/shared-mvn-coords.gradle @@ -23,5 +23,5 @@ ext { depVersion = [:] depVersion.slf4j = '1.7.28' depVersion.protobuf = '3.23.0' - depVersion.grpc = '1.59.1' + depVersion.grpc = '1.60.0' } diff --git a/netcdf-java-platform/build.gradle b/netcdf-java-platform/build.gradle index 300811adb4..f04863c81a 100644 --- a/netcdf-java-platform/build.gradle +++ b/netcdf-java-platform/build.gradle @@ -76,7 +76,7 @@ dependencies { // netcdf-java logging api "org.slf4j:slf4j-api:${depVersion.slf4j}" - runtime 'ch.qos.logback:logback-classic:1.4.13' + runtime 'ch.qos.logback:logback-classic:1.4.14' // legacy gradle module // todo: remove with legacy in 6 From 2f1021e848c8e6044f1b52ef438770e1005dc577 Mon Sep 17 00:00:00 2001 From: Megan Lerman Date: Wed, 20 Dec 2023 15:47:34 -0800 Subject: [PATCH 36/36] hide file paths in error messages --- .../main/java/ucar/nc2/ft/FeatureDatasetFactoryManager.java | 4 ++-- .../main/java/ucar/nc2/ft/point/standard/TableAnalyzer.java | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/ft/FeatureDatasetFactoryManager.java b/cdm/core/src/main/java/ucar/nc2/ft/FeatureDatasetFactoryManager.java index 64c55d081c..8f3bca76b8 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft/FeatureDatasetFactoryManager.java +++ b/cdm/core/src/main/java/ucar/nc2/ft/FeatureDatasetFactoryManager.java @@ -315,8 +315,8 @@ public static FeatureDataset wrap(FeatureType wantFeatureType, NetcdfDataset ncd } } - if (null == useFactory) { - errlog.format("**Failed to find FeatureDatasetFactory for= %s datatype=%s%n", ncd.getLocation(), wantFeatureType); + if (useFactory == null) { + errlog.format("**Failed to find FeatureDatasetFactory for datatype=%s%n", wantFeatureType); return null; } diff --git a/cdm/core/src/main/java/ucar/nc2/ft/point/standard/TableAnalyzer.java b/cdm/core/src/main/java/ucar/nc2/ft/point/standard/TableAnalyzer.java index a9e8abd087..99fdd929c0 100644 --- a/cdm/core/src/main/java/ucar/nc2/ft/point/standard/TableAnalyzer.java +++ b/cdm/core/src/main/java/ucar/nc2/ft/point/standard/TableAnalyzer.java @@ -619,7 +619,6 @@ private void writeConfigXML(java.util.Formatter sf) { private Document makeDocument() { Element rootElem = new Element("featureDataset"); Document doc = new Document(rootElem); - rootElem.setAttribute("location", ds.getLocation()); rootElem.addContent(new Element("analyser").setAttribute("class", getName())); if (ft != null) rootElem.setAttribute("featureType", ft.toString());