From 80bafd379811f58cb5f47ab0266adf37bdcd210b Mon Sep 17 00:00:00 2001 From: Lazar Petrovic Date: Thu, 26 Dec 2024 12:01:50 +0100 Subject: [PATCH 1/4] remove snake from config dep Signed-off-by: Lazar Petrovic --- hiero-dependency-versions/build.gradle.kts | 3 +++ .../extensions/sources/YamlConfigSource.java | 22 ++++++++----------- .../src/main/java/module-info.java | 2 +- .../test/sources/YamlConfigSourceTest.java | 2 +- .../gossip/config/GossipConfigTest.java | 14 +++++++++++- 5 files changed, 27 insertions(+), 16 deletions(-) diff --git a/hiero-dependency-versions/build.gradle.kts b/hiero-dependency-versions/build.gradle.kts index 67e5cd5f4a6..ec9352ed17a 100644 --- a/hiero-dependency-versions/build.gradle.kts +++ b/hiero-dependency-versions/build.gradle.kts @@ -45,6 +45,9 @@ dependencies.constraints { api("com.fasterxml.jackson.core:jackson-databind:$jackson") { because("com.fasterxml.jackson.databind") } + api("com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:$jackson") { + because("com.fasterxml.jackson.dataformat") + } api("com.github.ben-manes.caffeine:caffeine:3.1.1") { because("com.github.benmanes.caffeine") } api("com.github.docker-java:docker-java-api:3.2.13") { because("com.github.dockerjava.api") } api("com.github.spotbugs:spotbugs-annotations:4.7.3") { diff --git a/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java b/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java index c2a2e0ba8f9..3112b3dd8e1 100644 --- a/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java +++ b/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java @@ -20,6 +20,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.swirlds.config.api.source.ConfigSource; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; @@ -38,7 +39,6 @@ import java.util.stream.Stream; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.yaml.snakeyaml.Yaml; /** * A config source that reads properties from a YAML file. @@ -68,6 +68,7 @@ public class YamlConfigSource implements ConfigSource { * The {@link ObjectMapper} used to convert maps to JSON. */ private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private static final ObjectMapper YAML_MAPPER = new ObjectMapper(new YAMLFactory()); /** * The map that contains all not-list properties. @@ -151,25 +152,20 @@ public YamlConfigSource(@NonNull final Path filePath, final int ordinal) { } } - private void convertYamlToMaps(@NonNull final InputStream resource) { + @SuppressWarnings("unchecked") + private void convertYamlToMaps(@NonNull final InputStream resource) throws IOException { Objects.requireNonNull(resource, "resource must not be null"); - final Yaml yaml = new Yaml(); - final Object rawData = yaml.load(resource); - processYamlNode("", rawData, properties, listProperties); + final Map map = YAML_MAPPER.readValue(resource, Map.class); + processYamlNode("", map, properties, listProperties); } @SuppressWarnings("unchecked") private void processYamlNode( @NonNull final String prefix, - @NonNull final Object node, + @NonNull final Map map, @NonNull final Map simpleProps, @NonNull final Map> listProps) { - if (!(node instanceof Map)) { - return; - } - - final Map map = (Map) node; for (final Map.Entry entry : map.entrySet()) { final String newPrefix = prefix.isEmpty() ? entry.getKey() : prefix + "." + entry.getKey(); final Object value = entry.getValue(); @@ -177,7 +173,7 @@ private void processYamlNode( try { switch (value) { case final List list -> handleList(listProps, list, newPrefix); - case final Map mapValue -> handleMap(simpleProps, listProps, mapValue, newPrefix); + case final Map mapValue -> handleMap(simpleProps, listProps, (Map)mapValue, newPrefix); case null -> simpleProps.put(newPrefix, null); default -> simpleProps.put(newPrefix, value.toString()); } @@ -190,7 +186,7 @@ private void processYamlNode( private void handleMap( final @NonNull Map simpleProps, final @NonNull Map> listProps, - final Map mapValue, + final Map mapValue, final String newPrefix) throws JsonProcessingException { if (mapValue.values().stream().noneMatch(v -> v instanceof Map || v instanceof List)) { diff --git a/platform-sdk/swirlds-config-extensions/src/main/java/module-info.java b/platform-sdk/swirlds-config-extensions/src/main/java/module-info.java index be27eda4e69..ec10e386853 100644 --- a/platform-sdk/swirlds-config-extensions/src/main/java/module-info.java +++ b/platform-sdk/swirlds-config-extensions/src/main/java/module-info.java @@ -9,7 +9,7 @@ requires com.swirlds.base; requires com.fasterxml.jackson.core; requires com.fasterxml.jackson.databind; + requires com.fasterxml.jackson.dataformat.yaml; requires org.apache.logging.log4j; - requires org.yaml.snakeyaml; requires static transitive com.github.spotbugs.annotations; } diff --git a/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java b/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java index 086b972641c..af8e91e5e1e 100644 --- a/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java +++ b/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java @@ -34,7 +34,7 @@ void testPropertyConfigSourceWhenPropertyFileNotFound() { } @Test - void testLoadProperties() throws Exception { + void testLoadProperties() { // given final String existingFile = "test.yaml"; diff --git a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/gossip/config/GossipConfigTest.java b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/gossip/config/GossipConfigTest.java index 770f6125294..29b8a25476a 100644 --- a/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/gossip/config/GossipConfigTest.java +++ b/platform-sdk/swirlds-platform-core/src/test/java/com/swirlds/platform/gossip/config/GossipConfigTest.java @@ -22,12 +22,14 @@ import com.swirlds.config.api.Configuration; import com.swirlds.config.extensions.sources.YamlConfigSource; import com.swirlds.config.extensions.test.fixtures.TestConfigBuilder; +import java.net.InetAddress; +import java.net.UnknownHostException; import org.junit.jupiter.api.Test; public class GossipConfigTest { @Test - void testReadingFile() { + void testReadingFile() throws UnknownHostException { final Configuration configuration = new TestConfigBuilder() .withSource(new YamlConfigSource("overwrites.yaml")) .getOrCreateConfig(); @@ -39,5 +41,15 @@ void testReadingFile() { assertNotNull(gossipConfig.networkEndpoints()); assertEquals(4, gossipConfig.networkEndpoints().size()); assertEquals(4, gossipConfig.endpointOverrides().size()); + + final NetworkEndpoint endpoint = gossipConfig.networkEndpoints().getFirst(); + assertEquals(0, endpoint.nodeId()); + assertEquals(InetAddress.getByName("10.10.10.1"), endpoint.hostname()); + assertEquals(1234, endpoint.port()); + + final NetworkEndpoint override = gossipConfig.endpointOverrides().getFirst(); + assertEquals(5, override.nodeId()); + assertEquals(InetAddress.getByName("10.10.10.11"), override.hostname()); + assertEquals(1238, override.port()); } } From 900a71fbb225095a122bd237fd652518c5644c6e Mon Sep 17 00:00:00 2001 From: Lazar Petrovic Date: Thu, 26 Dec 2024 15:04:38 +0100 Subject: [PATCH 2/4] cleanup Signed-off-by: Lazar Petrovic --- .../extensions/sources/YamlConfigSource.java | 106 ++++++++---------- .../test/sources/YamlConfigSourceTest.java | 15 +++ .../src/test/resources/withNoList.yaml | 3 + 3 files changed, 63 insertions(+), 61 deletions(-) create mode 100644 platform-sdk/swirlds-config-extensions/src/test/resources/withNoList.yaml diff --git a/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java b/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java index 3112b3dd8e1..24a5a25a78c 100644 --- a/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java +++ b/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java @@ -18,7 +18,7 @@ import static java.util.Objects.requireNonNull; -import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.swirlds.config.api.source.ConfigSource; @@ -35,8 +35,10 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; +import java.util.Spliterators; import java.util.stream.Collectors; import java.util.stream.Stream; +import java.util.stream.StreamSupport; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -65,9 +67,8 @@ public class YamlConfigSource implements ConfigSource { private static final Logger logger = LogManager.getLogger(YamlConfigSource.class); /** - * The {@link ObjectMapper} used to convert maps to JSON. + * The {@link ObjectMapper} used to read YAML. */ - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); private static final ObjectMapper YAML_MAPPER = new ObjectMapper(new YAMLFactory()); /** @@ -107,13 +108,13 @@ public YamlConfigSource(@NonNull final String fileName, final int ordinal) { this.listProperties = new HashMap<>(); this.ordinal = ordinal; - try (InputStream resource = + try (final InputStream resource = Thread.currentThread().getContextClassLoader().getResourceAsStream(fileName)) { if (resource == null) { throw new UncheckedIOException(new IOException("Resource not found: " + fileName)); } - convertYamlToMaps(resource); - } catch (IOException e) { + processYamlFile(resource); + } catch (final IOException e) { throw new UncheckedIOException("Failed to read YAML file " + fileName, e); } } @@ -145,73 +146,56 @@ public YamlConfigSource(@NonNull final Path filePath, final int ordinal) { return; } - try (InputStream resource = Files.newInputStream(filePath)) { - convertYamlToMaps(resource); - } catch (IOException e) { + try (final InputStream resource = Files.newInputStream(filePath)) { + processYamlFile(resource); + } catch (final IOException e) { throw new UncheckedIOException("Failed to read YAML file " + filePath, e); } } - @SuppressWarnings("unchecked") - private void convertYamlToMaps(@NonNull final InputStream resource) throws IOException { + private void processYamlFile(@NonNull final InputStream resource) throws IOException { Objects.requireNonNull(resource, "resource must not be null"); - final Map map = YAML_MAPPER.readValue(resource, Map.class); - processYamlNode("", map, properties, listProperties); + processNode(YAML_MAPPER.readTree(resource), ""); } - @SuppressWarnings("unchecked") - private void processYamlNode( - @NonNull final String prefix, - @NonNull final Map map, - @NonNull final Map simpleProps, - @NonNull final Map> listProps) { - - for (final Map.Entry entry : map.entrySet()) { - final String newPrefix = prefix.isEmpty() ? entry.getKey() : prefix + "." + entry.getKey(); - final Object value = entry.getValue(); - - try { - switch (value) { - case final List list -> handleList(listProps, list, newPrefix); - case final Map mapValue -> handleMap(simpleProps, listProps, (Map)mapValue, newPrefix); - case null -> simpleProps.put(newPrefix, null); - default -> simpleProps.put(newPrefix, value.toString()); - } - } catch (JsonProcessingException e) { - throw new IllegalStateException("Failed to convert map to JSON for key: " + newPrefix, e); - } + private void processNode(@NonNull final JsonNode node, @NonNull final String prefix) { + // if it's a simple field we parse the value + if(node.isValueNode()){ + properties.put(prefix, toValueString(node)); + return; + } + // if it's an array we parse the values and put them in a list + if(node.isArray()){ + final List list = StreamSupport.stream(Spliterators.spliteratorUnknownSize(node.elements(), 0), + false) + .map(this::toValueString) + .toList(); + listProperties.put(prefix, list); + return; } - } - private void handleMap( - final @NonNull Map simpleProps, - final @NonNull Map> listProps, - final Map mapValue, - final String newPrefix) - throws JsonProcessingException { - if (mapValue.values().stream().noneMatch(v -> v instanceof Map || v instanceof List)) { - simpleProps.put(newPrefix, OBJECT_MAPPER.writeValueAsString(mapValue)); - } else { - processYamlNode(newPrefix, mapValue, simpleProps, listProps); + // if it's an object we iterate over the fields to check if they are all value nodes + final boolean allValueNodes = StreamSupport.stream(Spliterators.spliteratorUnknownSize(node.fields(), 0), false) + .allMatch(e->e.getValue().isValueNode()); + // if all the fields are value nodes we store the raw string representation + if(allValueNodes){ + properties.put(prefix, node.toString()); + return; } + // if none of these criteria are met, we process the children + node.fields().forEachRemaining(entry -> { + final String newPrefix = prefix.isEmpty() ? entry.getKey() : prefix + "." + entry.getKey(); + processNode(entry.getValue(), newPrefix); + }); } - private void handleList( - final @NonNull Map> listProps, final List list, final String newPrefix) { - if (!list.isEmpty() && list.getFirst() instanceof Map) { - final List jsonList = list.stream() - .map(item -> { - try { - return OBJECT_MAPPER.writeValueAsString(item); - } catch (JsonProcessingException e) { - throw new IllegalStateException("Failed to convert map to JSON", e); - } - }) - .toList(); - listProps.put(newPrefix, jsonList); - } else { - listProps.put(newPrefix, list.stream().map(Object::toString).toList()); + private String toValueString(@NonNull final JsonNode node) { + // if it's a simple field we parse the value + if (node.isValueNode()) { + return node.asText(); } + // if it's an object we don't parse it, we just return the raw string representation + return node.toString(); } /** {@inheritDoc} */ @@ -225,7 +209,7 @@ public Set getPropertyNames() { /** {@inheritDoc} */ @Nullable @Override - public String getValue(@NonNull String propertyName) throws NoSuchElementException { + public String getValue(@NonNull final String propertyName) throws NoSuchElementException { requireNonNull(propertyName, "propertyName must not be null"); if (isListProperty(propertyName)) { throw new NoSuchElementException("Property " + propertyName + " is a list property"); diff --git a/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java b/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java index af8e91e5e1e..0586a50cfc9 100644 --- a/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java +++ b/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java @@ -20,6 +20,7 @@ import java.io.UncheckedIOException; import java.util.List; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; class YamlConfigSourceTest { @@ -68,4 +69,18 @@ void testLoadProperties() { Assertions.assertTrue(yamlConfigSource.isListProperty("gossip.randomListOfList")); } + + @Test +// @Disabled(""" +// This test is disabled because the parser has a defect, it determines the depth of parsing based on the +// presence of lists. if not lists are present, it will change the way it interprets the values""") + void testYamlFileWithNoList() { + // given + final String ymlFile = "withNoList.yaml"; + + // when + final YamlConfigSource yamlConfigSource = new YamlConfigSource(ymlFile, 1); + Assertions.assertEquals("random", yamlConfigSource.getValue("gossip.randomStringValue")); + Assertions.assertEquals("42", yamlConfigSource.getValue("gossip.randomIntValue")); + } } diff --git a/platform-sdk/swirlds-config-extensions/src/test/resources/withNoList.yaml b/platform-sdk/swirlds-config-extensions/src/test/resources/withNoList.yaml new file mode 100644 index 00000000000..470365f7df0 --- /dev/null +++ b/platform-sdk/swirlds-config-extensions/src/test/resources/withNoList.yaml @@ -0,0 +1,3 @@ +gossip: + randomStringValue: "random" + randomIntValue: 42 From 94314b9d0d74137efa71d9e676e045e1db10731f Mon Sep 17 00:00:00 2001 From: Lazar Petrovic Date: Thu, 26 Dec 2024 15:06:06 +0100 Subject: [PATCH 3/4] s Signed-off-by: Lazar Petrovic --- hiero-dependency-versions/build.gradle.kts | 17 ++++++++++++++++- .../extensions/sources/YamlConfigSource.java | 12 ++++++------ .../test/sources/YamlConfigSourceTest.java | 6 +++--- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/hiero-dependency-versions/build.gradle.kts b/hiero-dependency-versions/build.gradle.kts index ec9352ed17a..6a823c0c9ed 100644 --- a/hiero-dependency-versions/build.gradle.kts +++ b/hiero-dependency-versions/build.gradle.kts @@ -1,4 +1,19 @@ -// SPDX-License-Identifier: Apache-2.0 +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + plugins { id("org.hiero.gradle.base.lifecycle") id("org.hiero.gradle.base.jpms-modules") diff --git a/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java b/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java index 24a5a25a78c..617f2d00ace 100644 --- a/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java +++ b/platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java @@ -160,14 +160,14 @@ private void processYamlFile(@NonNull final InputStream resource) throws IOExcep private void processNode(@NonNull final JsonNode node, @NonNull final String prefix) { // if it's a simple field we parse the value - if(node.isValueNode()){ + if (node.isValueNode()) { properties.put(prefix, toValueString(node)); return; } // if it's an array we parse the values and put them in a list - if(node.isArray()){ - final List list = StreamSupport.stream(Spliterators.spliteratorUnknownSize(node.elements(), 0), - false) + if (node.isArray()) { + final List list = StreamSupport.stream( + Spliterators.spliteratorUnknownSize(node.elements(), 0), false) .map(this::toValueString) .toList(); listProperties.put(prefix, list); @@ -176,9 +176,9 @@ private void processNode(@NonNull final JsonNode node, @NonNull final String pre // if it's an object we iterate over the fields to check if they are all value nodes final boolean allValueNodes = StreamSupport.stream(Spliterators.spliteratorUnknownSize(node.fields(), 0), false) - .allMatch(e->e.getValue().isValueNode()); + .allMatch(e -> e.getValue().isValueNode()); // if all the fields are value nodes we store the raw string representation - if(allValueNodes){ + if (allValueNodes) { properties.put(prefix, node.toString()); return; } diff --git a/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java b/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java index 0586a50cfc9..4754d8f2e4e 100644 --- a/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java +++ b/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java @@ -71,9 +71,9 @@ void testLoadProperties() { } @Test -// @Disabled(""" -// This test is disabled because the parser has a defect, it determines the depth of parsing based on the -// presence of lists. if not lists are present, it will change the way it interprets the values""") + @Disabled(""" + This test is disabled because the parser has a defect, it determines the depth of parsing based on the + presence of lists. if not lists are present, it will change the way it interprets the values""") void testYamlFileWithNoList() { // given final String ymlFile = "withNoList.yaml"; From f7b2fbd79b7cb139ff6b8e270d3618054fe4331a Mon Sep 17 00:00:00 2001 From: Lazar Petrovic Date: Thu, 26 Dec 2024 15:10:35 +0100 Subject: [PATCH 4/4] comment Signed-off-by: Lazar Petrovic --- .../extensions/test/sources/YamlConfigSourceTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java b/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java index 4754d8f2e4e..65af66cab8f 100644 --- a/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java +++ b/platform-sdk/swirlds-config-extensions/src/test/java/com/swirlds/config/extensions/test/sources/YamlConfigSourceTest.java @@ -71,9 +71,11 @@ void testLoadProperties() { } @Test - @Disabled(""" + @Disabled( + """ This test is disabled because the parser has a defect, it determines the depth of parsing based on the - presence of lists. if not lists are present, it will change the way it interprets the values""") + presence of lists and objects. if some fields (like lists) are not present, it will change the way it + interprets the values other unrelated fields""") void testYamlFileWithNoList() { // given final String ymlFile = "withNoList.yaml";