From e449ca75b385fe6713274a2b25377390a730a72f Mon Sep 17 00:00:00 2001 From: Yafang Deng Date: Fri, 24 May 2024 10:22:21 +0100 Subject: [PATCH 1/8] refactor:use a CommonSyncJobTemplate TIS21-5812 --- pom.xml | 4 +- .../tis/sync/job/CommonSyncJobTemplate.java | 191 ++++++++++++++++++ ...ersonDateChangeCaptureSyncJobTemplate.java | 160 +++------------ .../PersonPlacementEmployingBodyTrustJob.java | 50 ++--- .../PersonPlacementTrainingBodyTrustJob.java | 50 ++--- .../tis/sync/job/PersonRecordStatusJob.java | 14 +- .../sync/job/PostEmployingBodyTrustJob.java | 44 ++-- .../sync/job/PostTrainingBodyTrustJob.java | 46 ++--- .../sync/job/TrustAdminSyncJobTemplate.java | 157 +++----------- .../person/PersonElasticSearchSyncJob.java | 1 - .../reval/RevalCurrentPlacementSyncJob.java | 10 +- .../sync/job/reval/RevalCurrentPmSyncJob.java | 13 +- .../TrustAdminSyncJobTemplateUnitTest.java | 20 +- 13 files changed, 349 insertions(+), 411 deletions(-) create mode 100644 src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java diff --git a/pom.xml b/pom.xml index b701d78c..7001a966 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ uk.nhs.tis sync - 1.21.0 + 1.21.1 jar sync Separate Microservice for synchronisation @@ -235,7 +235,7 @@ org.projectlombok lombok - 1.18.16 + 1.18.20 org.mapstruct diff --git a/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java new file mode 100644 index 00000000..7e495f08 --- /dev/null +++ b/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java @@ -0,0 +1,191 @@ +package uk.nhs.tis.sync.job; + +import com.google.common.base.Stopwatch; +import com.google.common.collect.Sets; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.EntityTransaction; +import org.apache.commons.collections4.CollectionUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.jmx.export.annotation.ManagedOperation; +import uk.nhs.tis.sync.event.JobExecutionEvent; +import uk.nhs.tis.sync.model.EntityData; + +/** + * An abstract common sync job template. + * + * @param the entity type + */ +public abstract class CommonSyncJobTemplate implements RunnableJob { + + private static final Logger LOG = LoggerFactory.getLogger( + CommonSyncJobTemplate.class); + + private static int DEFAULT_PAGE_SIZE = 5000; + protected static final int FIFTEEN_MIN = 15 * 60 * 1000; + + protected Stopwatch mainStopWatch; + + @Autowired + private EntityManagerFactory entityManagerFactory; + + @Autowired(required = false) + protected ApplicationEventPublisher applicationEventPublisher; + + protected String getJobName() { + return this.getClass().getSimpleName(); + } + + protected int getPageSize() { + return DEFAULT_PAGE_SIZE; + } + + protected EntityManagerFactory getEntityManagerFactory() { + return this.entityManagerFactory; + } + + protected abstract void deleteData(); + + protected abstract int convertData(Set entitiesToSave, List entityData, + EntityManager entityManager); + + protected abstract void handleData(Set dataToSave, EntityManager entityManager); + + protected abstract List collectData(Map ids, String queryString, + EntityManager entityManager); + + @ManagedOperation(description = "Is the sync job currently running") + public boolean isCurrentlyRunning() { + return mainStopWatch != null; + } + + @ManagedOperation(description = "The current elapsed time of the current sync job") + public String elapsedTime() { + return mainStopWatch != null ? mainStopWatch.toString() : "0s"; + } + + protected void runSyncJob(String option) { + if (mainStopWatch != null) { + LOG.info("Sync job [{}] already running, exiting this execution", getJobName()); + return; + } + CompletableFuture.runAsync(() -> doDataSync(option)) + .exceptionally(t -> { + publishJobexecutionEvent( + new JobExecutionEvent(this, getFailureMessage(Optional.ofNullable(getJobName()), t))); + LOG.error("Job run ended due an Exception", t); + return null; + }); + } + + /** + * Put all ids needed for the query in a map. + * + * @return the map containing initialised ids + */ + protected abstract Map initIds(); + + /** + * Update ids in the map to the last entity id we collected. + * + * @param ids ids to be updated in the iteration of processing + * @param collectedData collected entities from db + */ + protected abstract void updateIds(Map ids, List collectedData); + + /** + * Assemble the query string in the sync job. + * This method is not necessary if the query is assembled in the `collectedData` method. + * + * @param option parameters needed to assemble the query string + * @return the query string + */ + protected abstract String assembleQueryString(String option); + + protected void doDataSync(String option) { + publishJobexecutionEvent(new JobExecutionEvent(this, "Sync [" + getJobName() + "] started.")); + LOG.info("Sync [{}] started", getJobName()); + + mainStopWatch = Stopwatch.createStarted(); + Stopwatch stopwatch = Stopwatch.createStarted(); + + int skipped = 0; + int totalRecords = 0; + boolean hasMoreResults = true; + Map ids = initIds(); + String queryString = assembleQueryString(option); + + EntityManager entityManager = null; + EntityTransaction transaction = null; + + Set dataToSave = Sets.newHashSet(); + + deleteData(); + stopwatch.reset().start(); + + try { + while (hasMoreResults) { + entityManager = getEntityManagerFactory().createEntityManager(); + transaction = entityManager.getTransaction(); + transaction.begin(); + + List collectedData = + collectData(ids, queryString, entityManager); + hasMoreResults = !collectedData.isEmpty(); + LOG.info("Time taken to read chunk : [{}]", stopwatch); + + if (CollectionUtils.isNotEmpty(collectedData)) { + updateIds(ids, collectedData); + totalRecords += collectedData.size(); + skipped += convertData(dataToSave, collectedData, entityManager); + } + stopwatch.reset().start(); + + handleData(dataToSave, entityManager); + + dataToSave.clear(); + + transaction.commit(); + LOG.info("Time taken to save chunk : [{}]", stopwatch); + stopwatch.reset().start(); + entityManager.close(); + } + LOG.info("Sync job [{}] finished. Total time taken {} for processing [{}] records", + getJobName(), mainStopWatch.stop(), totalRecords); + LOG.info("Skipped records {}", skipped); + publishJobexecutionEvent( + new JobExecutionEvent(this, getSuccessMessage(Optional.ofNullable(getJobName())))); + } finally { + mainStopWatch = null; + if (transaction != null && transaction.isActive()) { + transaction.rollback(); + } + if (entityManager != null && entityManager.isOpen()) { + entityManager.close(); + } + } + } + + protected String getSuccessMessage(Optional jobName) { + return "Sync [" + jobName.orElse(getJobName()) + "] completed successfully."; + } + + protected String getFailureMessage(Optional jobName, Throwable e) { + return " Sync [" + jobName.orElse(getJobName()) + "] failed with exception [" + + e.getMessage() + "]."; + } + + protected void publishJobexecutionEvent(JobExecutionEvent event) { + if (applicationEventPublisher != null) { + applicationEventPublisher.publishEvent(event); + } + } +} diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java index 7e9c95df..e7448326 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java @@ -1,153 +1,72 @@ package uk.nhs.tis.sync.job; -import com.google.common.base.Stopwatch; -import com.google.common.collect.Sets; +import static uk.nhs.tis.sync.job.TrustAdminSyncJobTemplate.LAST_ENTITY_ID; + import java.math.BigInteger; import java.time.LocalDate; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; -import java.util.Optional; import java.util.Set; -import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; import javax.persistence.EntityManager; -import javax.persistence.EntityManagerFactory; -import javax.persistence.EntityTransaction; import javax.persistence.Query; -import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.ApplicationEventPublisher; -import org.springframework.jmx.export.annotation.ManagedOperation; -import uk.nhs.tis.sync.event.JobExecutionEvent; +import uk.nhs.tis.sync.model.EntityData; /** * abstract template for Jobs which sync data when start and end dates have passed. */ -public abstract class PersonDateChangeCaptureSyncJobTemplate implements RunnableJob { +public abstract class PersonDateChangeCaptureSyncJobTemplate extends CommonSyncJobTemplate { - protected static final int DEFAULT_PAGE_SIZE = 5000; - protected static final int FIFTEEN_MIN = 15 * 60 * 1000; - protected static final String FULL_SYNC_DATE_STR = "ANY"; - protected static final String NO_DATE_OVERRIDE = "NONE"; private static final Logger LOG = LoggerFactory.getLogger( PersonDateChangeCaptureSyncJobTemplate.class); - protected Stopwatch mainStopWatch; - - protected String dateOfChangeOverride; - @Autowired - protected EntityManagerFactory entityManagerFactory; + protected static final String FULL_SYNC_DATE_STR = "ANY"; + protected static final String NO_DATE_OVERRIDE = "NONE"; - @Autowired(required = false) - protected ApplicationEventPublisher applicationEventPublisher; + protected String dateOfChangeOverride; protected abstract String buildQueryForDate(LocalDate dateOfChange); - protected abstract int convertData(Set entitiesToSave, List entityData, - EntityManager entityManager); - protected abstract void handleData(Set dataToSave, EntityManager entityManager); - protected String getJobName() { - return this.getClass().getSimpleName(); + @Override + protected String assembleQueryString(String dateOption) { + LocalDate dateOfChange = magicallyGetDateOfChanges(dateOption); + String queryString = buildQueryForDate(dateOfChange); + LOG.debug("Job will run with query:[{}]", queryString); + return queryString; + } + + protected void deleteData() { } - protected List collectData(long lastPersonId, String queryString, - EntityManager entityManager) { + @Override + protected List collectData(Map ids, String queryString, + EntityManager entityManager) { + long lastPersonId = ids.get(LAST_ENTITY_ID); Query query = entityManager.createNativeQuery(queryString).setParameter("lastPersonId", lastPersonId); List resultList = query.getResultList(); - return resultList.stream().filter(Objects::nonNull) - .map(result -> Long.parseLong(result.toString())) - .collect(Collectors.toList()); - } - - @ManagedOperation(description = "Is the sync job currently running") - public boolean isCurrentlyRunning() { - return mainStopWatch != null; + return resultList.stream().filter(Objects::nonNull).map(objArr -> + new EntityData().entityId(objArr.longValue()) + ).collect(Collectors.toList()); } - @ManagedOperation(description = "The current elapsed time of the current sync job") - public String elapsedTime() { - return mainStopWatch != null ? mainStopWatch.toString() : "0s"; + @Override + protected Map initIds() { + Map idMap = new HashMap<>(); + idMap.put(LAST_ENTITY_ID, 0L); + return idMap; } - protected void runSyncJob(String dateOption) { - if (mainStopWatch != null) { - LOG.info("Sync job [{}] already running, exiting this execution", getJobName()); - return; - } - CompletableFuture.runAsync(() -> doDataSync(dateOption)) - .exceptionally(t -> { - publishJobexecutionEvent( - new JobExecutionEvent(this, getFailureMessage(Optional.ofNullable(getJobName()), t))); - LOG.error("Job run ended due an Exception", t); - return null; - }); - } - - private void doDataSync(String dateOption) { - // Configure run - LocalDate dateOfChange = magicallyGetDateOfChanges(dateOption); - String queryString = buildQueryForDate(dateOfChange); - int skipped = 0; - int totalRecords = 0; - long lastEntityId = 0; - boolean hasMoreResults = true; - Set dataToSave = Sets.newHashSet(); - LOG.debug("Job will run with query:[{}]", queryString); - - publishJobexecutionEvent(new JobExecutionEvent(this, "Sync [" + getJobName() + "] started.")); - LOG.info("Sync [{}] started", getJobName()); - mainStopWatch = Stopwatch.createStarted(); - Stopwatch stopwatch = Stopwatch.createStarted(); - - EntityManager entityManager = null; - EntityTransaction transaction = null; - - try { - while (hasMoreResults) { - entityManager = this.entityManagerFactory.createEntityManager(); - transaction = entityManager.getTransaction(); - transaction.begin(); - List collectedData = - collectData(lastEntityId, queryString, entityManager); - hasMoreResults = !collectedData.isEmpty(); - LOG.info("Time taken to read chunk : [{}]", stopwatch); - if (CollectionUtils.isNotEmpty(collectedData)) { - lastEntityId = collectedData.get(collectedData.size() - 1); - totalRecords += collectedData.size(); - skipped += convertData(dataToSave, collectedData, entityManager); - } - stopwatch.reset().start(); - handleData(dataToSave, entityManager); - LOG.debug("Collected {} records and attempted to process {}.", collectedData.size(), - dataToSave.size()); - dataToSave.clear(); - transaction.commit(); - entityManager.close(); - LOG.info("Time taken to save/handle chunk : [{}]", stopwatch); - stopwatch.reset().start(); - } - LOG.info("Sync job [{}] finished. Total time taken {} for processing [{}] records", - getJobName(), mainStopWatch.stop(), totalRecords); - LOG.info("Skipped records {}", skipped); - mainStopWatch = null; - publishJobexecutionEvent( - new JobExecutionEvent(this, getSuccessMessage(Optional.ofNullable(getJobName())))); - } finally { - mainStopWatch = null; - if (transaction != null && transaction.isActive()) { - transaction.rollback(); - } - if (entityManager != null && entityManager.isOpen()) { - entityManager.close(); - } - } + protected void updateIds(Map ids, List collectedData) { + ids.put(LAST_ENTITY_ID, collectedData.get(collectedData.size() - 1).getEntityId()); } private LocalDate magicallyGetDateOfChanges(String dateToUse) { @@ -162,19 +81,4 @@ private LocalDate magicallyGetDateOfChanges(String dateToUse) { } return LocalDate.parse(dateToUse); } - - protected String getSuccessMessage(Optional jobName) { - return "Sync [" + jobName.orElse(getJobName()) + "] completed successfully."; - } - - protected String getFailureMessage(Optional jobName, Throwable e) { - return " Sync [" + jobName.orElse(getJobName()) + "] failed with exception [" - + e.getMessage() + "]."; - } - - protected void publishJobexecutionEvent(JobExecutionEvent event) { - if (applicationEventPublisher != null) { - applicationEventPublisher.publishEvent(event); - } - } } diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java index 95d0ffa0..9cd77e25 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java @@ -6,25 +6,23 @@ import com.transformuk.hee.tis.tcs.service.repository.PersonTrustRepository; import com.transformuk.hee.tis.tcs.service.service.helper.SqlQuerySupplier; import net.javacrumbs.shedlock.core.SchedulerLock; -import uk.nhs.tis.sync.model.EntityData; import org.apache.commons.collections4.CollectionUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.math.BigInteger; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; -import org.springframework.transaction.annotation.Transactional; +import uk.nhs.tis.sync.model.EntityData; import javax.persistence.EntityManager; -import javax.persistence.EntityManagerFactory; import javax.persistence.Query; -import java.math.BigInteger; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; @Component @ManagedResource(objectName = "sync.mbean:name=PersonPlacementEmployingBodyJob", @@ -34,13 +32,10 @@ public class PersonPlacementEmployingBodyTrustJob extends TrustAdminSyncJobTempl private static final Logger LOG = LoggerFactory.getLogger(PersonPlacementEmployingBodyTrustJob.class); - private static final int FIFTEEN_MIN = 15 * 60 * 1000; @Autowired private PersonTrustRepository personTrustRepository; @Autowired - private EntityManagerFactory entityManagerFactory; - @Autowired private PersonRepository personRepository; @Autowired private SqlQuerySupplier sqlQuerySupplier; @@ -51,22 +46,7 @@ public class PersonPlacementEmployingBodyTrustJob extends TrustAdminSyncJobTempl @ManagedOperation( description = "Run sync of the PersonTrust table with Person to Placement EmployingBody") public void doPersonPlacementEmployingBodyFullSync() { - runSyncJob(); - } - - @Override - protected String getJobName() { - return "PersonPlacementEmployingBodyTrustJob"; - } - - @Override - protected int getPageSize() { - return DEFAULT_PAGE_SIZE; - } - - @Override - protected EntityManagerFactory getEntityManagerFactory() { - return this.entityManagerFactory; + runSyncJob(null); } @Override @@ -77,8 +57,10 @@ protected void deleteData() { } @Override - protected List collectData(int pageSize, long lastId, long lastEmployingBodyId, - EntityManager entityManager) { + protected List collectData(Map ids, String queryString, + EntityManager entityManager) { + long lastId = ids.get(LAST_ENTITY_ID); + long lastEmployingBodyId = ids.get(LAST_SITE_ID); LOG.info("Querying with lastPersonId: [{}] and lastEmployingBodyId: [{}]", lastId, lastEmployingBodyId); String personPlacementQuery = @@ -86,7 +68,7 @@ protected List collectData(int pageSize, long lastId, long lastEmplo Query query = entityManager.createNativeQuery(personPlacementQuery) .setParameter("lastId", lastId).setParameter("lastEmployingBodyId", lastEmployingBodyId) - .setParameter("pageSize", pageSize); + .setParameter("pageSize", getPageSize()); List resultList = query.getResultList(); return resultList.stream().filter(Objects::nonNull) @@ -95,11 +77,11 @@ protected List collectData(int pageSize, long lastId, long lastEmplo .collect(Collectors.toList()); } - @Transactional(readOnly = true) @Override - protected int convertData(int skipped, Set entitiesToSave, - List entityData, EntityManager entityManager) { + protected int convertData(Set entitiesToSave, + List entityData, EntityManager entityManager) { + int skipped = 0; if (CollectionUtils.isNotEmpty(entityData)) { Set personIds = diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java index b07d6754..86d10de5 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java @@ -6,15 +6,6 @@ import com.transformuk.hee.tis.tcs.service.service.helper.SqlQuerySupplier; import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.jmx.export.annotation.ManagedOperation; -import org.springframework.jmx.export.annotation.ManagedResource; -import org.springframework.scheduling.annotation.Scheduled; -import org.springframework.stereotype.Component; -import org.springframework.transaction.annotation.Transactional; -import uk.nhs.tis.sync.model.EntityData; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.Query; @@ -24,6 +15,14 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.jmx.export.annotation.ManagedOperation; +import org.springframework.jmx.export.annotation.ManagedResource; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; +import uk.nhs.tis.sync.model.EntityData; @Component @ManagedResource(objectName = "sync.mbean:name=PersonPlacementTrainingBodyTrustJob", @@ -47,35 +46,20 @@ public class PersonPlacementTrainingBodyTrustJob extends TrustAdminSyncJobTempla @ManagedOperation( description = "Run sync of the PersonTrust table with Person to Placement TrainingBody") public void PersonPlacementTrainingBodyFullSync() { - runSyncJob(); + runSyncJob(null); } - @Override - protected String getJobName() { - return "PersonPlacementTrainingBodyTrustJob"; - } - - @Override - protected int getPageSize() { - return DEFAULT_PAGE_SIZE; - } - - @Override - protected EntityManagerFactory getEntityManagerFactory() { - return this.entityManagerFactory; - } - - @Override protected void deleteData() { // This job runs after the PostEmployingBodyEmployingBodyTrustJob and therefore shouldn't // truncate the table } - @Override - protected List collectData(int pageSize, long lastId, long lastTrainingBodyId, - EntityManager entityManager) { + protected List collectData(Map ids, String queryString, + EntityManager entityManager) { + long lastId = ids.get(LAST_ENTITY_ID); + long lastTrainingBodyId = ids.get(LAST_SITE_ID); LOG.info("Querying with lastPersonId: [{}] and lastTrainingBodyId: [{}]", lastId, lastTrainingBodyId); String personPlacementQuery = @@ -83,7 +67,7 @@ protected List collectData(int pageSize, long lastId, long lastTrain Query query = entityManager.createNativeQuery(personPlacementQuery) .setParameter("lastId", lastId).setParameter("lastTrainingBodyId", lastTrainingBodyId) - .setParameter("pageSize", pageSize); + .setParameter("pageSize", getPageSize()); List resultList = query.getResultList(); List result = resultList.stream().filter(Objects::nonNull) @@ -94,11 +78,11 @@ protected List collectData(int pageSize, long lastId, long lastTrain return result; } - @Transactional(readOnly = true) @Override - protected int convertData(int skipped, Set entitiesToSave, - List entityData, EntityManager entityManager) { + protected int convertData(Set entitiesToSave, + List entityData, EntityManager entityManager) { + int skipped = 0; if (CollectionUtils.isNotEmpty(entityData)) { Set personIds = diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java index cbbd7269..ce256ada 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java @@ -10,6 +10,7 @@ import java.util.Objects; import java.util.Set; import javax.persistence.EntityManager; + import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; @@ -20,6 +21,7 @@ import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; +import uk.nhs.tis.sync.model.EntityData; @Component @ManagedResource(objectName = "sync.mbean:name=PersonRecordStatusJob", @@ -93,10 +95,10 @@ private LocalDate validateDateParamFormat(String dateStr) throws DateTimeParseEx } @Override - protected int convertData(Set entitiesToSave, List entityData, - EntityManager entityManager) { + protected int convertData(Set entitiesToSave, List entityData, + EntityManager entityManager) { int entities = entityData.size(); - entityData.stream().map(id -> entityManager.find(Person.class, id)) + entityData.stream().map(entity -> entityManager.find(Person.class, entity.getEntityId())) .filter(Objects::nonNull) .filter(p -> p.getStatus() != p.programmeMembershipsStatus()) .forEach(p -> { @@ -120,11 +122,11 @@ protected String buildQueryForDate(LocalDate dateOfChange) { String startDate = dateOfChange.format(DateTimeFormatter.ISO_LOCAL_DATE); String endDate = dateOfChange.minusDays(1).format(DateTimeFormatter.ISO_LOCAL_DATE); return BASE_QUERY.replace(":endDate", endDate).replace(":startDate", startDate) - .replace(":pageSize", "" + DEFAULT_PAGE_SIZE); + .replace(":pageSize", "" + this.getPageSize()); } else { - return BASE_QUERY.replace(":pageSize", "" + DEFAULT_PAGE_SIZE) + return BASE_QUERY.replace(":pageSize", "" + this.getPageSize()) .replace(" AND (programmeEndDate = ':endDate' OR programmeStartDate = ':startDate')", ""); } } - + } diff --git a/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java index db1f127e..4d4188a4 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java @@ -4,6 +4,12 @@ import com.transformuk.hee.tis.tcs.service.model.PostTrust; import com.transformuk.hee.tis.tcs.service.repository.PostTrustRepository; import com.transformuk.hee.tis.tcs.service.service.helper.SqlQuerySupplier; +import java.math.BigInteger; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; import org.slf4j.Logger; @@ -17,11 +23,6 @@ import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.Query; -import java.math.BigInteger; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; /** * This job runs on a daily basis and must be the first job that works on the PostTrust table as it @@ -36,8 +37,6 @@ public class PostEmployingBodyTrustJob extends TrustAdminSyncJobTemplate { private static final Logger LOG = LoggerFactory.getLogger(PostEmployingBodyTrustJob.class); - private static final int FIFTEEN_MIN = 15 * 60 * 1000; - @Autowired private PostTrustRepository postTrustRepository; @Autowired @@ -51,23 +50,7 @@ public class PostEmployingBodyTrustJob extends TrustAdminSyncJobTemplate collectData(int pageSize, long lastId, long lastEmployingBodyId, - EntityManager entityManager) { + protected List collectData(Map ids, String queryString, + EntityManager entityManager) { + long lastId = ids.get(LAST_ENTITY_ID); + long lastEmployingBodyId = ids.get(LAST_SITE_ID); LOG.info("Querying with lastPostId: [{}] and lastEmployingBodyId: [{}]", lastId, lastEmployingBodyId); String postEmployingBodyQuery = sqlQuerySupplier.getQuery(SqlQuerySupplier.POST_EMPLOYINGBODY); Query query = entityManager.createNativeQuery(postEmployingBodyQuery) .setParameter("lastId", lastId).setParameter("lastEmployingBodyId", lastEmployingBodyId) - .setParameter("pageSize", pageSize); + .setParameter("pageSize", getPageSize()); List resultList = query.getResultList(); List result = resultList.stream().filter(Objects::nonNull).map(objArr -> { @@ -99,9 +84,10 @@ protected List collectData(int pageSize, long lastId, long lastEmplo } @Override - protected int convertData(int skipped, Set entitiesToSave, List entityData, - EntityManager entityManager) { + protected int convertData(Set entitiesToSave, List entityData, + EntityManager entityManager) { + int skipped = 0; if (CollectionUtils.isNotEmpty(entityData)) { for (EntityData ed : entityData) { if (ed != null) { diff --git a/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java index d4d25887..e5ccd73f 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java @@ -3,6 +3,12 @@ import com.transformuk.hee.tis.tcs.service.model.Post; import com.transformuk.hee.tis.tcs.service.model.PostTrust; import com.transformuk.hee.tis.tcs.service.service.helper.SqlQuerySupplier; +import java.math.BigInteger; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; import org.slf4j.Logger; @@ -14,13 +20,7 @@ import org.springframework.stereotype.Component; import uk.nhs.tis.sync.model.EntityData; import javax.persistence.EntityManager; -import javax.persistence.EntityManagerFactory; import javax.persistence.Query; -import java.math.BigInteger; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.stream.Collectors; /** * This job runs on a daily basis and MUST be run after the PostEmployingBodyTrustJob. @@ -34,36 +34,17 @@ public class PostTrainingBodyTrustJob extends TrustAdminSyncJobTemplate { private static final Logger LOG = LoggerFactory.getLogger(PostTrainingBodyTrustJob.class); - private static final int FIFTEEN_MIN = 15 * 60 * 1000; - @Autowired - private EntityManagerFactory entityManagerFactory; @Autowired private SqlQuerySupplier sqlQuerySupplier; - @Scheduled(cron = "${application.cron.postTrainingBodyTrustJob}") @SchedulerLock(name = "postTrustTrainingBodyScheduledTask", lockAtLeastFor = FIFTEEN_MIN, lockAtMostFor = FIFTEEN_MIN) @ManagedOperation( description = "Run sync of the PostTrust table with Post to Training Body Trust") public void PostTrainingBodyTrustFullSync() { - runSyncJob(); - } - - @Override - protected String getJobName() { - return "PostTrainingBodyTrustJob"; - } - - @Override - protected int getPageSize() { - return DEFAULT_PAGE_SIZE; - } - - @Override - protected EntityManagerFactory getEntityManagerFactory() { - return this.entityManagerFactory; + runSyncJob(null); } @Override @@ -72,15 +53,17 @@ protected void deleteData() { } @Override - protected List collectData(int pageSize, long lastId, long lastTrainingBodyId, - EntityManager entityManager) { + protected List collectData(Map ids, String queryString, + EntityManager entityManager) { + long lastId = ids.get(LAST_ENTITY_ID); + long lastTrainingBodyId = ids.get(LAST_SITE_ID); LOG.info("Querying with lastPostId: [{}] and lastTrainingBodyId: [{}]", lastId, lastTrainingBodyId); String postTrainingBodyQuery = sqlQuerySupplier.getQuery(SqlQuerySupplier.POST_TRAININGBODY); Query query = entityManager.createNativeQuery(postTrainingBodyQuery) .setParameter("lastId", lastId).setParameter("lastTrainingBodyId", lastTrainingBodyId) - .setParameter("pageSize", pageSize); + .setParameter("pageSize", getPageSize()); List resultList = query.getResultList(); List result = resultList.stream().filter(Objects::nonNull).map(objArr -> { @@ -93,9 +76,10 @@ protected List collectData(int pageSize, long lastId, long lastTrain } @Override - protected int convertData(int skipped, Set entitiesToSave, List entityData, - EntityManager entityManager) { + protected int convertData(Set entitiesToSave, List entityData, + EntityManager entityManager) { + int skipped = 0; if (CollectionUtils.isNotEmpty(entityData)) { for (EntityData ed : entityData) { if (ed != null) { diff --git a/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java index 48458a51..46dc6d33 100644 --- a/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java @@ -1,151 +1,48 @@ package uk.nhs.tis.sync.job; -import com.google.common.base.Stopwatch; -import com.google.common.collect.Sets; +import java.util.HashMap; import java.util.List; -import java.util.Optional; +import java.util.Map; import java.util.Set; -import java.util.concurrent.CompletableFuture; -import javax.persistence.EntityManager; -import javax.persistence.EntityManagerFactory; -import javax.persistence.EntityTransaction; import org.apache.commons.collections4.CollectionUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.ApplicationEventPublisher; -import org.springframework.jmx.export.annotation.ManagedOperation; -import uk.nhs.tis.sync.event.JobExecutionEvent; import uk.nhs.tis.sync.model.EntityData; +import javax.persistence.EntityManager; -public abstract class TrustAdminSyncJobTemplate implements RunnableJob { - - private static final Logger LOG = LoggerFactory.getLogger(TrustAdminSyncJobTemplate.class); - - @Autowired(required = false) - private ApplicationEventPublisher applicationEventPublisher; - - private Stopwatch mainStopWatch; - - protected static final int DEFAULT_PAGE_SIZE = 5000; - - @ManagedOperation(description = "Is the Post Trust sync just currently running") - public boolean isCurrentlyRunning() { - return mainStopWatch != null; - } - - @ManagedOperation(description = "The current elapsed time of the current sync job") - public String elapsedTime() { - return mainStopWatch != null ? mainStopWatch.toString() : "0s"; - } - - protected void runSyncJob() { - if (mainStopWatch != null) { - LOG.info("Sync job [{}] already running, exiting this execution", getJobName()); - return; - } - CompletableFuture.runAsync(this::run) - .exceptionally(t -> { - publishJobexecutionEvent( - new JobExecutionEvent(this, getFailureMessage(Optional.ofNullable(getJobName()), t))); - LOG.error("Job run ended due an Exception", t); - return null; - }); - } - - protected abstract String getJobName(); - - protected abstract int getPageSize(); - - protected abstract EntityManagerFactory getEntityManagerFactory(); - - protected abstract void deleteData(); - - protected abstract List collectData(int pageSize, long lastId, long lastSiteId, - EntityManager entityManager); +/** + * A template for trust admin sync jobs. + * + * @param the entity type + */ +public abstract class TrustAdminSyncJobTemplate extends CommonSyncJobTemplate { - protected abstract int convertData(int skipped, Set entitiesToSave, - List entityData, EntityManager entityManager); + protected static final String LAST_ENTITY_ID = "lastEntityId"; + protected static final String LAST_SITE_ID = "lastSiteId"; public void run(String params) { - runSyncJob(); + runSyncJob(params); } - protected void run() { - publishJobexecutionEvent(new JobExecutionEvent(this, "Sync [" + getJobName() + "] started.")); - LOG.info("Sync [{}] started", getJobName()); - mainStopWatch = Stopwatch.createStarted(); - Stopwatch stopwatch = Stopwatch.createStarted(); - - int skipped = 0; - int totalRecords = 0; - long lastEntityId = 0; - long lastSiteId = 0; - boolean hasMoreResults = true; - - deleteData(); - stopwatch.reset().start(); - - Set dataToSave = Sets.newHashSet(); - - EntityManager entityManager = null; - - EntityTransaction transaction = null; - try { - while (hasMoreResults) { - entityManager = getEntityManagerFactory().createEntityManager(); - transaction = entityManager.getTransaction(); - transaction.begin(); - - List collectedData = - collectData(getPageSize(), lastEntityId, lastSiteId, entityManager); - hasMoreResults = !collectedData.isEmpty(); - LOG.info("Time taken to read chunk : [{}]", stopwatch); - - if (CollectionUtils.isNotEmpty(collectedData)) { - lastEntityId = collectedData.get(collectedData.size() - 1).getEntityId(); - lastSiteId = collectedData.get(collectedData.size() - 1).getOtherId(); - totalRecords += collectedData.size(); - skipped = convertData(skipped, dataToSave, collectedData, entityManager); - } - stopwatch.reset().start(); - dataToSave.forEach(entityManager::persist); - entityManager.flush(); - dataToSave.clear(); - - transaction.commit(); - LOG.info("Time taken to save chunk : [{}]", stopwatch); - stopwatch.reset().start(); - entityManager.close(); - } - LOG.info("Sync job [{}] finished. Total time taken {} for processing [{}] records", - getJobName(), mainStopWatch.stop(), totalRecords); - LOG.info("Skipped records {}", skipped); - publishJobexecutionEvent( - new JobExecutionEvent(this, getSuccessMessage(Optional.ofNullable(getJobName())))); - } finally { - mainStopWatch = null; - if (transaction != null && transaction.isActive()) { - transaction.rollback(); - } - if (entityManager != null && entityManager.isOpen()) { - entityManager.close(); - } + protected void handleData(Set dataToSave, EntityManager entityManager) { + if (CollectionUtils.isNotEmpty(dataToSave)) { + dataToSave.forEach(entityManager::persist); + entityManager.flush(); } } - protected String getSuccessMessage(Optional jobName) { - return "Sync [" + jobName.orElse(getJobName()) + "] completed successfully."; + protected String assembleQueryString(String option) { + return null; } - protected String getFailureMessage(Optional jobName, Throwable e) { - return " Sync [" + jobName.orElse(getJobName()) + "] failed with exception [" - + e.getMessage() + "]."; + @Override + protected Map initIds() { + Map idMap = new HashMap<>(); + idMap.put(LAST_ENTITY_ID, 0L); + idMap.put(LAST_SITE_ID, 0L); + return idMap; } - private void publishJobexecutionEvent(JobExecutionEvent event) { - if (applicationEventPublisher != null) { - applicationEventPublisher.publishEvent(event); - } + protected void updateIds(Map ids, List collectedData) { + ids.put(LAST_ENTITY_ID, collectedData.get(collectedData.size() - 1).getEntityId()); + ids.put(LAST_SITE_ID, collectedData.get(collectedData.size() - 1).getOtherId()); } } diff --git a/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java index cc8dd481..ac3569e2 100644 --- a/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java @@ -125,7 +125,6 @@ public void run(@Nullable String params) { personElasticSearchSync(); } - protected void run() { if (applicationEventPublisher != null) { diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java index 03410f7e..0993eee2 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java @@ -15,6 +15,7 @@ import org.springframework.stereotype.Component; import uk.nhs.tis.sync.job.PersonDateChangeCaptureSyncJobTemplate; import uk.nhs.tis.sync.message.publisher.RabbitMqTcsRevalTraineeUpdatePublisher; +import uk.nhs.tis.sync.model.EntityData; /** * Get personIds whose current placement changes nightly. And sends messages to rabbitMq @@ -51,9 +52,10 @@ public void revalCurrentPlacementSyncJob() { } @Override - protected int convertData(Set entitiesToSave, List entityData, - EntityManager entityManager) { - entitiesToSave.addAll(entityData); + protected int convertData(Set entitiesToSave, List entityData, + EntityManager entityManager) { + entitiesToSave.addAll( + entityData.stream().map(EntityData::getEntityId).collect(Collectors.toList())); return 0; } @@ -70,6 +72,6 @@ protected String buildQueryForDate(LocalDate dateOfChange) { String today = dateOfChange.format(DateTimeFormatter.ISO_LOCAL_DATE); String yesterday = dateOfChange.minusDays(1).format(DateTimeFormatter.ISO_LOCAL_DATE); return BASE_QUERY.replace(":yesterday", yesterday).replace(":today", today) - .replace(":pageSize", "" + DEFAULT_PAGE_SIZE); + .replace(":pageSize", "" + getPageSize()); } } diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java index 6eceba44..8538a608 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java @@ -15,6 +15,7 @@ import org.springframework.stereotype.Component; import uk.nhs.tis.sync.job.PersonDateChangeCaptureSyncJobTemplate; import uk.nhs.tis.sync.message.publisher.RabbitMqTcsRevalTraineeUpdatePublisher; +import uk.nhs.tis.sync.model.EntityData; /** * Get personIds whose current programmeMembership changes nightly. And sends messages to rabbitMq @@ -52,9 +53,10 @@ public void revalCurrentPmSyncJob() { } @Override - protected int convertData(Set entitiesToSave, List entityData, - EntityManager entityManager) { - entitiesToSave.addAll(entityData); + protected int convertData(Set entitiesToSave, List entityData, + EntityManager entityManager) { + entitiesToSave.addAll( + entityData.stream().map(EntityData::getEntityId).collect(Collectors.toList())); return 0; } @@ -72,11 +74,10 @@ protected String buildQueryForDate(LocalDate dateOfChange) { String startDate = dateOfChange.format(DateTimeFormatter.ISO_LOCAL_DATE); String endDate = dateOfChange.minusDays(1).format(DateTimeFormatter.ISO_LOCAL_DATE); return BASE_QUERY.replace(":endDate", endDate).replace(":startDate", startDate) - .replace(":pageSize", "" + DEFAULT_PAGE_SIZE); + .replace(":pageSize", "" + getPageSize()); } else { - return BASE_QUERY.replace(":pageSize", "" + DEFAULT_PAGE_SIZE) + return BASE_QUERY.replace(":pageSize", "" + getPageSize()) .replace(" AND (programmeEndDate = ':endDate' OR programmeStartDate = ':startDate')", ""); } } - } diff --git a/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java b/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java index ea2df8ab..f19a48cd 100644 --- a/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java +++ b/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java @@ -13,11 +13,13 @@ import static org.mockito.Mockito.when; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.EntityTransaction; + import org.assertj.core.util.Lists; import org.junit.Test; import org.junit.runner.RunWith; @@ -54,10 +56,14 @@ public void runShouldPersistWhenDataReturnedFromQuery() { when(entityManagerFactoryMock.createEntityManager()).thenReturn(entityManagerMock); when(entityManagerMock.getTransaction()).thenReturn(entityTransactionMock); - testObj.run(); + testObj.run(null); + + with().pollDelay(1, TimeUnit.SECONDS) + .atLeast(1, TimeUnit.SECONDS) + .until(() -> !testObj.isCurrentlyRunning()); verify(entityManagerMock, times(100)).persist(any()); - verify(entityManagerMock, times(2)).flush(); + verify(entityManagerMock, times(1)).flush(); verify(entityTransactionMock, times(2)).commit(); } @@ -108,7 +114,7 @@ static class TrustAdminSyncJobTemplateStub extends TrustAdminSyncJobTemplate collectedData) { + List collectedData) { this.entityManagerFactoryMock = entityManagerFactoryMock; this.collectedData = collectedData; } @@ -134,8 +140,8 @@ protected void deleteData() { } @Override - protected List collectData(int pageSize, long lastId, long lastSiteId, - EntityManager entityManager) { + protected List collectData(Map ids, String queryString, + EntityManager entityManager) { if (firstCall) { firstCall = false; return this.collectedData; @@ -144,8 +150,8 @@ protected List collectData(int pageSize, long lastId, long lastSiteI } @Override - protected int convertData(int skipped, Set entitiesToSave, List entityData, - EntityManager entityManager) { + protected int convertData(Set entitiesToSave, List entityData, + EntityManager entityManager) { entityData.forEach(o -> entitiesToSave.add(new Object())); return 0; } From 874bb462566dff2f19695ffff9c4b34ed06c74fa Mon Sep 17 00:00:00 2001 From: JosephKelly Date: Wed, 29 May 2024 15:51:19 +0100 Subject: [PATCH 2/8] refactor: deduplicate Reval Person code Reducing LoC and redundancy TIS21-5812: Add nightly job for Funding Status TIS21-6044: Address code duplication smells --- .../tis/sync/job/CommonSyncJobTemplate.java | 35 ++++++++--------- ...ersonDateChangeCaptureSyncJobTemplate.java | 6 +-- .../reval/RevalCurrentPlacementSyncJob.java | 33 +++------------- .../sync/job/reval/RevalCurrentPmSyncJob.java | 28 +------------- .../reval/RevalPersonChangedJobTemplate.java | 38 +++++++++++++++++++ 5 files changed, 64 insertions(+), 76 deletions(-) create mode 100644 src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java diff --git a/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java index 7e495f08..a3a35f13 100644 --- a/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java @@ -10,9 +10,8 @@ import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.EntityTransaction; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.collections4.CollectionUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import org.springframework.jmx.export.annotation.ManagedOperation; @@ -22,14 +21,12 @@ /** * An abstract common sync job template. * - * @param the entity type + * @param the type of the entity loaded into the target */ +@Slf4j public abstract class CommonSyncJobTemplate implements RunnableJob { - private static final Logger LOG = LoggerFactory.getLogger( - CommonSyncJobTemplate.class); - - private static int DEFAULT_PAGE_SIZE = 5000; + private static final int DEFAULT_PAGE_SIZE = 5000; protected static final int FIFTEEN_MIN = 15 * 60 * 1000; protected Stopwatch mainStopWatch; @@ -55,12 +52,12 @@ protected EntityManagerFactory getEntityManagerFactory() { protected abstract void deleteData(); protected abstract int convertData(Set entitiesToSave, List entityData, - EntityManager entityManager); + EntityManager entityManager); protected abstract void handleData(Set dataToSave, EntityManager entityManager); protected abstract List collectData(Map ids, String queryString, - EntityManager entityManager); + EntityManager entityManager); @ManagedOperation(description = "Is the sync job currently running") public boolean isCurrentlyRunning() { @@ -74,14 +71,14 @@ public String elapsedTime() { protected void runSyncJob(String option) { if (mainStopWatch != null) { - LOG.info("Sync job [{}] already running, exiting this execution", getJobName()); + log.info("Sync job [{}] already running, exiting this execution", getJobName()); return; } CompletableFuture.runAsync(() -> doDataSync(option)) .exceptionally(t -> { publishJobexecutionEvent( new JobExecutionEvent(this, getFailureMessage(Optional.ofNullable(getJobName()), t))); - LOG.error("Job run ended due an Exception", t); + log.error("Job run ended due an Exception", t); return null; }); } @@ -96,14 +93,14 @@ protected void runSyncJob(String option) { /** * Update ids in the map to the last entity id we collected. * - * @param ids ids to be updated in the iteration of processing + * @param ids ids to be updated in the iteration of processing * @param collectedData collected entities from db */ protected abstract void updateIds(Map ids, List collectedData); /** - * Assemble the query string in the sync job. - * This method is not necessary if the query is assembled in the `collectedData` method. + * Assemble the query string in the sync job. This method is not necessary if the query is + * assembled in the `collectedData` method. * * @param option parameters needed to assemble the query string * @return the query string @@ -112,7 +109,7 @@ protected void runSyncJob(String option) { protected void doDataSync(String option) { publishJobexecutionEvent(new JobExecutionEvent(this, "Sync [" + getJobName() + "] started.")); - LOG.info("Sync [{}] started", getJobName()); + log.info("Sync [{}] started", getJobName()); mainStopWatch = Stopwatch.createStarted(); Stopwatch stopwatch = Stopwatch.createStarted(); @@ -140,7 +137,7 @@ protected void doDataSync(String option) { List collectedData = collectData(ids, queryString, entityManager); hasMoreResults = !collectedData.isEmpty(); - LOG.info("Time taken to read chunk : [{}]", stopwatch); + log.info("Time taken to read chunk : [{}]", stopwatch); if (CollectionUtils.isNotEmpty(collectedData)) { updateIds(ids, collectedData); @@ -154,13 +151,13 @@ protected void doDataSync(String option) { dataToSave.clear(); transaction.commit(); - LOG.info("Time taken to save chunk : [{}]", stopwatch); + log.info("Time taken to save chunk : [{}]", stopwatch); stopwatch.reset().start(); entityManager.close(); } - LOG.info("Sync job [{}] finished. Total time taken {} for processing [{}] records", + log.info("Sync job [{}] finished. Total time taken {} for processing [{}] records", getJobName(), mainStopWatch.stop(), totalRecords); - LOG.info("Skipped records {}", skipped); + log.info("Skipped records {}", skipped); publishJobexecutionEvent( new JobExecutionEvent(this, getSuccessMessage(Optional.ofNullable(getJobName())))); } finally { diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java index e7448326..f6869350 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java @@ -18,7 +18,7 @@ import uk.nhs.tis.sync.model.EntityData; /** - * abstract template for Jobs which sync data when start and end dates have passed. + * Abstract template for Jobs which sync data when start and end dates have passed. */ public abstract class PersonDateChangeCaptureSyncJobTemplate extends CommonSyncJobTemplate { @@ -47,14 +47,14 @@ protected void deleteData() { @Override protected List collectData(Map ids, String queryString, - EntityManager entityManager) { + EntityManager entityManager) { long lastPersonId = ids.get(LAST_ENTITY_ID); Query query = entityManager.createNativeQuery(queryString).setParameter("lastPersonId", lastPersonId); List resultList = query.getResultList(); return resultList.stream().filter(Objects::nonNull).map(objArr -> - new EntityData().entityId(objArr.longValue()) + new EntityData().entityId(objArr.longValue()) ).collect(Collectors.toList()); } diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java index 0993eee2..ffb1e0e8 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java @@ -2,38 +2,31 @@ import java.time.LocalDate; import java.time.format.DateTimeFormatter; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; -import javax.persistence.EntityManager; import net.javacrumbs.shedlock.core.SchedulerLock; -import org.apache.commons.collections4.CollectionUtils; import org.springframework.context.annotation.Profile; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; -import uk.nhs.tis.sync.job.PersonDateChangeCaptureSyncJobTemplate; import uk.nhs.tis.sync.message.publisher.RabbitMqTcsRevalTraineeUpdatePublisher; -import uk.nhs.tis.sync.model.EntityData; /** - * Get personIds whose current placement changes nightly. And sends messages to rabbitMq - * for tcs to fetch + * Get personIds whose current placement changed overnight, i.e. ended yesterday or started today. + * This sends messages for further processing, currently tcs to fetch and forward relevant data. */ @Profile("!nimdta") @Component @ManagedResource(objectName = "sync.mbean:name=RevalCurrentPlacementSyncJob", description = "Job message personIds if their placement(s) started/ended") -public class RevalCurrentPlacementSyncJob extends PersonDateChangeCaptureSyncJobTemplate { +public class RevalCurrentPlacementSyncJob extends RevalPersonChangedJobTemplate { + private static final String BASE_QUERY = "SELECT DISTINCT traineeId FROM Placement" + " WHERE traineeId > :lastPersonId" + " AND (dateFrom = ':today' OR dateTo = ':yesterday')" + " ORDER BY traineeId LIMIT :pageSize"; - private final RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher; public RevalCurrentPlacementSyncJob(RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { - this.rabbitMqPublisher = rabbitMqPublisher; + super(rabbitMqPublisher); } @Override @@ -51,22 +44,6 @@ public void revalCurrentPlacementSyncJob() { runSyncJob(null); } - @Override - protected int convertData(Set entitiesToSave, List entityData, - EntityManager entityManager) { - entitiesToSave.addAll( - entityData.stream().map(EntityData::getEntityId).collect(Collectors.toList())); - return 0; - } - - @Override - protected void handleData(Set dataToSave, EntityManager entityManager) { - if (CollectionUtils.isNotEmpty(dataToSave)) { - rabbitMqPublisher.publishToBroker( - dataToSave.stream().map(String::valueOf).collect(Collectors.toSet())); - } - } - @Override protected String buildQueryForDate(LocalDate dateOfChange) { String today = dateOfChange.format(DateTimeFormatter.ISO_LOCAL_DATE); diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java index 8538a608..09ea46d0 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java @@ -2,20 +2,13 @@ import java.time.LocalDate; import java.time.format.DateTimeFormatter; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; -import javax.persistence.EntityManager; import net.javacrumbs.shedlock.core.SchedulerLock; -import org.apache.commons.collections4.CollectionUtils; import org.springframework.context.annotation.Profile; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; -import uk.nhs.tis.sync.job.PersonDateChangeCaptureSyncJobTemplate; import uk.nhs.tis.sync.message.publisher.RabbitMqTcsRevalTraineeUpdatePublisher; -import uk.nhs.tis.sync.model.EntityData; /** * Get personIds whose current programmeMembership changes nightly. And sends messages to rabbitMq @@ -25,16 +18,15 @@ @Component @ManagedResource(objectName = "sync.mbean:name=RevalCurrentPmSyncJob", description = "Job message personIds if their programme membership(s) started/ended") -public class RevalCurrentPmSyncJob extends PersonDateChangeCaptureSyncJobTemplate { +public class RevalCurrentPmSyncJob extends RevalPersonChangedJobTemplate { private static final String BASE_QUERY = "SELECT DISTINCT personId FROM ProgrammeMembership" + " WHERE personId > :lastPersonId" + " AND (programmeEndDate = ':endDate' OR programmeStartDate = ':startDate')" + " ORDER BY personId LIMIT :pageSize"; - private final RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher; public RevalCurrentPmSyncJob(RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { - this.rabbitMqPublisher = rabbitMqPublisher; + super(rabbitMqPublisher); } @Override @@ -52,22 +44,6 @@ public void revalCurrentPmSyncJob() { runSyncJob(null); } - @Override - protected int convertData(Set entitiesToSave, List entityData, - EntityManager entityManager) { - entitiesToSave.addAll( - entityData.stream().map(EntityData::getEntityId).collect(Collectors.toList())); - return 0; - } - - @Override - protected void handleData(Set dataToSave, EntityManager entityManager) { - if (CollectionUtils.isNotEmpty(dataToSave)) { - rabbitMqPublisher.publishToBroker( - dataToSave.stream().map(String::valueOf).collect(Collectors.toSet())); - } - } - @Override protected String buildQueryForDate(LocalDate dateOfChange) { if (dateOfChange != null) { diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java new file mode 100644 index 00000000..e8860439 --- /dev/null +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java @@ -0,0 +1,38 @@ +package uk.nhs.tis.sync.job.reval; + +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import javax.persistence.EntityManager; +import org.apache.commons.collections4.CollectionUtils; +import uk.nhs.tis.sync.job.PersonDateChangeCaptureSyncJobTemplate; +import uk.nhs.tis.sync.message.publisher.RabbitMqTcsRevalTraineeUpdatePublisher; +import uk.nhs.tis.sync.model.EntityData; + +public abstract class RevalPersonChangedJobTemplate extends + PersonDateChangeCaptureSyncJobTemplate { + + private final RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher; + + protected RevalPersonChangedJobTemplate( + RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { + this.rabbitMqPublisher = rabbitMqPublisher; + } + + @Override + protected int convertData(Set entitiesToSave, List entityData, + EntityManager entityManager) { + entitiesToSave.addAll( + entityData.stream().map(EntityData::getEntityId).collect(Collectors.toList())); + return 0; + } + + @Override + protected void handleData(Set dataToSave, EntityManager entityManager) { + if (CollectionUtils.isNotEmpty(dataToSave)) { + rabbitMqPublisher.publishToBroker( + dataToSave.stream().map(String::valueOf).collect(Collectors.toSet())); + } + } + +} From 95d2f385b2d79a399f16d07aa5e8b42db9479f22 Mon Sep 17 00:00:00 2001 From: jayanta2018 Date: Thu, 30 May 2024 17:36:12 +0100 Subject: [PATCH 3/8] fix: code smells suggested by sonar --- .../tis/sync/job/CommonSyncJobTemplate.java | 13 ++-- ...ersonDateChangeCaptureSyncJobTemplate.java | 9 +++ .../PersonPlacementEmployingBodyTrustJob.java | 43 +++++++----- .../PersonPlacementTrainingBodyTrustJob.java | 33 ++++++---- .../tis/sync/job/PersonRecordStatusJob.java | 24 ++++--- .../sync/job/PostEmployingBodyTrustJob.java | 32 +++++---- .../sync/job/PostTrainingBodyTrustJob.java | 21 ++++-- .../sync/job/TrustAdminSyncJobTemplate.java | 11 +++- .../person/PersonElasticSearchSyncJob.java | 66 ++++++++++--------- .../reval/RevalCurrentPlacementSyncJob.java | 10 ++- .../sync/job/reval/RevalCurrentPmSyncJob.java | 10 ++- .../reval/RevalPersonChangedJobTemplate.java | 13 +++- .../TrustAdminSyncJobTemplateUnitTest.java | 10 ++- 13 files changed, 189 insertions(+), 106 deletions(-) diff --git a/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java index a3a35f13..7ba7ffa0 100644 --- a/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java @@ -31,11 +31,16 @@ public abstract class CommonSyncJobTemplate implements RunnableJob { protected Stopwatch mainStopWatch; - @Autowired - private EntityManagerFactory entityManagerFactory; + protected final ApplicationEventPublisher applicationEventPublisher; + + private final EntityManagerFactory entityManagerFactory; - @Autowired(required = false) - protected ApplicationEventPublisher applicationEventPublisher; + @Autowired + public CommonSyncJobTemplate(EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher) { + this.entityManagerFactory = entityManagerFactory; + this.applicationEventPublisher = applicationEventPublisher; + } protected String getJobName() { return this.getClass().getSimpleName(); diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java index f6869350..747373a6 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java @@ -11,10 +11,13 @@ import java.util.Set; import java.util.stream.Collectors; import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; import javax.persistence.Query; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import uk.nhs.tis.sync.model.EntityData; /** @@ -30,6 +33,12 @@ public abstract class PersonDateChangeCaptureSyncJobTemplate extends CommonSy protected String dateOfChangeOverride; + @Autowired + public PersonDateChangeCaptureSyncJobTemplate(EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher) { + super(entityManagerFactory, applicationEventPublisher); + } + protected abstract String buildQueryForDate(LocalDate dateOfChange); protected abstract void handleData(Set dataToSave, EntityManager entityManager); diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java index 9cd77e25..a388cecc 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java @@ -5,40 +5,50 @@ import com.transformuk.hee.tis.tcs.service.repository.PersonRepository; import com.transformuk.hee.tis.tcs.service.repository.PersonTrustRepository; import com.transformuk.hee.tis.tcs.service.service.helper.SqlQuerySupplier; -import net.javacrumbs.shedlock.core.SchedulerLock; -import org.apache.commons.collections4.CollectionUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.math.BigInteger; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Query; +import lombok.extern.slf4j.Slf4j; +import net.javacrumbs.shedlock.core.SchedulerLock; +import org.apache.commons.collections4.CollectionUtils; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; import uk.nhs.tis.sync.model.EntityData; -import javax.persistence.EntityManager; -import javax.persistence.Query; @Component @ManagedResource(objectName = "sync.mbean:name=PersonPlacementEmployingBodyJob", description = "Service that clears the PersonTrust table and links Person with Placement EmployingBody(Trust)") @SuppressWarnings("unchecked") +@Slf4j public class PersonPlacementEmployingBodyTrustJob extends TrustAdminSyncJobTemplate { - private static final Logger LOG = - LoggerFactory.getLogger(PersonPlacementEmployingBodyTrustJob.class); + private final PersonTrustRepository personTrustRepository; + + private final PersonRepository personRepository; + + private final SqlQuerySupplier sqlQuerySupplier; @Autowired - private PersonTrustRepository personTrustRepository; - @Autowired - private PersonRepository personRepository; - @Autowired - private SqlQuerySupplier sqlQuerySupplier; + public PersonPlacementEmployingBodyTrustJob( + EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, + SqlQuerySupplier sqlQuerySupplier, PersonTrustRepository personTrustRepository, + PersonRepository personRepository) { + super(entityManagerFactory, applicationEventPublisher); + this.sqlQuerySupplier = sqlQuerySupplier; + this.personTrustRepository = personTrustRepository; + this.personRepository = personRepository; + } @Scheduled(cron = "${application.cron.personPlacementEmployingBodyTrustJob}") @SchedulerLock(name = "personTrustEmployingBodyScheduledTask", lockAtLeastFor = FIFTEEN_MIN, @@ -51,9 +61,9 @@ public void doPersonPlacementEmployingBodyFullSync() { @Override protected void deleteData() { - LOG.info("deleting all data"); + log.info("deleting all data"); personTrustRepository.deleteAllInBatch(); - LOG.info("deleted all PersonTrust data"); + log.info("deleted all PersonTrust data"); } @Override @@ -61,7 +71,7 @@ protected List collectData(Map ids, String queryString EntityManager entityManager) { long lastId = ids.get(LAST_ENTITY_ID); long lastEmployingBodyId = ids.get(LAST_SITE_ID); - LOG.info("Querying with lastPersonId: [{}] and lastEmployingBodyId: [{}]", lastId, + log.info("Querying with lastPersonId: [{}] and lastEmployingBodyId: [{}]", lastId, lastEmployingBodyId); String personPlacementQuery = sqlQuerySupplier.getQuery(SqlQuerySupplier.PERSON_PLACEMENT_EMPLOYINGBODY); @@ -92,7 +102,6 @@ protected int convertData(Set entitiesToSave, for (EntityData ed : entityData) { if (ed != null) { - if (ed.getEntityId() != null || ed.getOtherId() != null) { PersonTrust personTrust = new PersonTrust(); personTrust.setPerson(personIdToPerson.get(ed.getEntityId())); diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java index 86d10de5..076daf8f 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java @@ -4,20 +4,20 @@ import com.transformuk.hee.tis.tcs.service.model.PersonTrust; import com.transformuk.hee.tis.tcs.service.repository.PersonRepository; import com.transformuk.hee.tis.tcs.service.service.helper.SqlQuerySupplier; -import net.javacrumbs.shedlock.core.SchedulerLock; -import org.apache.commons.collections4.CollectionUtils; -import javax.persistence.EntityManager; -import javax.persistence.EntityManagerFactory; -import javax.persistence.Query; import java.math.BigInteger; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Query; +import lombok.extern.slf4j.Slf4j; +import net.javacrumbs.shedlock.core.SchedulerLock; +import org.apache.commons.collections4.CollectionUtils; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; @@ -27,19 +27,24 @@ @Component @ManagedResource(objectName = "sync.mbean:name=PersonPlacementTrainingBodyTrustJob", description = "Service that clears the PersonTrust table and links Person with Placement TrainingBody (Trusts)") +@Slf4j public class PersonPlacementTrainingBodyTrustJob extends TrustAdminSyncJobTemplate { - private static final Logger LOG = - LoggerFactory.getLogger(PersonPlacementTrainingBodyTrustJob.class); private static final int FIFTEEN_MIN = 15 * 60 * 1000; - @Autowired - private EntityManagerFactory entityManagerFactory; - @Autowired private PersonRepository personRepository; - @Autowired + private SqlQuerySupplier sqlQuerySupplier; + @Autowired + public PersonPlacementTrainingBodyTrustJob(EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, + PersonRepository personRepository, SqlQuerySupplier sqlQuerySupplier) { + super(entityManagerFactory, applicationEventPublisher); + this.personRepository = personRepository; + this.sqlQuerySupplier = sqlQuerySupplier; + } + @Scheduled(cron = "${application.cron.personPlacementTrainingBodyTrustJob}") @SchedulerLock(name = "personTrustTrainingBodyScheduledTask", lockAtLeastFor = FIFTEEN_MIN, lockAtMostFor = FIFTEEN_MIN) @@ -60,7 +65,7 @@ protected List collectData(Map ids, String queryString EntityManager entityManager) { long lastId = ids.get(LAST_ENTITY_ID); long lastTrainingBodyId = ids.get(LAST_SITE_ID); - LOG.info("Querying with lastPersonId: [{}] and lastTrainingBodyId: [{}]", lastId, + log.info("Querying with lastPersonId: [{}] and lastTrainingBodyId: [{}]", lastId, lastTrainingBodyId); String personPlacementQuery = sqlQuerySupplier.getQuery(SqlQuerySupplier.PERSON_PLACEMENT_TRAININGBODY); diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java index ce256ada..3aae69ef 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java @@ -10,13 +10,14 @@ import java.util.Objects; import java.util.Set; import javax.persistence.EntityManager; - +import javax.persistence.EntityManagerFactory; +import lombok.extern.slf4j.Slf4j; import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; @@ -26,9 +27,9 @@ @Component @ManagedResource(objectName = "sync.mbean:name=PersonRecordStatusJob", description = "Job set a Person's (Training Record) Status if their programme membership(s) started/ended") +@Slf4j public class PersonRecordStatusJob extends PersonDateChangeCaptureSyncJobTemplate { - private static final Logger LOG = LoggerFactory.getLogger(PersonRecordStatusJob.class); private static final String BASE_QUERY = "SELECT DISTINCT personId FROM ProgrammeMembership" + " WHERE personId > :lastPersonId" + " AND (programmeEndDate = ':endDate' OR programmeStartDate = ':startDate')" @@ -36,7 +37,11 @@ public class PersonRecordStatusJob extends PersonDateChangeCaptureSyncJobTemplat private final ObjectMapper objectMapper; - public PersonRecordStatusJob(ObjectMapper objectMapper) { + @Autowired + public PersonRecordStatusJob(EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, + ObjectMapper objectMapper) { + super(entityManagerFactory, applicationEventPublisher); this.objectMapper = objectMapper; } @@ -65,20 +70,20 @@ public void personRecordStatusJob() { * "NONE", empty or a date in format yyyy-MM-dd */ public void personRecordStatusJob(String jsonParams) { - LOG.debug("Received run params [{}]", jsonParams); + log.debug("Received run params [{}]", jsonParams); String date = null; if (StringUtils.isNotEmpty(jsonParams)) { try { date = objectMapper.readTree(jsonParams).get("dateOverride").textValue(); validateDateParamFormat(date); - LOG.debug("Got validated date [{}]", date); + log.debug("Got validated date [{}]", date); } catch (JsonProcessingException e) { String errorMsg = "Unable to extract the dateOverride property"; - LOG.error(errorMsg, e); + log.error(errorMsg, e); throw new IllegalArgumentException(errorMsg); } catch (DateTimeParseException e) { String errorMsg = String.format("The date is not correct: %s", date); - LOG.error(errorMsg, e); + log.error(errorMsg, e); throw new IllegalArgumentException(errorMsg); } } @@ -128,5 +133,4 @@ protected String buildQueryForDate(LocalDate dateOfChange) { .replace(" AND (programmeEndDate = ':endDate' OR programmeStartDate = ':startDate')", ""); } } - } diff --git a/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java index 4d4188a4..cf992263 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java @@ -10,19 +10,18 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Query; +import lombok.extern.slf4j.Slf4j; import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; import uk.nhs.tis.sync.model.EntityData; -import javax.persistence.EntityManager; -import javax.persistence.EntityManagerFactory; -import javax.persistence.Query; /** * This job runs on a daily basis and must be the first job that works on the PostTrust table as it @@ -34,16 +33,21 @@ @Component @ManagedResource(objectName = "sync.mbean:name=PostEmployingBodyTrustJob", description = "Service that clears the PersonTrust table and links Post with Employing Body Trusts") +@Slf4j public class PostEmployingBodyTrustJob extends TrustAdminSyncJobTemplate { - private static final Logger LOG = LoggerFactory.getLogger(PostEmployingBodyTrustJob.class); - @Autowired private PostTrustRepository postTrustRepository; - @Autowired - private EntityManagerFactory entityManagerFactory; - @Autowired + private SqlQuerySupplier sqlQuerySupplier; + public PostEmployingBodyTrustJob(EntityManagerFactory entityManagerFactory, + ApplicationEventPublisher applicationEventPublisher, PostTrustRepository postTrustRepository, + SqlQuerySupplier sqlQuerySupplier) { + super(entityManagerFactory, applicationEventPublisher); + this.postTrustRepository = postTrustRepository; + this.sqlQuerySupplier = sqlQuerySupplier; + } + @Scheduled(cron = "${application.cron.postEmployingBodyTrustJob}") @SchedulerLock(name = "postTrustEmployingBodyScheduledTask", lockAtLeastFor = FIFTEEN_MIN, lockAtMostFor = FIFTEEN_MIN) @@ -55,9 +59,9 @@ public void PostEmployingBodyTrustFullSync() { @Override protected void deleteData() { - LOG.info("deleting all data"); + log.info("deleting all data"); postTrustRepository.deleteAllInBatch(); - LOG.info("deleted all PostTrust data"); + log.info("deleted all PostTrust data"); } @Override @@ -65,7 +69,7 @@ protected List collectData(Map ids, String queryString EntityManager entityManager) { long lastId = ids.get(LAST_ENTITY_ID); long lastEmployingBodyId = ids.get(LAST_SITE_ID); - LOG.info("Querying with lastPostId: [{}] and lastEmployingBodyId: [{}]", lastId, + log.info("Querying with lastPostId: [{}] and lastEmployingBodyId: [{}]", lastId, lastEmployingBodyId); String postEmployingBodyQuery = sqlQuerySupplier.getQuery(SqlQuerySupplier.POST_EMPLOYINGBODY); diff --git a/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java index e5ccd73f..e9fe3c34 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java @@ -9,18 +9,19 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Query; +import lombok.extern.slf4j.Slf4j; import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Component; import uk.nhs.tis.sync.model.EntityData; -import javax.persistence.EntityManager; -import javax.persistence.Query; /** * This job runs on a daily basis and MUST be run after the PostEmployingBodyTrustJob. @@ -31,12 +32,18 @@ @Component @ManagedResource(objectName = "sync.mbean:name=PostTrainingBodyTrustJob", description = "Service that links Post with Training Body Trusts") +@Slf4j public class PostTrainingBodyTrustJob extends TrustAdminSyncJobTemplate { - private static final Logger LOG = LoggerFactory.getLogger(PostTrainingBodyTrustJob.class); + private SqlQuerySupplier sqlQuerySupplier; @Autowired - private SqlQuerySupplier sqlQuerySupplier; + public PostTrainingBodyTrustJob(EntityManagerFactory entityManagerFactory, + ApplicationEventPublisher applicationEventPublisher, + SqlQuerySupplier sqlQuerySupplier) { + super(entityManagerFactory, applicationEventPublisher); + this.sqlQuerySupplier = sqlQuerySupplier; + } @Scheduled(cron = "${application.cron.postTrainingBodyTrustJob}") @SchedulerLock(name = "postTrustTrainingBodyScheduledTask", lockAtLeastFor = FIFTEEN_MIN, @@ -57,7 +64,7 @@ protected List collectData(Map ids, String queryString EntityManager entityManager) { long lastId = ids.get(LAST_ENTITY_ID); long lastTrainingBodyId = ids.get(LAST_SITE_ID); - LOG.info("Querying with lastPostId: [{}] and lastTrainingBodyId: [{}]", lastId, + log.info("Querying with lastPostId: [{}] and lastTrainingBodyId: [{}]", lastId, lastTrainingBodyId); String postTrainingBodyQuery = sqlQuerySupplier.getQuery(SqlQuerySupplier.POST_TRAININGBODY); diff --git a/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java index 46dc6d33..5aca9a59 100644 --- a/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java @@ -4,9 +4,12 @@ import java.util.List; import java.util.Map; import java.util.Set; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; import org.apache.commons.collections4.CollectionUtils; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import uk.nhs.tis.sync.model.EntityData; -import javax.persistence.EntityManager; /** * A template for trust admin sync jobs. @@ -18,6 +21,12 @@ public abstract class TrustAdminSyncJobTemplate extends CommonSyncJobTemplate protected static final String LAST_ENTITY_ID = "lastEntityId"; protected static final String LAST_SITE_ID = "lastSiteId"; + @Autowired + public TrustAdminSyncJobTemplate(EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher) { + super(entityManagerFactory, applicationEventPublisher); + } + public void run(String params) { runSyncJob(params); } diff --git a/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java index ac3569e2..83034aae 100644 --- a/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java @@ -6,11 +6,10 @@ import java.util.List; import java.util.concurrent.CompletableFuture; import javax.annotation.Nullable; +import lombok.extern.slf4j.Slf4j; import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; import org.elasticsearch.index.IndexNotFoundException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationEventPublisher; @@ -28,43 +27,37 @@ import uk.nhs.tis.sync.service.PersonElasticSearchService; import uk.nhs.tis.sync.service.impl.PersonViewRowMapper; +/** + * This job runs on a daily basis and provides support for faster data search + *

+ * Its purpose is to clear down the persons index in ES then repopulates the person data so that + * person search is quicker. + */ @Component -@ManagedResource(objectName = "sync.mbean:name=PersonElasticSearchJob", description = "Service that clears the persons index in ES and repopulates the data") +@ManagedResource(objectName = "sync.mbean:name=PersonElasticSearchJob", + description = "Service that clears the persons index in ES and repopulates the data") +@Slf4j public class PersonElasticSearchSyncJob implements RunnableJob { private static final String JOB_NAME = "Person sync job"; - private static final Logger LOG = LoggerFactory.getLogger(PersonElasticSearchSyncJob.class); private static final String ES_INDEX = "persons"; private static final int FIFTEEN_MIN = 15 * 60 * 1000; - private Stopwatch mainStopWatch; - - protected int pageSize = 8_000; - - @Autowired private final SqlQuerySupplier sqlQuerySupplier; - @Autowired private final NamedParameterJdbcTemplate namedParameterJdbcTemplate; - @Autowired private final ElasticsearchOperations elasticSearchOperations; - @Autowired private final PersonElasticSearchService personElasticSearchService; private ApplicationEventPublisher applicationEventPublisher; - @ManagedOperation(description = "Is the Person es sync just currently running") - public boolean isCurrentlyRunning() { - return mainStopWatch != null; - } + protected int pageSize = 8_000; + private Stopwatch mainStopWatch; - @ManagedOperation(description = "The current elapsed time of the current sync job") - public String elapsedTime() { - return mainStopWatch != null ? mainStopWatch.toString() : "0s"; - } + @Autowired public PersonElasticSearchSyncJob(NamedParameterJdbcTemplate namedParameterJdbcTemplate, SqlQuerySupplier sqlQuerySupplier, ElasticsearchOperations elasticSearchOperations, PersonElasticSearchService personElasticSearchService, @@ -76,16 +69,27 @@ public PersonElasticSearchSyncJob(NamedParameterJdbcTemplate namedParameterJdbcT this.pageSize = pageSize; } + @ManagedOperation(description = "Is the Person es sync just currently running") + public boolean isCurrentlyRunning() { + return mainStopWatch != null; + } + + @ManagedOperation(description = "The current elapsed time of the current sync job") + public String elapsedTime() { + return mainStopWatch != null ? mainStopWatch.toString() : "0s"; + } + protected void runSyncJob() { if (mainStopWatch != null) { - LOG.info("Sync job [{}] already running, exiting this execution", JOB_NAME); + log.info("Sync job [{}] already running, exiting this execution", JOB_NAME); return; } CompletableFuture.runAsync(this::run); } @Scheduled(cron = "${application.cron.personElasticSearchJob}") - @SchedulerLock(name = "personsElasticSearchScheduledTask", lockAtLeastFor = FIFTEEN_MIN, lockAtMostFor = FIFTEEN_MIN) + @SchedulerLock(name = "personsElasticSearchScheduledTask", lockAtLeastFor = FIFTEEN_MIN, + lockAtMostFor = FIFTEEN_MIN) @ManagedOperation(description = "Run sync of the persons es index") public void personElasticSearchSync() { runSyncJob(); @@ -96,11 +100,11 @@ private int getPageSize() { } private void deleteIndex() { - LOG.info("deleting person es index"); + log.info("deleting person es index"); try { elasticSearchOperations.indexOps(IndexCoordinates.of(ES_INDEX)).delete(); } catch (IndexNotFoundException e) { - LOG.info("Could not delete an index that does not exist, continuing"); + log.info("Could not delete an index that does not exist, continuing"); } } @@ -126,13 +130,12 @@ public void run(@Nullable String params) { } protected void run() { - if (applicationEventPublisher != null) { applicationEventPublisher .publishEvent(new JobExecutionEvent(this, "Sync [" + JOB_NAME + "] started.")); } try { - LOG.info("Sync [{}] started", JOB_NAME); + log.info("Sync [{}] started", JOB_NAME); mainStopWatch = Stopwatch.createStarted(); Stopwatch stopwatch = Stopwatch.createStarted(); @@ -151,7 +154,7 @@ protected void run() { hasMoreResults = !collectedData.isEmpty(); - LOG.info("Time taken to read chunk : [{}]", stopwatch); + log.info("Time taken to read chunk : [{}]", stopwatch); if (CollectionUtils.isNotEmpty(collectedData)) { totalRecords += collectedData.size(); } @@ -159,11 +162,11 @@ protected void run() { personElasticSearchService.saveDocuments(collectedData); - LOG.info("Time taken to save chunk : [{}]", stopwatch); + log.info("Time taken to save chunk : [{}]", stopwatch); } elasticSearchOperations.indexOps(PersonView.class).refresh(); stopwatch.reset().start(); - LOG.info("Sync job [{}] finished. Total time taken {} for processing [{}] records", + log.info("Sync job [{}] finished. Total time taken {} for processing [{}] records", JOB_NAME, mainStopWatch.stop(), totalRecords); mainStopWatch = null; if (applicationEventPublisher != null) { @@ -171,7 +174,7 @@ protected void run() { .publishEvent(new JobExecutionEvent(this, "Sync [" + JOB_NAME + "] finished.")); } } catch (Exception e) { - LOG.error(e.getLocalizedMessage(), e); + log.error(e.getLocalizedMessage(), e); mainStopWatch = null; if (applicationEventPublisher != null) { applicationEventPublisher.publishEvent(new JobExecutionEvent(this, @@ -181,7 +184,7 @@ protected void run() { } private void createIndex() { - LOG.info("creating and updating mappings"); + log.info("creating and updating mappings"); elasticSearchOperations.indexOps(IndexCoordinates.of(ES_INDEX)).create(); Document mapping = elasticSearchOperations.indexOps(IndexCoordinates.of(ES_INDEX)) .createMapping(PersonView.class); @@ -192,5 +195,4 @@ private void createIndex() { public void setApplicationEventPublisher(ApplicationEventPublisher applicationEventPublisher) { this.applicationEventPublisher = applicationEventPublisher; } - } diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java index ffb1e0e8..7468706f 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java @@ -2,7 +2,10 @@ import java.time.LocalDate; import java.time.format.DateTimeFormatter; +import javax.persistence.EntityManagerFactory; import net.javacrumbs.shedlock.core.SchedulerLock; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Profile; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; @@ -25,8 +28,11 @@ public class RevalCurrentPlacementSyncJob extends RevalPersonChangedJobTemplate + " AND (dateFrom = ':today' OR dateTo = ':yesterday')" + " ORDER BY traineeId LIMIT :pageSize"; - public RevalCurrentPlacementSyncJob(RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { - super(rabbitMqPublisher); + @Autowired + public RevalCurrentPlacementSyncJob(EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, + RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { + super(entityManagerFactory, applicationEventPublisher, rabbitMqPublisher); } @Override diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java index 09ea46d0..f328f885 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java @@ -2,7 +2,10 @@ import java.time.LocalDate; import java.time.format.DateTimeFormatter; +import javax.persistence.EntityManagerFactory; import net.javacrumbs.shedlock.core.SchedulerLock; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Profile; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; @@ -25,8 +28,11 @@ public class RevalCurrentPmSyncJob extends RevalPersonChangedJobTemplate { + " AND (programmeEndDate = ':endDate' OR programmeStartDate = ':startDate')" + " ORDER BY personId LIMIT :pageSize"; - public RevalCurrentPmSyncJob(RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { - super(rabbitMqPublisher); + @Autowired + public RevalCurrentPmSyncJob(EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, + RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { + super(entityManagerFactory, applicationEventPublisher, rabbitMqPublisher); } @Override diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java index e8860439..12562e7a 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java @@ -4,18 +4,30 @@ import java.util.Set; import java.util.stream.Collectors; import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; import org.apache.commons.collections4.CollectionUtils; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import uk.nhs.tis.sync.job.PersonDateChangeCaptureSyncJobTemplate; import uk.nhs.tis.sync.message.publisher.RabbitMqTcsRevalTraineeUpdatePublisher; import uk.nhs.tis.sync.model.EntityData; +/** + * This is a template abstract class for Revalidation Person job change + *

+ * Its purpose is to publish Persons based on their current Placement into Rabbit message queue + */ public abstract class RevalPersonChangedJobTemplate extends PersonDateChangeCaptureSyncJobTemplate { private final RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher; + @Autowired protected RevalPersonChangedJobTemplate( + EntityManagerFactory entityManagerFactory, + @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { + super(entityManagerFactory, applicationEventPublisher); this.rabbitMqPublisher = rabbitMqPublisher; } @@ -34,5 +46,4 @@ protected void handleData(Set dataToSave, EntityManager entityManager) { dataToSave.stream().map(String::valueOf).collect(Collectors.toSet())); } } - } diff --git a/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java b/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java index f19a48cd..7d1119d8 100644 --- a/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java +++ b/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java @@ -25,6 +25,8 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import uk.nhs.tis.sync.model.EntityData; @RunWith(MockitoJUnitRunner.class) @@ -42,8 +44,11 @@ public class TrustAdminSyncJobTemplateUnitTest { @Mock private List mockList; + @Mock + private ApplicationEventPublisher applicationEventPublisherMock; + public void instantiateJob(List data) { - testObj = new TrustAdminSyncJobTemplateStub(entityManagerFactoryMock, data); + testObj = new TrustAdminSyncJobTemplateStub(entityManagerFactoryMock, applicationEventPublisherMock, data); } @Test @@ -113,8 +118,9 @@ static class TrustAdminSyncJobTemplateStub extends TrustAdminSyncJobTemplate collectedData; private boolean firstCall = true; - public TrustAdminSyncJobTemplateStub(EntityManagerFactory entityManagerFactoryMock, + public TrustAdminSyncJobTemplateStub(EntityManagerFactory entityManagerFactoryMock, ApplicationEventPublisher applicationEventPublisherMock, List collectedData) { + super(entityManagerFactoryMock, applicationEventPublisherMock); this.entityManagerFactoryMock = entityManagerFactoryMock; this.collectedData = collectedData; } From 68da0719790995f3775c8ae352ca836ea1d3b958 Mon Sep 17 00:00:00 2001 From: jayanta2018 Date: Mon, 3 Jun 2024 09:57:39 +0100 Subject: [PATCH 4/8] fix: change visibility of constructors --- .../tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java | 2 +- .../java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java index 747373a6..0040f990 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java @@ -34,7 +34,7 @@ public abstract class PersonDateChangeCaptureSyncJobTemplate extends CommonSy protected String dateOfChangeOverride; @Autowired - public PersonDateChangeCaptureSyncJobTemplate(EntityManagerFactory entityManagerFactory, + protected PersonDateChangeCaptureSyncJobTemplate(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher) { super(entityManagerFactory, applicationEventPublisher); } diff --git a/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java index 5aca9a59..c52c2399 100644 --- a/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java @@ -22,7 +22,7 @@ public abstract class TrustAdminSyncJobTemplate extends CommonSyncJobTemplate protected static final String LAST_SITE_ID = "lastSiteId"; @Autowired - public TrustAdminSyncJobTemplate(EntityManagerFactory entityManagerFactory, + protected TrustAdminSyncJobTemplate(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher) { super(entityManagerFactory, applicationEventPublisher); } From 9757c750d09ce15862b9ac6c8313ab85e53c9883 Mon Sep 17 00:00:00 2001 From: jayanta2018 Date: Mon, 3 Jun 2024 11:13:04 +0100 Subject: [PATCH 5/8] fix: remove autowired from sole constructor --- src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java | 3 +-- .../tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java | 1 - .../nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java | 1 - .../nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java | 1 - src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java | 1 - .../java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java | 2 -- .../java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplate.java | 1 - .../uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java | 2 -- .../nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java | 1 - .../java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java | 1 - .../nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java | 1 - 11 files changed, 1 insertion(+), 14 deletions(-) diff --git a/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java index 7ba7ffa0..07f9a08f 100644 --- a/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/CommonSyncJobTemplate.java @@ -35,8 +35,7 @@ public abstract class CommonSyncJobTemplate implements RunnableJob { private final EntityManagerFactory entityManagerFactory; - @Autowired - public CommonSyncJobTemplate(EntityManagerFactory entityManagerFactory, + protected CommonSyncJobTemplate(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher) { this.entityManagerFactory = entityManagerFactory; this.applicationEventPublisher = applicationEventPublisher; diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java index 0040f990..cbc19116 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonDateChangeCaptureSyncJobTemplate.java @@ -33,7 +33,6 @@ public abstract class PersonDateChangeCaptureSyncJobTemplate extends CommonSy protected String dateOfChangeOverride; - @Autowired protected PersonDateChangeCaptureSyncJobTemplate(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher) { super(entityManagerFactory, applicationEventPublisher); diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java index a388cecc..a7eb18af 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java @@ -38,7 +38,6 @@ public class PersonPlacementEmployingBodyTrustJob extends TrustAdminSyncJobTempl private final SqlQuerySupplier sqlQuerySupplier; - @Autowired public PersonPlacementEmployingBodyTrustJob( EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java index 076daf8f..6d497f90 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java @@ -36,7 +36,6 @@ public class PersonPlacementTrainingBodyTrustJob extends TrustAdminSyncJobTempla private SqlQuerySupplier sqlQuerySupplier; - @Autowired public PersonPlacementTrainingBodyTrustJob(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, PersonRepository personRepository, SqlQuerySupplier sqlQuerySupplier) { diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java index 3aae69ef..e27e24ad 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java @@ -37,7 +37,6 @@ public class PersonRecordStatusJob extends PersonDateChangeCaptureSyncJobTemplat private final ObjectMapper objectMapper; - @Autowired public PersonRecordStatusJob(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, ObjectMapper objectMapper) { diff --git a/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java index e9fe3c34..4e0d8faf 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java @@ -15,7 +15,6 @@ import lombok.extern.slf4j.Slf4j; import net.javacrumbs.shedlock.core.SchedulerLock; import org.apache.commons.collections4.CollectionUtils; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import org.springframework.jmx.export.annotation.ManagedOperation; import org.springframework.jmx.export.annotation.ManagedResource; @@ -37,7 +36,6 @@ public class PostTrainingBodyTrustJob extends TrustAdminSyncJobTemplate extends CommonSyncJobTemplate protected static final String LAST_ENTITY_ID = "lastEntityId"; protected static final String LAST_SITE_ID = "lastSiteId"; - @Autowired protected TrustAdminSyncJobTemplate(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher) { super(entityManagerFactory, applicationEventPublisher); diff --git a/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java index 83034aae..0bbf100f 100644 --- a/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java @@ -56,8 +56,6 @@ public class PersonElasticSearchSyncJob implements RunnableJob { protected int pageSize = 8_000; private Stopwatch mainStopWatch; - - @Autowired public PersonElasticSearchSyncJob(NamedParameterJdbcTemplate namedParameterJdbcTemplate, SqlQuerySupplier sqlQuerySupplier, ElasticsearchOperations elasticSearchOperations, PersonElasticSearchService personElasticSearchService, diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java index 7468706f..a07f505b 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPlacementSyncJob.java @@ -28,7 +28,6 @@ public class RevalCurrentPlacementSyncJob extends RevalPersonChangedJobTemplate + " AND (dateFrom = ':today' OR dateTo = ':yesterday')" + " ORDER BY traineeId LIMIT :pageSize"; - @Autowired public RevalCurrentPlacementSyncJob(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java index f328f885..d99fe0c9 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalCurrentPmSyncJob.java @@ -28,7 +28,6 @@ public class RevalCurrentPmSyncJob extends RevalPersonChangedJobTemplate { + " AND (programmeEndDate = ':endDate' OR programmeStartDate = ':startDate')" + " ORDER BY personId LIMIT :pageSize"; - @Autowired public RevalCurrentPmSyncJob(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher) { diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java index 12562e7a..568b4809 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java @@ -22,7 +22,6 @@ public abstract class RevalPersonChangedJobTemplate extends private final RabbitMqTcsRevalTraineeUpdatePublisher rabbitMqPublisher; - @Autowired protected RevalPersonChangedJobTemplate( EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, From 5cc11d72d157bdc703eec4b078d210ed52aef6d0 Mon Sep 17 00:00:00 2001 From: jayanta2018 Date: Mon, 3 Jun 2024 15:21:12 +0100 Subject: [PATCH 6/8] fix: fix more code smells --- .../event/listener/JobRunningListener.java | 4 ++-- .../PersonPlacementEmployingBodyTrustJob.java | 18 +++++++++++++- .../PersonPlacementTrainingBodyTrustJob.java | 8 ++++++- .../tis/sync/job/PersonRecordStatusJob.java | 10 +++++++- .../sync/job/PostEmployingBodyTrustJob.java | 24 +++++++++++++------ .../sync/job/PostTrainingBodyTrustJob.java | 6 ++--- .../person/PersonElasticSearchSyncJob.java | 18 ++++++++++---- .../reval/RevalPersonChangedJobTemplate.java | 7 +++--- ...ntTrainingBodyTrustJobIntegrationTest.java | 2 +- ...tEmployingBodyTrustJobIntegrationTest.java | 2 +- .../TrustAdminSyncJobTemplateUnitTest.java | 14 +++++------ 11 files changed, 82 insertions(+), 31 deletions(-) diff --git a/src/main/java/uk/nhs/tis/sync/event/listener/JobRunningListener.java b/src/main/java/uk/nhs/tis/sync/event/listener/JobRunningListener.java index 35bca9f4..3c122c5d 100644 --- a/src/main/java/uk/nhs/tis/sync/event/listener/JobRunningListener.java +++ b/src/main/java/uk/nhs/tis/sync/event/listener/JobRunningListener.java @@ -79,11 +79,11 @@ public void runJobs() { do { Thread.sleep(SLEEP_DURATION); } while (personPlacementEmployingBodyTrustJob.isCurrentlyRunning()); - personPlacementTrainingBodyTrustJob.PersonPlacementTrainingBodyFullSync(); + personPlacementTrainingBodyTrustJob.personPlacementTrainingBodyFullSync(); do { Thread.sleep(SLEEP_DURATION); } while (personPlacementTrainingBodyTrustJob.isCurrentlyRunning()); - postEmployingBodyTrustJob.PostEmployingBodyTrustFullSync(); + postEmployingBodyTrustJob.postEmployingBodyTrustFullSync(); do { Thread.sleep(SLEEP_DURATION); } while (postEmployingBodyTrustJob.isCurrentlyRunning()); diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java index a7eb18af..bc2664f9 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java @@ -25,9 +25,16 @@ import org.springframework.stereotype.Component; import uk.nhs.tis.sync.model.EntityData; +/** + * A sync job for updating placement employing body for a person. + * + * This job clears the PersonTrust table and links Person with Placement EmployingBody (Trust). + * + */ @Component @ManagedResource(objectName = "sync.mbean:name=PersonPlacementEmployingBodyJob", - description = "Service that clears the PersonTrust table and links Person with Placement EmployingBody(Trust)") + description = "Service that clears the PersonTrust table and links " + + "Person with Placement EmployingBody(Trust)") @SuppressWarnings("unchecked") @Slf4j public class PersonPlacementEmployingBodyTrustJob extends TrustAdminSyncJobTemplate { @@ -38,6 +45,15 @@ public class PersonPlacementEmployingBodyTrustJob extends TrustAdminSyncJobTempl private final SqlQuerySupplier sqlQuerySupplier; + /** + * Constructs a new PersonPlacementEmployingBodyTrustJob with the specified dependencies. + * + * @param entityManagerFactory the factory to create EntityManager instances + * @param applicationEventPublisher the publisher for application events, may be null + * @param sqlQuerySupplier the supplier for SQL queries + * @param personTrustRepository the repository for PersonTrust entities + * @param personRepository the repository for Person entities + */ public PersonPlacementEmployingBodyTrustJob( EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java index 6d497f90..efcc4662 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java @@ -24,6 +24,12 @@ import org.springframework.stereotype.Component; import uk.nhs.tis.sync.model.EntityData; +/** + * A sync job for updating placement training body for a person. + * + * This job clears the PersonTrust table and links Person with Placement TrainingBody (Trust). + * + */ @Component @ManagedResource(objectName = "sync.mbean:name=PersonPlacementTrainingBodyTrustJob", description = "Service that clears the PersonTrust table and links Person with Placement TrainingBody (Trusts)") @@ -49,7 +55,7 @@ public PersonPlacementTrainingBodyTrustJob(EntityManagerFactory entityManagerFac lockAtMostFor = FIFTEEN_MIN) @ManagedOperation( description = "Run sync of the PersonTrust table with Person to Placement TrainingBody") - public void PersonPlacementTrainingBodyFullSync() { + public void personPlacementTrainingBodyFullSync() { runSyncJob(null); } diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java index e27e24ad..5d39f974 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java @@ -24,9 +24,17 @@ import org.springframework.stereotype.Component; import uk.nhs.tis.sync.model.EntityData; +/** + * A sync job for updating training record status for a person. + * + * This job sets a Person's (Training Record) Status if their + * programme membership(s) started/ended. + * + */ @Component @ManagedResource(objectName = "sync.mbean:name=PersonRecordStatusJob", - description = "Job set a Person's (Training Record) Status if their programme membership(s) started/ended") + description = "Job set a Person's (Training Record) Status if their " + + "programme membership(s) started/ended") @Slf4j public class PersonRecordStatusJob extends PersonDateChangeCaptureSyncJobTemplate { diff --git a/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java index cf992263..7f200fbf 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJob.java @@ -24,15 +24,17 @@ import uk.nhs.tis.sync.model.EntityData; /** - * This job runs on a daily basis and must be the first job that works on the PostTrust table as it - * truncates it at the beginning. - *

- * Its purpose is to clear down the PostTrust table then populate it with post ids and the linked - * employing body trust id + * This job runs on a daily basis and must be the first job that works on the + * PostTrust table as it truncates it at the beginning. + * + *

Its purpose is to clear down the PostTrust table + * then populate it with post ids and the linked + * employing body trust id. */ @Component @ManagedResource(objectName = "sync.mbean:name=PostEmployingBodyTrustJob", - description = "Service that clears the PersonTrust table and links Post with Employing Body Trusts") + description = "Service that clears the PersonTrust table and " + + "links Post with Employing Body Trusts") @Slf4j public class PostEmployingBodyTrustJob extends TrustAdminSyncJobTemplate { @@ -40,6 +42,14 @@ public class PostEmployingBodyTrustJob extends TrustAdminSyncJobTemplate - * Its purpose is to populate the PostTrust table with post ids and the linked training body trust - * id + * + *

Its purpose is to populate the PostTrust table with post ids and the linked + * training body trust id. */ @Component @ManagedResource(objectName = "sync.mbean:name=PostTrainingBodyTrustJob", diff --git a/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java b/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java index 0bbf100f..651020cd 100644 --- a/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/person/PersonElasticSearchSyncJob.java @@ -29,9 +29,9 @@ /** * This job runs on a daily basis and provides support for faster data search - *

- * Its purpose is to clear down the persons index in ES then repopulates the person data so that - * person search is quicker. + * + *

Its purpose is to clear down the persons index in ES then repopulates the + * person data so that person search is quicker. */ @Component @ManagedResource(objectName = "sync.mbean:name=PersonElasticSearchJob", @@ -56,6 +56,15 @@ public class PersonElasticSearchSyncJob implements RunnableJob { protected int pageSize = 8_000; private Stopwatch mainStopWatch; + /** + * Constructs a new PersonElasticSearchSyncJob with the specified dependencies. + * + * @param namedParameterJdbcTemplate the JDBC template for Named parameter + * @param elasticSearchOperations the elasticSearchOperations + * @param sqlQuerySupplier the supplier for SQL queries + * @param personElasticSearchService the personElasticSearchService + * @param pageSize the page size value + */ public PersonElasticSearchSyncJob(NamedParameterJdbcTemplate namedParameterJdbcTemplate, SqlQuerySupplier sqlQuerySupplier, ElasticsearchOperations elasticSearchOperations, PersonElasticSearchService personElasticSearchService, @@ -139,12 +148,13 @@ protected void run() { int totalRecords = 0; int page = 0; - boolean hasMoreResults = true; deleteIndex(); createIndex(); stopwatch.reset().start(); + boolean hasMoreResults = true; + while (hasMoreResults) { List collectedData = collectData(page, getPageSize()); diff --git a/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java b/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java index 568b4809..675be08f 100644 --- a/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java +++ b/src/main/java/uk/nhs/tis/sync/job/reval/RevalPersonChangedJobTemplate.java @@ -13,9 +13,10 @@ import uk.nhs.tis.sync.model.EntityData; /** - * This is a template abstract class for Revalidation Person job change - *

- * Its purpose is to publish Persons based on their current Placement into Rabbit message queue + * This is a template abstract class for Revalidation Person job change. + * + *

Its purpose is to publish Persons based on their current Placement + * into Rabbit message queue. */ public abstract class RevalPersonChangedJobTemplate extends PersonDateChangeCaptureSyncJobTemplate { diff --git a/src/test/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJobIntegrationTest.java b/src/test/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJobIntegrationTest.java index c945f93b..0915e2bd 100644 --- a/src/test/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJobIntegrationTest.java +++ b/src/test/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJobIntegrationTest.java @@ -36,7 +36,7 @@ public void tearDown() throws Exception { @Test public void testJobRun() throws Exception { - job.PersonPlacementTrainingBodyFullSync(); + job.personPlacementTrainingBodyFullSync(); int maxLoops = 1440, loops = 0; //Loop while the job is running up to 2 hours Thread.sleep(5 * 1000L); diff --git a/src/test/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJobIntegrationTest.java b/src/test/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJobIntegrationTest.java index 4c3b787f..af8bde4d 100644 --- a/src/test/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJobIntegrationTest.java +++ b/src/test/java/uk/nhs/tis/sync/job/PostEmployingBodyTrustJobIntegrationTest.java @@ -38,7 +38,7 @@ public void tearDown() throws Exception { @Test public void testJobRun() throws Exception { - job.PostEmployingBodyTrustFullSync(); + job.postEmployingBodyTrustFullSync(); int maxLoops = 1440, loops = 0; //Loop while the job is running up to 2 hours Thread.sleep(5 * 1000L); diff --git a/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java b/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java index 7d1119d8..40ba86bd 100644 --- a/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java +++ b/src/test/java/uk/nhs/tis/sync/job/TrustAdminSyncJobTemplateUnitTest.java @@ -19,13 +19,11 @@ import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.EntityTransaction; - import org.assertj.core.util.Lists; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; import uk.nhs.tis.sync.model.EntityData; @@ -48,7 +46,8 @@ public class TrustAdminSyncJobTemplateUnitTest { private ApplicationEventPublisher applicationEventPublisherMock; public void instantiateJob(List data) { - testObj = new TrustAdminSyncJobTemplateStub(entityManagerFactoryMock, applicationEventPublisherMock, data); + testObj = new TrustAdminSyncJobTemplateStub(entityManagerFactoryMock, + applicationEventPublisherMock, data); } @Test @@ -118,8 +117,9 @@ static class TrustAdminSyncJobTemplateStub extends TrustAdminSyncJobTemplate collectedData; private boolean firstCall = true; - public TrustAdminSyncJobTemplateStub(EntityManagerFactory entityManagerFactoryMock, ApplicationEventPublisher applicationEventPublisherMock, - List collectedData) { + public TrustAdminSyncJobTemplateStub(EntityManagerFactory entityManagerFactoryMock, + ApplicationEventPublisher applicationEventPublisherMock, + List collectedData) { super(entityManagerFactoryMock, applicationEventPublisherMock); this.entityManagerFactoryMock = entityManagerFactoryMock; this.collectedData = collectedData; @@ -147,7 +147,7 @@ protected void deleteData() { @Override protected List collectData(Map ids, String queryString, - EntityManager entityManager) { + EntityManager entityManager) { if (firstCall) { firstCall = false; return this.collectedData; @@ -157,7 +157,7 @@ protected List collectData(Map ids, String queryString @Override protected int convertData(Set entitiesToSave, List entityData, - EntityManager entityManager) { + EntityManager entityManager) { entityData.forEach(o -> entitiesToSave.add(new Object())); return 0; } From c1fedc96f4674b1cd3ec870e1090ae986e744f38 Mon Sep 17 00:00:00 2001 From: jayanta2018 Date: Tue, 4 Jun 2024 13:14:57 +0100 Subject: [PATCH 7/8] fix: fix few more code smells --- .../tis/sync/event/listener/JobRunningListener.java | 2 +- .../job/PersonPlacementEmployingBodyTrustJob.java | 3 +-- .../sync/job/PersonPlacementTrainingBodyTrustJob.java | 11 +++++++++-- .../uk/nhs/tis/sync/job/PersonRecordStatusJob.java | 3 +-- .../uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java | 2 +- .../job/PostTrainingBodyTrustJobIntegrationTest.java | 2 +- 6 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/main/java/uk/nhs/tis/sync/event/listener/JobRunningListener.java b/src/main/java/uk/nhs/tis/sync/event/listener/JobRunningListener.java index 3c122c5d..26bc38e2 100644 --- a/src/main/java/uk/nhs/tis/sync/event/listener/JobRunningListener.java +++ b/src/main/java/uk/nhs/tis/sync/event/listener/JobRunningListener.java @@ -87,7 +87,7 @@ public void runJobs() { do { Thread.sleep(SLEEP_DURATION); } while (postEmployingBodyTrustJob.isCurrentlyRunning()); - postTrainingBodyTrustJob.PostTrainingBodyTrustFullSync(); + postTrainingBodyTrustJob.postTrainingBodyTrustFullSync(); do { Thread.sleep(SLEEP_DURATION); } while (postTrainingBodyTrustJob.isCurrentlyRunning()); diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java index bc2664f9..21663f19 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementEmployingBodyTrustJob.java @@ -28,8 +28,7 @@ /** * A sync job for updating placement employing body for a person. * - * This job clears the PersonTrust table and links Person with Placement EmployingBody (Trust). - * + *

This job clears the PersonTrust table and links Person with Placement EmployingBody (Trust). */ @Component @ManagedResource(objectName = "sync.mbean:name=PersonPlacementEmployingBodyJob", diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java index efcc4662..5b981340 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java @@ -27,8 +27,7 @@ /** * A sync job for updating placement training body for a person. * - * This job clears the PersonTrust table and links Person with Placement TrainingBody (Trust). - * + *

This job clears the PersonTrust table and links Person with Placement TrainingBody (Trust). */ @Component @ManagedResource(objectName = "sync.mbean:name=PersonPlacementTrainingBodyTrustJob", @@ -42,6 +41,14 @@ public class PersonPlacementTrainingBodyTrustJob extends TrustAdminSyncJobTempla private SqlQuerySupplier sqlQuerySupplier; + /** + * Constructs a new PersonPlacementTrainingBodyTrustJob with the specified dependencies. + * + * @param entityManagerFactory the factory to create EntityManager instances + * @param applicationEventPublisher the publisher for application events, may be null + * @param sqlQuerySupplier the supplier for SQL queries + * @param personRepository the repository for Person entities + */ public PersonPlacementTrainingBodyTrustJob(EntityManagerFactory entityManagerFactory, @Autowired(required = false) ApplicationEventPublisher applicationEventPublisher, PersonRepository personRepository, SqlQuerySupplier sqlQuerySupplier) { diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java index 5d39f974..07457cac 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonRecordStatusJob.java @@ -27,9 +27,8 @@ /** * A sync job for updating training record status for a person. * - * This job sets a Person's (Training Record) Status if their + *

This job sets a Person's (Training Record) Status if their * programme membership(s) started/ended. - * */ @Component @ManagedResource(objectName = "sync.mbean:name=PersonRecordStatusJob", diff --git a/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java index 168a3434..86f9f7fc 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJob.java @@ -48,7 +48,7 @@ public PostTrainingBodyTrustJob(EntityManagerFactory entityManagerFactory, lockAtMostFor = FIFTEEN_MIN) @ManagedOperation( description = "Run sync of the PostTrust table with Post to Training Body Trust") - public void PostTrainingBodyTrustFullSync() { + public void postTrainingBodyTrustFullSync() { runSyncJob(null); } diff --git a/src/test/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJobIntegrationTest.java b/src/test/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJobIntegrationTest.java index 1eebf8ae..5c7a6db5 100644 --- a/src/test/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJobIntegrationTest.java +++ b/src/test/java/uk/nhs/tis/sync/job/PostTrainingBodyTrustJobIntegrationTest.java @@ -38,7 +38,7 @@ public void tearDown() throws Exception { @Test public void testJobRun() throws Exception { - job.PostTrainingBodyTrustFullSync(); + job.postTrainingBodyTrustFullSync(); int maxLoops = 1440, loops = 0; //Loop while the job is running up to 2 hours Thread.sleep(5 * 1000L); From 8f842db493b4e3d20fdd1a59acd5e19252e836fb Mon Sep 17 00:00:00 2001 From: Yafang Deng Date: Mon, 10 Jun 2024 11:45:55 +0100 Subject: [PATCH 8/8] fix:remove unnecessary static variable TIS21-5812 --- pom.xml | 2 +- .../nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 8957a374..c2632aba 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ uk.nhs.tis sync - 1.21.2 + 1.21.3 jar sync Separate Microservice for synchronisation diff --git a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java index 5b981340..3f7ce06d 100644 --- a/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java +++ b/src/main/java/uk/nhs/tis/sync/job/PersonPlacementTrainingBodyTrustJob.java @@ -35,8 +35,6 @@ @Slf4j public class PersonPlacementTrainingBodyTrustJob extends TrustAdminSyncJobTemplate { - private static final int FIFTEEN_MIN = 15 * 60 * 1000; - private PersonRepository personRepository; private SqlQuerySupplier sqlQuerySupplier;