From 58af8bd31ab5a4600573c644b28eceaea48cfbc7 Mon Sep 17 00:00:00 2001 From: tillias Date: Mon, 26 Oct 2020 18:27:25 +0100 Subject: [PATCH] Added BE validation when creating or updating dependency #54 1. For circular dependencies 2. For dependency with same source and target (self cycle) 3. For invalid situations / inputs 4. Unit tests for all the stuff --- pom.xml | 5 + .../service/custom/DependencyService.java | 122 ++++++++++ .../service/custom/GraphLoaderService.java | 42 ++++ .../custom/ReleasePathCustomService.java | 79 +++--- .../CircularDependenciesException.java | 22 ++ .../exceptions/SelfCircularException.java | 10 + .../web/rest/DependencyResource.java | 30 +-- .../microcatalog/service/GraphUtils.java | 26 ++ .../service/custom/DependencyServiceTest.java | 226 ++++++++++++++++++ .../custom/GraphLoaderServiceTest.java | 120 ++++++++++ .../custom/ReleasePathCustomServiceTest.java | 224 ++++++----------- 11 files changed, 691 insertions(+), 215 deletions(-) create mode 100644 src/main/java/com/github/microcatalog/service/custom/DependencyService.java create mode 100644 src/main/java/com/github/microcatalog/service/custom/GraphLoaderService.java create mode 100644 src/main/java/com/github/microcatalog/service/custom/exceptions/CircularDependenciesException.java create mode 100644 src/main/java/com/github/microcatalog/service/custom/exceptions/SelfCircularException.java create mode 100644 src/test/java/com/github/microcatalog/service/GraphUtils.java create mode 100644 src/test/java/com/github/microcatalog/service/custom/DependencyServiceTest.java create mode 100644 src/test/java/com/github/microcatalog/service/custom/GraphLoaderServiceTest.java diff --git a/pom.xml b/pom.xml index 89a4214..1f486e3 100644 --- a/pom.xml +++ b/pom.xml @@ -109,6 +109,11 @@ jgrapht-core ${jgrapht.version} + + org.jgrapht + jgrapht-io + ${jgrapht.version} + io.github.jhipster jhipster-framework diff --git a/src/main/java/com/github/microcatalog/service/custom/DependencyService.java b/src/main/java/com/github/microcatalog/service/custom/DependencyService.java new file mode 100644 index 0000000..05caa2c --- /dev/null +++ b/src/main/java/com/github/microcatalog/service/custom/DependencyService.java @@ -0,0 +1,122 @@ +package com.github.microcatalog.service.custom; + +import com.github.microcatalog.domain.Dependency; +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.SelfCircularException; +import com.github.microcatalog.web.rest.errors.BadRequestAlertException; +import org.jgrapht.Graph; +import org.jgrapht.alg.cycle.CycleDetector; +import org.jgrapht.graph.DefaultEdge; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; + +import javax.transaction.Transactional; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; + +@Service +@Transactional +public class DependencyService { + + private final Logger log = LoggerFactory.getLogger(DependencyService.class); + + private final GraphLoaderService graphLoaderService; + private final DependencyRepository repository; + + public DependencyService(GraphLoaderService graphLoaderService, DependencyRepository repository) { + this.graphLoaderService = graphLoaderService; + this.repository = repository; + } + + public Dependency create(final Dependency dependency) { + if (dependency.getId() != null) { + throw new BadRequestAlertException("A new dependency cannot already have an ID", getEntityName(), "idexists"); + } + + validateSelfCycle(dependency); + validateIfAdded(dependency); + + return repository.save(dependency); + } + + public Dependency update(final Dependency dependency) { + if (dependency.getId() == null) { + throw new BadRequestAlertException("Invalid id", getEntityName(), "idnull"); + } + + validateSelfCycle(dependency); + + // TODO + // FIXME + // We need second method which updates existing instead of adding new one! + validateIfUpdated(dependency); + + return repository.save(dependency); + } + + public List findAll() { + return repository.findAll(); + } + + public Optional findById(Long id) { + return repository.findById(id); + } + + public void deleteById(Long id) { + repository.deleteById(id); + } + + private String getEntityName() { + return Dependency.class.getSimpleName().toLowerCase(); + } + + private void validateSelfCycle(final Dependency dependency) { + if (dependency == null) { + return; + } + + if (dependency.getSource() == null || dependency.getTarget() == null) { + return; + } + + final Long sourceId = dependency.getSource().getId(); + if (Objects.equals(sourceId, dependency.getTarget().getId())) { + throw new SelfCircularException(String.format("Source id is the same as target id: %s", sourceId)); + } + } + + private void validateIfAdded(final Dependency toBeAdded) { + final Graph graph = graphLoaderService.loadGraph(); + graph.addEdge(toBeAdded.getSource(), toBeAdded.getTarget()); + + checkCycles(graph); + } + + private void validateIfUpdated(final Dependency dependency) { + final Graph graph = graphLoaderService.loadGraph(); + final Dependency persistent = repository.findById(dependency.getId()) + .orElseThrow(() -> new IllegalArgumentException("Trying to update dependency, but it was removed from data source")); + + // check what will be if we remove existing edge and replace it with updated one + + final DefaultEdge currentEdge = graph.getEdge(persistent.getSource(), persistent.getTarget()); + graph.removeEdge(currentEdge); + + graph.addEdge(dependency.getSource(), dependency.getTarget()); + + checkCycles(graph); + } + + private void checkCycles(final Graph graph) { + final CycleDetector cycleDetector = new CycleDetector<>(graph); + if (cycleDetector.detectCycles()) { + final Set cycles = cycleDetector.findCycles(); + throw new CircularDependenciesException("Circular dependency will be introduced.", cycles); + } + } +} diff --git a/src/main/java/com/github/microcatalog/service/custom/GraphLoaderService.java b/src/main/java/com/github/microcatalog/service/custom/GraphLoaderService.java new file mode 100644 index 0000000..d0ff6d9 --- /dev/null +++ b/src/main/java/com/github/microcatalog/service/custom/GraphLoaderService.java @@ -0,0 +1,42 @@ +package com.github.microcatalog.service.custom; + +import com.github.microcatalog.domain.Dependency; +import com.github.microcatalog.domain.Microservice; +import com.github.microcatalog.repository.DependencyRepository; +import com.github.microcatalog.repository.MicroserviceRepository; +import org.jgrapht.Graph; +import org.jgrapht.graph.DefaultDirectedGraph; +import org.jgrapht.graph.DefaultEdge; +import org.springframework.stereotype.Service; + +import javax.transaction.Transactional; +import java.util.List; + +@Service +@Transactional +public class GraphLoaderService { + + private final MicroserviceRepository microserviceRepository; + private final DependencyRepository dependencyRepository; + + public GraphLoaderService(MicroserviceRepository microserviceRepository, DependencyRepository dependencyRepository) { + this.microserviceRepository = microserviceRepository; + this.dependencyRepository = dependencyRepository; + } + + public Graph loadGraph() { + List microservices = microserviceRepository.findAll(); + List dependencies = dependencyRepository.findAll(); + + return createGraph(microservices, dependencies); + } + + private Graph createGraph(final List nodes, final List edges) { + final Graph graph = new DefaultDirectedGraph<>(DefaultEdge.class); + + nodes.forEach(graph::addVertex); + edges.forEach(d -> graph.addEdge(d.getSource(), d.getTarget())); + + return graph; + } +} diff --git a/src/main/java/com/github/microcatalog/service/custom/ReleasePathCustomService.java b/src/main/java/com/github/microcatalog/service/custom/ReleasePathCustomService.java index 69de34a..b60ae43 100644 --- a/src/main/java/com/github/microcatalog/service/custom/ReleasePathCustomService.java +++ b/src/main/java/com/github/microcatalog/service/custom/ReleasePathCustomService.java @@ -1,18 +1,16 @@ package com.github.microcatalog.service.custom; -import com.github.microcatalog.domain.*; +import com.github.microcatalog.domain.Microservice; import com.github.microcatalog.domain.custom.ReleaseGroup; import com.github.microcatalog.domain.custom.ReleasePath; import com.github.microcatalog.domain.custom.ReleaseStep; -import com.github.microcatalog.repository.DependencyRepository; -import com.github.microcatalog.repository.MicroserviceRepository; import org.jgrapht.Graph; import org.jgrapht.alg.connectivity.ConnectivityInspector; import org.jgrapht.alg.cycle.CycleDetector; import org.jgrapht.graph.AsSubgraph; -import org.jgrapht.graph.DefaultDirectedGraph; import org.jgrapht.graph.DefaultEdge; import org.jgrapht.graph.EdgeReversedGraph; +import org.jgrapht.nio.dot.DOTExporter; import org.jgrapht.traverse.DepthFirstIterator; import org.jgrapht.traverse.GraphIterator; import org.slf4j.Logger; @@ -20,6 +18,8 @@ import org.springframework.stereotype.Service; import javax.transaction.Transactional; +import java.io.StringWriter; +import java.io.Writer; import java.time.Instant; import java.util.*; import java.util.stream.Collectors; @@ -33,46 +33,38 @@ public class ReleasePathCustomService { private final Logger log = LoggerFactory.getLogger(ReleasePathCustomService.class); - private final MicroserviceRepository microserviceRepository; - private final DependencyRepository dependencyRepository; + private final GraphLoaderService graphLoaderService; - public ReleasePathCustomService(MicroserviceRepository microserviceRepository, - DependencyRepository dependencyRepository) { - this.microserviceRepository = microserviceRepository; - this.dependencyRepository = dependencyRepository; + public ReleasePathCustomService(GraphLoaderService graphLoaderService) { + this.graphLoaderService = graphLoaderService; } + public Optional getReleasePath(final Long microserviceId) { - final List microservices = microserviceRepository.findAll(); - final List dependencies = dependencyRepository.findAll(); - final Optional maybeTarget = - microservices.stream().filter(m -> Objects.equals(m.getId(), microserviceId)).findFirst(); + final Graph graph = graphLoaderService.loadGraph(); + final Microservice target = new Microservice(); + target.setId(microserviceId); - if (!maybeTarget.isPresent()) { + // can't build release path, cause microservice with given id is not present in graph + if (!graph.containsVertex(target)) { return Optional.empty(); } - final Graph graph = new DefaultDirectedGraph<>(DefaultEdge.class); - - microservices.forEach(m -> graph.addVertex(m.getId())); - dependencies.forEach(d -> graph.addEdge(d.getSource().getId(), d.getTarget().getId())); - - final ConnectivityInspector inspector = new ConnectivityInspector<>(graph); - final Microservice target = maybeTarget.get(); - final Set connectedSet = inspector.connectedSetOf(target.getId()); + final ConnectivityInspector inspector = new ConnectivityInspector<>(graph); + final Set connectedSet = inspector.connectedSetOf(target); // Connected subgraph, that contains target microservice - final AsSubgraph targetSubgraph = new AsSubgraph<>(graph, connectedSet); + final AsSubgraph targetSubgraph = new AsSubgraph<>(graph, connectedSet); log.debug("Connected subgraph, that contains target microservice: {}", targetSubgraph); - final CycleDetector cycleDetector = new CycleDetector<>(targetSubgraph); + final CycleDetector cycleDetector = new CycleDetector<>(targetSubgraph); if (cycleDetector.detectCycles()) { - final Set cycles = cycleDetector.findCycles(); + final Set cycles = cycleDetector.findCycles(); throw new IllegalArgumentException(String.format("There are cyclic dependencies between microservices : %s", cycles)); } - final Set pathMicroservices = new HashSet<>(); - GraphIterator iterator = new DepthFirstIterator<>(targetSubgraph, target.getId()); + final Set pathMicroservices = new HashSet<>(); + GraphIterator iterator = new DepthFirstIterator<>(targetSubgraph, target); while (iterator.hasNext()) { pathMicroservices.add(iterator.next()); } @@ -80,32 +72,28 @@ public Optional getReleasePath(final Long microserviceId) { // TODO new use-case and visualisation // For each element of pathSet calculate all nodes who depends on items from pathSet. Microservices which will be possibly affected if we build target and it's direct dependencies - final Graph pathGraph = new AsSubgraph<>(targetSubgraph, pathMicroservices); + final Graph pathGraph = new AsSubgraph<>(targetSubgraph, pathMicroservices); log.debug("Connected subgraph, which contains all paths from target microservice to it's dependencies {}", pathGraph); - final Graph reversed = new EdgeReversedGraph<>(pathGraph); - final Graph reversedCopy = new AsSubgraph<>(reversed); - - return Optional.of(convert(reversedCopy, microservices, target)); + final Graph reversed = new EdgeReversedGraph<>(pathGraph); + return Optional.of(convert(reversed, target)); } - private ReleasePath convert(final Graph graph, final List microservices, final Microservice target) { + private ReleasePath convert(final Graph graph, final Microservice target) { final ReleasePath result = new ReleasePath(); result.setCreatedOn(Instant.now()); result.setTarget(target); final List groups = new ArrayList<>(); - final Map microserviceMap = microservices.stream() - .collect(Collectors.toMap(Microservice::getId, m -> m)); do { - final List verticesWithoutIncomingEdges = graph.vertexSet().stream() + final List verticesWithoutIncomingEdges = graph.vertexSet().stream() .filter(v -> graph.incomingEdgesOf(v).isEmpty()) .collect(Collectors.toList()); log.debug("Leaves: {}", verticesWithoutIncomingEdges); final ReleaseGroup group = new ReleaseGroup(); - group.setSteps(convertSteps(microserviceMap, verticesWithoutIncomingEdges, graph)); + group.setSteps(convertSteps(verticesWithoutIncomingEdges, graph)); groups.add(group); verticesWithoutIncomingEdges.forEach(graph::removeVertex); @@ -116,23 +104,22 @@ private ReleasePath convert(final Graph graph, final List convertSteps(final Map microserviceMap, - final List microserviceIds, - final Graph graph) { + private Set convertSteps(final List verticesWithoutIncomingEdges, + final Graph graph) { final Set result = new HashSet<>(); - microserviceIds.forEach(id -> { + verticesWithoutIncomingEdges.forEach(microservice -> { final List parentWorkItems = new ArrayList<>(); - final Set outgoingEdges = graph.outgoingEdgesOf(id); + final Set outgoingEdges = graph.outgoingEdgesOf(microservice); for (DefaultEdge e : outgoingEdges) { - final Long edgeTarget = graph.getEdgeTarget(e); - parentWorkItems.add(microserviceMap.get(edgeTarget)); + final Microservice edgeTarget = graph.getEdgeTarget(e); + parentWorkItems.add(edgeTarget); } result.add( new ReleaseStep() - .workItem(microserviceMap.get(id)) + .workItem(microservice) .parentWorkItems(parentWorkItems) ); }); diff --git a/src/main/java/com/github/microcatalog/service/custom/exceptions/CircularDependenciesException.java b/src/main/java/com/github/microcatalog/service/custom/exceptions/CircularDependenciesException.java new file mode 100644 index 0000000..43b5e8d --- /dev/null +++ b/src/main/java/com/github/microcatalog/service/custom/exceptions/CircularDependenciesException.java @@ -0,0 +1,22 @@ +package com.github.microcatalog.service.custom.exceptions; + +import com.github.microcatalog.domain.Microservice; + +import java.util.Set; + +/** + * Occurs when adding new dependency will introduce cycle + */ +public class CircularDependenciesException extends RuntimeException { + private final Set cycles; + + public CircularDependenciesException(String message, Set cycles) { + super(message); + + this.cycles = cycles; + } + + public Set getCycles() { + return cycles; + } +} diff --git a/src/main/java/com/github/microcatalog/service/custom/exceptions/SelfCircularException.java b/src/main/java/com/github/microcatalog/service/custom/exceptions/SelfCircularException.java new file mode 100644 index 0000000..621011f --- /dev/null +++ b/src/main/java/com/github/microcatalog/service/custom/exceptions/SelfCircularException.java @@ -0,0 +1,10 @@ +package com.github.microcatalog.service.custom.exceptions; + +/** + * Occurs when there is attempt creating dependency with source and target pointing to the same microservice + */ +public class SelfCircularException extends RuntimeException { + public SelfCircularException(String message) { + super(message); + } +} diff --git a/src/main/java/com/github/microcatalog/web/rest/DependencyResource.java b/src/main/java/com/github/microcatalog/web/rest/DependencyResource.java index 2aec08e..4522526 100644 --- a/src/main/java/com/github/microcatalog/web/rest/DependencyResource.java +++ b/src/main/java/com/github/microcatalog/web/rest/DependencyResource.java @@ -1,9 +1,8 @@ package com.github.microcatalog.web.rest; import com.github.microcatalog.domain.Dependency; -import com.github.microcatalog.repository.DependencyRepository; -import com.github.microcatalog.web.rest.errors.BadRequestAlertException; - +import com.github.microcatalog.service.custom.DependencyService; +import com.github.microcatalog.service.custom.exceptions.SelfCircularException; import io.github.jhipster.web.util.HeaderUtil; import io.github.jhipster.web.util.ResponseUtil; import org.slf4j.Logger; @@ -34,10 +33,10 @@ public class DependencyResource { @Value("${jhipster.clientApp.name}") private String applicationName; - private final DependencyRepository dependencyRepository; + private final DependencyService dependencyService; - public DependencyResource(DependencyRepository dependencyRepository) { - this.dependencyRepository = dependencyRepository; + public DependencyResource(DependencyService dependencyService) { + this.dependencyService = dependencyService; } /** @@ -46,14 +45,13 @@ public DependencyResource(DependencyRepository dependencyRepository) { * @param dependency the dependency to create. * @return the {@link ResponseEntity} with status {@code 201 (Created)} and with body the new dependency, or with status {@code 400 (Bad Request)} if the dependency has already an ID. * @throws URISyntaxException if the Location URI syntax is incorrect. + * @throws SelfCircularException if source and target of dependency is the same microservice */ @PostMapping("/dependencies") public ResponseEntity createDependency(@Valid @RequestBody Dependency dependency) throws URISyntaxException { log.debug("REST request to save Dependency : {}", dependency); - if (dependency.getId() != null) { - throw new BadRequestAlertException("A new dependency cannot already have an ID", ENTITY_NAME, "idexists"); - } - Dependency result = dependencyRepository.save(dependency); + + Dependency result = dependencyService.create(dependency); return ResponseEntity.created(new URI("/api/dependencies/" + result.getId())) .headers(HeaderUtil.createEntityCreationAlert(applicationName, true, ENTITY_NAME, result.getId().toString())) .body(result); @@ -71,10 +69,8 @@ public ResponseEntity createDependency(@Valid @RequestBody Dependenc @PutMapping("/dependencies") public ResponseEntity updateDependency(@Valid @RequestBody Dependency dependency) throws URISyntaxException { log.debug("REST request to update Dependency : {}", dependency); - if (dependency.getId() == null) { - throw new BadRequestAlertException("Invalid id", ENTITY_NAME, "idnull"); - } - Dependency result = dependencyRepository.save(dependency); + + Dependency result = dependencyService.update(dependency); return ResponseEntity.ok() .headers(HeaderUtil.createEntityUpdateAlert(applicationName, true, ENTITY_NAME, dependency.getId().toString())) .body(result); @@ -88,7 +84,7 @@ public ResponseEntity updateDependency(@Valid @RequestBody Dependenc @GetMapping("/dependencies") public List getAllDependencies() { log.debug("REST request to get all Dependencies"); - return dependencyRepository.findAll(); + return dependencyService.findAll(); } /** @@ -100,7 +96,7 @@ public List getAllDependencies() { @GetMapping("/dependencies/{id}") public ResponseEntity getDependency(@PathVariable Long id) { log.debug("REST request to get Dependency : {}", id); - Optional dependency = dependencyRepository.findById(id); + Optional dependency = dependencyService.findById(id); return ResponseUtil.wrapOrNotFound(dependency); } @@ -113,7 +109,7 @@ public ResponseEntity getDependency(@PathVariable Long id) { @DeleteMapping("/dependencies/{id}") public ResponseEntity deleteDependency(@PathVariable Long id) { log.debug("REST request to delete Dependency : {}", id); - dependencyRepository.deleteById(id); + dependencyService.deleteById(id); return ResponseEntity.noContent().headers(HeaderUtil.createEntityDeletionAlert(applicationName, true, ENTITY_NAME, id.toString())).build(); } } diff --git a/src/test/java/com/github/microcatalog/service/GraphUtils.java b/src/test/java/com/github/microcatalog/service/GraphUtils.java new file mode 100644 index 0000000..948473d --- /dev/null +++ b/src/test/java/com/github/microcatalog/service/GraphUtils.java @@ -0,0 +1,26 @@ +package com.github.microcatalog.service; + +import com.github.microcatalog.domain.Microservice; +import com.github.microcatalog.utils.MicroserviceBuilder; +import org.jgrapht.Graph; +import org.jgrapht.graph.DefaultDirectedGraph; +import org.jgrapht.graph.DefaultEdge; +import org.jgrapht.nio.dot.DOTImporter; + +import java.io.StringReader; + +/** + * Test utils for graph manipulation + */ +public class GraphUtils { + public static Graph createGraph(final String dot) { + final DOTImporter importer = new DOTImporter<>(); + importer.setVertexFactory(s -> new MicroserviceBuilder().withId(Long.valueOf(s)).build()); + + final Graph graph = new DefaultDirectedGraph<>(DefaultEdge.class); + + importer.importGraph(graph, new StringReader(dot)); + + return graph; + } +} diff --git a/src/test/java/com/github/microcatalog/service/custom/DependencyServiceTest.java b/src/test/java/com/github/microcatalog/service/custom/DependencyServiceTest.java new file mode 100644 index 0000000..10b0783 --- /dev/null +++ b/src/test/java/com/github/microcatalog/service/custom/DependencyServiceTest.java @@ -0,0 +1,226 @@ +package com.github.microcatalog.service.custom; + +import com.github.microcatalog.domain.Dependency; +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.SelfCircularException; +import com.github.microcatalog.utils.DependencyBuilder; +import com.github.microcatalog.web.rest.errors.BadRequestAlertException; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; + +import java.util.Collections; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.then; + +@SpringBootTest(classes = DependencyService.class) +class DependencyServiceTest { + + @MockBean + private DependencyRepository repository; + + @MockBean + private GraphLoaderService graphLoaderService; + + @Autowired + private DependencyService service; + + @Test + void findAll() { + given(repository.findAll()).willReturn(Collections.emptyList()); + + service.findAll(); + + then(repository).should().findAll(); + } + + @Test + void findById() { + final long id = 3L; + + given(repository.findById(id)).willReturn(Optional.of(new DependencyBuilder().withId(id).build())); + + Optional maybeDependency = service.findById(id); + assertThat(maybeDependency).isPresent(); + assertThat(maybeDependency.get()).extracting(Dependency::getId).isEqualTo(id); + + then(repository).should().findById(id); + } + + @Test + void deleteById() { + final long id = 3L; + + service.deleteById(id); + + then(repository).should().deleteById(id); + } + + @Test + void create_WithId_ExceptionIsThrown() { + assertThatThrownBy(() -> service.create(new DependencyBuilder().withId(1L).build())) + .isInstanceOf(BadRequestAlertException.class) + .hasMessageStartingWith("A new dependency cannot already have an ID"); + } + + @Test + void create_SelfCycle_ExceptionIsThrown() { + assertThatThrownBy(() -> service.create( + new DependencyBuilder() + .withSource(2L) + .withTarget(2L) + .build() + )) + .isInstanceOf(SelfCircularException.class) + .hasMessageStartingWith("Source id is the same as target id"); + } + + @Test + void create_WillIntroduceCircularDependencies_ExceptionIsThrown() { + given(graphLoaderService.loadGraph()).willReturn( + GraphUtils.createGraph( + String.join("\n", + "strict digraph G { ", + "1; 2; 3; 4;", + "1 -> 2;", + "2 -> 3;", + "3 -> 4;}" + ) + ) + ); + + assertThatThrownBy(() -> service.create( + new DependencyBuilder() + .withSource(4L) + .withTarget(1L) + .build() + )) + .isInstanceOf(CircularDependenciesException.class) + .hasMessageStartingWith("Circular dependency will be introduced."); + } + + @Test + void create_Success() { + given(graphLoaderService.loadGraph()).willReturn( + GraphUtils.createGraph( + String.join("\n", + "strict digraph G { ", + "1; 2; 3; 4;", + "1 -> 2;", + "2 -> 3;}" + ) + ) + ); + + final Dependency expected = new DependencyBuilder() + .withId(1L) + .withSource(3L) + .withTarget(4L) + .build(); + + given(repository.save(any(Dependency.class))).willReturn(expected); + + Dependency persistent = service.create( + new DependencyBuilder() + .withSource(3L) + .withTarget(4L) + .build()); + + assertThat(persistent).isEqualTo(expected); + + then(repository).should().save(any(Dependency.class)); + } + + @Test + void update_NoId_ExceptionIsThrown() { + assertThatThrownBy(() -> service.update(new DependencyBuilder().withSource(1L).withTarget(2L).build())) + .isInstanceOf(BadRequestAlertException.class) + .hasMessageStartingWith("Invalid id"); + } + + @Test + void update_SelfCycle_ExceptionIsThrown() { + assertThatThrownBy(() -> service.update( + new DependencyBuilder() + .withId(1L) + .withSource(2L) + .withTarget(2L) + .build() + )) + .isInstanceOf(SelfCircularException.class) + .hasMessageStartingWith("Source id is the same as target id"); + } + + @Test + void update_WillIntroduceCircularDependencies_ExceptionIsThrown() { + given(graphLoaderService.loadGraph()).willReturn( + GraphUtils.createGraph( + String.join("\n", + "strict digraph G { ", + "1; 2; 3; 4;", + "1 -> 2;", + "2 -> 3;", + "3 -> 4;}" // has id = 3L + ) + ) + ); + + given(repository.findById(3L)) + .willReturn(Optional.of(new DependencyBuilder().withId(3L).withSource(3L).withTarget(4L).build())); + + assertThatThrownBy(() -> service.update( + new DependencyBuilder() + .withId(3L) + .withSource(3L) + .withTarget(1L) + .build() + )) + .isInstanceOf(CircularDependenciesException.class) + .hasMessageStartingWith("Circular dependency will be introduced."); + } + + @Test + void update_Success() { + given(graphLoaderService.loadGraph()).willReturn( + GraphUtils.createGraph( + String.join("\n", + "strict digraph G { ", + "1; 2; 3; 4;", + "1 -> 2;", + "2 -> 3;", // has it 2L + "3 -> 4;}" + ) + ) + ); + + given(repository.findById(2L)) + .willReturn(Optional.of(new DependencyBuilder().withId(2L).withSource(2L).withTarget(3L).build())); + + final Dependency expected = new DependencyBuilder() + .withId(2L) + .withSource(2L) + .withTarget(4L) + .build(); + + given(repository.save(any(Dependency.class))).willReturn(expected); + + Dependency persistent = service.update( + new DependencyBuilder() + .withId(2L) + .withSource(2L) + .withTarget(4L) + .build()); + + assertThat(persistent).isEqualTo(expected); + + then(repository).should().save(any(Dependency.class)); + } +} diff --git a/src/test/java/com/github/microcatalog/service/custom/GraphLoaderServiceTest.java b/src/test/java/com/github/microcatalog/service/custom/GraphLoaderServiceTest.java new file mode 100644 index 0000000..ea75043 --- /dev/null +++ b/src/test/java/com/github/microcatalog/service/custom/GraphLoaderServiceTest.java @@ -0,0 +1,120 @@ +package com.github.microcatalog.service.custom; + +import com.github.microcatalog.domain.Dependency; +import com.github.microcatalog.domain.Microservice; +import com.github.microcatalog.repository.DependencyRepository; +import com.github.microcatalog.repository.MicroserviceRepository; +import com.github.microcatalog.utils.DependencyBuilder; +import com.github.microcatalog.utils.MicroserviceBuilder; +import org.jgrapht.Graph; +import org.jgrapht.graph.DefaultEdge; +import org.jgrapht.nio.dot.DOTExporter; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; + +import java.io.StringWriter; +import java.io.Writer; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.LongStream; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.then; + +@SpringBootTest(classes = GraphLoaderService.class) +public class GraphLoaderServiceTest { + + @MockBean + private MicroserviceRepository microserviceRepository; + + @MockBean + private DependencyRepository dependencyRepository; + + @Autowired + private GraphLoaderService sut; + + @Test + void loadEmptyGraph() { + given(microserviceRepository.findAll()).willReturn(Collections.emptyList()); + given(dependencyRepository.findAll()).willReturn(Collections.emptyList()); + + Graph graph = sut.loadGraph(); + assertThat(graph).isNotNull(); + assertThat(graph.vertexSet()).hasSize(0); + assertThat(graph.edgeSet()).hasSize(0); + + then(microserviceRepository).should().findAll(); + then(dependencyRepository).should().findAll(); + } + + @Test + void loadSampleGraph() { + final List microservices = createMicroservices(); + given(microserviceRepository.findAll()).willReturn(microservices); + given(dependencyRepository.findAll()).willReturn(createDependenciesWithCycleInSameComponent()); + + Graph graph = sut.loadGraph(); + assertThat(graph).isNotNull(); + assertThat(graph.vertexSet()).containsExactlyInAnyOrder(microservices.toArray(new Microservice[0])); + + Set expectedEdges = new HashSet<>(); + expectedEdges.add(graph.getEdge(microservices.get(0), microservices.get(1))); + expectedEdges.add(graph.getEdge(microservices.get(1), microservices.get(2))); + expectedEdges.add(graph.getEdge(microservices.get(2), microservices.get(0))); + expectedEdges.add(graph.getEdge(microservices.get(3), microservices.get(4))); + expectedEdges.add(graph.getEdge(microservices.get(4), microservices.get(5))); + + assertThat(graph.edgeSet()).containsExactlyInAnyOrder(expectedEdges.toArray(new DefaultEdge[0])); + + then(microserviceRepository).should().findAll(); + then(dependencyRepository).should().findAll(); + } + + /** + * Graph will contain 6 vertices: 1..6 + * + * @return microservices + */ + private List createMicroservices() { + return LongStream.rangeClosed(0, 5) + .mapToObj(i -> new MicroserviceBuilder().withId(i).build()) + .collect(Collectors.toList()); + } + + /** + * Graph will contain two connected components, one of them has cycle + *

