Use ncml in dataset scans opened through ncss

tds/src/main/java/thredds/core/DatasetManager.java

@@ -12,6 +12,7 @@
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.stereotype.Component;
+import thredds.core.DataRootManager.DataRootMatch;
 import thredds.featurecollection.FeatureCollectionCache;
 import thredds.featurecollection.InvDatasetFeatureCollection;
 import thredds.server.admin.DebugCommands;
@@ -174,7 +175,7 @@ public NetcdfFile openNetcdfFile(HttpServletRequest req, HttpServletResponse res
     }
 
     // look for a match
-    DataRootManager.DataRootMatch match = dataRootManager.findDataRootMatch(reqPath);
+    DataRootMatch match = dataRootManager.findDataRootMatch(reqPath);
 
     // look for a feature collection dataset
     if ((match != null) && (match.dataRoot.getFeatureCollection() != null)) {
@@ -200,49 +201,12 @@ public NetcdfFile openNetcdfFile(HttpServletRequest req, HttpServletResponse res
 
     // common case - its a file
     if (match != null) {
-      org.jdom2.Element netcdfElem = null; // find ncml if it exists
-      if (match.dataRoot != null) {
-        DatasetScan dscan = match.dataRoot.getDatasetScan();
-        // if (dscan == null) dscan = match.dataRoot.getDatasetRootProxy(); // no ncml possible in getDatasetRootProxy
-        if (dscan != null)
-          netcdfElem = dscan.getNcmlElement();
-      }
-
       String location = dataRootManager.getLocationFromRequestPath(reqPath);
       if (location == null)
         throw new FileNotFoundException(reqPath);
 
-      // if there's an ncml element, open it through NcMLReader, supplying the underlying file
-      // from NetcdfFiles.open(), therefore not being cached.
-      // This is safer given all the trouble we have with ncml and caching.
-      if (netcdfElem != null) {
-        String ncmlLocation = "DatasetScan#" + location; // LOOK some descriptive name
-        // open with openFile(), not acquireFile, so we skip the caches
-        NetcdfDataset ncd = null;
-
-        // look for addRecords attribute on the netcdf element. The new API in netCDF-Java does not handle this,
-        // so we will handle it special here.
-        Attribute addRecordsAttr = netcdfElem.getAttribute("addRecords");
-        boolean addRecords = false;
-        if (addRecordsAttr != null) {
-          addRecords = Boolean.valueOf(addRecordsAttr.getValue());
-        }
-
-        NetcdfFile ncf;
-        if (addRecords) {
-          DatasetUrl datasetUrl = DatasetUrl.findDatasetUrl(location);
-          // work around for presence of addRecords="true" on a netcdf element
-          ncf = NetcdfDatasets.openFile(datasetUrl, -1, null, NetcdfFile.IOSP_MESSAGE_ADD_RECORD_STRUCTURE);
-        } else {
-          ncf = NetcdfDatasets.openFile(location, null);
-        }
-
-        NetcdfDataset.Builder modifiedDsBuilder = NcmlReader.mergeNcml(ncf, netcdfElem);
-
-        // set new location to indicate this is a dataset from a dataset scan wrapped with NcML.
-        modifiedDsBuilder.setLocation(ncmlLocation);
-        ncd = modifiedDsBuilder.build();
-        return ncd;
+      if (hasDatasetScanNcml(match)) {
+        return openNcmlDatasetScan(location, match);
       }
 
       DatasetUrl durl = DatasetUrl.findDatasetUrl(location);
@@ -258,6 +222,44 @@ public NetcdfFile openNetcdfFile(HttpServletRequest req, HttpServletResponse res
     return ncfile;
   }
 
+  private boolean hasDatasetScanNcml(DataRootMatch match) {
+    return match != null && match.dataRoot != null && match.dataRoot.getDatasetScan() != null
+        && match.dataRoot.getDatasetScan().getNcmlElement() != null;
+  }
+
+  private NetcdfFile openNcmlDatasetScan(String location, DataRootMatch match) throws IOException {
+    org.jdom2.Element netcdfElem = match.dataRoot.getDatasetScan().getNcmlElement();
+    // if there's an ncml element, open it through NcMLReader, supplying the underlying file
+    // from NetcdfFiles.open(), therefore not being cached.
+    // This is safer given all the trouble we have with ncml and caching.
+
+    String ncmlLocation = "DatasetScan#" + location; // LOOK some descriptive name
+    // open with openFile(), not acquireFile, so we skip the caches
+
+    // look for addRecords attribute on the netcdf element. The new API in netCDF-Java does not handle this,
+    // so we will handle it special here.
+    Attribute addRecordsAttr = netcdfElem.getAttribute("addRecords");
+    boolean addRecords = false;
+    if (addRecordsAttr != null) {
+      addRecords = Boolean.valueOf(addRecordsAttr.getValue());
+    }
+
+    NetcdfFile ncf;
+    if (addRecords) {
+      DatasetUrl datasetUrl = DatasetUrl.findDatasetUrl(location);
+      // work around for presence of addRecords="true" on a netcdf element
+      ncf = NetcdfDatasets.openFile(datasetUrl, -1, null, NetcdfFile.IOSP_MESSAGE_ADD_RECORD_STRUCTURE);
+    } else {
+      ncf = NetcdfDatasets.openFile(location, null);
+    }
+
+    NetcdfDataset.Builder<?> modifiedDsBuilder = NcmlReader.mergeNcml(ncf, netcdfElem);
+
+    // set new location to indicate this is a dataset from a dataset scan wrapped with NcML.
+    modifiedDsBuilder.setLocation(ncmlLocation);
+    return modifiedDsBuilder.build();
+  }
+
   /**
    * Open a file as a GridDataset, using getNetcdfFile(), so that it gets wrapped in NcML if needed.
    */
@@ -420,8 +422,14 @@ public CoverageCollection openCoverageDataset(HttpServletRequest req, HttpServle
     }
 
     // otherwise, assume it's a local file with a datasetRoot in the urlPath.
-    // try to open as a FeatureDatasetCoverage. This allows GRIB to be handled specially
     String location = getLocationFromRequestPath(reqPath);
+
+    // Ncml in datasetScan
+    if (location != null && hasDatasetScanNcml(match)) {
+      return openCoverageFromDatasetScanNcml(location, match, reqPath);
+    }
+
+    // try to open as a FeatureDatasetCoverage. This allows GRIB to be handled specially
     if (location != null) {
       Optional<FeatureDatasetCoverage> opt = CoverageDatasetFactory.openCoverageDataset(location);
       // hack - CoverageDatasetFactory bombs out on an object store location string during the grib check,
@@ -431,12 +439,8 @@ public CoverageCollection openCoverageDataset(HttpServletRequest req, HttpServle
         // and pass that to CoverageDataset
         DtCoverageDataset gds = new DtCoverageDataset(NetcdfDatasets.openDataset(location));
         if (!gds.getGrids().isEmpty()) {
-          Formatter errlog = new Formatter();
-          FeatureDatasetCoverage result = DtCoverageAdapter.factory(gds, errlog);
-          if (result != null)
-            opt = Optional.of(result);
-          else
-            opt = Optional.empty(errlog.toString());
+          FeatureDatasetCoverage result = DtCoverageAdapter.factory(gds, new Formatter());
+          opt = Optional.of(result);
         }
       }
 
@@ -451,6 +455,20 @@ public CoverageCollection openCoverageDataset(HttpServletRequest req, HttpServle
     return null;
   }
 
+  private CoverageCollection openCoverageFromDatasetScanNcml(String location, DataRootMatch match, String reqPath)
+      throws IOException {
+    final NetcdfFile ncf = openNcmlDatasetScan(location, match);
+    final NetcdfDataset ncd = NetcdfDatasets.enhance(ncf, NetcdfDataset.getDefaultEnhanceMode(), null);
+    final DtCoverageDataset gds = new DtCoverageDataset(ncd);
+
+    if (gds.getGrids().isEmpty()) {
+      throw new FileNotFoundException("Error opening grid dataset " + reqPath + ". err= no grids found.");
+    }
+
+    final FeatureDatasetCoverage coverage = DtCoverageAdapter.factory(gds, new Formatter());
+    return coverage.getSingleCoverageCollection();
+  }
+
   public SimpleGeometryFeatureDataset openSimpleGeometryDataset(HttpServletRequest req, HttpServletResponse res,
       String reqPath) throws IOException {
     // first look for a feature collection

tds/src/test/content/thredds/catalog.xml

@@ -173,9 +173,8 @@
 
   <dataset name="Ncml Tests">
 
-    <!-- UNTESTED test NcML in dataset LOOK doesnt work -->
-    <dataset name="Example NcML Modified - DODS" ID="ExampleNcMLModified" urlPath="ExampleNcML/Modified.nc">
-      <serviceName>odap</serviceName>
+    <dataset name="Example NcML Modified" ID="ExampleNcMLModified" urlPath="ExampleNcML/Modified.nc">
+      <serviceName>all</serviceName>
       <dataType>Grid</dataType>
       <ncml:netcdf xmlns="http://www.unidata.ucar.edu/namespaces/netcdf/ncml-2.2" location="${cdmUnitTest}/tds/example1.nc" addRecords="true">
         <attribute name="name" value="value"/>
@@ -185,6 +184,9 @@
           <attribute name="units" value="percent (%)"/>
           <remove type="attribute" name="description"/>
         </variable>
+        <variable name="time">
+          <attribute name="units" value="hours since 2000-01-01 00:00:00"/>
+        </variable>
       </ncml:netcdf>
     </dataset>
 
@@ -246,6 +248,9 @@
           <attribute name="units" value="percent (%)"/>
           <remove type="attribute" name="description"/>
         </variable>
+        <variable name="time">
+          <attribute name="units" value="hours since 2000-01-01 00:00:00"/>
+        </variable>
       </ncml:netcdf>
 
       <filter>

tds/src/test/java/thredds/server/ncss/controller/grid/NcmlTest.java

@@ -0,0 +1,111 @@
+package thredds.server.ncss.controller.grid;
+
+import java.util.List;
+import java.util.Optional;
+import org.jdom2.Document;
+import org.jdom2.Element;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.WebAppConfiguration;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.RequestBuilder;
+import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
+import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
+import org.springframework.test.web.servlet.setup.MockMvcBuilders;
+import org.springframework.web.context.WebApplicationContext;
+import thredds.mock.web.MockTdsContextLoader;
+import thredds.util.xml.XmlUtil;
+import ucar.nc2.Attribute;
+import ucar.nc2.NetcdfFile;
+import ucar.nc2.NetcdfFiles;
+import ucar.nc2.Variable;
+import java.io.IOException;
+import ucar.unidata.util.test.category.NeedsCdmUnitTest;
+
+import static com.google.common.truth.Truth.assertThat;
+
+@RunWith(SpringJUnit4ClassRunner.class)
+@Category(NeedsCdmUnitTest.class)
+@WebAppConfiguration
+@ContextConfiguration(locations = {"/WEB-INF/applicationContext.xml", "/WEB-INF/spring-servlet.xml"},
+    loader = MockTdsContextLoader.class)
+public class NcmlTest {
+  private static final String NCML_DATASET = "/ncss/grid/ExampleNcML/Modified.nc";
+  private static final String NCML_DATASET_SCAN = "/ncss/grid/ModifyDatasetScan/example1.nc";
+
+  @Autowired
+  private WebApplicationContext wac;
+
+  private MockMvc mockMvc;
+
+  @Before
+  public void setUp() throws IOException {
+    mockMvc = MockMvcBuilders.webAppContextSetup(this.wac).build();
+  }
+
+  @Test
+  public void shouldShowNcmlModifiedVariableOnDatasetPage() throws Exception {
+    assertShowsNcmlModifiedVariableOnDatasetPage(NCML_DATASET + "/dataset.xml");
+  }
+
+  @Test
+  public void shouldShowNcmlModifiedVariableOnDatasetPageForDatasetScan() throws Exception {
+    assertShowsNcmlModifiedVariableOnDatasetPage(NCML_DATASET_SCAN + "/dataset.xml");
+  }
+
+  @Test
+  public void shouldReturnNcmlModifiedVariable() throws Exception {
+    assertReturnsNcmlModifiedVariable(NCML_DATASET);
+  }
+
+  @Test
+  public void shouldReturnNcmlModifiedVariableForDatasetScan() throws Exception {
+    assertReturnsNcmlModifiedVariable(NCML_DATASET_SCAN);
+  }
+
+  private void assertShowsNcmlModifiedVariableOnDatasetPage(String servletPath) throws Exception {
+    final RequestBuilder requestBuilder = MockMvcRequestBuilders.get(servletPath).servletPath(servletPath);
+    final MvcResult mvcResult =
+        mockMvc.perform(requestBuilder).andExpect(MockMvcResultMatchers.status().isOk()).andReturn();
+
+    final Document doc = XmlUtil.getStringResponseAsDoc(mvcResult.getResponse());
+    final List<Element> grids = XmlUtil.evaluateXPath(doc, "//grid");
+    assertThat(grids).isNotNull();
+
+    final Optional<Element> origVar =
+        grids.stream().filter(e -> e.getAttribute("name").getValue().equals("rh")).findFirst();
+    assertThat(origVar.isPresent()).isFalse();
+
+    final Optional<Element> modifiedVar =
+        grids.stream().filter(e -> e.getAttribute("name").getValue().equals("ReletiveHumidity")).findFirst();
+    assertThat(modifiedVar.isPresent()).isTrue();
+    assertThat(modifiedVar.get().getAttribute("name").getValue()).isEqualTo("ReletiveHumidity");
+    assertThat(modifiedVar.get().getAttribute("desc").getValue()).isEqualTo("relatively humid");
+  }
+
+  private void assertReturnsNcmlModifiedVariable(String servletPath) throws Exception {
+    final RequestBuilder requestBuilder =
+        MockMvcRequestBuilders.get(servletPath).servletPath(servletPath).param("var", "all");
+    final MvcResult mvcResult =
+        mockMvc.perform(requestBuilder).andExpect(MockMvcResultMatchers.status().isOk()).andReturn();
+
+    try (NetcdfFile ncf = NetcdfFiles.openInMemory("ncmlTest.nc", mvcResult.getResponse().getContentAsByteArray())) {
+      final Variable origVar = ncf.findVariable("rh");
+      assertThat((Object) origVar).isNull();
+
+      final Variable modifiedVar = ncf.findVariable("ReletiveHumidity");
+      assertThat((Object) modifiedVar).isNotNull();
+
+      final Attribute att = modifiedVar.findAttribute("long_name");
+      assertThat(att).isNotNull();
+      assertThat(att.getStringValue()).isEqualTo("relatively humid");
+    }
+  }
+}
+