Fix numbers of queries ran in the project resource, fixes #488

src/main/java/nl/tudelft/ewi/devhub/server/database/controllers/Commits.java

@@ -1,6 +1,7 @@
 package nl.tudelft.ewi.devhub.server.database.controllers;
 
 import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.common.eventbus.EventBus;
 import com.google.inject.Inject;
 import com.google.inject.persist.Transactional;
@@ -14,13 +15,11 @@
 import nl.tudelft.ewi.git.web.api.RepositoriesApi;
 
 import javax.persistence.EntityManager;
-import java.util.Collection;
-import java.util.Date;
-import java.util.List;
-import java.util.Optional;
+import java.util.*;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static com.google.common.collect.Maps.uniqueIndex;
 import static nl.tudelft.ewi.devhub.server.database.entities.QCommit.commit;
 
 @Slf4j
@@ -42,36 +41,54 @@ public Optional<Commit> retrieve(RepositoryEntity repository, String commitId) {
 		return Optional.ofNullable(entityManager.find(Commit.class, key));
 	}
 
-	@Transactional
-	public List<Commit> retrieveCommits(RepositoryEntity repositoryEntity, Collection<String> commitIds) {
+	private List<Commit> retrieve(RepositoryEntity repositoryEntity, Collection<String> commitIds) {
 		return query().from(commit)
 			.where(commit.repository.eq(repositoryEntity).and(commit.commitId.in(commitIds)))
 			.list(commit);
 	}
 
+	protected Commit createCommit(RepositoryEntity repositoryEntity, String commitId) {
+		final Commit commit = new Commit();
+		commit.setCommitId(commitId);
+		commit.setRepository(repositoryEntity);
+		commit.setComments(Lists.newArrayList());
+		commit.setPushTime(new Date());
+		enhanceCommitSafely(commit);
+
+		CreateCommitEvent createCommitEvent = new CreateCommitEvent();
+		createCommitEvent.setCommitId(commitId);
+		createCommitEvent.setRepositoryName(repositoryEntity.getRepositoryName());
+		eventBus.post(createCommitEvent);
+		return persist(commit);
+	}
+
 	/**
 	 * Ensure that a commit exists in the database. Recursively check if the parents exists as well.
 	 *
 	 * @param repositoryEntity Repository to search commits for.
 	 * @param commitId Commit id of the commit.
-	 * @return The created commit entity.
+	 * @return The existing or created commit entity.
 	 */
 	@Transactional
 	public Commit ensureExists(RepositoryEntity repositoryEntity, String commitId) {
-		return retrieve(repositoryEntity, commitId).orElseGet(() -> {
-			final Commit commit = new Commit();
-			commit.setCommitId(commitId);
-			commit.setRepository(repositoryEntity);
-			commit.setComments(Lists.newArrayList());
-			commit.setPushTime(new Date());
-			enhanceCommitSafely(commit);
-
-			CreateCommitEvent createCommitEvent = new CreateCommitEvent();
-			createCommitEvent.setCommitId(commitId);
-			createCommitEvent.setRepositoryName(repositoryEntity.getRepositoryName());
-			eventBus.post(createCommitEvent);
-			return persist(commit);
-		});
+		return retrieve(repositoryEntity, commitId).orElseGet(() -> createCommit(repositoryEntity, commitId));
+	}
+
+	/**
+	 * Ensure that a commit exists in the database. Recursively check if the parents exists as well.
+	 *
+	 * @param repositoryEntity Repository to search commits for.
+	 * @param commitIds Commit ids of the commit.
+	 * @return The existing or created commit entities.
+	 */
+	@Transactional
+	public List<Commit> ensureExists(RepositoryEntity repositoryEntity, Collection<String> commitIds) {
+		Map<String, Commit> existingCommits = uniqueIndex(retrieve(repositoryEntity, commitIds), Commit::getCommitId);
+
+		return commitIds.stream()
+			.map(commitId -> Optional.ofNullable(existingCommits.get(commitId)).orElseGet(() ->
+						createCommit(repositoryEntity, commitId)))
+			.collect(Collectors.toList());
 	}
 
 	/**

src/main/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResource.java

@@ -202,9 +202,7 @@ protected Map<String, Object> getBranchOverviewParameters(String branchName, int
 			CommitSubList commits = branchApi.retrieveCommitsInBranch((page - 1) * PAGE_SIZE, PAGE_SIZE);
 
 			Collection<String> commitIds = getCommitIds(commits);
-			List<Commit> commitEntities = commitIds.stream()
-				.map(commitId -> this.commits.ensureExists(repositoryEntity, commitId))
-				.collect(Collectors.toList());
+			List<Commit> commitEntities = this.commits.ensureExists(repositoryEntity, commitIds);
 			Map<String, Commit> commitEntitiesByCommitId = Maps.uniqueIndex(commitEntities, Commit::getCommitId);
 
 			parameters.put("commits", commits);

src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/CheckstyleWarningGeneratorTest.java

@@ -6,6 +6,7 @@
 import nl.tudelft.ewi.devhub.server.database.entities.Commit;
 import nl.tudelft.ewi.devhub.server.database.entities.Group;
 import nl.tudelft.ewi.devhub.server.database.entities.GroupRepository;
+import nl.tudelft.ewi.devhub.server.database.entities.RepositoryEntity;
 import nl.tudelft.ewi.devhub.server.database.entities.warnings.CheckstyleWarning;
 import nl.tudelft.ewi.git.models.BlameModel;
 
@@ -24,14 +25,21 @@
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.util.Collections;
 import java.util.Set;
 
+import static java.util.Collections.singletonList;
 import static org.hamcrest.Matchers.contains;
 import static org.junit.Assert.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyList;
+import static org.mockito.ArgumentMatchers.anyListOf;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyString;
@@ -63,7 +71,7 @@ public void initializeMocks(){
         when(commit.getCommitId()).thenReturn(COMMIT_ID);
         when(commit.getRepository()).thenReturn(groupRepository);
         when(groupRepository.getRepositoryName()).thenReturn("");
-        when(commits.ensureExists(any(), any())).thenReturn(commit);
+        when(commits.ensureExists(any(RepositoryEntity.class), anyString())).thenReturn(commit);
 
         when(repositories.getRepository(anyString())).thenReturn(repository);
         when(repository.getCommit(COMMIT_ID)).thenReturn(commitApi);

src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/FindBugsWarningGeneratorTest.java

@@ -6,6 +6,7 @@
 import nl.tudelft.ewi.devhub.server.database.entities.Commit;
 import nl.tudelft.ewi.devhub.server.database.entities.Group;
 import nl.tudelft.ewi.devhub.server.database.entities.GroupRepository;
+import nl.tudelft.ewi.devhub.server.database.entities.RepositoryEntity;
 import nl.tudelft.ewi.devhub.server.database.entities.warnings.FindbugsWarning;
 import nl.tudelft.ewi.git.models.BlameModel;
 import nl.tudelft.ewi.git.models.DetailedCommitModel;
@@ -31,8 +32,10 @@
 import java.io.InputStreamReader;
 import java.util.Set;
 
+import static java.util.Collections.singletonList;
 import static org.hamcrest.Matchers.contains;
 import static org.junit.Assert.assertThat;
+import static org.mockito.ArgumentMatchers.anyList;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyString;
@@ -66,7 +69,7 @@ public void initializeMocks() {
         when(commit.getCommitId()).thenReturn(COMMIT_ID);
         when(commit.getRepository()).thenReturn(groupRepository);
         when(groupRepository.getRepositoryName()).thenReturn("");
-        when(commits.ensureExists(any(), any())).thenReturn(commit);
+        when(commits.ensureExists(any(RepositoryEntity.class), anyString())).thenReturn(commit);
 
         when(repositories.getRepository(anyString())).thenReturn(repository);
         when(repository.getCommit(COMMIT_ID)).thenReturn(commitApi);

src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/PMDWarningGeneratorTest.java

@@ -6,6 +6,7 @@
 import nl.tudelft.ewi.devhub.server.database.entities.Commit;
 import nl.tudelft.ewi.devhub.server.database.entities.Group;
 import nl.tudelft.ewi.devhub.server.database.entities.GroupRepository;
+import nl.tudelft.ewi.devhub.server.database.entities.RepositoryEntity;
 import nl.tudelft.ewi.devhub.server.database.entities.warnings.PMDWarning;
 import nl.tudelft.ewi.git.models.BlameModel;
 
@@ -30,8 +31,10 @@
 import java.io.InputStreamReader;
 import java.util.Set;
 
+import static java.util.Collections.singletonList;
 import static org.hamcrest.Matchers.hasItems;
 import static org.junit.Assert.assertThat;
+import static org.mockito.ArgumentMatchers.anyList;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.anyString;
@@ -63,7 +66,8 @@ public void initializeMocks() {
         when(commit.getCommitId()).thenReturn(COMMIT_ID);
         when(commit.getRepository()).thenReturn(groupRepository);
         when(groupRepository.getRepositoryName()).thenReturn("");
-        when(commits.ensureExists(any(), any())).thenReturn(commit);
+
+        when(commits.ensureExists(any(RepositoryEntity.class), anyString())).thenReturn(commit);
 
         when(repositories.getRepository(anyString())).thenReturn(repository);
         when(repository.getCommit(COMMIT_ID)).thenReturn(commitApi);

src/test/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResourceTest.java

@@ -38,6 +38,7 @@
 import java.util.*;
 import java.util.concurrent.Executors;
 
+import static java.util.Collections.singletonList;
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
@@ -90,7 +91,7 @@ public void setUp() throws Throwable {
         groupRepository.setRepositoryName(REPOSITORY_NAME);
         group.setRepository(groupRepository);
 
-        when(commits.ensureExists(any(), any())).thenReturn(commit);
+        when(commits.ensureExists(any(RepositoryEntity.class), anyString())).thenReturn(commit);
 
         projectResource = spy(new ProjectResource(templateEngine, currentUser, group, null, null,
                 null, repositoriesApi, null, commitComments, commentMailer, commits, null, null,