Merge changes I100e4422,I6fcd8090,Ifef2ed6a,I70237fcb,Id7e778c2, ...
* changes:
Use RetryingRestModifyView for RebaseChangeEdit.Rebase
Do not throw NotImplementedException from PublishChangeEdit and RebaseChangeEdit
Implement views() method in PublishChangeEdit and RebaseChangeEdit
Do not reuse ChangeEditResource for child resources of 3 different collections
Let REST API binding tests fail if '405 Method Not Allowed' is returned
Extend REST binding tests to cover endpoints that are implemented by AcceptsDelete
Extend REST binding tests to cover endpoints that are implemented by AcceptsPost
Test that the change edit REST endpoints are correctly bound
Test that the change message REST endpoints are correctly bound
diff --git a/WORKSPACE b/WORKSPACE
index 95dd520..51c3618 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -445,18 +445,18 @@
sha1 = "430b2fc839b5de1f3643b528853d5cf26096c1de",
)
-AUTO_VALUE_VERSION = "1.6"
+AUTO_VALUE_VERSION = "1.6.2"
maven_jar(
name = "auto-value",
artifact = "com.google.auto.value:auto-value:" + AUTO_VALUE_VERSION,
- sha1 = "a3b1b1404f8acaa88594a017185e013cd342c9a8",
+ sha1 = "e7eae562942315a983eea3e191b72d755c153620",
)
maven_jar(
name = "auto-value-annotations",
artifact = "com.google.auto.value:auto-value-annotations:" + AUTO_VALUE_VERSION,
- sha1 = "da725083ee79fdcd86d9f3d8a76e38174a01892a",
+ sha1 = "ed193d86e0af90cc2342aedbe73c5d86b03fa09b",
)
# Transitive dependency of commons-compress
@@ -906,8 +906,8 @@
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:5.6.9",
- sha1 = "895706412e2fba3f842fca82ec3dece1cb4ee7d1",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:6.3.0",
+ sha1 = "a95ef38262ef499aa07cdb736f4a47cb19162654",
)
JACKSON_VERSION = "2.8.9"
diff --git a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
index 4184ec0..20c3427 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticConfiguration.java
@@ -16,6 +16,7 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -29,19 +30,20 @@
@Singleton
class ElasticConfiguration {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
private static final String DEFAULT_HOST = "localhost";
private static final String DEFAULT_PORT = "9200";
private static final String DEFAULT_PROTOCOL = "http";
private final Config cfg;
- final List<HttpHost> urls;
+ final List<HttpHost> hosts;
final String username;
final String password;
final boolean requestCompression;
final long connectionTimeout;
final long maxConnectionIdleTime;
- final TimeUnit maxConnectionIdleUnit = TimeUnit.MILLISECONDS;
final int maxTotalConnection;
final int readTimeout;
final String prefix;
@@ -66,18 +68,20 @@
if (subsections.isEmpty()) {
HttpHost httpHost =
new HttpHost(DEFAULT_HOST, Integer.valueOf(DEFAULT_PORT), DEFAULT_PROTOCOL);
- this.urls = Collections.singletonList(httpHost);
+ this.hosts = Collections.singletonList(httpHost);
} else {
- this.urls = new ArrayList<>(subsections.size());
+ this.hosts = new ArrayList<>(subsections.size());
for (String subsection : subsections) {
String port = getString(cfg, subsection, "port", DEFAULT_PORT);
String host = getString(cfg, subsection, "hostname", DEFAULT_HOST);
String protocol = getString(cfg, subsection, "protocol", DEFAULT_PROTOCOL);
HttpHost httpHost = new HttpHost(host, Integer.valueOf(port), protocol);
- this.urls.add(httpHost);
+ this.hosts.add(httpHost);
}
}
+
+ logger.atInfo().log("Elasticsearch hosts: %s", hosts);
}
Config getConfig() {
diff --git a/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java b/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java
index c2c4548..e318d61 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java
@@ -22,6 +22,7 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
+import java.util.List;
import org.apache.http.HttpHost;
import org.apache.http.HttpStatus;
import org.apache.http.StatusLine;
@@ -38,7 +39,7 @@
class ElasticRestClientProvider implements Provider<RestClient>, LifecycleListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final HttpHost[] hosts;
+ private final List<HttpHost> hosts;
private final String username;
private final String password;
@@ -47,7 +48,7 @@
@Inject
ElasticRestClientProvider(ElasticConfiguration cfg) {
- hosts = cfg.urls.toArray(new HttpHost[cfg.urls.size()]);
+ hosts = cfg.hosts;
username = cfg.username;
password = cfg.password;
}
@@ -131,7 +132,7 @@
}
private RestClient build() {
- RestClientBuilder builder = RestClient.builder(hosts);
+ RestClientBuilder builder = RestClient.builder(hosts.toArray(new HttpHost[hosts.size()]));
setConfiguredCredentialsIfAny(builder);
return builder.build();
}
diff --git a/java/com/google/gerrit/extensions/restapi/RestApiModule.java b/java/com/google/gerrit/extensions/restapi/RestApiModule.java
index e65e6e5..ee31113 100644
--- a/java/com/google/gerrit/extensions/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/extensions/restapi/RestApiModule.java
@@ -75,7 +75,7 @@
return new ChildCollectionBinder<>(view(type, GET, name));
}
- protected <R extends RestResource> LinkedBindingBuilder<RestView<R>> view(
+ private <R extends RestResource> LinkedBindingBuilder<RestView<R>> view(
TypeLiteral<RestView<R>> viewType, String method, String name) {
return bind(viewType).annotatedWith(export(method, name));
}
diff --git a/java/com/google/gerrit/gpg/server/GpgKeys.java b/java/com/google/gerrit/gpg/server/GpgKeys.java
index a2d901f..3f090a1 100644
--- a/java/com/google/gerrit/gpg/server/GpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/GpgKeys.java
@@ -59,8 +59,6 @@
public class GpgKeys implements ChildCollection<AccountResource, GpgKey> {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- public static final String MIME_TYPE = "application/pgp-keys";
-
private final DynamicMap<RestView<GpgKey>> views;
private final Provider<CurrentUser> self;
private final Provider<PublicKeyStore> storeProvider;
diff --git a/java/com/google/gerrit/server/PatchSetUtil.java b/java/com/google/gerrit/server/PatchSetUtil.java
index 252eb61..c9ca972 100644
--- a/java/com/google/gerrit/server/PatchSetUtil.java
+++ b/java/com/google/gerrit/server/PatchSetUtil.java
@@ -214,14 +214,13 @@
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 {
+ /** Returns the commit for the given project at the given patchset revision */
+ public RevCommit getRevCommit(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();
+ return src;
}
}
}
diff --git a/java/com/google/gerrit/server/account/Emails.java b/java/com/google/gerrit/server/account/Emails.java
index 8a48167..e91ce49 100644
--- a/java/com/google/gerrit/server/account/Emails.java
+++ b/java/com/google/gerrit/server/account/Emails.java
@@ -95,6 +95,16 @@
return builder.build();
}
+ /**
+ * Returns the accounts with the given email.
+ *
+ * <p>This method behaves just like {@link #getAccountFor(String)}, except that accounts are not
+ * looked up by their preferred email. Thus, this method does not rely on the accounts index.
+ */
+ public ImmutableSet<Account.Id> getAccountForExternal(String email) throws IOException {
+ return externalIds.byEmail(email).stream().map(ExternalId::accountId).collect(toImmutableSet());
+ }
+
private <T> T executeIndexQuery(Action<T> action) throws OrmException {
try {
return retryHelper.execute(ActionType.INDEX_QUERY, action, OrmException.class::isInstance);
diff --git a/java/com/google/gerrit/server/change/ChangeEditResource.java b/java/com/google/gerrit/server/change/ChangeEditResource.java
index 2e41da7..5419ee9 100644
--- a/java/com/google/gerrit/server/change/ChangeEditResource.java
+++ b/java/com/google/gerrit/server/change/ChangeEditResource.java
@@ -20,7 +20,7 @@
import com.google.inject.TypeLiteral;
/**
- * Represents change edit resource, that is actualy two kinds of resources:
+ * Represents change edit resource, that is actually two kinds of resources:
*
* <ul>
* <li>the change edit itself
diff --git a/java/com/google/gerrit/server/change/PureRevert.java b/java/com/google/gerrit/server/change/PureRevert.java
index ee0e2cc..2c450a7 100644
--- a/java/com/google/gerrit/server/change/PureRevert.java
+++ b/java/com/google/gerrit/server/change/PureRevert.java
@@ -117,7 +117,7 @@
// Any differences between claimed original's parent and the rebase result indicate that the
// claimedRevert is not a pure revert but made content changes
try (DiffFormatter df = new DiffFormatter(new ByteArrayOutputStream())) {
- df.setRepository(repo);
+ df.setReader(oi.newReader(), repo.getConfig());
List<DiffEntry> entries =
df.scan(claimedOriginalCommit.getParent(0), merger.getResultTreeId());
return new PureRevertInfo(entries.isEmpty());
diff --git a/java/com/google/gerrit/server/config/GitwebConfig.java b/java/com/google/gerrit/server/config/GitwebConfig.java
index f38572d..b5f09fd 100644
--- a/java/com/google/gerrit/server/config/GitwebConfig.java
+++ b/java/com/google/gerrit/server/config/GitwebConfig.java
@@ -36,6 +36,8 @@
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import java.net.MalformedURLException;
+import java.net.URL;
import org.eclipse.jgit.lib.Config;
public class GitwebConfig {
@@ -178,7 +180,11 @@
private final GitwebType type;
@Inject
- GitwebConfig(GitwebCgiConfig cgiConfig, @GerritServerConfig Config cfg) {
+ GitwebConfig(
+ GitwebCgiConfig cgiConfig,
+ @GerritServerConfig Config cfg,
+ @Nullable @CanonicalWebUrl String gerritUrl)
+ throws MalformedURLException {
if (isDisabled(cfg)) {
type = null;
url = null;
@@ -191,7 +197,14 @@
// Use an externally managed gitweb instance, and not an internal one.
url = cfgUrl;
} else {
- url = firstNonNull(cfgUrl, "gitweb");
+ String baseGerritUrl;
+ if (gerritUrl != null) {
+ URL u = new URL(gerritUrl);
+ baseGerritUrl = u.getPath();
+ } else {
+ baseGerritUrl = "/";
+ }
+ url = firstNonNull(cfgUrl, baseGerritUrl + "gitweb");
}
}
}
diff --git a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index 3653bc7..010c5c0 100644
--- a/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -120,9 +120,7 @@
ChangeNoteUtil noteUtil, PersonIdent serverIdent, CurrentUser u, Date when) {
checkUserType(u);
if (u instanceof IdentifiedUser) {
- return noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
+ return noteUtil.newIdent(u.asIdentifiedUser().getAccount(), when, serverIdent);
} else if (u instanceof InternalUser) {
return serverIdent;
}
@@ -177,7 +175,7 @@
}
protected PersonIdent newIdent(Account.Id authorId, Date when) {
- return noteUtil.getLegacyChangeNoteWrite().newIdent(authorId, when, serverIdent);
+ return noteUtil.newIdent(authorId, when, serverIdent);
}
/** Whether no updates have been done. */
diff --git a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
index 6b4bea7..169fd2e 100644
--- a/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeDraftUpdate.java
@@ -178,12 +178,7 @@
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
updatedRevs.add(e.getKey());
ObjectId id = ObjectId.fromString(e.getKey().get());
- byte[] data =
- e.getValue()
- .build(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteWrite(),
- noteUtil.getChangeNoteJson().getWriteJson());
+ byte[] data = e.getValue().build(noteUtil.getChangeNoteJson());
if (!Arrays.equals(data, e.getValue().baseRaw)) {
touchedAnyRevs = true;
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
index 0475fe3..483b2e9 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteJson.java
@@ -14,18 +14,14 @@
package com.google.gerrit.server.notedb;
-import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
-import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.sql.Timestamp;
-import org.eclipse.jgit.lib.Config;
@Singleton
public class ChangeNoteJson {
private final Gson gson = newGson();
- private final boolean writeJson;
static Gson newGson() {
return new GsonBuilder()
@@ -37,13 +33,4 @@
public Gson getGson() {
return gson;
}
-
- public boolean getWriteJson() {
- return writeJson;
- }
-
- @Inject
- ChangeNoteJson(@GerritServerConfig Config config) {
- this.writeJson = config.getBoolean("notedb", "writeJson", true);
- }
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index 070f974..cf86c99 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -14,7 +14,15 @@
package com.google.gerrit.server.notedb;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.config.GerritServerId;
import com.google.inject.Inject;
+import java.util.Date;
+import java.util.Optional;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.revwalk.FooterKey;
public class ChangeNoteUtil {
@@ -56,17 +64,21 @@
static final String TAG = FOOTER_TAG.getName();
private final LegacyChangeNoteRead legacyChangeNoteRead;
- private final LegacyChangeNoteWrite legacyChangeNoteWrite;
private final ChangeNoteJson changeNoteJson;
+ private final AccountCache accountCache;
+ private final String serverId;
+
@Inject
public ChangeNoteUtil(
ChangeNoteJson changeNoteJson,
LegacyChangeNoteRead legacyChangeNoteRead,
- LegacyChangeNoteWrite legacyChangeNoteWrite) {
+ AccountCache accountCache,
+ @GerritServerId String serverId) {
+ this.accountCache = accountCache;
+ this.serverId = serverId;
this.changeNoteJson = changeNoteJson;
this.legacyChangeNoteRead = legacyChangeNoteRead;
- this.legacyChangeNoteWrite = legacyChangeNoteWrite;
}
public LegacyChangeNoteRead getLegacyChangeNoteRead() {
@@ -77,7 +89,18 @@
return changeNoteJson;
}
- public LegacyChangeNoteWrite getLegacyChangeNoteWrite() {
- return legacyChangeNoteWrite;
+ public PersonIdent newIdent(Account.Id authorId, Date when, PersonIdent serverIdent) {
+ Optional<Account> author = accountCache.get(authorId).map(AccountState::getAccount);
+ return new PersonIdent(
+ author.map(Account::getName).orElseGet(() -> Account.getName(authorId)),
+ authorId.get() + "@" + serverId,
+ when,
+ serverIdent.getTimeZone());
+ }
+
+ @VisibleForTesting
+ public PersonIdent newIdent(Account author, Date when, PersonIdent serverIdent) {
+ return new PersonIdent(
+ author.getName(), author.getId().get() + "@" + serverId, when, serverIdent.getTimeZone());
}
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
index 894e979..66dd5e8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
+++ b/java/com/google/gerrit/server/notedb/ChangeRevisionNote.java
@@ -88,7 +88,7 @@
return comments;
}
- private static boolean isJson(byte[] raw, int offset) {
+ static boolean isJson(byte[] raw, int offset) {
return raw[offset] == '{' || raw[offset] == '[';
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 445f7a0..69fbfe1 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -402,6 +402,7 @@
if (notes != null) {
draftUpdate = draftUpdateFactory.create(notes, accountId, realAccountId, authorIdent, when);
} else {
+ // tests will always take the notes != null path above.
draftUpdate =
draftUpdateFactory.create(getChange(), accountId, realAccountId, authorIdent, when);
}
@@ -526,14 +527,7 @@
checkComments(rnm.revisionNotes, builders);
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
- ObjectId data =
- inserter.insert(
- OBJ_BLOB,
- e.getValue()
- .build(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteWrite(),
- noteUtil.getChangeNoteJson().getWriteJson()));
+ ObjectId data = inserter.insert(OBJ_BLOB, e.getValue().build(noteUtil.getChangeNoteJson()));
rnm.noteMap.set(ObjectId.fromString(e.getKey().get()), data);
}
diff --git a/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java b/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
new file mode 100644
index 0000000..44cf02fb
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/CommentJsonMigrator.java
@@ -0,0 +1,190 @@
+// Copyright (C) 2017 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.notedb;
+
+import static com.google.gerrit.server.notedb.RevisionNote.MAX_NOTE_SZ;
+import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.server.config.GerritServerIdProvider;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.Note;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevSort;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.eclipse.jgit.util.MutableInteger;
+
+@Singleton
+public class CommentJsonMigrator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final LegacyChangeNoteRead legacyChangeNoteRead;
+ private final ChangeNoteJson changeNoteJson;
+
+ @Inject
+ CommentJsonMigrator(
+ ChangeNoteJson changeNoteJson, GerritServerIdProvider gerritServerIdProvider) {
+ this.changeNoteJson = changeNoteJson;
+ this.legacyChangeNoteRead = new LegacyChangeNoteRead(gerritServerIdProvider.get());
+ }
+
+ CommentJsonMigrator(ChangeNoteJson changeNoteJson, String serverId) {
+ this.changeNoteJson = changeNoteJson;
+ this.legacyChangeNoteRead = new LegacyChangeNoteRead(serverId);
+ }
+
+ public boolean migrateChanges(
+ Project.NameKey project, Repository repo, RevWalk rw, ObjectInserter ins, BatchRefUpdate bru)
+ throws IOException {
+ boolean ok = true;
+ for (Ref ref : repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES).values()) {
+ Change.Id changeId = Change.Id.fromRef(ref.getName());
+ if (changeId == null || !ref.getName().equals(RefNames.changeMetaRef(changeId))) {
+ continue;
+ }
+ ok &= migrateOne(project, rw, ins, bru, Status.PUBLISHED, changeId, ref);
+ }
+ return ok;
+ }
+
+ public boolean migrateDrafts(
+ Project.NameKey allUsers,
+ Repository allUsersRepo,
+ RevWalk rw,
+ ObjectInserter ins,
+ BatchRefUpdate bru)
+ throws IOException {
+ boolean ok = true;
+ for (Ref ref : allUsersRepo.getRefDatabase().getRefs(RefNames.REFS_DRAFT_COMMENTS).values()) {
+ Change.Id changeId = Change.Id.fromAllUsersRef(ref.getName());
+ if (changeId == null) {
+ continue;
+ }
+ ok &= migrateOne(allUsers, rw, ins, bru, Status.DRAFT, changeId, ref);
+ }
+ return ok;
+ }
+
+ private boolean migrateOne(
+ Project.NameKey project,
+ RevWalk rw,
+ ObjectInserter ins,
+ BatchRefUpdate bru,
+ Status status,
+ Change.Id changeId,
+ Ref ref) {
+ ObjectId oldId = ref.getObjectId();
+ try {
+ if (!hasAnyLegacyComments(rw, oldId)) {
+ return true;
+ }
+ } catch (IOException e) {
+ logger.atInfo().log(
+ String.format(
+ "Error reading change %s in %s; attempting migration anyway", changeId, project),
+ e);
+ }
+
+ try {
+ reset(rw, oldId);
+
+ ObjectReader reader = rw.getObjectReader();
+ ObjectId newId = null;
+ RevCommit c;
+ while ((c = rw.next()) != null) {
+ CommitBuilder cb = new CommitBuilder();
+ cb.setAuthor(c.getAuthorIdent());
+ cb.setCommitter(c.getCommitterIdent());
+ cb.setMessage(c.getFullMessage());
+ cb.setEncoding(c.getEncoding());
+ if (newId != null) {
+ cb.setParentId(newId);
+ }
+
+ // Read/write using the low-level RevisionNote API, which works regardless of NotesMigration
+ // state.
+ NoteMap noteMap = NoteMap.read(reader, c);
+ RevisionNoteMap<ChangeRevisionNote> revNoteMap =
+ RevisionNoteMap.parse(
+ changeNoteJson, legacyChangeNoteRead, changeId, reader, noteMap, status);
+ RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNoteMap);
+
+ for (RevId revId : revNoteMap.revisionNotes.keySet()) {
+ // Call cache.get on each known RevId to read the old note in whichever format, then write
+ // the note in JSON format.
+ byte[] data = cache.get(revId).build(changeNoteJson);
+ noteMap.set(ObjectId.fromString(revId.get()), ins.insert(OBJ_BLOB, data));
+ }
+ cb.setTreeId(noteMap.writeTree(ins));
+ newId = ins.insert(cb);
+ }
+
+ bru.addCommand(new ReceiveCommand(oldId, newId, ref.getName()));
+ return true;
+ } catch (ConfigInvalidException | IOException e) {
+ logger.atInfo().log(String.format("Error migrating change %s in %s", changeId, project), e);
+ return false;
+ }
+ }
+
+ private static boolean hasAnyLegacyComments(RevWalk rw, ObjectId id) throws IOException {
+ ObjectReader reader = rw.getObjectReader();
+ reset(rw, id);
+
+ // Check the note map at each commit, not just the tip. It's possible that the server switched
+ // from legacy to JSON partway through its history, which would have mixed legacy/JSON comments
+ // in its history. Although the tip commit would continue to parse once we remove the legacy
+ // parser, our goal is really to expunge all vestiges of the old format, which implies rewriting
+ // history (and thus returning true) in this case.
+ RevCommit c;
+ while ((c = rw.next()) != null) {
+ NoteMap noteMap = NoteMap.read(reader, c);
+ for (Note note : noteMap) {
+ // Match pre-parsing logic in RevisionNote#parse().
+ byte[] raw = reader.open(note.getData(), OBJ_BLOB).getCachedBytes(MAX_NOTE_SZ);
+ MutableInteger p = new MutableInteger();
+ RevisionNote.trimLeadingEmptyLines(raw, p);
+ if (!ChangeRevisionNote.isJson(raw, p.value)) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
+ private static void reset(RevWalk rw, ObjectId id) throws IOException {
+ rw.reset();
+ rw.sort(RevSort.TOPO);
+ rw.sort(RevSort.REVERSE);
+ rw.markStart(rw.parseCommit(id));
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
index 9a8c130..0cd3452 100644
--- a/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
+++ b/java/com/google/gerrit/server/notedb/DeleteCommentRewriter.java
@@ -239,13 +239,7 @@
Map<RevId, RevisionNoteBuilder> builders = cache.getBuilders();
for (Map.Entry<RevId, RevisionNoteBuilder> entry : builders.entrySet()) {
ObjectId objectId = ObjectId.fromString(entry.getKey().get());
- byte[] data =
- entry
- .getValue()
- .build(
- noteUtil.getChangeNoteJson(),
- noteUtil.getLegacyChangeNoteWrite(),
- noteUtil.getChangeNoteJson().getWriteJson());
+ byte[] data = entry.getValue().build(noteUtil.getChangeNoteJson());
if (data.length == 0) {
revNotesMap.noteMap.remove(objectId);
} else {
diff --git a/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java b/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
index 8bf286d..b8c7d7d 100644
--- a/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
+++ b/java/com/google/gerrit/server/notedb/RevisionNoteBuilder.java
@@ -82,19 +82,13 @@
delete = new HashSet<>();
}
- public byte[] build(ChangeNoteUtil noteUtil, boolean writeJson) throws IOException {
- return build(noteUtil.getChangeNoteJson(), noteUtil.getLegacyChangeNoteWrite(), writeJson);
+ public byte[] build(ChangeNoteUtil noteUtil) throws IOException {
+ return build(noteUtil.getChangeNoteJson());
}
- public byte[] build(
- ChangeNoteJson changeNoteJson, LegacyChangeNoteWrite legacyChangeNoteWrite, boolean writeJson)
- throws IOException {
+ public byte[] build(ChangeNoteJson changeNoteJson) throws IOException {
ByteArrayOutputStream out = new ByteArrayOutputStream();
- if (writeJson) {
- buildNoteJson(changeNoteJson, out);
- } else {
- buildNoteLegacy(legacyChangeNoteWrite, out);
- }
+ buildNoteJson(changeNoteJson, out);
return out.toByteArray();
}
@@ -142,22 +136,4 @@
noteUtil.getGson().toJson(data, osw);
}
}
-
- private void buildNoteLegacy(LegacyChangeNoteWrite noteUtil, OutputStream out)
- throws IOException {
- if (pushCert != null) {
- byte[] certBytes = pushCert.getBytes(UTF_8);
- out.write(certBytes, 0, trimTrailingNewlines(certBytes));
- out.write('\n');
- }
- noteUtil.buildNote(buildCommentMap(), out);
- }
-
- private static int trimTrailingNewlines(byte[] bytes) {
- int p = bytes.length;
- while (p > 1 && bytes[p - 1] == '\n') {
- p--;
- }
- return p;
- }
}
diff --git a/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java b/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
index 3a0d595..e7ac5b7 100644
--- a/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
+++ b/java/com/google/gerrit/server/notedb/RobotCommentUpdate.java
@@ -142,7 +142,7 @@
for (Map.Entry<RevId, RevisionNoteBuilder> e : builders.entrySet()) {
updatedRevs.add(e.getKey());
ObjectId id = ObjectId.fromString(e.getKey().get());
- byte[] data = e.getValue().build(noteUtil, true);
+ byte[] data = e.getValue().build(noteUtil);
if (!Arrays.equals(data, e.getValue().baseRaw)) {
touchedAnyRevs = true;
}
diff --git a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index 5d2ce97..ab2203e 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -546,7 +546,7 @@
if (id == null) {
return new PersonIdent(serverIdent, events.getWhen());
}
- return changeNoteUtil.getLegacyChangeNoteWrite().newIdent(id, events.getWhen(), serverIdent);
+ return changeNoteUtil.newIdent(id, events.getWhen(), serverIdent);
}
private List<HashtagsEvent> getHashtagsEvents(Change change, NoteDbUpdateManager manager)
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 3a17965..82001fb 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -327,8 +327,8 @@
case ABANDON:
return canAbandon();
case DELETE:
- return (isOwner() && refControl.canPerform(Permission.DELETE_OWN_CHANGES))
- || getProjectControl().isAdmin();
+ return (getProjectControl().isAdmin()
+ || (isOwner() && refControl.canDeleteOwnChanges(isOwner())));
case ADD_PATCH_SET:
return canAddPatchSet();
case EDIT_ASSIGNEE:
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index cd1f84a..0e3382c 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -133,6 +133,11 @@
return canPerform(Permission.EDIT_TOPIC_NAME, false, true);
}
+ /** @return true if this user can delete their own changes. */
+ boolean canDeleteOwnChanges(boolean isChangeOwner) {
+ return canPerform(Permission.DELETE_OWN_CHANGES, isChangeOwner, false);
+ }
+
/** The range of permitted values associated with a label permission. */
PermissionRange getRange(String permission) {
return getRange(permission, false);
diff --git a/java/com/google/gerrit/server/restapi/account/Module.java b/java/com/google/gerrit/server/restapi/account/Module.java
index 7b09bc9..f37687f 100644
--- a/java/com/google/gerrit/server/restapi/account/Module.java
+++ b/java/com/google/gerrit/server/restapi/account/Module.java
@@ -107,6 +107,8 @@
get(ACCOUNT_KIND, "external.ids").to(GetExternalIds.class);
post(ACCOUNT_KIND, "external.ids:delete").to(DeleteExternalIds.class);
+ // The gpgkeys REST endpoints are bound via GpgApiModule.
+
factory(AccountsUpdate.Factory.class);
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
index 1529dae..33155f1 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateAccessChange.java
@@ -157,6 +157,8 @@
bu.execute();
return Response.created(jsonFactory.noOptions().format(ins.getChange()));
}
+ } catch (InvalidNameException e) {
+ throw new BadRequestException(e.toString());
}
}
diff --git a/java/com/google/gerrit/server/rules/PrologEnvironment.java b/java/com/google/gerrit/server/rules/PrologEnvironment.java
index 083898b..412e0f9 100644
--- a/java/com/google/gerrit/server/rules/PrologEnvironment.java
+++ b/java/com/google/gerrit/server/rules/PrologEnvironment.java
@@ -18,6 +18,7 @@
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchListCache;
@@ -176,6 +177,7 @@
private final int reductionLimit;
private final int compileLimit;
private final PatchSetUtil patchsetUtil;
+ private Emails emails;
@Inject
Args(
@@ -187,7 +189,8 @@
IdentifiedUser.GenericFactory userFactory,
Provider<AnonymousUser> anonymousUser,
@GerritServerConfig Config config,
- PatchSetUtil patchsetUtil) {
+ PatchSetUtil patchsetUtil,
+ Emails emails) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.repositoryManager = repositoryManager;
@@ -196,6 +199,7 @@
this.userFactory = userFactory;
this.anonymousUser = anonymousUser;
this.patchsetUtil = patchsetUtil;
+ this.emails = emails;
int limit = config.getInt("rules", null, "reductionLimit", 100000);
reductionLimit = limit <= 0 ? Integer.MAX_VALUE : limit;
@@ -247,5 +251,9 @@
public PatchSetUtil getPatchsetUtil() {
return patchsetUtil;
}
+
+ public Emails getEmails() {
+ return emails;
+ }
}
}
diff --git a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
index 8e39b50..e61fc90 100644
--- a/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
+++ b/java/com/google/gerrit/server/rules/PrologRuleEvaluator.java
@@ -297,7 +297,8 @@
try {
return LabelType.checkName(name);
} catch (IllegalArgumentException e) {
- return LabelType.checkName("Invalid-Prolog-Rules-Label-Name--" + sanitizeLabelName(name));
+ String newName = "Invalid-Prolog-Rules-Label-Name-" + sanitizeLabelName(name);
+ return LabelType.checkName(newName.replace("--", "-"));
}
}
diff --git a/java/com/google/gerrit/server/rules/StoredValues.java b/java/com/google/gerrit/server/rules/StoredValues.java
index 6770732..8b9cfe3 100644
--- a/java/com/google/gerrit/server/rules/StoredValues.java
+++ b/java/com/google/gerrit/server/rules/StoredValues.java
@@ -20,7 +20,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
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.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
@@ -34,8 +33,6 @@
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -47,6 +44,7 @@
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
public final class StoredValues {
public static final StoredValue<Accounts> ACCOUNTS = create(Accounts.class);
@@ -74,32 +72,16 @@
}
}
- public static final StoredValue<PatchSetInfo> PATCH_SET_INFO =
- new StoredValue<PatchSetInfo>() {
+ public static final StoredValue<RevCommit> COMMIT =
+ new StoredValue<RevCommit>() {
@Override
- public PatchSetInfo createValue(Prolog engine) {
- Change change = getChange(engine);
- PatchSet ps = getPatchSet(engine);
- PrologEnvironment env = (PrologEnvironment) engine.control;
- PatchSetInfoFactory patchInfoFactory = env.getArgs().getPatchSetInfoFactory();
- try {
- return patchInfoFactory.get(change.getProject(), ps);
- } catch (PatchSetInfoNotAvailableException e) {
- throw new SystemException(e.getMessage());
- }
- }
- };
-
- public static final StoredValue<String> COMMIT_MESSAGE =
- new StoredValue<String>() {
- @Override
- public String createValue(Prolog engine) {
+ public RevCommit 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);
+ return patchSetUtil.getRevCommit(change.getProject(), ps);
} catch (IOException e) {
throw new SystemException(e.getMessage());
}
diff --git a/java/com/google/gerrit/server/schema/SchemaVersion.java b/java/com/google/gerrit/server/schema/SchemaVersion.java
index e8a59d0..48cc91e 100644
--- a/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/java/com/google/gerrit/server/schema/SchemaVersion.java
@@ -35,7 +35,7 @@
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
- public static final Class<Schema_168> C = Schema_168.class;
+ public static final Class<Schema_169> C = Schema_169.class;
public static int getBinaryVersion() {
return guessVersion(C);
diff --git a/java/com/google/gerrit/server/schema/Schema_169.java b/java/com/google/gerrit/server/schema/Schema_169.java
new file mode 100644
index 0000000..11aebdd
--- /dev/null
+++ b/java/com/google/gerrit/server/schema/Schema_169.java
@@ -0,0 +1,122 @@
+// Copyright (C) 2017 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.schema;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.notedb.CommentJsonMigrator;
+import com.google.gerrit.server.notedb.MutableNotesMigration;
+import com.google.gerrit.server.notedb.NotesMigration;
+import com.google.gerrit.server.update.RefUpdateUtil;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.SortedSet;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
+import org.eclipse.jgit.internal.storage.file.PackInserter;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.TextProgressMonitor;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/** Migrate NoteDb inline comments to JSON format. */
+public class Schema_169 extends SchemaVersion {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final AllUsersName allUsers;
+ private final CommentJsonMigrator migrator;
+ private final GitRepositoryManager repoManager;
+ private final NotesMigration notesMigration;
+
+ @Inject
+ Schema_169(
+ Provider<Schema_168> prior,
+ AllUsersName allUsers,
+ CommentJsonMigrator migrator,
+ GitRepositoryManager repoManager,
+ @GerritServerConfig Config config) {
+ super(prior);
+ this.allUsers = allUsers;
+ this.migrator = migrator;
+ this.repoManager = repoManager;
+ this.notesMigration = MutableNotesMigration.fromConfig(config);
+ }
+
+ @Override
+ protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
+ migrateData(ui);
+ }
+
+ private static ObjectInserter newPackInserter(Repository repo) {
+ if (!(repo instanceof FileRepository)) {
+ return repo.newObjectInserter();
+ }
+ PackInserter ins = ((FileRepository) repo).getObjectDatabase().newPackInserter();
+ ins.checkExisting(false);
+ return ins;
+ }
+
+ @VisibleForTesting
+ protected void migrateData(UpdateUI ui) throws OrmException {
+ // If the migration hasn't started, no need to look for non-JSON
+ if (!notesMigration.commitChangeWrites()) {
+ return;
+ }
+
+ boolean ok = true;
+ ProgressMonitor pm = new TextProgressMonitor();
+ SortedSet<Project.NameKey> projects = repoManager.list();
+ pm.beginTask("Migrating projects", projects.size());
+ int skipped = 0;
+ for (Project.NameKey project : projects) {
+ try (Repository repo = repoManager.openRepository(project);
+ RevWalk rw = new RevWalk(repo);
+ ObjectInserter ins = newPackInserter(repo)) {
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ bru.setAllowNonFastForwards(true);
+ ok &= migrator.migrateChanges(project, repo, rw, ins, bru);
+ if (project.equals(allUsers)) {
+ ok &= migrator.migrateDrafts(allUsers, repo, rw, ins, bru);
+ }
+
+ if (!bru.getCommands().isEmpty()) {
+ ins.flush();
+ RefUpdateUtil.executeChecked(bru, rw);
+ } else {
+ skipped++;
+ }
+ } catch (IOException e) {
+ logger.atWarning().log("Error migrating project " + project, e);
+ ok = false;
+ }
+ pm.update(1);
+ }
+ pm.endTask();
+ ui.message(
+ "Skipped " + skipped + " project" + (skipped == 1 ? "" : "s") + " with no legacy comments");
+
+ if (!ok) {
+ throw new OrmException("Migration failed");
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/submit/SubmoduleOp.java b/java/com/google/gerrit/server/submit/SubmoduleOp.java
index 7984c76e..7be1307 100644
--- a/java/com/google/gerrit/server/submit/SubmoduleOp.java
+++ b/java/com/google/gerrit/server/submit/SubmoduleOp.java
@@ -423,7 +423,8 @@
if (newCommit != null) {
if (author == null) {
author = newCommit.getAuthorIdent();
- } else if (!author.equals(newCommit.getAuthorIdent())) {
+ } else if (!author.getName().equals(newCommit.getAuthorIdent().getName())
+ || !author.getEmailAddress().equals(newCommit.getAuthorIdent().getEmailAddress())) {
author = myIdent;
}
}
diff --git a/java/gerrit/AbstractCommitUserIdentityPredicate.java b/java/gerrit/AbstractCommitUserIdentityPredicate.java
index c2aaa76..1bfc95c 100644
--- a/java/gerrit/AbstractCommitUserIdentityPredicate.java
+++ b/java/gerrit/AbstractCommitUserIdentityPredicate.java
@@ -14,9 +14,12 @@
package gerrit;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.UserIdentity;
+import com.google.gerrit.server.account.Emails;
+import com.google.gerrit.server.rules.PrologEnvironment;
import com.googlecode.prolog_cafe.exceptions.PrologException;
+import com.googlecode.prolog_cafe.exceptions.SystemException;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Predicate;
@@ -24,6 +27,8 @@
import com.googlecode.prolog_cafe.lang.StructureTerm;
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+import java.io.IOException;
+import org.eclipse.jgit.lib.PersonIdent;
abstract class AbstractCommitUserIdentityPredicate extends Predicate.P3 {
private static final SymbolTerm user = SymbolTerm.intern("user", 1);
@@ -36,7 +41,7 @@
cont = n;
}
- protected Operation exec(Prolog engine, UserIdentity userId) throws PrologException {
+ protected Operation exec(Prolog engine, PersonIdent userId) throws PrologException {
engine.setB0();
Term a1 = arg1.dereference();
Term a2 = arg2.dereference();
@@ -46,7 +51,18 @@
Term nameTerm = Prolog.Nil;
Term emailTerm = Prolog.Nil;
- Account.Id id = userId.getAccount();
+ PrologEnvironment env = (PrologEnvironment) engine.control;
+ Emails emails = env.getArgs().getEmails();
+ Account.Id id = null;
+ try {
+ ImmutableSet<Account.Id> ids = emails.getAccountForExternal(userId.getEmailAddress());
+ if (ids.size() == 1) {
+ id = ids.iterator().next();
+ }
+ } catch (IOException e) {
+ throw new SystemException(e.getMessage());
+ }
+
if (id == null) {
idTerm = anonymous;
} else {
@@ -58,7 +74,7 @@
nameTerm = SymbolTerm.create(name);
}
- String email = userId.getEmail();
+ String email = userId.getEmailAddress();
if (email != null && !email.equals("")) {
emailTerm = SymbolTerm.create(email);
}
diff --git a/java/gerrit/BUILD b/java/gerrit/BUILD
index 4644af87..8281d8e 100644
--- a/java/gerrit/BUILD
+++ b/java/gerrit/BUILD
@@ -11,5 +11,6 @@
"//lib/flogger:api",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/prolog:runtime",
+ "@guava//jar",
],
)
diff --git a/java/gerrit/PRED_commit_author_3.java b/java/gerrit/PRED_commit_author_3.java
index a876b5e..998b30e 100644
--- a/java/gerrit/PRED_commit_author_3.java
+++ b/java/gerrit/PRED_commit_author_3.java
@@ -14,13 +14,12 @@
package gerrit;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
public class PRED_commit_author_3 extends AbstractCommitUserIdentityPredicate {
public PRED_commit_author_3(Term a1, Term a2, Term a3, Operation n) {
@@ -29,8 +28,7 @@
@Override
public Operation exec(Prolog engine) throws PrologException {
- PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
- UserIdentity author = psInfo.getAuthor();
- return exec(engine, author);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ return exec(engine, revCommit.getAuthorIdent());
}
}
diff --git a/java/gerrit/PRED_commit_committer_3.java b/java/gerrit/PRED_commit_committer_3.java
index b24b004..293d8ce 100644
--- a/java/gerrit/PRED_commit_committer_3.java
+++ b/java/gerrit/PRED_commit_committer_3.java
@@ -14,13 +14,12 @@
package gerrit;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
-import com.google.gerrit.reviewdb.client.UserIdentity;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.Operation;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
public class PRED_commit_committer_3 extends AbstractCommitUserIdentityPredicate {
public PRED_commit_committer_3(Term a1, Term a2, Term a3, Operation n) {
@@ -29,8 +28,7 @@
@Override
public Operation exec(Prolog engine) throws PrologException {
- PatchSetInfo psInfo = StoredValues.PATCH_SET_INFO.get(engine);
- UserIdentity committer = psInfo.getCommitter();
- return exec(engine, committer);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ return exec(engine, revCommit.getCommitterIdent());
}
}
diff --git a/java/gerrit/PRED_commit_message_1.java b/java/gerrit/PRED_commit_message_1.java
index 05bb4bb..eb996d6 100644
--- a/java/gerrit/PRED_commit_message_1.java
+++ b/java/gerrit/PRED_commit_message_1.java
@@ -21,6 +21,7 @@
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.SymbolTerm;
import com.googlecode.prolog_cafe.lang.Term;
+import org.eclipse.jgit.revwalk.RevCommit;
/**
* Returns the commit message as a symbol
@@ -40,7 +41,8 @@
engine.setB0();
Term a1 = arg1.dereference();
- String commitMessage = StoredValues.COMMIT_MESSAGE.get(engine);
+ RevCommit revCommit = StoredValues.COMMIT.get(engine);
+ String commitMessage = revCommit.getFullMessage();
SymbolTerm msg = SymbolTerm.create(commitMessage);
if (!a1.unify(msg, engine.trail)) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index db71ef6..d64a9d7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1022,9 +1022,19 @@
@Test
@TestProjectInput(cloneAs = "user")
- public void deleteChangeAsUserWithDeleteOwnChangesPermission() throws Exception {
+ public void deleteChangeAsUserWithDeleteOwnChangesPermissionForGroup() throws Exception {
allow("refs/*", Permission.DELETE_OWN_CHANGES, REGISTERED_USERS);
+ deleteChangeAsUser();
+ }
+ @Test
+ @TestProjectInput(cloneAs = "user")
+ public void deleteChangeAsUserWithDeleteOwnChangesPermissionForOwners() throws Exception {
+ allow("refs/*", Permission.DELETE_OWN_CHANGES, CHANGE_OWNER);
+ deleteChangeAsUser();
+ }
+
+ private void deleteChangeAsUser() throws Exception {
try {
PushOneCommit.Result changeResult =
pushFactory.create(db, user.getIdent(), testRepo).to("refs/for/master");
@@ -2703,9 +2713,7 @@
assertThat(commitPatchSetCreation.getShortMessage()).isEqualTo("Create patch set 2");
PersonIdent expectedAuthor =
- changeNoteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.id), c.updated, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id), c.updated, serverIdent.get());
assertThat(commitPatchSetCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitPatchSetCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.updated));
@@ -2713,10 +2721,7 @@
RevCommit commitChangeCreation = rw.parseCommit(commitPatchSetCreation.getParent(0));
assertThat(commitChangeCreation.getShortMessage()).isEqualTo("Create change");
- expectedAuthor =
- changeNoteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.id), c.created, serverIdent.get());
+ expectedAuthor = changeNoteUtil.newIdent(getAccount(admin.id), c.created, serverIdent.get());
assertThat(commitChangeCreation.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commitChangeCreation.getCommitterIdent())
.isEqualTo(new PersonIdent(serverIdent.get(), c.created));
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
index 62138ca..ce73875 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractSubmoduleSubscription.java
@@ -32,6 +32,7 @@
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
@@ -436,4 +437,22 @@
RevCommit c = rw.parseCommit(commitId);
assertThat(c.getFullMessage()).isEqualTo(expectedMessage);
}
+
+ protected void expectToHaveAuthor(
+ TestRepository<?> repo, String branch, String expectedAuthorName, String expectedAuthorEmail)
+ throws Exception {
+ ObjectId commitId =
+ repo.git()
+ .fetch()
+ .setRemote("origin")
+ .call()
+ .getAdvertisedRef("refs/heads/" + branch)
+ .getObjectId();
+
+ RevWalk rw = repo.getRevWalk();
+ RevCommit c = rw.parseCommit(commitId);
+ PersonIdent authorIdent = c.getAuthorIdent();
+ assertThat(authorIdent.getName()).isEqualTo(expectedAuthorName);
+ assertThat(authorIdent.getEmailAddress()).isEqualTo(expectedAuthorEmail);
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 1de9d29..45294fb 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -433,9 +433,7 @@
if (notesMigration.commitChangeWrites()) {
PersonIdent committer = serverIdent.get();
PersonIdent author =
- noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
+ noteUtil.newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
tr.branch(RefNames.changeMetaRef(c3.getId()))
.commit()
.author(author)
diff --git a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
index 81ee3a0..4381ed137 100644
--- a/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/SubmoduleSubscriptionsIT.java
@@ -20,6 +20,7 @@
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.testing.ConfigSuite;
import org.eclipse.jgit.junit.TestRepository;
@@ -520,6 +521,87 @@
testSubmoduleSubjectCommitMessageAndExpectTruncation();
}
+ @Test
+ public void superRepoCommitHasSameAuthorAsSubmoduleCommits() throws Exception {
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+
+ TestRepository<?> superRepo = createProjectWithPush("super-project");
+ TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+ TestRepository<?> subRepo2 = createProjectWithPush("subscribed-to-project-2");
+
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+
+ Config config = new Config();
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ pushSubmoduleConfig(superRepo, "master", config);
+
+ String topic = "foo";
+
+ subRepo.tick(1); // Make sure that both changes have different timestamps.
+ String changeId1 =
+ createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic)
+ .getChangeId();
+ approve(changeId1);
+
+ subRepo2.tick(2); // Make sure that both changes have different timestamps.
+ String changeId2 =
+ createChange(subRepo2, "refs/heads/master", "Change 2", "b.txt", "other content", topic)
+ .getChangeId();
+ approve(changeId2);
+
+ // Submit the topic, 2 changes with the same author.
+ gApi.changes().id(changeId1).current().submit();
+
+ // Expect that the author is preserved for the superRepo commit.
+ expectToHaveAuthor(superRepo, "master", admin.fullName, admin.email);
+ }
+
+ @Test
+ public void superRepoCommitHasGerritAsAuthorIfAuthorsOfSubmoduleCommitsDiffer() throws Exception {
+ assume().that(isSubmitWholeTopicEnabled()).isTrue();
+
+ TestRepository<?> superRepo = createProjectWithPush("super-project");
+ TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
+
+ TestRepository<?> subRepo2 = createProjectWithPush("subscribed-to-project-2");
+ subRepo2 = cloneProject(new Project.NameKey(name("subscribed-to-project-2")), user);
+
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project", "refs/heads/master", "super-project", "refs/heads/master");
+ allowMatchingSubmoduleSubscription(
+ "subscribed-to-project-2", "refs/heads/master", "super-project", "refs/heads/master");
+
+ Config config = new Config();
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project", "master");
+ prepareSubmoduleConfigEntry(config, "subscribed-to-project-2", "master");
+ pushSubmoduleConfig(superRepo, "master", config);
+
+ String topic = "foo";
+
+ // Create change as admin.
+ String changeId1 =
+ createChange(subRepo, "refs/heads/master", "Change 1", "a.txt", "some content", topic)
+ .getChangeId();
+ approve(changeId1);
+
+ // Create change as user.
+ PushOneCommit push =
+ pushFactory.create(db, user.getIdent(), subRepo2, "Change 2", "b.txt", "other content");
+ String changeId2 = push.to("refs/for/master/" + name(topic)).getChangeId();
+ approve(changeId2);
+
+ // Submit the topic, 2 changes with the different author.
+ gApi.changes().id(changeId1).current().submit();
+
+ // Expect that the Gerrit server identity is chosen as author for the superRepo commit.
+ expectToHaveAuthor(
+ superRepo, "master", serverIdent.get().getName(), serverIdent.get().getEmailAddress());
+ }
+
private void testSubmoduleSubjectCommitMessageAndExpectTruncation() throws Exception {
TestRepository<?> superRepo = createProjectWithPush("super-project");
TestRepository<?> subRepo = createProjectWithPush("subscribed-to-project");
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
index e6f61fa..eb80092 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ImpersonationIT.java
@@ -189,7 +189,6 @@
testVoteOnBehalfOfWithComment();
}
- @GerritConfig(name = "notedb.writeJson", value = "true")
@Test
public void voteOnBehalfOfWithCommentWritingJson() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
@@ -225,7 +224,6 @@
assertThat(c.getRealAuthor().getId()).isEqualTo(admin.id);
}
- @GerritConfig(name = "notedb.writeJson", value = "true")
@Test
public void voteOnBehalfOfWithRobotComment() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index c723082..09c7e0b 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -289,9 +289,7 @@
assertThat(commit.getShortMessage()).isEqualTo("Create change");
PersonIdent expectedAuthor =
- changeNoteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.id), c.created, serverIdent.get());
+ changeNoteUtil.newIdent(getAccount(admin.id), c.created, serverIdent.get());
assertThat(commit.getAuthorIdent()).isEqualTo(expectedAuthor);
assertThat(commit.getCommitterIdent())
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
index f7903dd..3534959 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/project/AccessIT.java
@@ -649,6 +649,34 @@
assertThat(permissions2.keySet()).containsExactly(Permission.READ);
}
+ @Test
+ public void addAccessSectionForInvalidRef() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ // 'refs/heads/stable_*' is invalid, correct would be '^refs/heads/stable_.*'
+ String invalidRef = Constants.R_HEADS + "stable_*";
+ accessInput.add.put(invalidRef, accessSectionInfo);
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("Invalid Name: " + invalidRef);
+ pApi().access(accessInput);
+ }
+
+ @Test
+ public void createAccessChangeWithAccessSectionForInvalidRef() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ // 'refs/heads/stable_*' is invalid, correct would be '^refs/heads/stable_.*'
+ String invalidRef = Constants.R_HEADS + "stable_*";
+ accessInput.add.put(invalidRef, accessSectionInfo);
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("Invalid Name: " + invalidRef);
+ pApi().accessChange(accessInput);
+ }
+
private ProjectApi pApi() throws Exception {
return gApi.projects().name(newProjectName.get());
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 5555185..211af59 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -950,8 +950,6 @@
@Test
public void jsonCommentHasLegacyFormatFalse() throws Exception {
assume().that(notesMigration.readChanges()).isTrue();
- assertThat(noteUtil.getChangeNoteJson().getWriteJson()).isTrue();
-
PushOneCommit.Result result = createChange();
Change.Id changeId = result.getChange().getId();
addComment(result.getChangeId(), "comment");
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index ea44bd7..29c043a 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -902,9 +902,7 @@
}
PersonIdent committer = serverIdent.get();
PersonIdent author =
- noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
+ noteUtil.newIdent(getAccount(admin.getId()), committer.getWhen(), committer);
serverSideTestRepo
.branch(RefNames.changeMetaRef(id))
.commit()
diff --git a/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java
deleted file mode 100644
index e029e7a..0000000
--- a/javatests/com/google/gerrit/acceptance/server/change/LegacyCommentsIT.java
+++ /dev/null
@@ -1,76 +0,0 @@
-// Copyright (C) 2017 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.change;
-
-import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.TruthJUnit.assume;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
-import com.google.gerrit.acceptance.PushOneCommit;
-import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Comment;
-import com.google.gerrit.server.notedb.ChangeNoteUtil;
-import com.google.gerrit.testing.ConfigSuite;
-import com.google.inject.Inject;
-import java.util.Collection;
-import org.eclipse.jgit.lib.Config;
-import org.junit.Before;
-import org.junit.Test;
-
-@NoHttpd
-public class LegacyCommentsIT extends AbstractDaemonTest {
- @Inject private ChangeNoteUtil noteUtil;
-
- @ConfigSuite.Default
- public static Config writeJsonFalseConfig() {
- Config c = new Config();
- c.setBoolean("noteDb", null, "writeJson", false);
- return c;
- }
-
- @Before
- public void setUp() {
- setApiUser(user);
- }
-
- @Test
- public void legacyCommentHasLegacyFormatTrue() throws Exception {
- assume().that(notesMigration.readChanges()).isTrue();
- assertThat(noteUtil.getChangeNoteJson().getWriteJson()).isFalse();
-
- PushOneCommit.Result result = createChange();
- Change.Id changeId = result.getChange().getId();
-
- CommentInput cin = new CommentInput();
- cin.message = "comment";
- cin.path = PushOneCommit.FILE_NAME;
-
- ReviewInput rin = new ReviewInput();
- rin.comments = ImmutableMap.of(cin.path, ImmutableList.of(cin));
- gApi.changes().id(changeId.get()).current().review(rin);
-
- Collection<Comment> comments =
- notesFactory.createChecked(db, project, changeId).getComments().values();
- assertThat(comments).hasSize(1);
- Comment comment = comments.iterator().next();
- assertThat(comment.message).isEqualTo("comment");
- assertThat(comment.legacyFormat).isTrue();
- }
-}
diff --git a/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java b/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
index a4d9acb..98dca3e 100644
--- a/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/rules/RulesIT.java
@@ -71,18 +71,17 @@
@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);
+ modifySubmitRules(
+ String.format(
+ "gerrit:commit_author(user(%d), '%s', '%s')",
+ user.getId().get(), user.fullName, user.email));
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
}
@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);
+ assertThat(statusForRule()).isEqualTo(SubmitRecord.Status.OK);
}
private SubmitRecord.Status statusForRule() throws Exception {
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 3113a8a..f405c28 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -30,6 +30,7 @@
visibility = ["//visibility:public"],
runtime_deps = [
"//lib/bouncycastle:bcprov",
+ "//prolog:gerrit-prolog-common",
],
deps = [
":custom-truth-subjects",
diff --git a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
index c677be5..cc7302f 100644
--- a/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java
@@ -79,24 +79,10 @@
@Ignore
@RunWith(ConfigSuite.class)
public abstract class AbstractChangeNotesTest extends GerritBaseTests {
- @ConfigSuite.Default
- public static Config changeNotesLegacy() {
- Config cfg = new Config();
- cfg.setBoolean("notedb", null, "writeJson", false);
- return cfg;
- }
-
- @ConfigSuite.Config
- public static Config changeNotesJson() {
- Config cfg = new Config();
- cfg.setBoolean("notedb", null, "writeJson", true);
- return cfg;
- }
+ private static final TimeZone TZ = TimeZone.getTimeZone("America/Los_Angeles");
@ConfigSuite.Parameter public Config testConfig;
- private static final TimeZone TZ = TimeZone.getTimeZone("America/Los_Angeles");
-
protected Account.Id otherUserId;
protected FakeAccountCache accountCache;
protected IdentifiedUser changeOwner;
@@ -172,7 +158,6 @@
bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED);
bind(MetricMaker.class).to(DisabledMetricMaker.class);
bind(ReviewDb.class).toProvider(Providers.<ReviewDb>of(null));
-
MutableNotesMigration migration = MutableNotesMigration.newDisabled();
migration.setFrom(NotesMigrationState.FINAL);
bind(MutableNotesMigration.class).toInstance(migration);
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
index de964d8..313fa07 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesParserTest.java
@@ -498,11 +498,7 @@
private RevCommit writeCommit(String body) throws Exception {
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
- body,
- noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
- false);
+ body, noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent), false);
}
private RevCommit writeCommit(String body, PersonIdent author) throws Exception {
@@ -513,9 +509,7 @@
ChangeNoteUtil noteUtil = injector.getInstance(ChangeNoteUtil.class);
return writeCommit(
body,
- noteUtil
- .getLegacyChangeNoteWrite()
- .newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
+ noteUtil.newIdent(changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent),
initWorkInProgress);
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 74ba0c2..bea61ed 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -58,17 +58,14 @@
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.sql.Timestamp;
-import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.notes.Note;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -79,7 +76,6 @@
@Inject private ChangeNoteJson changeNoteJson;
@Inject private LegacyChangeNoteRead legacyChangeNoteRead;
- @Inject private ChangeNoteUtil noteUtil;
@Inject private @GerritServerId String serverId;
@@ -999,7 +995,7 @@
update.commit();
try {
- notes = newNotes(c);
+ newNotes(c);
fail("Expected IOException");
} catch (OrmException e) {
assertCause(
@@ -1149,10 +1145,8 @@
update.commit();
ChangeNotes notes = newNotes(c);
- String note = readNote(notes, commit);
- if (!testJson()) {
- assertThat(note).isEqualTo(pushCert);
- }
+ readNote(notes, commit);
+
Map<PatchSet.Id, PatchSet> patchSets = notes.getPatchSets();
assertThat(patchSets.get(psId1).getPushCertificate()).isNull();
assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert);
@@ -1185,27 +1179,6 @@
assertThat(patchSets.get(psId1).getPushCertificate()).isNull();
assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert);
assertThat(notes.getComments()).isNotEmpty();
-
- if (!testJson()) {
- assertThat(readNote(notes, commit))
- .isEqualTo(
- pushCert
- + "Revision: "
- + commit.name()
- + "\n"
- + "Patch-set: 2\n"
- + "File: a.txt\n"
- + "\n"
- + "1:2-3:4\n"
- + NoteDbUtil.formatTime(serverIdent, ts)
- + "\n"
- + "Author: Change Owner <1@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 7\n"
- + "Comment\n"
- + "\n");
- }
}
@Test
@@ -1591,307 +1564,6 @@
}
@Test
- public void patchLineCommentNotesFormatSide1() throws Exception {
- Change c = newChange();
- ChangeUpdate update = newUpdate(c, otherUser);
- String uuid1 = "uuid1";
- String uuid2 = "uuid2";
- String uuid3 = "uuid3";
- String message1 = "comment 1";
- String message2 = "comment 2";
- String message3 = "comment 3";
- CommentRange range1 = new CommentRange(1, 1, 2, 1);
- Timestamp time1 = TimeUtil.nowTs();
- Timestamp time2 = TimeUtil.nowTs();
- Timestamp time3 = TimeUtil.nowTs();
- PatchSet.Id psId = c.currentPatchSetId();
-
- Comment comment1 =
- newComment(
- psId,
- "file1",
- uuid1,
- range1,
- range1.getEndLine(),
- otherUser,
- null,
- time1,
- message1,
- (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
- update.commit();
-
- update = newUpdate(c, otherUser);
- CommentRange range2 = new CommentRange(2, 1, 3, 1);
- Comment comment2 =
- newComment(
- psId,
- "file1",
- uuid2,
- range2,
- range2.getEndLine(),
- otherUser,
- null,
- time2,
- message2,
- (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment2);
- update.commit();
-
- update = newUpdate(c, otherUser);
- CommentRange range3 = new CommentRange(3, 0, 4, 1);
- Comment comment3 =
- newComment(
- psId,
- "file2",
- uuid3,
- range3,
- range3.getEndLine(),
- otherUser,
- null,
- time3,
- message3,
- (short) 1,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment3);
- update.commit();
-
- ChangeNotes notes = newNotes(c);
-
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time1)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 9\n"
- + "comment 1\n"
- + "\n"
- + "2:1-3:1\n"
- + NoteDbUtil.formatTime(serverIdent, time2)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid2\n"
- + "Bytes: 9\n"
- + "comment 2\n"
- + "\n"
- + "File: file2\n"
- + "\n"
- + "3:0-4:1\n"
- + NoteDbUtil.formatTime(serverIdent, time3)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid3\n"
- + "Bytes: 9\n"
- + "comment 3\n"
- + "\n");
- }
- }
- }
-
- @Test
- public void patchLineCommentNotesFormatSide0() throws Exception {
- Change c = newChange();
- ChangeUpdate update = newUpdate(c, otherUser);
- String uuid1 = "uuid1";
- String uuid2 = "uuid2";
- String message1 = "comment 1";
- String message2 = "comment 2";
- CommentRange range1 = new CommentRange(1, 1, 2, 1);
- Timestamp time1 = TimeUtil.nowTs();
- Timestamp time2 = TimeUtil.nowTs();
- PatchSet.Id psId = c.currentPatchSetId();
-
- Comment comment1 =
- newComment(
- psId,
- "file1",
- uuid1,
- range1,
- range1.getEndLine(),
- otherUser,
- null,
- time1,
- message1,
- (short) 0,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
- update.commit();
-
- update = newUpdate(c, otherUser);
- CommentRange range2 = new CommentRange(2, 1, 3, 1);
- Comment comment2 =
- newComment(
- psId,
- "file1",
- uuid2,
- range2,
- range2.getEndLine(),
- otherUser,
- null,
- time2,
- message2,
- (short) 0,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment2);
- update.commit();
-
- ChangeNotes notes = newNotes(c);
-
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Base-for-patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time1)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 9\n"
- + "comment 1\n"
- + "\n"
- + "2:1-3:1\n"
- + NoteDbUtil.formatTime(serverIdent, time2)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid2\n"
- + "Bytes: 9\n"
- + "comment 2\n"
- + "\n");
- }
- }
- }
-
- @Test
- public void patchLineCommentNotesResolvedChangesValue() throws Exception {
- Change c = newChange();
- ChangeUpdate update = newUpdate(c, otherUser);
- String uuid1 = "uuid1";
- String uuid2 = "uuid2";
- String message1 = "comment 1";
- String message2 = "comment 2";
- CommentRange range1 = new CommentRange(1, 1, 2, 1);
- Timestamp time1 = TimeUtil.nowTs();
- Timestamp time2 = TimeUtil.nowTs();
- PatchSet.Id psId = c.currentPatchSetId();
-
- Comment comment1 =
- newComment(
- psId,
- "file1",
- uuid1,
- range1,
- range1.getEndLine(),
- otherUser,
- null,
- time1,
- message1,
- (short) 0,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- false);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment1);
- update.commit();
-
- update = newUpdate(c, otherUser);
- Comment comment2 =
- newComment(
- psId,
- "file1",
- uuid2,
- range1,
- range1.getEndLine(),
- otherUser,
- uuid1,
- time2,
- message2,
- (short) 0,
- "abcd1234abcd1234abcd1234abcd1234abcd1234",
- true);
- update.setPatchSetId(psId);
- update.putComment(Status.PUBLISHED, comment2);
- update.commit();
-
- ChangeNotes notes = newNotes(c);
-
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Base-for-patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time1)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 9\n"
- + "comment 1\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time2)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Parent: uuid1\n"
- + "Unresolved: true\n"
- + "UUID: uuid2\n"
- + "Bytes: 9\n"
- + "comment 2\n"
- + "\n");
- }
- }
- }
-
- @Test
public void patchLineCommentNotesFormatMultiplePatchSetsSameRevId() throws Exception {
Change c = newChange();
PatchSet.Id psId1 = c.currentPatchSetId();
@@ -1959,54 +1631,6 @@
update.commit();
ChangeNotes notes = newNotes(c);
-
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
- String timeStr = NoteDbUtil.formatTime(serverIdent, time);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Base-for-patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + timeStr
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid1\n"
- + "Bytes: 9\n"
- + "comment 1\n"
- + "\n"
- + "2:1-3:1\n"
- + timeStr
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid2\n"
- + "Bytes: 9\n"
- + "comment 2\n"
- + "\n"
- + "Base-for-patch-set: 2\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + timeStr
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid3\n"
- + "Bytes: 9\n"
- + "comment 3\n"
- + "\n");
- }
- }
assertThat(notes.getComments())
.isEqualTo(
ImmutableListMultimap.of(
@@ -2048,32 +1672,6 @@
ChangeNotes notes = newNotes(c);
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Patch-set: 1\n"
- + "File: file\n"
- + "\n"
- + "1:1-2:1\n"
- + NoteDbUtil.formatTime(serverIdent, time)
- + "\n"
- + "Author: Other Account <2@gerrit>\n"
- + "Real-author: Change Owner <1@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid\n"
- + "Bytes: 7\n"
- + "comment\n"
- + "\n");
- }
- }
assertThat(notes.getComments()).isEqualTo(ImmutableListMultimap.of(revId, comment));
}
@@ -2112,32 +1710,6 @@
ChangeNotes notes = newNotes(c);
- try (RevWalk walk = new RevWalk(repo)) {
- ArrayList<Note> notesInTree = Lists.newArrayList(notes.revisionNoteMap.noteMap.iterator());
- Note note = Iterables.getOnlyElement(notesInTree);
-
- byte[] bytes = walk.getObjectReader().open(note.getData(), Constants.OBJ_BLOB).getBytes();
- String noteString = new String(bytes, UTF_8);
- String timeStr = NoteDbUtil.formatTime(serverIdent, time);
-
- if (!testJson()) {
- assertThat(noteString)
- .isEqualTo(
- "Revision: abcd1234abcd1234abcd1234abcd1234abcd1234\n"
- + "Patch-set: 1\n"
- + "File: file1\n"
- + "\n"
- + "1:1-2:1\n"
- + timeStr
- + "\n"
- + "Author: Weird\u0002User <3@gerrit>\n"
- + "Unresolved: false\n"
- + "UUID: uuid\n"
- + "Bytes: 7\n"
- + "comment\n"
- + "\n");
- }
- }
assertThat(notes.getComments())
.isEqualTo(ImmutableListMultimap.of(new RevId(comment.revId), comment));
}
@@ -2604,7 +2176,6 @@
assertThat(notes.getDraftCommentNotes().getNoteMap().contains(objId)).isTrue();
update = newUpdate(c, otherUser);
- now = TimeUtil.nowTs();
update.setPatchSetId(psId);
update.deleteComment(comment);
update.commit();
@@ -2674,7 +2245,6 @@
assertThat(notes.getDraftComments(otherUserId)).hasSize(2);
update = newUpdate(c, otherUser);
- now = TimeUtil.nowTs();
update.setPatchSetId(ps2);
update.deleteComment(comment2);
update.commit();
@@ -3525,10 +3095,6 @@
update.commit();
}
- private boolean testJson() {
- return noteUtil.getChangeNoteJson().getWriteJson();
- }
-
private String readNote(ChangeNotes notes, ObjectId noteId) throws Exception {
ObjectId dataId = notes.revisionNoteMap.noteMap.getNote(noteId).getData();
return new String(rw.getObjectReader().open(dataId, OBJ_BLOB).getCachedBytes(), UTF_8);
diff --git a/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
new file mode 100644
index 0000000..a5c4e49
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/CommentJsonMigratorTest.java
@@ -0,0 +1,535 @@
+// Copyright (C) 2017 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.notedb;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableListMultimap;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ListMultimap;
+import com.google.common.collect.Multimaps;
+import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Comment;
+import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.server.CommentsUtil;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.update.RefUpdateUtil;
+import com.google.gerrit.testing.TestChanges;
+import com.google.inject.Inject;
+import java.io.ByteArrayOutputStream;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevSort;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.junit.Before;
+import org.junit.Test;
+
+public class CommentJsonMigratorTest extends AbstractChangeNotesTest {
+ private CommentJsonMigrator migrator;
+ @Inject private ChangeNoteUtil noteUtil;
+ @Inject private CommentsUtil commentsUtil;
+ @Inject private LegacyChangeNoteWrite legacyChangeNoteWrite;
+
+ private AtomicInteger uuidCounter;
+
+ @Before
+ public void setUpCounter() {
+ uuidCounter = new AtomicInteger();
+ migrator = new CommentJsonMigrator(new ChangeNoteJson(), "gerrit");
+ }
+
+ @Test
+ public void noOpIfAllCommentsAreJson() throws Exception {
+ Change c = newChange();
+ incrementPatchSet(c);
+
+ ChangeNotes notes = newNotes(c);
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ Comment ps1Comment = newComment(notes, 1, "comment on ps1");
+ update.putComment(Status.PUBLISHED, ps1Comment);
+ update.commit();
+
+ notes = newNotes(c);
+ update = newUpdate(c, changeOwner);
+ Comment ps2Comment = newComment(notes, 2, "comment on ps2");
+ update.putComment(Status.PUBLISHED, ps2Comment);
+ update.commit();
+
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment.toString(),
+ getRevId(notes, 2), ps2Comment.toString());
+
+ ChangeNotes oldNotes = notes;
+ migrate(project, migrator::migrateChanges, 0);
+ assertNoDifferences(notes, oldNotes);
+ assertThat(notes.getMetaId()).isEqualTo(oldNotes.getMetaId());
+ }
+
+ @Test
+ public void migratePublishedComments() throws Exception {
+ Change c = newChange();
+ incrementPatchSet(c);
+
+ ChangeNotes notes = newNotes(c);
+
+ Comment ps1Comment1 = newComment(notes, 1, "first comment on ps1");
+ Comment ps2Comment1 = newComment(notes, 2, "first comment on ps2");
+ Comment ps1Comment2 = newComment(notes, 1, "second comment on ps1");
+
+ // Construct legacy format 'by hand'.
+ ByteArrayOutputStream out1 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(1, ps1Comment1).build(), out1);
+
+ ByteArrayOutputStream out2 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(2, ps2Comment1).build(), out2);
+
+ ByteArrayOutputStream out3 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder()
+ .put(1, ps1Comment2)
+ .put(1, ps1Comment1)
+ .build(),
+ out3);
+
+ TestRepository<Repository> testRepository = new TestRepository<>(repo, rw);
+
+ String metaRefName = RefNames.changeMetaRef(c.getId());
+ testRepository
+ .branch(metaRefName)
+ .commit()
+ .message("Review ps 1\n\nPatch-set: 1")
+ .add(ps1Comment1.revId, out1.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ testRepository
+ .branch(metaRefName)
+ .commit()
+ .message("Review ps 2\n\nPatch-set: 2")
+ .add(ps2Comment1.revId, out2.toString())
+ .add(ps1Comment1.revId, out3.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment1.toString(),
+ getRevId(notes, 1), ps1Comment2.toString(),
+ getRevId(notes, 2), ps2Comment1.toString());
+
+ // Comments at each commit all have legacy format.
+ ImmutableList<RevCommit> oldLog = log(project, RefNames.changeMetaRef(c.getId()));
+ assertThat(oldLog).hasSize(4);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(0))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(1))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(2)))
+ .containsExactly(ps1Comment1.key, true);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(3)))
+ .containsExactly(ps1Comment1.key, true, ps1Comment2.key, true, ps2Comment1.key, true);
+
+ ChangeNotes oldNotes = notes;
+ migrate(project, migrator::migrateChanges, 1);
+
+ // Comment content is the same.
+ notes = newNotes(c);
+ assertNoDifferences(notes, oldNotes);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment1.toString(),
+ getRevId(notes, 1), ps1Comment2.toString(),
+ getRevId(notes, 2), ps2Comment1.toString());
+
+ // Comments at each commit all have JSON format.
+ ImmutableList<RevCommit> newLog = log(project, RefNames.changeMetaRef(c.getId()));
+ assertLogEqualExceptTrees(newLog, oldLog);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(0))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(1))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(2)))
+ .containsExactly(ps1Comment1.key, false);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(3)))
+ .containsExactly(ps1Comment1.key, false, ps1Comment2.key, false, ps2Comment1.key, false);
+ }
+
+ @Test
+ public void migrateDraftComments() throws Exception {
+ Change c = newChange();
+ incrementPatchSet(c);
+
+ ChangeNotes notes = newNotes(c);
+ ObjectId origMetaId = notes.getMetaId();
+
+ Comment ownerCommentPs1 = newComment(notes, 1, "owner comment on ps1", changeOwner);
+ Comment ownerCommentPs2 = newComment(notes, 2, "owner comment on ps2", changeOwner);
+ Comment otherCommentPs1 = newComment(notes, 1, "other user comment on ps1", otherUser);
+
+ ByteArrayOutputStream out1 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(1, ownerCommentPs1).build(), out1);
+
+ ByteArrayOutputStream out2 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(2, ownerCommentPs2).build(), out2);
+
+ ByteArrayOutputStream out3 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(1, otherCommentPs1).build(), out3);
+
+ try (Repository allUsersRepo = repoManager.openRepository(allUsers);
+ RevWalk allUsersRw = new RevWalk(allUsersRepo)) {
+ TestRepository<Repository> testRepository = new TestRepository<>(allUsersRepo, allUsersRw);
+
+ testRepository
+ .branch(RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()))
+ .commit()
+ .message("Review ps 1\n\nPatch-set: 1")
+ .add(ownerCommentPs1.revId, out1.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ testRepository
+ .branch(RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()))
+ .commit()
+ .message("Review ps 1\n\nPatch-set: 2")
+ .add(ownerCommentPs2.revId, out2.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ testRepository
+ .branch(RefNames.refsDraftComments(c.getId(), otherUser.getAccountId()))
+ .commit()
+ .message("Review ps 2\n\nPatch-set: 2")
+ .add(otherCommentPs1.revId, out3.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+ }
+
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getDraftComments(changeOwner.getAccountId())))
+ .containsExactly(
+ getRevId(notes, 1), ownerCommentPs1.toString(),
+ getRevId(notes, 2), ownerCommentPs2.toString());
+ assertThat(getToStringRepresentations(notes.getDraftComments(otherUser.getAccountId())))
+ .containsExactly(getRevId(notes, 1), otherCommentPs1.toString());
+
+ // Comments at each commit all have legacy format.
+ ImmutableList<RevCommit> oldOwnerLog =
+ log(allUsers, RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()));
+ assertThat(oldOwnerLog).hasSize(2);
+ assertThat(getLegacyFormatMapForDraftComments(notes, oldOwnerLog.get(0)))
+ .containsExactly(ownerCommentPs1.key, true);
+ assertThat(getLegacyFormatMapForDraftComments(notes, oldOwnerLog.get(1)))
+ .containsExactly(ownerCommentPs1.key, true, ownerCommentPs2.key, true);
+
+ ImmutableList<RevCommit> oldOtherLog =
+ log(allUsers, RefNames.refsDraftComments(c.getId(), otherUser.getAccountId()));
+ assertThat(oldOtherLog).hasSize(1);
+ assertThat(getLegacyFormatMapForDraftComments(notes, oldOtherLog.get(0)))
+ .containsExactly(otherCommentPs1.key, true);
+
+ ChangeNotes oldNotes = notes;
+ migrate(allUsers, migrator::migrateDrafts, 2);
+ assertNoDifferences(notes, oldNotes);
+
+ // Migration doesn't touch change ref.
+ assertThat(repo.exactRef(RefNames.changeMetaRef(c.getId())).getObjectId())
+ .isEqualTo(origMetaId);
+
+ // Comment content is the same.
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getDraftComments(changeOwner.getAccountId())))
+ .containsExactly(
+ getRevId(notes, 1), ownerCommentPs1.toString(),
+ getRevId(notes, 2), ownerCommentPs2.toString());
+ assertThat(getToStringRepresentations(notes.getDraftComments(otherUser.getAccountId())))
+ .containsExactly(getRevId(notes, 1), otherCommentPs1.toString());
+
+ // Comments at each commit all have JSON format.
+ ImmutableList<RevCommit> newOwnerLog =
+ log(allUsers, RefNames.refsDraftComments(c.getId(), changeOwner.getAccountId()));
+ assertLogEqualExceptTrees(newOwnerLog, oldOwnerLog);
+ assertThat(getLegacyFormatMapForDraftComments(notes, newOwnerLog.get(0)))
+ .containsExactly(ownerCommentPs1.key, false);
+ assertThat(getLegacyFormatMapForDraftComments(notes, newOwnerLog.get(1)))
+ .containsExactly(ownerCommentPs1.key, false, ownerCommentPs2.key, false);
+
+ ImmutableList<RevCommit> newOtherLog =
+ log(allUsers, RefNames.refsDraftComments(c.getId(), otherUser.getAccountId()));
+ assertLogEqualExceptTrees(newOtherLog, oldOtherLog);
+ assertThat(getLegacyFormatMapForDraftComments(notes, newOtherLog.get(0)))
+ .containsExactly(otherCommentPs1.key, false);
+ }
+
+ @Test
+ public void migrateMixOfJsonAndLegacyComments() throws Exception {
+ // 3 comments: legacy, JSON, legacy. Because adding a comment necessarily rewrites the entire
+ // note, these comments need to be on separate patch sets.
+ Change c = newChange();
+ incrementPatchSet(c);
+ incrementPatchSet(c);
+
+ ChangeNotes notes = newNotes(c);
+
+ Comment ps1Comment = newComment(notes, 1, "comment on ps1 (legacy)");
+
+ ByteArrayOutputStream out1 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(1, ps1Comment).build(), out1);
+
+ TestRepository<Repository> testRepository = new TestRepository<>(repo, rw);
+
+ String metaRefName = RefNames.changeMetaRef(c.getId());
+ testRepository
+ .branch(metaRefName)
+ .commit()
+ .message("Review ps 1\n\nPatch-set: 1")
+ .add(ps1Comment.revId, out1.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ notes = newNotes(c);
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ Comment ps2Comment = newComment(notes, 2, "comment on ps2 (JSON)");
+ update.putComment(Status.PUBLISHED, ps2Comment);
+ update.commit();
+
+ Comment ps3Comment = newComment(notes, 3, "comment on ps3 (legacy)");
+ ByteArrayOutputStream out3 = new ByteArrayOutputStream(0);
+ legacyChangeNoteWrite.buildNote(
+ ImmutableListMultimap.<Integer, Comment>builder().put(3, ps3Comment).build(), out3);
+
+ testRepository
+ .branch(metaRefName)
+ .commit()
+ .message("Review ps 3\n\nPatch-set: 3")
+ .add(ps3Comment.revId, out3.toString())
+ .author(serverIdent)
+ .committer(serverIdent)
+ .create();
+
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment.toString(),
+ getRevId(notes, 2), ps2Comment.toString(),
+ getRevId(notes, 3), ps3Comment.toString());
+
+ // Comments at each commit match expected format.
+ ImmutableList<RevCommit> oldLog = log(project, RefNames.changeMetaRef(c.getId()));
+ assertThat(oldLog).hasSize(6);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(0))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(1))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(2))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(3)))
+ .containsExactly(ps1Comment.key, true);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(4)))
+ .containsExactly(ps1Comment.key, true, ps2Comment.key, false);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, oldLog.get(5)))
+ .containsExactly(ps1Comment.key, true, ps2Comment.key, false, ps3Comment.key, true);
+
+ ChangeNotes oldNotes = notes;
+ migrate(project, migrator::migrateChanges, 1);
+ assertNoDifferences(notes, oldNotes);
+
+ // Comment content is the same.
+ notes = newNotes(c);
+ assertThat(getToStringRepresentations(notes.getComments()))
+ .containsExactly(
+ getRevId(notes, 1), ps1Comment.toString(),
+ getRevId(notes, 2), ps2Comment.toString(),
+ getRevId(notes, 3), ps3Comment.toString());
+
+ // Comments at each commit all have JSON format.
+ ImmutableList<RevCommit> newLog = log(project, RefNames.changeMetaRef(c.getId()));
+ assertLogEqualExceptTrees(newLog, oldLog);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(0))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(1))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(2))).isEmpty();
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(3)))
+ .containsExactly(ps1Comment.key, false);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(4)))
+ .containsExactly(ps1Comment.key, false, ps2Comment.key, false);
+ assertThat(getLegacyFormatMapForPublishedComments(notes, newLog.get(5)))
+ .containsExactly(ps1Comment.key, false, ps2Comment.key, false, ps3Comment.key, false);
+ }
+
+ @FunctionalInterface
+ interface MigrateFunction {
+ boolean call(
+ Project.NameKey project,
+ Repository repo,
+ RevWalk rw,
+ ObjectInserter ins,
+ BatchRefUpdate bru)
+ throws Exception;
+ }
+
+ private void migrate(Project.NameKey project, MigrateFunction func, int expectedCommands)
+ throws Exception {
+ try (Repository repo = repoManager.openRepository(project);
+ RevWalk rw = new RevWalk(repo);
+ ObjectInserter ins = repo.newObjectInserter()) {
+ BatchRefUpdate bru = repo.getRefDatabase().newBatchUpdate();
+ bru.setAllowNonFastForwards(true);
+ assertThat(func.call(project, repo, rw, ins, bru)).isTrue();
+ assertThat(bru.getCommands()).hasSize(expectedCommands);
+ if (!bru.getCommands().isEmpty()) {
+ ins.flush();
+ RefUpdateUtil.executeChecked(bru, rw);
+ }
+ }
+ }
+
+ private Comment newComment(ChangeNotes notes, int psNum, String message) {
+ return newComment(notes, psNum, message, changeOwner);
+ }
+
+ private Comment newComment(
+ ChangeNotes notes, int psNum, String message, IdentifiedUser commenter) {
+ return newComment(
+ new PatchSet.Id(notes.getChangeId(), psNum),
+ "filename",
+ "uuid-" + uuidCounter.getAndIncrement(),
+ null,
+ 0,
+ commenter,
+ null,
+ TimeUtil.nowTs(),
+ message,
+ (short) 1,
+ getRevId(notes, psNum).get(),
+ false);
+ }
+
+ private void incrementPatchSet(Change c) throws Exception {
+ TestChanges.incrementPatchSet(c);
+ RevCommit commit = tr.commit().message("PS" + c.currentPatchSetId().get()).create();
+ ChangeUpdate update = newUpdate(c, changeOwner);
+ update.setCommit(rw, commit);
+ update.commit();
+ }
+
+ private static RevId getRevId(ChangeNotes notes, int psNum) {
+ PatchSet.Id psId = new PatchSet.Id(notes.getChangeId(), psNum);
+ PatchSet ps = notes.getPatchSets().get(psId);
+ checkArgument(ps != null, "no patch set %s: %s", psNum, notes.getPatchSets());
+ return ps.getRevision();
+ }
+
+ private static ListMultimap<RevId, String> getToStringRepresentations(
+ ListMultimap<RevId, Comment> comments) {
+ // Use string representation for equality comparison in this test, because Comment#equals only
+ // compares keys.
+ return Multimaps.transformValues(comments, Comment::toString);
+ }
+
+ private ImmutableMap<Comment.Key, Boolean> getLegacyFormatMapForPublishedComments(
+ ChangeNotes notes, ObjectId metaId) throws Exception {
+ return getLegacyFormatMap(project, notes.getChangeId(), metaId, Status.PUBLISHED);
+ }
+
+ private ImmutableMap<Comment.Key, Boolean> getLegacyFormatMapForDraftComments(
+ ChangeNotes notes, ObjectId metaId) throws Exception {
+ return getLegacyFormatMap(allUsers, notes.getChangeId(), metaId, Status.DRAFT);
+ }
+
+ private ImmutableMap<Comment.Key, Boolean> getLegacyFormatMap(
+ Project.NameKey project, Change.Id changeId, ObjectId metaId, Status status)
+ throws Exception {
+ try (Repository repo = repoManager.openRepository(project);
+ ObjectReader reader = repo.newObjectReader();
+ RevWalk rw = new RevWalk(reader)) {
+ NoteMap noteMap = NoteMap.read(reader, rw.parseCommit(metaId));
+ RevisionNoteMap<ChangeRevisionNote> revNoteMap =
+ RevisionNoteMap.parse(
+ noteUtil.getChangeNoteJson(),
+ noteUtil.getLegacyChangeNoteRead(),
+ changeId,
+ reader,
+ noteMap,
+ status);
+ return revNoteMap
+ .revisionNotes
+ .values()
+ .stream()
+ .flatMap(crn -> crn.getComments().stream())
+ .collect(toImmutableMap(c -> c.key, c -> c.legacyFormat));
+ }
+ }
+
+ private ImmutableList<RevCommit> log(Project.NameKey project, String refName) throws Exception {
+ try (Repository repo = repoManager.openRepository(project);
+ RevWalk rw = new RevWalk(repo)) {
+ rw.sort(RevSort.TOPO);
+ rw.sort(RevSort.REVERSE);
+ Ref ref = repo.exactRef(refName);
+ checkArgument(ref != null, "missing ref: %s", refName);
+ rw.markStart(rw.parseCommit(ref.getObjectId()));
+ return ImmutableList.copyOf(rw);
+ }
+ }
+
+ private static void assertLogEqualExceptTrees(
+ ImmutableList<RevCommit> actualLog, ImmutableList<RevCommit> expectedLog) {
+ assertThat(actualLog).hasSize(expectedLog.size());
+ for (int i = 0; i < expectedLog.size(); i++) {
+ RevCommit actual = actualLog.get(i);
+ RevCommit expected = expectedLog.get(i);
+ assertThat(actual.getAuthorIdent())
+ .named("author of entry %s", i)
+ .isEqualTo(expected.getAuthorIdent());
+ assertThat(actual.getCommitterIdent())
+ .named("committer of entry %s", i)
+ .isEqualTo(expected.getCommitterIdent());
+ assertThat(actual.getFullMessage()).named("message of entry %s", i).isNotNull();
+ assertThat(actual.getFullMessage())
+ .named("message of entry %s", i)
+ .isEqualTo(expected.getFullMessage());
+ }
+ }
+
+ private void assertNoDifferences(ChangeNotes actual, ChangeNotes expected) throws Exception {
+ assertThat(
+ ChangeBundle.fromNotes(commentsUtil, actual)
+ .differencesFrom(ChangeBundle.fromNotes(commentsUtil, expected)))
+ .isEmpty();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
index 314941e..086dd65 100644
--- a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
+++ b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
@@ -49,7 +49,7 @@
bind(PrologEnvironment.Args.class)
.toInstance(
new PrologEnvironment.Args(
- null, null, null, null, null, null, null, cfg, null));
+ null, null, null, null, null, null, null, cfg, null, null));
}
});
}
diff --git a/javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java b/javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java
index f709f55..8622b32 100644
--- a/javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java
+++ b/javatests/com/google/gerrit/server/rules/PrologRuleEvaluatorTest.java
@@ -30,18 +30,18 @@
@Test
public void labelWithSpacesIsTransformed() {
assertThat(PrologRuleEvaluator.checkLabelName("Label with spaces"))
- .isEqualTo("Invalid-Prolog-Rules-Label-Name--Labelwithspaces");
+ .isEqualTo("Invalid-Prolog-Rules-Label-Name-Labelwithspaces");
}
@Test
public void labelStartingWithADashIsTransformed() {
assertThat(PrologRuleEvaluator.checkLabelName("-dashed-label"))
- .isEqualTo("Invalid-Prolog-Rules-Label-Name---dashed-label");
+ .isEqualTo("Invalid-Prolog-Rules-Label-Name-dashed-label");
}
@Test
public void labelWithInvalidCharactersIsTransformed() {
assertThat(PrologRuleEvaluator.checkLabelName("*urgent*"))
- .isEqualTo("Invalid-Prolog-Rules-Label-Name--urgent");
+ .isEqualTo("Invalid-Prolog-Rules-Label-Name-urgent");
}
}
diff --git a/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java b/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java
index ed94c97..c4844b1 100644
--- a/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java
+++ b/javatests/com/google/gerrit/server/schema/SchemaUpdaterTest.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.config.AnonymousCowardNameProvider;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.GerritServerId;
+import com.google.gerrit.server.config.GerritServerIdProvider;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.group.SystemGroupBackend;
@@ -94,7 +95,11 @@
Config cfg = new Config();
cfg.setString("user", null, "name", "Gerrit Code Review");
cfg.setString("user", null, "email", "gerrit@localhost");
-
+ cfg.setString(
+ GerritServerIdProvider.SECTION,
+ null,
+ GerritServerIdProvider.KEY,
+ "1234567");
bind(Config.class) //
.annotatedWith(GerritServerConfig.class) //
.toInstance(cfg);
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl
index f4497d3..adf2d4f8 100644
--- a/lib/jgit/jgit.bzl
+++ b/lib/jgit/jgit.bzl
@@ -1,8 +1,8 @@
load("//tools/bzl:maven_jar.bzl", "GERRIT", "MAVEN_LOCAL", "MAVEN_CENTRAL", "maven_jar")
-_JGIT_VERS = "5.0.0.201806131550-r"
+_JGIT_VERS = "5.0.1.201806211838-r"
-_DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot
+_DOC_VERS = "5.0.0.201806131550-r" # Set to _JGIT_VERS unless using a snapshot
JGIT_DOC_URL = "http://download.eclipse.org/jgit/site/" + _DOC_VERS + "/apidocs"
@@ -26,28 +26,28 @@
name = "jgit-lib",
artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "596edbf705924bd2defd9cfc83b29b1bceb56308",
- src_sha1 = "503a4c069baa672d3ff323d36c9b9a3a5edffc94",
+ sha1 = "dbba66a425d2153ccd749d0ba9c075b0ba424655",
+ src_sha1 = "c85725a96e20d940fe20e1be4ddf50133c322f65",
unsign = True,
)
maven_jar(
name = "jgit-servlet",
artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "be2b42633f4973921e4c4b976f592f12f33bffd9",
+ sha1 = "5d9cd43e880d49f14501ac48d59b55905f4ec5bf",
unsign = True,
)
maven_jar(
name = "jgit-archive",
artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "3948643a6e07375ed0e28f35d75c0deb1cd183d8",
+ sha1 = "1d94e2bfa505dd719f62cfb036295022543af17e",
)
maven_jar(
name = "jgit-junit",
artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "d57d749ad97f42d570236e7981f36458033bfda9",
+ sha1 = "f848735061fab81f2863f68cca8d533ff403c765",
unsign = True,
)
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html
index febd446..5d45811 100644
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.html
@@ -111,13 +111,13 @@
</a>
<gr-select
id="force"
- class$="[[_computeForceClass(permission)]]"
+ class$="[[_computeForceClass(permission, rule.value.action)]]"
bind-value="{{rule.value.force}}"
on-change="_handleValueChange">
<select disabled$="[[!editing]]">
<template
is="dom-repeat"
- items="[[_computeForceOptions(permission)]]">
+ items="[[_computeForceOptions(permission, rule.value.action)]]">
<option value="[[item.value]]">[[item.name]]</option>
</template>
</select>
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js
index 4af4952..b99125c 100644
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor.js
@@ -33,22 +33,24 @@
'INTERACTIVE',
];
- const DROPDOWN_OPTIONS = [
- 'ALLOW',
- 'DENY',
- 'BLOCK',
- ];
+ const Action = {
+ ALLOW: 'ALLOW',
+ DENY: 'DENY',
+ BLOCK: 'BLOCK',
+ };
- const FORCE_PUSH_OPTIONS = [
- {
- name: 'Block all pushes, block force push only',
- value: false,
- },
- {
- name: 'Allow fast-forward only push, allow all pushes',
- value: true,
- },
- ];
+ const DROPDOWN_OPTIONS = [Action.ALLOW, Action.DENY, Action.BLOCK];
+
+ const ForcePushOptions = {
+ ALLOW: [
+ {name: 'Allow pushing (but not force pushing)', value: false},
+ {name: 'Allow pushing with or without force', value: true},
+ ],
+ BLOCK: [
+ {name: 'Block pushing with or without force', value: false},
+ {name: 'Block force pushing', value: true},
+ ],
+ };
const FORCE_EDIT_OPTIONS = [
{
@@ -117,13 +119,17 @@
this._setOriginalRuleValues(rule.value);
},
- _computeForce(permission) {
- return this.permissionValues.push.id === permission ||
- this.permissionValues.editTopicName.id === permission;
+ _computeForce(permission, action) {
+ if (this.permissionValues.push.id === permission &&
+ action !== Action.DENY) {
+ return true;
+ }
+
+ return this.permissionValues.editTopicName.id === permission;
},
- _computeForceClass(permission) {
- return this._computeForce(permission) ? 'force' : '';
+ _computeForceClass(permission, action) {
+ return this._computeForce(permission, action) ? 'force' : '';
},
_computeGroupPath(group) {
@@ -156,9 +162,15 @@
return classList.join(' ');
},
- _computeForceOptions(permission) {
+ _computeForceOptions(permission, action) {
if (permission === this.permissionValues.push.id) {
- return FORCE_PUSH_OPTIONS;
+ if (action === Action.ALLOW) {
+ return ForcePushOptions.ALLOW;
+ } else if (action === Action.BLOCK) {
+ return ForcePushOptions.BLOCK;
+ } else {
+ return [];
+ }
} else if (permission === this.permissionValues.editTopicName.id) {
return FORCE_EDIT_OPTIONS;
}
@@ -166,6 +178,7 @@
},
_getDefaultRuleValues(permission, label) {
+ const ruleAction = Action.ALLOW;
const value = {};
if (permission === 'priority') {
value.action = PRIORITY_OPTIONS[0];
@@ -173,16 +186,17 @@
} else if (label) {
value.min = label.values[0].value;
value.max = label.values[label.values.length - 1].value;
- } else if (this._computeForce(permission)) {
- value.force = this._computeForceOptions(permission)[0].value;
+ } else if (this._computeForce(permission, ruleAction)) {
+ value.force =
+ this._computeForceOptions(permission, ruleAction)[0].value;
}
value.action = DROPDOWN_OPTIONS[0];
return value;
},
_setDefaultRuleValues() {
- this.set('rule.value',
- this._getDefaultRuleValues(this.permission, this.label));
+ this.set('rule.value', this._getDefaultRuleValues(this.permission,
+ this.label));
},
_computeOptions(permission) {
diff --git a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html
index 5b6f947..f85c2b2 100644
--- a/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html
+++ b/polygerrit-ui/app/elements/admin/gr-rule-editor/gr-rule-editor_test.html
@@ -50,16 +50,16 @@
suite('unit tests', () => {
test('_computeForce, _computeForceClass, and _computeForceOptions',
() => {
- const FORCE_PUSH_OPTIONS = [
- {
- name: 'Block all pushes, block force push only',
- value: false,
- },
- {
- name: 'Allow fast-forward only push, allow all pushes',
- value: true,
- },
- ];
+ const ForcePushOptions = {
+ ALLOW: [
+ {name: 'Allow pushing (but not force pushing)', value: false},
+ {name: 'Allow pushing with or without force', value: true},
+ ],
+ BLOCK: [
+ {name: 'Block pushing with or without force', value: false},
+ {name: 'Block force pushing', value: true},
+ ],
+ };
const FORCE_EDIT_OPTIONS = [
{
@@ -72,10 +72,26 @@
},
];
let permission = 'push';
- assert.isTrue(element._computeForce(permission));
- assert.equal(element._computeForceClass(permission), 'force');
- assert.deepEqual(element._computeForceOptions(permission),
- FORCE_PUSH_OPTIONS);
+ let action = 'ALLOW';
+ assert.isTrue(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action),
+ 'force');
+ assert.deepEqual(element._computeForceOptions(permission, action),
+ ForcePushOptions.ALLOW);
+
+ action = 'BLOCK';
+ assert.isTrue(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action),
+ 'force');
+ assert.deepEqual(element._computeForceOptions(permission, action),
+ ForcePushOptions.BLOCK);
+
+ action = 'DENY';
+ assert.isFalse(element._computeForce(permission, action));
+ assert.equal(element._computeForceClass(permission, action), '');
+ assert.equal(
+ element._computeForceOptions(permission, action).length, 0);
+
permission = 'editTopicName';
assert.isTrue(element._computeForce(permission));
assert.equal(element._computeForceClass(permission), 'force');
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
index dc41f59..9930bf5 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -21,6 +21,8 @@
const CLOSED_STATUS = ['MERGED', 'ABANDONED'];
+ const LABEL_PREFIX_INVALID_PROLOG = 'Invalid-Prolog-Rules-Label-Name--';
+
Polymer({
is: 'gr-change-list',
@@ -180,7 +182,11 @@
},
_computeLabelShortcut(labelName) {
+ if (labelName.startsWith(LABEL_PREFIX_INVALID_PROLOG)) {
+ labelName = labelName.slice(LABEL_PREFIX_INVALID_PROLOG.length);
+ }
return labelName.split('-').reduce((a, i) => {
+ if (!i) { return a; }
return a + i[0].toUpperCase();
}, '');
},
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index 3dd8c9d..9ce5764 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -127,7 +127,11 @@
assert.equal(element._computeLabelShortcut('PolyGerrit-Review'), 'PR');
assert.equal(element._computeLabelShortcut('polygerrit-review'), 'PR');
assert.equal(element._computeLabelShortcut(
+ 'Invalid-Prolog-Rules-Label-Name--Verified'), 'V');
+ assert.equal(element._computeLabelShortcut(
'Some-Special-Label-7'), 'SSL7');
+ assert.equal(element._computeLabelShortcut('--Too----many----dashes---'),
+ 'TMD');
});
test('colspans', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
index c94a716..582c83b 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
@@ -37,7 +37,6 @@
threshold="[[suggestFrom]]"
query="[[query]]"
allow-non-suggested-values="[[allowAnyInput]]"
- no-debounce
on-commit="_handleInputCommit"
clear-on-commit
warn-uncommitted
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 5cb3b77..86e3903 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
@@ -69,6 +69,8 @@
type: String,
observer: '_inputTextChanged',
},
+
+ _loggedIn: Boolean,
},
behaviors: [
@@ -79,6 +81,9 @@
this.$.restAPI.getConfig().then(cfg => {
this._config = cfg;
});
+ this.$.restAPI.getLoggedIn().then(loggedIn => {
+ this._loggedIn = loggedIn;
+ });
},
get focusStart() {
@@ -144,7 +149,9 @@
},
_getReviewerSuggestions(input) {
- if (!this.change || !this.change._number) { return Promise.resolve([]); }
+ if (!this.change || !this.change._number || !this._loggedIn) {
+ return Promise.resolve([]);
+ }
const api = this.$.restAPI;
const xhr = this.allowAnyUser ?
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 7d5ddd8..03a0be8 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
@@ -65,7 +65,7 @@
let suggestion3;
let element;
- setup(() => {
+ setup(done => {
owner = makeAccount();
existingReviewer1 = makeAccount();
existingReviewer2 = makeAccount();
@@ -78,6 +78,10 @@
},
};
+ stub('gr-rest-api-interface', {
+ getLoggedIn() { return Promise.resolve(true); },
+ });
+
element = fixture('basic');
element.change = {
_number: 42,
@@ -88,6 +92,7 @@
},
};
sandbox = sinon.sandbox.create();
+ return flush(done);
});
teardown(() => {
@@ -168,6 +173,19 @@
}).then(done);
});
});
+
+ test('_getReviewerSuggestions short circuits when logged out', () => {
+ // API call is already stubbed.
+ const xhrSpy = element.$.restAPI.getChangeSuggestedReviewers;
+ element._loggedIn = false;
+ return element._getReviewerSuggestions('').then(() => {
+ assert.isFalse(xhrSpy.called);
+ element._loggedIn = true;
+ return element._getReviewerSuggestions('').then(() => {
+ assert.isTrue(xhrSpy.called);
+ });
+ });
+ });
});
test('allowAnyUser', done => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index 4f466f4..da0d167 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -54,7 +54,7 @@
gr-button,
gr-dropdown {
/* px because don't have the same font size */
- margin-left: 12px;
+ margin-left: 8px;
}
#actionLoadingMessage {
align-items: center;
@@ -70,6 +70,14 @@
margin-right: .2rem;
width: 1.2rem;
}
+ gr-button {
+ min-height: 2.25em;
+ }
+ gr-dropdown {
+ --gr-button: {
+ min-height: 2.25em;
+ }
+ }
#moreActions iron-icon {
margin: 0;
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 29545c3..5a56475 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -121,7 +121,6 @@
}
.changeMetadata {
border-right: 1px solid var(--border-color);
- font-size: var(--font-size-small);
padding: 1em 0;
}
/* Prevent plugin text from overflowing. */
@@ -190,7 +189,6 @@
overflow: hidden;
}
#relatedChanges {
- font-size: var(--font-size-small);
}
#relatedChanges.collapsed {
margin-bottom: 1.1em;
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
index c0a09f3..234d903 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.html
@@ -78,7 +78,7 @@
assert.isOk(element._computeShowWebLink(element.change,
element.commitInfo, element.serverConfig));
assert.equal(element._computeWebLink(element.change, element.commitInfo,
- element.serverConfig), '../../link-url');
+ element.serverConfig), 'link-url');
});
test('does not relativize web links that begin with scheme', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
index ad14a18..299508b 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-submit-dialog/gr-confirm-submit-dialog.html
@@ -30,6 +30,12 @@
p {
margin-bottom: 1em;
}
+ @media screen and (max-width: 50em) {
+ #dialog {
+ min-width: inherit;
+ width: 100%;
+ }
+ }
</style>
<gr-confirm-dialog
id="dialog"
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 41e5227..93c351c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -178,10 +178,8 @@
display: none;
}
label.show-hide {
- color: var(--link-color);
cursor: pointer;
display: block;
- font-size: var(--font-size-small);
min-width: 2em;
}
gr-diff {
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.js b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
index 6adc286..3b4f1ec 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.js
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.js
@@ -309,12 +309,7 @@
if (!weblinks) { return null; }
const weblink = weblinks.find(this._isDirectCommit);
if (!weblink) { return null; }
- const url = weblink.url;
- if (url.startsWith('https:') || url.startsWith('http:')) {
- return url;
- } else {
- return `../../${url}`;
- }
+ return weblink.url;
},
_getChangeWeblinks({repo, commit, options: {weblinks}}) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
index 4599780..c3a1de4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
@@ -60,7 +60,6 @@
}
.descriptionText {
margin-left: .5rem;
- font-size: var(--font-size-small);
font-style: italic;
}
</style>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index b261a34..568c11f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -213,7 +213,6 @@
}
.resolve label {
color: var(--comment-text-color);
- font-size: var(--font-size-small);
}
gr-confirm-dialog .main {
display: flex;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index edee1ae..1b5203e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -171,7 +171,6 @@
}
.fullFileName {
display: block;
- font-size: var(--font-size-small);
font-style: italic;
min-width: 50%;
padding: 0 .1em;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 9862b3f..8798a8f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -990,6 +990,7 @@
},
_handleShiftXKey(e) {
+ if (this.shouldSuppressKeyboardShortcut(e)) { return; }
this.$.diff.expandAllContext();
},
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 540df98..718fa17 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -40,7 +40,7 @@
}
.diffContainer {
display: flex;
- font: var(--font-size-small) var(--monospace-font-family);
+ font-family: var(--monospace-font-family);
@apply --diff-container-styles;
}
.diffContainer.hiddenscroll {
@@ -89,7 +89,7 @@
.lineNum,
.content {
/* Set font size based the user's diff preference. */
- font-size: var(--font-size, var(--font-size-small));
+ font-size: var(--font-size, var(--font-size-normal));
vertical-align: top;
white-space: pre;
}
@@ -185,7 +185,7 @@
border-bottom: 1px solid var(--border-color);
color: var(--link-color);
font-family: var(--monospace-font-family);
- font-size: var(--font-size, var(--font-size-small));
+ font-size: var(--font-size, var(--font-size-normal));
padding: 0.5em 0 0.5em 4em;
}
#sizeWarning {
@@ -209,7 +209,7 @@
td.blame {
display: none;
font-family: var(--font-family);
- font-size: var(--font-size, var(--font-size-small));
+ font-size: var(--font-size, var(--font-size-normal));
padding: 0 .5em;
white-space: pre;
}
@@ -235,7 +235,7 @@
/** Since the line limit position is determined by charachter size, blank
lines also need to have the same font size as everything else */
.full-width .blank {
- font-size: var(--font-size, var(--font-size-small));
+ font-size: var(--font-size, var(--font-size-normal));
}
/** Support the line length indicator **/
.full-width td.content,
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
index 8fff850..6564abe 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.html
@@ -30,8 +30,6 @@
--background-color: var(--button-background-color, var(--default-button-background-color));
--text-color: var(--default-button-text-color);
display: inline-block;
- font-family: var(--font-family-bold);
- font-size: var(--font-size-small);
position: relative;
}
:host([hidden]) {
@@ -52,7 +50,7 @@
justify-content: center;
margin: var(--margin, 0);
min-width: var(--border, 0);
- padding: var(--padding, 5px 10px);
+ padding: var(--padding, 4px 8px);
@apply --gr-button;
}
paper-button:hover {
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
index 3abe28b..f4b120a 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.html
@@ -78,7 +78,6 @@
}
.bottomContent {
color: var(--deemphasized-text-color);
- font-size: var(--font-size-small);
/*
* Should be 16px when the base font size is 13px (browser default of
* 16px.
