Fix bug in partial CF Sub-convention name matches

Make sure CF Sub-conventions work even if the full conventions name
attribute is not a match. Add the partial match case to the existing
test.

Author: lesserwhirls

cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordSystemFactory.java

@@ -230,13 +230,12 @@ public static Optional<CoordSystemBuilder> factory(NetcdfDataset.Builder ds, Can
     }
 
     // if the match is on the main CF convention, we need to look at possible sub-conventions or incubating conventions
-    if (coordSysFactory != null && ds.orgFile != null && coordSysFactory.getClass().isInstance(CF1Convention.class)) {
+    if (coordSysFactory != null && ds.orgFile != null && coordSysFactory instanceof CF1Convention.Factory) {
       List<String> convs = breakupConventionNames(convName);
       if (convs.size() > 1) {
         CoordSystemBuilderFactory potentialCoordSysFactory = findCfSubConvention(ds.orgFile);
         // only use the potentialCoordSysFactory if it is a CF sub-convention
-        if (potentialCoordSysFactory != null
-            && potentialCoordSysFactory.getClass().isInstance(CFSubConventionProvider.class)) {
+        if (potentialCoordSysFactory != null && potentialCoordSysFactory instanceof CFSubConventionProvider) {
           coordSysFactory = potentialCoordSysFactory;
         }
       }
@@ -294,14 +293,14 @@ private static CoordSystemBuilderFactory findCfSubConvention(NetcdfFile orgFile)
     // Look for Convention using isMine()
     for (Convention conv : conventionList) {
       CoordSystemBuilderFactory candidate = conv.factory;
-      if (candidate.getClass().isInstance(CFSubConventionProvider.class) && candidate.isMine(orgFile)) {
+      if (candidate instanceof CFSubConventionProvider && candidate.isMine(orgFile)) {
         return candidate;
       }
     }
 
     // Use service loader mechanism isMine()
     for (CoordSystemBuilderFactory csb : ServiceLoader.load(CoordSystemBuilderFactory.class)) {
-      if (csb.getClass().isInstance(CFSubConventionProvider.class) && csb.isMine(orgFile)) {
+      if (csb instanceof CFSubConventionProvider && csb.isMine(orgFile)) {
         return csb;
       }
     }

cdm/core/src/test/java/ucar/nc2/dataset/TestCfSubConventionProvider.java

@@ -19,17 +19,21 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Optional;
 
 public class TestCfSubConventionProvider {
 
   private static final NetcdfDataset.Builder<?> CF_NCDB = NetcdfDataset.builder();
-  private static final NetcdfDataset.Builder<?> YOLO_NCDB = NetcdfDataset.builder();
+  private static final NetcdfDataset.Builder<?> YOLO_EXACT_NCDB = NetcdfDataset.builder();
+  private static final NetcdfDataset.Builder<?> YOLO_PARTIAL_NCDB = NetcdfDataset.builder();
 
   @BeforeClass
   public static void makeNcdBuilders() throws IOException {
     NetcdfFile.Builder<?> cfNcfb = NetcdfFile.builder();
-    NetcdfFile.Builder<?> yoloNcfb = NetcdfFile.builder();
+    NetcdfFile.Builder<?> yoloExactNcfb = NetcdfFile.builder();
+    NetcdfFile.Builder<?> yoloPartialNcfb = NetcdfFile.builder();
 
     Group.Builder cfRoot = Group.builder().setName("");
     cfNcfb.setRootGroup(cfRoot);
@@ -42,14 +46,30 @@ public static void makeNcdBuilders() throws IOException {
       throw new IOException("Error building NetcdfFile object to mock a CF netCDF file for testing.", ioe);
     }
 
-    Group.Builder yoloRoot = Group.builder().setName("");
-    yoloNcfb.setRootGroup(yoloRoot);
-    Attribute yoloConvAttr =
+    Group.Builder yoloExactRoot = Group.builder().setName("");
+    yoloExactNcfb.setRootGroup(yoloExactRoot);
+    // exactly match the convention name
+    Attribute yoloExactConvAttr =
         Attribute.builder(CDM.CONVENTIONS).setStringValue(CfSubConvForTest.CONVENTION_NAME).build();
-    yoloRoot.addAttribute(yoloConvAttr);
-    try (NetcdfFile yoloNcf = yoloNcfb.build()) {
-      YOLO_NCDB.copyFrom(yoloNcf);
-      YOLO_NCDB.setOrgFile(yoloNcf);
+    yoloExactRoot.addAttribute(yoloExactConvAttr);
+    try (NetcdfFile yoloExactNcf = yoloExactNcfb.build()) {
+      YOLO_EXACT_NCDB.copyFrom(yoloExactNcf);
+      YOLO_EXACT_NCDB.setOrgFile(yoloExactNcf);
+    } catch (IOException ioe) {
+      throw new IOException("Error building NetcdfFile object to mock a CF/YOLO netCDF file for testing.", ioe);
+    }
+
+    Group.Builder yoloPartialRoot = Group.builder().setName("");
+    yoloPartialNcfb.setRootGroup(yoloPartialRoot);
+    // change up the convention name used so that it is not an exact match with the test convention name
+    Attribute yoloPartialConvAttr = Attribute.builder(CDM.CONVENTIONS)
+        .setStringValue(
+            CfSubConvForTest.CONVENTION_NAME.replaceFirst(CfSubConvForTest.CONVENTAION_NAME_STARTS_WITH, "CF-1.200"))
+        .build();
+    yoloPartialRoot.addAttribute(yoloPartialConvAttr);
+    try (NetcdfFile yoloPartialNcf = yoloPartialNcfb.build()) {
+      YOLO_PARTIAL_NCDB.copyFrom(yoloPartialNcf);
+      YOLO_PARTIAL_NCDB.setOrgFile(yoloPartialNcf);
     } catch (IOException ioe) {
       throw new IOException("Error building NetcdfFile object to mock a CF/YOLO netCDF file for testing.", ioe);
     }
@@ -62,10 +82,15 @@ public void testCfSubLoadOrder() throws IOException {
     CoordSystemBuilder cfFac = cfFacOpt.get();
     assertThat(cfFac).isInstanceOf(CF1Convention.class);
 
-    Optional<CoordSystemBuilder> yoloFacOpt = CoordSystemFactory.factory(YOLO_NCDB, null);
-    assertThat(yoloFacOpt.isPresent());
-    CoordSystemBuilder yoloFac = yoloFacOpt.get();
-    assertThat(yoloFac).isInstanceOf(CfSubConvForTest.class);
-    assertThat(yoloFac.getConventionUsed()).isEqualTo(CfSubConvForTest.CONVENTION_NAME);
+    // both datasets (exact and partial matches of the sub-convention name) should result in the
+    // sub-convention being used.
+    List<NetcdfDataset.Builder<?>> subConvBuilders = Arrays.asList(YOLO_EXACT_NCDB, YOLO_PARTIAL_NCDB);
+    for (NetcdfDataset.Builder<?> subConvBuilder : subConvBuilders) {
+      Optional<CoordSystemBuilder> yoloFacOpt = CoordSystemFactory.factory(subConvBuilder, null);
+      assertThat(yoloFacOpt.isPresent());
+      CoordSystemBuilder yoloFac = yoloFacOpt.get();
+      assertThat(yoloFac).isInstanceOf(CfSubConvForTest.class);
+      assertThat(yoloFac.getConventionUsed()).isEqualTo(CfSubConvForTest.CONVENTION_NAME);
+    }
   }
 }

cdm/core/src/test/java/ucar/nc2/dataset/conv/CfSubConvForTest.java

@@ -12,13 +12,20 @@
 import ucar.nc2.internal.dataset.CoordSystemBuilder;
 import ucar.nc2.internal.dataset.spi.CFSubConventionProvider;
 
+import java.util.List;
+
+import static ucar.nc2.internal.dataset.CoordSystemFactory.breakupConventionNames;
+
 /**
  * An implementation of CoordSystemBuilder for testing CFSubConvention hooks.
  *
  * @see ucar.nc2.dataset.TestCfSubConventionProvider
  */
 public class CfSubConvForTest extends CoordSystemBuilder {
-  public static String CONVENTION_NAME = "CF-1.200/YOLO";
+  private static String SUBCONVENTAION_NAME = "YOLO";
+  // public for testing
+  public static String CONVENTAION_NAME_STARTS_WITH = "CF-1.X";
+  public static String CONVENTION_NAME = CONVENTAION_NAME_STARTS_WITH + "/" + SUBCONVENTAION_NAME;
 
   private CfSubConvForTest(NetcdfDataset.Builder<?> datasetBuilder) {
     super(datasetBuilder);
@@ -33,7 +40,13 @@ public boolean isMine(NetcdfFile ncfile) {
       if (conventionAttr != null) {
         String conventionValue = conventionAttr.getStringValue();
         if (conventionValue != null) {
-          mine = conventionValue.equals(CONVENTION_NAME);
+          List<String> names = breakupConventionNames(conventionValue);
+          for (String name : names) {
+            if (name.equalsIgnoreCase(SUBCONVENTAION_NAME)) {
+              mine = true;
+              break;
+            }
+          }
         }
       }
       return mine;