Merge "Move random Change-Id generation out of Change"

This commit is contained in:
Luca Milanesio 2020-02-02 19:03:34 +00:00 committed by Gerrit Code Review
commit 2d012897bd
8 changed files with 46 additions and 36 deletions

View File

@ -16,20 +16,14 @@ package com.google.gerrit.entities;
import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.entities.RefNames.REFS_CHANGES; import static com.google.gerrit.entities.RefNames.REFS_CHANGES;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.auto.value.AutoValue; import com.google.auto.value.AutoValue;
import com.google.common.primitives.Ints; import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.client.ChangeStatus; import com.google.gerrit.extensions.client.ChangeStatus;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import java.sql.Timestamp; import java.sql.Timestamp;
import java.util.Arrays; import java.util.Arrays;
import java.util.Optional; import java.util.Optional;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
/** /**
* A change proposed to be merged into a branch. * A change proposed to be merged into a branch.
@ -101,15 +95,6 @@ import org.eclipse.jgit.lib.ObjectInserter;
* notice of a replacement patch set is sent, or when notice of the change submission occurs. * notice of a replacement patch set is sent, or when notice of the change submission occurs.
*/ */
public final class Change { public final class Change {
private static final SecureRandom rng;
static {
try {
rng = SecureRandom.getInstance("SHA1PRNG");
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException("Cannot create RNG for Change-Id generator", e);
}
}
public static Id id(int id) { public static Id id(int id) {
return new AutoValue_Change_Id(id); return new AutoValue_Change_Id(id);
@ -282,20 +267,6 @@ public final class Change {
} }
} }
public static ObjectId generateChangeId() {
byte[] rand = new byte[Constants.OBJECT_ID_STRING_LENGTH];
rng.nextBytes(rand);
String randomString = new String(rand, UTF_8);
try (ObjectInserter f = new ObjectInserter.Formatter()) {
return f.idFor(Constants.OBJ_COMMIT, Constants.encode(randomString));
}
}
public static Key generateKey() {
return key("I" + generateChangeId().name());
}
public static Key key(String key) { public static Key key(String key) {
return new AutoValue_Change_Key(key); return new AutoValue_Change_Key(key);
} }

View File

@ -68,6 +68,7 @@ import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context; import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.InsertChangeOp; import com.google.gerrit.server.update.InsertChangeOp;
import com.google.gerrit.server.update.RepoContext; import com.google.gerrit.server.update.RepoContext;
import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gerrit.server.util.RequestScopePropagator; import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@ -207,7 +208,7 @@ public class ChangeInserter implements InsertChangeOp {
} }
// A Change-Id is generated for the review, but not appended to the commit message. // A Change-Id is generated for the review, but not appended to the commit message.
// This can happen if requireChangeId is false. // This can happen if requireChangeId is false.
return Change.generateKey(); return CommitMessageUtil.generateKey();
} }
public PatchSet.Id getPatchSetId() { public PatchSet.Id getPatchSetId() {

View File

@ -49,6 +49,7 @@ import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext; import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context; import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.UpdateException; import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
@ -151,7 +152,7 @@ public class CommitUtil {
ObjectInserter oi = git.newObjectInserter(); ObjectInserter oi = git.newObjectInserter();
ObjectReader reader = oi.newReader(); ObjectReader reader = oi.newReader();
RevWalk revWalk = new RevWalk(reader)) { RevWalk revWalk = new RevWalk(reader)) {
ObjectId generatedChangeId = Change.generateChangeId(); ObjectId generatedChangeId = CommitMessageUtil.generateChangeId();
ObjectId revCommit = ObjectId revCommit =
createRevertCommit(message, notes, user, timestamp, oi, revWalk, generatedChangeId); createRevertCommit(message, notes, user, timestamp, oi, revWalk, generatedChangeId);
return createRevertChangeFromCommit( return createRevertChangeFromCommit(

View File

@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument;
import com.google.common.base.MoreObjects; import com.google.common.base.MoreObjects;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Project; import com.google.gerrit.entities.Project;
import com.google.gerrit.git.GitUpdateFailureException; import com.google.gerrit.git.GitUpdateFailureException;
import com.google.gerrit.git.LockFailureException; import com.google.gerrit.git.LockFailureException;
@ -26,6 +25,7 @@ import com.google.gerrit.git.ObjectIds;
import com.google.gerrit.server.logging.Metadata; import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext; import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer; import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.util.CommitMessageUtil;
import java.io.BufferedReader; import java.io.BufferedReader;
import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
@ -330,7 +330,8 @@ public abstract class VersionedMetaData {
} }
if (update.insertChangeId()) { if (update.insertChangeId()) {
commit.setMessage(ChangeIdUtil.insertId(commit.getMessage(), Change.generateChangeId())); commit.setMessage(
ChangeIdUtil.insertId(commit.getMessage(), CommitMessageUtil.generateChangeId()));
} }
src = rw.parseCommit(inserter.insert(commit)); src = rw.parseCommit(inserter.insert(commit));

View File

@ -60,6 +60,7 @@ import com.google.gerrit.server.submit.IntegrationException;
import com.google.gerrit.server.submit.MergeIdenticalTreeException; import com.google.gerrit.server.submit.MergeIdenticalTreeException;
import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException; import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gerrit.server.util.time.TimeUtil; import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@ -304,7 +305,9 @@ public class CherryPickChange {
PersonIdent committerIdent = identifiedUser.newCommitterIdent(timestamp, serverTimeZone); PersonIdent committerIdent = identifiedUser.newCommitterIdent(timestamp, serverTimeZone);
final ObjectId generatedChangeId = final ObjectId generatedChangeId =
changeIdForNewChange != null ? changeIdForNewChange : Change.generateChangeId(); changeIdForNewChange != null
? changeIdForNewChange
: CommitMessageUtil.generateChangeId();
String commitMessage = ChangeIdUtil.insertId(message, generatedChangeId).trim() + '\n'; String commitMessage = ChangeIdUtil.insertId(message, generatedChangeId).trim() + '\n';
CodeReviewCommit cherryPickCommit; CodeReviewCommit cherryPickCommit;

View File

@ -76,6 +76,7 @@ import com.google.gerrit.server.restapi.project.CommitsCollection;
import com.google.gerrit.server.restapi.project.ProjectsCollection; import com.google.gerrit.server.restapi.project.ProjectsCollection;
import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.UpdateException; import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gerrit.server.util.time.TimeUtil; import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@ -461,7 +462,7 @@ public class CreateChange
// Add a Change-Id line if there isn't already one // Add a Change-Id line if there isn't already one
String commitMessage = subject; String commitMessage = subject;
if (ChangeIdUtil.indexOfChangeId(commitMessage, "\n") == -1) { if (ChangeIdUtil.indexOfChangeId(commitMessage, "\n") == -1) {
ObjectId id = Change.generateChangeId(); ObjectId id = CommitMessageUtil.generateChangeId();
commitMessage = ChangeIdUtil.insertId(commitMessage, id); commitMessage = ChangeIdUtil.insertId(commitMessage, id);
} }

View File

@ -68,6 +68,7 @@ import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext; import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context; import com.google.gerrit.server.update.Context;
import com.google.gerrit.server.update.UpdateException; import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.CommitMessageUtil;
import com.google.gerrit.server.util.time.TimeUtil; import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@ -302,7 +303,7 @@ public class RevertSubmission implements RestModifyView<ChangeResource, RevertIn
// TODO (paiking): As a future change, the revert should just be done directly on the // TODO (paiking): As a future change, the revert should just be done directly on the
// target rather than just creating a commit and then cherry-picking it. // target rather than just creating a commit and then cherry-picking it.
cherryPickInput.message = revertInput.message; cherryPickInput.message = revertInput.message;
ObjectId generatedChangeId = Change.generateChangeId(); ObjectId generatedChangeId = CommitMessageUtil.generateChangeId();
Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId()); Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
// TODO (paiking): In the the future, the timestamp should be the same for all the revert // TODO (paiking): In the the future, the timestamp should be the same for all the revert
// changes. // changes.

View File

@ -14,12 +14,29 @@
package com.google.gerrit.server.util; package com.google.gerrit.server.util;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.common.base.Strings; import com.google.common.base.Strings;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
import java.security.NoSuchAlgorithmException;
import java.security.SecureRandom;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
/** Utility functions to manipulate commit messages. */ /** Utility functions to manipulate commit messages. */
public class CommitMessageUtil { public class CommitMessageUtil {
private static final SecureRandom rng;
static {
try {
rng = SecureRandom.getInstance("SHA1PRNG");
} catch (NoSuchAlgorithmException e) {
throw new RuntimeException("Cannot create RNG for Change-Id generator", e);
}
}
private CommitMessageUtil() {} private CommitMessageUtil() {}
@ -42,4 +59,18 @@ public class CommitMessageUtil {
trimmed = trimmed + "\n"; trimmed = trimmed + "\n";
return trimmed; return trimmed;
} }
public static ObjectId generateChangeId() {
byte[] rand = new byte[Constants.OBJECT_ID_STRING_LENGTH];
rng.nextBytes(rand);
String randomString = new String(rand, UTF_8);
try (ObjectInserter f = new ObjectInserter.Formatter()) {
return f.idFor(Constants.OBJ_COMMIT, Constants.encode(randomString));
}
}
public static Change.Key generateKey() {
return Change.key("I" + generateChangeId().name());
}
} }