Merge pull request #29 from hmcts/static-analysis

add code analysis and various fixes

Author: duncancrawford

.github/pmd-ruleset.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset name="Custom Ruleset"
+         xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0
+         https://pmd.github.io/pmd-7.0.0/pmd-7.0.0.xsd">
+
+    <description>Custom ruleset including exclusions</description>
+
+    <rule ref="category/java/bestpractices.xml"/>
+    <rule ref="category/java/codestyle.xml">
+        <exclude name="AtLeastOneConstructor"/>
+        <exclude name="LongVariable"/>
+    </rule>
+    <rule ref="category/java/errorprone.xml"/>
+    <rule ref="category/java/performance.xml"/>
+    <rule ref="category/java/security.xml"/>
+</ruleset>

.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

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() {

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<CourtScheduleResponse> getCourtScheduleByCaseUrn(String caseUrn) {
-        String sanitizedCaseUrn;
-        CourtScheduleResponse courtScheduleResponse;
+    public ResponseEntity<CourtScheduleResponse> 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);
     }
 }

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<ErrorResponse> handleResponseStatusException(ResponseStatusException e) {
-        ErrorResponse error = ErrorResponse.builder()
-                .error(String.valueOf(e.getStatusCode().value()))
-                .message(e.getReason() != null ? e.getReason() : e.getMessage())
+    public ResponseEntity<ErrorResponse> 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);
     }
 }

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 {
 

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")

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;
     }