opendev/gerrit
Code Issues Proposed changes

Merge changes I4aa7ab9c,I5ba29aa5,Ie824acdb

Browse Source 
* changes:
  SubmittedTogether: Also work for already submitted changes
  MergeOp: Record the change set
  Move SubmittedTogether assertion to AbstractDaemon
This commit is contained in:
Dave Borowitz 2015-10-06 20:00:27 +00:00 committed by Gerrit Code Review
commit e628ec3ee6
13 changed files with 241 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -14,12 +14,15 @@
package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.initSsh;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.server.project.Util.block;
import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.primitives.Chars;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
@ -594,4 +597,17 @@ public abstract class AbstractDaemonTest {
.revision(1)
.actions();
}
protected void assertSubmittedTogether(String chId, String... expected)
throws Exception {
List<ChangeInfo> actual = gApi.changes().id(chId).submittedTogether();
assertThat(actual).hasSize(expected.length);
assertThat(Iterables.transform(actual,
new Function<ChangeInfo, String>() {
@Override
public String apply(ChangeInfo input) {
return input.changeId;
}
})).containsExactly((Object[])expected).inOrder();
}
}

8
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
View File

@ -49,8 +49,10 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
PushOneCommit.Result change = createChange();
PushOneCommit.Result change2 = createChange();
approve(change.getChangeId());
submit(change2.getChangeId());
String id1 = change.getChangeId();
String id2 = change2.getChangeId();
approve(id1);
submit(id2);
RevCommit head = getRemoteHead();
assertThat(head.getId()).isEqualTo(change2.getCommitId());
@ -59,6 +61,8 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
assertSubmitter(change2.getChangeId(), 1);
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
assertSubmittedTogether(id1, id1, id2);
assertSubmittedTogether(id2, id1, id2);
}
@Test

20
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/SubmittedTogetherIT.java
View File

