Merge "Make gr-change-list a true HTML table"
diff --git a/Documentation/dev-build-plugins.txt b/Documentation/dev-build-plugins.txt
index a7e8909..c777327 100644
--- a/Documentation/dev-build-plugins.txt
+++ b/Documentation/dev-build-plugins.txt
@@ -56,9 +56,10 @@
The fact that plugin contains `BUILD` file doesn't mean that building this
-plugin from the plugin directory works. For now it doesn't. Bazel in tree driven
-means it can only be built from within Gerrit tree. Clone or link the plugin
-into gerrit/plugins directory:
+plugin from the plugin directory works.
+
+Bazel in tree driven means it can only be built from within Gerrit tree. Clone
+or link the plugin into gerrit/plugins directory:
----
cd gerrit
@@ -74,6 +75,26 @@
Some plugins describe their build process in `src/main/resources/Documentation/build.md`
file. It may worth checking.
+=== Plugins with external dependencies ===
+
+If the plugin has external dependencies, then they must be included from Gerrit's
+own WORKSPACE file. This can be achieved by including them in `external_plugin_deps.bzl`.
+During the build in Gerrit tree, this file must be copied over the dummy one in
+`plugins` directory.
+
+Example for content of `external_plugin_deps.bzl` file:
+
+----
+load("//tools/bzl:maven_jar.bzl", "maven_jar")
+
+def external_plugin_deps():
+ maven_jar(
+ name = 'org_apache_tika_tika_core',
+ artifact = 'org.apache.tika:tika-core:1.12',
+ sha1 = '5ab95580d22fe1dee79cffbcd98bb509a32da09b',
+ )
+----
+
== Bazel standalone driven
Only few plugins support that mode for now:
diff --git a/WORKSPACE b/WORKSPACE
index 3329460..5233b0d 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -2,6 +2,7 @@
load("//tools/bzl:maven_jar.bzl", "maven_jar", "GERRIT", "MAVEN_LOCAL")
load("//lib/codemirror:cm.bzl", "CM_VERSION", "DIFF_MATCH_PATCH_VERSION")
+load("//plugins:external_plugin_deps.bzl", "external_plugin_deps")
ANTLR_VERS = "3.5.2"
@@ -1104,3 +1105,4 @@
load("//lib/js:bower_archives.bzl", "load_bower_archives")
load_bower_archives()
+external_plugin_deps()
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index cc8db0a..d4939e0 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -42,6 +42,7 @@
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.FileInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Patch;
@@ -839,8 +840,8 @@
r1.assertOkStatus();
// Try to create edit as admin
+ exception.expect(AuthException.class);
createEmptyEditFor(r1.getChangeId());
- assertThat(getEdit(changeId)).isAbsent();
}
private void createArbitraryEditFor(String changeId) throws Exception {
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 5b087a4..68b87da 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -150,6 +150,40 @@
}
@Test
+ public void postCommentWithReply() throws Exception {
+ for (Integer line : lines) {
+ String file = "file";
+ String contents = "contents " + line;
+ PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo,
+ "first subject", file, contents);
+ PushOneCommit.Result r = push.to("refs/for/master");
+ String changeId = r.getChangeId();
+ String revId = r.getCommit().getName();
+ ReviewInput input = new ReviewInput();
+ CommentInput comment =
+ newComment(file, Side.REVISION, line, "comment 1", false);
+ input.comments = new HashMap<>();
+ input.comments.put(comment.path, Lists.newArrayList(comment));
+ revision(r).review(input);
+ Map<String, List<CommentInfo>> result =
+ getPublishedComments(changeId, revId);
+ CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
+
+ input = new ReviewInput();
+ comment = newComment(file, Side.REVISION, line, "comment 1 reply", false);
+ comment.inReplyTo = actual.id;
+ input.comments = new HashMap<>();
+ input.comments.put(comment.path, Lists.newArrayList(comment));
+ revision(r).review(input);
+ result = getPublishedComments(changeId, revId);
+ actual = result.get(comment.path).get(1);
+ assertThat(comment).isEqualTo(infoToInput(file).apply(actual));
+ assertThat(comment).isEqualTo(infoToInput(file).apply(
+ getPublishedComment(changeId, revId, actual.id)));
+ }
+ }
+
+ @Test
public void postCommentWithUnresolved() throws Exception {
for (Integer line : lines) {
String file = "file";
@@ -786,5 +820,6 @@
to.message = from.message;
to.range = from.range;
to.unresolved = from.unresolved;
+ to.inReplyTo = from.inReplyTo;
}
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
index cb3a9ff..00be7ba 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java
@@ -208,7 +208,18 @@
public void publishedComment() throws Exception {
PushOneCommit.Result r = createChange();
Change.Id id = r.getPatchSetId().getParentKey();
- putComment(user, id, 1, "comment");
+ putComment(user, id, 1, "comment", null);
+ checker.rebuildAndCheckChanges(id);
+ }
+
+ @Test
+ public void publishedCommentAndReply() throws Exception {
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getPatchSetId().getParentKey();
+ putComment(user, id, 1, "comment", null);
+ Map<String, List<CommentInfo>> comments = getPublishedComments(id);
+ String parentUuid = comments.get("a.txt").get(0).id;
+ putComment(user, id, 1, "comment", parentUuid);
checker.rebuildAndCheckChanges(id);
}
@@ -242,7 +253,7 @@
PushOneCommit.Result r = createChange();
Change.Id id = r.getPatchSetId().getParentKey();
putDraft(user, id, 1, "draft comment", null);
- putComment(user, id, 1, "published comment");
+ putComment(user, id, 1, "published comment", null);
checker.rebuildAndCheckChanges(id);
}
@@ -1283,11 +1294,12 @@
}
}
- private void putComment(TestAccount account, Change.Id id, int line, String msg)
- throws Exception {
+ private void putComment(TestAccount account, Change.Id id, int line,
+ String msg, String inReplyTo) throws Exception {
CommentInput in = new CommentInput();
in.line = line;
in.message = msg;
+ in.inReplyTo = inReplyTo;
ReviewInput rin = new ReviewInput();
rin.comments = new HashMap<>();
rin.comments.put(PushOneCommit.FILE_NAME, ImmutableList.of(in));
@@ -1348,4 +1360,8 @@
saveProjectConfig(allProjects, cfg);
}
+ private Map<String, List<CommentInfo>> getPublishedComments(Change.Id id)
+ throws Exception {
+ return gApi.changes().id(id.get()).current().comments();
+ }
}
diff --git a/gerrit-pgm/src/main/resources/com/google/gerrit/pgm/init/libraries.config b/gerrit-pgm/src/main/resources/com/google/gerrit/pgm/init/libraries.config
index dec996d..3a0d7e5 100644
--- a/gerrit-pgm/src/main/resources/com/google/gerrit/pgm/init/libraries.config
+++ b/gerrit-pgm/src/main/resources/com/google/gerrit/pgm/init/libraries.config
@@ -39,7 +39,7 @@
[library "mysqlDriver"]
name = MySQL Connector/J 5.1.40
url = https://repo1.maven.org/maven2/mysql/mysql-connector-java/5.1.40/mysql-connector-java-5.1.40.jar
- sha1 = 1e7732c5ad1bca3e77f156ca5df1175b3305b902
+ sha1 = ef2a2ceab1735eaaae0b5d1cccf574fb7c6e1c52
remove = mysql-connector-java-.*[.]jar
[library "oracleDriver"]
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
index 0e15774..a9af7e0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/CommentsUtil.java
@@ -169,6 +169,7 @@
new Comment.Key(ChangeUtil.messageUuid(), path, psId.get()),
ctx.getUser().getAccountId(), ctx.getWhen(), side, message, serverId,
unresolved);
+ c.parentUuid = parentUuid;
ctx.getUser().updateRealAccountId(c::setRealAuthor);
return c;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
index 04ac741..b186767 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
@@ -31,7 +31,6 @@
import com.google.gerrit.server.change.PublishChangeEdit;
import com.google.gerrit.server.change.RebaseChangeEdit;
import com.google.gerrit.server.git.UpdateException;
-import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -98,7 +97,7 @@
public void create() throws RestApiException {
try {
changeEditsPost.apply(changeResource, null);
- } catch (InvalidChangeOperationException | IOException | OrmException e) {
+ } catch (IOException | OrmException e) {
throw new RestApiException("Cannot create change edit", e);
}
}
@@ -116,7 +115,7 @@
public void rebase() throws RestApiException {
try {
rebaseChangeEdit.apply(changeResource, null);
- } catch (IOException | InvalidChangeOperationException | OrmException e) {
+ } catch (IOException | OrmException e) {
throw new RestApiException("Cannot rebase change edit", e);
}
}
@@ -146,7 +145,7 @@
return fileResponse.isNone()
? Optional.empty()
: Optional.of(fileResponse.value());
- } catch (IOException | InvalidChangeOperationException | OrmException e) {
+ } catch (IOException | OrmException e) {
throw new RestApiException("Cannot retrieve file of change edit", e);
}
}
@@ -159,7 +158,7 @@
renameInput.oldPath = oldFilePath;
renameInput.newPath = newFilePath;
changeEditsPost.apply(changeResource, renameInput);
- } catch (InvalidChangeOperationException | IOException | OrmException e) {
+ } catch (IOException | OrmException e) {
throw new RestApiException("Cannot rename file of change edit", e);
}
}
@@ -170,7 +169,7 @@
ChangeEdits.Post.Input restoreInput = new ChangeEdits.Post.Input();
restoreInput.restorePath = filePath;
changeEditsPost.apply(changeResource, restoreInput);
- } catch (InvalidChangeOperationException | IOException | OrmException e) {
+ } catch (IOException | OrmException e) {
throw new RestApiException("Cannot restore file of change edit", e);
}
}
@@ -179,12 +178,8 @@
public void modifyFile(String filePath, RawInput newContent)
throws RestApiException {
try {
- ChangeEditResource changeEditResource =
- getOrCreateChangeEditResource(filePath);
- ChangeEdits.Put.Input input = new ChangeEdits.Put.Input();
- input.content = newContent;
- changeEditsPut.apply(changeEditResource, input);
- } catch (IOException | InvalidChangeOperationException | OrmException e) {
+ changeEditsPut.apply(changeResource.getControl(), filePath, newContent);
+ } catch (IOException | OrmException e) {
throw new RestApiException("Cannot modify file of change edit", e);
}
}
@@ -192,10 +187,8 @@
@Override
public void deleteFile(String filePath) throws RestApiException {
try {
- ChangeEditResource changeEditResource =
- getOrCreateChangeEditResource(filePath);
- changeEditDeleteContent.apply(changeEditResource, null);
- } catch (IOException | InvalidChangeOperationException | OrmException e) {
+ changeEditDeleteContent.apply(changeResource.getControl(), filePath);
+ } catch (IOException | OrmException e) {
throw new RestApiException("Cannot delete file of change edit", e);
}
}
@@ -219,28 +212,15 @@
input.message = newCommitMessage;
try {
modifyChangeEditCommitMessage.apply(changeResource, input);
- } catch (IOException | InvalidChangeOperationException | OrmException e) {
+ } catch (IOException | OrmException e) {
throw new RestApiException("Cannot modify commit message of change edit",
e);
}
}
- private ChangeEditResource getOrCreateChangeEditResource(String filePath)
- throws InvalidChangeOperationException, RestApiException, OrmException,
- IOException {
- // For some cases, the REST API automatically creates a change edit resource
- // if it doesn't exist. We mimic this behavior here.
- try {
- return getChangeEditResource(filePath);
- } catch (ResourceNotFoundException e) {
- create();
- return getChangeEditResource(filePath);
- }
- }
-
private ChangeEditResource getChangeEditResource(String filePath)
throws ResourceNotFoundException, AuthException, IOException,
- InvalidChangeOperationException, OrmException {
+ OrmException {
return changeEdits.parse(changeResource, IdString.fromDecoded(filePath));
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEditResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEditResource.java
index c57f5a0..9deb864 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEditResource.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEditResource.java
@@ -17,7 +17,6 @@
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.project.ChangeControl;
@@ -60,10 +59,6 @@
return getChangeResource().getControl();
}
- public Change getChange() {
- return edit.getChange();
- }
-
public ChangeEdit getChangeEdit() {
return edit;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
index 6fba99f..640d785 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java
@@ -38,8 +38,7 @@
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.PatchSetUtil;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.WebLinks;
import com.google.gerrit.server.edit.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditJson;
@@ -48,6 +47,7 @@
import com.google.gerrit.server.edit.UnchangedCommitMessageException;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
+import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -106,7 +106,7 @@
@Override
public ChangeEditResource parse(ChangeResource rsrc, IdString id)
throws ResourceNotFoundException, AuthException, IOException,
- InvalidChangeOperationException, OrmException {
+ OrmException {
Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getChange());
if (!edit.isPresent()) {
throw new ResourceNotFoundException(id);
@@ -118,7 +118,7 @@
@Override
public Create create(ChangeResource parent, IdString id)
throws RestApiException {
- return createFactory.create(parent.getChange(), id.get());
+ return createFactory.create(id.get());
}
@@ -148,58 +148,51 @@
RestModifyView<ChangeResource, Put.Input> {
interface Factory {
- Create create(Change change, String path);
+ Create create(String path);
}
- private final Provider<ReviewDb> db;
- private final ChangeEditUtil editUtil;
private final ChangeEditModifier editModifier;
- private final PatchSetUtil psUtil;
+ private final GitRepositoryManager repositoryManager;
private final Put putEdit;
- private final Change change;
private final String path;
@Inject
- Create(Provider<ReviewDb> db,
- ChangeEditUtil editUtil,
- ChangeEditModifier editModifier,
- PatchSetUtil psUtil,
+ Create(ChangeEditModifier editModifier,
+ GitRepositoryManager repositoryManager,
Put putEdit,
- @Assisted Change change,
@Assisted @Nullable String path) {
- this.db = db;
- this.editUtil = editUtil;
this.editModifier = editModifier;
- this.psUtil = psUtil;
+ this.repositoryManager = repositoryManager;
this.putEdit = putEdit;
- this.change = change;
this.path = path;
}
@Override
public Response<?> apply(ChangeResource resource, Put.Input input)
- throws AuthException, IOException, ResourceConflictException,
- OrmException, InvalidChangeOperationException {
- Optional<ChangeEdit> edit = editUtil.byChange(change);
- if (edit.isPresent()) {
- throw new ResourceConflictException(String.format(
- "edit already exists for the change %s",
- resource.getId()));
- }
- edit = createEdit(resource);
- if (!Strings.isNullOrEmpty(path)) {
- putEdit.apply(new ChangeEditResource(resource, edit.get(), path),
- input);
+ throws AuthException, ResourceConflictException, IOException,
+ OrmException {
+
+ if (Strings.isNullOrEmpty(path)) {
+ // TODO(aliceks): Consider the following:
+ // Generating an edit with a put (instead of a post) isn't mentioned in
+ // the documentation of the REST API. We should consider whether we want
+ // to keep this hidden 'feature', make it public via the documentation,
+ // or remove it.
+ createEdit(resource);
+ } else {
+ putEdit.apply(resource.getControl(), path, input.content);
}
return Response.none();
}
- private Optional<ChangeEdit> createEdit(ChangeResource resource)
- throws AuthException, IOException, ResourceConflictException,
- OrmException {
- editModifier.createEdit(change,
- psUtil.current(db.get(), resource.getNotes()));
- return editUtil.byChange(change);
+ private void createEdit(ChangeResource resource) throws AuthException,
+ IOException, OrmException, ResourceConflictException {
+ Project.NameKey project = resource.getProject();
+ try (Repository repository = repositoryManager.openRepository(project)) {
+ editModifier.createEdit(repository, resource.getControl());
+ } catch (InvalidChangeOperationException e) {
+ throw new ResourceConflictException(e.getMessage());
+ }
}
}
@@ -214,20 +207,17 @@
private final ChangeEditUtil editUtil;
private final ChangeEditModifier editModifier;
- private final PatchSetUtil psUtil;
- private final Provider<ReviewDb> db;
+ private final GitRepositoryManager repoManager;
private final String path;
@Inject
DeleteFile(ChangeEditUtil editUtil,
ChangeEditModifier editModifier,
- PatchSetUtil psUtil,
- Provider<ReviewDb> db,
+ GitRepositoryManager repoManager,
@Assisted String path) {
this.editUtil = editUtil;
this.editModifier = editModifier;
- this.psUtil = psUtil;
- this.db = db;
+ this.repoManager = repoManager;
this.path = path;
}
@@ -235,6 +225,10 @@
public Response<?> apply(ChangeResource rsrc, DeleteFile.Input in)
throws IOException, AuthException, ResourceConflictException,
OrmException, InvalidChangeOperationException, BadRequestException {
+ // TODO(aliceks): Consider the following:
+ // This check is strange. If there was already a change edit, this method
+ // wouldn't be called because DeleteContent would take over. We should
+ // consider to remove the positive case.
Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getChange());
if (edit.isPresent()) {
// Edit is wiped out
@@ -244,10 +238,10 @@
// Even if the latest patch set changed since the user triggered
// the operation, deleting the whole file is probably still what
// they intended.
- editModifier.createEdit(rsrc.getChange(),
- psUtil.current(db.get(), rsrc.getNotes()));
- edit = editUtil.byChange(rsrc.getChange());
- editModifier.deleteFile(edit.get(), path);
+ Project.NameKey project = rsrc.getProject();
+ try (Repository repository = repoManager.openRepository(project)) {
+ editModifier.deleteFile(repository, rsrc.getControl(), path);
+ }
}
return Response.none();
}
@@ -329,48 +323,46 @@
public String newPath;
}
- private final Provider<ReviewDb> db;
- private final ChangeEditUtil editUtil;
private final ChangeEditModifier editModifier;
- private final PatchSetUtil psUtil;
+ private final GitRepositoryManager repositoryManager;
@Inject
- Post(Provider<ReviewDb> db,
- ChangeEditUtil editUtil,
- ChangeEditModifier editModifier,
- PatchSetUtil psUtil) {
- this.db = db;
- this.editUtil = editUtil;
+ Post(ChangeEditModifier editModifier,
+ GitRepositoryManager repositoryManager) {
this.editModifier = editModifier;
- this.psUtil = psUtil;
+ this.repositoryManager = repositoryManager;
}
@Override
public Response<?> apply(ChangeResource resource, Post.Input input)
- throws AuthException, InvalidChangeOperationException, IOException,
- ResourceConflictException, OrmException {
- Optional<ChangeEdit> edit = editUtil.byChange(resource.getChange());
- if (!edit.isPresent()) {
- edit = createEdit(resource);
- }
-
- if (input != null) {
- if (!Strings.isNullOrEmpty(input.restorePath)) {
- editModifier.restoreFile(edit.get(), input.restorePath);
- } else if (!Strings.isNullOrEmpty(input.oldPath)
- && !Strings.isNullOrEmpty(input.newPath)) {
- editModifier.renameFile(edit.get(), input.oldPath, input.newPath);
+ throws AuthException, IOException, ResourceConflictException,
+ OrmException {
+ Project.NameKey project = resource.getProject();
+ try (Repository repository = repositoryManager.openRepository(project)) {
+ ChangeControl changeControl = resource.getControl();
+ if (isRestoreFile(input)) {
+ editModifier.restoreFile(repository, changeControl,
+ input.restorePath);
+ } else if (isRenameFile(input)) {
+ editModifier.renameFile(repository, changeControl, input.oldPath,
+ input.newPath);
+ } else {
+ editModifier.createEdit(repository, changeControl);
}
+ } catch (InvalidChangeOperationException e) {
+ throw new ResourceConflictException(e.getMessage());
}
return Response.none();
}
- private Optional<ChangeEdit> createEdit(ChangeResource resource)
- throws AuthException, IOException, ResourceConflictException,
- OrmException {
- editModifier.createEdit(resource.getChange(),
- psUtil.current(db.get(), resource.getNotes()));
- return editUtil.byChange(resource.getChange());
+ private static boolean isRestoreFile(Input input) {
+ return input != null && !Strings.isNullOrEmpty(input.restorePath);
+ }
+
+ private static boolean isRenameFile(Input input) {
+ return input != null
+ && !Strings.isNullOrEmpty(input.oldPath)
+ && !Strings.isNullOrEmpty(input.newPath);
}
}
@@ -387,26 +379,33 @@
}
private final ChangeEditModifier editModifier;
+ private final GitRepositoryManager repositoryManager;
@Inject
- Put(ChangeEditModifier editModifier) {
+ Put(ChangeEditModifier editModifier,
+ GitRepositoryManager repositoryManager) {
this.editModifier = editModifier;
+ this.repositoryManager = repositoryManager;
}
@Override
public Response<?> apply(ChangeEditResource rsrc, Input input)
- throws AuthException, ResourceConflictException {
- String path = rsrc.getPath();
+ throws AuthException, ResourceConflictException, IOException,
+ OrmException {
+ return apply(rsrc.getControl(), rsrc.getPath(), input.content);
+ }
+
+ public Response<?> apply(ChangeControl changeControl, String path,
+ RawInput newContent) throws ResourceConflictException, AuthException,
+ IOException, OrmException {
if (Strings.isNullOrEmpty(path) || path.charAt(0) == '/') {
throw new ResourceConflictException("Invalid path: " + path);
}
- try {
- editModifier.modifyFile(
- rsrc.getChangeEdit(),
- rsrc.getPath(),
- input.content);
- } catch (InvalidChangeOperationException | IOException e) {
+ Project.NameKey project = changeControl.getChange().getProject();
+ try (Repository repository = repositoryManager.openRepository(project)) {
+ editModifier.modifyFile(repository, changeControl, path, newContent);
+ } catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
return Response.none();
@@ -426,18 +425,29 @@
}
private final ChangeEditModifier editModifier;
+ private final GitRepositoryManager repositoryManager;
@Inject
- DeleteContent(ChangeEditModifier editModifier) {
+ DeleteContent(ChangeEditModifier editModifier,
+ GitRepositoryManager repositoryManager) {
this.editModifier = editModifier;
+ this.repositoryManager = repositoryManager;
}
@Override
public Response<?> apply(ChangeEditResource rsrc, DeleteContent.Input input)
- throws AuthException, ResourceConflictException {
- try {
- editModifier.deleteFile(rsrc.getChangeEdit(), rsrc.getPath());
- } catch (InvalidChangeOperationException | IOException e) {
+ throws AuthException, ResourceConflictException, OrmException,
+ IOException {
+ return apply(rsrc.getControl(), rsrc.getPath());
+ }
+
+ public Response<?> apply(ChangeControl changeControl, String filePath)
+ throws AuthException, IOException, OrmException,
+ ResourceConflictException {
+ Project.NameKey project = changeControl.getChange().getProject();
+ try (Repository repository = repositoryManager.openRepository(project)) {
+ editModifier.deleteFile(repository, changeControl, filePath);
+ } catch (InvalidChangeOperationException e) {
throw new ResourceConflictException(e.getMessage());
}
return Response.none();
@@ -515,41 +525,30 @@
public String message;
}
- private final Provider<ReviewDb> db;
private final ChangeEditModifier editModifier;
- private final ChangeEditUtil editUtil;
- private final PatchSetUtil psUtil;
+ private final GitRepositoryManager repositoryManager;
@Inject
- EditMessage(Provider<ReviewDb> db,
- ChangeEditModifier editModifier,
- ChangeEditUtil editUtil,
- PatchSetUtil psUtil) {
- this.db = db;
+ EditMessage(ChangeEditModifier editModifier,
+ GitRepositoryManager repositoryManager) {
this.editModifier = editModifier;
- this.editUtil = editUtil;
- this.psUtil = psUtil;
+ this.repositoryManager = repositoryManager;
}
@Override
public Object apply(ChangeResource rsrc, Input input) throws AuthException,
- IOException, InvalidChangeOperationException, BadRequestException,
- ResourceConflictException, OrmException {
- Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getChange());
- if (!edit.isPresent()) {
- editModifier.createEdit(rsrc.getChange(),
- psUtil.current(db.get(), rsrc.getNotes()));
- edit = editUtil.byChange(rsrc.getChange());
- }
-
+ IOException, BadRequestException, ResourceConflictException,
+ OrmException {
if (input == null || Strings.isNullOrEmpty(input.message)) {
throw new BadRequestException("commit message must be provided");
}
- try {
- editModifier.modifyMessage(edit.get(), input.message);
- } catch (UnchangedCommitMessageException ucm) {
- throw new ResourceConflictException(ucm.getMessage());
+ Project.NameKey project = rsrc.getProject();
+ try (Repository repository = repositoryManager.openRepository(project)) {
+ ChangeControl changeControl = rsrc.getControl();
+ editModifier.modifyMessage(repository, changeControl, input.message);
+ } catch (UnchangedCommitMessageException e) {
+ throw new ResourceConflictException(e.getMessage());
}
return Response.none();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeEdit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeEdit.java
index ac1b770..db125eb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeEdit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeEdit.java
@@ -25,20 +25,17 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.PatchSetUtil;
-import com.google.gerrit.server.edit.ChangeEdit;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.edit.ChangeEditModifier;
-import com.google.gerrit.server.edit.ChangeEditUtil;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
+import org.eclipse.jgit.lib.Repository;
+
import java.io.IOException;
-import java.util.Optional;
@Singleton
public class RebaseChangeEdit implements
@@ -78,41 +75,26 @@
public static class Input {
}
+ private final GitRepositoryManager repositoryManager;
private final ChangeEditModifier editModifier;
- private final ChangeEditUtil editUtil;
- private final PatchSetUtil psUtil;
- private final Provider<ReviewDb> db;
@Inject
- Rebase(ChangeEditModifier editModifier,
- ChangeEditUtil editUtil,
- PatchSetUtil psUtil,
- Provider<ReviewDb> db) {
+ Rebase(GitRepositoryManager repositoryManager,
+ ChangeEditModifier editModifier) {
+ this.repositoryManager = repositoryManager;
this.editModifier = editModifier;
- this.editUtil = editUtil;
- this.psUtil = psUtil;
- this.db = db;
}
@Override
public Response<?> apply(ChangeResource rsrc, Rebase.Input in)
throws AuthException, ResourceConflictException, IOException,
- InvalidChangeOperationException, OrmException {
- Optional<ChangeEdit> edit = editUtil.byChange(rsrc.getChange());
- if (!edit.isPresent()) {
- throw new ResourceConflictException(String.format(
- "no edit exists for change %s",
- rsrc.getChange().getChangeId()));
+ OrmException {
+ Project.NameKey project = rsrc.getProject();
+ try (Repository repository = repositoryManager.openRepository(project)) {
+ editModifier.rebaseEdit(repository, rsrc.getControl());
+ } catch (InvalidChangeOperationException e) {
+ throw new ResourceConflictException(e.getMessage());
}
-
- PatchSet current = psUtil.current(db.get(), rsrc.getNotes());
- if (current.getId().equals(edit.get().getBasePatchSet().getId())) {
- throw new ResourceConflictException(String.format(
- "edit for change %s is already on latest patch set: %s",
- rsrc.getChange().getChangeId(),
- current.getId()));
- }
- editModifier.rebaseEdit(edit.get(), current);
return Response.none();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
index d8046ce..e7e093a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
@@ -19,27 +19,25 @@
import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.MergeConflictException;
import com.google.gerrit.extensions.restapi.RawInput;
-import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.edit.tree.ChangeFileContentModification;
import com.google.gerrit.server.edit.tree.DeleteFileModification;
import com.google.gerrit.server.edit.tree.RenameFileModification;
import com.google.gerrit.server.edit.tree.RestoreFileModification;
import com.google.gerrit.server.edit.tree.TreeCreator;
import com.google.gerrit.server.edit.tree.TreeModification;
-import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -51,19 +49,18 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
-import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.merge.ThreeWayMerger;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import java.io.IOException;
import java.sql.Timestamp;
-import java.util.Map;
+import java.util.Optional;
import java.util.TimeZone;
/**
@@ -78,321 +75,454 @@
public class ChangeEditModifier {
private final TimeZone tz;
- private final GitRepositoryManager gitManager;
private final ChangeIndexer indexer;
private final Provider<ReviewDb> reviewDb;
private final Provider<CurrentUser> currentUser;
- private final ChangeControl.GenericFactory changeControlFactory;
+ private final ChangeEditUtil changeEditUtil;
+ private final PatchSetUtil patchSetUtil;
@Inject
ChangeEditModifier(@GerritPersonIdent PersonIdent gerritIdent,
- GitRepositoryManager gitManager,
ChangeIndexer indexer,
Provider<ReviewDb> reviewDb,
Provider<CurrentUser> currentUser,
- ChangeControl.GenericFactory changeControlFactory) {
- this.gitManager = gitManager;
+ ChangeEditUtil changeEditUtil,
+ PatchSetUtil patchSetUtil) {
this.indexer = indexer;
this.reviewDb = reviewDb;
this.currentUser = currentUser;
this.tz = gerritIdent.getTimeZone();
- this.changeControlFactory = changeControlFactory;
+ this.changeEditUtil = changeEditUtil;
+ this.patchSetUtil = patchSetUtil;
}
/**
- * Create new change edit.
+ * Creates a new change edit.
*
- * @param change to create change edit for
- * @param ps patch set to create change edit on
- * @return result
- * @throws AuthException
- * @throws IOException
- * @throws ResourceConflictException When change edit already
- * exists for the change
- * @throws OrmException
+ * @param repository the affected Git repository
+ * @param changeControl the {@code ChangeControl} of the change for which
+ * the change edit should be created
+ * @throws AuthException if the user isn't authenticated or not allowed to
+ * use change edits
+ * @throws InvalidChangeOperationException if a change edit already existed
+ * for the change
*/
- public RefUpdate.Result createEdit(Change change, PatchSet ps)
- throws AuthException, IOException, ResourceConflictException, OrmException {
- if (!currentUser.get().isIdentifiedUser()) {
- throw new AuthException("Authentication required");
- }
- IdentifiedUser me = currentUser.get().asIdentifiedUser();
- String refPrefix = RefNames.refsEditPrefix(me.getAccountId(), change.getId());
+ public void createEdit(Repository repository, ChangeControl changeControl)
+ throws AuthException, IOException, InvalidChangeOperationException,
+ OrmException {
+ ensureAuthenticatedAndPermitted(changeControl);
- try {
- ChangeControl c =
- changeControlFactory.controlFor(reviewDb.get(), change, me);
- if (!c.canAddPatchSet(reviewDb.get())) {
- return RefUpdate.Result.REJECTED;
- }
- } catch (NoSuchChangeException e) {
- return RefUpdate.Result.NO_CHANGE;
+ Optional<ChangeEdit> changeEdit = lookupChangeEdit(changeControl);
+ if (changeEdit.isPresent()) {
+ throw new InvalidChangeOperationException(String.format("A change edit "
+ + "already exists for change %s", changeControl.getId()));
}
- try (Repository repo = gitManager.openRepository(change.getProject())) {
- Map<String, Ref> refs = repo.getRefDatabase().getRefs(refPrefix);
- if (!refs.isEmpty()) {
- throw new ResourceConflictException("edit already exists");
- }
-
- try (RevWalk rw = new RevWalk(repo)) {
- ObjectId revision = ObjectId.fromString(ps.getRevision().get());
- String editRefName = RefNames.refsEdit(me.getAccountId(), change.getId(),
- ps.getId());
- Result res = update(repo, me, editRefName, rw, ObjectId.zeroId(),
- revision, TimeUtil.nowTs());
- indexer.index(reviewDb.get(), change);
- return res;
- }
- }
+ PatchSet currentPatchSet = lookupCurrentPatchSet(changeControl);
+ ObjectId patchSetCommitId = getPatchSetCommitId(currentPatchSet);
+ createEditReference(repository, changeControl, currentPatchSet,
+ patchSetCommitId, TimeUtil.nowTs());
}
/**
* Rebase change edit on latest patch set
*
- * @param edit change edit that contains edit to rebase
- * @param current patch set to rebase the edit on
- * @throws AuthException
- * @throws ResourceConflictException thrown if rebase fails due to merge conflicts
- * @throws InvalidChangeOperationException
- * @throws IOException
+ * @param repository the affected Git repository
+ * @param changeControl the {@code ChangeControl} of the change whose change
+ * edit should be rebased
+ * @throws AuthException if the user isn't authenticated or not allowed to
+ * use change edits
+ * @throws InvalidChangeOperationException if a change edit doesn't exist
+ * for the specified change, the change edit is already based on the latest
+ * patch set, or the change represents the root commit
+ * @throws MergeConflictException if rebase fails due to merge conflicts
*/
- public void rebaseEdit(ChangeEdit edit, PatchSet current)
- throws AuthException, ResourceConflictException,
- InvalidChangeOperationException, IOException {
- if (!currentUser.get().isIdentifiedUser()) {
- throw new AuthException("Authentication required");
+ public void rebaseEdit(Repository repository, ChangeControl changeControl)
+ throws AuthException, InvalidChangeOperationException, IOException,
+ OrmException, MergeConflictException {
+ ensureAuthenticatedAndPermitted(changeControl);
+
+ Optional<ChangeEdit> optionalChangeEdit = lookupChangeEdit(changeControl);
+ if (!optionalChangeEdit.isPresent()) {
+ throw new InvalidChangeOperationException(String.format(
+ "No change edit exists for change %s", changeControl.getId()));
+ }
+ ChangeEdit changeEdit = optionalChangeEdit.get();
+
+ PatchSet currentPatchSet = lookupCurrentPatchSet(changeControl);
+ if (isBasedOn(changeEdit, currentPatchSet)) {
+ throw new InvalidChangeOperationException(String.format(
+ "Change edit for change %s is already based on latest patch set %s",
+ changeControl.getId(), currentPatchSet.getId()));
}
- Change change = edit.getChange();
- IdentifiedUser me = currentUser.get().asIdentifiedUser();
- String refName = RefNames.refsEdit(me.getAccountId(), change.getId(),
- current.getId());
- try (Repository repo = gitManager.openRepository(change.getProject());
- RevWalk rw = new RevWalk(repo);
- ObjectInserter inserter = repo.newObjectInserter()) {
- BatchRefUpdate ru = repo.getRefDatabase().newBatchUpdate();
- RevCommit editCommit = edit.getEditCommit();
- if (editCommit.getParentCount() == 0) {
- throw new InvalidChangeOperationException(
- "Rebase edit against root commit not supported");
- }
- RevCommit tip = rw.parseCommit(ObjectId.fromString(
- current.getRevision().get()));
- ThreeWayMerger m = MergeStrategy.RESOLVE.newMerger(repo, true);
- m.setObjectInserter(inserter);
- m.setBase(ObjectId.fromString(
- edit.getBasePatchSet().getRevision().get()));
+ rebase(repository, changeEdit, currentPatchSet);
+ }
- if (m.merge(tip, editCommit)) {
- ObjectId tree = m.getResultTreeId();
-
- CommitBuilder commit = new CommitBuilder();
- commit.setTreeId(tree);
- for (int i = 0; i < tip.getParentCount(); i++) {
- commit.addParentId(tip.getParent(i));
- }
- commit.setAuthor(editCommit.getAuthorIdent());
- commit.setCommitter(new PersonIdent(
- editCommit.getCommitterIdent(), TimeUtil.nowTs()));
- commit.setMessage(editCommit.getFullMessage());
- ObjectId newEdit = inserter.insert(commit);
- inserter.flush();
-
- ru.addCommand(new ReceiveCommand(ObjectId.zeroId(), newEdit,
- refName));
- ru.addCommand(new ReceiveCommand(edit.getRef().getObjectId(),
- ObjectId.zeroId(), edit.getRefName()));
- ru.execute(rw, NullProgressMonitor.INSTANCE);
- for (ReceiveCommand cmd : ru.getCommands()) {
- if (cmd.getResult() != ReceiveCommand.Result.OK) {
- throw new IOException("failed: " + cmd);
- }
- }
- } else {
- // TODO(davido): Allow to resolve conflicts inline
- throw new ResourceConflictException("merge conflict");
- }
+ private void rebase(Repository repository, ChangeEdit changeEdit,
+ PatchSet currentPatchSet) throws IOException, MergeConflictException,
+ InvalidChangeOperationException, OrmException {
+ RevCommit currentEditCommit = changeEdit.getEditCommit();
+ if (currentEditCommit.getParentCount() == 0) {
+ throw new InvalidChangeOperationException(
+ "Rebase change edit against root commit not supported");
}
+
+ Change change = changeEdit.getChange();
+ RevCommit basePatchSetCommit = lookupCommit(repository, currentPatchSet);
+ RevTree basePatchSetTree = basePatchSetCommit.getTree();
+
+ ObjectId newTreeId = merge(repository, changeEdit, basePatchSetTree);
+ Timestamp nowTimestamp = TimeUtil.nowTs();
+ String commitMessage = currentEditCommit.getFullMessage();
+ ObjectId newEditCommitId = createCommit(repository, basePatchSetCommit,
+ newTreeId, commitMessage, nowTimestamp);
+
+ String newEditRefName = getEditRefName(change, currentPatchSet);
+ updateReferenceWithNameChange(repository, changeEdit.getRefName(),
+ currentEditCommit, newEditRefName, newEditCommitId, nowTimestamp);
+ reindex(change);
}
/**
- * Modify commit message in existing change edit.
+ * Modifies the commit message of a change edit. If the change edit doesn't
+ * exist, a new one will be created based on the current patch set.
*
- * @param edit change edit
- * @param msg new commit message
- * @return result
- * @throws AuthException
- * @throws InvalidChangeOperationException
- * @throws IOException
- * @throws UnchangedCommitMessageException
+ * @param repository the affected Git repository
+ * @param changeControl the {@code ChangeControl} of the change whose change
+ * edit's message should be modified
+ * @param newCommitMessage the new commit message
+ * @throws AuthException if the user isn't authenticated or not allowed to
+ * use change edits
+ * @throws UnchangedCommitMessageException if the commit message is the same
+ * as before
*/
- public RefUpdate.Result modifyMessage(ChangeEdit edit, String msg)
- throws AuthException, InvalidChangeOperationException, IOException,
- UnchangedCommitMessageException {
- msg = msg.trim() + "\n";
- checkState(!Strings.isNullOrEmpty(msg), "message cannot be null");
- if (!currentUser.get().isIdentifiedUser()) {
- throw new AuthException("Authentication required");
- }
+ public void modifyMessage(Repository repository, ChangeControl changeControl,
+ String newCommitMessage) throws AuthException, IOException,
+ UnchangedCommitMessageException, OrmException {
+ ensureAuthenticatedAndPermitted(changeControl);
+ newCommitMessage = getWellFormedCommitMessage(newCommitMessage);
- RevCommit prevEdit = edit.getEditCommit();
- if (prevEdit.getFullMessage().equals(msg)) {
+ Optional<ChangeEdit> optionalChangeEdit = lookupChangeEdit(changeControl);
+ PatchSet basePatchSet = getBasePatchSet(optionalChangeEdit, changeControl);
+ RevCommit basePatchSetCommit = lookupCommit(repository, basePatchSet);
+ RevCommit baseCommit = optionalChangeEdit.map(ChangeEdit::getEditCommit)
+ .orElse(basePatchSetCommit);
+
+ String currentCommitMessage = baseCommit.getFullMessage();
+ if (newCommitMessage.equals(currentCommitMessage)) {
throw new UnchangedCommitMessageException();
}
- IdentifiedUser me = currentUser.get().asIdentifiedUser();
- Project.NameKey project = edit.getChange().getProject();
- try (Repository repo = gitManager.openRepository(project);
- RevWalk rw = new RevWalk(repo);
- ObjectInserter inserter = repo.newObjectInserter()) {
- String refName = edit.getRefName();
- Timestamp now = TimeUtil.nowTs();
- ObjectId commit = createCommit(me, inserter, prevEdit,
- prevEdit.getTree(),
- msg, now);
- inserter.flush();
- return update(repo, me, refName, rw, prevEdit, commit, now);
+ RevTree baseTree = baseCommit.getTree();
+ Timestamp nowTimestamp = TimeUtil.nowTs();
+ ObjectId newEditCommit = createCommit(repository, basePatchSetCommit,
+ baseTree, newCommitMessage, nowTimestamp);
+
+ if (optionalChangeEdit.isPresent()) {
+ updateEditReference(repository, optionalChangeEdit.get(), newEditCommit,
+ nowTimestamp);
+ } else {
+ createEditReference(repository, changeControl, basePatchSet,
+ newEditCommit, nowTimestamp);
}
}
/**
- * Modify file in existing change edit from its base commit.
+ * Modifies the contents of a file of a change edit. If the change edit
+ * doesn't exist, a new one will be created based on the current patch set.
*
- * @param edit change edit
- * @param file path to modify
- * @param content new content
- * @return result
- * @throws AuthException
- * @throws InvalidChangeOperationException
- * @throws IOException
+ * @param repository the affected Git repository
+ * @param changeControl the {@code ChangeControl} of the change whose change
+ * edit should be modified
+ * @param filePath the path of the file whose contents should be modified
+ * @param newContent the new file content
+ * @throws AuthException if the user isn't authenticated or not allowed to
+ * use change edits
+ * @throws InvalidChangeOperationException if the file already had the
+ * specified content
*/
- public RefUpdate.Result modifyFile(ChangeEdit edit,
- String file, RawInput content) throws AuthException,
- InvalidChangeOperationException, IOException {
- return modify(edit, new ChangeFileContentModification(file, content));
+ public void modifyFile(Repository repository, ChangeControl changeControl,
+ String filePath, RawInput newContent) throws AuthException,
+ InvalidChangeOperationException, IOException, OrmException {
+ modifyTree(repository, changeControl,
+ new ChangeFileContentModification(filePath, newContent));
}
/**
- * Delete file in existing change edit.
+ * Deletes a file from the Git tree of a change edit. If the change edit
+ * doesn't exist, a new one will be created based on the current patch set.
*
- * @param edit change edit
- * @param file path to delete
- * @return result
- * @throws AuthException
- * @throws InvalidChangeOperationException
- * @throws IOException
+ * @param repository the affected Git repository
+ * @param changeControl the {@code ChangeControl} of the change whose change
+ * edit should be modified
+ * @param file path of the file which should be deleted
+ * @throws AuthException if the user isn't authenticated or not allowed to
+ * use change edits
+ * @throws InvalidChangeOperationException if the file does not exist
*/
- public RefUpdate.Result deleteFile(ChangeEdit edit,
+ public void deleteFile(Repository repository, ChangeControl changeControl,
String file) throws AuthException, InvalidChangeOperationException,
- IOException {
- return modify(edit, new DeleteFileModification(file));
+ IOException, OrmException {
+ modifyTree(repository, changeControl, new DeleteFileModification(file));
}
/**
- * Rename file in existing change edit.
+ * Renames a file of a change edit or moves it to another directory. If the
+ * change edit doesn't exist, a new one will be created based on the current
+ * patch set.
*
- * @param edit change edit
- * @param file path to rename
- * @param newFile path to rename the file to
- * @return result
- * @throws AuthException
- * @throws InvalidChangeOperationException
- * @throws IOException
+ * @param repository the affected Git repository
+ * @param changeControl the {@code ChangeControl} of the change whose change
+ * edit should be modified
+ * @param currentFilePath the current path/name of the file
+ * @param newFilePath the desired path/name of the file
+ * @throws AuthException if the user isn't authenticated or not allowed to
+ * use change edits
+ * @throws InvalidChangeOperationException if the file was already renamed
+ * to the specified new name
*/
- public RefUpdate.Result renameFile(ChangeEdit edit, String file,
- String newFile) throws AuthException, InvalidChangeOperationException,
- IOException {
- RevCommit editCommit = edit.getEditCommit();
- return modify(edit, new RenameFileModification(editCommit, file, newFile));
+ public void renameFile(Repository repository, ChangeControl changeControl,
+ String currentFilePath, String newFilePath) throws AuthException,
+ InvalidChangeOperationException, IOException, OrmException {
+ modifyTree(repository, changeControl,
+ new RenameFileModification(currentFilePath, newFilePath));
}
/**
- * Restore file in existing change edit.
+ * Restores a file of a change edit to the state it was in before the patch
+ * set on which the change edit is based. If the change edit doesn't exist, a
+ * new one will be created based on the current patch set.
*
- * @param edit change edit
- * @param file path to restore
- * @return result
- * @throws AuthException
- * @throws InvalidChangeOperationException
- * @throws IOException
+ * @param repository the affected Git repository
+ * @param changeControl the {@code ChangeControl} of the change whose change
+ * edit should be modified
+ * @param file the path of the file which should be restored
+ * @throws AuthException if the user isn't authenticated or not allowed to
+ * use change edits
+ * @throws InvalidChangeOperationException if the file was already restored
*/
- public RefUpdate.Result restoreFile(ChangeEdit edit,
+ public void restoreFile(Repository repository, ChangeControl changeControl,
String file) throws AuthException, InvalidChangeOperationException,
- IOException {
- RevCommit editCommit = edit.getEditCommit();
- return modify(edit, new RestoreFileModification(editCommit, file));
+ IOException, OrmException {
+ modifyTree(repository, changeControl, new RestoreFileModification(file));
}
- private RefUpdate.Result modify(ChangeEdit edit,
+ private void modifyTree(Repository repository, ChangeControl changeControl,
TreeModification treeModification) throws AuthException, IOException,
- InvalidChangeOperationException {
+ OrmException, InvalidChangeOperationException {
+ ensureAuthenticatedAndPermitted(changeControl);
+
+ Optional<ChangeEdit> optionalChangeEdit = lookupChangeEdit(changeControl);
+ PatchSet basePatchSet = getBasePatchSet(optionalChangeEdit, changeControl);
+ RevCommit basePatchSetCommit = lookupCommit(repository, basePatchSet);
+ RevCommit baseCommit = optionalChangeEdit.map(ChangeEdit::getEditCommit)
+ .orElse(basePatchSetCommit);
+
+ ObjectId newTreeId = createNewTree(repository, baseCommit,
+ treeModification);
+
+ String commitMessage = baseCommit.getFullMessage();
+ Timestamp nowTimestamp = TimeUtil.nowTs();
+ ObjectId newEditCommit = createCommit(repository, basePatchSetCommit,
+ newTreeId, commitMessage, nowTimestamp);
+
+ if (optionalChangeEdit.isPresent()) {
+ updateEditReference(repository, optionalChangeEdit.get(), newEditCommit,
+ nowTimestamp);
+ } else {
+ createEditReference(repository, changeControl, basePatchSet,
+ newEditCommit, nowTimestamp);
+ }
+ }
+
+ private void ensureAuthenticatedAndPermitted(ChangeControl changeControl)
+ throws AuthException, OrmException {
+ ensureAuthenticated();
+ ensurePermitted(changeControl);
+ }
+
+ private void ensureAuthenticated() throws AuthException {
if (!currentUser.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
}
- IdentifiedUser me = currentUser.get().asIdentifiedUser();
- Project.NameKey project = edit.getChange().getProject();
- try (Repository repo = gitManager.openRepository(project);
- RevWalk rw = new RevWalk(repo);
- ObjectInserter inserter = repo.newObjectInserter()) {
- String refName = edit.getRefName();
- RevCommit prevEdit = edit.getEditCommit();
+ }
- TreeCreator treeCreator = new TreeCreator(prevEdit.getTree());
- treeCreator.addTreeModification(treeModification);
- ObjectId newTree = treeCreator.createNewTreeAndGetId(repo);
-
- if (ObjectId.equals(newTree, prevEdit.getTree())) {
- throw new InvalidChangeOperationException("no changes were made");
- }
-
- Timestamp now = TimeUtil.nowTs();
- ObjectId commit = createCommit(me, inserter, prevEdit, newTree, now);
- inserter.flush();
- return update(repo, me, refName, rw, prevEdit, commit, now);
+ private void ensurePermitted(ChangeControl changeControl)
+ throws OrmException, AuthException {
+ if (!changeControl.canAddPatchSet(reviewDb.get())) {
+ throw new AuthException("Not allowed to edit a change.");
}
}
- private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter,
- RevCommit revision, ObjectId tree, Timestamp when) throws IOException {
- return createCommit(me, inserter, revision, tree,
- revision.getFullMessage(), when);
+ private String getWellFormedCommitMessage(String commitMessage) {
+ String wellFormedMessage = Strings.nullToEmpty(commitMessage).trim();
+ checkState(!wellFormedMessage.isEmpty(),
+ "Commit message cannot be null or empty");
+ wellFormedMessage = wellFormedMessage + "\n";
+ return wellFormedMessage;
}
- private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter,
- RevCommit revision, ObjectId tree, String msg, Timestamp when)
+ private Optional<ChangeEdit> lookupChangeEdit(ChangeControl changeControl)
+ throws AuthException, IOException {
+ return changeEditUtil.byChange(changeControl);
+ }
+
+ private PatchSet getBasePatchSet(Optional<ChangeEdit> optionalChangeEdit,
+ ChangeControl changeControl) throws OrmException {
+ Optional<PatchSet> editBasePatchSet =
+ optionalChangeEdit.map(ChangeEdit::getBasePatchSet);
+ return editBasePatchSet.isPresent()
+ ? editBasePatchSet.get()
+ : lookupCurrentPatchSet(changeControl);
+ }
+
+ private PatchSet lookupCurrentPatchSet(ChangeControl changeControl)
+ throws OrmException {
+ return patchSetUtil.current(reviewDb.get(), changeControl.getNotes());
+ }
+
+ private static boolean isBasedOn(ChangeEdit changeEdit, PatchSet patchSet) {
+ PatchSet editBasePatchSet = changeEdit.getBasePatchSet();
+ return editBasePatchSet.getId().equals(patchSet.getId());
+ }
+
+ private static RevCommit lookupCommit(Repository repository,
+ PatchSet patchSet) throws IOException {
+ ObjectId patchSetCommitId = getPatchSetCommitId(patchSet);
+ try (RevWalk revWalk = new RevWalk(repository)) {
+ return revWalk.parseCommit(patchSetCommitId);
+ }
+ }
+
+ private static ObjectId createNewTree(Repository repository,
+ RevCommit baseCommit, TreeModification treeModification)
+ throws IOException, InvalidChangeOperationException {
+ TreeCreator treeCreator = new TreeCreator(baseCommit);
+ treeCreator.addTreeModification(treeModification);
+ ObjectId newTreeId = treeCreator.createNewTreeAndGetId(repository);
+
+ if (ObjectId.equals(newTreeId, baseCommit.getTree())) {
+ throw new InvalidChangeOperationException("no changes were made");
+ }
+ return newTreeId;
+ }
+
+ private ObjectId merge(Repository repository, ChangeEdit changeEdit,
+ ObjectId newTreeId) throws IOException, MergeConflictException {
+ PatchSet basePatchSet = changeEdit.getBasePatchSet();
+ ObjectId basePatchSetCommitId = getPatchSetCommitId(basePatchSet);
+ ObjectId editCommitId = changeEdit.getEditCommit();
+
+ ThreeWayMerger threeWayMerger =
+ MergeStrategy.RESOLVE.newMerger(repository, true);
+ threeWayMerger.setBase(basePatchSetCommitId);
+ boolean successful = threeWayMerger.merge(newTreeId, editCommitId);
+
+ if (!successful) {
+ throw new MergeConflictException(
+ "The existing change edit could not be merged with another tree.");
+ }
+ return threeWayMerger.getResultTreeId();
+ }
+
+ private ObjectId createCommit(Repository repository,
+ RevCommit basePatchSetCommit, ObjectId tree, String commitMessage,
+ Timestamp timestamp) throws IOException {
+ try (ObjectInserter objectInserter = repository.newObjectInserter()) {
+ CommitBuilder builder = new CommitBuilder();
+ builder.setTreeId(tree);
+ builder.setParentIds(basePatchSetCommit.getParents());
+ builder.setAuthor(basePatchSetCommit.getAuthorIdent());
+ builder.setCommitter(getCommitterIdent(timestamp));
+ builder.setMessage(commitMessage);
+ ObjectId newCommitId = objectInserter.insert(builder);
+ objectInserter.flush();
+ return newCommitId;
+ }
+ }
+
+ private PersonIdent getCommitterIdent(Timestamp commitTimestamp) {
+ IdentifiedUser user = currentUser.get().asIdentifiedUser();
+ return user.newCommitterIdent(commitTimestamp, tz);
+ }
+
+ private static ObjectId getPatchSetCommitId(PatchSet patchSet) {
+ return ObjectId.fromString(patchSet.getRevision().get());
+ }
+
+ private void createEditReference(Repository repository,
+ ChangeControl changeControl, PatchSet basePatchSet,
+ ObjectId newEditCommit, Timestamp timestamp)
+ throws IOException, OrmException {
+ Change change = changeControl.getChange();
+ String editRefName = getEditRefName(change, basePatchSet);
+ updateReference(repository, editRefName, ObjectId.zeroId(), newEditCommit,
+ timestamp);
+ reindex(change);
+ }
+
+ private String getEditRefName(Change change, PatchSet basePatchSet) {
+ IdentifiedUser me = currentUser.get().asIdentifiedUser();
+ return RefNames.refsEdit(me.getAccountId(), change.getId(),
+ basePatchSet.getId());
+ }
+
+ private void updateEditReference(Repository repository, ChangeEdit changeEdit,
+ ObjectId newEditCommit, Timestamp timestamp)
+ throws IOException, OrmException {
+ String editRefName = changeEdit.getRefName();
+ RevCommit currentEditCommit = changeEdit.getEditCommit();
+ updateReference(repository, editRefName, currentEditCommit, newEditCommit,
+ timestamp);
+ reindex(changeEdit.getChange());
+ }
+
+ private void updateReference(Repository repository, String refName,
+ ObjectId currentObjectId, ObjectId targetObjectId, Timestamp timestamp)
throws IOException {
- CommitBuilder builder = new CommitBuilder();
- builder.setTreeId(tree);
- builder.setParentIds(revision.getParents());
- builder.setAuthor(revision.getAuthorIdent());
- builder.setCommitter(getCommitterIdent(me, when));
- builder.setMessage(msg);
- return inserter.insert(builder);
- }
-
- private RefUpdate.Result update(Repository repo, IdentifiedUser me,
- String refName, RevWalk rw, ObjectId oldObjectId, ObjectId newEdit,
- Timestamp when) throws IOException {
- RefUpdate ru = repo.updateRef(refName);
- ru.setExpectedOldObjectId(oldObjectId);
- ru.setNewObjectId(newEdit);
- ru.setRefLogIdent(getRefLogIdent(me, when));
+ RefUpdate ru = repository.updateRef(refName);
+ ru.setExpectedOldObjectId(currentObjectId);
+ ru.setNewObjectId(targetObjectId);
+ ru.setRefLogIdent(getRefLogIdent(timestamp));
ru.setRefLogMessage("inline edit (amend)", false);
ru.setForceUpdate(true);
- RefUpdate.Result res = ru.update(rw);
- if (res != RefUpdate.Result.NEW &&
- res != RefUpdate.Result.FORCED) {
- throw new IOException("update failed: " + ru);
+ try (RevWalk revWalk = new RevWalk(repository)) {
+ RefUpdate.Result res = ru.update(revWalk);
+ if (res != RefUpdate.Result.NEW && res != RefUpdate.Result.FORCED) {
+ throw new IOException("update failed: " + ru);
+ }
}
- return res;
}
- private PersonIdent getCommitterIdent(IdentifiedUser user, Timestamp when) {
- return user.newCommitterIdent(when, tz);
+ private void updateReferenceWithNameChange(Repository repository,
+ String currentRefName, ObjectId currentObjectId, String newRefName,
+ ObjectId targetObjectId, Timestamp timestamp)
+ throws IOException {
+ BatchRefUpdate batchRefUpdate =
+ repository.getRefDatabase().newBatchUpdate();
+ batchRefUpdate.addCommand(new ReceiveCommand(ObjectId.zeroId(),
+ targetObjectId, newRefName));
+ batchRefUpdate.addCommand(new ReceiveCommand(currentObjectId,
+ ObjectId.zeroId(), currentRefName));
+ batchRefUpdate.setRefLogMessage("rebase edit", false);
+ batchRefUpdate.setRefLogIdent(getRefLogIdent(timestamp));
+ try (RevWalk revWalk = new RevWalk(repository)) {
+ batchRefUpdate.execute(revWalk, NullProgressMonitor.INSTANCE);
+ }
+ for (ReceiveCommand cmd : batchRefUpdate.getCommands()) {
+ if (cmd.getResult() != ReceiveCommand.Result.OK) {
+ throw new IOException("failed: " + cmd);
+ }
+ }
}
- private PersonIdent getRefLogIdent(IdentifiedUser user, Timestamp when) {
- return user.newRefLogIdent(when, tz);
+ private PersonIdent getRefLogIdent(Timestamp timestamp) {
+ IdentifiedUser user = currentUser.get().asIdentifiedUser();
+ return user.newRefLogIdent(timestamp, tz);
+ }
+
+ private void reindex(Change change) throws IOException, OrmException {
+ indexer.index(reviewDb.get(), change);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/ChangeFileContentModification.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/ChangeFileContentModification.java
index fbe4bef..e83155d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/ChangeFileContentModification.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/ChangeFileContentModification.java
@@ -27,6 +27,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -52,7 +53,8 @@
}
@Override
- public List<DirCacheEditor.PathEdit> getPathEdits(Repository repository) {
+ public List<DirCacheEditor.PathEdit> getPathEdits(Repository repository,
+ RevCommit baseCommit) {
DirCacheEditor.PathEdit changeContentEdit = new ChangeContent(filePath,
newContent, repository);
return Collections.singletonList(changeContentEdit);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/DeleteFileModification.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/DeleteFileModification.java
index 887fb4f..64fe63b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/DeleteFileModification.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/DeleteFileModification.java
@@ -16,6 +16,7 @@
import org.eclipse.jgit.dircache.DirCacheEditor;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import java.util.Collections;
import java.util.List;
@@ -32,7 +33,8 @@
}
@Override
- public List<DirCacheEditor.PathEdit> getPathEdits(Repository repository) {
+ public List<DirCacheEditor.PathEdit> getPathEdits(Repository repository,
+ RevCommit baseCommit) {
DirCacheEditor.DeletePath deletePathEdit =
new DirCacheEditor.DeletePath(filePath);
return Collections.singletonList(deletePathEdit);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/RenameFileModification.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/RenameFileModification.java
index 4978446..32c1e0d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/RenameFileModification.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/RenameFileModification.java
@@ -31,24 +31,22 @@
*/
public class RenameFileModification implements TreeModification {
- private final RevCommit currentCommit;
private final String currentFilePath;
private final String newFilePath;
- public RenameFileModification(RevCommit currentCommit,
- String currentFilePath, String newFilePath) {
- this.currentCommit = currentCommit;
+ public RenameFileModification(String currentFilePath, String newFilePath) {
this.currentFilePath = currentFilePath;
this.newFilePath = newFilePath;
}
@Override
- public List<DirCacheEditor.PathEdit> getPathEdits(Repository repository)
+ public List<DirCacheEditor.PathEdit> getPathEdits(Repository repository,
+ RevCommit baseCommit)
throws IOException {
try (RevWalk revWalk = new RevWalk(repository)) {
- revWalk.parseHeaders(currentCommit);
+ revWalk.parseHeaders(baseCommit);
try (TreeWalk treeWalk = TreeWalk.forPath(revWalk.getObjectReader(),
- currentFilePath, currentCommit.getTree())) {
+ currentFilePath, baseCommit.getTree())) {
if (treeWalk == null) {
return Collections.emptyList();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/RestoreFileModification.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/RestoreFileModification.java
index cbe40a3..b5cc9e6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/RestoreFileModification.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/RestoreFileModification.java
@@ -30,24 +30,23 @@
*/
public class RestoreFileModification implements TreeModification {
- private final RevCommit currentCommit;
private final String filePath;
- public RestoreFileModification(RevCommit currentCommit, String filePath) {
- this.currentCommit = currentCommit;
+ public RestoreFileModification(String filePath) {
this.filePath = filePath;
}
@Override
- public List<DirCacheEditor.PathEdit> getPathEdits(Repository repository)
+ public List<DirCacheEditor.PathEdit> getPathEdits(Repository repository,
+ RevCommit baseCommit)
throws IOException {
- if (currentCommit.getParentCount() == 0) {
+ if (baseCommit.getParentCount() == 0) {
DirCacheEditor.DeletePath deletePath =
new DirCacheEditor.DeletePath(filePath);
return Collections.singletonList(deletePath);
}
- RevCommit base = currentCommit.getParent(0);
+ RevCommit base = baseCommit.getParent(0);
try (RevWalk revWalk = new RevWalk(repository)) {
revWalk.parseHeaders(base);
try (TreeWalk treeWalk = TreeWalk.forPath(revWalk.getObjectReader(),
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/TreeCreator.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/TreeCreator.java
index ed8d719..661956f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/TreeCreator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/TreeCreator.java
@@ -24,25 +24,25 @@
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.revwalk.RevTree;
+import org.eclipse.jgit.revwalk.RevCommit;
import java.io.IOException;
import java.util.LinkedList;
import java.util.List;
/**
- * A creator for a new Git tree. To create the new tree, another tree is taken
- * as a basis and modified.
+ * A creator for a new Git tree. To create the new tree, the tree of another
+ * commit is taken as a basis and modified.
*/
public class TreeCreator {
- private final RevTree baseTree;
+ private final RevCommit baseCommit;
// At the moment, a list wouldn't be necessary as only one modification is
// applied per created tree. This is going to change in the near future.
private final List<TreeModification> treeModifications = new LinkedList<>();
- public TreeCreator(RevTree baseTree) {
- this.baseTree = checkNotNull(baseTree, "baseTree is required");
+ public TreeCreator(RevCommit baseCommit) {
+ this.baseCommit = checkNotNull(baseCommit, "baseCommit is required");
}
/**
@@ -85,7 +85,7 @@
DirCache dirCache = DirCache.newInCore();
DirCacheBuilder dirCacheBuilder = dirCache.builder();
dirCacheBuilder.addTree(new byte[0], DirCacheEntry.STAGE_0, objectReader,
- baseTree);
+ baseCommit.getTree());
dirCacheBuilder.finish();
return dirCache;
}
@@ -95,7 +95,7 @@
throws IOException {
List<DirCacheEditor.PathEdit> pathEdits = new LinkedList<>();
for (TreeModification treeModification : treeModifications) {
- pathEdits.addAll(treeModification.getPathEdits(repository));
+ pathEdits.addAll(treeModification.getPathEdits(repository, baseCommit));
}
return pathEdits;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/TreeModification.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/TreeModification.java
index bbb10ec..4b66cd4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/TreeModification.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/tree/TreeModification.java
@@ -16,6 +16,7 @@
import org.eclipse.jgit.dircache.DirCacheEditor;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
import java.io.IOException;
import java.util.List;
@@ -31,10 +32,11 @@
* {@code PathEdit}s can be crucial and hence shouldn't be changed.
*
* @param repository the affected Git repository
+ * @param baseCommit the commit to whose tree this modification is applied
* @return an ordered list of necessary {@code PathEdit}s
* @throws IOException if problems arise when accessing the repository
*/
- List<DirCacheEditor.PathEdit> getPathEdits(Repository repository)
- throws IOException;
+ List<DirCacheEditor.PathEdit> getPathEdits(Repository repository,
+ RevCommit baseCommit) throws IOException;
}
diff --git a/plugins/external_plugin_deps.bzl b/plugins/external_plugin_deps.bzl
new file mode 100644
index 0000000..391f920
--- /dev/null
+++ b/plugins/external_plugin_deps.bzl
@@ -0,0 +1,2 @@
+def external_plugin_deps():
+ pass
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index b7a76a2..f54cfc9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -19,6 +19,7 @@
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
+<link rel="import" href="../../shared/gr-dropdown/gr-dropdown.html">
<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<link rel="import" href="../../shared/gr-overlay/gr-overlay.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
@@ -38,7 +39,8 @@
section {
display: inline-block;
}
- gr-button {
+ gr-button,
+ gr-dropdown {
margin-left: .5em;
}
gr-button:before {
@@ -63,29 +65,29 @@
}
</style>
<div>
- <section hidden$="[[_shouldHideActions(actions.*, _additionalActions.*, _loading)]]">
- <template is="dom-repeat" items="[[_changeActionValues]]" as="action">
+ <gr-dropdown
+ id="moreActions"
+ down-arrow
+ vertical-offset="32"
+ horizontal-align="right"
+ on-tap-item-abandon="_handleAbandonTap"
+ on-tap-item-cherrypick="_handleCherrypickTap"
+ on-tap-item-delete="_handleDeleteTap"
+ hidden$="[[_shouldHideActions(_menuActions.*, _loading)]]"
+ items="[[_menuActions]]">More</gr-dropdown>
+ <section hidden$="[[_shouldHideActions(_topLevelActions.*, _loading)]]">
+ <template
+ is="dom-repeat"
+ items="[[_topLevelActions]]"
+ as="action">
<gr-button title$="[[action.title]]"
- hidden$="[[_computeActionHidden(action.__key, _hiddenChangeActions.*)]]"
+ hidden$="[[_computeActionHidden(action.__key, _hiddenActions.*)]]"
primary$="[[action.__primary]]"
- hidden$="[[!action.enabled]]"
data-action-key$="[[action.__key]]"
data-action-type$="[[action.__type]]"
data-label$="[[action.label]]"
data-loading-label$="[[_computeLoadingLabel(action.__key)]]"
- on-tap="_handleActionTap"></gr-button>
- </template>
- </section>
- <section hidden$="[[_shouldHideActions(revisionActions.*, _additionalActions.*, _loading)]]">
- <template is="dom-repeat" items="[[_revisionActionValues]]" as="action">
- <gr-button title$="[[action.title]]"
- hidden$="[[_computeActionHidden(action.__key, _hiddenRevisionActions.*)]]"
- primary$="[[action.__primary]]"
disabled$="[[!action.enabled]]"
- data-action-key$="[[action.__key]]"
- data-action-type$="[[action.__type]]"
- data-label$="[[action.label]]"
- data-loading-label$="[[_computeLoadingLabel(action.__key)]]"
on-tap="_handleActionTap"></gr-button>
</template>
</section>
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 6fdc3ad..331c3c5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -87,6 +87,16 @@
method: 'POST',
};
+ /**
+ * Keys for actions to appear in the overflow menu rather than the top-level
+ * set of action buttons.
+ */
+ var MENU_ACTION_KEYS = [
+ 'abandon',
+ 'cherrypick',
+ '/', // '/' is the key for the delete action.
+ ];
+
Polymer({
is: 'gr-change-actions',
@@ -96,6 +106,12 @@
* @event reload-change
*/
+ /**
+ * Fired when an action is tapped.
+ *
+ * @event <action key>-tap
+ */
+
properties: {
change: Object,
actions: {
@@ -129,25 +145,26 @@
type: Boolean,
value: true,
},
- _revisionActionValues: {
+
+ _allActionValues: {
type: Array,
- computed: '_computeRevisionActionValues(revisionActions.*, ' +
- 'primaryActionKeys.*, _additionalActions.*)',
- },
- _changeActionValues: {
- type: Array,
- computed: '_computeChangeActionValues(actions.*, ' +
+ computed: '_computeAllActions(actions.*, revisionActions.*,' +
'primaryActionKeys.*, _additionalActions.*, change)',
},
+ _topLevelActions: {
+ type: Array,
+ computed: '_computeTopLevelActions(_allActionValues.*)',
+ },
+ _menuActions: {
+ type: Array,
+ computed: '_computeMenuActions(_allActionValues.*, _hiddenActions.*)',
+ },
+
_additionalActions: {
type: Array,
value: function() { return []; },
},
- _hiddenChangeActions: {
- type: Array,
- value: function() { return []; },
- },
- _hiddenRevisionActions: {
+ _hiddenActions: {
type: Array,
value: function() { return []; },
},
@@ -220,20 +237,15 @@
},
setActionHidden: function(type, key, hidden) {
- var path;
- if (type === ActionType.CHANGE) {
- path = '_hiddenChangeActions';
- } else if (type === ActionType.REVISION) {
- path = '_hiddenRevisionActions';
- } else {
+ if (type !== ActionType.CHANGE && type !== ActionType.REVISION) {
throw Error('Invalid action type given: ' + type);
}
- var idx = this.get(path).indexOf(key);
+ var idx = this._hiddenActions.indexOf(key);
if (hidden && idx === -1) {
- this.push(path, key);
+ this.push('_hiddenActions', key);
} else if (!hidden && idx !== -1) {
- this.splice(path, idx, 1);
+ this.splice('_hiddenActions', idx, 1);
}
},
@@ -251,16 +263,8 @@
this.patchNum);
},
- _shouldHideActions: function(actionsRecord, additionalActionsRecord,
- loading) {
- if (loading) { return true; }
- return !this._actionCount(actionsRecord, additionalActionsRecord);
- },
-
- _actionCount: function(actionsRecord, additionalActionsRecord) {
- var additionalActions = (additionalActionsRecord &&
- additionalActionsRecord.base) || [];
- return this._keyCount(actionsRecord) + additionalActions.length;
+ _shouldHideActions: function(actions, loading) {
+ return loading || !actions || !actions.base || !actions.base.length;
},
_keyCount: function(changeRecord) {
@@ -282,24 +286,6 @@
});
},
- _computeRevisionActionValues: function(actionsChangeRecord,
- primariesChangeRecord, additionalActionsChangeRecord) {
- return this._getActionValues(actionsChangeRecord, primariesChangeRecord,
- additionalActionsChangeRecord, ActionType.REVISION);
- },
-
- _computeChangeActionValues: function(actionsChangeRecord,
- primariesChangeRecord, additionalActionsChangeRecord, change) {
- var actions = this._getActionValues(
- actionsChangeRecord, primariesChangeRecord,
- additionalActionsChangeRecord, ActionType.CHANGE, change);
- var quickApprove = this._getQuickApproveAction();
- if (quickApprove) {
- actions.unshift(quickApprove);
- }
- return actions;
- },
-
_getLabelStatus: function(label) {
if (label.approved) {
return LabelStatus.OK;
@@ -466,14 +452,10 @@
var type = el.getAttribute('data-action-type');
if (type === ActionType.REVISION) {
this._handleRevisionAction(key);
- } else if (key === ChangeActions.DELETE) {
- this._showActionDialog(this.$.confirmDeleteDialog);
} else if (key === ChangeActions.REVERT) {
this.showRevertDialog();
- } else if (key === ChangeActions.ABANDON) {
- this._showActionDialog(this.$.confirmAbandonDialog);
} else if (key === QUICK_APPROVE_ACTION.key) {
- var action = this._changeActionValues.find(function(o) {
+ var action = this._allActionValues.find(function(o) {
return o.key === key;
});
this._fireAction(
@@ -488,10 +470,6 @@
case RevisionActions.REBASE:
this._showActionDialog(this.$.confirmRebase);
break;
- case RevisionActions.CHERRYPICK:
- this.$.confirmCherrypick.branch = '';
- this._showActionDialog(this.$.confirmCherrypick);
- break;
case RevisionActions.SUBMIT:
if (!this._canSubmitChange()) {
return;
@@ -664,5 +642,92 @@
return response;
}.bind(this)).then(this._handleResponseError.bind(this));
},
+
+ _handleAbandonTap: function() {
+ this._showActionDialog(this.$.confirmAbandonDialog);
+ },
+
+ _handleCherrypickTap: function() {
+ this.$.confirmCherrypick.branch = '';
+ this._showActionDialog(this.$.confirmCherrypick);
+ },
+
+ _handleDeleteTap: function() {
+ this._showActionDialog(this.$.confirmDeleteDialog);
+ },
+
+ /**
+ * Merge sources of change actions into a single ordered array of action
+ * values.
+ * @param {splices} changeActionsRecord
+ * @param {splices} revisionActionsRecord
+ * @param {splices} primariesRecord
+ * @param {splices} additionalActionsRecord
+ * @param {Object} change The change object.
+ * @return {Array}
+ */
+ _computeAllActions: function(changeActionsRecord, revisionActionsRecord,
+ primariesRecord, additionalActionsRecord, change) {
+ var revisionActionValues = this._getActionValues(revisionActionsRecord,
+ primariesRecord, additionalActionsRecord, ActionType.REVISION);
+ var changeActionValues = this._getActionValues(changeActionsRecord,
+ primariesRecord, additionalActionsRecord, ActionType.CHANGE, change);
+ var quickApprove = this._getQuickApproveAction();
+ if (quickApprove) {
+ changeActionValues.unshift(quickApprove);
+ }
+ return revisionActionValues
+ .concat(changeActionValues)
+ .sort(this._actionComparator);
+ },
+
+ /**
+ * Sort comparator to define the order of change actions.
+ */
+ _actionComparator: function(actionA, actionB) {
+ // The code review action always appears first.
+ if (actionA.__key === 'review') {
+ return -1;
+ } else if (actionB.__key === 'review') {
+ return 1;
+ }
+
+ // Primary actions always appear last.
+ if (actionA.__primary) {
+ return 1;
+ } else if (actionB.__primary) {
+ return -1;
+ }
+
+ // Change actions appear before revision actions.
+ if (actionA.__type === 'change' && actionB.__type === 'revision') {
+ return -1;
+ } else if (actionA.__type === 'revision' && actionB.__type === 'change') {
+ return 1;
+ }
+
+ // Otherwise, sort by the button label.
+ return actionA.label > actionB.label ? 1 : -1;
+ },
+
+ _computeTopLevelActions: function(actionRecord) {
+ return actionRecord.base.filter(function(a) {
+ return MENU_ACTION_KEYS.indexOf(a.__key) === -1;
+ });
+ },
+
+ _computeMenuActions: function(actionRecord, hiddenActionsRecord) {
+ var hiddenActions = hiddenActionsRecord.base || [];
+ return actionRecord.base
+ .filter(function(a) {
+ return MENU_ACTION_KEYS.indexOf(a.__key) !== -1 &&
+ hiddenActions.indexOf(a.__key) === -1;
+ })
+ .map(function(action) {
+ var key = action.__key;
+ if (key === '/') { key = 'delete'; }
+ return {name: action.label, id: key, };
+ });
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 7dbd903..dbdf00f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -99,11 +99,9 @@
});
test('_shouldHideActions', function() {
- assert.isTrue(element._shouldHideActions(undefined, undefined, true));
- assert.isTrue(element._shouldHideActions({base: {}},
- {base: []}, false));
- assert.isFalse(element._shouldHideActions({base: { test: 'test' }},
- {base: ['test']}, false));
+ assert.isTrue(element._shouldHideActions(undefined, true));
+ assert.isTrue(element._shouldHideActions({base: {}}, false));
+ assert.isFalse(element._shouldHideActions({base: ['test']}, false));
});
test('hide revision action', function(done) {
@@ -114,10 +112,10 @@
assert.throws(element.setActionHidden.bind(element, 'invalid type'));
element.setActionHidden(element.ActionType.REVISION,
element.RevisionActions.SUBMIT, true);
- assert.lengthOf(element._hiddenRevisionActions, 1);
+ assert.lengthOf(element._hiddenActions, 1);
element.setActionHidden(element.ActionType.REVISION,
element.RevisionActions.SUBMIT, true);
- assert.lengthOf(element._hiddenRevisionActions, 1);
+ assert.lengthOf(element._hiddenActions, 1);
flush(function() {
var buttonEl = element.$$('[data-action-key="submit"]');
assert.isOk(buttonEl);
@@ -135,29 +133,26 @@
});
});
- test('hide change action', function(done) {
+ test('hide menu action', function(done) {
flush(function() {
- var buttonEl = element.$$('[data-action-key="/"]');
+ var buttonEl = element.$.moreActions.$$('span[data-id="delete"]');
assert.isOk(buttonEl);
- assert.isFalse(buttonEl.hasAttribute('hidden'));
assert.throws(element.setActionHidden.bind(element, 'invalid type'));
element.setActionHidden(element.ActionType.CHANGE,
element.ChangeActions.DELETE, true);
- assert.lengthOf(element._hiddenChangeActions, 1);
+ assert.lengthOf(element._hiddenActions, 1);
element.setActionHidden(element.ActionType.CHANGE,
element.ChangeActions.DELETE, true);
- assert.lengthOf(element._hiddenChangeActions, 1);
+ assert.lengthOf(element._hiddenActions, 1);
flush(function() {
- var buttonEl = element.$$('[data-action-key="/"]');
- assert.isOk(buttonEl);
- assert.isTrue(buttonEl.hasAttribute('hidden'));
+ var buttonEl = element.$.moreActions.$$('span[data-id="delete"]');
+ assert.isNotOk(buttonEl);
element.setActionHidden(element.ActionType.CHANGE,
element.RevisionActions.DELETE, false);
flush(function() {
- var buttonEl = element.$$('[data-action-key="/"]');
+ var buttonEl = element.$.moreActions.$$('span[data-id="delete"]');
assert.isOk(buttonEl);
- assert.isFalse(buttonEl.hasAttribute('hidden'));
done();
});
});
@@ -169,7 +164,8 @@
flush(function() {
var buttonEls = Polymer.dom(element.root)
.querySelectorAll('gr-button');
- assert.equal(buttonEls.length, 6);
+ var menuItems = element.$.moreActions.items;
+ assert.equal(buttonEls.length + menuItems.length, 6);
assert.isFalse(element.hidden);
done();
});
@@ -177,18 +173,18 @@
test('delete buttons have explicit labels', function(done) {
flush(function() {
- var buttonEls =
- Polymer.dom(element.root).querySelectorAll('[data-action-key="/"]');
- assert.equal(buttonEls.length, 2);
- assert.notEqual(buttonEls[0].getAttribute('data-label'),
- buttonEls[1].getAttribute['data-label']);
+ var deleteItems = element.$.moreActions.items.filter(function(item) {
+ return item.id === 'delete';
+ });
+ assert.equal(deleteItems.length, 2);
+ assert.notEqual(deleteItems[0].name, deleteItems[1].name)
assert.isTrue(
- buttonEls[0].getAttribute('data-label') === 'Delete Revision' ||
- buttonEls[0].getAttribute('data-label') === 'Delete Change'
+ deleteItems[0].name === 'Delete Revision' ||
+ deleteItems[0].name === 'Delete Change'
);
assert.isTrue(
- buttonEls[1].getAttribute('data-label') === 'Delete Revision' ||
- buttonEls[1].getAttribute('data-label') === 'Delete Change'
+ deleteItems[1].name === 'Delete Revision' ||
+ deleteItems[1].name === 'Delete Change'
);
done();
});
@@ -205,6 +201,18 @@
assert.deepEqual(element._getRevision(change, '2'), revObj);
});
+ test('_actionComparator sort order', function() {
+ var actions = [
+ {label: '123', type: 'change', __key: 'review'},
+ {label: 'abc',type: 'revision'},
+ {label: 'abc',type: 'change'},
+ {label: 'def', type: 'change'},
+ {label: 'def', type: 'change',__primary: true},
+ ];
+ var result = actions.slice().sort(element._actionComparator);
+ assert.deepEqual(result, actions);
+ });
+
test('submit change', function(done) {
element.change = {
revisions: {
@@ -285,10 +293,7 @@
flushAsynchronousOperations();
assert.isFalse(element.$.confirmRebase.hidden);
- var cherryPickButton =
- element.$$('gr-button[data-action-key="cherrypick"]');
- assert.ok(cherryPickButton);
- MockInteractions.tap(cherryPickButton);
+ element._handleCherrypickTap();
flushAsynchronousOperations();
assert.isTrue(element.$.confirmRebase.hidden);
assert.isFalse(element.$.confirmCherrypick.hidden);
@@ -311,9 +316,7 @@
});
test('works', function() {
- var cherryPickButton =
- element.$$('gr-button[data-action-key="cherrypick"]');
- MockInteractions.tap(cherryPickButton);
+ element._handleCherrypickTap();
var action = {
__key: 'cherrypick',
__type: 'revision',
@@ -350,12 +353,10 @@
});
test('branch name cleared when re-open cherrypick', function() {
- var cherryPickButton =
- element.$$('gr-button[data-action-key="cherrypick"]');
var emptyBranchName = '';
element.$.confirmCherrypick.branch = 'master';
- MockInteractions.tap(cherryPickButton);
+ element._handleCherrypickTap();
assert.equal(element.$.confirmCherrypick.branch, emptyBranchName);
});
});
@@ -457,12 +458,6 @@
var fireActionStub;
var deleteAction;
- var tapDeleteAction = function() {
- var deleteButton = element.$$('gr-button[data-action-key=\'/\']');
- MockInteractions.tap(deleteButton);
- flushAsynchronousOperations();
- };
-
setup(function() {
fireActionStub = sinon.stub(element, '_fireAction');
element.change = {
@@ -484,12 +479,12 @@
});
test('does not delete on action', function() {
- tapDeleteAction();
+ element._handleDeleteTap();
assert.isFalse(fireActionStub.called);
});
test('shows confirm dialog', function() {
- tapDeleteAction();
+ element._handleDeleteTap();
assert.isFalse(element.$$('#confirmDeleteDialog').hidden);
MockInteractions.tap(
element.$$('#confirmDeleteDialog').$$('gr-button[primary]'));
@@ -498,7 +493,7 @@
});
test('hides delete confirm on cancel', function() {
- tapDeleteAction();
+ element._handleDeleteTap();
MockInteractions.tap(
element.$$('#confirmDeleteDialog').$$('gr-button:not([primary])'));
flushAsynchronousOperations();
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 18dfdc0..873acddc 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -255,12 +255,14 @@
<a href$="[[_computeChangePermalink(_change._number)]]">[[_change._number]]</a><!--
--><template is="dom-if" if="[[_changeStatus]]">
([[_changeStatus]]<!--
- --><template is="dom-if" if="[[_computeShowCommitInfo(_changeStatus)]]">
+ --><template
+ is="dom-if"
+ if="[[_computeShowCommitInfo(_changeStatus, _change.current_revision)]]">
as
<gr-commit-info
- change="[[_change]]"
- commit-info="[[_computeMergedCommitInfo(_change.current_revision, _change.revisions)]]"
- server-config="[[serverConfig]]"></gr-commit-info><!--
+ change="[[_change]]"
+ commit-info="[[_computeMergedCommitInfo(_change.current_revision, _change.revisions)]]"
+ server-config="[[serverConfig]]"></gr-commit-info><!--
--></template><!--
-->)<!--
--></template><!--
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index d66917c..9c7ce99 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -563,8 +563,8 @@
return statusString || '';
},
- _computeShowCommitInfo: function(changeStatus) {
- return changeStatus === 'Merged';
+ _computeShowCommitInfo: function(changeStatus, current_revision) {
+ return changeStatus === 'Merged' && current_revision;
},
_computeMergedCommitInfo: function(current_revision, revisions) {
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.html b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.html
index cead416..07c922a 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.html
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.html
@@ -34,7 +34,10 @@
vertical-align: middle;
}
</style>
- <gr-dropdown items=[[links]] top-content=[[topContent]]
+ <gr-dropdown
+ link
+ items=[[links]]
+ top-content=[[topContent]]
horizontal-align="right">
<span hidden$="[[_hasAvatars]]" hidden>[[account.name]]</span>
<gr-avatar account="[[account]]" hidden$="[[!_hasAvatars]]" hidden
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
index c3d9b16..74dc9e7 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header.html
@@ -52,23 +52,8 @@
.linksTitle {
color: black;
display: inline-block;
- padding-right: 1em;
position: relative;
}
- .downArrow {
- border-left: .36em solid transparent;
- border-right: .36em solid transparent;
- border-top: .36em solid #ccc;
- height: 0;
- position: absolute;
- right: 0;
- top: calc(50% - .05em);
- transition: border-top-color 200ms;
- width: 0;
- }
- .links li:hover .downArrow {
- border-top-color: #666;
- }
.rightItems {
align-items: center;
display: flex;
@@ -119,10 +104,12 @@
<template is="dom-repeat" items="[[_links]]" as="linkGroup">
<li>
<gr-dropdown
+ link
+ down-arrow
items = [[linkGroup.links]]
horizontal-align="left">
<span class="linksTitle" id="[[linkGroup.title]]">
- [[linkGroup.title]] <i class="downArrow"></i>
+ [[linkGroup.title]]
</span>
</gr-dropdown>
</li>
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
index 7e91b0e..800b1df 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
@@ -18,6 +18,10 @@
is: 'gr-button',
properties: {
+ link: {
+ type: Boolean,
+ reflectToAttribute: true,
+ },
disabled: {
type: Boolean,
observer: '_disabledChanged',
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
index 95b03fc..f640dd6 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
@@ -38,6 +38,9 @@
font: inherit;
padding: .3em 0;
}
+ :host[down-arrow] .dropdown-trigger {
+ padding-right: 1.4em;
+ }
gr-avatar {
height: 2em;
width: 2em;
@@ -46,11 +49,6 @@
gr-button[link] {
padding: 1em 0;
}
- gr-button:focus:not([primary]):not([secondary]),
- gr-button:hover:not([primary]):not([secondary]) {
- background-color: transparent;
- border-color: transparent;
- }
ul {
list-style: none;
}
@@ -58,16 +56,17 @@
font-weight: bold;
}
li .accountInfo,
- li a {
+ li .itemAction {
+ cursor: pointer;
display: block;
padding: .85em 1em;
}
- li a:link,
- li a:visited {
+ li .itemAction:link,
+ li .itemAction:visited {
color: #00e;
text-decoration: none;
}
- li a:hover {
+ li .itemAction:hover {
background-color: #6B82D6;
color: #fff;
}
@@ -78,14 +77,30 @@
.bold-text {
font-weight: bold;
}
+ :host:not([down-arrow]) .downArrow { display: none; }
+ :host[down-arrow] .downArrow {
+ border-left: .36em solid transparent;
+ border-right: .36em solid transparent;
+ border-top: .36em solid #ccc;
+ height: 0;
+ position: absolute;
+ right: .3em;
+ top: calc(50% - .05em);
+ transition: border-top-color 200ms;
+ width: 0;
+ }
+ .dropdown-trigger:hover .downArrow {
+ border-top-color: #666;
+ }
</style>
- <gr-button link class="dropdown-trigger" id="trigger"
+ <gr-button link="[[link]]" class="dropdown-trigger" id="trigger"
on-tap="_showDropdownTapHandler">
<content></content>
+ <i class="downArrow"></i>
</gr-button>
<iron-dropdown id="dropdown"
vertical-align="top"
- vertical-offset="40"
+ vertical-offset="[[verticalOffset]]"
allow-outside-scroll="true"
horizontal-align="[[horizontalAlign]]"
on-tap="_handleDropdownTap">
@@ -109,7 +124,16 @@
items="[[items]]"
as="link"
initial-count="75">
- <li><a href$="[[_computeRelativeURL(link.url)]]">[[link.name]]</a>
+ <li>
+ <span
+ class="itemAction"
+ data-id$="[[link.id]]"
+ on-tap="_handleItemTap"
+ hidden$="[[link.url]]">[[link.name]]</span>
+ <a
+ class="itemAction"
+ href$="[[_computeRelativeURL(link.url)]]"
+ hidden$="[[!link.url]]">[[link.name]]</a>
</li>
</template>
</ul>
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
index d10d219..d1fae7f 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
@@ -17,6 +17,12 @@
Polymer({
is: 'gr-dropdown',
+ /**
+ * Fired when a non-link dropdown item with the given ID is tapped.
+ *
+ * @event tap-item-<id>
+ */
+
properties: {
items: Array,
topContent: Object,
@@ -24,6 +30,20 @@
type: String,
value: 'left',
},
+
+ /**
+ * Style the dropdown trigger as a link (rather than a button).
+ */
+ link: {
+ type: Boolean,
+ value: false,
+ },
+
+ verticalOffset: {
+ type: Number,
+ value: 40,
+ },
+
_hasAvatars: String,
},
@@ -53,5 +73,10 @@
var host = window.location.host;
return this._computeURLHelper(host, path);
},
+
+ _handleItemTap: function(e) {
+ var id = e.target.getAttribute('data-id');
+ if (id) { this.dispatchEvent(new CustomEvent('tap-item-' + id)); }
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
index b2f2d21..2794caf 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
@@ -70,5 +70,15 @@
assert.isTrue(topItems[0].classList.contains('bold-text'));
assert.isFalse(topItems[1].classList.contains('bold-text'));
});
+
+ test('non link items', function() {
+ element.items = [
+ {name: 'item one', id: 'foo'}, {name: 'item two', id: 'bar'}];
+ var stub = sinon.stub();
+ element.addEventListener('tap-item-foo', stub);
+ flushAsynchronousOperations();
+ MockInteractions.tap(element.$$('.itemAction'));
+ assert.isTrue(stub.called);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html
index 93e676c..c02583a 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html
@@ -49,6 +49,7 @@
setup(function() {
element = fixture('basic');
+ element.change = {};
var plugin;
Gerrit.install(function(p) { plugin = p; }, '0.1',
'http://test.com/plugins/testplugin/static/test.js');
