Merge "Revert "Set CommitInfo.commit in ChangeJson#toCommit""
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index ca5d460..48ed355 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2291,6 +2291,7 @@
{
"reviewers": [
{
+ "input": "john.doe@example.com",
"approvals": {
"Verified": " 0",
"Code-Review": " 0"
@@ -2328,6 +2329,7 @@
)]}'
{
+ "input": "MyProjectVerifiers",
"error": "The group My Group has 15 members. Do you want to add them all as reviewers?",
"confirm": true
}
@@ -2342,7 +2344,7 @@
Content-Type: application/json; charset=UTF-8
{
- "reviewer": "MyProjectVerifiers",
+ "input": "MyProjectVerifiers",
"confirmed": true
}
----
@@ -4051,6 +4053,9 @@
[options="header",cols="1,^1,5"]
|===========================
|Field Name ||Description
+|`input` ||
+Value of the `reviewer` field from link:#reviewer-input[ReviewerInput]
+set while adding the reviewer.
|`reviewers` |optional|
The newly added reviewers as a list of link:#reviewer-info[
ReviewerInfo] entities.
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
new file mode 100644
index 0000000..0bd70ef
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/ChangeReviewersIT.java
@@ -0,0 +1,110 @@
+// Copyright (C) 2016 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.acceptance.rest.change;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerResult;
+import com.google.gerrit.server.change.PostReviewers;
+import com.google.gson.stream.JsonReader;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class ChangeReviewersIT extends AbstractDaemonTest {
+ @Test
+ public void addGroupAsReviewer() throws Exception {
+ // Set up two groups, one that is too large too add as reviewer, and one
+ // that is too large to add without confirmation.
+ String largeGroup = createGroup("largeGroup");
+ String mediumGroup = createGroup("mediumGroup");
+
+ int largeGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS + 1;
+ int mediumGroupSize = PostReviewers.DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK + 1;
+ List<TestAccount> users = new ArrayList<>(largeGroupSize);
+ List<String> largeGroupUsernames = new ArrayList<>(mediumGroupSize);
+ List<String> mediumGroupUsernames = new ArrayList<>(mediumGroupSize);
+ for (int i = 0; i < largeGroupSize; i++) {
+ users.add(accounts.create(name("u" + i), name("u" + i + "@example.com"),
+ "Full Name " + i));
+ largeGroupUsernames.add(users.get(i).username);
+ if (i < mediumGroupSize) {
+ mediumGroupUsernames.add(users.get(i).username);
+ }
+ }
+ gApi.groups().id(largeGroup).addMembers(
+ largeGroupUsernames.toArray(new String[largeGroupSize]));
+ gApi.groups().id(mediumGroup).addMembers(
+ mediumGroupUsernames.toArray(new String[mediumGroupSize]));
+
+ // Attempt to add overly large group as reviewers.
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ AddReviewerResult result = addReviewer(changeId, largeGroup);
+ assertThat(result.input).isEqualTo(largeGroup);
+ assertThat(result.confirm).isNull();
+ assertThat(result.error)
+ .contains("has too many members to add them all as reviewers");
+ assertThat(result.reviewers).isNull();
+
+ // Attempt to add medium group without confirmation.
+ result = addReviewer(changeId, mediumGroup);
+ assertThat(result.input).isEqualTo(mediumGroup);
+ assertThat(result.confirm).isTrue();
+ assertThat(result.error)
+ .contains("has " + mediumGroupSize + " members. Do you want to add them"
+ + " all as reviewers?");
+ assertThat(result.reviewers).isNull();
+
+ // Add medium group with confirmation.
+ AddReviewerInput in = new AddReviewerInput();
+ in.reviewer = mediumGroup;
+ in.confirmed = true;
+ result = addReviewer(changeId, in);
+ assertThat(result.input).isEqualTo(mediumGroup);
+ assertThat(result.confirm).isNull();
+ assertThat(result.error).isNull();
+ assertThat(result.reviewers).hasSize(mediumGroupSize);
+ }
+
+ private AddReviewerResult addReviewer(String changeId, String reviewer)
+ throws Exception {
+ AddReviewerInput in = new AddReviewerInput();
+ in.reviewer = reviewer;
+ return addReviewer(changeId, in);
+ }
+
+ private AddReviewerResult addReviewer(String changeId, AddReviewerInput in)
+ throws Exception {
+ RestResponse resp =
+ adminRestSession.post("/changes/" + changeId + "/reviewers", in);
+ return readContentFromJson(resp, AddReviewerResult.class);
+ }
+
+ private static <T> T readContentFromJson(RestResponse r, Class<T> clazz)
+ throws Exception {
+ r.assertOK();
+ JsonReader jsonReader = new JsonReader(r.getReader());
+ jsonReader.setLenient(true);
+ return newGson().fromJson(jsonReader, clazz);
+ }
+}
\ No newline at end of file
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AddReviewerResult.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AddReviewerResult.java
new file mode 100644
index 0000000..3bd93a4
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/AddReviewerResult.java
@@ -0,0 +1,80 @@
+// Copyright (C) 2016 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.extensions.api.changes;
+
+import com.google.gerrit.common.Nullable;
+
+import java.util.List;
+
+/**
+ * Result object representing the outcome of a request to add a reviewer.
+ */
+public class AddReviewerResult {
+ /**
+ * The identifier of an account or group that was to be added as a reviewer.
+ */
+ public String input;
+
+ /**
+ * If non-null, a string describing why the reviewer could not be added.
+ */
+ @Nullable
+ public String error;
+
+ /**
+ * Non-null and true if the reviewer cannot be added without explicit
+ * confirmation. This may be the case for groups of a certain size.
+ */
+ @Nullable
+ public Boolean confirm;
+
+ /**
+ * @{List} of individual reviewers added to the change. The size of this list
+ * may be greater than one (e.g. when a group is added). Null if no reviewers
+ * were added.
+ */
+ @Nullable
+ public List<ReviewerInfo> reviewers;
+
+ /**
+ * Constructs a partially initialized result for the given reviewer.
+ *
+ * @param input String identifier of an account or group, from user request
+ */
+ public AddReviewerResult(String input) {
+ this.input = input;
+ }
+
+ /**
+ * Constructs an error result for the given account.
+ *
+ * @param reviewer String identifier of an account or group
+ * @param error Error message
+ */
+ public AddReviewerResult(String reviewer, String error) {
+ this(reviewer);
+ this.error = error;
+ }
+
+ /**
+ * Constructs a needs-confirmation result for the given account.
+ *
+ * @param confirm Whether confirmation is needed.
+ */
+ public AddReviewerResult(String reviewer, boolean confirm) {
+ this(reviewer);
+ this.confirm = confirm;
+ }
+}
\ No newline at end of file
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerInfo.java
new file mode 100644
index 0000000..c81f8aa
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ReviewerInfo.java
@@ -0,0 +1,41 @@
+// Copyright (C) 2016 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.extensions.api.changes;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.common.AccountInfo;
+
+import java.util.Map;
+
+/**
+ * Account and approval details for an added reviewer.
+ */
+public class ReviewerInfo extends AccountInfo {
+ /**
+ * {@link Map} of label name to initial value for each approval the reviewer
+ * is responsible for.
+ */
+ @Nullable
+ public Map<String, String> approvals;
+
+ public ReviewerInfo(Integer id) {
+ super(id);
+ }
+
+ @Override
+ public String toString() {
+ return username;
+ }
+}
\ No newline at end of file
diff --git a/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/safehtml/client/HighlightSuggestOracle.java b/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/safehtml/client/HighlightSuggestOracle.java
index 06245c6..f4ff9ea4 100644
--- a/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/safehtml/client/HighlightSuggestOracle.java
+++ b/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/safehtml/client/HighlightSuggestOracle.java
@@ -115,7 +115,8 @@
@Override
public int compare(String s1, String s2) {
return Integer.compare(s2.length(), s1.length());
- }});
+ }
+ });
List<String> result = new ArrayList<>();
for (String s : queryTerms) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java
index c90b3bc..533468d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetReviewer.java
@@ -14,8 +14,8 @@
package com.google.gerrit.server.change;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
index 9a0c691..ccbd552 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java
@@ -14,11 +14,11 @@
package com.google.gerrit.server.change;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
-import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
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 432ae4d..5090f8a 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
@@ -72,7 +72,6 @@
// Injected fields.
private final PatchSetInfoFactory patchSetInfoFactory;
- private final ReviewDb db;
private final CommitValidators.Factory commitValidatorsFactory;
private final ReplacePatchSetSender.Factory replacePatchSetFactory;
private final RevisionCreated revisionCreated;
@@ -109,8 +108,7 @@
private ReviewerSet oldReviewers;
@AssistedInject
- public PatchSetInserter(ReviewDb db,
- ApprovalsUtil approvalsUtil,
+ public PatchSetInserter(ApprovalsUtil approvalsUtil,
ApprovalCopier approvalCopier,
ChangeMessagesUtil cmUtil,
PatchSetInfoFactory patchSetInfoFactory,
@@ -121,7 +119,6 @@
@Assisted ChangeControl ctl,
@Assisted PatchSet.Id psId,
@Assisted RevCommit commit) {
- this.db = db;
this.approvalsUtil = approvalsUtil;
this.approvalCopier = approvalCopier;
this.cmUtil = cmUtil;
@@ -208,6 +205,7 @@
@Override
public boolean updateChange(ChangeContext ctx)
throws ResourceConflictException, OrmException, IOException {
+ ReviewDb db = ctx.getDb();
ChangeControl ctl = ctx.getControl();
change = ctx.getChange();
@@ -222,12 +220,12 @@
List<String> newGroups = groups;
if (newGroups.isEmpty()) {
- PatchSet prevPs = psUtil.current(ctx.getDb(), ctx.getNotes());
+ PatchSet prevPs = psUtil.current(db, ctx.getNotes());
if (prevPs != null) {
newGroups = prevPs.getGroups();
}
}
- patchSet = psUtil.insert(ctx.getDb(), ctx.getRevWalk(), ctx.getUpdate(psId),
+ patchSet = psUtil.insert(db, ctx.getRevWalk(), ctx.getUpdate(psId),
psId, commit, draft, newGroups, null);
if (sendMail) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
index fabedd7..4b15fce 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java
@@ -21,6 +21,8 @@
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.AddReviewerResult;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -38,8 +40,6 @@
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.AccountsCollection;
import com.google.gerrit.server.account.GroupMembers;
-import com.google.gerrit.server.change.ReviewerJson.PostResult;
-import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.extensions.events.ReviewerAdded;
import com.google.gerrit.server.git.BatchUpdate;
@@ -128,7 +128,7 @@
}
@Override
- public PostResult apply(ChangeResource rsrc, AddReviewerInput input)
+ public AddReviewerResult apply(ChangeResource rsrc, AddReviewerInput input)
throws UpdateException, OrmException, RestApiException, IOException {
if (input.reviewer == null) {
throw new BadRequestException("missing reviewer field");
@@ -136,7 +136,7 @@
try {
Account.Id accountId = accounts.parse(input.reviewer).getAccountId();
- return putAccount(reviewerFactory.create(rsrc, accountId));
+ return putAccount(input.reviewer, reviewerFactory.create(rsrc, accountId));
} catch (UnprocessableEntityException e) {
try {
return putGroup(rsrc, input);
@@ -148,11 +148,11 @@
}
}
- private PostResult putAccount(ReviewerResource rsrc)
+ private AddReviewerResult putAccount(String reviewer, ReviewerResource rsrc)
throws OrmException, UpdateException, RestApiException {
Account member = rsrc.getReviewerUser().getAccount();
ChangeControl control = rsrc.getReviewerControl();
- PostResult result = new PostResult();
+ AddReviewerResult result = new AddReviewerResult(reviewer);
if (isValidReviewer(member, control)) {
addReviewers(rsrc.getChangeResource(), result,
ImmutableMap.of(member.getId(), control));
@@ -160,10 +160,10 @@
return result;
}
- private PostResult putGroup(ChangeResource rsrc, AddReviewerInput input)
+ private AddReviewerResult putGroup(ChangeResource rsrc, AddReviewerInput input)
throws UpdateException, RestApiException, OrmException, IOException {
GroupDescription.Basic group = groupsCollection.parseInternal(input.reviewer);
- PostResult result = new PostResult();
+ AddReviewerResult result = new AddReviewerResult(input.reviewer);
if (!isLegalReviewerGroup(group.getGroupUUID())) {
result.error = MessageFormat.format(
ChangeMessages.get().groupIsNotAllowed, group.getName());
@@ -227,7 +227,7 @@
private void addReviewers(
- ChangeResource rsrc, PostResult result, Map<Account.Id, ChangeControl> reviewers)
+ ChangeResource rsrc, AddReviewerResult result, Map<Account.Id, ChangeControl> reviewers)
throws OrmException, RestApiException, UpdateException {
try (BatchUpdate bu = batchUpdateFactory.create(
dbProvider.get(), rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
@@ -240,8 +240,8 @@
for (PatchSetApproval psa : op.added) {
// New reviewers have value 0, don't bother normalizing.
result.reviewers.add(
- json.format(new ReviewerInfo(
- psa.getAccountId()), reviewers.get(psa.getAccountId()),
+ json.format(new ReviewerInfo(psa.getAccountId().get()),
+ reviewers.get(psa.getAccountId()),
ImmutableList.of(psa)));
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
index e24a290..69cd439 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java
@@ -23,7 +23,7 @@
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRange;
import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.api.changes.ReviewerInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -40,7 +40,6 @@
import java.util.Collection;
import java.util.List;
-import java.util.Map;
import java.util.TreeMap;
@Singleton
@@ -67,7 +66,7 @@
AccountLoader loader = accountLoaderFactory.create(true);
for (ReviewerResource rsrc : rsrcs) {
ReviewerInfo info = format(new ReviewerInfo(
- rsrc.getReviewerUser().getAccountId()),
+ rsrc.getReviewerUser().getAccountId().get()),
rsrc.getReviewerControl());
loader.put(info);
infos.add(info);
@@ -132,18 +131,4 @@
return out;
}
-
- public static class ReviewerInfo extends AccountInfo {
- Map<String, String> approvals;
-
- protected ReviewerInfo(Account.Id id) {
- super(id.get());
- }
- }
-
- public static class PostResult {
- public List<ReviewerInfo> reviewers;
- public String error;
- Boolean confirm;
- }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdateReviewDb.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdateReviewDb.java
index 8351e41..1de98d3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdateReviewDb.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdateReviewDb.java
@@ -37,6 +37,18 @@
return changesWrapper;
}
+ @Override
+ public void commit() {
+ throw new UnsupportedOperationException(
+ "do not call commit; BatchUpdate always manages transactions");
+ }
+
+ @Override
+ public void rollback() {
+ throw new UnsupportedOperationException(
+ "do not call rollback; BatchUpdate always manages transactions");
+ }
+
private static class BatchUpdateChanges extends ChangeAccessWrapper {
private BatchUpdateChanges(ChangeAccess delegate) {
super(delegate);
@@ -51,14 +63,14 @@
@Override
public void upsert(Iterable<Change> instances) {
throw new UnsupportedOperationException(
- "do not call upsert; either use InsertChangeOp for insertion, or"
- + " ChangeContext#saveChange() for update");
+ "do not call upsert; existing changes are updated automatically,"
+ + " or use InsertChangeOp for insertion");
}
@Override
public void update(Iterable<Change> instances) {
throw new UnsupportedOperationException(
- "do not call update; use ChangeContext#saveChange()");
+ "do not call update; change is updated automatically");
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollection.java
index cfdedd0..741fee1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GarbageCollection.java
@@ -17,7 +17,6 @@
import com.google.common.collect.Sets;
import com.google.gerrit.common.data.GarbageCollectionResult;
import com.google.gerrit.extensions.events.GarbageCollectorListener;
-import com.google.gerrit.extensions.events.GarbageCollectorListener.Event;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.GcConfig;
@@ -115,17 +114,10 @@
}
private void fire(final Project.NameKey p, final Properties statistics) {
- Event event = new GarbageCollectorListener.Event() {
- @Override
- public String getProjectName() {
- return p.get();
- }
-
- @Override
- public Properties getStatistics() {
- return statistics;
- }
- };
+ if (!listeners.iterator().hasNext()) {
+ return;
+ }
+ Event event = new Event(p, statistics);
for (GarbageCollectorListener l : listeners) {
try {
l.onGarbageCollected(event);
@@ -204,4 +196,24 @@
writer.print(message);
}
}
+
+ private static class Event implements GarbageCollectorListener.Event {
+ private final Project.NameKey p;
+ private final Properties statistics;
+
+ Event(Project.NameKey p, Properties statistics) {
+ this.p = p;
+ this.statistics = statistics;
+ }
+
+ @Override
+ public String getProjectName() {
+ return p.get();
+ }
+
+ @Override
+ public Properties getStatistics() {
+ return statistics;
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
index 2a447e4..4c77038 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java
@@ -94,7 +94,7 @@
private final ProjectJson json;
private final ProjectControl.GenericFactory projectControlFactory;
private final GitRepositoryManager repoManager;
- private final DynamicSet<NewProjectCreatedListener> createdListener;
+ private final DynamicSet<NewProjectCreatedListener> createdListeners;
private final ProjectCache projectCache;
private final GroupBackend groupBackend;
private final ProjectOwnerGroupsProvider.Factory projectOwnerGroups;
@@ -113,7 +113,7 @@
DynamicSet<ProjectCreationValidationListener> projectCreationValidationListeners,
ProjectControl.GenericFactory projectControlFactory,
GitRepositoryManager repoManager,
- DynamicSet<NewProjectCreatedListener> createdListener,
+ DynamicSet<NewProjectCreatedListener> createdListeners,
ProjectCache projectCache,
GroupBackend groupBackend,
ProjectOwnerGroupsProvider.Factory projectOwnerGroups,
@@ -131,7 +131,7 @@
this.json = json;
this.projectControlFactory = projectControlFactory;
this.repoManager = repoManager;
- this.createdListener = createdListener;
+ this.createdListeners = createdListeners;
this.projectCache = projectCache;
this.groupBackend = groupBackend;
this.projectOwnerGroups = projectOwnerGroups;
@@ -253,24 +253,7 @@
createEmptyCommits(repo, nameKey, args.branch);
}
- NewProjectCreatedListener.Event event = new NewProjectCreatedListener.Event() {
- @Override
- public String getProjectName() {
- return nameKey.get();
- }
-
- @Override
- public String getHeadName() {
- return head;
- }
- };
- for (NewProjectCreatedListener l : createdListener) {
- try {
- l.onNewProjectCreated(event);
- } catch (RuntimeException e) {
- log.warn("Failure in NewProjectCreatedListener", e);
- }
- }
+ fire(nameKey, head);
return projectCache.get(nameKey).getProject();
}
@@ -395,4 +378,39 @@
throw e;
}
}
+
+ private void fire(Project.NameKey name, String head) {
+ if (!createdListeners.iterator().hasNext()) {
+ return;
+ }
+
+ Event event = new Event(name, head);
+ for (NewProjectCreatedListener l : createdListeners) {
+ try {
+ l.onNewProjectCreated(event);
+ } catch (RuntimeException e) {
+ log.warn("Failure in NewProjectCreatedListener", e);
+ }
+ }
+ }
+
+ static class Event implements NewProjectCreatedListener.Event {
+ private final Project.NameKey name;
+ private final String head;
+
+ Event(Project.NameKey name, String head) {
+ this.name = name;
+ this.head = head;
+ }
+
+ @Override
+ public String getProjectName() {
+ return name.get();
+ }
+
+ @Override
+ public String getHeadName() {
+ return head;
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetHead.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetHead.java
index aa06024..f964311 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/SetHead.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/SetHead.java
@@ -23,6 +23,7 @@
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -53,15 +54,15 @@
private final GitRepositoryManager repoManager;
private final Provider<IdentifiedUser> identifiedUser;
- private final DynamicSet<HeadUpdatedListener> headUpdatedListener;
+ private final DynamicSet<HeadUpdatedListener> headUpdatedListeners;
@Inject
SetHead(GitRepositoryManager repoManager,
Provider<IdentifiedUser> identifiedUser,
- DynamicSet<HeadUpdatedListener> headUpdatedListener) {
+ DynamicSet<HeadUpdatedListener> headUpdatedListeners) {
this.repoManager = repoManager;
this.identifiedUser = identifiedUser;
- this.headUpdatedListener = headUpdatedListener;
+ this.headUpdatedListeners = headUpdatedListeners;
}
@Override
@@ -106,33 +107,52 @@
throw new IOException("Setting HEAD failed with " + res);
}
- HeadUpdatedListener.Event event = new HeadUpdatedListener.Event() {
- @Override
- public String getProjectName() {
- return rsrc.getNameKey().get();
- }
-
- @Override
- public String getOldHeadName() {
- return oldHead;
- }
-
- @Override
- public String getNewHeadName() {
- return newHead;
- }
- };
- for (HeadUpdatedListener l : headUpdatedListener) {
- try {
- l.onHeadUpdated(event);
- } catch (RuntimeException e) {
- log.warn("Failure in HeadUpdatedListener", e);
- }
- }
+ fire(rsrc.getNameKey(), oldHead, newHead);
}
return ref;
} catch (RepositoryNotFoundException e) {
throw new ResourceNotFoundException(rsrc.getName());
}
}
+
+ private void fire(Project.NameKey nameKey, String oldHead, String newHead) {
+ if (!headUpdatedListeners.iterator().hasNext()) {
+ return;
+ }
+ Event event = new Event(nameKey, oldHead, newHead);
+ for (HeadUpdatedListener l : headUpdatedListeners) {
+ try {
+ l.onHeadUpdated(event);
+ } catch (RuntimeException e) {
+ log.warn("Failure in HeadUpdatedListener", e);
+ }
+ }
+ }
+
+ static class Event implements HeadUpdatedListener.Event {
+ private final Project.NameKey nameKey;
+ private final String oldHead;
+ private final String newHead;
+
+ Event(Project.NameKey nameKey, String oldHead, String newHead) {
+ this.nameKey = nameKey;
+ this.oldHead = oldHead;
+ this.newHead = newHead;
+ }
+
+ @Override
+ public String getProjectName() {
+ return nameKey.get();
+ }
+
+ @Override
+ public String getOldHeadName() {
+ return oldHead;
+ }
+
+ @Override
+ public String getNewHeadName() {
+ return newHead;
+ }
+ }
}
diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java
index 3208d00..f98d63f 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java
@@ -83,7 +83,7 @@
}
} catch (RepositoryNotFoundException e) {
repo = new Repo(name);
- repos.put(name.get().toLowerCase(), repo);
+ repos.put(normalize(name), repo);
}
return repo;
}
@@ -113,12 +113,20 @@
}
}
+ public synchronized void deleteRepository(Project.NameKey name) {
+ repos.remove(normalize(name));
+ }
+
private synchronized Repo get(Project.NameKey name)
throws RepositoryNotFoundException {
- Repo repo = repos.get(name.get().toLowerCase());
+ Repo repo = repos.get(normalize(name));
if (repo != null) {
return repo;
}
throw new RepositoryNotFoundException(name.get());
}
+
+ private static String normalize(Project.NameKey name) {
+ return name.get().toLowerCase();
+ }
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index c7705a4..40364fa 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -77,10 +77,12 @@
this._clearDiffContent();
+ console.time('diff render');
this.$.processor.process(diff.content).then(function() {
if (this.isImageDiff) {
this._builder.renderDiffImages();
}
+ console.timeEnd('diff render');
this.fire('render');
}.bind(this));
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index f4fa310..b035af1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -17,6 +17,8 @@
// Prevent redefinition.
if (window.GrDiffBuilder) { return; }
+ var REGEX_ASTRAL_SYMBOL = /[\uD800-\uDBFF][\uDC00-\uDFFF]/;
+
function GrDiffBuilder(diff, comments, prefs, outputEl) {
this._diff = diff;
this._comments = comments;
@@ -30,8 +32,6 @@
GrDiffBuilder.AMPERSAND_CODE = '&'.charCodeAt(0);
GrDiffBuilder.SEMICOLON_CODE = ';'.charCodeAt(0);
- GrDiffBuilder.TAB_REGEX = /\t/g;
-
GrDiffBuilder.LINE_FEED_HTML =
'<span class="style-scope gr-diff br"></span>';
@@ -298,10 +298,10 @@
td.classList.add(line.type);
var text = line.text;
var html = util.escapeHTML(text);
+ html = this._addTabWrappers(html, this._prefs.tab_size);
td.classList.add(line.highlights.length > 0 ?
'lightHighlight' : 'darkHighlight');
-
if (line.highlights.length > 0) {
html = this._addIntralineHighlights(text, html, line.highlights);
}
@@ -310,7 +310,6 @@
this._prefs.line_length) {
html = this._addNewlines(text, html);
}
- html = this._addTabWrappers(html);
var contentText = this._createElement('div', 'contentText');
@@ -327,12 +326,16 @@
return td;
};
+ /**
+ * Returns the text length after normalizing unicode and tabs.
+ * @return {Number} The normalized length of the text.
+ */
GrDiffBuilder.prototype._textLength = function(text, tabSize) {
- // TODO(andybons): Unicode support.
+ text = text.replace(REGEX_ASTRAL_SYMBOL, '_');
var numChars = 0;
for (var i = 0; i < text.length; i++) {
if (text[i] === '\t') {
- numChars += tabSize;
+ numChars += tabSize - (numChars % tabSize);
} else {
numChars++;
}
@@ -395,10 +398,34 @@
return result;
};
- GrDiffBuilder.prototype._addTabWrappers = function(html) {
- var htmlStr = this._getTabWrapper(this._prefs.tab_size,
- this._prefs.show_tabs);
- return html.replace(GrDiffBuilder.TAB_REGEX, htmlStr);
+ /**
+ * Takes a string of text (not HTML) and returns a string of HTML with tab
+ * elements in place of tab characters. In each case tab elements are given
+ * the width needed to reach the next tab-stop.
+ *
+ * @param {String} A line of text potentially containing tab characters.
+ * @param {Number} The width for tabs.
+ * @return {String} An HTML string potentially containing tab elements.
+ */
+ GrDiffBuilder.prototype._addTabWrappers = function(line, tabSize) {
+ if (!line.length) { return ''; }
+
+ var result = '';
+ var offset = 0;
+ var split = line.split('\t');
+ var width;
+
+ for (var i = 0; i < split.length - 1; i++) {
+ offset += split[i].length;
+ width = tabSize - (offset % tabSize);
+ result += split[i] + this._getTabWrapper(width, this._prefs.show_tabs);
+ offset += width;
+ }
+ if (split.length) {
+ result += split[split.length - 1];
+ }
+
+ return result;
};
GrDiffBuilder.prototype._addIntralineHighlights = function(content, html,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index cda4dc8..2c633f1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -104,19 +104,29 @@
'6789');
});
- test('text length with tabs', function() {
+ test('text length with tabs and unicode', function() {
assert.equal(builder._textLength('12345', 4), 5);
assert.equal(builder._textLength('\t\t12', 4), 10);
+ assert.equal(builder._textLength('abc💢123', 4), 7);
+
+ assert.equal(builder._textLength('abc\t', 8), 8);
+ assert.equal(builder._textLength('abc\t\t', 10), 20);
+ assert.equal(builder._textLength('', 10), 0);
+ assert.equal(builder._textLength('', 10), 0);
+ assert.equal(builder._textLength('abc\tde', 10), 12);
+ assert.equal(builder._textLength('abc\tde\t', 10), 20);
+ assert.equal(builder._textLength('\t\t\t\t\t', 20), 100);
});
test('tab wrapper insertion', function() {
var html = 'abc\tdef';
var wrapper = builder._getTabWrapper(
- builder._prefs.tab_size,
+ builder._prefs.tab_size - 3,
builder._prefs.show_tabs);
assert.ok(wrapper);
assert.isAbove(wrapper.length, 0);
- assert.equal(builder._addTabWrappers(html), 'abc' + wrapper + 'def');
+ assert.equal(builder._addTabWrappers(html, builder._prefs.tab_size),
+ 'abc' + wrapper + 'def');
assert.throws(builder._getTabWrapper.bind(
builder,
// using \x3c instead of < in string so gjslint can parse
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js
new file mode 100644
index 0000000..ca3e5ec
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-annotation.js
@@ -0,0 +1,206 @@
+// Copyright (C) 2016 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.
+(function(window) {
+ 'use strict';
+
+ // Prevent redefinition.
+ if (window.GrAnnotation) { return; }
+
+ // TODO(wyatta): refactor this to be <MARK> rather than <HL>.
+ var ANNOTATION_TAG = 'HL';
+
+ // Astral code point as per https://mathiasbynens.be/notes/javascript-unicode
+ var REGEX_ASTRAL_SYMBOL = /[\uD800-\uDBFF][\uDC00-\uDFFF]/;
+
+ var GrAnnotation = {
+
+ /**
+ * The DOM API textContent.length calculation is broken when the text
+ * contains Unicode. See https://mathiasbynens.be/notes/javascript-unicode .
+ * @param {Text} A text node.
+ * @return {Number} The length of the text.
+ */
+ getLength: function(node) {
+ return node.textContent.replace(REGEX_ASTRAL_SYMBOL, '_').length;
+ },
+
+ /**
+ * Surrounds the element's text at specified range in an ANNOTATION_TAG
+ * element. If the element has child elements, the range is split and
+ * applied as deeply as possible.
+ */
+ annotateElement: function(parent, offset, length, cssClass) {
+ var nodes = [].slice.apply(parent.childNodes);
+ var node;
+ var nodeLength;
+ var subLength;
+
+ for (var i = 0; i < nodes.length; i++) {
+ node = nodes[i];
+ nodeLength = this.getLength(node);
+
+ // If the current node is completely before the offset.
+ if (nodeLength <= offset) {
+ offset -= nodeLength;
+ continue;
+ }
+
+ // Sublength is the annotation length for the current node.
+ subLength = Math.min(length, nodeLength - offset);
+
+ if (node instanceof Text) {
+ this._annotateText(node, offset, subLength, cssClass);
+ } else if (node instanceof HTMLElement) {
+ this.annotateElement(node, offset, subLength, cssClass);
+ }
+
+ // If there is still more to annotate, then shift the indices, otherwise
+ // work is done, so break the loop.
+ if (subLength < length) {
+ length -= subLength;
+ offset = 0;
+ } else {
+ break;
+ }
+ }
+ },
+
+ /**
+ * Wraps node in annotation tag with cssClass, replacing the node in DOM.
+ *
+ * @return {!Element} Wrapped node.
+ */
+ wrapInHighlight: function(node, cssClass) {
+ var hl;
+ if (node.tagName === ANNOTATION_TAG) {
+ hl = node;
+ hl.classList.add(cssClass);
+ } else {
+ hl = document.createElement(ANNOTATION_TAG);
+ hl.className = cssClass;
+ Polymer.dom(node.parentElement).replaceChild(hl, node);
+ hl.appendChild(node);
+ }
+ return hl;
+ },
+
+ /**
+ * Splits Text Node and wraps it in hl with cssClass.
+ * Wraps trailing part after split, tailing one if opt_firstPart is true.
+ *
+ * @param {!Node} node
+ * @param {number} offset
+ * @param {string} cssClass
+ * @param {boolean=} opt_firstPart
+ */
+ splitAndWrapInHighlight: function(node, offset, cssClass, opt_firstPart) {
+ if (this.getLength(node) === offset || offset === 0) {
+ return this.wrapInHighlight(node, cssClass);
+ } else {
+ if (opt_firstPart) {
+ this.splitNode(node, offset);
+ // Node points to first part of the Text, second one is sibling.
+ } else {
+ node = this.splitNode(node, offset);
+ }
+ return this.wrapInHighlight(node, cssClass);
+ }
+ },
+
+ /**
+ * Splits Node at offset.
+ * If Node is Element, it's cloned and the node at offset is split too.
+ *
+ * @param {!Node} node
+ * @param {number} offset
+ * @return {!Node} Trailing Node.
+ */
+ splitNode: function(element, offset) {
+ if (element instanceof Text) {
+ return this.splitTextNode(element, offset);
+ }
+ var tail = element.cloneNode(false);
+ element.parentElement.insertBefore(tail, element.nextSibling);
+ // Skip nodes before offset.
+ var node = element.firstChild;
+ while (node &&
+ this.getLength(node) <= offset ||
+ this.getLength(node) === 0) {
+ offset -= this.getLength(node);
+ node = node.nextSibling;
+ }
+ if (this.getLength(node) > offset) {
+ tail.appendChild(this.splitNode(node, offset));
+ }
+ while (node.nextSibling) {
+ tail.appendChild(node.nextSibling);
+ }
+ return tail;
+ },
+
+ /**
+ * Node.prototype.splitText Unicode-valid alternative.
+ *
+ * DOM Api for splitText() is broken for Unicode:
+ * https://mathiasbynens.be/notes/javascript-unicode
+ *
+ * @param {!Text} node
+ * @param {number} offset
+ * @return {!Text} Trailing Text Node.
+ */
+ splitTextNode: function(node, offset) {
+ if (node.textContent.match(REGEX_ASTRAL_SYMBOL)) {
+ // TODO (viktard): Polyfill Array.from for IE10.
+ var head = Array.from(node.textContent);
+ var tail = head.splice(offset);
+ var parent = node.parentElement;
+ var headNode = document.createTextNode(head.join(''));
+ parent.replaceChild(headNode, node);
+ var tailNode = document.createTextNode(tail.join(''));
+ parent.insertBefore(tailNode, headNode.nextSibling);
+ return tailNode;
+ } else {
+ return node.splitText(offset);
+ }
+ },
+
+ _annotateText: function(node, offset, length, cssClass) {
+ var nodeLength = this.getLength(node);
+
+ // There are four cases:
+ // 1) Entire node is highlighted.
+ // 2) Highlight is at the start.
+ // 3) Highlight is at the end.
+ // 4) Highlight is in the middle.
+
+ if (offset === 0 && nodeLength === length) {
+ // Case 1.
+ this.wrapInHighlight(node, cssClass);
+ } else if (offset === 0) {
+ // Case 2.
+ this.splitAndWrapInHighlight(node, length, cssClass, true);
+ } else if (offset + length === nodeLength) {
+ // Case 3
+ this.splitAndWrapInHighlight(node, offset, cssClass, false);
+ } else {
+ // Case 4
+ this.splitAndWrapInHighlight(this.splitTextNode(node, offset), length,
+ cssClass, true);
+ }
+ },
+ };
+
+ window.GrAnnotation = GrAnnotation;
+})(window);
+
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.html
index bc3b23f..e44086e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.html
@@ -36,5 +36,6 @@
<content></content>
</div>
</template>
+ <script src="gr-annotation.js"></script>
<script src="gr-diff-highlight.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
index c4e79eb..47a02a6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
@@ -333,107 +333,7 @@
if (node instanceof Element && node.classList.contains('content')) {
return this._getLength(node.querySelector('.contentText'));
} else {
- // DOM API for textContent.length is broken for Unicode:
- // https://mathiasbynens.be/notes/javascript-unicode
- return node.textContent.replace(REGEX_ASTRAL_SYMBOL, '_').length;
- }
- },
-
- /**
- * Wraps node in hl tag with cssClass, replacing the node in DOM.
- *
- * @return {!Element} Wrapped node.
- */
- _wrapInHighlight: function(node, cssClass) {
- var hl;
- if (node.tagName === 'HL') {
- hl = node;
- hl.classList.add(cssClass);
- } else {
- hl = document.createElement('hl');
- hl.className = cssClass;
- Polymer.dom(node.parentElement).replaceChild(hl, node);
- hl.appendChild(node);
- }
- return hl;
- },
-
- /**
- * Node.prototype.splitText Unicode-valid alternative.
- *
- * @param {!Text} node
- * @param {number} offset
- * @return {!Text} Trailing Text Node.
- */
- _splitTextNode: function(node, offset) {
- if (node.textContent.match(REGEX_ASTRAL_SYMBOL)) {
- // DOM Api for splitText() is broken for Unicode:
- // https://mathiasbynens.be/notes/javascript-unicode
- // TODO (viktard): Polyfill Array.from for IE10.
- var head = Array.from(node.textContent);
- var tail = head.splice(offset);
- var parent = node.parentElement;
- var headNode = document.createTextNode(head.join(''));
- parent.replaceChild(headNode, node);
- var tailNode = document.createTextNode(tail.join(''));
- parent.insertBefore(tailNode, headNode.nextSibling);
- return tailNode;
- } else {
- return node.splitText(offset);
- }
- },
-
- /**
- * Split Node at offset.
- * If Node is Element, it's cloned and the node at offset is split too.
- *
- * @param {!Node} node
- * @param {number} offset
- * @return {!Node} Trailing Node.
- */
- _splitNode: function(element, offset) {
- if (element instanceof Text) {
- return this._splitTextNode(element, offset);
- }
- var tail = element.cloneNode(false);
- element.parentElement.insertBefore(tail, element.nextSibling);
- // Skip nodes before offset.
- var node = element.firstChild;
- while (node &&
- this._getLength(node) <= offset ||
- this._getLength(node) === 0) {
- offset -= this._getLength(node);
- node = node.nextSibling;
- }
- if (this._getLength(node) > offset) {
- tail.appendChild(this._splitNode(node, offset));
- }
- while (node.nextSibling) {
- tail.appendChild(node.nextSibling);
- }
- return tail;
- },
-
- /**
- * Split Text Node and wrap it in hl with cssClass.
- * Wraps trailing part after split, tailing one if opt_firstPart is true.
- *
- * @param {!Node} node
- * @param {number} offset
- * @param {string} cssClass
- * @param {boolean=} opt_firstPart
- */
- _splitAndWrapInHighlight: function(node, offset, cssClass, opt_firstPart) {
- if (this._getLength(node) === offset || offset === 0) {
- return this._wrapInHighlight(node, cssClass);
- } else {
- if (opt_firstPart) {
- this._splitNode(node, offset);
- // Node points to first part of the Text, second one is sibling.
- } else {
- node = this._splitNode(node, offset);
- }
- return this._wrapInHighlight(node, cssClass);
+ return GrAnnotation.getLength(node);
}
},
@@ -470,11 +370,11 @@
// Split Text node.
if (startNode instanceof Text) {
- startNode =
- this._splitAndWrapInHighlight(startNode, startOffset, cssClass);
+ startNode = GrAnnotation.splitAndWrapInHighlight(
+ startNode, startOffset, cssClass);
// Edge case: single line, text node wraps the highlight.
if (isOneLine && this._getLength(startNode) > length) {
- var extra = this._splitTextNode(startNode.firstChild, length);
+ var extra = GrAnnotation.splitTextNode(startNode.firstChild, length);
startContent.insertBefore(extra, startNode.nextSibling);
startContent.normalize();
}
@@ -483,10 +383,10 @@
// Edge case: single line, <hl> wraps the highlight.
// Should leave wrapping HL's content after the highlight.
if (isOneLine && startOffset + length < this._getLength(startNode)) {
- this._splitNode(startNode, startOffset + length);
+ GrAnnotation.splitNode(startNode, startOffset + length);
}
- startNode =
- this._splitAndWrapInHighlight(startNode, startOffset, cssClass);
+ startNode = GrAnnotation.splitAndWrapInHighlight(
+ startNode, startOffset, cssClass);
}
} else {
startNode = null;
@@ -524,13 +424,13 @@
if (!endNode) { return null; }
if (endNode instanceof Text) {
- endNode =
- this._splitAndWrapInHighlight(endNode, endOffset, cssClass, true);
+ endNode = GrAnnotation.splitAndWrapInHighlight(
+ endNode, endOffset, cssClass, true);
} else if (endNode.tagName == 'HL') {
if (!endNode.classList.contains(cssClass)) {
// Split text inside HL.
var hl = endNode;
- endNode = this._splitAndWrapInHighlight(
+ endNode = GrAnnotation.splitAndWrapInHighlight(
endNode, endOffset, cssClass, true);
if (hl.textContent.length === 0) {
hl.remove();
@@ -627,7 +527,7 @@
if (threadEl) {
content.appendChild(threadEl);
}
- this._wrapInHighlight(text, cssClass);
+ GrAnnotation.wrapInHighlight(text, cssClass);
}, this);
}
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
index ddc081e..88f6be5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
@@ -20,6 +20,7 @@
<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<script src="gr-annotation.js"></script>
<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="gr-diff-highlight.html">
@@ -891,6 +892,159 @@
assert.equal(getActionSide(), 'left');
});
+ suite('annotation', function() {
+ var str;
+ var parent;
+ var textNode;
+
+ setup(function() {
+ str = 'Lorem ipsum dolor sit amet, suspendisse inceptos vehicula';
+ parent = document.createElement('div');
+ parent.appendChild(document.createTextNode(str));
+ textNode = parent.childNodes[0];
+ });
+
+ test('library is loaded', function() {
+ assert.isOk(GrAnnotation);
+ });
+
+ test('_annotateText Case 1', function() {
+ assert.equal(parent.childNodes.length, 1);
+ assert.instanceOf(parent.childNodes[0], Text);
+
+ GrAnnotation._annotateText(textNode, 0, str.length, 'foobar');
+
+ assert.equal(parent.childNodes.length, 1);
+ assert.instanceOf(parent.childNodes[0], HTMLElement);
+ assert.equal(parent.childNodes[0].className, 'foobar');
+ assert.instanceOf(parent.childNodes[0].childNodes[0], Text);
+ assert.equal(parent.childNodes[0].childNodes[0].textContent, str);
+ });
+
+ test('_annotateText Case 2', function() {
+ var length = 12;
+ var substr = str.substr(0, length);
+ var remainder = str.substr(length);
+
+ assert.equal(parent.childNodes.length, 1);
+ assert.instanceOf(parent.childNodes[0], Text);
+
+ GrAnnotation._annotateText(textNode, 0, length, 'foobar');
+
+ assert.equal(parent.childNodes.length, 2);
+
+ assert.instanceOf(parent.childNodes[0], HTMLElement);
+ assert.equal(parent.childNodes[0].className, 'foobar');
+ assert.instanceOf(parent.childNodes[0].childNodes[0], Text);
+ assert.equal(parent.childNodes[0].childNodes[0].textContent, substr);
+
+ assert.instanceOf(parent.childNodes[1], Text);
+ assert.equal(parent.childNodes[1].textContent, remainder);
+ });
+
+ test('_annotateText Case 3', function() {
+ var index = 12;
+ var length = str.length - index;
+ var remainder = str.substr(0, index);
+ var substr = str.substr(index);
+
+ assert.equal(parent.childNodes.length, 1);
+ assert.instanceOf(parent.childNodes[0], Text);
+
+ GrAnnotation._annotateText(textNode, index, length, 'foobar');
+
+ assert.equal(parent.childNodes.length, 2);
+
+ assert.instanceOf(parent.childNodes[0], Text);
+ assert.equal(parent.childNodes[0].textContent, remainder);
+
+ assert.instanceOf(parent.childNodes[1], HTMLElement);
+ assert.equal(parent.childNodes[1].className, 'foobar');
+ assert.instanceOf(parent.childNodes[1].childNodes[0], Text);
+ assert.equal(parent.childNodes[1].childNodes[0].textContent, substr);
+ });
+
+ test('_annotateText Case 4', function() {
+ var index = str.indexOf('dolor');
+ var length = 'dolor '.length;
+
+ var remainderPre = str.substr(0, index);
+ var substr = str.substr(index, length);
+ var remainderPost = str.substr(index + length);
+
+ assert.equal(parent.childNodes.length, 1);
+ assert.instanceOf(parent.childNodes[0], Text);
+
+ GrAnnotation._annotateText(textNode, index, length, 'foobar');
+
+ assert.equal(parent.childNodes.length, 3);
+
+ assert.instanceOf(parent.childNodes[0], Text);
+ assert.equal(parent.childNodes[0].textContent, remainderPre);
+
+ assert.instanceOf(parent.childNodes[1], HTMLElement);
+ assert.equal(parent.childNodes[1].className, 'foobar');
+ assert.instanceOf(parent.childNodes[1].childNodes[0], Text);
+ assert.equal(parent.childNodes[1].childNodes[0].textContent, substr);
+
+ assert.instanceOf(parent.childNodes[2], Text);
+ assert.equal(parent.childNodes[2].textContent, remainderPost);
+ });
+
+ test('_annotateElement design doc example', function() {
+ var layers = [
+ 'amet, ',
+ 'inceptos ',
+ 'amet, ',
+ 'et, suspendisse ince'
+ ];
+
+ // Apply the layers successively.
+ layers.forEach(function(layer, i) {
+ GrAnnotation.annotateElement(
+ parent, str.indexOf(layer), layer.length, 'layer-' + (i + 1));
+ });
+
+ assert.equal(parent.textContent, str);
+
+ // Layer 1:
+ var layer1 = parent.querySelectorAll('.layer-1');
+ assert.equal(layer1.length, 1);
+ assert.equal(layer1[0].textContent, layers[0]);
+ assert.equal(layer1[0].parentElement, parent);
+
+ // Layer 2:
+ var layer2 = parent.querySelectorAll('.layer-2');
+ assert.equal(layer2.length, 1);
+ assert.equal(layer2[0].textContent, layers[1]);
+ assert.equal(layer2[0].parentElement, parent);
+
+ // Layer 3:
+ var layer3 = parent.querySelectorAll('.layer-3');
+ assert.equal(layer3.length, 1);
+ assert.equal(layer3[0].textContent, layers[2]);
+ assert.equal(layer3[0].parentElement, layer1[0]);
+
+ // Layer 4:
+ var layer4 = parent.querySelectorAll('.layer-4');
+ assert.equal(layer4.length, 3);
+
+ assert.equal(layer4[0].textContent, 'et, ');
+ assert.equal(layer4[0].parentElement, layer3[0]);
+
+ assert.equal(layer4[1].textContent, 'suspendisse ');
+ assert.equal(layer4[1].parentElement, parent);
+
+ assert.equal(layer4[2].textContent, 'ince');
+ assert.equal(layer4[2].parentElement, layer2[0]);
+
+ assert.equal(layer4[0].textContent +
+ layer4[1].textContent +
+ layer4[2].textContent,
+ layers[3]);
+ });
+ });
+
// TODO (viktard): Selection starts in line number.
// TODO (viktard): Empty lines in selection start.
// TODO (viktard): Empty lines in selection end.
