Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AB2D-5918] Add Null Attestation Handling #1308

Merged
merged 15 commits into from
Feb 15, 2024
Merged

[AB2D-5918] Add Null Attestation Handling #1308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b3ffd70
add logs to contractserviceimpl
Rwolfe-Nava Jan 17, 2024
640a13a
fixed whitespace issues
Rwolfe-Nava Jan 17, 2024
4d51e11
remove log for attestationDate
Rwolfe-Nava Jan 17, 2024
70880c1
add null check for attestedOn
Rwolfe-Nava Jan 17, 2024
1924a05
revert logs in ContractServiceImpl
Rwolfe-Nava Jan 17, 2024
9c4005b
indent line for eventlogger to match others
Rwolfe-Nava Jan 18, 2024
e6566c3
update event_logger status message for null attestation
Rwolfe-Nava Jan 18, 2024
356ef45
update unit test to handle new logic
Rwolfe-Nava Jan 24, 2024
fc51ba9
some cleanup
Rwolfe-Nava Jan 24, 2024
7416721
temporary sonarqube workaround
Rwolfe-Nava Jan 24, 2024
74c1913
temporary - comment out failing integreation tests
Rwolfe-Nava Jan 24, 2024
d234e6e
Revert "temporary - comment out failing integreation tests"
Rwolfe-Nava Jan 24, 2024
2532f2f
Revert "temporary sonarqube workaround"
Rwolfe-Nava Jan 24, 2024
68479e0
commented out failed integration tests and switched steps in github a…
smirnovaae Jan 25, 2024
b0525b9
Revert "commented out failed integration tests and switched steps in …
Rwolfe-Nava Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions worker/src/main/java/gov/cms/ab2d/worker/processor/JobPreProcessorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static gov.cms.ab2d.eventclient.config.Ab2dEnvironment.PUBLIC_LIST;
import static gov.cms.ab2d.eventclient.events.SlackEvents.EOB_JOB_COVERAGE_ISSUE;
import static gov.cms.ab2d.eventclient.events.SlackEvents.EOB_JOB_STARTED;
import static gov.cms.ab2d.eventclient.events.SlackEvents.EOB_JOB_FAILURE;
import static gov.cms.ab2d.job.model.JobStatus.FAILED;
import static gov.cms.ab2d.job.model.JobStatus.IN_PROGRESS;
import static gov.cms.ab2d.job.model.JobStatus.SUBMITTED;
Expand Down Expand Up @@ -72,6 +73,20 @@ public Job preprocess(String jobUuid) {
if (contract == null) {
throw new IllegalArgumentException("A job must always have a contract.");
}

Rwolfe-Nava marked this conversation as resolved.
Show resolved Hide resolved
if (contract.getAttestedOn() == null) {
log.warn("JobPreProcessorImpl > preprocess: job FAILED because the contract attestation date is null.");

eventLogger.logAndAlert(job.buildJobStatusChangeEvent(FAILED, EOB_JOB_FAILURE + " Job " + jobUuid
+ "failed for contract: " + contract.getContractNumber() + " because contract attestation date is null"), PUBLIC_LIST);

job.setStatus(FAILED);
job.setStatusMessage("failed because contract attestation date is null.");

jobRepository.save(job);
return job;
}

Optional<OffsetDateTime> sinceValue = Optional.ofNullable(job.getSince());
if (sinceValue.isPresent()) {
// If the user provided a 'since' value
Expand Down
42 changes: 33 additions & 9 deletions worker/src/test/java/gov/cms/ab2d/worker/processor/JobPreProcessorUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import gov.cms.ab2d.job.model.JobStatus;
import gov.cms.ab2d.job.repository.JobRepository;
import gov.cms.ab2d.worker.processor.coverage.CoverageDriver;
import gov.cms.ab2d.worker.util.ContractWorkerClientMock;
import gov.cms.ab2d.worker.service.ContractWorkerClient;
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
Expand Down Expand Up @@ -53,8 +53,8 @@ class JobPreProcessorUnitTest {

private static final String JOB_UUID = "6d08bf08-f926-4e19-8d89-ad67ef89f17e";

private final ContractWorkerClientMock contractWorkerClient = new ContractWorkerClientMock();

@Mock
private ContractWorkerClient contractWorkerClient;
@Mock
private JobRepository jobRepository;
@Mock
Expand All @@ -67,7 +67,7 @@ class JobPreProcessorUnitTest {

@BeforeEach
void setUp() {
contract = new ContractDTO(null, "JPP5678", "JPP5678", null, null, 0, 0);
contract = new ContractDTO(null, "JPP5678", "JPP5678", OffsetDateTime.now(), Contract.ContractType.NORMAL, 0, 0);
cut = new JobPreProcessorImpl(contractWorkerClient, jobRepository, eventLogger, coverageDriver);
job = createJob();
}
Expand Down Expand Up @@ -123,6 +123,7 @@ void whenJobIsInSubmittedStatus_ThenJobShouldBePutInProgress() throws Interrupte

when(jobRepository.save(Mockito.any())).thenReturn(job);
when(jobRepository.findByJobUuid(job.getJobUuid())).thenReturn(job);
when(contractWorkerClient.getContractByContractNumber(job.getContractNumber())).thenReturn(contract);
when(coverageDriver.isCoverageAvailable(any(Job.class), any(ContractDTO.class))).thenReturn(true);

var processedJob = cut.preprocess(job.getJobUuid());
Expand All @@ -131,12 +132,30 @@ void whenJobIsInSubmittedStatus_ThenJobShouldBePutInProgress() throws Interrupte
verify(jobRepository).save(Mockito.any());
}

@DisplayName("Job is FAILED if contract is not attested")
@Test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a few line comments about mock tests also would be great for readability in future. Everything else looks great and Great job on finishing this super quickly!

void jobFailedIfContractNotAttested() {

contract.setAttestedOn(null);

job.setContractNumber(contract.getContractNumber());
job.setStatus(JobStatus.SUBMITTED);

when(jobRepository.findByJobUuid(job.getJobUuid())).thenReturn(job);
when(contractWorkerClient.getContractByContractNumber(job.getContractNumber())).thenReturn(contract);

job = cut.preprocess(job.getJobUuid());

assertEquals(JobStatus.FAILED, job.getStatus());
}

@DisplayName("Job is not started if coverage is not available")
@Test
void proccessingNotTriggeredIfCoverageNotAvailable() throws InterruptedException {

job.setStatus(JobStatus.SUBMITTED);
when(jobRepository.findByJobUuid(job.getJobUuid())).thenReturn(job);
when(contractWorkerClient.getContractByContractNumber(job.getContractNumber())).thenReturn(contract);
when(coverageDriver.isCoverageAvailable(any(Job.class), any(ContractDTO.class))).thenReturn(false);

Job result = cut.preprocess(job.getJobUuid());
Expand All @@ -149,6 +168,7 @@ void proccessingNotTriggeredIfCoverageCheckInterrupted() throws InterruptedExcep

job.setStatus(JobStatus.SUBMITTED);
when(jobRepository.findByJobUuid(job.getJobUuid())).thenReturn(job);
when(contractWorkerClient.getContractByContractNumber(job.getContractNumber())).thenReturn(contract);
when(coverageDriver.isCoverageAvailable(any(Job.class), any(ContractDTO.class))).thenThrow(InterruptedException.class);

var exceptionThrown = assertThrows(RuntimeException.class,
Expand All @@ -165,6 +185,7 @@ void testDefaultSinceSTU3() {
job.setStatus(JobStatus.SUBMITTED);
job.setContractNumber(contract.getContractNumber());
when(jobRepository.findByJobUuid(job.getJobUuid())).thenReturn(job);
when(contractWorkerClient.getContractByContractNumber(job.getContractNumber())).thenReturn(contract);
cut.preprocess(job.getJobUuid());
assertNull(job.getSince());
verify(jobRepository, never()).findByContractNumberEqualsAndStatusInAndStartedByOrderByCompletedAtDesc(anyString(), any(), any());
Expand All @@ -178,8 +199,10 @@ void testDefaultSinceR4FirstRun() {
job.setFhirVersion(R4);
job.setStatus(JobStatus.SUBMITTED);
when(jobRepository.findByJobUuid(job.getJobUuid())).thenReturn(job);
when(contractWorkerClient.getContractByContractNumber(job.getContractNumber())).thenReturn(contract);
when(jobRepository.findByContractNumberEqualsAndStatusInAndStartedByOrderByCompletedAtDesc(anyString(), any(), any())).thenReturn(Collections.emptyList());
cut.preprocess(job.getJobUuid());

assertNull(job.getSince());
assertEquals(SinceSource.FIRST_RUN, job.getSinceSource());
}
Expand All @@ -200,6 +223,7 @@ void testDefaultSinceR4FirstRunByPdp() {
oldJob.setCreatedAt(oldJobTime);

when(jobRepository.findByJobUuid(newJob.getJobUuid())).thenReturn(newJob);
when(contractWorkerClient.getContractByContractNumber(newJob.getContractNumber())).thenReturn(contract);
when(jobRepository.findByContractNumberEqualsAndStatusInAndStartedByOrderByCompletedAtDesc(anyString(), any(), any())).thenReturn(List.of(oldJob));

cut.preprocess(newJob.getJobUuid());
Expand All @@ -225,6 +249,7 @@ void testDefaultSinceR4SuppliedSince() {
oldJob.setJobUuid(oldJob.getJobUuid() + "-2");

when(jobRepository.findByJobUuid(newJob.getJobUuid())).thenReturn(newJob);
when(contractWorkerClient.getContractByContractNumber(newJob.getContractNumber())).thenReturn(contract);

cut.preprocess(newJob.getJobUuid());

Expand All @@ -240,24 +265,22 @@ void testDifferentSinceConditions() {
newJob.setStatus(JobStatus.SUBMITTED);
newJob.setCreatedAt(OffsetDateTime.now());

ContractDTO contract = new ContractDTO(null, "contractNum", null, null, Contract.ContractType.CLASSIC_TEST, 0, 0);
newJob.setContractNumber(contract.getContractNumber());

Job oldJob = createJob();
oldJob.setStatus(SUCCESSFUL);
oldJob.setJobUuid(oldJob.getJobUuid() + "-2");
OffsetDateTime oldJobTime = OffsetDateTime.parse("2021-01-01T00:00:00.000-05:00", DateTimeFormatter.ISO_DATE_TIME);
oldJob.setCreatedAt(oldJobTime);

when(jobRepository.findByJobUuid(newJob.getJobUuid())).thenReturn(newJob);
when(contractWorkerClient.getContractByContractNumber(newJob.getContractNumber())).thenReturn(contract);

cut.preprocess(newJob.getJobUuid());
newJob = cut.preprocess(newJob.getJobUuid());

assertNull(newJob.getSince());
// No longer allow null contracts so things get flagged as first run now.
assertEquals(SinceSource.FIRST_RUN, newJob.getSinceSource());

contract = new ContractDTO(null, "contractNum", null, null, Contract.ContractType.SYNTHEA, 0, 0);
contract = new ContractDTO(null, "contractNum", null, OffsetDateTime.now(), Contract.ContractType.SYNTHEA, 0, 0);

when(jobRepository.findByContractNumberEqualsAndStatusInAndStartedByOrderByCompletedAtDesc(anyString(), any(), any())).thenReturn(List.of(oldJob));

Expand Down Expand Up @@ -303,6 +326,7 @@ void testGetLatestFullySuccessfulJob() {
assertTrue(impl.getLastSuccessfulJobWithDownloads(List.of(job3)).isEmpty());

when(jobRepository.findByJobUuid(newJob.getJobUuid())).thenReturn(newJob);
when(contractWorkerClient.getContractByContractNumber(job.getContractNumber())).thenReturn(contract);
when(jobRepository.findByContractNumberEqualsAndStatusInAndStartedByOrderByCompletedAtDesc(anyString(), any(), any())).thenReturn(List.of(job1, job2, job3, job4));
cut.preprocess(newJob.getJobUuid());
assertEquals(newJob.getSince().getNano(), job4.getCreatedAt().getNano());
Expand Down
Loading