+ * First component with cycle 0->1->2->0 + * Second component without cycle 3->4, 4->5 + * + * @return dependencies + */ + private List createDependenciesWithCycleInSameComponent() { + final List dependencies = new ArrayList<>(); + + // First component with cycle 0->1->2->0 + dependencies.add(new DependencyBuilder() + .withId(1L).withSource(0L).withTarget(1L) + .build()); + dependencies.add(new DependencyBuilder() + .withId(2L).withSource(1L).withTarget(2L) + .build()); + dependencies.add(new DependencyBuilder() + .withId(3L).withSource(2L).withTarget(0L) + .build()); + + // Second component without cycle 3->4, 4->5 + dependencies.add(new DependencyBuilder() + .withId(4L).withSource(3L).withTarget(4L) + .build()); + dependencies.add(new DependencyBuilder() + .withId(5L).withSource(4L).withTarget(5L) + .build()); + + + return dependencies; + } +} diff --git a/src/test/java/com/github/microcatalog/service/custom/ReleasePathCustomServiceTest.java b/src/test/java/com/github/microcatalog/service/custom/ReleasePathCustomServiceTest.java index dc8f795..f9e09c4 100644 --- a/src/test/java/com/github/microcatalog/service/custom/ReleasePathCustomServiceTest.java +++ b/src/test/java/com/github/microcatalog/service/custom/ReleasePathCustomServiceTest.java @@ -1,43 +1,70 @@ package com.github.microcatalog.service.custom; -import com.github.microcatalog.domain.Dependency; import com.github.microcatalog.domain.Microservice; import com.github.microcatalog.domain.custom.ReleaseGroup; import com.github.microcatalog.domain.custom.ReleasePath; import com.github.microcatalog.domain.custom.ReleaseStep; -import com.github.microcatalog.repository.DependencyRepository; -import com.github.microcatalog.repository.MicroserviceRepository; -import com.github.microcatalog.utils.DependencyBuilder; -import com.github.microcatalog.utils.MicroserviceBuilder; +import com.github.microcatalog.service.GraphUtils; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; -import java.util.*; -import java.util.stream.Collectors; -import java.util.stream.LongStream; +import java.util.Collections; +import java.util.Optional; +import java.util.Set; -import static org.assertj.core.api.Assertions.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.mockito.BDDMockito.given; -@SpringBootTest(classes = ReleasePathCustomService.class) +@SpringBootTest(classes = {ReleasePathCustomService.class}) class ReleasePathCustomServiceTest { @MockBean - private MicroserviceRepository microserviceRepository; - - @MockBean - private DependencyRepository dependencyRepository; + private GraphLoaderService graphLoaderService; @Autowired private ReleasePathCustomService service; @Test - void getReleasePath_NoCycles_Success() { - given(microserviceRepository.findAll()).willReturn(createMicroservices()); + void getReleasePath_NodeOutsideGraph_EmptyPath() { + given(graphLoaderService.loadGraph()) + .willReturn( + GraphUtils.createGraph(String.join("\n", + "strict digraph G { ", + "1; 2; 3;", + "1 -> 2;", + "2 -> 3;}" + ) + ) + ); + + Optional maybePath = service.getReleasePath(4L); + assertThat(maybePath).isEmpty(); + } - given(dependencyRepository.findAll()).willReturn(createDependencies()); + @Test + void getReleasePath_NoCycles_Success() { + given(graphLoaderService.loadGraph()) + .willReturn( + GraphUtils.createGraph(String.join("\n", + "strict digraph G { ", + "1; 2; 3; 4; 5; 6; 7; 8; 9; 10; 11; 12;", + "1 -> 2;", + "2 -> 4;", + "6 -> 4;", + "4 -> 5;", + "4 -> 7;", + "4 -> 8;", + "3 -> 9;", + "3 -> 11;", + "12 -> 1;", + "7 -> 5;", + "10 -> 1;}" + ) + ) + ); Optional maybePath = service.getReleasePath(1L); assertThat(maybePath).isPresent(); @@ -71,9 +98,19 @@ void getReleasePath_NoCycles_Success() { @Test void getReleasePath_ContainsCyclesInSameComponent_ExceptionIsThrown() { - given(microserviceRepository.findAll()).willReturn(createMicroservices()); - - given(dependencyRepository.findAll()).willReturn(createDependenciesWithCycleInSameComponent()); + given(graphLoaderService.loadGraph()) + .willReturn( + GraphUtils.createGraph(String.join("\n", + "strict digraph G { ", + "1; 2; 3; 5; 6; 7;", + "1 -> 2;", + "2 -> 3;", + "3 -> 1;", + "5 -> 6;", + "5 -> 7;}" + ) + ) + ); assertThatIllegalArgumentException() .isThrownBy(() -> @@ -84,9 +121,21 @@ void getReleasePath_ContainsCyclesInSameComponent_ExceptionIsThrown() { @Test void getReleasePath_ContainsCyclesInOtherComponent_Success() { - given(microserviceRepository.findAll()).willReturn(createMicroservices()); - - given(dependencyRepository.findAll()).willReturn(createDependenciesWithCycleInOtherComponent()); + given(graphLoaderService.loadGraph()) + .willReturn( + GraphUtils.createGraph(String.join("\n", + "strict digraph G { ", + "1; 2; 3; 4; 5; 6; 7; 8;", + "1 -> 2;", + "2 -> 3;", + "2 -> 4;", + "5 -> 6;", + "6 -> 7;", + "7 -> 8;", + "8 -> 6;}" + ) + ) + ); Optional maybePath = service.getReleasePath(1L); assertThat(maybePath).isPresent(); @@ -139,133 +188,4 @@ private Set getSteps(ReleasePath path, int groupIndex) { return Collections.emptySet(); } } - - private List createMicroservices() { - return LongStream.rangeClosed(1, 12) - .mapToObj(i -> new MicroserviceBuilder().withId(i).build()) - .collect(Collectors.toList()); - } - - /** - * Graph will contain two connected components, one of them has cycle - *

- * First component with cycle 1->2->3->1 - * Second component without cycle 5->6, 5->7 - * - * @return dependencies - */ - private List createDependenciesWithCycleInSameComponent() { - final List dependencies = new ArrayList<>(); - - // First component with cycle 1->2->3->1 - dependencies.add(new DependencyBuilder() - .withId(1L).withSource(1L).withTarget(2L) - .build()); - dependencies.add(new DependencyBuilder() - .withId(2L).withSource(2L).withTarget(3L) - .build()); - dependencies.add(new DependencyBuilder() - .withId(3L).withSource(3L).withTarget(1L) - .build()); - - // Second component without cycle 5->6, 5->7 - dependencies.add(new DependencyBuilder() - .withId(4L).withSource(5L).withTarget(6L) - .build()); - dependencies.add(new DependencyBuilder() - .withId(5L).withSource(5L).withTarget(7L) - .build()); - - - return dependencies; - } - - /** - * Graph will contain two connected components, one of them has cycle - * First component without cycle 1->2, 2->3, 2->4 - * Second component with cycle 6->7->8->6 - * - * @return dependencies - */ - private List createDependenciesWithCycleInOtherComponent() { - final List dependencies = new ArrayList<>(); - - // First component without cycle 1->2, 2->3, 2->4 - dependencies.add(new DependencyBuilder() - .withId(1L).withSource(1L).withTarget(2L) - .build()); - dependencies.add(new DependencyBuilder() - .withId(2L).withSource(2L).withTarget(3L) - .build()); - dependencies.add(new DependencyBuilder() - .withId(3L).withSource(2L).withTarget(4L) - .build()); - - // Second component with cycle 6->7->8->6 - dependencies.add(new DependencyBuilder() - .withId(4L).withSource(5L).withTarget(6L) - .build()); - dependencies.add(new DependencyBuilder() - .withId(5L).withSource(6L).withTarget(7L) - .build()); - dependencies.add(new DependencyBuilder() - .withId(6L).withSource(7L).withTarget(8L) - .build()); - dependencies.add(new DependencyBuilder() - .withId(7L).withSource(8L).withTarget(6L) - .build()); - - - return dependencies; - } - - private List createDependencies() { - final List dependencies = new ArrayList<>(); - - dependencies.add(new DependencyBuilder() - .withId(1L).withSource(1L).withTarget(2L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(2L).withSource(2L).withTarget(4L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(3L).withSource(6L).withTarget(4L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(4L).withSource(4L).withTarget(5L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(5L).withSource(4L).withTarget(7L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(6L).withSource(4L).withTarget(8L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(7L).withSource(3L).withTarget(9L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(8L).withSource(3L).withTarget(11L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(9L).withSource(12L).withTarget(1L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(10L).withSource(7L).withTarget(5L) - .build()); - - dependencies.add(new DependencyBuilder() - .withId(11L).withSource(10L).withTarget(1L) - .build()); - - return dependencies; - } }