TSPS-136 Require a JobControl object containing a job id when creating a new job #52
Conversation
…ks for me locally
| properties: | ||
| description: | ||
| description: >- | ||
| User-provided description of the job request. | ||
| type: string | ||
| jobControl: |
There was a problem hiding this comment.
just curious, what is the benefit of this schema being an object vs a string like pipelineVersion?
There was a problem hiding this comment.
it's the spec. it's an object that contains currently just the "id" field, but in future it could additionally contain things like notification preferences. does this answer your question?
| String errorMessage = error.getDefaultMessage(); | ||
| errors.put(fieldName, errorMessage); | ||
| }); | ||
| String validationErrorMessage = "Request could not be parsed or was invalid: " + errors; |
There was a problem hiding this comment.
i would probaby put at least a \n between each error so it can be a bit easier to read (if only slightly)
| .getAllErrors() | ||
| .forEach( | ||
| error -> { | ||
| String fieldName = ((FieldError) error).getField(); |
There was a problem hiding this comment.
do we have to do the cast here? surprised thats necessary if we know what the exception we're starting with is.
There was a problem hiding this comment.
we seemingly do... .getAllErrors returns a List, and ObjectError doesn't have a .getField method. let me know if I'm missing something though!
There was a problem hiding this comment.
nevermind! there's a .getFieldErrors method!
| @@ -119,17 +120,20 @@ public ResponseEntity<ApiCreateJobResponse> createJob( | |||
| @PathVariable("pipelineId") String pipelineId, @RequestBody ApiCreateJobRequestBody body) { | |||
| final SamUser userRequest = getAuthenticatedInfo(); | |||
| String userId = userRequest.getSubjectId(); | |||
| String jobIdString = body.getJobControl().getId(); | |||
| UUID jobId = validateJobId(jobIdString); | |||
There was a problem hiding this comment.
what do we think abotu using the openapi yaml to validate this for us. something like what is found in https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.1.md#support-for-x-www-form-urlencoded-request-bodies
There was a problem hiding this comment.
nice, I didn't see the "format: uuid" option. that throws a different error that we have to handle differently in the GlobalExceptionHandler. I've implemented it... see what you think.
| logger.info("Create new imputation version {} job for user {}", pipelineVersion, userId); | ||
|
|
||
| JobBuilder jobBuilder = | ||
| jobService | ||
| .newJob() | ||
| .jobId(createJobId()) | ||
| .jobId(jobId) |
There was a problem hiding this comment.
woooo! things are comin together
|
Description
Per DSP's Async API best practices, the caller must provide a job id contained in a JobControl object. This PR implements that requirement and validates that the string the user has provided is a uuid.
Some other minor edits:
MissingRequiredFieldsExceptionclass and use TCL'sMissingRequiredFieldExceptioninsteadJira Ticket
https://broadworkbench.atlassian.net/browse/TSPS-136