featureCollection featureType Station file handles

gradle/any/dependencies.gradle

@@ -180,7 +180,7 @@ libraries["objenesis"] = "org.objenesis:objenesis:3.0.1"
 libraries["mockito"] = "org.mockito:mockito-core:3.3.3"
 
 // Fluent assertions for Java
-libraries["truth"] = "com.google.truth:truth:1.0.1"
+libraries["truth"] = "com.google.truth:truth:1.0"
 
 
 ////////////////////////////////////////// Other //////////////////////////////////////////

tds/build.gradle

@@ -105,6 +105,7 @@ dependencies {
     testCompile libraries["hamcrest-core"]
     testCompile libraries["commons-io"]
     testCompile libraries["JUnitParams"]
+    testCompile libraries["truth"]
 
     // Logging
     compile libraries["slf4j-api"]

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

@@ -79,6 +79,14 @@
 
     <featureCollection name="Surface Synoptic Point Data" harvest="true" featureType="Point" path="testSurfaceSynopticFeatureCollection">
       <collection spec="${cdmUnitTest}/ft/point/ldm/synop/Surface_Synoptic_#yyyyMMdd_HHmm#.nc$"/>
+      <!-- update startup="true"/ -->
+      <update startup="true" rescan="0 0/15 * * * ? *" trigger="allow"/>
+      <protoDataset choice="Penultimate"/>
+      <pointConfig datasetTypes="cdmrFeature Files"/>
+    </featureCollection>
+
+    <featureCollection name="Surface Point Data - GEMKAP" harvest="true" featureType="Station" path="testSurfaceGempakFeatureCollection">
+      <collection spec="${cdmUnitTest}/formats/gempak/surface/#yyyyMMdd#_sao.gem$"/>
       <update startup="true"/>
       <protoDataset choice="Penultimate"/>
       <pointConfig datasetTypes="cdmrFeature Files"/>
@@ -107,7 +115,35 @@
       <!--netcdf xmlns="http://www.unidata.ucar.edu/namespaces/netcdf/ncml-2.2">
           <attribute name="Conventions" value="CF-1.6"/>
       </netcdf-->
+
   </featureCollection>
 
+  <featureCollection name="2019 Archived Metar Station Data" harvest="true" featureType="Station" path="metarArchive2019/ncdecoded">
+    <metadata inherited="true">
+      <dataType>Station</dataType>
+    </metadata>
+
+    <property name="raw" value="report"/>
+    <property name="resolution" value="20 min"/>
+
+    <collection spec="${cdmUnitTest}/ft/station/gempak/collection_with_missing_station_features/#yyMMdd#.sf$" />
+    <update startup="true" rescan="0 0/15 * * * ? *" trigger="allow"/>
+    <protoDataset choice="Latest" />
+    <pointConfig datasetTypes="cdmrFeature Files"/>
+  </featureCollection>
+
+  <featureCollection name="2019 Archived Metar Point Data" harvest="true" featureType="Point" path="metarArchive2019Point/ncdecoded">
+    <metadata inherited="true">
+      <dataType>Point</dataType>
+    </metadata>
+
+    <property name="raw" value="report"/>
+    <property name="resolution" value="20 min"/>
+
+    <collection spec="${cdmUnitTest}/ft/station/gempak/collection_with_missing_station_features/#yyMMdd#.sf$" />
+    <update startup="true" rescan="0 0/15 * * * ? *" trigger="allow"/>
+    <protoDataset choice="Penultimate" />
+    <pointConfig datasetTypes="cdmrFeature Files"/>
+  </featureCollection>
 
 </catalog>

tds/src/test/java/thredds/server/ncss/controller/point/TestStationFcOpenFiles.java

@@ -0,0 +1,156 @@
+package thredds.server.ncss.controller.point;
+
+import static com.google.common.truth.Truth.assertThat;
+import java.lang.invoke.MethodHandles;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.mock.web.MockHttpServletResponse;
+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.ResultActions;
+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 ucar.nc2.util.cache.FileCache;
+import ucar.unidata.io.RandomAccessFile;
+import ucar.unidata.util.test.category.NeedsCdmUnitTest;
+
+@RunWith(SpringJUnit4ClassRunner.class)
+@WebAppConfiguration
+@ContextConfiguration(locations = {"/WEB-INF/applicationContext.xml"}, loader = MockTdsContextLoader.class)
+@Category(NeedsCdmUnitTest.class)
+public class TestStationFcOpenFiles {
+  private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @Autowired
+  private WebApplicationContext wac;
+
+  private MockMvc mockMvc;
+  private FileCache rafCache;
+
+  @Before
+  public void setup() {
+    mockMvc = MockMvcBuilders.webAppContextSetup(wac).build();
+    if (rafCache == null) {
+      rafCache = (FileCache) RandomAccessFile.getGlobalFileCache();
+    }
+  }
+
+  @Test
+  public void checkFeatureTypePoint() throws Exception {
+    // This collection uses FeatureType=Station
+    String dataset = "/ncss/point/testSurfaceGempakFeatureCollection/Surface_Point_Data_-_GEMKAP_fc.cdmr";
+    String partialReq = "?var=TMPC&west=-71&east=-70&south=42.5&north=42.6&accept=csv";
+
+    // Will cause TDS to look in one file for data
+    String req = String.format(partialReq + "&time=%s", "2009-05-22T13:00:00");
+    testReq(dataset, req);
+
+    // Will cause TDS to look in multiple multiple files for data, including the
+    // the file needed by the previous request
+    req = String.format(partialReq + "&time=%s", "2009-05-22T00:00:00");
+    testReq(dataset, req);
+  }
+
+  @Test
+  public void checkFeatureTypeStation() throws Exception {
+    // This collection uses FeatureType=Station
+    String dataset = "/ncss/point/testStationFeatureCollection/Metar_Station_Data_fc.cdmr";
+    // String partialReq = "?var=air_temperature&west=-71&east=-70&south=42.5&north=42.6&accept=csv";
+    String partialReq = "?var=air_temperature&west=-88&east=-62&south=34&north=50&accept=csv";
+
+    // Will cause TDS to look in one file for data
+    String req = String.format(partialReq + "&time=%s", "2006-03-26T13:00:00");
+    testReq(dataset, req);
+
+    // Will cause TDS to look in multiple multiple files for data, including the
+    // the file needed by the previous request
+    req = String.format(partialReq + "&time=%s", "2006-03-26T00:00:00");
+    testReq(dataset, req);
+  }
+
+  @Test
+  public void checkFeatureTypePointWithMissing() throws Exception {
+    // This collection uses FeatureType=Point
+    String dataset = "/ncss/point/metarArchive2019Point/ncdecoded/2019_Archived_Metar_Point_Data_fc.cdmr";
+    String partialReq =
+        "?var=CTYM&var=WNUM&var=CTYH&var=CHC2&var=CHC3&var=CHC1&var=DWPC&var=ALTI&var=PMSL&var=VSBY&var=TMPC&var=CTYL&var=DRCT&var=SKNT&west=-88&east=-62&south=34&north=50&accept=csv";
+
+    // Will cause TDS to look in one file for data
+    String req = String.format(partialReq + "&time=%s", "2019-04-03T00:30:00");
+    testReq(dataset, req);
+
+    // Will cause TDS to look in multiple multiple files for data, including the
+    // the file needed by the previous request
+    req = String.format(partialReq + "&time=%s", "2019-04-03T00:00:00");
+    testReq(dataset, req);
+  }
+
+  @Test
+  public void checkFeatureTypeStationWithMissing() throws Exception {
+    // This collection uses FeatureType=Station, but has a combination of data and
+    // a request that results in a StationTimeSeriesFeatureCollection not containing
+    // a given station for a given file, which exposed a bug.
+    String dataset = "/ncss/point/metarArchive2019/ncdecoded/2019_Archived_Metar_Station_Data_fc.cdmr";
+    String partialReq =
+        "?var=CTYM&var=WNUM&var=CTYH&var=CHC2&var=CHC3&var=CHC1&var=DWPC&var=ALTI&var=PMSL&var=VSBY&var=TMPC&var=CTYL&var=DRCT&var=SKNT&west=-88&east=-62&south=34&north=50&accept=csv";
+
+    // Will cause TDS to look in one file for data
+    String req = String.format(partialReq + "&time=%s", "2019-04-03T00:30:00");
+    testReq(dataset, req);
+
+    // Will cause TDS to look in multiple multiple files for data, including the
+    // the file needed by the previous request
+    // This triggered multiple enteries of the same file in the cache, as we
+    // we not releasing/closing resource in the CompositeStationFeatureIterator
+    req = String.format(partialReq + "&time=%s", "2019-04-03T00:00:00");
+    testReq(dataset, req);
+  }
+
+  private void testReq(String dataset, String req) throws Exception {
+    List<String> cacheEntries = rafCache.showCache();
+
+    RequestBuilder rb = MockMvcRequestBuilders.get(dataset + req).servletPath(dataset);
+    System.out.printf("%nURL='%s'%n", dataset + req);
+    ResultActions mockMvc = this.mockMvc.perform(rb);
+    MvcResult result = mockMvc.andExpect(MockMvcResultMatchers.status().isOk()).andReturn();
+    MockHttpServletResponse response = result.getResponse();
+    // if any file in the cache is locked, the string representation of that cache entry
+    // will start with true. We do not want any entry to be locked, so make sure this
+    // none of these entries start with "true"
+    cacheEntries = rafCache.showCache();
+    int numberOfCacheEnteries = cacheEntries.size();
+    boolean isAnyFileLocked = cacheEntries.stream().anyMatch(entry -> entry.startsWith("true"));
+    assertThat(isAnyFileLocked).isFalse();
+
+    // make another request for the same data
+    // the cache should not change in size. If so, we're not releasing
+    // resources properly.
+    mockMvc = this.mockMvc.perform(rb);
+    MvcResult result2 = mockMvc.andExpect(MockMvcResultMatchers.status().isOk()).andReturn();
+    MockHttpServletResponse response2 = result2.getResponse();
+
+    cacheEntries = rafCache.showCache();
+    // make sure we didn't end up with more or less files in the cache after a new request
+    assertThat(numberOfCacheEnteries).isEqualTo(cacheEntries.size());
+    // make sure none of the entries are locked
+    isAnyFileLocked = cacheEntries.stream().anyMatch(entry -> entry.startsWith("true"));
+    assertThat(isAnyFileLocked).isFalse();
+
+    // Not directly related to the resource issue, but while we're here, let's
+    // make sure the first and second requests, which are identical, return the same data.
+    assertThat(response.getContentAsByteArray()).isEqualTo(response2.getContentAsByteArray());
+  }
+}