From d26293f7671bf999c90104a8c053dfe3c18c36b3 Mon Sep 17 00:00:00 2001 From: tillias Date: Wed, 28 Oct 2020 19:36:58 +0100 Subject: [PATCH] Duplicated dependencies validation #71 and bugfixing #68 --- .../service/custom/DependencyService.java | 13 +++++ .../DuplicateDependencyException.java | 16 +++++ .../microcatalog/utils/DependencyBuilder.java | 8 ++- .../utils/MicroserviceBuilder.java | 7 +++ .../errors/custom/ReleasePathAdviceTrait.java | 31 +++++++--- .../app/shared/alert/alert-error.component.ts | 2 +- src/main/webapp/i18n/en/error.json | 5 ++ .../service/custom/DependencyServiceTest.java | 58 +++++++++++++++++++ .../utils/DependencyBuilderTest.java | 10 +++- .../utils/MicroserviceBuilderTest.java | 5 +- .../custom/ReleasePathAdviceTraitTest.java | 47 +++++++++++---- 11 files changed, 179 insertions(+), 23 deletions(-) create mode 100644 src/main/java/com/github/microcatalog/service/custom/exceptions/DuplicateDependencyException.java diff --git a/src/main/java/com/github/microcatalog/service/custom/DependencyService.java b/src/main/java/com/github/microcatalog/service/custom/DependencyService.java index d787b75..292775a 100644 --- a/src/main/java/com/github/microcatalog/service/custom/DependencyService.java +++ b/src/main/java/com/github/microcatalog/service/custom/DependencyService.java @@ -4,6 +4,7 @@ import com.github.microcatalog.domain.Microservice; import com.github.microcatalog.repository.DependencyRepository; import com.github.microcatalog.service.custom.exceptions.CircularDependenciesException; +import com.github.microcatalog.service.custom.exceptions.DuplicateDependencyException; import com.github.microcatalog.service.custom.exceptions.SelfCircularException; import org.jgrapht.Graph; import org.jgrapht.alg.cycle.CycleDetector; @@ -83,6 +84,9 @@ private void validateSelfCycle(final Dependency dependency) { private void validateIfAdded(final Dependency toBeAdded) { final Graph graph = graphLoaderService.loadGraph(); + + checkDuplicateWillBeIntroduced(graph,toBeAdded); + graph.addEdge(toBeAdded.getSource(), toBeAdded.getTarget()); checkCycles(graph); @@ -98,11 +102,20 @@ private void validateIfUpdated(final Dependency dependency) { final DefaultEdge currentEdge = graph.getEdge(persistent.getSource(), persistent.getTarget()); graph.removeEdge(currentEdge); + checkDuplicateWillBeIntroduced(graph,dependency); + graph.addEdge(dependency.getSource(), dependency.getTarget()); checkCycles(graph); } + private void checkDuplicateWillBeIntroduced(final Graph graph, final Dependency dependency){ + final DefaultEdge existingEdge = graph.getEdge(dependency.getSource(), dependency.getTarget()); + if (existingEdge != null) { + throw new DuplicateDependencyException("Dependency already exists", dependency); + } + } + private void checkCycles(final Graph graph) { final CycleDetector cycleDetector = new CycleDetector<>(graph); if (cycleDetector.detectCycles()) { diff --git a/src/main/java/com/github/microcatalog/service/custom/exceptions/DuplicateDependencyException.java b/src/main/java/com/github/microcatalog/service/custom/exceptions/DuplicateDependencyException.java new file mode 100644 index 0000000..828d7c6 --- /dev/null +++ b/src/main/java/com/github/microcatalog/service/custom/exceptions/DuplicateDependencyException.java @@ -0,0 +1,16 @@ +package com.github.microcatalog.service.custom.exceptions; + +import com.github.microcatalog.domain.Dependency; + +public class DuplicateDependencyException extends RuntimeException { + private final Dependency dependency; + + public DuplicateDependencyException(String message, Dependency dependency) { + super(message); + this.dependency = dependency; + } + + public Dependency getDependency() { + return dependency; + } +} diff --git a/src/main/java/com/github/microcatalog/utils/DependencyBuilder.java b/src/main/java/com/github/microcatalog/utils/DependencyBuilder.java index 07097f0..afcdb9d 100644 --- a/src/main/java/com/github/microcatalog/utils/DependencyBuilder.java +++ b/src/main/java/com/github/microcatalog/utils/DependencyBuilder.java @@ -4,6 +4,7 @@ public class DependencyBuilder { private Long id; + private String name; private Long source; private Long target; @@ -12,6 +13,11 @@ public DependencyBuilder withId(Long id) { return this; } + public DependencyBuilder withName(String name) { + this.name = name; + return this; + } + public DependencyBuilder withSource(Long sourceId) { this.source = sourceId; return this; @@ -24,7 +30,7 @@ public DependencyBuilder withTarget(Long targetId) { public Dependency build() { final Dependency result = new Dependency(); - + result.setName(name); result.setId(id); result.setSource(new MicroserviceBuilder().withId(source).build()); result.setTarget(new MicroserviceBuilder().withId(target).build()); diff --git a/src/main/java/com/github/microcatalog/utils/MicroserviceBuilder.java b/src/main/java/com/github/microcatalog/utils/MicroserviceBuilder.java index 4e448fd..d8e4fee 100644 --- a/src/main/java/com/github/microcatalog/utils/MicroserviceBuilder.java +++ b/src/main/java/com/github/microcatalog/utils/MicroserviceBuilder.java @@ -4,15 +4,22 @@ public class MicroserviceBuilder { private Long id; + private String name; public MicroserviceBuilder withId(Long id) { this.id = id; return this; } + public MicroserviceBuilder withName(String name){ + this.name = name; + return this; + } + public Microservice build() { final Microservice result = new Microservice(); result.setId(id); + result.setName(name); return result; } } diff --git a/src/main/java/com/github/microcatalog/web/rest/errors/custom/ReleasePathAdviceTrait.java b/src/main/java/com/github/microcatalog/web/rest/errors/custom/ReleasePathAdviceTrait.java index 0a02790..d25b47a 100644 --- a/src/main/java/com/github/microcatalog/web/rest/errors/custom/ReleasePathAdviceTrait.java +++ b/src/main/java/com/github/microcatalog/web/rest/errors/custom/ReleasePathAdviceTrait.java @@ -1,7 +1,9 @@ package com.github.microcatalog.web.rest.errors.custom; +import com.github.microcatalog.domain.Dependency; import com.github.microcatalog.domain.Microservice; import com.github.microcatalog.service.custom.exceptions.CircularDependenciesException; +import com.github.microcatalog.service.custom.exceptions.DuplicateDependencyException; import com.github.microcatalog.service.custom.exceptions.SelfCircularException; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; @@ -19,18 +21,33 @@ */ public interface ReleasePathAdviceTrait extends AdviceTrait { - String HEADER_KEY = "BusinessException"; - String PAYLOAD_KEY = "BusinessPayload"; + String EXCEPTION_KEY = "BusinessException"; String MESSAGE_KEY = "BusinessMessage"; + @ExceptionHandler + default ResponseEntity handleDuplicateDependencyException(final DuplicateDependencyException exception, final NativeWebRequest request) { + final Dependency dependency = exception.getDependency(); + + final Problem problem = Problem.builder() + .withStatus(UNPROCESSABLE_ENTITY) + .with(EXCEPTION_KEY, DuplicateDependencyException.class.getSimpleName()) + .with(MESSAGE_KEY, exception.getMessage()) + .with("dependencyId", String.valueOf(dependency.getId())) + .with("dependencyName", dependency.getName()) + .build(); + + return create(exception, problem, request); + + } + @ExceptionHandler default ResponseEntity handleSelfCircularException(final SelfCircularException exception, final NativeWebRequest request) { final Microservice source = exception.getSource(); final Problem problem = Problem.builder() .withStatus(UNPROCESSABLE_ENTITY) - .with(HEADER_KEY, SelfCircularException.class.getName()) - .with(PAYLOAD_KEY, String.format("id = %s, name = %s", source.getId(), source.getName())) + .with(EXCEPTION_KEY, SelfCircularException.class.getSimpleName()) .with(MESSAGE_KEY, exception.getMessage()) + .with("microserviceName", source.getName()) .build(); return create(exception, problem, request); @@ -38,13 +55,13 @@ default ResponseEntity handleSelfCircularException(final SelfCircularEx @ExceptionHandler default ResponseEntity handleCircularDependenciesException(final CircularDependenciesException exception, final NativeWebRequest request) { - final List microserviceIds = exception.getCycles().stream().map(m -> String.valueOf(m.getId())).collect(Collectors.toList()); + final List microservices = exception.getCycles().stream().map(Microservice::getName).collect(Collectors.toList()); final Problem problem = Problem.builder() .withStatus(UNPROCESSABLE_ENTITY) - .with(HEADER_KEY, CircularDependenciesException.class.getName()) - .with(PAYLOAD_KEY, String.join(",", microserviceIds)) + .with(EXCEPTION_KEY, CircularDependenciesException.class.getSimpleName()) .with(MESSAGE_KEY, exception.getMessage()) + .with("microservices", String.join(",", microservices)) .build(); return create(exception, problem, request); diff --git a/src/main/webapp/app/shared/alert/alert-error.component.ts b/src/main/webapp/app/shared/alert/alert-error.component.ts index d82eae0..2563276 100644 --- a/src/main/webapp/app/shared/alert/alert-error.component.ts +++ b/src/main/webapp/app/shared/alert/alert-error.component.ts @@ -73,7 +73,7 @@ export class AlertErrorComponent implements OnDestroy { break; case 422: - this.addErrorAlert(httpErrorResponse.error.BusinessMessage); + this.addErrorAlert('', 'business.' + httpErrorResponse.error.BusinessException, httpErrorResponse.error); break; default: diff --git a/src/main/webapp/i18n/en/error.json b/src/main/webapp/i18n/en/error.json index d257563..7eb8bcb 100644 --- a/src/main/webapp/i18n/en/error.json +++ b/src/main/webapp/i18n/en/error.json @@ -1,4 +1,9 @@ { + "business": { + "DuplicateDependencyException": "Dependency already exists: {{dependencyName}}", + "CircularDependenciesException": "Circular dependency will be introduced. Cycle: {{microservices}}", + "SelfCircularException": "Source of dependency can't be the same as target. Source: {{microserviceName}}" + }, "error": { "title": "Error page!", "http": { diff --git a/src/test/java/com/github/microcatalog/service/custom/DependencyServiceTest.java b/src/test/java/com/github/microcatalog/service/custom/DependencyServiceTest.java index aa9f91b..0c23f7c 100644 --- a/src/test/java/com/github/microcatalog/service/custom/DependencyServiceTest.java +++ b/src/test/java/com/github/microcatalog/service/custom/DependencyServiceTest.java @@ -4,8 +4,10 @@ import com.github.microcatalog.repository.DependencyRepository; import com.github.microcatalog.service.GraphUtils; import com.github.microcatalog.service.custom.exceptions.CircularDependenciesException; +import com.github.microcatalog.service.custom.exceptions.DuplicateDependencyException; import com.github.microcatalog.service.custom.exceptions.SelfCircularException; import com.github.microcatalog.utils.DependencyBuilder; +import com.github.microcatalog.utils.MicroserviceBuilder; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -103,6 +105,32 @@ void create_WillIntroduceCircularDependencies_ExceptionIsThrown() { .containsExactlyInAnyOrder(1L, 2L, 3L, 4L); } + @Test + void create_DuplicateWillBeIntroduced_ExceptionIsThrown() { + given(graphLoaderService.loadGraph()).willReturn( + GraphUtils.createGraph( + String.join("\n", + "strict digraph G { ", + "1; 2; 3;", + "1 -> 2;", + "2 -> 3;}" + ) + ) + ); + + final Dependency dependency = dependency(null, 1, 2); + + assertThatThrownBy(() -> service.create(dependency)) + .isInstanceOf(DuplicateDependencyException.class) + .hasMessageStartingWith("Dependency already exists") + .extracting("dependency", InstanceOfAssertFactories.type(Dependency.class)) + .extracting(Dependency::getSource, Dependency::getTarget) + .containsExactlyInAnyOrder( + new MicroserviceBuilder().withId(1L).build(), + new MicroserviceBuilder().withId(2L).build() + ); + } + @Test void create_Success() { given(graphLoaderService.loadGraph()).willReturn( @@ -171,6 +199,36 @@ void update_WillIntroduceCircularDependencies_ExceptionIsThrown() { .containsExactlyInAnyOrder(1L, 2L, 3L); } + @Test + void update_DuplicateWillBeIntroduced_ExceptionIsThrown() { + given(graphLoaderService.loadGraph()).willReturn( + GraphUtils.createGraph( + String.join("\n", + "strict digraph G { ", + "1; 2; 3;", + "1 -> 2;", + "2 -> 3;}" // has id = 2L + ) + ) + ); + + given(repository.findById(2L)) + .willReturn(Optional.of(dependency(2, 2, 3))); + + final Dependency dependency = dependency(2, 1, 2); + + + assertThatThrownBy(() -> service.update(dependency)) + .isInstanceOf(DuplicateDependencyException.class) + .hasMessageStartingWith("Dependency already exists") + .extracting("dependency", InstanceOfAssertFactories.type(Dependency.class)) + .extracting(Dependency::getSource, Dependency::getTarget) + .containsExactlyInAnyOrder( + new MicroserviceBuilder().withId(1L).build(), + new MicroserviceBuilder().withId(2L).build() + ); + } + @Test void update_Success() { given(graphLoaderService.loadGraph()).willReturn( diff --git a/src/test/java/com/github/microcatalog/utils/DependencyBuilderTest.java b/src/test/java/com/github/microcatalog/utils/DependencyBuilderTest.java index 56e6472..511bff7 100644 --- a/src/test/java/com/github/microcatalog/utils/DependencyBuilderTest.java +++ b/src/test/java/com/github/microcatalog/utils/DependencyBuilderTest.java @@ -11,8 +11,14 @@ class DependencyBuilderTest { @Test void build() { DependencyBuilder builder = new DependencyBuilder(); - Dependency dependency = builder.withId(1L).withSource(2L).withTarget(3L).build(); - assertThat(dependency).isNotNull().extracting(Dependency::getId).isEqualTo(1L); + Dependency dependency = builder + .withId(1L) + .withName("test") + .withSource(2L) + .withTarget(3L) + .build(); + + assertThat(dependency).isNotNull().extracting(Dependency::getId, Dependency::getName).containsExactly(1L, "test"); assertThat(dependency.getSource()).isNotNull().extracting(Microservice::getId).isEqualTo(2L); assertThat(dependency.getTarget()).isNotNull().extracting(Microservice::getId).isEqualTo(3L); } diff --git a/src/test/java/com/github/microcatalog/utils/MicroserviceBuilderTest.java b/src/test/java/com/github/microcatalog/utils/MicroserviceBuilderTest.java index 0ad8ec6..4aa502a 100644 --- a/src/test/java/com/github/microcatalog/utils/MicroserviceBuilderTest.java +++ b/src/test/java/com/github/microcatalog/utils/MicroserviceBuilderTest.java @@ -10,7 +10,8 @@ class MicroserviceBuilderTest { @Test void build() { MicroserviceBuilder builder = new MicroserviceBuilder(); - Microservice microservice = builder.withId(1L).build(); - assertThat(microservice).isNotNull().extracting(Microservice::getId).isEqualTo(1L); + Microservice microservice = builder.withId(1L).withName("Test microservice").build(); + assertThat(microservice).isNotNull() + .extracting(Microservice::getId, Microservice::getName).containsExactly(1L, "Test microservice"); } } diff --git a/src/test/java/com/github/microcatalog/web/rest/errors/custom/ReleasePathAdviceTraitTest.java b/src/test/java/com/github/microcatalog/web/rest/errors/custom/ReleasePathAdviceTraitTest.java index 9b71515..be29918 100644 --- a/src/test/java/com/github/microcatalog/web/rest/errors/custom/ReleasePathAdviceTraitTest.java +++ b/src/test/java/com/github/microcatalog/web/rest/errors/custom/ReleasePathAdviceTraitTest.java @@ -1,7 +1,9 @@ package com.github.microcatalog.web.rest.errors.custom; import com.github.microcatalog.service.custom.exceptions.CircularDependenciesException; +import com.github.microcatalog.service.custom.exceptions.DuplicateDependencyException; import com.github.microcatalog.service.custom.exceptions.SelfCircularException; +import com.github.microcatalog.utils.DependencyBuilder; import com.github.microcatalog.utils.MicroserviceBuilder; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.Test; @@ -17,8 +19,7 @@ import java.util.Arrays; import java.util.HashSet; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.entry; +import static org.assertj.core.api.Assertions.*; import static org.mockito.Mockito.spy; @ExtendWith(MockitoExtension.class) @@ -29,11 +30,37 @@ class ReleasePathAdviceTraitTest { @Mock private NativeWebRequest webRequest; + @Test + void handleDuplicateDependencyException() { + final DuplicateDependencyException exception = new DuplicateDependencyException("Test message", + new DependencyBuilder() + .withId(1L) + .withName("Test Dependency") + .withSource(1L) + .withTarget(2L) + .build()); + + ResponseEntity response = cut.handleDuplicateDependencyException(exception, webRequest); + + assertThat(response).isNotNull() + .extracting(HttpEntity::getBody).isNotNull(); + + assertThat(response.getStatusCodeValue()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY.value()); + assertThat(response.getBody()).isNotNull() + .extracting(Problem::getParameters, InstanceOfAssertFactories.map(String.class, Object.class)) + .contains( + entry(ReleasePathAdviceTrait.EXCEPTION_KEY, exception.getClass().getSimpleName()), + entry(ReleasePathAdviceTrait.MESSAGE_KEY, "Test message"), + entry("dependencyId", "1"), + entry("dependencyName", "Test Dependency") + ); + } + @Test void handleSelfCircularException() { final SelfCircularException exception = - new SelfCircularException("Test message", new MicroserviceBuilder().withId(1L).build()); + new SelfCircularException("Test message", new MicroserviceBuilder().withId(1L).withName("Test").build()); ResponseEntity response = cut.handleSelfCircularException(exception, webRequest); assertThat(response).isNotNull() @@ -44,9 +71,9 @@ void handleSelfCircularException() { assertThat(response.getBody()).isNotNull() .extracting(Problem::getParameters, InstanceOfAssertFactories.map(String.class, Object.class)) .contains( - entry(ReleasePathAdviceTrait.HEADER_KEY, exception.getClass().getName()), + entry(ReleasePathAdviceTrait.EXCEPTION_KEY, exception.getClass().getSimpleName()), entry(ReleasePathAdviceTrait.MESSAGE_KEY, "Test message"), - entry(ReleasePathAdviceTrait.PAYLOAD_KEY, "id = 1, name = null") + entry("microserviceName", "Test") ); } @@ -54,9 +81,9 @@ void handleSelfCircularException() { void handleCircularDependenciesException() { final CircularDependenciesException exception = new CircularDependenciesException("Test message", new HashSet<>(Arrays.asList( - new MicroserviceBuilder().withId(1L).build(), - new MicroserviceBuilder().withId(2L).build(), - new MicroserviceBuilder().withId(3L).build() + new MicroserviceBuilder().withId(1L).withName("First").build(), + new MicroserviceBuilder().withId(2L).withName("Second").build(), + new MicroserviceBuilder().withId(3L).withName("Third").build() ))); ResponseEntity response = cut.handleCircularDependenciesException(exception, webRequest); @@ -69,9 +96,9 @@ void handleCircularDependenciesException() { assertThat(response.getBody()).isNotNull() .extracting(Problem::getParameters, InstanceOfAssertFactories.map(String.class, Object.class)) .contains( - entry(ReleasePathAdviceTrait.HEADER_KEY, exception.getClass().getName()), + entry(ReleasePathAdviceTrait.EXCEPTION_KEY, exception.getClass().getSimpleName()), entry(ReleasePathAdviceTrait.MESSAGE_KEY, "Test message"), - entry(ReleasePathAdviceTrait.PAYLOAD_KEY, "1,2,3") + entry("microservices", "First,Second,Third") ); } }