chore: Refactor exception classes

server-core/src/main/java/io/onedev/server/entitymanager/impl/DefaultIssueChangeManager.java

@@ -41,6 +41,7 @@ import io.onedev.commons.utils.match.Matcher;
 import io.onedev.commons.utils.match.PathMatcher;
 import io.onedev.commons.utils.match.StringMatcher;
 import io.onedev.server.OneDev;
+import io.onedev.server.buildspecmodel.inputspec.Input;
 import io.onedev.server.cluster.ClusterManager;
 import io.onedev.server.entitymanager.IssueChangeManager;
 import io.onedev.server.entitymanager.IssueDescriptionRevisionManager;
@@ -59,7 +60,6 @@ import io.onedev.server.event.project.issue.IssueChanged;
 import io.onedev.server.event.project.pullrequest.PullRequestChanged;
 import io.onedev.server.event.system.SystemStarted;
 import io.onedev.server.event.system.SystemStopping;
-import io.onedev.server.exception.InvalidIssueFieldsException;
 import io.onedev.server.git.GitUtils;
 import io.onedev.server.model.Build;
 import io.onedev.server.model.Issue;
@@ -113,7 +113,6 @@ import io.onedev.server.search.entity.issue.StateCriteria;
 import io.onedev.server.security.SecurityUtils;
 import io.onedev.server.taskschedule.SchedulableTask;
 import io.onedev.server.taskschedule.TaskScheduler;
-import io.onedev.server.buildspecmodel.inputspec.Input;
 import io.onedev.server.util.ProjectScope;
 import io.onedev.server.util.ProjectScopedCommit;
 import io.onedev.server.util.concurrent.BatchWorkManager;
@@ -442,11 +441,7 @@ public class DefaultIssueChangeManager extends BaseEntityManager<IssueChange>
 				issueScheduleManager.syncIterations(issue, iterations);
 			
 			issue.setFieldValues(fieldValues);
-			try {
+			issue.validateFields();
-				issue.validateFields();
-			} catch (InvalidIssueFieldsException e) {
-				throw new InvalidIssueFieldsException("Error validating fields for issue " + issue.getReference().toString(issue.getProject()) + ": " + InvalidIssueFieldsException.buildMessage(e.getInvalidFields()), e.getInvalidFields());
-			}
 			issueFieldManager.saveFields(issue);
 
 			if (!prevState.equals(issue.getState()) 

server-core/src/main/java/io/onedev/server/entityreference/EntityReference.java

@@ -3,13 +3,13 @@ package io.onedev.server.entityreference;
 import java.io.Serializable;
 
 import javax.annotation.Nullable;
+import javax.validation.ValidationException;
 
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 
 import io.onedev.server.OneDev;
 import io.onedev.server.entitymanager.ProjectManager;
-import io.onedev.server.exception.InvalidReferenceException;
 import io.onedev.server.model.Project;
 
 public abstract class EntityReference implements Serializable {
@@ -54,7 +54,7 @@ public abstract class EntityReference implements Serializable {
 		try {
 			return Long.valueOf(numberString);
 		} catch (NumberFormatException e) {
-			throw new InvalidReferenceException("Invalid reference number: " + numberString);
+			throw new ValidationException("Invalid reference number: " + numberString);
 		}
 	}
 
@@ -68,13 +68,13 @@ public abstract class EntityReference implements Serializable {
 				if (currentProject != null)
 					return EntityReference.of(type, currentProject, number);
 				else
-					throw new InvalidReferenceException("Reference project not specified: " + referenceString);
+					throw new ValidationException("Reference project not specified: " + referenceString);
 			} else {
 				var project = projectManager.findByPath(projectPath);
 				if (project != null)
 					return EntityReference.of(type, project, number);
 				else
-					throw new InvalidReferenceException("Reference project not found: " + projectPath);
+					throw new ValidationException("Reference project not found: " + projectPath);
 			}
 		}
 		index = referenceString.indexOf('-');
@@ -85,9 +85,9 @@ public abstract class EntityReference implements Serializable {
 			if (project != null)
 				return EntityReference.of(type, project, number);
 			else
-				throw new InvalidReferenceException("Reference project not found with key: " + projectKey);
+				throw new ValidationException("Reference project not found with key: " + projectKey);
 		}
-		throw new InvalidReferenceException("Invalid entity reference: " + referenceString);
+		throw new ValidationException("Invalid entity reference: " + referenceString);
 	}
 	
 	@Override

