From 5931d4834e4d4e1fde4d5a4f5ef2c3386d060497 Mon Sep 17 00:00:00 2001 From: Michael Edgar Date: Tue, 13 Feb 2024 20:07:35 -0500 Subject: [PATCH] Remove reflection from YAML parser detection (#129) Signed-off-by: Michael Edgar --- .../io/xlate/yamljson/SettingsBuilder.java | 54 ++++------------ src/main/java/io/xlate/yamljson/Yaml.java | 34 +--------- .../xlate/yamljson/YamlGeneratorFactory.java | 54 ++++++++++++++-- .../io/xlate/yamljson/YamlParserFactory.java | 64 ++++++++++++++----- .../java/io/xlate/yamljson/YamlProvider.java | 17 +---- .../io/xlate/yamljson/YamlProviderTest.java | 17 ++--- .../io/xlate/yamljson/YamlTestHelper.java | 24 ++++++- 7 files changed, 143 insertions(+), 121 deletions(-) diff --git a/src/main/java/io/xlate/yamljson/SettingsBuilder.java b/src/main/java/io/xlate/yamljson/SettingsBuilder.java index 7af8b59..c943666 100644 --- a/src/main/java/io/xlate/yamljson/SettingsBuilder.java +++ b/src/main/java/io/xlate/yamljson/SettingsBuilder.java @@ -2,69 +2,43 @@ import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.function.Function; -import java.util.function.Supplier; - -import org.snakeyaml.engine.v2.api.DumpSettings; -import org.snakeyaml.engine.v2.api.DumpSettingsBuilder; -import org.snakeyaml.engine.v2.api.LoadSettings; -import org.snakeyaml.engine.v2.api.LoadSettingsBuilder; -import org.yaml.snakeyaml.DumperOptions; -import org.yaml.snakeyaml.LoaderOptions; interface SettingsBuilder { static final String MOD_SNAKEYAML = "org.yaml.snakeyaml"; - static final String MARKER_SNAKEYAML = "org.yaml.snakeyaml.Yaml"; - static final String MOD_SNAKEYAML_ENGINE = "org.snakeyaml.engine"; - static final String MARKER_SNAKEYAML_ENGINE = "org.snakeyaml.engine.v2.api.lowlevel.Parse"; static final String MISSING_MODULE_MESSAGE = "Required module not found: %s. " + "Ensure module is present on module path. Add to application module-info or " + "include with --add-modules command line option."; - default T loadProvider(Supplier providerSupplier, String providerModule) { + static Optional loadProvider(Map properties, Function, T> providerFactory) { try { - return providerSupplier.get(); + return Optional.of(providerFactory.apply(properties)); } catch (Exception | NoClassDefFoundError e) { - throw new IllegalStateException(String.format(MISSING_MODULE_MESSAGE, providerModule), e); + return Optional.empty(); } } - default LoaderOptions buildLoaderOptions(Map properties) { - LoaderOptions options = new LoaderOptions(); - replace(properties, Yaml.Settings.LOAD_MAX_ALIAS_EXPANSION_SIZE, Long::valueOf, Long.MAX_VALUE); - // No load properties supported currently - return options; - } - - default DumperOptions buildDumperOptions(Map properties) { - DumperOptions settings = new DumperOptions(); - settings.setExplicitStart(getProperty(properties, Yaml.Settings.DUMP_EXPLICIT_START, Boolean::valueOf, false)); - settings.setExplicitEnd(getProperty(properties, Yaml.Settings.DUMP_EXPLICIT_END, Boolean::valueOf, false)); - return settings; - } - - default LoadSettings buildLoadSettings(Map properties) { - LoadSettingsBuilder settings = LoadSettings.builder(); - settings.setUseMarks(getProperty(properties, Yaml.Settings.LOAD_USE_MARKS, Boolean::valueOf, true)); - replace(properties, Yaml.Settings.LOAD_MAX_ALIAS_EXPANSION_SIZE, Long::valueOf, Long.MAX_VALUE); - return settings.build(); + static T loadProvider(Map properties, Function, T> providerFactory, String providerModule) { + try { + return providerFactory.apply(properties); + } catch (Exception | NoClassDefFoundError e) { + throw new IllegalStateException(String.format(MISSING_MODULE_MESSAGE, providerModule), e); + } } - default DumpSettings buildDumpSettings(Map properties) { - DumpSettingsBuilder settings = DumpSettings.builder(); - settings.setExplicitStart(getProperty(properties, Yaml.Settings.DUMP_EXPLICIT_START, Boolean::valueOf, false)); - settings.setExplicitEnd(getProperty(properties, Yaml.Settings.DUMP_EXPLICIT_END, Boolean::valueOf, false)); - return settings.build(); + static IllegalStateException noProvidersFound() { + return new IllegalStateException("No YAML providers found on class/module path!"); } - default T getProperty(Map properties, String key, Function parser, T defaultValue) { + static T getProperty(Map properties, String key, Function parser, T defaultValue) { return parser.apply(String.valueOf(Objects.requireNonNullElse(properties.get(key), defaultValue))); } - default void replace(Map properties, String key, Function parser, T defaultValue) { + static void replace(Map properties, String key, Function parser, T defaultValue) { properties.put(key, getProperty(properties, key, parser, defaultValue)); } } diff --git a/src/main/java/io/xlate/yamljson/Yaml.java b/src/main/java/io/xlate/yamljson/Yaml.java index 147e3c9..dd52d69 100644 --- a/src/main/java/io/xlate/yamljson/Yaml.java +++ b/src/main/java/io/xlate/yamljson/Yaml.java @@ -21,10 +21,7 @@ import java.io.OutputStream; import java.io.Reader; import java.io.Writer; -import java.util.Collections; -import java.util.LinkedHashSet; import java.util.Map; -import java.util.Set; import jakarta.json.JsonArray; import jakarta.json.JsonException; @@ -33,7 +30,6 @@ import jakarta.json.JsonReaderFactory; import jakarta.json.JsonWriter; import jakarta.json.JsonWriterFactory; -import jakarta.json.spi.JsonProvider; import jakarta.json.stream.JsonGenerator; import jakarta.json.stream.JsonGeneratorFactory; import jakarta.json.stream.JsonParser; @@ -77,35 +73,9 @@ public final class Yaml { public static final class Versions { public static final String V1_1 = "v1.1"; public static final String V1_2 = "v1.2"; - static final Set VERSIONS_PRESENT; - - private interface ClassSupplier { - Class get() throws LinkageError, ClassNotFoundException; - } - - static { - Set versions = new LinkedHashSet<>(2); - addIfPresent(versions, V1_1, () -> Class.forName(SettingsBuilder.MARKER_SNAKEYAML)); - addIfPresent(versions, V1_2, () -> Class.forName(SettingsBuilder.MARKER_SNAKEYAML_ENGINE)); - VERSIONS_PRESENT = Collections.unmodifiableSet(versions); - } private Versions() { } - - private static void addIfPresent(Set versions, String version, ClassSupplier loader) { - try { - loader.get(); - versions.add(version); - } catch (ClassNotFoundException | LinkageError e) { - // Ignored - } - } - - static Set supportedVersions() { - return VERSIONS_PRESENT; - } - } /** @@ -148,12 +118,12 @@ private Settings() { public static final String DUMP_EXPLICIT_END = "DUMP_EXPLICIT_END"; } - private static final JsonProvider PROVIDER = new YamlProvider(); + private static final YamlProvider PROVIDER = new YamlProvider(); private Yaml() { } - private static JsonProvider provider() { + private static YamlProvider provider() { return PROVIDER; } diff --git a/src/main/java/io/xlate/yamljson/YamlGeneratorFactory.java b/src/main/java/io/xlate/yamljson/YamlGeneratorFactory.java index ae16810..cded889 100644 --- a/src/main/java/io/xlate/yamljson/YamlGeneratorFactory.java +++ b/src/main/java/io/xlate/yamljson/YamlGeneratorFactory.java @@ -15,6 +15,9 @@ */ package io.xlate.yamljson; +import static io.xlate.yamljson.SettingsBuilder.getProperty; +import static io.xlate.yamljson.SettingsBuilder.loadProvider; + import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.Writer; @@ -24,12 +27,45 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Function; import jakarta.json.stream.JsonGenerator; import jakarta.json.stream.JsonGeneratorFactory; +import org.snakeyaml.engine.v2.api.DumpSettings; +import org.snakeyaml.engine.v2.api.DumpSettingsBuilder; +import org.yaml.snakeyaml.DumperOptions; + class YamlGeneratorFactory implements JsonGeneratorFactory, SettingsBuilder { + private static final String SNAKEYAML_ENGINE_PROVIDER = "org.snakeyaml.engine.v2.api.DumpSettings"; + + static final Function, Object> SNAKEYAML_FACTORY = + props -> buildDumperOptions(props); + + static final Function, Object> SNAKEYAML_ENGINE_FACTORY = + props -> buildDumpSettings(props); + + static void copyBoolean(Map properties, String name, Consumer target) { + target.accept(getProperty(properties, name, Boolean::valueOf, false)); + } + + static DumperOptions buildDumperOptions(Map properties) { + DumperOptions options = new DumperOptions(); + copyBoolean(properties, Yaml.Settings.DUMP_EXPLICIT_START, options::setExplicitStart); + copyBoolean(properties, Yaml.Settings.DUMP_EXPLICIT_END, options::setExplicitEnd); + return options; + } + + static DumpSettings buildDumpSettings(Map properties) { + DumpSettingsBuilder settings = DumpSettings.builder(); + copyBoolean(properties, Yaml.Settings.DUMP_EXPLICIT_START, settings::setExplicitStart); + copyBoolean(properties, Yaml.Settings.DUMP_EXPLICIT_END, settings::setExplicitEnd); + return settings.build(); + } + private final Map properties; private final boolean useSnakeYamlEngine; private final Object snakeYamlSettings; @@ -38,12 +74,22 @@ class YamlGeneratorFactory implements JsonGeneratorFactory, SettingsBuilder { this.properties = new HashMap<>(properties); Object version = properties.get(Yaml.Settings.YAML_VERSION); - useSnakeYamlEngine = Yaml.Versions.V1_2.equals(version); - if (useSnakeYamlEngine) { - snakeYamlSettings = loadProvider(() -> buildDumpSettings(this.properties), MOD_SNAKEYAML_ENGINE); + if (version == null) { + snakeYamlSettings = Optional.empty() + .or(() -> loadProvider(this.properties, SNAKEYAML_FACTORY)) + .or(() -> loadProvider(this.properties, SNAKEYAML_ENGINE_FACTORY)) + .orElseThrow(SettingsBuilder::noProvidersFound); + + useSnakeYamlEngine = SNAKEYAML_ENGINE_PROVIDER.equals(snakeYamlSettings.getClass().getName()); } else { - snakeYamlSettings = loadProvider(() -> buildDumperOptions(this.properties), MOD_SNAKEYAML); + useSnakeYamlEngine = Yaml.Versions.V1_2.equals(version); + + if (useSnakeYamlEngine) { + snakeYamlSettings = loadProvider(this.properties, SNAKEYAML_ENGINE_FACTORY, MOD_SNAKEYAML_ENGINE); + } else { + snakeYamlSettings = loadProvider(this.properties, SNAKEYAML_FACTORY, MOD_SNAKEYAML); + } } } diff --git a/src/main/java/io/xlate/yamljson/YamlParserFactory.java b/src/main/java/io/xlate/yamljson/YamlParserFactory.java index cd7d799..ab60eb0 100644 --- a/src/main/java/io/xlate/yamljson/YamlParserFactory.java +++ b/src/main/java/io/xlate/yamljson/YamlParserFactory.java @@ -15,6 +15,10 @@ */ package io.xlate.yamljson; +import static io.xlate.yamljson.SettingsBuilder.getProperty; +import static io.xlate.yamljson.SettingsBuilder.loadProvider; +import static io.xlate.yamljson.SettingsBuilder.replace; + import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; @@ -23,45 +27,71 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.function.Function; import jakarta.json.JsonArray; import jakarta.json.JsonObject; import jakarta.json.stream.JsonParser; import jakarta.json.stream.JsonParserFactory; +import org.snakeyaml.engine.v2.api.LoadSettings; +import org.yaml.snakeyaml.LoaderOptions; + class YamlParserFactory implements JsonParserFactory, SettingsBuilder { + private static final String SNAKEYAML_ENGINE_PROVIDER = "org.snakeyaml.engine.v2.api.lowlevel.Parse"; + + static final Function, Object> SNAKEYAML_FACTORY = + // No load properties supported currently + props -> new org.yaml.snakeyaml.Yaml(new LoaderOptions()); + + static final Function, Object> SNAKEYAML_ENGINE_FACTORY = + props -> new org.snakeyaml.engine.v2.api.lowlevel.Parse(buildLoadSettings(props)); + + static LoadSettings buildLoadSettings(Map properties) { + return LoadSettings.builder() + .setUseMarks(getProperty(properties, Yaml.Settings.LOAD_USE_MARKS, Boolean::valueOf, true)) + .build(); + } + private final Map properties; private final boolean useSnakeYamlEngine; private final Object snakeYamlProvider; + private final Function yamlReaderProvider; YamlParserFactory(Map properties) { this.properties = new HashMap<>(properties); Object version = properties.get(Yaml.Settings.YAML_VERSION); - useSnakeYamlEngine = Yaml.Versions.V1_2.equals(version); - if (useSnakeYamlEngine) { - snakeYamlProvider = loadProvider(() -> new org.snakeyaml.engine.v2.api.lowlevel.Parse(buildLoadSettings(this.properties)), - MOD_SNAKEYAML_ENGINE); + if (version == null) { + snakeYamlProvider = Optional.empty() + .or(() -> loadProvider(this.properties, SNAKEYAML_FACTORY)) + .or(() -> loadProvider(this.properties, SNAKEYAML_ENGINE_FACTORY)) + .orElseThrow(SettingsBuilder::noProvidersFound); + + useSnakeYamlEngine = SNAKEYAML_ENGINE_PROVIDER.equals(snakeYamlProvider.getClass().getName()); } else { - snakeYamlProvider = loadProvider(() -> new org.yaml.snakeyaml.Yaml(buildLoaderOptions(this.properties)), - MOD_SNAKEYAML); + useSnakeYamlEngine = Yaml.Versions.V1_2.equals(version); + + if (useSnakeYamlEngine) { + snakeYamlProvider = loadProvider(this.properties, SNAKEYAML_ENGINE_FACTORY, MOD_SNAKEYAML_ENGINE); + } else { + snakeYamlProvider = loadProvider(this.properties, SNAKEYAML_FACTORY, MOD_SNAKEYAML); + } } - } - YamlParser createYamlParser(InputStream stream) { // NOSONAR - ignore use of wildcards - Reader reader; + yamlReaderProvider = useSnakeYamlEngine + ? org.snakeyaml.engine.v2.api.YamlUnicodeReader::new + : org.yaml.snakeyaml.reader.UnicodeReader::new; - if (useSnakeYamlEngine) { - reader = loadProvider(() -> new org.snakeyaml.engine.v2.api.YamlUnicodeReader(stream), - MOD_SNAKEYAML_ENGINE); - } else { - reader = loadProvider(() -> new org.yaml.snakeyaml.reader.UnicodeReader(stream), - MOD_SNAKEYAML); - } + // Ensure this property is always set, defaulting to Long.MAX_VALUE + replace(this.properties, Yaml.Settings.LOAD_MAX_ALIAS_EXPANSION_SIZE, Long::valueOf, Long.MAX_VALUE); + } - return createYamlParser(reader); + YamlParser createYamlParser(InputStream stream) { // NOSONAR - ignore use of wildcards + return createYamlParser(yamlReaderProvider.apply(stream)); } YamlParser createYamlParser(Reader reader) { // NOSONAR - ignore use of wildcards diff --git a/src/main/java/io/xlate/yamljson/YamlProvider.java b/src/main/java/io/xlate/yamljson/YamlProvider.java index 8444f69..d724148 100644 --- a/src/main/java/io/xlate/yamljson/YamlProvider.java +++ b/src/main/java/io/xlate/yamljson/YamlProvider.java @@ -21,8 +21,8 @@ import java.io.OutputStream; import java.io.Reader; import java.io.Writer; +import java.util.Collections; import java.util.Map; -import java.util.Set; import jakarta.json.JsonArrayBuilder; import jakarta.json.JsonBuilderFactory; @@ -48,20 +48,9 @@ final class YamlProvider extends JsonProvider { private final YamlReaderFactory defaultReaderFactory; private final YamlGeneratorFactory defaultGeneratorFactory; private final YamlWriterFactory defaultWriterFactory; - final String defaultVersion; - - public YamlProvider() { - Set supportedVersions = Yaml.Versions.supportedVersions(); - - if (supportedVersions.isEmpty()) { - throw new IllegalStateException("No YAML providers found on class/module path!"); - } else { - // v1.1 if available, otherwise v1.2 - defaultVersion = supportedVersions.iterator().next(); - } - - var defaultProperties = Map.of(Yaml.Settings.YAML_VERSION, defaultVersion); + YamlProvider() { + Map defaultProperties = Collections.emptyMap(); defaultParserFactory = new YamlParserFactory(defaultProperties); defaultReaderFactory = new YamlReaderFactory(defaultParserFactory); defaultGeneratorFactory = new YamlGeneratorFactory(defaultProperties); diff --git a/src/test/java/io/xlate/yamljson/YamlProviderTest.java b/src/test/java/io/xlate/yamljson/YamlProviderTest.java index d7e8b3b..d79344f 100644 --- a/src/test/java/io/xlate/yamljson/YamlProviderTest.java +++ b/src/test/java/io/xlate/yamljson/YamlProviderTest.java @@ -2,32 +2,25 @@ import static io.xlate.yamljson.YamlTestHelper.VERSIONS_SOURCE; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; -import org.junit.jupiter.api.condition.EnabledIfSystemProperty; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; class YamlProviderTest { - @Test - @EnabledIfSystemProperty(named = Yaml.Settings.YAML_VERSION, matches = "NONE") - void testIllegalStateExceptionThrown() { - assertThrows(IllegalStateException.class, () -> new YamlProvider()); - } - @DisabledIfSystemProperty(named = Yaml.Settings.YAML_VERSION, matches = "NONE") @ParameterizedTest @MethodSource(VERSIONS_SOURCE) void testDefaultVersion(String version) throws IOException { - if (Yaml.Versions.supportedVersions().size() == 1) { - assertEquals(version, new YamlProvider().defaultVersion); + String defaultVersion = YamlTestHelper.detectedVersions().first(); + + if (YamlTestHelper.detectedVersions().size() == 1) { + assertEquals(version, defaultVersion); } else { - assertEquals(Yaml.Versions.V1_1, new YamlProvider().defaultVersion); + assertEquals(Yaml.Versions.V1_1, defaultVersion); } } } diff --git a/src/test/java/io/xlate/yamljson/YamlTestHelper.java b/src/test/java/io/xlate/yamljson/YamlTestHelper.java index 45f34e9..4c8f8c6 100644 --- a/src/test/java/io/xlate/yamljson/YamlTestHelper.java +++ b/src/test/java/io/xlate/yamljson/YamlTestHelper.java @@ -20,8 +20,11 @@ import java.io.Reader; import java.io.Writer; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.SortedSet; +import java.util.TreeSet; import jakarta.json.JsonReader; import jakarta.json.JsonWriter; @@ -32,6 +35,23 @@ final class YamlTestHelper { static final String VERSIONS_SOURCE = "io.xlate.yamljson.YamlTestHelper#getTestVersions"; + private static final SortedSet VERSIONS_PRESENT; + + static { + SortedSet versions = new TreeSet<>(); + + SettingsBuilder.loadProvider(new HashMap<>(), YamlParserFactory.SNAKEYAML_FACTORY) + .ifPresent(provider -> versions.add(Yaml.Versions.V1_1)); + SettingsBuilder.loadProvider(new HashMap<>(), YamlParserFactory.SNAKEYAML_ENGINE_FACTORY) + .ifPresent(provider -> versions.add(Yaml.Versions.V1_2)); + + VERSIONS_PRESENT = versions; + } + + static SortedSet detectedVersions() { + return VERSIONS_PRESENT; + } + interface ThrowingConsumer { void accept(T t) throws Exception; } @@ -40,7 +60,7 @@ static Set getTestVersions() { String testVersions = System.getProperty(Yaml.Settings.YAML_VERSION); if (testVersions == null || testVersions.isBlank()) { - return Yaml.Versions.supportedVersions(); + return detectedVersions(); } if ("NONE".equals(testVersions)) { @@ -61,7 +81,7 @@ static void testEachVersion(ThrowingConsumer testCase) { } static boolean isOnlySupportedVersion(String version) { - return Yaml.Versions.supportedVersions().size() == 1 && Yaml.Versions.supportedVersions().contains(version); + return detectedVersions().size() == 1 && detectedVersions().contains(version); } //////////