Merge "Document library upgrade process"
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
index d0f998c..00051b1 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AccountCreator.java
@@ -31,7 +31,6 @@
import com.google.gerrit.server.account.VersionedAuthorizedKeys;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
-import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.ssh.SshKeyCache;
import com.google.gerrit.testutil.SshMode;
import com.google.gwtorm.server.SchemaFactory;
@@ -59,7 +58,6 @@
private final SshKeyCache sshKeyCache;
private final AccountCache accountCache;
private final AccountByEmailCache byEmailCache;
- private final AccountIndexer indexer;
private final ExternalIdsUpdate.Server externalIdsUpdate;
@Inject
@@ -71,7 +69,6 @@
SshKeyCache sshKeyCache,
AccountCache accountCache,
AccountByEmailCache byEmailCache,
- AccountIndexer indexer,
ExternalIdsUpdate.Server externalIdsUpdate) {
accounts = new HashMap<>();
reviewDbProvider = schema;
@@ -81,7 +78,6 @@
this.sshKeyCache = sshKeyCache;
this.accountCache = accountCache;
this.byEmailCache = byEmailCache;
- this.indexer = indexer;
this.externalIdsUpdate = externalIdsUpdate;
}
@@ -123,6 +119,7 @@
checkArgument(g != null, "group not found: %s", n);
AccountGroupMember m = new AccountGroupMember(new AccountGroupMember.Key(id, g.getId()));
db.accountGroupMembers().insert(Collections.singleton(m));
+ accountCache.evict(id);
}
}
@@ -138,8 +135,6 @@
}
byEmailCache.evict(email);
- indexer.index(id);
-
account = new TestAccount(id, username, email, fullName, sshKey, httpPass);
if (username != null) {
accounts.put(username, account);
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritConfig.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritConfig.java
index 44323e4..fe0c628 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritConfig.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GerritConfig.java
@@ -34,13 +34,9 @@
*/
String name();
- /**
- * Single value. Takes precedence over values specified in {@code values}.
- */
+ /** Single value. Takes precedence over values specified in {@code values}. */
String value() default "";
- /**
- * Multiple values (list). Ignored if {@code value} is specified.
- */
+ /** Multiple values (list). Ignored if {@code value} is specified. */
String[] values() default "";
}
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GlobalPluginConfig.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GlobalPluginConfig.java
index 3337f68..6edb3f0 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GlobalPluginConfig.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/GlobalPluginConfig.java
@@ -25,23 +25,15 @@
@Retention(RUNTIME)
@Repeatable(GlobalPluginConfigs.class)
public @interface GlobalPluginConfig {
- /**
- * Name of the plugin, corresponding to {@code $site/etc/@pluginName.comfig}.
- */
+ /** Name of the plugin, corresponding to {@code $site/etc/@pluginName.comfig}. */
String pluginName();
- /**
- * @see GerritConfig#name()
- */
+ /** @see GerritConfig#name() */
String name();
- /**
- * @see GerritConfig#value()
- */
+ /** @see GerritConfig#value() */
String value() default "";
- /**
- * @see GerritConfig#values()
- */
+ /** @see GerritConfig#values() */
String[] values() default "";
}
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
index 6ca8384..fe1d0b7 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -16,9 +16,11 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
+import static java.util.stream.Collectors.toList;
import static org.junit.Assert.assertEquals;
import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
@@ -29,15 +31,19 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.testutil.TestNotesMigration;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Stream;
import org.eclipse.jgit.api.TagCommand;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.PersonIdent;
@@ -138,6 +144,7 @@
private final ChangeNotes.Factory notesFactory;
private final ApprovalsUtil approvalsUtil;
private final Provider<InternalChangeQuery> queryProvider;
+ private final TestNotesMigration notesMigration;
private final ReviewDb db;
private final TestRepository<?> testRepo;
@@ -155,6 +162,7 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
+ TestNotesMigration notesMigration,
@Assisted ReviewDb db,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo)
@@ -163,6 +171,7 @@
notesFactory,
approvalsUtil,
queryProvider,
+ notesMigration,
db,
i,
testRepo,
@@ -176,6 +185,7 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
+ TestNotesMigration notesMigration,
@Assisted ReviewDb db,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -185,6 +195,7 @@
notesFactory,
approvalsUtil,
queryProvider,
+ notesMigration,
db,
i,
testRepo,
@@ -199,6 +210,7 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
+ TestNotesMigration notesMigration,
@Assisted ReviewDb db,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -210,6 +222,7 @@
notesFactory,
approvalsUtil,
queryProvider,
+ notesMigration,
db,
i,
testRepo,
@@ -224,13 +237,24 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
+ TestNotesMigration notesMigration,
@Assisted ReviewDb db,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@Assisted String subject,
@Assisted Map<String, String> files)
throws Exception {
- this(notesFactory, approvalsUtil, queryProvider, db, i, testRepo, subject, files, null);
+ this(
+ notesFactory,
+ approvalsUtil,
+ queryProvider,
+ notesMigration,
+ db,
+ i,
+ testRepo,
+ subject,
+ files,
+ null);
}
@AssistedInject
@@ -238,6 +262,7 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
+ TestNotesMigration notesMigration,
@Assisted ReviewDb db,
@Assisted PersonIdent i,
@Assisted TestRepository<?> testRepo,
@@ -250,6 +275,7 @@
notesFactory,
approvalsUtil,
queryProvider,
+ notesMigration,
db,
i,
testRepo,
@@ -262,6 +288,7 @@
ChangeNotes.Factory notesFactory,
ApprovalsUtil approvalsUtil,
Provider<InternalChangeQuery> queryProvider,
+ TestNotesMigration notesMigration,
ReviewDb db,
PersonIdent i,
TestRepository<?> testRepo,
@@ -274,6 +301,7 @@
this.notesFactory = notesFactory;
this.approvalsUtil = approvalsUtil;
this.queryProvider = queryProvider;
+ this.notesMigration = notesMigration;
this.subject = subject;
this.files = files;
this.changeId = changeId;
@@ -392,16 +420,36 @@
public void assertChange(
Change.Status expectedStatus, String expectedTopic, TestAccount... expectedReviewers)
throws OrmException {
+ assertChange(
+ expectedStatus, expectedTopic, Arrays.asList(expectedReviewers), ImmutableList.of());
+ }
+
+ public void assertChange(
+ Change.Status expectedStatus,
+ String expectedTopic,
+ List<TestAccount> expectedReviewers,
+ List<TestAccount> expectedCcs)
+ throws OrmException {
Change c = getChange().change();
assertThat(c.getSubject()).isEqualTo(resSubj);
assertThat(c.getStatus()).isEqualTo(expectedStatus);
assertThat(Strings.emptyToNull(c.getTopic())).isEqualTo(expectedTopic);
- assertReviewers(c, expectedReviewers);
+ if (notesMigration.readChanges()) {
+ assertReviewers(c, ReviewerStateInternal.REVIEWER, expectedReviewers);
+ assertReviewers(c, ReviewerStateInternal.CC, expectedCcs);
+ } else {
+ assertReviewers(
+ c,
+ ReviewerStateInternal.REVIEWER,
+ Stream.concat(expectedReviewers.stream(), expectedCcs.stream()).collect(toList()));
+ }
}
- private void assertReviewers(Change c, TestAccount... expectedReviewers) throws OrmException {
+ private void assertReviewers(
+ Change c, ReviewerStateInternal state, List<TestAccount> expectedReviewers)
+ throws OrmException {
Iterable<Account.Id> actualIds =
- approvalsUtil.getReviewers(db, notesFactory.createChecked(db, c)).all();
+ approvalsUtil.getReviewers(db, notesFactory.createChecked(db, c)).byState(state);
assertThat(actualIds)
.containsExactlyElementsIn(Sets.newHashSet(TestAccount.ids(expectedReviewers)));
}
diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestAccount.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestAccount.java
index 3ab4a88..7acb135 100644
--- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestAccount.java
+++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestAccount.java
@@ -32,10 +32,6 @@
return accounts.stream().map(a -> a.id).collect(toList());
}
- public static List<Account.Id> ids(TestAccount... accounts) {
- return ids(Arrays.asList(accounts));
- }
-
public static List<String> names(List<TestAccount> accounts) {
return accounts.stream().map(a -> a.fullName).collect(toList());
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/annotation/UseGerritConfigAnnotationTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/annotation/UseGerritConfigAnnotationTest.java
index 451af0f..53f1839 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/annotation/UseGerritConfigAnnotationTest.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/annotation/UseGerritConfigAnnotationTest.java
@@ -74,8 +74,6 @@
values = {"value-2", "value-3"}
)
public void valueHasPrecedenceOverValues() {
- assertThat(cfg.getStringList("section", null, "name"))
- .asList()
- .containsExactly("value-1");
+ assertThat(cfg.getStringList("section", null, "name")).asList().containsExactly("value-1");
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
index 9ac5a70..89d54e1 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/accounts/AccountIT.java
@@ -172,8 +172,6 @@
externalIdsUpdate.delete(getExternalIds(user));
externalIdsUpdate.insert(savedExternalIds);
}
- accountCache.evict(admin.getId());
- accountCache.evict(user.getId());
}
@After
@@ -213,9 +211,10 @@
AccountInfo info = gApi.accounts().id(foo.id.get()).get();
assertThat(info.username).isEqualTo("foo");
if (SshMode.useSsh()) {
- accountIndexedCounter.assertReindexOf(foo, 2); // account creation + adding SSH keys
+ accountIndexedCounter.assertReindexOf(
+ foo, 3); // account creation + external ID creation + adding SSH keys
} else {
- accountIndexedCounter.assertReindexOf(foo, 1); // account creation
+ accountIndexedCounter.assertReindexOf(foo, 2); // account creation + external ID creation
}
// check user branch
@@ -534,7 +533,6 @@
ExternalId.createWithEmail(ExternalId.Key.parse(extId1), admin.id, email),
ExternalId.createWithEmail(ExternalId.Key.parse(extId2), admin.id, email));
externalIdsUpdateFactory.create().insert(extIds);
- accountCache.evict(admin.id);
accountIndexedCounter.assertReindexOf(admin);
assertThat(
gApi.accounts().self().getExternalIds().stream().map(e -> e.identity).collect(toSet()))
@@ -589,7 +587,6 @@
externalIdsUpdateFactory
.create()
.insert(ExternalId.createWithEmail(ExternalId.Key.parse("foo:bar"), admin.id, email));
- accountCache.evict(admin.id);
assertEmail(byEmailCache.get(email), admin);
// wrong case doesn't match
@@ -750,6 +747,79 @@
@Test
@Sandboxed
+ public void cannotCreateUserBranch() throws Exception {
+ grant(allUsers, RefNames.REFS_USERS + "*", Permission.CREATE);
+ grant(allUsers, RefNames.REFS_USERS + "*", Permission.PUSH);
+
+ String userRef = RefNames.refsUsers(new Account.Id(db.nextAccountId()));
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ PushOneCommit.Result r = pushFactory.create(db, admin.getIdent(), allUsersRepo).to(userRef);
+ r.assertErrorStatus();
+ assertThat(r.getMessage()).contains("Not allowed to create user branch.");
+
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ assertThat(repo.exactRef(userRef)).isNull();
+ }
+ }
+
+ @Test
+ @Sandboxed
+ public void createUserBranchWithAccessDatabaseCapability() throws Exception {
+ allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+ grant(allUsers, RefNames.REFS_USERS + "*", Permission.CREATE);
+ grant(allUsers, RefNames.REFS_USERS + "*", Permission.PUSH);
+
+ String userRef = RefNames.refsUsers(new Account.Id(db.nextAccountId()));
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ pushFactory.create(db, admin.getIdent(), allUsersRepo).to(userRef).assertOkStatus();
+
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ assertThat(repo.exactRef(userRef)).isNotNull();
+ }
+ }
+
+ @Test
+ @Sandboxed
+ public void cannotCreateNonUserBranchUnderRefsUsersWithAccessDatabaseCapability()
+ throws Exception {
+ allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
+ grant(allUsers, RefNames.REFS_USERS + "*", Permission.CREATE);
+ grant(allUsers, RefNames.REFS_USERS + "*", Permission.PUSH);
+
+ String userRef = RefNames.REFS_USERS + "foo";
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ PushOneCommit.Result r = pushFactory.create(db, admin.getIdent(), allUsersRepo).to(userRef);
+ r.assertErrorStatus();
+ assertThat(r.getMessage()).contains("Not allowed to create non-user branch under refs/users/.");
+
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ assertThat(repo.exactRef(userRef)).isNull();
+ }
+ }
+
+ @Test
+ @Sandboxed
+ public void createDefaultUserBranch() throws Exception {
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ assertThat(repo.exactRef(RefNames.REFS_USERS_DEFAULT)).isNull();
+ }
+
+ grant(allUsers, RefNames.REFS_USERS_DEFAULT, Permission.CREATE);
+ grant(allUsers, RefNames.REFS_USERS_DEFAULT, Permission.PUSH);
+
+ TestRepository<InMemoryRepository> allUsersRepo = cloneProject(allUsers);
+ pushFactory
+ .create(db, admin.getIdent(), allUsersRepo)
+ .to(RefNames.REFS_USERS_DEFAULT)
+ .assertOkStatus();
+
+ try (Repository repo = repoManager.openRepository(allUsers)) {
+ assertThat(repo.exactRef(RefNames.REFS_USERS_DEFAULT)).isNotNull();
+ }
+ }
+
+ @Test
+ @Sandboxed
public void cannotDeleteUserBranch() throws Exception {
grant(
allUsers,
@@ -831,7 +901,6 @@
// Both users have a matching external ID for this key.
addExternalIdEmail(admin, "test5@example.com");
externalIdsUpdate.insert(ExternalId.create("foo", "myId", user.getId()));
- accountCache.evict(user.getId());
accountIndexedCounter.assertReindexOf(user);
TestKey key = validKeyWithSecondUserId();
@@ -1073,8 +1142,6 @@
checkNotNull(email);
externalIdsUpdate.insert(
ExternalId.createWithEmail(name("test"), email, account.getId(), email));
- // Clear saved AccountState and ExternalIds.
- accountCache.evict(account.getId());
accountIndexedCounter.assertReindexOf(account);
setApiUser(account);
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index dbe4a6c..5a2cb2a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -448,6 +448,40 @@
}
@Test
+ public void revertPreservesReviewersAndCcs() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ ReviewInput in = ReviewInput.approve();
+ in.reviewer(user.email);
+ in.reviewer(accounts.user2().email, ReviewerState.CC, true);
+ // Add user as reviewer that will create the revert
+ in.reviewer(accounts.admin2().email);
+
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).review(in);
+ gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).submit();
+
+ // expect both the original reviewers and CCs to be preserved
+ // original owner should be added as reviewer, user requesting the revert (new owner) removed
+ setApiUser(accounts.admin2());
+ Map<ReviewerState, Collection<AccountInfo>> result =
+ gApi.changes().id(r.getChangeId()).revert().get().reviewers;
+ assertThat(result).containsKey(ReviewerState.REVIEWER);
+
+ List<Integer> reviewers =
+ result.get(ReviewerState.REVIEWER).stream().map(a -> a._accountId).collect(toList());
+ if (notesMigration.readChanges()) {
+ assertThat(result).containsKey(ReviewerState.CC);
+ List<Integer> ccs =
+ result.get(ReviewerState.CC).stream().map(a -> a._accountId).collect(toList());
+ assertThat(ccs).containsExactly(accounts.user2().id.get());
+ assertThat(reviewers).containsExactly(user.id.get(), admin.id.get());
+ } else {
+ assertThat(reviewers)
+ .containsExactly(user.id.get(), admin.id.get(), accounts.user2().id.get());
+ }
+ }
+
+ @Test
@TestProjectInput(createEmptyCommit = false)
public void revertInitialCommit() throws Exception {
PushOneCommit.Result r = createChange();
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index b52136e..cdb8fbb 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -323,10 +323,13 @@
String topic = "my/topic";
PushOneCommit.Result r = pushTo("refs/for/master/" + topic + "%cc=" + user.email);
r.assertOkStatus();
- r.assertChange(Change.Status.NEW, topic);
+ r.assertChange(
+ Change.Status.NEW,
+ topic,
+ ImmutableList.of(),
+ ImmutableList.of(user));
// cc several users
- TestAccount user2 = accounts.create("another-user", "another.user@example.com", "Another User");
r =
pushTo(
"refs/for/master/"
@@ -336,9 +339,14 @@
+ ",cc="
+ user.email
+ ",cc="
- + user2.email);
+ + accounts.user2().email);
r.assertOkStatus();
- r.assertChange(Change.Status.NEW, topic);
+ // Check that admin isn't CC'd as they own the change
+ r.assertChange(
+ Change.Status.NEW,
+ topic,
+ ImmutableList.of(),
+ ImmutableList.of(user, accounts.user2()));
// cc non-existing user
String nonExistingEmail = "non.existing@example.com";
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
index 5f1f487..1976edc 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
@@ -671,6 +671,7 @@
ExternalIdsUpdate update =
new ExternalIdsUpdate(
repoManager,
+ accountCache,
allUsers,
metricMaker,
externalIds,
@@ -706,6 +707,7 @@
ExternalIdsUpdate update =
new ExternalIdsUpdate(
repoManager,
+ accountCache,
allUsers,
metricMaker,
externalIds,
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
index 4c38d83..d6ad269 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/AbstractMailIT.java
@@ -132,9 +132,9 @@
+ "> \n";
}
- protected static String textFooterForChange(String changeId, String timestamp) {
- return "Gerrit-Change-Id: "
- + changeId
+ protected static String textFooterForChange(int changeNumber, String timestamp) {
+ return "Gerrit-Change-Number: "
+ + changeNumber
+ "\n"
+ "Gerrit-PatchSet: 1\n"
+ "Gerrit-MessageType: comment\n"
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ListMailFilterIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ListMailFilterIT.java
index 1dcdd97..a96c6ec 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ListMailFilterIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ListMailFilterIT.java
@@ -113,7 +113,7 @@
null,
null,
null);
- b.textContent(txt + textFooterForChange(changeId, ts));
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
return changeInfo;
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
index ada222a..7cef8e7 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
@@ -67,7 +67,8 @@
Map<String, Object> expectedHeaders = new HashMap<>();
expectedHeaders.put("Gerrit-PatchSet", "1");
- expectedHeaders.put("Gerrit-Change-Id", newChange.getChangeId());
+ expectedHeaders.put(
+ "Gerrit-Change-Number", String.valueOf(newChange.getChange().getId().get()));
expectedHeaders.put("Gerrit-MessageType", "newchange");
expectedHeaders.put("Gerrit-Commit", newChange.getCommit().getId().name());
expectedHeaders.put("Gerrit-ChangeURL", changeURL);
@@ -102,7 +103,8 @@
String changeURL = "<" + canonicalWebUrl.get() + newChange.getChange().getId().get() + ">";
Map<String, Object> expectedHeaders = new HashMap<>();
expectedHeaders.put("Gerrit-PatchSet", "1");
- expectedHeaders.put("Gerrit-Change-Id", newChange.getChangeId());
+ expectedHeaders.put(
+ "Gerrit-Change-Number", String.valueOf(newChange.getChange().getId().get()));
expectedHeaders.put("Gerrit-MessageType", "comment");
expectedHeaders.put("Gerrit-Commit", newChange.getCommit().getId().name());
expectedHeaders.put("Gerrit-ChangeURL", changeURL);
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index e7a0cda..9de4797 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -51,7 +51,7 @@
null,
null,
null);
- b.textContent(txt + textFooterForChange(changeId, ts));
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
@@ -79,7 +79,7 @@
"Some Inline Comment",
null,
null);
- b.textContent(txt + textFooterForChange(changeId, ts));
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
@@ -115,7 +115,7 @@
null,
"Some Comment on File 1",
null);
- b.textContent(txt + textFooterForChange(changeId, ts));
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
@@ -152,7 +152,7 @@
"Some Inline Comment",
null,
null);
- b.textContent(txt + textFooterForChange(changeId, ts));
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
comments = gApi.changes().id(changeId).current().commentsAsList();
@@ -183,7 +183,7 @@
"Some Inline Comment",
null,
null);
- b.textContent(txt + textFooterForChange(changeId, ts));
+ b.textContent(txt + textFooterForChange(changeInfo._number, ts));
// Set account state to inactive
gApi.accounts().id("user").setActive(false);
@@ -219,7 +219,7 @@
MailMessage.Builder b =
messageBuilderWithDefaultFields()
.from(user.emailAddress)
- .textContent(txt + textFooterForChange(changeId, ts));
+ .textContent(txt + textFooterForChange(changeInfo._number, ts));
sender.clear();
mailProcessor.process(b.build());
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/RequiresAnyCapability.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/RequiresAnyCapability.java
index f97abd9..1e3a2c8 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/RequiresAnyCapability.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/RequiresAnyCapability.java
@@ -33,4 +33,7 @@
/** Scope of the named capabilities. */
CapabilityScope scope() default CapabilityScope.CONTEXT;
+
+ /** Fall back to admin credentials. Only applies to plugin capability check. */
+ boolean fallBackToAdmin() default true;
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/RequiresCapability.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/RequiresCapability.java
index 7717c84..b9ef7e0 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/RequiresCapability.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/annotations/RequiresCapability.java
@@ -32,4 +32,7 @@
/** Scope of the named capability. */
CapabilityScope scope() default CapabilityScope.CONTEXT;
+
+ /** Fall back to admin credentials. Only applies to plugin capability check. */
+ boolean fallBackToAdmin() default true;
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/PluginPermission.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/PluginPermission.java
index 33a85cd..7a467b8 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/PluginPermission.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/access/PluginPermission.java
@@ -22,10 +22,16 @@
public class PluginPermission implements GlobalOrPluginPermission {
private final String pluginName;
private final String capability;
+ private final boolean fallBackToAdmin;
public PluginPermission(String pluginName, String capability) {
+ this(pluginName, capability, true);
+ }
+
+ public PluginPermission(String pluginName, String capability, boolean fallBackToAdmin) {
this.pluginName = checkNotNull(pluginName, "pluginName");
this.capability = checkNotNull(capability, "capability");
+ this.fallBackToAdmin = fallBackToAdmin;
}
public String pluginName() {
@@ -36,6 +42,10 @@
return capability;
}
+ public boolean fallBackToAdmin() {
+ return fallBackToAdmin;
+ }
+
@Override
public String permissionName() {
return pluginName + '-' + capability;
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java
index 4e6ea66..779d5d4 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/DeleteGpgKey.java
@@ -24,7 +24,6 @@
import com.google.gerrit.gpg.PublicKeyStore;
import com.google.gerrit.gpg.server.DeleteGpgKey.Input;
import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
import com.google.gwtorm.server.OrmException;
@@ -43,18 +42,15 @@
private final Provider<PersonIdent> serverIdent;
private final Provider<PublicKeyStore> storeProvider;
- private final AccountCache accountCache;
private final ExternalIdsUpdate.User externalIdsUpdateFactory;
@Inject
DeleteGpgKey(
@GerritPersonIdent Provider<PersonIdent> serverIdent,
Provider<PublicKeyStore> storeProvider,
- AccountCache accountCache,
ExternalIdsUpdate.User externalIdsUpdateFactory) {
this.serverIdent = serverIdent;
this.storeProvider = storeProvider;
- this.accountCache = accountCache;
this.externalIdsUpdateFactory = externalIdsUpdateFactory;
}
@@ -69,7 +65,6 @@
rsrc.getUser().getAccountId(),
ExternalId.Key.create(
SCHEME_GPGKEY, BaseEncoding.base16().encode(key.getFingerprint())));
- accountCache.evict(rsrc.getUser().getAccountId());
try (PublicKeyStore store = storeProvider.get()) {
store.remove(rsrc.getKeyRing().getPublicKey().getFingerprint());
diff --git a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
index af4d6bb..41fcd04 100644
--- a/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
+++ b/gerrit-gpg/src/main/java/com/google/gerrit/gpg/server/PostGpgKeys.java
@@ -43,7 +43,6 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -88,7 +87,6 @@
private final Provider<PublicKeyStore> storeProvider;
private final GerritPublicKeyChecker.Factory checkerFactory;
private final AddKeySender.Factory addKeyFactory;
- private final AccountCache accountCache;
private final Provider<InternalAccountQuery> accountQueryProvider;
private final ExternalIds externalIds;
private final ExternalIdsUpdate.User externalIdsUpdateFactory;
@@ -100,7 +98,6 @@
Provider<PublicKeyStore> storeProvider,
GerritPublicKeyChecker.Factory checkerFactory,
AddKeySender.Factory addKeyFactory,
- AccountCache accountCache,
Provider<InternalAccountQuery> accountQueryProvider,
ExternalIds externalIds,
ExternalIdsUpdate.User externalIdsUpdateFactory) {
@@ -109,7 +106,6 @@
this.storeProvider = storeProvider;
this.checkerFactory = checkerFactory;
this.addKeyFactory = addKeyFactory;
- this.accountCache = accountCache;
this.accountQueryProvider = accountQueryProvider;
this.externalIds = externalIds;
this.externalIdsUpdateFactory = externalIdsUpdateFactory;
@@ -148,7 +144,6 @@
externalIdsUpdateFactory
.create()
.replace(rsrc.getUser().getAccountId(), extIdKeysToRemove, newExtIds);
- accountCache.evict(rsrc.getUser().getAccountId());
return toJson(newKeys, toRemove, store, rsrc.getUser());
}
}
diff --git a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
index deb0dc4..64311e0 100644
--- a/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
+++ b/gerrit-gpg/src/test/java/com/google/gerrit/gpg/GerritPublicKeyCheckerTest.java
@@ -36,8 +36,9 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager;
+import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
@@ -52,7 +53,6 @@
import com.google.inject.Injector;
import com.google.inject.Provider;
import com.google.inject.util.Providers;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -71,7 +71,9 @@
/** Unit tests for {@link GerritPublicKeyChecker}. */
public class GerritPublicKeyCheckerTest {
- @Inject private AccountCache accountCache;
+ @Inject private Accounts accounts;
+
+ @Inject private AccountsUpdate.Server accountsUpdate;
@Inject private AccountManager accountManager;
@@ -115,10 +117,10 @@
db = schemaFactory.open();
schemaCreator.create(db);
userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId();
- Account userAccount = db.accounts().get(userId);
+ Account userAccount = accounts.get(db, userId);
// Note: does not match any key in TestKeys.
userAccount.setPreferredEmail("user@example.com");
- db.accounts().update(ImmutableList.of(userAccount));
+ accountsUpdate.create().update(db, userAccount);
user = reloadUser();
requestContext.setContext(
@@ -150,8 +152,7 @@
return userFactory.create(id);
}
- private IdentifiedUser reloadUser() throws IOException {
- accountCache.evict(userId);
+ private IdentifiedUser reloadUser() {
user = userFactory.create(userId);
return user;
}
@@ -407,7 +408,6 @@
assertThat(store.save(cb)).isAnyOf(NEW, FAST_FORWARD, FORCED);
externalIdsUpdateFactory.create().insert(newExtIds);
- accountCache.evict(user.getAccountId());
}
private TestKey add(TestKey k, IdentifiedUser user) throws Exception {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
index 7f6255a..d0adc4b 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/auth/become/BecomeAnyAccountLoginServlet.java
@@ -25,16 +25,17 @@
import com.google.gerrit.httpd.template.SiteHeaderFooter;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountException;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.AuthResult;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.query.account.InternalAccountQuery;
import com.google.gwtexpui.server.CacheHeaders;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -54,8 +55,10 @@
@SuppressWarnings("serial")
@Singleton
class BecomeAnyAccountLoginServlet extends HttpServlet {
- private final SchemaFactory<ReviewDb> schema;
private final DynamicItem<WebSession> webSession;
+ private final SchemaFactory<ReviewDb> schema;
+ private final Accounts accounts;
+ private final AccountCache accountCache;
private final AccountManager accountManager;
private final SiteHeaderFooter headers;
private final InternalAccountQuery accountQuery;
@@ -64,11 +67,15 @@
BecomeAnyAccountLoginServlet(
DynamicItem<WebSession> ws,
SchemaFactory<ReviewDb> sf,
+ Accounts a,
+ AccountCache ac,
AccountManager am,
SiteHeaderFooter shf,
InternalAccountQuery aq) {
webSession = ws;
schema = sf;
+ accounts = a;
+ accountCache = ac;
accountManager = am;
headers = shf;
accountQuery = aq;
@@ -149,8 +156,8 @@
Element userlistElement = HtmlDomUtil.find(doc, "userlist");
try (ReviewDb db = schema.open()) {
- ResultSet<Account> accounts = db.accounts().firstNById(100);
- for (Account a : accounts) {
+ for (Account.Id accountId : accounts.firstNIds(100)) {
+ Account a = accountCache.get(accountId).getAccount();
String displayName;
if (a.getUserName() != null) {
displayName = a.getUserName();
@@ -159,7 +166,7 @@
} else if (a.getPreferredEmail() != null) {
displayName = a.getPreferredEmail();
} else {
- displayName = a.getId().toString();
+ displayName = accountId.toString();
}
Element linkElement = doc.createElement("a");
@@ -223,7 +230,7 @@
return null;
}
try (ReviewDb db = schema.open()) {
- return auth(db.accounts().get(id));
+ return auth(accounts.get(db, id));
} catch (OrmException e) {
getServletContext().log("cannot query database", e);
return null;
diff --git a/gerrit-httpd/src/test/java/com/google/gerrit/httpd/raw/IndexServletTest.java b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/raw/IndexServletTest.java
index 0c3e1be..d106eec 100644
--- a/gerrit-httpd/src/test/java/com/google/gerrit/httpd/raw/IndexServletTest.java
+++ b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/raw/IndexServletTest.java
@@ -22,6 +22,8 @@
public class IndexServletTest {
class TestIndexServlet extends IndexServlet {
+ private static final long serialVersionUID = 1L;
+
TestIndexServlet(String canonicalURL, String cdnPath) throws URISyntaxException {
super(canonicalURL, cdnPath);
}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/AccountsOnInit.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/AccountsOnInit.java
index 09626d7..ee939d6 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/AccountsOnInit.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/AccountsOnInit.java
@@ -21,6 +21,7 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdentProvider;
+import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.config.SitePaths;
import com.google.gwtorm.server.OrmException;
@@ -60,6 +61,17 @@
}
}
+ public boolean hasAnyAccount() throws IOException {
+ File path = getPath();
+ if (path == null) {
+ return false;
+ }
+
+ try (Repository repo = new FileRepository(path)) {
+ return Accounts.hasAnyAccount(repo);
+ }
+ }
+
private File getPath() {
Path basePath = site.resolve(flags.cfg.getString("gerrit", null, "basePath"));
checkArgument(basePath != null, "gerrit.basePath must be configured");
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
index 466404b..7162eab 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/InitAdminUser.java
@@ -88,7 +88,7 @@
}
try (ReviewDb db = dbFactory.open()) {
- if (db.accounts().anyAccounts().toList().isEmpty()) {
+ if (!accounts.hasAnyAccount()) {
ui.header("Gerrit Administrator");
if (ui.yesno(true, "Create administrator user")) {
Account.Id id = new Account.Id(db.nextAccountId());
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountAccess.java
index b015af8..db74caa 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountAccess.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountAccess.java
@@ -31,22 +31,6 @@
@Query("WHERE preferredEmail = ? LIMIT 2")
ResultSet<Account> byPreferredEmail(String email) throws OrmException;
- @Query("WHERE fullName = ? LIMIT 2")
- ResultSet<Account> byFullName(String name) throws OrmException;
-
- @Query("WHERE fullName >= ? AND fullName <= ? ORDER BY fullName LIMIT ?")
- ResultSet<Account> suggestByFullName(String nameA, String nameB, int limit) throws OrmException;
-
- @Query("WHERE preferredEmail >= ? AND preferredEmail <= ? ORDER BY preferredEmail LIMIT ?")
- ResultSet<Account> suggestByPreferredEmail(String nameA, String nameB, int limit)
- throws OrmException;
-
- @Query("LIMIT 1")
- ResultSet<Account> anyAccounts() throws OrmException;
-
- @Query("ORDER BY accountId LIMIT ?")
- ResultSet<Account> firstNById(int n) throws OrmException;
-
@Query("ORDER BY accountId")
ResultSet<Account> all() throws OrmException;
}
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
index 871ed20..5a83dc4 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
@@ -6,14 +6,10 @@
-- *********************************************************************
-- AccountAccess
--- covers: byPreferredEmail, suggestByPreferredEmail
+-- covers: byPreferredEmail
CREATE INDEX accounts_byPreferredEmail
ON accounts (preferred_email);
--- covers: suggestByFullName
-CREATE INDEX accounts_byFullName
-ON accounts (full_name);
-
-- *********************************************************************
-- AccountGroupMemberAccess
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
index c349241..98f05ca 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
@@ -7,16 +7,11 @@
-- *********************************************************************
-- AccountAccess
--- covers: byPreferredEmail, suggestByPreferredEmail
+-- covers: byPreferredEmail
CREATE INDEX accounts_byPreferredEmail
ON accounts (preferred_email)
#
--- covers: suggestByFullName
-CREATE INDEX accounts_byFullName
-ON accounts (full_name)
-#
-
-- *********************************************************************
-- AccountGroupMemberAccess
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
index da99fef..dde86a4 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
@@ -53,14 +53,10 @@
-- *********************************************************************
-- AccountAccess
--- covers: byPreferredEmail, suggestByPreferredEmail
+-- covers: byPreferredEmail
CREATE INDEX accounts_byPreferredEmail
ON accounts (preferred_email);
--- covers: suggestByFullName
-CREATE INDEX accounts_byFullName
-ON accounts (full_name);
-
-- *********************************************************************
-- AccountGroupMemberAccess
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
index 12fc938..1600982 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java
@@ -154,6 +154,7 @@
static class ByIdLoader extends CacheLoader<Account.Id, Optional<AccountState>> {
private final SchemaFactory<ReviewDb> schema;
+ private final Accounts accounts;
private final GroupCache groupCache;
private final GeneralPreferencesLoader loader;
private final LoadingCache<String, Optional<Account.Id>> byName;
@@ -163,11 +164,13 @@
@Inject
ByIdLoader(
SchemaFactory<ReviewDb> sf,
+ Accounts accounts,
GroupCache groupCache,
GeneralPreferencesLoader loader,
@Named(BYUSER_NAME) LoadingCache<String, Optional<Account.Id>> byUsername,
Provider<WatchConfig.Accessor> watchConfig,
ExternalIds externalIds) {
+ this.accounts = accounts;
this.schema = sf;
this.groupCache = groupCache;
this.loader = loader;
@@ -193,7 +196,7 @@
private Optional<AccountState> load(final ReviewDb db, final Account.Id who)
throws OrmException, IOException, ConfigInvalidException {
- Account account = db.accounts().get(who);
+ Account account = accounts.get(db, who);
if (account == null) {
return Optional.empty();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
index 49a20fa..9f2b869 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountManager.java
@@ -52,6 +52,7 @@
private static final Logger log = LoggerFactory.getLogger(AccountManager.class);
private final SchemaFactory<ReviewDb> schema;
+ private final Accounts accounts;
private final AccountsUpdate.Server accountsUpdateFactory;
private final AccountCache byIdCache;
private final AccountByEmailCache byEmailCache;
@@ -68,6 +69,7 @@
@Inject
AccountManager(
SchemaFactory<ReviewDb> schema,
+ Accounts accounts,
AccountsUpdate.Server accountsUpdateFactory,
AccountCache byIdCache,
AccountByEmailCache byEmailCache,
@@ -80,6 +82,7 @@
ExternalIds externalIds,
ExternalIdsUpdate.Server externalIdsUpdateFactory) {
this.schema = schema;
+ this.accounts = accounts;
this.accountsUpdateFactory = accountsUpdateFactory;
this.byIdCache = byIdCache;
this.byEmailCache = byEmailCache;
@@ -190,21 +193,18 @@
}
if (toUpdate != null) {
- db.accounts().update(Collections.singleton(toUpdate));
+ accountsUpdateFactory.create().update(db, toUpdate);
}
if (newEmail != null && !newEmail.equals(oldEmail)) {
byEmailCache.evict(oldEmail);
byEmailCache.evict(newEmail);
}
- if (toUpdate != null) {
- byIdCache.evict(toUpdate.getId());
- }
}
private Account load(Account toUpdate, Account.Id accountId, ReviewDb db) throws OrmException {
if (toUpdate == null) {
- toUpdate = db.accounts().get(accountId);
+ toUpdate = accounts.get(db, accountId);
if (toUpdate == null) {
throw new OrmException("Account " + accountId + " has been deleted");
}
@@ -226,8 +226,7 @@
account.setFullName(who.getDisplayName());
account.setPreferredEmail(extId.email());
- boolean isFirstAccount =
- awaitsFirstAccountCheck.getAndSet(false) && db.accounts().anyAccounts().toList().isEmpty();
+ boolean isFirstAccount = awaitsFirstAccountCheck.getAndSet(false) && !accounts.hasAnyAccount();
try {
AccountsUpdate accountsUpdate = accountsUpdateFactory.create();
@@ -301,7 +300,6 @@
}
byEmailCache.evict(account.getPreferredEmail());
- byIdCache.evict(account.getId());
realm.onCreateAccount(who, account);
return new AuthResult(newId, extId.key(), true);
}
@@ -374,17 +372,13 @@
.insert(ExternalId.createWithEmail(who.getExternalIdKey(), to, who.getEmailAddress()));
if (who.getEmailAddress() != null) {
- Account a = db.accounts().get(to);
+ Account a = accounts.get(db, to);
if (a.getPreferredEmail() == null) {
a.setPreferredEmail(who.getEmailAddress());
- db.accounts().update(Collections.singleton(a));
+ accountsUpdateFactory.create().update(db, a);
}
- }
-
- if (who.getEmailAddress() != null) {
byEmailCache.evict(who.getEmailAddress());
}
- byIdCache.evict(to);
}
return new AuthResult(to, who.getExternalIdKey(), false);
@@ -418,7 +412,6 @@
.isPresent())) {
externalIdsUpdateFactory.create().delete(filteredExtIdsByScheme);
}
- byIdCache.evict(to);
return link(to, who);
}
@@ -443,14 +436,13 @@
externalIdsUpdateFactory.create().delete(extId);
if (who.getEmailAddress() != null) {
- Account a = db.accounts().get(from);
+ Account a = accounts.get(db, from);
if (a.getPreferredEmail() != null
&& a.getPreferredEmail().equals(who.getEmailAddress())) {
a.setPreferredEmail(null);
- db.accounts().update(Collections.singleton(a));
+ accountsUpdateFactory.create().update(db, a);
}
byEmailCache.evict(who.getEmailAddress());
- byIdCache.evict(from);
}
} else {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
index 9803143..8ce5f4c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountResolver.java
@@ -33,6 +33,7 @@
@Singleton
public class AccountResolver {
private final Realm realm;
+ private final Accounts accounts;
private final AccountByEmailCache byEmail;
private final AccountCache byId;
private final Provider<InternalAccountQuery> accountQueryProvider;
@@ -40,10 +41,12 @@
@Inject
AccountResolver(
Realm realm,
+ Accounts accounts,
AccountByEmailCache byEmail,
AccountCache byId,
Provider<InternalAccountQuery> accountQueryProvider) {
this.realm = realm;
+ this.accounts = accounts;
this.byEmail = byEmail;
this.byId = byId;
this.accountQueryProvider = accountQueryProvider;
@@ -116,7 +119,7 @@
}
private boolean exists(ReviewDb db, Account.Id id) throws OrmException {
- return db.accounts().get(id) != null;
+ return accounts.get(db, id) != null;
}
/**
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java
new file mode 100644
index 0000000..4c23745
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/Accounts.java
@@ -0,0 +1,100 @@
+// 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.account;
+
+import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toSet;
+
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Stream;
+import org.eclipse.jgit.lib.Repository;
+
+/** Class to access accounts. */
+@Singleton
+public class Accounts {
+ private final GitRepositoryManager repoManager;
+ private final AllUsersName allUsersName;
+
+ @Inject
+ Accounts(GitRepositoryManager repoManager, AllUsersName allUsersName) {
+ this.repoManager = repoManager;
+ this.allUsersName = allUsersName;
+ }
+
+ public Account get(ReviewDb db, Account.Id accountId) throws OrmException {
+ return db.accounts().get(accountId);
+ }
+
+ /**
+ * Returns all account IDs.
+ *
+ * @return all account IDs
+ */
+ public Set<Account.Id> allIds() throws IOException {
+ return readUserRefs().collect(toSet());
+ }
+
+ /**
+ * Returns the first n account IDs.
+ *
+ * @param n the number of account IDs that should be returned
+ * @return first n account IDs
+ */
+ public List<Account.Id> firstNIds(int n) throws IOException {
+ return readUserRefs().sorted(comparing(id -> id.get())).limit(n).collect(toList());
+ }
+
+ /**
+ * Checks if any account exists.
+ *
+ * @return {@code true} if at least one account exists, otherwise {@code false}
+ */
+ public boolean hasAnyAccount() throws IOException {
+ try (Repository repo = repoManager.openRepository(allUsersName)) {
+ return hasAnyAccount(repo);
+ }
+ }
+
+ public static boolean hasAnyAccount(Repository repo) throws IOException {
+ return readUserRefs(repo).findAny().isPresent();
+ }
+
+ private Stream<Account.Id> readUserRefs() throws IOException {
+ try (Repository repo = repoManager.openRepository(allUsersName)) {
+ return readUserRefs(repo);
+ }
+ }
+
+ private static Stream<Account.Id> readUserRefs(Repository repo) throws IOException {
+ return repo.getRefDatabase()
+ .getRefs(RefNames.REFS_USERS)
+ .values()
+ .stream()
+ .map(r -> Account.Id.fromRef(r.getName()))
+ .filter(Objects::nonNull);
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
index de87fc1..069380f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
@@ -31,6 +31,7 @@
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
+import java.util.function.Consumer;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@@ -49,26 +50,32 @@
*
* <p>The Gerrit server identity will be used as author and committer for all commits that update
* the accounts.
+ *
+ * <p>On updating accounts this class takes care to evict them from the account cache and thus
+ * triggers reindex for them.
*/
@Singleton
public static class Server {
private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final Provider<PersonIdent> serverIdent;
@Inject
public Server(
GitRepositoryManager repoManager,
+ AccountCache accountCache,
AllUsersName allUsersName,
@GerritPersonIdent Provider<PersonIdent> serverIdent) {
this.repoManager = repoManager;
+ this.accountCache = accountCache;
this.allUsersName = allUsersName;
this.serverIdent = serverIdent;
}
public AccountsUpdate create() {
PersonIdent i = serverIdent.get();
- return new AccountsUpdate(repoManager, allUsersName, i, i);
+ return new AccountsUpdate(repoManager, accountCache, allUsersName, i, i);
}
}
@@ -81,6 +88,7 @@
@Singleton
public static class User {
private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final Provider<PersonIdent> serverIdent;
private final Provider<IdentifiedUser> identifiedUser;
@@ -88,10 +96,12 @@
@Inject
public User(
GitRepositoryManager repoManager,
+ AccountCache accountCache,
AllUsersName allUsersName,
@GerritPersonIdent Provider<PersonIdent> serverIdent,
Provider<IdentifiedUser> identifiedUser) {
this.repoManager = repoManager;
+ this.accountCache = accountCache;
this.allUsersName = allUsersName;
this.serverIdent = serverIdent;
this.identifiedUser = identifiedUser;
@@ -100,7 +110,7 @@
public AccountsUpdate create() {
PersonIdent i = serverIdent.get();
return new AccountsUpdate(
- repoManager, allUsersName, createPersonIdent(i, identifiedUser.get()), i);
+ repoManager, accountCache, allUsersName, createPersonIdent(i, identifiedUser.get()), i);
}
private PersonIdent createPersonIdent(PersonIdent ident, IdentifiedUser user) {
@@ -109,16 +119,19 @@
}
private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final PersonIdent committerIdent;
private final PersonIdent authorIdent;
private AccountsUpdate(
GitRepositoryManager repoManager,
+ AccountCache accountCache,
AllUsersName allUsersName,
PersonIdent committerIdent,
PersonIdent authorIdent) {
this.repoManager = checkNotNull(repoManager, "repoManager");
+ this.accountCache = checkNotNull(accountCache, "accountCache");
this.allUsersName = checkNotNull(allUsersName, "allUsersName");
this.committerIdent = checkNotNull(committerIdent, "committerIdent");
this.authorIdent = checkNotNull(authorIdent, "authorIdent");
@@ -133,6 +146,7 @@
public void insert(ReviewDb db, Account account) throws OrmException, IOException {
db.accounts().insert(ImmutableSet.of(account));
createUserBranch(account);
+ accountCache.evict(account.getId());
}
/**
@@ -143,18 +157,50 @@
public void upsert(ReviewDb db, Account account) throws OrmException, IOException {
db.accounts().upsert(ImmutableSet.of(account));
createUserBranchIfNeeded(account);
+ accountCache.evict(account.getId());
+ }
+
+ /** Updates the account. */
+ public void update(ReviewDb db, Account account) throws OrmException, IOException {
+ db.accounts().update(ImmutableSet.of(account));
+ accountCache.evict(account.getId());
+ }
+
+ /**
+ * Gets the account and updates it atomically.
+ *
+ * @param db ReviewDb
+ * @param accountId ID of the account
+ * @param consumer consumer to update the account, only invoked if the account exists
+ * @return the updated account, {@code null} if the account doesn't exist
+ * @throws OrmException if updating the account fails
+ */
+ public Account atomicUpdate(ReviewDb db, Account.Id accountId, Consumer<Account> consumer)
+ throws OrmException, IOException {
+ Account account =
+ db.accounts()
+ .atomicUpdate(
+ accountId,
+ a -> {
+ consumer.accept(a);
+ return a;
+ });
+ accountCache.evict(accountId);
+ return account;
}
/** Deletes the account. */
public void delete(ReviewDb db, Account account) throws OrmException, IOException {
db.accounts().delete(ImmutableSet.of(account));
deleteUserBranch(account.getId());
+ accountCache.evict(account.getId());
}
/** Deletes the account. */
public void deleteByKey(ReviewDb db, Account.Id accountId) throws OrmException, IOException {
db.accounts().deleteKeys(ImmutableSet.of(accountId));
deleteUserBranch(accountId);
+ accountCache.evict(accountId);
}
private void createUserBranch(Account account) throws IOException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java
index 01e16d7..fa4c35b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java
@@ -238,7 +238,9 @@
if (perm instanceof GlobalPermission) {
return can((GlobalPermission) perm);
} else if (perm instanceof PluginPermission) {
- return canPerform(perm.permissionName()) || isAdmin_DoNotUse();
+ PluginPermission pluginPermission = (PluginPermission) perm;
+ return canPerform(pluginPermission.permissionName())
+ || (pluginPermission.fallBackToAdmin() && isAdmin_DoNotUse());
}
throw new PermissionBackendException(perm + " unsupported");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
index d2a9610..230e516 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/ChangeUserName.java
@@ -116,7 +116,6 @@
accountCache.evictByUsername(extId.key().id());
}
- accountCache.evict(user.getAccountId());
accountCache.evictByUsername(newUsername);
sshKeyCache.evict(newUsername);
return VoidResult.INSTANCE;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
index 5ea5e96..d617365 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateAccount.java
@@ -41,7 +41,6 @@
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
import com.google.gerrit.server.api.accounts.AccountExternalIdCreator;
import com.google.gerrit.server.group.GroupsCollection;
-import com.google.gerrit.server.index.account.AccountIndexer;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.gerrit.server.ssh.SshKeyCache;
import com.google.gwtorm.server.OrmDuplicateKeyException;
@@ -70,7 +69,6 @@
private final SshKeyCache sshKeyCache;
private final AccountCache accountCache;
private final AccountsUpdate.User accountsUpdate;
- private final AccountIndexer indexer;
private final AccountByEmailCache byEmailCache;
private final AccountLoader.Factory infoLoader;
private final DynamicSet<AccountExternalIdCreator> externalIdCreators;
@@ -89,7 +87,6 @@
SshKeyCache sshKeyCache,
AccountCache accountCache,
AccountsUpdate.User accountsUpdate,
- AccountIndexer indexer,
AccountByEmailCache byEmailCache,
AccountLoader.Factory infoLoader,
DynamicSet<AccountExternalIdCreator> externalIdCreators,
@@ -105,7 +102,6 @@
this.sshKeyCache = sshKeyCache;
this.accountCache = accountCache;
this.accountsUpdate = accountsUpdate;
- this.indexer = indexer;
this.byEmailCache = byEmailCache;
this.infoLoader = infoLoader;
this.externalIdCreators = externalIdCreators;
@@ -198,7 +194,6 @@
accountCache.evictByUsername(username);
byEmailCache.evict(input.email);
- indexer.index(id);
AccountLoader loader = infoLoader.create(true);
AccountInfo info = loader.get(id);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
index d013120..3a996f2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
@@ -25,7 +25,6 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.DeleteActive.Input;
-import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -39,14 +38,16 @@
public static class Input {}
private final Provider<ReviewDb> dbProvider;
- private final AccountCache byIdCache;
+ private final AccountsUpdate.Server accountsUpdate;
private final Provider<IdentifiedUser> self;
@Inject
DeleteActive(
- Provider<ReviewDb> dbProvider, AccountCache byIdCache, Provider<IdentifiedUser> self) {
+ Provider<ReviewDb> dbProvider,
+ AccountsUpdate.Server accountsUpdate,
+ Provider<IdentifiedUser> self) {
this.dbProvider = dbProvider;
- this.byIdCache = byIdCache;
+ this.accountsUpdate = accountsUpdate;
this.self = self;
}
@@ -58,30 +59,25 @@
}
AtomicBoolean alreadyInactive = new AtomicBoolean(false);
- Account a =
- dbProvider
- .get()
- .accounts()
+ Account account =
+ accountsUpdate
+ .create()
.atomicUpdate(
+ dbProvider.get(),
rsrc.getUser().getAccountId(),
- new AtomicUpdate<Account>() {
- @Override
- public Account update(Account a) {
- if (!a.isActive()) {
- alreadyInactive.set(true);
- } else {
- a.setActive(false);
- }
- return a;
+ a -> {
+ if (!a.isActive()) {
+ alreadyInactive.set(true);
+ } else {
+ a.setActive(false);
}
});
- if (a == null) {
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
if (alreadyInactive.get()) {
throw new ResourceConflictException("account not active");
}
- byIdCache.evict(a.getId());
return Response.none();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
index 4c525c2..bc7c1d0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
@@ -22,7 +22,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.PutActive.Input;
-import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -36,39 +35,34 @@
public static class Input {}
private final Provider<ReviewDb> dbProvider;
- private final AccountCache byIdCache;
+ private final AccountsUpdate.Server accountsUpdate;
@Inject
- PutActive(Provider<ReviewDb> dbProvider, AccountCache byIdCache) {
+ PutActive(Provider<ReviewDb> dbProvider, AccountsUpdate.Server accountsUpdate) {
this.dbProvider = dbProvider;
- this.byIdCache = byIdCache;
+ this.accountsUpdate = accountsUpdate;
}
@Override
public Response<String> apply(AccountResource rsrc, Input input)
throws ResourceNotFoundException, OrmException, IOException {
AtomicBoolean alreadyActive = new AtomicBoolean(false);
- Account a =
- dbProvider
- .get()
- .accounts()
+ Account account =
+ accountsUpdate
+ .create()
.atomicUpdate(
+ dbProvider.get(),
rsrc.getUser().getAccountId(),
- new AtomicUpdate<Account>() {
- @Override
- public Account update(Account a) {
- if (a.isActive()) {
- alreadyActive.set(true);
- } else {
- a.setActive(true);
- }
- return a;
+ a -> {
+ if (a.isActive()) {
+ alreadyActive.set(true);
+ } else {
+ a.setActive(true);
}
});
- if (a == null) {
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
- byIdCache.evict(a.getId());
return alreadyActive.get() ? Response.ok("") : Response.created("");
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java
index 395f078..e00f6b3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java
@@ -59,7 +59,6 @@
private final Provider<CurrentUser> self;
private final PermissionBackend permissionBackend;
- private final AccountCache accountCache;
private final ExternalIds externalIds;
private final ExternalIdsUpdate.User externalIdsUpdate;
@@ -67,12 +66,10 @@
PutHttpPassword(
Provider<CurrentUser> self,
PermissionBackend permissionBackend,
- AccountCache accountCache,
ExternalIds externalIds,
ExternalIdsUpdate.User externalIdsUpdate) {
this.self = self;
this.permissionBackend = permissionBackend;
- this.accountCache = accountCache;
this.externalIds = externalIds;
this.externalIdsUpdate = externalIdsUpdate;
}
@@ -117,7 +114,6 @@
ExternalId newExtId =
ExternalId.createWithPassword(extId.key(), extId.accountId(), extId.email(), newPassword);
externalIdsUpdate.create().upsert(newExtId);
- accountCache.evict(user.getAccountId());
return Strings.isNullOrEmpty(newPassword) ? Response.<String>none() : Response.ok(newPassword);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
index 7a2868e..0911820 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
@@ -30,7 +30,6 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -47,7 +46,7 @@
private final Realm realm;
private final PermissionBackend permissionBackend;
private final Provider<ReviewDb> dbProvider;
- private final AccountCache byIdCache;
+ private final AccountsUpdate.Server accountsUpdate;
@Inject
PutName(
@@ -55,12 +54,12 @@
Realm realm,
PermissionBackend permissionBackend,
Provider<ReviewDb> dbProvider,
- AccountCache byIdCache) {
+ AccountsUpdate.Server accountsUpdate) {
this.self = self;
this.realm = realm;
this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
- this.byIdCache = byIdCache;
+ this.accountsUpdate = accountsUpdate;
}
@Override
@@ -84,25 +83,15 @@
}
String newName = input.name;
- Account a =
- dbProvider
- .get()
- .accounts()
- .atomicUpdate(
- user.getAccountId(),
- new AtomicUpdate<Account>() {
- @Override
- public Account update(Account a) {
- a.setFullName(newName);
- return a;
- }
- });
- if (a == null) {
+ Account account =
+ accountsUpdate
+ .create()
+ .atomicUpdate(dbProvider.get(), user.getAccountId(), a -> a.setFullName(newName));
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
- byIdCache.evict(a.getId());
- return Strings.isNullOrEmpty(a.getFullName())
- ? Response.<String>none()
- : Response.ok(a.getFullName());
+ return Strings.isNullOrEmpty(account.getFullName())
+ ? Response.none()
+ : Response.ok(account.getFullName());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
index 4941cc8..d473d53 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
@@ -26,7 +26,6 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -41,18 +40,18 @@
private final Provider<CurrentUser> self;
private final Provider<ReviewDb> dbProvider;
private final PermissionBackend permissionBackend;
- private final AccountCache byIdCache;
+ private final AccountsUpdate.Server accountsUpdate;
@Inject
PutPreferred(
Provider<CurrentUser> self,
Provider<ReviewDb> dbProvider,
PermissionBackend permissionBackend,
- AccountCache byIdCache) {
+ AccountsUpdate.Server accountsUpdate) {
this.self = self;
this.dbProvider = dbProvider;
this.permissionBackend = permissionBackend;
- this.byIdCache = byIdCache;
+ this.accountsUpdate = accountsUpdate;
}
@Override
@@ -68,27 +67,22 @@
public Response<String> apply(IdentifiedUser user, String email)
throws ResourceNotFoundException, OrmException, IOException {
AtomicBoolean alreadyPreferred = new AtomicBoolean(false);
- Account a =
- dbProvider
- .get()
- .accounts()
+ Account account =
+ accountsUpdate
+ .create()
.atomicUpdate(
+ dbProvider.get(),
user.getAccountId(),
- new AtomicUpdate<Account>() {
- @Override
- public Account update(Account a) {
- if (email.equals(a.getPreferredEmail())) {
- alreadyPreferred.set(true);
- } else {
- a.setPreferredEmail(email);
- }
- return a;
+ a -> {
+ if (email.equals(a.getPreferredEmail())) {
+ alreadyPreferred.set(true);
+ } else {
+ a.setPreferredEmail(email);
}
});
- if (a == null) {
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
- byIdCache.evict(a.getId());
return alreadyPreferred.get() ? Response.ok("") : Response.created("");
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
index 73a720b..81c0694 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -50,18 +49,18 @@
private final Provider<CurrentUser> self;
private final Provider<ReviewDb> dbProvider;
private final PermissionBackend permissionBackend;
- private final AccountCache byIdCache;
+ private final AccountsUpdate.Server accountsUpdate;
@Inject
PutStatus(
Provider<CurrentUser> self,
Provider<ReviewDb> dbProvider,
PermissionBackend permissionBackend,
- AccountCache byIdCache) {
+ AccountsUpdate.Server accountsUpdate) {
this.self = self;
this.dbProvider = dbProvider;
this.permissionBackend = permissionBackend;
- this.byIdCache = byIdCache;
+ this.accountsUpdate = accountsUpdate;
}
@Override
@@ -81,23 +80,18 @@
}
String newStatus = input.status;
- Account a =
- dbProvider
- .get()
- .accounts()
+ Account account =
+ accountsUpdate
+ .create()
.atomicUpdate(
+ dbProvider.get(),
user.getAccountId(),
- new AtomicUpdate<Account>() {
- @Override
- public Account update(Account a) {
- a.setStatus(Strings.nullToEmpty(newStatus));
- return a;
- }
- });
- if (a == null) {
+ a -> a.setStatus(Strings.nullToEmpty(newStatus)));
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
- byIdCache.evict(a.getId());
- return Strings.isNullOrEmpty(a.getStatus()) ? Response.none() : Response.ok(a.getStatus());
+ return Strings.isNullOrEmpty(account.getStatus())
+ ? Response.none()
+ : Response.ok(account.getStatus());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
index c21ff95..9beae0e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
@@ -33,6 +33,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Throwables;
import com.google.common.collect.Iterables;
+import com.google.common.collect.Streams;
import com.google.common.util.concurrent.Runnables;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.metrics.Counter0;
@@ -42,6 +43,7 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LockFailureException;
@@ -53,6 +55,8 @@
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -83,6 +87,9 @@
* </pre>
*
* For NoteDb each method call results in one commit on refs/meta/external-ids branch.
+ *
+ * <p>On updating external IDs this class takes care to evict affected accounts from the account
+ * cache and thus triggers reindex for them.
*/
public class ExternalIdsUpdate {
private static final String COMMIT_MSG = "Update external IDs";
@@ -96,6 +103,7 @@
@Singleton
public static class Server {
private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final MetricMaker metricMaker;
private final ExternalIds externalIds;
@@ -105,12 +113,14 @@
@Inject
public Server(
GitRepositoryManager repoManager,
+ AccountCache accountCache,
AllUsersName allUsersName,
MetricMaker metricMaker,
ExternalIds externalIds,
ExternalIdCache externalIdCache,
@GerritPersonIdent Provider<PersonIdent> serverIdent) {
this.repoManager = repoManager;
+ this.accountCache = accountCache;
this.allUsersName = allUsersName;
this.metricMaker = metricMaker;
this.externalIds = externalIds;
@@ -121,7 +131,7 @@
public ExternalIdsUpdate create() {
PersonIdent i = serverIdent.get();
return new ExternalIdsUpdate(
- repoManager, allUsersName, metricMaker, externalIds, externalIdCache, i, i);
+ repoManager, accountCache, allUsersName, metricMaker, externalIds, externalIdCache, i, i);
}
}
@@ -134,6 +144,7 @@
@Singleton
public static class User {
private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final MetricMaker metricMaker;
private final ExternalIds externalIds;
@@ -144,6 +155,7 @@
@Inject
public User(
GitRepositoryManager repoManager,
+ AccountCache accountCache,
AllUsersName allUsersName,
MetricMaker metricMaker,
ExternalIds externalIds,
@@ -151,6 +163,7 @@
@GerritPersonIdent Provider<PersonIdent> serverIdent,
Provider<IdentifiedUser> identifiedUser) {
this.repoManager = repoManager;
+ this.accountCache = accountCache;
this.allUsersName = allUsersName;
this.metricMaker = metricMaker;
this.externalIds = externalIds;
@@ -163,6 +176,7 @@
PersonIdent i = serverIdent.get();
return new ExternalIdsUpdate(
repoManager,
+ accountCache,
allUsersName,
metricMaker,
externalIds,
@@ -190,6 +204,7 @@
private static final Retryer<RefsMetaExternalIdsUpdate> RETRYER = retryerBuilder().build();
private final GitRepositoryManager repoManager;
+ private final AccountCache accountCache;
private final AllUsersName allUsersName;
private final ExternalIds externalIds;
private final ExternalIdCache externalIdCache;
@@ -201,6 +216,7 @@
private ExternalIdsUpdate(
GitRepositoryManager repoManager,
+ AccountCache accountCache,
AllUsersName allUsersName,
MetricMaker metricMaker,
ExternalIds externalIds,
@@ -209,6 +225,7 @@
PersonIdent authorIdent) {
this(
repoManager,
+ accountCache,
allUsersName,
metricMaker,
externalIds,
@@ -222,6 +239,7 @@
@VisibleForTesting
public ExternalIdsUpdate(
GitRepositoryManager repoManager,
+ AccountCache accountCache,
AllUsersName allUsersName,
MetricMaker metricMaker,
ExternalIds externalIds,
@@ -231,6 +249,7 @@
Runnable afterReadRevision,
Retryer<RefsMetaExternalIdsUpdate> retryer) {
this.repoManager = checkNotNull(repoManager, "repoManager");
+ this.accountCache = accountCache;
this.allUsersName = checkNotNull(allUsersName, "allUsersName");
this.committerIdent = checkNotNull(committerIdent, "committerIdent");
this.externalIds = checkNotNull(externalIds, "externalIds");
@@ -269,6 +288,7 @@
}
});
externalIdCache.onCreate(u.oldRev(), u.newRev(), extIds);
+ evictAccounts(extIds);
}
/**
@@ -295,6 +315,7 @@
}
});
externalIdCache.onUpdate(u.oldRev(), u.newRev(), extIds);
+ evictAccounts(extIds);
}
/**
@@ -324,6 +345,7 @@
}
});
externalIdCache.onRemove(u.oldRev(), u.newRev(), extIds);
+ evictAccounts(extIds);
}
/**
@@ -353,6 +375,7 @@
}
});
externalIdCache.onRemoveByKeys(u.oldRev(), u.newRev(), accountId, extIdKeys);
+ accountCache.evict(accountId);
}
/**
@@ -362,14 +385,19 @@
*/
public void deleteByKeys(Collection<ExternalId.Key> extIdKeys)
throws IOException, ConfigInvalidException, OrmException {
+ Set<ExternalId> deletedExtIds = new HashSet<>();
RefsMetaExternalIdsUpdate u =
updateNoteMap(
o -> {
for (ExternalId.Key extIdKey : extIdKeys) {
- remove(o.rw(), o.noteMap(), extIdKey, null);
+ ExternalId extId = remove(o.rw(), o.noteMap(), extIdKey, null);
+ if (extId != null) {
+ deletedExtIds.add(extId);
+ }
}
});
externalIdCache.onRemoveByKeys(u.oldRev(), u.newRev(), extIdKeys);
+ evictAccounts(deletedExtIds);
}
/** Deletes all external IDs of the specified account. */
@@ -406,6 +434,7 @@
}
});
externalIdCache.onReplaceByKeys(u.oldRev(), u.newRev(), accountId, toDelete, toAdd);
+ accountCache.evict(accountId);
}
/**
@@ -420,11 +449,15 @@
*/
public void replaceByKeys(Collection<ExternalId.Key> toDelete, Collection<ExternalId> toAdd)
throws IOException, ConfigInvalidException, OrmException {
+ Set<ExternalId> deletedExtIds = new HashSet<>();
RefsMetaExternalIdsUpdate u =
updateNoteMap(
o -> {
for (ExternalId.Key extIdKey : toDelete) {
- remove(o.rw(), o.noteMap(), extIdKey, null);
+ ExternalId extId = remove(o.rw(), o.noteMap(), extIdKey, null);
+ if (extId != null) {
+ deletedExtIds.add(extId);
+ }
}
for (ExternalId extId : toAdd) {
@@ -432,6 +465,7 @@
}
});
externalIdCache.onReplaceByKeys(u.oldRev(), u.newRev(), toDelete, toAdd);
+ evictAccounts(Streams.concat(deletedExtIds.stream(), toAdd.stream()).collect(toSet()));
}
/**
@@ -567,13 +601,15 @@
*
* @throws IllegalStateException is thrown if an expected account ID is provided and an external
* ID with the specified key exists, but belongs to another account.
+ * @return the external ID that was removed, {@code null} if no external ID with the specified key
+ * exists
*/
- private static void remove(
+ private static ExternalId remove(
RevWalk rw, NoteMap noteMap, ExternalId.Key extIdKey, Account.Id expectedAccountId)
throws IOException, ConfigInvalidException {
ObjectId noteId = extIdKey.sha1();
if (!noteMap.contains(noteId)) {
- return;
+ return null;
}
byte[] raw =
@@ -589,6 +625,7 @@
extId.accountId().get());
}
noteMap.remove(noteId);
+ return extId;
}
private RefsMetaExternalIdsUpdate updateNoteMap(MyConsumer<OpenRepo> update)
@@ -689,6 +726,12 @@
return ins.insert(OBJ_TREE, new byte[] {});
}
+ private void evictAccounts(Collection<ExternalId> extIds) throws IOException {
+ for (Account.Id id : extIds.stream().map(ExternalId::accountId).collect(toSet())) {
+ accountCache.evict(id);
+ }
+ }
+
@FunctionalInterface
private static interface MyConsumer<T> {
void accept(T t) throws IOException, ConfigInvalidException, OrmException;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index 5391635..80d644f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -51,6 +51,7 @@
import com.google.gerrit.server.mail.send.CreateChangeSender;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchProjectException;
@@ -68,6 +69,7 @@
import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -100,6 +102,7 @@
private final CommitValidators.Factory commitValidatorsFactory;
private final RevisionCreated revisionCreated;
private final CommentAdded commentAdded;
+ private final NotesMigration migration;
private final Change.Id changeId;
private final PatchSet.Id psId;
@@ -147,6 +150,7 @@
CommitValidators.Factory commitValidatorsFactory,
CommentAdded commentAdded,
RevisionCreated revisionCreated,
+ NotesMigration migration,
@Assisted Change.Id changeId,
@Assisted ObjectId commitId,
@Assisted String refName) {
@@ -162,6 +166,7 @@
this.commitValidatorsFactory = commitValidatorsFactory;
this.revisionCreated = revisionCreated;
this.commentAdded = commentAdded;
+ this.migration = migration;
this.changeId = changeId;
this.psId = new PatchSet.Id(changeId, INITIAL_PATCH_SET_ID);
@@ -408,6 +413,14 @@
*/
update.fixStatus(change.getStatus());
+ Set<Account.Id> reviewersToAdd = new HashSet<>(reviewers);
+ if (migration.readChanges()) {
+ approvalsUtil.addCcs(
+ ctx.getNotes(), update, filterOnChangeVisibility(db, ctx.getNotes(), extraCC));
+ } else {
+ reviewersToAdd.addAll(extraCC);
+ }
+
LabelTypes labelTypes = ctl.getProjectControl().getLabelTypes();
approvalsUtil.addReviewers(
db,
@@ -416,7 +429,7 @@
change,
patchSet,
patchSetInfo,
- filterOnChangeVisibility(db, ctx.getNotes(), reviewers),
+ filterOnChangeVisibility(db, ctx.getNotes(), reviewersToAdd),
Collections.<Account.Id>emptySet());
approvalsUtil.addApprovalsForNewPatchSet(
db, update, labelTypes, patchSet, ctx.getControl(), approvals);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
index e1ccff9..ca70b0e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -44,6 +44,7 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.PatchSetState;
@@ -105,6 +106,7 @@
private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeNotes.Factory notesFactory;
+ private final Accounts accounts;
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
private final GitRepositoryManager repoManager;
private final PatchSetInfoFactory patchSetInfoFactory;
@@ -134,6 +136,7 @@
@GerritPersonIdent Provider<PersonIdent> serverIdent,
ChangeControl.GenericFactory changeControlFactory,
ChangeNotes.Factory notesFactory,
+ Accounts accounts,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore,
GitRepositoryManager repoManager,
PatchSetInfoFactory patchSetInfoFactory,
@@ -142,6 +145,7 @@
Provider<CurrentUser> user,
Provider<ReviewDb> db,
RetryHelper retryHelper) {
+ this.accounts = accounts;
this.accountPatchReviewStore = accountPatchReviewStore;
this.changeControlFactory = changeControlFactory;
this.db = db;
@@ -219,7 +223,7 @@
private void checkOwner() {
try {
- if (db.get().accounts().get(change().getOwner()) == null) {
+ if (accounts.get(db.get(), change().getOwner()) == null) {
problem("Missing change owner: " + change().getOwner());
}
} catch (OrmException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
index 039aa9e..56d54ee 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Revert.java
@@ -37,10 +37,12 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.Sequences;
import com.google.gerrit.server.extensions.events.ChangeReverted;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.mail.send.RevertedSender;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectControl;
@@ -206,12 +208,18 @@
.setTopic(changeToRevert.getTopic());
ins.setMessage("Uploaded patch set 1.");
+ ReviewerSet reviewerSet = approvalsUtil.getReviewers(db.get(), ctl.getNotes());
+
Set<Account.Id> reviewers = new HashSet<>();
reviewers.add(changeToRevert.getOwner());
- reviewers.addAll(approvalsUtil.getReviewers(db.get(), ctl.getNotes()).all());
+ reviewers.addAll(reviewerSet.byState(ReviewerStateInternal.REVIEWER));
reviewers.remove(user.getAccountId());
ins.setReviewers(reviewers);
+ Set<Account.Id> ccs = new HashSet<>(reviewerSet.byState(ReviewerStateInternal.CC));
+ ccs.remove(user.getAccountId());
+ ins.setExtraCC(ccs);
+
try (BatchUpdate bu = updateFactory.create(db.get(), project, user, now)) {
bu.setRepository(git, revWalk, oi);
bu.insertChange(ins);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 2956a4e..b7c017c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -82,8 +82,9 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.Sequences;
-import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.SetHashtagsOp;
import com.google.gerrit.server.config.AllProjectsName;
@@ -299,6 +300,8 @@
private final Sequences seq;
private final Provider<InternalChangeQuery> queryProvider;
private final ChangeNotes.Factory notesFactory;
+ private final Accounts accounts;
+ private final AccountsUpdate.Server accountsUpdate;
private final AccountResolver accountResolver;
private final PermissionBackend permissionBackend;
private final PermissionBackend.ForProject permissions;
@@ -310,7 +313,6 @@
private final CommitValidators.Factory commitValidatorsFactory;
private final RefOperationValidators.Factory refValidatorsFactory;
private final TagCache tagCache;
- private final AccountCache accountCache;
private final ChangeInserter.Factory changeInserterFactory;
private final RequestScopePropagator requestScopePropagator;
private final SshInfo sshInfo;
@@ -373,6 +375,8 @@
Sequences seq,
Provider<InternalChangeQuery> queryProvider,
ChangeNotes.Factory notesFactory,
+ Accounts accounts,
+ AccountsUpdate.Server accountsUpdate,
AccountResolver accountResolver,
PermissionBackend permissionBackend,
CmdLineParser.Factory optionParserFactory,
@@ -380,7 +384,6 @@
PatchSetUtil psUtil,
ProjectCache projectCache,
TagCache tagCache,
- AccountCache accountCache,
@Nullable SearchingChangeCacheImpl changeCache,
ChangeInserter.Factory changeInserterFactory,
CommitValidators.Factory commitValidatorsFactory,
@@ -412,6 +415,8 @@
this.seq = seq;
this.queryProvider = queryProvider;
this.notesFactory = notesFactory;
+ this.accounts = accounts;
+ this.accountsUpdate = accountsUpdate;
this.accountResolver = accountResolver;
this.permissionBackend = permissionBackend;
this.optionParserFactory = optionParserFactory;
@@ -420,7 +425,6 @@
this.projectCache = projectCache;
this.canonicalWebUrl = canonicalWebUrl;
this.tagCache = tagCache;
- this.accountCache = accountCache;
this.changeInserterFactory = changeInserterFactory;
this.commitValidatorsFactory = commitValidatorsFactory;
this.refValidatorsFactory = refValidatorsFactory;
@@ -1334,7 +1338,7 @@
if (!hashtag.isEmpty()) {
hashtags.add(hashtag);
}
- //TODO(dpursehouse): validate hashtags
+ // TODO(dpursehouse): validate hashtags
}
MagicBranchInput(
@@ -2737,12 +2741,11 @@
if (defaultName && user.hasEmailAddress(c.getCommitterIdent().getEmailAddress())) {
try {
- Account a = db.accounts().get(user.getAccountId());
+ Account a = accounts.get(db, user.getAccountId());
if (a != null && Strings.isNullOrEmpty(a.getFullName())) {
a.setFullName(c.getCommitterIdent().getName());
- db.accounts().update(Collections.singleton(a));
+ accountsUpdate.create().update(db, a);
user.getAccount().setFullName(a.getFullName());
- accountCache.evict(a.getId());
}
} catch (OrmException e) {
logWarn("Cannot default full_name", e);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java
index 0c4eb89..6d5824f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/RefOperationValidators.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.IdentifiedUser;
@@ -68,7 +69,7 @@
List<ValidationMessage> messages = new ArrayList<>();
boolean withException = false;
List<RefOperationValidationListener> listeners = new ArrayList<>();
- listeners.add(new DisallowDeletionOfUserBranches(allUsersName));
+ listeners.add(new DisallowCreationAndDeletionOfUserBranches(allUsersName));
refOperationValidationListeners.forEach(l -> listeners.add(l));
try {
for (RefOperationValidationListener listener : listeners) {
@@ -104,10 +105,11 @@
}
}
- private static class DisallowDeletionOfUserBranches implements RefOperationValidationListener {
+ private static class DisallowCreationAndDeletionOfUserBranches
+ implements RefOperationValidationListener {
private final AllUsersName allUsersName;
- DisallowDeletionOfUserBranches(AllUsersName allUsersName) {
+ DisallowCreationAndDeletionOfUserBranches(AllUsersName allUsersName) {
this.allUsersName = allUsersName;
}
@@ -116,10 +118,20 @@
throws ValidationException {
if (refEvent.project.getNameKey().equals(allUsersName)
&& (refEvent.command.getRefName().startsWith(RefNames.REFS_USERS)
- && !refEvent.command.getRefName().equals(RefNames.REFS_USERS_DEFAULT))
- && refEvent.command.getType().equals(ReceiveCommand.Type.DELETE)) {
- if (!refEvent.user.getCapabilities().canAccessDatabase()) {
- throw new ValidationException("Not allowed to delete user branch.");
+ && !refEvent.command.getRefName().equals(RefNames.REFS_USERS_DEFAULT))) {
+ if (refEvent.command.getType().equals(ReceiveCommand.Type.CREATE)) {
+ if (!refEvent.user.getCapabilities().canAccessDatabase()) {
+ throw new ValidationException("Not allowed to create user branch.");
+ }
+ if (Account.Id.fromRef(refEvent.command.getRefName()) == null) {
+ throw new ValidationException(
+ String.format(
+ "Not allowed to create non-user branch under %s.", RefNames.REFS_USERS));
+ }
+ } else if (refEvent.command.getType().equals(ReceiveCommand.Type.DELETE)) {
+ if (!refEvent.user.getCapabilities().canAccessDatabase()) {
+ throw new ValidationException("Not allowed to delete user branch.");
+ }
}
}
return ImmutableList.of();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java
index 36a409a..47a8cad 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/account/AllAccountsIndexer.java
@@ -21,15 +21,14 @@
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.index.IndexExecutor;
import com.google.gerrit.server.index.SiteIndexer;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import java.io.IOException;
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.List;
@@ -45,17 +44,17 @@
public class AllAccountsIndexer extends SiteIndexer<Account.Id, AccountState, AccountIndex> {
private static final Logger log = LoggerFactory.getLogger(AllAccountsIndexer.class);
- private final SchemaFactory<ReviewDb> schemaFactory;
private final ListeningExecutorService executor;
+ private final Accounts accounts;
private final AccountCache accountCache;
@Inject
AllAccountsIndexer(
- SchemaFactory<ReviewDb> schemaFactory,
@IndexExecutor(BATCH) ListeningExecutorService executor,
+ Accounts accounts,
AccountCache accountCache) {
- this.schemaFactory = schemaFactory;
this.executor = executor;
+ this.accounts = accounts;
this.accountCache = accountCache;
}
@@ -67,7 +66,7 @@
List<Account.Id> ids;
try {
ids = collectAccounts(progress);
- } catch (OrmException e) {
+ } catch (IOException e) {
log.error("Error collecting accounts", e);
return new SiteIndexer.Result(sw, false, 0, 0);
}
@@ -113,13 +112,12 @@
return new SiteIndexer.Result(sw, ok.get(), done.get(), failed.get());
}
- private List<Account.Id> collectAccounts(ProgressMonitor progress) throws OrmException {
+ private List<Account.Id> collectAccounts(ProgressMonitor progress) throws IOException {
progress.beginTask("Collecting accounts", ProgressMonitor.UNKNOWN);
List<Account.Id> ids = new ArrayList<>();
- try (ReviewDb db = schemaFactory.open()) {
- for (Account account : db.accounts().all()) {
- ids.add(account.getId());
- }
+ for (Account.Id accountId : accounts.allIds()) {
+ ids.add(accountId);
+ progress.update(1);
}
progress.endTask();
return ids;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MetadataName.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MetadataName.java
index 3db55c0..3080e4f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/MetadataName.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/MetadataName.java
@@ -15,7 +15,7 @@
package com.google.gerrit.server.mail;
public final class MetadataName {
- public static final String CHANGE_ID = "Gerrit-Change-Id";
+ public static final String CHANGE_NUMBER = "Gerrit-Change-Number";
public static final String PATCH_SET = "Gerrit-PatchSet";
public static final String MESSAGE_TYPE = "Gerrit-MessageType";
public static final String TIMESTAMP = "Gerrit-Comment-Date";
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailMetadata.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailMetadata.java
index f85291c..04c2add 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailMetadata.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailMetadata.java
@@ -19,14 +19,14 @@
/** MailMetadata represents metadata parsed from inbound email. */
public class MailMetadata {
- public String changeId;
+ public Integer changeNumber;
public Integer patchSet;
public String author; // Author of the email
public Timestamp timestamp;
public String messageType; // we expect comment here
public boolean hasRequiredFields() {
- return changeId != null
+ return changeNumber != null
&& patchSet != null
&& author != null
&& timestamp != null
@@ -36,7 +36,7 @@
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
- .add("Change-Id", changeId)
+ .add("Change-Number", changeNumber)
.add("Patch-Set", patchSet)
.add("Author", author)
.add("Timestamp", timestamp)
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 0c2cf04..7ad282b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -169,12 +169,12 @@
try (ManualRequestContext ctx = oneOffRequestContext.openAs(account)) {
List<ChangeData> changeDataList =
- queryProvider.get().byKey(Change.Key.parse(metadata.changeId));
+ queryProvider.get().byLegacyChangeId(new Change.Id(metadata.changeNumber));
if (changeDataList.size() != 1) {
log.error(
String.format(
"Message %s references unique change %s, but there are %d matching changes in the index. Will delete message.",
- message.id(), metadata.changeId, changeDataList.size()));
+ message.id(), metadata.changeNumber, changeDataList.size()));
return;
}
ChangeData cd = changeDataList.get(0);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MetadataParser.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MetadataParser.java
index a18da68..7085051 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MetadataParser.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/receive/MetadataParser.java
@@ -38,9 +38,9 @@
// Check email headers for X-Gerrit-<Name>
for (String header : m.additionalHeaders()) {
- if (header.startsWith(toHeaderWithDelimiter(MetadataName.CHANGE_ID))) {
- metadata.changeId =
- header.substring(toHeaderWithDelimiter(MetadataName.CHANGE_ID).length());
+ if (header.startsWith(toHeaderWithDelimiter(MetadataName.CHANGE_NUMBER))) {
+ String num = header.substring(toHeaderWithDelimiter(MetadataName.CHANGE_NUMBER).length());
+ metadata.changeNumber = Ints.tryParse(num);
} else if (header.startsWith(toHeaderWithDelimiter(MetadataName.PATCH_SET))) {
String ps = header.substring(toHeaderWithDelimiter(MetadataName.PATCH_SET).length());
metadata.patchSet = Ints.tryParse(ps);
@@ -84,8 +84,9 @@
private static void extractFooters(String[] lines, MailMetadata metadata, MailMessage m) {
for (String line : lines) {
- if (metadata.changeId == null && line.contains(MetadataName.CHANGE_ID)) {
- metadata.changeId = extractFooter(toFooterWithDelimiter(MetadataName.CHANGE_ID), line);
+ if (metadata.changeNumber == null && line.contains(MetadataName.CHANGE_NUMBER)) {
+ metadata.changeNumber =
+ Ints.tryParse(extractFooter(toFooterWithDelimiter(MetadataName.CHANGE_NUMBER), line));
} else if (metadata.patchSet == null && line.contains(MetadataName.PATCH_SET)) {
metadata.patchSet =
Ints.tryParse(extractFooter(toFooterWithDelimiter(MetadataName.PATCH_SET), line));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/GlobalPermission.java b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/GlobalPermission.java
index 926057b..4d293c8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/permissions/GlobalPermission.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/permissions/GlobalPermission.java
@@ -91,11 +91,24 @@
throw new PermissionBackendException("cannot extract permission");
} else if (rc != null) {
return Collections.singleton(
- resolve(pluginName, rc.value(), rc.scope(), clazz, RequiresCapability.class));
+ resolve(
+ pluginName,
+ rc.value(),
+ rc.scope(),
+ rc.fallBackToAdmin(),
+ clazz,
+ RequiresCapability.class));
} else if (rac != null) {
Set<GlobalOrPluginPermission> r = new LinkedHashSet<>();
for (String capability : rac.value()) {
- r.add(resolve(pluginName, capability, rac.scope(), clazz, RequiresAnyCapability.class));
+ r.add(
+ resolve(
+ pluginName,
+ capability,
+ rac.scope(),
+ rac.fallBackToAdmin(),
+ clazz,
+ RequiresAnyCapability.class));
}
return Collections.unmodifiableSet(r);
} else {
@@ -129,13 +142,14 @@
@Nullable String pluginName,
String capability,
CapabilityScope scope,
+ boolean fallBackToAdmin,
Class<?> clazz,
Class<?> annotationClass)
throws PermissionBackendException {
if (pluginName != null
&& !"gerrit".equals(pluginName)
&& (scope == CapabilityScope.PLUGIN || scope == CapabilityScope.CONTEXT)) {
- return new PluginPermission(pluginName, capability);
+ return new PluginPermission(pluginName, capability, fallBackToAdmin);
}
if (scope == CapabilityScope.PLUGIN) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
index b68c81a..7fe89cd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
+++ b/gerrit-server/src/main/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_151> C = Schema_151.class;
+ public static final Class<Schema_152> C = Schema_152.class;
public static int getBinaryVersion() {
return guessVersion(C);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_152.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_152.java
new file mode 100644
index 0000000..2ed7273
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_152.java
@@ -0,0 +1,41 @@
+// 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.gerrit.reviewdb.server.ReviewDb;
+import com.google.gwtorm.jdbc.JdbcSchema;
+import com.google.gwtorm.schema.sql.SqlDialect;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.StatementExecutor;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.sql.SQLException;
+
+/** Drop unused indexes from accounts table. */
+public class Schema_152 extends SchemaVersion {
+ @Inject
+ Schema_152(Provider<Schema_151> prior) {
+ super(prior);
+ }
+
+ @Override
+ protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
+ JdbcSchema schema = (JdbcSchema) db;
+ SqlDialect dialect = schema.getDialect();
+ try (StatementExecutor e = newExecutor(db)) {
+ dialect.dropIndex(e, "accounts", "accounts_byFullName");
+ }
+ }
+}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/MetadataParserTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/MetadataParserTest.java
index 0c4507e..84bae96 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/MetadataParserTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/mail/receive/MetadataParserTest.java
@@ -34,7 +34,7 @@
b.dateReceived(new DateTime());
b.subject("");
- b.addAdditionalHeader(toHeaderWithDelimiter(MetadataName.CHANGE_ID) + "cid");
+ b.addAdditionalHeader(toHeaderWithDelimiter(MetadataName.CHANGE_NUMBER) + "123");
b.addAdditionalHeader(toHeaderWithDelimiter(MetadataName.PATCH_SET) + "1");
b.addAdditionalHeader(toHeaderWithDelimiter(MetadataName.MESSAGE_TYPE) + "comment");
b.addAdditionalHeader(
@@ -45,7 +45,7 @@
MailMetadata meta = MetadataParser.parse(b.build());
assertThat(meta.author).isEqualTo(author.getEmail());
- assertThat(meta.changeId).isEqualTo("cid");
+ assertThat(meta.changeNumber).isEqualTo(123);
assertThat(meta.patchSet).isEqualTo(1);
assertThat(meta.messageType).isEqualTo("comment");
assertThat(meta.timestamp.getTime())
@@ -62,7 +62,7 @@
b.subject("");
StringBuilder stringBuilder = new StringBuilder();
- stringBuilder.append(toFooterWithDelimiter(MetadataName.CHANGE_ID) + "cid\r\n");
+ stringBuilder.append(toFooterWithDelimiter(MetadataName.CHANGE_NUMBER) + "123\r\n");
stringBuilder.append("> " + toFooterWithDelimiter(MetadataName.PATCH_SET) + "1\n");
stringBuilder.append(toFooterWithDelimiter(MetadataName.MESSAGE_TYPE) + "comment\n");
stringBuilder.append(
@@ -74,7 +74,7 @@
MailMetadata meta = MetadataParser.parse(b.build());
assertThat(meta.author).isEqualTo(author.getEmail());
- assertThat(meta.changeId).isEqualTo("cid");
+ assertThat(meta.changeNumber).isEqualTo(123);
assertThat(meta.patchSet).isEqualTo(1);
assertThat(meta.messageType).isEqualTo("comment");
assertThat(meta.timestamp.getTime())
@@ -92,7 +92,7 @@
StringBuilder stringBuilder = new StringBuilder();
stringBuilder.append(
- "<div id\"someid\">" + toFooterWithDelimiter(MetadataName.CHANGE_ID) + "cid</div>");
+ "<div id\"someid\">" + toFooterWithDelimiter(MetadataName.CHANGE_NUMBER) + "123</div>");
stringBuilder.append("<div>" + toFooterWithDelimiter(MetadataName.PATCH_SET) + "1</div>");
stringBuilder.append(
"<div>" + toFooterWithDelimiter(MetadataName.MESSAGE_TYPE) + "comment</div>");
@@ -108,7 +108,7 @@
MailMetadata meta = MetadataParser.parse(b.build());
assertThat(meta.author).isEqualTo(author.getEmail());
- assertThat(meta.changeId).isEqualTo("cid");
+ assertThat(meta.changeNumber).isEqualTo(123);
assertThat(meta.patchSet).isEqualTo(1);
assertThat(meta.messageType).isEqualTo("comment");
assertThat(meta.timestamp.getTime())
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
index a83db4a..042865e 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/account/AbstractQueryAccountsTest.java
@@ -18,7 +18,7 @@
import static java.util.stream.Collectors.toList;
import static org.junit.Assert.fail;
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.accounts.Accounts.QueryRequest;
@@ -37,6 +37,8 @@
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.schema.SchemaCreator;
@@ -53,7 +55,6 @@
import com.google.inject.util.Providers;
import java.util.ArrayList;
import java.util.Arrays;
-import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import org.eclipse.jgit.lib.Config;
@@ -71,6 +72,10 @@
return cfg;
}
+ @Inject protected Accounts accounts;
+
+ @Inject protected AccountsUpdate.Server accountsUpdate;
+
@Inject protected AccountCache accountCache;
@Inject protected AccountManager accountManager;
@@ -380,9 +385,9 @@
// update account in the database so that account index is stale
String newName = "Test User";
- Account account = db.accounts().get(new Account.Id(user1._accountId));
+ Account account = accounts.get(db, new Account.Id(user1._accountId));
account.setFullName(newName);
- db.accounts().update(Collections.singleton(account));
+ db.accounts().update(ImmutableSet.of(account));
assertQuery("name:" + quote(user1.name), user1);
assertQuery("name:" + quote(newName));
@@ -464,12 +469,11 @@
if (email != null) {
accountManager.link(id, AuthRequest.forEmail(email));
}
- Account a = db.accounts().get(id);
+ Account a = accounts.get(db, id);
a.setFullName(fullName);
a.setPreferredEmail(email);
a.setActive(active);
- db.accounts().update(ImmutableList.of(a));
- accountCache.evict(id);
+ accountsUpdate.create().update(db, a);
return id;
}
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index c58162f..7853b2d 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -70,6 +70,8 @@
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager;
+import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.externalids.ExternalId;
import com.google.gerrit.server.account.externalids.ExternalIdsUpdate;
@@ -147,7 +149,9 @@
return cfg;
}
+ @Inject protected Accounts accounts;
@Inject protected AccountCache accountCache;
+ @Inject protected AccountsUpdate.Server accountsUpdate;
@Inject protected AccountManager accountManager;
@Inject protected AllUsersName allUsersName;
@Inject protected BatchUpdate.Factory updateFactory;
@@ -204,11 +208,11 @@
db = schemaFactory.open();
userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId();
- Account userAccount = db.accounts().get(userId);
+ Account userAccount = accounts.get(db, userId);
String email = "user@example.com";
externalIdsUpdate.create().insert(ExternalId.createEmail(userId, email));
userAccount.setPreferredEmail(email);
- db.accounts().update(ImmutableList.of(userAccount));
+ accountsUpdate.create().update(db, userAccount);
user = userFactory.create(userId);
requestContext.setContext(newRequestContext(userAccount.getId()));
}
@@ -2304,12 +2308,11 @@
if (email != null) {
accountManager.link(id, AuthRequest.forEmail(email));
}
- Account a = db.accounts().get(id);
+ Account a = accounts.get(db, id);
a.setFullName(fullName);
a.setPreferredEmail(email);
a.setActive(active);
- db.accounts().update(ImmutableList.of(a));
- accountCache.evict(id);
+ accountsUpdate.create().update(db, a);
return id;
}
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
index b508f8b..fe2a8f3 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/group/AbstractQueryGroupsTest.java
@@ -18,7 +18,6 @@
import static java.util.stream.Collectors.toList;
import com.google.common.base.CharMatcher;
-import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.api.groups.Groups.QueryRequest;
@@ -34,6 +33,8 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountManager;
+import com.google.gerrit.server.account.Accounts;
+import com.google.gerrit.server.account.AccountsUpdate;
import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.config.AllProjectsName;
@@ -70,6 +71,10 @@
return cfg;
}
+ @Inject protected Accounts accounts;
+
+ @Inject protected AccountsUpdate.Server accountsUpdate;
+
@Inject protected AccountCache accountCache;
@Inject protected AccountManager accountManager;
@@ -314,12 +319,11 @@
if (email != null) {
accountManager.link(id, AuthRequest.forEmail(email));
}
- Account a = db.accounts().get(id);
+ Account a = accounts.get(db, id);
a.setFullName(fullName);
a.setPreferredEmail(email);
a.setActive(active);
- db.accounts().update(ImmutableList.of(a));
- accountCache.evict(id);
+ accountsUpdate.create().update(db, a);
return id;
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index 358340d..976d980 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -1196,9 +1196,8 @@
*/
_updateRelatedChangeMaxHeight() {
// Takes into account approximate height for the expand button and
- // bottom margin
- const extraHeight = 24;
- let maxExistingHeight;
+ // bottom margin.
+ const EXTRA_HEIGHT = 30;
let newHeight;
const hasCommitToggle =
!this._computeCommitToggleHidden(this._latestCommitMessage);
@@ -1216,7 +1215,7 @@
// Note: extraHeight is to take into account margin/padding.
const medRelatedHeight = Math.max(
this._getOffsetHeight(this.$.mainChangeInfo) -
- this._getOffsetHeight(this.$.commitMessage) - 2 * extraHeight,
+ this._getOffsetHeight(this.$.commitMessage) - 2 * EXTRA_HEIGHT,
MINIMUM_RELATED_MAX_HEIGHT);
newHeight = medRelatedHeight;
} else {
@@ -1224,29 +1223,30 @@
// Make sure the content is lined up if both areas have buttons. If
// the commit message is not collapsed, instead use the change info
// height.
- maxExistingHeight = this._getOffsetHeight(this.$.commitMessage);
+ newHeight = this._getOffsetHeight(this.$.commitMessage);
} else {
- maxExistingHeight = this._getOffsetHeight(this.$.mainChangeInfo) -
- extraHeight;
- }
- // Get the line height of related changes, and convert it to the nearest
- // integer.
- const lineHeight = this._getLineHeight(this.$.relatedChanges);
-
- // Figure out a new height that is divisible by the rounded line height.
- const remainder = maxExistingHeight % lineHeight;
- newHeight = maxExistingHeight - remainder;
-
- // Update the max-height of the relation chain to this new height;
- if (hasCommitToggle) {
- this.customStyle['--related-change-btn-top-padding'] =
- remainder + 'px';
+ newHeight = this._getOffsetHeight(this.$.commitAndRelated) -
+ EXTRA_HEIGHT;
}
}
if (this.$.relatedChanges.hidden) {
this.customStyle['--commit-message-max-width'] = 'none';
}
+ // Get the line height of related changes, and convert it to the nearest
+ // integer.
+ const lineHeight = this._getLineHeight(this.$.relatedChanges);
+
+ // Figure out a new height that is divisible by the rounded line height.
+ const remainder = newHeight % lineHeight;
+ newHeight = newHeight - remainder;
+
this.customStyle['--relation-chain-max-height'] = newHeight + 'px';
+
+ // Update the max-height of the relation chain to this new height.
+ if (hasCommitToggle) {
+ this.customStyle['--related-change-btn-top-padding'] =
+ remainder + 'px';
+ }
this.updateStyles();
},
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index e91f372..0cc9994 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -1203,13 +1203,13 @@
sandbox.stub(element, '_getLineHeight', () => 12);
sandbox.stub(window, 'matchMedia', () => ({matches: false}));
- // 50 (existing height) - 24 (extra height) = 26 (adjusted height).
- // 50 (existing height) % 12 (line height) = 2 (remainder).
- // 26 (adjusted height) - 2 (remainder) = 24 (max height to set).
+ // 50 (existing height) - 30 (extra height) = 20 (adjusted height).
+ // 20 (max existing height) % 12 (line height) = 6 (remainder).
+ // 20 (adjusted height) - 8 (remainder) = 12 (max height to set).
element._updateRelatedChangeMaxHeight();
assert.equal(element.customStyle['--relation-chain-max-height'],
- '24px');
+ '12px');
assert.equal(element.customStyle['--related-change-btn-top-padding'],
undefined);
});
@@ -1237,8 +1237,12 @@
sandbox.stub(window, 'matchMedia', () => ({matches: true}));
element._updateRelatedChangeMaxHeight();
+
+ // 400 (new height) % 12 (line height) = 4 (remainder).
+ // 400 (new height) - 4 (remainder) = 396.
+
assert.equal(element.customStyle['--relation-chain-max-height'],
- '400px');
+ '396px');
});
test('_updateRelatedChangeMaxHeight in medium screen mode', () => {
@@ -1253,9 +1257,11 @@
}
});
+ // 100 (new height) % 12 (line height) = 4 (remainder).
+ // 100 (new height) - 4 (remainder) = 96.
element._updateRelatedChangeMaxHeight();
assert.equal(element.customStyle['--relation-chain-max-height'],
- '100px');
+ '96px');
});
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
index aa9c5fd..9094933 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.html
@@ -16,6 +16,7 @@
<link rel="import" href="../../../behaviors/base-url-behavior/base-url-behavior.html">
<link rel="import" href="../../../behaviors/gr-path-list-behavior/gr-path-list-behavior.html">
+<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../shared/gr-formatted-text/gr-formatted-text.html">
<link rel="import" href="../../../styles/shared-styles.html">
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
index ab2ad11..d8cdb02 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list.js
@@ -23,6 +23,7 @@
behaviors: [
Gerrit.BaseUrlBehavior,
Gerrit.PathListBehavior,
+ Gerrit.URLEncodingBehavior,
],
properties: {
@@ -38,7 +39,8 @@
},
_computeFileDiffURL(file, changeNum, patchNum) {
- return `${this.getBaseUrl()}/c/${changeNum}/${patchNum}/${file}`;
+ return this.getBaseUrl() + '/c/' + changeNum + '/' + patchNum + '/' +
+ this.encodeURL(file);
},
_computeFileDisplayName(path) {
diff --git a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
index 4c8b443..d25664e 100644
--- a/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-comment-list/gr-comment-list_test.html
@@ -61,9 +61,8 @@
});
test('_computeFileDiffURL', () => {
- const expected = '/c/<change>/<patch>/<file>';
- const actual =
- element._computeFileDiffURL('<file>', '<change>', '<patch>');
+ const expected = '/c/change/patch/file';
+ const actual = element._computeFileDiffURL('file', 'change', 'patch');
assert.equal(actual, expected);
});
@@ -78,17 +77,23 @@
test('_computeDiffLineURL', () => {
const comment = {line: 123, side: 'REVISION', patch_set: 10};
- let expected = '/c/<change>/<patch>/<file>#123';
- let actual = element._computeDiffLineURL('<file>', '<change>', '<patch>',
+ let expected = '/c/change/patch/file#123';
+ let actual = element._computeDiffLineURL('file', 'change', 'patch',
comment);
assert.equal(actual, expected);
comment.line = 321;
comment.side = 'PARENT';
- expected = '/c/<change>/<patch>/<file>#b321';
- actual = element._computeDiffLineURL('<file>', '<change>', '<patch>',
- comment);
+ expected = '/c/change/patch/file#b321';
+ actual = element._computeDiffLineURL('file', 'change', 'patch', comment);
+ });
+
+ test('_computeDiffLineURL encoding', () => {
+ const comment = {line: 123, side: 'REVISION', patch_set: 10};
+ const expected = '/c/123/2/x%252By.md#123';
+ const actual = element._computeDiffLineURL('x+y.md', '123', '2', comment);
+ assert.equal(actual, expected);
});
test('_computePatchDisplayName', () => {
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 81135ae..c0371ee 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
@@ -246,7 +246,7 @@
as="file"
initial-count="[[fileListIncrement]]"
target-framerate="1">
- <div class="file-row row" data-path$="[[file.__path]]">
+ <div class="file-row row" data-path$="[[file.__path]]" tabindex="-1">
<div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
<input type="checkbox" checked="[[file.isReviewed]]"
class="reviewed" aria-label="Reviewed checkbox">
@@ -401,6 +401,7 @@
<gr-cursor-manager
id="fileCursor"
scroll-behavior="keep-visible"
+ focus-on-move
cursor-target-class="selected"></gr-cursor-manager>
<gr-reporting id="reporting"></gr-reporting>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 7621a9d..0bcd9fd 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -135,7 +135,7 @@
'c': '_handleCKey',
'[': '_handleLeftBracketKey',
']': '_handleRightBracketKey',
- 'o enter': '_handleEnterKey',
+ 'o': '_handleOKey',
'n': '_handleNKey',
'p': '_handlePKey',
'shift+a': '_handleCapitalAKey',
@@ -537,14 +537,10 @@
this._openSelectedFile(0);
},
- _handleEnterKey(e) {
+ _handleOKey(e) {
if (this.shouldSuppressKeyboardShortcut(e) ||
this.modifierPressed(e)) { return; }
- // Use native handling if an anchor is selected. @see Issue 5754
- if (e.detail && e.detail.keyboardEvent && e.detail.keyboardEvent.target &&
- e.detail.keyboardEvent.target.tagName === 'A') { return; }
-
e.preventDefault();
if (this._showInlineDiffs) {
this._openCursorFile();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index f0f5403..4577a05 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -331,9 +331,7 @@
const showStub = sandbox.stub(page, 'show');
assert.equal(element.$.fileCursor.index, 2);
assert.equal(element.selectedIndex, 2);
- MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter');
- assert(showStub.lastCall.calledWith('/c/42/2/myfile.txt'),
- 'Should navigate to /c/42/2/myfile.txt');
+
// k with a modifier should not move the cursor.
MockInteractions.pressAndReleaseKeyOn(element, 75, 'shift', 'k');
assert.equal(element.$.fileCursor.index, 2);
@@ -384,7 +382,7 @@
}
});
- suite('_handleEnterKey', () => {
+ suite('_handleOKey', () => {
let interact;
setup(() => {
@@ -402,7 +400,7 @@
const e = new CustomEvent('fake-keyboard-event', opt_payload);
sinon.stub(e, 'preventDefault');
- element._handleEnterKey(e);
+ element._handleOKey(e);
assert.isTrue(e.preventDefault.called);
const result = {};
if (openCursorStub.called) {
@@ -440,14 +438,6 @@
element._userPrefs.expand_inline_diffs = true;
assert.deepEqual(interact(), {expanded: true});
});
-
- test('noop when anchor focused', () => {
- const e = new CustomEvent('fake-keyboard-event',
- {detail: {keyboardEvent: {target: document.createElement('a')}}});
- sinon.stub(e, 'preventDefault');
- element._handleEnterKey(e);
- assert.isFalse(e.preventDefault.called);
- });
});
});
@@ -1242,4 +1232,6 @@
assert.isFalse(element._displayLine);
});
});
+
+ a11ySuite('basic');
</script>
diff --git a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
index 1396a2b..fef0f87 100644
--- a/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
+++ b/polygerrit-ui/app/elements/core/gr-keyboard-shortcuts-dialog/gr-keyboard-shortcuts-dialog.html
@@ -231,7 +231,7 @@
<td>Select previous file</td>
</tr>
<tr>
- <td><span class="key">Enter</span> or <span class="key">o</span></td>
+ <td><span class="key">o</span></td>
<td>Show selected file</td>
</tr>
<tr>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
index 6f47f9b..9695b18 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
@@ -66,6 +66,7 @@
row.classList.add('diff-row', 'side-by-side');
row.setAttribute('left-type', leftLine.type);
row.setAttribute('right-type', rightLine.type);
+ row.tabIndex = -1;
this._appendPair(section, row, leftLine, leftLine.beforeNumber,
GrDiffBuilder.Side.LEFT);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
index 64c8f95..b1dd789 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
@@ -67,6 +67,7 @@
lineEl.classList.add('right');
row.appendChild(lineEl);
row.classList.add('diff-row', 'unified');
+ row.tabIndex = -1;
const action = this._createContextControl(section, line);
if (action) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index bb23572..3cb1a39 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -385,6 +385,13 @@
if (opt_class) {
td.classList.add(opt_class);
}
+
+ if (line.type === GrDiffLine.Type.REMOVE) {
+ td.setAttribute('aria-label', `${number} removed`);
+ } else if (line.type === GrDiffLine.Type.ADD) {
+ td.setAttribute('aria-label', `${number} added`);
+ }
+
if (line.type === GrDiffLine.Type.BLANK) {
return td;
} else if (line.type === GrDiffLine.Type.CONTEXT_CONTROL) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
index 2d0786a..3ae3159 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.html
@@ -23,6 +23,7 @@
id="cursorManager"
scroll-behavior="[[_scrollBehavior]]"
cursor-target-class="target-row"
+ focus-on-move
target="{{diffRow}}"></gr-cursor-manager>
</template>
<script src="gr-diff-cursor.js"></script>
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 d101fea..a61b9ca 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -77,6 +77,9 @@
font-family: var(--font-family);
font-style: italic;
}
+ .diff-row {
+ outline: none;
+ }
.diff-row.target-row.target-side-left .lineNum.left,
.diff-row.target-row.target-side-right .lineNum.right,
.diff-row.target-row.unified .lineNum {
@@ -210,7 +213,10 @@
is-image-diff="[[isImageDiff]]"
base-image="[[_baseImage]]"
revision-image="[[_revisionImage]]">
- <table id="diffTable" class$="[[_diffTableClass]]"></table>
+ <table
+ id="diffTable"
+ class$="[[_diffTableClass]]"
+ role="presentation"></table>
</gr-diff-builder>
</gr-diff-highlight>
</gr-diff-selection>
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
new file mode 100644
index 0000000..64e5439
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.html
@@ -0,0 +1,64 @@
+<!--
+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.
+-->
+
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../bower_components/iron-input/iron-input.html">
+<link rel="import" href="../../shared/gr-button/gr-button.html">
+<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
+<link rel="import" href="../../../styles/shared-styles.html">
+
+<dom-module id="gr-copy-clipboard">
+ <template>
+ <style include="shared-styles">
+ .text {
+ display: flex;
+ flex-wrap: wrap;
+ margin-bottom: .5em;
+ width: 60em;
+ }
+ .text label {
+ flex: 0 0 100%;
+ }
+ .copyText {
+ flex-grow: 1;
+ margin-right: .3em;
+ }
+ .hideInput {
+ display: none;
+ }
+ input {
+ font-family: var(--monospace-font-family);
+ font-size: inherit;
+ }
+ </style>
+ <div class="text">
+ <label>[[title]]</label>
+ <input id="input" is="iron-input"
+ class$="copyText [[_computeInputClass(hideInput)]]"
+ type="text"
+ bind-value="[[text]]"
+ on-tap="_handleInputTap"
+ readonly>
+ <gr-button id="button"
+ class="copyToClipboard"
+ on-tap="_copyToClipboard">
+ copy
+ </gr-button>
+ </div>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
+ </template>
+ <script src="gr-copy-clipboard.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
new file mode 100644
index 0000000..a371374
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard.js
@@ -0,0 +1,52 @@
+// 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.
+(function() {
+ 'use strict';
+
+ const COPY_TIMEOUT_MS = 1000;
+
+ Polymer({
+ is: 'gr-copy-clipboard',
+
+ properties: {
+ text: String,
+ title: String,
+ hideInput: {
+ type: Boolean,
+ value: false,
+ },
+ },
+
+ focusOnCopy() {
+ this.$.button.focus();
+ },
+
+ _computeInputClass(hideInput) {
+ return hideInput ? 'hideInput' : '';
+ },
+
+ _handleInputTap(e) {
+ e.preventDefault();
+ Polymer.dom(e).rootTarget.select();
+ },
+
+ _copyToClipboard(e) {
+ this.$.input.select();
+ document.execCommand('copy');
+ window.getSelection().removeAllRanges();
+ e.target.textContent = 'done';
+ this.async(() => { e.target.textContent = 'copy'; }, COPY_TIMEOUT_MS);
+ },
+ });
+})();
diff --git a/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
new file mode 100644
index 0000000..7310629
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-copy-clipboard/gr-copy-clipboard_test.html
@@ -0,0 +1,82 @@
+<!DOCTYPE html>
+<!--
+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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-copy-clipboard</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+<link rel="import" href="gr-copy-clipboard.html">
+
+<script>void(0);</script>
+
+<test-fixture id="basic">
+ <template>
+ <gr-copy-clipboard></gr-copy-clipboard>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-copy-clipboard tests', () => {
+ let element;
+ let sandbox;
+
+ setup(() => {
+ stub('gr-rest-api-interface', {
+ saveChangeStarred() { return Promise.resolve({ok: true}); },
+ });
+ sandbox = sinon.sandbox.create();
+ element = fixture('basic');
+ element.title = 'Checkout';
+ element.text = `git fetch http://gerrit@localhost:8080/a/test-project
+ refs/changes/05/5/1 && git checkout FETCH_HEAD`;
+ flushAsynchronousOperations();
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('copy to clipboard', () => {
+ const clipboardSpy = sandbox.spy(element, '_copyToClipboard');
+ const copyBtn = element.$$('.copyToClipboard');
+ MockInteractions.tap(copyBtn);
+ assert.isTrue(clipboardSpy.called);
+ });
+
+ test('focusOnCopy', () => {
+ element.focusOnCopy();
+ assert.deepEqual(Polymer.dom(element.root).activeElement,
+ element.$$('.copyToClipboard'));
+ });
+
+ test('_handleInputTap', () => {
+ const inputElement = element.$$('input');
+ MockInteractions.tap(inputElement);
+ assert.equal(inputElement.selectionStart, 0);
+ assert.equal(inputElement.selectionEnd, element.text.length - 1);
+ });
+
+ test('hideInput', () => {
+ assert.notEqual(getComputedStyle(element.$.input).display, 'none');
+ element.hideInput = true;
+ flushAsynchronousOperations();
+ assert.equal(getComputedStyle(element.$.input).display, 'none');
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
index 0e68899..c79e838 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.html
@@ -51,35 +51,35 @@
display: flex;
flex-direction: column;
}
- ::content tr.table {
- border-bottom: 1px solid #eee;
- }
::content #list {
border-collapse: collapse;
width: 100%;
}
- ::content td {
+ ::content #list tr.table {
+ border-bottom: 1px solid #eee;
+ }
+ ::content #list td {
flex-shrink: 0;
padding: .3em .5em;
}
- ::content th {
+ ::content #list th {
background-color: #ddd;
border-bottom: 1px solid #eee;
font-weight: bold;
padding: .3em .5em;
text-align: left;
}
- ::content a {
+ ::content #list a {
color: var(--default-text-color);
text-decoration: none;
}
- ::content a:hover {
+ ::content #list a:hover {
text-decoration: underline;
}
- ::content .description {
+ ::content #list .description {
width: 70%;
}
- ::content .loading {
+ ::content #list .loading {
color: #666;
padding: 1em var(--default-horizontal-margin);
}
diff --git a/polygerrit-ui/app/embed/change-diff-views.html b/polygerrit-ui/app/embed/change-diff-views.html
new file mode 100644
index 0000000..b4f521b
--- /dev/null
+++ b/polygerrit-ui/app/embed/change-diff-views.html
@@ -0,0 +1,18 @@
+<!--
+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.
+-->
+<link rel="import" href="../bower_components/polymer/polymer.html">
+<link rel="import" href="../elements/change/gr-change-view/gr-change-view.html">
+<link rel="import" href="../elements/diff/gr-diff-view/gr-diff-view.html">
diff --git a/polygerrit-ui/app/styles/shared-styles.html b/polygerrit-ui/app/styles/shared-styles.html
index 5396679..cc1dabd 100644
--- a/polygerrit-ui/app/styles/shared-styles.html
+++ b/polygerrit-ui/app/styles/shared-styles.html
@@ -23,15 +23,18 @@
sup, tt, var, b, u, i, center, dl, dt, dd, ol, ul, li, fieldset, form,
label, legend, table, caption, tbody, tfoot, thead, tr, th, td, article,
aside, canvas, details, embed, figure, figcaption, footer, header, hgroup,
- menu, nav, output, ruby, section, summary, time, mark, audio, video {
+ main, menu, nav, output, ruby, section, summary, time, mark, audio, video {
border: 0;
+ box-sizing: border-box;
font-size: 100%;
font: inherit;
margin: 0;
padding: 0;
vertical-align: baseline;
}
- input {
+ input,
+ iron-autogrow-textarea {
+ box-sizing: border-box;
margin: 0;
padding: 0;
}
@@ -53,7 +56,6 @@
border-collapse: collapse;
border-spacing: 0;
}
-
/* Other Shared Styles*/
h1 {
font-size: 2em;
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 0f552a00..bd82da0 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -101,6 +101,7 @@
'shared/gr-button/gr-button_test.html',
'shared/gr-change-star/gr-change-star_test.html',
'shared/gr-confirm-dialog/gr-confirm-dialog_test.html',
+ 'shared/gr-copy-clipboard/gr-copy-clipboard_test.html',
'shared/gr-cursor-manager/gr-cursor-manager_test.html',
'shared/gr-date-formatter/gr-date-formatter_test.html',
'shared/gr-editable-content/gr-editable-content_test.html',
