fix review findings

This commit is contained in:
Konstantin Schaper
2020-12-01 11:18:19 +01:00
parent 51981dbece
commit e8044747e3
10 changed files with 49 additions and 128 deletions

View File

@@ -1,41 +0,0 @@
/*
* 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;
import sonia.scm.event.Event;
/**
* This event is fired when a new tag was created by the SCM-Manager.
* Warning: This event will not be fired if a new tag was pushed.
* @since 2.10.0
*/
@Event
@Value
public class TagCreatedEvent {
Repository repository;
String revision;
String tagName;
}

View File

@@ -1,40 +0,0 @@
/*
* 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;
import sonia.scm.event.Event;
/**
* This event is fired when a tag was deleted by the SCM-Manager.
* Warning: This event will not be fired if a tag was removed by a push of a git client.
* @since 2.10.0
*/
@Event
@Value
public class TagDeletedEvent {
Repository repository;
String tagName;
}

View File

@@ -386,7 +386,7 @@ public final class RepositoryService implements Closeable {
* by the implementation of the repository service provider.
*/
public TagCommandBuilder getTagCommand() {
return new TagCommandBuilder(repository, provider.getTagCommand());
return new TagCommandBuilder(provider.getTagCommand());
}
/**

View File

@@ -24,24 +24,16 @@
package sonia.scm.repository.api;
import sonia.scm.event.ScmEventBus;
import sonia.scm.repository.Repository;
import sonia.scm.repository.Tag;
import sonia.scm.repository.TagCreatedEvent;
import sonia.scm.repository.TagDeletedEvent;
import sonia.scm.repository.spi.TagCommand;
import java.io.IOException;
public class TagCommandBuilder {
private final Repository repository;
private final TagCommand command;
private final ScmEventBus eventBus;
public TagCommandBuilder(Repository repository, TagCommand command) {
this.repository = repository;
public TagCommandBuilder(TagCommand command) {
this.command = command;
this.eventBus = ScmEventBus.getInstance();
}
public TagCreateCommandBuilder create() {
@@ -53,7 +45,7 @@ public class TagCommandBuilder {
}
public class TagCreateCommandBuilder {
private TagCreateRequest request = new TagCreateRequest();
private final TagCreateRequest request = new TagCreateRequest();
public TagCreateCommandBuilder setRevision(String revision) {
request.setRevision(revision);
@@ -66,14 +58,12 @@ public class TagCommandBuilder {
}
public Tag execute() throws IOException {
Tag tag = command.create(request);
eventBus.post(new TagCreatedEvent(repository, request.getRevision(), request.getName()));
return tag;
return command.create(request);
}
}
public class TagDeleteCommandBuilder {
private TagDeleteRequest request = new TagDeleteRequest();
private final TagDeleteRequest request = new TagDeleteRequest();
public TagDeleteCommandBuilder setName(String name) {
request.setName(name);
@@ -82,7 +72,6 @@ public class TagCommandBuilder {
public void execute() throws IOException {
command.delete(request);
eventBus.post(new TagDeletedEvent(repository, request.getName()));
}
}
}

View File

@@ -254,7 +254,7 @@ public abstract class RepositoryServiceProvider implements Closeable
*/
public TagCommand getTagCommand()
{
throw new CommandNotSupportedException(Command.TAGS);
throw new CommandNotSupportedException(Command.TAG);
}
/**

View File

@@ -64,7 +64,7 @@ public class HgTagsCommandTest extends AbstractHgCommandTestBase {
@Test
public void shouldNotDie() throws IOException {
HgTagCommand hgTagCommand = new HgTagCommand(cmdContext, workingCopyFactory);
new TagCommandBuilder(cmdContext.get(), hgTagCommand).create().setRevision("79b6baf49711").setName("newtag").execute();
new TagCommandBuilder(hgTagCommand).create().setRevision("79b6baf49711").setName("newtag").execute();
HgTagsCommand hgTagsCommand = new HgTagsCommand(cmdContext);
final List<Tag> tags = hgTagsCommand.getTags();

View File

@@ -52,13 +52,12 @@ public class TagCollectionToDtoMapper {
this.tagToTagDtoMapper = tagToTagDtoMapper;
}
public HalRepresentation map(String namespace, String name, Collection<Tag> tags, Repository repository) {
return new HalRepresentation(createLinks(namespace, name), embedDtos(getTagDtoList(namespace, name, tags, repository)));
public HalRepresentation map(Collection<Tag> tags, Repository repository) {
return new HalRepresentation(createLinks(repository.getNamespace(), repository.getName()), embedDtos(getTagDtoList(tags, repository)));
}
public List<TagDto> getTagDtoList(String namespace, String name, Collection<Tag> tags, Repository repository) {
final NamespaceAndName namespaceAndName = new NamespaceAndName(namespace, name);
return tags.stream().map(tag -> tagToTagDtoMapper.map(tag, namespaceAndName, repository)).collect(toList());
public List<TagDto> getTagDtoList(Collection<Tag> tags, Repository repository) {
return tags.stream().map(tag -> tagToTagDtoMapper.map(tag, repository)).collect(toList());
}
public List<TagDto> getMinimalEmbeddedTagDtoList(String namespace, String name, Collection<String> tags) {

View File

@@ -27,7 +27,9 @@ package sonia.scm.api.v2.resources;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.headers.Header;
import io.swagger.v3.oas.annotations.media.Content;
import io.swagger.v3.oas.annotations.media.ExampleObject;
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.parameters.RequestBody;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import sonia.scm.NotFoundException;
import sonia.scm.repository.Branch;
@@ -100,7 +102,7 @@ public class TagRootResource {
try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) {
Tags tags = getTags(repositoryService);
if (tags != null && tags.getTags() != null) {
return Response.ok(tagCollectionToDtoMapper.map(namespace, name, tags.getTags(), repositoryService.getRepository())).build();
return Response.ok(tagCollectionToDtoMapper.map(tags.getTags(), repositoryService.getRepository())).build();
} else {
return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
.entity("Error on getting tag from repository.")
@@ -112,7 +114,20 @@ public class TagRootResource {
@POST
@Path("")
@Produces(VndMediaType.TAG_REQUEST)
@Operation(summary = "Create tag", description = "Creates a new tag and returns it", tags = "Repository")
@Operation(summary = "Create tag",
description = "Creates a new tag.",
tags = "Repository",
requestBody = @RequestBody(
content = @Content(
mediaType = VndMediaType.TAG_REQUEST,
schema = @Schema(implementation = TagRequestDto.class),
examples = @ExampleObject(
name = "Create a new tag for a revision",
value = "{\n \"revision\":\"734713bc047d87bf7eac9674765ae793478c50d3\",\n \"name\":\"v1.1.0\"\n}",
summary = "Create a tag"
)
)
))
@ApiResponse(
responseCode = "201",
description = "create success",
@@ -144,7 +159,7 @@ public class TagRootResource {
String tagName = tagRequest.getName();
try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) {
if (tagExists(tagName, repositoryService)) {
throw alreadyExists(entity(Tag.class, tagName).in(Repository.class, namespace + "/" + name));
throw alreadyExists(entity(Tag.class, tagName).in(repositoryService.getRepository()));
}
Repository repository = repositoryService.getRepository();
RepositoryPermissions.push(repository).check();
@@ -194,7 +209,7 @@ public class TagRootResource {
.filter(t -> tagName.equals(t.getName()))
.findFirst()
.orElseThrow(() -> createNotFoundException(namespace, name, tagName));
return Response.ok(tagToTagDtoMapper.map(tag, namespaceAndName, repositoryService.getRepository())).build();
return Response.ok(tagToTagDtoMapper.map(tag, repositoryService.getRepository())).build();
} else {
return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
.entity("Error on getting tag from repository.")
@@ -227,7 +242,7 @@ public class TagRootResource {
mediaType = VndMediaType.ERROR_TYPE,
schema = @Schema(implementation = ErrorDto.class)
))
public Response delete(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("tagName") String tagName) {
public Response delete(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("tagName") String tagName) throws IOException {
NamespaceAndName namespaceAndName = new NamespaceAndName(namespace, name);
try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) {
RepositoryPermissions.push(repositoryService.getRepository()).check();
@@ -239,8 +254,6 @@ public class TagRootResource {
}
return Response.noContent().build();
} catch (IOException e) {
return Response.serverError().build();
}
}

View File

@@ -55,22 +55,22 @@ public abstract class TagToTagDtoMapper extends HalAppenderMapper {
@Mapping(target = "date", source = "date", qualifiedByName = "mapDate")
@Mapping(target = "attributes", ignore = true) // We do not map HAL attributes
@Mapping(target = "signatures")
public abstract TagDto map(Tag tag, @Context NamespaceAndName namespaceAndName, @Context Repository repository);
public abstract TagDto map(Tag tag, @Context Repository repository);
@ObjectFactory
TagDto createDto(@Context NamespaceAndName namespaceAndName, @Context Repository repository, Tag tag) {
TagDto createDto(@Context Repository repository, Tag tag) {
Links.Builder linksBuilder = linkingTo()
.self(resourceLinks.tag().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), tag.getName()))
.single(link("sources", resourceLinks.source().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), tag.getRevision())))
.single(link("changeset", resourceLinks.changeset().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), tag.getRevision())));
.self(resourceLinks.tag().self(repository.getNamespace(), repository.getName(), tag.getName()))
.single(link("sources", resourceLinks.source().self(repository.getNamespace(), repository.getName(), tag.getRevision())))
.single(link("changeset", resourceLinks.changeset().self(repository.getNamespace(), repository.getName(), tag.getRevision())));
if (tag.getDeletable() && RepositoryPermissions.push(repository).isPermitted()) {
linksBuilder
.single(link("delete", resourceLinks.tag().delete(namespaceAndName.getNamespace(), namespaceAndName.getName(), tag.getName())));
.single(link("delete", resourceLinks.tag().delete(repository.getNamespace(), repository.getName(), tag.getName())));
}
Embedded.Builder embeddedBuilder = embeddedBuilder();
applyEnrichers(new EdisonHalAppender(linksBuilder, embeddedBuilder), tag, namespaceAndName);
applyEnrichers(new EdisonHalAppender(linksBuilder, embeddedBuilder), tag, repository);
return new TagDto(linksBuilder.build(), embeddedBuilder.build());
}

View File

@@ -74,26 +74,27 @@ class TagToTagDtoMapperTest {
void shouldAppendLinks() {
HalEnricherRegistry registry = new HalEnricherRegistry();
registry.register(Tag.class, (ctx, appender) -> {
NamespaceAndName repository = ctx.oneRequireByType(NamespaceAndName.class);
Repository repository = ctx.oneRequireByType(Repository.class);
Tag tag = ctx.oneRequireByType(Tag.class);
appender.appendLink("yo", "http://" + repository.logString() + "/" + tag.getName());
appender.appendLink("yo", "http://" + repository.getNamespace() + "/" + repository.getName() + "/" + tag.getName());
});
mapper.setRegistry(registry);
TagDto dto = mapper.map(new Tag("1.0.0", "42"), new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold());
assertThat(dto.getLinks().getLinkBy("yo").get().getHref()).isEqualTo("http://hitchhiker/hog/1.0.0");
TagDto dto = mapper.map(new Tag("1.0.0", "42"), RepositoryTestData.createHeartOfGold());
assertThat(dto.getLinks().getLinkBy("yo").get().getHref()).isEqualTo("http://hitchhiker/HeartOfGold/1.0.0");
}
@Test
void shouldMapDate() {
final long now = Instant.now().getEpochSecond() * 1000;
TagDto dto = mapper.map(new Tag("1.0.0", "42", now), new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold());
TagDto dto = mapper.map(new Tag("1.0.0", "42", now), RepositoryTestData.createHeartOfGold());
assertThat(dto.getDate()).isEqualTo(Instant.ofEpochMilli(now));
}
@Test
void shouldContainSignatureArray() {
TagDto dto = mapper.map(new Tag("1.0.0", "42"), new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold());
TagDto dto = mapper.map(new Tag("1.0.0", "42"), RepositoryTestData.createHeartOfGold());
assertThat(dto.getSignatures()).isNotNull();
}
@@ -101,7 +102,7 @@ class TagToTagDtoMapperTest {
void shouldMapSignatures() {
final Tag tag = new Tag("1.0.0", "42");
tag.addSignature(new Signature("29v391239v", "gpg", SignatureStatus.VERIFIED, "me", Collections.emptySet()));
TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold());
TagDto dto = mapper.map(tag, RepositoryTestData.createHeartOfGold());
assertThat(dto.getSignatures()).isNotEmpty();
}
@@ -110,21 +111,21 @@ class TagToTagDtoMapperTest {
Repository repository = RepositoryTestData.createHeartOfGold();
when(subject.isPermitted("repository:push:" + repository.getId())).thenReturn(true);
final Tag tag = new Tag("1.0.0", "42");
TagDto dto = mapper.map(tag, new NamespaceAndName(repository.getNamespace(), repository.getName()), repository);
TagDto dto = mapper.map(tag, repository);
assertThat(dto.getLinks().getLinkBy("delete")).isNotEmpty();
}
@Test
void shouldNotAddDeleteLinkIfPermissionsAreMissing() {
final Tag tag = new Tag("1.0.0", "42");
TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold());
TagDto dto = mapper.map(tag, RepositoryTestData.createHeartOfGold());
assertThat(dto.getLinks().getLinkBy("delete")).isEmpty();
}
@Test
void shouldNotAddDeleteLinksForUndeletableTags() {
final Tag tag = new Tag("1.0.0", "42", null, false);
TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold());
TagDto dto = mapper.map(tag, RepositoryTestData.createHeartOfGold());
assertThat(dto.getLinks().getLinkBy("delete")).isEmpty();
}