Merge branch 'stable-2.15' into stable-2.16
* stable-2.15:
Set version to 2.15.9
ListProjects: Refactor to avoid excessive heap usage
ListProjectsIT: Add test for parent candidates option
Change-Id: If133e258196163e0ce4c58fc9bcf38ef84485bb5
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index aa455e6..39475fa 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -16,6 +16,7 @@
import static java.util.stream.Collectors.toSet;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Throwables;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
@@ -308,4 +309,14 @@
}
}
}
+
+ @VisibleForTesting
+ public void evictAllByName() {
+ byName.invalidateAll();
+ }
+
+ @VisibleForTesting
+ public long sizeAllByName() {
+ return byName.size();
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/project/ListProjects.java b/java/com/google/gerrit/server/restapi/project/ListProjects.java
index e5f14064..a503323 100644
--- a/java/com/google/gerrit/server/restapi/project/ListProjects.java
+++ b/java/com/google/gerrit/server/restapi/project/ListProjects.java
@@ -16,8 +16,6 @@
import static com.google.gerrit.extensions.client.ProjectState.HIDDEN;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Objects.requireNonNull;
-import static java.util.stream.Collectors.toList;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
@@ -62,7 +60,6 @@
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
@@ -75,6 +72,7 @@
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
@@ -352,7 +350,8 @@
PermissionBackend.WithUser perm = permissionBackend.user(currentUser);
final TreeMap<Project.NameKey, ProjectNode> treeMap = new TreeMap<>();
try {
- for (Project.NameKey projectName : filter(perm)) {
+ Iterable<Project.NameKey> projectNames = filter(perm)::iterator;
+ for (Project.NameKey projectName : projectNames) {
final ProjectState e = projectCache.get(projectName);
if (e == null || (e.getProject().getState() == HIDDEN && !all && state != HIDDEN)) {
// If we can't get it from the cache, pretend it's not present.
@@ -506,55 +505,28 @@
}
}
- private Collection<Project.NameKey> filter(PermissionBackend.WithUser perm)
- throws BadRequestException, PermissionBackendException {
- Stream<Project.NameKey> matches = scan();
+ private Stream<Project.NameKey> filter(PermissionBackend.WithUser perm)
+ throws BadRequestException {
+ Stream<Project.NameKey> matches = StreamSupport.stream(scan().spliterator(), false);
if (type == FilterType.PARENT_CANDIDATES) {
- matches = parentsOf(matches);
+ matches =
+ matches.map(projectCache::get).map(this::parentOf).filter(Objects::nonNull).sorted();
}
-
- List<Project.NameKey> results = new ArrayList<>();
- List<Project.NameKey> projectNameKeys = matches.sorted().collect(toList());
- for (Project.NameKey nameKey : projectNameKeys) {
- ProjectState state = projectCache.get(nameKey);
- requireNonNull(state, () -> String.format("Failed to load project %s", nameKey));
-
- // Hidden projects(permitsRead = false) should only be accessible by the project owners.
- // READ_CONFIG is checked here because it's only allowed to project owners(ACCESS may also
- // be allowed for other users). Allowing project owners to access here will help them to view
- // and update the config of hidden projects easily.
- ProjectPermission permissionToCheck =
- state.statePermitsRead() ? ProjectPermission.ACCESS : ProjectPermission.READ_CONFIG;
- try {
- perm.project(nameKey).check(permissionToCheck);
- results.add(nameKey);
- } catch (AuthException e) {
- // Not added to results.
- }
- }
-
- return results;
+ return matches.filter(p -> perm.project(p).testOrFalse(ProjectPermission.ACCESS));
}
- private Stream<Project.NameKey> parentsOf(Stream<Project.NameKey> matches) {
- return matches
- .map(
- p -> {
- ProjectState ps = projectCache.get(p);
- if (ps != null) {
- Project.NameKey parent = ps.getProject().getParent();
- if (parent != null) {
- if (projectCache.get(parent) != null) {
- return parent;
- }
- logger.atWarning().log(
- "parent project %s of project %s not found", parent.get(), ps.getName());
- }
- }
- return null;
- })
- .filter(Objects::nonNull)
- .distinct();
+ private Project.NameKey parentOf(ProjectState ps) {
+ if (ps == null) {
+ return null;
+ }
+ Project.NameKey parent = ps.getProject().getParent();
+ if (parent != null) {
+ if (projectCache.get(parent) != null) {
+ return parent;
+ }
+ logger.atWarning().log("parent project %s of project %s not found", ps.getName());
+ }
+ return null;
}
private boolean isParentAccessible(
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
index cd88a56..8aa42a2 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ListProjectsIT.java
@@ -32,6 +32,7 @@
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.project.ProjectCacheImpl;
import com.google.gerrit.server.project.testing.Util;
import java.util.List;
import java.util.Map;
@@ -87,6 +88,7 @@
@Test
public void listProjectsWithLimit() throws Exception {
+ ProjectCacheImpl projectCacheImpl = (ProjectCacheImpl) projectCache;
for (int i = 0; i < 5; i++) {
createProject("someProject" + i);
}
@@ -94,9 +96,12 @@
String p = name("");
// 5, plus p which was automatically created.
int n = 6;
+ projectCacheImpl.evictAllByName();
for (int i = 1; i <= n + 2; i++) {
assertThatNameList(gApi.projects().list().withPrefix(p).withLimit(i).get())
.hasSize(Math.min(i, n));
+ assertThat(projectCacheImpl.sizeAllByName())
+ .isAtMost((long) (i + 2)); // 2 = AllProjects + AllUsers
}
}
@@ -190,6 +195,27 @@
}
@Test
+ public void listParentCandidates() throws Exception {
+ Map<String, ProjectInfo> result =
+ gApi.projects().list().withType(FilterType.PARENT_CANDIDATES).getAsMap();
+ assertThat(result).hasSize(1);
+ assertThat(result).containsKey(allProjects.get());
+
+ // Create a new project with 'project' as parent
+ Project.NameKey testProject = createProject(name("test"), project);
+
+ // Parent candidates are All-Projects and 'project'
+ assertThatNameList(filter(gApi.projects().list().withType(FilterType.PARENT_CANDIDATES).get()))
+ .containsExactly(allProjects, project)
+ .inOrder();
+
+ // All projects are listed
+ assertThatNameList(filter(gApi.projects().list().get()))
+ .containsExactly(allProjects, allUsers, testProject, project)
+ .inOrder();
+ }
+
+ @Test
public void listWithHiddenAndReadonlyProjects() throws Exception {
Project.NameKey hidden = createProject("project-to-hide");
Project.NameKey readonly = createProject("project-to-read");