From 1904b9b8e507bfc53e09ad2115f2fb8b408694aa Mon Sep 17 00:00:00 2001 From: Duncan Crawford Date: Sat, 31 May 2025 20:37:01 +0100 Subject: [PATCH] add code analysis and various fixes --- .github/pmd-ruleset.xml | 18 ++++++ .github/workflows/code-analysis.yml | 59 +++++++++++++++++++ .../hmcts/cp/config/OpenAPIConfiguration.java | 2 +- .../controllers/CourtScheduleController.java | 20 ++++--- .../controllers/GlobalExceptionHandler.java | 12 ++-- .../repositories/CourtScheduleRepository.java | 1 + .../InMemoryCourtScheduleRepositoryImpl.java | 4 +- .../cp/services/CourtScheduleService.java | 12 ++-- 8 files changed, 104 insertions(+), 24 deletions(-) create mode 100644 .github/pmd-ruleset.xml create mode 100644 .github/workflows/code-analysis.yml diff --git a/.github/pmd-ruleset.xml b/.github/pmd-ruleset.xml new file mode 100644 index 0000000..fa591fa --- /dev/null +++ b/.github/pmd-ruleset.xml @@ -0,0 +1,18 @@ + + + + Custom ruleset including exclusions + + + + + + + + + + \ No newline at end of file diff --git a/.github/workflows/code-analysis.yml b/.github/workflows/code-analysis.yml new file mode 100644 index 0000000..6c0b61e --- /dev/null +++ b/.github/workflows/code-analysis.yml @@ -0,0 +1,59 @@ +name: Code analysis + +on: + pull_request: + branches: + - master + - main + +jobs: + pmd-analysis: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Install PMD CLI + run: | + curl -L -o pmd-dist-7.13.0-bin.zip https://github.com/pmd/pmd/releases/download/pmd_releases%2F7.13.0/pmd-dist-7.13.0-bin.zip + unzip pmd-dist-7.13.0-bin.zip + mv pmd-bin-7.13.0 /opt/pmd + echo "/opt/pmd/bin" >> $GITHUB_PATH + + - name: Run PMD CLI analysis + run: | + mkdir -p build/reports/pmd + + set +e + /opt/pmd/bin/pmd check \ + --dir src/main/java \ + --rulesets \ + .github/pmd-ruleset.xml \ + --format html \ + -r build/reports/pmd/pmd-report.html + PMD_EXIT_CODE=$? + set -e + + if [[ "$PMD_EXIT_CODE" -eq 1 ]]; then + echo "Unexpected internal error during PMD execution" + exit 1 + elif [[ "$PMD_EXIT_CODE" -eq 2 ]]; then + echo "PMD CLI usage error" + exit 1 + fi + echo "PMD_EXIT_CODE=$PMD_EXIT_CODE" >> $GITHUB_ENV + + - name: Upload static analysis report on failure + if: env.PMD_EXIT_CODE != '0' + uses: actions/upload-artifact@v4 + with: + name: pmd-report + path: build/reports/pmd/pmd-report.html + + - name: Check for static code analysis violations + run: | + if [[ "$PMD_EXIT_CODE" -eq 0 ]]; then + echo "No PMD violations" + else + echo "PMD violations found" + exit 1 + fi diff --git a/src/main/java/uk/gov/hmcts/cp/config/OpenAPIConfiguration.java b/src/main/java/uk/gov/hmcts/cp/config/OpenAPIConfiguration.java index adca540..be9e3ec 100644 --- a/src/main/java/uk/gov/hmcts/cp/config/OpenAPIConfiguration.java +++ b/src/main/java/uk/gov/hmcts/cp/config/OpenAPIConfiguration.java @@ -7,7 +7,7 @@ @Configuration public class OpenAPIConfiguration { - private OpenAPIConfigurationLoader openAPIConfigLoader = new OpenAPIConfigurationLoader(); + private final OpenAPIConfigurationLoader openAPIConfigLoader = new OpenAPIConfigurationLoader(); @Bean public OpenAPI openAPI() { diff --git a/src/main/java/uk/gov/hmcts/cp/controllers/CourtScheduleController.java b/src/main/java/uk/gov/hmcts/cp/controllers/CourtScheduleController.java index d435954..0bfa09f 100644 --- a/src/main/java/uk/gov/hmcts/cp/controllers/CourtScheduleController.java +++ b/src/main/java/uk/gov/hmcts/cp/controllers/CourtScheduleController.java @@ -14,32 +14,34 @@ @RestController public class CourtScheduleController implements CourtScheduleApi { - private static final Logger log = LoggerFactory.getLogger(CourtScheduleController.class); + private static final Logger LOG = LoggerFactory.getLogger(CourtScheduleController.class); private final CourtScheduleService courtScheduleService; - public CourtScheduleController(CourtScheduleService courtScheduleService) { + public CourtScheduleController(final CourtScheduleService courtScheduleService) { this.courtScheduleService = courtScheduleService; } @Override - public ResponseEntity getCourtScheduleByCaseUrn(String caseUrn) { - String sanitizedCaseUrn; - CourtScheduleResponse courtScheduleResponse; + public ResponseEntity getCourtScheduleByCaseUrn(final String caseUrn) { + final String sanitizedCaseUrn; + final CourtScheduleResponse courtScheduleResponse; try { sanitizedCaseUrn = sanitizeCaseUrn(caseUrn); courtScheduleResponse = courtScheduleService.getCourtScheduleByCaseUrn(sanitizedCaseUrn); } catch (ResponseStatusException e) { - log.error(e.getMessage()); + LOG.atError().log(e.getMessage()); throw e; } - log.debug("Found court schedule for caseUrn: {}", sanitizedCaseUrn); + LOG.atDebug().log("Found court schedule for caseUrn: {}", sanitizedCaseUrn); return ResponseEntity.ok() .contentType(MediaType.APPLICATION_JSON) .body(courtScheduleResponse); } - private String sanitizeCaseUrn(String urn) { - if (urn == null) throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "caseUrn is required"); + private String sanitizeCaseUrn(final String urn) { + if (urn == null) { + throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "caseUrn is required"); + } return StringEscapeUtils.escapeHtml4(urn); } } diff --git a/src/main/java/uk/gov/hmcts/cp/controllers/GlobalExceptionHandler.java b/src/main/java/uk/gov/hmcts/cp/controllers/GlobalExceptionHandler.java index 27834a5..970d308 100644 --- a/src/main/java/uk/gov/hmcts/cp/controllers/GlobalExceptionHandler.java +++ b/src/main/java/uk/gov/hmcts/cp/controllers/GlobalExceptionHandler.java @@ -16,21 +16,21 @@ public class GlobalExceptionHandler { private final Tracer tracer; - public GlobalExceptionHandler(Tracer tracer) { + public GlobalExceptionHandler(final Tracer tracer) { this.tracer = tracer; } @ExceptionHandler(ResponseStatusException.class) - public ResponseEntity handleResponseStatusException(ResponseStatusException e) { - ErrorResponse error = ErrorResponse.builder() - .error(String.valueOf(e.getStatusCode().value())) - .message(e.getReason() != null ? e.getReason() : e.getMessage()) + public ResponseEntity handleResponseStatusException(final ResponseStatusException responseStatusException) { + final ErrorResponse error = ErrorResponse.builder() + .error(String.valueOf(responseStatusException.getStatusCode().value())) + .message(responseStatusException.getReason() != null ? responseStatusException.getReason() : responseStatusException.getMessage()) .timestamp(OffsetDateTime.now(ZoneOffset.UTC)) .traceId(Objects.requireNonNull(tracer.currentSpan()).context().traceId()) .build(); return ResponseEntity - .status(e.getStatusCode()) + .status(responseStatusException.getStatusCode()) .body(error); } } \ No newline at end of file diff --git a/src/main/java/uk/gov/hmcts/cp/repositories/CourtScheduleRepository.java b/src/main/java/uk/gov/hmcts/cp/repositories/CourtScheduleRepository.java index b676193..c60038f 100644 --- a/src/main/java/uk/gov/hmcts/cp/repositories/CourtScheduleRepository.java +++ b/src/main/java/uk/gov/hmcts/cp/repositories/CourtScheduleRepository.java @@ -3,6 +3,7 @@ import org.springframework.stereotype.Repository; import uk.gov.hmcts.cp.openapi.model.CourtScheduleResponse; +@FunctionalInterface @Repository public interface CourtScheduleRepository { diff --git a/src/main/java/uk/gov/hmcts/cp/repositories/InMemoryCourtScheduleRepositoryImpl.java b/src/main/java/uk/gov/hmcts/cp/repositories/InMemoryCourtScheduleRepositoryImpl.java index 9b45914..4c867e0 100644 --- a/src/main/java/uk/gov/hmcts/cp/repositories/InMemoryCourtScheduleRepositoryImpl.java +++ b/src/main/java/uk/gov/hmcts/cp/repositories/InMemoryCourtScheduleRepositoryImpl.java @@ -13,8 +13,8 @@ @Component public class InMemoryCourtScheduleRepositoryImpl implements CourtScheduleRepository { - public CourtScheduleResponse getCourtScheduleByCaseUrn(String caseUrn) { - CourtScheduleResponseCourtScheduleInnerHearingsInner courtScheduleHearing = CourtScheduleResponseCourtScheduleInnerHearingsInner.builder() + public CourtScheduleResponse getCourtScheduleByCaseUrn(final String caseUrn) { + final CourtScheduleResponseCourtScheduleInnerHearingsInner courtScheduleHearing = CourtScheduleResponseCourtScheduleInnerHearingsInner.builder() .hearingId(UUID.randomUUID().toString()) .listNote("Requires interpreter") .hearingDescription("Sentencing for theft case") diff --git a/src/main/java/uk/gov/hmcts/cp/services/CourtScheduleService.java b/src/main/java/uk/gov/hmcts/cp/services/CourtScheduleService.java index 5e0677a..b20029e 100644 --- a/src/main/java/uk/gov/hmcts/cp/services/CourtScheduleService.java +++ b/src/main/java/uk/gov/hmcts/cp/services/CourtScheduleService.java @@ -14,18 +14,18 @@ @RequiredArgsConstructor public class CourtScheduleService { - private static final Logger log = LoggerFactory.getLogger(CourtScheduleService.class); + private static final Logger LOG = LoggerFactory.getLogger(CourtScheduleService.class); private final CourtScheduleRepository courtScheduleRepository; - public CourtScheduleResponse getCourtScheduleByCaseUrn(String caseUrn) throws ResponseStatusException { + public CourtScheduleResponse getCourtScheduleByCaseUrn(final String caseUrn) throws ResponseStatusException { if (StringUtils.isEmpty(caseUrn)) { - log.warn("No case urn provided"); + LOG.atWarn().log("No case urn provided"); throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "caseUrn is required"); } - log.warn("NOTE: System configured to return stubbed Court Schedule details. Ignoring provided caseUrn : {}", caseUrn); - CourtScheduleResponse stubbedCourtScheduleResponse = courtScheduleRepository.getCourtScheduleByCaseUrn(caseUrn); - log.debug("Court Schedule response: {}", stubbedCourtScheduleResponse); + LOG.atWarn().log("NOTE: System configured to return stubbed Court Schedule details. Ignoring provided caseUrn : {}", caseUrn); + final CourtScheduleResponse stubbedCourtScheduleResponse = courtScheduleRepository.getCourtScheduleByCaseUrn(caseUrn); + LOG.atDebug().log("Court Schedule response: {}", stubbedCourtScheduleResponse); return stubbedCourtScheduleResponse; }