server-core/src/main/java/io/onedev/server/exception/InvalidIssueFieldsException.java

@@ -1,33 +0,0 @@
-package io.onedev.server.exception;
-
-import java.util.ArrayList;
-import java.util.Map;
-
-import io.onedev.commons.utils.ExplicitException;
-
-public class InvalidIssueFieldsException extends ExplicitException {
-
-    private final Map<String, String> invalidFields;
-
-    public InvalidIssueFieldsException(Map<String, String> invalidFields) {
-        this(buildMessage(invalidFields), invalidFields);
-    }
-
-    public InvalidIssueFieldsException(String message, Map<String, String> invalidFields) {
-        super(message);
-        this.invalidFields = invalidFields;
-    }
-
-    public static String buildMessage(Map<String, String> invalidFields) {
-        var fieldErrors = new ArrayList<String>();
-        for (var entry: invalidFields.entrySet()) {
-            fieldErrors.add(entry.getKey() + " (" + entry.getValue() + ")");
-        }
-        return "Invalid fields: " + String.join(", ", fieldErrors);
-    }
-
-    public Map<String, String> getInvalidFields() {
-        return invalidFields;
-    }
-    
-}

server-core/src/main/java/io/onedev/server/exception/InvalidReferenceException.java

@@ -1,11 +0,0 @@
-package io.onedev.server.exception;
-
-import io.onedev.commons.utils.ExplicitException;
-
-public class InvalidReferenceException extends ExplicitException {
-
-    public InvalidReferenceException(String message) {
-        super(message);
-    }
-
-}

server-core/src/main/java/io/onedev/server/exception/IssueLinkValidationException.java

@@ -1,11 +0,0 @@
-package io.onedev.server.exception;
-
-import io.onedev.commons.utils.ExplicitException;
-
-public class IssueLinkValidationException extends ExplicitException {
-
-	public IssueLinkValidationException(String message) {
-		super(message);
-	}
-
-}

server-core/src/main/java/io/onedev/server/exception/handler/ValidationExceptionHandler.java

@@ -14,7 +14,7 @@ public class ValidationExceptionHandler extends AbstractExceptionHandler<Validat
 		var errorMessage = exception.getMessage();
 		if (errorMessage == null)
 			errorMessage = "Validation error";
-		return new HttpResponse(HttpServletResponse.SC_BAD_REQUEST, errorMessage);
+		return new HttpResponse(HttpServletResponse.SC_NOT_ACCEPTABLE, errorMessage);
     }
     
 }

server-core/src/main/java/io/onedev/server/model/Issue.java

@@ -54,6 +54,7 @@ import javax.persistence.ManyToOne;
 import javax.persistence.OneToMany;
 import javax.persistence.Table;
 import javax.persistence.UniqueConstraint;
+import javax.validation.ValidationException;
 
 import org.apache.commons.lang3.StringUtils;
 import org.eclipse.jgit.lib.ObjectId;
@@ -79,7 +80,6 @@ import io.onedev.server.entitymanager.SettingManager;
 import io.onedev.server.entitymanager.UserManager;
 import io.onedev.server.entityreference.EntityReference;
 import io.onedev.server.entityreference.IssueReference;
-import io.onedev.server.exception.InvalidIssueFieldsException;
 import io.onedev.server.model.support.EntityWatch;
 import io.onedev.server.model.support.LastActivity;
 import io.onedev.server.model.support.ProjectBelonging;
@@ -1086,8 +1086,13 @@ public class Issue extends ProjectBelonging implements AttachmentStorageSupport
 					}
 				}
 			}
