From 6650a79c68501e88dd00ce71874cb0d5d278f25e Mon, 7 May 2012 13:39:37 +0200
From: Benjamin Muskalla <benjamin.muskalla@tasktop.com>
Date: Mon, 7 May 2012 12:39:49 +0200
Subject: [PATCH] NEW - bug 376939: provide default impl for isValidUrl in settings page  https://bugs.eclipse.org/bugs/show_bug.cgi?id=376939


Change-Id: I850dab1abc17bc5e3f69ed021d1d424a211b6582

diff --git a/org.eclipse.mylyn.bugzilla.core/src/org/eclipse/mylyn/internal/bugzilla/core/BugzillaClient.java b/org.eclipse.mylyn.bugzilla.core/src/org/eclipse/mylyn/internal/bugzilla/core/BugzillaClient.java
index 09f72c3..a8ebe39 100644
--- a/org.eclipse.mylyn.bugzilla.core/src/org/eclipse/mylyn/internal/bugzilla/core/BugzillaClient.java
+++ b/org.eclipse.mylyn.bugzilla.core/src/org/eclipse/mylyn/internal/bugzilla/core/BugzillaClient.java
@@ -50,7 +50,6 @@
 import org.apache.commons.httpclient.HttpStatus;
 import org.apache.commons.httpclient.NameValuePair;
 import org.apache.commons.httpclient.RedirectException;
-import org.apache.commons.httpclient.URI;
 import org.apache.commons.httpclient.methods.GetMethod;
 import org.apache.commons.httpclient.methods.HeadMethod;
 import org.apache.commons.httpclient.methods.multipart.FilePart;
@@ -2352,18 +2351,6 @@
 			}
 			return newText;
 		}
-	}
-
-	public static boolean isValidUrl(String url) {
-		if (url.startsWith("https://") || url.startsWith("http://")) { //$NON-NLS-1$//$NON-NLS-2$
-			try {
-				new URI(url, true, "UTF-8"); //$NON-NLS-1$
-				return true;
-			} catch (Exception e) {
-				return false;
-			}
-		}
-		return false;
 	}
 
 	private class HtmlInformation {
diff --git a/org.eclipse.mylyn.bugzilla.ui/src/org/eclipse/mylyn/internal/bugzilla/ui/tasklist/BugzillaRepositorySettingsPage.java b/org.eclipse.mylyn.bugzilla.ui/src/org/eclipse/mylyn/internal/bugzilla/ui/tasklist/BugzillaRepositorySettingsPage.java
index ce31a72..57a7116 100644
--- a/org.eclipse.mylyn.bugzilla.ui/src/org/eclipse/mylyn/internal/bugzilla/ui/tasklist/BugzillaRepositorySettingsPage.java
+++ b/org.eclipse.mylyn.bugzilla.ui/src/org/eclipse/mylyn/internal/bugzilla/ui/tasklist/BugzillaRepositorySettingsPage.java
@@ -610,11 +610,6 @@
 	}
 
 	@Override
-	protected boolean isValidUrl(String url) {
-		return BugzillaClient.isValidUrl(url);
-	}
-
-	@Override
 	protected Validator getValidator(TaskRepository repository) {
 		return new BugzillaValidator(repository);
 	}
diff --git a/org.eclipse.mylyn.oslc.ui/src/org/eclipse/mylyn/internal/oslc/ui/OslcRepositorySettingsPage.java b/org.eclipse.mylyn.oslc.ui/src/org/eclipse/mylyn/internal/oslc/ui/OslcRepositorySettingsPage.java
index 09e4026..639482b 100644
--- a/org.eclipse.mylyn.oslc.ui/src/org/eclipse/mylyn/internal/oslc/ui/OslcRepositorySettingsPage.java
+++ b/org.eclipse.mylyn.oslc.ui/src/org/eclipse/mylyn/internal/oslc/ui/OslcRepositorySettingsPage.java
@@ -104,18 +104,6 @@
 	}
 
 	@Override
-	protected boolean isValidUrl(String url) {
-		if (url.startsWith(URL_PREFIX_HTTPS) || url.startsWith(URL_PREFIX_HTTP)) {
-			try {
-				new URL(url);
-				return true;
-			} catch (MalformedURLException e) {
-			}
-		}
-		return false;
-	}
-
-	@Override
 	public void applyTo(TaskRepository repository) {
 		repository.setProperty(IOslcCoreConstants.OSLC_BASEURL, baseText.getText());
 		super.applyTo(repository);
diff --git a/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/RepositorySettingsPageTest.java b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/RepositorySettingsPageTest.java
index fdaa9ae..70ba21b 100644
--- a/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/RepositorySettingsPageTest.java
+++ b/org.eclipse.mylyn.tasks.tests/src/org/eclipse/mylyn/tasks/tests/RepositorySettingsPageTest.java
@@ -215,6 +215,19 @@
 		}
 	}
 
