Merge pull request #22 from sap-production/reviewRemoteAddress

review remote address

Author: alejandraferreirovidal

src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteBuildConfiguration.java

@@ -92,7 +92,7 @@ public class RemoteBuildConfiguration extends Builder implements SimpleBuildStep
      */
     private final static Auth2 DEFAULT_AUTH = NullAuth.INSTANCE;
 
-    
+
     private static final int      DEFAULT_POLLINTERVALL = 10;
     private static final String   paramerizedBuildUrl   = "/buildWithParameters";
     private static final String   normalBuildUrl        = "/build";
@@ -502,7 +502,10 @@ private String buildTriggerUrl(String jobNameOrUrl, String securityToken, Collec
 
         if (context.effectiveRemoteServer.getHasBuildTokenRootSupport()) {
           // start building the proper URL based on known capabiltiies of the remote server
-            triggerUrlString = context.effectiveRemoteServer.getRemoteAddress();
+            if (context.effectiveRemoteServer.getAddress() == null) {
+                throw new AbortException("The remote server address can not be empty, or it must be overridden on the job configuration.");
+            }
+            triggerUrlString = context.effectiveRemoteServer.getAddress();
             triggerUrlString += buildTokenRootUrl;
             triggerUrlString += getBuildTypeUrl(isRemoteJobParameterized);
             query = addToQueryString(query, "job=" + encodeValue(jobNameOrUrl)); //TODO: does it work with full URL?
@@ -815,7 +818,10 @@ public void performWaitForBuild(BuildContext context, Handle handle)
     private QueueItemData getQueueItemData(@Nonnull String queueId, @Nonnull BuildContext context)
             throws IOException {
 
-      String queueQuery = String.format("%s/queue/item/%s/api/json/", context.effectiveRemoteServer.getRemoteAddress(), queueId);
+      if (context.effectiveRemoteServer.getAddress() == null) {
+          throw new AbortException("The remote server address can not be empty, or it must be overridden on the job configuration.");
+      }
+      String queueQuery = String.format("%s/queue/item/%s/api/json/", context.effectiveRemoteServer.getAddress(), queueId);
       ConnectionResponse response = sendHTTPCall( queueQuery, "GET", context, 1 );
       JSONObject queueResponse = response.getBody();
 
@@ -1187,7 +1193,10 @@ private String readInputStream(HttpURLConnection connection) throws IOException
     @Nonnull
     private JenkinsCrumb getCrumb(BuildContext context) throws IOException
     {
-        String address = context.effectiveRemoteServer.getRemoteAddress();
+        String address = context.effectiveRemoteServer.getAddress();
+        if (address == null) {
+            throw new AbortException("The remote server address can not be empty, or it must be overridden on the job configuration.");
+        }
         URL crumbProviderUrl;
         try {
             String xpathValue = URLEncoder.encode("concat(//crumbRequestField,\":\",//crumb)", "UTF-8");
@@ -1461,15 +1470,18 @@ private boolean isRemoteJobParameterized(JSONObject remoteJobMetadata) throws IO
         return isParameterized;
     }
 
-    protected static String generateJobUrl(RemoteJenkinsServer remoteServer, String jobNameOrUrl)
+    protected static String generateJobUrl(RemoteJenkinsServer remoteServer, String jobNameOrUrl) throws AbortException
     {
         if(isEmpty(jobNameOrUrl)) throw new IllegalArgumentException("Invalid job name/url: " + jobNameOrUrl);
         String remoteJobUrl;
         String _jobNameOrUrl = jobNameOrUrl.trim();
         if(FormValidationUtils.isURL(_jobNameOrUrl)) {
             remoteJobUrl = _jobNameOrUrl;
         } else {
-            remoteJobUrl = remoteServer.getRemoteAddress();
+            remoteJobUrl = remoteServer.getAddress();
+            if (remoteJobUrl == null) {
+                throw new AbortException("The remote server address can not be empty, or it must be overridden on the job configuration.");
+            }
             while(remoteJobUrl.endsWith("/")) remoteJobUrl = remoteJobUrl.substring(0, remoteJobUrl.length()-1);
 
             String[] split = _jobNameOrUrl.trim().split("/");

src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteJenkinsServer.java

@@ -135,24 +135,6 @@ public DescriptorImpl getDescriptor() {
         return (DescriptorImpl) super.getDescriptor();
     }
 
-    /**
-     * @return the remote server address
-     * @throws RuntimeException
-     *             if the address of the remote server was not set
-     */
-    @Nonnull
-    public String getRemoteAddress() {
-        if (address == null) {
-            throw new RuntimeException("The remote address can not be empty.");
-        } else {
-            try {
-                new URL(address);
-            } catch (MalformedURLException e) {
-                throw new RuntimeException("Malformed address (" + address + "). Remember to indicate the protocol, i.e. http, https, etc.");
-            }
-        }
-        return address;
-    }
 
     @Extension
     public static class DescriptorImpl extends Descriptor<RemoteJenkinsServer> {

src/main/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/pipeline/Handle.java

@@ -335,7 +335,7 @@ public void setBuildData(BuildData buildData)
     public String toString() {
 
         StringBuilder sb = new StringBuilder();
-        sb.append(String.format("Handle [job=%s, remoteServerURL=%s, queueId=%s", remoteBuildConfiguration.getJob(), effectiveRemoteServer.getRemoteAddress(), queueId));
+        sb.append(String.format("Handle [job=%s, remoteServerURL=%s, queueId=%s", remoteBuildConfiguration.getJob(), effectiveRemoteServer.getAddress(), queueId));
         if(buildStatus != null) sb.append(String.format(", buildStatus=%s", buildStatus));
         if(buildData != null) sb.append(String.format(", buildNumber=%s, buildUrl=%s", buildData.getBuildNumber(), buildData.getURL()));
         sb.append("]");

src/test/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteBuildConfigurationTest.java

@@ -193,7 +193,7 @@ public void testJobUrlHandling_withJobNameAndRemoteUrl() throws IOException {
         config.setJob("MyJob");
         config.setRemoteJenkinsUrl("http://test:8080");
         assertEquals("MyJob", config.getJob());
-        assertEquals("http://test:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://test:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
@@ -204,7 +204,7 @@ public void testJobUrlHandling_withJobNameAndRemoteName() throws IOException {
 
         config.setRemoteJenkinsName("remoteJenkinsName");
         assertEquals("MyJob", config.getJob());
-        assertEquals("http://test:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://test:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
@@ -215,15 +215,15 @@ public void testJobUrlHandling_withMultiFolderJobNameAndRemoteName() throws IOEx
 
         config.setRemoteJenkinsName("remoteJenkinsName");
         assertEquals("A/B/C/D/MyJob", config.getJob());
-        assertEquals("http://test:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://test:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
     public void testJobUrlHandling_withJobUrl() throws IOException {
         RemoteBuildConfiguration config = new RemoteBuildConfiguration();
         config.setJob("http://test:8080/job/folder/job/MyJob");
         assertEquals("http://test:8080/job/folder/job/MyJob", config.getJob()); //The value configured for "job"
-        assertEquals("http://test:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://test:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
@@ -233,7 +233,7 @@ public void testJobUrlHandling_withJobUrlAndRemoteUrl() throws IOException {
         config.setJob("http://testA:8080/job/folder/job/MyJobA");
         config.setRemoteJenkinsUrl("http://testB:8080");
         assertEquals("http://testA:8080/job/folder/job/MyJobA", config.getJob()); //The value configured for "job"
-        assertEquals("http://testA:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://testA:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
@@ -245,7 +245,7 @@ public void testJobUrlHandling_withJobUrlAndRemoteName() throws IOException {
 
         config.setRemoteJenkinsName("remoteJenkinsName");
         assertEquals("http://testA:8080/job/folder/job/MyJobA", config.getJob()); //The value configured for "job"
-        assertEquals("http://testA:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://testA:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
@@ -270,12 +270,12 @@ public void testRemoteUrlOverridesRemoteName() throws IOException {
         config = mockGlobalRemoteHost(config, "remoteJenkinsName", "http://globallyConfigured:8080");
 
         config.setRemoteJenkinsName("remoteJenkinsName");
-        assertEquals("http://globallyConfigured:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://globallyConfigured:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
 
         //Now override remote host URL
         config.setRemoteJenkinsUrl("http://locallyOverridden:8080");
         assertEquals("MyJob", config.getJob());
-        assertEquals("http://locallyOverridden:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://locallyOverridden:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
@@ -312,7 +312,7 @@ public void testEvaluateEffectiveRemoteHost_globalConfigMissing_localOverrideHos
 
         config.setRemoteJenkinsName("notConfiguredRemoteHost");
         config.setRemoteJenkinsUrl("http://locallyOverridden:8080");
-        assertEquals("http://locallyOverridden:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://locallyOverridden:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
@@ -322,15 +322,15 @@ public void testEvaluateEffectiveRemoteHost_globalConfigMissing_localOverrideJob
         config = mockGlobalRemoteHost(config, "remoteJenkinsName", "http://globallyConfigured:8080");
 
         config.setRemoteJenkinsName("notConfiguredRemoteHost");
-        assertEquals("http://localJobUrl:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://localJobUrl:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
     public void testEvaluateEffectiveRemoteHost_localOverrideHostURL() throws IOException {
         RemoteBuildConfiguration config = new RemoteBuildConfiguration();
         config.setJob("MyJob");
         config.setRemoteJenkinsUrl("http://hostname:8080");
-        assertEquals("http://hostname:8080", config.evaluateEffectiveRemoteHost(null).getRemoteAddress());
+        assertEquals("http://hostname:8080", config.evaluateEffectiveRemoteHost(null).getAddress());
     }
 
     @Test @WithoutJenkins
@@ -384,7 +384,7 @@ public void testRemoveHashParameters() {
     }
 
     @Test @WithoutJenkins
-    public void testGenerateJobUrl() throws MalformedURLException {
+    public void testGenerateJobUrl() throws MalformedURLException, AbortException {
         RemoteJenkinsServer remoteServer = new RemoteJenkinsServer();
         remoteServer.setAddress("https://server:8080/jenkins");
 
@@ -411,8 +411,8 @@ public void testGenerateJobUrl() throws MalformedURLException {
         try {
             RemoteJenkinsServer missingUrl = new RemoteJenkinsServer();
             RemoteBuildConfiguration.generateJobUrl(missingUrl, "JobName");
-            Assert.fail("Expected RuntimeException");
-        } catch(RuntimeException e) {}
+            Assert.fail("Expected AbortException");
+        } catch(AbortException e) {}
 
     }
 

src/test/java/org/jenkinsci/plugins/ParameterizedRemoteTrigger/RemoteJenkinsServerTest.java

@@ -37,8 +37,6 @@ public void testCloneBehaviour() throws Exception {
         verifyEqualsHashCode(server, clone);
         assertEquals("address", ADDRESS, clone.getAddress());
         assertEquals("address", server.getAddress(), clone.getAddress());
-        assertEquals("remoteAddress", ADDRESS, clone.getRemoteAddress());
-        assertEquals("remoteAddress", server.getRemoteAddress(), clone.getRemoteAddress());
         assertEquals("auth2", server.getAuth2(), clone.getAuth2());
         assertEquals("displayName", DISPLAY_NAME, clone.getDisplayName());
         assertEquals("displayName", server.getDisplayName(), clone.getDisplayName());