Decouple extension point from dto

scm-core/src/main/java/sonia/scm/repository/ChangesetTrailers.java

@@ -31,5 +31,5 @@ import java.util.List;
 
 @ExtensionPoint
 public interface ChangesetTrailers {
-  List<TrailerPersonDto> getTrailers(Repository repository, Changeset changeset);
+  List<Trailer> getTrailers(Repository repository, Changeset changeset);
 }

scm-core/src/main/java/sonia/scm/repository/Trailer.java

@@ -0,0 +1,34 @@
+/*
+ * 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;
+
+import lombok.Value;
+
+@Value
+public class Trailer {
+  private String trailerType;
+  private String mail;
+  private String name;
+}

scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailers.java

@@ -24,59 +24,64 @@
 
 package sonia.scm.api.v2.resources;
 
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import sonia.scm.plugin.Extension;
 import sonia.scm.repository.Changeset;
 import sonia.scm.repository.ChangesetTrailers;
 import sonia.scm.repository.Repository;
+import sonia.scm.repository.Trailer;
 
 import javax.inject.Inject;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
+import java.util.Optional;
 import java.util.Scanner;
+import java.util.regex.MatchResult;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static java.util.Optional.empty;
+import static java.util.Optional.of;
+
 @Extension
 public class ChangesetDescriptionTrailers implements ChangesetTrailers {
 
-  private static final List<String> types = ImmutableList.of("Co-authored-by", "Reviewed-by", "Signed-off-by", "Committed-by");
+  private static final Collection<String> SUPPORTED_TRAILER_TYPES = ImmutableSet.of("Co-authored-by", "Reviewed-by", "Signed-off-by", "Committed-by");
+  private static final Pattern PERSON_PATTERN = Pattern.compile("^\\W*(.*)\\W+<(.*)>\\W*$");
 
   @Inject
   public ChangesetDescriptionTrailers() {}
 
   @Override
-  public List<TrailerPersonDto> getTrailers(Repository repository, Changeset changeset) {
+  public List<Trailer> getTrailers(Repository repository, Changeset changeset) {
-    List<TrailerPersonDto> persons = new ArrayList<>();
+    List<Trailer> trailers = new ArrayList<>();
 
     try (Scanner scanner = new Scanner(changeset.getDescription())) {
-      scanner.useDelimiter(Pattern.compile("[\\n]"));
+      while (scanner.hasNextLine()) {
-      while (scanner.hasNext()) {
+        String line = scanner.nextLine();
-        String line = scanner.next();
 
-        for (String trailerType : types) {
+        String[] typeAndUser = line.split(":\\W");
-          if (line.contains(trailerType)) {
+        if (typeAndUser.length == 2) {
-            TrailerPersonDto personDto = createPersonDtoFromUser(line);
+          String type = typeAndUser[0];
-            personDto.setTrailerType(trailerType);
+          String person = typeAndUser[1];
-            persons.add(personDto);
+          if (SUPPORTED_TRAILER_TYPES.contains(type)) {
+          Optional<Trailer> trailer = createTrailer(type, person);
+          trailer.ifPresent(trailers::add);
           }
         }
       }
     }
-    return persons;
+    return trailers;
   }
 
-  private TrailerPersonDto createPersonDtoFromUser(String line) {
+  private Optional<Trailer> createTrailer(String type, String person) {
-    TrailerPersonDto personDto = new TrailerPersonDto();
+    Matcher matcher = PERSON_PATTERN.matcher(person.trim());
-
+    if (matcher.matches()) {
-    String[] splittedTrailer = line.split("[:<>]");
+      MatchResult matchResult = matcher.toMatchResult();
-
+      return of(new Trailer(type, matchResult.group(2), matchResult.group(1)));
-    if (splittedTrailer.length > 1) {
+    } else {
-      personDto.setName(splittedTrailer[1].trim());
+      return empty();
-      if (splittedTrailer.length > 2) {
-        personDto.setMail(splittedTrailer[2]);
     }
   }
-
-    return personDto;
-  }
 }

scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java

@@ -37,6 +37,7 @@ import sonia.scm.repository.Changeset;
 import sonia.scm.repository.ChangesetTrailers;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.Tag;
+import sonia.scm.repository.Trailer;
 import sonia.scm.repository.api.Command;
 import sonia.scm.repository.api.RepositoryService;
 import sonia.scm.repository.api.RepositoryServiceFactory;
@@ -76,13 +77,18 @@ public abstract class DefaultChangesetToChangesetDtoMapper extends HalAppenderMa
   @Inject
   private Set<ChangesetTrailers> changesetTrailersSet;
 
-  @Mapping(target = "attributes", ignore = true) // We do not map HAL attributes
+//  @Mapping(target = "attributes", ignore = true) // We do not map HAL attributes
-  public abstract ChangesetDto map(Changeset changeset, @Context Repository repository);
+//  public abstract ChangesetDto map(Changeset changeset, @Context Repository repository);
+
+  abstract TrailerPersonDto map(Trailer trailer);
 
   @AfterMapping
   void appendTrailerPersons(Changeset changeset, @MappingTarget ChangesetDto target, @Context Repository repository) {
     List<TrailerPersonDto> collectedTrailers = new ArrayList<>();
-    changesetTrailersSet.forEach(changesetTrailers -> collectedTrailers.addAll(changesetTrailers.getTrailers(repository, changeset)));
+    changesetTrailersSet.stream()
+      .flatMap(changesetTrailers -> changesetTrailers.getTrailers(repository, changeset).stream())
+      .map(this::map)
+      .forEach(collectedTrailers::add);
     target.setTrailerPersons(collectedTrailers);
   }
 

scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetDescriptionTrailersTest.java

@@ -30,6 +30,7 @@ import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.repository.Changeset;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.RepositoryTestData;
+import sonia.scm.repository.Trailer;
 import sonia.scm.user.DisplayUser;
 import sonia.scm.user.User;
 
@@ -48,73 +49,65 @@ class ChangesetDescriptionTrailersTest {
   void shouldReturnEmptyList() {
     Changeset changeset = createChangeset("zaphod beeblebrox");
 
-    List<TrailerPersonDto> trailerPersons = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
+    List<Trailer> trailer = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
 
-    assertThat(trailerPersons).isNotNull();
+    assertThat(trailer).isNotNull();
-    assertThat(trailerPersons).isEmpty();
+    assertThat(trailer).isEmpty();
   }
 
   @Test
-  void shouldReturnTrailerPersonsWithCoAuthors() {
+  void shouldReturnTrailerWithCoAuthors() {
     DisplayUser displayUser = createDisplayUser("Arthur Dent", "dent@hitchhiker.org");
     Changeset changeset = createChangeset("zaphod beeblebrox\n\nCo-authored-by: Arthur Dent <dent@hitchhiker.org>");
 
-    PersonDto personDto = createPersonDto(displayUser);
+    List<Trailer> Trailer = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
 
-    List<TrailerPersonDto> trailerPersons = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
+    Trailer first = Trailer.get(0);
-
+    assertThat(first.getTrailerType()).isEqualTo("Co-authored-by");
-    TrailerPersonDto trailerPerson = trailerPersons.get(0);
+    assertThat(first.getName()).isEqualTo(displayUser.getDisplayName());
-    assertThat(trailerPerson.getTrailerType()).isEqualTo("Co-authored-by");
+    assertThat(first.getMail()).isEqualTo(displayUser.getMail());
-    assertThat(trailerPerson.getName()).isEqualTo(personDto.getName());
-    assertThat(trailerPerson.getMail()).isEqualTo(personDto.getMail());
   }
 
   @Test
-  void shouldReturnTrailerPersonsWithReviewers() {
+  void shouldReturnTrailerWithReviewers() {
     DisplayUser displayUser = createDisplayUser("Tricia McMillan", "trillian@hitchhiker.org");
     Changeset changeset = createChangeset("zaphod beeblebrox\nReviewed-by: Tricia McMillan <trillian@hitchhiker.org>");
 
-    PersonDto personDto = createPersonDto(displayUser);
+    List<Trailer> trailer = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
 
-    List<TrailerPersonDto> trailerPersons = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
+    Trailer Trailer = trailer.get(0);
 
-    TrailerPersonDto trailerPersonDto = trailerPersons.get(0);
+    assertThat(Trailer.getTrailerType()).isEqualTo("Reviewed-by");
-
+    assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName());
-    assertThat(trailerPersonDto.getTrailerType()).isEqualTo("Reviewed-by");
+    assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail());
-    assertThat(trailerPersonDto.getName()).isEqualTo(personDto.getName());
-    assertThat(trailerPersonDto.getMail()).isEqualTo(personDto.getMail());
   }
 
   @Test
-  void shouldReturnTrailerPersonsWithSigner() {
+  void shouldReturnTrailerWithSigner() {
     DisplayUser displayUser = createDisplayUser("Tricia McMillan", "trillian@hitchhiker.org");
     Changeset changeset = createChangeset("zaphod beeblebrox\nSigned-off-by: Tricia McMillan <trillian@hitchhiker.org>");
 
-    PersonDto personDto = createPersonDto(displayUser);
+    List<Trailer> trailer = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
 
-    List<TrailerPersonDto> trailerPersons = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
+    Trailer Trailer = trailer.get(0);
 
-    TrailerPersonDto trailerPersonDto = trailerPersons.get(0);
+    assertThat(Trailer.getTrailerType()).isEqualTo("Signed-off-by");
-
+    assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName());
-    assertThat(trailerPersonDto.getTrailerType()).isEqualTo("Signed-off-by");
+    assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail());
-    assertThat(trailerPersonDto.getName()).isEqualTo(personDto.getName());
-    assertThat(trailerPersonDto.getMail()).isEqualTo(personDto.getMail());
   }
 
   @Test
-  void shouldReturnTrailerPersonsWithCommitter() {
+  void shouldReturnTrailerWithCommitter() {
     DisplayUser displayUser = createDisplayUser("Tricia McMillan", "trillian@hitchhiker.org");
     Changeset changeset = createChangeset("zaphod beeblebrox\nCommitted-by: Tricia McMillan <trillian@hitchhiker.org>");
 
-    PersonDto personDto = createPersonDto(displayUser);
+    List<Trailer> trailer = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
 
-    List<TrailerPersonDto> trailerPersons = changesetDescriptionTrailers.getTrailers(REPOSITORY, changeset);
+    Trailer Trailer = trailer.get(0);
 
-    TrailerPersonDto trailerPersonDto = trailerPersons.get(0);
+    assertThat(Trailer.getTrailerType()).isEqualTo("Committed-by");
-
+    assertThat(Trailer.getName()).isEqualTo(displayUser.getDisplayName());
-    assertThat(trailerPersonDto.getTrailerType()).isEqualTo("Committed-by");
+    assertThat(Trailer.getMail()).isEqualTo(displayUser.getMail());
-    assertThat(trailerPersonDto.getName()).isEqualTo(personDto.getName());
-    assertThat(trailerPersonDto.getMail()).isEqualTo(personDto.getMail());
   }
 
   private Changeset createChangeset(String commitMessage) {
@@ -126,12 +119,4 @@ class ChangesetDescriptionTrailersTest {
   private DisplayUser createDisplayUser(String name, String mail) {
     return DisplayUser.from(new User(name, name, mail));
   }
-
-  private PersonDto createPersonDto(DisplayUser displayUser) {
-    PersonDto personDto = new PersonDto();
-    personDto.setName(displayUser.getDisplayName());
-    personDto.setMail(displayUser.getMail());
-    return personDto;
-  }
-
 }