From b397acf45e61013c202569710cb6d3faae733a27 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Mon, 17 Dec 2018 09:41:52 -0800 Subject: [PATCH] Update references to ReviewDb in comments This changed considered all instances of "ReviewDb" in .java files. Many of the comments changed, but there are exceptions: * Comments referencing old ReviewDb behavior in order to explain why things work a certain way in NoteDb. * Some TODO comments suggesting that we do something different after removing ReviewDb. Eliminating these would require actual code changes. Change-Id: I18c0ac4038ae98aa3b9eabe4e68e23b6516ea3e8 --- .../gerrit/extensions/api/changes/ChangeApi.java | 5 ++--- java/com/google/gerrit/reviewdb/client/Account.java | 5 +---- java/com/google/gerrit/reviewdb/client/Comment.java | 8 ++++---- .../gerrit/reviewdb/client/PatchLineComment.java | 11 ++--------- java/com/google/gerrit/server/ChangeMessagesUtil.java | 6 +++--- java/com/google/gerrit/server/IdentifiedUser.java | 4 ++-- .../google/gerrit/server/group/db/AuditLogReader.java | 8 ++++---- .../gerrit/server/index/change/ChangeIndexer.java | 2 +- .../gerrit/server/notedb/ChangeNotesParser.java | 8 ++------ .../gerrit/server/schema/SchemaCreatorImpl.java | 3 ++- java/com/google/gerrit/server/update/RepoView.java | 3 +-- .../gerrit/testing/InMemoryTestEnvironment.java | 5 +++-- 12 files changed, 27 insertions(+), 41 deletions(-) diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java index eb27260f09..7257da17dd 100644 --- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java +++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java @@ -346,9 +346,8 @@ public interface ChangeApi { /** * Look up a change message of a change by its id. * - * @param id the id of the change message. Note that in NoteDb, this id is the {@code ObjectId} of - * a commit on the change meta branch. In ReviewDb, it's a UUID generated randomly. That means - * a change message id could be different between NoteDb and ReviewDb. + * @param id the id of the change message. In NoteDb, this id is the {@code ObjectId} of a commit + * on the change meta branch. * @return API for accessing a change message. * @throws RestApiException if the id is invalid. */ diff --git a/java/com/google/gerrit/reviewdb/client/Account.java b/java/com/google/gerrit/reviewdb/client/Account.java index 717090e993..6acf33aada 100644 --- a/java/com/google/gerrit/reviewdb/client/Account.java +++ b/java/com/google/gerrit/reviewdb/client/Account.java @@ -151,10 +151,7 @@ public final class Account { /** The user-settable status of this account (e.g. busy, OOO, available) */ private String status; - /** - * ID of the user branch from which the account was read, {@code null} if the account was read - * from ReviewDb. - */ + /** ID of the user branch from which the account was read. */ private String metaId; protected Account() {} diff --git a/java/com/google/gerrit/reviewdb/client/Comment.java b/java/com/google/gerrit/reviewdb/client/Comment.java index ce51ddefa1..e03d0fa24e 100644 --- a/java/com/google/gerrit/reviewdb/client/Comment.java +++ b/java/com/google/gerrit/reviewdb/client/Comment.java @@ -25,7 +25,7 @@ import java.util.Objects; *

Changing fields in this class changes the storage format of inline comments in NoteDb and may * require a corresponding data migration (adding new optional fields is generally okay). * - *

{@link PatchLineComment} also represents inline comments, but in ReviewDb. There are a few + *

{@link PatchLineComment} historically represented comments in ReviewDb. There are a few * notable differences: * *

* *

For all utility classes and middle layer functionality using Comment over PatchLineComment is - * preferred, as PatchLineComment will go away together with ReviewDb. This means Comment should be - * used everywhere and only for storing inline comment in ReviewDb a conversion to PatchLineComment - * is done. Converting Comments to PatchLineComments and vice verse is done by + * preferred, as ReviewDb is gone so PatchLineComment is slated for deletion as well. This means + * Comment should be used everywhere and only for storing inline comment in ReviewDb a conversion to + * PatchLineComment is done. Converting Comments to PatchLineComments and vice verse is done by * CommentsUtil#toPatchLineComments(Change.Id, PatchLineComment.Status, Iterable) and * CommentsUtil#toComments(String, Iterable). */ diff --git a/java/com/google/gerrit/reviewdb/client/PatchLineComment.java b/java/com/google/gerrit/reviewdb/client/PatchLineComment.java index de953dcb2b..d7da2ca22c 100644 --- a/java/com/google/gerrit/reviewdb/client/PatchLineComment.java +++ b/java/com/google/gerrit/reviewdb/client/PatchLineComment.java @@ -23,9 +23,7 @@ import java.util.Objects; /** * A comment left by a user on a specific line of a {@link Patch}. * - *

This class represents an inline comment in ReviewDb. It should only be used for - * writing/reading inline comments to/from ReviewDb. For all other purposes inline comments should - * be represented by {@link Comment}. + *

New APIs should not expose this class. * * @see Comment */ @@ -169,12 +167,7 @@ public final class PatchLineComment { @Column(id = 12) protected boolean unresolved; - /** - * The RevId for the commit to which this comment is referring. - * - *

Note that this field is not stored in the database. It is just provided for users of this - * class to avoid a lookup when they don't have easy access to a ReviewDb. - */ + /** The RevId for the commit to which this comment is referring. */ protected RevId revId; protected PatchLineComment() {} diff --git a/java/com/google/gerrit/server/ChangeMessagesUtil.java b/java/com/google/gerrit/server/ChangeMessagesUtil.java index c2b6b23fb2..8b9a6f4aad 100644 --- a/java/com/google/gerrit/server/ChangeMessagesUtil.java +++ b/java/com/google/gerrit/server/ChangeMessagesUtil.java @@ -105,14 +105,14 @@ public class ChangeMessagesUtil { * Replace an existing change message with the provided new message. * *

The ID of a change message is different between NoteDb and ReviewDb. In NoteDb, it's the - * commit SHA-1, but in ReviewDb it's generated randomly. To make sure the change message can be - * deleted from both NoteDb and ReviewDb, the index of the change message must be used rather than - * its ID. + * commit SHA-1, but in ReviewDb it was generated randomly. Taking the target message as an index + * rather than an ID allowed us to delete the message from both NoteDb and ReviewDb. * * @param update change update. * @param targetMessageIdx the index of the target change message. * @param newMessage the new message which is going to replace the old. */ + // TODO(xchangcheng): Reconsider implementation now that there is only a single ID. public void replaceChangeMessage(ChangeUpdate update, int targetMessageIdx, String newMessage) { update.deleteChangeMessageByRewritingHistory(targetMessageIdx, newMessage); } diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java index d9a4cae71a..b1761f4c03 100644 --- a/java/com/google/gerrit/server/IdentifiedUser.java +++ b/java/com/google/gerrit/server/IdentifiedUser.java @@ -128,8 +128,8 @@ public class IdentifiedUser extends CurrentUser { /** * Create an IdentifiedUser, relying on current request state. * - *

Can only be used from within a module that has defined request scoped {@code @RemotePeer - * SocketAddress} and {@code ReviewDb} providers. + *

Can only be used from within a module that has defined a request scoped {@code @RemotePeer + * SocketAddress} provider. */ @Singleton public static class RequestFactory { diff --git a/java/com/google/gerrit/server/group/db/AuditLogReader.java b/java/com/google/gerrit/server/group/db/AuditLogReader.java index 61fdb60863..106ee6b472 100644 --- a/java/com/google/gerrit/server/group/db/AuditLogReader.java +++ b/java/com/google/gerrit/server/group/db/AuditLogReader.java @@ -59,8 +59,8 @@ public class AuditLogReader { } // Having separate methods for reading the two types of audit records mirrors the split in - // ReviewDb. Once ReviewDb is gone, the audit record interface becomes more flexible and we can - // revisit this, e.g. to do only a single walk, or even change the record types. + // ReviewDb. Now that ReviewDb is gone, the audit record interface is more flexible and this may + // be changed, e.g. to do only a single walk, or even change the record types. public ImmutableList getMembersAudit( Repository allUsersRepo, AccountGroup.UUID uuid) throws IOException, ConfigInvalidException { @@ -134,8 +134,8 @@ public class AuditLogReader { private Optional parse(AccountGroup.UUID uuid, RevCommit c) { Optional authorId = NoteDbUtil.parseIdent(c.getAuthorIdent(), serverId); if (!authorId.isPresent()) { - // Only report audit events from identified users, since this is a non-nullable field in - // ReviewDb. May be revisited after groups are fully migrated to NoteDb. + // Only report audit events from identified users, since this was a non-nullable field in + // ReviewDb. May be revisited. return Optional.empty(); } diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java index 580da5d55b..030b1150d5 100644 --- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java +++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java @@ -346,7 +346,7 @@ public class ChangeIndexer { } } - // Not AbstractIndexTask as it doesn't need ReviewDb. + // Not AbstractIndexTask as it doesn't need a request context. private class DeleteTask implements Callable { private final Change.Id id; diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java index e53a37f073..ddfaa4e0c7 100644 --- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java +++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java @@ -828,12 +828,8 @@ class ChangeNotesParser { throw pe; } - // Store an actual 0-vote approval in the map for a removed approval, for - // several reasons: - // - This is closer to the ReviewDb representation, which leads to less - // confusion and special-casing of NoteDb. - // - More importantly, ApprovalCopier needs an actual approval in order to - // block copying an earlier approval over a later delete. + // Store an actual 0-vote approval in the map for a removed approval, because ApprovalCopier + // needs an actual approval in order to block copying an earlier approval over a later delete. PatchSetApproval remove = new PatchSetApproval( new PatchSetApproval.Key(psId, effectiveAccountId, new LabelId(label)), (short) 0, ts); diff --git a/java/com/google/gerrit/server/schema/SchemaCreatorImpl.java b/java/com/google/gerrit/server/schema/SchemaCreatorImpl.java index 740d977aea..0e854d6ba9 100644 --- a/java/com/google/gerrit/server/schema/SchemaCreatorImpl.java +++ b/java/com/google/gerrit/server/schema/SchemaCreatorImpl.java @@ -98,7 +98,8 @@ public class SchemaCreatorImpl implements SchemaCreator { // We have to create the All-Users repository before we can use it to store the groups in it. allUsersCreator.setAdministrators(admins).create(); - // Don't rely on injection to construct Sequences, as it requires ReviewDb. + // Don't rely on injection to construct Sequences, as the default GitReferenceUpdated has a + // thick dependency stack which may not all be available at schema creation time. Sequences seqs = new Sequences( config, diff --git a/java/com/google/gerrit/server/update/RepoView.java b/java/com/google/gerrit/server/update/RepoView.java index 6b1ffa558a..23853eec59 100644 --- a/java/com/google/gerrit/server/update/RepoView.java +++ b/java/com/google/gerrit/server/update/RepoView.java @@ -40,8 +40,7 @@ import org.eclipse.jgit.revwalk.RevWalk; * *

Second, the read methods take into account any pending operations on the repository that * implementations have staged using the write methods on {@link RepoContext}. Callers do not have - * to worry about whether operations have been performed yet, and the implementation details may - * differ between ReviewDb and NoteDb, but callers just don't need to care. + * to worry about whether operations have been performed yet. */ public class RepoView { private final Repository repo; diff --git a/java/com/google/gerrit/testing/InMemoryTestEnvironment.java b/java/com/google/gerrit/testing/InMemoryTestEnvironment.java index 4a9b98fb0a..5a1e29d033 100644 --- a/java/com/google/gerrit/testing/InMemoryTestEnvironment.java +++ b/java/com/google/gerrit/testing/InMemoryTestEnvironment.java @@ -36,7 +36,7 @@ import org.junit.runners.model.Statement; * An in-memory test environment for integration tests. * *

This test environment emulates the internals of a Gerrit server without starting a Gerrit - * site. ReviewDb as well as NoteDb are represented by in-memory representations. + * site. Git repositories, including NoteDb, are stored in memory. * *

Each test is executed with a fresh and clean test environment. Hence, modifications applied in * one test don't carry over to subsequent ones. @@ -109,7 +109,8 @@ public final class InMemoryTestEnvironment implements MethodRule { // The first user is added to the "Administrators" group. See AccountManager#create(). setApiUser(accountManager.authenticate(AuthRequest.forUser("admin")).getAccountId()); - // Inject target members after setting API user, so it can @Inject a ReviewDb if it wants. + // Inject target members after setting API user, so it can @Inject request-scoped objects if it + // wants. injector.injectMembers(target); }