feat(sdk): add linter and fix linter findings

.github/workflows/ci.yaml

@@ -27,6 +27,9 @@ jobs:
       - name: Check code format
         run: ./gradlew spotlessCheck
 
+      - name: Lint
+        run: make lint
+
       - name: Test
         run: make test
 

Makefile

@@ -5,7 +5,7 @@ fmt:
 	@./gradlew spotlessApply
 
 lint:
-	@echo "linting not ready yet"
+	@./gradlew pmdMain
 
 test:
 	@./gradlew test

build.gradle

@@ -3,6 +3,7 @@ plugins {
 	id 'signing'
 	id 'idea'
 	id 'eclipse'
+	id 'pmd'
 
 	id 'com.diffplug.spotless' version '6.21.0'
 
@@ -13,6 +14,7 @@ plugins {
 
 allprojects {
 	apply plugin: 'com.diffplug.spotless'
+	apply plugin: 'pmd'
 
 	repositories {
 		mavenCentral()
@@ -64,6 +66,17 @@ allprojects {
 			endWithNewline()
 		}
 	}
+
+	pmd {
+		consoleOutput = true
+		toolVersion = "7.12.0"
+
+		// This tells PMD to use your custom ruleset file.
+		ruleSetFiles = rootProject.files("config/pmd/pmd-ruleset.xml")
+
+		// This is important: it prevents PMD from using its default rules.
+		ruleSets = []
+	}
 }
 
 def configureMavenCentralPublishing(Project project) {

config/pmd/pmd-ruleset.xml

@@ -0,0 +1,68 @@
+<?xml version="1.0"?>
+<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.sourceforge.io/ruleset_2_0_0.xsd">
+
+    <description>
+        Custom ruleset that excludes generated code from PMD analysis.
+    </description>
+
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/model/.*</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/api/.*</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/ApiCallback.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/ApiClient.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/ApiResponse.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/GzipRequestInterceptor.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/JSON.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/Pair.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/ProgressRequestBody.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/ProgressResponseBody.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/ServerConfiguration.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/ServerVariable.java</exclude-pattern>
+    <exclude-pattern>.*/cloud/stackit/sdk/.*/StringUtil.java</exclude-pattern>
+
+    <rule ref="category/java/bestpractices.xml">
+        <exclude name="UnitTestContainsTooManyAsserts"/>
+        <exclude name="UnitTestAssertionsShouldIncludeMessage"/>
+    </rule>
+
+    <rule ref="category/java/codestyle.xml">
+        <exclude name="LocalVariableCouldBeFinal"/>
+        <exclude name="MethodArgumentCouldBeFinal"/>
+        <exclude name="AtLeastOneConstructor"/>
+        <exclude name="LongVariable"/>
+        <exclude name="OnlyOneReturn"/>
+    </rule>
+
+    <!-- Excluded this rule to allow simple behaviour like parameter.method() 
+    or chained getter calls (transversing dtos) -->
+    <rule ref="category/java/design.xml">
+        <exclude name="LawOfDemeter"/>
+    </rule>
+
+    <rule ref="category/java/documentation.xml">
+        <exclude name="CommentRequired"/>
+    </rule>
+
+    <rule ref="category/java/documentation.xml/CommentSize">
+        <properties>
+            <property name="maxLineLength" value="100"/>
+            <property name="maxLines" value="40"/>
+        </properties>
+    </rule>
+
+    <rule ref="category/java/errorprone.xml">
+        <exclude name="AvoidFieldNameMatchingMethodName"/>
+    </rule>
+
+    <rule ref="category/java/multithreading.xml">
+    </rule>
+
+    <rule ref="category/java/performance.xml">
+    </rule>
+
+    <rule ref="category/java/security.xml">
+    </rule>
+
+</ruleset>

core/src/main/java/cloud/stackit/sdk/core/KeyFlowAuthenticator.java

@@ -19,14 +19,16 @@
 import java.security.interfaces.RSAPrivateKey;
 import java.security.spec.InvalidKeySpecException;
 import java.util.Date;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import okhttp3.*;
 import org.jetbrains.annotations.NotNull;
 
-/* KeyFlowAuthenticator handles the Key Flow Authentication based on the Service Account Key. */
+/*
+ * KeyFlowAuthenticator handles the Key Flow Authentication based on the Service Account Key.
+ */
 public class KeyFlowAuthenticator implements Authenticator {
 	private static final String REFRESH_TOKEN = "refresh_token";
 	private static final String ASSERTION = "assertion";
@@ -44,6 +46,8 @@ public class KeyFlowAuthenticator implements Authenticator {
 	private final String tokenUrl;
 	private long tokenLeewayInSeconds = DEFAULT_TOKEN_LEEWAY;
 
+	private final Object tokenRefreshMonitor = new Object();
+
 	/**
 	 * Creates the initial service account and refreshes expired access token.
 	 *
@@ -128,7 +132,7 @@ public Request authenticate(Route route, @NotNull Response response) throws IOEx
 		try {
 			accessToken = getAccessToken();
 		} catch (ApiException | InvalidKeySpecException e) {
-			throw new RuntimeException(e);
+			throw new IllegalStateException(e);
 		}
 
 		// Return a new request with the refreshed token
@@ -140,19 +144,19 @@ public Request authenticate(Route route, @NotNull Response response) throws IOEx
 
 	protected static class KeyFlowTokenResponse {
 		@SerializedName("access_token")
-		private String accessToken;
+		private final String accessToken;
 
 		@SerializedName("refresh_token")
-		private String refreshToken;
+		private final String refreshToken;
 
 		@SerializedName("expires_in")
 		private long expiresIn;
 
 		@SerializedName("scope")
-		private String scope;
+		private final String scope;
 
 		@SerializedName("token_type")
-		private String tokenType;
+		private final String tokenType;
 
 		public KeyFlowTokenResponse(
 				String accessToken,
@@ -184,14 +188,16 @@ protected String getAccessToken() {
 	 * @throws IOException request for new access token failed
 	 * @throws ApiException response for new access token with bad status code
 	 */
-	public synchronized String getAccessToken()
-			throws IOException, ApiException, InvalidKeySpecException {
-		if (token == null) {
-			createAccessToken();
-		} else if (token.isExpired()) {
-			createAccessTokenWithRefreshToken();
+	@SuppressWarnings("PMD.AvoidSynchronizedStatement")
+	public String getAccessToken() throws IOException, ApiException, InvalidKeySpecException {
+		synchronized (tokenRefreshMonitor) {
+			if (token == null) {
+				createAccessToken();
+			} else if (token.isExpired()) {
+				createAccessTokenWithRefreshToken();
+			}
+			return token.getAccessToken();
 		}
-		return token.getAccessToken();
 	}
 
 	/**
@@ -202,20 +208,23 @@ public synchronized String getAccessToken()
 	 * @throws ApiException response for new access token with bad status code
 	 * @throws JsonSyntaxException parsing of the created access token failed
 	 */
-	protected void createAccessToken()
-			throws InvalidKeySpecException, IOException, JsonSyntaxException, ApiException {
-		String grant = "urn:ietf:params:oauth:grant-type:jwt-bearer";
-		String assertion;
-		try {
-			assertion = generateSelfSignedJWT();
-		} catch (NoSuchAlgorithmException e) {
-			throw new RuntimeException(
-					"could not find required algorithm for jwt signing. This should not happen and should be reported on https://github.com/stackitcloud/stackit-sdk-java/issues",
-					e);
+	@SuppressWarnings("PMD.AvoidSynchronizedStatement")
+	protected void createAccessToken() throws InvalidKeySpecException, IOException, ApiException {
+		synchronized (tokenRefreshMonitor) {
+			String assertion;
+			try {
+				assertion = generateSelfSignedJWT();
+			} catch (NoSuchAlgorithmException e) {
+				throw new IllegalStateException(
+						"could not find required algorithm for jwt signing. This should not happen and should be reported on https://github.com/stackitcloud/stackit-sdk-java/issues",
+						e);
+			}
+
+			String grant = "urn:ietf:params:oauth:grant-type:jwt-bearer";
+			try (Response response = requestToken(grant, assertion).execute()) {
+				parseTokenResponse(response);
+			}
 		}
-		Response response = requestToken(grant, assertion).execute();
-		parseTokenResponse(response);
-		response.close();
 	}
 
 	/**
@@ -225,16 +234,24 @@ protected void createAccessToken()
 	 * @throws ApiException response for new access token with bad status code
 	 * @throws JsonSyntaxException can not parse new access token
 	 */
-	protected synchronized void createAccessTokenWithRefreshToken()
-			throws IOException, JsonSyntaxException, ApiException {
-		String refreshToken = token.refreshToken;
-		Response response = requestToken(REFRESH_TOKEN, refreshToken).execute();
-		parseTokenResponse(response);
-		response.close();
+	@SuppressWarnings("PMD.AvoidSynchronizedStatement")
+	protected void createAccessTokenWithRefreshToken() throws IOException, ApiException {
+		synchronized (tokenRefreshMonitor) {
+			String refreshToken = token.refreshToken;
+			try (Response response = requestToken(REFRESH_TOKEN, refreshToken).execute()) {
+				parseTokenResponse(response);
+			}
+		}
 	}
 
-	private synchronized void parseTokenResponse(Response response)
-			throws ApiException, JsonSyntaxException, IOException {
+	/**
+	 * Parses the token response from the server
+	 *
+	 * @param response HTTP response containing the token
+	 * @throws ApiException if the response has a bad status code
+	 * @throws JsonSyntaxException if the response body cannot be parsed
+	 */
+	private void parseTokenResponse(Response response) throws ApiException {
 		if (response.code() != HttpURLConnection.HTTP_OK) {
 			String body = null;
 			if (response.body() != null) {
@@ -256,10 +273,10 @@ private synchronized void parseTokenResponse(Response response)
 		response.body().close();
 	}
 
-	private Call requestToken(String grant, String assertionValue) throws IOException {
+	private Call requestToken(String grant, String assertionValue) {
 		FormBody.Builder bodyBuilder = new FormBody.Builder();
 		bodyBuilder.addEncoded("grant_type", grant);
-		String assertionKey = grant.equals(REFRESH_TOKEN) ? REFRESH_TOKEN : ASSERTION;
+		String assertionKey = REFRESH_TOKEN.equals(grant) ? REFRESH_TOKEN : ASSERTION;
 		bodyBuilder.addEncoded(assertionKey, assertionValue);
 		FormBody body = bodyBuilder.build();
 
@@ -289,7 +306,7 @@ private String generateSelfSignedJWT()
 		prvKey = saKey.getCredentials().getPrivateKeyParsed();
 		Algorithm algorithm = Algorithm.RSA512(prvKey);
 
-		Map<String, Object> jwtHeader = new HashMap<>();
+		Map<String, Object> jwtHeader = new ConcurrentHashMap<>();
 		jwtHeader.put("kid", saKey.getCredentials().getKid());
 
 		return JWT.create()

core/src/main/java/cloud/stackit/sdk/core/KeyFlowInterceptor.java

@@ -37,7 +37,7 @@ public Response intercept(Chain chain) throws IOException {
 		} catch (InvalidKeySpecException | ApiException e) {
 			// try-catch required, because ApiException can not be thrown in the implementation
 			// of Interceptor.intercept(Chain chain)
-			throw new RuntimeException(e);
+			throw new IllegalStateException(e);
 		}
 
 		Request authenticatedRequest =