Merge "Add test for multiple VersionedMetaDatas in one BatchRefUpdate"
diff --git a/java/com/google/gerrit/reviewdb/client/AccountGroupByIdAud.java b/java/com/google/gerrit/reviewdb/client/AccountGroupByIdAud.java
index b058f04..33955c4 100644
--- a/java/com/google/gerrit/reviewdb/client/AccountGroupByIdAud.java
+++ b/java/com/google/gerrit/reviewdb/client/AccountGroupByIdAud.java
@@ -93,6 +93,14 @@
return key;
}
+ public AccountGroup.Id getGroupId() {
+ return key.getParentKey();
+ }
+
+ public AccountGroup.UUID getIncludeUUID() {
+ return key.getIncludeUUID();
+ }
+
public boolean isActive() {
return removedOn == null;
}
@@ -106,6 +114,10 @@
return addedBy;
}
+ public Timestamp getAddedOn() {
+ return key.getAddedOn();
+ }
+
public Account.Id getRemovedBy() {
return removedBy;
}
diff --git a/java/com/google/gerrit/reviewdb/client/AccountGroupMemberAudit.java b/java/com/google/gerrit/reviewdb/client/AccountGroupMemberAudit.java
index 5b838ce..9968b7d 100644
--- a/java/com/google/gerrit/reviewdb/client/AccountGroupMemberAudit.java
+++ b/java/com/google/gerrit/reviewdb/client/AccountGroupMemberAudit.java
@@ -93,6 +93,14 @@
return key;
}
+ public AccountGroup.Id getGroupId() {
+ return key.getGroupId();
+ }
+
+ public Account.Id getMemberId() {
+ return key.getParentKey();
+ }
+
public boolean isActive() {
return removedOn == null;
}
@@ -111,6 +119,10 @@
return addedBy;
}
+ public Timestamp getAddedOn() {
+ return key.getAddedOn();
+ }
+
public Account.Id getRemovedBy() {
return removedBy;
}
diff --git a/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java b/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
index 29b4be3..7fd2c73 100644
--- a/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
+++ b/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
@@ -138,6 +138,7 @@
}
@Override
+ @SuppressWarnings("deprecation")
public int nextAccountGroupId() throws OrmException {
return delegate.nextAccountGroupId();
}
diff --git a/java/com/google/gerrit/server/group/GetAuditLog.java b/java/com/google/gerrit/server/group/GetAuditLog.java
index 8c94f65..ebada0b 100644
--- a/java/com/google/gerrit/server/group/GetAuditLog.java
+++ b/java/com/google/gerrit/server/group/GetAuditLog.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.group;
+import static java.util.Comparator.comparing;
+
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.GroupAuditEventInfo;
@@ -29,13 +31,13 @@
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.group.db.Groups;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.Comparator;
import java.util.List;
import java.util.Optional;
@@ -46,6 +48,7 @@
private final GroupCache groupCache;
private final GroupJson groupJson;
private final GroupBackend groupBackend;
+ private final Groups groups;
@Inject
public GetAuditLog(
@@ -53,12 +56,14 @@
AccountLoader.Factory accountLoaderFactory,
GroupCache groupCache,
GroupJson groupJson,
- GroupBackend groupBackend) {
+ GroupBackend groupBackend,
+ Groups groups) {
this.db = db;
this.accountLoaderFactory = accountLoaderFactory;
this.groupCache = groupCache;
this.groupJson = groupJson;
this.groupBackend = groupBackend;
+ this.groups = groups;
}
@Override
@@ -75,14 +80,12 @@
List<GroupAuditEventInfo> auditEvents = new ArrayList<>();
for (AccountGroupMemberAudit auditEvent :
- db.get().accountGroupMembersAudit().byGroup(group.getId()).toList()) {
- AccountInfo member = accountLoader.get(auditEvent.getKey().getParentKey());
+ groups.getMembersAudit(db.get(), group.getGroupUUID())) {
+ AccountInfo member = accountLoader.get(auditEvent.getMemberId());
auditEvents.add(
GroupAuditEventInfo.createAddUserEvent(
- accountLoader.get(auditEvent.getAddedBy()),
- auditEvent.getKey().getAddedOn(),
- member));
+ accountLoader.get(auditEvent.getAddedBy()), auditEvent.getAddedOn(), member));
if (!auditEvent.isActive()) {
auditEvents.add(
@@ -92,8 +95,8 @@
}
for (AccountGroupByIdAud auditEvent :
- db.get().accountGroupByIdAud().byGroup(group.getId()).toList()) {
- AccountGroup.UUID includedGroupUUID = auditEvent.getKey().getIncludeUUID();
+ groups.getSubgroupsAudit(db.get(), group.getGroupUUID())) {
+ AccountGroup.UUID includedGroupUUID = auditEvent.getIncludeUUID();
Optional<InternalGroup> includedGroup = groupCache.get(includedGroupUUID);
GroupInfo member;
if (includedGroup.isPresent()) {
@@ -121,14 +124,7 @@
accountLoader.fill();
// sort by date in reverse order so that the newest audit event comes first
- Collections.sort(
- auditEvents,
- new Comparator<GroupAuditEventInfo>() {
- @Override
- public int compare(GroupAuditEventInfo e1, GroupAuditEventInfo e2) {
- return e2.date.compareTo(e1.date);
- }
- });
+ Collections.sort(auditEvents, comparing((GroupAuditEventInfo a) -> a.date).reversed());
return auditEvents;
}
diff --git a/java/com/google/gerrit/server/group/db/GroupConfig.java b/java/com/google/gerrit/server/group/db/GroupConfig.java
index ea2e22b..af05bf8 100644
--- a/java/com/google/gerrit/server/group/db/GroupConfig.java
+++ b/java/com/google/gerrit/server/group/db/GroupConfig.java
@@ -394,12 +394,12 @@
Sets.difference(oldSubgroups, newSubgroups)
.stream()
.map(groupNameRetriever)
- .map("Remove: group "::concat);
+ .map("Remove-group: "::concat);
Stream<String> addedMembers =
Sets.difference(newSubgroups, oldSubgroups)
.stream()
.map(groupNameRetriever)
- .map("Add: group "::concat);
+ .map("Add-group: "::concat);
return Stream.concat(removedMembers, addedMembers);
}
}
diff --git a/java/com/google/gerrit/server/group/db/Groups.java b/java/com/google/gerrit/server/group/db/Groups.java
index 53051c1..9ea78be 100644
--- a/java/com/google/gerrit/server/group/db/Groups.java
+++ b/java/com/google/gerrit/server/group/db/Groups.java
@@ -16,6 +16,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Streams;
@@ -23,7 +24,9 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroupById;
+import com.google.gerrit.reviewdb.client.AccountGroupByIdAud;
import com.google.gerrit.reviewdb.client.AccountGroupMember;
+import com.google.gerrit.reviewdb.client.AccountGroupMemberAudit;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -279,4 +282,46 @@
.distinct()
.filter(groupUuid -> !AccountGroup.isInternalGroup(groupUuid));
}
+
+ /**
+ * Returns the membership audit records for a given group.
+ *
+ * @param db the {@code ReviewDb} instance to use for lookups
+ * @param groupUuid the UUID of the group
+ * @return the audit records, in arbitrary order; empty if the group does not exist
+ * @throws OrmException if an error occurs while reading from ReviewDb
+ */
+ public List<AccountGroupMemberAudit> getMembersAudit(ReviewDb db, AccountGroup.UUID groupUuid)
+ throws OrmException {
+ if (readFromNoteDb) {
+ // TODO(dborowitz): Implement.
+ throw new OrmException("Audit logs not yet implemented in NoteDb");
+ }
+ Optional<AccountGroup> group = getGroupFromReviewDb(db, groupUuid);
+ if (!group.isPresent()) {
+ return ImmutableList.of();
+ }
+ return db.accountGroupMembersAudit().byGroup(group.get().getId()).toList();
+ }
+
+ /**
+ * Returns the subgroup audit records for a given group.
+ *
+ * @param db the {@code ReviewDb} instance to use for lookups
+ * @param groupUuid the UUID of the group
+ * @return the audit records, in arbitrary order; empty if the group does not exist
+ * @throws OrmException if an error occurs while reading from ReviewDb
+ */
+ public List<AccountGroupByIdAud> getSubgroupsAudit(ReviewDb db, AccountGroup.UUID groupUuid)
+ throws OrmException {
+ if (readFromNoteDb) {
+ // TODO(dborowitz): Implement.
+ throw new OrmException("Audit logs not yet implemented in NoteDb");
+ }
+ Optional<AccountGroup> group = getGroupFromReviewDb(db, groupUuid);
+ if (!group.isPresent()) {
+ return ImmutableList.of();
+ }
+ return db.accountGroupByIdAud().byGroup(group.get().getId()).toList();
+ }
}
diff --git a/java/com/google/gerrit/server/group/db/GroupsUpdate.java b/java/com/google/gerrit/server/group/db/GroupsUpdate.java
index 24d5bfa..799064f 100644
--- a/java/com/google/gerrit/server/group/db/GroupsUpdate.java
+++ b/java/com/google/gerrit/server/group/db/GroupsUpdate.java
@@ -516,14 +516,7 @@
.map(AccountState::getAccount)
.map(account -> account.getName(anonymousCowardName))
.orElse(anonymousCowardName);
- String email = getEmailForAuditLog(accountId, serverId);
-
- StringBuilder formattedResult = new StringBuilder();
- PersonIdent.appendSanitized(formattedResult, accountName);
- formattedResult.append(" <");
- PersonIdent.appendSanitized(formattedResult, email);
- formattedResult.append(">");
- return formattedResult.toString();
+ return formatNameEmail(accountName, getEmailForAuditLog(accountId, serverId));
}
private static String getEmailForAuditLog(Account.Id accountId, String serverId) {
@@ -534,10 +527,19 @@
return groupCache
.get(groupUuid)
.map(InternalGroup::getName)
- .map(name -> String.format("%s <%s>", name, groupUuid))
+ .map(name -> formatNameEmail(name, groupUuid.get()))
.orElse(groupUuid.get());
}
+ private static String formatNameEmail(String name, String email) {
+ StringBuilder formattedResult = new StringBuilder();
+ PersonIdent.appendSanitized(formattedResult, name);
+ formattedResult.append(" <");
+ PersonIdent.appendSanitized(formattedResult, email);
+ formattedResult.append(">");
+ return formattedResult.toString();
+ }
+
private void commit(GroupConfig groupConfig) throws IOException {
try (MetaDataUpdate metaDataUpdate = metaDataUpdateFactory.create(allUsersName)) {
groupConfig.commit(metaDataUpdate);
diff --git a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
index b23c1de..89326c8 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java
@@ -23,7 +23,6 @@
import com.google.common.base.CharMatcher;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ListMultimap;
-import com.google.common.primitives.Ints;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Comment;
@@ -158,18 +157,14 @@
public Account.Id parseIdent(PersonIdent ident, Change.Id changeId)
throws ConfigInvalidException {
- String email = ident.getEmailAddress();
- int at = email.indexOf('@');
- if (at >= 0) {
- String host = email.substring(at + 1, email.length());
- if (host.equals(serverId)) {
- Integer id = Ints.tryParse(email.substring(0, at));
- if (id != null) {
- return new Account.Id(id);
- }
- }
- }
- throw parseException(changeId, "invalid identity, expected <id>@%s: %s", serverId, email);
+ return NoteDbUtil.parseIdent(ident, serverId)
+ .orElseThrow(
+ () ->
+ parseException(
+ changeId,
+ "invalid identity, expected <id>@%s: %s",
+ serverId,
+ ident.getEmailAddress()));
}
private static boolean match(byte[] note, MutableInteger p, byte[] expected) {
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUtil.java b/java/com/google/gerrit/server/notedb/NoteDbUtil.java
new file mode 100644
index 0000000..59c4c62
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/NoteDbUtil.java
@@ -0,0 +1,39 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.notedb;
+
+import com.google.common.primitives.Ints;
+import com.google.gerrit.reviewdb.client.Account;
+import java.util.Optional;
+import org.eclipse.jgit.lib.PersonIdent;
+
+public class NoteDbUtil {
+ public static Optional<Account.Id> parseIdent(PersonIdent ident, String serverId) {
+ String email = ident.getEmailAddress();
+ int at = email.indexOf('@');
+ if (at >= 0) {
+ String host = email.substring(at + 1, email.length());
+ if (host.equals(serverId)) {
+ Integer id = Ints.tryParse(email.substring(0, at));
+ if (id != null) {
+ return Optional.of(new Account.Id(id));
+ }
+ }
+ }
+ return Optional.empty();
+ }
+
+ private NoteDbUtil() {}
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 3ac975d..6d0a31e 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.api.group;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static com.google.gerrit.acceptance.api.group.GroupAssert.assertGroupInfo;
import static com.google.gerrit.acceptance.rest.account.AccountAssert.assertAccountInfos;
@@ -429,6 +430,7 @@
assertThat(gApi.groups().id(name).options().visibleToAll).isTrue();
}
+ @SuppressWarnings("deprecation")
@Test
public void groupOwner() throws Exception {
String adminUUID = getFromCache("Administrators").getGroupUUID().get();
@@ -722,6 +724,7 @@
@Test
public void getAuditLog() throws Exception {
+ assume().that(cfg.getBoolean("user", null, "readGroupsFromNoteDb", false)).isFalse();
GroupApi g = gApi.groups().create(name("group"));
List<? extends GroupAuditEventInfo> auditEvents = g.auditLog();
assertThat(auditEvents).hasSize(1);
diff --git a/javatests/com/google/gerrit/acceptance/api/project/SetParentIT.java b/javatests/com/google/gerrit/acceptance/api/project/SetParentIT.java
index 486a29e..0d9e582 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/SetParentIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/SetParentIT.java
@@ -19,14 +19,19 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.AllProjectsNameProvider;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.inject.Inject;
import org.junit.Test;
@NoHttpd
public class SetParentIT extends AbstractDaemonTest {
+ @Inject private AllUsersName allUsers;
+
@Test
public void setParentNotAllowed() throws Exception {
String parent = createProject("parent", null, true).get();
@@ -86,4 +91,15 @@
exception.expectMessage("not found");
gApi.projects().name(project.get()).parent("non-existing");
}
+
+ @Test
+ public void setParentForAllUsersMustBeAllProjects() throws Exception {
+ gApi.projects().name(allUsers.get()).parent(allProjects.get());
+
+ String parent = createProject("parent", null, true).get();
+
+ exception.expect(BadRequestException.class);
+ exception.expectMessage("All-Users must inherit from All-Projects");
+ gApi.projects().name(allUsers.get()).parent(parent);
+ }
}
diff --git a/polygerrit-ui/app/styles/app-theme.html b/polygerrit-ui/app/styles/app-theme.html
index 2fe7662..2d89272 100644
--- a/polygerrit-ui/app/styles/app-theme.html
+++ b/polygerrit-ui/app/styles/app-theme.html
@@ -18,7 +18,7 @@
/* Following vars have LTS for plugin API. */
--primary-text-color: #000;
--header-background-color: #eee;
- --header-title-content: 'PolyGerrit';
+ --header-title-content: 'Gerrit';
--header-icon: none;
--header-icon-size: 0em;
--footer-background-color: var(--header-background-color);
