VisibleRefFilter: reuse PermissionBackend.ForProject across refs
This allows the PermissionBackend to perform caching at the project level, which may allow it to perform faster ref evaluation. The DefaultPermissionBackend for example does this with ForProject containing a ProjectControl, which caches relevant AccessSections to quickly build the RefControl that backs ForRef. Change-Id: I82c80ecb324765c7f0525b6817618790833d3948
This commit is contained in:
parent
542ed2d07f
commit
d46fb98646
@ -77,6 +77,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
private final Provider<ReviewDb> db;
|
||||
private final Provider<CurrentUser> user;
|
||||
private final PermissionBackend permissionBackend;
|
||||
private final PermissionBackend.ForProject perm;
|
||||
private final ProjectState projectState;
|
||||
private final Repository git;
|
||||
private ProjectControl projectCtl;
|
||||
@ -100,6 +101,8 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
this.db = db;
|
||||
this.user = user;
|
||||
this.permissionBackend = permissionBackend;
|
||||
this.perm =
|
||||
permissionBackend.user(user).database(db).project(projectState.getProject().getNameKey());
|
||||
this.projectState = projectState;
|
||||
this.git = git;
|
||||
}
|
||||
@ -265,31 +268,25 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
}
|
||||
|
||||
private Map<Change.Id, Branch.NameKey> visibleChangesBySearch() {
|
||||
Project project = projectCtl.getProject();
|
||||
Project.NameKey project = projectState.getProject().getNameKey();
|
||||
try {
|
||||
Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>();
|
||||
for (ChangeData cd : changeCache.getChangeData(db.get(), project.getNameKey())) {
|
||||
if (permissionBackend
|
||||
.user(user)
|
||||
.indexedChange(cd, changeNotesFactory.createFromIndexedChange(cd.change()))
|
||||
.database(db)
|
||||
.test(ChangePermission.READ)) {
|
||||
for (ChangeData cd : changeCache.getChangeData(db.get(), project)) {
|
||||
ChangeNotes notes = changeNotesFactory.createFromIndexedChange(cd.change());
|
||||
if (perm.indexedChange(cd, notes).test(ChangePermission.READ)) {
|
||||
visibleChanges.put(cd.getId(), cd.change().getDest());
|
||||
}
|
||||
}
|
||||
return visibleChanges;
|
||||
} catch (OrmException | PermissionBackendException e) {
|
||||
log.error(
|
||||
"Cannot load changes for project "
|
||||
+ project.getName()
|
||||
+ ", assuming no changes are visible",
|
||||
e);
|
||||
"Cannot load changes for project " + project + ", assuming no changes are visible", e);
|
||||
return Collections.emptyMap();
|
||||
}
|
||||
}
|
||||
|
||||
private Map<Change.Id, Branch.NameKey> visibleChangesByScan() {
|
||||
Project.NameKey p = projectCtl.getProject().getNameKey();
|
||||
Project.NameKey p = projectState.getProject().getNameKey();
|
||||
Stream<ChangeNotesResult> s;
|
||||
try {
|
||||
s = changeNotesFactory.scan(git, db.get(), p);
|
||||
@ -309,7 +306,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
if (permissionBackend.user(user).change(r.notes()).database(db).test(ChangePermission.READ)) {
|
||||
if (perm.change(r.notes()).test(ChangePermission.READ)) {
|
||||
return r.notes();
|
||||
}
|
||||
} catch (PermissionBackendException e) {
|
||||
@ -330,17 +327,13 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
|
||||
|
||||
private boolean canReadRef(String ref) {
|
||||
try {
|
||||
permissionBackend
|
||||
.user(user)
|
||||
.project(projectCtl.getProject().getNameKey())
|
||||
.ref(ref)
|
||||
.check(RefPermission.READ);
|
||||
perm.ref(ref).check(RefPermission.READ);
|
||||
return true;
|
||||
} catch (AuthException e) {
|
||||
return false;
|
||||
} catch (PermissionBackendException e) {
|
||||
log.error("unable to check permissions", e);
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
@ -268,6 +268,15 @@ public abstract class PermissionBackend {
|
||||
return ref(notes.getChange().getDest().get()).change(notes);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return instance scoped for the change loaded from index, and its destination ref and
|
||||
* project. This method should only be used when database access is harmful and potentially
|
||||
* stale data from the index is acceptable.
|
||||
*/
|
||||
public ForChange indexedChange(ChangeData cd, ChangeNotes notes) {
|
||||
return ref(notes.getChange().getDest().get()).indexedChange(cd, notes);
|
||||
}
|
||||
|
||||
/** Verify scoped user can {@code perm}, throwing if denied. */
|
||||
public abstract void check(ProjectPermission perm)
|
||||
throws AuthException, PermissionBackendException;
|
||||
|
Loading…
Reference in New Issue
Block a user