405647: clicking on review file URL should open file editor
Change-Id: I5c32a86e649f9ad7b04de3cfc7976b3cf9749b62
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=405647
Signed-off-by: chris.poon <chris.poon@tasktop.com>
diff --git a/org.eclipse.mylyn.gerrit.ui.tests/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandlerTest.java b/org.eclipse.mylyn.gerrit.ui.tests/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandlerTest.java
index 1c202df..7fd15bb 100644
--- a/org.eclipse.mylyn.gerrit.ui.tests/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandlerTest.java
+++ b/org.eclipse.mylyn.gerrit.ui.tests/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandlerTest.java
@@ -12,9 +12,31 @@
package org.eclipse.mylyn.internal.gerrit.ui;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+import java.util.Arrays;
+
+import org.eclipse.mylyn.commons.workbench.EditorHandle;
import org.eclipse.mylyn.internal.gerrit.core.GerritConnector;
+import org.eclipse.mylyn.internal.gerrit.ui.editor.GerritTaskEditorPage;
+import org.eclipse.mylyn.reviews.core.model.IFileItem;
+import org.eclipse.mylyn.reviews.core.model.IReview;
+import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
import org.eclipse.mylyn.tasks.core.TaskRepository;
+import org.eclipse.mylyn.tasks.ui.TasksUi;
+import org.eclipse.ui.IWorkbenchPage;
+import org.eclipse.ui.PlatformUI;
import org.junit.Test;
/**
@@ -22,125 +44,243 @@
*/
public class GerritUrlHandlerTest {
+ private final TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND,
+ "http://review.mylyn.org");
+
private final GerritUrlHandler handler = new GerritUrlHandler();
+ private IWorkbenchPage page;
+
+ private String taskId;
+
+ private int patchSetNumber;
+
+ private String path;
+
@Test
public void testGetTaskId() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org");
assertEquals("123", handler.getTaskId(repository, "http://review.mylyn.org/123"));
}
@Test
public void testGetTaskIdTrailingSlashAfterId() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org");
assertEquals("123", handler.getTaskId(repository, "http://review.mylyn.org/123/foo/bar"));
}
@Test
public void testGetTaskIdInvalidId() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://mylyn.org/reviews");
- assertEquals(null, handler.getTaskId(repository, "http://mylyn.org/reviews/ab123"));
+ assertEquals(null, handler.getTaskId(repository, "http://review.mylyn.org/ab123"));
}
@Test
public void testGetTaskIdRepositoryMismatch() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
assertEquals(null, handler.getTaskId(repository, "http://mylyn.org/reviews/123"));
}
@Test
public void testGetTaskIdSubPath() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://mylyn.org/reviews");
- assertEquals("123", handler.getTaskId(repository, "http://mylyn.org/reviews/123"));
- }
-
- @Test
- public void testGetTaskIdTrailingSlash() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
assertEquals("123", handler.getTaskId(repository, "http://review.mylyn.org/123"));
}
@Test
+ public void testGetTaskIdTrailingSlash() {
+ TaskRepository trailingSlashRepository = new TaskRepository(GerritConnector.CONNECTOR_KIND,
+ "http://review.mylyn.org/");
+ assertEquals("123", handler.getTaskId(trailingSlashRepository, "http://review.mylyn.org/123"));
+ }
+
+ @Test
public void testGetTaskIdAbsolute() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
assertEquals("123", handler.getTaskId(repository, "http://review.mylyn.org/#/c/123"));
}
@Test
public void testGetTaskIdLetters() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org");
assertEquals(null, handler.getTaskId(repository, "http://review.mylyn.org/#/c/abc/"));
}
@Test
public void testGetTaskIdEmpty() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org");
assertEquals(null, handler.getTaskId(repository, "http://review.mylyn.org/#/c//"));
}
@Test
public void testGetTaskIdAbsoluteTrailingSlash() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
assertEquals("123", handler.getTaskId(repository, "http://review.mylyn.org/#/c/123/"));
}
@Test
public void testGetTaskIdPatchSet() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
assertEquals("4698", handler.getTaskId(repository, "http://review.mylyn.org/#/c/4698/5"));
}
@Test
public void testGetTaskIdFile() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
assertEquals("4698", handler.getTaskId(repository, "http://review.mylyn.org/#/c/4698/5/foo/bar"));
}
@Test
public void testGetPatchSetNumberPatchSet() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
String url = "http://review.mylyn.org/#/c/4698/5";
- String taskId = handler.getTaskId(repository, url);
+ taskId = handler.getTaskId(repository, url);
assertEquals(5, handler.getPatchSetNumber(repository, url, taskId));
}
@Test
public void testGetPatchSetNumberPatchSetTrailingSlash() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
String url = "http://review.mylyn.org/#/c/4698/5/";
- String taskId = handler.getTaskId(repository, url);
+ taskId = handler.getTaskId(repository, url);
assertEquals(5, handler.getPatchSetNumber(repository, url, taskId));
}
@Test
public void testGetPatchSetNumberPatchSetFile() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
String url = "http://review.mylyn.org/#/c/4698/5/foo/bar";
- String taskId = handler.getTaskId(repository, url);
+ taskId = handler.getTaskId(repository, url);
assertEquals(5, handler.getPatchSetNumber(repository, url, taskId));
}
@Test
public void testGetPatchSetNumberNoneSpecified() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
String url = "http://review.mylyn.org/#/c/4698";
- String taskId = handler.getTaskId(repository, url);
+ taskId = handler.getTaskId(repository, url);
assertEquals(-1, handler.getPatchSetNumber(repository, url, taskId));
}
@Test
public void testGetPatchSetNumberNoneSpecifiedTrailingSlash() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
String url = "http://review.mylyn.org/#/c/4698/";
- String taskId = handler.getTaskId(repository, url);
+ taskId = handler.getTaskId(repository, url);
assertEquals(-1, handler.getPatchSetNumber(repository, url, taskId));
}
@Test
public void testGetPatchSetNumberNoneSpecifiedNotAnInteger() {
- TaskRepository repository = new TaskRepository(GerritConnector.CONNECTOR_KIND, "http://review.mylyn.org/");
String url = "http://review.mylyn.org/#/c/A1";
- String taskId = handler.getTaskId(repository, url);
+ taskId = handler.getTaskId(repository, url);
assertEquals(-1, handler.getPatchSetNumber(repository, url, taskId));
}
+
+ @Test
+ public void testGetPathNoneSpecified() {
+ assertPath(null, "http://review.mylyn.org/#/c/4698/5");
+ }
+
+ @Test
+ public void testGetPathNoneSpecifiedTrailingSlash() {
+ assertPath(null, "http://review.mylyn.org/#/c/4698/5/");
+ }
+
+ @Test
+ public void testGetPathNoneSpecifiedInvalidPatchNumber() {
+ assertPath(null, "http://review.mylyn.org/#/c/4698/-1abcd");
+ }
+
+ @Test
+ public void testGetPathNoneSpecifiedInvalidPatchNumberTrailingSlash() {
+ assertPath(null, "http://review.mylyn.org/#/c/4698/-1abcd/");
+ }
+
+ @Test
+ public void testGetPath() {
+ assertPath("foo/bar.java", "http://review.mylyn.org/#/c/4698/5/foo/bar.java");
+ }
+
+ @Test
+ public void testGetPathWithTrailingSlash() {
+ assertPath("foo/bar.java", "http://review.mylyn.org/#/c/4698/5/foo/bar.java/");
+ }
+
+ @Test
+ public void testOpenUrlWithInvalidReview() throws Exception {
+ String url = "http://review.mylyn.org/#/c/4698/1/foo/bar.java";
+ GerritUrlHandler spy = setUpOpenUrlTests(url);
+ doReturn(null).when(spy).revealPatchSet(any(EditorHandle.class), anyInt());
+
+ spy.openUrl(page, url, 0);
+
+ verify(spy, times(1)).revealPatchSet(any(EditorHandle.class), anyInt());
+ verify(spy, never()).getFileItem(any(IReview.class), anyInt(), anyString());
+ verify(spy, never()).openCompareEditor(null);
+ }
+
+ @Test
+ public void testOpenUrlWithValidPathOpenReview() throws Exception {
+ String url = "http://review.mylyn.org/#/c/4698/1/foo/bar.java";
+ GerritUrlHandler spy = setUpOpenUrlTests(url);
+ GerritTaskEditorPage page = mock(GerritTaskEditorPage.class);
+ IReview review = createMockReview();
+ when(page.getReview()).thenReturn(review);
+ doReturn(page).when(spy).revealPatchSet(any(EditorHandle.class), anyInt());
+ openReviewTest(spy, url);
+
+ verify(spy, times(1)).getFileItem(review, 1, "foo/bar.java");
+ assertEquals(path, spy.getFileItem(review, patchSetNumber, path).getName());
+ }
+
+ @Test
+ public void testOpenUrlWithInvalidPatchSetNumberOpenReview() throws Exception {
+ String url = "http://review.mylyn.org/#/c/4698/5/foo/bar.java";
+ GerritUrlHandler spy = setUpOpenUrlTests(url);
+ GerritTaskEditorPage page = mock(GerritTaskEditorPage.class);
+ IReview review = createMockReview();
+ when(page.getReview()).thenReturn(review);
+ doReturn(page).when(spy).revealPatchSet(any(EditorHandle.class), anyInt());
+ openReviewTest(spy, url);
+
+ verify(spy, times(1)).getFileItem(review, 5, "foo/bar.java");
+ assertNull(spy.getFileItem(review, patchSetNumber, path));
+ }
+
+ @Test
+ public void testOpenUrlWithInvalidPathOpenReview() throws Exception {
+ String url = "http://review.mylyn.org/#/c/4698/1/foo/bar.jav";
+ GerritUrlHandler spy = setUpOpenUrlTests(url);
+ GerritTaskEditorPage page = mock(GerritTaskEditorPage.class);
+ IReview review = createMockReview();
+ when(page.getReview()).thenReturn(review);
+ doReturn(page).when(spy).revealPatchSet(any(EditorHandle.class), anyInt());
+ openReviewTest(spy, url);
+
+ verify(spy, times(1)).getFileItem(review, 1, "foo/bar.jav");
+ assertNull(spy.getFileItem(review, patchSetNumber, path));
+ }
+
+ private GerritUrlHandler setUpOpenUrlTests(String url) {
+ page = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage();
+ GerritUrlHandler spy = spy(handler);
+ TasksUi.getRepositoryManager().addRepository(repository);
+ taskId = spy.getTaskId(repository, url);
+ patchSetNumber = spy.getPatchSetNumber(repository, url, taskId);
+ path = spy.getFilePath(repository, url, taskId, patchSetNumber);
+
+ return spy;
+ }
+
+ private IReview createMockReview() {
+ IReview mockReview = mock(IReview.class);
+ IReviewItemSet mockSet = mock(IReviewItemSet.class);
+ IFileItem mockFile = mock(IFileItem.class);
+
+ when(mockFile.getName()).thenReturn("foo/bar.java");
+ when(mockSet.getId()).thenReturn("1");
+ when(mockSet.getItems()).thenReturn(Arrays.asList(mockFile));
+ when(mockReview.getSets()).thenReturn(Arrays.asList(mockSet));
+
+ return mockReview;
+ }
+
+ private void openReviewTest(GerritUrlHandler spy, String url) {
+ doNothing().when(spy).openCompareEditor(any(IFileItem.class));
+ spy.openUrl(page, url, 0);
+
+ verify(spy, times(1)).revealPatchSet(any(EditorHandle.class), anyInt());
+ verify(spy, times(1)).openCompareEditor(any(IFileItem.class));
+ }
+
+ private void assertPath(String expectedPath, String testUrl) {
+ taskId = handler.getTaskId(repository, testUrl);
+ patchSetNumber = handler.getPatchSetNumber(repository, testUrl, taskId);
+ assertEquals(expectedPath, handler.getFilePath(repository, testUrl, taskId, patchSetNumber));
+ }
}
diff --git a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandler.java b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandler.java
index 6d660ef..8bbc2d6 100644
--- a/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandler.java
+++ b/org.eclipse.mylyn.gerrit.ui/src/org/eclipse/mylyn/internal/gerrit/ui/GerritUrlHandler.java
@@ -11,14 +11,19 @@
package org.eclipse.mylyn.internal.gerrit.ui;
+import java.util.Arrays;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.commons.lang.StringUtils;
+import org.eclipse.compare.CompareConfiguration;
import org.eclipse.mylyn.commons.workbench.EditorHandle;
import org.eclipse.mylyn.commons.workbench.browser.AbstractUrlHandler;
import org.eclipse.mylyn.internal.gerrit.core.GerritConnector;
import org.eclipse.mylyn.internal.gerrit.ui.editor.GerritTaskEditorPage;
+import org.eclipse.mylyn.reviews.core.model.IFileItem;
+import org.eclipse.mylyn.reviews.core.model.IReview;
+import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
import org.eclipse.mylyn.reviews.ui.spi.editor.ReviewSetSection;
import org.eclipse.mylyn.tasks.core.TaskRepository;
import org.eclipse.mylyn.tasks.ui.TasksUi;
@@ -28,6 +33,8 @@
import org.eclipse.ui.IWorkbenchPart;
import org.eclipse.ui.forms.editor.IFormPage;
+import com.google.common.base.Joiner;
+
/**
* @author Steffen Pingel
* @author Miles Parker
@@ -46,10 +53,15 @@
for (TaskRepository repository : TasksUi.getRepositoryManager().getRepositories(GerritConnector.CONNECTOR_KIND)) {
String taskId = getTaskId(repository, url);
if (taskId != null) {
- EditorHandle editorHandle = TasksUiUtil.openTaskWithResult(repository, taskId);
int patchSetNumber = getPatchSetNumber(repository, url, taskId);
+ EditorHandle editorHandle = TasksUiUtil.openTaskWithResult(repository, taskId);
if (patchSetNumber > 0) {
- revealPatchSet(editorHandle, patchSetNumber);
+ GerritTaskEditorPage gerritPage = revealPatchSet(editorHandle, patchSetNumber);
+ if (gerritPage != null) {
+ IFileItem file = getFileItem(gerritPage.getReview(), patchSetNumber,
+ getFilePath(repository, url, taskId, patchSetNumber));
+ openCompareEditor(file);
+ }
}
return editorHandle;
}
@@ -57,7 +69,13 @@
return null;
}
- private void revealPatchSet(EditorHandle editorHandle, Integer patchSetNumber) {
+ public void openCompareEditor(IFileItem file) {
+ if (file != null) {
+ GerritCompareUi.openFileComparisonEditor(new CompareConfiguration(), file, new GerritReviewBehavior(null));
+ }
+ }
+
+ protected GerritTaskEditorPage revealPatchSet(EditorHandle editorHandle, Integer patchSetNumber) {
IWorkbenchPart part = editorHandle.getPart();
if (part instanceof TaskEditor) {
TaskEditor taskEditor = (TaskEditor) part;
@@ -68,8 +86,10 @@
if (section != null && !section.getControl().isDisposed()) {
section.revealPatchSet(patchSetNumber);
}
+ return gerritPage;
}
}
+ return null;
}
public String getTaskId(TaskRepository repository, String url) {
@@ -89,10 +109,7 @@
* is not an integer.
*/
int getPatchSetNumber(TaskRepository repository, String url, String taskId) {
- String taskUrl = TasksUi.getRepositoryConnector(GerritConnector.CONNECTOR_KIND).getTaskUrl(repository.getUrl(),
- taskId);
- String urlQualifiers = StringUtils.remove(url, taskUrl);
- String[] fragments = StringUtils.split(urlQualifiers, "/"); //$NON-NLS-1$
+ String[] fragments = StringUtils.split(extractUrlQualifiers(repository, url, taskId), "/"); //$NON-NLS-1$
if (fragments.length > 0) {
String patchSetFragment = fragments[0];
try {
@@ -104,6 +121,37 @@
return -1;
}
+ public String getFilePath(TaskRepository repository, String url, String taskId, int patchSetNumber) {
+ if (patchSetNumber > 0) {
+ String[] fragments = StringUtils.split(extractUrlQualifiers(repository, url, taskId), "/"); //$NON-NLS-1$
+ if (fragments.length > 1) {
+ return Joiner.on("/").join(Arrays.copyOfRange(fragments, 1, fragments.length));
+ }
+ }
+ return null;
+ }
+
+ private String extractUrlQualifiers(TaskRepository repository, String url, String taskId) {
+ String taskUrl = TasksUi.getRepositoryConnector(GerritConnector.CONNECTOR_KIND).getTaskUrl(repository.getUrl(),
+ taskId);
+ return StringUtils.remove(url, taskUrl);
+ }
+
+ public IFileItem getFileItem(IReview review, final int patchSetNumber, final String path) {
+ if (review != null) {
+ for (IReviewItemSet set : review.getSets()) {
+ if (set.getId().equals(Integer.toString(patchSetNumber))) {
+ for (final IFileItem item : set.getItems()) {
+ if (path.equals(item.getName())) {
+ return item;
+ }
+ }
+ }
+ }
+ }
+ return null;
+ }
+
@Override
public int getPriority() {
return 200;