@ -14,15 +14,11 @@
package com.google.gerrit.acceptance.server.change;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import com.google.common.base.Function;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.ProjectConfig;
@ -33,8 +29,6 @@ import org.eclipse.jgit.revwalk.RevWalk;
import org.junit.Test;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
public class SubmittedTogetherIT extends AbstractDaemonTest {
@ -167,20 +161,6 @@ public class SubmittedTogetherIT extends AbstractDaemonTest {
assertSubmittedTogether(id2);
}
private void assertSubmittedTogether(String chId, String... expected)
throws Exception {
List<ChangeInfo> actual = gApi.changes().id(chId).submittedTogether();
assertThat(actual).hasSize(expected.length);
assertThat(Arrays.asList(expected))
.containsExactlyElementsIn(
Iterables.transform(actual, new Function<ChangeInfo, String>() {
@Override
public String apply(ChangeInfo input) {
return input.changeId;
}
})).inOrder();
}
private RevCommit getRemoteHead() throws IOException {
try (Repository repo = repoManager.openRepository(project);
RevWalk rw = new RevWalk(repo)) {

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.java
View File

@ -228,11 +228,9 @@ public class RelatedChanges extends TabPanel {
new TabChangeListCallback(Tab.SAME_TOPIC, info.project(), revision));
} else {
// TODO(sbeller): show only on latest revision
if (info.status().isOpen()) {
ChangeApi.change(info.legacyId().get()).view("submitted_together")
.get(new TabChangeListCallback(Tab.SUBMITTED_TOGETHER,
info.project(), revision));
}
ChangeApi.change(info.legacyId().get()).view("submitted_together")
.get(new TabChangeListCallback(Tab.SUBMITTED_TOGETHER,
info.project(), revision));
}
}

16
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java
View File

@ -453,6 +453,13 @@ public final class Change {
@Column(id = 17, notNull = false)
protected String originalSubject;
/**
* Unique id for the changes submitted together assigned during merging.
* Only set if the status is MERGED.
*/
@Column(id = 18, notNull = false)
protected String submissionId;
protected Change() {
}
@ -479,6 +486,7 @@ public final class Change {
currentPatchSetId = other.currentPatchSetId;
subject = other.subject;
originalSubject = other.originalSubject;
submissionId = other.submissionId;
topic = other.topic;
}
@ -562,6 +570,14 @@ public final class Change {
}
}
public String getSubmissionId() {
return submissionId;
}
public void setSubmissionId(String id) {
this.submissionId = id;
}
public Status getStatus() {
return Status.forCode(status);
}

40
gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java
View File

@ -14,15 +14,20 @@
package com.google.gerrit.server.change;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
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.RestReadView;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.ChangeSet;
import com.google.gerrit.server.git.MergeSuperSet;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -43,14 +48,17 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
private final ChangeJson.Factory json;
private final Provider<ReviewDb> dbProvider;
private final Provider<InternalChangeQuery> queryProvider;
private final MergeSuperSet mergeSuperSet;
@Inject
SubmittedTogether(ChangeJson.Factory json,
Provider<ReviewDb> dbProvider,
Provider<InternalChangeQuery> queryProvider,
MergeSuperSet mergeSuperSet) {
this.json = json;
this.dbProvider = dbProvider;
this.queryProvider = queryProvider;
this.mergeSuperSet = mergeSuperSet;
}
@ -59,16 +67,32 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
throws AuthException, BadRequestException,
ResourceConflictException, Exception {
try {
ChangeSet cs = mergeSuperSet.completeChangeSet(dbProvider.get(),
resource.getChange());
if (cs.size() > 1) {
return json.create(EnumSet.of(
ListChangesOption.CURRENT_REVISION,
ListChangesOption.CURRENT_COMMIT))
.format(cs.ids());
Change c = resource.getChange();
List<Change.Id> ids;
if (c.getStatus().isOpen()) {
ChangeSet cs = mergeSuperSet.completeChangeSet(dbProvider.get(), c);
ids = cs.ids().asList();
} else if (c.getStatus().asChangeStatus() == ChangeStatus.MERGED) {
ids = Lists.newArrayList();
String subId = c.getSubmissionId();
if (subId.isEmpty()) {
ids = Collections.emptyList();
} else {
for (ChangeData cd : queryProvider.get().bySubmissionId(subId)) {
ids.add(cd.getId());
}
}
} else {
return Collections.emptyList();
// ABANDONED
ids = Collections.emptyList();
}
if (ids.size() <= 1) {
ids = Collections.emptyList();
}
return json.create(EnumSet.of(
ListChangesOption.CURRENT_REVISION,
ListChangesOption.CURRENT_COMMIT))
.format(ids);
} catch (OrmException | IOException e) {
log.error("Error on getting a ChangeSet", e);
throw e;

40
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@ -27,6 +27,8 @@ import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimap;
import com.google.common.collect.Table;
import com.google.common.hash.Hasher;
import com.google.common.hash.Hashing;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.SubmitRecord;
@ -91,6 +93,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Collection;
@ -143,7 +147,19 @@ public class MergeOp {
private final Map<Change.Id, List<SubmitRecord>> records;
private final Map<Change.Id, CodeReviewCommit> commits;
private String logPrefix;
private static final String MACHINE_ID;
static {
String id;
try {
id = InetAddress.getLocalHost().getHostAddress();
} catch (UnknownHostException e) {
id = "unknown";
}
MACHINE_ID = id;
}
private String staticSubmissionId;
private String submissionId;
private ProjectState destProject;
private ReviewDb db;
@ -336,10 +352,19 @@ public class MergeOp {
}
}
private void updateSubmissionId(Change change) {
Hasher h = Hashing.sha1().newHasher();
h.putLong(Thread.currentThread().getId())
.putUnencodedChars(MACHINE_ID);
staticSubmissionId = h.hash().toString().substring(0, 8);
submissionId = change.getId().get() + "-" + TimeUtil.nowMs() +
"-" + staticSubmissionId;
}
public void merge(ReviewDb db, Change change, IdentifiedUser caller,
boolean checkSubmitRules) throws NoSuchChangeException,
OrmException, ResourceConflictException {
logPrefix = String.format("[%s]: ", String.valueOf(change.hashCode()));
updateSubmissionId(change);
this.db = db;
logDebug("Beginning integration of {}", change);
try {
@ -997,6 +1022,7 @@ public class MergeOp {
@Override
public Change update(Change c) {
c.setStatus(Change.Status.MERGED);
c.setSubmissionId(submissionId);
if (!merged.equals(c.currentPatchSetId())) {
// Uncool; the patch set changed after we merged it.
// Go back to the patch set that was actually merged.
@ -1253,28 +1279,28 @@ public class MergeOp {
private void logDebug(String msg, Object... args) {
if (log.isDebugEnabled()) {
log.debug(logPrefix + msg, args);
log.debug("[" + submissionId + "]" + msg, args);
}
}
private void logWarn(String msg, Throwable t) {
if (log.isWarnEnabled()) {
log.warn(logPrefix + msg, t);
log.warn("[" + submissionId + "]" + msg, t);
}
}
private void logWarn(String msg) {
if (log.isWarnEnabled()) {
log.warn(logPrefix + msg);
log.warn("[" + submissionId + "]" + msg);
}
}
private void logError(String msg, Throwable t) {
if (log.isErrorEnabled()) {
if (t != null) {
log.error(logPrefix + msg, t);
log.error("[" + submissionId + "]" + msg, t);
} else {
log.error(logPrefix + msg);
log.error("[" + submissionId + "]" + msg);
}
}
}

15
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeField.java
View File

@ -207,6 +207,21 @@ public class ChangeField {
}
};
/** Submission id assigned by MergeOp. */
public static final FieldDef<ChangeData, String> SUBMISSIONID =
new FieldDef.Single<ChangeData, String>(
"submissionid", FieldType.EXACT, false) {
@Override
public String get(ChangeData input, FillArgs args)
throws OrmException {
Change c = input.change();
if (c == null) {
return null;
}
return c.getSubmissionId();
}
};
/** Last update time since January 1, 1970. */
public static final FieldDef<ChangeData, Timestamp> UPDATED =
new FieldDef.Single<ChangeData, Timestamp>(

36
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeSchemas.java
View File

@ -377,6 +377,42 @@ public class ChangeSchemas {
ChangeField.AUTHOR,
ChangeField.COMMITTER);
static final Schema<ChangeData> V25 = schema(
ChangeField.LEGACY_ID2,
ChangeField.ID,
ChangeField.STATUS,
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
ChangeField.EXACT_TOPIC,
ChangeField.FUZZY_TOPIC,
ChangeField.UPDATED,
ChangeField.FILE_PART,
ChangeField.PATH,
ChangeField.OWNER,
ChangeField.REVIEWER,
ChangeField.COMMIT,
ChangeField.TR,
ChangeField.LABEL,
ChangeField.COMMIT_MESSAGE,
ChangeField.COMMENT,
ChangeField.CHANGE,
ChangeField.APPROVAL,
ChangeField.MERGEABLE,
ChangeField.ADDED,
ChangeField.DELETED,
ChangeField.DELTA,
ChangeField.HASHTAG,
ChangeField.COMMENTBY,
ChangeField.PATCH_SET,
ChangeField.GROUP,
ChangeField.SUBMISSIONID,
ChangeField.EDITBY,
ChangeField.REVIEWEDBY,
ChangeField.EXACT_COMMIT,
ChangeField.AUTHOR,
ChangeField.COMMITTER);
private static Schema<ChangeData> schema(Collection<FieldDef<ChangeData, ?>> fields) {
return new Schema<>(ImmutableList.copyOf(fields));
}

9
gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
View File

@ -41,6 +41,7 @@ import org.eclipse.jgit.lib.ObjectId;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
/**
@ -194,6 +195,14 @@ public class InternalChangeQuery {
return query(commit(schema(indexes), id.name()));
}
public List<ChangeData> bySubmissionId(String cs) throws OrmException {
if (cs.isEmpty()) {
return Collections.emptyList();
} else {
return query(new SubmissionIdPredicate(cs));
}
}
public List<ChangeData> byProjectGroups(Project.NameKey project,
Collection<String> groups) throws OrmException {
List<GroupPredicate> groupPredicates = new ArrayList<>(groups.size());

44
gerrit-server/src/main/java/com/google/gerrit/server/query/change/SubmissionIdPredicate.java Normal file
View File

@ -0,0 +1,44 @@
// Copyright (C) 2015 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.query.change;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.index.ChangeField;
import com.google.gerrit.server.index.IndexPredicate;
import com.google.gwtorm.server.OrmException;
class SubmissionIdPredicate extends IndexPredicate<ChangeData> {
SubmissionIdPredicate(String changeSet) {
super(ChangeField.SUBMISSIONID, changeSet);
}
@Override
public boolean match(ChangeData object) throws OrmException {
Change change = object.change();
if (change == null) {
return false;
}
if (change.getSubmissionId() == null) {
return false;
}
return getValue().equals(change.getSubmissionId());
}
@Override
public int getCost() {
return 1;
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java
View File

@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
public static final Class<Schema_111> C = Schema_111.class;
public static final Class<Schema_112> C = Schema_112.class;
public static int getBinaryVersion() {
return guessVersion(C);

30
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_112.java Normal file
View File

@ -0,0 +1,30 @@
// Copyright (C) 2015 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.inject.Inject;
import com.google.inject.Provider;
import java.sql.SQLException;
import java.sql.Statement;
public class Schema_112 extends SchemaVersion {
@Inject
Schema_112(Provider<Schema_111> prior) {
super(prior);
}
}