From 5a142312e9f83cf2a230263fbc1cafb838cf5940 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 5 Jan 2017 12:30:27 -0500 Subject: [PATCH] Remove sequence-based UID generation for messages Unique identifiers for ChangeMessages and PatchLineComments were generated using a seed from a database sequence, mixed together with IdGenerator. In the spirit of killing ReviewDb, use a cryptographically secure pseudorandom number generator to generate these IDs instead. Follow the same "xxxxxxxx_xxxxxxxx" format so IDs look similar. I suppose the goal of the sequence-based approach was to decrease the chance of collision by including a piece of state in each ID that is guaranteed unique across the server. However, the chance of collision for any 8-byte identifier can never be less than 1/2^64 no matter what implementation we choose. By definition a cryptographically secure PRNG generates random data that is evenly distributed across the entire byte space, so the PRNG implementation also has a 1/2^64 chance of collisions; we're just swapping one rare-collision implementation with another. (This assumes no bugs or weaknesses in the PRNG implementation, but with all due respect to Shawn, I don't think the JDK's PRNG is any more likely to have collision-introducing flaws than our hand-written IdGenerator.) This change actually has no effect on ChangeMessage IDs that are stored in or read from NoteDb, because ChangeNotesParser always populates the ChangeMessage UUID field with the SHA-1 in the notes graph of the commit that introduced the message; ReviewDb UUIDs are simply discarded. The messageUuid method is still called during creation because of the implementation details of ChangeMessagesUtil, but that ID is not used for storage. This change does have an effect on the IDs generated for PatchLineComments, which also use messageUuid for their IDs, despite the somewhat confusing fact that the ReviewDb interface implies the ID is just for ChangeMessages. While we're in there, rename the method to messageUuid to match the Google Java Style Guide for camelCase names. Change-Id: I7d157b9e8f87b41d4e7a146b705012863c27d42e --- .../server/notedb/ChangeRebuilderIT.java | 4 +- .../gerrit/reviewdb/server/ReviewDb.java | 9 - .../reviewdb/server/ReviewDbWrapper.java | 5 - .../gerrit/server/ChangeMessagesUtil.java | 12 +- .../com/google/gerrit/server/ChangeUtil.java | 41 ++--- .../google/gerrit/server/CommentsUtil.java | 6 +- .../gerrit/server/change/ChangeInserter.java | 2 +- .../server/change/CherryPickChange.java | 2 +- .../server/change/PatchSetInserter.java | 2 +- .../gerrit/server/change/PostReview.java | 19 +-- .../server/change/PublishDraftPatchSet.java | 7 +- .../gerrit/server/change/PutDescription.java | 2 +- .../google/gerrit/server/change/Restore.java | 2 +- .../google/gerrit/server/git/AbandonOp.java | 2 +- .../com/google/gerrit/server/git/MergeOp.java | 2 +- .../gerrit/server/git/MergedByPushOp.java | 2 +- .../google/gerrit/server/git/ReplaceOp.java | 2 +- .../server/git/strategy/SubmitStrategyOp.java | 4 +- .../gerrit/server/schema/SchemaVersion.java | 2 +- .../gerrit/server/schema/Schema_140.java | 26 +++ .../google/gerrit/server/ChangeUtilTest.java | 154 +----------------- .../gerrit/testutil/DisabledReviewDb.java | 5 - 22 files changed, 75 insertions(+), 237 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_140.java diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java index 555a81984a..59fbb321d6 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java @@ -457,7 +457,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { public boolean updateChange(ChangeContext ctx) throws OrmException { PatchSet.Id psId = ctx.getChange().currentPatchSetId(); ChangeMessage cm = new ChangeMessage( - new ChangeMessage.Key(id, ChangeUtil.messageUUID(ctx.getDb())), + new ChangeMessage.Key(id, ChangeUtil.messageUuid()), ctx.getAccountId(), ctx.getWhen(), psId); cm.setMessage(msg); ctx.getDb().changeMessages().insert(Collections.singleton(cm)); @@ -1278,7 +1278,7 @@ public class ChangeRebuilderIT extends AbstractDaemonTest { private ChangeMessage insertMessage(Change.Id id, PatchSet.Id psId, Account.Id author, Timestamp ts, String message) throws Exception { ChangeMessage msg = new ChangeMessage( - new ChangeMessage.Key(id, ChangeUtil.messageUUID(db)), + new ChangeMessage.Key(id, ChangeUtil.messageUuid()), author, ts, psId); msg.setMessage(message); db.changeMessages().insert(Collections.singleton(msg)); diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java index 9df3169c8d..dafca819a6 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java @@ -17,7 +17,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.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.SystemConfig; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.Relation; @@ -118,12 +117,4 @@ public interface ReviewDb extends Schema { @Sequence(startWith = FIRST_CHANGE_ID) @Deprecated int nextChangeId() throws OrmException; - - /** - * Next id for a block of {@link ChangeMessage} records. - * - * @see com.google.gerrit.server.ChangeUtil#messageUUID(ReviewDb) - */ - @Sequence - int nextChangeMessageId() throws OrmException; } diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java index 31503d6786..50f22ce65e 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java @@ -159,11 +159,6 @@ public class ReviewDbWrapper implements ReviewDb { return delegate.nextChangeId(); } - @Override - public int nextChangeMessageId() throws OrmException { - return delegate.nextChangeMessageId(); - } - public static class ChangeAccessWrapper implements ChangeAccess { protected final ChangeAccess delegate; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java index 55c7e218b0..e49b61719e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeMessagesUtil.java @@ -73,20 +73,20 @@ public class ChangeMessagesUtil { public static final String TAG_UPLOADED_PATCH_SET = "autogenerated:gerrit:newPatchSet"; - public static ChangeMessage newMessage( BatchUpdate.ChangeContext ctx, - String body, @Nullable String tag) throws OrmException { + public static ChangeMessage newMessage(BatchUpdate.ChangeContext ctx, + String body, @Nullable String tag) { return newMessage( - ctx.getDb(), ctx.getChange().currentPatchSetId(), + ctx.getChange().currentPatchSetId(), ctx.getUser(), ctx.getWhen(), body, tag); } public static ChangeMessage newMessage( - ReviewDb db, PatchSet.Id psId, CurrentUser user, Timestamp when, - String body, @Nullable String tag) throws OrmException { + PatchSet.Id psId, CurrentUser user, Timestamp when, + String body, @Nullable String tag) { checkNotNull(psId); Account.Id accountId = user.isInternalUser() ? null : user.getAccountId(); ChangeMessage m = new ChangeMessage( - new ChangeMessage.Key(psId.getParentKey(), ChangeUtil.messageUUID(db)), + new ChangeMessage.Key(psId.getParentKey(), ChangeUtil.messageUuid()), accountId, when, psId); m.setMessage(body); m.setTag(tag); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 7866ed3f2c..fbf6d03b6e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -17,10 +17,8 @@ package com.google.gerrit.server; import static java.util.Comparator.comparingInt; import com.google.common.collect.Ordering; +import com.google.common.io.BaseEncoding; import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.util.IdGenerator; -import com.google.gwtorm.server.OrmException; import com.google.inject.Singleton; import org.eclipse.jgit.lib.Ref; @@ -28,14 +26,15 @@ import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.Repository; import java.io.IOException; +import java.security.SecureRandom; import java.util.Map; +import java.util.Random; @Singleton public class ChangeUtil { - private static final Object uuidLock = new Object(); - private static final int SEED = 0x2418e6f9; - private static int uuidPrefix; - private static int uuidSeq; + private static final Random UUID_RANDOM = new SecureRandom(); + private static final BaseEncoding UUID_ENCODING = + BaseEncoding.base16().lowerCase(); private static final int SUBJECT_MAX_LENGTH = 80; private static final String SUBJECT_CROP_APPENDIX = "..."; @@ -44,28 +43,12 @@ public class ChangeUtil { public static final Ordering PS_ID_ORDER = Ordering.from(comparingInt(PatchSet::getPatchSetId)); - /** - * Generate a new unique identifier for change message entities. - * - * @param db the database connection, used to increment the change message - * allocation sequence. - * @return the new unique identifier. - * @throws OrmException the database couldn't be incremented. - */ - public static String messageUUID(ReviewDb db) throws OrmException { - int p; - int s; - synchronized (uuidLock) { - if (uuidSeq == 0) { - uuidPrefix = db.nextChangeMessageId(); - uuidSeq = Integer.MAX_VALUE; - } - p = uuidPrefix; - s = uuidSeq--; - } - String u = IdGenerator.format(IdGenerator.mix(SEED, p)); - String l = IdGenerator.format(IdGenerator.mix(p, s)); - return u + '_' + l; + /** @return a new unique identifier for change message entities. */ + public static String messageUuid() { + byte[] buf = new byte[8]; + UUID_RANDOM.nextBytes(buf); + return UUID_ENCODING.encode(buf, 0, 4) + '_' + + UUID_ENCODING.encode(buf, 4, 4); } public static PatchSet.Id nextPatchSetId(Map allRefs, diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java index f012c2107e..9de46c60ba 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java @@ -166,7 +166,7 @@ public class CommentsUtil { } } Comment c = new Comment( - new Comment.Key(ChangeUtil.messageUUID(ctx.getDb()), path, psId.get()), + new Comment.Key(ChangeUtil.messageUuid(), path, psId.get()), ctx.getUser().getAccountId(), ctx.getWhen(), side, message, serverId, unresolved); ctx.getUser().updateRealAccountId(c::setRealAuthor); @@ -175,9 +175,9 @@ public class CommentsUtil { public RobotComment newRobotComment(ChangeContext ctx, String path, PatchSet.Id psId, short side, String message, String robotId, - String robotRunId) throws OrmException { + String robotRunId) { RobotComment c = new RobotComment( - new Comment.Key(ChangeUtil.messageUUID(ctx.getDb()), path, psId.get()), + new Comment.Key(ChangeUtil.messageUuid(), path, psId.get()), ctx.getUser().getAccountId(), ctx.getWhen(), side, message, serverId, robotId, robotRunId); ctx.getUser().updateRealAccountId(c::setRealAuthor); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index 50d791758c..d36be886e0 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -381,7 +381,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp { db, update, labelTypes, patchSet, ctx.getControl(), approvals); if (message != null) { changeMessage = ChangeMessagesUtil.newMessage( - db, patchSet.getId(), ctx.getUser(), patchSet.getCreatedOn(), + patchSet.getId(), ctx.getUser(), patchSet.getCreatedOn(), message, ChangeMessagesUtil.TAG_UPLOADED_PATCH_SET); cmUtil.addChangeMessage(db, update, changeMessage); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java index 42a96fa81f..f09e268673 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java @@ -290,7 +290,7 @@ public class CherryPickChange { .append(" as commit ") .append(cherryPickCommit.name()); ChangeMessage changeMessage = ChangeMessagesUtil.newMessage( - ctx.getDb(), psId, ctx.getUser(), ctx.getWhen(), sb.toString(), + psId, ctx.getUser(), ctx.getWhen(), sb.toString(), ChangeMessagesUtil.TAG_CHERRY_PICK_CHANGE); cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId), changeMessage); return true; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index 8b899c3de7..6b942325db 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -250,7 +250,7 @@ public class PatchSetInserter extends BatchUpdate.Op { if (message != null) { changeMessage = ChangeMessagesUtil.newMessage( - db, patchSet.getId(), ctx.getUser(), ctx.getWhen(), message, + patchSet.getId(), ctx.getUser(), ctx.getWhen(), message, ChangeMessagesUtil.TAG_UPLOADED_PATCH_SET); changeMessage.setMessage(message); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index bd953c22d2..9e5418063a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -846,13 +846,13 @@ public class PostReview implements RestModifyView robotCommentInput.range); robotComment.tag = in.tag; setCommentRevId(robotComment, patchListCache, ctx.getChange(), ps); - robotComment.fixSuggestions = createFixSuggestionsFromInput(ctx, - robotCommentInput.fixSuggestions); + robotComment.fixSuggestions = + createFixSuggestionsFromInput(robotCommentInput.fixSuggestions); return robotComment; } - private List createFixSuggestionsFromInput(ChangeContext ctx, - List fixSuggestionInfos) throws OrmException { + private List createFixSuggestionsFromInput( + List fixSuggestionInfos) { if (fixSuggestionInfos == null) { return Collections.emptyList(); } @@ -860,17 +860,16 @@ public class PostReview implements RestModifyView List fixSuggestions = new ArrayList<>(fixSuggestionInfos.size()); for (FixSuggestionInfo fixSuggestionInfo : fixSuggestionInfos) { - fixSuggestions.add( - createFixSuggestionFromInput(ctx, fixSuggestionInfo)); + fixSuggestions.add(createFixSuggestionFromInput(fixSuggestionInfo)); } return fixSuggestions; } - private FixSuggestion createFixSuggestionFromInput(ChangeContext ctx, - FixSuggestionInfo fixSuggestionInfo) throws OrmException { + private FixSuggestion createFixSuggestionFromInput( + FixSuggestionInfo fixSuggestionInfo) { List fixReplacements = toFixReplacements(fixSuggestionInfo.replacements); - String fixId = ChangeUtil.messageUUID(ctx.getDb()); + String fixId = ChangeUtil.messageUuid(); return new FixSuggestion(fixId, fixSuggestionInfo.description, fixReplacements); } @@ -1235,7 +1234,7 @@ public class PostReview implements RestModifyView } message = ChangeMessagesUtil.newMessage( - ctx.getDb(), psId, user, ctx.getWhen(), + psId, user, ctx.getWhen(), "Patch Set " + psId.get() + ":" + buf, in.tag); cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId), message); return true; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java index ba2700500b..5a9a1e5565 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java @@ -235,7 +235,7 @@ public class PublishDraftPatchSet implements RestModifyView, return true; } - private ChangeMessage newMessage(ChangeContext ctx) throws OrmException { + private ChangeMessage newMessage(ChangeContext ctx) { StringBuilder msg = new StringBuilder(); msg.append("Restored"); if (!Strings.nullToEmpty(input.message).trim().isEmpty()) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java index 84ac33b429..f152e6b313 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/AbandonOp.java @@ -111,7 +111,7 @@ public class AbandonOp extends BatchUpdate.Op { return true; } - private ChangeMessage newMessage(ChangeContext ctx) throws OrmException { + private ChangeMessage newMessage(ChangeContext ctx) { StringBuilder msg = new StringBuilder(); msg.append("Abandoned"); if (!Strings.nullToEmpty(msgTxt).trim().isEmpty()) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index 1ed671a799..de8c5b7cfb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -761,7 +761,7 @@ public class MergeOp implements AutoCloseable { change.setStatus(Change.Status.ABANDONED); ChangeMessage msg = ChangeMessagesUtil.newMessage( - ctx.getDb(), change.currentPatchSetId(), + change.currentPatchSetId(), internalUserFactory.create(), change.getLastUpdatedOn(), ChangeMessagesUtil.TAG_MERGED, "Project was deleted."); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergedByPushOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergedByPushOp.java index d7c1cda3b6..1502c4ae28 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergedByPushOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergedByPushOp.java @@ -151,7 +151,7 @@ public class MergedByPushOp extends BatchUpdate.Op { } msgBuf.append("."); ChangeMessage msg = ChangeMessagesUtil.newMessage( - ctx.getDb(), psId, ctx.getUser(), ctx.getWhen(), msgBuf.toString(), + psId, ctx.getUser(), ctx.getWhen(), msgBuf.toString(), ChangeMessagesUtil.TAG_MERGED); cmUtil.addChangeMessage(ctx.getDb(), update, msg); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java index 42180143f7..2de5378e66 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java @@ -285,7 +285,7 @@ public class ReplaceOp extends BatchUpdate.Op { if (!Strings.isNullOrEmpty(reviewMessage)) { message.append("\n").append(reviewMessage); } - msg = ChangeMessagesUtil.newMessage(ctx.getDb(), patchSetId, ctx.getUser(), + msg = ChangeMessagesUtil.newMessage(patchSetId, ctx.getUser(), ctx.getWhen(), message.toString(), ChangeMessagesUtil.TAG_UPLOADED_PATCH_SET); cmUtil.addChangeMessage(ctx.getDb(), update, msg); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java index 79642fc1bc..d556de7efe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategyOp.java @@ -449,9 +449,9 @@ abstract class SubmitStrategyOp extends BatchUpdate.Op { } private ChangeMessage message(ChangeContext ctx, PatchSet.Id psId, - String body) throws OrmException { + String body) { return ChangeMessagesUtil.newMessage( - ctx.getDb(), psId, ctx.getUser(), ctx.getWhen(), body, + psId, ctx.getUser(), ctx.getWhen(), body, ChangeMessagesUtil.TAG_MERGED); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index f4e5f7f9e7..142c60e8c1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -36,7 +36,7 @@ import java.util.concurrent.TimeUnit; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - public static final Class C = Schema_139.class; + public static final Class C = Schema_140.class; public static int getBinaryVersion() { return guessVersion(C); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_140.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_140.java new file mode 100644 index 0000000000..bdc5f5525c --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_140.java @@ -0,0 +1,26 @@ +// Copyright (C) 2017 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.inject.Inject; +import com.google.inject.Provider; + +/** Remove ChangeMessage sequence. */ +public class Schema_140 extends SchemaVersion { + @Inject + Schema_140(Provider prior) { + super(prior); + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/ChangeUtilTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/ChangeUtilTest.java index 4aaa036ca7..92d3a1ceaf 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/ChangeUtilTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/ChangeUtilTest.java @@ -16,28 +16,8 @@ package com.google.gerrit.server; import static com.google.common.truth.Truth.assertThat; -import com.google.gerrit.reviewdb.server.AccountAccess; -import com.google.gerrit.reviewdb.server.AccountExternalIdAccess; -import com.google.gerrit.reviewdb.server.AccountGroupAccess; -import com.google.gerrit.reviewdb.server.AccountGroupByIdAccess; -import com.google.gerrit.reviewdb.server.AccountGroupByIdAudAccess; -import com.google.gerrit.reviewdb.server.AccountGroupMemberAccess; -import com.google.gerrit.reviewdb.server.AccountGroupMemberAuditAccess; -import com.google.gerrit.reviewdb.server.AccountGroupNameAccess; -import com.google.gerrit.reviewdb.server.ChangeAccess; -import com.google.gerrit.reviewdb.server.ChangeMessageAccess; -import com.google.gerrit.reviewdb.server.PatchLineCommentAccess; -import com.google.gerrit.reviewdb.server.PatchSetAccess; -import com.google.gerrit.reviewdb.server.PatchSetApprovalAccess; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.reviewdb.server.SchemaVersionAccess; -import com.google.gerrit.reviewdb.server.SystemConfigAccess; -import com.google.gwtorm.server.Access; -import com.google.gwtorm.server.StatementExecutor; - import org.junit.Test; -import java.util.concurrent.atomic.AtomicInteger; import java.util.regex.Pattern; public class ChangeUtilTest { @@ -46,141 +26,11 @@ public class ChangeUtilTest { Pattern pat = Pattern.compile("^[0-9a-f]{8}_[0-9a-f]{8}$"); assertThat("abcd1234_0987fedc").matches(pat); - ReviewDb db = new FakeDb(); - String id1 = ChangeUtil.messageUUID(db); + String id1 = ChangeUtil.messageUuid(); assertThat(id1).matches(pat); - String id2 = ChangeUtil.messageUUID(db); + String id2 = ChangeUtil.messageUuid(); assertThat(id2).isNotEqualTo(id1); assertThat(id2).matches(pat); } - - private static class FakeDb implements ReviewDb { - private final AtomicInteger SEQ = new AtomicInteger(100); - - @Override - public int nextChangeMessageId() { - return SEQ.getAndIncrement(); - } - - @Override - public void commit() { - throw new UnsupportedOperationException(); - } - - @Override - public void rollback() { - throw new UnsupportedOperationException(); - } - - @Override - public void updateSchema(StatementExecutor e) { - throw new UnsupportedOperationException(); - } - - @Override - public void pruneSchema(StatementExecutor e) { - throw new UnsupportedOperationException(); - } - - @Override - public Access[] allRelations() { - throw new UnsupportedOperationException(); - } - - @Override - public void close() { - throw new UnsupportedOperationException(); - } - - @Override - public SchemaVersionAccess schemaVersion() { - throw new UnsupportedOperationException(); - } - - @Override - public SystemConfigAccess systemConfig() { - throw new UnsupportedOperationException(); - } - - @Override - public AccountAccess accounts() { - throw new UnsupportedOperationException(); - } - - @Override - public AccountExternalIdAccess accountExternalIds() { - throw new UnsupportedOperationException(); - } - - @Override - public AccountGroupAccess accountGroups() { - throw new UnsupportedOperationException(); - } - - @Override - public AccountGroupNameAccess accountGroupNames() { - throw new UnsupportedOperationException(); - } - - @Override - public AccountGroupMemberAccess accountGroupMembers() { - throw new UnsupportedOperationException(); - } - - @Override - public AccountGroupMemberAuditAccess accountGroupMembersAudit() { - throw new UnsupportedOperationException(); - } - - @Override - public ChangeAccess changes() { - throw new UnsupportedOperationException(); - } - - @Override - public PatchSetApprovalAccess patchSetApprovals() { - throw new UnsupportedOperationException(); - } - - @Override - public ChangeMessageAccess changeMessages() { - throw new UnsupportedOperationException(); - } - - @Override - public PatchSetAccess patchSets() { - throw new UnsupportedOperationException(); - } - - @Override - public PatchLineCommentAccess patchComments() { - throw new UnsupportedOperationException(); - } - - @Override - public AccountGroupByIdAccess accountGroupById() { - throw new UnsupportedOperationException(); - } - - @Override - public AccountGroupByIdAudAccess accountGroupByIdAud() { - throw new UnsupportedOperationException(); - } - - @Override - public int nextAccountId() { - throw new UnsupportedOperationException(); - } - - @Override - public int nextAccountGroupId() { - throw new UnsupportedOperationException(); - } - - @Override - public int nextChangeId() { - throw new UnsupportedOperationException(); - } - } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java index 47dbd48f91..885a1f52de 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java @@ -162,9 +162,4 @@ public class DisabledReviewDb implements ReviewDb { public int nextChangeId() { throw new Disabled(); } - - @Override - public int nextChangeMessageId() { - throw new Disabled(); - } }