From 318d11c2bd53026cd4ed90b8f7d30a41cfc1ad45 Mon Sep 17 00:00:00 2001 From: Gertjan van Oosten Date: Mon, 6 Jan 2025 16:15:17 +0100 Subject: [PATCH] AB#1011 Clean up after MartReportImporterWorkerTest Delete imported reports, thus fixing MartImportedReportsResourceTest. Properly report MART import errors as a list. --- insertBaseData-psql.sql | 2 +- .../anet/database/MartImportedReportDao.java | 20 ++++ .../mart/MartReportImporterWorker.java | 70 +++++++------ src/test/java/mil/dds/anet/test/TestData.java | 30 +++++- .../mart/MartReportImporterWorkerTest.java | 97 +++++++++++++------ 5 files changed, 155 insertions(+), 64 deletions(-) diff --git a/insertBaseData-psql.sql b/insertBaseData-psql.sql index 3441b7231e..2760d7f463 100644 --- a/insertBaseData-psql.sql +++ b/insertBaseData-psql.sql @@ -1402,7 +1402,7 @@ INSERT INTO "entityAvatars" ("relatedObjectType", "relatedObjectUuid", "attachme -- Add mart imported reports for testing INSERT INTO "martImportedReports" ("personUuid", "reportUuid", "success", "createdAt", "errors") VALUES - ('87fdbc6a-3109-4e11-9702-a894d6ca31ef', '59be259b-30b9-4d04-9e21-e8ceb58cbe9c', TRUE, CURRENT_TIMESTAMP, 'some error'); + ('87fdbc6a-3109-4e11-9702-a894d6ca31ef', '59be259b-30b9-4d04-9e21-e8ceb58cbe9c', TRUE, CURRENT_TIMESTAMP, NULL); -- Update the link-text indexes REFRESH MATERIALIZED VIEW CONCURRENTLY "mv_lts_attachments"; diff --git a/src/main/java/mil/dds/anet/database/MartImportedReportDao.java b/src/main/java/mil/dds/anet/database/MartImportedReportDao.java index 6e52a75c9d..bbd33ad0f7 100644 --- a/src/main/java/mil/dds/anet/database/MartImportedReportDao.java +++ b/src/main/java/mil/dds/anet/database/MartImportedReportDao.java @@ -69,4 +69,24 @@ public int insert(final MartImportedReport martImportedReport) { } } + @Transactional + public int delete(final MartImportedReport martImportedReport) { + final Handle handle = getDbHandle(); + try { + final StringBuilder sql = + new StringBuilder("/* deleteMartImportedReport */ DELETE FROM \"martImportedReports\" " + + "WHERE success = :success AND \"createdAt\" = :createdAt"); + sql.append(martImportedReport.getPersonUuid() == null ? " AND \"personUuid\" IS NULL" + : " AND \"personUuid\" = :personUuid"); + sql.append(martImportedReport.getReportUuid() == null ? " AND \"reportUuid\" IS NULL" + : " AND \"reportUuid\" = :reportUuid"); + sql.append( + martImportedReport.getErrors() == null ? " AND errors IS NULL" : " AND errors = :errors"); + return handle.createUpdate(sql).bindBean(martImportedReport) + .bind("createdAt", DaoUtils.asLocalDateTime(martImportedReport.getCreatedAt())).execute(); + } finally { + closeDbHandle(handle); + } + } + } diff --git a/src/main/java/mil/dds/anet/threads/mart/MartReportImporterWorker.java b/src/main/java/mil/dds/anet/threads/mart/MartReportImporterWorker.java index a254e56972..c171da74f4 100644 --- a/src/main/java/mil/dds/anet/threads/mart/MartReportImporterWorker.java +++ b/src/main/java/mil/dds/anet/threads/mart/MartReportImporterWorker.java @@ -108,13 +108,14 @@ protected void runInternal(Instant now, JobHistory jobHistory, GraphQLContext co private void processEmailMessage(EmailMessage email) { final MartImportedReport martImportedReport = new MartImportedReport(); - final StringBuilder errors = new StringBuilder(); + ReportDto reportDto = null; + final List errors = new ArrayList<>(); try { email.load(); logger.debug("Processing e-mail sent on: {}", email.getDateTimeCreated()); // Get the report JSON - final Report anetReport = - processReportInfo(getReportInfo(email.getBody().toString()), errors); + reportDto = getReportInfo(email.getBody().toString()); + final Report anetReport = processReportInfo(reportDto, errors); if (anetReport != null) { processAttachments(email, anetReport, errors); martImportedReport.setSuccess(true); @@ -122,14 +123,21 @@ private void processEmailMessage(EmailMessage email) { martImportedReport.setPerson(anetReport.getReportPeople().get(0)); } } catch (Exception e) { - errors.append("Could not process email").append(email.toString()); + errors.add(String.format("Could not process email %s", email)); + } + if (!errors.isEmpty()) { + final StringBuilder errorMsg = new StringBuilder(); + if (reportDto != null) { + errorMsg.append(String.format("While importing report %s:", reportDto.getUuid())); + } + errorMsg.append(String.format("", String.join("
  • ", errors))); + martImportedReport.setErrors(errorMsg.toString()); } - martImportedReport.setErrors(errors.toString()); martImportedReport.setCreatedAt(Instant.now()); martImportedReportDao.insert(martImportedReport); } - private void processAttachments(EmailMessage email, Report anetReport, StringBuilder errors) { + private void processAttachments(EmailMessage email, Report anetReport, List errors) { try { for (final microsoft.exchange.webservices.data.property.complex.Attachment attachment : email .getAttachments()) { @@ -152,26 +160,27 @@ private void processAttachments(EmailMessage email, Report anetReport, StringBui attachmentDao.saveContentBlob(anetAttachment.getUuid(), TikaInputStream.get(fileAttachment.getContent())); } else { - errors.append("Attachment found in e-mail is not valid: ") - .append(fileAttachment.getName()); + errors.add(String.format("Attachment found in e-mail is not valid: %s", + fileAttachment.getName())); } } } } catch (Exception e) { - errors.append("Could not process report attachments"); + errors.add("Could not process report attachments"); } } - private void getTasks(Map martTasks, List tasks, StringBuilder errors) { - martTasks.keySet().forEach(martTask -> { - final Task task = taskDao.getByUuid(martTask); + private List getTasks(Map martTasks, List errors) { + final List tasks = new ArrayList<>(); + martTasks.forEach((key, value) -> { + final Task task = taskDao.getByUuid(key); if (task != null) { tasks.add(task); } else { - errors.append("Can not find task: '").append(martTasks.get(martTask)) - .append("' with uuid: ").append(martTask).append("
    "); + errors.add(String.format("Can not find task: '%s' with uuid: %s", value, key)); } }); + return tasks; } private boolean assertAllowedMimeType(final String mimeType) { @@ -199,11 +208,11 @@ private ReportDto getReportInfo(String reportJson) { return report; } - private Report processReportInfo(ReportDto martReport, StringBuilder errors) { + private Report processReportInfo(ReportDto martReport, List errors) { // Do we have this report already? if (reportDao.getByUuid(martReport.getUuid()) != null) { logger.info("Report with UUID={} already exists", martReport.getUuid()); - errors.append("Report with UUID already exists: ").append(martReport.getUuid()); + errors.add(String.format("Report with UUID already exists: %s", martReport.getUuid())); return null; } @@ -211,21 +220,22 @@ private Report processReportInfo(ReportDto martReport, StringBuilder errors) { final List matchingPersons = personDao.findByEmailAddress(martReport.getEmail()); final List reportPeople = new ArrayList<>(); - // Validate author organization, if not valid finish + // Validate author organization final Organization organization = organizationDao.getByUuid(martReport.getOrganizationUuid()); if (organization == null) { - errors.append("Can not find submitter organization: '") - .append(martReport.getOrganizationName()).append("' with uuid: ") - .append(martReport.getOrganizationUuid()); - return null; + errors.add(String.format("Can not find submitter organization: '%s' with uuid: %s", + martReport.getOrganizationName(), martReport.getOrganizationUuid())); } - // Validate report location, it not valid finish + // Validate report location final Location location = locationDao.getByUuid(martReport.getLocationUuid()); if (location == null) { - errors.append("Can not find report location: ").append(martReport.getLocationName()); - .append("' with uuid: ") - .append(martReport.getLocationUuid()); + errors.add(String.format("Can not find report location: '%s' with uuid: %s", + martReport.getLocationName(), martReport.getLocationUuid())); + } + + // Return early if there are errors + if (!errors.isEmpty()) { return null; } @@ -285,8 +295,7 @@ private Report processReportInfo(ReportDto martReport, StringBuilder errors) { anetReport.setAdvisorOrg(organization); // Report tasks - final List tasks = new ArrayList<>(); - getTasks(martReport.getTasks(), tasks, errors); + final List tasks = getTasks(martReport.getTasks(), errors); anetReport.setTasks(tasks); // Custom fields anetReport.setCustomFields(martReport.getCustomFields()); @@ -298,7 +307,7 @@ private Report processReportInfo(ReportDto martReport, StringBuilder errors) { anetReport = reportDao.insertWithExistingUuid(anetReport); } catch (Exception e) { logger.info("Report with UUID={} already exists", martReport.getUuid()); - errors.append("Report with UUID already exists: ").append(martReport.getUuid()); + errors.add(String.format("Report with UUID already exists: %s", martReport.getUuid())); return null; } @@ -308,13 +317,12 @@ private Report processReportInfo(ReportDto martReport, StringBuilder errors) { return anetReport; } - private void getPersonCountry(Person person, ReportDto martReport, StringBuilder errors) { + private void getPersonCountry(Person person, ReportDto martReport, List errors) { final LocationSearchQuery searchQuery = new LocationSearchQuery(); searchQuery.setText(martReport.getCountry()); final List countries = locationDao.search(searchQuery).getList(); if (countries.isEmpty()) { - errors.append("Can not find submitter country '").append(martReport.getCountry()) - .append("
    "); + errors.add(String.format("Can not find submitter country '%s'", martReport.getCountry())); } else { person.setCountry(countries.get(0)); } diff --git a/src/test/java/mil/dds/anet/test/TestData.java b/src/test/java/mil/dds/anet/test/TestData.java index a9d6ecf4f1..ad30dd19d6 100644 --- a/src/test/java/mil/dds/anet/test/TestData.java +++ b/src/test/java/mil/dds/anet/test/TestData.java @@ -97,7 +97,7 @@ public static TaskInput createTaskInput(String shortName, String longName, Strin public static ReportDto createGoodMartReport() { final ReportDto reportDto = new ReportDto(); // User Info - reportDto.setUuid(UUID.randomUUID().toString()); + reportDto.setUuid("231196f5-3b13-45ea-9d73-524d042b16e7"); reportDto.setOrganizationUuid("9a35caa7-a095-4963-ac7b-b784fde4d583"); reportDto.setOrganizationName("Planning Programming, Budgeting and Execution"); reportDto.setRank("OF-6"); @@ -111,6 +111,7 @@ public static ReportDto createGoodMartReport() { reportDto.setReportText("Report Text"); reportDto.setEngagementDate(Instant.now()); reportDto.setLocationUuid("0855fb0a-995e-4a79-a132-4024ee2983ff"); + reportDto.setLocationName("General Hospital"); reportDto.setCountry("British"); reportDto.setPositionName("MART Team Member"); reportDto.setSubmittedAt(Instant.now()); @@ -122,24 +123,49 @@ public static ReportDto createGoodMartReport() { // Tasks final Map tasks = new HashMap<>(); tasks.put("19364d81-3203-483d-a6bf-461d58888c76", "Intelligence"); - tasks.put("does not exist", "does not exist"); reportDto.setTasks(tasks); return reportDto; } + public static ReportDto createGoodMartReportWithUnknownTask() { + final ReportDto reportDto = createGoodMartReport(); + reportDto.setUuid("34faac7c-8c85-4dec-8e9f-57d9254b5ae2"); + reportDto.getTasks().put("does not exist", "does not exist"); + return reportDto; + } + public static ReportDto createMartReportWrongOrganization() { final ReportDto reportDto = new ReportDto(); + reportDto.setUuid("fb875171-2501-46c9-9246-60dafabb656d"); reportDto.setOrganizationUuid("does not exist"); reportDto.setOrganizationName("does not exist"); + reportDto.setLocationUuid("0855fb0a-995e-4a79-a132-4024ee2983ff"); + reportDto.setLocationName("General Hospital"); return reportDto; } public static ReportDto createMartReportWrongLocation() { final ReportDto reportDto = new ReportDto(); + reportDto.setUuid("2d6c7a19-d878-4792-bdaf-7a73dc3bfc83"); reportDto.setOrganizationUuid("9a35caa7-a095-4963-ac7b-b784fde4d583"); + reportDto.setOrganizationName("Planning Programming, Budgeting and Execution"); + reportDto.setLocationUuid("does not exist"); + reportDto.setLocationName("does not exist"); + return reportDto; + } + + public static ReportDto createMartReportCompletelyWrong() { + final ReportDto reportDto = new ReportDto(); + reportDto.setUuid("68077002-b766-4a79-bcf2-40b7dbffe6e6"); + reportDto.setOrganizationUuid("does not exist"); + reportDto.setOrganizationName("does not exist"); reportDto.setLocationUuid("does not exist"); reportDto.setLocationName("does not exist"); + final Map tasks = new HashMap<>(); + tasks.put("19364d81-3203-483d-a6bf-461d58888c76", "Intelligence"); + tasks.put("does not exist", "does not exist"); + reportDto.setTasks(tasks); return reportDto; } } diff --git a/src/test/java/mil/dds/anet/test/integration/mart/MartReportImporterWorkerTest.java b/src/test/java/mil/dds/anet/test/integration/mart/MartReportImporterWorkerTest.java index 1a0c441836..731cf1caa5 100644 --- a/src/test/java/mil/dds/anet/test/integration/mart/MartReportImporterWorkerTest.java +++ b/src/test/java/mil/dds/anet/test/integration/mart/MartReportImporterWorkerTest.java @@ -91,11 +91,14 @@ void setUp() throws Exception { EmailMessage emailMessage3 = createMockEmail(TestData.createMartReportWrongOrganization(), false); EmailMessage emailMessage4 = createMockEmail(TestData.createMartReportWrongLocation(), false); + EmailMessage emailMessage5 = createMockEmail(TestData.createMartReportCompletelyWrong(), false); + EmailMessage emailMessage6 = + createMockEmail(TestData.createGoodMartReportWithUnknownTask(), true); // Mock the mail exchange server IMailReceiver iMailReceiverMock = Mockito.mock(); - when(iMailReceiverMock.downloadEmails()) - .thenReturn(List.of(emailMessage1, emailMessage2, emailMessage3, emailMessage4)); + when(iMailReceiverMock.downloadEmails()).thenReturn(List.of(emailMessage1, emailMessage2, + emailMessage3, emailMessage4, emailMessage5, emailMessage6)); martReportImporterWorker = new MartReportImporterWorker(dict, jobHistoryDao, reportDao, personDao, positionDao, taskDao, organizationDao, locationDao, martImportedReportDao, @@ -113,7 +116,7 @@ void testWorker() { final PersonSearchQueryInput queryPerson = PersonSearchQueryInput.builder().withOrgUuid(List.of(reportDto.getOrganizationUuid())) .withEmailNetwork("Internet").withHasBiography(false).withRank("OF-6").build(); - AnetBeanList_Person searchResults = withCredentials("arthur", + final AnetBeanList_Person searchResults = withCredentials("arthur", t -> queryExecutor.personList(getListFields(PersonResourceTest.FIELDS), queryPerson)); assertThat(searchResults.getTotalCount()).isPositive(); Person person = searchResults.getList().get(0); @@ -134,44 +137,78 @@ void testWorker() { assertThat(createdReport.getAttachments().get(0).getFileName()).isEqualTo(ATTACHMENT_NAME); assertThat(createdReport.getTasks().get(0).getLongName()).isEqualTo("Intelligence"); + // Six new records in MartImportedReports, verify them + final List martImportedReports = martImportedReportDao.getAll(); - // Four new records in MartImportedReports, verify then - List martImportedReports = martImportedReportDao.getAll(); - - assertThat(martImportedReports.stream() - .filter( - martImportedReport -> !martImportedReport.isSuccess() && martImportedReport.getErrors() - .equals("Can not find report location: 'does not exist' with uuid: does not exist")) - .hasSize(1); - - assertThat(martImportedReports.stream() - .filter(martImportedReport -> !martImportedReport.isSuccess() - && martImportedReport.getErrors().equals( - "Can not find submitter organization: 'does not exist' with uuid: does not exist"))) - .hasSize(1); - - assertThat(martImportedReports.stream() - .filter(martImportedReport -> !martImportedReport.isSuccess() && martImportedReport - .getErrors().equals("Report with UUID already exists: " + reportDto.getUuid()))) - .hasSize(1); - - assertThat(martImportedReports.stream() + List reportList = martImportedReports.stream() .filter(martImportedReport -> martImportedReport.isSuccess() && martImportedReport.getReportUuid().equals(reportDto.getUuid()) && martImportedReport.getPersonUuid().equals(person.getUuid()) - && martImportedReport.getErrors() - .equals("Can not find task: 'does not exist' with uuid: does not exist
    "))) - .hasSize(1); + && martImportedReport.getErrors() == null) + .toList(); + assertThat(reportList).hasSize(1); + assertThat(martImportedReportDao.delete(reportList.get(0))).isOne(); + + reportList = martImportedReports.stream().filter(martImportedReport -> !martImportedReport + .isSuccess() + && martImportedReport.getErrors() != null + && martImportedReport.getErrors() + .equals("While importing report 231196f5-3b13-45ea-9d73-524d042b16e7:" + + "
    • Report with UUID already exists: 231196f5-3b13-45ea-9d73-524d042b16e7
    ")) + .toList(); + assertThat(reportList).hasSize(1); + assertThat(martImportedReportDao.delete(reportList.get(0))).isOne(); + + reportList = martImportedReports.stream().filter(martImportedReport -> !martImportedReport + .isSuccess() + && martImportedReport.getErrors() != null + && martImportedReport.getErrors() + .equals("While importing report fb875171-2501-46c9-9246-60dafabb656d:" + + "
    • Can not find submitter organization: 'does not exist' with uuid: does not exist
    ")) + .toList(); + assertThat(reportList).hasSize(1); + assertThat(martImportedReportDao.delete(reportList.get(0))).isOne(); + + reportList = martImportedReports.stream().filter(martImportedReport -> !martImportedReport + .isSuccess() + && martImportedReport.getErrors() != null + && martImportedReport.getErrors() + .equals("While importing report 2d6c7a19-d878-4792-bdaf-7a73dc3bfc83:" + + "
    • Can not find report location: 'does not exist' with uuid: does not exist
    ")) + .toList(); + assertThat(reportList).hasSize(1); + assertThat(martImportedReportDao.delete(reportList.get(0))).isOne(); + + reportList = martImportedReports.stream().filter(martImportedReport -> !martImportedReport + .isSuccess() + && martImportedReport.getErrors() != null + && martImportedReport.getErrors() + .equals("While importing report 68077002-b766-4a79-bcf2-40b7dbffe6e6:" + + "
    • Can not find submitter organization: 'does not exist' with uuid: does not exist
      • " + + "
    • Can not find report location: 'does not exist' with uuid: does not exist
    ")) + .toList(); + assertThat(reportList).hasSize(1); + assertThat(martImportedReportDao.delete(reportList.get(0))).isOne(); + + reportList = martImportedReports.stream().filter(martImportedReport -> martImportedReport + .isSuccess() + && martImportedReport.getErrors() != null + && martImportedReport.getErrors() + .equals("While importing report 34faac7c-8c85-4dec-8e9f-57d9254b5ae2:" + + "
    • Can not find task: 'does not exist' with uuid: does not exist
    ")) + .toList(); + assertThat(reportList).hasSize(1); + assertThat(martImportedReportDao.delete(reportList.get(0))).isOne(); } private EmailMessage createMockEmail(ReportDto reportDto, boolean withAttachment) throws ServiceLocalException, IOException { - EmailMessage emailMessageMock = Mockito.mock(); - MessageBody messageBody = new MessageBody(); + final EmailMessage emailMessageMock = Mockito.mock(); + final MessageBody messageBody = new MessageBody(); messageBody.setText(ignoringMapper.writeValueAsString(reportDto)); when(emailMessageMock.getBody()).thenReturn(messageBody); if (withAttachment) { - AttachmentCollection attachmentCollection = new AttachmentCollection(); + final AttachmentCollection attachmentCollection = new AttachmentCollection(); attachmentCollection.addFileAttachment(ATTACHMENT_NAME, IOUtils.toByteArray(Objects.requireNonNull( this.getClass().getClassLoader().getResourceAsStream("assets/default_avatar.png"))));