Merge pull request #27 from KilianKrause/fix-city-view

Fix city view

coderadar-graph/src/main/java/io/reflectoring/coderadar/graph/analyzer/repository/FileRepository.java

@@ -1,7 +1,6 @@
 package io.reflectoring.coderadar.graph.analyzer.repository;
 
 import io.reflectoring.coderadar.graph.analyzer.domain.FileEntity;
-import io.reflectoring.coderadar.graph.analyzer.domain.FileToCommitRelationshipEntity;
 import io.reflectoring.coderadar.graph.analyzer.domain.MetricValueEntity;
 import java.util.List;
 import org.springframework.data.neo4j.annotation.Query;

coderadar-graph/src/main/java/io/reflectoring/coderadar/graph/analyzer/service/CommitAdapter.java

@@ -2,7 +2,6 @@
 
 import com.google.common.collect.Iterables;
 import io.reflectoring.coderadar.analyzer.domain.Commit;
-import io.reflectoring.coderadar.analyzer.domain.File;
 import io.reflectoring.coderadar.analyzer.domain.FileToCommitRelationship;
 import io.reflectoring.coderadar.graph.analyzer.domain.CommitEntity;
 import io.reflectoring.coderadar.graph.analyzer.domain.FileEntity;
@@ -31,10 +30,10 @@ public class CommitAdapter implements SaveCommitPort, UpdateCommitsPort {
 
   @Autowired
   public CommitAdapter(
-          CommitRepository commitRepository,
-          ProjectRepository projectRepository,
-          FileRepository fileRepository,
-          MetricRepository metricRepository) {
+      CommitRepository commitRepository,
+      ProjectRepository projectRepository,
+      FileRepository fileRepository,
+      MetricRepository metricRepository) {
     this.commitRepository = commitRepository;
     this.projectRepository = projectRepository;
     this.fileRepository = fileRepository;
@@ -52,10 +51,10 @@ public CommitAdapter(
   public void saveCommits(List<Commit> commits, Long projectId) {
     if (!commits.isEmpty()) {
       ProjectEntity projectEntity =
-              projectRepository
-                      .findById(projectId)
-                      .orElseThrow(() -> new ProjectNotFoundException(projectId));
-      HashMap<String,  List<FileEntity>> walkedFiles = new HashMap<>();
+          projectRepository
+              .findById(projectId)
+              .orElseThrow(() -> new ProjectNotFoundException(projectId));
+      HashMap<String, List<FileEntity>> walkedFiles = new HashMap<>();
       HashMap<String, CommitEntity> walkedCommits = new HashMap<>();
 
       commits.sort(Comparator.comparing(Commit::getTimestamp));
@@ -65,15 +64,22 @@ public void saveCommits(List<Commit> commits, Long projectId) {
 
       walkedCommits.putIfAbsent(commitEntity.getName(), commitEntity);
 
-
       List<CommitEntity> commitEntities = new ArrayList<>(walkedCommits.values());
       commitEntities.sort(Comparator.comparing(CommitEntity::getTimestamp));
-      for(CommitEntity c : commitEntities){
-        getFiles(commits.stream().filter(commit -> commit.getName().equals(c.getName())).findFirst().get().getTouchedFiles(), c, walkedFiles);
+      for (CommitEntity c : commitEntities) {
+        getFiles(
+            commits
+                .stream()
+                .filter(commit -> commit.getName().equals(c.getName()))
+                .findFirst()
+                .get()
+                .getTouchedFiles(),
+            c,
+            walkedFiles);
       }
 
       List<FileEntity> allFiles = new ArrayList<>();
-      for(List<FileEntity> files : walkedFiles.values()){
+      for (List<FileEntity> files : walkedFiles.values()) {
         allFiles.addAll(files);
       }
       projectEntity.getFiles().addAll(allFiles);
@@ -94,21 +100,21 @@ public void saveCommits(List<Commit> commits, Long projectId) {
   public void updateCommits(List<Commit> commits, Long projectId) {
     if (!commits.isEmpty()) {
       ProjectEntity projectEntity =
-              projectRepository
-                      .findById(projectId)
-                      .orElseThrow(() -> new ProjectNotFoundException(projectId));
+          projectRepository
+              .findById(projectId)
+              .orElseThrow(() -> new ProjectNotFoundException(projectId));
 
       List<CommitEntity> commitsInProject = // First we need all of the commits in the project
-              commitRepository.findByProjectId(projectId);
+          commitRepository.findByProjectId(projectId);
 
       commits.sort(Comparator.comparing(Commit::getTimestamp));
       Commit newLastCommit = commits.get(0);
       CommitEntity lastCommit =
-              commitsInProject.get(commitsInProject.size() - 1); // We grab the last one (oldest)
+          commitsInProject.get(commitsInProject.size() - 1); // We grab the last one (oldest)
       if (!lastCommit.getName().equals(newLastCommit.getName())) {
         metricRepository.deleteMetricsInCommit(
-                lastCommit
-                        .getId()); // And delete all of it's metrics, as the files this commit contains are
+            lastCommit
+                .getId()); // And delete all of it's metrics, as the files this commit contains are
         // about to change.
       }
 
@@ -117,9 +123,9 @@ public void updateCommits(List<Commit> commits, Long projectId) {
       // Get all of the files in this project and save them in a map so that we can pass it to the
       // getFiles method
       List<FileEntity> filesInProject = fileRepository.findAllinProject(projectId);
-      Map<String,  List<FileEntity>> walkedFiles = new HashMap<>();
+      Map<String, List<FileEntity>> walkedFiles = new HashMap<>();
       for (FileEntity f : filesInProject) {
-        if(walkedFiles.containsKey(f.getPath())){
+        if (walkedFiles.containsKey(f.getPath())) {
           walkedFiles.get(f.getPath()).add(f);
         } else {
           List<FileEntity> files = new ArrayList<>();
@@ -138,11 +144,11 @@ public void updateCommits(List<Commit> commits, Long projectId) {
       // See if any of the commits we had before have been deleted and delete their metrics.
       for (CommitEntity commit : commitsInProject) {
         Optional<CommitEntity> existingCommit =
-                walkedCommits
-                        .values()
-                        .stream()
-                        .filter(c -> c.getName().equals(commit.getName()))
-                        .findAny();
+            walkedCommits
+                .values()
+                .stream()
+                .filter(c -> c.getName().equals(commit.getName()))
+                .findAny();
         if (!existingCommit.isPresent()) {
           metricRepository.deleteMetricsInCommit(commit.getId());
         }
@@ -154,15 +160,23 @@ public void updateCommits(List<Commit> commits, Long projectId) {
 
       List<CommitEntity> commitEntities = new ArrayList<>(walkedCommits.values());
       commitEntities.sort(Comparator.comparing(CommitEntity::getTimestamp));
-      for(CommitEntity c : commitEntities){
-        getFiles(commits.stream().filter(commit -> commit.getName().equals(c.getName())).findFirst().get().getTouchedFiles(), c, walkedFiles);
+      for (CommitEntity c : commitEntities) {
+        getFiles(
+            commits
+                .stream()
+                .filter(commit -> commit.getName().equals(c.getName()))
+                .findFirst()
+                .get()
+                .getTouchedFiles(),
+            c,
+            walkedFiles);
       }
 
       // Save the newly created tree
       commitRepository.save(walkedCommits.values(), 1);
 
       List<FileEntity> allFiles = new ArrayList<>();
-      for(List<FileEntity> files : walkedFiles.values()){
+      for (List<FileEntity> files : walkedFiles.values()) {
         allFiles.addAll(files);
       }
       fileRepository.save(allFiles, 1);
@@ -194,9 +208,9 @@ private CommitEntity mapCommitBaseData(Commit commit, Long commitId) {
       commitEntity = new CommitEntity();
     } else {
       commitEntity =
-              commitRepository
-                      .findById(commitId)
-                      .orElseThrow(() -> new CommitNotFoundException(commitId));
+          commitRepository
+              .findById(commitId)
+              .orElseThrow(() -> new CommitNotFoundException(commitId));
     }
     commitEntity.setAnalyzed(commit.isAnalyzed());
     commitEntity.setAuthor(commit.getAuthor());
@@ -229,7 +243,7 @@ public void saveCommit(Commit commit) {
    * @return A list of fully initialized parents for the given commit.
    */
   private List<CommitEntity> findAndSaveParents(
-          Commit commit, Map<String, CommitEntity> walkedCommits) {
+      Commit commit, Map<String, CommitEntity> walkedCommits) {
     List<CommitEntity> parents = new ArrayList<>();
     for (Commit c : commit.getParents()) {
       CommitEntity commitEntity = walkedCommits.get(c.getName());
@@ -258,26 +272,25 @@ private List<CommitEntity> findAndSaveParents(
    *     mapping.
    */
   private void getFiles(
-          List<FileToCommitRelationship> relationships,
-          CommitEntity entity,
-          Map<String, List<FileEntity>> walkedFiles) {
+      List<FileToCommitRelationship> relationships,
+      CommitEntity entity,
+      Map<String, List<FileEntity>> walkedFiles) {
     for (FileToCommitRelationship fileToCommitRelationship : relationships) {
       FileToCommitRelationshipEntity fileToCommitRelationshipEntity =
-              new FileToCommitRelationshipEntity();
+          new FileToCommitRelationshipEntity();
       fileToCommitRelationshipEntity.setCommit(entity);
       fileToCommitRelationshipEntity.setChangeType(fileToCommitRelationship.getChangeType());
       fileToCommitRelationshipEntity.setOldPath(fileToCommitRelationship.getOldPath());
 
       FileEntity fileEntity;
-      List<FileEntity> fileList =
-              walkedFiles.get(fileToCommitRelationship.getFile().getPath());
+      List<FileEntity> fileList = walkedFiles.get(fileToCommitRelationship.getFile().getPath());
 
       if (fileList == null) {
         fileList = new ArrayList<>();
         fileEntity = new FileEntity();
         fileList.add(fileEntity);
       } else {
-        if((fileToCommitRelationship.getChangeType().equals(ChangeType.ADD))){
+        if ((fileToCommitRelationship.getChangeType().equals(ChangeType.ADD))) {
           fileEntity = new FileEntity();
           fileList.add(fileEntity);
         } else {
@@ -289,10 +302,10 @@ private void getFiles(
       fileEntity.setPath(fileToCommitRelationship.getFile().getPath());
       fileEntity.getCommits().add(fileToCommitRelationshipEntity);
       if (fileEntity.getId()
-              != null) { // If the file entity already exists, check if it has any metrics and attach
+          != null) { // If the file entity already exists, check if it has any metrics and attach
         // those to the commit entity.
         for (MetricValueEntity metric :
-                fileRepository.findMetricsByFileAndCommitName(fileEntity.getId(), entity.getName())) {
+            fileRepository.findMetricsByFileAndCommitName(fileEntity.getId(), entity.getName())) {
           entity.getMetricValues().add(metric);
           entity.setAnalyzed(true);
         }

coderadar-graph/src/main/java/io/reflectoring/coderadar/graph/query/service/GetMetricValuesForTwoCommitsAdapter.java

@@ -46,7 +46,13 @@ public DeltaTree get(GetMetricsForTwoCommitsCommand command, Long projectId) {
             .getTimestamp();
 
     if (commit1Time.after(commit2Time)) {
-      throw new IllegalArgumentException("commit1 cannot be newer than commit2");
+      MetricTree temp = commit1Tree;
+      commit1Tree = commit2Tree;
+      commit2Tree = temp;
+
+      Date tempDate = commit1Time;
+      commit1Time = commit2Time;
+      commit2Time = tempDate;
     }
 
     List<String> addedFiles = new ArrayList<>();

coderadar-test/src/test/java/io/reflectoring/coderadar/rest/query/GetMetricValuesOfTwoCommitsControllerTest.java

@@ -109,20 +109,56 @@ void returnsDeltaTreeForFirstAndLatestCommit() throws Exception {
     }
 
     @Test
-    void returnsErrorWhenCommit1IsAfterCommit2() throws Exception {
+    void returnsTreeWhenCommit1IsAfterCommit2() throws Exception {
         GetMetricsForTwoCommitsCommand command = new GetMetricsForTwoCommitsCommand();
         command.setMetrics(Arrays.asList("coderadar:size:loc:java", "coderadar:size:sloc:java", "coderadar:size:cloc:java", "coderadar:size:eloc:java"));
         command.setCommit1("d3272b3793bc4b2bc36a1a3a7c8293fcf8fe27df");
         command.setCommit2("fd68136dd6489504e829b11f2fce1fe97c9f5c0c");
 
         MvcResult result = mvc().perform(get("/projects/" + projectId + "/metricvalues/deltaTree")
-                .contentType(MediaType.APPLICATION_JSON).content(toJson(command)))
-                .andExpect(status().isUnprocessableEntity())
-                .andReturn();
+                .contentType(MediaType.APPLICATION_JSON).content(toJson(command))).andReturn();
 
-        ErrorMessageResponse response = fromJson(result.getResponse().getContentAsString(), ErrorMessageResponse.class);
+        DeltaTree deltaTree = fromJson(result.getResponse().getContentAsString(), DeltaTree.class);
+
+        Assertions.assertEquals("root", deltaTree.getName());
+        Assertions.assertEquals(MetricsTreeNodeType.MODULE, deltaTree.getType());
+
+        List<MetricValueForCommit> commit1Metrics = deltaTree.getCommit1Metrics();
+        List<MetricValueForCommit> commit2Metrics = deltaTree.getCommit2Metrics();
+
+        commit1Metrics.sort(Comparator.comparing(MetricValueForCommit::getMetricName));
+        commit2Metrics.sort(Comparator.comparing(MetricValueForCommit::getMetricName));
+
+        Assertions.assertEquals(0L, commit1Metrics.get(0).getValue().longValue());
+        Assertions.assertEquals(8L, commit1Metrics.get(1).getValue().longValue());
+        Assertions.assertEquals(12L, commit1Metrics.get(2).getValue().longValue());
+        Assertions.assertEquals(10L, commit1Metrics.get(3).getValue().longValue());
+
+        Assertions.assertEquals(0L, commit2Metrics.get(0).getValue().longValue());
+        Assertions.assertEquals(8L, commit2Metrics.get(1).getValue().longValue());
+        Assertions.assertEquals(18L, commit2Metrics.get(2).getValue().longValue());
+        Assertions.assertEquals(15L, commit2Metrics.get(3).getValue().longValue());
+
+        DeltaTree firstChild = deltaTree.getChildren().get(0);  // Finding.java
+        Assertions.assertEquals("Finding.java", firstChild.getName());
+        Assertions.assertEquals(MetricsTreeNodeType.FILE, firstChild.getType());
+        Assertions.assertEquals(4, firstChild.getCommit1Metrics().size());
+        Assertions.assertTrue(firstChild.getCommit2Metrics().isEmpty());
+        Assertions.assertTrue(firstChild.getChanges().isDeleted());
+
+        DeltaTree secondChild = deltaTree.getChildren().get(1); // GetMetricsForCommitCommand.java
+        Assertions.assertEquals("GetMetricsForCommitCommand.java", secondChild.getName());
+        Assertions.assertEquals(MetricsTreeNodeType.FILE, secondChild.getType());
+        Assertions.assertTrue(secondChild.getCommit1Metrics().isEmpty());
+        Assertions.assertEquals(4, secondChild.getCommit2Metrics().size());
+        Assertions.assertTrue(secondChild.getChanges().isAdded());
 
-        Assertions.assertEquals("commit1 cannot be newer than commit2", response.getErrorMessage());
+        DeltaTree thirdChild = deltaTree.getChildren().get(2);  // testModule1/NewRandomFile.java
+        Assertions.assertEquals("testModule1/NewRandomFile.java", thirdChild.getName());
+        Assertions.assertEquals(MetricsTreeNodeType.FILE, thirdChild.getType());
+        Assertions.assertTrue(thirdChild.getCommit1Metrics().isEmpty());
+        Assertions.assertEquals(4, thirdChild.getCommit2Metrics().size());
+        Assertions.assertTrue(thirdChild.getChanges().isAdded());
     }
 
     @Test

coderadar-ui/src/app/city-map/control-panel/control-panel.component.ts

@@ -9,6 +9,7 @@ import {Observable, of} from 'rxjs';
 import {Commit} from '../../model/commit';
 import {map} from 'rxjs/operators';
 import { AppEffects } from '../shared/effects';
+import {loadAvailableMetrics} from '../visualization/visualization.actions';
 
 @Component({
   selector: 'app-control-panel',
@@ -30,7 +31,6 @@ export class ControlPanelComponent implements OnInit {
   uniqueFileList$: Observable<string[]>;
 
   activeViewType$: Observable<ViewType>;
-  screenShots$: Observable<any[]>;
 
   // disable the second commit chooser for demo purposes
   disableRightSelect: true;
@@ -42,6 +42,7 @@ export class ControlPanelComponent implements OnInit {
   ngOnInit() {
     if (this.store !== undefined) {
       this.store.dispatch(loadCommits());
+      this.store.dispatch(loadAvailableMetrics());
 
       this.commits$ = this.store.select(fromRoot.getCommits).pipe(map(elements => elements.sort((a, b) => {
         if (a.timestamp === b.timestamp) {

coderadar-ui/src/app/city-map/helper/element-analyzer.ts

@@ -2,6 +2,7 @@ import {INode} from '../interfaces/INode';
 import {NodeType} from '../enum/NodeType';
 import {ScreenType} from '../enum/ScreenType';
 import {CommitReferenceType} from '../enum/CommitReferenceType';
+import {MetricValue} from "../../model/metric-value";
 
 export class ElementAnalyzer {
 
@@ -122,11 +123,13 @@ export class ElementAnalyzer {
     }
 
     if (commit1Metrics === null) {
-      return commit2Metrics[metricName];
+      return this.getValueFromMetric(commit2Metrics, metricName)
     } else if (commit2Metrics === null) {
-      return commit1Metrics[metricName];
+      return this.getValueFromMetric(commit1Metrics, metricName);
     } else {
-      return commit1Metrics[metricName] > commit2Metrics[metricName] ? commit1Metrics[metricName] : commit2Metrics[metricName];
+      let commit1 = this.getValueFromMetric(commit1Metrics, metricName), commit2 = this.getValueFromMetric(commit2Metrics, metricName);
+
+      return commit1 > commit2 ? commit1 : commit2;
     }
   }
 
@@ -172,18 +175,18 @@ export class ElementAnalyzer {
   ): number {
     if (screenType === ScreenType.LEFT) {
       if (commitReferenceType === CommitReferenceType.THIS) {
-        return node.commit1Metrics ? node.commit1Metrics[metricName] : undefined;
+        return this.getValueFromMetric(node.commit1Metrics, metricName);
       } else if (commitReferenceType === CommitReferenceType.OTHER) {
-        return node.commit2Metrics ? node.commit2Metrics[metricName] : undefined;
+        return this.getValueFromMetric(node.commit2Metrics, metricName);
       } else {
         throw new Error(`Unknown commitReferenceType ${commitReferenceType}!`);
       }
 
     } else if (screenType === ScreenType.RIGHT) {
       if (commitReferenceType === CommitReferenceType.THIS) {
-        return node.commit2Metrics ? node.commit2Metrics[metricName] : undefined;
+        return this.getValueFromMetric(node.commit2Metrics, metricName);
       } else if (commitReferenceType === CommitReferenceType.OTHER) {
-        return node.commit1Metrics ? node.commit1Metrics[metricName] : undefined;
+        return this.getValueFromMetric(node.commit1Metrics, metricName);
       } else {
         throw new Error(`Unknown commitReferenceType ${commitReferenceType}!`);
       }
@@ -193,4 +196,9 @@ export class ElementAnalyzer {
     }
   }
 
+  static getValueFromMetric(metrics: MetricValue[], metricName: String) {
+    let index = metrics ? Object.values(metrics).findIndex(object => object.metricName === metricName): -1;
+    return index >= 0 ? Number(metrics[index].value) : undefined;
+  }
+
 }