opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.8' into stable-2.9

Browse Source 
* stable-2.8:
  SideBySide2: Show [ and ] shortcut keys in nav arrow tooltips
  Always auto confirm adding reviewers for set-reviewers SSH command
  Improve wording of 'parents' field description in CommitInfo
  Fix 'parents' field name of CommitInfo in REST documentation
  Implement pagination in project list screen
  Add option 'S' to projects REST API to support query offset
  Fix: Failure of acceptance tests.
  Do not refresh project list if filter did not change
  Fix inconsistent behavior of diff view when viewing binary files
  Update cookbook plugin to latest revision
  Correct Javadoc of RestReadView in extension API
  Fix memory leak of SubIndex.NrtFuture objects

Conflicts:
	gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
	gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
	gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java

Change-Id: I4d432c6ce27d3fd76dd9f86b8685be15ec8f123d
This commit is contained in:
David Pursehouse 2014-04-10 16:52:33 +09:00
commit ecc86e2bcb
17 changed files with 220 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Documentation/rest-api-changes.txt
View File

@ -2903,10 +2903,10 @@ The `CommitInfo` entity contains information about a commit.
|==========================
|Field Name |Description
|`commit` |The commit ID.
|`parent` |
|`parents` |
The parent commits of this commit as a list of
link:#commit-info[CommitInfo] entities. In parent
only `commit` and `subject` fields are populated.
link:#commit-info[CommitInfo] entities. In each parent
only the `commit` and `subject` fields are populated.
|`author` |The author of the commit as a
link:#git-person-info[GitPersonInfo] entity.
|`committer` |The committer of the commit as a

16
Documentation/rest-api-projects.txt
View File

@ -92,6 +92,22 @@ List all projects that start with `platform/`:
----
E.g. this feature can be used by suggestion client UI's to limit results.
The `/projects/` URL also accepts a limit integer in the `n` parameter.
This limits the results to show `n` projects.
Query the first 25 projects in project list.
----
GET /projects/?n=25 HTTP/1.0
----
The `/projects/` URL also accepts a start integer in the `S` parameter.
The results will skip `S` projects from project list.
Query 25 projects starting from index 50.
----
GET /projects/?n=25&S=50 HTTP/1.0
----
[[get-project]]
=== Get Project
--

2
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/restapi/RestReadView.java
View File

