From 32dad5ffbfd131cb295f1f8885359bfe61e821be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Mon, 13 Jun 2022 08:19:28 +0200 Subject: [PATCH] Do not close hg context for diff as string (#2067) Normally, all resources (like the hg process) will be released after the stream of the diff. But this may lead to errors, when DiffCommandBuilder#getContent is used and other commands are used afterwards. So with this we introduce a new interface method in the DiffCommand that can be implemented without closing resources. This method is used for the computation of the diff as string. We use this to distinguish between the computation of diffs as a stream like in rest calls, and the computation as a single string result that may we followed by other commands using the same context. --- gradle/changelog/diff_hg_context.yaml | 2 ++ .../repository/api/DiffCommandBuilder.java | 14 +++++++-- .../sonia/scm/repository/spi/DiffCommand.java | 26 ++++++++++++----- .../scm/repository/spi/HgDiffCommand.java | 9 +++++- .../scm/repository/spi/HgDiffCommandTest.java | 29 ++++++++++++++++++- 5 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 gradle/changelog/diff_hg_context.yaml diff --git a/gradle/changelog/diff_hg_context.yaml b/gradle/changelog/diff_hg_context.yaml new file mode 100644 index 0000000000..f0d21458c4 --- /dev/null +++ b/gradle/changelog/diff_hg_context.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Do not close hg context for diff as string ([#2067](https://github.com/scm-manager/scm-manager/pull/2067)) diff --git a/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java index 0b9dffd16e..6418444689 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java @@ -110,8 +110,12 @@ public final class DiffCommandBuilder extends AbstractDiffCommandBuilderIf this closes resources that will prevent further commands from execution, you have to override + * {@link #getDiffResultInternal(DiffCommandRequest)} that will return the same as this functions, but + * must not release these resources so that subsequent commands will still function. */ DiffCommandBuilder.OutputStreamConsumer getDiffResult(DiffCommandRequest request) throws IOException; + + /** + * Override this if {@link #getDiffResult(DiffCommandRequest)} releases resources that will prevent other + * commands from execution, so that these resources are not released with this function. + * + * Defaults to a simple call to {@link #getDiffResult(DiffCommandRequest)}. + * + * @since 2.36.0 + */ + default DiffCommandBuilder.OutputStreamConsumer getDiffResultInternal(DiffCommandRequest request) throws IOException { + return getDiffResult(request); + } } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgDiffCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgDiffCommand.java index 03e6ed0d57..425e44bfce 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgDiffCommand.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgDiffCommand.java @@ -58,7 +58,14 @@ public class HgDiffCommand extends AbstractCommand implements DiffCommand { }; } - @SuppressWarnings("UnstableApiUsage") + @Override + public DiffCommandBuilder.OutputStreamConsumer getDiffResultInternal(DiffCommandRequest request) { + return output -> { + Repository hgRepo = open(); + diff(hgRepo, request, output); + }; + } + private void diff(Repository hgRepo, DiffCommandRequest request, OutputStream output) throws IOException { HgDiffInternalCommand cmd = createDiffCommand(hgRepo, request); try (InputStream inputStream = streamDiff(hgRepo, cmd, request.getPath())) { diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgDiffCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgDiffCommandTest.java index 3884ac1d30..7d68976bfb 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgDiffCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgDiffCommandTest.java @@ -31,7 +31,9 @@ import sonia.scm.repository.api.DiffFormat; import java.io.ByteArrayOutputStream; import java.io.IOException; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -53,6 +55,27 @@ public class HgDiffCommandTest extends AbstractHgCommandTestBase { assertThat(content).contains("git"); } + @Test + public void shouldCreateDiffInternal() throws IOException { + DiffCommandRequest request = new DiffCommandRequest(); + request.setRevision("3049df33fdbbded08b707bac3eccd0f7b453c58b"); + + String content = getStringFromDiffStream(new HgDiffCommand(cmdContext).getDiffResultInternal(request)); + assertThat(content) + .contains("+++ b/c/d.txt"); + } + + @Test + public void shouldNotCloseInternalStream() throws IOException { + HgCommandContext context = spy(cmdContext); + DiffCommandRequest request = new DiffCommandRequest(); + request.setRevision("3049df33fdbbded08b707bac3eccd0f7b453c58b"); + + new HgDiffCommand(cmdContext).getDiffResultInternal(request); + + verify(context, never()).close(); + } + @Test public void shouldCreateDiffComparedToAncestor() throws IOException { DiffCommandRequest request = new DiffCommandRequest(); @@ -92,9 +115,13 @@ public class HgDiffCommandTest extends AbstractHgCommandTestBase { private String diff(HgCommandContext context, DiffCommandRequest request) throws IOException { HgDiffCommand diff = new HgDiffCommand(context); DiffCommandBuilder.OutputStreamConsumer consumer = diff.getDiffResult(request); + return getStringFromDiffStream(consumer); + } + + private String getStringFromDiffStream(DiffCommandBuilder.OutputStreamConsumer consumer) throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); consumer.accept(baos); - return baos.toString("UTF-8"); + return baos.toString(UTF_8); } }