Replay inline comments
Requires [1,2]
If the author of an inline comment does not exist in the target system
an exception is thrown and the project import is aborted. This means
that at the moment it is not possible to import a project that has
inline comments from a user that never logged in into the target
system, which is e.g. a problem if this user has left the company. For
now this problem is ignored, but it must be addressed later in a
follow-up change.
[1] https://gerrit-review.googlesource.com/65011
[2] https://gerrit-review.googlesource.com/65010
Change-Id: If0b56741350b72c1cdd00bf528afbb39ef7df9db
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/importer/ImportProjectTask.java b/src/main/java/com/googlesource/gerrit/plugins/importer/ImportProjectTask.java
index 616e3c0..9397ed8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/importer/ImportProjectTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/importer/ImportProjectTask.java
@@ -16,16 +16,23 @@
import static java.lang.String.format;
+import com.google.common.collect.Ordering;
+import com.google.common.collect.Table;
+import com.google.common.collect.TreeBasedTable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.errors.NoSuchAccountException;
import com.google.gerrit.extensions.annotations.PluginData;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ApprovalInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
+import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
@@ -42,6 +49,9 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.PostReview;
+import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeUpdate;
@@ -49,7 +59,6 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
@@ -78,7 +87,9 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
@@ -97,12 +108,13 @@
private final GitRepositoryManager git;
private final File lockRoot;
- private final SchemaFactory<ReviewDb> schemaFactory;
+ private final ReviewDb db;
private final AccountCache accountCache;
private final CurrentUser currentUser;
private final IdentifiedUser.GenericFactory genericUserFactory;
private final ChangeControl.GenericFactory changeControlFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
+ private final Provider<PostReview> postReview;
private final ChangeUpdate.Factory updateFactory;
private final ChangeMessagesUtil cmUtil;
private final ChangeIndexer indexer;
@@ -111,6 +123,7 @@
private final Project.NameKey name;
private final String user;
private final String password;
+ private final RemoteApi api;
private final StringBuffer messages;
private Repository repo;
@@ -118,12 +131,13 @@
@Inject
ImportProjectTask(GitRepositoryManager git,
@PluginData File data,
- SchemaFactory<ReviewDb> schemaFactory,
+ ReviewDb db,
AccountCache accountCache,
- Provider<CurrentUser> currentUser,
+ CurrentUser currentUser,
IdentifiedUser.GenericFactory genericUserFactory,
ChangeControl.GenericFactory changeControlFactory,
PatchSetInfoFactory patchSetInfoFactory,
+ Provider<PostReview> postReview,
ChangeUpdate.Factory updateFactory,
ChangeMessagesUtil cmUtil,
ChangeIndexer indexer,
@@ -134,12 +148,13 @@
@Assisted StringBuffer messages) {
this.git = git;
this.lockRoot = data;
- this.schemaFactory = schemaFactory;
+ this.db = db;
this.accountCache = accountCache;
- this.currentUser = currentUser.get();
+ this.currentUser = currentUser;
this.genericUserFactory = genericUserFactory;
this.changeControlFactory = changeControlFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
+ this.postReview = postReview;
this.updateFactory = updateFactory;
this.cmUtil = cmUtil;
this.indexer = indexer;
@@ -149,6 +164,7 @@
this.user = user;
this.password = password;
this.messages = messages;
+ this.api = new RemoteApi(fromGerrit, user, password);
}
@Override
@@ -169,7 +185,7 @@
gitFetch();
replayChanges();
} catch (IOException | GitAPIException | OrmException
- | NoSuchAccountException | NoSuchChangeException e) {
+ | NoSuchAccountException | NoSuchChangeException | RestApiException e) {
messages.append(format("Unable to transfer project '%s' from"
+ " source gerrit host '%s': %s. Check log for details.",
name.get(), fromGerrit, e.getMessage()));
@@ -245,37 +261,36 @@
}
private void replayChanges() throws IOException, OrmException,
- NoSuchAccountException, NoSuchChangeException {
- List<ChangeInfo> changes =
+ NoSuchAccountException, NoSuchChangeException, RestApiException {
new RemoteApi(fromGerrit, user, password).queryChanges(name.get());
- ReviewDb db = schemaFactory.open();
+ List<ChangeInfo> changes = api.queryChanges(name.get());
RevWalk rw = new RevWalk(repo);
try {
for (ChangeInfo c : changes) {
- replayChange(rw, db, c);
+ replayChange(rw, c);
}
} finally {
rw.release();
- db.close();
}
}
- private void replayChange(RevWalk rw, ReviewDb db, ChangeInfo c)
+ private void replayChange(RevWalk rw, ChangeInfo c)
throws IOException, OrmException, NoSuchAccountException,
- NoSuchChangeException {
- Change change = createChange(db, c);
- replayRevisions(db, rw, change, c);
+ NoSuchChangeException, RestApiException {
+ Change change = createChange(c);
+ replayRevisions(rw, change, c);
db.changes().insert(Collections.singleton(change));
- replayMessages(db, change, c);
- addApprovals(db, change, c);
+ replayInlineComments(change, c);
+ replayMessages(change, c);
+ addApprovals(change, c);
- insertLinkToOriginalChange(db, change, c);
+ insertLinkToOriginalChange(change, c);
indexer.index(db, change);
}
- private Change createChange(ReviewDb db, ChangeInfo c) throws OrmException,
+ private Change createChange(ChangeInfo c) throws OrmException,
NoSuchAccountException {
Change.Id changeId = new Change.Id(db.nextChangeId());
@@ -300,7 +315,7 @@
/**
* @return the current patch set for the given change
*/
- private void replayRevisions(ReviewDb db, RevWalk rw, Change change,
+ private void replayRevisions(RevWalk rw, Change change,
ChangeInfo c) throws IOException, OrmException, NoSuchAccountException {
List<RevisionInfo> revisions = new ArrayList<>(c.revisions.values());
sortRevisionInfoByNumber(revisions);
@@ -332,10 +347,6 @@
ChangeUtil.insertAncestors(db, ps.getId(), commit);
- // TODO fetch comments:
- // GET /changes/{change-id}/revisions/{revision-id}/comments/'
- // TODO replay comments
-
createRef(repo, ps);
deleteRef(repo, ps, origRef);
}
@@ -356,6 +367,71 @@
});
}
+ private void replayInlineComments(Change change, ChangeInfo c) throws OrmException,
+ RestApiException, IOException, NoSuchChangeException,
+ NoSuchAccountException {
+ for (PatchSet ps : db.patchSets().byChange(change.getId())) {
+ Iterable<CommentInfo> comments = api.getComments(
+ c._number, ps.getRevision().get());
+
+ Table<Timestamp, Account.Id, List<CommentInfo>> t = TreeBasedTable.create(
+ Ordering.natural(), new Comparator<Account.Id>() {
+ @Override
+ public int compare(Account.Id a1, Account.Id a2) {
+ return a1.get() - a2.get();
+ }}
+ );
+
+ for (CommentInfo comment : comments) {
+ Account.Id id = resolveUser(comment.author);
+ List<CommentInfo> ci = t.get(comment.updated, id);
+ if (ci == null) {
+ ci = new ArrayList<>();
+ t.put(comment.updated, id, ci);
+ }
+ ci.add(comment);
+ }
+
+ for (Timestamp ts : t.rowKeySet()) {
+ for (Map.Entry<Account.Id, List<CommentInfo>> e : t.row(ts).entrySet()) {
+ postComments(change, ps, e.getValue(), e.getKey(), ts);
+ }
+ }
+ }
+ }
+
+ private void postComments(Change change, PatchSet ps,
+ List<CommentInfo> comments, Account.Id author, Timestamp ts)
+ throws RestApiException, OrmException, IOException, NoSuchChangeException {
+ ReviewInput input = new ReviewInput();
+ input.notify = NotifyHandling.NONE;
+ input.comments = new HashMap<>();
+
+ for (CommentInfo comment : comments) {
+ if (!input.comments.containsKey(comment.path)) {
+ input.comments.put(comment.path,
+ new ArrayList<ReviewInput.CommentInput>());
+ }
+
+ ReviewInput.CommentInput commentInput = new ReviewInput.CommentInput();
+ commentInput.id = comment.id;
+ commentInput.inReplyTo = comment.inReplyTo;
+ commentInput.line = comment.line;
+ commentInput.message = comment.message;
+ commentInput.path = comment.path;
+ commentInput.range = comment.range;
+ commentInput.side = comment.side;
+ commentInput.updated = comment.updated;
+
+ input.comments.get(comment.path).add(commentInput);
+ }
+
+ postReview.get().apply(
+ new RevisionResource(new ChangeResource(control(change, author)), ps),
+ input, ts);
+ }
+
+
private void createRef(Repository repo, PatchSet ps) throws IOException {
String ref = ps.getId().toRefName();
RefUpdate ru = repo.updateRef(ref);
@@ -402,7 +478,7 @@
return a.getAccount().getId();
}
- private void replayMessages(ReviewDb db, Change change, ChangeInfo c)
+ private void replayMessages(Change change, ChangeInfo c)
throws IOException, NoSuchChangeException, OrmException,
NoSuchAccountException {
for (ChangeMessageInfo msg : c.messages) {
@@ -418,7 +494,7 @@
}
}
- private void addApprovals(ReviewDb db, Change change, ChangeInfo c)
+ private void addApprovals(Change change, ChangeInfo c)
throws OrmException, NoSuchChangeException, IOException,
NoSuchAccountException {
List<PatchSetApproval> approvals = new ArrayList<>();
@@ -439,12 +515,12 @@
}
}
}
- db.patchSetApprovals().insert(approvals);
+ db.patchSetApprovals().upsert(approvals);
}
- private void insertLinkToOriginalChange(ReviewDb db, Change change,
+ private void insertLinkToOriginalChange(Change change,
ChangeInfo c) throws NoSuchChangeException, OrmException, IOException {
- insertMessage(db, change, "Imported from " + changeUrl(c));
+ insertMessage(change, "Imported from " + changeUrl(c));
}
private String changeUrl(ChangeInfo c) {
@@ -453,7 +529,7 @@
return url.toString();
}
- private void insertMessage(ReviewDb db, Change change, String message)
+ private void insertMessage(Change change, String message)
throws NoSuchChangeException, OrmException, IOException {
Account.Id userId = ((IdentifiedUser) currentUser).getAccountId();
ChangeUpdate update = updateFactory.create(control(change, userId));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/importer/ProjectRestEndpoint.java b/src/main/java/com/googlesource/gerrit/plugins/importer/ProjectRestEndpoint.java
index 584d014..422349e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/importer/ProjectRestEndpoint.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/importer/ProjectRestEndpoint.java
@@ -24,9 +24,16 @@
import com.google.gerrit.extensions.events.LifecycleListener;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.ConfigResource;
import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.util.RequestContext;
+import com.google.gerrit.server.util.ThreadLocalRequestContext;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.importer.ProjectRestEndpoint.Input;
@@ -50,19 +57,28 @@
private final WorkQueue queue;
private final ImportProjectTask.Factory importFactory;
+ private final ThreadLocalRequestContext tl;
+ private final CurrentUser user;
+ private final SchemaFactory<ReviewDb> schemaFactory;
private WorkQueue.Executor executor;
private ListeningExecutorService pool;
@Inject
ProjectRestEndpoint(WorkQueue queue,
- ImportProjectTask.Factory importFactory) {
+ ImportProjectTask.Factory importFactory,
+ ThreadLocalRequestContext tl,
+ CurrentUser user,
+ SchemaFactory<ReviewDb> schemaFactory) {
this.queue = queue;
this.importFactory = importFactory;
+ this.tl = tl;
+ this.user = user;
+ this.schemaFactory = schemaFactory;
}
@Override
- public String apply(ConfigResource rsrc, Input input) {
+ public String apply(ConfigResource rsrc, Input input) throws OrmException {
long startTime = System.currentTimeMillis();
StringBuffer result = new StringBuffer();
@@ -71,7 +87,7 @@
Project.NameKey name = new Project.NameKey(projectName);
Runnable task = importFactory.create(input.from, name, input.user,
input.pass, result);
- tasks.add(pool.submit(task));
+ tasks.add(pool.submit(withRequestContext(task)));
}
Futures.getUnchecked(Futures.allAsList(tasks));
// TODO: the log message below does not take the failed imports into account.
@@ -95,4 +111,36 @@
pool = null;
}
}
+
+ private Runnable withRequestContext(final Runnable task) throws OrmException {
+ final ReviewDb db = schemaFactory.open();
+ return new Runnable() {
+ @Override
+ public void run() {
+ RequestContext old = tl.setContext(new RequestContext() {
+ @Override
+
+ public CurrentUser getCurrentUser() {
+ return user;
+ }
+
+ @Override
+ public Provider<ReviewDb> getReviewDbProvider() {
+ return new Provider<ReviewDb>() {
+ @Override
+ public ReviewDb get() {
+ return db;
+ }
+ };
+ }
+ });
+ try {
+ task.run();
+ } finally {
+ tl.setContext(old);
+ db.close();
+ }
+ }
+ };
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/importer/RemoteApi.java b/src/main/java/com/googlesource/gerrit/plugins/importer/RemoteApi.java
index d72362d..ea951be 100755
--- a/src/main/java/com/googlesource/gerrit/plugins/importer/RemoteApi.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/importer/RemoteApi.java
@@ -14,8 +14,11 @@
package com.googlesource.gerrit.plugins.importer;
+import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.server.OutputFormat;
import com.google.gson.Gson;
import com.google.gson.reflect.TypeToken;
@@ -23,6 +26,7 @@
import java.io.IOException;
import java.util.EnumSet;
import java.util.List;
+import java.util.Map;
public class RemoteApi {
private final RestSession restSession;
@@ -40,14 +44,37 @@
ListChangesOption.DETAILED_ACCOUNTS,
ListChangesOption.MESSAGES,
ListChangesOption.CURRENT_REVISION,
- ListChangesOption.ALL_REVISIONS)));
+ ListChangesOption.ALL_REVISIONS,
+ ListChangesOption.ALL_COMMITS)));
RestResponse r = restSession.get(endPoint);
List<ChangeInfo> result =
newGson().fromJson(r.getReader(),
new TypeToken<List<ChangeInfo>>() {}.getType());
+
+ for (ChangeInfo c : result) {
+ for (Map.Entry<String, RevisionInfo> e : c.revisions.entrySet()) {
+ e.getValue().commit.commit = e.getKey();
+ }
+ }
+
return result;
}
+ public Iterable<CommentInfo> getComments(int changeId, String rev)
+ throws IOException {
+ String endPoint = "/changes/" + changeId + "/revisions/" + rev + "/comments";
+ RestResponse r = restSession.get(endPoint);
+ Map<String, List<CommentInfo>> result =
+ newGson().fromJson(r.getReader(),
+ new TypeToken<Map<String, List<CommentInfo>>>() {}.getType());
+ for (Map.Entry<String, List<CommentInfo>> e : result.entrySet()) {
+ for (CommentInfo i : e.getValue()) {
+ i.path = e.getKey();
+ }
+ }
+ return Iterables.concat(result.values());
+ }
+
private static Gson newGson() {
return OutputFormat.JSON_COMPACT.newGson();
}