diff --git a/cdm/core/src/main/java/thredds/inventory/MFiles.java b/cdm/core/src/main/java/thredds/inventory/MFiles.java index c982da6952..799267ea50 100644 --- a/cdm/core/src/main/java/thredds/inventory/MFiles.java +++ b/cdm/core/src/main/java/thredds/inventory/MFiles.java @@ -12,6 +12,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import thredds.filesystem.MFileOS; +import thredds.filesystem.MFileOS7; import ucar.nc2.internal.ncml.NcmlReader; /** @@ -66,4 +67,14 @@ public static MFile createIfExists(String location) { final MFile mFile = create(location); return mFile.exists() ? mFile : null; } + + /** + * Checks if MFile represents a file stored on a local filesystem + * + * @param mfile MFile to check + * @return true if MFile is a local filesystem + */ + public static boolean isLocal(MFile mfile) { + return mfile instanceof MFileOS || mfile instanceof MFileOS7; + } } diff --git a/grib/src/main/java/ucar/nc2/grib/GribIndexCache.java b/grib/src/main/java/ucar/nc2/grib/GribIndexCache.java index 58b61aa168..ce3c1737f3 100644 --- a/grib/src/main/java/ucar/nc2/grib/GribIndexCache.java +++ b/grib/src/main/java/ucar/nc2/grib/GribIndexCache.java @@ -51,6 +51,17 @@ public static MFile getFileOrCache(String fileLocation) { * @return existing file if you can find it, else null */ public static MFile getExistingFileOrCache(String fileLocation) { + MFile idxMFile = MFiles.create(fileLocation); + if (!MFiles.isLocal(idxMFile)) { + // for remote file systems, check to see if the index file exists and, if so, use it + // note: the remote index file may require updating, which isn't support at this point, + // so opening the remote GRIB file may ultimately fail. + if (idxMFile.exists()) { + return idxMFile; + } + } + // if the GRIB index file is local OR the remote index does not exist, check + // the DiskCache. File result = getDiskCache2().getExistingFileOrCache(fileLocation); if (result == null && Grib.debugGbxIndexOnly && fileLocation.endsWith(".gbx9.ncx4")) { // might create only from // gbx9 for debugging diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib1CollectionWriter.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib1CollectionWriter.java index 9b8639700b..6ab4ae6b97 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib1CollectionWriter.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib1CollectionWriter.java @@ -91,23 +91,26 @@ public Set getCoordinateRuntimes() { */ // indexFile is in the cache - boolean writeIndex(String name, MFile idxFile, CoordinateRuntime masterRuntime, List groups, List files, - GribCollectionImmutable.Type type, CalendarDateRange dateRange) throws IOException { + boolean writeIndex(String name, MFile idxMFile, CoordinateRuntime masterRuntime, List groups, + List files, GribCollectionImmutable.Type type, CalendarDateRange dateRange) throws IOException { + if (!MFiles.isLocal(idxMFile)) { + throw new IllegalArgumentException("Only local file systems are supported for index creation"); + } Grib1Record first = null; // take global metadata from here boolean deleteOnClose = false; - if (idxFile.exists()) { - RandomAccessFile.eject(idxFile.getPath()); - if (idxFile instanceof thredds.filesystem.MFileOS) { - File f = new File(idxFile.getPath()); + if (idxMFile.exists()) { + RandomAccessFile.eject(idxMFile.getPath()); + if (idxMFile instanceof thredds.filesystem.MFileOS) { + File f = new File(idxMFile.getPath()); if (!f.delete()) { - logger.warn(" gc1 cant delete index file {}", idxFile.getPath()); + logger.warn(" gc1 cant delete index file {}", idxMFile.getPath()); } } } - logger.debug(" createIndex for {}", idxFile.getPath()); + logger.debug(" createIndex for {}", idxMFile.getPath()); - try (RandomAccessFile raf = new RandomAccessFile(idxFile.getPath(), "rw")) { + try (RandomAccessFile raf = new RandomAccessFile(idxMFile.getPath(), "rw")) { raf.order(RandomAccessFile.BIG_ENDIAN); //// header message @@ -234,10 +237,10 @@ boolean writeIndex(String name, MFile idxFile, CoordinateRuntime masterRuntime, } finally { // remove it on failure - if (deleteOnClose && idxFile instanceof thredds.filesystem.MFileOS) { - File f = new File(idxFile.getPath()); + if (deleteOnClose && idxMFile instanceof thredds.filesystem.MFileOS) { + File f = new File(idxMFile.getPath()); if (!f.delete()) - logger.error(" gc1 cant deleteOnClose index file {}", idxFile.getPath()); + logger.error(" gc1 cant deleteOnClose index file {}", idxMFile.getPath()); } } } diff --git a/grib/src/main/java/ucar/nc2/grib/collection/Grib2CollectionWriter.java b/grib/src/main/java/ucar/nc2/grib/collection/Grib2CollectionWriter.java index eb44d35790..7a45eb511e 100644 --- a/grib/src/main/java/ucar/nc2/grib/collection/Grib2CollectionWriter.java +++ b/grib/src/main/java/ucar/nc2/grib/collection/Grib2CollectionWriter.java @@ -92,23 +92,26 @@ public List getCoordinates() { * GribCollectionIndex (sizeIndex bytes) */ - boolean writeIndex(String name, MFile idxFile, CoordinateRuntime masterRuntime, List groups, List files, - GribCollectionImmutable.Type type, CalendarDateRange dateRange) throws IOException { + boolean writeIndex(String name, MFile idxMFile, CoordinateRuntime masterRuntime, List groups, + List files, GribCollectionImmutable.Type type, CalendarDateRange dateRange) throws IOException { + if (!MFiles.isLocal(idxMFile)) { + throw new IllegalArgumentException("Only local file systems are supported for index creation"); + } Grib2Record first = null; // take global metadata from here boolean deleteOnClose = false; - if (idxFile.exists()) { - RandomAccessFile.eject(idxFile.getPath()); - if (idxFile instanceof thredds.filesystem.MFileOS) { - File f = new File(idxFile.getPath()); + if (idxMFile.exists()) { + RandomAccessFile.eject(idxMFile.getPath()); + if (idxMFile instanceof thredds.filesystem.MFileOS) { + File f = new File(idxMFile.getPath()); if (!f.delete()) { - logger.error("gc2 cant delete index file {}", idxFile.getPath()); + logger.error("gc2 cant delete index file {}", idxMFile.getPath()); } } } - logger.debug(" createIndex for {}", idxFile.getPath()); + logger.debug(" createIndex for {}", idxMFile.getPath()); - try (RandomAccessFile raf = new RandomAccessFile(idxFile.getPath(), "rw")) { + try (RandomAccessFile raf = new RandomAccessFile(idxMFile.getPath(), "rw")) { //// header message raf.order(RandomAccessFile.BIG_ENDIAN); raf.write(MAGIC_START.getBytes(StandardCharsets.UTF_8)); @@ -230,10 +233,10 @@ boolean writeIndex(String name, MFile idxFile, CoordinateRuntime masterRuntime, } finally { // remove it on failure - if (deleteOnClose && idxFile instanceof thredds.filesystem.MFileOS) { - File f = new File(idxFile.getPath()); + if (deleteOnClose && idxMFile instanceof thredds.filesystem.MFileOS) { + File f = new File(idxMFile.getPath()); if (!f.delete()) - logger.error(" gc2 cant deleteOnClose index file {}", idxFile.getPath()); + logger.error(" gc2 cant deleteOnClose index file {}", idxMFile.getPath()); } } diff --git a/grib/src/main/java/ucar/nc2/grib/grib1/Grib1Index.java b/grib/src/main/java/ucar/nc2/grib/grib1/Grib1Index.java index 7ee69e5505..563c9fca5d 100644 --- a/grib/src/main/java/ucar/nc2/grib/grib1/Grib1Index.java +++ b/grib/src/main/java/ucar/nc2/grib/grib1/Grib1Index.java @@ -8,10 +8,9 @@ import com.google.protobuf.ByteString; import java.io.InputStream; import java.nio.charset.StandardCharsets; -import thredds.filesystem.MFileOS; -import thredds.filesystem.MFileOS7; import thredds.inventory.CollectionUpdateType; import thredds.inventory.MFile; +import thredds.inventory.MFiles; import ucar.nc2.NetcdfFiles; import ucar.nc2.grib.GribIndex; import ucar.nc2.grib.GribIndexCache; @@ -173,7 +172,7 @@ public boolean makeIndex(String filename, RandomAccessFile dataRaf) throws IOExc String idxPathTmp = GribUtils.makeIndexFileName(idxPath, ".tmp"); MFile idxMFileTmp = GribIndexCache.getFileOrCache(idxPathTmp); - if (!(idxMFile instanceof MFileOS || idxMFile instanceof MFileOS7)) { + if (!MFiles.isLocal(idxMFile)) { throw new IllegalArgumentException("Only local file systems are supported for index creation"); } diff --git a/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Index.java b/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Index.java index da4f2a94bd..881ffcb44d 100644 --- a/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Index.java +++ b/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Index.java @@ -8,10 +8,9 @@ import com.google.protobuf.ByteString; import java.io.InputStream; import java.nio.charset.StandardCharsets; -import thredds.filesystem.MFileOS; -import thredds.filesystem.MFileOS7; import thredds.inventory.CollectionUpdateType; import thredds.inventory.MFile; +import thredds.inventory.MFiles; import ucar.nc2.NetcdfFiles; import ucar.nc2.grib.GribIndex; import ucar.nc2.grib.GribIndexCache; @@ -208,7 +207,7 @@ public boolean makeIndex(String filename, RandomAccessFile dataRaf) throws IOExc String idxPathTmp = GribUtils.makeIndexFileName(idxPath, ".tmp"); MFile idxMFileTmp = GribIndexCache.getFileOrCache(idxPathTmp); - if (!(idxMFile instanceof MFileOS || idxMFile instanceof MFileOS7)) { + if (!MFiles.isLocal(idxMFile)) { throw new IllegalArgumentException("Only local file systems are supported for index creation"); } diff --git a/grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3.java b/grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3LocalIndex.java similarity index 98% rename from grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3.java rename to grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3LocalIndex.java index 3598d884b1..cf0e4fc497 100644 --- a/grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3.java +++ b/grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3LocalIndex.java @@ -18,7 +18,7 @@ import ucar.nc2.util.CompareNetcdf2; import ucar.unidata.util.test.TestDir; -public class TestSingleGribFileS3 { +public class TestSingleGribFileS3LocalIndex { private static final String GRIB1_BUCKET_KEY = "thredds-test-data?test-grib-without-index/radar_national.grib1"; private static final String GRIB2_BUCKET_KEY = "thredds-test-data?test-grib-without-index/cosmo-eu.grib2"; diff --git a/grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3RemoteIndex.java b/grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3RemoteIndex.java new file mode 100644 index 0000000000..c10499ffff --- /dev/null +++ b/grib/src/test/java/ucar/nc2/grib/TestSingleGribFileS3RemoteIndex.java @@ -0,0 +1,124 @@ +/* + * Copyright (c) 2026 University Corporation for Atmospheric Research/Unidata + * See LICENSE for license information. + */ + +package ucar.nc2.grib; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.util.Formatter; +import org.junit.Test; +import ucar.ma2.Array; +import ucar.nc2.Variable; +import ucar.nc2.dataset.NetcdfDataset; +import ucar.nc2.dataset.NetcdfDatasets; +import ucar.nc2.util.CompareNetcdf2; +import ucar.unidata.util.test.TestDir; + +/** + * Tests reading single grib files from S3 when index files already exists in the bucket + */ +public class TestSingleGribFileS3RemoteIndex { + + private static final String GRIB1_BUCKET_KEY = "thredds-test-data?test-grib-index/radar_national.grib1"; + private static final String GRIB2_BUCKET_KEY = "thredds-test-data?test-grib-index/cosmo-eu.grib2"; + private static final String GRIB1_LOCAL = TestDir.localTestDataDir + "radar_national.grib1"; + private static final String GRIB2_LOCAL = TestDir.localTestDataDir + "cosmo-eu.grib2"; + + @Test + public void testGrib1S3Full() throws IOException { + String location = String.format("cdms3://s3.us-east-1.amazonaws.com/%s#delimiter=/", GRIB1_BUCKET_KEY); + // .ncx4 object is older than .gbx9 object, so an update is required (which isn't supported) + assertThrows(IOException.class, () -> basicDatasetValidation(location)); + } + + @Test + public void testGrib1S3Short() throws IOException { + String location = String.format("cdms3:%s#delimiter=/", GRIB1_BUCKET_KEY); + // .ncx4 object is older than .gbx9 object, so an update is required (which isn't supported) + assertThrows(IOException.class, () -> basicDatasetValidation(location)); + } + + @Test + public void testGrib1SFullNoDelimiter() throws IOException { + String location = String.format("cdms3://s3.us-east-1.amazonaws.com/%s", GRIB1_BUCKET_KEY); + // .ncx4 object is older than .gbx9 object, so an update is required (which isn't supported) + assertThrows(IOException.class, () -> basicDatasetValidation(location)); + } + + @Test + public void testGrib1S3ShortNoDelimiter() throws IOException { + String location = String.format("cdms3:%s", GRIB1_BUCKET_KEY); + // .ncx4 object is older than .gbx9 object, so an update is required (which isn't supported) + assertThrows(IOException.class, () -> basicDatasetValidation(location)); + } + + @Test + public void testGrib2S3Full() throws IOException { + // .ncx4 is missing, so will be written locally and used successfully + String location = String.format("cdms3://s3.us-east-1.amazonaws.com/%s#delimiter=/", GRIB2_BUCKET_KEY); + basicDatasetValidation(location); + } + + @Test + public void testGrib2S3Short() throws IOException { + // .ncx4 is missing, so will be written locally and used successfully + String location = String.format("cdms3:%s#delimiter=/", GRIB2_BUCKET_KEY); + basicDatasetValidation(location); + } + + @Test + public void testGrib2SFullNoDelimiter() throws IOException { + // .ncx4 is missing, so will be written locally and used successfully + String location = String.format("cdms3://s3.us-east-1.amazonaws.com/%s", GRIB2_BUCKET_KEY); + basicDatasetValidation(location); + } + + @Test + public void testGrib2S3ShortNoDelimiter() throws IOException { + // .ncx4 is missing, so will be written locally and used successfully + String location = String.format("cdms3:%s", GRIB2_BUCKET_KEY); + basicDatasetValidation(location); + } + + @Test + public void compareGrib1() throws IOException { + // .ncx4 object is older than .gbx9 object, so an update is required (which isn't supported) + String location = String.format("cdms3://s3.us-east-1.amazonaws.com/%s#delimiter=/", GRIB1_BUCKET_KEY); + assertThrows(IOException.class, () -> compareWithLocal(location, GRIB1_LOCAL)); + } + + @Test + public void compareGrib2() throws IOException { + // .ncx4 is missing, so will be written locally and used successfully + String location = String.format("cdms3://s3.us-east-1.amazonaws.com/%s#delimiter=/", GRIB2_BUCKET_KEY); + compareWithLocal(location, GRIB2_LOCAL); + } + + private static void compareWithLocal(String remoteLocation, String localLocation) throws IOException { + try (NetcdfDataset remoteNetcdfDataset = NetcdfDatasets.openDataset(remoteLocation); + NetcdfDataset localNetcdfDataset = NetcdfDatasets.openDataset(localLocation)) { + Formatter f = new Formatter(); + CompareNetcdf2 compare = new CompareNetcdf2(f, false, false, true); + if (!compare.compare(localNetcdfDataset, remoteNetcdfDataset, null)) { + System.out.printf("Compare %s%n%s%n", localLocation, f); + fail(); + } + } + } + + private static void basicDatasetValidation(String location) throws IOException { + try (NetcdfDataset netcdfDataset = NetcdfDatasets.openDataset(location)) { + assertThat(netcdfDataset.getLastModified()).isGreaterThan(0); + assertThat(netcdfDataset.getVariables()).isNotEmpty(); + + Variable variable = netcdfDataset.getVariables().get(1); + Array data = variable.read(); + assertThat(data).isNotNull(); + } + } +} diff --git a/uicdm/src/main/java/ucar/nc2/ui/MFileTable.java b/uicdm/src/main/java/ucar/nc2/ui/MFileTable.java index 54fc6a9a54..ce456d9300 100644 --- a/uicdm/src/main/java/ucar/nc2/ui/MFileTable.java +++ b/uicdm/src/main/java/ucar/nc2/ui/MFileTable.java @@ -1,13 +1,12 @@ /* - * Copyright (c) 1998-2018 University Corporation for Atmospheric Research/Unidata + * Copyright (c) 1998-2026 University Corporation for Atmospheric Research/Unidata * See LICENSE for license information. */ package ucar.nc2.ui; -import thredds.filesystem.MFileOS; -import thredds.filesystem.MFileOS7; import thredds.inventory.MFile; +import thredds.inventory.MFiles; import ucar.nc2.time.CalendarDateFormatter; import ucar.ui.widget.BAMutil; import ucar.ui.widget.IndependentWindow; @@ -102,7 +101,7 @@ public void save() { } public void setFiles(MFile mdir, Collection files) { - if (!(mdir instanceof MFileOS || mdir instanceof MFileOS7)) { + if (!MFiles.isLocal(mdir)) { throw new IllegalArgumentException("Only local file systems are supported for index creation"); } setFiles(new File(mdir.getPath()), files);