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

APP-2884 #175

Merged
merged 6 commits into from
Aug 5, 2020
Merged

APP-2884 #175

merged 6 commits into from
Aug 5, 2020

Conversation

symphony-hong
Copy link
Contributor

InputStream cannot be read again after the first read
-> read content from InputStream first and detect format
from the extracted content

Using JsonMapper#convertValue instead of JsonMapper#readValue
to avoid JsonProcessingException

Move LegacyConfigMapper to legacy package

Using JsonNode#isMissingNode() instead of equals null

Add very first unittest to make sure the Loader is working

Define new structure for Configuration in 2.0 in BdkConfig.java.

Both of yaml and json formats are supported in 2.0, so Yaml parser
is added for parsing Yaml config

LegacyConfig model for supporting the legacy version of configuration.

Configuration is loaded by BdkConfigLoader in three ways:
- From path of config file: BdkConfigLoader#loadFromFile
- From inputstream: BdkConfigLoader#loadFromInputStream
- From classpath: BdkConfigLoader#loadFromClasspath

Set minimum code coverage score: 0.9
Refactor config loading function, handle both Config v1 and v2
in one function. Configuration v1 will be automatically converted
to config v2 instance.

Add a static function BdkConfig#fromLegacyConfig to convert
config from v1 to v2.

Class BdkConfigParser to parse config input stream to detect if
config is in JSON or YAML format.
InputStream cannot be read again after the first read
-> read content from InputStream first and detect format
from the extracted content

Using JsonMapper#convertValue instead of JsonMapper#readValue
to avoid JsonProcessingException

Move LegacyConfigMapper to legacy package

Using JsonNode#isMissingNode() instead of equals null

Add very first unittest to make sure the Loader is working
@symphony-hong symphony-hong requested review from a team and thibauult August 5, 2020 16:10
Copy link
Member

@thibauult thibauult left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix 👍

@symphony-hong symphony-hong merged commit a0c64e9 into finos:master Aug 5, 2020
@@ -20,14 +24,18 @@
public static JsonNode parse(InputStream inputStream) throws BdkConfigException {
JSON_MAPPER.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
YAML_MAPPER.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
String content = new BufferedReader(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we have commons io as a dependency we could use IOUtils.toString(inputstream)


@Test
public void loadFromJsonFileTest() throws JsonProcessingException, BdkConfigException {
String configPath = System.getProperty("user.dir") + "/src/test/resources/config.json";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work in IDEA? not sure the default current folder is the module's folder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants