Merge changes I20ec7033,I49f8eb03,Ie45b16ae,Iad397318
* changes:
GroupsIT: Remove dead code
GroupsIT: Correct format of Javadoc comment
GroupsIT: Use simpler stream/collection statements
GroupsIT: Remove never queried list
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index b0a39cf..7becf99 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -26,6 +26,7 @@
"//java/com/google/gerrit/pgm/util",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/audit",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/server/restapi",
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index e74c4b2..f4b0a9a 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -61,6 +61,7 @@
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpServletResponseWrapper;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.http.server.GitServlet;
import org.eclipse.jgit.http.server.GitSmartHttpTools;
@@ -130,6 +131,55 @@
}
}
+ static class HttpServletResponseWithStatusWrapper extends HttpServletResponseWrapper {
+ private int responseStatus;
+
+ HttpServletResponseWithStatusWrapper(HttpServletResponse response) {
+ super(response);
+ /* Even if we could read the status from response, we assume that it is all
+ * fine because we entered the filter without any prior issues.
+ * When Google will have upgraded to Servlet 3.0, we could actually
+ * call response.getStatus() and the code will be clearer.
+ */
+ responseStatus = HttpServletResponse.SC_OK;
+ }
+
+ @Override
+ public void setStatus(int sc) {
+ responseStatus = sc;
+ super.setStatus(sc);
+ }
+
+ @SuppressWarnings("deprecation")
+ @Override
+ public void setStatus(int sc, String sm) {
+ responseStatus = sc;
+ super.setStatus(sc, sm);
+ }
+
+ @Override
+ public void sendError(int sc) throws IOException {
+ this.responseStatus = sc;
+ super.sendError(sc);
+ }
+
+ @Override
+ public void sendError(int sc, String msg) throws IOException {
+ this.responseStatus = sc;
+ super.sendError(sc, msg);
+ }
+
+ @Override
+ public void sendRedirect(String location) throws IOException {
+ this.responseStatus = HttpServletResponse.SC_MOVED_TEMPORARILY;
+ super.sendRedirect(location);
+ }
+
+ public int getResponseStatus() {
+ return responseStatus;
+ }
+ }
+
@Inject
GitOverHttpServlet(
Resolver resolver,
@@ -163,8 +213,8 @@
.getParameterMap()
.forEach(
(k, v) -> {
- for (int i = 0; i < v.length; i++) {
- multiMap.put(k, v[i]);
+ for (String aV : v) {
+ multiMap.put(k, aV);
}
});
}
@@ -301,41 +351,48 @@
UploadPack up = (UploadPack) request.getAttribute(ServletUtils.ATTRIBUTE_HANDLER);
PermissionBackend.ForProject perm =
permissionBackend.currentUser().project(state.getNameKey());
+ HttpServletResponseWithStatusWrapper responseWrapper =
+ new HttpServletResponseWithStatusWrapper((HttpServletResponse) response);
+ HttpServletRequest httpRequest = (HttpServletRequest) request;
+ String sessionId = httpRequest.getSession().getId();
+
try {
- perm.check(ProjectPermission.RUN_UPLOAD_PACK);
- } catch (AuthException e) {
- GitSmartHttpTools.sendError(
- (HttpServletRequest) request,
- (HttpServletResponse) response,
- HttpServletResponse.SC_FORBIDDEN,
- "upload-pack not permitted on this server");
- return;
- } catch (PermissionBackendException e) {
- throw new ServletException(e);
+ try {
+ perm.check(ProjectPermission.RUN_UPLOAD_PACK);
+ } catch (AuthException e) {
+ GitSmartHttpTools.sendError(
+ (HttpServletRequest) request,
+ responseWrapper,
+ HttpServletResponse.SC_FORBIDDEN,
+ "upload-pack not permitted on this server");
+ return;
+ } catch (PermissionBackendException e) {
+ responseWrapper.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ throw new ServletException(e);
+ }
+
+ // We use getRemoteHost() here instead of getRemoteAddr() because REMOTE_ADDR
+ // may have been overridden by a proxy server -- we'll try to avoid this.
+ UploadValidators uploadValidators =
+ uploadValidatorsFactory.create(state.getProject(), repo, request.getRemoteHost());
+ up.setPreUploadHook(
+ PreUploadHookChain.newChain(
+ Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
+ up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
+ next.doFilter(httpRequest, responseWrapper);
} finally {
- HttpServletRequest httpRequest = (HttpServletRequest) request;
- HttpServletResponse httpResponse = (HttpServletResponse) response;
groupAuditService.dispatch(
new HttpAuditEvent(
- httpRequest.getSession().getId(),
+ sessionId,
userProvider.get(),
extractWhat(httpRequest),
TimeUtil.nowMs(),
extractParameters(httpRequest),
httpRequest.getMethod(),
httpRequest,
- httpResponse.getStatus(),
- httpResponse));
+ responseWrapper.getResponseStatus(),
+ responseWrapper));
}
-
- // We use getRemoteHost() here instead of getRemoteAddr() because REMOTE_ADDR
- // may have been overridden by a proxy server -- we'll try to avoid this.
- UploadValidators uploadValidators =
- uploadValidatorsFactory.create(state.getProject(), repo, request.getRemoteHost());
- up.setPreUploadHook(
- PreUploadHookChain.newChain(Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
- up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
- next.doFilter(request, response);
}
@Override
@@ -411,25 +468,28 @@
rp.getAdvertiseRefsHook().advertiseRefs(rp);
ProjectState state = (ProjectState) request.getAttribute(ATT_STATE);
+ HttpServletResponseWithStatusWrapper responseWrapper =
+ new HttpServletResponseWithStatusWrapper((HttpServletResponse) response);
+ HttpServletRequest httpRequest = (HttpServletRequest) request;
Capable canUpload;
try {
- permissionBackend
- .currentUser()
- .project(state.getNameKey())
- .check(ProjectPermission.RUN_RECEIVE_PACK);
- canUpload = arc.canUpload();
- } catch (AuthException e) {
- GitSmartHttpTools.sendError(
- (HttpServletRequest) request,
- (HttpServletResponse) response,
- HttpServletResponse.SC_FORBIDDEN,
- "receive-pack not permitted on this server");
- return;
- } catch (PermissionBackendException e) {
- throw new RuntimeException(e);
+ try {
+ permissionBackend
+ .currentUser()
+ .project(state.getNameKey())
+ .check(ProjectPermission.RUN_RECEIVE_PACK);
+ canUpload = arc.canUpload();
+ } catch (AuthException e) {
+ GitSmartHttpTools.sendError(
+ httpRequest,
+ responseWrapper,
+ HttpServletResponse.SC_FORBIDDEN,
+ "receive-pack not permitted on this server");
+ return;
+ } catch (PermissionBackendException e) {
+ throw new RuntimeException(e);
+ }
} finally {
- HttpServletRequest httpRequest = (HttpServletRequest) request;
- HttpServletResponse httpResponse = (HttpServletResponse) response;
groupAuditService.dispatch(
new HttpAuditEvent(
httpRequest.getSession().getId(),
@@ -439,26 +499,26 @@
extractParameters(httpRequest),
httpRequest.getMethod(),
httpRequest,
- httpResponse.getStatus(),
- httpResponse));
+ responseWrapper.getResponseStatus(),
+ responseWrapper));
}
if (canUpload != Capable.OK) {
GitSmartHttpTools.sendError(
- (HttpServletRequest) request,
- (HttpServletResponse) response,
+ httpRequest,
+ responseWrapper,
HttpServletResponse.SC_FORBIDDEN,
"\n" + canUpload.getMessage());
return;
}
if (!rp.isCheckReferencedObjectsAreReachable()) {
- chain.doFilter(request, response);
+ chain.doFilter(request, responseWrapper);
return;
}
if (!(userProvider.get().isIdentifiedUser())) {
- chain.doFilter(request, response);
+ chain.doFilter(request, responseWrapper);
return;
}
@@ -475,7 +535,7 @@
}
}
- chain.doFilter(request, response);
+ chain.doFilter(request, responseWrapper);
if (isGet) {
cache.put(cacheKey, Collections.unmodifiableSet(new HashSet<>(rp.getAdvertisedObjects())));
diff --git a/java/com/google/gerrit/pgm/init/GroupsOnInit.java b/java/com/google/gerrit/pgm/init/GroupsOnInit.java
index 8e06aa1..584d8af 100644
--- a/java/com/google/gerrit/pgm/init/GroupsOnInit.java
+++ b/java/com/google/gerrit/pgm/init/GroupsOnInit.java
@@ -25,7 +25,6 @@
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdentProvider;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerIdProvider;
@@ -37,7 +36,6 @@
import com.google.gerrit.server.group.db.GroupConfig;
import com.google.gerrit.server.group.db.GroupNameNotes;
import com.google.gerrit.server.group.db.InternalGroupUpdate;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.File;
import java.io.IOException;
@@ -54,9 +52,8 @@
/**
* A database accessor for calls related to groups.
*
- * <p>All calls which read or write group related details to the database <strong>during
- * init</strong> (either ReviewDb or NoteDb) are gathered here. For non-init cases, use {@code
- * Groups} or {@code GroupsUpdate} instead.
+ * <p>All calls which read or write group related details to the NoteDb <strong>during init</strong>
+ * are gathered here. For non-init cases, use {@code Groups} or {@code GroupsUpdate} instead.
*
* <p>All methods of this class refer to <em>internal</em> groups.
*/
@@ -76,16 +73,14 @@
/**
* Returns the {@code AccountGroup} for the specified {@code GroupReference}.
*
- * @param db the {@code ReviewDb} instance to use for lookups
* @param groupReference the {@code GroupReference} of the group
* @return the {@code InternalGroup} represented by the {@code GroupReference}
- * @throws OrmException if the group couldn't be retrieved from ReviewDb
* @throws IOException if an error occurs while reading from NoteDb
* @throws ConfigInvalidException if the data in NoteDb is in an incorrect format
* @throws NoSuchGroupException if a group with such a name doesn't exist
*/
- public InternalGroup getExistingGroup(ReviewDb db, GroupReference groupReference)
- throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
+ public InternalGroup getExistingGroup(GroupReference groupReference)
+ throws NoSuchGroupException, IOException, ConfigInvalidException {
File allUsersRepoPath = getPathToAllUsersRepository();
if (allUsersRepoPath != null) {
try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) {
@@ -102,14 +97,11 @@
/**
* Returns {@code GroupReference}s for all internal groups.
*
- * @param db the {@code ReviewDb} instance to use for lookups
* @return a stream of the {@code GroupReference}s of all internal groups
- * @throws OrmException if an error occurs while reading from ReviewDb
* @throws IOException if an error occurs while reading from NoteDb
* @throws ConfigInvalidException if the data in NoteDb is in an incorrect format
*/
- public Stream<GroupReference> getAllGroupReferences(ReviewDb db)
- throws OrmException, IOException, ConfigInvalidException {
+ public Stream<GroupReference> getAllGroupReferences() throws IOException, ConfigInvalidException {
File allUsersRepoPath = getPathToAllUsersRepository();
if (allUsersRepoPath != null) {
try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) {
@@ -126,14 +118,12 @@
* <p><strong>Note</strong>: This method doesn't check whether the account exists! It also doesn't
* update the account index!
*
- * @param db the {@code ReviewDb} instance to update
* @param groupUuid the UUID of the group
* @param account the account to add
- * @throws OrmException if an error occurs while reading/writing from/to ReviewDb
* @throws NoSuchGroupException if the specified group doesn't exist
*/
- public void addGroupMember(ReviewDb db, AccountGroup.UUID groupUuid, Account account)
- throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
+ public void addGroupMember(AccountGroup.UUID groupUuid, Account account)
+ throws NoSuchGroupException, IOException, ConfigInvalidException {
File allUsersRepoPath = getPathToAllUsersRepository();
if (allUsersRepoPath != null) {
try (Repository repository = new FileRepository(allUsersRepoPath)) {
diff --git a/java/com/google/gerrit/pgm/init/InitAdminUser.java b/java/com/google/gerrit/pgm/init/InitAdminUser.java
index f12fa50..f19cf39 100644
--- a/java/com/google/gerrit/pgm/init/InitAdminUser.java
+++ b/java/com/google/gerrit/pgm/init/InitAdminUser.java
@@ -26,7 +26,6 @@
import com.google.gerrit.pgm.init.api.InitStep;
import com.google.gerrit.pgm.init.api.SequencesOnInit;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountSshKey;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -37,7 +36,6 @@
import com.google.gerrit.server.index.group.GroupIndex;
import com.google.gerrit.server.index.group.GroupIndexCollection;
import com.google.gerrit.server.util.time.TimeUtil;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import java.io.IOException;
import java.nio.file.Files;
@@ -57,7 +55,6 @@
private final ExternalIdsOnInit externalIds;
private final SequencesOnInit sequencesOnInit;
private final GroupsOnInit groupsOnInit;
- private SchemaFactory<ReviewDb> dbFactory;
private AccountIndexCollection accountIndexCollection;
private GroupIndexCollection groupIndexCollection;
@@ -84,11 +81,6 @@
@Override
public void run() {}
- @Inject(optional = true)
- void set(SchemaFactory<ReviewDb> dbFactory) {
- this.dbFactory = dbFactory;
- }
-
@Inject
void set(AccountIndexCollection accountIndexCollection) {
this.accountIndexCollection = accountIndexCollection;
@@ -106,58 +98,56 @@
return;
}
- try (ReviewDb db = dbFactory.open()) {
- if (!accounts.hasAnyAccount()) {
- ui.header("Gerrit Administrator");
- if (ui.yesno(true, "Create administrator user")) {
- Account.Id id = new Account.Id(sequencesOnInit.nextAccountId(db));
- String username = ui.readString("admin", "username");
- String name = ui.readString("Administrator", "name");
- String httpPassword = ui.readString("secret", "HTTP password");
- AccountSshKey sshKey = readSshKey(id);
- String email = readEmail(sshKey);
+ if (!accounts.hasAnyAccount()) {
+ ui.header("Gerrit Administrator");
+ if (ui.yesno(true, "Create administrator user")) {
+ Account.Id id = new Account.Id(sequencesOnInit.nextAccountId());
+ String username = ui.readString("admin", "username");
+ String name = ui.readString("Administrator", "name");
+ String httpPassword = ui.readString("secret", "HTTP password");
+ AccountSshKey sshKey = readSshKey(id);
+ String email = readEmail(sshKey);
- List<ExternalId> extIds = new ArrayList<>(2);
- extIds.add(ExternalId.createUsername(username, id, httpPassword));
+ List<ExternalId> extIds = new ArrayList<>(2);
+ extIds.add(ExternalId.createUsername(username, id, httpPassword));
- if (email != null) {
- extIds.add(ExternalId.createEmail(id, email));
- }
- externalIds.insert("Add external IDs for initial admin user", extIds);
+ if (email != null) {
+ extIds.add(ExternalId.createEmail(id, email));
+ }
+ externalIds.insert("Add external IDs for initial admin user", extIds);
- Account a = new Account(id, TimeUtil.nowTs());
- a.setFullName(name);
- a.setPreferredEmail(email);
- accounts.insert(a);
+ Account a = new Account(id, TimeUtil.nowTs());
+ a.setFullName(name);
+ a.setPreferredEmail(email);
+ accounts.insert(a);
- // Only two groups should exist at this point in time and hence iterating over all of them
- // is cheap.
- Optional<GroupReference> adminGroupReference =
- groupsOnInit
- .getAllGroupReferences(db)
- .filter(group -> group.getName().equals("Administrators"))
- .findAny();
- if (!adminGroupReference.isPresent()) {
- throw new NoSuchGroupException("Administrators");
- }
- GroupReference adminGroup = adminGroupReference.get();
- groupsOnInit.addGroupMember(db, adminGroup.getUUID(), a);
+ // Only two groups should exist at this point in time and hence iterating over all of them
+ // is cheap.
+ Optional<GroupReference> adminGroupReference =
+ groupsOnInit
+ .getAllGroupReferences()
+ .filter(group -> group.getName().equals("Administrators"))
+ .findAny();
+ if (!adminGroupReference.isPresent()) {
+ throw new NoSuchGroupException("Administrators");
+ }
+ GroupReference adminGroup = adminGroupReference.get();
+ groupsOnInit.addGroupMember(adminGroup.getUUID(), a);
- if (sshKey != null) {
- VersionedAuthorizedKeysOnInit authorizedKeys = authorizedKeysFactory.create(id).load();
- authorizedKeys.addKey(sshKey.sshPublicKey());
- authorizedKeys.save("Add SSH key for initial admin user\n");
- }
+ if (sshKey != null) {
+ VersionedAuthorizedKeysOnInit authorizedKeys = authorizedKeysFactory.create(id).load();
+ authorizedKeys.addKey(sshKey.sshPublicKey());
+ authorizedKeys.save("Add SSH key for initial admin user\n");
+ }
- AccountState as = AccountState.forAccount(new AllUsersName(allUsers.get()), a, extIds);
- for (AccountIndex accountIndex : accountIndexCollection.getWriteIndexes()) {
- accountIndex.replace(as);
- }
+ AccountState as = AccountState.forAccount(new AllUsersName(allUsers.get()), a, extIds);
+ for (AccountIndex accountIndex : accountIndexCollection.getWriteIndexes()) {
+ accountIndex.replace(as);
+ }
- InternalGroup adminInternalGroup = groupsOnInit.getExistingGroup(db, adminGroup);
- for (GroupIndex groupIndex : groupIndexCollection.getWriteIndexes()) {
- groupIndex.replace(adminInternalGroup);
- }
+ InternalGroup adminInternalGroup = groupsOnInit.getExistingGroup(adminGroup);
+ for (GroupIndex groupIndex : groupIndexCollection.getWriteIndexes()) {
+ groupIndex.replace(adminInternalGroup);
}
}
}
diff --git a/java/com/google/gerrit/pgm/init/api/SequencesOnInit.java b/java/com/google/gerrit/pgm/init/api/SequencesOnInit.java
index c9c3a64..1716a3c 100644
--- a/java/com/google/gerrit/pgm/init/api/SequencesOnInit.java
+++ b/java/com/google/gerrit/pgm/init/api/SequencesOnInit.java
@@ -35,16 +35,14 @@
this.allUsersName = allUsersName;
}
- public int nextAccountId(ReviewDb db) throws OrmException {
- @SuppressWarnings("deprecation")
- RepoSequence.Seed accountSeed = db::nextAccountId;
+ public int nextAccountId() throws OrmException {
RepoSequence accountSeq =
new RepoSequence(
repoManager,
GitReferenceUpdated.DISABLED,
new Project.NameKey(allUsersName.get()),
Sequences.NAME_ACCOUNTS,
- accountSeed,
+ () -> ReviewDb.FIRST_ACCOUNT_ID,
1);
return accountSeq.next();
}
diff --git a/java/com/google/gerrit/reviewdb/server/ReviewDb.java b/java/com/google/gerrit/reviewdb/server/ReviewDb.java
index 8e4292c..90cd066 100644
--- a/java/com/google/gerrit/reviewdb/server/ReviewDb.java
+++ b/java/com/google/gerrit/reviewdb/server/ReviewDb.java
@@ -14,8 +14,6 @@
package com.google.gerrit.reviewdb.server;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.Relation;
@@ -28,7 +26,6 @@
* <p>Root entities that are at the top level of some important data graph:
*
* <ul>
- * <li>{@link Account}: Per-user account registration, preferences, identity.
* <li>{@link Change}: All review information about a single proposed change.
* </ul>
*/
@@ -91,22 +88,8 @@
int FIRST_ACCOUNT_ID = 1000000;
- /**
- * Next unique id for a {@link Account}.
- *
- * @deprecated use {@link com.google.gerrit.server.Sequences#nextAccountId()}.
- */
- @Sequence(startWith = FIRST_ACCOUNT_ID)
- @Deprecated
- int nextAccountId() throws OrmException;
-
int FIRST_GROUP_ID = 1;
- /** Next unique id for a {@link AccountGroup}. */
- @Sequence(startWith = FIRST_GROUP_ID)
- @Deprecated
- int nextAccountGroupId() throws OrmException;
-
int FIRST_CHANGE_ID = 1;
/**
diff --git a/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java b/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
index 202729e..c420254 100644
--- a/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
+++ b/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
@@ -136,18 +136,6 @@
@Override
@SuppressWarnings("deprecation")
- public int nextAccountId() throws OrmException {
- return delegate.nextAccountId();
- }
-
- @Override
- @SuppressWarnings("deprecation")
- public int nextAccountGroupId() throws OrmException {
- return delegate.nextAccountGroupId();
- }
-
- @Override
- @SuppressWarnings("deprecation")
public int nextChangeId() throws OrmException {
return delegate.nextChangeId();
}
diff --git a/java/com/google/gerrit/server/Sequences.java b/java/com/google/gerrit/server/Sequences.java
index fcf0759..70a02a8 100644
--- a/java/com/google/gerrit/server/Sequences.java
+++ b/java/com/google/gerrit/server/Sequences.java
@@ -93,11 +93,15 @@
new RepoSequence(
repoManager, gitRefUpdated, allProjects, NAME_CHANGES, changeSeed, changeBatchSize);
- RepoSequence.Seed groupSeed = () -> nextGroupId(db.get());
int groupBatchSize = 1;
groupSeq =
new RepoSequence(
- repoManager, gitRefUpdated, allUsers, NAME_GROUPS, groupSeed, groupBatchSize);
+ repoManager,
+ gitRefUpdated,
+ allUsers,
+ NAME_GROUPS,
+ () -> ReviewDb.FIRST_GROUP_ID,
+ groupBatchSize);
nextIdLatency =
metrics.newTimer(
@@ -158,9 +162,4 @@
private static int nextChangeId(ReviewDb db) throws OrmException {
return db.nextChangeId();
}
-
- @SuppressWarnings("deprecation")
- static int nextGroupId(ReviewDb db) throws OrmException {
- return db.nextAccountGroupId();
- }
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index b23fe71..a3b094a 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -2727,10 +2727,13 @@
/** prints a warning if the new PS has the same tree as the previous commit. */
private void sameTreeWarning() throws IOException {
- RevCommit newCommit = receivePack.getRevWalk().parseCommit(newCommitId);
+ RevWalk rw = receivePack.getRevWalk();
+ RevCommit newCommit = rw.parseCommit(newCommitId);
RevCommit priorCommit = revisions.inverse().get(priorPatchSet);
if (newCommit.getTree().equals(priorCommit.getTree())) {
+ rw.parseBody(newCommit);
+ rw.parseBody(priorCommit);
boolean messageEq =
Objects.equals(newCommit.getFullMessage(), priorCommit.getFullMessage());
boolean parentsEq = parentsEqual(newCommit, priorCommit);
diff --git a/java/com/google/gerrit/server/schema/Schema_155.java b/java/com/google/gerrit/server/schema/Schema_155.java
index 812d7a6..e9372a5 100644
--- a/java/com/google/gerrit/server/schema/Schema_155.java
+++ b/java/com/google/gerrit/server/schema/Schema_155.java
@@ -40,15 +40,13 @@
@Override
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
- @SuppressWarnings("deprecation")
- RepoSequence.Seed accountSeed = db::nextAccountId;
RepoSequence accountSeq =
new RepoSequence(
repoManager,
GitReferenceUpdated.DISABLED,
allUsersName,
Sequences.NAME_ACCOUNTS,
- accountSeed,
+ () -> ReviewDb.FIRST_ACCOUNT_ID,
1);
// consume one account ID to ensure that the account sequence is initialized in NoteDb
diff --git a/java/com/google/gerrit/server/schema/Schema_163.java b/java/com/google/gerrit/server/schema/Schema_163.java
index 9eb5d5e..4b3659de 100644
--- a/java/com/google/gerrit/server/schema/Schema_163.java
+++ b/java/com/google/gerrit/server/schema/Schema_163.java
@@ -40,15 +40,13 @@
@Override
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
- @SuppressWarnings("deprecation")
- RepoSequence.Seed groupSeed = db::nextAccountGroupId;
RepoSequence groupSeq =
new RepoSequence(
repoManager,
GitReferenceUpdated.DISABLED,
allUsersName,
Sequences.NAME_GROUPS,
- groupSeed,
+ () -> ReviewDb.FIRST_GROUP_ID,
1);
// consume one account ID to ensure that the group sequence is initialized in NoteDb
diff --git a/java/com/google/gerrit/testing/DisabledReviewDb.java b/java/com/google/gerrit/testing/DisabledReviewDb.java
index 2bf95b0..d06beb9 100644
--- a/java/com/google/gerrit/testing/DisabledReviewDb.java
+++ b/java/com/google/gerrit/testing/DisabledReviewDb.java
@@ -95,16 +95,6 @@
}
@Override
- public int nextAccountId() {
- throw new Disabled();
- }
-
- @Override
- public int nextAccountGroupId() {
- throw new Disabled();
- }
-
- @Override
public int nextChangeId() {
throw new Disabled();
}
diff --git a/java/com/google/gerrit/testing/FakeGroupAuditService.java b/java/com/google/gerrit/testing/FakeGroupAuditService.java
index f2e85cd..ddf03f5 100644
--- a/java/com/google/gerrit/testing/FakeGroupAuditService.java
+++ b/java/com/google/gerrit/testing/FakeGroupAuditService.java
@@ -63,7 +63,10 @@
@Override
public void dispatch(AuditEvent action) {
- auditEvents.add(action);
+ synchronized (auditEvents) {
+ auditEvents.add(action);
+ auditEvents.notifyAll();
+ }
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 228784b..5e82f84 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -1464,14 +1464,7 @@
@Test
public void pushSameCommitTwice() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .getProject()
- .setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- InheritableBoolean.TRUE);
- u.save();
- }
+ enableCreateNewChangeForAllNotInTarget();
PushOneCommit push =
pushFactory.create(
@@ -1493,14 +1486,7 @@
@Test
public void pushSameCommitTwiceWhenIndexFailed() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .getProject()
- .setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- InheritableBoolean.TRUE);
- u.save();
- }
+ enableCreateNewChangeForAllNotInTarget();
PushOneCommit push =
pushFactory.create(
@@ -2356,6 +2342,113 @@
assertPushOk(pr, "refs/heads/permitted");
}
+ @Test
+ public void pushCommitsWithSameTreeNoChanges() throws Exception {
+ RevCommit c =
+ testRepo
+ .commit()
+ .message("Foo")
+ .parent(getHead(testRepo.getRepository()))
+ .insertChangeId()
+ .create();
+ testRepo.reset(c);
+
+ String r = "refs/for/master";
+ PushResult pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ RevCommit amended = testRepo.amend(c).create();
+ testRepo.reset(amended);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+ assertThat(pr.getMessages())
+ .contains(
+ "warning: no changes between prior commit "
+ + c.abbreviate(7).name()
+ + " and new commit "
+ + amended.abbreviate(7).name());
+ }
+
+ @Test
+ public void pushCommitsWithSameTreeNoFilesChangedMessageUpdated() throws Exception {
+ RevCommit c =
+ testRepo
+ .commit()
+ .message("Foo")
+ .parent(getHead(testRepo.getRepository()))
+ .insertChangeId()
+ .create();
+ String id = GitUtil.getChangeId(testRepo, c).get();
+ testRepo.reset(c);
+
+ String r = "refs/for/master";
+ PushResult pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ RevCommit amended =
+ testRepo.amend(c).message("Foo Bar").insertChangeId(id.substring(1)).create();
+ testRepo.reset(amended);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+ assertThat(pr.getMessages())
+ .contains(
+ "warning: " + amended.abbreviate(7).name() + ": no files changed, message updated");
+ }
+
+ @Test
+ public void pushCommitsWithSameTreeNoFilesChangedAuthorChanged() throws Exception {
+ RevCommit c =
+ testRepo
+ .commit()
+ .message("Foo")
+ .parent(getHead(testRepo.getRepository()))
+ .insertChangeId()
+ .create();
+ testRepo.reset(c);
+
+ String r = "refs/for/master";
+ PushResult pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ RevCommit amended = testRepo.amend(c).author(user.getIdent()).create();
+ testRepo.reset(amended);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+ assertThat(pr.getMessages())
+ .contains(
+ "warning: " + amended.abbreviate(7).name() + ": no files changed, author changed");
+ }
+
+ @Test
+ public void pushCommitsWithSameTreeNoFilesChangedWasRebased() throws Exception {
+ RevCommit head = getHead(testRepo.getRepository());
+ RevCommit c = testRepo.commit().message("Foo").parent(head).insertChangeId().create();
+ testRepo.reset(c);
+
+ String r = "refs/for/master";
+ PushResult pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ testRepo.reset(head);
+ RevCommit newBase = testRepo.commit().message("Base").parent(head).insertChangeId().create();
+ testRepo.reset(newBase);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ testRepo.reset(c);
+ RevCommit amended = testRepo.amend(c).parent(newBase).create();
+ testRepo.reset(amended);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+ assertThat(pr.getMessages())
+ .contains("warning: " + amended.abbreviate(7).name() + ": no files changed, was rebased");
+ }
+
private DraftInput newDraft(String path, int line, String message) {
DraftInput d = new DraftInput();
d.path = path;
diff --git a/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java b/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
index 42e046a..90f4134 100644
--- a/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
@@ -17,7 +17,9 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.server.AuditEvent;
+import com.google.gerrit.server.audit.HttpAuditEvent;
import java.util.Collections;
+import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.transport.CredentialsProvider;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
@@ -25,6 +27,7 @@
import org.junit.Test;
public class GitOverHttpServletIT extends AbstractPushForReview {
+ private static final long AUDIT_EVENT_TIMEOUT = 500L;
@Before
public void beforeEach() throws Exception {
@@ -42,6 +45,7 @@
.setRemote("origin")
.setRefSpecs(new RefSpec("HEAD:refs/for/master"))
.call();
+ waitForAudit();
// Git smart protocol makes two requests:
// https://github.com/git/git/blob/master/Documentation/technical/http-protocol.txt
@@ -51,11 +55,13 @@
assertThat(e.who.getAccountId()).isEqualTo(admin.id);
assertThat(e.what).endsWith("/git-receive-pack");
assertThat(e.params).isEmpty();
+ assertThat(((HttpAuditEvent) e).httpStatus).isEqualTo(HttpServletResponse.SC_OK);
}
@Test
public void uploadPackAuditEventLog() throws Exception {
testRepo.git().fetch().call();
+ waitForAudit();
assertThat(auditService.auditEvents.size()).isEqualTo(1);
@@ -64,5 +70,12 @@
assertThat(e.params.get("service"))
.containsExactlyElementsIn(Collections.singletonList("git-upload-pack"));
assertThat(e.what).endsWith("service=git-upload-pack");
+ assertThat(((HttpAuditEvent) e).httpStatus).isEqualTo(HttpServletResponse.SC_OK);
+ }
+
+ private void waitForAudit() throws InterruptedException {
+ synchronized (auditService.auditEvents) {
+ auditService.auditEvents.wait(AUDIT_EVENT_TIMEOUT);
+ }
}
}
