diff --git a/gooddata-java/src/main/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactory.java b/gooddata-java/src/main/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactory.java index edf192907..858f63a68 100644 --- a/gooddata-java/src/main/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactory.java +++ b/gooddata-java/src/main/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactory.java @@ -5,6 +5,7 @@ */ package com.gooddata.sdk.common; +import com.gooddata.http.client.GoodDataHttpClient; import org.apache.hc.client5.http.classic.HttpClient; import org.apache.hc.client5.http.classic.methods.HttpDelete; import org.apache.hc.client5.http.classic.methods.HttpGet; @@ -14,13 +15,13 @@ import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.classic.methods.HttpPut; import org.apache.hc.client5.http.classic.methods.HttpTrace; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.HttpEntity; import org.apache.hc.core5.http.HttpEntityContainer; import org.apache.hc.core5.http.io.entity.ByteArrayEntity; -import org.apache.hc.core5.http.protocol.HttpContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpHeaders; @@ -40,39 +41,26 @@ import java.util.Map; /** - * Spring 6 compatible {@link ClientHttpRequestFactory} implementation that uses Apache HttpComponents HttpClient 5.x Classic API. - * This implementation bridges the gap between Spring 6 and HttpClient 5.x, supporting the Classic API pattern - * used by gooddata-http-client. + * Spring {@link ClientHttpRequestFactory} backed by Apache HttpClient 5 Classic API. */ public class HttpClient5ComponentsClientHttpRequestFactory implements ClientHttpRequestFactory { private static final Logger logger = LoggerFactory.getLogger(HttpClient5ComponentsClientHttpRequestFactory.class); + private final HttpClient httpClient; - /** - * Create a factory with the given HttpClient 5.x instance. - * - * @param httpClient the HttpClient 5.x instance to use - */ - public HttpClient5ComponentsClientHttpRequestFactory(HttpClient httpClient) { + public HttpClient5ComponentsClientHttpRequestFactory(final HttpClient httpClient) { Assert.notNull(httpClient, "HttpClient must not be null"); this.httpClient = httpClient; } @Override - public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException { - ClassicHttpRequest httpRequest = createHttpRequest(httpMethod, uri); + public ClientHttpRequest createRequest(final URI uri, final HttpMethod httpMethod) throws IOException { + final ClassicHttpRequest httpRequest = createHttpRequest(httpMethod, uri); return new HttpClient5ComponentsClientHttpRequest(httpClient, httpRequest); } - /** - * Create an Apache HttpComponents ClassicHttpRequest object for the given HTTP method and URI. - * - * @param httpMethod the HTTP method - * @param uri the URI - * @return the ClassicHttpRequest - */ - private ClassicHttpRequest createHttpRequest(HttpMethod httpMethod, URI uri) { + private ClassicHttpRequest createHttpRequest(final HttpMethod httpMethod, final URI uri) { if (HttpMethod.GET.equals(httpMethod)) { return new HttpGet(uri); } else if (HttpMethod.HEAD.equals(httpMethod)) { @@ -95,7 +83,7 @@ private ClassicHttpRequest createHttpRequest(HttpMethod httpMethod, URI uri) { } /** - * {@link ClientHttpRequest} implementation based on Apache HttpComponents HttpClient 5.x Classic API. + * {@link ClientHttpRequest} implementation using Apache HttpClient 5 Classic API. */ private static class HttpClient5ComponentsClientHttpRequest implements ClientHttpRequest { @@ -104,7 +92,7 @@ private static class HttpClient5ComponentsClientHttpRequest implements ClientHtt private final HttpHeaders headers; private final ByteArrayOutputStream bufferedOutput = new ByteArrayOutputStream(1024); - public HttpClient5ComponentsClientHttpRequest(HttpClient httpClient, ClassicHttpRequest httpRequest) { + HttpClient5ComponentsClientHttpRequest(final HttpClient httpClient, final ClassicHttpRequest httpRequest) { this.httpClient = httpClient; this.httpRequest = httpRequest; this.headers = new HttpHeaders(); @@ -135,85 +123,94 @@ public HttpHeaders getHeaders() { } @Override - public OutputStream getBody() throws IOException { + public OutputStream getBody() { return bufferedOutput; } @Override public ClientHttpResponse execute() throws IOException { - // Create entity first (matching reference implementation exactly) - byte[] bytes = bufferedOutput.toByteArray(); - if (bytes.length > 0) { - if (httpRequest != null) { - - // Ensure proper UTF-8 encoding before creating entity - // This is crucial for @JsonTypeInfo annotated classes like Execution - ContentType contentType = ContentType.APPLICATION_JSON; - - if (logger.isDebugEnabled()) { - // Check if Content-Type is already set in headers - boolean hasContentType = false; - for (org.apache.hc.core5.http.Header header : httpRequest.getHeaders()) { - if ("Content-Type".equalsIgnoreCase(header.getName())) { - hasContentType = true; - contentType = ContentType.parse(header.getValue()); - break; - } - } - - if (!hasContentType) { - logger.debug("No Content-Type header found, using application/json as default"); - } + final byte[] bytes = bufferedOutput.toByteArray(); + if (bytes.length > 0 && httpRequest instanceof HttpEntityContainer) { + // Determine content type from Spring headers first, then fallback to default + ContentType contentType = ContentType.APPLICATION_JSON; + final String contentTypeHeader = headers.getFirst(org.springframework.http.HttpHeaders.CONTENT_TYPE); + if (contentTypeHeader != null) { + try { + contentType = ContentType.parse(contentTypeHeader); + } catch (Exception e) { + logger.warn("Failed to parse Content-Type header '{}', using default", contentTypeHeader, e); } - - ByteArrayEntity requestEntity = new ByteArrayEntity(bytes, contentType); - ((HttpEntityContainer) httpRequest).setEntity(requestEntity); } + + final ByteArrayEntity requestEntity = new ByteArrayEntity(bytes, contentType); + ((HttpEntityContainer) httpRequest).setEntity(requestEntity); } - // Add headers to request, excluding Content-Length as HttpClient 5.x manages it internally + // Add headers after setting entity to avoid conflicts addHeaders(httpRequest); - // Execute request - ClassicHttpResponse httpResponse; - if (httpClient.getClass().getName().contains("GoodDataHttpClient")) { - // Use reflection to call the execute method on GoodDataHttpClient + final ClassicHttpResponse httpResponse = executeClassic(httpClient, httpRequest); + return new HttpClient5ComponentsClientHttpResponse(httpResponse); + } + + /** + * Execute the request while keeping the response stream open for callers (e.g. RestTemplate) + * to consume. Avoids using response handlers which auto-close streams. + * + *

GoodDataHttpClient provides direct execute() methods that return ClassicHttpResponse + * without auto-closing streams. For standard CloseableHttpClient, we use executeOpen().

+ */ + private ClassicHttpResponse executeClassic(final HttpClient httpClient, + final ClassicHttpRequest httpRequest) throws IOException { + if (logger.isDebugEnabled()) { try { - // Try the single parameter execute method first - java.lang.reflect.Method executeMethod = httpClient.getClass().getMethod("execute", - ClassicHttpRequest.class); - httpResponse = (ClassicHttpResponse) executeMethod.invoke(httpClient, httpRequest); - } catch (NoSuchMethodException e) { - // If that doesn't work, try the two parameter version with HttpContext - try { - java.lang.reflect.Method executeMethod = httpClient.getClass().getMethod("execute", - ClassicHttpRequest.class, HttpContext.class); - httpResponse = (ClassicHttpResponse) executeMethod.invoke(httpClient, httpRequest, null); - } catch (Exception e2) { - throw new IOException("Failed to execute request with GoodDataHttpClient", e2); - } + logger.debug("Executing request: {} {}", httpRequest.getMethod(), httpRequest.getUri()); } catch (Exception e) { - throw new IOException("Failed to execute request with GoodDataHttpClient", e); + logger.debug("Executing request: {}", httpRequest.getMethod()); } - } else { - httpResponse = httpClient.execute(httpRequest, (ClassicHttpResponse response) -> response); } - return new HttpClient5ComponentsClientHttpResponse(httpResponse); + + // GoodDataHttpClient has direct execute() that returns ClassicHttpResponse + // without auto-closing streams - preferred for authenticated GoodData API calls + if (httpClient instanceof GoodDataHttpClient) { + logger.debug("Using GoodDataHttpClient execute"); + return ((GoodDataHttpClient) httpClient).execute(httpRequest); + } + + // For CloseableHttpClient (and subclasses), use executeOpen to keep streams alive + if (httpClient instanceof CloseableHttpClient) { + logger.debug("Using CloseableHttpClient executeOpen"); + return ((CloseableHttpClient) httpClient).executeOpen(null, httpRequest, null); + } + + // Fallback for other HttpClient implementations + logger.debug("Using standard HttpClient executeOpen"); + return httpClient.executeOpen(null, httpRequest, null); } /** * Add the headers from the HttpHeaders to the HttpRequest. - * Excludes Content-Length headers to avoid conflicts with HttpClient 5.x internal management. - * Uses setHeader instead of addHeader to match the reference implementation. - * Follows HttpClient5 Classic API patterns. + * Excludes headers that HttpClient 5 manages internally or that were already set. */ - private void addHeaders(ClassicHttpRequest httpRequest) { + private void addHeaders(final ClassicHttpRequest httpRequest) { for (Map.Entry> entry : headers.entrySet()) { - String headerName = entry.getKey(); - // Skip Content-Length as HttpClient 5.x manages it internally - if (!"Content-Length".equalsIgnoreCase(headerName)) { - for (String headerValue : entry.getValue()) { - httpRequest.setHeader(headerName, headerValue); + final String headerName = entry.getKey(); + + // Skip headers that HttpClient manages internally or that would cause conflicts + if ("Content-Length".equalsIgnoreCase(headerName) || + "Transfer-Encoding".equalsIgnoreCase(headerName) || + "Connection".equalsIgnoreCase(headerName) || + "Host".equalsIgnoreCase(headerName)) { + continue; + } + + // Remove any existing headers with the same name first to avoid duplicates + httpRequest.removeHeaders(headerName); + + // Add all values for this header + for (String headerValue : entry.getValue()) { + if (headerValue != null) { + httpRequest.addHeader(headerName, headerValue); } } } @@ -221,7 +218,7 @@ private void addHeaders(ClassicHttpRequest httpRequest) { } /** - * {@link ClientHttpResponse} implementation based on Apache HttpComponents HttpClient 5.x Classic API. + * {@link ClientHttpResponse} implementation backed by Apache HttpClient 5 Classic API. */ private static class HttpClient5ComponentsClientHttpResponse implements ClientHttpResponse { @@ -249,32 +246,55 @@ public String getStatusText() throws IOException { @Override public HttpHeaders getHeaders() { - if (this.headers == null) { - this.headers = new HttpHeaders(); + if (headers == null) { + headers = new HttpHeaders(); for (org.apache.hc.core5.http.Header header : httpResponse.getHeaders()) { - this.headers.add(header.getName(), header.getValue()); + headers.add(header.getName(), header.getValue()); } } - return this.headers; + return headers; } @Override public InputStream getBody() throws IOException { - HttpEntity entity = httpResponse.getEntity(); - return entity != null ? entity.getContent() : InputStream.nullInputStream(); + final HttpEntity entity = httpResponse.getEntity(); + if (entity == null) { + logger.debug("No entity in response, returning empty stream"); + return InputStream.nullInputStream(); + } + + try { + final InputStream content = entity.getContent(); + if (content == null) { + logger.debug("Entity content is null, returning empty stream"); + return InputStream.nullInputStream(); + } + + // Log entity details for debugging + if (logger.isTraceEnabled()) { + logger.trace("Returning entity content stream - repeatable: {}, streaming: {}, content length: {}", + entity.isRepeatable(), entity.isStreaming(), entity.getContentLength()); + } + + return content; + } catch (IllegalStateException e) { + logger.warn("Entity content stream is in illegal state (likely already consumed): {}", e.getMessage()); + throw new IOException("Response stream is no longer available: " + e.getMessage(), e); + } } @Override public void close() { try { - // Properly close the response to return connection to pool + logger.debug("Closing HTTP response"); if (httpResponse != null) { + // For Apache HttpClient 5, we should just close the response directly + // The connection manager handles proper connection cleanup automatically httpResponse.close(); } } catch (IOException e) { - // Log the error but don't throw - closing should be best effort - // This ensures connections are returned to the pool + logger.debug("Unable to close HTTP response", e); } } } -} \ No newline at end of file +} diff --git a/gooddata-java/src/main/java/com/gooddata/sdk/service/account/AccountService.java b/gooddata-java/src/main/java/com/gooddata/sdk/service/account/AccountService.java index 688f51202..ab8a148a9 100644 --- a/gooddata-java/src/main/java/com/gooddata/sdk/service/account/AccountService.java +++ b/gooddata-java/src/main/java/com/gooddata/sdk/service/account/AccountService.java @@ -13,6 +13,8 @@ import com.gooddata.sdk.model.gdc.UriResponse; import com.gooddata.sdk.service.AbstractService; import com.gooddata.sdk.service.GoodDataSettings; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; import org.springframework.http.converter.json.MappingJacksonValue; import org.springframework.web.client.RestClientException; @@ -28,6 +30,8 @@ */ public class AccountService extends AbstractService { + private static final Logger logger = LoggerFactory.getLogger(AccountService.class); + public static final UriTemplate ACCOUNT_TEMPLATE = new UriTemplate(Account.URI); public static final UriTemplate ACCOUNTS_TEMPLATE = new UriTemplate(Account.ACCOUNTS_URI); public static final UriTemplate ACCOUNT_BY_LOGIN_TEMPLATE = new UriTemplate(Account.ACCOUNT_BY_EMAIL_URI); @@ -63,11 +67,47 @@ public void logout() { try { final String id = getCurrent().getId(); restTemplate.delete(Account.LOGIN_URI, id); + } catch (GoodDataRestException e) { + if (isAlreadyLoggedOutError(e)) { + logger.debug("User already logged out (status={}, errorCode={}, message={})", + e.getStatusCode(), e.getErrorCode(), e.getMessage()); + return; + } + throw new GoodDataException("Unable to logout", e); } catch (GoodDataException | RestClientException e) { throw new GoodDataException("Unable to logout", e); } } + /** + * Checks if the exception indicates the user is already logged out. + * This is not an error condition - it means the logout goal is already achieved. + */ + private static boolean isAlreadyLoggedOutError(final GoodDataRestException e) { + // HTTP 400 indicates a client error, which includes "not logged in" + if (e.getStatusCode() != HttpStatus.BAD_REQUEST.value()) { + return false; + } + + // Prefer error code matching (API contract) over message content (locale-sensitive) + final String errorCode = e.getErrorCode(); + if (errorCode != null && !errorCode.isEmpty()) { + // Match known error codes for "not logged in" state + // Common patterns: gdc.login.not_logged_in, NOT_LOGGED_IN, etc. + final String lowerErrorCode = errorCode.toLowerCase(); + return lowerErrorCode.contains("not_logged") || lowerErrorCode.contains("notlogged"); + } + + // Fallback to message matching if error code is not available + final String message = e.getMessage(); + if (message != null) { + final String lowerMessage = message.toLowerCase(); + return lowerMessage.contains("not logged in") || lowerMessage.contains("not logged-in"); + } + + return false; + } + /** * Creates new account in given organization (domain). * Only domain admin is allowed create new accounts! This means rest request has to authorized as domain admin. diff --git a/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryAT.java b/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryAT.java new file mode 100644 index 000000000..5e2a5c859 --- /dev/null +++ b/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryAT.java @@ -0,0 +1,560 @@ +/* + * (C) 2025 GoodData Corporation. + * This source code is licensed under the BSD-style license found in the + * LICENSE.txt file in the root directory of this source tree. + */ +package com.gooddata.sdk.common; + +import com.gooddata.sdk.service.AbstractGoodDataAT; +import com.gooddata.sdk.service.GoodData; +import com.gooddata.sdk.service.GoodDataEndpoint; +import com.gooddata.sdk.service.GoodDataSettings; + +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.impl.classic.HttpClientBuilder; +import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; +import org.apache.hc.core5.util.Timeout; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpResponse; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; + +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; + +/** + * Comprehensive acceptance tests for {@link HttpClient5ComponentsClientHttpRequestFactory} + * + *

These tests verify real-world usage scenarios with actual HTTP communication to GoodData endpoints, + * focusing on areas that cannot be adequately tested with mocks:

+ * + * + * + *

Environment Setup:

+ * + */ +@Test(groups = "at") +public class HttpClient5ComponentsClientHttpRequestFactoryAT extends AbstractGoodDataAT { + + private HttpClient5ComponentsClientHttpRequestFactory factory; + private PoolingHttpClientConnectionManager connectionManager; + + // Override parent's static fields to avoid initialization issues with environment variables + protected GoodDataEndpoint testEndpoint; + protected GoodData testGd; + + // Performance tracking + private static final int PERFORMANCE_THRESHOLD_MS = 5000; // Max acceptable response time + private static final int CONNECTION_POOL_SIZE = 50; + private static final int MAX_CONNECTIONS_PER_ROUTE = 10; + + /** + * Get configuration value from system properties first, then environment variables. + * This supports both Maven -D properties and shell environment variables. + * + * @param name the property/variable name + * @return the value or null if not found + */ + private static String getEnvironmentValue(String name) { + // First try system properties (from mvn -Dhost=value) + String value = System.getProperty(name); + if (value != null && !value.trim().isEmpty()) { + return value.trim(); + } + + // Then try environment variables (from shell export or host=value mvn) + value = System.getenv(name); + if (value != null && !value.trim().isEmpty()) { + return value.trim(); + } + + return null; + } + + @BeforeMethod + public void setUp() throws Exception { + // Skip setup if environment variables are not available + try { + // Try to get host from both system properties and environment variables + String host = getEnvironmentValue("host"); + if (host == null || host.trim().isEmpty()) { + throw new IllegalArgumentException("Host not configured"); + } + + // Initialize test-specific endpoint and GoodData client + testEndpoint = new GoodDataEndpoint(host); + + // Also validate other required variables are available + String login = getEnvironmentValue("login"); + String password = getEnvironmentValue("password"); + + if (login != null && password != null) { + testGd = new GoodData(host, login, password, new GoodDataSettings()); + } + + System.out.println("AT Test Environment - Host: " + host + ", Login: " + + (login != null ? login.replaceAll("(.{3}).*(@.*)", "$1***$2") : "null") + + ", Password: " + (password != null ? "***" : "null")); + + } catch (Exception e) { + throw new org.testng.SkipException("Skipping AT tests - environment not configured: " + e.getMessage()); + } + + connectionManager = new PoolingHttpClientConnectionManager(); + connectionManager.setMaxTotal(CONNECTION_POOL_SIZE); + connectionManager.setDefaultMaxPerRoute(MAX_CONNECTIONS_PER_ROUTE); + + // Create factory for standard tests + final HttpClientBuilder clientBuilder = HttpClientBuilder.create() + .setConnectionManager(connectionManager); + factory = new HttpClient5ComponentsClientHttpRequestFactory(clientBuilder.build()); + } + + /** + * Tests basic HTTP GET functionality against real GoodData infrastructure. + * Validates request creation, execution, and response stream handling. + */ + @Test(groups = "httpFactory") + public void shouldHandleBasicGetRequest() throws Exception { + if (testEndpoint == null) { + String host = getEnvironmentValue("host"); + throw new org.testng.SkipException("Endpoint not configured. Host from config: " + host); + } + + URI uri = URI.create(testEndpoint.toUri() + "/gdc"); + long startTime = System.currentTimeMillis(); + + ClientHttpRequest request = factory.createRequest(uri, HttpMethod.GET); + assertNotNull(request, "Request should be created successfully"); + assertEquals(HttpMethod.GET, request.getMethod(), "Request method should match"); + assertEquals(uri, request.getURI(), "Request URI should match"); + + try (ClientHttpResponse response = request.execute()) { + long responseTime = System.currentTimeMillis() - startTime; + + assertNotNull(response, "Response should not be null"); + assertEquals(200, response.getStatusCode().value(), "Should get successful response"); + assertTrue(responseTime < PERFORMANCE_THRESHOLD_MS, + "Response time (" + responseTime + "ms) should be under " + PERFORMANCE_THRESHOLD_MS + "ms"); + + // Validate response headers + HttpHeaders headers = response.getHeaders(); + assertNotNull(headers, "Response headers should not be null"); + + // Verify stream handling without StreamClosedException + try (InputStream inputStream = response.getBody()) { + assertNotNull(inputStream, "Response body stream should not be null"); + + ByteArrayOutputStream result = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + int length; + while ((length = inputStream.read(buffer)) != -1) { + result.write(buffer, 0, length); + } + + String responseBody = result.toString(StandardCharsets.UTF_8); + assertNotNull(responseBody, "Response body should not be null"); + assertTrue(responseBody.length() > 0, "Response body should contain data"); + } + } + } + + /** + * Tests network error scenarios including timeouts and connection failures. + * Validates proper exception handling and resource cleanup under error conditions. + */ + @Test(groups = "httpFactory") + public void shouldHandleNetworkErrors() throws Exception { + // Test 1: Invalid host (should fail quickly) + URI invalidHostUri = URI.create("http://invalid-host-that-does-not-exist.example"); + + try { + ClientHttpRequest request = factory.createRequest(invalidHostUri, HttpMethod.GET); + request.execute(); + assertTrue(false, "Should have thrown an exception for invalid host"); + } catch (Exception e) { + // Expected - should fail with connection error + assertNotNull(e, "Exception should be thrown for invalid host"); + assertTrue(e instanceof IOException || e.getCause() instanceof IOException, + "Should throw IOException or have IOException as cause"); + } + + // Test 2: Timeout scenario with short timeout configuration + if (testEndpoint != null) { + // Create factory with very short timeouts for testing + final RequestConfig timeoutConfig = RequestConfig.custom() + .setConnectionRequestTimeout(Timeout.ofMilliseconds(100)) + .setResponseTimeout(Timeout.ofMilliseconds(500)) + .build(); + final HttpClientBuilder timeoutClientBuilder = HttpClientBuilder.create() + .setConnectionManager(connectionManager) + .setDefaultRequestConfig(timeoutConfig); + HttpClient5ComponentsClientHttpRequestFactory timeoutFactory = + new HttpClient5ComponentsClientHttpRequestFactory(timeoutClientBuilder.build()); + + URI timeoutUri = URI.create(testEndpoint.toUri() + "/gdc"); + + try { + ClientHttpRequest timeoutRequest = timeoutFactory.createRequest(timeoutUri, HttpMethod.GET); + long start = System.currentTimeMillis(); + try (ClientHttpResponse response = timeoutRequest.execute()) { + // If it succeeds, verify it was reasonably fast + long duration = System.currentTimeMillis() - start; + assertTrue(duration < 2000, "Should either timeout or complete quickly"); + } + } catch (Exception e) { + // Timeout exceptions are acceptable + assertTrue(e instanceof IOException, "Should throw IOException on timeout"); + } + } + } + + /** + * Tests various HTTP methods (POST, PUT, DELETE) with real payloads. + * Uses safe endpoints that accept these methods without side effects. + */ + @Test(groups = "httpFactory") + public void shouldHandleAllHttpMethods() throws Exception { + if (testEndpoint == null) { + throw new org.testng.SkipException("Endpoint not configured"); + } + + // Test POST with JSON payload + URI postUri = URI.create(testEndpoint.toUri() + "/gdc/account/login"); + ClientHttpRequest postRequest = factory.createRequest(postUri, HttpMethod.POST); + + // Add headers and body + postRequest.getHeaders().add("Content-Type", "application/json"); + try (var outputStream = postRequest.getBody()) { + String jsonPayload = "{\"postUserLogin\": {\"login\": \"test\", \"password\": \"test\"}}"; + outputStream.write(jsonPayload.getBytes(StandardCharsets.UTF_8)); + } + + // Execute and validate (expect 401 for invalid credentials, which is fine) + try (ClientHttpResponse response = postRequest.execute()) { + assertNotNull(response, "POST response should not be null"); + assertTrue(response.getStatusCode().value() >= 200 && response.getStatusCode().value() < 500, + "POST should get valid HTTP response (2xx-4xx)"); + } + + // Test HEAD method + URI headUri = URI.create(testEndpoint.toUri() + "/gdc"); + ClientHttpRequest headRequest = factory.createRequest(headUri, HttpMethod.HEAD); + + try (ClientHttpResponse headResponse = headRequest.execute()) { + assertNotNull(headResponse, "HEAD response should not be null"); + assertEquals(200, headResponse.getStatusCode().value(), "HEAD should succeed"); + + // HEAD should have no body but same headers as GET + try (InputStream body = headResponse.getBody()) { + int firstByte = body.read(); + assertTrue(firstByte == -1, "HEAD response should have no body content"); + } + } + } + + /** + * Tests multiple consecutive requests to validate connection reuse and stream handling. + * Ensures proper cleanup between requests without resource leaks. + */ + @Test(groups = "httpFactory") + public void shouldHandleMultipleConsecutiveRequests() throws Exception { + if (testEndpoint == null) { + throw new org.testng.SkipException("Endpoint not configured"); + } + + URI uri = URI.create(testEndpoint.toUri() + "/gdc"); + + // Make multiple requests to test connection reuse and stream handling + for (int i = 0; i < 5; i++) { + ClientHttpRequest request = factory.createRequest(uri, HttpMethod.GET); + + try (ClientHttpResponse response = request.execute()) { + assertEquals(response.getStatusCode().value(), 200); + + // Read response completely to ensure proper stream cleanup + try (InputStream inputStream = response.getBody()) { + ByteArrayOutputStream result = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + int length; + while ((length = inputStream.read(buffer)) != -1) { + result.write(buffer, 0, length); + } + assertTrue(result.size() > 0); + } + } + } + } + + /** + * Tests thread safety under concurrent load. + * Validates that multiple threads can safely use the factory simultaneously. + */ + @Test(groups = "httpFactory") + public void shouldHandleConcurrentRequests() throws Exception { + if (testEndpoint == null) { + throw new org.testng.SkipException("Endpoint not configured"); + } + + URI uri = URI.create(testEndpoint.toUri() + "/gdc"); + ExecutorService executor = Executors.newFixedThreadPool(10); + + try { + @SuppressWarnings("unchecked") + CompletableFuture[] futures = new CompletableFuture[20]; + + for (int i = 0; i < 20; i++) { + futures[i] = CompletableFuture.supplyAsync(() -> { + try { + ClientHttpRequest request = factory.createRequest(uri, HttpMethod.GET); + + try (ClientHttpResponse response = request.execute()) { + if (response.getStatusCode().value() != 200) { + return false; + } + + try (InputStream inputStream = response.getBody()) { + ByteArrayOutputStream result = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + int length; + while ((length = inputStream.read(buffer)) != -1) { + result.write(buffer, 0, length); + } + return result.size() > 0; + } + } + } catch (IOException e) { + return false; + } + }, executor); + } + + // Wait for all requests to complete + for (CompletableFuture future : futures) { + assertTrue(future.get(30, TimeUnit.SECONDS), "Concurrent request failed"); + } + } finally { + executor.shutdown(); + executor.awaitTermination(10, TimeUnit.SECONDS); + } + } + + /** + * Tests authenticated requests using the integrated GoodData client. + * Validates that the HTTP factory properly handles authentication flows. + */ + @Test(groups = "httpFactory") + public void shouldHandleAuthenticatedRequests() throws Exception { + try { + // Test authenticated endpoint access through SDK + long startTime = System.currentTimeMillis(); + var account = gd.getAccountService().getCurrent(); + long authTime = System.currentTimeMillis() - startTime; + + assertNotNull(account, "Should successfully retrieve authenticated account data"); + assertTrue(authTime < PERFORMANCE_THRESHOLD_MS, + "Authentication request time (" + authTime + "ms) should be reasonable"); + + } catch (Exception e) { + // More specific error handling - don't silently ignore all exceptions + if (e.getMessage() != null && e.getMessage().contains("401")) { + throw new org.testng.SkipException("Authentication credentials not valid: " + e.getMessage()); + } else if (e.getMessage() != null && e.getMessage().contains("host")) { + throw new org.testng.SkipException("Host configuration issue: " + e.getMessage()); + } else { + throw new AssertionError("Unexpected authentication error: " + e.getMessage(), e); + } + } + } + + /** + * Tests stream safety and proper resource cleanup. + * Validates that streams can be safely closed and don't throw StreamClosedException. + */ + @Test(groups = "httpFactory") + public void shouldHandleStreamSafety() throws Exception { + if (testEndpoint == null) { + throw new org.testng.SkipException("Endpoint not configured"); + } + + URI uri = URI.create(testEndpoint.toUri() + "/gdc"); + + ClientHttpRequest request = factory.createRequest(uri, HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + // Get the input stream + InputStream inputStream = response.getBody(); + + // Read part of the stream + byte[] buffer = new byte[100]; + int bytesRead = inputStream.read(buffer); + assertTrue(bytesRead > 0); + + // Close the response (which should close the stream) + response.close(); + + // Try to read from the stream again - this should not throw StreamClosedException + try { + inputStream.read(); + // If we get here, either the stream is still readable or returned -1 (EOF) + // Both are acceptable - we just want to avoid StreamClosedException + } catch (IOException e) { + // IOException is acceptable, but not StreamClosedException specifically + assertTrue(!e.getClass().getSimpleName().equals("StreamClosedException"), + "Should not throw StreamClosedException: " + e.getMessage()); + } + } + + /** + * Tests connection pooling and reuse efficiency. + * Validates that connections are properly managed and returned to the pool. + */ + @Test(groups = "httpFactory") + public void shouldReuseConnectionsEfficiently() throws Exception { + if (testEndpoint == null) { + throw new org.testng.SkipException("Endpoint not configured"); + } + + URI uri = URI.create(testEndpoint.toUri() + "/gdc"); + + // Record initial connection pool state + var initialStats = connectionManager.getTotalStats(); + int initialActive = initialStats.getLeased(); + int initialAvailable = initialStats.getAvailable(); + + // Make multiple requests to same host - should reuse connections + final int requestCount = 5; + long totalResponseTime = 0; + + for (int i = 0; i < requestCount; i++) { + long requestStart = System.currentTimeMillis(); + + ClientHttpRequest request = factory.createRequest(uri, HttpMethod.GET); + + try (ClientHttpResponse response = request.execute()) { + assertEquals(200, response.getStatusCode().value(), + "Request " + (i + 1) + " should succeed"); + + // Fully consume response to ensure connection is reusable + try (InputStream inputStream = response.getBody()) { + byte[] buffer = new byte[1024]; + while (inputStream.read(buffer) != -1) { + // consume stream completely + } + } + + totalResponseTime += (System.currentTimeMillis() - requestStart); + } + + // Brief pause to allow connection pool management + Thread.sleep(10); + } + + // Validate connection pool efficiency + var finalStats = connectionManager.getTotalStats(); + int finalActive = finalStats.getLeased(); + int finalAvailable = finalStats.getAvailable(); + + // Connections should be returned to pool (not many more active than initially) + assertTrue(finalActive <= initialActive + MAX_CONNECTIONS_PER_ROUTE, + "Active connections (" + finalActive + ") should not exceed reasonable limit"); + + // Should have some available connections from reuse + assertTrue(finalAvailable >= initialAvailable, + "Available connections should not decrease: " + initialAvailable + " -> " + finalAvailable); + + // Performance should improve with connection reuse (later requests faster) + double avgResponseTime = (double) totalResponseTime / requestCount; + assertTrue(avgResponseTime < PERFORMANCE_THRESHOLD_MS / 2, + "Average response time (" + avgResponseTime + "ms) should benefit from connection reuse"); + } + + /** + * Tests performance characteristics under normal load. + * Validates response times and connection pool efficiency metrics. + */ + @Test(groups = "httpFactory") + public void shouldMeetPerformanceRequirements() throws Exception { + if (testEndpoint == null) { + throw new org.testng.SkipException("Endpoint not configured"); + } + + URI uri = URI.create(testEndpoint.toUri() + "/gdc"); + final int warmupRequests = 3; + final int testRequests = 10; + + // Warmup - establish connections + for (int i = 0; i < warmupRequests; i++) { + try (ClientHttpResponse response = factory.createRequest(uri, HttpMethod.GET).execute()) { + // Just consume to warm up connections + try (InputStream is = response.getBody()) { + while (is.read() != -1) { /* consume */ } + } + } + } + + // Measure performance of established connections + long totalTime = 0; + long minTime = Long.MAX_VALUE; + long maxTime = 0; + + for (int i = 0; i < testRequests; i++) { + long start = System.currentTimeMillis(); + + try (ClientHttpResponse response = factory.createRequest(uri, HttpMethod.GET).execute()) { + assertEquals(200, response.getStatusCode().value(), "Performance test request should succeed"); + + try (InputStream is = response.getBody()) { + while (is.read() != -1) { /* consume */ } + } + } + + long requestTime = System.currentTimeMillis() - start; + totalTime += requestTime; + minTime = Math.min(minTime, requestTime); + maxTime = Math.max(maxTime, requestTime); + } + + double avgTime = (double) totalTime / testRequests; + + // Performance assertions + assertTrue(avgTime < PERFORMANCE_THRESHOLD_MS, + "Average response time (" + avgTime + "ms) should be under threshold"); + assertTrue(maxTime < PERFORMANCE_THRESHOLD_MS * 2, + "Max response time (" + maxTime + "ms) should be reasonable"); + assertTrue(maxTime - minTime < PERFORMANCE_THRESHOLD_MS, + "Response time variance (" + (maxTime - minTime) + "ms) should be consistent"); + + // Connection pool should be healthy + var stats = connectionManager.getTotalStats(); + assertTrue(stats.getLeased() <= MAX_CONNECTIONS_PER_ROUTE, + "Should not leak connections: " + stats.getLeased() + " active"); + } +} \ No newline at end of file diff --git a/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryIT.java b/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryIT.java new file mode 100644 index 000000000..03cc4207d --- /dev/null +++ b/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryIT.java @@ -0,0 +1,458 @@ +/* + * (C) 2025 GoodData Corporation. + * This source code is licensed under the BSD-style license found in the + * LICENSE.txt file in the root directory of this source tree. + */ +package com.gooddata.sdk.common; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.hc.client5.http.classic.HttpClient; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; +import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.core5.http.StreamClosedException; +import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpResponse; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; + +import static net.jadler.Jadler.closeJadler; +import static net.jadler.Jadler.initJadler; +import static net.jadler.Jadler.onRequest; +import static net.jadler.Jadler.port; +import static org.testng.Assert.assertEquals; + +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertThrows; +import static org.testng.Assert.assertTrue; + +/** + * Integration tests for {@link HttpClient5ComponentsClientHttpRequestFactory} + * Tests real HTTP communication, stream handling, and error scenarios + */ +public class HttpClient5ComponentsClientHttpRequestFactoryIT { + + private HttpClient5ComponentsClientHttpRequestFactory factory; + private CloseableHttpClient httpClient; + private String baseUrl; + + @BeforeMethod + void setUp() { + initJadler().withDefaultResponseContentType("application/json"); + baseUrl = "http://localhost:" + port(); + httpClient = HttpClients.createDefault(); + factory = new HttpClient5ComponentsClientHttpRequestFactory(httpClient); + } + + @AfterMethod + void tearDown() throws IOException { + if (httpClient != null) { + httpClient.close(); + } + closeJadler(); + } + + @Test + void shouldPerformGetRequest() throws IOException { + // Given + Map responseData = new HashMap<>(); + responseData.put("message", "Hello World"); + responseData.put("status", "success"); + + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/test") + .respond() + .withStatus(200) + .withBody(new ObjectMapper().writeValueAsString(responseData)); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/test"), HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + // Then + assertEquals(200, response.getStatusCode().value()); + assertEquals("OK", response.getStatusText()); + + String responseBody = readResponseBody(response); + assertTrue(responseBody.contains("Hello World")); + assertTrue(responseBody.contains("success")); + + response.close(); + } + + @Test + void shouldPerformPostRequestWithJsonBody() throws IOException { + // Given + Map requestData = new HashMap<>(); + requestData.put("name", "Test User"); + requestData.put("email", "test@example.com"); + + onRequest() + .havingMethodEqualTo("POST") + .havingPathEqualTo("/api/users") + .havingHeaderEqualTo("Content-Type", "application/json") + .havingBodyEqualTo(new ObjectMapper().writeValueAsString(requestData)) + .respond() + .withStatus(201) + .withBody("{\"id\": 123, \"message\": \"User created\"}"); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/users"), HttpMethod.POST); + request.getHeaders().setContentType(MediaType.APPLICATION_JSON); + + String jsonBody = new ObjectMapper().writeValueAsString(requestData); + request.getBody().write(jsonBody.getBytes(StandardCharsets.UTF_8)); + + ClientHttpResponse response = request.execute(); + + // Then + assertEquals(201, response.getStatusCode().value()); + assertEquals("Created", response.getStatusText()); + + String responseBody = readResponseBody(response); + assertTrue(responseBody.contains("123")); + assertTrue(responseBody.contains("User created")); + + response.close(); + } + + @Test + void shouldHandleCustomHeaders() throws IOException { + // Given + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/secure") + .havingHeaderEqualTo("Authorization", "Bearer token123") + .havingHeaderEqualTo("X-Custom-Header", "custom-value") + .respond() + .withStatus(200) + .withBody("{\"authenticated\": true}"); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/secure"), HttpMethod.GET); + request.getHeaders().add("Authorization", "Bearer token123"); + request.getHeaders().add("X-Custom-Header", "custom-value"); + + ClientHttpResponse response = request.execute(); + + // Then + assertEquals(200, response.getStatusCode().value()); + String responseBody = readResponseBody(response); + assertTrue(responseBody.contains("authenticated")); + + response.close(); + } + + @Test + void shouldHandleErrorResponse() throws IOException { + // Given + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/notfound") + .respond() + .withStatus(404) + .withBody("{\"error\": \"Not Found\", \"message\": \"Resource not found\"}"); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/notfound"), HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + // Then + assertEquals(404, response.getStatusCode().value()); + assertEquals("Not Found", response.getStatusText()); + + String responseBody = readResponseBody(response); + assertTrue(responseBody.contains("Resource not found")); + + response.close(); + } + + @Test + void shouldHandleLargeResponseBody() throws IOException { + // Given + StringBuilder largeContent = new StringBuilder(); + for (int i = 0; i < 10000; i++) { + largeContent.append("This is line ").append(i).append(" of a large response.\n"); + } + + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/large") + .respond() + .withStatus(200) + .withBody(largeContent.toString()); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/large"), HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + // Then + assertEquals(200, response.getStatusCode().value()); + + String responseBody = readResponseBody(response); + assertTrue(responseBody.length() > 100000); + assertTrue(responseBody.contains("This is line 9999")); + + response.close(); + } + + @Test + void shouldHandleEmptyResponse() throws IOException { + // Given + onRequest() + .havingMethodEqualTo("DELETE") + .havingPathEqualTo("/api/resource/123") + .respond() + .withStatus(204); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/resource/123"), HttpMethod.DELETE); + ClientHttpResponse response = request.execute(); + + // Then + assertEquals(204, response.getStatusCode().value()); + assertEquals("No Content", response.getStatusText()); + + InputStream body = response.getBody(); + assertNotNull(body); + assertEquals(0, body.available()); + + response.close(); + } + + @Test + void shouldHandleResponseHeaders() throws IOException { + // Given + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/headers") + .respond() + .withStatus(200) + .withHeader("X-Response-Id", "abc123") + .withHeader("X-Rate-Limit", "1000") + .withHeader("Content-Type", "application/json; charset=utf-8") + .withBody("{\"data\": \"test\"}"); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/headers"), HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + // Then + assertEquals(200, response.getStatusCode().value()); + assertEquals("abc123", response.getHeaders().getFirst("X-Response-Id")); + assertEquals("1000", response.getHeaders().getFirst("X-Rate-Limit")); + assertTrue(response.getHeaders().getFirst("Content-Type").contains("application/json")); + + response.close(); + } + + @Test + void shouldHandleConnectionTimeout() throws IOException { + // Given - Create request to non-responsive endpoint + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/slow") + .respond() + .withDelay(5000, TimeUnit.MILLISECONDS) // 5 second delay + .withStatus(200) + .withBody("{\"delayed\": true}"); + + // When & Then + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/slow"), HttpMethod.GET); + + // This should work, but slowly + ClientHttpResponse response = request.execute(); + assertNotNull(response); + response.close(); + } + + @Test + void shouldPreventStreamClosedException() throws IOException { + // Given + String responseContent = "{\"test\": \"data\", \"large_field\": \"" + "x".repeat(10000) + "\"}"; + + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/stream-test") + .respond() + .withStatus(200) + .withBody(responseContent); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/stream-test"), HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + // Then - Multiple reads should not cause StreamClosedException + InputStream body1 = response.getBody(); + assertNotNull(body1); + + // Read part of the stream + byte[] buffer = new byte[1024]; + int bytesRead = body1.read(buffer); + assertTrue(bytesRead > 0); + + // Get body again - this should work with our implementation + InputStream body2 = response.getBody(); + assertNotNull(body2); + + // Clean close should not throw StreamClosedException + response.close(); + } + + @Test + void shouldHandleConcurrentRequests() throws Exception { + // Given + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/concurrent") + .respond() + .withStatus(200) + .withBody("{\"thread\": \"response\"}"); + + ExecutorService executor = Executors.newFixedThreadPool(10); + + // When - Execute 20 concurrent requests + @SuppressWarnings("unchecked") + CompletableFuture[] futures = new CompletableFuture[20]; + for (int i = 0; i < 20; i++) { + futures[i] = CompletableFuture.supplyAsync(() -> { + try { + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/concurrent"), HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + boolean success = response.getStatusCode().value() == 200; + response.close(); + return success; + } catch (Exception e) { + return false; + } + }, executor); + } + + // Then - All requests should succeed + for (CompletableFuture future : futures) { + assertTrue(future.get(10, TimeUnit.SECONDS)); + } + + executor.shutdown(); + } + + @Test + void shouldHandleConnectionRefused() { + // Given - Valid port that should refuse connection (assuming nothing runs on 54321) + HttpClient5ComponentsClientHttpRequestFactory invalidFactory = + new HttpClient5ComponentsClientHttpRequestFactory(httpClient); + + // When & Then + assertThrows(IOException.class, () -> { + ClientHttpRequest request = invalidFactory.createRequest( + URI.create("http://localhost:54321/nonexistent"), HttpMethod.GET); + request.execute(); + }); + } + + @Test + void shouldHandleInvalidUrl() throws IOException { + // Given + ClientHttpRequest request = factory.createRequest( + URI.create("http://invalid-host-that-does-not-exist.invalid/api"), HttpMethod.GET); + + // When & Then + assertThrows(IOException.class, () -> request.execute()); + } + + @Test + void shouldHandleMultipleCloseCallsGracefully() throws IOException { + // Given + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/close-test") + .respond() + .withStatus(200) + .withBody("{\"test\": \"close\"}"); + + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/close-test"), HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + // When & Then - Multiple closes should not throw exceptions + response.close(); + response.close(); // Should not throw + response.close(); // Should not throw + + assertEquals(200, response.getStatusCode().value()); + } + + @Test + void shouldHandleResponseBodyAfterPartialRead() throws IOException { + // Given + String longContent = "START:" + "x".repeat(50000) + ":END"; + + onRequest() + .havingMethodEqualTo("GET") + .havingPathEqualTo("/api/partial-read") + .respond() + .withStatus(200) + .withBody(longContent); + + // When + ClientHttpRequest request = factory.createRequest( + URI.create(baseUrl + "/api/partial-read"), HttpMethod.GET); + ClientHttpResponse response = request.execute(); + + InputStream body = response.getBody(); + + // Read only first 100 bytes + byte[] buffer = new byte[100]; + int bytesRead = body.read(buffer); + assertEquals(100, bytesRead); + + String partialContent = new String(buffer, StandardCharsets.UTF_8); + assertTrue(partialContent.startsWith("START:")); + + // Then - Close should handle remaining content gracefully + response.close(); // Should not throw StreamClosedException + + assertEquals(200, response.getStatusCode().value()); + } + + /** + * Helper method to read response body as string + */ + private String readResponseBody(ClientHttpResponse response) throws IOException { + try (InputStream inputStream = response.getBody()) { + ByteArrayOutputStream result = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + int length; + while ((length = inputStream.read(buffer)) != -1) { + result.write(buffer, 0, length); + } + return result.toString(StandardCharsets.UTF_8); + } + } +} \ No newline at end of file diff --git a/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryTest.java b/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryTest.java index c1acc3ddb..eb98a17f6 100644 --- a/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryTest.java +++ b/gooddata-java/src/test/java/com/gooddata/sdk/common/HttpClient5ComponentsClientHttpRequestFactoryTest.java @@ -6,21 +6,58 @@ package com.gooddata.sdk.common; import org.apache.hc.client5.http.classic.HttpClient; +import org.apache.hc.client5.http.classic.methods.HttpPost; +import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.impl.classic.HttpClients; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.ClassicHttpResponse; +import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpEntity; +import org.mockito.ArgumentCaptor; import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; import org.springframework.http.client.ClientHttpRequest; +import org.springframework.http.client.ClientHttpResponse; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Method; import java.net.URI; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertThrows; +import static org.testng.Assert.assertTrue; /** - * Test for {@link HttpClient5ComponentsClientHttpRequestFactory} + * Comprehensive unit tests for {@link HttpClient5ComponentsClientHttpRequestFactory} */ class HttpClient5ComponentsClientHttpRequestFactoryTest { + private HttpClient mockHttpClient; + private CloseableHttpClient mockCloseableHttpClient; + private HttpClient5ComponentsClientHttpRequestFactory factory; + private ClassicHttpResponse mockResponse; + + @BeforeMethod + void setUp() { + mockHttpClient = mock(HttpClient.class); + mockCloseableHttpClient = mock(CloseableHttpClient.class); + mockResponse = mock(ClassicHttpResponse.class); + factory = new HttpClient5ComponentsClientHttpRequestFactory(mockHttpClient); + } + @Test void shouldCreateRequestWithHttpClient5() throws Exception { // Given @@ -64,4 +101,258 @@ void shouldRequireNonNullHttpClient() { // When & Then new HttpClient5ComponentsClientHttpRequestFactory(null); } + + @Test + public void shouldThrowExceptionForInvalidHttpMethod() throws Exception { + // Given + HttpClient httpClient = HttpClients.createDefault(); + HttpClient5ComponentsClientHttpRequestFactory factory = new HttpClient5ComponentsClientHttpRequestFactory(httpClient); + + // When & Then - Test that creating request with invalid method throws exception + try { + factory.createRequest(URI.create("http://test.com"), HttpMethod.valueOf("INVALID")); + assertTrue(false, "Should have thrown exception for invalid HTTP method"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Invalid HTTP method")); + } + } + + @Test + void shouldHandleRequestExecution() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getReasonPhrase()).thenReturn("OK"); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.GET); + + // When + ClientHttpResponse response = request.execute(); + + // Then + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + assertEquals("OK", response.getStatusText()); + } + + @Test + void shouldHandleRequestBodyAndHeaders() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.POST); + + // When + request.getHeaders().setContentType(MediaType.APPLICATION_JSON); + request.getHeaders().add("Custom-Header", "test-value"); + request.getBody().write("{\"test\": \"data\"}".getBytes()); + + // Then + assertNotNull(request.getHeaders()); + assertEquals(MediaType.APPLICATION_JSON, request.getHeaders().getContentType()); + assertTrue(request.getHeaders().containsKey("Custom-Header")); + + ClientHttpResponse response = request.execute(); + assertNotNull(response); + } + + @Test + void shouldHandleResponseWithEntity() throws Exception { + // Given + HttpEntity mockEntity = mock(HttpEntity.class); + InputStream testStream = new ByteArrayInputStream("test response".getBytes()); + + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + when(mockResponse.getEntity()).thenReturn(mockEntity); + when(mockEntity.getContent()).thenReturn(testStream); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.GET); + + // When + ClientHttpResponse response = request.execute(); + + // Then + assertNotNull(response.getBody()); + assertTrue(response.getBody().available() > 0); + } + + @Test + void shouldHandleResponseWithoutEntity() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(204); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + when(mockResponse.getEntity()).thenReturn(null); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.DELETE); + + // When + ClientHttpResponse response = request.execute(); + + // Then + assertNotNull(response.getBody()); + assertEquals(0, response.getBody().available()); + } + + @Test + void shouldHandleCloseableHttpClient() throws Exception { + // Given + factory = new HttpClient5ComponentsClientHttpRequestFactory(mockCloseableHttpClient); + when(mockCloseableHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.GET); + + // When + ClientHttpResponse response = request.execute(); + + // Then + assertNotNull(response); + verify(mockCloseableHttpClient).executeOpen(isNull(), any(ClassicHttpRequest.class), isNull()); + } + + @Test + void shouldSkipSystemManagedHeaders() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.POST); + + // When - Add headers that should be skipped + request.getHeaders().add("Content-Length", "123"); + request.getHeaders().add("Transfer-Encoding", "chunked"); + request.getHeaders().add("Connection", "keep-alive"); + request.getHeaders().add("Host", "test.com"); + request.getHeaders().add("Custom-Header", "should-be-added"); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ClassicHttpRequest.class); + ClientHttpResponse response = request.execute(); + + // Then + assertNotNull(response); + verify(mockHttpClient).executeOpen(isNull(), requestCaptor.capture(), isNull()); + ClassicHttpRequest capturedRequest = requestCaptor.getValue(); + + // System managed headers should not be present + assertFalse(capturedRequest.containsHeader("Content-Length")); + assertFalse(capturedRequest.containsHeader("Transfer-Encoding")); + assertFalse(capturedRequest.containsHeader("Connection")); + assertFalse(capturedRequest.containsHeader("Host")); + + // Custom header should be present + assertTrue(capturedRequest.containsHeader("Custom-Header")); + } + + @Test + void shouldHandleContentTypeFromHeaders() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.POST); + + // When + request.getHeaders().setContentType(MediaType.APPLICATION_XML); + request.getBody().write("test".getBytes()); + + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ClassicHttpRequest.class); + ClientHttpResponse response = request.execute(); + + // Then + assertNotNull(response); + verify(mockHttpClient).executeOpen(isNull(), requestCaptor.capture(), isNull()); + ClassicHttpRequest capturedRequest = requestCaptor.getValue(); + + if (capturedRequest instanceof HttpPost) { + HttpPost postRequest = (HttpPost) capturedRequest; + HttpEntity entity = postRequest.getEntity(); + assertNotNull(entity); + assertEquals(ContentType.APPLICATION_XML.getMimeType(), + entity.getContentType().split(";")[0]); + } + } + + @Test + void shouldHandleInvalidContentType() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.POST); + + // When - Set invalid content type + request.getHeaders().add("Content-Type", "invalid/content-type/malformed"); + request.getBody().write("test".getBytes()); + + // Then - Should not throw exception, should use default + ClientHttpResponse response = request.execute(); + assertNotNull(response); + assertEquals(200, response.getStatusCode().value()); + } + + @Test + void shouldHandleResponseClose() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.GET); + + // When + ClientHttpResponse response = request.execute(); + response.close(); + + // Then + verify(mockResponse).close(); + } + + @Test + void shouldHandleResponseCloseException() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenReturn(mockResponse); + when(mockResponse.getCode()).thenReturn(200); + when(mockResponse.getHeaders()).thenReturn(new org.apache.hc.core5.http.Header[0]); + doThrow(new IOException("Close failed")).when(mockResponse).close(); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.GET); + + // When + ClientHttpResponse response = request.execute(); + + // Then - Should not throw exception + response.close(); // Should handle IOException gracefully + } + + @Test + void shouldHandleExecutionException() throws Exception { + // Given + when(mockHttpClient.executeOpen(isNull(), any(ClassicHttpRequest.class), isNull())) + .thenThrow(new IOException("Connection failed")); + + ClientHttpRequest request = factory.createRequest(URI.create("http://test.com"), HttpMethod.GET); + + // When & Then + assertThrows(IOException.class, () -> request.execute()); + } } \ No newline at end of file diff --git a/gooddata-java/src/test/java/com/gooddata/sdk/service/AbstractGoodDataAT.java b/gooddata-java/src/test/java/com/gooddata/sdk/service/AbstractGoodDataAT.java index 3ef7c9b7c..f3d5026b9 100644 --- a/gooddata-java/src/test/java/com/gooddata/sdk/service/AbstractGoodDataAT.java +++ b/gooddata-java/src/test/java/com/gooddata/sdk/service/AbstractGoodDataAT.java @@ -63,9 +63,18 @@ public static String getProperty(String name) { @AfterSuite public static void removeProjectAndLogout() { - if (project != null) { - gd.getProjectService().removeProject(project); + try { + if (project != null) { + gd.getProjectService().removeProject(project); + } + } catch (Exception e) { + System.err.println("Failed to remove project during cleanup: " + e.getMessage()); + } + + try { + gd.logout(); + } catch (Exception e) { + System.err.println("Failed to logout during cleanup: " + e.getMessage()); } - gd.logout(); } } diff --git a/gooddata-java/src/test/java/com/gooddata/sdk/service/dataload/OutputStageServiceAT.java b/gooddata-java/src/test/java/com/gooddata/sdk/service/dataload/OutputStageServiceAT.java index a7a7774c6..04ab14b4e 100644 --- a/gooddata-java/src/test/java/com/gooddata/sdk/service/dataload/OutputStageServiceAT.java +++ b/gooddata-java/src/test/java/com/gooddata/sdk/service/dataload/OutputStageServiceAT.java @@ -25,10 +25,15 @@ public class OutputStageServiceAT extends AbstractGoodDataAT { private static final String CLIENT_ID = "clientId"; private static final String PREFIX = "prefix"; - private final Warehouse warehouse; - private final WarehouseSchema warehouseSchema; + private Warehouse warehouse; + private WarehouseSchema warehouseSchema; public OutputStageServiceAT() { + // Constructor should be lightweight - no I/O operations + } + + @Test(groups = "warehouse", dependsOnGroups = {"project"}) + public void setupWarehouse() { final String warehouseToken = getProperty("warehouseToken"); final Warehouse wh = new Warehouse(title, warehouseToken); wh.setEnvironment(Environment.TESTING); @@ -36,7 +41,7 @@ public OutputStageServiceAT() { warehouseSchema = gd.getWarehouseService().getDefaultWarehouseSchema(warehouse); } - @Test(groups = "output_stage", dependsOnGroups = {"warehouse", "project"}) + @Test(groups = "output_stage", dependsOnGroups = {"warehouse", "project"}, dependsOnMethods = "setupWarehouse") public void shouldReturnNullObjectWhenNoOutputStage() { final OutputStage outputStage = gd.getOutputStageService().getOutputStage(project); diff --git a/gooddata-java/src/test/java/com/gooddata/sdk/service/warehouse/WarehouseServiceAT.java b/gooddata-java/src/test/java/com/gooddata/sdk/service/warehouse/WarehouseServiceAT.java index 42a62aa31..637cf375c 100644 --- a/gooddata-java/src/test/java/com/gooddata/sdk/service/warehouse/WarehouseServiceAT.java +++ b/gooddata-java/src/test/java/com/gooddata/sdk/service/warehouse/WarehouseServiceAT.java @@ -46,7 +46,7 @@ public class WarehouseServiceAT extends AbstractGoodDataAT { private static final String SCHEMA_NAME = "default"; private final String warehouseToken; - private final WarehouseService service; + private WarehouseService service; private Warehouse warehouse; private Warehouse warehouse2; @@ -56,6 +56,10 @@ public class WarehouseServiceAT extends AbstractGoodDataAT { public WarehouseServiceAT() { warehouseToken = getProperty("warehouseToken"); + } + + @BeforeClass + public void initWarehouseService() { service = gd.getWarehouseService(); } @@ -64,7 +68,7 @@ public void initIsolatedDomainGroup() { account = gd.getAccountService().createAccount(new Account(LOGIN, "nnPvcGXU7f", "FirstName", "LastName"), getProperty("domain")); } - @Test(groups = "warehouse", dependsOnGroups = "account") + @Test(groups = "warehouse") public void createWarehouse() { final Warehouse wh = new Warehouse(title, warehouseToken); wh.setEnvironment(Environment.TESTING);