From 8d797a454a22b308aab3483ede20fa2e54deb294 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 10 Nov 2020 08:29:19 +0100 Subject: [PATCH 1/8] Fix typings --- .../java/sonia/scm/ConcurrentModificationException.java | 6 +++--- scm-core/src/main/java/sonia/scm/ContextEntry.java | 9 +++++---- scm-core/src/main/java/sonia/scm/NotFoundException.java | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/ConcurrentModificationException.java b/scm-core/src/main/java/sonia/scm/ConcurrentModificationException.java index 694120dc2b..93c84e01cc 100644 --- a/scm-core/src/main/java/sonia/scm/ConcurrentModificationException.java +++ b/scm-core/src/main/java/sonia/scm/ConcurrentModificationException.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; import java.util.Collections; @@ -33,7 +33,7 @@ public class ConcurrentModificationException extends ExceptionWithContext { private static final String CODE = "2wR7UzpPG1"; - public ConcurrentModificationException(Class type, String id) { + public ConcurrentModificationException(Class type, String id) { this(Collections.singletonList(new ContextEntry(type, id))); } @@ -56,4 +56,4 @@ public class ConcurrentModificationException extends ExceptionWithContext { .collect(joining(" in ", "", " has been modified concurrently")); } } - + diff --git a/scm-core/src/main/java/sonia/scm/ContextEntry.java b/scm-core/src/main/java/sonia/scm/ContextEntry.java index d398e54609..15217e3173 100644 --- a/scm-core/src/main/java/sonia/scm/ContextEntry.java +++ b/scm-core/src/main/java/sonia/scm/ContextEntry.java @@ -21,22 +21,23 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.Repository; import sonia.scm.util.AssertUtil; +import java.io.Serializable; import java.util.Collections; import java.util.LinkedList; import java.util.List; -public class ContextEntry { +public class ContextEntry implements Serializable { private final String type; private final String id; - ContextEntry(Class type, String id) { + ContextEntry(Class type, String id) { this(type.getSimpleName(), id); } @@ -91,7 +92,7 @@ public class ContextEntry { return this.in(Repository.class, namespaceAndName.logString()); } - public ContextBuilder in(Class type, String id) { + public ContextBuilder in(Class type, String id) { context.add(new ContextEntry(type, id)); return this; } diff --git a/scm-core/src/main/java/sonia/scm/NotFoundException.java b/scm-core/src/main/java/sonia/scm/NotFoundException.java index 2ca74fc60c..0c0a06926c 100644 --- a/scm-core/src/main/java/sonia/scm/NotFoundException.java +++ b/scm-core/src/main/java/sonia/scm/NotFoundException.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; import java.util.Collections; @@ -35,7 +35,7 @@ public class NotFoundException extends ExceptionWithContext { private static final String CODE = "AGR7UzkhA1"; - public NotFoundException(Class type, String id) { + public NotFoundException(Class type, String id) { this(Collections.singletonList(new ContextEntry(type, id))); } From b2533db6841f9843447bf73948180238046cd775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Tue, 10 Nov 2020 08:32:14 +0100 Subject: [PATCH 2/8] Simplify mapper --- ...ScmViolationExceptionToErrorDtoMapper.java | 33 +++++-------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ScmViolationExceptionToErrorDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ScmViolationExceptionToErrorDtoMapper.java index edf685ce87..0f9f267fb5 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ScmViolationExceptionToErrorDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ScmViolationExceptionToErrorDtoMapper.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 org.mapstruct.AfterMapping; @@ -32,46 +32,29 @@ import org.slf4j.MDC; import sonia.scm.ScmConstraintViolationException; import sonia.scm.ScmConstraintViolationException.ScmConstraintViolation; -import java.util.List; -import java.util.stream.Collectors; - @Mapper -public abstract class ScmViolationExceptionToErrorDtoMapper { +public interface ScmViolationExceptionToErrorDtoMapper { @Mapping(target = "errorCode", ignore = true) @Mapping(target = "transactionId", ignore = true) @Mapping(target = "context", ignore = true) - public abstract ErrorDto map(ScmConstraintViolationException exception); + ErrorDto map(ScmConstraintViolationException exception); @AfterMapping - void setTransactionId(@MappingTarget ErrorDto dto) { + default void setTransactionId(@MappingTarget ErrorDto dto) { dto.setTransactionId(MDC.get("transaction_id")); } - @AfterMapping - void mapViolations(ScmConstraintViolationException exception, @MappingTarget ErrorDto dto) { - List violations = - exception.getViolations() - .stream() - .map(this::createViolationDto) - .collect(Collectors.toList()); - dto.setViolations(violations); - } - - private ErrorDto.ConstraintViolationDto createViolationDto(ScmConstraintViolation violation) { - ErrorDto.ConstraintViolationDto constraintViolationDto = new ErrorDto.ConstraintViolationDto(); - constraintViolationDto.setMessage(violation.getMessage()); - constraintViolationDto.setPath(violation.getPropertyPath()); - return constraintViolationDto; - } + @Mapping(source = "propertyPath", target = "path") + ErrorDto.ConstraintViolationDto map(ScmConstraintViolation violation); @AfterMapping - void setErrorCode(@MappingTarget ErrorDto dto) { + default void setErrorCode(@MappingTarget ErrorDto dto) { dto.setErrorCode("3zR9vPNIE1"); } @AfterMapping - void setMessage(@MappingTarget ErrorDto dto) { + default void setMessage(@MappingTarget ErrorDto dto) { dto.setMessage("input violates conditions (see violation list)"); } } 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 3/8] 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 From 5b44be5e5bf8df2c095fa86bca6e4d328599811c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 11 Nov 2020 11:04:14 +0100 Subject: [PATCH 4/8] Show error message once only --- .../main/java/sonia/scm/repository/spi/SvnModifyCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 756170453a..326b710874 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 @@ -87,7 +87,7 @@ public class SvnModifyCommand implements ModifyCommand { ); return String.valueOf(svnCommitInfo.getNewRevision()); } catch (SVNException e) { - throw withPattern(SVN_ERROR_PATTERN).forMessage(repository, e.getMessage()); + throw withPattern(SVN_ERROR_PATTERN).forMessage(repository, e.getErrorMessage().getRootErrorMessage().getFullMessage()); } } From 8cfbc60feac36daa939e8598f645946f0dfdaa56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 11 Nov 2020 11:37:24 +0100 Subject: [PATCH 5/8] Distinguish between errors with and without details --- .../IntegrateChangesFromWorkdirException.java | 5 +++-- ...IntegrateChangesFromWorkdirExceptionTest.java | 16 +++++++++++++++- .../src/BackendErrorNotification.tsx | 12 +++++++++++- .../src/main/resources/locales/de/plugins.json | 6 +++++- .../src/main/resources/locales/en/plugins.json | 10 +++++++--- 5 files changed, 41 insertions(+), 8 deletions(-) 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 d0dcb28e65..f8dc0ba2d0 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 @@ -37,7 +37,8 @@ import static sonia.scm.ContextEntry.ContextBuilder.entity; public class IntegrateChangesFromWorkdirException extends ExceptionWithContext { - private static final String CODE = "CHRM7IQzo1"; + static final String CODE_WITH_ADDITIONAL_MESSAGES = "CHRM7IQzo1"; + static final String CODE_WITHOUT_ADDITIONAL_MESSAGES = "ASSG1ehZ01"; private static final Pattern SCM_MESSAGE_PATTERN = Pattern.compile(".*\\[SCM\\] (.*)"); @@ -55,7 +56,7 @@ public class IntegrateChangesFromWorkdirException extends ExceptionWithContext { @Override public String getCode() { - return CODE; + return getAdditionalMessages().isEmpty()? CODE_WITHOUT_ADDITIONAL_MESSAGES : CODE_WITH_ADDITIONAL_MESSAGES; } public static class MessageExtractor { 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 index 6a4580361e..d990a64ef7 100644 --- a/scm-core/src/test/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirExceptionTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/spi/IntegrateChangesFromWorkdirExceptionTest.java @@ -30,6 +30,8 @@ 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.CODE_WITHOUT_ADDITIONAL_MESSAGES; +import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.CODE_WITH_ADDITIONAL_MESSAGES; import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.forMessage; import static sonia.scm.repository.spi.IntegrateChangesFromWorkdirException.withPattern; @@ -39,11 +41,13 @@ class IntegrateChangesFromWorkdirExceptionTest { @Test void shouldExtractMessagesWithDefaultPrefix() { - IntegrateChangesFromWorkdirException exception = forMessage(REPOSITORY, "prefix [SCM] line 1\nprefix [SCM] line 2\nirrelevant line\n"); + 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"); + assertThat(exception.getCode()).isEqualTo(CODE_WITH_ADDITIONAL_MESSAGES); } @Test @@ -55,5 +59,15 @@ class IntegrateChangesFromWorkdirExceptionTest { assertThat(exception.getAdditionalMessages()) .extracting("message") .containsExactly("line"); + assertThat(exception.getCode()).isEqualTo(CODE_WITH_ADDITIONAL_MESSAGES); + } + + @Test + void shouldCreateSpecialMessageForMissingAdditionalMessages() { + IntegrateChangesFromWorkdirException exception = + forMessage(REPOSITORY, "There is no message"); + + assertThat(exception.getAdditionalMessages()).isEmpty(); + assertThat(exception.getCode()).isEqualTo(CODE_WITHOUT_ADDITIONAL_MESSAGES); } } diff --git a/scm-ui/ui-components/src/BackendErrorNotification.tsx b/scm-ui/ui-components/src/BackendErrorNotification.tsx index 01386a917a..5b0aac0d9b 100644 --- a/scm-ui/ui-components/src/BackendErrorNotification.tsx +++ b/scm-ui/ui-components/src/BackendErrorNotification.tsx @@ -71,7 +71,17 @@ class BackendErrorNotification extends React.Component { 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}

); + return ( + <> +
+ {error.additionalMessages + .map(a => (a.key ? t(`errors.${a.key}.description`) : a.message)) + .map(m => ( +

{m}

+ ))} +
+ + ); } }; diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index 2226986629..300fa3d17a 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -197,7 +197,11 @@ }, "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. Folgend sind eventuelle weitere Meldungen." + "description": "Die gewünschte Änderung am Repository konnte nicht durchgeführt werden. Höchst wahrscheinlich liegt dieses an Prüfungen von Plugins. Bitte prüfen Sie die Einstellungen. Im Folgenden finden Sie weitere Meldungen zu dem Fehler:" + }, + "ASSG1ehZ01": { + "displayName": "Änderung des Repositories nicht möglich", + "description": "Die gewünschte Änderung am Repository konnte nicht durchgeführt werden. Es gab keine weiteren 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 246f667e7b..8d6086ac19 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -197,7 +197,11 @@ }, "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. Following are potential additional messages." + "description": "The requested modification to the repository was rejected. The most likely reason for this are checks from plugins. Please check your settings. See the following messages for more details:" + }, + "ASSG1ehZ01": { + "displayName": "Could not modify repository", + "description": "The requested modification to the repository was rejected. There were no more messages." }, "thbsUFokjk": { "displayName": "Illegal change of an identifier", @@ -205,7 +209,7 @@ }, "40RaYIeeR1": { "displayName": "No changes were made", - "description": "No changes were made to the files of the repository. Therefor no new commit could be created. Possibly changes cannot be applied due to an .ignore-File definition." + "description": "No changes were made to the files of the repository. Therefore no new commit could be created. Possibly changes cannot be applied due to an .ignore-File definition." }, "ERS2vYb7U1": { "displayName": "Illegal change of namespace", @@ -213,7 +217,7 @@ }, "4iRct4avG1": { "displayName": "The revisions have unrelated histories", - "description": "The revisions have unrelated histories. Therefor there is no common commit to compare with." + "description": "The revisions have unrelated histories. Therefore there is no common commit to compare with." }, "65RdZ5atX1": { "displayName": "Error removing plugin files", From a479fcbfe13cc62538efaef5b5571e4a4d100395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 11 Nov 2020 13:50:55 +0100 Subject: [PATCH 6/8] Log change --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a74919a7e..59fddd12c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Added - Lookup command which provides further repository information ([#1415](https://github.com/scm-manager/scm-manager/pull/1415)) +- Include messages from scm protocol in modification or merge errors ([#1420](https://github.com/scm-manager/scm-manager/pull/1420)) ### Fixed - Error on repository initialization with least-privilege user ([#1414](https://github.com/scm-manager/scm-manager/pull/1414)) From f8219305a3cc42fb6b7191eebbe7c3e98cb4b7c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 12 Nov 2020 13:46:36 +0100 Subject: [PATCH 7/8] Strip 'Error:' from hg error messages --- .../sonia/scm/repository/spi/HgModifyCommand.java | 7 ++++++- .../scm/repository/spi/HgModifyCommandTest.java | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) 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 4b1ce27d38..302bea6d4f 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 @@ -39,9 +39,12 @@ import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.util.List; +import java.util.regex.Pattern; public class HgModifyCommand implements ModifyCommand { + static final Pattern HG_MESSAGE_PATTERN = Pattern.compile(".*\\[SCM\\](?: Error:)? (.*)"); + private HgCommandContext context; private final HgWorkingCopyFactory workingCopyFactory; @@ -115,7 +118,9 @@ public class HgModifyCommand implements ModifyCommand { workingCopyFactory.configure(pullCommand); return pullCommand.execute(workingCopy.getDirectory().getAbsolutePath()); } catch (ExecutionException e) { - throw IntegrateChangesFromWorkdirException.forMessage(context.getScmRepository(), e.getMessage()); + throw IntegrateChangesFromWorkdirException + .withPattern(HG_MESSAGE_PATTERN) + .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()), diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java index d55005f675..a2323f69e7 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgModifyCommandTest.java @@ -42,6 +42,7 @@ import sonia.scm.web.HgRepositoryEnvironmentBuilder; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.util.regex.Matcher; import static org.assertj.core.api.Assertions.assertThat; @@ -186,4 +187,18 @@ public class HgModifyCommandTest extends AbstractHgCommandTestBase { public void shouldThrowNoChangesMadeExceptionIfEmptyLocalChangesetAfterRequest() { hgModifyCommand.execute(new ModifyCommandRequest()); } + + @Test + public void shouldExtractSimpleMessage() { + Matcher matcher = HgModifyCommand.HG_MESSAGE_PATTERN.matcher("[SCM] This is a simple message"); + matcher.matches(); + assertThat(matcher.group(1)).isEqualTo("This is a simple message"); + } + + @Test + public void shouldExtractErrorMessage() { + Matcher matcher = HgModifyCommand.HG_MESSAGE_PATTERN.matcher("[SCM] Error: This is an error message"); + matcher.matches(); + assertThat(matcher.group(1)).isEqualTo("This is an error message"); + } } From 08a2cca06da588fedee9815f0aac0987cff7c06a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 12 Nov 2020 13:55:01 +0100 Subject: [PATCH 8/8] Heed review comments --- .../src/main/java/sonia/scm/ExceptionWithContext.java | 8 ++++---- scm-ui/ui-components/src/BackendErrorNotification.tsx | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/ExceptionWithContext.java b/scm-core/src/main/java/sonia/scm/ExceptionWithContext.java index a972c4e794..16c0f0218c 100644 --- a/scm-core/src/main/java/sonia/scm/ExceptionWithContext.java +++ b/scm-core/src/main/java/sonia/scm/ExceptionWithContext.java @@ -37,21 +37,21 @@ public abstract class ExceptionWithContext extends RuntimeException { private final List context; private final List additionalMessages; - public ExceptionWithContext(List context, String message) { + protected ExceptionWithContext(List context, String message) { this(context, null, message); } - public ExceptionWithContext(List context, List additionalMessages, String message) { + protected ExceptionWithContext(List context, List additionalMessages, String message) { super(message); this.context = context; this.additionalMessages = additionalMessages; } - public ExceptionWithContext(List context, String message, Exception cause) { + protected ExceptionWithContext(List context, String message, Exception cause) { this(context, null, message, cause); } - public ExceptionWithContext(List context, List additionalMessages, String message, Exception cause) { + protected ExceptionWithContext(List context, List additionalMessages, String message, Exception cause) { super(message, cause); this.context = context; this.additionalMessages = additionalMessages; diff --git a/scm-ui/ui-components/src/BackendErrorNotification.tsx b/scm-ui/ui-components/src/BackendErrorNotification.tsx index 5b0aac0d9b..a680304e73 100644 --- a/scm-ui/ui-components/src/BackendErrorNotification.tsx +++ b/scm-ui/ui-components/src/BackendErrorNotification.tsx @@ -75,9 +75,11 @@ class BackendErrorNotification extends React.Component { <>
{error.additionalMessages - .map(a => (a.key ? t(`errors.${a.key}.description`) : a.message)) - .map(m => ( -

{m}

+ .map(additionalMessage => + additionalMessage.key ? t(`errors.${additionalMessage.key}.description`) : additionalMessage.message + ) + .map(message => ( +

{message}

))}