From ae725797b0a624bd1e25c10fa72bbb587924f397 Mon Sep 17 00:00:00 2001 From: Martin Morissette Date: Tue, 28 Mar 2023 19:40:53 -0400 Subject: [PATCH 1/8] issue-501: Added fileoutput parameters on maven plugin. No tests added yet. --- .../openapidiff/maven/OpenApiDiffMojo.java | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java b/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java index 7b1e679d4..ad6e36d63 100644 --- a/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java +++ b/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java @@ -1,5 +1,7 @@ package org.openapitools.openapidiff.maven; +import java.io.FileWriter; +import java.io.IOException; import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; @@ -9,6 +11,7 @@ import org.openapitools.openapidiff.core.OpenApiCompare; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.openapitools.openapidiff.core.output.ConsoleRender; +import org.openapitools.openapidiff.core.output.MarkdownRender; /** A Maven Mojo that diffs two OpenAPI specifications and reports on differences. */ @Mojo(name = "diff", defaultPhase = LifecyclePhase.TEST) @@ -28,6 +31,15 @@ public class OpenApiDiffMojo extends AbstractMojo { @Parameter(property = "skip", defaultValue = "false") Boolean skip = false; + @Parameter(property = "consoleOutputFileName", defaultValue = "") + String consoleOutputFileName; + + @Parameter(property = "jsonOutputFileName", defaultValue = "") + String jsonOutputFileName; + + @Parameter(property = "markdownOutputFileName", defaultValue = "") + String markdownOutputFileName; + @Override public void execute() throws MojoExecutionException, MojoFailureException { if (Boolean.TRUE.equals(skip)) { @@ -37,7 +49,10 @@ public void execute() throws MojoExecutionException, MojoFailureException { try { final ChangedOpenApi diff = OpenApiCompare.fromLocations(oldSpec, newSpec); - getLog().info(new ConsoleRender().render(diff)); + + generateConsoleOutput(diff); + generateJson(diff); + generateMarkdown(diff); if (failOnIncompatible && diff.isIncompatible()) { throw new BackwardIncompatibilityException("The API changes broke backward compatibility"); @@ -50,4 +65,40 @@ public void execute() throws MojoExecutionException, MojoFailureException { throw new MojoExecutionException("Unexpected error", e); } } + + private void generateJson(final ChangedOpenApi diff) { + if (isValidFileName(jsonOutputFileName)) { + final String render = new MarkdownRender().render(diff); + writeToFile(render, jsonOutputFileName); + } + } + + private void generateConsoleOutput(final ChangedOpenApi diff) { + final String render = new ConsoleRender().render(diff); + if (isValidFileName(consoleOutputFileName)) { + writeToFile(render, consoleOutputFileName); + } + getLog().info(render); + } + + private void generateMarkdown(final ChangedOpenApi diff) { + if (isValidFileName(markdownOutputFileName)) { + final String render = new MarkdownRender().render(diff); + writeToFile(render, markdownOutputFileName); + } + } + + private boolean isValidFileName(String fileName) { + return fileName != null && !fileName.isEmpty(); + } + + private static void writeToFile(final String render, final String filename) { + try { + final FileWriter fileWriter = new FileWriter(filename); + fileWriter.write(render); + fileWriter.close(); + } catch (IOException e) { + e.printStackTrace(); + } + } } From 76456e064d65931c4ef4ce143af959d2bf257d82 Mon Sep 17 00:00:00 2001 From: Martin Morissette Date: Thu, 30 Mar 2023 20:59:59 -0400 Subject: [PATCH 2/8] issue-501: Refactored. Added unit tests. Added logging. --- .../openapidiff/core/utils/ChangedUtils.java | 4 +- .../openapidiff/core/utils/Copy.java | 4 +- .../openapidiff/core/utils/EndpointUtils.java | 4 +- .../openapidiff/core/utils/FileUtils.java | 51 ++++++++ core/src/main/resources/logback.xml | 11 ++ .../openapidiff/core/utils/FileUtilsTest.java | 103 +++++++++++++++ .../openapidiff/maven/OpenApiDiffMojo.java | 47 +++---- .../maven/OpenApiDiffMojoTest.java | 119 +++++++++++++++--- maven/src/test/resources/logback.xml | 11 ++ 9 files changed, 304 insertions(+), 50 deletions(-) create mode 100644 core/src/main/java/org/openapitools/openapidiff/core/utils/FileUtils.java create mode 100644 core/src/main/resources/logback.xml create mode 100644 core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java create mode 100644 maven/src/test/resources/logback.xml diff --git a/core/src/main/java/org/openapitools/openapidiff/core/utils/ChangedUtils.java b/core/src/main/java/org/openapitools/openapidiff/core/utils/ChangedUtils.java index 27a5e0e20..6da806a96 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/utils/ChangedUtils.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/utils/ChangedUtils.java @@ -5,7 +5,9 @@ public class ChangedUtils { - private ChangedUtils() {} + private ChangedUtils() { + throw new UnsupportedOperationException("Utility class. Do not instantiate"); + } public static boolean isUnchanged(Changed changed) { return changed == null || changed.isUnchanged(); diff --git a/core/src/main/java/org/openapitools/openapidiff/core/utils/Copy.java b/core/src/main/java/org/openapitools/openapidiff/core/utils/Copy.java index 4179fd359..73c9a073b 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/utils/Copy.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/utils/Copy.java @@ -5,7 +5,9 @@ public class Copy { - private Copy() {} + private Copy() { + throw new UnsupportedOperationException("Utility class. Do not instantiate"); + } public static Map copyMap(Map map) { if (map == null) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/utils/EndpointUtils.java b/core/src/main/java/org/openapitools/openapidiff/core/utils/EndpointUtils.java index 749b0bdbd..c749d8a54 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/utils/EndpointUtils.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/utils/EndpointUtils.java @@ -10,7 +10,9 @@ public class EndpointUtils { - private EndpointUtils() {} + private EndpointUtils() { + throw new UnsupportedOperationException("Utility class. Do not instantiate"); + } public static Collection convert2Endpoints( String pathUrl, Map map) { diff --git a/core/src/main/java/org/openapitools/openapidiff/core/utils/FileUtils.java b/core/src/main/java/org/openapitools/openapidiff/core/utils/FileUtils.java new file mode 100644 index 000000000..8846c15d8 --- /dev/null +++ b/core/src/main/java/org/openapitools/openapidiff/core/utils/FileUtils.java @@ -0,0 +1,51 @@ +package org.openapitools.openapidiff.core.utils; + +import java.io.FileWriter; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.nio.file.Paths; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public final class FileUtils { + + private static final Logger logger = LoggerFactory.getLogger(FileUtils.class); + + private FileUtils() { + throw new UnsupportedOperationException("Utility class. Do not instantiate"); + } + + public static boolean isValidFileName(final String fileName) { + return fileName != null && !fileName.isEmpty(); + } + + public static void writeToFile(final String content, final String fileName) { + if (content == null) { + logger.debug("Content cannot be null"); + return; + } + + if (!isValidFileName(fileName)) { + logger.debug("File name cannot be null or empty."); + return; + } + + try { + final Path filePath = Paths.get(fileName); + if (Files.exists(filePath)) { + logger.info("File {} already exists. Skip writing to file.", fileName); + return; + } + } catch (final InvalidPathException invalidPathException) { + throw new IllegalArgumentException("File name is an invalid path."); + } + + try (final FileWriter fileWriter = new FileWriter(fileName)) { + fileWriter.write(content); + } catch (final IOException ioException) { + logger.error("Exception while writing to file {}", fileName); + } + } +} diff --git a/core/src/main/resources/logback.xml b/core/src/main/resources/logback.xml new file mode 100644 index 000000000..018a4277c --- /dev/null +++ b/core/src/main/resources/logback.xml @@ -0,0 +1,11 @@ + + + + [%thread] %-5level %logger{36} - %msg%n + + + + + + + \ No newline at end of file diff --git a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java new file mode 100644 index 000000000..d2b49bc1f --- /dev/null +++ b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java @@ -0,0 +1,103 @@ +package org.openapitools.openapidiff.core.utils; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import org.apache.commons.lang3.StringUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +class FileUtilsTest { + + private static final Logger logger = LoggerFactory.getLogger(FileUtilsTest.class); + + private final String filePath = "target/output.txt"; + private final String sampleContent = "someContent"; + + @BeforeEach + void setup() { + cleanupGeneratedFiles(); + } + + @AfterEach + void tearDown() { + cleanupGeneratedFiles(); + } + + @Test + void isValidFileName_fileNameIsValid_returnsTrue() { + assertTrue(FileUtils.isValidFileName(filePath)); + } + + @Test + void isValidFileName_fileNameIsNull_returnsFalse() { + assertFalse(FileUtils.isValidFileName(null)); + } + + @Test + void isValidFileName_fileNameIsEmpty_returnsFalse() { + final String filename = StringUtils.EMPTY; + assertFalse(FileUtils.isValidFileName(filename)); + } + + @Test + void writeToFile_contentIsNull_doesNothing() { + assertDoesNotThrow(() -> FileUtils.writeToFile(null, filePath)); + assertFalse(Files.exists(Paths.get(filePath))); + } + + @Test + void writeToFile_filenameIsNull_doesNothing() { + assertDoesNotThrow(() -> FileUtils.writeToFile(sampleContent, null)); + assertFalse(Files.exists(Paths.get(filePath))); + } + + @Test + void writeToFile_filenameIsEmpty_doesNothing() { + assertDoesNotThrow(() -> FileUtils.writeToFile(sampleContent, StringUtils.EMPTY)); + assertFalse(Files.exists(Paths.get(filePath))); + } + + @Test + void writeToFile_filenamePathIsInvalid_ThrowsIllegalArgumentException() { + assertThrows( + IllegalArgumentException.class, () -> FileUtils.writeToFile(sampleContent, "?%#$")); + assertFalse(Files.exists(Paths.get(filePath))); + } + + @Test + void writeToFile_fileExists_doesNothing() { + final Path path = Paths.get(filePath); + try { + Files.createFile(path); + } catch (IOException e) { + fail("Cannot create file for test."); + } + assertDoesNotThrow(() -> FileUtils.writeToFile(sampleContent, filePath)); + assertTrue(Files.exists(path)); + } + + @Test + void writeToFile_fileDoesNotExist_createsFile() { + assertDoesNotThrow(() -> FileUtils.writeToFile(sampleContent, filePath)); + assertTrue(Files.exists(Paths.get(filePath))); + } + + private void cleanupGeneratedFiles() { + try { + Files.deleteIfExists(Paths.get(filePath)); + } catch (IOException ioException) { + logger.warn("Exception while trying to delete file {}", filePath); + } + } +} diff --git a/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java b/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java index ad6e36d63..3f0391935 100644 --- a/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java +++ b/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java @@ -1,7 +1,7 @@ package org.openapitools.openapidiff.maven; -import java.io.FileWriter; -import java.io.IOException; +import static org.openapitools.openapidiff.core.utils.FileUtils.writeToFile; + import org.apache.maven.plugin.AbstractMojo; import org.apache.maven.plugin.MojoExecutionException; import org.apache.maven.plugin.MojoFailureException; @@ -16,6 +16,7 @@ /** A Maven Mojo that diffs two OpenAPI specifications and reports on differences. */ @Mojo(name = "diff", defaultPhase = LifecyclePhase.TEST) public class OpenApiDiffMojo extends AbstractMojo { + @Parameter(property = "oldSpec") String oldSpec; @@ -50,9 +51,9 @@ public void execute() throws MojoExecutionException, MojoFailureException { try { final ChangedOpenApi diff = OpenApiCompare.fromLocations(oldSpec, newSpec); - generateConsoleOutput(diff); - generateJson(diff); - generateMarkdown(diff); + writeDiffAsTextToFile(diff); + writeDiffAsJsonToFile(diff); + writeDiffAsMarkdownToFile(diff); if (failOnIncompatible && diff.isIncompatible()) { throw new BackwardIncompatibilityException("The API changes broke backward compatibility"); @@ -66,39 +67,19 @@ public void execute() throws MojoExecutionException, MojoFailureException { } } - private void generateJson(final ChangedOpenApi diff) { - if (isValidFileName(jsonOutputFileName)) { - final String render = new MarkdownRender().render(diff); - writeToFile(render, jsonOutputFileName); - } + private void writeDiffAsJsonToFile(final ChangedOpenApi diff) { + final String render = new MarkdownRender().render(diff); + writeToFile(render, jsonOutputFileName); } - private void generateConsoleOutput(final ChangedOpenApi diff) { + private void writeDiffAsTextToFile(final ChangedOpenApi diff) { final String render = new ConsoleRender().render(diff); - if (isValidFileName(consoleOutputFileName)) { - writeToFile(render, consoleOutputFileName); - } + writeToFile(render, consoleOutputFileName); getLog().info(render); } - private void generateMarkdown(final ChangedOpenApi diff) { - if (isValidFileName(markdownOutputFileName)) { - final String render = new MarkdownRender().render(diff); - writeToFile(render, markdownOutputFileName); - } - } - - private boolean isValidFileName(String fileName) { - return fileName != null && !fileName.isEmpty(); - } - - private static void writeToFile(final String render, final String filename) { - try { - final FileWriter fileWriter = new FileWriter(filename); - fileWriter.write(render); - fileWriter.close(); - } catch (IOException e) { - e.printStackTrace(); - } + private void writeDiffAsMarkdownToFile(final ChangedOpenApi diff) { + final String render = new MarkdownRender().render(diff); + writeToFile(render, markdownOutputFileName); } } diff --git a/maven/src/test/java/org/openapitools/openapidiff/maven/OpenApiDiffMojoTest.java b/maven/src/test/java/org/openapitools/openapidiff/maven/OpenApiDiffMojoTest.java index 7deae4233..10bd186e5 100644 --- a/maven/src/test/java/org/openapitools/openapidiff/maven/OpenApiDiffMojoTest.java +++ b/maven/src/test/java/org/openapitools/openapidiff/maven/OpenApiDiffMojoTest.java @@ -1,15 +1,44 @@ package org.openapitools.openapidiff.maven; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; import org.apache.maven.plugin.MojoExecutionException; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; class OpenApiDiffMojoTest { + + private static final Logger logger = LoggerFactory.getLogger(OpenApiDiffMojoTest.class); + + private final File oldSpecFile = new File("src/test/resources/oldspec.yaml"); + private final File newSpecFile = new File("src/test/resources/newspec.yaml"); + + private final File consoleOutputfile = new File("target/diff.txt"); + private final File markdownOutputfile = new File("target/diff.md"); + private final File jsonOutputfile = new File("target/diff.json"); + + @BeforeEach + void setup() { + cleanupGeneratedFiles(); + } + + @AfterEach + void tearDown() { + cleanupGeneratedFiles(); + } + @Test void Should_NotThrow_When_SpecHasNoChanges() { - final String oldSpec = new File("src/test/resources/oldspec.yaml").getAbsolutePath(); + final String oldSpec = oldSpecFile.getAbsolutePath(); final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); mojo.oldSpec = oldSpec; @@ -22,8 +51,8 @@ void Should_NotThrow_When_SpecHasNoChanges() { @Test void Should_NotThrow_When_SpecIsCompatible() { final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); - mojo.oldSpec = new File("src/test/resources/oldspec.yaml").getAbsolutePath(); - mojo.newSpec = new File("src/test/resources/newspec.yaml").getAbsolutePath(); + mojo.oldSpec = oldSpecFile.getAbsolutePath(); + mojo.newSpec = newSpecFile.getAbsolutePath(); mojo.failOnIncompatible = true; assertDoesNotThrow(mojo::execute); @@ -32,8 +61,8 @@ void Should_NotThrow_When_SpecIsCompatible() { @Test void Should_Throw_When_SpecIsDifferent() { final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); - mojo.oldSpec = new File("src/test/resources/oldspec.yaml").getAbsolutePath(); - mojo.newSpec = new File("src/test/resources/newspec.yaml").getAbsolutePath(); + mojo.oldSpec = oldSpecFile.getAbsolutePath(); + mojo.newSpec = newSpecFile.getAbsolutePath(); mojo.failOnChanged = true; assertThrows(ApiChangedException.class, mojo::execute); @@ -43,7 +72,7 @@ void Should_Throw_When_SpecIsDifferent() { void Should_MojoExecutionException_When_MissingOldSpec() { final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); mojo.oldSpec = new File("DOES_NOT_EXIST").getAbsolutePath(); - mojo.newSpec = new File("src/test/resources/newspec.yaml").getAbsolutePath(); + mojo.newSpec = newSpecFile.getAbsolutePath(); assertThrows(MojoExecutionException.class, mojo::execute); } @@ -51,7 +80,7 @@ void Should_MojoExecutionException_When_MissingOldSpec() { @Test void Should_MojoExecutionException_When_MissingNewSpec() { final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); - mojo.oldSpec = new File("src/test/resources/oldspec.yaml").getAbsolutePath(); + mojo.oldSpec = oldSpecFile.getAbsolutePath(); mojo.newSpec = new File("DOES_NOT_EXIST").getAbsolutePath(); assertThrows(MojoExecutionException.class, mojo::execute); @@ -60,8 +89,8 @@ void Should_MojoExecutionException_When_MissingNewSpec() { @Test void Should_NotThrow_When_DefaultsAndSpecIsIncompatible() { final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); - mojo.oldSpec = new File("src/test/resources/newspec.yaml").getAbsolutePath(); - mojo.newSpec = new File("src/test/resources/oldspec.yaml").getAbsolutePath(); + mojo.oldSpec = newSpecFile.getAbsolutePath(); + mojo.newSpec = oldSpecFile.getAbsolutePath(); assertDoesNotThrow(mojo::execute); } @@ -69,8 +98,8 @@ void Should_NotThrow_When_DefaultsAndSpecIsIncompatible() { @Test void Should_BackwardIncompatibilityException_When_WantsExceptionAndSpecIsIncompatible() { final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); - mojo.oldSpec = new File("src/test/resources/newspec.yaml").getAbsolutePath(); - mojo.newSpec = new File("src/test/resources/oldspec.yaml").getAbsolutePath(); + mojo.oldSpec = newSpecFile.getAbsolutePath(); + mojo.newSpec = oldSpecFile.getAbsolutePath(); mojo.failOnIncompatible = true; assertThrows(BackwardIncompatibilityException.class, mojo::execute); @@ -79,11 +108,73 @@ void Should_BackwardIncompatibilityException_When_WantsExceptionAndSpecIsIncompa @Test void Should_Skip_Mojo_WhenSkipIsTrue() { final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); - mojo.oldSpec = new File("src/test/resources/newspec.yaml").getAbsolutePath(); - mojo.newSpec = new File("src/test/resources/oldspec.yaml").getAbsolutePath(); + mojo.oldSpec = newSpecFile.getAbsolutePath(); + mojo.newSpec = oldSpecFile.getAbsolutePath(); mojo.failOnIncompatible = true; mojo.skip = true; assertDoesNotThrow(mojo::execute); } + + @Test + void Should_outputToTextFile_When_SpecIsDifferent() { + final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); + mojo.oldSpec = oldSpecFile.getAbsolutePath(); + mojo.newSpec = newSpecFile.getAbsolutePath(); + + mojo.consoleOutputFileName = consoleOutputfile.getAbsolutePath(); + mojo.failOnChanged = true; + + assertThrows(ApiChangedException.class, mojo::execute); + + assertTrue(Files.exists(consoleOutputfile.toPath())); + } + + @Test + void Should_outputToMarkdownFile_When_SpecIsDifferent() { + final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); + mojo.oldSpec = oldSpecFile.getAbsolutePath(); + mojo.newSpec = newSpecFile.getAbsolutePath(); + + mojo.markdownOutputFileName = markdownOutputfile.getAbsolutePath(); + mojo.failOnChanged = true; + + assertThrows(ApiChangedException.class, mojo::execute); + + assertTrue(Files.exists(markdownOutputfile.toPath())); + } + + @Test + void Should_outputToJsonFile_When_SpecIsDifferent() { + final OpenApiDiffMojo mojo = new OpenApiDiffMojo(); + mojo.oldSpec = oldSpecFile.getAbsolutePath(); + mojo.newSpec = newSpecFile.getAbsolutePath(); + + mojo.jsonOutputFileName = jsonOutputfile.getAbsolutePath(); + mojo.failOnChanged = true; + + assertThrows(ApiChangedException.class, mojo::execute); + + assertTrue(Files.exists(jsonOutputfile.toPath())); + } + + private void cleanupGeneratedFiles() { + try { + Files.deleteIfExists(Paths.get(consoleOutputfile.getPath())); + } catch (IOException ioException) { + logger.warn("Exception while trying to delete file {}", consoleOutputfile.getAbsolutePath()); + } + + try { + Files.deleteIfExists(Paths.get(markdownOutputfile.getPath())); + } catch (IOException ioException) { + logger.warn("Exception while trying to delete file {}", markdownOutputfile.getAbsolutePath()); + } + + try { + Files.deleteIfExists(Paths.get(jsonOutputfile.getPath())); + } catch (IOException ioException) { + logger.warn("Exception while trying to delete file {}", jsonOutputfile.getAbsolutePath()); + } + } } diff --git a/maven/src/test/resources/logback.xml b/maven/src/test/resources/logback.xml new file mode 100644 index 000000000..018a4277c --- /dev/null +++ b/maven/src/test/resources/logback.xml @@ -0,0 +1,11 @@ + + + + [%thread] %-5level %logger{36} - %msg%n + + + + + + + \ No newline at end of file From 99be56e430d085d484f6d4c0ec596a85eff52f4d Mon Sep 17 00:00:00 2001 From: Martin Morissette Date: Thu, 30 Mar 2023 21:08:33 -0400 Subject: [PATCH 3/8] issue-501: Fix unit tests. --- .../org/openapitools/openapidiff/core/utils/FileUtilsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java index d2b49bc1f..79943d0ba 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java @@ -71,7 +71,7 @@ void writeToFile_filenameIsEmpty_doesNothing() { @Test void writeToFile_filenamePathIsInvalid_ThrowsIllegalArgumentException() { assertThrows( - IllegalArgumentException.class, () -> FileUtils.writeToFile(sampleContent, "?%#$")); + IllegalArgumentException.class, () -> FileUtils.writeToFile(sampleContent, " ")); assertFalse(Files.exists(Paths.get(filePath))); } From 28ddfc9ce307a33ed5c3abf956e51951abc94c65 Mon Sep 17 00:00:00 2001 From: Martin Morissette Date: Thu, 30 Mar 2023 21:12:04 -0400 Subject: [PATCH 4/8] issue-501: Attempting to fix unit tests. --- .../org/openapitools/openapidiff/core/utils/FileUtilsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java index 79943d0ba..ad2211181 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java @@ -71,7 +71,7 @@ void writeToFile_filenameIsEmpty_doesNothing() { @Test void writeToFile_filenamePathIsInvalid_ThrowsIllegalArgumentException() { assertThrows( - IllegalArgumentException.class, () -> FileUtils.writeToFile(sampleContent, " ")); + IllegalArgumentException.class, () -> FileUtils.writeToFile(sampleContent, " .<")); assertFalse(Files.exists(Paths.get(filePath))); } From 36f7f196ceb0fc3b6eb5e73129bcbf114d2c66c6 Mon Sep 17 00:00:00 2001 From: Martin Morissette Date: Thu, 30 Mar 2023 21:15:20 -0400 Subject: [PATCH 5/8] issue-501: Removed system dependent test. --- .../openapitools/openapidiff/core/utils/FileUtilsTest.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java index ad2211181..f9f5c8188 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java @@ -68,13 +68,6 @@ void writeToFile_filenameIsEmpty_doesNothing() { assertFalse(Files.exists(Paths.get(filePath))); } - @Test - void writeToFile_filenamePathIsInvalid_ThrowsIllegalArgumentException() { - assertThrows( - IllegalArgumentException.class, () -> FileUtils.writeToFile(sampleContent, " .<")); - assertFalse(Files.exists(Paths.get(filePath))); - } - @Test void writeToFile_fileExists_doesNothing() { final Path path = Paths.get(filePath); From 9f7e3f8f715c04130e982e92e65a83fe26e860d1 Mon Sep 17 00:00:00 2001 From: Martin Morissette Date: Thu, 30 Mar 2023 21:16:42 -0400 Subject: [PATCH 6/8] issue-501: Cleanup --- .../org/openapitools/openapidiff/core/utils/FileUtilsTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java index f9f5c8188..2778dd2ad 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java @@ -2,7 +2,6 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; From 37b520c6c7432395dc95dba1cc9222cb61dcb89d Mon Sep 17 00:00:00 2001 From: Martin Morissette Date: Thu, 30 Mar 2023 21:30:32 -0400 Subject: [PATCH 7/8] issue-501: Fix json output. Updated maven example. Updated documentation. --- README.md | 19 ++++++++++--------- maven-example/pom.xml | 3 +++ .../openapidiff/maven/OpenApiDiffMojo.java | 11 ++++++----- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index c9cf9f78c..60d722e7f 100644 --- a/README.md +++ b/README.md @@ -145,6 +145,12 @@ Add openapi-diff to your POM to show diffs when you test your Maven project. You true true + + ${project.basedir}/../maven/target/diff.txt + + ${project.basedir}/../maven/target/diff.json + + ${project.basedir}/../maven/target/diff.md @@ -167,6 +173,7 @@ public class Main { ``` ### Render difference + --- #### HTML @@ -176,11 +183,9 @@ String html = new HtmlRender("Changelog", .render(diff); try { - FileWriter fw = new FileWriter( - "testNewApi.html"); + FileWriter fw = new FileWriter("testNewApi.html"); fw.write(html); fw.close(); - } catch (IOException e) { e.printStackTrace(); } @@ -191,11 +196,9 @@ try { ```java String render = new MarkdownRender().render(diff); try { - FileWriter fw = new FileWriter( - "testDiff.md"); + FileWriter fw = new FileWriter("testDiff.md"); fw.write(render); fw.close(); - } catch (IOException e) { e.printStackTrace(); } @@ -207,11 +210,9 @@ try { ```java String render = new JsonRender().render(diff); try { - FileWriter fw = new FileWriter( - "testDiff.json"); + FileWriter fw = new FileWriter("testDiff.json"); fw.write(render); fw.close(); - } catch (IOException e) { e.printStackTrace(); } diff --git a/maven-example/pom.xml b/maven-example/pom.xml index 46d9dda3b..bb3d9c7c5 100644 --- a/maven-example/pom.xml +++ b/maven-example/pom.xml @@ -33,6 +33,9 @@ ${project.basedir}/../maven/src/test/resources/oldspec.yaml ${project.basedir}/../maven/src/test/resources/newspec.yaml true + ${project.basedir}/../maven/target/diff.txt + ${project.basedir}/../maven/target/diff.json + ${project.basedir}/../maven/target/diff.md diff --git a/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java b/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java index 3f0391935..6f0206acd 100644 --- a/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java +++ b/maven/src/main/java/org/openapitools/openapidiff/maven/OpenApiDiffMojo.java @@ -11,6 +11,7 @@ import org.openapitools.openapidiff.core.OpenApiCompare; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.openapitools.openapidiff.core.output.ConsoleRender; +import org.openapitools.openapidiff.core.output.JsonRender; import org.openapitools.openapidiff.core.output.MarkdownRender; /** A Maven Mojo that diffs two OpenAPI specifications and reports on differences. */ @@ -67,17 +68,17 @@ public void execute() throws MojoExecutionException, MojoFailureException { } } - private void writeDiffAsJsonToFile(final ChangedOpenApi diff) { - final String render = new MarkdownRender().render(diff); - writeToFile(render, jsonOutputFileName); - } - private void writeDiffAsTextToFile(final ChangedOpenApi diff) { final String render = new ConsoleRender().render(diff); writeToFile(render, consoleOutputFileName); getLog().info(render); } + private void writeDiffAsJsonToFile(final ChangedOpenApi diff) { + final String render = new JsonRender().render(diff); + writeToFile(render, jsonOutputFileName); + } + private void writeDiffAsMarkdownToFile(final ChangedOpenApi diff) { final String render = new MarkdownRender().render(diff); writeToFile(render, markdownOutputFileName); From fd995fce08625dd7211ffc629bf678881d5b4a3e Mon Sep 17 00:00:00 2001 From: Jochen Schalanda Date: Tue, 25 Jul 2023 23:00:39 +0200 Subject: [PATCH 8/8] Maven plugin should overwrite output files --- .../openapidiff/core/utils/FileUtils.java | 21 ++++--------------- .../openapidiff/core/utils/FileUtilsTest.java | 20 +++--------------- 2 files changed, 7 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/org/openapitools/openapidiff/core/utils/FileUtils.java b/core/src/main/java/org/openapitools/openapidiff/core/utils/FileUtils.java index df2abd70d..a2c8b147e 100644 --- a/core/src/main/java/org/openapitools/openapidiff/core/utils/FileUtils.java +++ b/core/src/main/java/org/openapitools/openapidiff/core/utils/FileUtils.java @@ -3,11 +3,8 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStreamWriter; -import java.nio.file.Files; -import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; -import org.apache.commons.lang3.StringUtils; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.openapitools.openapidiff.core.output.Render; import org.slf4j.Logger; @@ -22,27 +19,17 @@ private FileUtils() { public static void writeToFile( final Render render, final ChangedOpenApi diff, final String fileName) { - if (StringUtils.isEmpty(fileName)) { + if (fileName == null || fileName.isEmpty()) { logger.debug("File name cannot be null or empty."); return; } - final Path filePath; - try { - filePath = Paths.get(fileName); - if (Files.exists(filePath)) { - logger.warn("File {} already exists. Skip writing to file.", fileName); - return; - } - } catch (final InvalidPathException invalidPathException) { - throw new IllegalArgumentException("File name is an invalid path."); - } - + final Path filePath = Paths.get(fileName); try (final FileOutputStream outputStream = new FileOutputStream(filePath.toFile()); final OutputStreamWriter outputStreamWriter = new OutputStreamWriter(outputStream)) { render.render(diff, outputStreamWriter); - } catch (final IOException ioException) { - logger.error("Exception while writing to file {}", fileName); + } catch (final IOException e) { + logger.error("Exception while writing to file {}", fileName, e); } } } diff --git a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java index f7927c80c..5d9b4d79b 100644 --- a/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java +++ b/core/src/test/java/org/openapitools/openapidiff/core/utils/FileUtilsTest.java @@ -14,13 +14,8 @@ import org.junit.jupiter.api.io.TempDir; import org.openapitools.openapidiff.core.model.ChangedOpenApi; import org.openapitools.openapidiff.core.output.ConsoleRender; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; class FileUtilsTest { - - private static final Logger logger = LoggerFactory.getLogger(FileUtilsTest.class); - private ChangedOpenApi changedOpenApi; @BeforeEach @@ -32,15 +27,6 @@ void setup() { changedOpenApi.setMissingEndpoints(Collections.emptyList()); } - @Test - void writeToFile_contentIsNull_doesNothing(@TempDir Path tempDir) throws IOException { - Path path = tempDir.resolve("output.txt"); - Files.write(path, "Test".getBytes(StandardCharsets.UTF_8)); - - assertDoesNotThrow(() -> FileUtils.writeToFile(null, changedOpenApi, path.toString())); - assertThat(path).exists().content().isEqualTo("Test"); - } - @Test void writeToFile_filenameIsNull_doesNothing() { assertDoesNotThrow(() -> FileUtils.writeToFile(new ConsoleRender(), changedOpenApi, null)); @@ -53,13 +39,13 @@ void writeToFile_filenameIsEmpty_doesNothing() { } @Test - void writeToFile_fileExists_doesNothing(@TempDir Path tempDir) throws IOException { + void writeToFile_fileExists_overwrites_file(@TempDir Path tempDir) throws IOException { final Path path = tempDir.resolve("output.txt"); Files.write(path, "Test".getBytes(StandardCharsets.UTF_8)); assertDoesNotThrow( () -> FileUtils.writeToFile(new ConsoleRender(), changedOpenApi, path.toString())); - assertThat(path).exists().content().isEqualTo("Test"); + assertThat(path).exists().content().isNotEqualTo("Test"); } @Test @@ -67,6 +53,6 @@ void writeToFile_fileDoesNotExist_createsFile(@TempDir Path tempDir) { final Path path = tempDir.resolve("output.txt"); assertDoesNotThrow( () -> FileUtils.writeToFile(new ConsoleRender(), changedOpenApi, path.toString())); - assertThat(path).exists(); + assertThat(path).exists().content().isNotBlank(); } }