Skip to content

Commit 3778b9b

Browse files
Release/12.5 > main (#325)
* Implement idempotency check for spotlight submissions in SpotlightBatchService * Enhance SpotlightBatchService to handle multiple QUEUED batches and prevent duplicate submissions. Updated query to order results by creation date and added logging for better traceability. * Update SpotlightBatchServiceTest to return lists instead of Optionals for batch retrieval methods, enhancing compatibility with multiple QUEUED batches. * Refactor SpotlightBatchServiceTest to prevent re-queuing of GGIS_ERROR submissions, ensuring they require manual intervention.
1 parent ff9697d commit 3778b9b

3 files changed

Lines changed: 81 additions & 16 deletions

File tree

src/main/java/gov/cabinetoffice/gap/adminbackend/repositories/SpotlightBatchRepository.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ Optional<SpotlightBatch> findByStatusAndSpotlightSubmissionsSizeLessThan(
2828

2929
Optional<List<SpotlightBatch>> findByStatus(@Param("status") SpotlightBatchStatus status);
3030

31-
@Query("select s from SpotlightBatch s inner join s.spotlightSubmissions spotlightSubmissions where s.status =:status and spotlightSubmissions.mandatoryQuestions.gapId = :gapId")
32-
Optional<SpotlightBatch> findByStatusAndSpotlightSubmissions_MandatoryQuestions_GapId(
31+
@Query("select s from SpotlightBatch s inner join s.spotlightSubmissions spotlightSubmissions where s.status =:status and spotlightSubmissions.mandatoryQuestions.gapId = :gapId ORDER BY s.created DESC")
32+
List<SpotlightBatch> findByStatusAndSpotlightSubmissions_MandatoryQuestions_GapId(
3333
@Param("status") SpotlightBatchStatus status, @Param("gapId") String gapId);
3434

3535
}

src/main/java/gov/cabinetoffice/gap/adminbackend/services/SpotlightBatchService.java

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,12 @@
4444
import java.time.Instant;
4545
import java.util.ArrayList;
4646
import java.util.Arrays;
47+
import java.util.HashSet;
4748
import java.util.List;
49+
import java.util.Set;
4850
import java.util.UUID;
4951
import java.util.concurrent.atomic.AtomicInteger;
52+
import java.util.stream.Collectors;
5053

5154
import static gov.cabinetoffice.gap.adminbackend.enums.DraftAssessmentResponseDtoStatus.SUCCESS;
5255

@@ -136,11 +139,21 @@ private SpotlightBatch getSpotlightBatch(UUID spotlightBatchId) {
136139
}
137140

138141
public SpotlightBatch getSpotlightBatchWithQueuedStatusByMandatoryQuestionGapId(String gapId) {
139-
return spotlightBatchRepository
140-
.findByStatusAndSpotlightSubmissions_MandatoryQuestions_GapId(SpotlightBatchStatus.QUEUED, gapId)
141-
.orElseThrow(() -> new NotFoundException(
142-
"A spotlight batch with spotlightSubmission for mandatory question with gap id " + gapId
143-
+ " could not be found"));
142+
List<SpotlightBatch> batches = spotlightBatchRepository
143+
.findByStatusAndSpotlightSubmissions_MandatoryQuestions_GapId(SpotlightBatchStatus.QUEUED, gapId);
144+
145+
if (batches.isEmpty()) {
146+
throw new NotFoundException(
147+
"A spotlight batch with spotlightSubmission for mandatory question with gap id " + gapId
148+
+ " could not be found");
149+
}
150+
151+
if (batches.size() > 1) {
152+
log.warn("Found {} QUEUED batches for gapId {}. Using the most recent one.", batches.size(), gapId);
153+
}
154+
155+
// Return the first batch (already ordered by created DESC in the query)
156+
return batches.get(0);
144157
}
145158

146159
public List<SpotlightBatch> getSpotlightBatchesByStatus(SpotlightBatchStatus status) {
@@ -152,10 +165,39 @@ public List<SendToSpotlightDto> generateSendToSpotlightDtosList(SpotlightBatchSt
152165
final List<SendToSpotlightDto> sendToSpotlightDtos = new ArrayList<>();
153166
final List<SpotlightBatch> spotlightBatches = getSpotlightBatchesByStatus(status);
154167

168+
// Track submissions we've already included to prevent duplicates across batches
169+
final Set<UUID> processedSubmissionIds = new HashSet<>();
170+
155171
for (SpotlightBatch spotlightBatch : spotlightBatches) {
156172
try {
173+
final List<SpotlightSubmission> submissions = spotlightBatch.getSpotlightSubmissions();
174+
175+
// Filter: Only non-SENT submissions that haven't been processed yet
176+
final List<SpotlightSubmission> submissionsToProcess = submissions.stream()
177+
.filter(sub -> !SpotlightSubmissionStatus.SENT.toString().equals(sub.getStatus()))
178+
.filter(sub -> !processedSubmissionIds.contains(sub.getId()))
179+
.collect(Collectors.toList());
180+
181+
if (submissionsToProcess.isEmpty()) {
182+
log.info("Batch {} skipped - no eligible submissions (all are SENT or already processed)",
183+
spotlightBatch.getId());
184+
// Mark batch as SUCCESS since all submissions are already SENT
185+
if (submissions.stream().allMatch(s -> SpotlightSubmissionStatus.SENT.toString().equals(s.getStatus()))) {
186+
spotlightBatch.setStatus(SpotlightBatchStatus.SUCCESS);
187+
spotlightBatch.setLastUpdated(Instant.now());
188+
spotlightBatchRepository.save(spotlightBatch);
189+
}
190+
continue;
191+
}
192+
193+
// Mark these submissions as processed to prevent duplicates in other batches
194+
submissionsToProcess.forEach(sub -> processedSubmissionIds.add(sub.getId()));
195+
196+
log.info("Processing batch {} with {} submissions (filtered from {})",
197+
spotlightBatch.getId(), submissionsToProcess.size(), submissions.size());
198+
157199
final List<SpotlightSchemeDto> schemes = new ArrayList<>();
158-
addSpotlightSchemeDtoToList(spotlightBatch, schemes);
200+
addSpotlightSchemeDtoToListFiltered(spotlightBatch, schemes, submissionsToProcess);
159201

160202
final SendToSpotlightDto sendToSpotlightDto = SendToSpotlightDto.builder().schemes(schemes).build();
161203
sendToSpotlightDtos.add(sendToSpotlightDto);
@@ -169,6 +211,21 @@ public List<SendToSpotlightDto> generateSendToSpotlightDtosList(SpotlightBatchSt
169211
return sendToSpotlightDtos;
170212
}
171213

214+
/**
215+
* Adds spotlight scheme DTOs to the list using a pre-filtered list of submissions.
216+
* This prevents duplicate submissions being sent to Spotlight.
217+
*/
218+
private void addSpotlightSchemeDtoToListFiltered(SpotlightBatch spotlightBatch,
219+
List<SpotlightSchemeDto> schemes, List<SpotlightSubmission> submissionsToProcess) {
220+
221+
final List<String> uniqueSchemeIds = getUniqueSchemeIds(submissionsToProcess);
222+
log.info("uniqueSchemeIds for batch {}: {}", spotlightBatch.getId(), uniqueSchemeIds);
223+
224+
uniqueSchemeIds.stream()
225+
.map(uniqueSchemeId -> generateSchemeDto(uniqueSchemeId, submissionsToProcess))
226+
.forEach(schemes::add);
227+
}
228+
172229
public List<String> getUniqueSchemeIds(List<SpotlightSubmission> spotlightSubmissions) {
173230
return spotlightSubmissions.stream()
174231
.map(submission -> submission.getMandatoryQuestions().getSchemeEntity().getGgisIdentifier()).distinct()
@@ -294,16 +351,21 @@ private void handleError(SpotlightSubmission spotlightSubmission,
294351
private void handleErrorMessage(SpotlightSubmission spotlightSubmission, String errorMessage) {
295352
if (errorMessage.contains(RESPONSE_MESSAGE_406_SCHEME_NOT_EXIST)) {
296353
spotlightSubmission.setStatus(SpotlightSubmissionStatus.GGIS_ERROR.toString());
297-
sendMessageToQueue(spotlightSubmission);
354+
// DON'T re-queue GGIS errors - they require manual intervention to fix the scheme ID
355+
// Re-queuing these creates an infinite loop of failures
356+
log.warn("GGIS_ERROR for submission {} - scheme doesn't exist in Spotlight. " +
357+
"Manual intervention required to fix the GGIS identifier.", spotlightSubmission.getId());
298358
}
299359
else if (errorMessage.contains(RESPONSE_MESSAGE_409_FIELD_MISSING)
300360
|| errorMessage.contains(RESPONSE_MESSAGE_409_LENGTH)) {
301-
// has a validation error
361+
// has a validation error - DON'T re-queue as these need manual data fixes
302362
spotlightSubmission.setStatus(SpotlightSubmissionStatus.VALIDATION_ERROR.toString());
303363

304364
log.info("Sending Spotlight validation support email using SNS for status code: 409");
305365
final String snsResponse = snsService.spotlightValidationError();
306366
log.info(snsResponse);
367+
log.warn("VALIDATION_ERROR for submission {} - {}. Manual intervention required.",
368+
spotlightSubmission.getId(), errorMessage);
307369
}
308370
}
309371

@@ -327,8 +389,11 @@ public SpotlightResponseResultsDto sendBatchToSpotlight(SendToSpotlightDto spotl
327389

328390
final HttpEntity<String> requestEntity = new HttpEntity<>(spotlightBatchAsJsonString, requestHeaders);
329391

330-
final String draftAssessmentsEndpoint = spotlightConfig.getSpotlightUrl()
331-
+ "/services/apexrest/DraftAssessments";
392+
// Construct the endpoint URL - avoid duplicating the path if already in config
393+
String baseUrl = spotlightConfig.getSpotlightUrl();
394+
final String draftAssessmentsEndpoint = baseUrl.contains("/services/apexrest/DraftAssessments")
395+
? baseUrl
396+
: baseUrl + "/services/apexrest/DraftAssessments";
332397

333398
SpotlightResponseResultsDto list = SpotlightResponseResultsDto.builder().build();
334399

src/test/java/gov/cabinetoffice/gap/adminbackend/services/SpotlightBatchServiceTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ void getSpotlightBatchByMandatoryQuestionGapId_success() {
520520
final SpotlightBatch spotlightBatch = SpotlightBatch.builder().id(uuid).build();
521521

522522
when(spotlightBatchRepository.findByStatusAndSpotlightSubmissions_MandatoryQuestions_GapId(any(), any()))
523-
.thenReturn(Optional.of(spotlightBatch));
523+
.thenReturn(List.of(spotlightBatch));
524524

525525
final SpotlightBatch result = spotlightBatchService
526526
.getSpotlightBatchWithQueuedStatusByMandatoryQuestionGapId("GAP123");
@@ -531,7 +531,7 @@ void getSpotlightBatchByMandatoryQuestionGapId_success() {
531531
@Test
532532
void getSpotlightBatchByMandatoryQuestionGapId_notFound() {
533533
when(spotlightBatchRepository.findByStatusAndSpotlightSubmissions_MandatoryQuestions_GapId(any(), any()))
534-
.thenReturn(Optional.empty());
534+
.thenReturn(List.of());
535535

536536
assertThrows(NotFoundException.class,
537537
() -> spotlightBatchService.getSpotlightBatchWithQueuedStatusByMandatoryQuestionGapId("GAP123"));
@@ -722,7 +722,6 @@ void spotlightResponsesResultIsNotNullAndDraftAssessmentStatusIsFailureAndMessag
722722

723723
doNothing().when(spotlightBatchService).updateSpotlightBatchStatus(sendToSpotlightDto,
724724
SpotlightBatchStatus.FAILURE);
725-
doNothing().when(spotlightBatchService).sendMessageToQueue(spotlightSubmission);
726725

727726
spotlightBatchService.processSpotlightResponse(sendToSpotlightDto, spotlightResponseResults);
728727

@@ -733,7 +732,8 @@ void spotlightResponsesResultIsNotNullAndDraftAssessmentStatusIsFailureAndMessag
733732

734733
verify(spotlightBatchService, times(1)).updateSpotlightBatchStatus(sendToSpotlightDto,
735734
SpotlightBatchStatus.FAILURE);
736-
verify(spotlightBatchService, times(1)).sendMessageToQueue(spotlightSubmission);
735+
// GGIS_ERROR submissions should NOT be re-queued - they require manual intervention
736+
verify(spotlightBatchService, never()).sendMessageToQueue(any());
737737
}
738738

739739
@Test

0 commit comments

Comments
 (0)