From abf16e4bf0c8f37e159cd1ad8707819f6a1e4fc0 Mon Sep 17 00:00:00 2001 From: Julius Krah Date: Wed, 11 Oct 2017 20:25:26 +0000 Subject: [PATCH] Added exception handling --- pom.xml | 28 +++--- .../quartz/model/JobDescriptor.java | 20 ++-- .../quartz/model/TriggerDescriptor.java | 3 +- .../quartz/service/AbstractJobService.java | 4 + .../quartz/service/EmailService.java | 17 ++-- .../quartz/web/rest/EmailResource.java | 2 +- .../quartz/web/rest/errors/ErrorVO.java | 20 ++++ .../web/rest/errors/ExceptionTranslator.java | 95 +++++++++++++++++++ .../quartz/web/rest/errors/FieldErrorVO.java | 19 ++++ 9 files changed, 176 insertions(+), 32 deletions(-) create mode 100644 src/main/java/com/juliuskrah/quartz/web/rest/errors/ErrorVO.java create mode 100644 src/main/java/com/juliuskrah/quartz/web/rest/errors/ExceptionTranslator.java create mode 100644 src/main/java/com/juliuskrah/quartz/web/rest/errors/FieldErrorVO.java diff --git a/pom.xml b/pom.xml index 53d8230..b9fb890 100644 --- a/pom.xml +++ b/pom.xml @@ -92,6 +92,12 @@ spring-boot-configuration-processor true + + org.immutables + value + 2.5.6 + provided + org.springframework.boot spring-boot-starter-test @@ -132,23 +138,13 @@ org.springframework.boot spring-boot-maven-plugin - + - + maven-surefire-plugin diff --git a/src/main/java/com/juliuskrah/quartz/model/JobDescriptor.java b/src/main/java/com/juliuskrah/quartz/model/JobDescriptor.java index c0903b2..7c79920 100644 --- a/src/main/java/com/juliuskrah/quartz/model/JobDescriptor.java +++ b/src/main/java/com/juliuskrah/quartz/model/JobDescriptor.java @@ -22,6 +22,8 @@ import java.util.Map; import java.util.Set; +import javax.validation.Valid; + import org.hibernate.validator.constraints.NotBlank; import org.hibernate.validator.constraints.NotEmpty; import static org.quartz.JobBuilder.*; @@ -56,6 +58,7 @@ public class JobDescriptor { private List cc; private List bcc; private Map data = new LinkedHashMap<>(); + @Valid @JsonProperty("triggers") private List triggerDescriptors = new ArrayList<>(); @@ -124,16 +127,17 @@ public Set buildTriggers() { * * @return the JobDetail built from this descriptor */ + @JsonIgnore public JobDetail buildJobDetail() { // @formatter:off - JobDataMap jobDataMap = new JobDataMap(getData()); + JobDataMap jobDataMap = new JobDataMap(data); jobDataMap.put("subject", subject); jobDataMap.put("messageBody", messageBody); jobDataMap.put("to", to); jobDataMap.put("cc", cc); jobDataMap.put("bcc", bcc); return newJob(EmailJob.class) - .withIdentity(getName(), getGroup()) + .withIdentity(name, group) .usingJobData(jobDataMap) .build(); // @formatter:on @@ -160,12 +164,12 @@ public static JobDescriptor buildDescriptor(JobDetail jobDetail, List)jobDetail.getJobDataMap().get("to")) - .setCc((List)jobDetail.getJobDataMap().get("cc")) - .setBcc((List)jobDetail.getJobDataMap().get("bcc")) - // .setData(jobDetail.getJobDataMap().getWrappedMap()) + .setSubject((String) jobDetail.getJobDataMap().remove("subject")) + .setMessageBody((String) jobDetail.getJobDataMap().remove("messageBody")) + .setTo((List)jobDetail.getJobDataMap().remove("to")) + .setCc((List)jobDetail.getJobDataMap().remove("cc")) + .setBcc((List)jobDetail.getJobDataMap().remove("bcc")) + .setData(jobDetail.getJobDataMap().getWrappedMap()) .setTriggerDescriptors(triggerDescriptors); // @formatter:on } diff --git a/src/main/java/com/juliuskrah/quartz/model/TriggerDescriptor.java b/src/main/java/com/juliuskrah/quartz/model/TriggerDescriptor.java index 33d5e5b..1eabdc0 100644 --- a/src/main/java/com/juliuskrah/quartz/model/TriggerDescriptor.java +++ b/src/main/java/com/juliuskrah/quartz/model/TriggerDescriptor.java @@ -43,6 +43,7 @@ public class TriggerDescriptor { @NotBlank private String name; + @NotBlank private String group; private LocalDateTime fireTime; private String cron; @@ -80,7 +81,7 @@ public Trigger buildTrigger() { // @formatter:off if (!isEmpty(cron)) { if (!isValidExpression(cron)) - throw new IllegalArgumentException("Provided expression " + cron + " is not a valid cron expression"); + throw new IllegalArgumentException("Provided expression '" + cron + "' is not a valid cron expression"); return newTrigger() .withIdentity(buildName(), group) .withSchedule(cronSchedule(cron) diff --git a/src/main/java/com/juliuskrah/quartz/service/AbstractJobService.java b/src/main/java/com/juliuskrah/quartz/service/AbstractJobService.java index 1c7b729..95b1634 100644 --- a/src/main/java/com/juliuskrah/quartz/service/AbstractJobService.java +++ b/src/main/java/com/juliuskrah/quartz/service/AbstractJobService.java @@ -62,6 +62,7 @@ public Optional findJob(String group, String name) { scheduler.getTriggersOfJob(jobKey(name, group)))); } catch (SchedulerException e) { log.error("Could not find job with key - {}.{} due to error - {}", group, name, e.getLocalizedMessage()); + throw new RuntimeException(e.getLocalizedMessage()); } // @formatter:on log.warn("Could not find job with key - {}.{}", group, name); @@ -84,6 +85,7 @@ public void deleteJob(String group, String name) { log.info("Deleted job with key - {}.{}", group, name); } catch (SchedulerException e) { log.error("Could not delete job with key - {}.{} due to error - {}", group, name, e.getLocalizedMessage()); + throw new RuntimeException(e.getLocalizedMessage()); } } @@ -97,6 +99,7 @@ public void pauseJob(String group, String name) { log.info("Paused job with key - {}.{}", group, name); } catch (SchedulerException e) { log.error("Could not pause job with key - {}.{} due to error - {}", group, name, e.getLocalizedMessage()); + throw new RuntimeException(e.getLocalizedMessage()); } } @@ -110,6 +113,7 @@ public void resumeJob(String group, String name) { log.info("Resumed job with key - {}.{}", group, name); } catch (SchedulerException e) { log.error("Could not resume job with key - {}.{} due to error - {}", group, name, e.getLocalizedMessage()); + throw new RuntimeException(e.getLocalizedMessage()); } } } diff --git a/src/main/java/com/juliuskrah/quartz/service/EmailService.java b/src/main/java/com/juliuskrah/quartz/service/EmailService.java index b437aa0..39daeff 100644 --- a/src/main/java/com/juliuskrah/quartz/service/EmailService.java +++ b/src/main/java/com/juliuskrah/quartz/service/EmailService.java @@ -26,6 +26,7 @@ import org.quartz.Scheduler; import org.quartz.SchedulerException; import org.quartz.Trigger; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -53,16 +54,19 @@ public EmailService(Scheduler scheduler) { */ @Override public JobDescriptor createJob(String group, JobDescriptor descriptor) { - descriptor.setGroup(group); - JobDetail jobDetail = descriptor.buildJobDetail(); - Set triggersForJob = descriptor.buildTriggers(); - log.info("About to save job with key - {}", jobDetail.getKey()); + String name = descriptor.getName(); try { + if (scheduler.checkExists(jobKey(name, group))) + throw new DataIntegrityViolationException("Job with Key '" + group + "." + name +"' already exists"); + descriptor.setGroup(group); + JobDetail jobDetail = descriptor.buildJobDetail(); + Set triggersForJob = descriptor.buildTriggers(); + log.info("About to save job with key - {}", jobDetail.getKey()); scheduler.scheduleJob(jobDetail, triggersForJob, false); log.info("Job with key - {} saved sucessfully", jobDetail.getKey()); } catch (SchedulerException e) { - log.error("Could not save job with key - {} due to error - {}", jobDetail.getKey(), e.getLocalizedMessage()); - throw new IllegalArgumentException(e.getLocalizedMessage()); + log.error("Could not save job with key - {}.{} due to error - {}", group, name, e.getLocalizedMessage()); + throw new RuntimeException(e.getLocalizedMessage()); } return descriptor; } @@ -90,6 +94,7 @@ public void updateJob(String group, String name, JobDescriptor descriptor) { log.warn("Could not find job with key - {}.{} to update", group, name); } catch (SchedulerException e) { log.error("Could not find job with key - {}.{} to update due to error - {}", group, name, e.getLocalizedMessage()); + throw new RuntimeException(e.getLocalizedMessage()); } } diff --git a/src/main/java/com/juliuskrah/quartz/web/rest/EmailResource.java b/src/main/java/com/juliuskrah/quartz/web/rest/EmailResource.java index 4e43117..e88e25f 100644 --- a/src/main/java/com/juliuskrah/quartz/web/rest/EmailResource.java +++ b/src/main/java/com/juliuskrah/quartz/web/rest/EmailResource.java @@ -74,7 +74,7 @@ public ResponseEntity findJob(@PathVariable String group, @PathVa * @return */ @PutMapping(path = "/groups/{group}/jobs/{name}") - public ResponseEntity updateJob(@PathVariable String group, @PathVariable String name, @RequestBody JobDescriptor descriptor) { + public ResponseEntity updateJob(@PathVariable String group, @PathVariable String name, @Valid @RequestBody JobDescriptor descriptor) { jobService.updateJob(group, name, descriptor); return ResponseEntity.noContent().build(); } diff --git a/src/main/java/com/juliuskrah/quartz/web/rest/errors/ErrorVO.java b/src/main/java/com/juliuskrah/quartz/web/rest/errors/ErrorVO.java new file mode 100644 index 0000000..4ee08bd --- /dev/null +++ b/src/main/java/com/juliuskrah/quartz/web/rest/errors/ErrorVO.java @@ -0,0 +1,20 @@ +package com.juliuskrah.quartz.web.rest.errors; + +import java.io.Serializable; +import java.util.List; + +import org.immutables.value.Value; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + +@Value.Immutable +@JsonSerialize(as = ImmutableErrorVO.class) +@JsonDeserialize(as = ImmutableErrorVO.class) +public interface ErrorVO extends Serializable { + String message(); + + String description(); + + List fieldErrors(); +} diff --git a/src/main/java/com/juliuskrah/quartz/web/rest/errors/ExceptionTranslator.java b/src/main/java/com/juliuskrah/quartz/web/rest/errors/ExceptionTranslator.java new file mode 100644 index 0000000..9e324c9 --- /dev/null +++ b/src/main/java/com/juliuskrah/quartz/web/rest/errors/ExceptionTranslator.java @@ -0,0 +1,95 @@ +package com.juliuskrah.quartz.web.rest.errors; + +import static org.springframework.http.HttpStatus.BAD_REQUEST; +import static org.springframework.http.HttpStatus.CONFLICT; +import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; + +import java.util.List; + +import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.http.ResponseEntity; +import org.springframework.validation.BindingResult; +import org.springframework.validation.FieldError; +import org.springframework.web.bind.MethodArgumentNotValidException; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestControllerAdvice; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@RestControllerAdvice +public class ExceptionTranslator { + + @ExceptionHandler(IllegalStateException.class) + @ResponseStatus(BAD_REQUEST) + public ErrorVO processUnsupportedTriggerError(IllegalStateException ex) { + ErrorVO dto = ImmutableErrorVO.builder() + .message("400: Bad Request") + .description(ex.getMessage()) + .build(); + return dto; + } + + @ExceptionHandler(IllegalArgumentException.class) + @ResponseStatus(BAD_REQUEST) + public ErrorVO processInvalidCronExpressionError(IllegalArgumentException ex) { + ErrorVO dto = ImmutableErrorVO.builder() + .message("400: Bad Request") + .description(ex.getMessage()) + .build(); + return dto; + } + + @ExceptionHandler(MethodArgumentNotValidException.class) + @ResponseStatus(BAD_REQUEST) + public ErrorVO processValidationError(MethodArgumentNotValidException ex) { + BindingResult result = ex.getBindingResult(); + List fieldErrors = result.getFieldErrors(); + ImmutableErrorVO.Builder builder = ImmutableErrorVO.builder() + .message("400: Bad Request") + .description("Validation errors exist"); + + for (FieldError fieldError : fieldErrors) { + builder.addFieldErrors(ImmutableFieldErrorVO.builder() + .objectName(fieldError.getObjectName()) + .field(fieldError.getField()) + .message(fieldError.getCode()) + .build()); + } + return builder.build(); + } + + @ExceptionHandler(DataIntegrityViolationException.class) + @ResponseStatus(CONFLICT) + public ErrorVO processDataIntegrityViolationError(DataIntegrityViolationException ex) { + ErrorVO dto = ImmutableErrorVO.builder() + .message("409: Conflict") + .description(ex.getMessage()) + .build(); + return dto; + } + + @ExceptionHandler(Exception.class) + public ResponseEntity processException(Exception ex) throws Exception { + if (log.isDebugEnabled()) { + log.debug("An unexpected error occurred: {}", ex.getMessage(), ex); + } else { + log.error("An unexpected error occurred: {}", ex.getMessage()); + } + // If the exception is annotated with @ResponseStatus rethrow it and let + // the framework handle it + // AnnotationUtils is a Spring Framework utility class. + if (AnnotationUtils.findAnnotation + (ex.getClass(), ResponseStatus.class) != null) + throw ex; + + ErrorVO dto = ImmutableErrorVO.builder() + .message("500: Internal server error") + .description("Internal server error has occurred") + .build(); + return ResponseEntity.status(INTERNAL_SERVER_ERROR).body(dto); + } + +} diff --git a/src/main/java/com/juliuskrah/quartz/web/rest/errors/FieldErrorVO.java b/src/main/java/com/juliuskrah/quartz/web/rest/errors/FieldErrorVO.java new file mode 100644 index 0000000..c9fc529 --- /dev/null +++ b/src/main/java/com/juliuskrah/quartz/web/rest/errors/FieldErrorVO.java @@ -0,0 +1,19 @@ +package com.juliuskrah.quartz.web.rest.errors; + +import java.io.Serializable; + +import org.immutables.value.Value; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; + +@Value.Immutable +@JsonSerialize(as = ImmutableFieldErrorVO.class) +@JsonDeserialize(as = ImmutableFieldErrorVO.class) +public interface FieldErrorVO extends Serializable { + String objectName(); + + String field(); + + String message(); +}