-			if (invalidFields.size() > 0) 
+			if (invalidFields.size() > 0) {
-				throw new InvalidIssueFieldsException(invalidFields);
+				var fieldErrors = new ArrayList<String>();
+				for (var entry: invalidFields.entrySet()) {
+					fieldErrors.add(entry.getKey() + " (" + entry.getValue() + ")");
+				}
+				throw new ValidationException("Invalid fields: " + String.join(", ", fieldErrors));
+			}
 		} finally {
 			Project.pop();
 		}

server-core/src/main/java/io/onedev/server/model/IssueLink.java

@@ -7,8 +7,7 @@ import javax.persistence.JoinColumn;
 import javax.persistence.ManyToOne;
 import javax.persistence.Table;
 import javax.persistence.UniqueConstraint;
-
+import javax.validation.ValidationException;
-import io.onedev.server.exception.IssueLinkValidationException;
 
 @Entity
 @Table(
@@ -66,35 +65,35 @@ public class IssueLink extends AbstractEntity {
 
 	public void validate() {
 		if (getSource().equals(getTarget()))
-			throw new IssueLinkValidationException("Can not link to self");
+			throw new ValidationException("Can not link to self");
 		if (getSpec().getOpposite() != null) {
 			if (getSource().getTargetLinks().stream()
 					.anyMatch(it -> it.getSpec().equals(getSpec()) && it.getTarget().equals(getTarget())))
-				throw new IssueLinkValidationException("Source issue already linked to target issue via specified link spec");
+				throw new ValidationException("Source issue already linked to target issue via specified link spec");
 			if (!getSpec().isMultiple()
 					&& getSource().getTargetLinks().stream().anyMatch(it -> it.getSpec().equals(getSpec())))
-				throw new IssueLinkValidationException(
+				throw new ValidationException(
 						"Link spec is not multiple and the source issue is already linked to another issue via this link spec");
 			if (!getSpec().getParsedIssueQuery(getSource().getProject()).matches(getTarget()))
-				throw new IssueLinkValidationException("Link spec not allowed to link to the target issue");
+				throw new ValidationException("Link spec not allowed to link to the target issue");
 			if (!getSpec().getOpposite().isMultiple()
 					&& getTarget().getSourceLinks().stream().anyMatch(it -> it.getSpec().equals(getSpec())))
-				throw new IssueLinkValidationException(
+				throw new ValidationException(
 						"Opposite side of link spec is not multiple and the target issue is already linked to another issue via this link spec");
 			if (!getSpec().getOpposite().getParsedIssueQuery(getSource().getProject())
 					.matches(getSource()))
-				throw new IssueLinkValidationException("Opposite side of link spec not allowed to link to the source issue");
+				throw new ValidationException("Opposite side of link spec not allowed to link to the source issue");
 		} else {
 			if (getSource().getLinks().stream().anyMatch(it -> it.getSpec().equals(getSpec())
 					&& it.getLinked(getSource()).equals(getTarget())))
-				throw new IssueLinkValidationException("Specified issues already linked via specified link spec");
+				throw new ValidationException("Specified issues already linked via specified link spec");
 			if (!getSpec().isMultiple()
 					&& getSource().getLinks().stream().anyMatch(it -> it.getSpec().equals(getSpec())))
-				throw new IssueLinkValidationException(
+				throw new ValidationException(
 						"Link spec is not multiple and source issue is already linked to another issue via this link spec");
 			var parsedIssueQuery = getSpec().getParsedIssueQuery(getSource().getProject());
 			if (!parsedIssueQuery.matches(getSource()) || !parsedIssueQuery.matches(getTarget()))
-				throw new IssueLinkValidationException("Link spec not allowed to link specified issues");
+				throw new ValidationException("Link spec not allowed to link specified issues");
 		}
 	}
 }

server-core/src/main/java/io/onedev/server/rest/JerseyApplication.java

@@ -30,7 +30,7 @@ public class JerseyApplication extends ResourceConfig {
         // Add this in order to send build log entries as soon as possible in 
         // KubernetesResource.runServerStep
         property(ServerProperties.OUTBOUND_CONTENT_LENGTH_BUFFER, 0);
-        
+
         // add the default Jackson exception mappers
         register(JacksonFeature.class);
         

server-core/src/main/java/io/onedev/server/rest/JerseyExceptionMapper.java

@@ -1,16 +1,17 @@
 package io.onedev.server.rest;
 
-import io.onedev.server.exception.ExceptionUtils;
+import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
-import org.eclipse.jetty.http.HttpStatus;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 import javax.ws.rs.ext.ExceptionMapper;
 import javax.ws.rs.ext.Provider;
 
-import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+import org.eclipse.jetty.http.HttpStatus;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import io.onedev.server.exception.ExceptionUtils;
 
 @Provider
 public class JerseyExceptionMapper implements ExceptionMapper<Throwable> {

server-core/src/main/java/io/onedev/server/rest/ValidationExceptionMapper.java

@@ -0,0 +1,28 @@
+package io.onedev.server.rest;
+
+import javax.annotation.Priority;
+import javax.servlet.http.HttpServletResponse;
+import javax.validation.ValidationException;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.ExceptionMapper;
+import javax.ws.rs.ext.Provider;
+
+/**
+ * Must have this although we have a general JerseyExceptionMapper (and in turn takes care of ValidationException 
+ * via exception handler), as Jersey has an internal ValidationExceptionMapper whose parameter for ExceptionMapper 
+ * has smaller distance to ValidationException than JerseyExceptionMapper (Jersey uses this to find the most 
+ * relevant exception mapper for a given exception). 
+ */
+@Provider
+@Priority(1)
+public class ValidationExceptionMapper implements ExceptionMapper<ValidationException> {
+	
+    @Override
+    public Response toResponse(ValidationException t) {
+		var errorMessage = t.getMessage();
+		if (errorMessage == null)
+			errorMessage = "Validation error";
+		return Response.status(HttpServletResponse.SC_NOT_ACCEPTABLE).entity(errorMessage).build();
+    }
+    
+}

server-core/src/main/java/io/onedev/server/rest/resource/IssueLinkResource.java

@@ -9,7 +9,6 @@ import javax.validation.constraints.NotNull;
 import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
 import javax.ws.rs.GET;
-import javax.ws.rs.NotAcceptableException;
 import javax.ws.rs.POST;
 import javax.ws.rs.Path;
 import javax.ws.rs.PathParam;
@@ -20,7 +19,6 @@ import javax.ws.rs.core.Response;
 import org.apache.shiro.authz.UnauthorizedException;
 
 import io.onedev.server.entitymanager.IssueLinkManager;
-import io.onedev.server.exception.IssueLinkValidationException;
 import io.onedev.server.model.IssueLink;
 import io.onedev.server.rest.annotation.Api;
 
@@ -57,11 +55,7 @@ public class IssueLinkResource {
 			throw new UnauthorizedException("No permission to add specified link for specified issues");
 
 		}
-		try {
+		link.validate();
-			link.validate();
-		} catch (IssueLinkValidationException e) {
-			throw new NotAcceptableException(e.getMessage());
-		}
 				
 		linkManager.create(link);
 		return link.getId();

server-core/src/main/java/io/onedev/server/rest/resource/IssueResource.java

@@ -45,7 +45,6 @@ import io.onedev.server.entitymanager.IssueManager;
 import io.onedev.server.entitymanager.IterationManager;
 import io.onedev.server.entitymanager.ProjectManager;
 import io.onedev.server.entitymanager.SettingManager;
-import io.onedev.server.exception.InvalidIssueFieldsException;
 import io.onedev.server.model.Issue;
 import io.onedev.server.model.IssueChange;
 import io.onedev.server.model.IssueComment;
@@ -336,12 +335,7 @@ public class IssueResource {
 		}
 
 		issue.setFieldValues(FieldUtils.getFieldValues(project, data.fields));
-
+		issueManager.open(issue);
-		try {
-			issueManager.open(issue);
-		} catch (InvalidIssueFieldsException e) {
-			throw new NotAcceptableException(e.getMessage());
-		}
 
 		return issue.getId();
     }
@@ -428,11 +422,7 @@ public class IssueResource {
 			throw new UnauthorizedException();
 		}
 
-		try {
+		issueChangeManager.changeFields(issue, FieldUtils.getFieldValues(issue.getProject(), fields));
-			issueChangeManager.changeFields(issue, FieldUtils.getFieldValues(issue.getProject(), fields));
-		} catch (InvalidIssueFieldsException e) {
-			throw new NotAcceptableException(e.getMessage());
-		}
 		return Response.ok().build();
     }
 
@@ -457,11 +447,7 @@ public class IssueResource {
 		}
     	
 		var fieldValues = FieldUtils.getFieldValues(issue.getProject(), data.getFields());
-		try {
+		issueChangeManager.changeState(issue, data.getState(), fieldValues, transition.getPromptFields(), transition.getRemoveFields(), data.getComment());
-			issueChangeManager.changeState(issue, data.getState(), fieldValues, transition.getPromptFields(), transition.getRemoveFields(), data.getComment());
-		} catch (InvalidIssueFieldsException e) {
-			throw new NotAcceptableException(e.getMessage());
-		}
 		return Response.ok().build();
     }
 

server-core/src/main/java/io/onedev/server/rest/resource/McpHelperResource.java

@@ -73,9 +73,6 @@ import io.onedev.server.entitymanager.UserManager;
 import io.onedev.server.entityreference.BuildReference;
 import io.onedev.server.entityreference.IssueReference;
 import io.onedev.server.entityreference.PullRequestReference;
-import io.onedev.server.exception.InvalidIssueFieldsException;
-import io.onedev.server.exception.InvalidReferenceException;
-import io.onedev.server.exception.IssueLinkValidationException;
 import io.onedev.server.exception.PullRequestReviewRejectedException;
 import io.onedev.server.git.GitUtils;
 import io.onedev.server.git.service.GitService;
@@ -946,12 +943,7 @@ public class McpHelperResource {
     }
 
     private Issue getIssue(Project currentProject, String referenceString) {
-        IssueReference issueReference;
+        var issueReference = IssueReference.of(referenceString, currentProject);
-        try {
-            issueReference = IssueReference.of(referenceString, currentProject);
-        } catch (InvalidReferenceException e) {
-            throw new NotAcceptableException(e.getMessage());
-        }
         var issue = issueManager.find(issueReference.getProject(), issueReference.getNumber());
         if (issue != null) {
             if (!SecurityUtils.canAccessIssue(issue))
@@ -1126,11 +1118,7 @@ public class McpHelperResource {
 
         issue.setFieldValues(FieldUtils.getFieldValues(issue.getProject(), data));
 
-        try {
+        issueManager.open(issue);
-            issueManager.open(issue);
-        } catch (InvalidIssueFieldsException e) {
-            throw newNotAcceptableException(e);
-        }
 
         return "Created issue " + issue.getReference().toString(projectInfo.currentProject) + ": " + urlManager.urlFor(issue, true);
     }
@@ -1201,8 +1189,8 @@ public class McpHelperResource {
 
             try {
                 issueChangeManager.changeFields(issue, FieldUtils.getFieldValues(issue.getProject(), data));
-            } catch (InvalidIssueFieldsException e) {
+            } catch (ValidationException e) {
-                throw newNotAcceptableException(e);
+                throw new NotAcceptableException(e.getMessage());
             }
         }
 
@@ -1238,8 +1226,8 @@ public class McpHelperResource {
         try {
             issueChangeManager.changeState(issue, state, fieldValues, transition.getPromptFields(),
                     transition.getRemoveFields(), comment);
-        } catch (InvalidIssueFieldsException e) {
+        } catch (ValidationException e) {
-            throw newNotAcceptableException(e);
+            throw new NotAcceptableException(e.getMessage());
         }
         var feedback = "Issue " + issueReference + " transited to state \"" + state + "\"";
         var stateDescription = settingManager.getIssueSetting().getStateSpec(state).getDescription();
@@ -1280,11 +1268,7 @@ public class McpHelperResource {
             link.setSource(targetIssue);
             link.setTarget(sourceIssue);
         }
-        try {
+        link.validate();
-            link.validate();
-        } catch (IssueLinkValidationException e) {
-            throw new NotAcceptableException(e.getMessage());
-        }
         issueLinkManager.create(link);
 
         return "Issue " + targetReference + " added as \"" + linkName + "\" of " + sourceReference;
@@ -1762,12 +1746,7 @@ public class McpHelperResource {
     }
 
     private PullRequest getPullRequest(Project currentProject, String referenceString) {
-        PullRequestReference requestReference;        
+        var requestReference = PullRequestReference.of(referenceString, currentProject);
-        try {
-            requestReference = PullRequestReference.of(referenceString, currentProject);
-        } catch (InvalidReferenceException e) {
-            throw new NotAcceptableException(e.getMessage());
-        }
         var request = pullRequestManager.find(requestReference.getProject(), requestReference.getNumber());
         if (request != null) {
             if (!SecurityUtils.canReadCode(request.getProject()))
@@ -2186,12 +2165,7 @@ public class McpHelperResource {
     }
 
     private Build getBuild(Project currentProject, String referenceString) {
-        BuildReference buildReference;        
+        var buildReference = BuildReference.of(referenceString, currentProject);
-        try {
-            buildReference = BuildReference.of(referenceString, currentProject);
-        } catch (InvalidReferenceException e) {
-            throw new NotAcceptableException(e.getMessage());
-        }
         var build = buildManager.find(buildReference.getProject(), buildReference.getNumber());
         if (build != null) {
             if (!SecurityUtils.canAccessBuild(build))
@@ -2209,14 +2183,6 @@ public class McpHelperResource {
         }
     }    
 
-    private NotAcceptableException newNotAcceptableException(InvalidIssueFieldsException e) {
-        var invaliParams = new ArrayList<String>();
-        for (var entry: e.getInvalidFields().entrySet()) {
-            invaliParams.add(getToolParamName(entry.getKey()) + ": " + entry.getValue());
-        }
-        return new NotAcceptableException("Invalid tool parameters:\n\n" + String.join("\n", invaliParams));
-    }
-
     private static class ProjectInfo {
         
         Project project;

server-core/src/main/java/io/onedev/server/web/component/issue/list/BatchEditPanel.java

@@ -21,6 +21,7 @@ import java.util.Set;
 import java.util.stream.Collectors;
 
 import javax.annotation.Nullable;
+import javax.validation.ValidationException;
 
 import org.apache.wicket.AttributeModifier;
 import org.apache.wicket.ajax.AjaxRequestTarget;
@@ -46,7 +47,6 @@ import io.onedev.server.buildspecmodel.inputspec.InputContext;
 import io.onedev.server.buildspecmodel.inputspec.InputSpec;
 import io.onedev.server.entitymanager.IssueChangeManager;
 import io.onedev.server.entitymanager.SettingManager;
-import io.onedev.server.exception.InvalidIssueFieldsException;
 import io.onedev.server.model.Issue;
 import io.onedev.server.model.Iteration;
 import io.onedev.server.model.Project;
@@ -315,7 +315,7 @@ abstract class BatchEditPanel extends Panel implements InputContext {
 						try {
 							OneDev.getInstance(IssueChangeManager.class).batchUpdate(
 									getIssueIterator(), state, confidential, iterations, fieldValues, comment, sendNotifications);
-						} catch (InvalidIssueFieldsException e) {
+						} catch (ValidationException e) {
 							form.error(e.getMessage());
 							target.add(form);
 							return;