From 951446346448753b1f86089d1a7fe9de14bae121 Mon Sep 17 00:00:00 2001 From: michaeljacob Date: Thu, 23 Oct 2025 11:55:20 +0100 Subject: [PATCH 1/7] resolve code smells --- .../batch/DocumentTaskItemProcessor.java | 3 - .../RemoveSpringBatchHistoryTasklet.java | 5 +- .../stitching/config/BatchConfiguration.java | 49 +++++++----- .../ExceptionHandlingAsyncTaskExecutor.java | 11 +-- .../config/audit/AuditEventConverter.java | 3 +- .../audit/EntityAuditEventListener.java | 41 ++++------ .../security/SecurityConfiguration.java | 12 +-- .../config/security/SecurityUtils.java | 45 +++++------ .../reform/em/stitching/domain/Bundle.java | 78 ++++++++++--------- .../em/stitching/domain/BundleContainer.java | 2 +- .../reform/em/stitching/pdf/PDFOutline.java | 6 +- .../stitching/rest/DocumentTaskResource.java | 4 +- .../service/StringFormattingUtils.java | 3 +- .../service/dto/ByteArrayMultipartFile.java | 2 +- .../service/impl/DmStoreUploaderImpl.java | 6 +- 15 files changed, 128 insertions(+), 142 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/batch/DocumentTaskItemProcessor.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/batch/DocumentTaskItemProcessor.java index 9a802507a..91b95a2d7 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/batch/DocumentTaskItemProcessor.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/batch/DocumentTaskItemProcessor.java @@ -37,9 +37,6 @@ public class DocumentTaskItemProcessor implements ItemProcessor { private final Logger log = LoggerFactory.getLogger(DocumentTaskItemProcessor.class); - private static final String FAILURE_DESCRIPTION_TEMPLATE = - "Document taskId %d, caseId: %s reached max retry count: %d"; - private final DmStoreDownloader dmStoreDownloader; private final DmStoreUploader dmStoreUploader; private final DocumentConversionService documentConverter; diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/batch/RemoveSpringBatchHistoryTasklet.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/batch/RemoveSpringBatchHistoryTasklet.java index 267f9a1a0..97fd04b06 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/batch/RemoveSpringBatchHistoryTasklet.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/batch/RemoveSpringBatchHistoryTasklet.java @@ -69,8 +69,9 @@ public RepeatStatus execute(StepContribution contribution, ChunkContext chunkCon int totalCount = 0; Date date = DateUtils.addMilliseconds(new Date(), -historicRetentionMiliseconds); DateFormat df = new SimpleDateFormat(); - LOG.info("Remove the Spring Batch history before the {}", df.format(date)); - + if (LOG.isInfoEnabled()) { + LOG.info("Remove the Spring Batch history before the {}", df.format(date)); + } int rowCount = jdbcTemplate.update(getQuery(SQL_DELETE_BATCH_STEP_EXECUTION_CONTEXT), date); LOG.info("Deleted rows number from the BATCH_STEP_EXECUTION_CONTEXT table: {}", rowCount); totalCount += rowCount; diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/BatchConfiguration.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/BatchConfiguration.java index 34efc3a77..34a45ab03 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/BatchConfiguration.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/BatchConfiguration.java @@ -8,7 +8,6 @@ import net.javacrumbs.shedlock.provider.jdbctemplate.JdbcTemplateLockProvider; import net.javacrumbs.shedlock.spring.annotation.EnableSchedulerLock; import net.javacrumbs.shedlock.spring.annotation.SchedulerLock; -import org.hibernate.LockOptions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.batch.core.Job; @@ -58,31 +57,23 @@ public class BatchConfiguration { private static final Logger LOGGER = LoggerFactory.getLogger(BatchConfiguration.class); - @Autowired - PlatformTransactionManager transactionManager; + private final PlatformTransactionManager transactionManager; - @Autowired - JobRepository jobRepository; - @Autowired - EntityManagerFactory entityManagerFactory; + private final JobRepository jobRepository; - @Autowired - JobLauncher jobLauncher; + private final EntityManagerFactory entityManagerFactory; - @Autowired - BuildInfo buildInfo; + private final JobLauncher jobLauncher; - @Autowired - DocumentTaskItemProcessor documentTaskItemProcessor; + private final BuildInfo buildInfo; - @Autowired - DocumentTaskCallbackProcessor documentTaskCallbackProcessor; + private final DocumentTaskItemProcessor documentTaskItemProcessor; - @Autowired - JdbcTemplate jdbcTemplate; + private final DocumentTaskCallbackProcessor documentTaskCallbackProcessor; - @Autowired - DocumentTaskRepository documentTaskRepository; + private final JdbcTemplate jdbcTemplate; + + private final DocumentTaskRepository documentTaskRepository; @Value("${spring.batch.historicExecutionsRetentionMilliseconds}") int historicExecutionsRetentionMilliseconds; @@ -106,6 +97,23 @@ public class BatchConfiguration { private Random random = new Random(); + @Autowired + public BatchConfiguration(PlatformTransactionManager transactionManager, JobRepository jobRepository, + EntityManagerFactory entityManagerFactory, JobLauncher jobLauncher, BuildInfo buildInfo, + DocumentTaskItemProcessor documentTaskItemProcessor, + DocumentTaskCallbackProcessor documentTaskCallbackProcessor, JdbcTemplate jdbcTemplate, + DocumentTaskRepository documentTaskRepository) { + this.transactionManager = transactionManager; + this.jobRepository = jobRepository; + this.entityManagerFactory = entityManagerFactory; + this.jobLauncher = jobLauncher; + this.buildInfo = buildInfo; + this.documentTaskItemProcessor = documentTaskItemProcessor; + this.documentTaskCallbackProcessor = documentTaskCallbackProcessor; + this.jdbcTemplate = jdbcTemplate; + this.documentTaskRepository = documentTaskRepository; + } + @Scheduled(fixedDelayString = "${spring.batch.document-task-milliseconds}") @SchedulerLock(name = "${task.env}") public void schedule() throws JobParametersInvalidException, @@ -213,8 +221,7 @@ public Query createQuery() { + " where t.taskState = 'NEW' and " + " t.version <= " + buildInfo.getBuildNumber() + " order by t.createdDate") - .setLockMode(LockModeType.PESSIMISTIC_WRITE) - .setHint("jakarta.persistence.lock.timeout", LockOptions.SKIP_LOCKED); + .setLockMode(LockModeType.PESSIMISTIC_WRITE); } @Override diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/ExceptionHandlingAsyncTaskExecutor.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/ExceptionHandlingAsyncTaskExecutor.java index fb4c1e5e7..15d097d0f 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/ExceptionHandlingAsyncTaskExecutor.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/ExceptionHandlingAsyncTaskExecutor.java @@ -46,11 +46,6 @@ public void execute(Runnable task) { executor.execute(createWrappedRunnable(task)); } - @Override - public void execute(Runnable task, long startTimeout) { - executor.execute(createWrappedRunnable(task), startTimeout); - } - private Callable createCallable(final Callable task) { return () -> { try { @@ -88,16 +83,14 @@ public Future submit(Callable task) { @Override public void destroy() throws Exception { - if (executor instanceof DisposableBean) { - DisposableBean bean = (DisposableBean) executor; + if (executor instanceof DisposableBean bean) { bean.destroy(); } } @Override public void afterPropertiesSet() throws Exception { - if (executor instanceof InitializingBean) { - InitializingBean bean = (InitializingBean) executor; + if (executor instanceof InitializingBean bean) { bean.afterPropertiesSet(); } } diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/audit/AuditEventConverter.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/audit/AuditEventConverter.java index 24956086d..8ce3a20ce 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/audit/AuditEventConverter.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/audit/AuditEventConverter.java @@ -77,8 +77,7 @@ public Map convertDataToStrings(Map data) { if (data != null) { for (Map.Entry entry : data.entrySet()) { // Extract the data that will be saved. - if (entry.getValue() instanceof WebAuthenticationDetails) { - WebAuthenticationDetails authenticationDetails = (WebAuthenticationDetails) entry.getValue(); + if (entry.getValue() instanceof WebAuthenticationDetails authenticationDetails) { results.put("remoteAddress", authenticationDetails.getRemoteAddress()); results.put("sessionId", authenticationDetails.getSessionId()); } else { diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/audit/EntityAuditEventListener.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/audit/EntityAuditEventListener.java index 765825a98..b6e4c1ede 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/audit/EntityAuditEventListener.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/audit/EntityAuditEventListener.java @@ -18,45 +18,32 @@ public class EntityAuditEventListener extends AuditingEntityListener { @PostPersist public void onPostCreate(AbstractAuditingEntity target) { - try { - AsyncEntityAuditEventWriter asyncEntityAuditEventWriter = - beanFactory.getBean(AsyncEntityAuditEventWriter.class); - asyncEntityAuditEventWriter.writeAuditEvent(target, EntityAuditAction.CREATE); - } catch (NoSuchBeanDefinitionException e) { - log.error("No bean found for AsyncEntityAuditEventWriter"); - } catch (Exception e) { - log.error("Exception while persisting create audit entity {}", e.toString()); - } + writeAuditEvent(target, EntityAuditAction.CREATE); } @PostUpdate public void onPostUpdate(AbstractAuditingEntity target) { - try { - AsyncEntityAuditEventWriter asyncEntityAuditEventWriter = - beanFactory.getBean(AsyncEntityAuditEventWriter.class); - asyncEntityAuditEventWriter.writeAuditEvent(target, EntityAuditAction.UPDATE); - } catch (NoSuchBeanDefinitionException e) { - log.error("No bean found for AsyncEntityAuditEventWriter"); - } catch (Exception e) { - log.error("Exception while persisting update audit entity {}", e.toString()); - } + writeAuditEvent(target, EntityAuditAction.UPDATE); } @PostRemove public void onPostRemove(AbstractAuditingEntity target) { + writeAuditEvent(target, EntityAuditAction.DELETE); + } + + static void setBeanFactory(BeanFactory beanFactory) { + EntityAuditEventListener.beanFactory = beanFactory; + } + + private void writeAuditEvent(AbstractAuditingEntity target, EntityAuditAction action) { try { AsyncEntityAuditEventWriter asyncEntityAuditEventWriter = - beanFactory.getBean(AsyncEntityAuditEventWriter.class); - asyncEntityAuditEventWriter.writeAuditEvent(target, EntityAuditAction.DELETE); + beanFactory.getBean(AsyncEntityAuditEventWriter.class); + asyncEntityAuditEventWriter.writeAuditEvent(target, action); } catch (NoSuchBeanDefinitionException e) { log.error("No bean found for AsyncEntityAuditEventWriter"); } catch (Exception e) { - log.error("Exception while persisting delete audit entity {}", e.toString()); + log.error("Exception while persisting {} audit entity: {}", action.value(), e.toString()); } } - - static void setBeanFactory(BeanFactory beanFactory) { - EntityAuditEventListener.beanFactory = beanFactory; - } - -} +} \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/security/SecurityConfiguration.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/security/SecurityConfiguration.java index 52920f278..55dd37956 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/security/SecurityConfiguration.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/security/SecurityConfiguration.java @@ -1,6 +1,5 @@ package uk.gov.hmcts.reform.em.stitching.config.security; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -18,24 +17,21 @@ import org.springframework.security.oauth2.jwt.JwtDecoders; import org.springframework.security.oauth2.jwt.JwtTimestampValidator; import org.springframework.security.oauth2.jwt.NimbusJwtDecoder; -import org.springframework.security.oauth2.server.resource.web.authentication.BearerTokenAuthenticationFilter; +import org.springframework.security.oauth2.server.resource.web.authentication.BearerTokenAuthenticationFilter; import org.springframework.security.web.SecurityFilterChain; import uk.gov.hmcts.reform.authorisation.filters.ServiceAuthFilter; @Configuration @EnableWebSecurity -@EnableMethodSecurity(prePostEnabled = true) +@EnableMethodSecurity() public class SecurityConfiguration { @Value("${spring.security.oauth2.client.provider.oidc.issuer-uri}") private String issuerUri; - @Autowired - private ServiceAuthFilter serviceAuthFilter; - @Bean public WebSecurityCustomizer webSecurityCustomizer() { - return (web) -> web.ignoring().requestMatchers("/swagger-ui.html", + return web -> web.ignoring().requestMatchers("/swagger-ui.html", "/swagger-ui/**", "/swagger-resources/**", "/v3/**", @@ -48,7 +44,7 @@ public WebSecurityCustomizer webSecurityCustomizer() { } @Bean - public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { + public SecurityFilterChain filterChain(HttpSecurity http, ServiceAuthFilter serviceAuthFilter) throws Exception { http.csrf(AbstractHttpConfigurer::disable) .formLogin(AbstractHttpConfigurer::disable) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/security/SecurityUtils.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/security/SecurityUtils.java index cff77cdbe..628929cbc 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/config/security/SecurityUtils.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/config/security/SecurityUtils.java @@ -1,6 +1,7 @@ package uk.gov.hmcts.reform.em.stitching.config.security; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContext; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.core.userdetails.UserDetails; @@ -9,8 +10,8 @@ import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; import org.springframework.stereotype.Service; import uk.gov.hmcts.reform.em.stitching.repository.IdamRepository; +import uk.gov.hmcts.reform.idam.client.models.UserInfo; -import java.util.Map; import java.util.Optional; import static org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames.ACCESS_TOKEN; @@ -37,29 +38,29 @@ public SecurityUtils(final IdamRepository idamRepository) { */ public Optional getCurrentUserLogin() { SecurityContext securityContext = SecurityContextHolder.getContext(); + return Optional.ofNullable(securityContext.getAuthentication()) - .map(authentication -> { - if (authentication.getPrincipal() instanceof UserDetails) { - UserDetails springSecurityUser = (UserDetails) authentication.getPrincipal(); - return springSecurityUser.getUsername(); - } else if (authentication.getPrincipal() instanceof String) { - return (String) authentication.getPrincipal(); - } else if (authentication instanceof JwtAuthenticationToken) { - Jwt jwt = ((JwtAuthenticationToken) authentication).getToken(); - if (ACCESS_TOKEN.equals(jwt.getClaim(TOKEN_NAME))) { - uk.gov.hmcts.reform.idam.client.models.UserInfo userInfo = - idamRepository.getUserInfo(jwt.getTokenValue()); - return userInfo.getUid(); - } - } else if (authentication.getPrincipal() instanceof DefaultOidcUser) { - Map attributes = - ((DefaultOidcUser) authentication.getPrincipal()).getAttributes(); - if (attributes.containsKey("preferred_username")) { - return (String) attributes.get("preferred_username"); - } + .map(authentication -> switch (authentication) { + case Authentication auth when auth.getPrincipal() instanceof UserDetails userDetails -> + userDetails.getUsername(); + + case Authentication auth when auth.getPrincipal() instanceof String s -> s; + + case JwtAuthenticationToken jwtAuth -> { + Jwt jwt = jwtAuth.getToken(); + if (jwt.hasClaim(TOKEN_NAME) + && jwt.getClaim(TOKEN_NAME).equals(ACCESS_TOKEN)) { + UserInfo userInfo = idamRepository.getUserInfo(jwt.getTokenValue()); + yield userInfo.getUid(); } - return null; - }); + yield null; + } + + case Authentication auth when auth.getPrincipal() instanceof DefaultOidcUser oidcUser -> + (String) oidcUser.getAttributes().get("preferred_username"); + + default -> null; + }); } } diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java index 7ec076af6..ac576ab00 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java @@ -27,12 +27,13 @@ import java.io.IOException; import java.io.Serializable; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.function.Function; -import java.util.stream.Collectors; +import java.util.function.ToIntFunction; import java.util.stream.Stream; @Entity @@ -260,11 +261,11 @@ public void setDocumentImage(DocumentImage documentImage) { public String toString() { return "Bundle(id=" + this.getId() + ", bundleTitle=" + this.getBundleTitle() - + ", description=" + this.getDescription() + ", stitchedDocumentURI=" + this.getStitchedDocumentURI() - + ", stitchStatus=" + this.getStitchStatus() - + ", fileName=" + this.getFileName() + ", hasTableOfContents=" - + this.hasTableOfContents + ", hasCoversheets=" + this.hasCoversheets + ", hasFolderCoversheets=" - + this.hasFolderCoversheets + ")"; + + ", description=" + this.getDescription() + ", stitchedDocumentURI=" + this.getStitchedDocumentURI() + + ", stitchStatus=" + this.getStitchStatus() + + ", fileName=" + this.getFileName() + ", hasTableOfContents=" + + this.hasTableOfContents + ", hasCoversheets=" + this.hasCoversheets + ", hasFolderCoversheets=" + + this.hasFolderCoversheets + ")"; } @Transient @@ -273,13 +274,13 @@ public Integer getNumberOfSubtitles(SortableBundleItem container, if (container.getSortedDocuments().count() == documentBundledFilesRef.size()) { List docsToClose = new ArrayList<>(); int subtitles = extractDocumentOutlineStream(container, documentBundledFilesRef, docsToClose) - .mapToInt(pdDocumentOutline -> getItemsFromOutline.apply(pdDocumentOutline)).sum(); + .mapToInt(getItemsFromOutline) + .sum(); closeDocuments(docsToClose); return subtitles; } else { return 0; } - } @Transient @@ -287,9 +288,9 @@ public List getSubtitles(SortableBundleItem container, Map docsToClose = new ArrayList<>(); List subtitles = extractDocumentOutlineStream(container, documentBundledFilesRef, docsToClose) - .map(pdDocumentOutline -> getItemTitlesFromOutline.apply(pdDocumentOutline)) + .map(getItemTitlesFromOutline) .flatMap(List::stream) - .collect(Collectors.toList()); + .toList(); closeDocuments(docsToClose); return subtitles; } @@ -328,12 +329,12 @@ private void closeDocuments(List docsToClose) { @Transient private PDDocumentOutline extractDocumentOutline( - BundleDocument bd, - Map documentContainingFiles) { + BundleDocument bd, + Map documentContainingFiles) { try (PDDocument pdDocument = Loader.loadPDF(documentContainingFiles.get(bd))) { return pdDocument - .getDocumentCatalog() - .getDocumentOutline(); + .getDocumentCatalog() + .getDocumentOutline(); } catch (IOException e) { e.getStackTrace(); } @@ -341,35 +342,36 @@ private PDDocumentOutline extractDocumentOutline( } @Transient - private Function getItemsFromOutline = (outline) -> { - ArrayList firstSiblings = new ArrayList<>(); - PDOutlineItem anySubtitlesForItem = outline.getFirstChild(); - - while (anySubtitlesForItem != null) { - firstSiblings.add(anySubtitlesForItem.getTitle()); - anySubtitlesForItem = anySubtitlesForItem.getNextSibling(); - } - - return firstSiblings.size(); - }; + private final transient ToIntFunction getItemsFromOutline = outline -> { + if (Objects.isNull(outline)) { + return 0; + } + int count = 0; + PDOutlineItem current = outline.getFirstChild(); + while (Objects.nonNull(current)) { + count++; + current = current.getNextSibling(); + } + return count; + }; @Transient - private Function> getItemTitlesFromOutline = (outline) -> { - ArrayList firstSiblings = new ArrayList<>(); - PDOutlineItem anySubtitlesForItem = outline.getFirstChild(); - - while (anySubtitlesForItem != null) { - firstSiblings.add(anySubtitlesForItem.getTitle()); - anySubtitlesForItem = anySubtitlesForItem.getNextSibling(); - } - - return firstSiblings; - }; + private final transient Function> getItemTitlesFromOutline = outline -> { + if (Objects.isNull(outline)) { + return Collections.emptyList(); + } + List titles = new ArrayList<>(); + PDOutlineItem current = outline.getFirstChild(); + while (Objects.nonNull(current)) { + titles.add(current.getTitle()); + current = current.getNextSibling(); + } + return titles; + }; @Override @Transient public BundleItemType getType() { return BundleItemType.BUNDLE; } -} - +} \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleContainer.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleContainer.java index bcfd24496..33f860993 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleContainer.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleContainer.java @@ -13,7 +13,7 @@ default Stream getNestedFolders() { return Stream.concat( getFolders().stream(), getFolders().stream().flatMap(BundleContainer::getNestedFolders) - ).filter(f -> hasAnyDoc(f)); + ).filter(this::hasAnyDoc); } private boolean hasAnyDoc(BundleFolder folder) { diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/pdf/PDFOutline.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/pdf/PDFOutline.java index 23fb27090..88079da03 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/pdf/PDFOutline.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/pdf/PDFOutline.java @@ -190,10 +190,8 @@ public PDOutlineItem removeNullObject(PDOutlineItem outline) { } List removeCOSName = new ArrayList<>(); for (Map.Entry entry : outline.getCOSObject().entrySet()) { - if (entry.getValue() instanceof COSObject csCosObject) { - if (Objects.isNull(csCosObject.getObject())) { - removeCOSName.add(entry.getKey()); - } + if (entry.getValue() instanceof COSObject csCosObject && Objects.isNull(csCosObject.getObject())) { + removeCOSName.add(entry.getKey()); } } diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/rest/DocumentTaskResource.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/rest/DocumentTaskResource.java index d9d5ce51f..3154d4502 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/rest/DocumentTaskResource.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/rest/DocumentTaskResource.java @@ -79,8 +79,10 @@ public ResponseEntity createDocumentTask( @RequestHeader(value = "Authorization") String authorisationHeader, HttpServletRequest request) throws URISyntaxException, DocumentTaskProcessingException { - log.debug("REST request to save DocumentTask : {}, with headers {}", documentTaskDTO, + if (log.isDebugEnabled()) { + log.debug("REST request to save DocumentTask : {}, with headers {}", documentTaskDTO, Arrays.toString(Collections.toArray(request.getHeaderNames(), new String[]{}))); + } if (Objects.nonNull(documentTaskDTO.getId())) { throw new BadRequestAlertException( diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/StringFormattingUtils.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/StringFormattingUtils.java index 1ac35e208..a3b063523 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/StringFormattingUtils.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/StringFormattingUtils.java @@ -1,6 +1,7 @@ package uk.gov.hmcts.reform.em.stitching.service; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Strings; public class StringFormattingUtils { @@ -13,7 +14,7 @@ private StringFormattingUtils() { public static String generateFileName(String fileName) { if (StringUtils.isNotBlank(fileName)) { - return StringUtils.endsWithIgnoreCase(fileName, SUFFIX) + return Strings.CI.endsWith(fileName, SUFFIX) ? fileName : fileName + SUFFIX; } else { return PREFIX + System.currentTimeMillis() + SUFFIX; diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/ByteArrayMultipartFile.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/ByteArrayMultipartFile.java index 0a44463fa..10e52dac4 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/ByteArrayMultipartFile.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/ByteArrayMultipartFile.java @@ -25,7 +25,7 @@ public String getName() { @Override public String getOriginalFilename() { - return name; + return getName(); } @Override diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/impl/DmStoreUploaderImpl.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/impl/DmStoreUploaderImpl.java index aa5108c9d..1068782ce 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/impl/DmStoreUploaderImpl.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/impl/DmStoreUploaderImpl.java @@ -110,9 +110,11 @@ private void uploadNewDocument(File file, DocumentTask documentTask) throws Docu private void uploadNewDocumentVersion(File file, DocumentTask documentTask) throws DocumentTaskProcessingException { Response response = null; try { - log.debug("Uploading new document version '{}' for {}", + if (log.isDebugEnabled()) { + log.debug("Uploading new document version '{}' for {}", StringFormattingUtils.generateFileName(documentTask.getBundle().getFileName()), - documentTask); + documentTask); + } MultipartBody requestBody = new MultipartBody .Builder() From 05601c6c6291a96900de64a383907c6f82ac0a8e Mon Sep 17 00:00:00 2001 From: michaeljacob Date: Thu, 23 Oct 2025 12:04:40 +0100 Subject: [PATCH 2/7] checkstyle --- .../reform/em/stitching/domain/Bundle.java | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java index ac576ab00..f7d5aead3 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java @@ -343,31 +343,31 @@ private PDDocumentOutline extractDocumentOutline( @Transient private final transient ToIntFunction getItemsFromOutline = outline -> { - if (Objects.isNull(outline)) { - return 0; - } - int count = 0; - PDOutlineItem current = outline.getFirstChild(); - while (Objects.nonNull(current)) { - count++; - current = current.getNextSibling(); - } - return count; - }; + if (Objects.isNull(outline)) { + return 0; + } + int count = 0; + PDOutlineItem current = outline.getFirstChild(); + while (Objects.nonNull(current)) { + count++; + current = current.getNextSibling(); + } + return count; + }; @Transient private final transient Function> getItemTitlesFromOutline = outline -> { - if (Objects.isNull(outline)) { - return Collections.emptyList(); - } - List titles = new ArrayList<>(); - PDOutlineItem current = outline.getFirstChild(); - while (Objects.nonNull(current)) { - titles.add(current.getTitle()); - current = current.getNextSibling(); - } - return titles; - }; + if (Objects.isNull(outline)) { + return Collections.emptyList(); + } + List titles = new ArrayList<>(); + PDOutlineItem current = outline.getFirstChild(); + while (Objects.nonNull(current)) { + titles.add(current.getTitle()); + current = current.getNextSibling(); + } + return titles; + }; @Override @Transient From d978df5376e2d74f51e2cd739907e5146a89f6cf Mon Sep 17 00:00:00 2001 From: michaeljacob Date: Thu, 23 Oct 2025 14:19:09 +0100 Subject: [PATCH 3/7] resolve code smells --- .../java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java | 4 ++-- .../gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java index f7d5aead3..82c374505 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java @@ -67,11 +67,11 @@ public class Bundle extends AbstractAuditingEntity implements SortableBundleItem @Type(JsonType.class) @Column(columnDefinition = "jsonb") - private DocumentImage documentImage; + private transient DocumentImage documentImage; @Type(JsonType.class) @Column(columnDefinition = "jsonb") - private JsonNode coverpageTemplateData; + private transient JsonNode coverpageTemplateData; @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) @Fetch(FetchMode.SUBSELECT) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java index 3d87c9518..405744ed4 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java @@ -30,7 +30,7 @@ public class BundleDTO extends AbstractAuditingDTO implements Serializable { @JsonInclude(JsonInclude.Include.NON_NULL) private String fileNameIdentifier; private String coverpageTemplate; - private JsonNode coverpageTemplateData; + private transient JsonNode coverpageTemplateData; private PageNumberFormat pageNumberFormat = PageNumberFormat.NUMBER_OF_PAGES; private boolean hasTableOfContents = true; private boolean hasCoversheets = true; @@ -39,7 +39,7 @@ public class BundleDTO extends AbstractAuditingDTO implements Serializable { private String hashToken; @JsonInclude(JsonInclude.Include.NON_NULL) - private DocumentImage documentImage; + private transient DocumentImage documentImage; @JsonInclude(JsonInclude.Include.NON_NULL) private Boolean enableEmailNotification; From 47e94ec37d03fc7e7db3ce0f446a5fe909c44603 Mon Sep 17 00:00:00 2001 From: michaeljacob Date: Fri, 24 Oct 2025 09:41:32 +0100 Subject: [PATCH 4/7] Revert "resolve code smells" This reverts commit d978df5376e2d74f51e2cd739907e5146a89f6cf. --- .../java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java | 4 ++-- .../gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java index 82c374505..f7d5aead3 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java @@ -67,11 +67,11 @@ public class Bundle extends AbstractAuditingEntity implements SortableBundleItem @Type(JsonType.class) @Column(columnDefinition = "jsonb") - private transient DocumentImage documentImage; + private DocumentImage documentImage; @Type(JsonType.class) @Column(columnDefinition = "jsonb") - private transient JsonNode coverpageTemplateData; + private JsonNode coverpageTemplateData; @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) @Fetch(FetchMode.SUBSELECT) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java index 405744ed4..3d87c9518 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java @@ -30,7 +30,7 @@ public class BundleDTO extends AbstractAuditingDTO implements Serializable { @JsonInclude(JsonInclude.Include.NON_NULL) private String fileNameIdentifier; private String coverpageTemplate; - private transient JsonNode coverpageTemplateData; + private JsonNode coverpageTemplateData; private PageNumberFormat pageNumberFormat = PageNumberFormat.NUMBER_OF_PAGES; private boolean hasTableOfContents = true; private boolean hasCoversheets = true; @@ -39,7 +39,7 @@ public class BundleDTO extends AbstractAuditingDTO implements Serializable { private String hashToken; @JsonInclude(JsonInclude.Include.NON_NULL) - private transient DocumentImage documentImage; + private DocumentImage documentImage; @JsonInclude(JsonInclude.Include.NON_NULL) private Boolean enableEmailNotification; From e15566e6d7a9f094bce344c7f8d59460a2fa51ad Mon Sep 17 00:00:00 2001 From: michaeljacob Date: Fri, 24 Oct 2025 09:59:41 +0100 Subject: [PATCH 5/7] suppress code smells --- .../java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java | 2 ++ .../uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java index f7d5aead3..6c9afd367 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/Bundle.java @@ -38,6 +38,8 @@ @Entity @Table(name = "bundle") +@SuppressWarnings("squid:S1948") +// Suppress SonarQube warning as serialization is handled by the JsonType converter for the jsonb column. public class Bundle extends AbstractAuditingEntity implements SortableBundleItem, Serializable, BundleContainer { @Id diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java index 3d87c9518..8525be9f1 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/service/dto/BundleDTO.java @@ -15,6 +15,8 @@ import static uk.gov.hmcts.reform.em.stitching.domain.enumeration.PaginationStyle.off; @ToString(callSuper = true) +@SuppressWarnings("squid:S1948") +// Suppress SonarQube warning as serialization is handled by the JsonType converter for the jsonb column. public class BundleDTO extends AbstractAuditingDTO implements Serializable { @JsonIgnore From b6a0005c9ffe9be907365bfd841fe39e03fcf79a Mon Sep 17 00:00:00 2001 From: michaeljacob Date: Fri, 24 Oct 2025 11:01:01 +0100 Subject: [PATCH 6/7] resolve code smell --- .../reform/em/stitching/domain/BundleDocument.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleDocument.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleDocument.java index 07c097e8a..69299a3b9 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleDocument.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleDocument.java @@ -10,6 +10,7 @@ import java.io.Serializable; import java.util.Comparator; +import java.util.Objects; import java.util.stream.Stream; @Entity @@ -105,4 +106,16 @@ public int compareTo(BundleDocument other) { .thenComparing(BundleDocument::getId, Comparator.nullsFirst(Long::compare)) .compare(this, other); } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + BundleDocument that = (BundleDocument) o; + return Objects.equals(id, that.id); + } + + @Override + public int hashCode() { + return Objects.hashCode(id); + } } From 47d41b550d222b443ab026e1597fdabbf325e713 Mon Sep 17 00:00:00 2001 From: michaeljacob Date: Fri, 24 Oct 2025 11:27:43 +0100 Subject: [PATCH 7/7] suppress code smell --- .../reform/em/stitching/domain/BundleDocument.java | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleDocument.java b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleDocument.java index 69299a3b9..09cbb85a9 100644 --- a/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleDocument.java +++ b/src/main/java/uk/gov/hmcts/reform/em/stitching/domain/BundleDocument.java @@ -10,11 +10,11 @@ import java.io.Serializable; import java.util.Comparator; -import java.util.Objects; import java.util.stream.Stream; @Entity @Table(name = "bundle_document") +@SuppressWarnings("squid:S1210") //Note: this class has a natural ordering that is inconsistent with equals. public class BundleDocument extends AbstractAuditingEntity implements SortableBundleItem, Serializable, Comparable { @@ -106,16 +106,4 @@ public int compareTo(BundleDocument other) { .thenComparing(BundleDocument::getId, Comparator.nullsFirst(Long::compare)) .compare(this, other); } - - @Override - public boolean equals(Object o) { - if (o == null || getClass() != o.getClass()) return false; - BundleDocument that = (BundleDocument) o; - return Objects.equals(id, that.id); - } - - @Override - public int hashCode() { - return Objects.hashCode(id); - } }