bug fixes

- Don't create org folder if there was error
- collect various 400 errors and return in same response

blueocean-commons/src/main/java/io/jenkins/blueocean/commons/ErrorMessage.java

@@ -5,6 +5,7 @@
 
 import javax.annotation.Nonnull;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 
 /**
@@ -42,6 +43,12 @@ public ErrorMessage add(Error error){
         return this;
     }
 
+    @JsonIgnore
+    public ErrorMessage addAll(Collection<Error> errors){
+        this.errors.addAll(errors);
+        return this;
+    }
+
     @JsonProperty("errors")
     public List<Error> getErrors(){
         return errors;

blueocean-git-pipeline/src/main/java/io/jenkins/blueocean/blueocean_git_pipeline/GitPipelineCreateRequest.java

@@ -1,7 +1,10 @@
 package io.jenkins.blueocean.blueocean_git_pipeline;
 
+import com.cloudbees.plugins.credentials.common.StandardUsernameCredentials;
 import hudson.model.Cause;
+import hudson.model.Failure;
 import hudson.model.TopLevelItem;
+import hudson.plugins.git.GitException;
 import io.jenkins.blueocean.commons.ErrorMessage;
 import io.jenkins.blueocean.commons.ServiceException;
 import io.jenkins.blueocean.rest.Reachable;
@@ -15,26 +18,27 @@
 import jenkins.plugins.git.GitSCMSource;
 import org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProject;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
 
 /**
  * @author Vivek Pandey
  */
 public class GitPipelineCreateRequest extends AbstractPipelineCreateRequestImpl {
 
     private static final String MODE = "org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProject";
-
+    private static final Logger logger = LoggerFactory.getLogger(GitPipelineCreateRequest.class);
 
     private BlueScmConfig scmConfig;
 
     @DataBoundConstructor
     public GitPipelineCreateRequest(String name, BlueScmConfig scmConfig) {
+        validate(name, scmConfig);
         setName(name);
-        if(scmConfig == null){
-            throw new ServiceException.BadRequestExpception(new ErrorMessage(400, "Failed to create Git pipeline:"+name)
-                    .add(new ErrorMessage.Error("scmConfig", ErrorMessage.Error.ErrorCodes.MISSING.toString(), "scmConfig is required")));
-        }
         this.scmConfig = scmConfig;
     }
 
@@ -43,16 +47,10 @@ public BluePipeline create(Reachable parent) throws IOException {
 
         String sourceUri = scmConfig.getUri();
 
-        if (sourceUri == null) {
-            throw new ServiceException.BadRequestExpception(new ErrorMessage(400, "Failed to create Git pipeline:"+getName())
-                    .add(new ErrorMessage.Error("scmConfig.uri", ErrorMessage.Error.ErrorCodes.MISSING.toString(), "uri is required")));
-        }
-
         TopLevelItem item = create(Jenkins.getInstance(), getName(), MODE, MultiBranchProjectDescriptor.class);
 
         if (item instanceof WorkflowMultiBranchProject) {
             WorkflowMultiBranchProject project = (WorkflowMultiBranchProject) item;
-            GitUtils.validateCredentials(project, sourceUri, scmConfig.getCredentialId());
 
             //XXX: set credentialId to empty string if null or we get NPE later on
             String credentialId = scmConfig.getCredentialId() == null ? "" : scmConfig.getCredentialId();
@@ -70,4 +68,51 @@ public BluePipeline create(Reachable parent) throws IOException {
         return null;
     }
 
+    private void validate(String name, BlueScmConfig scmConfig){
+        if(scmConfig == null){
+            throw new ServiceException.BadRequestExpception(new ErrorMessage(400, "Failed to create Git pipeline")
+                    .add(new ErrorMessage.Error("scmConfig", ErrorMessage.Error.ErrorCodes.MISSING.toString(), "scmConfig is required")));
+        }
+
+        List<ErrorMessage.Error> errors = new ArrayList<>();
+
+        String sourceUri = scmConfig.getUri();
+
+        if (sourceUri == null) {
+            errors.add(new ErrorMessage.Error("scmConfig.uri", ErrorMessage.Error.ErrorCodes.MISSING.toString(), "uri is required"));
+        }else {
+            try {
+                StandardUsernameCredentials credentials = null;
+                if(scmConfig.getCredentialId() != null){
+                    credentials = GitUtils.getCredentials(Jenkins.getInstance(), sourceUri, scmConfig.getCredentialId());
+                    if (credentials == null) {
+                        errors.add(new ErrorMessage.Error("scmConfig.credentialId",
+                                        ErrorMessage.Error.ErrorCodes.NOT_FOUND.toString(),
+                                        String.format("credentialId: %s not found", scmConfig.getCredentialId())));
+                    }
+                }
+                GitUtils.validateCredentials(sourceUri, credentials);
+            } catch (GitException e) {
+                logger.error("Error validating credential: " + e.getMessage(), e);
+                errors.add(new ErrorMessage.Error("scmConfig.uri", ErrorMessage.Error.ErrorCodes.INVALID.toString(),
+                        e.getMessage()));
+            }
+        }
+
+        try {
+            Jenkins.getInstance().getProjectNamingStrategy().checkName(getName());
+        }catch (Failure f){
+            errors.add(new ErrorMessage.Error("scmConfig.name", ErrorMessage.Error.ErrorCodes.INVALID.toString(), getName() + "in not a valid name"));
+        }
+
+        if(Jenkins.getInstance().getItem(name)!=null) {
+            errors.add(new ErrorMessage.Error("name",
+                    ErrorMessage.Error.ErrorCodes.ALREADY_EXISTS.toString(), name + " already exists"));
+        }
+
+        if(!errors.isEmpty()){
+            throw new ServiceException.BadRequestExpception(new ErrorMessage(400, "Failed to create Git pipeline:"+getName()).addAll(errors));
+        }
+    }
+
 }

blueocean-git-pipeline/src/main/java/io/jenkins/blueocean/blueocean_git_pipeline/GitPipelineUpdateRequest.java

@@ -1,10 +1,13 @@
 package io.jenkins.blueocean.blueocean_git_pipeline;
 
+import com.cloudbees.plugins.credentials.common.StandardUsernameCredentials;
 import hudson.model.Cause;
 import hudson.model.CauseAction;
 import hudson.model.Item;
+import hudson.plugins.git.GitException;
 import hudson.security.ACL;
 import hudson.util.PersistedList;
+import io.jenkins.blueocean.commons.ErrorMessage;
 import io.jenkins.blueocean.commons.ServiceException;
 import io.jenkins.blueocean.rest.model.BluePipeline;
 import io.jenkins.blueocean.rest.model.BluePipelineUpdateRequest;
@@ -15,15 +18,21 @@
 import jenkins.plugins.git.GitSCMSource;
 import org.acegisecurity.Authentication;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.annotation.CheckForNull;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.List;
 
 /**
  * @author Vivek Pandey
  */
 public class GitPipelineUpdateRequest extends BluePipelineUpdateRequest {
+    private static final Logger logger = LoggerFactory.getLogger(GitPipelineUpdateRequest.class);
+
     private final BlueScmConfig scmConfig;
 
     @DataBoundConstructor
@@ -64,10 +73,34 @@ private BranchSource getGitScmSource(MultiBranchProject mbp){
 
         if(scmConfig != null){
             sourceUri = scmConfig.getUri();
+            List<ErrorMessage.Error> errors = new ArrayList<>();
+
+            StandardUsernameCredentials credentials = null;
+            if(scmConfig.getCredentialId() != null){
+                credentials = GitUtils.getCredentials(Jenkins.getInstance(), sourceUri, scmConfig.getCredentialId());
+                if (credentials == null) {
+                    errors.add(new ErrorMessage.Error("scmConfig.credentialId",
+                                    ErrorMessage.Error.ErrorCodes.NOT_FOUND.toString(),
+                                    String.format("credentialId: %s not found", scmConfig.getCredentialId())));
+                }
+            }
+
             if(sourceUri != null) {
-                GitUtils.validateCredentials(mbp, sourceUri, scmConfig.getCredentialId());
+                try {
+                    GitUtils.validateCredentials(sourceUri, credentials);
+                }catch (GitException e){
+                    logger.error("Error validating git request: "+e.getMessage(), e);
+                    errors.add(new ErrorMessage.Error("scmConfig.uri", ErrorMessage.Error.ErrorCodes.INVALID.toString(),
+                                    e.getMessage()));
+                }
             }
             credentialId = scmConfig.getCredentialId() == null ? "" : scmConfig.getCredentialId();
+
+            if (!errors.isEmpty()){
+                throw new ServiceException.BadRequestExpception(new ErrorMessage(400, "Failed to create Git pipeline")
+                        .addAll(errors));
+
+            }
         }
 
         PersistedList<BranchSource> sources = mbp.getSourcesList();

blueocean-git-pipeline/src/main/java/io/jenkins/blueocean/blueocean_git_pipeline/GitUtils.java

@@ -5,14 +5,16 @@
 import com.cloudbees.plugins.credentials.common.StandardUsernameCredentials;
 import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
 import hudson.EnvVars;
+import hudson.model.ItemGroup;
 import hudson.model.TaskListener;
 import hudson.plugins.git.GitException;
 import hudson.security.ACL;
 import io.jenkins.blueocean.commons.ErrorMessage;
 import io.jenkins.blueocean.commons.ServiceException;
-import jenkins.scm.api.SCMSourceOwner;
 import org.jenkinsci.plugins.gitclient.Git;
 import org.jenkinsci.plugins.gitclient.GitClient;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -22,24 +24,15 @@
  * @author Vivek Pandey
  */
 class GitUtils {
+    private static final Logger logger = LoggerFactory.getLogger(GitUtils.class);
+
     /**
      *  Calls 'git ls-remote -h uri' to check if git uri or supplied credentials are valid
      *
-     * @param owner SCM owner, such as MultiBranchProject
      * @param uri git repo uri
-     * @param credentialId credential id to use when accessing git
+     * @param credentials credential to use when accessing git
      */
-    static void validateCredentials(@Nonnull SCMSourceOwner owner, @Nonnull String uri, @Nullable String credentialId){
-        StandardUsernameCredentials credentials = null;
-        if(credentialId != null) {
-            credentials = getCredentials(owner, uri, credentialId);
-            if (credentials == null) {
-                throw new ServiceException.BadRequestExpception(new ErrorMessage(400, "Failed to create Git pipeline")
-                        .add(new ErrorMessage.Error("scmConfig.credentialId",
-                                ErrorMessage.Error.ErrorCodes.NOT_FOUND.toString(),
-                                String.format("credentialId: %s not found", credentialId))));
-            }
-        }
+    static void validateCredentials(@Nonnull String uri, @Nullable StandardUsernameCredentials credentials) throws GitException{
         Git git = new Git(TaskListener.NULL, new EnvVars());
         try {
             GitClient gitClient = git.getClient();
@@ -48,21 +41,30 @@ static void validateCredentials(@Nonnull SCMSourceOwner owner, @Nonnull String u
             }
             gitClient.getRemoteReferences(uri,null, true,false);
         } catch (IOException | InterruptedException e) {
+            logger.error("Error running git remote-ls: " + e.getMessage(), e);
             throw  new ServiceException.UnexpectedErrorException("Failed to create pipeline due to unexpected error: "+e.getMessage(), e);
-        } catch (GitException e){
-            throw new ServiceException.BadRequestExpception(new ErrorMessage(400, "Failed to create Git pipeline")
-                    .add(new ErrorMessage.Error("scmConfig.uri",
-                            ErrorMessage.Error.ErrorCodes.INVALID.toString(),
-                            "Invalid uri: " + uri)), e);
-        } catch (IllegalStateException e){
-            throw new ServiceException.ForbiddenException(new ErrorMessage(403, "Failed to create Git pipeline")
-                    .add(new ErrorMessage.Error("scmConfig.credentialId",
-                            ErrorMessage.Error.ErrorCodes.INVALID.toString(),
-                            "Invalid credentialId: " + credentialId)), e);
+        } catch (IllegalStateException | GitException e){
+            logger.error("Error running git remote-ls: " + e.getMessage(), e);
+            if(credentials != null) {
+                // XXX: check for 'not authorized' is hack. Git plugin API (org.eclipse.jgit.transport.TransportHttp.connect())does not send
+                //      back any error code so that we can distinguish between unauthorized vs bad url or some other type of errors.
+                //      Where org.eclipse.jgit.transport.SshTransport.connect() throws IllegalStateException in case of unauthorized,
+                //      org.eclipse.jgit.transport.HttpTransport.connect() throws TransportException with error code 'not authorized'
+                //      appended to the message.
+                if(e instanceof IllegalStateException || e.getMessage().endsWith("not authorized")){
+                    throw new ServiceException.ForbiddenException(new ErrorMessage(403, "Failed to create Git pipeline")
+                            .add(new ErrorMessage.Error("scmConfig.credentialId",
+                                    ErrorMessage.Error.ErrorCodes.INVALID.toString(),
+                                    "Invalid credentialId: " + credentials.getId())), e);
+                }
+                throw e; // throw GitException so that later on it can be reported as 400 error
+            } else{
+                throw new GitException(e);
+            }
         }
     }
 
-    private static StandardUsernameCredentials getCredentials(SCMSourceOwner owner, String uri, String credentialId){
+    static StandardUsernameCredentials getCredentials(ItemGroup owner, String uri, String credentialId){
         return CredentialsMatchers
                 .firstOrNull(
                         CredentialsProvider.lookupCredentials(StandardUsernameCredentials.class, owner,

blueocean-git-pipeline/src/test/java/io/jenkins/blueocean/blueocean_git_pipeline/GitScmTest.java

@@ -21,9 +21,7 @@
 import java.util.List;
 import java.util.Map;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 /**
  * @author Vivek Pandey
@@ -240,6 +238,7 @@ public void shouldFailOnValidation2(){
 
         assertEquals(errors.get(0).get("field"), "scmConfig");
         assertEquals(errors.get(0).get("code"), "MISSING");
+        assertNull(Jenkins.getInstance().getItem("demo"));
     }
 
     @Test
@@ -255,6 +254,8 @@ public void shouldFailOnValidation3(){
 
         assertEquals(errors.get(0).get("field"), "scmConfig.uri");
         assertEquals(errors.get(0).get("code"), "MISSING");
+        assertNull(Jenkins.getInstance().getItem("demo"));
+
     }
 
 
@@ -273,13 +274,38 @@ public void shouldFailOnValidation4(){
         resp = post("/organizations/jenkins/pipelines/",
                 ImmutableMap.of("name", "demo",
                         "$class", "io.jenkins.blueocean.blueocean_git_pipeline.GitPipelineCreateRequest",
-                        "scmConfig", ImmutableMap.of("uri", sampleRepo.fileUrl())
+                        "scmConfig", ImmutableMap.of("uri", sampleRepo.fileUrl(), "credentialId", "sdsdsd")
                 ), 400);
-        List<Map> errors = (List<Map>) resp.get("errors");
+        List<Map<String,String>> errors = (List<Map<String,String>>) resp.get("errors");
+
+        boolean nameFound = false;
+        boolean credentialIdFound = false;
+        for(Map<String,String> error:errors){
+            if(error.get("field").equals("name")){
+                nameFound = true;
+                assertEquals(error.get("code"), "ALREADY_EXISTS");
+            }else if(error.get("field").equals("scmConfig.credentialId")){
+                credentialIdFound = true;
+                assertEquals(error.get("code"), "NOT_FOUND");
+            }
+        }
+        assertTrue(nameFound);
+        assertTrue(credentialIdFound);
+    }
 
-        assertEquals(errors.get(0).get("field"), "name");
-        assertEquals(errors.get(0).get("code"), "ALREADY_EXISTS");
+    @Test
+    public void shouldFailOnValidation5(){
+
+        Map<String,Object> resp = post("/organizations/jenkins/pipelines/",
+                ImmutableMap.of("name", "demo",
+                        "$class", "io.jenkins.blueocean.blueocean_git_pipeline.GitPipelineCreateRequest",
+                        "scmConfig", ImmutableMap.of("uri", sampleRepo.fileUrl(), "credentialId", "sdsdsd")
+                ), 400);
+        List<Map<String,String>> errors = (List<Map<String,String>>) resp.get("errors");
 
+        assertEquals("scmConfig.credentialId", errors.get(0).get("field"));
+        assertEquals("NOT_FOUND", errors.get(0).get("code"));
+        assertNull(Jenkins.getInstance().getItem("demo"));
     }
 
     private String createMbp(){

blueocean-github-pipeline/src/main/java/io/jenkins/blueocean/blueocean_github_pipeline/GithubPipelineCreateRequest.java

@@ -80,10 +80,10 @@ static void validateCredentialId(String credentialId, OrganizationFolder item, G
                 try {
                     item.delete();
                 } catch (InterruptedException e) {
-                    throw new ServiceException.UnexpectedErrorException("Invalid credentialId: " + credentialId + ". Failure during cleaing up folder: " + item.getName() + ". Error: " + e.getMessage(), e);
+                    throw new ServiceException.UnexpectedErrorException("No Credentials instance found for credentialId: " + credentialId + ". Failure during cleaing up folder: " + item.getName() + ". Error: " + e.getMessage(), e);
                 }
                 throw new ServiceException.BadRequestExpception(new ErrorMessage(400, "Failed to create Git pipeline")
-                        .add(new ErrorMessage.Error("credentialId", ErrorMessage.Error.ErrorCodes.INVALID.toString(), "Invalid credentialId")));
+                        .add(new ErrorMessage.Error("credentialId", ErrorMessage.Error.ErrorCodes.NOT_FOUND.toString(), "No Credentials instance found for credentialId: "+credentialId)));
 
             }
         }