Fix building on recent Java and all build warnings (#119)

hmcts · Jan 4, 2023 · cfabefe · cfabefe
1 parent 7483a6a
commit cfabefe
Show file tree

Hide file tree

Showing 22 changed files with 144 additions and 166 deletions.
diff --git a/.github/workflows/gradle.yml b/.github/workflows/gradle.yml
@@ -15,10 +15,11 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v1
-      - name: Set up JDK 11
-        uses: actions/setup-java@v1
+      - uses: actions/checkout@v3
+      - uses: actions/setup-java@v3
         with:
-          java-version: 11
+          distribution: 'temurin'
+          java-version: '17'
+          cache: 'gradle'
       - name: Build
         run: ./gradlew check
diff --git a/README.md b/README.md
@@ -1,6 +1,3 @@
-[![Known Vulnerabilities](https://snyk.io/test/github/hmcts/service-auth-provider-java-client/badge.svg)](https://snyk.io/test/github/hmcts/service-auth-provider-java-client)
-[ ![Download](https://api.bintray.com/packages/hmcts/hmcts-maven/service-auth-provider-client/images/download.svg) ](https://bintray.com/hmcts/hmcts-maven/service-auth-provider-client/_latestVersion)
-
 # service-auth-provider-java-client
 
 This is the client library for the service-auth-provider api microservice.
@@ -11,18 +8,18 @@ The tool provides a method to generate s2s auth token for a microservice and, op
 
 ### Prerequisites
 
-- [JDK 8](https://www.oracle.com/java)
+- [Java 17](https://adoptium.net/)
 - [Docker](https://www.docker.com)
 
 ### Building
 
-The project uses [Gradle](https://gradle.org) as a build tool but you don't have install it locally since there is a
+The project uses [Gradle](https://gradle.org) as a build tool, but you don't have to install it locally since there is a
 `./gradlew` wrapper script.  
 
-To build project execute the following command:
+To build the project run the following command:
 
 ```bash
-    ./gradlew build
+./gradlew build
 ```
 ## Configuration
 The following values must be provided:
@@ -36,19 +33,17 @@ idam:
 
 A spring bean:
 ```java
-   @Configuration
-   public class ServiceTokenGeneratorConfiguration {
-
-       @Bean
-       public AuthTokenGenerator serviceAuthTokenGenerator(
-               @Value("${idam.s2s-auth.totp_secret}") final String secret,
-               @Value("${idam.s2s-auth.microservice}") final String microService,
-               final ServiceAuthorisationApi serviceAuthorisationApi
-       ) {
-           return AuthTokenGeneratorFactory.createDefaultGenerator(secret, microService, serviceAuthorisationApi);
-       }
-
+@Configuration
+public class ServiceTokenGeneratorConfiguration {
+   @Bean
+   public AuthTokenGenerator serviceAuthTokenGenerator(
+           @Value("${idam.s2s-auth.totp_secret}") final String secret,
+           @Value("${idam.s2s-auth.microservice}") final String microService,
+           final ServiceAuthorisationApi serviceAuthorisationApi
+   ) {
+       return AuthTokenGeneratorFactory.createDefaultGenerator(secret, microService, serviceAuthorisationApi);
    }
+}
 ``` 
 ## Configuration for Service Authentication filter
 The following values must be provided to enable a ServiceAuthFilter bean:
@@ -63,23 +58,25 @@ to approve the request. Any requests from services that are not in your authoris
 to your service and return an HTTP response status code 403 (forbidden) and for any other reasons if the token is
 missing, invalid or failure to verify will result in 401(unauthorized).
 
-## Running in a non spring context
+## Running without Sprint
 
 You might want to use this client when not running in a spring context, i.e. a scheduled job possibly.
 
 ```java
-private static AuthTokenGenerator getAuthTokenGenerator(String s2sURL, String clientId, String clientSecret) {
-    HttpMessageConverter<?> jsonConverter = new MappingJackson2HttpMessageConverter(new ObjectMapper());
-    ObjectFactory<HttpMessageConverters> converter = () -> new HttpMessageConverters(jsonConverter);
-
-    ServiceAuthorisationApi serviceAuthorisationApi = Feign.builder()
-            .contract(new SpringMvcContract())
-            .encoder(new SpringEncoder(converter))
-            .decoder(new StringDecoder())
-            .target(ServiceAuthorisationApi.class, s2sURL);
-
-    return AuthTokenGeneratorFactory
-            .createDefaultGenerator(clientSecret, clientId, serviceAuthorisationApi);
+class ServiceTokenGenerator {
+    private static AuthTokenGenerator getAuthTokenGenerator(String s2sURL, String clientId, String clientSecret) {
+        HttpMessageConverter<?> jsonConverter = new MappingJackson2HttpMessageConverter(new ObjectMapper());
+        ObjectFactory<HttpMessageConverters> converter = () -> new HttpMessageConverters(jsonConverter);
+    
+        ServiceAuthorisationApi serviceAuthorisationApi = Feign.builder()
+                .contract(new SpringMvcContract())
+                .encoder(new SpringEncoder(converter))
+                .decoder(new StringDecoder())
+                .target(ServiceAuthorisationApi.class, s2sURL);
+    
+        return AuthTokenGeneratorFactory
+                .createDefaultGenerator(clientSecret, clientId, serviceAuthorisationApi);
+    }
 }
 ```
 
@@ -90,15 +87,15 @@ private static AuthTokenGenerator getAuthTokenGenerator(String s2sURL, String cl
 To run all unit tests execute the following command:
 
 ```bash
-    ./gradlew test
+./gradlew test
 ```
 
 ### Coding style tests
 
 To run all checks (including unit tests) execute the following command:
 
 ```bash
-    ./gradlew check
+./gradlew check
 ```
 
 ## Versioning

diff --git a/build.gradle b/build.gradle
@@ -41,8 +41,8 @@ checkstyle {
 }
 
 pmd {
-    toolVersion = "6.12.0"
-    ignoreFailures = true
+    toolVersion = "6.53.0"
+    ignoreFailures = false
     sourceSets = [sourceSets.main, sourceSets.test, sourceSets.integrationTest]
     reportsDir = file("$project.buildDir/reports/pmd")
     ruleSetFiles = files("config/pmd/ruleset.xml")
@@ -105,10 +105,8 @@ task contract(type: Test) {
 }
 
 task runAndPublishConsumerPactTests(type: Test){
-    logger.lifecycle("Runs pact Tests")
     testClassesDirs = sourceSets.contractTest.output.classesDirs
     classpath = sourceSets.contractTest.runtimeClasspath
-
 }
 
 runAndPublishConsumerPactTests.dependsOn contract
@@ -136,8 +134,8 @@ publishing {
                 url = gitRepo
                 licenses {
                     license {
-                        name = 'MIT License, Copyright (c) 2019 HM Courts & Tribunals Service'
-                        url = "http://www.opensource.org/licenses/mit-license.php"
+                        name = 'MIT License, Copyright (c) 2023 HM Courts & Tribunals Service'
+                        url = "https://www.opensource.org/licenses/MIT"
                     }
                 }
                 scm {
@@ -158,7 +156,8 @@ dependencies {
 
     testImplementation group: 'junit', name: 'junit', version: '4.13.2'
     testImplementation group: 'org.assertj', name: 'assertj-core', version: '3.23.1'
-    testImplementation group: 'org.mockito', name: 'mockito-all', version: '1.10.19'
+    testImplementation group: 'org.mockito', name: 'mockito-core', version: '4.11.0'
+
 
     integrationTestImplementation sourceSets.main.runtimeClasspath
     integrationTestImplementation sourceSets.test.runtimeClasspath

diff --git a/config/pmd/ruleset.xml b/config/pmd/ruleset.xml
@@ -1,70 +1,70 @@
 <?xml version="1.0"?>
 <ruleset name="PMD rule set"
-  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 http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
+         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.sourceforge.io/ruleset_2_0_0.xsd">
 
   <description>HMCTS PMD rule set</description>
 
   <rule ref="category/java/bestpractices.xml">
-    <exclude name="GuardLogStatement" />
-    <exclude name="JUnitTestContainsTooManyAsserts" />
+    <exclude name="GuardLogStatement"/>
+    <exclude name="JUnitTestContainsTooManyAsserts"/>
   </rule>
   <rule ref="category/java/codestyle.xml">
-    <exclude name="AtLeastOneConstructor" />
-    <exclude name="LocalVariableCouldBeFinal" />
-    <exclude name="LongVariable" />
-    <exclude name="MethodArgumentCouldBeFinal" />
-    <exclude name="OnlyOneReturn" />
-    <exclude name="TooManyStaticImports" />
+    <exclude name="AtLeastOneConstructor"/>
+    <exclude name="LocalVariableCouldBeFinal"/>
+    <exclude name="LongVariable"/>
+    <exclude name="MethodArgumentCouldBeFinal"/>
+    <exclude name="OnlyOneReturn"/>
+    <exclude name="TooManyStaticImports"/>
+    <exclude name="DefaultPackage"/>
+    <exclude name="CommentDefaultAccessModifier"/>
   </rule>
   <rule ref="category/java/codestyle.xml/ClassNamingConventions">
     <properties>
       <!-- same as any other class -->
-      <property name="utilityClassPattern" value="[A-Z][a-zA-Z]+" />
+      <property name="utilityClassPattern" value="[A-Z][a-zA-Z]+"/>
     </properties>
   </rule>
   <rule ref="category/java/codestyle.xml/MethodNamingConventions">
     <properties>
-      <property name="junit4TestPattern" value="[a-z][a-zA-Z0-9_]+" />
+      <property name="junit4TestPattern" value="[a-z][a-zA-Z0-9_]+"/>
     </properties>
   </rule>
   <rule ref="category/java/codestyle.xml/ShortVariable">
     <properties>
-      <property name="minimum" value="2" />
+      <property name="minimum" value="2"/>
     </properties>
   </rule>
   <rule ref="category/java/design.xml">
-    <exclude name="AvoidCatchingGenericException" />
-    <exclude name="UseUtilityClass" />
-    <exclude name="LoosePackageCoupling" />
-    <exclude name="DataClass" />
+    <exclude name="AvoidCatchingGenericException"/>
+    <exclude name="UseUtilityClass"/>
+    <exclude name="LoosePackageCoupling"/>
+    <exclude name="DataClass"/>
   </rule>
   <rule ref="category/java/design.xml/ExcessiveImports">
     <properties>
-      <property name="minimum" value="35" /><!-- should be reduced -->
+      <property name="minimum" value="40"/><!-- should be reduced -->
     </properties>
   </rule>
   <rule ref="category/java/design.xml/LawOfDemeter">
     <properties>
-      <property name="violationSuppressRegex" value="(.*method chain calls.*|.*object not created locally.*)" />
+      <property name="violationSuppressRegex" value="(.*method chain calls.*|.*object not created locally.*)"/>
     </properties>
   </rule>
   <rule ref="category/java/design.xml/SignatureDeclareThrowsException">
     <properties>
-      <property name="IgnoreJUnitCompletely" value="true" />
+      <property name="IgnoreJUnitCompletely" value="true"/>
     </properties>
   </rule>
   <rule ref="category/java/documentation.xml">
-    <exclude name="CommentRequired" />
-    <exclude name="CommentSize" />
-    <exclude name="UncommentedEmptyMethodBody" />
+    <exclude name="CommentRequired"/>
+    <exclude name="CommentSize"/>
+    <exclude name="UncommentedEmptyMethodBody"/>
   </rule>
-  <rule ref="category/java/errorprone.xml">
-    <exclude name="BeanMembersShouldSerialize" />
-  </rule>
-  <rule ref="category/java/multithreading.xml" />
-  <rule ref="category/java/performance.xml" />
-  <rule ref="category/java/security.xml" />
+  <rule ref="category/java/errorprone.xml" />
+  <rule ref="category/java/multithreading.xml"/>
+  <rule ref="category/java/performance.xml"/>
+  <rule ref="category/java/security.xml"/>
 
-</ruleset>
+</ruleset>
diff --git a/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApiTest.java b/src/integrationTest/java/uk/gov/hmcts/reform/authorisation/ServiceAuthorisationApiTest.java
@@ -1,19 +1,15 @@
 package uk.gov.hmcts.reform.authorisation;
 
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.mockito.Matchers;
-import org.mockito.Mockito;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.cloud.contract.wiremock.AutoConfigureWireMock;
 import org.springframework.context.annotation.ComponentScan;
 import org.springframework.test.context.ActiveProfiles;
-import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.context.junit4.SpringRunner;
 import uk.gov.hmcts.reform.authorisation.config.IntegrationTestInitializer;
@@ -32,16 +28,17 @@
 import static com.github.tomakehurst.wiremock.client.WireMock.givenThat;
 import static com.github.tomakehurst.wiremock.client.WireMock.status;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.springframework.http.HttpStatus.OK;
 
 @ActiveProfiles("wiremock")
-@AutoConfigureWireMock
-@ContextConfiguration(initializers = IntegrationTestInitializer.class)
+@AutoConfigureWireMock(port = 0)
 @ComponentScan
 @EnableAutoConfiguration
 @EnableConfigurationProperties()
@@ -50,8 +47,11 @@
 })
 @RunWith(SpringRunner.class)
 @SpringBootTest(classes = IntegrationTestInitializer.class)
+@SuppressWarnings("PMD.ExcessiveImports")
 public class ServiceAuthorisationApiTest {
 
+    private static final String DETAILS_ENDPOINT = "/details";
+    private static final String DEFAULT_SERVICE = "service";
     @Autowired
     private ServiceAuthorisationApi s2sApi;
 
@@ -64,7 +64,6 @@ public class ServiceAuthorisationApiTest {
 
     @Before
     public void before() {
-        Assert.assertNotNull(serviceAuthFilter);
         filterChain = spy(FilterChain.class);
         httpServletRequest = mock(HttpServletRequest.class);
         when(httpServletRequest.getHeader(ServiceAuthFilter.AUTHORISATION)).thenReturn("token");
@@ -73,32 +72,32 @@ public void before() {
     @Test
     public void should_get_service_name_providing_valid_token() {
         AuthTokenValidator validator = new ServiceAuthTokenValidator(s2sApi);
-        givenThat(get("/details").willReturn(status(OK.value()).withBody("service")));
-        assertThat(validator.getServiceName("token")).isEqualTo("service");
+        givenThat(get(DETAILS_ENDPOINT).willReturn(status(OK.value()).withBody(DEFAULT_SERVICE)));
+        assertThat(validator.getServiceName("token")).isEqualTo(DEFAULT_SERVICE);
     }
 
     @Test
     public void should_pass_serviceAuthFilter_with_authorized_access() throws ServletException, IOException {
-        givenThat(get("/details").willReturn(status(OK.value()).withBody("service1")));
+        givenThat(get(DETAILS_ENDPOINT).willReturn(status(OK.value()).withBody("service1")));
         serviceAuthFilter.doFilter(httpServletRequest, mock(HttpServletResponse.class), filterChain);
-        Mockito.verify(filterChain, times(1)).doFilter(Matchers.any(), Matchers.any());
+        verify(filterChain, times(1)).doFilter(any(), any());
     }
 
     @Test
     public void should_fail_serviceAuthFilter_with_Unauthorized_access() throws ServletException, IOException {
-        givenThat(get("/details").willReturn(status(OK.value()).withStatus(HttpStatus.GATEWAY_TIMEOUT_504)));
+        givenThat(get(DETAILS_ENDPOINT).willReturn(status(OK.value()).withStatus(HttpStatus.GATEWAY_TIMEOUT_504)));
         HttpServletResponse response = mock(HttpServletResponse.class);
         serviceAuthFilter.doFilter(httpServletRequest, response, filterChain);
-        Mockito.verify(response, times(1)).setStatus(HttpStatus.UNAUTHORIZED_401);
-        Mockito.verify(filterChain, never()).doFilter(Matchers.any(), Matchers.any());
+        verify(response, times(1)).setStatus(HttpStatus.UNAUTHORIZED_401);
+        verify(filterChain, never()).doFilter(any(), any());
     }
 
     @Test
     public void should_fail_serviceAuthFilter_with_Forbidden_access() throws ServletException, IOException {
-        givenThat(get("/details").willReturn(status(OK.value()).withBody("service")));
+        givenThat(get(DETAILS_ENDPOINT).willReturn(status(OK.value()).withBody(DEFAULT_SERVICE)));
         HttpServletResponse response = mock(HttpServletResponse.class);
         serviceAuthFilter.doFilter(httpServletRequest, response, filterChain);
-        Mockito.verify(response, times(1)).setStatus(HttpStatus.FORBIDDEN_403);
-        Mockito.verify(filterChain, never()).doFilter(Matchers.any(), Matchers.any());
+        verify(response, times(1)).setStatus(HttpStatus.FORBIDDEN_403);
+        verify(filterChain, never()).doFilter(any(), any());
     }
 }