From e451bb618eea442643e755f77b3a655c163397b1 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Thu, 28 May 2020 09:50:16 +0200 Subject: [PATCH] adjust ChangesetTrailerExtractor so that we don't check if user already exists --- .../scm/api/v2/resources/ChangesetDto.java | 4 +- .../api/v2/resources/TrailerPersonDto.java | 36 ++++++++ .../resources/ChangesetTrailerExtractor.java | 46 +++++----- .../scm/api/v2/resources/MapperModule.java | 2 + .../ChangesetTrailerExtractorTest.java | 83 +++++++++---------- 5 files changed, 99 insertions(+), 72 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerPersonDto.java diff --git a/scm-core/src/main/java/sonia/scm/api/v2/resources/ChangesetDto.java b/scm-core/src/main/java/sonia/scm/api/v2/resources/ChangesetDto.java index 801593be6e..08517c2ae1 100644 --- a/scm-core/src/main/java/sonia/scm/api/v2/resources/ChangesetDto.java +++ b/scm-core/src/main/java/sonia/scm/api/v2/resources/ChangesetDto.java @@ -32,7 +32,7 @@ import lombok.NoArgsConstructor; import lombok.Setter; import java.time.Instant; -import java.util.Map; +import java.util.List; @Getter @Setter @@ -59,7 +59,7 @@ public class ChangesetDto extends HalRepresentation { */ private String description; - private Map trailerPersons; + private List trailerPersons; public ChangesetDto(Links links, Embedded embedded) { super(links, embedded); diff --git a/scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerPersonDto.java b/scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerPersonDto.java new file mode 100644 index 0000000000..ecf09c3552 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/api/v2/resources/TrailerPersonDto.java @@ -0,0 +1,36 @@ +/* + * 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.api.v2.resources; + +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +@Getter +@Setter +@NoArgsConstructor +public class TrailerPersonDto extends PersonDto { + private String trailerType; +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetTrailerExtractor.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetTrailerExtractor.java index b5661c27db..91ff5261f4 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetTrailerExtractor.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangesetTrailerExtractor.java @@ -26,27 +26,24 @@ package sonia.scm.api.v2.resources; import com.google.common.collect.ImmutableList; import sonia.scm.repository.ChangesetTrailerTypes; -import sonia.scm.user.DisplayUser; -import sonia.scm.user.UserDisplayManager; import javax.inject.Inject; -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Scanner; import java.util.regex.Pattern; public class ChangesetTrailerExtractor { - private final UserDisplayManager userDisplayManager; private final ChangesetTrailerTypes changesetTrailerTypes; @Inject - public ChangesetTrailerExtractor(UserDisplayManager userDisplayManager, ChangesetTrailerTypes changesetTrailerTypes) { - this.userDisplayManager = userDisplayManager; + public ChangesetTrailerExtractor(ChangesetTrailerTypes changesetTrailerTypes) { this.changesetTrailerTypes = changesetTrailerTypes; } - Map extractTrailersFromCommitMessage(String commitMessage) { - - HashMap persons = new HashMap<>(); + List extractTrailersFromCommitMessage(String commitMessage) { + List persons = new ArrayList<>(); try (Scanner scanner = new Scanner(commitMessage)) { scanner.useDelimiter(Pattern.compile("[\\n;]")); @@ -55,30 +52,29 @@ public class ChangesetTrailerExtractor { for (String trailerType : changesetTrailerTypes.getTrailerTypes()) { if (line.contains(trailerType)) { - String mail = line.split("<|>")[1]; - persons.put(trailerType, createPersonDtoFromUser(mail)); + TrailerPersonDto personDto = createPersonDtoFromUser(line); + personDto.setTrailerType(trailerType); + persons.add(personDto); } } - -/* if (line.contains("Co-authored-by")) { - persons.put("Co-authored-by", createPersonDtoFromUser(mail)); - } - if (line.contains("Reviewed-by")) { - persons.put("Reviewed-by", createPersonDtoFromUser(mail)); - }*/ - } } - return persons; } - private PersonDto createPersonDtoFromUser(String mail) { - DisplayUser displayUser = userDisplayManager.autocomplete(mail).iterator().next(); - PersonDto personDto = new PersonDto(); - personDto.setName(displayUser.getDisplayName()); - personDto.setMail(displayUser.getMail()); + private TrailerPersonDto createPersonDtoFromUser(String line) { + TrailerPersonDto personDto = new TrailerPersonDto(); + + String[] splittedTrailer = line.split("[:<>]"); + + if (splittedTrailer.length > 1) { + personDto.setName(splittedTrailer[1].trim()); + if (splittedTrailer.length > 2) { + personDto.setMail(splittedTrailer[2]); + } + } + return personDto; } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java index 83be1f3636..26d430e706 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java @@ -27,6 +27,7 @@ package sonia.scm.api.v2.resources; import com.google.inject.AbstractModule; import com.google.inject.servlet.ServletScopes; import org.mapstruct.factory.Mappers; +import sonia.scm.repository.ChangesetTrailerTypes; import sonia.scm.web.api.RepositoryToHalMapper; public class MapperModule extends AbstractModule { @@ -77,6 +78,7 @@ public class MapperModule extends AbstractModule { bind(MeDtoFactory.class); bind(UIPluginDtoMapper.class); bind(UIPluginDtoCollectionMapper.class); + bind(ChangesetTrailerTypes.class).to(ChangesetTrailerExtractor.TrailerTypes.class); bind(ScmPathInfoStore.class).in(ServletScopes.REQUEST); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetTrailerExtractorTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetTrailerExtractorTest.java index 58657cab8f..bac94060de 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetTrailerExtractorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ChangesetTrailerExtractorTest.java @@ -24,111 +24,104 @@ package sonia.scm.api.v2.resources; -import com.google.common.collect.ImmutableList; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.api.v2.resources.ChangesetTrailerExtractor.TrailerTypes; import sonia.scm.user.DisplayUser; import sonia.scm.user.User; -import sonia.scm.user.UserDisplayManager; -import java.util.Map; +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class ChangesetTrailerExtractorTest { - @Mock - private UserDisplayManager userDisplayManager; - private ChangesetTrailerExtractor extractor; @BeforeEach void initExtractor() { TrailerTypes trailerTypes = new TrailerTypes(); - extractor = new ChangesetTrailerExtractor(userDisplayManager, trailerTypes); + extractor = new ChangesetTrailerExtractor(trailerTypes); } @Test - void shouldReturnEmptyMap() { + void shouldReturnEmptyList() { String commitMessage = "zaphod beetlebrox"; - Map map = extractor.extractTrailersFromCommitMessage(commitMessage); + List trailerPersons = extractor.extractTrailersFromCommitMessage(commitMessage); - assertThat(map).isInstanceOf(Map.class); - assertThat(map).isNotNull(); - assertThat(map.keySet()).isEmpty(); + assertThat(trailerPersons).isNotNull(); + assertThat(trailerPersons).isEmpty(); } @Test - void shouldReturnMapWithCoAuthors() { - DisplayUser displayUser = mockDisplayUser("Arthur Dent", "dent@hitchhiker.org"); + void shouldReturnTrailerPersonsWithCoAuthors() { + DisplayUser displayUser = createDisplayUser("Arthur Dent", "dent@hitchhiker.org"); String commitMessage = "zaphod beetlebrox\n\nCo-authored-by: Arthur Dent "; PersonDto personDto = createPersonDto(displayUser); - Map map = extractor.extractTrailersFromCommitMessage(commitMessage); + List trailerPersons = extractor.extractTrailersFromCommitMessage(commitMessage); - assertThat(map.keySet().iterator().next()).isEqualTo("Co-authored-by"); - PersonDto person = map.values().iterator().next(); - assertThat(person.getName()).isEqualTo(personDto.getName()); - assertThat(person.getMail()).isEqualTo(personDto.getMail()); + TrailerPersonDto trailerPersonDto = trailerPersons.get(0); + assertThat(trailerPersonDto.getTrailerType()).isEqualTo("Co-authored-by"); + assertThat(trailerPersonDto.getName()).isEqualTo(personDto.getName()); + assertThat(trailerPersonDto.getMail()).isEqualTo(personDto.getMail()); } @Test - void shouldReturnMapWithReviewers() { - DisplayUser displayUser = mockDisplayUser("Tricia McMillan", "trillian@hitchhiker.org"); + void shouldReturnTrailerPersonsWithReviewers() { + DisplayUser displayUser = createDisplayUser("Tricia McMillan", "trillian@hitchhiker.org"); String commitMessage = "zaphod beetlebrox\nReviewed-by: Tricia McMillan "; PersonDto personDto = createPersonDto(displayUser); - Map map = extractor.extractTrailersFromCommitMessage(commitMessage); + List trailerPersons = extractor.extractTrailersFromCommitMessage(commitMessage); - assertThat(map.keySet().iterator().next()).isEqualTo("Reviewed-by"); - PersonDto person = map.values().iterator().next(); - assertThat(person.getName()).isEqualTo(personDto.getName()); - assertThat(person.getMail()).isEqualTo(personDto.getMail()); + TrailerPersonDto trailerPersonDto = trailerPersons.get(0); + + assertThat(trailerPersonDto.getTrailerType()).isEqualTo("Reviewed-by"); + assertThat(trailerPersonDto.getName()).isEqualTo(personDto.getName()); + assertThat(trailerPersonDto.getMail()).isEqualTo(personDto.getMail()); } @Test - void shouldReturnMapWithSigner() { - DisplayUser displayUser = mockDisplayUser("Tricia McMillan", "trillian@hitchhiker.org"); + void shouldReturnTrailerPersonsWithSigner() { + DisplayUser displayUser = createDisplayUser("Tricia McMillan", "trillian@hitchhiker.org"); String commitMessage = "zaphod beetlebrox\nSigned-off-by: Tricia McMillan "; PersonDto personDto = createPersonDto(displayUser); - Map map = extractor.extractTrailersFromCommitMessage(commitMessage); + List trailerPersons = extractor.extractTrailersFromCommitMessage(commitMessage); - assertThat(map.keySet().iterator().next()).isEqualTo("Signed-off-by"); - PersonDto person = map.values().iterator().next(); - assertThat(person.getName()).isEqualTo(personDto.getName()); - assertThat(person.getMail()).isEqualTo(personDto.getMail()); + TrailerPersonDto trailerPersonDto = trailerPersons.get(0); + + assertThat(trailerPersonDto.getTrailerType()).isEqualTo("Signed-off-by"); + assertThat(trailerPersonDto.getName()).isEqualTo(personDto.getName()); + assertThat(trailerPersonDto.getMail()).isEqualTo(personDto.getMail()); } @Test - void shouldReturnMapWithCommitter() { - DisplayUser displayUser = mockDisplayUser("Tricia McMillan", "trillian@hitchhiker.org"); + void shouldReturnTrailerPersonsWithCommitter() { + DisplayUser displayUser = createDisplayUser("Tricia McMillan", "trillian@hitchhiker.org"); String commitMessage = "zaphod beetlebrox\nCommitted-by: Tricia McMillan "; PersonDto personDto = createPersonDto(displayUser); - Map map = extractor.extractTrailersFromCommitMessage(commitMessage); + List trailerPersons = extractor.extractTrailersFromCommitMessage(commitMessage); - assertThat(map.keySet().iterator().next()).isEqualTo("Committed-by"); - PersonDto person = map.values().iterator().next(); - assertThat(person.getName()).isEqualTo(personDto.getName()); - assertThat(person.getMail()).isEqualTo(personDto.getMail()); + TrailerPersonDto trailerPersonDto = trailerPersons.get(0); + + assertThat(trailerPersonDto.getTrailerType()).isEqualTo("Committed-by"); + assertThat(trailerPersonDto.getName()).isEqualTo(personDto.getName()); + assertThat(trailerPersonDto.getMail()).isEqualTo(personDto.getMail()); } - - private DisplayUser mockDisplayUser(String name, String mail) { + private DisplayUser createDisplayUser(String name, String mail) { DisplayUser displayUser = DisplayUser.from(new User(name, name, mail)); - when(userDisplayManager.autocomplete(mail)).thenReturn(ImmutableList.of(displayUser)); return displayUser; }