PIT-Lücken in adapter-out gezielt geschlossen
This commit is contained in:
@@ -602,6 +602,64 @@ class AnthropicClaudeHttpAdapterTest {
|
|||||||
.startsWith("https://api.anthropic.com");
|
.startsWith("https://api.anthropic.com");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Verifies that a custom, non-default base URL is used in the request.
|
||||||
|
* <p>
|
||||||
|
* This test uses a URL that differs from the default {@code https://api.anthropic.com},
|
||||||
|
* ensuring the conditional that selects between the configured URL and the default
|
||||||
|
* is correctly evaluated. If the conditional were negated, the request would be sent
|
||||||
|
* to the default URL instead of the custom one.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
@DisplayName("should use custom non-default base URL when provided")
|
||||||
|
void customNonDefaultBaseUrlIsUsedInRequest() throws Exception {
|
||||||
|
String customBaseUrl = "http://internal.proxy.example.com:8080";
|
||||||
|
ProviderConfiguration configWithCustomUrl = new ProviderConfiguration(
|
||||||
|
API_MODEL, TIMEOUT_SECONDS, customBaseUrl, API_KEY);
|
||||||
|
AnthropicClaudeHttpAdapter adapterWithCustomUrl =
|
||||||
|
new AnthropicClaudeHttpAdapter(configWithCustomUrl, httpClient);
|
||||||
|
|
||||||
|
HttpResponse<String> httpResponse = mockHttpResponse(200,
|
||||||
|
buildAnthropicSuccessResponse("{\"title\":\"T\",\"reasoning\":\"R\"}"));
|
||||||
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
|
adapterWithCustomUrl.invoke(createTestRequest("p", "d"));
|
||||||
|
|
||||||
|
ArgumentCaptor<HttpRequest> requestCaptor = ArgumentCaptor.forClass(HttpRequest.class);
|
||||||
|
verify(httpClient).send(requestCaptor.capture(), any());
|
||||||
|
assertThat(requestCaptor.getValue().uri().toString())
|
||||||
|
.as("Custom non-default base URL must be used, not the default api.anthropic.com")
|
||||||
|
.startsWith("http://internal.proxy.example.com:8080");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Verifies that a port value of 0 in the base URL is not included in the endpoint URI.
|
||||||
|
* <p>
|
||||||
|
* {@link java.net.URI#getPort()} returns {@code 0} when the URL explicitly specifies
|
||||||
|
* port 0. The endpoint builder must only include the port when it is greater than 0,
|
||||||
|
* not when it is equal to 0 or negative.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
@DisplayName("should not include port 0 in the endpoint URI")
|
||||||
|
void buildEndpointUri_doesNotIncludePortZero() throws Exception {
|
||||||
|
ProviderConfiguration configWithPortZero = new ProviderConfiguration(
|
||||||
|
API_MODEL, TIMEOUT_SECONDS, "http://example.com:0", API_KEY);
|
||||||
|
AnthropicClaudeHttpAdapter adapterWithPortZero =
|
||||||
|
new AnthropicClaudeHttpAdapter(configWithPortZero, httpClient);
|
||||||
|
|
||||||
|
HttpResponse<String> httpResponse = mockHttpResponse(200,
|
||||||
|
buildAnthropicSuccessResponse("{\"title\":\"T\",\"reasoning\":\"R\"}"));
|
||||||
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
|
adapterWithPortZero.invoke(createTestRequest("p", "d"));
|
||||||
|
|
||||||
|
ArgumentCaptor<HttpRequest> requestCaptor = ArgumentCaptor.forClass(HttpRequest.class);
|
||||||
|
verify(httpClient).send(requestCaptor.capture(), any());
|
||||||
|
assertThat(requestCaptor.getValue().uri().toString())
|
||||||
|
.as("Port 0 must not appear in the endpoint URI")
|
||||||
|
.doesNotContain(":0");
|
||||||
|
}
|
||||||
|
|
||||||
// =========================================================================
|
// =========================================================================
|
||||||
// Helper methods
|
// Helper methods
|
||||||
// =========================================================================
|
// =========================================================================
|
||||||
|
|||||||
@@ -555,6 +555,33 @@ class OpenAiHttpAdapterTest {
|
|||||||
assertThat(((AiInvocationTechnicalFailure) result).failureReason()).isEqualTo("TIMEOUT");
|
assertThat(((AiInvocationTechnicalFailure) result).failureReason()).isEqualTo("TIMEOUT");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Verifies that a port value of 0 in the base URL is not included in the endpoint URI.
|
||||||
|
* <p>
|
||||||
|
* {@link java.net.URI#getPort()} returns {@code 0} when the URL explicitly specifies
|
||||||
|
* port 0. The endpoint builder must only include the port when it is greater than 0,
|
||||||
|
* not when it is equal to 0 or negative.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
@DisplayName("should not include port 0 in the endpoint URI")
|
||||||
|
void buildEndpointUri_doesNotIncludePortZero() throws Exception {
|
||||||
|
ProviderConfiguration configWithPortZero = new ProviderConfiguration(
|
||||||
|
API_MODEL, TIMEOUT_SECONDS, "http://example.com:0", API_KEY);
|
||||||
|
OpenAiHttpAdapter adapterWithPortZero = new OpenAiHttpAdapter(configWithPortZero, httpClient);
|
||||||
|
|
||||||
|
HttpResponse<String> httpResponse = mockHttpResponse(200,
|
||||||
|
"{\"choices\":[{\"message\":{\"content\":\"test\"}}]}");
|
||||||
|
doReturn(httpResponse).when(httpClient).send(any(HttpRequest.class), any());
|
||||||
|
|
||||||
|
adapterWithPortZero.invoke(createTestRequest("p", "d"));
|
||||||
|
|
||||||
|
ArgumentCaptor<HttpRequest> requestCaptor = ArgumentCaptor.forClass(HttpRequest.class);
|
||||||
|
verify(httpClient).send(requestCaptor.capture(), any());
|
||||||
|
assertThat(requestCaptor.getValue().uri().toString())
|
||||||
|
.as("Port 0 must not appear in the endpoint URI")
|
||||||
|
.doesNotContain(":0");
|
||||||
|
}
|
||||||
|
|
||||||
// Helper methods
|
// Helper methods
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -348,4 +348,100 @@ class LegacyConfigurationMigratorTest {
|
|||||||
assertTrue(migrated.containsKey("ai.provider.openai-compatible.model"),
|
assertTrue(migrated.containsKey("ai.provider.openai-compatible.model"),
|
||||||
"Migrated file must contain the new namespaced model key (complete write confirmed)");
|
"Migrated file must contain the new namespaced model key (complete write confirmed)");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// =========================================================================
|
||||||
|
// Tests: isLegacyForm – each individual legacy key triggers detection
|
||||||
|
// =========================================================================
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A properties set containing only {@code api.baseUrl} (without {@code ai.provider.active})
|
||||||
|
* must be detected as legacy.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void isLegacyForm_detectedWhenOnlyBaseUrlPresent() {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.setProperty(LegacyConfigurationMigrator.LEGACY_BASE_URL, "https://api.example.com");
|
||||||
|
assertTrue(defaultMigrator().isLegacyForm(props),
|
||||||
|
"Properties with only api.baseUrl must be detected as legacy");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A properties set containing only {@code api.model} (without {@code ai.provider.active})
|
||||||
|
* must be detected as legacy.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void isLegacyForm_detectedWhenOnlyModelPresent() {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.setProperty(LegacyConfigurationMigrator.LEGACY_MODEL, "gpt-4o");
|
||||||
|
assertTrue(defaultMigrator().isLegacyForm(props),
|
||||||
|
"Properties with only api.model must be detected as legacy");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A properties set containing only {@code api.timeoutSeconds} (without {@code ai.provider.active})
|
||||||
|
* must be detected as legacy.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void isLegacyForm_detectedWhenOnlyTimeoutPresent() {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.setProperty(LegacyConfigurationMigrator.LEGACY_TIMEOUT, "30");
|
||||||
|
assertTrue(defaultMigrator().isLegacyForm(props),
|
||||||
|
"Properties with only api.timeoutSeconds must be detected as legacy");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A properties set containing only {@code api.key} (without {@code ai.provider.active})
|
||||||
|
* must be detected as legacy.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void isLegacyForm_detectedWhenOnlyApiKeyPresent() {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.setProperty(LegacyConfigurationMigrator.LEGACY_API_KEY, "sk-test");
|
||||||
|
assertTrue(defaultMigrator().isLegacyForm(props),
|
||||||
|
"Properties with only api.key must be detected as legacy");
|
||||||
|
}
|
||||||
|
|
||||||
|
// =========================================================================
|
||||||
|
// Tests: lineDefinesKey / generateMigratedContent – prefix-only match must not fire
|
||||||
|
// =========================================================================
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A line whose key is a prefix of a legacy key (e.g. {@code api.baseUrlExtra}) must not
|
||||||
|
* be treated as defining the legacy key ({@code api.baseUrl}) and must survive migration
|
||||||
|
* unchanged while the actual legacy key is correctly replaced.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void generateMigratedContent_doesNotReplacePrefixMatchKey() {
|
||||||
|
String content = "api.baseUrlExtra=should-not-change\n"
|
||||||
|
+ "api.baseUrl=https://real.example.com\n"
|
||||||
|
+ "api.model=gpt-4o\n"
|
||||||
|
+ "api.timeoutSeconds=30\n"
|
||||||
|
+ "api.key=sk-real\n";
|
||||||
|
|
||||||
|
String migrated = defaultMigrator().generateMigratedContent(content);
|
||||||
|
|
||||||
|
assertTrue(migrated.contains("api.baseUrlExtra=should-not-change"),
|
||||||
|
"Line with key that is a prefix of a legacy key must not be modified");
|
||||||
|
assertTrue(migrated.contains("ai.provider.openai-compatible.baseUrl=https://real.example.com"),
|
||||||
|
"The actual legacy key api.baseUrl must be replaced with the namespaced key");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A line that defines a legacy key with no value (key only, no separator)
|
||||||
|
* must be recognized as defining that key and be replaced in migration.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void generateMigratedContent_handlesKeyWithoutValue() {
|
||||||
|
String content = "api.baseUrl\n"
|
||||||
|
+ "api.model=gpt-4o\n"
|
||||||
|
+ "api.timeoutSeconds=30\n"
|
||||||
|
+ "api.key=sk-test\n";
|
||||||
|
|
||||||
|
String migrated = defaultMigrator().generateMigratedContent(content);
|
||||||
|
|
||||||
|
assertTrue(migrated.contains("ai.provider.openai-compatible.baseUrl"),
|
||||||
|
"Key-only line (no value, no separator) must still be recognized and replaced");
|
||||||
|
assertFalse(migrated.contains("api.baseUrl\n") || migrated.contains("api.baseUrl\r"),
|
||||||
|
"Original key-only line must not survive unchanged");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -416,4 +416,48 @@ class MultiProviderConfigurationTest {
|
|||||||
config.claudeConfig().apiKey(),
|
config.claudeConfig().apiKey(),
|
||||||
"Inactive Claude config should still pick up its own env var");
|
"Inactive Claude config should still pick up its own env var");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// =========================================================================
|
||||||
|
// Tests: timeout validation
|
||||||
|
// =========================================================================
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Active provider has timeout set to 0. Validation must fail and mention timeoutSeconds.
|
||||||
|
* This verifies that validateTimeoutSeconds is called and that the boundary is strictly
|
||||||
|
* positive (i.e. 0 is rejected, not just negative values).
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void rejectsZeroTimeoutForActiveProvider() {
|
||||||
|
Properties props = fullOpenAiProperties();
|
||||||
|
props.setProperty("ai.provider.openai-compatible.timeoutSeconds", "0");
|
||||||
|
|
||||||
|
MultiProviderConfigurationParser parser = new MultiProviderConfigurationParser(NO_ENV);
|
||||||
|
MultiProviderConfiguration config = parser.parse(props);
|
||||||
|
|
||||||
|
InvalidStartConfigurationException ex = assertThrows(
|
||||||
|
InvalidStartConfigurationException.class,
|
||||||
|
() -> new MultiProviderConfigurationValidator().validate(config));
|
||||||
|
|
||||||
|
assertTrue(ex.getMessage().contains("timeoutSeconds"),
|
||||||
|
"Error message must reference timeoutSeconds");
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Active Claude provider has timeout set to 0. Same invariant for the other provider family.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void rejectsZeroTimeoutForActiveClaudeProvider() {
|
||||||
|
Properties props = fullClaudeProperties();
|
||||||
|
props.setProperty("ai.provider.claude.timeoutSeconds", "0");
|
||||||
|
|
||||||
|
MultiProviderConfigurationParser parser = new MultiProviderConfigurationParser(NO_ENV);
|
||||||
|
MultiProviderConfiguration config = parser.parse(props);
|
||||||
|
|
||||||
|
InvalidStartConfigurationException ex = assertThrows(
|
||||||
|
InvalidStartConfigurationException.class,
|
||||||
|
() -> new MultiProviderConfigurationValidator().validate(config));
|
||||||
|
|
||||||
|
assertTrue(ex.getMessage().contains("timeoutSeconds"),
|
||||||
|
"Error message must reference timeoutSeconds");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -209,4 +209,22 @@ class SourceDocumentCandidatesPortAdapterTest {
|
|||||||
assertTrue(candidates.stream().allMatch(c -> c.uniqueIdentifier().endsWith(".pdf")),
|
assertTrue(candidates.stream().allMatch(c -> c.uniqueIdentifier().endsWith(".pdf")),
|
||||||
"All candidates should be PDF files");
|
"All candidates should be PDF files");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A directory whose name ends with {@code .pdf} must not be included as a candidate.
|
||||||
|
* <p>
|
||||||
|
* The regular-file filter must exclude directories even when their name matches the
|
||||||
|
* PDF extension, so that only actual PDF files are returned.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void testLoadCandidates_DirectoryWithPdfExtensionIsExcluded() throws IOException {
|
||||||
|
Files.write(tempDir.resolve("real.pdf"), "content".getBytes());
|
||||||
|
Files.createDirectory(tempDir.resolve("looks-like.pdf"));
|
||||||
|
|
||||||
|
List<SourceDocumentCandidate> candidates = adapter.loadCandidates();
|
||||||
|
|
||||||
|
assertEquals(1, candidates.size(),
|
||||||
|
"A directory with .pdf extension must not be included as a candidate");
|
||||||
|
assertEquals("real.pdf", candidates.get(0).uniqueIdentifier());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
package de.gecheckt.pdf.umbenenner.adapter.out.sqlite;
|
package de.gecheckt.pdf.umbenenner.adapter.out.sqlite;
|
||||||
|
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||||
import static org.junit.jupiter.api.Assertions.assertSame;
|
import static org.junit.jupiter.api.Assertions.assertSame;
|
||||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||||
@@ -194,4 +195,40 @@ class SqliteUnitOfWorkAdapterTest {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Verifies that a document record written inside a successful transaction is persisted.
|
||||||
|
* <p>
|
||||||
|
* This confirms that the actual write operation is invoked and the transaction is
|
||||||
|
* committed. Without an actual call to the underlying repository, the record would
|
||||||
|
* not be retrievable after the transaction completes.
|
||||||
|
*/
|
||||||
|
@Test
|
||||||
|
void executeInTransaction_committedRecordIsRetrievable() {
|
||||||
|
DocumentFingerprint fingerprint = new DocumentFingerprint(
|
||||||
|
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
|
||||||
|
Instant now = Instant.now().truncatedTo(ChronoUnit.MICROS);
|
||||||
|
DocumentRecord record = new DocumentRecord(
|
||||||
|
fingerprint,
|
||||||
|
new SourceDocumentLocator("/source/commit-test.pdf"),
|
||||||
|
"commit-test.pdf",
|
||||||
|
ProcessingStatus.PROCESSING,
|
||||||
|
FailureCounters.zero(),
|
||||||
|
null,
|
||||||
|
null,
|
||||||
|
now,
|
||||||
|
now,
|
||||||
|
null,
|
||||||
|
null
|
||||||
|
);
|
||||||
|
|
||||||
|
SqliteDocumentRecordRepositoryAdapter docRepository =
|
||||||
|
new SqliteDocumentRecordRepositoryAdapter(jdbcUrl);
|
||||||
|
|
||||||
|
unitOfWorkAdapter.executeInTransaction(txOps -> txOps.createDocumentRecord(record));
|
||||||
|
|
||||||
|
var result = docRepository.findByFingerprint(fingerprint);
|
||||||
|
assertFalse(result instanceof de.gecheckt.pdf.umbenenner.application.port.out.DocumentUnknown,
|
||||||
|
"Record must be persisted and retrievable after a successfully committed transaction");
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user