opendev/gerrit
Code Issues Proposed changes

Don't throw IOException from ChangeInserter.createChange

Browse Source 
This IOException is only thrown when the UTF-8 encoding is not
available which is an illegal state. Hence we should rather wrap the
IOException into an IllegalStateException. This makes the method
signature cleaner and callers don't need to handle the IOException.

In JGit the ChangeIdUtil.computeChangeId method was updated so that it
no longer throws the IOException [1]. Once we upgrade to a JGit
version that contains this change we can remove the exception handling
completely.

[1] https://git.eclipse.org/r/64670

Change-Id: I2839b1f640159a30d791c5063351162f004a65b0
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-01-20 07:51:11 +01:00
parent 6e1b2aa088
commit d5e930c05f
4 changed files with 18 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
View File

@@ -149,7 +149,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
} }
@Override @Override
public Change createChange(Context ctx) throws IOException { public Change createChange(Context ctx) {
change = new Change( change = new Change(
getChangeKey(commit), getChangeKey(commit),
changeId, changeId,
@@ -163,18 +163,22 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
return change; return change;
} }
private static Change.Key getChangeKey(RevCommit commit) throws IOException { private static Change.Key getChangeKey(RevCommit commit) {
List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID); List<String> idList = commit.getFooterLines(FooterConstants.CHANGE_ID);
if (!idList.isEmpty()) { if (!idList.isEmpty()) {
return new Change.Key(idList.get(idList.size() - 1).trim()); return new Change.Key(idList.get(idList.size() - 1).trim());
} }
ObjectId id = ChangeIdUtil.computeChangeId(commit.getTree(), commit, try {
commit.getAuthorIdent(), commit.getCommitterIdent(), ObjectId id = ChangeIdUtil.computeChangeId(commit.getTree(), commit,
commit.getShortMessage()); commit.getAuthorIdent(), commit.getCommitterIdent(),
StringBuilder changeId = new StringBuilder(); commit.getShortMessage());
changeId.append("I").append(ObjectId.toString(id)); StringBuilder changeId = new StringBuilder();
return new Change.Key(changeId.toString()); changeId.append("I").append(ObjectId.toString(id));
return new Change.Key(changeId.toString());
} catch (IOException e) {
throw new IllegalStateException(e);
}
} }
public Change getChange() { public Change getChange() {

2
gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
View File

@@ -236,7 +236,7 @@ public class CherryPickChange {
ObjectInserter oi, Project.NameKey project, ObjectInserter oi, Project.NameKey project,
CodeReviewCommit cherryPickCommit, String refName, CodeReviewCommit cherryPickCommit, String refName,
IdentifiedUser identifiedUser, String topic, Branch.NameKey sourceBranch) IdentifiedUser identifiedUser, String topic, Branch.NameKey sourceBranch)
throws RestApiException, UpdateException, OrmException, IOException { throws RestApiException, UpdateException, OrmException {
Change.Id changeId = new Change.Id(seq.nextChangeId()); Change.Id changeId = new Change.Id(seq.nextChangeId());
ChangeInserter ins = changeInserterFactory.create( ChangeInserter ins = changeInserterFactory.create(
changeId, cherryPickCommit, refName) changeId, cherryPickCommit, refName)

4
gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
View File

@@ -231,7 +231,7 @@ public class BatchUpdate implements AutoCloseable {
} }
public abstract static class InsertChangeOp extends Op { public abstract static class InsertChangeOp extends Op {
public abstract Change createChange(Context ctx) throws IOException; public abstract Change createChange(Context ctx);
} }
private static class ChainedReceiveCommands { private static class ChainedReceiveCommands {
@@ -481,7 +481,7 @@ public class BatchUpdate implements AutoCloseable {
return this; return this;
} }
public BatchUpdate insertChange(InsertChangeOp op) throws IOException { public BatchUpdate insertChange(InsertChangeOp op) {
Context ctx = new Context(); Context ctx = new Context();
Change c = op.createChange(ctx); Change c = op.createChange(ctx);
checkArgument(!newChanges.containsKey(c.getId()), checkArgument(!newChanges.containsKey(c.getId()),

6
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@@ -1751,8 +1751,8 @@ public class ReceiveCommits {
ListenableFuture<Void> future = changeUpdateExector.submit( ListenableFuture<Void> future = changeUpdateExector.submit(
requestScopePropagator.wrap(new Callable<Void>() { requestScopePropagator.wrap(new Callable<Void>() {
@Override @Override
public Void call() throws IOException, OrmException, public Void call() throws OrmException, RestApiException,
RestApiException, UpdateException { UpdateException {
if (caller == Thread.currentThread()) { if (caller == Thread.currentThread()) {
insertChange(ReceiveCommits.this.db); insertChange(ReceiveCommits.this.db);
} else { } else {
@@ -1768,7 +1768,7 @@ public class ReceiveCommits {
} }
private void insertChange(ReviewDb threadLocalDb) private void insertChange(ReviewDb threadLocalDb)
throws IOException, OrmException, RestApiException, UpdateException { throws OrmException, RestApiException, UpdateException {
final PatchSet ps = ins.setGroups(groups).getPatchSet(); final PatchSet ps = ins.setGroups(groups).getPatchSet();
final Account.Id me = user.getAccountId(); final Account.Id me = user.getAccountId();
final List<FooterLine> footerLines = commit.getFooterLines(); final List<FooterLine> footerLines = commit.getFooterLines();