diff --git a/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordSystemFactory.java b/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordSystemFactory.java index e514e803b5..8320f317cb 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordSystemFactory.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/dataset/CoordSystemFactory.java @@ -230,13 +230,12 @@ public static Optional 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 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; } } diff --git a/cdm/core/src/test/java/ucar/nc2/dataset/TestCfSubConventionProvider.java b/cdm/core/src/test/java/ucar/nc2/dataset/TestCfSubConventionProvider.java index bcfe1f320d..3736c6847e 100644 --- a/cdm/core/src/test/java/ucar/nc2/dataset/TestCfSubConventionProvider.java +++ b/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 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> subConvBuilders = Arrays.asList(YOLO_EXACT_NCDB, YOLO_PARTIAL_NCDB); + for (NetcdfDataset.Builder subConvBuilder : subConvBuilders) { + Optional yoloFacOpt = CoordSystemFactory.factory(subConvBuilder, null); + assertThat(yoloFacOpt.isPresent()); + CoordSystemBuilder yoloFac = yoloFacOpt.get(); + assertThat(yoloFac).isInstanceOf(CfSubConvForTest.class); + assertThat(yoloFac.getConventionUsed()).isEqualTo(CfSubConvForTest.CONVENTION_NAME); + } } } diff --git a/cdm/core/src/test/java/ucar/nc2/dataset/conv/CfSubConvForTest.java b/cdm/core/src/test/java/ucar/nc2/dataset/conv/CfSubConvForTest.java index f4311e9a55..e87ac67c06 100644 --- a/cdm/core/src/test/java/ucar/nc2/dataset/conv/CfSubConvForTest.java +++ b/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 names = breakupConventionNames(conventionValue); + for (String name : names) { + if (name.equalsIgnoreCase(SUBCONVENTAION_NAME)) { + mine = true; + break; + } + } } } return mine;