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

Add Yaml2Json command #13

Merged
merged 1 commit into from
Jul 8, 2023
Merged

Add Yaml2Json command #13

merged 1 commit into from
Jul 8, 2023

Conversation

fpapon
Copy link
Member

@fpapon fpapon commented Jun 29, 2023

Fix #12

@fpapon fpapon requested a review from rmannibucau June 29, 2023 18:51

private Path convertAndWrite(String filename, String content) {
log.info("Converting " + filename);
final var target = Path.of(this.output + "/" + filename + ".json");
Copy link
Member

Choose a reason for hiding this comment

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

final var out = Path.of(output);

to do before the stream and pass it to convertAndWrite

here you just do out.resolve(filename + ".json")

Files.walkFileTree(sourceDir.toAbsolutePath(), new SimpleFileVisitor<>() {
@Override
public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException {
if (file.getFileName().toString().endsWith(".yaml")) {
Copy link
Member

Choose a reason for hiding this comment

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

                    final var name = file.getFileName().toString();
                    if (name.endsWith(".yaml")) {
                        sources.put(name.substring(0, name.length() - ".yaml".length(), Files.readString(file));
                    }

will avoid to drop unexpected data and to recompile a Pattern for each file

}
});
} catch (IOException e) {
throw new RuntimeException(e);
Copy link
Member

Choose a reason for hiding this comment

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

IllegalStateException?

}

private Path convertAndWrite(String filename, String content) {
log.info("Converting " + filename);
Copy link
Member

Choose a reason for hiding this comment

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

log.info(() -> "...") + add simple quotes around filename?

final var target = Path.of(this.output + "/" + filename + ".json");
try {
final var jsonString = jsonb.toJson(yaml2json.convert(JsonValue.class, content.trim()));
Files.deleteIfExists(target);
Copy link
Member

Choose a reason for hiding this comment

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

think write will truncate/overwrite by default so not needed?

log.info("Converting " + filename);
final var target = Path.of(this.output + "/" + filename + ".json");
try {
final var jsonString = jsonb.toJson(yaml2json.convert(JsonValue.class, content.trim()));
Copy link
Member

Choose a reason for hiding this comment

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

can need another round trip in a formatted json writer if you want to prettify it (maybe another option)

final var logs = executor.wrap(null, INFO, () -> new BundleBee().launch("yaml2json",
"--bundlebee.yaml2json.input", "src/test/resources/bundlebee/kubernetes",
"--bundlebee.yaml2json.output", "target/"));
assertTrue
Copy link
Member

Choose a reason for hiding this comment

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

can need to assert the output and use @TempDir Path work to get a temporary dir instead of target/, then just assert the expected files are there and contains what you expected for a few of them (smoke test like)


@Override
public CompletionStage<?> execute() {
final var sources = new HashMap<String, String>();
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure we need the staging (map), maybe we can just convert directly?
If the staging makes sense we should probably stage the converted value before writing them all to allow to not do anything if one will fail.

@Override
public FileVisitResult visitFile(final Path file, final BasicFileAttributes attrs) throws IOException {
if (file.getFileName().toString().endsWith(".yaml")) {
sources.put(file.getFileName().toString().replaceAll(".yaml", ""), Files.readString(file));
Copy link
Member

Choose a reason for hiding this comment

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

don't think filename is what you want, sounds more like sourceDir.toAbsolutePath().relativize(file).toString() (+ substring to drop .yaml) - indeed keep the source path ref once for all the visitor.

@fpapon fpapon merged commit 818c13f into yupiik:master Jul 8, 2023
2 checks passed
@fpapon fpapon deleted the yaml2json branch July 8, 2023 15:58
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.

Add a yaml2json command
2 participants