From ff82d4450518a7e98ecbc70ed778acf3592b926c Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Tue, 20 Jun 2017 12:40:35 +0200 Subject: [PATCH] FetchGerritChangePage: validate change ref against advertised refs If the background git ls-remote has finished when we validate the page, also check that the Gerrit server actually advertises the specified ref. Change-Id: I0c5ec3da3d08f9995167291f6d2d3441124474f7 Signed-off-by: Thomas Wolf --- .../src/org/eclipse/egit/ui/internal/UIText.java | 3 ++ .../ui/internal/fetch/FetchGerritChangePage.java | 52 ++++++++++++++++++---- .../org/eclipse/egit/ui/internal/uitext.properties | 1 + 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java index a2cf8274a..6f2b73930 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/UIText.java @@ -2885,6 +2885,9 @@ public class UIText extends NLS { public static String FetchGerritChangePage_TagRadio; /** */ + public static String FetchGerritChangePage_UnknownChangeRefMessage; + + /** */ public static String FetchGerritChangePage_UpdateRadio; /** */ diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java index 0371ccd7d..3ccc8af1a 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/fetch/FetchGerritChangePage.java @@ -17,10 +17,14 @@ import java.lang.reflect.InvocationTargetException; import java.net.URISyntaxException; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.regex.Matcher; @@ -667,6 +671,13 @@ public class FetchGerritChangePage extends WizardPage { setErrorMessage(UIText.FetchGerritChangePage_MissingChangeMessage); return; } + ChangeList list = changeRefs.get(uriCombo.getText()); + if (list != null && list.isDone() + && !list.getResult().contains(change)) { + setErrorMessage( + UIText.FetchGerritChangePage_UnknownChangeRefMessage); + return; + } } else { setErrorMessage(UIText.FetchGerritChangePage_MissingChangeMessage); return; @@ -681,20 +692,20 @@ public class FetchGerritChangePage extends WizardPage { } } - private List getRefsForContentAssist() + private Collection getRefsForContentAssist() throws InvocationTargetException, InterruptedException { String uriText = uriCombo.getText(); if (!changeRefs.containsKey(uriText)) { changeRefs.put(uriText, new ChangeList(repository, uriText)); } ChangeList list = changeRefs.get(uriText); - if (!list.isDone()) { + if (!list.isFinished()) { IWizardContainer container = getContainer(); IRunnableWithProgress operation = monitor -> { monitor.beginTask(MessageFormat.format( UIText.FetchGerritChangePage_FetchingRemoteRefsMessage, uriText), IProgressMonitor.UNKNOWN); - List result = list.get(); + Collection result = list.get(); if (monitor.isCanceled()) { return; } @@ -938,7 +949,7 @@ public class FetchGerritChangePage extends WizardPage { @Override public IContentProposal[] getProposals(String contents, int position) { - List proposals; + Collection proposals; try { proposals = getRefsForContentAssist(); } catch (InvocationTargetException e) { @@ -1046,6 +1057,19 @@ public class FetchGerritChangePage extends WizardPage { } @Override + public boolean equals(Object obj) { + if (!(obj instanceof Change)) { + return false; + } + return compareTo((Change) obj) == 0; + } + + @Override + public int hashCode() { + return Objects.hash(changeNumber, patchSetNumber); + } + + @Override public int compareTo(Change o) { int changeDiff = this.changeNumber.compareTo(o.changeNumber); if (changeDiff == 0) { @@ -1142,7 +1166,7 @@ public class FetchGerritChangePage extends WizardPage { private State state = State.PRISTINE; - private List result; + private Set result; private InterruptibleJob job; @@ -1200,10 +1224,14 @@ public class FetchGerritChangePage extends WizardPage { } } - public synchronized boolean isDone() { + public synchronized boolean isFinished() { return state == State.CANCELED || state == State.DONE; } + public synchronized boolean isDone() { + return state == State.DONE; + } + /** * Retrieves the result. If the result is not yet available, the method * blocks until it is or {@link #cancel(CancelMode)} is called with @@ -1216,7 +1244,7 @@ public class FetchGerritChangePage extends WizardPage { * @throws InvocationTargetException * if the future's job cannot be created */ - public synchronized List get() + public synchronized Collection get() throws InterruptedException, InvocationTargetException { switch (state) { case DONE: @@ -1235,6 +1263,14 @@ public class FetchGerritChangePage extends WizardPage { } } + public synchronized Collection getResult() { + if (isFinished()) { + return result; + } + throw new IllegalStateException( + "Fetching change list is not finished"); //$NON-NLS-1$ + } + private synchronized void finish(boolean done) { state = done ? State.DONE : State.CANCELED; job = null; @@ -1301,7 +1337,7 @@ public class FetchGerritChangePage extends WizardPage { } } Collections.sort(changes, Collections.reverseOrder()); - result = changes; + result = new LinkedHashSet<>(changes); return Status.OK_STATUS; } diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties index 8c708d222..4eed8cf0a 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/uitext.properties @@ -969,6 +969,7 @@ FetchGerritChangePage_ShowingProposalsJobName=Showing proposals FetchGerritChangePage_SuggestedRefNamePattern=change/{0}/{1} FetchGerritChangePage_TagNameText=Tag &name: FetchGerritChangePage_TagRadio=Create and check out a &tag +FetchGerritChangePage_UnknownChangeRefMessage=The Gerrit server has no patch set for this change reference. FetchGerritChangePage_UpdateRadio=U&pdate FETCH_HEAD only FetchGerritChangePage_UriLabel=&URI: FetchGerritChangeWizard_WizardTitle=Fetch a change from Gerrit -- 2.11.4.GIT