+	public void testValidUrl() throws Exception {
+		TaskRepository repository = new TaskRepository(MockRepositoryConnector.CONNECTOR_KIND, "http://localhost/");
+		MockRepositorySettingsPage page = new MockRepositorySettingsPage(repository);
+		assertFalse(page.isValidUrl(""));
+		assertFalse(page.isValidUrl("http:/google.com"));
+		assertFalse(page.isValidUrl("http:/google.com/"));
+
+		assertTrue(page.isValidUrl("http://google.com"));
+		assertTrue(page.isValidUrl("https://google.com"));
+		assertTrue(page.isValidUrl("http://mylyn.org/bugzilla34"));
+		assertTrue(page.isValidUrl("http://www.mylyn.org/bugzilla34"));
+	}
+
 	private class MockRepositorySettingsPage extends AbstractRepositorySettingsPage {
 
 		public MockRepositorySettingsPage(TaskRepository taskRepository) {
@@ -227,9 +240,8 @@
 		}
 
 		@Override
-		protected boolean isValidUrl(String name) {
-			// ignore
-			return false;
+		protected boolean isValidUrl(String url) {
+			return super.isValidUrl(url);
 		}
 
 		@Override
diff --git a/org.eclipse.mylyn.tasks.ui/src/org/eclipse/mylyn/tasks/ui/wizards/AbstractRepositorySettingsPage.java b/org.eclipse.mylyn.tasks.ui/src/org/eclipse/mylyn/tasks/ui/wizards/AbstractRepositorySettingsPage.java
index 76f9b3a..6003abe 100644
--- a/org.eclipse.mylyn.tasks.ui/src/org/eclipse/mylyn/tasks/ui/wizards/AbstractRepositorySettingsPage.java
+++ b/org.eclipse.mylyn.tasks.ui/src/org/eclipse/mylyn/tasks/ui/wizards/AbstractRepositorySettingsPage.java
@@ -20,6 +20,7 @@
 import java.util.List;
 import java.util.Set;
 
+import org.apache.commons.httpclient.URI;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IAdaptable;
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -1317,7 +1318,17 @@
 	/**
 	 * @since 2.0
 	 */
-	protected abstract boolean isValidUrl(String url);
+	protected boolean isValidUrl(String url) {
+		if (url.startsWith(URL_PREFIX_HTTPS) || url.startsWith(URL_PREFIX_HTTP)) {
+			try {
+				new URI(url, true, "UTF-8"); //$NON-NLS-1$
+				return true;
+			} catch (Exception e) {
+				return false;
+			}
+		}
+		return false;
+	}
 
 	private void updateHyperlinks() {
 		if (getRepositoryUrl() != null && getRepositoryUrl().length() > 0) {
diff --git a/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/ui/TracRepositorySettingsPageTest.java b/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/ui/TracRepositorySettingsPageTest.java
index dc683b0..33daada 100644
--- a/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/ui/TracRepositorySettingsPageTest.java
+++ b/org.eclipse.mylyn.trac.tests/src/org/eclipse/mylyn/trac/tests/ui/TracRepositorySettingsPageTest.java
@@ -47,6 +47,11 @@
 			super.applyValidatorResult(validator);
 		}
 
+		@Override
+		protected boolean isValidUrl(String name) {
+			return super.isValidUrl(name);
+		}
+
 	}
 
 	private MyTracRepositorySettingsPage page;
@@ -172,4 +177,17 @@
 		assertNull(page.getTracVersion());
 		assertEquals(IMessageProvider.ERROR, page.getMessageType());
 	}
+
+	public void testValidUrl() throws Exception {
+		assertFalse(page.isValidUrl(""));
+		assertFalse(page.isValidUrl("http:/google.com"));
+		assertFalse(page.isValidUrl("http:/google.com/"));
+		assertFalse(page.isValidUrl("http://google.com/"));
+		assertFalse(page.isValidUrl("http://google.com/foo /space"));
+
+		assertTrue(page.isValidUrl("http://google.com"));
+		assertTrue(page.isValidUrl("https://google.com"));
+		assertTrue(page.isValidUrl("http://mylyn.org/trac30"));
+		assertTrue(page.isValidUrl("http://www.mylyn.org/trac30"));
+	}
 }
diff --git a/org.eclipse.mylyn.trac.ui/src/org/eclipse/mylyn/internal/trac/ui/wizard/TracRepositorySettingsPage.java b/org.eclipse.mylyn.trac.ui/src/org/eclipse/mylyn/internal/trac/ui/wizard/TracRepositorySettingsPage.java
index f083780..a326525 100644
--- a/org.eclipse.mylyn.trac.ui/src/org/eclipse/mylyn/internal/trac/ui/wizard/TracRepositorySettingsPage.java
+++ b/org.eclipse.mylyn.trac.ui/src/org/eclipse/mylyn/internal/trac/ui/wizard/TracRepositorySettingsPage.java
@@ -13,7 +13,6 @@
 package org.eclipse.mylyn.internal.trac.ui.wizard;
 
 import java.net.MalformedURLException;
-import java.net.URL;
 
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -124,15 +123,9 @@
 	}
 
 	@Override
-	protected boolean isValidUrl(String name) {
-		if ((name.startsWith(URL_PREFIX_HTTPS) || name.startsWith(URL_PREFIX_HTTP)) && !name.endsWith("/")) { //$NON-NLS-1$
-			try {
-				new URL(name);
-				return true;
-			} catch (MalformedURLException e) {
-			}
-		}
-		return false;
+	protected boolean isValidUrl(String url) {
+		boolean isValid = super.isValidUrl(url);
+		return isValid && !url.endsWith("/"); //$NON-NLS-1$
 	}
 
 	public Version getTracVersion() {