From a09032b7b6888cf07478fde9b6b7e850de4e914e Mon Sep 17 00:00:00 2001 From: Jan-Willem Gmelig Meyling Date: Fri, 30 Jun 2017 13:49:29 +0200 Subject: [PATCH] Fix numbers of queries ran in the project resource --- .../server/database/controllers/Commits.java | 59 ++++++++++++------- .../repository/AbstractProjectResource.java | 4 +- .../CheckstyleWarningGeneratorTest.java | 10 +++- .../FindBugsWarningGeneratorTest.java | 5 +- .../warnings/PMDWarningGeneratorTest.java | 6 +- .../AbstractProjectResourceTest.java | 3 +- 6 files changed, 59 insertions(+), 28 deletions(-) diff --git a/src/main/java/nl/tudelft/ewi/devhub/server/database/controllers/Commits.java b/src/main/java/nl/tudelft/ewi/devhub/server/database/controllers/Commits.java index 26d53b0c..fd2ce0c8 100644 --- a/src/main/java/nl/tudelft/ewi/devhub/server/database/controllers/Commits.java +++ b/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 retrieve(RepositoryEntity repository, String commitId) { return Optional.ofNullable(entityManager.find(Commit.class, key)); } - @Transactional - public List retrieveCommits(RepositoryEntity repositoryEntity, Collection commitIds) { + private List retrieve(RepositoryEntity repositoryEntity, Collection 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 ensureExists(RepositoryEntity repositoryEntity, Collection commitIds) { + Map existingCommits = uniqueIndex(retrieve(repositoryEntity, commitIds), Commit::getCommitId); + + return commitIds.stream() + .map(commitId -> Optional.ofNullable(existingCommits.get(commitId)).orElseGet(() -> + createCommit(repositoryEntity, commitId))) + .collect(Collectors.toList()); } /** diff --git a/src/main/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResource.java b/src/main/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResource.java index 5f05d097..9a029403 100644 --- a/src/main/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResource.java +++ b/src/main/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResource.java @@ -202,9 +202,7 @@ protected Map getBranchOverviewParameters(String branchName, int CommitSubList commits = branchApi.retrieveCommitsInBranch((page - 1) * PAGE_SIZE, PAGE_SIZE); Collection commitIds = getCommitIds(commits); - List commitEntities = commitIds.stream() - .map(commitId -> this.commits.ensureExists(repositoryEntity, commitId)) - .collect(Collectors.toList()); + List commitEntities = this.commits.ensureExists(repositoryEntity, commitIds); Map commitEntitiesByCommitId = Maps.uniqueIndex(commitEntities, Commit::getCommitId); parameters.put("commits", commits); diff --git a/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/CheckstyleWarningGeneratorTest.java b/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/CheckstyleWarningGeneratorTest.java index 1f831851..ac76574b 100644 --- a/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/CheckstyleWarningGeneratorTest.java +++ b/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); diff --git a/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/FindBugsWarningGeneratorTest.java b/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/FindBugsWarningGeneratorTest.java index df9ca69c..6b5929b9 100644 --- a/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/FindBugsWarningGeneratorTest.java +++ b/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); diff --git a/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/PMDWarningGeneratorTest.java b/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/PMDWarningGeneratorTest.java index d4cf7f00..46a3a734 100644 --- a/src/test/java/nl/tudelft/ewi/devhub/server/backend/warnings/PMDWarningGeneratorTest.java +++ b/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); diff --git a/src/test/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResourceTest.java b/src/test/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResourceTest.java index ac40deb2..7f338deb 100644 --- a/src/test/java/nl/tudelft/ewi/devhub/server/web/resources/repository/AbstractProjectResourceTest.java +++ b/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,