Skip to content

Commit

Permalink
[AB2D-6305] - Implement a fix for job cancel polling response (#1395)
Browse files Browse the repository at this point in the history
* initial commit

* fix checkstyle in StatusCommon

* fix instancing errors

* address code smell and add a test

* update tests

* set resourceType on OperationOutcome

* Fetch all git history for sonarqube blame info

* Drop compile from sonar command

* Run package for sonar command

* fix checkstyle

* add sonar properties

* change severity to error based on feedback

---------

Co-authored-by: Sean Fern <[email protected]>
Co-authored-by: Maboh Christopher <[email protected]>
  • Loading branch information
3 people authored Oct 1, 2024
1 parent 5fe4c70 commit 82d2398
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 15 deletions.
10 changes: 6 additions & 4 deletions .github/workflows/unit-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ jobs:
steps:
- name: Checkout Code
uses: actions/checkout@v3
with:
fetch-depth: 0 # Get entire history for SonarQube

- name: Setup Java
uses: actions/setup-java@v3
Expand Down Expand Up @@ -54,10 +56,10 @@ jobs:
run: |
mvn -ntp -U clean
- name: SonarQube analysis
- name: Run unit and integration tests
run: |
mvn -ntp -s settings.xml ${RUNNER_DEBUG:+"--debug"} compile sonar:sonar -Dsonar.projectKey=ab2d-project -Dsonar.qualitygate.wait=true -DskipTests -Dusername=${ARTIFACTORY_USER} -Dpassword=${ARTIFACTORY_PASSWORD} -Drepository_url=${ARTIFACTORY_URL}
mvn -ntp -s settings.xml ${RUNNER_DEBUG:+"--debug"} -Dusername=${ARTIFACTORY_USER} -Dpassword=${ARTIFACTORY_PASSWORD} -Drepository_url=${ARTIFACTORY_URL} test -pl common,job,coverage,api,worker
- name: Run unit and integration tests
- name: SonarQube analysis
run: |
mvn -ntp -s settings.xml ${RUNNER_DEBUG:+"--debug"} -Dusername=${ARTIFACTORY_USER} -Dpassword=${ARTIFACTORY_PASSWORD} -Drepository_url=${ARTIFACTORY_URL} test -pl common,job,coverage,api,worker
mvn -ntp -s settings.xml ${RUNNER_DEBUG:+"--debug"} package sonar:sonar -Dsonar.projectKey=ab2d-project -Dsonar.qualitygate.wait=true -DskipTests -Dusername=${ARTIFACTORY_USER} -Dpassword=${ARTIFACTORY_PASSWORD} -Drepository_url=${ARTIFACTORY_URL}
4 changes: 2 additions & 2 deletions api/src/main/java/gov/cms/ab2d/api/config/OpenAPIConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public OpenApiCustomiser defaultResponseMessages() {
@JsonPropertyOrder({
"text"
})
static class Details {
public static class Details {

@JsonProperty("text")
private String text;
Expand Down Expand Up @@ -177,7 +177,7 @@ public void setAdditionalProperty(String name, Object value) {
"code",
"details"
})
static class Issue {
public static class Issue {
@JsonProperty("severity")
private String severity;
@JsonProperty("code")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package gov.cms.ab2d.api.controller.common;

import gov.cms.ab2d.api.config.OpenAPIConfig;
import gov.cms.ab2d.api.controller.JobCompletedResponse;
import gov.cms.ab2d.api.controller.JobProcessingException;
import gov.cms.ab2d.api.controller.TooManyRequestsException;
Expand All @@ -11,6 +12,7 @@
import gov.cms.ab2d.eventclient.events.ApiResponseEvent;
import gov.cms.ab2d.job.dto.JobPollResult;
import gov.cms.ab2d.job.model.JobOutput;

import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
Expand All @@ -25,14 +27,14 @@
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Service;


import static gov.cms.ab2d.api.controller.common.ApiText.X_PROG;
import static gov.cms.ab2d.common.util.Constants.FHIR_PREFIX;
import static gov.cms.ab2d.common.util.Constants.JOB_LOG;
import static gov.cms.ab2d.common.util.Constants.ORGANIZATION;
import static gov.cms.ab2d.common.util.Constants.REQUEST_ID;
import static org.springframework.http.HttpHeaders.EXPIRES;
import static org.springframework.http.HttpHeaders.RETRY_AFTER;
import static org.springframework.http.MediaType.APPLICATION_JSON;

@Service
@Slf4j
Expand All @@ -41,13 +43,16 @@ public class StatusCommon {
private final JobClient jobClient;
private final SQSEventClient eventLogger;
private final int retryAfterDelay;
private final OpenAPIConfig openApi;

StatusCommon(PdpClientService pdpClientService, JobClient jobClient,
SQSEventClient eventLogger, @Value("${api.retry-after.delay}") int retryAfterDelay) {
this.pdpClientService = pdpClientService;
this.jobClient = jobClient;
this.eventLogger = eventLogger;
this.retryAfterDelay = retryAfterDelay;

this.openApi = new OpenAPIConfig();
}

public void throwFailedResponse(String msg) {
Expand Down Expand Up @@ -80,6 +85,8 @@ public ResponseEntity doStatus(String jobUuid, HttpServletRequest request, Strin
"Job in progress", jobPollResult.getProgress() + "% complete",
(String) request.getAttribute(REQUEST_ID)));
return new ResponseEntity<>(null, responseHeaders, HttpStatus.ACCEPTED);
case CANCELLED:
return getCanceledResponse(jobPollResult, jobUuid, request);
case FAILED:
throwFailedResponse("Job failed while processing");
break;
Expand Down Expand Up @@ -117,6 +124,31 @@ protected JobCompletedResponse getJobCompletedResponse(JobPollResult jobPollResu
return resp;
}

protected ResponseEntity<OpenAPIConfig.OperationOutcome> getCanceledResponse(JobPollResult jobPollResult, String jobUuid, HttpServletRequest request) {
HttpHeaders responseHeaders = new HttpHeaders();
responseHeaders.setContentType(APPLICATION_JSON);

OpenAPIConfig.OperationOutcome outcome = openApi.new OperationOutcome();
outcome.setResourceType("OperationOutcome");

OpenAPIConfig.Details details = new OpenAPIConfig.Details();
details.setText("Job is canceled.");

OpenAPIConfig.Issue issue = new OpenAPIConfig.Issue();
issue.setDetails(details);
issue.setCode("deleted");
issue.setSeverity("error");

List<OpenAPIConfig.Issue> issuesList = new ArrayList<>();
issuesList.add(issue);
outcome.setIssue(issuesList);

eventLogger.sendLogs(new ApiResponseEvent(MDC.get(ORGANIZATION), jobUuid, HttpStatus.NOT_FOUND,
"Job was previously canceled", null, (String) request.getAttribute(REQUEST_ID)));

return new ResponseEntity<OpenAPIConfig.OperationOutcome>(outcome, responseHeaders, HttpStatus.NOT_FOUND);
}

private String getUrlPath(String jobUuid, String filePath, HttpServletRequest request, String apiPrefix) {
return Common.getUrl(apiPrefix + FHIR_PREFIX + "/Job/" + jobUuid + "/file/" + filePath, request);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,43 +67,43 @@ void testTooFrequentInvocations() {
}

@Test
void testDoStatus1() {
void testDoStatusSuccessful() {
when(jobPollResult.getStatus()).thenReturn(JobStatus.SUCCESSFUL);
assertNotNull(
statusCommon.doStatus("1234", req, "prefix")
);
}

@Test
void testDoStatus2() {
void testDoStatusSubmitted() {
when(jobPollResult.getStatus()).thenReturn(JobStatus.SUBMITTED);
assertNotNull(
statusCommon.doStatus("1234", req, "prefix")
);
}

@Test
void testDoStatus3() {
void testDoStatusInProgress() {
when(jobPollResult.getStatus()).thenReturn(JobStatus.IN_PROGRESS);
assertNotNull(
statusCommon.doStatus("1234", req, "prefix")
);
}

@Test
void testDoStatus4() {
void testDoStatusFailed() {
when(jobPollResult.getStatus()).thenReturn(JobStatus.FAILED);
assertThrows(JobProcessingException.class, () -> {
statusCommon.doStatus("1234", req, "prefix");
});
}

@Test
void testDoStatus5() {
void testDoStatusCanceled() {
when(jobPollResult.getStatus()).thenReturn(JobStatus.CANCELLED);
assertThrows(JobProcessingException.class, () -> {
statusCommon.doStatus("1234", req, "prefix");
});
assertNotNull(
statusCommon.doStatus("1234", req, "prefix")
);
}

@Test
Expand All @@ -120,4 +120,11 @@ void testGetJobCompletedResponse() {
);
}

@Test
void testGetJobCanceledResponse() {
assertNotNull(
statusCommon.getCanceledResponse(jobPollResult, "1234", req)
);
}

}

0 comments on commit 82d2398

Please sign in to comment.