BREAKING CHANGE: upgrade to JDK17, httpclinet 5, jnuit5, spring 6

[alexole01]

JIRA:GRIF-316

[zhabba]

lgtm but I'd like to have second pair of eyes here
@jiri-staroba would you please?

[jiri-staroba]

There are several concerns from a review in Cursor. Please verify them.

Critical issues

1. Potential race condition in token refresh synchronization

Location: GoodDataHttpClient.java:109-124

The token refresh synchronization may race. A thread can read tt after waiting, but the value might not be updated yet:

        synchronized (tokenRefreshMonitor) {
            if (tokenRefreshing) {
                while (tokenRefreshing) {
                    try {
                        tokenRefreshMonitor.wait();
                    } catch (InterruptedException e) {
                        Thread.currentThread().interrupt();
                        throw new IOException("Interrupted while waiting for token refresh", e);
                    }
                }
                final ClassicHttpRequest retryRequest = cloneRequestWithNewTT(originalRequest, tt);
                return this.httpClient.execute(httpHost, retryRequest, context, response -> copyResponseEntity(response));
            } else {
                tokenRefreshing = true;
            }
        }

Issue: After notifyAll() at line 149, a waiting thread may proceed and read tt before it’s set. The tt assignment happens inside the writeLock block (lines 127-145), but waiting threads don’t re-check tt validity.

Recommendation: Add a validation check or ensure tt is set atomically before notifying waiters.

2. Lock contention: always using write lock

Location: GoodDataHttpClient.java:216

        // FIX: use only writeLock to avoid deadlock during handleResponse
        final Lock lock = rwLock.writeLock();

Impact: Using a write lock for all requests serializes reads, hurting throughput. The comment references a deadlock fix, but we should validate the deadlock scenario and consider alternatives.

Recommendation: Document why a read lock isn’t sufficient, or add a comment explaining the trade-off.

3. Unsupported HTTP methods in request cloning

Location: GoodDataHttpClient.java:163-183

    private ClassicHttpRequest cloneRequestWithNewTT(ClassicHttpRequest original, String newTT) {
        ClassicHttpRequest copy;
        // Clone basic types (extend if needed)
        switch (original.getMethod()) {
            case "GET":
                copy = new HttpGet(original.getRequestUri());
                break;
            case "DELETE":
                copy = new org.apache.hc.client5.http.classic.methods.HttpDelete(original.getRequestUri());
                break;
            default:
                throw new UnsupportedOperationException("Unsupported HTTP method: " + original.getMethod());
        }
        // Copy original headers
        for (Header header : original.getHeaders()) {
            copy.addHeader(header.getName(), header.getValue());
        }
        // Set the new TT
        copy.addHeader(TT_HEADER, newTT);
        return copy;
    }

Issue: Only GET and DELETE are supported; POST/PUT/PATCH fail. This blocks token refresh retries for those methods.

Recommendation: Support POST, PUT, and PATCH (clone request body as needed).

Major issues

4. Dead code and commented-out code

Location: GoodDataHttpClient.java:259, pom.xml:167,191

    public ClassicHttpResponse execute(HttpHost target, ClassicHttpRequest request) throws IOException {
       //  return httpClient.execute(target, request, (HttpContext) null, response -> response);
       return execute(target, request, null);
    }

                <!--    <skip>true</skip> -->

Remove commented-out code or justify its retention.

5. Missing null check in copyResponseEntity

Location: GoodDataHttpClient.java:287

        ContentType contentType = ContentType.parseLenient(response.getEntity().getContentType());

Issue: getContentType() may return null. parseLenient handles null, but calling it on a null value is risky.

Recommendation: Add explicit null handling or document the assumption.

6. Non-English comments in test code

Location: GoodDataHttpClientTest.java (multiple locations)

        // English comment: There should be exactly 3 httpClient.execute calls!

Issue: Comments like "English comment:" suggest non-English comments exist elsewhere.

Recommendation: Ensure all comments are in English and remove "English comment:" markers.

[jiri-staroba]

The field is not used anymore.

[jiri-staroba]

Why are we keeping the old version of the artifact 1.0.1-SNAPSHOT?