@ -23,7 +23,7 @@ public interface RestReadView<R extends RestResource> extends RestView<R> {
/**
* Process the view operation by reading from the resource.
*
* @param resource resource to modify.
* @param resource resource to read.
* @return result to return to the client. Use {@link BinaryResult} to avoid
* automatic conversion to JSON.
* @throws AuthException the client is not permitted to access this view.

17
gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
View File

@ -585,7 +585,7 @@ public class Dispatcher {
}
}
private static boolean isChangeScreen2() {
public static boolean isChangeScreen2() {
if (!Gerrit.getConfig().getNewFeatures()) {
return false;
} else if (changeScreen2) {
@ -650,7 +650,20 @@ public class Dispatcher {
panel = 0 <= c ? token.substring(c + 1) : "";
}
if ("unified".equals(panel)) {
if ("".equals(panel)) {
if (isChangeScreen2()) {
return new SideBySide2(baseId, id.getParentKey(), id.get(),
side, line);
}
return new PatchScreen.SideBySide( //
id, //
patchIndex, //
patchSetDetail, //
patchTable, //
top, //
baseId //
);
} else if ("unified".equals(panel)) {
return new PatchScreen.Unified( //
id, //
patchIndex, //

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
View File

@ -134,4 +134,7 @@ public interface AdminConstants extends Constants {
String sectionTypeReference();
String sectionTypeSection();
Map<String, String> sectionNames();
String pagedProjectListPrev();
String pagedProjectListNext();
}

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
View File

@ -99,6 +99,8 @@ noGroupSelected = (No group selected)
errorNoMatchingGroups = No Matching Groups
errorNoGitRepository = No Git Repository
pagedProjectListPrev = &#x21e6;Prev
pagedProjectListNext = Next&#x21e8;
addPermission = Add Permission ...

93
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectListScreen.java
View File

@ -24,11 +24,13 @@ import com.google.gerrit.client.projects.ProjectMap;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.FilteredUserInterface;
import com.google.gerrit.client.ui.HighlightingInlineHyperlink;
import com.google.gerrit.client.ui.Hyperlink;
import com.google.gerrit.client.ui.IgnoreOutdatedFilterResultsCallbackWrapper;
import com.google.gerrit.client.ui.ProjectSearchLink;
import com.google.gerrit.client.ui.ProjectsTable;
import com.google.gerrit.client.ui.Screen;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences;
import com.google.gwt.event.dom.client.KeyCodes;
import com.google.gwt.event.dom.client.KeyUpEvent;
import com.google.gwt.event.dom.client.KeyUpHandler;
@ -42,11 +44,16 @@ import com.google.gwt.user.client.ui.Label;
import com.google.gwtexpui.globalkey.client.NpTextBox;
public class ProjectListScreen extends Screen implements FilteredUserInterface {
private Hyperlink prev;
private Hyperlink next;
private ProjectsTable projects;
private NpTextBox filterTxt;
private String subname;
private String subname = "";
private int startPosition;
private int pageSize;
public ProjectListScreen() {
configurePageSize();
}
public ProjectListScreen(String params) {
@ -59,6 +66,22 @@ public class ProjectListScreen extends Screen implements FilteredUserInterface {
if ("filter".equals(kv[0])) {
subname = URL.decodeQueryString(kv[1]);
}
if ("skip".equals(kv[0]) && URL.decodeQueryString(kv[1]).matches("^[\\d]+")) {
startPosition = Integer.parseInt(URL.decodeQueryString(kv[1]));
}
}
configurePageSize();
}
private void configurePageSize() {
if (Gerrit.isSignedIn()) {
final AccountGeneralPreferences p =
Gerrit.getUserAccount().getGeneralPreferences();
final short m = p.getMaximumPageSize();
pageSize = 0 < m ? m : AccountGeneralPreferences.DEFAULT_PAGESIZE;
} else {
pageSize = AccountGeneralPreferences.DEFAULT_PAGESIZE;
}
}
@ -66,13 +89,17 @@ public class ProjectListScreen extends Screen implements FilteredUserInterface {
protected void onLoad() {
super.onLoad();
display();
refresh(false);
refresh(false, false);
}
private void refresh(final boolean open) {
setToken(subname == null || "".equals(subname) ? ADMIN_PROJECTS
: ADMIN_PROJECTS + "?filter=" + URL.encodeQueryString(subname));
ProjectMap.match(subname,
private void refresh(final boolean open, final boolean filterModified) {
if (filterModified){
startPosition = 0;
}
setToken(getTokenForScreen(subname, startPosition));
// Retrieve one more project than page size to determine if there are more
// projects to display
ProjectMap.match(subname, pageSize + 1, startPosition,
new IgnoreOutdatedFilterResultsCallbackWrapper<ProjectMap>(this,
new GerritCallback<ProjectMap>() {
@Override
@ -81,12 +108,44 @@ public class ProjectListScreen extends Screen implements FilteredUserInterface {
Gerrit.display(PageLinks.toProject(
result.values().get(0).name_key()));
} else {
projects.display(result);
if (result.size() <= pageSize) {
projects.display(result);
next.setVisible(false);
} else {
projects.displaySubset(result, 0, result.size() - 1);
setupNavigationLink(next, subname, startPosition + pageSize);
}
if (startPosition > 0) {
setupNavigationLink(prev, subname, startPosition - pageSize);
} else {
prev.setVisible(false);
}
}
}
}));
}
private void setupNavigationLink(Hyperlink link, String filter, int skip) {
link.setTargetHistoryToken(getTokenForScreen(filter, skip));
link.setVisible(true);
}
private String getTokenForScreen(String filter, int skip) {
String token = ADMIN_PROJECTS;
if (filter != null && !filter.isEmpty()) {
token += "?filter=" + URL.encodeQueryString(filter);
}
if (skip > 0) {
if (token.contains("?filter=")) {
token += ",";
} else {
token += "?";
}
token += "skip=" + skip;
}
return token;
}
@Override
public String getCurrentFilter() {
return subname;
@ -98,6 +157,12 @@ public class ProjectListScreen extends Screen implements FilteredUserInterface {
setPageTitle(Util.C.projectListTitle());
initPageHeader();
prev = new Hyperlink(Util.C.pagedProjectListPrev(), true, "");
prev.setVisible(false);
next = new Hyperlink(Util.C.pagedProjectListNext(), true, "");
next.setVisible(false);
projects = new ProjectsTable() {
@Override
protected void initColumnHeaders() {
@ -167,6 +232,11 @@ public class ProjectListScreen extends Screen implements FilteredUserInterface {
projects.setSavePointerId(PageLinks.ADMIN_PROJECTS);
add(projects);
final HorizontalPanel buttons = new HorizontalPanel();
buttons.setStyleName(Gerrit.RESOURCES.css().changeTablePrevNextLinks());
buttons.add(prev);
buttons.add(next);
add(buttons);
}
private void initPageHeader() {
@ -180,8 +250,13 @@ public class ProjectListScreen extends Screen implements FilteredUserInterface {
filterTxt.addKeyUpHandler(new KeyUpHandler() {
@Override
public void onKeyUp(KeyUpEvent event) {
subname = filterTxt.getValue();
refresh(event.getNativeEvent().getKeyCode() == KeyCodes.KEY_ENTER);
boolean enterPressed =
event.getNativeEvent().getKeyCode() == KeyCodes.KEY_ENTER;
boolean filterModified = !filterTxt.getValue().equals(subname);
if (enterPressed || filterModified) {
subname = filterTxt.getValue();
refresh(enterPressed, filterModified);
}
}
});
hp.add(filterTxt);

23
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchTable.java
View File

@ -22,6 +22,7 @@ import com.google.gerrit.client.ui.ListenableAccountDiffPreference;
import com.google.gerrit.client.ui.NavigationTable;
import com.google.gerrit.client.ui.PatchLink;
import com.google.gerrit.common.data.PatchSetDetail;
import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DiffView;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.Patch.ChangeType;
import com.google.gerrit.reviewdb.client.Patch.Key;
@ -238,21 +239,23 @@ public class PatchTable extends Composite {
/**
* @return a link to the the given patch.
* @param index The patch to link to
* @param patchType The type of patch display
* @param screenType The screen type of patch display
* @param before A string to display at the beginning of the href text
* @param after A string to display at the end of the href text
*/
public PatchLink createLink(int index, PatchScreen.Type patchType,
public PatchLink createLink(int index, PatchScreen.Type screenType,
SafeHtml before, SafeHtml after) {
Patch patch = patchList.get(index);
Key thisKey = patch.getKey();
PatchLink link;
if (patchType == PatchScreen.Type.SIDE_BY_SIDE) {
link = new PatchLink.SideBySide("", base, thisKey, index, detail, this);
} else {
if (isUnifiedPatchLink(patch, screenType)) {
link = new PatchLink.Unified("", base, thisKey, index, detail, this);
} else {
link = new PatchLink.SideBySide("", base, thisKey, index, detail, this);
}
SafeHtmlBuilder text = new SafeHtmlBuilder();
text.append(before);
text.append(getFileNameOnly(patch));
@ -261,6 +264,16 @@ public class PatchTable extends Composite {
return link;
}
private static boolean isUnifiedPatchLink(final Patch patch,
final PatchScreen.Type screenType) {
if (Dispatcher.isChangeScreen2()) {
return (patch.getPatchType().equals(PatchType.BINARY)
|| Gerrit.getUserAccount().getGeneralPreferences().getDiffView()
.equals(DiffView.UNIFIED_DIFF));
}
return screenType == PatchScreen.Type.UNIFIED;
}
private static String getFileNameOnly(Patch patch) {
// Note: use '/' here and not File.pathSeparator since git paths
// are always separated by /

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
View File

@ -250,11 +250,13 @@ class Header extends Composite {
: Dispatcher.toSideBySide(base, patchSetId, info.path());
}
private KeyCommand setupNav(InlineHyperlink link, int key, String help, FileInfo info) {
private KeyCommand setupNav(InlineHyperlink link, char key, String help, FileInfo info) {
if (info != null) {
final String url = url(info);
link.setTargetHistoryToken(url);
link.setTitle(FileInfo.getFileName(info.path()));
link.setTitle(PatchUtil.M.fileNameWithShortcutKey(
FileInfo.getFileName(info.path()),
Character.toString(key)));
KeyCommand k = new KeyCommand(0, key, help) {
@Override
public void onKeyPress(KeyPressEvent event) {

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.java
View File

@ -23,4 +23,5 @@ public interface PatchMessages extends Messages {
String expandAfter(int cnt);
String draftSaved(Date when);
String patchSkipRegion(String lineNumber);
String fileNameWithShortcutKey(String file, String key);
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchMessages.properties
View File

@ -2,3 +2,4 @@ expandBefore = +{0}&#x21e7;
expandAfter = +{0}&#x21e9;
draftSaved = Draft saved at {0,time,short}
patchSkipRegion = ... skipped {0} common lines ...
fileNameWithShortcutKey = {0} (Shortcut: {1})

26
gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ProjectMap.java
View File

@ -53,16 +53,24 @@ public class ProjectMap extends NativeMap<ProjectInfo> {
.get(NativeMap.copyKeysIntoChildren(cb));
}
public static void match(String match, AsyncCallback<ProjectMap> cb) {
if (match == null || "".equals(match)) {
all(cb);
} else {
new RestApi("/projects/")
.addParameter("m", match)
.addParameterRaw("type", "ALL")
.addParameterTrue("d") // description
.get(NativeMap.copyKeysIntoChildren(cb));
public static void match(String match, int limit, int start, AsyncCallback<ProjectMap> cb) {
RestApi call = new RestApi("/projects/");
if (match != null) {
call.addParameter("m", match);
}
if (limit > 0) {
call.addParameter("n", limit);
}
if (start > 0) {
call.addParameter("S", start);
}
call.addParameterRaw("type", "ALL");
call.addParameterTrue("d"); // description
call.get(NativeMap.copyKeysIntoChildren(cb));
}
public static void match(String match, AsyncCallback<ProjectMap> cb) {
match(match, 0, 0, cb);
}
protected ProjectMap() {

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ProjectsTable.java
View File

@ -61,6 +61,10 @@ public class ProjectsTable extends NavigationTable<ProjectInfo> {
}
public void display(ProjectMap projects) {
displaySubset(projects, 0, projects.size());
}
public void displaySubset(ProjectMap projects, int fromIndex, int toIndex) {
while (1 < table.getRowCount())
table.removeRow(table.getRowCount() - 1);
@ -71,7 +75,7 @@ public class ProjectsTable extends NavigationTable<ProjectInfo> {
return a.name().compareTo(b.name());
}
});
for(ProjectInfo p : list)
for(ProjectInfo p : list.subList(fromIndex, toIndex))
insert(table.getRowCount(), p);
finishDisplay();

56
gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java
View File

@ -16,7 +16,7 @@ package com.google.gerrit.lucene;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.AbstractFuture;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
@ -39,13 +39,12 @@ import org.slf4j.LoggerFactory;
import java.io.File;
import java.io.IOException;
import java.util.concurrent.ConcurrentMap;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
/** Piece of the change index that is implemented as a separate Lucene index. */
class SubIndex {
@ -55,7 +54,7 @@ class SubIndex {
private final TrackingIndexWriter writer;
private final SearcherManager searcherManager;
private final ControlledRealTimeReopenThread<IndexSearcher> reopenThread;
private final ConcurrentMap<RefreshListener, Boolean> refreshListeners;
private final Set<NrtFuture> notDoneNrtFutures;
SubIndex(File file, GerritIndexWriterConfig writerConfig) throws IOException {
this(FSDirectory.open(file), file.getName(), writerConfig);
@ -107,7 +106,7 @@ class SubIndex {
searcherManager = new SearcherManager(
writer.getIndexWriter(), true, new SearcherFactory());
refreshListeners = Maps.newConcurrentMap();
notDoneNrtFutures = Sets.newConcurrentHashSet();
searcherManager.addListener(new RefreshListener() {
@Override
public void beforeRefresh() throws IOException {
@ -115,8 +114,8 @@ class SubIndex {
@Override
public void afterRefresh(boolean didRefresh) throws IOException {
for (RefreshListener l : refreshListeners.keySet()) {
l.afterRefresh(didRefresh);
for (NrtFuture f : notDoneNrtFutures) {
f.removeIfDone();
}
}
});
@ -176,10 +175,8 @@ class SubIndex {
searcherManager.release(searcher);
}
private final class NrtFuture extends AbstractFuture<Void>
implements RefreshListener {
private final class NrtFuture extends AbstractFuture<Void> {
private final long gen;
private final AtomicBoolean hasListeners = new AtomicBoolean();
NrtFuture(long gen) {
this.gen = gen;
@ -198,9 +195,12 @@ class SubIndex {
public Void get(long timeout, TimeUnit unit) throws InterruptedException,
TimeoutException, ExecutionException {
if (!isDone()) {
reopenThread.waitForGeneration(gen,
(int) MILLISECONDS.convert(timeout, unit));
set(null);
if (reopenThread.waitForGeneration(gen,
(int) MILLISECONDS.convert(timeout, unit))) {
set(null);
} else {
throw new TimeoutException();
}
}
return super.get(timeout, unit);
}
@ -209,7 +209,7 @@ class SubIndex {
public boolean isDone() {
if (super.isDone()) {
return true;
} else if (isSearcherCurrent()) {
} else if (isGenAvailableNowForCurrentSearcher()) {
set(null);
return true;
}
@ -218,33 +218,31 @@ class SubIndex {
@Override
public void addListener(Runnable listener, Executor executor) {
if (hasListeners.compareAndSet(false, true) && !isDone()) {
searcherManager.addListener(this);
if (!isDone()) {
notDoneNrtFutures.add(this);
}
super.addListener(listener, executor);
}
@Override
public boolean cancel(boolean mayInterruptIfRunning) {
if (hasListeners.get()) {
refreshListeners.put(this, true);
boolean result = super.cancel(mayInterruptIfRunning);
if (result) {
notDoneNrtFutures.remove(this);
}
return super.cancel(mayInterruptIfRunning);
return result;
}
@Override
public void beforeRefresh() throws IOException {
}
@Override
public void afterRefresh(boolean didRefresh) throws IOException {
if (isSearcherCurrent()) {
refreshListeners.remove(this);
set(null);
void removeIfDone() {
if (isGenAvailableNowForCurrentSearcher()) {
notDoneNrtFutures.remove(this);
if (!isCancelled()) {
set(null);
}
}
}
private boolean isSearcherCurrent() {
private boolean isGenAvailableNowForCurrentSearcher() {
try {
return reopenThread.waitForGeneration(gen, 0);
} catch (InterruptedException e) {

11
gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java
View File

@ -143,6 +143,11 @@ public class ListProjects implements RestReadView<TopLevelResource> {
this.limit = limit;
}
@Option(name = "-S", metaVar = "CNT", usage = "number of projects to skip")
public void setStart(int start) {
this.start = start;
}
@Option(name = "-p", metaVar = "PREFIX", usage = "match project prefix")
public void setMatchPrefix(String matchPrefix) {
this.matchPrefix = matchPrefix;
@ -165,6 +170,7 @@ public class ListProjects implements RestReadView<TopLevelResource> {
private boolean showDescription;
private boolean all;
private int limit;
private int start;
private String matchPrefix;
private String matchSubstring;
private AccountGroup.UUID groupUuid;
@ -230,6 +236,7 @@ public class ListProjects implements RestReadView<TopLevelResource> {
}
}
int foundIndex = 0;
int found = 0;
Map<String, ProjectInfo> output = Maps.newTreeMap();
Map<String, String> hiddenNames = Maps.newHashMap();
@ -362,6 +369,10 @@ public class ListProjects implements RestReadView<TopLevelResource> {
}
}
if (foundIndex++ < start) {
continue;
}
if (limit > 0 && ++found > limit) {
break;
}

11
gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java
View File

@ -39,8 +39,10 @@ import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.config.TrackingFootersProvider;
import com.google.gerrit.server.git.EmailReviewCommentsExecutor;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.PerThreadRequestScope;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.index.ChangeSchemas;
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier;
@ -174,6 +176,15 @@ public class InMemoryModule extends FactoryModule {
}
}
@Provides
@Singleton
@EmailReviewCommentsExecutor
public WorkQueue.Executor createEmailReviewCommentsExecutor(
@GerritServerConfig Config config, WorkQueue queues) {
int poolSize = config.getInt("sendemail", null, "threadPoolSize", 1);
return queues.createQueue(poolSize, "EmailReviewComments");
}
@Provides
@Singleton
InMemoryDatabase getInMemoryDatabase(@Current SchemaVersion schemaVersion,

1
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
View File

@ -143,6 +143,7 @@ public class SetReviewersCommand extends SshCommand {
for (String reviewer : toAdd) {
AddReviewerInput input = new AddReviewerInput();
input.reviewer = reviewer;
input.confirmed = true;
String error;
try {
error = post.apply(changeRsrc, input).error;