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

@@ -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))

scm-core/src/main/java/sonia/scm/repository/api/DiffCommandBuilder.java

@@ -110,8 +110,12 @@ public final class DiffCommandBuilder extends AbstractDiffCommandBuilder<DiffCom
    * @throws IOException
    */
   public String getContent() throws IOException {
+    checkArguments();
+
+    logger.debug("create diff content for {}", request);
+
     try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
-      getDiffResult().accept(baos);
+      diffCommand.getDiffResultInternal(request).accept(baos);
       return baos.toString(StandardCharsets.UTF_8);
     }
   }
@@ -147,14 +151,18 @@ public final class DiffCommandBuilder extends AbstractDiffCommandBuilder<DiffCom
    * @return
    */
   private OutputStreamConsumer getDiffResult() throws IOException {
-    Preconditions.checkArgument(request.isValid(),
-      "path and/or revision is required");
+    checkArguments();
 
     logger.debug("create diff for {}", request);
 
     return diffCommand.getDiffResult(request);
   }
 
+  private void checkArguments() {
+    Preconditions.checkArgument(request.isValid(),
+      "path and/or revision is required");
+  }
+
   @Override
   DiffCommandBuilder self() {
     return this;

scm-core/src/main/java/sonia/scm/repository/spi/DiffCommand.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.repository.spi;
 
 import sonia.scm.repository.api.DiffCommandBuilder;
@@ -37,13 +37,25 @@ public interface DiffCommand
 {
 
   /**
-   * Method description
+   * Implementations of this method have to ensure, that all resources are released when the stream ends.
+   * This implementation is used for streaming results, where there are no more requests with the same
+   * context will be expected, whatsoever.
    *
-   *
-   * @param request
-   * @throws IOException
-   * @throws RuntimeException
-   * @return
+   * <b>If</b> 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);
+  }
 }

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())) {

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);
   }
 
 }