opendev/gerrit
Code Issues Proposed changes

Get rid of null check in ChangeMessagesUtil.addChangeMessage

Browse Source 
There was a check to ensure that the message passed into the
aforementioned function was not null. However, it does not make sense
to have this check here because we should force the caller to enforce
that. If a message passed into this function is null, we should
instead be throwing an NPE. Thus, with this change, an NPE will be
thrown by update.putChangeMessage if the message is null instead of
the method just being a no-op.

Additionally, I added a check for null in all of the callers where
there is a possibility that the argument it would pass could be null.

Change-Id: Ib39359704db08d06e9117a55f76727b0e798c60d
This commit is contained in:
Yacob Yonas 2014-07-09 16:06:11 -07:00
parent d710c0cfd8
commit 10c7dbd6bb
2 changed files with 8 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -68,9 +68,7 @@ public class ChangeMessagesUtil {
public void addChangeMessage(ReviewDb db, ChangeUpdate update,
ChangeMessage changeMessage) throws OrmException {
if (changeMessage != null) {
update.setChangeMessage(changeMessage.getMessage());
db.changeMessages().insert(Collections.singleton(changeMessage));
}
update.setChangeMessage(changeMessage.getMessage());
db.changeMessages().insert(Collections.singleton(changeMessage));
}
}

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

@ -849,7 +849,9 @@ public class MergeOp {
// TODO(yyonas): we need to be able to change the author of the message
// is not the person for whom the change was made. addMergedMessage
// did this in the past.
cmUtil.addChangeMessage(db, update, msg);
if (msg != null) {
cmUtil.addChangeMessage(db, update, msg);
}
db.commit();
sendMergedEmail(c, submitter);
@ -1022,7 +1024,9 @@ public class MergeOp {
//TODO(yyonas): atomic change is not propagated.
update = updateFactory.create(control, c.getLastUpdatedOn());
cmUtil.addChangeMessage(db, update, msg);
if (msg != null) {
cmUtil.addChangeMessage(db, update, msg);
}
db.commit();
} finally {
db.rollback();