Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove snake yml #17170

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion hiero-dependency-versions/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -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")
Expand Down Expand Up @@ -45,6 +60,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") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@

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;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
Expand All @@ -34,11 +35,12 @@
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;
import org.yaml.snakeyaml.Yaml;

/**
* A config source that reads properties from a YAML file.
Expand All @@ -65,9 +67,9 @@
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());

/**
* The map that contains all not-list properties.
Expand Down Expand Up @@ -106,13 +108,13 @@
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) {

Check warning on line 117 in platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java

View check run for this annotation

Codecov / codecov/patch

platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java#L117

Added line #L117 was not covered by tests
throw new UncheckedIOException("Failed to read YAML file " + fileName, e);
}
}
Expand Down Expand Up @@ -144,78 +146,56 @@
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) {

Check warning on line 151 in platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java

View check run for this annotation

Codecov / codecov/patch

platform-sdk/swirlds-config-extensions/src/main/java/com/swirlds/config/extensions/sources/YamlConfigSource.java#L149-L151

Added lines #L149 - L151 were not covered by tests
throw new UncheckedIOException("Failed to read YAML file " + filePath, e);
}
}

private void convertYamlToMaps(@NonNull final InputStream resource) {
private void processYamlFile(@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);
processNode(YAML_MAPPER.readTree(resource), "");
}

@SuppressWarnings("unchecked")
private void processYamlNode(
@NonNull final String prefix,
@NonNull final Object node,
@NonNull final Map<String, String> simpleProps,
@NonNull final Map<String, List<String>> listProps) {

if (!(node instanceof Map)) {
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;
}

final Map<String, Object> map = (Map<String, Object>) node;
for (final Map.Entry<String, Object> 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, 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);
}
// if it's an array we parse the values and put them in a list
if (node.isArray()) {
final List<String> list = StreamSupport.stream(
Spliterators.spliteratorUnknownSize(node.elements(), 0), false)
.map(this::toValueString)
.toList();
listProperties.put(prefix, list);
return;
}
}

private void handleMap(
final @NonNull Map<String, String> simpleProps,
final @NonNull Map<String, List<String>> 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<String, List<String>> listProps, final List<?> list, final String newPrefix) {
if (!list.isEmpty() && list.getFirst() instanceof Map) {
final List<String> 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} */
Expand All @@ -229,7 +209,7 @@
/** {@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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -34,7 +35,7 @@ void testPropertyConfigSourceWhenPropertyFileNotFound() {
}

@Test
void testLoadProperties() throws Exception {
void testLoadProperties() {
// given
final String existingFile = "test.yaml";

Expand Down Expand Up @@ -68,4 +69,20 @@ void testLoadProperties() throws Exception {

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 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";

// when
final YamlConfigSource yamlConfigSource = new YamlConfigSource(ymlFile, 1);
Assertions.assertEquals("random", yamlConfigSource.getValue("gossip.randomStringValue"));
Assertions.assertEquals("42", yamlConfigSource.getValue("gossip.randomIntValue"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
gossip:
randomStringValue: "random"
randomIntValue: 42
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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());
}
}
Loading