Merge changes from topic "change-messages-rest-api"
* changes:
Add REST endpoint to get one change message
Add REST endpoint for listing change messages
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 0b458ae..a79b7a5 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1171,6 +1171,15 @@
+
Default is true.
+[[change.enableParallelFormatting]]change.enableParallelFormatting::
++
+Whether or not changes can be formatted in parallel when requesting
+multiple changes at once. An example for this is Dashboards.
++
+This setting is experimental.
++
+Default is `false`.
+
[[change.showAssigneeInChangesTable]]change.showAssigneeInChangesTable::
+
Show assignee field in changes table. If set to false, assignees will
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 8cde19e..ac7be2c 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -5819,7 +5819,11 @@
Whether the new change should be set to work in progress.
|`base_change` |optional|
A link:#change-id[\{change-id\}] that identifies the base change for a create
-change operation.
+change operation. Mutually exclusive with `base_commit`.
+|`base_commit` |optional|
+A 40-digit hex SHA-1 of the commit which will be the parent commit of the newly
+created change. If set, it must be a merged commit on the destination branch.
+Mutually exclusive with `base_change`.
|`new_branch` |optional, default to `false`|
Allow creating a new branch when set to `true`.
|`merge` |optional|
diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/AuthInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/AuthInfo.java
index d2e5d49..43281bd 100644
--- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/AuthInfo.java
+++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/AuthInfo.java
@@ -69,9 +69,7 @@
List<AgreementInfo> agreements = new ArrayList<>();
JsArray<AgreementInfo> contributorAgreements = _contributorAgreements();
if (contributorAgreements != null) {
- for (AgreementInfo a : Natives.asList(contributorAgreements)) {
- agreements.add(a);
- }
+ agreements.addAll(Natives.asList(contributorAgreements));
}
return agreements;
}
diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/DownloadInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/DownloadInfo.java
index 8d56a1c..a22a1e8 100644
--- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/DownloadInfo.java
+++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/DownloadInfo.java
@@ -31,9 +31,7 @@
public final List<String> archives() {
List<String> archives = new ArrayList<>();
- for (String f : Natives.asList(_archives())) {
- archives.add(f);
- }
+ archives.addAll(Natives.asList(_archives()));
return archives;
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml
index 536af4f..d629fc2 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen.ui.xml
@@ -526,6 +526,7 @@
<td ui:field='actionDate'/>
</tr>
<tr ui:field='hashtagTableRow'>
+ <th><ui:msg>Hashtags</ui:msg></th>
<td colspan='2'>
<c:Hashtags ui:field='hashtags'/>
</td>
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index fbf3b84..bf01615 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -101,6 +101,8 @@
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.group.db.Groups;
+import com.google.gerrit.server.index.account.AccountIndex;
+import com.google.gerrit.server.index.account.AccountIndexCollection;
import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -271,6 +273,7 @@
protected BlockStrategy noSleepBlockStrategy = t -> {}; // Don't sleep in tests.
@Inject private ChangeIndexCollection changeIndexes;
+ @Inject private AccountIndexCollection accountIndexes;
@Inject private EventRecorder.Factory eventRecorderFactory;
@Inject private InProcessProtocol inProcessProtocol;
@Inject private Provider<AnonymousUser> anonymousUser;
@@ -878,6 +881,23 @@
};
}
+ protected AutoCloseable disableAccountIndex() {
+ AccountIndex searchIndex = accountIndexes.getSearchIndex();
+ if (!(searchIndex instanceof DisabledAccountIndex)) {
+ accountIndexes.setSearchIndex(new DisabledAccountIndex(searchIndex), false);
+ }
+
+ return new AutoCloseable() {
+ @Override
+ public void close() {
+ AccountIndex searchIndex = accountIndexes.getSearchIndex();
+ if (searchIndex instanceof DisabledAccountIndex) {
+ accountIndexes.setSearchIndex(((DisabledAccountIndex) searchIndex).unwrap(), false);
+ }
+ }
+ };
+ }
+
protected static Gson newGson() {
return OutputFormat.JSON_COMPACT.newGson();
}
@@ -1286,9 +1306,7 @@
protected void assertDiffForNewFile(
DiffInfo diff, RevCommit commit, String path, String expectedContentSideB) throws Exception {
List<String> expectedLines = new ArrayList<>();
- for (String line : expectedContentSideB.split("\n")) {
- expectedLines.add(line);
- }
+ Collections.addAll(expectedLines, expectedContentSideB.split("\n"));
assertThat(diff.binary).isNull();
assertThat(diff.changeType).isEqualTo(ChangeType.ADDED);
diff --git a/java/com/google/gerrit/acceptance/DisabledAccountIndex.java b/java/com/google/gerrit/acceptance/DisabledAccountIndex.java
new file mode 100644
index 0000000..91baafb
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/DisabledAccountIndex.java
@@ -0,0 +1,71 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance;
+
+import com.google.gerrit.index.QueryOptions;
+import com.google.gerrit.index.Schema;
+import com.google.gerrit.index.query.DataSource;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.index.account.AccountIndex;
+
+/** This class wraps an index and assumes the search index can't handle any queries. */
+public class DisabledAccountIndex implements AccountIndex {
+ private final AccountIndex index;
+
+ public DisabledAccountIndex(AccountIndex index) {
+ this.index = index;
+ }
+
+ public AccountIndex unwrap() {
+ return index;
+ }
+
+ @Override
+ public Schema<AccountState> getSchema() {
+ return index.getSchema();
+ }
+
+ @Override
+ public void close() {
+ index.close();
+ }
+
+ @Override
+ public void replace(AccountState obj) {
+ throw new UnsupportedOperationException("AccountIndex is disabled");
+ }
+
+ @Override
+ public void delete(Account.Id key) {
+ throw new UnsupportedOperationException("AccountIndex is disabled");
+ }
+
+ @Override
+ public void deleteAll() {
+ throw new UnsupportedOperationException("AccountIndex is disabled");
+ }
+
+ @Override
+ public DataSource<AccountState> getSource(Predicate<AccountState> p, QueryOptions opts) {
+ throw new UnsupportedOperationException("AccountIndex is disabled");
+ }
+
+ @Override
+ public void markReady(boolean ready) {
+ throw new UnsupportedOperationException("AccountIndex is disabled");
+ }
+}
diff --git a/java/com/google/gerrit/common/data/AccessSection.java b/java/com/google/gerrit/common/data/AccessSection.java
index 95f48b1..82dc620 100644
--- a/java/com/google/gerrit/common/data/AccessSection.java
+++ b/java/com/google/gerrit/common/data/AccessSection.java
@@ -18,7 +18,6 @@
import com.google.gerrit.reviewdb.client.Project;
import java.util.ArrayList;
import java.util.HashSet;
-import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -94,11 +93,7 @@
public void removePermission(String name) {
if (permissions != null) {
- for (Iterator<Permission> itr = permissions.iterator(); itr.hasNext(); ) {
- if (name.equalsIgnoreCase(itr.next().getName())) {
- itr.remove();
- }
- }
+ permissions.removeIf(permission -> name.equalsIgnoreCase(permission.getName()));
}
}
diff --git a/java/com/google/gerrit/common/data/Permission.java b/java/com/google/gerrit/common/data/Permission.java
index 4ee176b..dff30d7 100644
--- a/java/com/google/gerrit/common/data/Permission.java
+++ b/java/com/google/gerrit/common/data/Permission.java
@@ -177,11 +177,7 @@
public void removeRule(GroupReference group) {
if (rules != null) {
- for (Iterator<PermissionRule> itr = rules.iterator(); itr.hasNext(); ) {
- if (sameGroup(itr.next(), group)) {
- itr.remove();
- }
- }
+ rules.removeIf(permissionRule -> sameGroup(permissionRule, group));
}
}
diff --git a/java/com/google/gerrit/common/data/SubmitRecord.java b/java/com/google/gerrit/common/data/SubmitRecord.java
index ae8b2bb..8638d6d 100644
--- a/java/com/google/gerrit/common/data/SubmitRecord.java
+++ b/java/com/google/gerrit/common/data/SubmitRecord.java
@@ -32,7 +32,7 @@
}
// The change can be submitted, unless at least one plugin prevents it.
- return in.stream().noneMatch(r -> r.status != Status.OK);
+ return in.stream().map(SubmitRecord::status).allMatch(SubmitRecord.Status::allowsSubmission);
}
public enum Status {
@@ -56,7 +56,11 @@
*
* <p>Additional detail may be available in {@link SubmitRecord#errorMessage}.
*/
- RULE_ERROR
+ RULE_ERROR;
+
+ private boolean allowsSubmission() {
+ return this == OK || this == FORCED;
+ }
}
public Status status;
@@ -178,4 +182,8 @@
public int hashCode() {
return Objects.hash(status, labels, errorMessage, requirements);
}
+
+ private Status status() {
+ return status;
+ }
}
diff --git a/java/com/google/gerrit/extensions/common/ChangeInput.java b/java/com/google/gerrit/extensions/common/ChangeInput.java
index c8e7bca..36dd8f2 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInput.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInput.java
@@ -30,6 +30,7 @@
public Boolean isPrivate;
public Boolean workInProgress;
public String baseChange;
+ public String baseCommit;
public Boolean newBranch;
public MergeInput merge;
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index dc2639b..913128e 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -296,7 +296,7 @@
RestCollection<RestResource, RestResource> rc = members.get();
globals
.permissionBackend
- .user(globals.currentUser.get())
+ .currentUser()
.checkAny(GlobalPermission.fromAnnotation(rc.getClass()));
viewData = new ViewData(null, null);
@@ -1182,7 +1182,7 @@
throws AuthException, PermissionBackendException {
globals
.permissionBackend
- .user(globals.currentUser.get())
+ .currentUser()
.checkAny(GlobalPermission.fromAnnotation(d.pluginName, d.view.getClass()));
}
diff --git a/java/com/google/gerrit/metrics/dropwizard/BucketedCallback.java b/java/com/google/gerrit/metrics/dropwizard/BucketedCallback.java
index c7a92a3..ee87397 100644
--- a/java/com/google/gerrit/metrics/dropwizard/BucketedCallback.java
+++ b/java/com/google/gerrit/metrics/dropwizard/BucketedCallback.java
@@ -21,7 +21,6 @@
import com.google.common.collect.Maps;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.Field;
-import java.util.Iterator;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
@@ -67,12 +66,7 @@
}
void doPrune() {
- Iterator<Map.Entry<Object, ValueGauge>> i = cells.entrySet().iterator();
- while (i.hasNext()) {
- if (!i.next().getValue().set) {
- i.remove();
- }
- }
+ cells.entrySet().removeIf(objectValueGaugeEntry -> !objectValueGaugeEntry.getValue().set);
}
void doEndSet() {
diff --git a/java/com/google/gerrit/metrics/dropwizard/GetMetric.java b/java/com/google/gerrit/metrics/dropwizard/GetMetric.java
index f0ae97e..ae1e6ec 100644
--- a/java/com/google/gerrit/metrics/dropwizard/GetMetric.java
+++ b/java/com/google/gerrit/metrics/dropwizard/GetMetric.java
@@ -16,7 +16,6 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -25,23 +24,21 @@
class GetMetric implements RestReadView<MetricResource> {
private final PermissionBackend permissionBackend;
- private final CurrentUser user;
private final DropWizardMetricMaker metrics;
@Option(name = "--data-only", usage = "return only values")
boolean dataOnly;
@Inject
- GetMetric(PermissionBackend permissionBackend, CurrentUser user, DropWizardMetricMaker metrics) {
+ GetMetric(PermissionBackend permissionBackend, DropWizardMetricMaker metrics) {
this.permissionBackend = permissionBackend;
- this.user = user;
this.metrics = metrics;
}
@Override
public MetricJson apply(MetricResource resource)
throws AuthException, PermissionBackendException {
- permissionBackend.user(user).check(GlobalPermission.VIEW_CACHES);
+ permissionBackend.currentUser().check(GlobalPermission.VIEW_CACHES);
return new MetricJson(
resource.getMetric(), metrics.getAnnotations(resource.getName()), dataOnly);
}
diff --git a/java/com/google/gerrit/metrics/dropwizard/ListMetrics.java b/java/com/google/gerrit/metrics/dropwizard/ListMetrics.java
index 59f6b97..b028a16 100644
--- a/java/com/google/gerrit/metrics/dropwizard/ListMetrics.java
+++ b/java/com/google/gerrit/metrics/dropwizard/ListMetrics.java
@@ -17,7 +17,6 @@
import com.codahale.metrics.Metric;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -32,7 +31,6 @@
class ListMetrics implements RestReadView<ConfigResource> {
private final PermissionBackend permissionBackend;
- private final CurrentUser user;
private final DropWizardMetricMaker metrics;
@Option(name = "--data-only", usage = "return only values")
@@ -47,17 +45,15 @@
List<String> query = new ArrayList<>();
@Inject
- ListMetrics(
- PermissionBackend permissionBackend, CurrentUser user, DropWizardMetricMaker metrics) {
+ ListMetrics(PermissionBackend permissionBackend, DropWizardMetricMaker metrics) {
this.permissionBackend = permissionBackend;
- this.user = user;
this.metrics = metrics;
}
@Override
public Map<String, MetricJson> apply(ConfigResource resource)
throws AuthException, PermissionBackendException {
- permissionBackend.user(user).check(GlobalPermission.VIEW_CACHES);
+ permissionBackend.currentUser().check(GlobalPermission.VIEW_CACHES);
SortedMap<String, MetricJson> out = new TreeMap<>();
List<String> prefixes = new ArrayList<>(query.size());
diff --git a/java/com/google/gerrit/pgm/init/BaseInit.java b/java/com/google/gerrit/pgm/init/BaseInit.java
index a5b4f46..88e48aa 100644
--- a/java/com/google/gerrit/pgm/init/BaseInit.java
+++ b/java/com/google/gerrit/pgm/init/BaseInit.java
@@ -72,7 +72,6 @@
import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.Iterator;
import java.util.List;
import java.util.Set;
import javax.sql.DataSource;
@@ -202,12 +201,7 @@
}
List<String> names = pluginsDistribution.listPluginNames();
if (pluginsToInstall != null) {
- for (Iterator<String> i = names.iterator(); i.hasNext(); ) {
- String n = i.next();
- if (!pluginsToInstall.contains(n)) {
- i.remove();
- }
- }
+ names.removeIf(n -> !pluginsToInstall.contains(n));
}
return names;
} catch (FileNotFoundException e) {
diff --git a/java/com/google/gerrit/server/ApprovalsUtil.java b/java/com/google/gerrit/server/ApprovalsUtil.java
index 90ee38d..8ffe33d 100644
--- a/java/com/google/gerrit/server/ApprovalsUtil.java
+++ b/java/com/google/gerrit/server/ApprovalsUtil.java
@@ -108,7 +108,6 @@
}
private final NotesMigration migration;
- private final IdentifiedUser.GenericFactory userFactory;
private final ApprovalCopier copier;
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
@@ -117,12 +116,10 @@
@Inject
public ApprovalsUtil(
NotesMigration migration,
- IdentifiedUser.GenericFactory userFactory,
ApprovalCopier copier,
PermissionBackend permissionBackend,
ProjectCache projectCache) {
this.migration = migration;
- this.userFactory = userFactory;
this.copier = copier;
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
@@ -267,9 +264,12 @@
private boolean canSee(ReviewDb db, ChangeNotes notes, Account.Id accountId) {
try {
- IdentifiedUser user = userFactory.create(accountId);
return projectCache.checkedGet(notes.getProjectName()).statePermitsRead()
- && permissionBackend.user(user).change(notes).database(db).test(ChangePermission.READ);
+ && permissionBackend
+ .absentUser(accountId)
+ .change(notes)
+ .database(db)
+ .test(ChangePermission.READ);
} catch (IOException | PermissionBackendException e) {
log.warn(
String.format(
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 70b3d68..ab2c26b 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -89,6 +89,7 @@
"//lib/ow2:ow2-asm-tree",
"//lib/ow2:ow2-asm-util",
"//lib/prolog:runtime",
+ "//proto:cache_java_proto",
],
)
diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java
index 82ca8d2..252eb61 100644
--- a/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/java/com/google/gerrit/server/PatchSetUtil.java
@@ -26,15 +26,24 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
+import com.google.gerrit.common.data.LabelFunction;
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
@@ -42,16 +51,31 @@
import java.util.List;
import java.util.Set;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
/** Utilities for manipulating patch sets. */
@Singleton
public class PatchSetUtil {
private final NotesMigration migration;
+ private final Provider<ApprovalsUtil> approvalsUtilProvider;
+ private final ProjectCache projectCache;
+ private final Provider<ReviewDb> dbProvider;
+ private final GitRepositoryManager repoManager;
@Inject
- PatchSetUtil(NotesMigration migration) {
+ PatchSetUtil(
+ NotesMigration migration,
+ Provider<ApprovalsUtil> approvalsUtilProvider,
+ ProjectCache projectCache,
+ Provider<ReviewDb> dbProvider,
+ GitRepositoryManager repoManager) {
this.migration = migration;
+ this.approvalsUtilProvider = approvalsUtilProvider;
+ this.projectCache = projectCache;
+ this.dbProvider = dbProvider;
+ this.repoManager = repoManager;
}
public PatchSet current(ReviewDb db, ChangeNotes notes) throws OrmException {
@@ -155,4 +179,49 @@
update.setGroups(groups);
db.patchSets().update(Collections.singleton(ps));
}
+
+ /** Check if the current patch set of the change is locked. */
+ public void checkPatchSetNotLocked(ChangeNotes notes, CurrentUser user)
+ throws OrmException, IOException, ResourceConflictException {
+ if (isPatchSetLocked(notes, user)) {
+ throw new ResourceConflictException(
+ String.format("The current patch set of change %s is locked", notes.getChangeId()));
+ }
+ }
+
+ /** Is the current patch set locked against state changes? */
+ public boolean isPatchSetLocked(ChangeNotes notes, CurrentUser user)
+ throws OrmException, IOException {
+ Change change = notes.getChange();
+ if (change.getStatus() == Change.Status.MERGED) {
+ return false;
+ }
+
+ ProjectState projectState = projectCache.checkedGet(notes.getProjectName());
+ checkNotNull(projectState, "Failed to load project %s", notes.getProjectName());
+
+ ApprovalsUtil approvalsUtil = approvalsUtilProvider.get();
+ for (PatchSetApproval ap :
+ approvalsUtil.byPatchSet(
+ dbProvider.get(), notes, user, change.currentPatchSetId(), null, null)) {
+ LabelType type = projectState.getLabelTypes(notes, user).byLabel(ap.getLabel());
+ if (type != null
+ && ap.getValue() == 1
+ && type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /** Returns the full commit message for the given project at the given patchset revision */
+ public String getFullCommitMessage(Project.NameKey project, PatchSet patchSet)
+ throws IOException {
+ try (Repository repo = repoManager.openRepository(project);
+ RevWalk rw = new RevWalk(repo)) {
+ RevCommit src = rw.parseCommit(ObjectId.fromString(patchSet.getRevision().get()));
+ rw.parseBody(src);
+ return src.getFullMessage();
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java b/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
index c375067..6f5318e 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdNotes.java
@@ -27,6 +27,10 @@
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.metrics.Counter0;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.DisabledMetricMaker;
+import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.AccountCache;
@@ -113,28 +117,33 @@
private final ExternalIdCache externalIdCache;
private final AccountCache accountCache;
private final Provider<AccountIndexer> accountIndexer;
+ private final MetricMaker metricMaker;
@Inject
Factory(
ExternalIdCache externalIdCache,
AccountCache accountCache,
- Provider<AccountIndexer> accountIndexer) {
+ Provider<AccountIndexer> accountIndexer,
+ MetricMaker metricMaker) {
this.externalIdCache = externalIdCache;
this.accountCache = accountCache;
this.accountIndexer = accountIndexer;
+ this.metricMaker = metricMaker;
}
@Override
public ExternalIdNotes load(Repository allUsersRepo)
throws IOException, ConfigInvalidException {
- return new ExternalIdNotes(externalIdCache, accountCache, accountIndexer, allUsersRepo)
+ return new ExternalIdNotes(
+ externalIdCache, accountCache, accountIndexer, metricMaker, allUsersRepo)
.load();
}
@Override
public ExternalIdNotes load(Repository allUsersRepo, @Nullable ObjectId rev)
throws IOException, ConfigInvalidException {
- return new ExternalIdNotes(externalIdCache, accountCache, accountIndexer, allUsersRepo)
+ return new ExternalIdNotes(
+ externalIdCache, accountCache, accountIndexer, metricMaker, allUsersRepo)
.load(rev);
}
}
@@ -142,22 +151,24 @@
@Singleton
public static class FactoryNoReindex implements ExternalIdNotesLoader {
private final ExternalIdCache externalIdCache;
+ private final MetricMaker metricMaker;
@Inject
- FactoryNoReindex(ExternalIdCache externalIdCache) {
+ FactoryNoReindex(ExternalIdCache externalIdCache, MetricMaker metricMaker) {
this.externalIdCache = externalIdCache;
+ this.metricMaker = metricMaker;
}
@Override
public ExternalIdNotes load(Repository allUsersRepo)
throws IOException, ConfigInvalidException {
- return new ExternalIdNotes(externalIdCache, null, null, allUsersRepo).load();
+ return new ExternalIdNotes(externalIdCache, null, null, metricMaker, allUsersRepo).load();
}
@Override
public ExternalIdNotes load(Repository allUsersRepo, @Nullable ObjectId rev)
throws IOException, ConfigInvalidException {
- return new ExternalIdNotes(externalIdCache, null, null, allUsersRepo).load(rev);
+ return new ExternalIdNotes(externalIdCache, null, null, metricMaker, allUsersRepo).load(rev);
}
}
@@ -169,7 +180,8 @@
*/
public static ExternalIdNotes loadReadOnly(Repository allUsersRepo)
throws IOException, ConfigInvalidException {
- return new ExternalIdNotes(new DisabledExternalIdCache(), null, null, allUsersRepo)
+ return new ExternalIdNotes(
+ new DisabledExternalIdCache(), null, null, new DisabledMetricMaker(), allUsersRepo)
.setReadOnly()
.load();
}
@@ -186,7 +198,8 @@
*/
public static ExternalIdNotes loadReadOnly(Repository allUsersRepo, @Nullable ObjectId rev)
throws IOException, ConfigInvalidException {
- return new ExternalIdNotes(new DisabledExternalIdCache(), null, null, allUsersRepo)
+ return new ExternalIdNotes(
+ new DisabledExternalIdCache(), null, null, new DisabledMetricMaker(), allUsersRepo)
.setReadOnly()
.load(rev);
}
@@ -195,16 +208,23 @@
* Loads the external ID notes for updates without cache evictions. The external ID notes are
* loaded from the current tip of the {@code refs/meta/external-ids} branch.
*
+ * <p>Use this only from init, schema upgrades and tests.
+ *
+ * <p>Metrics are disabled.
+ *
* @return {@link ExternalIdNotes} instance that doesn't updates caches on save
*/
public static ExternalIdNotes loadNoCacheUpdate(Repository allUsersRepo)
throws IOException, ConfigInvalidException {
- return new ExternalIdNotes(new DisabledExternalIdCache(), null, null, allUsersRepo).load();
+ return new ExternalIdNotes(
+ new DisabledExternalIdCache(), null, null, new DisabledMetricMaker(), allUsersRepo)
+ .load();
}
private final ExternalIdCache externalIdCache;
@Nullable private final AccountCache accountCache;
@Nullable private final Provider<AccountIndexer> accountIndexer;
+ private final Counter0 updateCount;
private final Repository repo;
private NoteMap noteMap;
@@ -223,10 +243,15 @@
ExternalIdCache externalIdCache,
@Nullable AccountCache accountCache,
@Nullable Provider<AccountIndexer> accountIndexer,
+ MetricMaker metricMaker,
Repository allUsersRepo) {
this.externalIdCache = checkNotNull(externalIdCache, "externalIdCache");
this.accountCache = accountCache;
this.accountIndexer = accountIndexer;
+ this.updateCount =
+ metricMaker.newCounter(
+ "notedb/external_id_update_count",
+ new Description("Total number of external ID updates.").setRate().setUnit("updates"));
this.repo = checkNotNull(allUsersRepo, "allUsersRepo");
}
@@ -600,7 +625,9 @@
@Override
public RevCommit commit(MetaDataUpdate update) throws IOException {
oldRev = revision != null ? revision.copy() : ObjectId.zeroId();
- return super.commit(update);
+ RevCommit commit = super.commit(update);
+ updateCount.increment();
+ return commit;
}
/**
diff --git a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
index 501b3a4..46d9180 100644
--- a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
+++ b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
@@ -48,7 +48,6 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectJson;
@@ -88,7 +87,6 @@
ProjectApiImpl create(String name);
}
- private final CurrentUser user;
private final PermissionBackend permissionBackend;
private final CreateProject.Factory createProjectFactory;
private final ProjectApiImpl.Factory projectApi;
@@ -123,7 +121,6 @@
@AssistedInject
ProjectApiImpl(
- CurrentUser user,
PermissionBackend permissionBackend,
CreateProject.Factory createProjectFactory,
ProjectApiImpl.Factory projectApi,
@@ -155,7 +152,6 @@
SetParent setParent,
@Assisted ProjectResource project) {
this(
- user,
permissionBackend,
createProjectFactory,
projectApi,
@@ -191,7 +187,6 @@
@AssistedInject
ProjectApiImpl(
- CurrentUser user,
PermissionBackend permissionBackend,
CreateProject.Factory createProjectFactory,
ProjectApiImpl.Factory projectApi,
@@ -223,7 +218,6 @@
SetParent setParent,
@Assisted String name) {
this(
- user,
permissionBackend,
createProjectFactory,
projectApi,
@@ -258,7 +252,6 @@
}
private ProjectApiImpl(
- CurrentUser user,
PermissionBackend permissionBackend,
CreateProject.Factory createProjectFactory,
ProjectApiImpl.Factory projectApi,
@@ -290,7 +283,6 @@
GetParent getParent,
SetParent setParent,
String name) {
- this.user = user;
this.permissionBackend = permissionBackend;
this.createProjectFactory = createProjectFactory;
this.projectApi = projectApi;
@@ -339,7 +331,7 @@
throw new BadRequestException("name must match input.name");
}
CreateProject impl = createProjectFactory.create(name);
- permissionBackend.user(user).checkAny(GlobalPermission.fromAnnotation(impl.getClass()));
+ permissionBackend.currentUser().checkAny(GlobalPermission.fromAnnotation(impl.getClass()));
impl.apply(TopLevelResource.INSTANCE, in);
return projectApi.create(projects.parse(name));
} catch (Exception e) {
diff --git a/java/com/google/gerrit/server/auth/ldap/Helper.java b/java/com/google/gerrit/server/auth/ldap/Helper.java
index 795686c..5af730f 100644
--- a/java/com/google/gerrit/server/auth/ldap/Helper.java
+++ b/java/com/google/gerrit/server/auth/ldap/Helper.java
@@ -406,9 +406,7 @@
throw new IllegalArgumentException("No variables in ldap.groupMemberPattern");
}
- for (String name : groupMemberQuery.getParameters()) {
- accountAtts.add(name);
- }
+ accountAtts.addAll(groupMemberQuery.getParameters());
groupMemberQueryList.add(groupMemberQuery);
}
diff --git a/java/com/google/gerrit/server/cache/CacheBinding.java b/java/com/google/gerrit/server/cache/CacheBinding.java
index ff164ba..1c0d7ef 100644
--- a/java/com/google/gerrit/server/cache/CacheBinding.java
+++ b/java/com/google/gerrit/server/cache/CacheBinding.java
@@ -16,8 +16,6 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.Weigher;
-import com.google.gerrit.common.Nullable;
-import com.google.inject.TypeLiteral;
import java.util.concurrent.TimeUnit;
/** Configure a cache declared within a {@link CacheModule} instance. */
@@ -25,9 +23,6 @@
/** Set the total size of the cache. */
CacheBinding<K, V> maximumWeight(long weight);
- /** Set the total on-disk limit of the cache */
- CacheBinding<K, V> diskLimit(long limit);
-
/** Set the time an element lives before being expired. */
CacheBinding<K, V> expireAfterWrite(long duration, TimeUnit durationUnits);
@@ -40,41 +35,7 @@
/**
* Set the config name to something other than the cache name.
*
- * @see #configKey()
+ * @see CacheDef#configKey()
*/
CacheBinding<K, V> configKey(String configKey);
-
- /**
- * Unique name for this cache.
- *
- * <p>The name can be used in a binding annotation {@code @Named(name)} to inject the cache
- * configured with this binding.
- */
- String name();
-
- /**
- * Key to use when looking up configuration for this cache.
- *
- * <p>Typically, this will match the result of {@link #name()}, so that configuration is keyed by
- * the actual cache name. However, it may be changed, for example to reuse the size limits of some
- * other cache.
- */
- String configKey();
-
- TypeLiteral<K> keyType();
-
- TypeLiteral<V> valueType();
-
- long maximumWeight();
-
- long diskLimit();
-
- @Nullable
- Long expireAfterWrite(TimeUnit unit);
-
- @Nullable
- Weigher<K, V> weigher();
-
- @Nullable
- CacheLoader<K, V> loader();
}
diff --git a/java/com/google/gerrit/server/cache/CacheDef.java b/java/com/google/gerrit/server/cache/CacheDef.java
new file mode 100644
index 0000000..adb7585
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/CacheDef.java
@@ -0,0 +1,55 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.Weigher;
+import com.google.gerrit.common.Nullable;
+import com.google.inject.TypeLiteral;
+import java.util.concurrent.TimeUnit;
+
+public interface CacheDef<K, V> {
+ /**
+ * Unique name for this cache.
+ *
+ * <p>The name can be used in a binding annotation {@code @Named(name)} to inject the cache
+ * configured with this binding.
+ */
+ String name();
+
+ /**
+ * Key to use when looking up configuration for this cache.
+ *
+ * <p>Typically, this will match the result of {@link #name()}, so that configuration is keyed by
+ * the actual cache name. However, it may be changed, for example to reuse the size limits of some
+ * other cache.
+ */
+ String configKey();
+
+ TypeLiteral<K> keyType();
+
+ TypeLiteral<V> valueType();
+
+ long maximumWeight();
+
+ @Nullable
+ Long expireAfterWrite(TimeUnit unit);
+
+ @Nullable
+ Weigher<K, V> weigher();
+
+ @Nullable
+ CacheLoader<K, V> loader();
+}
diff --git a/java/com/google/gerrit/server/cache/CacheModule.java b/java/com/google/gerrit/server/cache/CacheModule.java
index 0e0c16f..ca399e7 100644
--- a/java/com/google/gerrit/server/cache/CacheModule.java
+++ b/java/com/google/gerrit/server/cache/CacheModule.java
@@ -24,9 +24,9 @@
import com.google.inject.Provider;
import com.google.inject.Scopes;
import com.google.inject.TypeLiteral;
+import com.google.inject.name.Named;
import com.google.inject.name.Names;
import com.google.inject.util.Types;
-import java.io.Serializable;
import java.lang.reflect.Type;
/** Miniature DSL to support binding {@link Cache} instances in Guice. */
@@ -67,15 +67,9 @@
*/
protected <K, V> CacheBinding<K, V> cache(
String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
- Type type = Types.newParameterizedType(Cache.class, keyType.getType(), valType.getType());
-
- @SuppressWarnings("unchecked")
- Key<Cache<K, V>> key = (Key<Cache<K, V>>) Key.get(type, Names.named(name));
-
CacheProvider<K, V> m = new CacheProvider<>(this, name, keyType, valType);
- bind(key).toProvider(m).asEagerSingleton();
- bind(ANY_CACHE).annotatedWith(Exports.named(name)).to(key);
- return m.maximumWeight(1024);
+ bindCache(m, name, keyType, valType);
+ return m;
}
<K, V> Provider<CacheLoader<K, V>> bindCacheLoader(
@@ -126,7 +120,7 @@
* @param <V> type of value stored by the cache.
* @return binding to describe the cache.
*/
- protected <K extends Serializable, V extends Serializable> CacheBinding<K, V> persist(
+ protected <K, V> PersistentCacheBinding<K, V> persist(
String name, Class<K> keyType, Class<V> valType) {
return persist(name, TypeLiteral.get(keyType), TypeLiteral.get(valType));
}
@@ -138,7 +132,7 @@
* @param <V> type of value stored by the cache.
* @return binding to describe the cache.
*/
- protected <K extends Serializable, V extends Serializable> CacheBinding<K, V> persist(
+ protected <K, V> PersistentCacheBinding<K, V> persist(
String name, Class<K> keyType, TypeLiteral<V> valType) {
return persist(name, TypeLiteral.get(keyType), valType);
}
@@ -150,8 +144,40 @@
* @param <V> type of value stored by the cache.
* @return binding to describe the cache.
*/
- protected <K extends Serializable, V extends Serializable> CacheBinding<K, V> persist(
+ protected <K, V> PersistentCacheBinding<K, V> persist(
String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
- return ((CacheProvider<K, V>) cache(name, keyType, valType)).persist(true);
+ PersistentCacheProvider<K, V> m = new PersistentCacheProvider<>(this, name, keyType, valType);
+ bindCache(m, name, keyType, valType);
+
+ Type cacheDefType =
+ Types.newParameterizedType(PersistentCacheDef.class, keyType.getType(), valType.getType());
+ @SuppressWarnings("unchecked")
+ Key<PersistentCacheDef<K, V>> cacheDefKey =
+ (Key<PersistentCacheDef<K, V>>) Key.get(cacheDefType, Names.named(name));
+ bind(cacheDefKey).toInstance(m);
+
+ // TODO(dborowitz): Once default Java serialization is removed, leave no default.
+ return m.version(0)
+ .keySerializer(new JavaCacheSerializer<>())
+ .valueSerializer(new JavaCacheSerializer<>());
+ }
+
+ private <K, V> void bindCache(
+ CacheProvider<K, V> m, String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
+ Type type = Types.newParameterizedType(Cache.class, keyType.getType(), valType.getType());
+ Named named = Names.named(name);
+
+ @SuppressWarnings("unchecked")
+ Key<Cache<K, V>> key = (Key<Cache<K, V>>) Key.get(type, named);
+ bind(key).toProvider(m).asEagerSingleton();
+ bind(ANY_CACHE).annotatedWith(Exports.named(name)).to(key);
+
+ Type cacheDefType =
+ Types.newParameterizedType(CacheDef.class, keyType.getType(), valType.getType());
+ @SuppressWarnings("unchecked")
+ Key<CacheDef<K, V>> cacheDefKey = (Key<CacheDef<K, V>>) Key.get(cacheDefType, named);
+ bind(cacheDefKey).toInstance(m);
+
+ m.maximumWeight(1024);
}
}
diff --git a/java/com/google/gerrit/server/cache/CacheProvider.java b/java/com/google/gerrit/server/cache/CacheProvider.java
index b1a72ca..ce94bfa 100644
--- a/java/com/google/gerrit/server/cache/CacheProvider.java
+++ b/java/com/google/gerrit/server/cache/CacheProvider.java
@@ -14,9 +14,10 @@
package com.google.gerrit.server.cache;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
import static java.util.concurrent.TimeUnit.SECONDS;
-import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheLoader;
@@ -28,22 +29,19 @@
import com.google.inject.TypeLiteral;
import java.util.concurrent.TimeUnit;
-class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V> {
+class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>, CacheDef<K, V> {
private final CacheModule module;
final String name;
private final TypeLiteral<K> keyType;
private final TypeLiteral<V> valType;
private String configKey;
- private boolean persist;
private long maximumWeight;
- private long diskLimit;
private Long expireAfterWrite;
private Provider<CacheLoader<K, V>> loader;
private Provider<Weigher<K, V>> weigher;
private String plugin;
private MemoryCacheFactory memoryCacheFactory;
- private PersistentCacheFactory persistentCacheFactory;
private boolean frozen;
CacheProvider(CacheModule module, String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
@@ -63,57 +61,38 @@
this.memoryCacheFactory = factory;
}
- @Inject(optional = true)
- void setPersistentCacheFactory(@Nullable PersistentCacheFactory factory) {
- this.persistentCacheFactory = factory;
- }
-
- CacheBinding<K, V> persist(boolean p) {
- Preconditions.checkState(!frozen, "binding frozen, cannot be modified");
- persist = p;
- return this;
- }
-
@Override
public CacheBinding<K, V> maximumWeight(long weight) {
- Preconditions.checkState(!frozen, "binding frozen, cannot be modified");
+ checkNotFrozen();
maximumWeight = weight;
return this;
}
@Override
- public CacheBinding<K, V> diskLimit(long limit) {
- Preconditions.checkState(!frozen, "binding frozen, cannot be modified");
- Preconditions.checkState(persist, "diskLimit supported for persistent caches only");
- diskLimit = limit;
- return this;
- }
-
- @Override
public CacheBinding<K, V> expireAfterWrite(long duration, TimeUnit unit) {
- Preconditions.checkState(!frozen, "binding frozen, cannot be modified");
+ checkNotFrozen();
expireAfterWrite = SECONDS.convert(duration, unit);
return this;
}
@Override
public CacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> impl) {
- Preconditions.checkState(!frozen, "binding frozen, cannot be modified");
+ checkNotFrozen();
loader = module.bindCacheLoader(this, impl);
return this;
}
@Override
public CacheBinding<K, V> weigher(Class<? extends Weigher<K, V>> impl) {
- Preconditions.checkState(!frozen, "binding frozen, cannot be modified");
+ checkNotFrozen();
weigher = module.bindWeigher(this, impl);
return this;
}
@Override
public CacheBinding<K, V> configKey(String name) {
- Preconditions.checkState(!frozen, "binding frozen, cannot be modified");
- configKey = Preconditions.checkNotNull(name);
+ checkNotFrozen();
+ configKey = checkNotNull(name);
return this;
}
@@ -146,14 +125,6 @@
}
@Override
- public long diskLimit() {
- if (diskLimit > 0) {
- return diskLimit;
- }
- return 128 << 20;
- }
-
- @Override
@Nullable
public Long expireAfterWrite(TimeUnit unit) {
return expireAfterWrite != null ? unit.convert(expireAfterWrite, SECONDS) : null;
@@ -173,18 +144,16 @@
@Override
public Cache<K, V> get() {
- frozen = true;
+ freeze();
+ CacheLoader<K, V> ldr = loader();
+ return ldr != null ? memoryCacheFactory.build(this, ldr) : memoryCacheFactory.build(this);
+ }
- if (loader != null) {
- CacheLoader<K, V> ldr = loader.get();
- if (persist && persistentCacheFactory != null) {
- return persistentCacheFactory.build(this, ldr);
- }
- return memoryCacheFactory.build(this, ldr);
- } else if (persist && persistentCacheFactory != null) {
- return persistentCacheFactory.build(this);
- } else {
- return memoryCacheFactory.build(this);
- }
+ protected void checkNotFrozen() {
+ checkState(!frozen, "binding frozen, cannot be modified");
+ }
+
+ protected void freeze() {
+ frozen = true;
}
}
diff --git a/java/com/google/gerrit/server/cache/CacheSerializer.java b/java/com/google/gerrit/server/cache/CacheSerializer.java
new file mode 100644
index 0000000..6bd1322
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/CacheSerializer.java
@@ -0,0 +1,26 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import java.io.IOException;
+
+/** Interface for serializing/deserializing a type to/from a persistent cache. */
+public interface CacheSerializer<T> {
+ /** Serializes the object to a new byte array. */
+ byte[] serialize(T object) throws IOException;
+
+ /** Deserializes a single object from the given byte array. */
+ T deserialize(byte[] in) throws IOException;
+}
diff --git a/java/com/google/gerrit/server/cache/EnumCacheSerializer.java b/java/com/google/gerrit/server/cache/EnumCacheSerializer.java
new file mode 100644
index 0000000..6ea61215
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/EnumCacheSerializer.java
@@ -0,0 +1,41 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.base.Enums;
+import java.io.IOException;
+
+public class EnumCacheSerializer<E extends Enum<E>> implements CacheSerializer<E> {
+ private final Class<E> clazz;
+
+ public EnumCacheSerializer(Class<E> clazz) {
+ this.clazz = clazz;
+ }
+
+ @Override
+ public byte[] serialize(E object) throws IOException {
+ return object.name().getBytes(UTF_8);
+ }
+
+ @Override
+ public E deserialize(byte[] in) throws IOException {
+ String name = new String(in, UTF_8);
+ return Enums.getIfPresent(clazz, name)
+ .toJavaUtil()
+ .orElseThrow(() -> new IOException("Invalid " + clazz.getName() + " value: " + name));
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/JavaCacheSerializer.java b/java/com/google/gerrit/server/cache/JavaCacheSerializer.java
new file mode 100644
index 0000000..750c5df
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/JavaCacheSerializer.java
@@ -0,0 +1,52 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+
+/**
+ * Serializer that uses default Java serialization.
+ *
+ * @param <T> type to serialize. Must implement {@code Serializable}, but due to implementation
+ * details this is only checked at runtime.
+ */
+public class JavaCacheSerializer<T> implements CacheSerializer<T> {
+ @Override
+ public byte[] serialize(T object) throws IOException {
+ try (ByteArrayOutputStream bout = new ByteArrayOutputStream();
+ ObjectOutputStream oout = new ObjectOutputStream(bout)) {
+ oout.writeObject(object);
+ oout.flush();
+ return bout.toByteArray();
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ @Override
+ public T deserialize(byte[] in) throws IOException {
+ Object object;
+ try (ByteArrayInputStream bin = new ByteArrayInputStream(in);
+ ObjectInputStream oin = new ObjectInputStream(bin)) {
+ object = oin.readObject();
+ } catch (ClassNotFoundException e) {
+ throw new IOException("Failed to deserialize object of type", e);
+ }
+ return (T) object;
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/MemoryCacheFactory.java b/java/com/google/gerrit/server/cache/MemoryCacheFactory.java
index 49fcd5b..fc55753 100644
--- a/java/com/google/gerrit/server/cache/MemoryCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/MemoryCacheFactory.java
@@ -19,7 +19,7 @@
import com.google.common.cache.LoadingCache;
public interface MemoryCacheFactory {
- <K, V> Cache<K, V> build(CacheBinding<K, V> def);
+ <K, V> Cache<K, V> build(CacheDef<K, V> def);
- <K, V> LoadingCache<K, V> build(CacheBinding<K, V> def, CacheLoader<K, V> loader);
+ <K, V> LoadingCache<K, V> build(CacheDef<K, V> def, CacheLoader<K, V> loader);
}
diff --git a/java/com/google/gerrit/server/cache/PersistentCacheBinding.java b/java/com/google/gerrit/server/cache/PersistentCacheBinding.java
new file mode 100644
index 0000000..429f5ab
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/PersistentCacheBinding.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.Weigher;
+import java.util.concurrent.TimeUnit;
+
+/** Configure a persistent cache declared within a {@link CacheModule} instance. */
+public interface PersistentCacheBinding<K, V> extends CacheBinding<K, V> {
+ @Override
+ PersistentCacheBinding<K, V> maximumWeight(long weight);
+
+ @Override
+ PersistentCacheBinding<K, V> expireAfterWrite(long duration, TimeUnit durationUnits);
+
+ @Override
+ PersistentCacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> clazz);
+
+ @Override
+ PersistentCacheBinding<K, V> weigher(Class<? extends Weigher<K, V>> clazz);
+
+ PersistentCacheBinding<K, V> version(int version);
+
+ /** Set the total on-disk limit of the cache */
+ PersistentCacheBinding<K, V> diskLimit(long limit);
+
+ PersistentCacheBinding<K, V> keySerializer(CacheSerializer<K> keySerializer);
+
+ PersistentCacheBinding<K, V> valueSerializer(CacheSerializer<V> valueSerializer);
+}
diff --git a/java/com/google/gerrit/server/cache/PersistentCacheDef.java b/java/com/google/gerrit/server/cache/PersistentCacheDef.java
new file mode 100644
index 0000000..9bd120f
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/PersistentCacheDef.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+public interface PersistentCacheDef<K, V> extends CacheDef<K, V> {
+ long diskLimit();
+
+ int version();
+
+ CacheSerializer<K> keySerializer();
+
+ CacheSerializer<V> valueSerializer();
+}
diff --git a/java/com/google/gerrit/server/cache/PersistentCacheFactory.java b/java/com/google/gerrit/server/cache/PersistentCacheFactory.java
index d134adf..27fa9ca 100644
--- a/java/com/google/gerrit/server/cache/PersistentCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/PersistentCacheFactory.java
@@ -19,9 +19,9 @@
import com.google.common.cache.LoadingCache;
public interface PersistentCacheFactory {
- <K, V> Cache<K, V> build(CacheBinding<K, V> def);
+ <K, V> Cache<K, V> build(PersistentCacheDef<K, V> def);
- <K, V> LoadingCache<K, V> build(CacheBinding<K, V> def, CacheLoader<K, V> loader);
+ <K, V> LoadingCache<K, V> build(PersistentCacheDef<K, V> def, CacheLoader<K, V> loader);
void onStop(String plugin);
}
diff --git a/java/com/google/gerrit/server/cache/PersistentCacheProvider.java b/java/com/google/gerrit/server/cache/PersistentCacheProvider.java
new file mode 100644
index 0000000..405de4f
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/PersistentCacheProvider.java
@@ -0,0 +1,143 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.Weigher;
+import com.google.gerrit.common.Nullable;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.TypeLiteral;
+import java.io.Serializable;
+import java.util.concurrent.TimeUnit;
+
+class PersistentCacheProvider<K, V> extends CacheProvider<K, V>
+ implements Provider<Cache<K, V>>, PersistentCacheBinding<K, V>, PersistentCacheDef<K, V> {
+ private int version;
+ private long diskLimit;
+ private CacheSerializer<K> keySerializer;
+ private CacheSerializer<V> valueSerializer;
+
+ private PersistentCacheFactory persistentCacheFactory;
+
+ PersistentCacheProvider(
+ CacheModule module, String name, TypeLiteral<K> keyType, TypeLiteral<V> valType) {
+ super(module, name, keyType, valType);
+ version = -1;
+ }
+
+ @Inject(optional = true)
+ void setPersistentCacheFactory(@Nullable PersistentCacheFactory factory) {
+ this.persistentCacheFactory = factory;
+ }
+
+ @Override
+ public PersistentCacheBinding<K, V> maximumWeight(long weight) {
+ return (PersistentCacheBinding<K, V>) super.maximumWeight(weight);
+ }
+
+ @Override
+ public PersistentCacheBinding<K, V> expireAfterWrite(long duration, TimeUnit durationUnits) {
+ return (PersistentCacheBinding<K, V>) super.expireAfterWrite(duration, durationUnits);
+ }
+
+ @Override
+ public PersistentCacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> clazz) {
+ return (PersistentCacheBinding<K, V>) super.loader(clazz);
+ }
+
+ @Override
+ public PersistentCacheBinding<K, V> weigher(Class<? extends Weigher<K, V>> clazz) {
+ return (PersistentCacheBinding<K, V>) super.weigher(clazz);
+ }
+
+ @Override
+ public PersistentCacheBinding<K, V> version(int version) {
+ this.version = version;
+ return this;
+ }
+
+ @Override
+ public PersistentCacheBinding<K, V> keySerializer(CacheSerializer<K> keySerializer) {
+ this.keySerializer = keySerializer;
+ return this;
+ }
+
+ @Override
+ public PersistentCacheBinding<K, V> valueSerializer(CacheSerializer<V> valueSerializer) {
+ this.valueSerializer = valueSerializer;
+ return this;
+ }
+
+ @Override
+ public PersistentCacheBinding<K, V> diskLimit(long limit) {
+ checkNotFrozen();
+ diskLimit = limit;
+ return this;
+ }
+
+ @Override
+ public long diskLimit() {
+ if (diskLimit > 0) {
+ return diskLimit;
+ }
+ return 128 << 20;
+ }
+
+ @Override
+ public int version() {
+ return version;
+ }
+
+ @Override
+ public CacheSerializer<K> keySerializer() {
+ return keySerializer;
+ }
+
+ @Override
+ public CacheSerializer<V> valueSerializer() {
+ return valueSerializer;
+ }
+
+ @Override
+ public Cache<K, V> get() {
+ if (persistentCacheFactory == null) {
+ return super.get();
+ }
+ checkState(version >= 0, "version is required");
+ checkSerializer(keyType(), keySerializer, "key");
+ checkSerializer(valueType(), valueSerializer, "value");
+ freeze();
+ CacheLoader<K, V> ldr = loader();
+ return ldr != null
+ ? persistentCacheFactory.build(this, ldr)
+ : persistentCacheFactory.build(this);
+ }
+
+ private static <T> void checkSerializer(
+ TypeLiteral<T> type, CacheSerializer<T> serializer, String name) {
+ checkState(serializer != null, "%sSerializer is required", name);
+ if (serializer instanceof JavaCacheSerializer) {
+ checkState(
+ Serializable.class.isAssignableFrom(type.getRawType()),
+ "%s type %s must implement Serializable",
+ name,
+ type);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java b/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java
new file mode 100644
index 0000000..795df72
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/ProtoCacheSerializers.java
@@ -0,0 +1,47 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import com.google.protobuf.CodedOutputStream;
+import com.google.protobuf.MessageLite;
+import java.io.IOException;
+
+/** Static utilities for writing protobuf-based {@link CacheSerializer} implementations. */
+public class ProtoCacheSerializers {
+ /**
+ * Serializes a proto to a byte array.
+ *
+ * <p>Guarantees deterministic serialization and thus is suitable for use as a persistent cache
+ * key. Should be used in preference to {@link MessageLite#toByteArray()}, which is not guaranteed
+ * deterministic.
+ *
+ * @param message the proto message to serialize.
+ * @return a byte array with the message contents.
+ */
+ public static byte[] toByteArray(MessageLite message) {
+ byte[] bytes = new byte[message.getSerializedSize()];
+ CodedOutputStream cout = CodedOutputStream.newInstance(bytes);
+ cout.useDeterministicSerialization();
+ try {
+ message.writeTo(cout);
+ cout.checkNoSpaceLeft();
+ return bytes;
+ } catch (IOException e) {
+ throw new IllegalStateException("exception writing to byte array");
+ }
+ }
+
+ private ProtoCacheSerializers() {}
+}
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheBindingProxy.java b/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java
similarity index 66%
rename from java/com/google/gerrit/server/cache/h2/H2CacheBindingProxy.java
rename to java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java
index 2871080..7b3da31 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheBindingProxy.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java
@@ -16,18 +16,16 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.Weigher;
-import com.google.gerrit.server.cache.CacheBinding;
+import com.google.gerrit.server.cache.CacheSerializer;
+import com.google.gerrit.server.cache.PersistentCacheDef;
import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder;
import com.google.inject.TypeLiteral;
import java.util.concurrent.TimeUnit;
-class H2CacheBindingProxy<K, V> implements CacheBinding<K, V> {
- private static final String MSG_NOT_SUPPORTED =
- "This is read-only wrapper. Modifications are not supported";
+class H2CacheDefProxy<K, V> implements PersistentCacheDef<K, V> {
+ private final PersistentCacheDef<K, V> source;
- private final CacheBinding<K, V> source;
-
- H2CacheBindingProxy(CacheBinding<K, V> source) {
+ H2CacheDefProxy(PersistentCacheDef<K, V> source) {
this.source = source;
}
@@ -91,32 +89,17 @@
}
@Override
- public CacheBinding<K, V> configKey(String configKey) {
- throw new RuntimeException(MSG_NOT_SUPPORTED);
+ public int version() {
+ return source.version();
}
@Override
- public CacheBinding<K, V> maximumWeight(long weight) {
- throw new RuntimeException(MSG_NOT_SUPPORTED);
+ public CacheSerializer<K> keySerializer() {
+ return source.keySerializer();
}
@Override
- public CacheBinding<K, V> diskLimit(long limit) {
- throw new RuntimeException(MSG_NOT_SUPPORTED);
- }
-
- @Override
- public CacheBinding<K, V> expireAfterWrite(long duration, TimeUnit durationUnits) {
- throw new RuntimeException(MSG_NOT_SUPPORTED);
- }
-
- @Override
- public CacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> clazz) {
- throw new RuntimeException(MSG_NOT_SUPPORTED);
- }
-
- @Override
- public CacheBinding<K, V> weigher(Class<? extends Weigher<K, V>> clazz) {
- throw new RuntimeException(MSG_NOT_SUPPORTED);
+ public CacheSerializer<V> valueSerializer() {
+ return source.valueSerializer();
}
}
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
index db964c3..451540d 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheFactory.java
@@ -20,8 +20,8 @@
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.server.cache.CacheBinding;
import com.google.gerrit.server.cache.MemoryCacheFactory;
+import com.google.gerrit.server.cache.PersistentCacheDef;
import com.google.gerrit.server.cache.PersistentCacheFactory;
import com.google.gerrit.server.cache.h2.H2CacheImpl.SqlStore;
import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder;
@@ -30,7 +30,6 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import com.google.inject.TypeLiteral;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -154,16 +153,15 @@
@SuppressWarnings({"unchecked"})
@Override
- public <K, V> Cache<K, V> build(CacheBinding<K, V> in) {
+ public <K, V> Cache<K, V> build(PersistentCacheDef<K, V> in) {
long limit = config.getLong("cache", in.configKey(), "diskLimit", in.diskLimit());
if (cacheDir == null || limit <= 0) {
return memCacheFactory.build(in);
}
- H2CacheBindingProxy<K, V> def = new H2CacheBindingProxy<>(in);
- SqlStore<K, V> store =
- newSqlStore(def.name(), def.keyType(), limit, def.expireAfterWrite(TimeUnit.SECONDS));
+ H2CacheDefProxy<K, V> def = new H2CacheDefProxy<>(in);
+ SqlStore<K, V> store = newSqlStore(def, limit);
H2CacheImpl<K, V> cache =
new H2CacheImpl<>(
executor, store, def.keyType(), (Cache<K, ValueHolder<V>>) memCacheFactory.build(def));
@@ -175,16 +173,15 @@
@SuppressWarnings("unchecked")
@Override
- public <K, V> LoadingCache<K, V> build(CacheBinding<K, V> in, CacheLoader<K, V> loader) {
+ public <K, V> LoadingCache<K, V> build(PersistentCacheDef<K, V> in, CacheLoader<K, V> loader) {
long limit = config.getLong("cache", in.configKey(), "diskLimit", in.diskLimit());
if (cacheDir == null || limit <= 0) {
return memCacheFactory.build(in, loader);
}
- H2CacheBindingProxy<K, V> def = new H2CacheBindingProxy<>(in);
- SqlStore<K, V> store =
- newSqlStore(def.name(), def.keyType(), limit, def.expireAfterWrite(TimeUnit.SECONDS));
+ H2CacheDefProxy<K, V> def = new H2CacheDefProxy<>(in);
+ SqlStore<K, V> store = newSqlStore(def, limit);
Cache<K, ValueHolder<V>> mem =
(Cache<K, ValueHolder<V>>)
memCacheFactory.build(
@@ -208,10 +205,9 @@
}
}
- private <V, K> SqlStore<K, V> newSqlStore(
- String name, TypeLiteral<K> keyType, long maxSize, Long expireAfterWrite) {
+ private <V, K> SqlStore<K, V> newSqlStore(PersistentCacheDef<K, V> def, long maxSize) {
StringBuilder url = new StringBuilder();
- url.append("jdbc:h2:").append(cacheDir.resolve(name).toUri());
+ url.append("jdbc:h2:").append(cacheDir.resolve(def.name()).toUri());
if (h2CacheSize >= 0) {
url.append(";CACHE_SIZE=");
// H2 CACHE_SIZE is always given in KB
@@ -220,9 +216,13 @@
if (h2AutoServer) {
url.append(";AUTO_SERVER=TRUE");
}
+ Long expireAfterWrite = def.expireAfterWrite(TimeUnit.SECONDS);
return new SqlStore<>(
url.toString(),
- keyType,
+ def.keyType(),
+ def.keySerializer(),
+ def.valueSerializer(),
+ def.version(),
maxSize,
expireAfterWrite == null ? 0 : expireAfterWrite.longValue());
}
diff --git a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
index eaa9af9..7db1a35 100644
--- a/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
+++ b/java/com/google/gerrit/server/cache/h2/H2CacheImpl.java
@@ -22,23 +22,18 @@
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableSet;
import com.google.common.hash.BloomFilter;
-import com.google.common.hash.Funnel;
-import com.google.common.hash.Funnels;
-import com.google.common.hash.PrimitiveSink;
import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.server.cache.CacheSerializer;
import com.google.gerrit.server.cache.PersistentCache;
import com.google.inject.TypeLiteral;
import java.io.IOException;
import java.io.InvalidClassException;
-import java.io.ObjectOutputStream;
-import java.io.OutputStream;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.sql.Timestamp;
-import java.sql.Types;
import java.util.Calendar;
import java.util.Map;
import java.util.concurrent.ArrayBlockingQueue;
@@ -254,74 +249,11 @@
}
}
- private static class KeyType<K> {
- String columnType() {
- return "OTHER";
- }
-
- @SuppressWarnings("unchecked")
- K get(ResultSet rs, int col) throws SQLException {
- return (K) rs.getObject(col);
- }
-
- void set(PreparedStatement ps, int col, K value) throws SQLException {
- ps.setObject(col, value, Types.JAVA_OBJECT);
- }
-
- Funnel<K> funnel() {
- return new Funnel<K>() {
- private static final long serialVersionUID = 1L;
-
- @Override
- public void funnel(K from, PrimitiveSink into) {
- try (ObjectOutputStream ser = new ObjectOutputStream(new SinkOutputStream(into))) {
- ser.writeObject(from);
- ser.flush();
- } catch (IOException err) {
- throw new RuntimeException("Cannot hash as Serializable", err);
- }
- }
- };
- }
-
- @SuppressWarnings("unchecked")
- static <K> KeyType<K> create(TypeLiteral<K> type) {
- if (type.getRawType() == String.class) {
- return (KeyType<K>) STRING;
- }
- return (KeyType<K>) OTHER;
- }
-
- static final KeyType<?> OTHER = new KeyType<>();
- static final KeyType<String> STRING =
- new KeyType<String>() {
- @Override
- String columnType() {
- return "VARCHAR(4096)";
- }
-
- @Override
- String get(ResultSet rs, int col) throws SQLException {
- return rs.getString(col);
- }
-
- @Override
- void set(PreparedStatement ps, int col, String value) throws SQLException {
- ps.setString(col, value);
- }
-
- @SuppressWarnings("unchecked")
- @Override
- Funnel<String> funnel() {
- Funnel<?> s = Funnels.unencodedCharsFunnel();
- return (Funnel<String>) s;
- }
- };
- }
-
static class SqlStore<K, V> {
private final String url;
private final KeyType<K> keyType;
+ private final CacheSerializer<V> valueSerializer;
+ private final int version;
private final long maxSize;
private final long expireAfterWrite;
private final BlockingQueue<SqlHandle> handles;
@@ -330,9 +262,18 @@
private volatile BloomFilter<K> bloomFilter;
private int estimatedSize;
- SqlStore(String jdbcUrl, TypeLiteral<K> keyType, long maxSize, long expireAfterWrite) {
+ SqlStore(
+ String jdbcUrl,
+ TypeLiteral<K> keyType,
+ CacheSerializer<K> keySerializer,
+ CacheSerializer<V> valueSerializer,
+ int version,
+ long maxSize,
+ long expireAfterWrite) {
this.url = jdbcUrl;
- this.keyType = KeyType.create(keyType);
+ this.keyType = createKeyType(keyType, keySerializer);
+ this.valueSerializer = valueSerializer;
+ this.version = version;
this.maxSize = maxSize;
this.expireAfterWrite = expireAfterWrite;
@@ -341,6 +282,15 @@
this.handles = new ArrayBlockingQueue<>(keep);
}
+ @SuppressWarnings("unchecked")
+ private static <T> KeyType<T> createKeyType(
+ TypeLiteral<T> type, CacheSerializer<T> serializer) {
+ if (type.getRawType() == String.class) {
+ return (KeyType<T>) StringKeyTypeImpl.INSTANCE;
+ }
+ return new ObjectKeyTypeImpl<>(serializer);
+ }
+
synchronized void open() {
if (bloomFilter == null) {
bloomFilter = buildBloomFilter();
@@ -372,33 +322,43 @@
SqlHandle c = null;
try {
c = acquire();
- try (Statement s = c.conn.createStatement()) {
- if (estimatedSize <= 0) {
- try (ResultSet r = s.executeQuery("SELECT COUNT(*) FROM data")) {
+ if (estimatedSize <= 0) {
+ try (PreparedStatement ps =
+ c.conn.prepareStatement("SELECT COUNT(*) FROM data WHERE version=?")) {
+ ps.setInt(1, version);
+ try (ResultSet r = ps.executeQuery()) {
estimatedSize = r.next() ? r.getInt(1) : 0;
}
}
+ }
- BloomFilter<K> b = newBloomFilter();
- try (ResultSet r = s.executeQuery("SELECT k FROM data")) {
+ BloomFilter<K> b = newBloomFilter();
+ try (PreparedStatement ps = c.conn.prepareStatement("SELECT k FROM data WHERE version=?")) {
+ ps.setInt(1, version);
+ try (ResultSet r = ps.executeQuery()) {
while (r.next()) {
b.put(keyType.get(r, 1));
}
- } catch (JdbcSQLException e) {
- if (e.getCause() instanceof InvalidClassException) {
- log.warn(
- "Entries cached for "
- + url
- + " have an incompatible class and can't be deserialized. "
- + "Cache is flushed.");
- invalidateAll();
- } else {
- throw e;
- }
}
- return b;
+ } catch (JdbcSQLException e) {
+ if (e.getCause() instanceof InvalidClassException) {
+ // If deserialization failed using default Java serialization, this means we are using
+ // the old serialVersionUID-based invalidation strategy. In that case, authors are
+ // most likely bumping serialVersionUID rather than using the new versioning in the
+ // CacheBinding. That's ok; we'll continue to support both for now.
+ // TODO(dborowitz): Remove this case when Java serialization is no longer used.
+ log.warn(
+ "Entries cached for "
+ + url
+ + " have an incompatible class and can't be deserialized. "
+ + "Cache is flushed.");
+ invalidateAll();
+ } else {
+ throw e;
+ }
}
- } catch (SQLException e) {
+ return b;
+ } catch (IOException | SQLException e) {
log.warn("Cannot build BloomFilter for " + url + ": " + e.getMessage());
c = close(c);
return null;
@@ -412,9 +372,14 @@
try {
c = acquire();
if (c.get == null) {
- c.get = c.conn.prepareStatement("SELECT v, created FROM data WHERE k=?");
+ c.get = c.conn.prepareStatement("SELECT v, created FROM data WHERE k=? AND version=?");
}
keyType.set(c.get, 1, key);
+
+ // Silently no results when the only value in the database is an older version. This will
+ // result in put overwriting the stored value with the new version, which is intended.
+ c.get.setInt(2, version);
+
try (ResultSet r = c.get.executeQuery()) {
if (!r.next()) {
missCount.incrementAndGet();
@@ -428,8 +393,7 @@
return null;
}
- @SuppressWarnings("unchecked")
- V val = (V) r.getObject(1);
+ V val = valueSerializer.deserialize(r.getBytes(1));
ValueHolder<V> h = new ValueHolder<>(val);
h.clean = true;
hitCount.incrementAndGet();
@@ -438,7 +402,7 @@
} finally {
c.get.clearParameters();
}
- } catch (SQLException e) {
+ } catch (IOException | SQLException e) {
if (!isOldClassNameError(e)) {
log.warn("Cannot read cache " + url + " for " + key, e);
}
@@ -466,13 +430,14 @@
return 1000 * expireAfterWrite < age;
}
- private void touch(SqlHandle c, K key) throws SQLException {
+ private void touch(SqlHandle c, K key) throws IOException, SQLException {
if (c.touch == null) {
- c.touch = c.conn.prepareStatement("UPDATE data SET accessed=? WHERE k=?");
+ c.touch = c.conn.prepareStatement("UPDATE data SET accessed=? WHERE k=? AND version=?");
}
try {
c.touch.setTimestamp(1, TimeUtil.nowTs());
keyType.set(c.touch, 2, key);
+ c.touch.setInt(3, version);
c.touch.executeUpdate();
} finally {
c.touch.clearParameters();
@@ -495,19 +460,21 @@
c = acquire();
if (c.put == null) {
c.put =
- c.conn.prepareStatement("MERGE INTO data (k, v, created, accessed) VALUES(?,?,?,?)");
+ c.conn.prepareStatement(
+ "MERGE INTO data (k, v, version, created, accessed) VALUES(?,?,?,?,?)");
}
try {
keyType.set(c.put, 1, key);
- c.put.setObject(2, holder.value, Types.JAVA_OBJECT);
- c.put.setTimestamp(3, new Timestamp(holder.created));
- c.put.setTimestamp(4, TimeUtil.nowTs());
+ c.put.setBytes(2, valueSerializer.serialize(holder.value));
+ c.put.setInt(3, version);
+ c.put.setTimestamp(4, new Timestamp(holder.created));
+ c.put.setTimestamp(5, TimeUtil.nowTs());
c.put.executeUpdate();
holder.clean = true;
} finally {
c.put.clearParameters();
}
- } catch (SQLException e) {
+ } catch (IOException | SQLException e) {
log.warn("Cannot put into cache " + url, e);
c = close(c);
} finally {
@@ -520,7 +487,7 @@
try {
c = acquire();
invalidate(c, key);
- } catch (SQLException e) {
+ } catch (IOException | SQLException e) {
log.warn("Cannot invalidate cache " + url, e);
c = close(c);
} finally {
@@ -528,12 +495,13 @@
}
}
- private void invalidate(SqlHandle c, K key) throws SQLException {
+ private void invalidate(SqlHandle c, K key) throws IOException, SQLException {
if (c.invalidate == null) {
- c.invalidate = c.conn.prepareStatement("DELETE FROM data WHERE k=?");
+ c.invalidate = c.conn.prepareStatement("DELETE FROM data WHERE k=? and version=?");
}
try {
keyType.set(c.invalidate, 1, key);
+ c.invalidate.setInt(2, version);
c.invalidate.executeUpdate();
} finally {
c.invalidate.clearParameters();
@@ -560,7 +528,15 @@
SqlHandle c = null;
try {
c = acquire();
+ try (PreparedStatement ps = c.conn.prepareStatement("DELETE FROM data WHERE version!=?")) {
+ ps.setInt(1, version);
+ int oldEntries = ps.executeUpdate();
+ log.info(
+ "Pruned {} entries not matching version {} from cache {}", oldEntries, version, url);
+ }
try (Statement s = c.conn.createStatement()) {
+ // Compute size without restricting to version (although obsolete data was just pruned
+ // anyway).
long used = 0;
try (ResultSet r = s.executeQuery("SELECT SUM(space) FROM data")) {
used = r.next() ? r.getLong(1) : 0;
@@ -570,8 +546,7 @@
}
try (ResultSet r =
- s.executeQuery(
- "SELECT" + " k" + ",space" + ",created" + " FROM data" + " ORDER BY accessed")) {
+ s.executeQuery("SELECT k, space, created FROM data ORDER BY accessed")) {
while (maxSize < used && r.next()) {
K key = keyType.get(r, 1);
Timestamp created = r.getTimestamp(3);
@@ -584,7 +559,7 @@
}
}
}
- } catch (SQLException e) {
+ } catch (IOException | SQLException e) {
log.warn("Cannot prune cache " + url, e);
c = close(c);
} finally {
@@ -599,7 +574,8 @@
try {
c = acquire();
try (Statement s = c.conn.createStatement();
- ResultSet r = s.executeQuery("SELECT" + " COUNT(*)" + ",SUM(space)" + " FROM data")) {
+ // Stats include total size regardless of version.
+ ResultSet r = s.executeQuery("SELECT COUNT(*), SUM(space) FROM data")) {
if (r.next()) {
size = r.getLong(1);
space = r.getLong(2);
@@ -662,6 +638,7 @@
stmt.addBatch(
"ALTER TABLE data ADD COLUMN IF NOT EXISTS "
+ "space BIGINT AS OCTET_LENGTH(k) + OCTET_LENGTH(v)");
+ stmt.addBatch("ALTER TABLE data ADD COLUMN IF NOT EXISTS version INT DEFAULT 0 NOT NULL");
stmt.executeBatch();
}
}
@@ -694,22 +671,4 @@
return null;
}
}
-
- private static class SinkOutputStream extends OutputStream {
- private final PrimitiveSink sink;
-
- SinkOutputStream(PrimitiveSink sink) {
- this.sink = sink;
- }
-
- @Override
- public void write(int b) {
- sink.putByte((byte) b);
- }
-
- @Override
- public void write(byte[] b, int p, int n) {
- sink.putBytes(b, p, n);
- }
- }
}
diff --git a/java/com/google/gerrit/server/cache/h2/KeyType.java b/java/com/google/gerrit/server/cache/h2/KeyType.java
new file mode 100644
index 0000000..3045e20
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/h2/KeyType.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache.h2;
+
+import com.google.common.hash.Funnel;
+import java.io.IOException;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+interface KeyType<K> {
+ String columnType();
+
+ K get(ResultSet rs, int col) throws IOException, SQLException;
+
+ void set(PreparedStatement ps, int col, K key) throws IOException, SQLException;
+
+ Funnel<K> funnel();
+}
diff --git a/java/com/google/gerrit/server/cache/h2/ObjectKeyTypeImpl.java b/java/com/google/gerrit/server/cache/h2/ObjectKeyTypeImpl.java
new file mode 100644
index 0000000..b1a65fe
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/h2/ObjectKeyTypeImpl.java
@@ -0,0 +1,63 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache.h2;
+
+import com.google.common.hash.Funnel;
+import com.google.common.hash.Funnels;
+import com.google.common.hash.PrimitiveSink;
+import com.google.gerrit.server.cache.CacheSerializer;
+import java.io.IOException;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+class ObjectKeyTypeImpl<K> implements KeyType<K> {
+ private final CacheSerializer<K> serializer;
+
+ ObjectKeyTypeImpl(CacheSerializer<K> serializer) {
+ this.serializer = serializer;
+ }
+
+ @Override
+ public String columnType() {
+ return "OTHER";
+ }
+
+ @Override
+ public K get(ResultSet rs, int col) throws IOException, SQLException {
+ return serializer.deserialize(rs.getBytes(col));
+ }
+
+ @Override
+ public void set(PreparedStatement ps, int col, K key) throws IOException, SQLException {
+ ps.setBytes(col, serializer.serialize(key));
+ }
+
+ @Override
+ public Funnel<K> funnel() {
+ return new Funnel<K>() {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public void funnel(K from, PrimitiveSink into) {
+ try {
+ Funnels.byteArrayFunnel().funnel(serializer.serialize(from), into);
+ } catch (IOException e) {
+ throw new RuntimeException("Cannot hash", e);
+ }
+ }
+ };
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/h2/StringKeyTypeImpl.java b/java/com/google/gerrit/server/cache/h2/StringKeyTypeImpl.java
new file mode 100644
index 0000000..8083ea5
--- /dev/null
+++ b/java/com/google/gerrit/server/cache/h2/StringKeyTypeImpl.java
@@ -0,0 +1,47 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache.h2;
+
+import com.google.common.hash.Funnel;
+import com.google.common.hash.Funnels;
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+enum StringKeyTypeImpl implements KeyType<String> {
+ INSTANCE;
+
+ @Override
+ public String columnType() {
+ return "VARCHAR(4096)";
+ }
+
+ @Override
+ public String get(ResultSet rs, int col) throws SQLException {
+ return rs.getString(col);
+ }
+
+ @Override
+ public void set(PreparedStatement ps, int col, String value) throws SQLException {
+ ps.setString(col, value);
+ }
+
+ @SuppressWarnings("unchecked")
+ @Override
+ public Funnel<String> funnel() {
+ Funnel<?> s = Funnels.unencodedCharsFunnel();
+ return (Funnel<String>) s;
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
index 126da8e..f16912a 100644
--- a/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
+++ b/java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java
@@ -20,7 +20,7 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.cache.Weigher;
-import com.google.gerrit.server.cache.CacheBinding;
+import com.google.gerrit.server.cache.CacheDef;
import com.google.gerrit.server.cache.ForwardingRemovalListener;
import com.google.gerrit.server.cache.MemoryCacheFactory;
import com.google.gerrit.server.config.ConfigUtil;
@@ -42,17 +42,17 @@
}
@Override
- public <K, V> Cache<K, V> build(CacheBinding<K, V> def) {
+ public <K, V> Cache<K, V> build(CacheDef<K, V> def) {
return create(def).build();
}
@Override
- public <K, V> LoadingCache<K, V> build(CacheBinding<K, V> def, CacheLoader<K, V> loader) {
+ public <K, V> LoadingCache<K, V> build(CacheDef<K, V> def, CacheLoader<K, V> loader) {
return create(def).build(loader);
}
@SuppressWarnings("unchecked")
- private <K, V> CacheBuilder<K, V> create(CacheBinding<K, V> def) {
+ private <K, V> CacheBuilder<K, V> create(CacheDef<K, V> def) {
CacheBuilder<K, V> builder = newCacheBuilder();
builder.recordStats();
builder.maximumWeight(
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index a08203e..34cbea3 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -39,7 +39,6 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.config.SendEmailExecutor;
import com.google.gerrit.server.events.CommitReceivedEvent;
@@ -93,7 +92,6 @@
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
- private final IdentifiedUser.GenericFactory userFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final PatchSetUtil psUtil;
private final ApprovalsUtil approvalsUtil;
@@ -143,7 +141,6 @@
ChangeInserter(
PermissionBackend permissionBackend,
ProjectCache projectCache,
- IdentifiedUser.GenericFactory userFactory,
PatchSetInfoFactory patchSetInfoFactory,
PatchSetUtil psUtil,
ApprovalsUtil approvalsUtil,
@@ -159,7 +156,6 @@
@Assisted String refName) {
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
- this.userFactory = userFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.psUtil = psUtil;
this.approvalsUtil = approvalsUtil;
@@ -465,9 +461,8 @@
.filter(
accountId -> {
try {
- IdentifiedUser user = userFactory.create(accountId);
return permissionBackend
- .user(user)
+ .absentUser(accountId)
.change(notes)
.database(db)
.test(ChangePermission.READ)
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 31702ba..33dc5cb 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -117,6 +117,7 @@
import com.google.gerrit.server.account.AccountInfoComparator;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.GpgApiAdapter;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
@@ -158,6 +159,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
+import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
@@ -273,6 +275,7 @@
private final RemoveReviewerControl removeReviewerControl;
private final TrackingFooters trackingFooters;
private final Metrics metrics;
+ private final boolean enableParallelFormatting;
private final ExecutorService fanOutExecutor;
private boolean lazyLoad = true;
@@ -307,6 +310,7 @@
RemoveReviewerControl removeReviewerControl,
TrackingFooters trackingFooters,
Metrics metrics,
+ @GerritServerConfig Config config,
@FanOutExecutor ExecutorService fanOutExecutor,
@Assisted Iterable<ListChangesOption> options) {
this.db = db;
@@ -334,6 +338,7 @@
this.removeReviewerControl = removeReviewerControl;
this.trackingFooters = trackingFooters;
this.metrics = metrics;
+ this.enableParallelFormatting = config.getBoolean("change", "enableParallelFormatting", false);
this.fanOutExecutor = fanOutExecutor;
this.options = Sets.immutableEnumSet(options);
}
@@ -504,7 +509,7 @@
}
long numProjects = changes.stream().map(c -> c.project()).distinct().count();
- if (!lazyLoad || changes.size() < 3 || numProjects < 2) {
+ if (!enableParallelFormatting || !lazyLoad || changes.size() < 3 || numProjects < 2) {
// Format these changes in the request thread as the multithreading overhead would be too
// high.
List<ChangeInfo> result = new ArrayList<>(changes.size());
diff --git a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
index 7b44d8f..6449662 100644
--- a/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
+++ b/java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
@@ -16,8 +16,6 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
-import static org.eclipse.jgit.lib.ObjectIdSerializer.readWithoutMarker;
-import static org.eclipse.jgit.lib.ObjectIdSerializer.writeWithoutMarker;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.cache.Cache;
@@ -30,6 +28,10 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.CacheSerializer;
+import com.google.gerrit.server.cache.EnumCacheSerializer;
+import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.proto.Cache.ChangeKindKeyProto;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.InMemoryInserter;
@@ -39,10 +41,8 @@
import com.google.inject.Inject;
import com.google.inject.Module;
import com.google.inject.name.Named;
+import com.google.protobuf.ByteString;
import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
-import java.io.Serializable;
import java.util.Arrays;
import java.util.Collection;
import java.util.Objects;
@@ -51,6 +51,7 @@
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.errors.LargeObjectException;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.Repository;
@@ -72,7 +73,10 @@
bind(ChangeKindCache.class).to(ChangeKindCacheImpl.class);
persist(ID_CACHE, Key.class, ChangeKind.class)
.maximumWeight(2 << 20)
- .weigher(ChangeKindWeigher.class);
+ .weigher(ChangeKindWeigher.class)
+ .version(1)
+ .keySerializer(new Key.Serializer())
+ .valueSerializer(new EnumCacheSerializer<>(ChangeKind.class));
}
};
}
@@ -122,9 +126,7 @@
}
}
- public static class Key implements Serializable {
- private static final long serialVersionUID = 1L;
-
+ public static class Key {
private transient ObjectId prior;
private transient ObjectId next;
private transient String strategyName;
@@ -171,16 +173,28 @@
return Objects.hash(prior, next, strategyName);
}
- private void writeObject(ObjectOutputStream out) throws IOException {
- writeWithoutMarker(out, prior);
- writeWithoutMarker(out, next);
- out.writeUTF(strategyName);
- }
+ @VisibleForTesting
+ static class Serializer implements CacheSerializer<Key> {
+ @Override
+ public byte[] serialize(Key object) throws IOException {
+ byte[] buf = new byte[Constants.OBJECT_ID_LENGTH];
+ ChangeKindKeyProto.Builder b = ChangeKindKeyProto.newBuilder();
+ object.getPrior().copyRawTo(buf, 0);
+ b.setPrior(ByteString.copyFrom(buf));
+ object.getNext().copyRawTo(buf, 0);
+ b.setNext(ByteString.copyFrom(buf));
+ b.setStrategyName(object.getStrategyName());
+ return ProtoCacheSerializers.toByteArray(b.build());
+ }
- private void readObject(ObjectInputStream in) throws IOException {
- prior = readWithoutMarker(in);
- next = readWithoutMarker(in);
- strategyName = in.readUTF();
+ @Override
+ public Key deserialize(byte[] in) throws IOException {
+ ChangeKindKeyProto proto = ChangeKindKeyProto.parseFrom(in);
+ return new Key(
+ ObjectId.fromRaw(proto.getPrior().toByteArray()),
+ ObjectId.fromRaw(proto.getNext().toByteArray()),
+ proto.getStrategyName());
+ }
}
}
diff --git a/java/com/google/gerrit/server/change/PatchSetInserter.java b/java/com/google/gerrit/server/change/PatchSetInserter.java
index 3a32f8f..d05d133 100644
--- a/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -311,7 +311,11 @@
}
private void validate(RepoContext ctx)
- throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
+ throws AuthException, ResourceConflictException, IOException, PermissionBackendException,
+ OrmException {
+ // Not allowed to create a new patch set if the current patch set is locked.
+ psUtil.checkPatchSetNotLocked(origNotes, ctx.getUser());
+
if (checkAddPatchSetPermission) {
permissionBackend
.user(ctx.getUser())
diff --git a/java/com/google/gerrit/server/config/TrackingFootersProvider.java b/java/com/google/gerrit/server/config/TrackingFootersProvider.java
index 2b1af36..7b23fcc 100644
--- a/java/com/google/gerrit/server/config/TrackingFootersProvider.java
+++ b/java/com/google/gerrit/server/config/TrackingFootersProvider.java
@@ -62,7 +62,7 @@
} else if (system.length() > TrackingId.TRACKING_SYSTEM_MAX_CHAR) {
configValid = false;
log.error(
- "String to long \""
+ "String too long \""
+ system
+ "\" in gerrit.config "
+ TRACKING_ID_TAG
diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index 83f0120..80d1cd1 100644
--- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -394,7 +394,8 @@
}
private void assertCanEdit(ChangeNotes notes)
- throws AuthException, PermissionBackendException, IOException, ResourceConflictException {
+ throws AuthException, PermissionBackendException, IOException, ResourceConflictException,
+ OrmException {
if (!currentUser.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
@@ -406,6 +407,8 @@
"change %s is %s", c.getChangeId(), c.getStatus().toString().toLowerCase()));
}
+ // Not allowed to edit if the current patch set is locked.
+ patchSetUtil.checkPatchSetNotLocked(notes, currentUser.get());
try {
permissionBackend
.currentUser()
diff --git a/java/com/google/gerrit/server/events/EventFactory.java b/java/com/google/gerrit/server/events/EventFactory.java
index e557250..34b0357 100644
--- a/java/com/google/gerrit/server/events/EventFactory.java
+++ b/java/com/google/gerrit/server/events/EventFactory.java
@@ -158,7 +158,7 @@
try {
a.commitMessage = changeDataFactory.create(db, change).commitMessage();
} catch (Exception e) {
- log.error("Error while getting full commit message for change " + a.number);
+ log.error("Error while getting full commit message for change " + a.number, e);
}
a.url = getChangeUrl(change);
a.owner = asAccountAttribute(change.getOwner());
@@ -452,7 +452,7 @@
} catch (PatchListObjectTooLargeException e) {
log.warn("Cannot get patch list: " + e.getMessage());
} catch (PatchListNotAvailableException e) {
- log.warn("Cannot get patch list", e);
+ log.error("Cannot get patch list", e);
}
}
diff --git a/java/com/google/gerrit/server/extensions/events/EventUtil.java b/java/com/google/gerrit/server/extensions/events/EventUtil.java
index 1dd5537..b0bbce4 100644
--- a/java/com/google/gerrit/server/extensions/events/EventUtil.java
+++ b/java/com/google/gerrit/server/extensions/events/EventUtil.java
@@ -129,7 +129,7 @@
error);
} else {
log.warn(
- "Error in listener {} for event {}: {}",
+ "Error in event listener {} for event {}: {}",
listener.getClass().getName(),
event.getClass().getName(),
error.getMessage());
@@ -140,7 +140,7 @@
if (log.isDebugEnabled()) {
log.debug(String.format("Error in event listener %s", listener.getClass().getName()), error);
} else {
- log.warn("Error in listener {}: {}", listener.getClass().getName(), error.getMessage());
+ log.warn("Error in event listener {}: {}", listener.getClass().getName(), error.getMessage());
}
}
}
diff --git a/java/com/google/gerrit/server/git/TagCache.java b/java/com/google/gerrit/server/git/TagCache.java
index d346997..b8acd0a 100644
--- a/java/com/google/gerrit/server/git/TagCache.java
+++ b/java/com/google/gerrit/server/git/TagCache.java
@@ -103,7 +103,7 @@
cache.put(name.get(), val);
}
- static class EntryVal implements Serializable {
+ public static class EntryVal implements Serializable {
static final long serialVersionUID = 1L;
transient TagSetHolder holder;
diff --git a/java/com/google/gerrit/server/git/WorkQueue.java b/java/com/google/gerrit/server/git/WorkQueue.java
index 11e149c..3bc44c5 100644
--- a/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -85,7 +85,7 @@
};
private ScheduledExecutorService defaultQueue;
- private int defaultQueueSize;
+ private final int defaultQueueSize;
private final IdGenerator idGenerator;
private final CopyOnWriteArrayList<Executor> queues;
@@ -94,6 +94,7 @@
this(idGenerator, cfg.getInt("execution", "defaultThreadPoolSize", 1));
}
+ /** Constructor to allow binding the WorkQueue more explicitly in a vhost setup. */
public WorkQueue(IdGenerator idGenerator, int defaultThreadPoolSize) {
this.idGenerator = idGenerator;
this.queues = new CopyOnWriteArrayList<>();
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 5f65814..dbb6bcf 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2452,6 +2452,13 @@
RevCommit newCommit = rp.getRevWalk().parseCommit(newCommitId);
RevCommit priorCommit = revisions.inverse().get(priorPatchSet);
+
+ // Not allowed to create a new patch set if the current patch set is locked.
+ if (psUtil.isPatchSetLocked(notes, user)) {
+ reject(inputCommand, "cannot add patch set to " + ontoChange + ".");
+ return false;
+ }
+
try {
permissions.change(notes).database(db).check(ChangePermission.ADD_PATCH_SET);
} catch (AuthException no) {
diff --git a/java/com/google/gerrit/server/mail/AutoReplyMailFilter.java b/java/com/google/gerrit/server/mail/AutoReplyMailFilter.java
index 481c2e9..199731e 100644
--- a/java/com/google/gerrit/server/mail/AutoReplyMailFilter.java
+++ b/java/com/google/gerrit/server/mail/AutoReplyMailFilter.java
@@ -33,7 +33,7 @@
if (prec.equals("list") || prec.equals("junk") || prec.equals("bulk")) {
log.error(
- "Message %s has a Precedence header. Will ignore and delete message.", message.id());
+ "Message {} has a Precedence header. Will ignore and delete message.", message.id());
return false;
}
@@ -43,7 +43,7 @@
if (!autoSubmitted.equals("no")) {
log.error(
- "Message %s has an Auto-Submitted header. Will ignore and delete message.",
+ "Message {} has an Auto-Submitted header. Will ignore and delete message.",
message.id());
return false;
}
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 6c05ecc..11f50a9 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -27,7 +27,6 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
import com.google.gerrit.server.mail.MailHeader;
@@ -100,8 +99,7 @@
/** Is the from user in an email squelching group? */
try {
- IdentifiedUser user = args.identifiedUserFactory.create(id);
- args.permissionBackend.user(user).check(GlobalPermission.EMAIL_REVIEWERS);
+ args.permissionBackend.absentUser(id).check(GlobalPermission.EMAIL_REVIEWERS);
} catch (AuthException | PermissionBackendException e) {
emailOnlyAuthors = true;
}
diff --git a/java/com/google/gerrit/server/mail/send/EmailHeader.java b/java/com/google/gerrit/server/mail/send/EmailHeader.java
index 0bfe428..29354f2 100644
--- a/java/com/google/gerrit/server/mail/send/EmailHeader.java
+++ b/java/com/google/gerrit/server/mail/send/EmailHeader.java
@@ -23,7 +23,6 @@
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
@@ -185,11 +184,7 @@
}
void remove(java.lang.String email) {
- for (Iterator<Address> i = list.iterator(); i.hasNext(); ) {
- if (i.next().getEmail().equals(email)) {
- i.remove();
- }
- }
+ list.removeIf(address -> address.getEmail().equals(email));
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java b/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
index b08e594..10e8b0b 100644
--- a/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
+++ b/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
@@ -114,9 +114,7 @@
smtpPass = cfg.getString("sendemail", null, "smtppass");
Set<String> rcpt = new HashSet<>();
- for (String addr : cfg.getStringList("sendemail", null, "allowrcpt")) {
- rcpt.add(addr);
- }
+ Collections.addAll(rcpt, cfg.getStringList("sendemail", null, "allowrcpt"));
allowrcpt = Collections.unmodifiableSet(rcpt);
importance = cfg.getString("sendemail", null, "importance");
expiryDays = cfg.getInt("sendemail", null, "expiryDays", 0);
diff --git a/java/com/google/gerrit/server/notedb/RepoSequence.java b/java/com/google/gerrit/server/notedb/RepoSequence.java
index 3554f4b..47fec59 100644
--- a/java/com/google/gerrit/server/notedb/RepoSequence.java
+++ b/java/com/google/gerrit/server/notedb/RepoSequence.java
@@ -92,6 +92,7 @@
private final Project.NameKey projectName;
private final String refName;
private final Seed seed;
+ private final int floor;
private final int batchSize;
private final Runnable afterReadRef;
private final Retryer<RefUpdate.Result> retryer;
@@ -119,7 +120,28 @@
seed,
batchSize,
Runnables.doNothing(),
- RETRYER);
+ RETRYER,
+ 0);
+ }
+
+ public RepoSequence(
+ GitRepositoryManager repoManager,
+ GitReferenceUpdated gitRefUpdated,
+ Project.NameKey projectName,
+ String name,
+ Seed seed,
+ int batchSize,
+ int floor) {
+ this(
+ repoManager,
+ gitRefUpdated,
+ projectName,
+ name,
+ seed,
+ batchSize,
+ Runnables.doNothing(),
+ RETRYER,
+ floor);
}
@VisibleForTesting
@@ -132,6 +154,19 @@
int batchSize,
Runnable afterReadRef,
Retryer<RefUpdate.Result> retryer) {
+ this(repoManager, gitRefUpdated, projectName, name, seed, batchSize, afterReadRef, retryer, 0);
+ }
+
+ RepoSequence(
+ GitRepositoryManager repoManager,
+ GitReferenceUpdated gitRefUpdated,
+ Project.NameKey projectName,
+ String name,
+ Seed seed,
+ int batchSize,
+ Runnable afterReadRef,
+ Retryer<RefUpdate.Result> retryer,
+ int floor) {
this.repoManager = checkNotNull(repoManager, "repoManager");
this.gitRefUpdated = checkNotNull(gitRefUpdated, "gitRefUpdated");
this.projectName = checkNotNull(projectName, "projectName");
@@ -145,6 +180,7 @@
this.refName = RefNames.REFS_SEQUENCES + name;
this.seed = checkNotNull(seed, "seed");
+ this.floor = floor;
checkArgument(batchSize > 0, "expected batchSize > 0, got: %s", batchSize);
this.batchSize = batchSize;
@@ -282,6 +318,7 @@
oldId = ref.getObjectId();
next = parse(rw, oldId);
}
+ next = Math.max(floor, next);
return store(repo, rw, oldId, next + count);
}
}
diff --git a/java/com/google/gerrit/server/notedb/rebuild/CommentEvent.java b/java/com/google/gerrit/server/notedb/rebuild/CommentEvent.java
index 611f32e..1fffab4 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/CommentEvent.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/CommentEvent.java
@@ -68,7 +68,7 @@
setCommentRevId(c, cache, change, ps);
} catch (PatchListNotAvailableException e) {
log.warn(
- "Unable to determine parent commit of patch set {} ({}); omitting inline comment",
+ "Unable to determine parent commit of patch set {} ({}); omitting inline comment {}",
ps.getId(),
ps.getRevision(),
c);
diff --git a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index b408355..59fdde7 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -560,6 +560,8 @@
throws OrmException, IOException {
try (ReviewDb db = schemaFactory.open()) {
@SuppressWarnings("deprecation")
+ final int nextChangeId = db.nextChangeId();
+
RepoSequence seq =
new RepoSequence(
repoManager,
@@ -569,8 +571,9 @@
// If sequenceGap is 0, this writes into the sequence ref the same ID that is returned
// by the call to seq.next() below. If we actually used this as a change ID, that
// would be a problem, but we just discard it, so this is safe.
- () -> db.nextChangeId() + sequenceGap - 1,
- 1);
+ () -> nextChangeId + sequenceGap - 1,
+ 1,
+ nextChangeId);
seq.next();
}
return saveState(prev, READ_WRITE_WITH_SEQUENCE_REVIEW_DB_PRIMARY);
diff --git a/java/com/google/gerrit/server/patch/DiffSummaryLoader.java b/java/com/google/gerrit/server/patch/DiffSummaryLoader.java
index 4d614ff..8bca19f 100644
--- a/java/com/google/gerrit/server/patch/DiffSummaryLoader.java
+++ b/java/com/google/gerrit/server/patch/DiffSummaryLoader.java
@@ -22,12 +22,8 @@
import java.util.Collections;
import java.util.List;
import java.util.concurrent.Callable;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class DiffSummaryLoader implements Callable<DiffSummary> {
- static final Logger log = LoggerFactory.getLogger(DiffSummaryLoader.class);
-
public interface Factory {
DiffSummaryLoader create(DiffSummaryKey key, Project.NameKey project);
}
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 990c318..b13d921 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -21,19 +21,14 @@
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.common.data.LabelFunction;
-import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend.ForChange;
import com.google.gerrit.server.query.change.ChangeData;
@@ -52,19 +47,11 @@
static class Factory {
private final ChangeData.Factory changeDataFactory;
private final ChangeNotes.Factory notesFactory;
- private final ApprovalsUtil approvalsUtil;
- private final PatchSetUtil patchSetUtil;
@Inject
- Factory(
- ChangeData.Factory changeDataFactory,
- ChangeNotes.Factory notesFactory,
- ApprovalsUtil approvalsUtil,
- PatchSetUtil patchSetUtil) {
+ Factory(ChangeData.Factory changeDataFactory, ChangeNotes.Factory notesFactory) {
this.changeDataFactory = changeDataFactory;
this.notesFactory = notesFactory;
- this.approvalsUtil = approvalsUtil;
- this.patchSetUtil = patchSetUtil;
}
ChangeControl create(
@@ -74,27 +61,19 @@
}
ChangeControl create(RefControl refControl, ChangeNotes notes) {
- return new ChangeControl(changeDataFactory, approvalsUtil, refControl, notes, patchSetUtil);
+ return new ChangeControl(changeDataFactory, refControl, notes);
}
}
private final ChangeData.Factory changeDataFactory;
- private final ApprovalsUtil approvalsUtil;
private final RefControl refControl;
private final ChangeNotes notes;
- private final PatchSetUtil patchSetUtil;
private ChangeControl(
- ChangeData.Factory changeDataFactory,
- ApprovalsUtil approvalsUtil,
- RefControl refControl,
- ChangeNotes notes,
- PatchSetUtil patchSetUtil) {
+ ChangeData.Factory changeDataFactory, RefControl refControl, ChangeNotes notes) {
this.changeDataFactory = changeDataFactory;
- this.approvalsUtil = approvalsUtil;
this.refControl = refControl;
this.notes = notes;
- this.patchSetUtil = patchSetUtil;
}
ForChange asForChange(@Nullable ChangeData cd, @Nullable Provider<ReviewDb> db) {
@@ -105,8 +84,7 @@
if (getUser().equals(who)) {
return this;
}
- return new ChangeControl(
- changeDataFactory, approvalsUtil, refControl.forUser(who), notes, patchSetUtil);
+ return new ChangeControl(changeDataFactory, refControl.forUser(who), notes);
}
private CurrentUser getUser() {
@@ -130,26 +108,24 @@
}
/** Can this user abandon this change? */
- private boolean canAbandon(ReviewDb db) throws OrmException {
- return (isOwner() // owner (aka creator) of the change can abandon
- || refControl.isOwner() // branch owner can abandon
- || getProjectControl().isOwner() // project owner can abandon
- || refControl.canPerform(Permission.ABANDON) // user can abandon a specific ref
- || getProjectControl().isAdmin())
- && !isPatchSetLocked(db);
+ private boolean canAbandon() {
+ return isOwner() // owner (aka creator) of the change can abandon
+ || refControl.isOwner() // branch owner can abandon
+ || getProjectControl().isOwner() // project owner can abandon
+ || refControl.canPerform(Permission.ABANDON) // user can abandon a specific ref
+ || getProjectControl().isAdmin();
}
/** Can this user rebase this change? */
- private boolean canRebase(ReviewDb db) throws OrmException {
+ private boolean canRebase() {
return (isOwner() || refControl.canSubmit(isOwner()) || refControl.canRebase())
- && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE)
- && !isPatchSetLocked(db);
+ && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
}
/** Can this user restore this change? */
- private boolean canRestore(ReviewDb db) throws OrmException {
+ private boolean canRestore() {
// Anyone who can abandon the change can restore it, as long as they can create changes.
- return canAbandon(db) && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
+ return canAbandon() && refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE);
}
/** The range of permitted values associated with a label permission. */
@@ -158,8 +134,8 @@
}
/** Can this user add a patch set to this change? */
- private boolean canAddPatchSet(ReviewDb db) throws OrmException {
- if (!(refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE)) || isPatchSetLocked(db)) {
+ private boolean canAddPatchSet() {
+ if (!refControl.asForRef().testOrFalse(RefPermission.CREATE_CHANGE)) {
return false;
}
if (isOwner()) {
@@ -168,29 +144,6 @@
return refControl.canAddPatchSet();
}
- /** Is the current patch set locked against state changes? */
- private boolean isPatchSetLocked(ReviewDb db) throws OrmException {
- if (getChange().getStatus() == Change.Status.MERGED) {
- return false;
- }
-
- for (PatchSetApproval ap :
- approvalsUtil.byPatchSet(
- db, notes, getUser(), getChange().currentPatchSetId(), null, null)) {
- LabelType type =
- getProjectControl()
- .getProjectState()
- .getLabelTypes(notes, getUser())
- .byLabel(ap.getLabel());
- if (type != null
- && ap.getValue() == 1
- && type.getFunction() == LabelFunction.PATCH_SET_LOCK) {
- return true;
- }
- }
- return false;
- }
-
/** Is this user the owner of the change? */
private boolean isOwner() {
if (getUser().isIdentifiedUser()) {
@@ -355,12 +308,12 @@
case READ:
return isVisible(db(), changeData());
case ABANDON:
- return canAbandon(db());
+ return canAbandon();
case DELETE:
return (isOwner() && refControl.canPerform(Permission.DELETE_OWN_CHANGES))
|| getProjectControl().isAdmin();
case ADD_PATCH_SET:
- return canAddPatchSet(db());
+ return canAddPatchSet();
case EDIT_ASSIGNEE:
return canEditAssignee();
case EDIT_DESCRIPTION:
@@ -370,9 +323,9 @@
case EDIT_TOPIC_NAME:
return canEditTopicName();
case REBASE:
- return canRebase(db());
+ return canRebase();
case RESTORE:
- return canRestore(db());
+ return canRestore();
case SUBMIT:
return refControl.canSubmit(isOwner());
diff --git a/java/com/google/gerrit/server/permissions/ChangePermission.java b/java/com/google/gerrit/server/permissions/ChangePermission.java
index 6a23cdd..ba1785d 100644
--- a/java/com/google/gerrit/server/permissions/ChangePermission.java
+++ b/java/com/google/gerrit/server/permissions/ChangePermission.java
@@ -20,15 +20,39 @@
public enum ChangePermission implements ChangePermissionOrLabel {
READ,
+ /**
+ * The change can't be restored if its current patch set is locked.
+ *
+ * <p>Before checking this permission, the caller should first verify the current patch set of the
+ * change is not locked by calling {@code PatchSetUtil.isPatchSetLocked}.
+ */
RESTORE,
DELETE,
+ /**
+ * The change can't be abandoned if its current patch set is locked.
+ *
+ * <p>Before checking this permission, the caller should first verify the current patch set of the
+ * change is not locked by calling {@code PatchSetUtil.isPatchSetLocked}.
+ */
ABANDON,
EDIT_ASSIGNEE,
EDIT_DESCRIPTION,
EDIT_HASHTAGS,
EDIT_TOPIC_NAME,
REMOVE_REVIEWER,
+ /**
+ * A new patch set can't be added if the patch set is locked for the change.
+ *
+ * <p>Before checking this permission, the caller should first verify the current patch set of the
+ * change is not locked by calling {@code PatchSetUtil.isPatchSetLocked}.
+ */
ADD_PATCH_SET,
+ /**
+ * The change can't be rebased if its current patch set is locked.
+ *
+ * <p>Before checking this permission, the caller should first verify the current patch set of the
+ * change is not locked by calling {@code PatchSetUtil.isPatchSetLocked}.
+ */
REBASE,
SUBMIT,
SUBMIT_AS("submit on behalf of other users");
diff --git a/java/com/google/gerrit/server/permissions/PermissionBackend.java b/java/com/google/gerrit/server/permissions/PermissionBackend.java
index 3872a38..4cbd77e 100644
--- a/java/com/google/gerrit/server/permissions/PermissionBackend.java
+++ b/java/com/google/gerrit/server/permissions/PermissionBackend.java
@@ -357,8 +357,14 @@
}
/**
- * Returns a partition of the provided refs that are visible to the user that this instance is
- * scoped to.
+ * Filter a map of references by visibility.
+ *
+ * @param refs a map of references to filter.
+ * @param repo an open {@link Repository} handle for this instance's project
+ * @param opts further options for filtering.
+ * @return a partition of the provided refs that are visible to the user that this instance is
+ * scoped to.
+ * @throws PermissionBackendException if failure consulting backend configuration.
*/
public abstract Map<String, Ref> filter(
Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
diff --git a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
index 5d5a95f..89f302b 100644
--- a/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
+++ b/java/com/google/gerrit/server/query/change/ChangeIsVisibleToPredicate.java
@@ -17,6 +17,7 @@
import com.google.gerrit.index.query.IsVisibleToPredicate;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.index.IndexUtils;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -40,19 +41,22 @@
protected final CurrentUser user;
protected final PermissionBackend permissionBackend;
protected final ProjectCache projectCache;
+ private final Provider<AnonymousUser> anonymousUserProvider;
public ChangeIsVisibleToPredicate(
Provider<ReviewDb> db,
ChangeNotes.Factory notesFactory,
CurrentUser user,
PermissionBackend permissionBackend,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ Provider<AnonymousUser> anonymousUserProvider) {
super(ChangeQueryBuilder.FIELD_VISIBLETO, IndexUtils.describe(user));
this.db = db;
this.notesFactory = notesFactory;
this.user = user;
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
+ this.anonymousUserProvider = anonymousUserProvider;
}
@Override
@@ -81,13 +85,12 @@
}
boolean visible;
+ PermissionBackend.WithUser withUser =
+ user.isIdentifiedUser()
+ ? permissionBackend.absentUser(user.getAccountId())
+ : permissionBackend.user(anonymousUserProvider.get());
try {
- visible =
- permissionBackend
- .user(user)
- .indexedChange(cd, notes)
- .database(db)
- .test(ChangePermission.READ);
+ visible = withUser.indexedChange(cd, notes).database(db).test(ChangePermission.READ);
} catch (PermissionBackendException e) {
Throwable cause = e.getCause();
if (cause instanceof RepositoryNotFoundException) {
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 630f2f3..68b139f 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -44,6 +44,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
@@ -214,6 +215,7 @@
final StarredChangesUtil starredChangesUtil;
final SubmitDryRun submitDryRun;
final GroupMembers groupMembers;
+ final Provider<AnonymousUser> anonymousUserProvider;
private final Provider<CurrentUser> self;
@@ -246,7 +248,8 @@
StarredChangesUtil starredChangesUtil,
AccountCache accountCache,
NotesMigration notesMigration,
- GroupMembers groupMembers) {
+ GroupMembers groupMembers,
+ Provider<AnonymousUser> anonymousUserProvider) {
this(
db,
queryProvider,
@@ -274,7 +277,8 @@
starredChangesUtil,
accountCache,
notesMigration,
- groupMembers);
+ groupMembers,
+ anonymousUserProvider);
}
private Arguments(
@@ -304,7 +308,8 @@
StarredChangesUtil starredChangesUtil,
AccountCache accountCache,
NotesMigration notesMigration,
- GroupMembers groupMembers) {
+ GroupMembers groupMembers,
+ Provider<AnonymousUser> anonymousUserProvider) {
this.db = db;
this.queryProvider = queryProvider;
this.rewriter = rewriter;
@@ -332,6 +337,7 @@
this.hasOperands = hasOperands;
this.notesMigration = notesMigration;
this.groupMembers = groupMembers;
+ this.anonymousUserProvider = anonymousUserProvider;
}
Arguments asUser(CurrentUser otherUser) {
@@ -362,7 +368,8 @@
starredChangesUtil,
accountCache,
notesMigration,
- groupMembers);
+ groupMembers,
+ anonymousUserProvider);
}
Arguments asUser(Account.Id otherId) {
@@ -909,7 +916,12 @@
public Predicate<ChangeData> visibleto(CurrentUser user) {
return new ChangeIsVisibleToPredicate(
- args.db, args.notesFactory, user, args.permissionBackend, args.projectCache);
+ args.db,
+ args.notesFactory,
+ user,
+ args.permissionBackend,
+ args.projectCache,
+ args.anonymousUserProvider);
}
public Predicate<ChangeData> is_visible() throws QueryParseException {
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index 5fe918e..21d7b2d 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -26,6 +26,7 @@
import com.google.gerrit.index.query.QueryProcessor;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountLimits;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -65,6 +66,7 @@
private final DynamicMap<ChangeAttributeFactory> attributeFactories;
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
+ private final Provider<AnonymousUser> anonymousUserProvider;
static {
// It is assumed that basic rewrites do not touch visibleto predicates.
@@ -85,7 +87,8 @@
ChangeNotes.Factory notesFactory,
DynamicMap<ChangeAttributeFactory> attributeFactories,
PermissionBackend permissionBackend,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ Provider<AnonymousUser> anonymousUserProvider) {
super(
metricMaker,
ChangeSchemaDefinitions.INSTANCE,
@@ -100,6 +103,7 @@
this.attributeFactories = attributeFactories;
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
+ this.anonymousUserProvider = anonymousUserProvider;
}
@Override
@@ -143,7 +147,12 @@
return new AndChangeSource(
pred,
new ChangeIsVisibleToPredicate(
- db, notesFactory, userProvider.get(), permissionBackend, projectCache),
+ db,
+ notesFactory,
+ userProvider.get(),
+ permissionBackend,
+ projectCache,
+ anonymousUserProvider),
start);
}
}
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 7240452..19fd4a1 100644
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -123,7 +123,7 @@
// Check the user has 'READ' permission.
try {
PermissionBackend.ForChange perm =
- permissionBackend.user(reviewer).database(dbProvider).change(cd);
+ permissionBackend.absentUser(approver).database(dbProvider).change(cd);
ProjectState projectState = projectCache.checkedGet(cd.project());
return projectState != null
&& projectState.statePermitsRead()
diff --git a/java/com/google/gerrit/server/restapi/change/Abandon.java b/java/com/google/gerrit/server/restapi/change/Abandon.java
index 5485cf6..116cbdb 100644
--- a/java/com/google/gerrit/server/restapi/change/Abandon.java
+++ b/java/com/google/gerrit/server/restapi/change/Abandon.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.restapi.change;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
-
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.gerrit.common.TimeUtil;
@@ -29,6 +27,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.change.AbandonOp;
import com.google.gerrit.server.change.ChangeJson;
@@ -47,14 +46,19 @@
import com.google.inject.Singleton;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
@Singleton
public class Abandon extends RetryingRestModifyView<ChangeResource, AbandonInput, ChangeInfo>
implements UiAction<ChangeResource> {
+ private static final Logger log = LoggerFactory.getLogger(Abandon.class);
+
private final Provider<ReviewDb> dbProvider;
private final ChangeJson.Factory json;
private final AbandonOp.Factory abandonOpFactory;
private final NotifyUtil notifyUtil;
+ private final PatchSetUtil patchSetUtil;
@Inject
Abandon(
@@ -62,27 +66,32 @@
ChangeJson.Factory json,
RetryHelper retryHelper,
AbandonOp.Factory abandonOpFactory,
- NotifyUtil notifyUtil) {
+ NotifyUtil notifyUtil,
+ PatchSetUtil patchSetUtil) {
super(retryHelper);
this.dbProvider = dbProvider;
this.json = json;
this.abandonOpFactory = abandonOpFactory;
this.notifyUtil = notifyUtil;
+ this.patchSetUtil = patchSetUtil;
}
@Override
protected ChangeInfo applyImpl(
- BatchUpdate.Factory updateFactory, ChangeResource req, AbandonInput input)
+ BatchUpdate.Factory updateFactory, ChangeResource rsrc, AbandonInput input)
throws RestApiException, UpdateException, OrmException, PermissionBackendException,
IOException, ConfigInvalidException {
- req.permissions().database(dbProvider).check(ChangePermission.ABANDON);
+ // Not allowed to abandon if the current patch set is locked.
+ patchSetUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
- NotifyHandling notify = input.notify == null ? defaultNotify(req.getChange()) : input.notify;
+ rsrc.permissions().database(dbProvider).check(ChangePermission.ABANDON);
+
+ NotifyHandling notify = input.notify == null ? defaultNotify(rsrc.getChange()) : input.notify;
Change change =
abandon(
updateFactory,
- req.getNotes(),
- req.getUser(),
+ rsrc.getNotes(),
+ rsrc.getUser(),
input.message,
notify,
notifyUtil.resolveAccounts(input.notifyDetails));
@@ -135,13 +144,29 @@
@Override
public UiAction.Description getDescription(ChangeResource rsrc) {
+ UiAction.Description description =
+ new UiAction.Description()
+ .setLabel("Abandon")
+ .setTitle("Abandon the change")
+ .setVisible(false);
+
Change change = rsrc.getChange();
- return new UiAction.Description()
- .setLabel("Abandon")
- .setTitle("Abandon the change")
- .setVisible(
- and(
- change.getStatus().isOpen(),
- rsrc.permissions().database(dbProvider).testCond(ChangePermission.ABANDON)));
+ if (!change.getStatus().isOpen()) {
+ return description;
+ }
+
+ try {
+ if (patchSetUtil.isPatchSetLocked(rsrc.getNotes(), rsrc.getUser())) {
+ return description;
+ }
+ } catch (OrmException | IOException e) {
+ log.error(
+ String.format(
+ "Failed to check if the current patch set of change %s is locked", change.getId()),
+ e);
+ return description;
+ }
+
+ return description.setVisible(rsrc.permissions().testOrFalse(ChangePermission.ABANDON));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/CreateChange.java b/java/com/google/gerrit/server/restapi/change/CreateChange.java
index d8aa880..9f86e46 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateChange.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateChange.java
@@ -81,6 +81,7 @@
import java.util.List;
import java.util.TimeZone;
import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.InvalidObjectIdException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
@@ -180,6 +181,10 @@
}
}
+ if (input.baseChange != null && input.baseCommit != null) {
+ throw new BadRequestException("only provide one of base_change or base_commit");
+ }
+
ProjectResource rsrc = projectsCollection.parse(input.project);
boolean privateByDefault = rsrc.getProjectState().is(BooleanProjectConfig.PRIVATE_BY_DEFAULT);
boolean isPrivate = input.isPrivate == null ? privateByDefault : input.isPrivate;
@@ -205,6 +210,7 @@
RevWalk rw = new RevWalk(reader)) {
ObjectId parentCommit;
List<String> groups;
+ Ref destRef = git.getRefDatabase().exactRef(refName);
if (input.baseChange != null) {
List<ChangeNotes> notes = changeFinder.find(input.baseChange);
if (notes.size() != 1) {
@@ -219,8 +225,21 @@
PatchSet ps = psUtil.current(db.get(), change);
parentCommit = ObjectId.fromString(ps.getRevision().get());
groups = ps.getGroups();
+ } else if (input.baseCommit != null) {
+ try {
+ parentCommit = ObjectId.fromString(input.baseCommit);
+ } catch (InvalidObjectIdException e) {
+ throw new UnprocessableEntityException(
+ String.format("Base %s doesn't represent a valid SHA-1", input.baseCommit));
+ }
+ RevCommit parentRevCommit = rw.parseCommit(parentCommit);
+ RevCommit destRefRevCommit = rw.parseCommit(destRef.getObjectId());
+ if (!rw.isMergedInto(parentRevCommit, destRefRevCommit)) {
+ throw new BadRequestException(
+ String.format("Commit %s doesn't exist on ref %s", input.baseCommit, refName));
+ }
+ groups = Collections.emptyList();
} else {
- Ref destRef = git.getRefDatabase().exactRef(refName);
if (destRef != null) {
if (Boolean.TRUE.equals(input.newBranch)) {
throw new ResourceConflictException(
@@ -231,8 +250,7 @@
if (Boolean.TRUE.equals(input.newBranch)) {
parentCommit = null;
} else {
- throw new UnprocessableEntityException(
- String.format("Branch %s does not exist.", refName));
+ throw new BadRequestException("Must provide a destination branch");
}
}
groups = Collections.emptyList();
diff --git a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
index 374aaec..ff85880 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java
@@ -50,7 +50,6 @@
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.InvalidChangeOperationException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.restapi.project.CommitsCollection;
@@ -126,8 +125,11 @@
@Override
protected Response<ChangeInfo> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, MergePatchSetInput in)
- throws OrmException, IOException, InvalidChangeOperationException, RestApiException,
- UpdateException, PermissionBackendException {
+ throws OrmException, IOException, RestApiException, UpdateException,
+ PermissionBackendException {
+ // Not allowed to create a new patch set if the current patch set is locked.
+ psUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+
rsrc.permissions().database(db).check(ChangePermission.ADD_PATCH_SET);
ProjectState projectState = projectCache.checkedGet(rsrc.getProject());
diff --git a/java/com/google/gerrit/server/restapi/change/Move.java b/java/com/google/gerrit/server/restapi/change/Move.java
index 2ad954d..6e049438 100644
--- a/java/com/google/gerrit/server/restapi/change/Move.java
+++ b/java/com/google/gerrit/server/restapi/change/Move.java
@@ -134,6 +134,9 @@
throw new ResourceConflictException("Change is already destined for the specified branch");
}
+ // Not allowed to move if the current patch set is locked.
+ psUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+
// Move requires abandoning this change, and creating a new change.
try {
rsrc.permissions().database(dbProvider).check(ABANDON);
@@ -279,24 +282,41 @@
@Override
public UiAction.Description getDescription(ChangeResource rsrc) {
+ UiAction.Description description =
+ new UiAction.Description()
+ .setLabel("Move Change")
+ .setTitle("Move change to a different branch")
+ .setVisible(false);
+
Change change = rsrc.getChange();
- boolean projectStatePermitsWrite = false;
+ if (!change.getStatus().isOpen()) {
+ return description;
+ }
+
try {
- projectStatePermitsWrite = projectCache.checkedGet(rsrc.getProject()).statePermitsWrite();
+ if (!projectCache.checkedGet(rsrc.getProject()).statePermitsWrite()) {
+ return description;
+ }
} catch (IOException e) {
log.error("Failed to check if project state permits write: " + rsrc.getProject(), e);
+ return description;
}
- return new UiAction.Description()
- .setLabel("Move Change")
- .setTitle("Move change to a different branch")
- .setVisible(
- and(
- change.getStatus().isOpen() && projectStatePermitsWrite,
- and(
- permissionBackend
- .user(rsrc.getUser())
- .ref(change.getDest())
- .testCond(CREATE_CHANGE),
- rsrc.permissions().database(dbProvider).testCond(ABANDON))));
+
+ try {
+ if (psUtil.isPatchSetLocked(rsrc.getNotes(), rsrc.getUser())) {
+ return description;
+ }
+ } catch (OrmException | IOException e) {
+ log.error(
+ String.format(
+ "Failed to check if the current patch set of change %s is locked", change.getId()),
+ e);
+ return description;
+ }
+
+ return description.setVisible(
+ and(
+ permissionBackend.user(rsrc.getUser()).ref(change.getDest()).testCond(CREATE_CHANGE),
+ rsrc.permissions().database(dbProvider).testCond(ABANDON)));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index c9c43cb..eb46521 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -33,7 +33,6 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyUtil;
import com.google.gerrit.server.change.PatchSetInserter;
-import com.google.gerrit.server.edit.UnchangedCommitMessageException;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
@@ -68,7 +67,7 @@
extends RetryingRestModifyView<ChangeResource, CommitMessageInput, Response<?>> {
private final GitRepositoryManager repositoryManager;
- private final Provider<CurrentUser> currentUserProvider;
+ private final Provider<CurrentUser> userProvider;
private final Provider<ReviewDb> db;
private final TimeZone tz;
private final PatchSetInserter.Factory psInserterFactory;
@@ -81,7 +80,7 @@
PutMessage(
RetryHelper retryHelper,
GitRepositoryManager repositoryManager,
- Provider<CurrentUser> currentUserProvider,
+ Provider<CurrentUser> userProvider,
Provider<ReviewDb> db,
PatchSetInserter.Factory psInserterFactory,
PermissionBackend permissionBackend,
@@ -91,7 +90,7 @@
ProjectCache projectCache) {
super(retryHelper);
this.repositoryManager = repositoryManager;
- this.currentUserProvider = currentUserProvider;
+ this.userProvider = userProvider;
this.db = db;
this.psInserterFactory = psInserterFactory;
this.tz = gerritIdent.getTimeZone();
@@ -104,8 +103,8 @@
@Override
protected Response<String> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource resource, CommitMessageInput input)
- throws IOException, UnchangedCommitMessageException, RestApiException, UpdateException,
- PermissionBackendException, OrmException, ConfigInvalidException {
+ throws IOException, RestApiException, UpdateException, PermissionBackendException,
+ OrmException, ConfigInvalidException {
PatchSet ps = psUtil.current(db.get(), resource.getNotes());
if (ps == null) {
throw new ResourceConflictException("current revision is missing");
@@ -140,7 +139,7 @@
Timestamp ts = TimeUtil.nowTs();
try (BatchUpdate bu =
updateFactory.create(
- db.get(), resource.getChange().getProject(), currentUserProvider.get(), ts)) {
+ db.get(), resource.getChange().getProject(), userProvider.get(), ts)) {
// Ensure that BatchUpdate will update the same repo
bu.setRepository(repository, new RevWalk(objectInserter.newReader()), objectInserter);
@@ -170,8 +169,7 @@
builder.setTreeId(basePatchSetCommit.getTree());
builder.setParentIds(basePatchSetCommit.getParents());
builder.setAuthor(basePatchSetCommit.getAuthorIdent());
- builder.setCommitter(
- currentUserProvider.get().asIdentifiedUser().newCommitterIdent(timestamp, tz));
+ builder.setCommitter(userProvider.get().asIdentifiedUser().newCommitterIdent(timestamp, tz));
builder.setMessage(commitMessage);
ObjectId newCommitId = objectInserter.insert(builder);
objectInserter.flush();
@@ -179,13 +177,17 @@
}
private void ensureCanEditCommitMessage(ChangeNotes changeNotes)
- throws AuthException, PermissionBackendException, IOException, ResourceConflictException {
- if (!currentUserProvider.get().isIdentifiedUser()) {
+ throws AuthException, PermissionBackendException, IOException, ResourceConflictException,
+ OrmException {
+ if (!userProvider.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
+
+ // Not allowed to put message if the current patch set is locked.
+ psUtil.checkPatchSetNotLocked(changeNotes, userProvider.get());
try {
permissionBackend
- .user(currentUserProvider.get())
+ .user(userProvider.get())
.database(db.get())
.change(changeNotes)
.check(ChangePermission.ADD_PATCH_SET);
diff --git a/java/com/google/gerrit/server/restapi/change/Rebase.java b/java/com/google/gerrit/server/restapi/change/Rebase.java
index 767ef7b..02e7c18 100644
--- a/java/com/google/gerrit/server/restapi/change/Rebase.java
+++ b/java/com/google/gerrit/server/restapi/change/Rebase.java
@@ -14,16 +14,12 @@
package com.google.gerrit.server.restapi.change;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
-
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.common.errors.EmailException;
import com.google.gerrit.extensions.api.changes.RebaseInput;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.conditions.BooleanCondition;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -81,6 +77,7 @@
private final Provider<ReviewDb> dbProvider;
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
+ private final PatchSetUtil patchSetUtil;
@Inject
public Rebase(
@@ -91,7 +88,8 @@
ChangeJson.Factory json,
Provider<ReviewDb> dbProvider,
PermissionBackend permissionBackend,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ PatchSetUtil patchSetUtil) {
super(retryHelper);
this.repoManager = repoManager;
this.rebaseFactory = rebaseFactory;
@@ -100,13 +98,17 @@
this.dbProvider = dbProvider;
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
+ this.patchSetUtil = patchSetUtil;
}
@Override
protected ChangeInfo applyImpl(
BatchUpdate.Factory updateFactory, RevisionResource rsrc, RebaseInput input)
- throws EmailException, OrmException, UpdateException, RestApiException, IOException,
- NoSuchChangeException, PermissionBackendException {
+ throws OrmException, UpdateException, RestApiException, IOException,
+ PermissionBackendException {
+ // Not allowed to rebase if the current patch set is locked.
+ patchSetUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+
rsrc.permissions().database(dbProvider).check(ChangePermission.REBASE);
projectCache.checkedGet(rsrc.getProject()).checkStatePermitsWrite();
@@ -203,40 +205,54 @@
}
@Override
- public UiAction.Description getDescription(RevisionResource resource) {
- PatchSet patchSet = resource.getPatchSet();
- Change change = resource.getChange();
- Branch.NameKey dest = change.getDest();
- boolean visible = change.getStatus().isOpen() && resource.isCurrent();
- boolean enabled = false;
+ public UiAction.Description getDescription(RevisionResource rsrc) {
+ UiAction.Description description =
+ new UiAction.Description()
+ .setLabel("Rebase")
+ .setTitle("Rebase onto tip of branch or parent change")
+ .setVisible(false);
+
+ Change change = rsrc.getChange();
+ if (!(change.getStatus().isOpen() && rsrc.isCurrent())) {
+ return description;
+ }
try {
- visible &= projectCache.checkedGet(resource.getProject()).statePermitsWrite();
- } catch (IOException e) {
- log.error("Failed to check if project state permits write: " + resource.getProject(), e);
- visible = false;
- }
-
- if (visible) {
- try (Repository repo = repoManager.openRepository(dest.getParentKey());
- RevWalk rw = new RevWalk(repo)) {
- visible = hasOneParent(rw, resource.getPatchSet());
- if (visible) {
- enabled = rebaseUtil.canRebase(patchSet, dest, repo, rw);
- }
- } catch (IOException e) {
- log.error("Failed to check if patch set can be rebased: " + resource.getPatchSet(), e);
- visible = false;
+ if (!projectCache.checkedGet(rsrc.getProject()).statePermitsWrite()) {
+ return description;
}
+ } catch (IOException e) {
+ log.error("Failed to check if project state permits write: " + rsrc.getProject(), e);
+ return description;
}
- BooleanCondition permissionCond =
- resource.permissions().database(dbProvider).testCond(ChangePermission.REBASE);
- return new UiAction.Description()
- .setLabel("Rebase")
- .setTitle("Rebase onto tip of branch or parent change")
- .setVisible(and(visible, permissionCond))
- .setEnabled(and(enabled, permissionCond));
+ try {
+ if (patchSetUtil.isPatchSetLocked(rsrc.getNotes(), rsrc.getUser())) {
+ return description;
+ }
+ } catch (OrmException | IOException e) {
+ log.error(
+ String.format(
+ "Failed to check if the current patch set of change %s is locked", change.getId()),
+ e);
+ return description;
+ }
+
+ boolean enabled = false;
+ try (Repository repo = repoManager.openRepository(change.getDest().getParentKey());
+ RevWalk rw = new RevWalk(repo)) {
+ if (hasOneParent(rw, rsrc.getPatchSet())) {
+ enabled = rebaseUtil.canRebase(rsrc.getPatchSet(), change.getDest(), repo, rw);
+ }
+ } catch (IOException e) {
+ log.error("Failed to check if patch set can be rebased: " + rsrc.getPatchSet(), e);
+ return description;
+ }
+
+ if (rsrc.permissions().database(dbProvider).testOrFalse(ChangePermission.REBASE)) {
+ return description.setVisible(true).setEnabled(enabled);
+ }
+ return description;
}
public static class CurrentRevision
@@ -254,7 +270,7 @@
@Override
protected ChangeInfo applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, RebaseInput input)
- throws EmailException, OrmException, UpdateException, RestApiException, IOException,
+ throws OrmException, UpdateException, RestApiException, IOException,
PermissionBackendException {
PatchSet ps = psUtil.current(rebase.dbProvider.get(), rsrc.getNotes());
if (ps == null) {
diff --git a/java/com/google/gerrit/server/restapi/change/Restore.java b/java/com/google/gerrit/server/restapi/change/Restore.java
index 642c35a..d5bfea1 100644
--- a/java/com/google/gerrit/server/restapi/change/Restore.java
+++ b/java/com/google/gerrit/server/restapi/change/Restore.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.restapi.change;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
-
import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.RestoreInput;
@@ -90,17 +88,20 @@
@Override
protected ChangeInfo applyImpl(
- BatchUpdate.Factory updateFactory, ChangeResource req, RestoreInput input)
+ BatchUpdate.Factory updateFactory, ChangeResource rsrc, RestoreInput input)
throws RestApiException, UpdateException, OrmException, PermissionBackendException,
IOException {
- req.permissions().database(dbProvider).check(ChangePermission.RESTORE);
- projectCache.checkedGet(req.getProject()).checkStatePermitsWrite();
+ // Not allowed to restore if the current patch set is locked.
+ psUtil.checkPatchSetNotLocked(rsrc.getNotes(), rsrc.getUser());
+
+ rsrc.permissions().database(dbProvider).check(ChangePermission.RESTORE);
+ projectCache.checkedGet(rsrc.getProject()).checkStatePermitsWrite();
Op op = new Op(input);
try (BatchUpdate u =
updateFactory.create(
- dbProvider.get(), req.getChange().getProject(), req.getUser(), TimeUtil.nowTs())) {
- u.addOp(req.getId(), op).execute();
+ dbProvider.get(), rsrc.getChange().getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
+ u.addOp(rsrc.getId(), op).execute();
}
return json.noOptions().format(op.change);
}
@@ -161,18 +162,39 @@
@Override
public UiAction.Description getDescription(ChangeResource rsrc) {
- boolean projectStatePermitsWrite = false;
+ UiAction.Description description =
+ new UiAction.Description()
+ .setLabel("Restore")
+ .setTitle("Restore the change")
+ .setVisible(false);
+
+ Change change = rsrc.getChange();
+ if (change.getStatus() != Status.ABANDONED) {
+ return description;
+ }
+
try {
- projectStatePermitsWrite = projectCache.checkedGet(rsrc.getProject()).statePermitsWrite();
+ if (!projectCache.checkedGet(rsrc.getProject()).statePermitsWrite()) {
+ return description;
+ }
} catch (IOException e) {
log.error("Failed to check if project state permits write: " + rsrc.getProject(), e);
+ return description;
}
- return new UiAction.Description()
- .setLabel("Restore")
- .setTitle("Restore the change")
- .setVisible(
- and(
- rsrc.getChange().getStatus() == Status.ABANDONED && projectStatePermitsWrite,
- rsrc.permissions().database(dbProvider).testCond(ChangePermission.RESTORE)));
+
+ try {
+ if (psUtil.isPatchSetLocked(rsrc.getNotes(), rsrc.getUser())) {
+ return description;
+ }
+ } catch (OrmException | IOException e) {
+ log.error(
+ String.format(
+ "Failed to check if the current patch set of change %s is locked", change.getId()),
+ e);
+ return description;
+ }
+
+ boolean visible = rsrc.permissions().database(dbProvider).testOrFalse(ChangePermission.RESTORE);
+ return description.setVisible(visible);
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewerJson.java b/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
index d74be7d..cfd20c2 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewerJson.java
@@ -80,7 +80,10 @@
ReviewerInfo info =
format(
new ReviewerInfo(rsrc.getReviewerUser().getAccountId().get()),
- permissionBackend.user(rsrc.getReviewerUser()).database(db).change(cd),
+ permissionBackend
+ .absentUser(rsrc.getReviewerUser().getAccountId())
+ .database(db)
+ .change(cd),
cd);
loader.put(info);
infos.add(info);
diff --git a/java/com/google/gerrit/server/rules/PrologEnvironment.java b/java/com/google/gerrit/server/rules/PrologEnvironment.java
index 170ff23..9dd0b86 100644
--- a/java/com/google/gerrit/server/rules/PrologEnvironment.java
+++ b/java/com/google/gerrit/server/rules/PrologEnvironment.java
@@ -16,6 +16,7 @@
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchListCache;
@@ -175,6 +176,7 @@
private final Provider<AnonymousUser> anonymousUser;
private final int reductionLimit;
private final int compileLimit;
+ private final PatchSetUtil patchsetUtil;
@Inject
Args(
@@ -185,7 +187,8 @@
PatchSetInfoFactory patchSetInfoFactory,
IdentifiedUser.GenericFactory userFactory,
Provider<AnonymousUser> anonymousUser,
- @GerritServerConfig Config config) {
+ @GerritServerConfig Config config,
+ PatchSetUtil patchsetUtil) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.repositoryManager = repositoryManager;
@@ -193,6 +196,7 @@
this.patchSetInfoFactory = patchSetInfoFactory;
this.userFactory = userFactory;
this.anonymousUser = anonymousUser;
+ this.patchsetUtil = patchsetUtil;
int limit = config.getInt("rules", null, "reductionLimit", 100000);
reductionLimit = limit <= 0 ? Integer.MAX_VALUE : limit;
@@ -240,5 +244,9 @@
public AnonymousUser getAnonymousUser() {
return anonymousUser.get();
}
+
+ public PatchSetUtil getPatchsetUtil() {
+ return patchsetUtil;
+ }
}
}
diff --git a/java/com/google/gerrit/server/rules/StoredValues.java b/java/com/google/gerrit/server/rules/StoredValues.java
index 9fc3557..6770732 100644
--- a/java/com/google/gerrit/server/rules/StoredValues.java
+++ b/java/com/google/gerrit/server/rules/StoredValues.java
@@ -25,6 +25,7 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails;
@@ -89,6 +90,22 @@
}
};
+ public static final StoredValue<String> COMMIT_MESSAGE =
+ new StoredValue<String>() {
+ @Override
+ public String createValue(Prolog engine) {
+ Change change = getChange(engine);
+ PatchSet ps = getPatchSet(engine);
+ PrologEnvironment env = (PrologEnvironment) engine.control;
+ PatchSetUtil patchSetUtil = env.getArgs().getPatchsetUtil();
+ try {
+ return patchSetUtil.getFullCommitMessage(change.getProject(), ps);
+ } catch (IOException e) {
+ throw new SystemException(e.getMessage());
+ }
+ }
+ };
+
public static final StoredValue<PatchList> PATCH_LIST =
new StoredValue<PatchList>() {
@Override
diff --git a/java/com/google/gerrit/server/schema/GroupBundle.java b/java/com/google/gerrit/server/schema/GroupBundle.java
index 3c1c409..b001d6f 100644
--- a/java/com/google/gerrit/server/schema/GroupBundle.java
+++ b/java/com/google/gerrit/server/schema/GroupBundle.java
@@ -742,7 +742,7 @@
ADD(1),
REMOVE(2);
- private int order;
+ private final int order;
Action(int order) {
this.order = order;
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index d09615f..22d463d 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -38,6 +38,7 @@
import com.google.gerrit.server.update.RepoOnlyOp;
import com.google.gerrit.server.update.UpdateException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayDeque;
@@ -95,7 +96,7 @@
@Singleton
public static class Factory {
private final GitModules.Factory gitmodulesFactory;
- private final PersonIdent myIdent;
+ private final Provider<PersonIdent> serverIdent;
private final Config cfg;
private final ProjectCache projectCache;
private final BatchUpdate.Factory batchUpdateFactory;
@@ -103,12 +104,12 @@
@Inject
Factory(
GitModules.Factory gitmodulesFactory,
- @GerritPersonIdent PersonIdent myIdent,
+ @GerritPersonIdent Provider<PersonIdent> serverIdent,
@GerritServerConfig Config cfg,
ProjectCache projectCache,
BatchUpdate.Factory batchUpdateFactory) {
this.gitmodulesFactory = gitmodulesFactory;
- this.myIdent = myIdent;
+ this.serverIdent = serverIdent;
this.cfg = cfg;
this.projectCache = projectCache;
this.batchUpdateFactory = batchUpdateFactory;
@@ -117,7 +118,13 @@
public SubmoduleOp create(Set<Branch.NameKey> updatedBranches, MergeOpRepoManager orm)
throws SubmoduleException {
return new SubmoduleOp(
- gitmodulesFactory, myIdent, cfg, projectCache, batchUpdateFactory, updatedBranches, orm);
+ gitmodulesFactory,
+ serverIdent.get(),
+ cfg,
+ projectCache,
+ batchUpdateFactory,
+ updatedBranches,
+ orm);
}
}
diff --git a/java/com/google/gerrit/sshd/AliasCommand.java b/java/com/google/gerrit/sshd/AliasCommand.java
index 0ac7765..cb61651 100644
--- a/java/com/google/gerrit/sshd/AliasCommand.java
+++ b/java/com/google/gerrit/sshd/AliasCommand.java
@@ -18,7 +18,6 @@
import com.google.common.util.concurrent.Atomics;
import com.google.gerrit.extensions.api.access.GlobalOrPluginPermission;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -33,7 +32,6 @@
/** Command that executes some other command. */
public class AliasCommand extends BaseCommand {
private final DispatchCommandProvider root;
- private final CurrentUser currentUser;
private final PermissionBackend permissionBackend;
private final CommandName command;
private final AtomicReference<Command> atomicCmd;
@@ -41,11 +39,9 @@
AliasCommand(
@CommandName(Commands.ROOT) DispatchCommandProvider root,
PermissionBackend permissionBackend,
- CurrentUser currentUser,
CommandName command) {
this.root = root;
this.permissionBackend = permissionBackend;
- this.currentUser = currentUser;
this.command = command;
this.atomicCmd = Atomics.newReference();
}
@@ -114,7 +110,7 @@
try {
Set<GlobalOrPluginPermission> check = GlobalPermission.fromAnnotation(cmd.getClass());
try {
- permissionBackend.user(currentUser).checkAny(check);
+ permissionBackend.currentUser().checkAny(check);
} catch (AuthException err) {
throw new UnloggedFailure(BaseCommand.STATUS_NOT_ADMIN, "fatal: " + err.getMessage());
}
diff --git a/java/com/google/gerrit/sshd/AliasCommandProvider.java b/java/com/google/gerrit/sshd/AliasCommandProvider.java
index 0ef0473..085b6d6 100644
--- a/java/com/google/gerrit/sshd/AliasCommandProvider.java
+++ b/java/com/google/gerrit/sshd/AliasCommandProvider.java
@@ -14,7 +14,6 @@
package com.google.gerrit.sshd;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -29,7 +28,6 @@
private DispatchCommandProvider root;
@Inject private PermissionBackend permissionBackend;
- @Inject private CurrentUser currentUser;
public AliasCommandProvider(CommandName command) {
this.command = command;
@@ -37,6 +35,6 @@
@Override
public Command get() {
- return new AliasCommand(root, permissionBackend, currentUser, command);
+ return new AliasCommand(root, permissionBackend, command);
}
}
diff --git a/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/java/com/google/gerrit/sshd/ChangeArgumentParser.java
index 39ee13a..9fc4cda 100644
--- a/java/com/google/gerrit/sshd/ChangeArgumentParser.java
+++ b/java/com/google/gerrit/sshd/ChangeArgumentParser.java
@@ -20,7 +20,6 @@
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeFinder;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.ChangePermission;
@@ -39,7 +38,6 @@
import java.util.Map;
public class ChangeArgumentParser {
- private final CurrentUser currentUser;
private final ChangesCollection changesCollection;
private final ChangeFinder changeFinder;
private final ReviewDb db;
@@ -48,13 +46,11 @@
@Inject
ChangeArgumentParser(
- CurrentUser currentUser,
ChangesCollection changesCollection,
ChangeFinder changeFinder,
ReviewDb db,
ChangeNotes.Factory changeNotesFactory,
PermissionBackend permissionBackend) {
- this.currentUser = currentUser;
this.changesCollection = changesCollection;
this.changeFinder = changeFinder;
this.db = db;
@@ -83,7 +79,7 @@
List<ChangeNotes> toAdd = new ArrayList<>(changes.size());
boolean canMaintainServer;
try {
- permissionBackend.user(currentUser).check(GlobalPermission.MAINTAIN_SERVER);
+ permissionBackend.currentUser().check(GlobalPermission.MAINTAIN_SERVER);
canMaintainServer = true;
} catch (AuthException | PermissionBackendException e) {
canMaintainServer = false;
@@ -93,7 +89,7 @@
&& inProject(projectState, notes.getProjectName())
&& (canMaintainServer
|| (permissionBackend
- .user(currentUser)
+ .currentUser()
.change(notes)
.database(db)
.test(ChangePermission.READ)
diff --git a/java/com/google/gerrit/sshd/DispatchCommand.java b/java/com/google/gerrit/sshd/DispatchCommand.java
index 3f2e258..490dd52 100644
--- a/java/com/google/gerrit/sshd/DispatchCommand.java
+++ b/java/com/google/gerrit/sshd/DispatchCommand.java
@@ -19,7 +19,6 @@
import com.google.common.collect.Sets;
import com.google.common.util.concurrent.Atomics;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.args4j.SubcommandHandler;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -42,7 +41,6 @@
DispatchCommand create(Map<String, CommandProvider> map);
}
- private final CurrentUser currentUser;
private final PermissionBackend permissionBackend;
private final Map<String, CommandProvider> commands;
private final AtomicReference<Command> atomicCmd;
@@ -54,11 +52,7 @@
private List<String> args = new ArrayList<>();
@Inject
- DispatchCommand(
- CurrentUser user,
- PermissionBackend permissionBackend,
- @Assisted Map<String, CommandProvider> all) {
- this.currentUser = user;
+ DispatchCommand(PermissionBackend permissionBackend, @Assisted Map<String, CommandProvider> all) {
this.permissionBackend = permissionBackend;
commands = all;
atomicCmd = Atomics.newReference();
@@ -125,7 +119,7 @@
}
try {
permissionBackend
- .user(currentUser)
+ .currentUser()
.checkAny(GlobalPermission.fromAnnotation(pluginName, cmd.getClass()));
} catch (AuthException e) {
throw new UnloggedFailure(BaseCommand.STATUS_NOT_ADMIN, e.getMessage());
diff --git a/java/com/google/gerrit/sshd/commands/AdminQueryShell.java b/java/com/google/gerrit/sshd/commands/AdminQueryShell.java
index ef1cd81..c520e79 100644
--- a/java/com/google/gerrit/sshd/commands/AdminQueryShell.java
+++ b/java/com/google/gerrit/sshd/commands/AdminQueryShell.java
@@ -17,7 +17,6 @@
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -34,7 +33,6 @@
final class AdminQueryShell extends SshCommand {
@Inject private PermissionBackend permissionBackend;
@Inject private QueryShell.Factory factory;
- @Inject private IdentifiedUser currentUser;
@Option(name = "--format", usage = "Set output format")
private QueryShell.OutputFormat format = QueryShell.OutputFormat.PRETTY;
@@ -45,7 +43,7 @@
@Override
protected void run() throws Failure {
try {
- permissionBackend.user(currentUser).check(GlobalPermission.ACCESS_DATABASE);
+ permissionBackend.currentUser().check(GlobalPermission.ACCESS_DATABASE);
} catch (AuthException err) {
throw die(err.getMessage());
} catch (PermissionBackendException e) {
diff --git a/java/gerrit/PRED_commit_message_1.java b/java/gerrit/PRED_commit_message_1.java
index c5aa367..05bb4bb 100644
--- a/java/gerrit/PRED_commit_message_1.java
+++ b/java/gerrit/PRED_commit_message_1.java
@@ -14,7 +14,6 @@
package gerrit;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.Operation;
@@ -41,9 +40,9 @@
engine.setB0();
Term a1 = arg1.dereference();
- PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
+ String commitMessage = StoredValues.COMMIT_MESSAGE.get(engine);
- SymbolTerm msg = SymbolTerm.create(psInfo.getMessage());
+ SymbolTerm msg = SymbolTerm.create(commitMessage);
if (!a1.unify(msg, engine.trail)) {
return engine.fail();
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
index ee5d3b0..129d98a 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -53,6 +53,7 @@
import com.google.gson.stream.JsonReader;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -839,9 +840,7 @@
private static void assertReviewers(
ChangeInfo c, ReviewerState reviewerState, TestAccount... accounts) throws Exception {
List<TestAccount> accountList = new ArrayList<>(accounts.length);
- for (TestAccount a : accounts) {
- accountList.add(a);
- }
+ Collections.addAll(accountList, accounts);
assertReviewers(c, reviewerState, accountList);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index ac17ec8..e510e25 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -208,6 +208,36 @@
}
@Test
+ public void createChangeWithParentCommit() throws Exception {
+ Map<String, PushOneCommit.Result> setup =
+ changeInTwoBranches("foo", "foo.txt", "bar", "bar.txt");
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.baseCommit = setup.get("master").getCommit().getId().name();
+ assertCreateSucceeds(input);
+ }
+
+ @Test
+ public void createChangeWithBadParentCommitFails() throws Exception {
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.baseCommit = "notasha1";
+ assertCreateFails(
+ input, UnprocessableEntityException.class, "Base notasha1 doesn't represent a valid SHA-1");
+ }
+
+ @Test
+ public void createChangeWithParentCommitOnWrongBranchFails() throws Exception {
+ Map<String, PushOneCommit.Result> setup =
+ changeInTwoBranches("foo", "foo.txt", "bar", "bar.txt");
+ ChangeInput input = newChangeInput(ChangeStatus.NEW);
+ input.branch = "foo";
+ input.baseCommit = setup.get("bar").getCommit().getId().name();
+ assertCreateFails(
+ input,
+ BadRequestException.class,
+ String.format("Commit %s doesn't exist on ref refs/heads/foo", input.baseCommit));
+ }
+
+ @Test
public void createChangeWithoutAccessToParentCommitFails() throws Exception {
Map<String, PushOneCommit.Result> results =
changeInTwoBranches("invisible-branch", "a.txt", "visible-branch", "b.txt");
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
index 93ad2fe..be0879a 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/MoveChangeIT.java
@@ -229,8 +229,9 @@
grant(project, "refs/heads/*", Permission.LABEL + "Patch-Set-Lock");
revision(r).review(new ReviewInput().label("Patch-Set-Lock", 1));
- exception.expect(AuthException.class);
- exception.expectMessage("move not permitted");
+ exception.expect(ResourceConflictException.class);
+ exception.expectMessage(
+ String.format("The current patch set of change %s is locked", r.getChange().getId()));
move(r.getChangeId(), newBranch.get());
}
diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index 27dc951..a5d78c6 100644
--- a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -333,7 +333,7 @@
@Test
public void enableSequencesNoGap() throws Exception {
- testEnableSequences(0, 2, "12");
+ testEnableSequences(0, 3, "13");
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java b/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
new file mode 100644
index 0000000..a4d9acb
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
@@ -0,0 +1,123 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.server.rules;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
+import com.google.gerrit.server.project.SubmitRuleOptions;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.inject.Inject;
+import java.util.Collection;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Tests the Prolog rules to make sure they work even when the change and account indexes are not
+ * available.
+ */
+@NoHttpd
+public class RulesIT extends AbstractDaemonTest {
+ private static final String RULE_TEMPLATE =
+ "submit_rule(submit(W)) :- \n" + "%s,\n" + "W = label('OK', ok(user(1000000))).";
+
+ @Inject private SubmitRuleEvaluator.Factory evaluatorFactory;
+
+ @Before
+ public void setUp() {
+ // We don't want caches to interfere with our tests. If we didn't, the cache would take
+ // precedence over the index, which would never be called.
+ baseConfig.setString("cache", "changes", "memoryLimit", "0");
+ baseConfig.setString("cache", "projects", "memoryLimit", "0");
+ }
+
+ @Test
+ public void testUnresolvedCommentsCountPredicate() throws Exception {
+ modifySubmitRules("gerrit:unresolved_comments_count(0)");
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
+ }
+
+ @Test
+ public void testUploaderPredicate() throws Exception {
+ modifySubmitRules("gerrit:uploader(U)");
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
+ }
+
+ @Test
+ public void testUnresolvedCommentsCount() throws Exception {
+ modifySubmitRules("gerrit:commit_message_matches('.*')");
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
+ }
+
+ @Test
+ public void testUserPredicate() throws Exception {
+ // This test results in a RULE_ERROR as Prolog tries to find accounts by email, using the index.
+ // TODO(maximeg) get OK results
+ modifySubmitRules("commit_author(user(1000000), 'John Doe', 'john.doe@example.com')");
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.RULE_ERROR);
+ }
+
+ @Test
+ public void testCommitAuthorPredicate() throws Exception {
+ // This test results in a RULE_ERROR as Prolog tries to find accounts by email, using the index.
+ // TODO(maximeg) get OK results
+ modifySubmitRules("gerrit:commit_author(Id)");
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.RULE_ERROR);
+ }
+
+ private SubmitRecord.Status statusForRule() throws Exception {
+ String oldHead = getRemoteHead().name();
+ PushOneCommit.Result result1 =
+ pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
+ testRepo.reset(oldHead);
+ ChangeData cd = result1.getChange();
+
+ Collection<SubmitRecord> records;
+ try (AutoCloseable changeIndex = disableChangeIndex()) {
+ try (AutoCloseable accountIndex = disableAccountIndex()) {
+ SubmitRuleEvaluator ruleEvaluator = evaluatorFactory.create(SubmitRuleOptions.defaults());
+ records = ruleEvaluator.evaluate(cd);
+ }
+ }
+
+ assertThat(records).hasSize(1);
+ SubmitRecord record = records.iterator().next();
+ return record.status;
+ }
+
+ private void modifySubmitRules(String ruleTested) throws Exception {
+ String newContent = String.format(RULE_TEMPLATE, ruleTested);
+
+ try (Repository repo = repoManager.openRepository(project)) {
+ TestRepository<?> testRepo = new TestRepository<>((InMemoryRepository) repo);
+ testRepo
+ .branch(RefNames.REFS_CONFIG)
+ .commit()
+ .author(admin.getIdent())
+ .committer(admin.getIdent())
+ .add("rules.pl", newContent)
+ .message("Modify rules.pl")
+ .create();
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/common/data/SubmitRecordTest.java b/javatests/com/google/gerrit/common/data/SubmitRecordTest.java
index 6a90f45..5386b87 100644
--- a/javatests/com/google/gerrit/common/data/SubmitRecordTest.java
+++ b/javatests/com/google/gerrit/common/data/SubmitRecordTest.java
@@ -21,14 +21,17 @@
import org.junit.Test;
public class SubmitRecordTest {
-
- private static SubmitRecord OK_RECORD;
- private static SubmitRecord NOT_READY_RECORD;
+ private static final SubmitRecord OK_RECORD;
+ private static final SubmitRecord FORCED_RECORD;
+ private static final SubmitRecord NOT_READY_RECORD;
static {
OK_RECORD = new SubmitRecord();
OK_RECORD.status = SubmitRecord.Status.OK;
+ FORCED_RECORD = new SubmitRecord();
+ FORCED_RECORD.status = SubmitRecord.Status.FORCED;
+
NOT_READY_RECORD = new SubmitRecord();
NOT_READY_RECORD.status = SubmitRecord.Status.NOT_READY;
}
@@ -49,6 +52,14 @@
}
@Test
+ public void okWhenForced() {
+ Collection<SubmitRecord> submitRecords = new ArrayList<>();
+ submitRecords.add(FORCED_RECORD);
+
+ assertThat(SubmitRecord.allRecordsOK(submitRecords)).isTrue();
+ }
+
+ @Test
public void emptyResultIfInvalid() {
Collection<SubmitRecord> submitRecords = new ArrayList<>();
submitRecords.add(NOT_READY_RECORD);
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 2ddfaa9..3864676 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -53,6 +53,7 @@
"//lib:gson",
"//lib:guava-retrying",
"//lib:gwtorm",
+ "//lib:protobuf",
"//lib:truth-java8-extension",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
@@ -60,5 +61,6 @@
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//proto:cache_java_proto",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/EnumCacheSerializerTest.java b/javatests/com/google/gerrit/server/cache/EnumCacheSerializerTest.java
new file mode 100644
index 0000000..0e04d32
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/EnumCacheSerializerTest.java
@@ -0,0 +1,39 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+
+public class EnumCacheSerializerTest {
+ @Test
+ public void serialize() throws Exception {
+ assertRoundTrip(MyEnum.FOO);
+ assertRoundTrip(MyEnum.BAR);
+ assertRoundTrip(MyEnum.BAZ);
+ }
+
+ private enum MyEnum {
+ FOO,
+ BAR,
+ BAZ;
+ }
+
+ private static void assertRoundTrip(MyEnum e) throws Exception {
+ CacheSerializer<MyEnum> s = new EnumCacheSerializer<>(MyEnum.class);
+ assertThat(s.deserialize(s.serialize(e))).isEqualTo(e);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/JavaCacheSerializerTest.java b/javatests/com/google/gerrit/server/cache/JavaCacheSerializerTest.java
new file mode 100644
index 0000000..41d07b9
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cache/JavaCacheSerializerTest.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.cache;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.auto.value.AutoValue;
+import java.io.Serializable;
+import org.junit.Test;
+
+public class JavaCacheSerializerTest {
+ @Test
+ public void builtInTypes() throws Exception {
+ assertRoundTrip("foo");
+ assertRoundTrip(Integer.valueOf(1234));
+ assertRoundTrip(Boolean.TRUE);
+ }
+
+ @Test
+ public void customType() throws Exception {
+ assertRoundTrip(new AutoValue_JavaCacheSerializerTest_MyType(123, "four five six"));
+ }
+
+ @AutoValue
+ abstract static class MyType implements Serializable {
+ private static final long serialVersionUID = 1L;
+
+ abstract Integer anInt();
+
+ abstract String aString();
+ }
+
+ private static <T extends Serializable> void assertRoundTrip(T input) throws Exception {
+ JavaCacheSerializer<T> s = new JavaCacheSerializer<>();
+ assertThat(s.deserialize(s.serialize(input))).isEqualTo(input);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/cache/h2/BUILD b/javatests/com/google/gerrit/server/cache/h2/BUILD
index 0ac4aef..e2b9257 100644
--- a/javatests/com/google/gerrit/server/cache/h2/BUILD
+++ b/javatests/com/google/gerrit/server/cache/h2/BUILD
@@ -9,6 +9,7 @@
"//lib:guava",
"//lib:h2",
"//lib:junit",
+ "//lib:truth",
"//lib/guice",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java b/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
index 80bca6d..32d0d27 100644
--- a/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
+++ b/javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java
@@ -14,62 +14,134 @@
package com.google.gerrit.server.cache.h2;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import com.google.common.util.concurrent.MoreExecutors;
+import com.google.gerrit.server.cache.CacheSerializer;
import com.google.gerrit.server.cache.h2.H2CacheImpl.SqlStore;
import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder;
import com.google.inject.TypeLiteral;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
-import org.junit.Before;
import org.junit.Test;
public class H2CacheTest {
+ private static final TypeLiteral<String> KEY_TYPE = new TypeLiteral<String>() {};
+ private static final int DEFAULT_VERSION = 1234;
private static int dbCnt;
- private Cache<String, ValueHolder<Boolean>> mem;
- private H2CacheImpl<String, Boolean> impl;
+ private static int nextDbId() {
+ return ++dbCnt;
+ }
- @Before
- public void setUp() {
- mem = CacheBuilder.newBuilder().build();
-
- TypeLiteral<String> keyType = new TypeLiteral<String>() {};
- SqlStore<String, Boolean> store =
- new SqlStore<>("jdbc:h2:mem:Test_" + (++dbCnt), keyType, 1 << 20, 0);
- impl = new H2CacheImpl<>(MoreExecutors.directExecutor(), store, keyType, mem);
+ private static H2CacheImpl<String, String> newH2CacheImpl(
+ int id, Cache<String, ValueHolder<String>> mem, int version) {
+ SqlStore<String, String> store =
+ new SqlStore<>(
+ "jdbc:h2:mem:Test_" + id,
+ KEY_TYPE,
+ StringSerializer.INSTANCE,
+ StringSerializer.INSTANCE,
+ version,
+ 1 << 20,
+ 0);
+ return new H2CacheImpl<>(MoreExecutors.directExecutor(), store, KEY_TYPE, mem);
}
@Test
public void get() throws ExecutionException {
- assertNull(impl.getIfPresent("foo"));
+ Cache<String, ValueHolder<String>> mem = CacheBuilder.newBuilder().build();
+ H2CacheImpl<String, String> impl = newH2CacheImpl(nextDbId(), mem, DEFAULT_VERSION);
- final AtomicBoolean called = new AtomicBoolean();
- assertTrue(
- impl.get(
- "foo",
- () -> {
- called.set(true);
- return true;
- }));
- assertTrue("used Callable", called.get());
- assertTrue("exists in cache", impl.getIfPresent("foo"));
+ assertThat(impl.getIfPresent("foo")).isNull();
+
+ AtomicBoolean called = new AtomicBoolean();
+ assertThat(
+ impl.get(
+ "foo",
+ () -> {
+ called.set(true);
+ return "bar";
+ }))
+ .isEqualTo("bar");
+ assertThat(called.get()).named("Callable was called").isTrue();
+ assertThat(impl.getIfPresent("foo")).named("in-memory value").isEqualTo("bar");
mem.invalidate("foo");
- assertTrue("exists on disk", impl.getIfPresent("foo"));
+ assertThat(impl.getIfPresent("foo")).named("persistent value").isEqualTo("bar");
called.set(false);
- assertTrue(
- impl.get(
- "foo",
- () -> {
- called.set(true);
- return true;
- }));
- assertFalse("did not invoke Callable", called.get());
+ assertThat(
+ impl.get(
+ "foo",
+ () -> {
+ called.set(true);
+ return "baz";
+ }))
+ .named("cached value")
+ .isEqualTo("bar");
+ assertThat(called.get()).named("Callable was called").isFalse();
+ }
+
+ @Test
+ public void stringSerializer() {
+ String input = "foo";
+ byte[] serialized = StringSerializer.INSTANCE.serialize(input);
+ assertThat(serialized).isEqualTo(new byte[] {'f', 'o', 'o'});
+ assertThat(StringSerializer.INSTANCE.deserialize(serialized)).isEqualTo(input);
+ }
+
+ @Test
+ public void version() throws Exception {
+ int id = nextDbId();
+ H2CacheImpl<String, String> oldImpl = newH2CacheImpl(id, disableMemCache(), DEFAULT_VERSION);
+ H2CacheImpl<String, String> newImpl =
+ newH2CacheImpl(id, disableMemCache(), DEFAULT_VERSION + 1);
+
+ assertThat(oldImpl.diskStats().space()).isEqualTo(0);
+ assertThat(newImpl.diskStats().space()).isEqualTo(0);
+ oldImpl.put("key", "val");
+ assertThat(oldImpl.getIfPresent("key")).isEqualTo("val");
+ assertThat(oldImpl.diskStats().space()).isEqualTo(12);
+ assertThat(oldImpl.diskStats().hitCount()).isEqualTo(1);
+
+ // Can't find key in cache with wrong version, but the data is still there.
+ assertThat(newImpl.diskStats().requestCount()).isEqualTo(0);
+ assertThat(newImpl.diskStats().space()).isEqualTo(12);
+ assertThat(newImpl.getIfPresent("key")).isNull();
+ assertThat(newImpl.diskStats().space()).isEqualTo(12);
+
+ // Re-putting it via the new cache works, and uses the same amount of space.
+ newImpl.put("key", "val2");
+ assertThat(newImpl.getIfPresent("key")).isEqualTo("val2");
+ assertThat(newImpl.diskStats().hitCount()).isEqualTo(1);
+ assertThat(newImpl.diskStats().space()).isEqualTo(14);
+
+ // Now it's no longer in the old cache.
+ assertThat(oldImpl.diskStats().space()).isEqualTo(14);
+ assertThat(oldImpl.getIfPresent("key")).isNull();
+ }
+
+ // TODO(dborowitz): Won't be necessary when we use a real StringSerializer in the server code.
+ private enum StringSerializer implements CacheSerializer<String> {
+ INSTANCE;
+
+ @Override
+ public byte[] serialize(String object) {
+ return object.getBytes(UTF_8);
+ }
+
+ @Override
+ public String deserialize(byte[] in) {
+ // TODO(dborowitz): Consider using CharsetDecoder directly in the real implementation, to get
+ // checked exceptions.
+ return new String(in, UTF_8);
+ }
+ }
+
+ private static <K, V> Cache<K, ValueHolder<V>> disableMemCache() {
+ return CacheBuilder.newBuilder().maximumSize(0).build();
}
}
diff --git a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
new file mode 100644
index 0000000..4470f55
--- /dev/null
+++ b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
@@ -0,0 +1,55 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.server.cache.CacheSerializer;
+import com.google.gerrit.server.cache.proto.Cache.ChangeKindKeyProto;
+import com.google.protobuf.ByteString;
+import org.eclipse.jgit.lib.ObjectId;
+import org.junit.Test;
+
+public class ChangeKindCacheImplTest {
+ @Test
+ public void keySerializer() throws Exception {
+ ChangeKindCacheImpl.Key key =
+ new ChangeKindCacheImpl.Key(
+ ObjectId.zeroId(),
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
+ "aStrategy");
+ CacheSerializer<ChangeKindCacheImpl.Key> s = new ChangeKindCacheImpl.Key.Serializer();
+ byte[] serialized = s.serialize(key);
+ assertThat(ChangeKindKeyProto.parseFrom(serialized))
+ .isEqualTo(
+ ChangeKindKeyProto.newBuilder()
+ .setPrior(bytes(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0))
+ .setNext(
+ bytes(
+ 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef,
+ 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef))
+ .setStrategyName("aStrategy")
+ .build());
+ assertThat(s.deserialize(serialized)).isEqualTo(key);
+ }
+
+ private static ByteString bytes(int... ints) {
+ byte[] bytes = new byte[ints.length];
+ for (int i = 0; i < ints.length; i++) {
+ bytes[i] = (byte) ints[i];
+ }
+ return ByteString.copyFrom(bytes);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
index edd4abf..b525504 100644
--- a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
+++ b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java
@@ -27,7 +27,7 @@
new FakeQueryBuilder.Definition<>(FakeQueryBuilder.class),
new ChangeQueryBuilder.Arguments(
null, null, null, null, null, null, null, null, null, null, null, null, null, null,
- null, null, null, null, null, indexes, null, null, null, null, null, null, null));
+ null, null, null, null, null, indexes, null, null, null, null, null, null, null, null));
}
@Operator
diff --git a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
index 152b057..314941e 100644
--- a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
+++ b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
@@ -48,7 +48,8 @@
cfg.setInt("rules", null, "compileReductionLimit", (int) 1e6);
bind(PrologEnvironment.Args.class)
.toInstance(
- new PrologEnvironment.Args(null, null, null, null, null, null, null, cfg));
+ new PrologEnvironment.Args(
+ null, null, null, null, null, null, null, cfg, null));
}
});
}
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
index a652e26..85f01e7 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.html
@@ -72,6 +72,17 @@
</gr-autocomplete>
</span>
</section>
+ <section class$="[[_computeBranchClass(baseChange)]]">
+ <span class="title">Provide base commit sha1 for change</span>
+ <span class="value">
+ <input
+ is="iron-input"
+ id="baseCommitInput"
+ maxlength="40"
+ placeholder="(optional)"
+ bind-value="{{baseCommit}}">
+ </span>
+ </section>
<section>
<span class="title">Enter topic for new change</span>
<span class="value">
diff --git a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.js b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.js
index 8252d88..b738829 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.js
+++ b/polygerrit-ui/app/elements/admin/gr-create-change-dialog/gr-create-change-dialog.js
@@ -37,6 +37,7 @@
},
},
baseChange: String,
+ baseCommit: String,
privateByDefault: String,
canCreate: {
type: Boolean,
@@ -73,7 +74,8 @@
const isPrivate = this.$.privateChangeCheckBox.checked;
const isWip = true;
return this.$.restAPI.createChange(this.repoName, this.branch,
- this.subject, this.topic, isPrivate, isWip, this.baseChange)
+ this.subject, this.topic, isPrivate, isWip, this.baseChange,
+ this.baseCommit)
.then(changeCreated => {
if (!changeCreated) { return; }
Gerrit.Nav.navigateToChange(changeCreated);
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html
index d57dd329..e9bc674 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html
+++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.html
@@ -19,8 +19,8 @@
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
-<link rel="import" href="../../../styles/gr-table-styles.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
+<link rel="import" href="../../../styles/gr-table-styles.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
@@ -34,6 +34,7 @@
<dom-module id="gr-repo-detail-list">
<template>
<style include="gr-form-styles"></style>
+ <style include="gr-table-styles"></style>
<style include="shared-styles">
.tags td.name {
min-width: 25em;
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
index 6a3ea54..df09dfe 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
@@ -144,7 +144,9 @@
},
_getReviewerSuggestions(input) {
- if (!this.change) { return Promise.resolve([]); }
+ if (!this.change || !this.change._number) { return Promise.resolve([]); }
+
+ input = `cansee:${this.change._number} ${input}`;
const api = this.$.restAPI;
const xhr = this.allowAnyUser ?
api.getSuggestedAccounts(input) :
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
index 07c1554..c63f64d 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
@@ -80,6 +80,7 @@
element = fixture('basic');
element.change = {
+ _number: 42,
owner,
reviewers: {
CC: [existingReviewer1],
@@ -179,12 +180,14 @@
element._getReviewerSuggestions('').then(() => {
assert.isTrue(suggestReviewerStub.calledOnce);
+ assert.isTrue(suggestReviewerStub.calledWith(42, 'cansee:42 '));
assert.isFalse(suggestAccountStub.called);
element.allowAnyUser = true;
element._getReviewerSuggestions('').then(() => {
assert.isTrue(suggestReviewerStub.calledOnce);
assert.isTrue(suggestAccountStub.calledOnce);
+ assert.isTrue(suggestAccountStub.calledWith('cansee:42 '));
done();
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index 7ef62ea..cd7ec2b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -287,7 +287,7 @@
</section>
<template is="dom-if" if="[[serverConfig.note_db_enabled]]">
<section class="hashtag">
- <span class="title">Hashtag</span>
+ <span class="title">Hashtags</span>
<span class="value">
<template is="dom-repeat" items="[[change.hashtags]]">
<gr-linked-chip
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html
index 536b943..2a33343 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.html
@@ -18,6 +18,7 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../styles/shared-styles.html">
+<link rel="import" href="../../shared/gr-icons/gr-icons.html">
<dom-module id="gr-change-requirements">
<template strip-whitespace>
@@ -26,16 +27,15 @@
display: inline-block;
font-weight: initial;
text-align: center;
- width: 1em;
}
- .unsatisfied .status {
+ .unsatisfied .icon {
color: #FFA62F;
}
- .satisfied .status {
+ .satisfied .icon {
color: #388E3C;
}
.requirement {
- padding: .1em .3em;
+ padding: .1em 0;
}
.requirementContainer:not(:first-of-type) {
margin-top: .25em;
@@ -46,7 +46,7 @@
</style>
<template is="dom-if" if="[[_showWip]]">
<div class="requirement unsatisfied changeIsWip">
- <span class="status">⧗</span>
+ <span class="status"><iron-icon class="icon" icon="gr-icons:hourglass"></iron-icon></span>
Work in Progress
</div>
</template>
@@ -55,7 +55,9 @@
is="dom-repeat"
items="[[labels]]">
<div class$="requirement [[item.style]]">
- <span class="status">[[item.status]]</span>
+ <span class="status">
+ <iron-icon class="icon" icon="[[item.icon]]"></iron-icon>
+ </span>
Label <span class="labelName">[[item.label]]</span>
</div>
</template>
@@ -65,7 +67,7 @@
items="[[requirements]]">
<div class$="requirement [[_computeRequirementClass(item.satisfied)]]">
<span class="status">
- [[_computeRequirementStatus(item.satisfied)]]
+ <iron-icon id="icon" icon="[[_computeRequirementIcon(item.satisfied)]]"></iron-icon>
</span>
[[item.fallback_text]]
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
index 884e9cd..765f639 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
@@ -75,9 +75,9 @@
const obj = labels[label];
if (obj.optional) { continue; }
- const status = this._computeRequirementStatus(obj.approved);
+ const icon = this._computeRequirementIcon(obj.approved);
const style = this._computeRequirementClass(obj.approved);
- _labels.push({label, status, style});
+ _labels.push({label, icon, style});
}
return _labels;
@@ -91,11 +91,11 @@
}
},
- _computeRequirementStatus(requirementStatus) {
+ _computeRequirementIcon(requirementStatus) {
if (requirementStatus) {
- return '✓';
+ return 'gr-icons:check';
} else {
- return '⧗';
+ return 'gr-icons:hourglass';
}
},
});
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
index 560a33d..efac0c2 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
@@ -51,8 +51,8 @@
assert.equal(element._computeRequirementClass(true), 'satisfied');
assert.equal(element._computeRequirementClass(false), 'unsatisfied');
- assert.equal(element._computeRequirementStatus(true), '✓');
- assert.equal(element._computeRequirementStatus(false), '⧗');
+ assert.equal(element._computeRequirementIcon(true), 'gr-icons:check');
+ assert.equal(element._computeRequirementIcon(false), 'gr-icons:hourglass');
});
test('properly converts satisfied labels', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index c978281..2f30fb0 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -186,8 +186,8 @@
keyBindings: {
'shift+left': '_handleShiftLeftKey',
'shift+right': '_handleShiftRightKey',
- 'i': '_handleIKey',
- 'shift+i': '_handleCapitalIKey',
+ 'i:keyup': '_handleIKey',
+ 'shift+i:keyup': '_handleCapitalIKey',
'down j': '_handleDownKey',
'up k': '_handleUpKey',
'c': '_handleCKey',
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index df3f04f..9987521 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -530,24 +530,24 @@
const files = Polymer.dom(element.root).querySelectorAll('.file-row');
element.$.fileCursor.stops = files;
element.$.fileCursor.setCursorAtIndex(0);
- MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
+ MockInteractions.keyUpOn(element, 73, null, 'i');
flushAsynchronousOperations();
assert.include(element._expandedFilePaths, element.diffs[0].path);
- MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
+ MockInteractions.keyUpOn(element, 73, null, 'i');
flushAsynchronousOperations();
assert.notInclude(element._expandedFilePaths, element.diffs[0].path);
element.$.fileCursor.setCursorAtIndex(1);
- MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
+ MockInteractions.keyUpOn(element, 73, null, 'i');
flushAsynchronousOperations();
assert.include(element._expandedFilePaths, element.diffs[1].path);
- MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i');
+ MockInteractions.keyUpOn(element, 73, 'shift', 'i');
flushAsynchronousOperations();
for (const index in element.diffs) {
if (!element.diffs.hasOwnProperty(index)) { continue; }
assert.include(element._expandedFilePaths, element.diffs[index].path);
}
- MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i');
+ MockInteractions.keyUpOn(element, 73, 'shift', 'i');
flushAsynchronousOperations();
for (const index in element.diffs) {
if (!element.diffs.hasOwnProperty(index)) { continue; }
@@ -1338,7 +1338,7 @@
});
test('cursor with individually opened files', () => {
- MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
+ MockInteractions.keyUpOn(element, 73, null, 'i');
flushAsynchronousOperations();
let diffs = renderAndGetNewDiffs(0);
const diffStops = diffs[0].getCursorStops();
@@ -1364,7 +1364,7 @@
// The file cusor is now at 1.
assert.equal(element.$.fileCursor.index, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
+ MockInteractions.keyUpOn(element, 73, null, 'i');
flushAsynchronousOperations();
diffs = renderAndGetNewDiffs(1);
@@ -1379,7 +1379,7 @@
});
test('cursor with toggle all files', () => {
- MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i');
+ MockInteractions.keyUpOn(element, 73, 'shift', 'i');
flushAsynchronousOperations();
const diffs = renderAndGetNewDiffs(0);
@@ -1426,7 +1426,7 @@
});
test('n key with some files expanded and no shift key', () => {
- MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, null, 'i');
+ MockInteractions.keyUpOn(fileRows[0], 73, null, 'i');
flushAsynchronousOperations();
assert.equal(nextChunkStub.callCount, 1);
@@ -1441,7 +1441,7 @@
});
test('n key with some files expanded and shift key', () => {
- MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, null, 'i');
+ MockInteractions.keyUpOn(fileRows[0], 73, null, 'i');
flushAsynchronousOperations();
assert.equal(nextChunkStub.callCount, 1);
@@ -1455,7 +1455,7 @@
});
test('n key without all files expanded and shift key', () => {
- MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, 'shift', 'i');
+ MockInteractions.keyUpOn(fileRows[0], 73, 'shift', 'i');
flushAsynchronousOperations();
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
@@ -1468,7 +1468,7 @@
});
test('n key without all files expanded and no shift key', () => {
- MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, 'shift', 'i');
+ MockInteractions.keyUpOn(fileRows[0], 73, 'shift', 'i');
flushAsynchronousOperations();
MockInteractions.pressAndReleaseKeyOn(element, 78, 'shift', 'n');
@@ -1601,7 +1601,7 @@
commentStub.withArgs('ecf0b9fa_fe1a5f62').returns(
commentStubRes2);
// Expand the commit message diff
- MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i');
+ MockInteractions.keyUpOn(element, 73, 'shift', 'i');
const diffs = renderAndGetNewDiffs(0);
flushAsynchronousOperations();
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index e8c720a..97e7e78 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -217,7 +217,8 @@
_sendDisabled: {
type: Boolean,
computed: '_computeSendButtonDisabled(_sendButtonLabel, diffDrafts, ' +
- 'draft, _reviewersMutated, _labelsChanged, _includeComments)',
+ 'draft, _reviewersMutated, _labelsChanged, _includeComments, ' +
+ 'disabled)',
observer: '_sendDisabledChanged',
},
},
@@ -859,10 +860,9 @@
},
_computeSendButtonDisabled(buttonLabel, drafts, text, reviewersMutated,
- labelsChanged, includeComments) {
- if (buttonLabel === ButtonLabels.START_REVIEW) {
- return false;
- }
+ labelsChanged, includeComments, disabled) {
+ if (disabled) { return true; }
+ if (buttonLabel === ButtonLabels.START_REVIEW) { return false; }
const hasDrafts = includeComments && Object.keys(drafts).length;
return !hasDrafts && !text.length && !reviewersMutated && !labelsChanged;
},
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 41d7b49..f9108c7 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -1087,17 +1087,17 @@
assert.isFalse(fn('Start review'));
assert.isTrue(fn('Send', {}, '', false, false, false));
// Mock nonempty comment draft array, with seding comments.
- assert.isFalse(fn('Send', {file: ['draft']}, '', false, false,
- true));
+ assert.isFalse(fn('Send', {file: ['draft']}, '', false, false, true));
// Mock nonempty comment draft array, without seding comments.
- assert.isTrue(fn('Send', {file: ['draft']}, '', false, false,
- false));
+ assert.isTrue(fn('Send', {file: ['draft']}, '', false, false, false));
// Mock nonempty change message.
assert.isFalse(fn('Send', {}, 'test', false, false, false));
// Mock reviewers mutated.
assert.isFalse(fn('Send', {}, '', true, false, false));
// Mock labels changed.
assert.isFalse(fn('Send', {}, '', false, true, false));
+ // Whole dialog is disabled.
+ assert.isTrue(fn('Send', {}, '', false, true, false, true));
});
test('_submit blocked when no mutations exist', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
index 1ca678932..2201a9a 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
@@ -54,10 +54,6 @@
display: flex;
margin-right: 1rem;
}
- .draftsOnly gr-diff-comment-thread,
- .unresolvedOnly gr-diff-comment-thread {
- display: none
- }
.draftsOnly:not(.unresolvedOnly) gr-diff-comment-thread[has-draft],
.unresolvedOnly:not(.draftsOnly) gr-diff-comment-thread[unresolved],
.draftsOnly.unresolvedOnly gr-diff-comment-thread[has-draft][unresolved] {
@@ -68,12 +64,12 @@
<div class="toggleItem">
<paper-toggle-button
id="unresolvedToggle"
- on-change="_toggleUnresolved"></paper-toggle-button>
+ checked="{{_unresolvedOnly}}"></paper-toggle-button>
Only unresolved threads</div>
<div class$="toggleItem draftToggle [[_computeShowDraftToggle(loggedIn)]]">
<paper-toggle-button
id="draftToggle"
- on-change="_toggleDrafts"></paper-toggle-button>
+ checked="{{_draftsOnly}}"></paper-toggle-button>
Only threads with drafts</div>
</div>
<div id="threads">
@@ -82,7 +78,7 @@
</template>
<template
is="dom-repeat"
- items="[[_sortedThreads]]"
+ items="[[_filteredThreads]]"
as="thread"
initial-count="5"
target-framerate="60">
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
index 934f0f2..69d77f6 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
@@ -34,10 +34,24 @@
loggedIn: Boolean,
_sortedThreads: {
type: Array,
- computed: '_computeSortedThreads(threads.*)',
+ },
+ _filteredThreads: {
+ type: Array,
+ computed: '_computeFilteredThreads(_sortedThreads, _unresolvedOnly, ' +
+ '_draftsOnly)',
+ },
+ _unresolvedOnly: {
+ type: Boolean,
+ value: false,
+ },
+ _draftsOnly: {
+ type: Boolean,
+ value: false,
},
},
+ observers: ['_computeSortedThreads(threads.*)'],
+
_computeShowDraftToggle(loggedIn) {
return loggedIn ? 'show' : '';
},
@@ -49,36 +63,60 @@
* - Resolved threads with drafts (reverse chronological)
* - Resolved threads without drafts (reverse chronological)
* @param {!Object} changeRecord
- * @return {!Array}
*/
_computeSortedThreads(changeRecord) {
const threads = changeRecord.base;
if (!threads) { return []; }
- return threads.map(this._getThreadWithSortInfo).sort((c1, c2) => {
- const c1Date = c1.__date || util.parseDate(c1.updated);
- const c2Date = c2.__date || util.parseDate(c2.updated);
- const dateCompare = c2Date - c1Date;
- if (c2.unresolved || c1.unresolved) {
- if (!c1.unresolved) { return 1; }
- if (!c2.unresolved) { return -1; }
- }
- if (c2.hasDraft || c1.hasDraft) {
- if (!c1.hasDraft) { return 1; }
- if (!c2.hasDraft) { return -1; }
- }
+ this._updateSortedThreads(threads);
+ },
- if (dateCompare === 0 && (!c1.id || !c1.id.localeCompare)) {
- return 0;
+ _updateSortedThreads(threads) {
+ this._sortedThreads =
+ threads.map(this._getThreadWithSortInfo).sort((c1, c2) => {
+ const c1Date = c1.__date || util.parseDate(c1.updated);
+ const c2Date = c2.__date || util.parseDate(c2.updated);
+ const dateCompare = c2Date - c1Date;
+ if (c2.unresolved || c1.unresolved) {
+ if (!c1.unresolved) { return 1; }
+ if (!c2.unresolved) { return -1; }
+ }
+ if (c2.hasDraft || c1.hasDraft) {
+ if (!c1.hasDraft) { return 1; }
+ if (!c2.hasDraft) { return -1; }
+ }
+
+ if (dateCompare === 0 && (!c1.id || !c1.id.localeCompare)) {
+ return 0;
+ }
+ return dateCompare ? dateCompare : c1.id.localeCompare(c2.id);
+ });
+ },
+
+ _computeFilteredThreads(sortedThreads, unresolvedOnly, draftsOnly) {
+ return sortedThreads.filter(c => {
+ if (draftsOnly) {
+ return c.hasDraft;
+ } else if (unresolvedOnly) {
+ return c.unresolved;
+ } else {
+ return c;
}
- return dateCompare ? dateCompare : c1.id.localeCompare(c2.id);
}).map(threadInfo => threadInfo.thread);
},
_getThreadWithSortInfo(thread) {
const lastComment = thread.comments[thread.comments.length - 1] || {};
+
+ const lastNonDraftComment =
+ (lastComment.__draft && thread.comments.length > 1) ?
+ thread.comments[thread.comments.length - 2] :
+ lastComment;
+
return {
thread,
- unresolved: !!lastComment.unresolved,
+ // Use the unresolved bit for the last non draft comment. This is what
+ // anybody other than the current user would see.
+ unresolved: !!lastNonDraftComment.unresolved,
hasDraft: !!lastComment.__draft,
updated: lastComment.updated,
};
@@ -100,6 +138,10 @@
},
_handleCommentsChanged(e) {
+ // Reset threads so thread computations occur on deep array changes to
+ // threads comments that are not observed naturally.
+ this._updateSortedThreads(this.threads);
+
this.dispatchEvent(new CustomEvent('thread-list-modified',
{detail: {rootId: e.detail.rootId, path: e.detail.path}}));
},
@@ -107,13 +149,5 @@
_isOnParent(side) {
return !!side;
},
-
- _toggleUnresolved() {
- this.$.threads.classList.toggle('unresolvedOnly');
- },
-
- _toggleDrafts() {
- this.$.threads.classList.toggle('draftsOnly');
- },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
index 53557c6..804446a 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
@@ -37,15 +37,6 @@
let element;
let sandbox;
let threadElements;
- const computeVisibleNumber = threads => {
- let count = 0;
- for (const thread of threads) {
- if (getComputedStyle(thread).display !== 'none') {
- count++;
- }
- }
- return count;
- };
setup(() => {
sandbox = sinon.sandbox.create();
@@ -74,7 +65,7 @@
in_reply_to: 'ecf0b9fa_fe1a5f62',
updated: '2018-02-13 22:48:48.018000000',
message: 'draft',
- unresolved: true,
+ unresolved: false,
__draft: true,
__draftID: '0.m683trwff68',
__editing: false,
@@ -196,54 +187,67 @@
});
test('there are five threads by default', () => {
- assert.equal(computeVisibleNumber(threadElements), 5);
+ assert.equal(Polymer.dom(element.root)
+ .querySelectorAll('gr-diff-comment-thread').length, 5);
});
test('_computeSortedThreads', () => {
assert.equal(element._sortedThreads.length, 5);
// Draft and unresolved
- assert.equal(element._sortedThreads[0].rootId, 'ecf0b9fa_fe1a5f62');
+ assert.equal(element._sortedThreads[0].thread.rootId,
+ 'ecf0b9fa_fe1a5f62');
// unresolved
- assert.equal(element._sortedThreads[1].rootId, 'scaddf38_44770ec1');
+ assert.equal(element._sortedThreads[1].thread.rootId,
+ 'scaddf38_44770ec1');
// unresolved
- assert.equal(element._sortedThreads[2].rootId, '8caddf38_44770ec1');
+ assert.equal(element._sortedThreads[2].thread.rootId,
+ '8caddf38_44770ec1');
// resolved and draft
- assert.equal(element._sortedThreads[3].rootId, 'zcf0b9fa_fe1a5f62');
+ assert.equal(element._sortedThreads[3].thread.rootId,
+ 'zcf0b9fa_fe1a5f62');
// resolved
- assert.equal(element._sortedThreads[4].rootId, '09a9fb0a_1484e6cf');
+ assert.equal(element._sortedThreads[4].thread.rootId,
+ '09a9fb0a_1484e6cf');
});
test('thread removal', () => {
threadElements[1].fire('thread-discard', {rootId: 'scaddf38_44770ec1'});
flushAsynchronousOperations();
assert.equal(element._sortedThreads.length, 4);
- assert.equal(element._sortedThreads[0].rootId, 'ecf0b9fa_fe1a5f62');
+ assert.equal(element._sortedThreads[0].thread.rootId,
+ 'ecf0b9fa_fe1a5f62');
// unresolved
- assert.equal(element._sortedThreads[1].rootId, '8caddf38_44770ec1');
+ assert.equal(element._sortedThreads[1].thread.rootId,
+ '8caddf38_44770ec1');
// resolved and draft
- assert.equal(element._sortedThreads[2].rootId, 'zcf0b9fa_fe1a5f62');
+ assert.equal(element._sortedThreads[2].thread.rootId,
+ 'zcf0b9fa_fe1a5f62');
// resolved
- assert.equal(element._sortedThreads[3].rootId, '09a9fb0a_1484e6cf');
+ assert.equal(element._sortedThreads[3].thread.rootId,
+ '09a9fb0a_1484e6cf');
});
test('toggle unresolved only shows unressolved comments', () => {
MockInteractions.tap(element.$.unresolvedToggle);
flushAsynchronousOperations();
- assert.equal(computeVisibleNumber(threadElements), 3);
+ assert.equal(Polymer.dom(element.root)
+ .querySelectorAll('gr-diff-comment-thread').length, 3);
});
test('toggle drafts only shows threads with draft comments', () => {
MockInteractions.tap(element.$.draftToggle);
flushAsynchronousOperations();
- assert.equal(computeVisibleNumber(threadElements), 2);
+ assert.equal(Polymer.dom(element.root)
+ .querySelectorAll('gr-diff-comment-thread').length, 2);
});
test('toggle drafts and unresolved only shows threads with drafts and ' +
- 'unresolved', () => {
+ 'publicly unresolved ', () => {
MockInteractions.tap(element.$.draftToggle);
MockInteractions.tap(element.$.unresolvedToggle);
flushAsynchronousOperations();
- assert.equal(computeVisibleNumber(threadElements), 1);
+ assert.equal(Polymer.dom(element.root)
+ .querySelectorAll('gr-diff-comment-thread').length, 2);
});
test('modification events are consumed and displatched', () => {
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
index 0b78df5..e4cb243 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
@@ -55,10 +55,11 @@
padding-top: 1em;
}
.key {
+ background-color: var(--chip-background-color);
+ border: 1px solid var(--border-color);
+ border-radius: 3px;
display: inline-block;
font-family: var(--font-family-bold);
- border-radius: 3px;
- background-color: #f1f2f3;
padding: .1em .5em;
text-align: center;
}
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html
index d31825f..daa86fa 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html
@@ -33,7 +33,7 @@
</test-fixture>
<script>
- suite('gr-diff tests', () => {
+ suite('gr-plugin-host tests', () => {
let element;
let sandbox;
let url;
@@ -54,8 +54,8 @@
sandbox.stub(Gerrit, '_setPluginsCount');
element.config = {
plugin: {
- html_resource_paths: ['foo/bar', 'baz'],
- js_resource_paths: ['42'],
+ html_resource_paths: ['plugins/foo/bar', 'plugins/baz'],
+ js_resource_paths: ['plugins/42'],
},
};
assert.isTrue(Gerrit._setPluginsCount.calledWith(3));
diff --git a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
index 8a86f93..89ab8fe 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
+++ b/polygerrit-ui/app/elements/shared/gr-account-chip/gr-account-chip.html
@@ -31,7 +31,7 @@
}
.container {
align-items: center;
- background: var(--table-header-background-color);
+ background: var(--chip-background-color);
border-radius: .75em;
display: inline-flex;
padding: 0 .5em;
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index 1eaad4e..f39398f 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -312,6 +312,12 @@
// For any normal keypress, return focus to the input to allow for
// unbroken user input.
this.$.input.inputElement.focus();
+
+ // Since this has been a normal keypress, the suggestions will have
+ // been based on a previous input. Clear them. This prevents an
+ // outdated suggestion from being used if the input keystroke is
+ // immediately followed by a commit keystroke. @see Issue 8655
+ this._suggestions = [];
}
this.fire('input-keydown', {keyCode: e.keyCode, input: this.$.input});
},
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
index 872f79f..d257cdd 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
@@ -379,6 +379,21 @@
assert.isTrue(commitStub.calledOnce);
});
+ test('issue 8655', () => {
+ function makeSuggestion(s) { return {name: s, text: s, value: s}; }
+ const keydownSpy = sandbox.spy(element, '_handleKeydown');
+ element.setText('file:');
+ element._suggestions =
+ [makeSuggestion('file:'), makeSuggestion('-file:')];
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 88, null, 'x');
+ // Must set the value, because the MockInteraction does not.
+ element.$.input.value = 'file:x';
+ assert.isTrue(keydownSpy.calledOnce);
+ MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null, 'enter');
+ assert.isTrue(keydownSpy.calledTwice);
+ assert.equal(element.text, 'file:x');
+ });
+
suite('focus', () => {
let commitSpy;
let focusSpy;
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.html b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.html
index 96066de..8581396 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.html
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.html
@@ -76,6 +76,7 @@
--paper-input-container-input: {
font-size: var(--font-size-normal);
}
+ --paper-input-container-focus-color: var(--link-color);
}
</style>
<label
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
index ae363ea..cbae987 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.html
@@ -40,6 +40,8 @@
<g id="chevron-left"><path d="M15.41 7.41L14 6l-6 6 6 6 1.41-1.41L10.83 12z"/></g>
<!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.html -->
<g id="chevron-right"><path d="M10 6L8.59 7.41 13.17 12l-4.58 4.59L10 18l6-6z"/></g>
+ <!-- This SVG is a copy from material.io https://material.io/icons/#ic_hourglass_full-->
+ <g id="hourglass"><path d="M6 2v6h.01L6 8.01 10 12l-4 4 .01.01H6V22h12v-5.99h-.01L18 16l-4-4 4-3.99-.01-.01H18V2H6z"/><path d="M0 0h24v24H0V0z" fill="none"/></g>
<!-- This is a custom PolyGerrit SVG -->
<g id="side-by-side"><path d="M17.1578947,10.8888889 L2.84210526,10.8888889 C2.37894737,10.8888889 2,11.2888889 2,11.7777778 L2,17.1111111 C2,17.6 2.37894737,18 2.84210526,18 L17.1578947,18 C17.6210526,18 18,17.6 18,17.1111111 L18,11.7777778 C18,11.2888889 17.6210526,10.8888889 17.1578947,10.8888889 Z M17.1578947,2 L2.84210526,2 C2.37894737,2 2,2.4 2,2.88888889 L2,8.22222222 C2,8.71111111 2.37894737,9.11111111 2.84210526,9.11111111 L17.1578947,9.11111111 C17.6210526,9.11111111 18,8.71111111 18,8.22222222 L18,2.88888889 C18,2.4 17.6210526,2 17.1578947,2 Z M16.1973628,2 L2.78874238,2 C2.35493407,2 2,2.4 2,2.88888889 L2,8.22222222 C2,8.71111111 2.35493407,9.11111111 2.78874238,9.11111111 L16.1973628,9.11111111 C16.6311711,9.11111111 16.9861052,8.71111111 16.9861052,8.22222222 L16.9861052,2.88888889 C16.9861052,2.4 16.6311711,2 16.1973628,2 Z" id="Shape" transform="scale(1.2) translate(10.000000, 10.000000) rotate(-90.000000) translate(-10.000000, -10.000000)"/></g>
<!-- This is a custom PolyGerrit SVG -->
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
index d9fdc59..7d91f41 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
@@ -351,6 +351,15 @@
return Gerrit.awaitPluginsLoaded();
});
+ test('multiple ui plugins per java plugin', () => {
+ const file1 = 'http://test.com/plugins/qaz/static/foo.nocache.js';
+ const file2 = 'http://test.com/plugins/qaz/static/bar.js';
+ Gerrit._setPluginsPending([file1, file2]);
+ Gerrit.install(() => {}, '0.1', file1);
+ Gerrit.install(() => {}, '0.1', file2);
+ return Gerrit.awaitPluginsLoaded();
+ });
+
test('plugin install errors shows toasts', () => {
const alertStub = sandbox.stub();
document.addEventListener('show-alert', alertStub);
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
index ef20206..0b2133b 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
@@ -34,7 +34,7 @@
CHANGE_SCREEN_BELOW_CHANGE_INFO_BLOCK: 'change-metadata-item',
};
- const PLUGIN_LOADING_TIMEOUT_MS = 60000;
+ const PLUGIN_LOADING_TIMEOUT_MS = 10000;
let _restAPI;
const getRestAPI = () => {
@@ -561,7 +561,7 @@
o[getPluginNameFromUrl(url)] = url;
return o;
}, {});
- Gerrit._setPluginsCount(plugins.length);
+ Gerrit._setPluginsCount(Object.keys(_pluginsPending).length);
};
Gerrit._setPluginsCount = function(count) {
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
index 71999acaf..7df77ca 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
@@ -28,10 +28,15 @@
font-size: var(--font-size-normal);
max-width: 25em;
}
+ #filter:focus {
+ outline: none;
+ }
#topContainer {
+ align-items: center;
display: flex;
+ height: 3rem;
justify-content: space-between;
- margin: 1em;
+ margin: 0 1em;
}
#createNewContainer:not(.show) {
display: none;
@@ -44,18 +49,24 @@
text-decoration: underline;
}
nav {
- padding: .5em 0;
- text-align: center;
+ align-items: center;
+ display: flex;
+ height: 3rem;
+ justify-content: flex-end;
+ margin-right: 20px;
}
- nav a {
- display: inline-block;
+ nav,
+ iron-icon {
+ color: var(--deemphasized-text-color);
}
- nav a:first-of-type {
- margin-right: .5em;
+ iron-icon {
+ height: 1.85rem;
+ margin-left: 16px;
+ width: 1.85rem;
}
</style>
<div id="topContainer">
- <div>
+ <div class="filterContainer">
<label>Filter:</label>
<input is="iron-input"
type="text"
@@ -64,7 +75,7 @@
</div>
<div id="createNewContainer"
class$="[[_computeCreateClass(createNew)]]">
- <gr-button id="createNew" on-tap="_createNewItem">
+ <gr-button primary link id="createNew" on-tap="_createNewItem">
Create New
</gr-button>
</div>
@@ -73,11 +84,14 @@
<nav>
<a id="prevArrow"
href$="[[_computeNavLink(offset, -1, itemsPerPage, filter, path)]]"
- hidden$="[[_hidePrevArrow(offset)]]" hidden>← Prev</a>
+ hidden$="[[_hidePrevArrow(loading, offset)]]" hidden>
+ <iron-icon icon="gr-icons:chevron-left"></iron-icon>
+ </a>
<a id="nextArrow"
href$="[[_computeNavLink(offset, 1, itemsPerPage, filter, path)]]"
hidden$="[[_hideNextArrow(loading, items)]]" hidden>
- Next →</a>
+ <iron-icon icon="gr-icons:chevron-right"></iron-icon>
+ </a>
</nav>
</template>
<script src="gr-list-view.js"></script>
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js
index 53691fc..8b83eb3 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js
@@ -84,8 +84,8 @@
return createNew ? 'show' : '';
},
- _hidePrevArrow(offset) {
- return offset === 0;
+ _hidePrevArrow(loading, offset) {
+ return loading || offset === 0;
},
_hideNextArrow(loading, items) {
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
index d1cf80a..09e68dd 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view_test.html
@@ -114,11 +114,12 @@
});
test('prev button', () => {
+ assert.isTrue(element._hidePrevArrow(true, 0));
flush(() => {
let offset = 0;
- assert.isTrue(element._hidePrevArrow(offset));
+ assert.isTrue(element._hidePrevArrow(false, offset));
offset = 5;
- assert.isFalse(element._hidePrevArrow(offset));
+ assert.isFalse(element._hidePrevArrow(false, offset));
});
});
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index b5e7e40..9dc51ba 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -1410,9 +1410,10 @@
* @param {boolean=} opt_isPrivate
* @param {boolean=} opt_workInProgress
* @param {string=} opt_baseChange
+ * @param {string=} opt_baseCommit
*/
createChange(project, branch, subject, opt_topic, opt_isPrivate,
- opt_workInProgress, opt_baseChange) {
+ opt_workInProgress, opt_baseChange, opt_baseCommit) {
return this.send('POST', '/changes/', {
project,
branch,
@@ -1421,6 +1422,7 @@
is_private: opt_isPrivate,
work_in_progress: opt_workInProgress,
base_change: opt_baseChange,
+ base_commit: opt_baseCommit,
}).then(response => this.getResponseObject(response));
},
diff --git a/polygerrit-ui/app/styles/gr-form-styles.html b/polygerrit-ui/app/styles/gr-form-styles.html
index 88c75c8..f4b367b 100644
--- a/polygerrit-ui/app/styles/gr-form-styles.html
+++ b/polygerrit-ui/app/styles/gr-form-styles.html
@@ -59,9 +59,6 @@
.gr-form-styles .emptyHeader {
text-align: right;
}
- .gr-form-styles tbody tr:nth-child(even):not(.loading) {
- background-color: #f4f4f4;
- }
.gr-form-styles table {
width: 50em;
}
diff --git a/polygerrit-ui/app/styles/gr-page-nav-styles.html b/polygerrit-ui/app/styles/gr-page-nav-styles.html
index 6d62f23..6eee5a8 100644
--- a/polygerrit-ui/app/styles/gr-page-nav-styles.html
+++ b/polygerrit-ui/app/styles/gr-page-nav-styles.html
@@ -26,7 +26,7 @@
display: block;
padding: 0 2em;
}
- .navStyles li a {
+ .navStyles li a {
display: block;
overflow: hidden;
text-overflow: ellipsis;
@@ -50,8 +50,8 @@
}
.navStyles .selected {
background-color: var(--view-background-color);
- border-bottom: 1px dotted #808080;
- border-top: 1px dotted #808080;
+ border-bottom: 1px solid var(--border-color);
+ border-top: 1px solid var(--border-color);
font-family: var(--font-family-bold);
}
.navStyles a {
diff --git a/polygerrit-ui/app/styles/gr-table-styles.html b/polygerrit-ui/app/styles/gr-table-styles.html
index 2bcd743..5e40735 100644
--- a/polygerrit-ui/app/styles/gr-table-styles.html
+++ b/polygerrit-ui/app/styles/gr-table-styles.html
@@ -18,29 +18,64 @@
<dom-module id="gr-table-styles">
<template>
<style>
- .genericList .loading {
- display: none;
- }
.genericList {
border-collapse: collapse;
width: 100%;
}
- .genericList tr.table {
+ .genericList td {
+ height: 2.25rem;
+ padding: .3rem 0;
+ vertical-align: middle;
+ }
+ .genericList tr {
border-bottom: 1px solid var(--border-color);
}
+ .genericList th {
+ white-space: nowrap;
+ }
+ .genericList th,
+ .genericList td {
+ padding-right: 1rem;
+ }
+ .genericList tr th:first-of-type,
+ .genericList tr td:first-of-type {
+ padding-left: 1rem;
+ }
+ .genericList tr:first-of-type {
+ border-top: 1px solid var(--border-color);
+ }
+ .genericList tr th:last-of-type,
+ .genericList tr td:last-of-type {
+ border-left: 1px solid var(--border-color);
+ text-align: center;
+ padding: 0 1em;
+ }
+ .genericList tr th.delete,
+ .genericList tr td.delete,
+ .genericList tr.loadingMsg td {
+ border-left: none;
+ }
+ .genericList .loading {
+ border: none;
+ display: none;
+ }
.genericList td {
flex-shrink: 0;
- padding: .3em .5em;
}
- .genericList th {
- background-color: var(--border-color);
- border-bottom: 1px solid var(--border-color);
+ .genericList .topHeader,
+ .genericList .groupHeader {
+ color: var(--primary-text-color);
font-family: var(--font-family-bold);
- padding: .3em .5em;
text-align: left;
+ vertical-align: middle
+ }
+ .genericList .topHeader {
+ background-color: var(--table-header-background-color);
+ font-size: var(--font-size-large);
+ height: 3rem;
}
.genericList .groupHeader {
- background-color: #eee;
+ background-color: var(--table-subheader-background-color);
}
.genericList a {
color: var(--primary-text-color);
@@ -50,12 +85,12 @@
text-decoration: underline;
}
.genericList .description {
- width: 70%;
+ width: 99%;
}
.genericList .loadingMsg {
color: var(--deemphasized-text-color);
display: block;
- padding: 1em var(--default-horizontal-margin);
+ padding: .3em var(--default-horizontal-margin);
}
.genericList .loadingMsg:not(.loading) {
display: none;
diff --git a/proto/BUILD b/proto/BUILD
new file mode 100644
index 0000000..4528dcb
--- /dev/null
+++ b/proto/BUILD
@@ -0,0 +1,10 @@
+proto_library(
+ name = "cache_proto",
+ srcs = ["cache.proto"],
+)
+
+java_proto_library(
+ name = "cache_java_proto",
+ visibility = ["//visibility:public"],
+ deps = [":cache_proto"],
+)
diff --git a/proto/cache.proto b/proto/cache.proto
new file mode 100644
index 0000000..4a84ab1
--- /dev/null
+++ b/proto/cache.proto
@@ -0,0 +1,27 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+syntax = "proto3";
+
+package gerrit.cache;
+
+option java_package = "com.google.gerrit.server.cache.proto";
+
+// Serialized form of com.google.gerrit.server.change.CHangeKindCacheImpl.Key.
+// Next ID: 4
+message ChangeKindKeyProto {
+ bytes prior = 1;
+ bytes next = 2;
+ string strategy_name = 3;
+}
diff --git a/tools/eclipse/project.py b/tools/eclipse/project.py
index f054fad..a6b0964 100755
--- a/tools/eclipse/project.py
+++ b/tools/eclipse/project.py
@@ -145,6 +145,7 @@
doc = make_classpath()
src = set()
lib = set()
+ proto = set()
gwt_src = set()
gwt_lib = set()
plugins = set()
@@ -170,6 +171,9 @@
# JGit dependency from external repository
if 'gerrit-' not in p and 'jgit' in p:
lib.add(p)
+ # Assume any jars in /proto/ are from java_proto_library rules
+ if '/bin/proto/' in p:
+ proto.add(p)
else:
# Don't mess up with Bazel internal test runner dependencies.
# When we use Eclipse we rely on it for running the tests
@@ -239,6 +243,11 @@
continue
classpathentry('lib', j, s)
+ for p in sorted(proto):
+ s = p.replace('-fastbuild/bin/proto/lib', '-fastbuild/genfiles/proto/')
+ s = s.replace('.jar', '-src.jar')
+ classpathentry('lib', p, s)
+
for s in sorted(gwt_src):
p = path.join(ROOT, s, 'src', 'main', 'java')
if path.exists(p):
