From 01a5dbd09167150a40a5e2d4d7c9f2ae6e3b6570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 10 Nov 2020 08:33:22 +0100 Subject: [PATCH] Show messages from native scm protocol --- .../java/sonia/scm/ExceptionWithContext.java | 34 +++++++++++ .../sonia/scm/api/v2/resources/ErrorDto.java | 13 +++- .../IntegrateChangesFromWorkdirException.java | 42 +++++++++++-- ...egrateChangesFromWorkdirExceptionTest.java | 59 +++++++++++++++++++ .../repository/spi/AbstractGitCommand.java | 3 +- .../scm/repository/spi/HgBranchCommand.java | 7 ++- .../scm/repository/spi/HgModifyCommand.java | 6 +- .../scm/repository/spi/SvnModifyCommand.java | 8 ++- .../src/BackendErrorNotification.tsx | 12 +++- scm-ui/ui-components/src/errors.ts | 7 +++ .../ExceptionWithContextToErrorDtoMapper.java | 10 ++-- .../main/resources/locales/de/plugins.json | 2 +- .../main/resources/locales/en/plugins.json | 2 +- ...eptionWithContextToErrorDtoMapperTest.java | 21 ++++++- 14 files changed, 202 insertions(+), 24 deletions(-) create mode 100644 scm-core/src/test/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirExceptionTest.java diff --git a/scm-core/src/main/java/sonia/scm/ExceptionWithContext.java b/scm-core/src/main/java/sonia/scm/ExceptionWithContext.java index 1631aea178..a972c4e794 100644 --- a/scm-core/src/main/java/sonia/scm/ExceptionWithContext.java +++ b/scm-core/src/main/java/sonia/scm/ExceptionWithContext.java @@ -24,6 +24,7 @@ package sonia.scm; +import java.io.Serializable; import java.util.List; import java.util.Optional; @@ -34,15 +35,26 @@ public abstract class ExceptionWithContext extends RuntimeException { private static final long serialVersionUID = 4327413456580409224L; private final List context; + private final List additionalMessages; public ExceptionWithContext(List context, String message) { + this(context, null, message); + } + + public ExceptionWithContext(List context, List additionalMessages, String message) { super(message); this.context = context; + this.additionalMessages = additionalMessages; } public ExceptionWithContext(List context, String message, Exception cause) { + this(context, null, message, cause); + } + + public ExceptionWithContext(List context, List additionalMessages, String message, Exception cause) { super(message, cause); this.context = context; + this.additionalMessages = additionalMessages; } public List getContext() { @@ -61,4 +73,26 @@ public abstract class ExceptionWithContext extends RuntimeException { public Optional getUrl() { return Optional.empty(); } + + public List getAdditionalMessages() { + return additionalMessages; + } + + public static class AdditionalMessage implements Serializable { + private final String key; + private final String message; + + public AdditionalMessage(String key, String message) { + this.key = key; + this.message = message; + } + + public String getKey() { + return key; + } + + public String getMessage() { + return message; + } + } } diff --git a/scm-core/src/main/java/sonia/scm/api/v2/resources/ErrorDto.java b/scm-core/src/main/java/sonia/scm/api/v2/resources/ErrorDto.java index 5199446a4f..737d4b1fc2 100644 --- a/scm-core/src/main/java/sonia/scm/api/v2/resources/ErrorDto.java +++ b/scm-core/src/main/java/sonia/scm/api/v2/resources/ErrorDto.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.api.v2.resources; import com.fasterxml.jackson.annotation.JsonInclude; @@ -40,7 +40,10 @@ public class ErrorDto { private List context; private String message; - @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonInclude(JsonInclude.Include.NON_EMPTY) + private List additionalMessages; + + @JsonInclude(JsonInclude.Include.NON_EMPTY) @XmlElementWrapper(name = "violations") private List violations; @@ -53,4 +56,10 @@ public class ErrorDto { private String path; private String message; } + + @Getter @Setter + public static class AdditionalMessageDto { + private String key; + private String message; + } } diff --git a/scm-core/src/main/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirException.java b/scm-core/src/main/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirException.java index 3259cc2094..d0dcb28e65 100644 --- a/scm-core/src/main/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirException.java +++ b/scm-core/src/main/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirException.java @@ -21,27 +21,57 @@ * 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.ContextEntry; import sonia.scm.ExceptionWithContext; import sonia.scm.repository.Repository; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +import static java.util.Arrays.stream; +import static sonia.scm.ContextEntry.ContextBuilder.entity; + public class IntegrateChangesFromWorkdirException extends ExceptionWithContext { private static final String CODE = "CHRM7IQzo1"; - public IntegrateChangesFromWorkdirException(Repository repository, String message) { - super(ContextEntry.ContextBuilder.entity(repository).build(), message); + private static final Pattern SCM_MESSAGE_PATTERN = Pattern.compile(".*\\[SCM\\] (.*)"); + + public static MessageExtractor withPattern(Pattern pattern) { + return new MessageExtractor(pattern); } - public IntegrateChangesFromWorkdirException(Repository repository, String message, Exception cause) { - super(ContextEntry.ContextBuilder.entity(repository).build(), message, cause); + public static IntegrateChangesFromWorkdirException forMessage(Repository repository, String message) { + return new MessageExtractor(SCM_MESSAGE_PATTERN).forMessage(repository, message); + } + + private IntegrateChangesFromWorkdirException(Repository repository, List additionalMessages) { + super(entity(repository).build(), additionalMessages, "errors from hook"); } @Override public String getCode() { return CODE; } + + public static class MessageExtractor { + + private final Pattern extractorPattern; + + public MessageExtractor(Pattern extractorPattern) { + this.extractorPattern = extractorPattern; + } + + public IntegrateChangesFromWorkdirException forMessage(Repository repository, String message) { + return new IntegrateChangesFromWorkdirException(repository, stream(message.split("\\n")) + .map(extractorPattern::matcher) + .filter(Matcher::matches) + .map(matcher -> new AdditionalMessage(null, matcher.group(1))) + .collect(Collectors.toList())); + } + } } diff --git a/scm-core/src/test/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirExceptionTest.java b/scm-core/src/test/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirExceptionTest.java new file mode 100644 index 0000000000..6a4580361e --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirExceptionTest.java @@ -0,0 +1,59 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.repository.spi; + +import org.junit.jupiter.api.Test; +import sonia.scm.repository.Repository; + +import java.util.regex.Pattern; + +import static org.assertj.core.api.Assertions.assertThat; +import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.forMessage; +import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.withPattern; + +class IntegrateChangesFromWorkdirExceptionTest { + + private static final Repository REPOSITORY = new Repository("1", "git", "hitchhiker", "hog"); + + @Test + void shouldExtractMessagesWithDefaultPrefix() { + IntegrateChangesFromWorkdirException exception = forMessage(REPOSITORY, "prefix [SCM] line 1\nprefix [SCM] line 2\nirrelevant line\n"); + + assertThat(exception.getAdditionalMessages()) + .extracting("message") + .containsExactly("line 1", "line 2"); + } + + @Test + void shouldExtractMessagesWithCustomPattern() { + IntegrateChangesFromWorkdirException exception = + withPattern(Pattern.compile("-custom- (.*)")) + .forMessage(REPOSITORY, "to be ignored\n-custom- line\n"); + + assertThat(exception.getAdditionalMessages()) + .extracting("message") + .containsExactly("line"); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java index c083123242..8ad6e3ddf4 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java @@ -59,6 +59,7 @@ import static java.util.Optional.of; import static sonia.scm.ContextEntry.ContextBuilder.entity; import static sonia.scm.NotFoundException.notFound; import static sonia.scm.repository.GitUtil.getBranchIdOrCurrentHead; +import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.forMessage; //~--- JDK imports ------------------------------------------------------------ @@ -255,7 +256,7 @@ class AbstractGitCommand { .findAny() .ifPresent(remoteRefUpdate -> { logger.info("message for failed push: {}", pushResult.getMessages()); - throw new IntegrateChangesFromWorkdirException(repository, "could not push changes into central repository: " + remoteRefUpdate.getStatus()); + throw forMessage(repository, pushResult.getMessages()); }); } catch (GitAPIException e) { throw new InternalRepositoryException(repository, "could not push changes into central repository", e); diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBranchCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBranchCommand.java index 3a291b4501..7319a312ff 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBranchCommand.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBranchCommand.java @@ -37,6 +37,8 @@ import sonia.scm.repository.api.BranchRequest; import sonia.scm.repository.work.WorkingCopy; import sonia.scm.user.User; +import java.io.IOException; + /** * Mercurial implementation of the {@link BranchCommand}. * Note that this creates an empty commit to "persist" the new branch. @@ -106,9 +108,8 @@ public class HgBranchCommand extends AbstractCommand implements BranchCommand { PullCommand pullCommand = PullCommand.on(workingCopy.getCentralRepository()); workingCopyFactory.configure(pullCommand); pullCommand.execute(workingCopy.getDirectory().getAbsolutePath()); - } catch (Exception e) { - // TODO handle failed update - throw new IntegrateChangesFromWorkdirException(getRepository(), + } catch (IOException e) { + throw new InternalRepositoryException(getRepository(), String.format("Could not pull changes '%s' into central repository", branch), e); } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java index 012592c7a4..4b1ce27d38 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java @@ -114,8 +114,10 @@ public class HgModifyCommand implements ModifyCommand { com.aragost.javahg.commands.PullCommand pullCommand = PullCommand.on(workingCopy.getCentralRepository()); workingCopyFactory.configure(pullCommand); return pullCommand.execute(workingCopy.getDirectory().getAbsolutePath()); - } catch (Exception e) { - throw new IntegrateChangesFromWorkdirException(context.getScmRepository(), + } catch (ExecutionException e) { + throw IntegrateChangesFromWorkdirException.forMessage(context.getScmRepository(), e.getMessage()); + } catch (IOException e) { + throw new InternalRepositoryException(context.getScmRepository(), String.format("Could not pull modify changes from working copy to central repository for branch %s", request.getBranch()), e); } diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java index 849d5619a5..756170453a 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnModifyCommand.java @@ -27,6 +27,7 @@ package sonia.scm.repository.spi; import org.apache.shiro.SecurityUtils; import org.tmatesoft.svn.core.SVNCommitInfo; import org.tmatesoft.svn.core.SVNDepth; +import org.tmatesoft.svn.core.SVNErrorCode; import org.tmatesoft.svn.core.SVNException; import org.tmatesoft.svn.core.wc.SVNClientManager; import org.tmatesoft.svn.core.wc.SVNWCClient; @@ -40,9 +41,14 @@ import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.regex.Pattern; + +import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.withPattern; public class SvnModifyCommand implements ModifyCommand { + public static final Pattern SVN_ERROR_PATTERN = Pattern.compile(".*E" + SVNErrorCode.CANCELLED.getCode() + ": (.*)"); + private final SvnContext context; private final SvnWorkingCopyFactory workingCopyFactory; private final Repository repository; @@ -81,7 +87,7 @@ public class SvnModifyCommand implements ModifyCommand { ); return String.valueOf(svnCommitInfo.getNewRevision()); } catch (SVNException e) { - throw new InternalRepositoryException(repository, "could not commit changes on repository"); + throw withPattern(SVN_ERROR_PATTERN).forMessage(repository, e.getMessage()); } } diff --git a/scm-ui/ui-components/src/BackendErrorNotification.tsx b/scm-ui/ui-components/src/BackendErrorNotification.tsx index b61e25d777..01386a917a 100644 --- a/scm-ui/ui-components/src/BackendErrorNotification.tsx +++ b/scm-ui/ui-components/src/BackendErrorNotification.tsx @@ -42,6 +42,7 @@ class BackendErrorNotification extends React.Component {

{this.renderErrorName()}

{this.renderErrorDescription()}

+ {this.renderAdditionalMessages()}

{this.renderViolations()}

{this.renderMetadata()}
@@ -51,7 +52,7 @@ class BackendErrorNotification extends React.Component { renderErrorName = () => { const { error, t } = this.props; - const translation = t("errors." + error.errorCode + ".displayName"); + const translation = t(`errors.${error.errorCode}.displayName`); if (translation === error.errorCode) { return error.message; } @@ -60,13 +61,20 @@ class BackendErrorNotification extends React.Component { renderErrorDescription = () => { const { error, t } = this.props; - const translation = t("errors." + error.errorCode + ".description"); + const translation = t(`errors.${error.errorCode}.description`); if (translation === error.errorCode) { return ""; } return translation; }; + renderAdditionalMessages = () => { + const { error, t } = this.props; + if (error.additionalMessages) { + return error.additionalMessages.map(a => a.key ? t(`errors.${a.key}.description`) : a.message).map(m =>

{m}

); + } + }; + renderViolations = () => { const { error, t } = this.props; if (error.violations) { diff --git a/scm-ui/ui-components/src/errors.ts b/scm-ui/ui-components/src/errors.ts index d1f7aef226..41e44f0702 100644 --- a/scm-ui/ui-components/src/errors.ts +++ b/scm-ui/ui-components/src/errors.ts @@ -31,6 +31,10 @@ export type Violation = { message: string; key?: string; }; +export type AdditionalMessage = { + key?: string; + message?: string; +}; export type BackendErrorContent = { transactionId: string; @@ -39,6 +43,7 @@ export type BackendErrorContent = { url?: string; context: Context; violations: Violation[]; + additionalMessages?: AdditionalMessage[]; }; export class BackendError extends Error { @@ -48,6 +53,7 @@ export class BackendError extends Error { context: Context = []; statusCode: number; violations: Violation[]; + additionalMessages?: AdditionalMessage[]; constructor(content: BackendErrorContent, name: string, statusCode: number) { super(content.message); @@ -58,6 +64,7 @@ export class BackendError extends Error { this.context = content.context; this.statusCode = statusCode; this.violations = content.violations; + this.additionalMessages = content.additionalMessages; } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ExceptionWithContextToErrorDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ExceptionWithContextToErrorDtoMapper.java index bdf14e5bd5..767a37522a 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ExceptionWithContextToErrorDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ExceptionWithContextToErrorDtoMapper.java @@ -34,20 +34,22 @@ import sonia.scm.ExceptionWithContext; import java.util.Optional; @Mapper -public abstract class ExceptionWithContextToErrorDtoMapper { +public interface ExceptionWithContextToErrorDtoMapper { @Mapping(target = "errorCode", source = "code") @Mapping(target = "transactionId", ignore = true) @Mapping(target = "violations", ignore = true) - public abstract ErrorDto map(ExceptionWithContext exception); + ErrorDto map(ExceptionWithContext exception); @SuppressWarnings("OptionalUsedAsFieldOrParameterType") // is ok for mapping - public String mapOptional(Optional optionalString) { + default String mapOptional(Optional optionalString) { return optionalString.orElse(null); } @AfterMapping - void setTransactionId(@MappingTarget ErrorDto dto) { + default void setTransactionId(@MappingTarget ErrorDto dto) { dto.setTransactionId(MDC.get("transaction_id")); } + + ErrorDto.AdditionalMessageDto map(ExceptionWithContext.AdditionalMessage message); } diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index cf2c29f21f..2226986629 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -197,7 +197,7 @@ }, "CHRM7IQzo1": { "displayName": "Änderung des Repositories nicht möglich", - "description": "Die gewünschte Änderung am Repository konnte nicht durchgeführt werden. Höchst wahrscheinlich liegt dieses an installierten Plugins mit Hooks oder nativen Hooks." + "description": "Die gewünschte Änderung am Repository konnte nicht durchgeführt werden. Höchst wahrscheinlich liegt dieses an installierten Plugins mit Hooks oder nativen Hooks. Folgend sind eventuelle weitere Meldungen." }, "thbsUFokjk": { "displayName": "Unerlaubte Änderung eines Schlüsselwerts", diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index 4de2202976..246f667e7b 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -197,7 +197,7 @@ }, "CHRM7IQzo1": { "displayName": "Could not modify repository", - "description": "The requested modification to the repository was rejected. Most probably this was due to plugins with repository hooks or native hooks." + "description": "The requested modification to the repository was rejected. Most probably this was due to plugins with repository hooks or native hooks. Following are potential additional messages." }, "thbsUFokjk": { "displayName": "Illegal change of an identifier", diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ExceptionWithContextToErrorDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ExceptionWithContextToErrorDtoMapperTest.java index 649493b7e8..e7d764f0ae 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ExceptionWithContextToErrorDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ExceptionWithContextToErrorDtoMapperTest.java @@ -31,6 +31,7 @@ import sonia.scm.ExceptionWithContext; import java.util.Optional; +import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; class ExceptionWithContextToErrorDtoMapperTest { @@ -51,9 +52,27 @@ class ExceptionWithContextToErrorDtoMapperTest { assertThat(dto.getUrl()).isNull(); } + @Test + void shouldMapAdditionalMessages() { + ExceptionWithUrl exception = new ExceptionWithUrl(); + ErrorDto dto = mapper.map(exception); + assertThat(dto.getAdditionalMessages()) + .extracting("message") + .containsExactly("line 1", "line 2", null); + assertThat(dto.getAdditionalMessages()) + .extracting("key") + .containsExactly(null, null, "KEY"); + } + private static class ExceptionWithUrl extends ExceptionWithContext { public ExceptionWithUrl() { - super(ContextEntry.ContextBuilder.noContext(), "With Url"); + super( + ContextEntry.ContextBuilder.noContext(), + asList( + new AdditionalMessage(null, "line 1"), + new AdditionalMessage(null, "line 2"), + new AdditionalMessage("KEY", null)), + "With Url"); } @Override