opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.15'

Browse Source 
* stable-2.15:
  ForcePushIT#deleteAllowedWithDeletePermission: Fix permission setting
  PermissionBackend#filter: Don't fail if project doesn't exist
  Reload actions when new change is loaded, not on new changeNum
  Fix lint errors
  Filter out changes for vanished projects
  Propagate cause of a project not found in cache

Change-Id: I595235f32993802bfc1f27034734abdd4508ce91
This commit is contained in:
Luca Milanesio
2018-04-24 10:18:37 +01:00
parent bdb1213b37 f90efd8cf0
commit 4bc31b3206
7 changed files with 56 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
java/com/google/gerrit/server/permissions/DefaultPermissionBackend.java
View File

@@ -32,13 +32,11 @@ import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PeerDaemonUser;
import com.google.gerrit.server.account.CapabilityCollection;
import com.google.gerrit.server.cache.PerThreadCache;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collection;
import java.util.EnumSet;
import java.util.List;
@@ -108,16 +106,15 @@ public class DefaultPermissionBackend extends PermissionBackend {
public ForProject project(Project.NameKey project) {
try {
ProjectState state = projectCache.checkedGet(project);
if (state != null) {
ProjectControl control =
PerThreadCache.getOrCompute(
PerThreadCache.Key.create(ProjectControl.class, project, user.getCacheKey()),
() -> projectControlFactory.create(user, state));
return control.asForProject().database(db);
}
return FailedPermissionBackend.project("not found", new NoSuchProjectException(project));
} catch (IOException e) {
return FailedPermissionBackend.project("unavailable", e);
} catch (Exception e) {
Throwable cause = e.getCause() != null ? e.getCause() : e;
return FailedPermissionBackend.project(
"project '" + project.get() + "' is unavailable", cause);
}
}

8
java/com/google/gerrit/server/permissions/PermissionBackend.java
View File

@@ -41,6 +41,7 @@ import java.util.EnumSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
@@ -280,6 +281,13 @@ public abstract class PermissionBackend {
allowed.add(project);
} catch (AuthException e) {
// Do not include this project in allowed.
} catch (PermissionBackendException e) {
if (e.getCause() instanceof RepositoryNotFoundException) {
logger.warn("Could not find repository of the project {} : ", project.get(), e);
// Do not include this project because doesn't exist
} else {
throw e;
}
}
}
return allowed;

11
java/com/google/gerrit/server/project/ProjectCache.java
View File

@@ -48,6 +48,17 @@ public interface ProjectCache {
*/
ProjectState checkedGet(@Nullable Project.NameKey projectName) throws IOException;
/**
* Get the cached data for a project by its unique name.
*
* @param projectName name of the project.
* @param strict true when any error generates an exception
* @throws Exception in case of any error (strict = true) or only for I/O or other internal
* errors.
* @return the cached data or null when strict = false
*/
public ProjectState checkedGet(Project.NameKey projectName, boolean strict) throws Exception;
/**
* Invalidate the cached information about the given project, and triggers reindexing for it
*

23
java/com/google/gerrit/server/project/ProjectCacheImpl.java
View File

@@ -142,13 +142,8 @@ public class ProjectCacheImpl implements ProjectCache {
return null;
}
try {
ProjectState state = byName.get(projectName.get());
if (state != null && state.needsRefresh(clock.read())) {
byName.invalidate(projectName.get());
state = byName.get(projectName.get());
}
return state;
} catch (ExecutionException e) {
return strictCheckedGet(projectName);
} catch (Exception e) {
if (!(e.getCause() instanceof RepositoryNotFoundException)) {
log.warn(String.format("Cannot read project %s", projectName.get()), e);
Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
@@ -159,6 +154,20 @@ public class ProjectCacheImpl implements ProjectCache {
}
}
@Override
public ProjectState checkedGet(Project.NameKey projectName, boolean strict) throws Exception {
return strict ? strictCheckedGet(projectName) : checkedGet(projectName);
}
private ProjectState strictCheckedGet(Project.NameKey projectName) throws Exception {
ProjectState state = byName.get(projectName.get());
if (state != null && state.needsRefresh(clock.read())) {
byName.invalidate(projectName.get());
state = byName.get(projectName.get());
}
return state;
}
@Override
public void evict(Project p) throws IOException {
evict(p.getNameKey());

8
java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
View File

@@ -23,12 +23,12 @@ import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider;
import java.io.IOException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -89,8 +89,10 @@ public class ChangeIsVisibleToPredicate extends IsVisibleToPredicate<ChangeData>
.database(db)
.test(ChangePermission.READ);
} catch (PermissionBackendException e) {
if (e.getCause() instanceof NoSuchProjectException) {
logger.info("No such project: {}", cd.project());
Throwable cause = e.getCause();
if (cause instanceof RepositoryNotFoundException) {
logger.warn(
"Skipping change {} because the corresponding repository was not found", cd.getId(), e);
return false;
}
throw new OrmException("unable to check permissions on change " + cd.getId(), e);

2
javatests/com/google/gerrit/acceptance/git/ForcePushIT.java
View File

@@ -94,7 +94,7 @@ public class ForcePushIT extends AbstractDaemonTest {
@Test
public void deleteAllowedWithDeletePermission() throws Exception {
grant(project, "refs/*", Permission.PUSH, true);
grant(project, "refs/*", Permission.DELETE, true);
assertDeleteRef(OK);
}

6
javatests/com/google/gerrit/server/permissions/RefControlTest.java
View File

@@ -259,6 +259,12 @@ public class RefControlTest {
@Override
public void evict(Project.NameKey p) {}
@Override
public ProjectState checkedGet(Project.NameKey projectName, boolean strict)
throws Exception {
return all.get(projectName);
}
};
Injector injector = Guice.createInjector(new InMemoryModule());