opendev/gerrit
Code Issues Proposed changes

Update references to ReviewDb in comments

Browse Source 
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
This commit is contained in:
Dave Borowitz
2018-12-17 09:41:52 -08:00
parent e0c459b389
commit b397acf45e
12 changed files with 27 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
java/com/google/gerrit/extensions/api/changes/ChangeApi.java
View File

@@ -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.
*/

5
java/com/google/gerrit/reviewdb/client/Account.java
View File

@@ -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() {}

8
java/com/google/gerrit/reviewdb/client/Comment.java
View File

@@ -25,7 +25,7 @@ import java.util.Objects;
* <p>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).
*
* <p>{@link PatchLineComment} also represents inline comments, but in ReviewDb. There are a few
* <p>{@link PatchLineComment} historically represented comments in ReviewDb. There are a few
* notable differences:
*
* <ul>
@@ -41,9 +41,9 @@ import java.util.Objects;
* </ul>
*
* <p>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).
*/

11
java/com/google/gerrit/reviewdb/client/PatchLineComment.java
View File

@@ -23,9 +23,7 @@ import java.util.Objects;
/**
* A comment left by a user on a specific line of a {@link Patch}.
*
* <p>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}.
* <p>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.
*
* <p>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() {}

6
java/com/google/gerrit/server/ChangeMessagesUtil.java
View File

@@ -105,14 +105,14 @@ public class ChangeMessagesUtil {
* Replace an existing change message with the provided new message.
*
* <p>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);
}

4
java/com/google/gerrit/server/IdentifiedUser.java
View File

@@ -128,8 +128,8 @@ public class IdentifiedUser extends CurrentUser {
/**
* Create an IdentifiedUser, relying on current request state.
*
* <p>Can only be used from within a module that has defined request scoped {@code @RemotePeer
* SocketAddress} and {@code ReviewDb} providers.
* <p>Can only be used from within a module that has defined a request scoped {@code @RemotePeer
* SocketAddress} provider.
*/
@Singleton
public static class RequestFactory {

8
java/com/google/gerrit/server/group/db/AuditLogReader.java
View File

@@ -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<AccountGroupMemberAudit> getMembersAudit(
Repository allUsersRepo, AccountGroup.UUID uuid) throws IOException, ConfigInvalidException {
@@ -134,8 +134,8 @@ public class AuditLogReader {
private Optional<ParsedCommit> parse(AccountGroup.UUID uuid, RevCommit c) {
Optional<Account.Id> 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();
}

2
java/com/google/gerrit/server/index/change/ChangeIndexer.java
View File

@@ -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<Void> {
private final Change.Id id;

8
java/com/google/gerrit/server/notedb/ChangeNotesParser.java
View File

@@ -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);

3
java/com/google/gerrit/server/schema/SchemaCreatorImpl.java
View File

@@ -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,

3
java/com/google/gerrit/server/update/RepoView.java
View File

@@ -40,8 +40,7 @@ import org.eclipse.jgit.revwalk.RevWalk;
*
* <p>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;

5
java/com/google/gerrit/testing/InMemoryTestEnvironment.java
View File

@@ -36,7 +36,7 @@ import org.junit.runners.model.Statement;
* An in-memory test environment for integration tests.
*
* <p>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.
*
* <p>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);
}