Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

polish openai RAG #135

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions rag/rag-springai-openai-llm/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@
<groupId>org.springframework.ai</groupId>
<artifactId>spring-ai-tika-document-reader</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
<version>1.27.1</version>
</dependency>
<dependency>
<groupId>org.springframework.retry</groupId>
<artifactId>spring-retry</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.boot.web.client.ClientHttpRequestFactories;
import org.springframework.boot.web.client.ClientHttpRequestFactorySettings;
import org.springframework.boot.http.client.ClientHttpRequestFactoryBuilder;
import org.springframework.boot.web.client.RestClientCustomizer;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
Expand All @@ -34,7 +33,7 @@ class ResponseHeadersModification {
RestClientCustomizer restClientCustomizer() {
return restClientBuilder -> restClientBuilder
.requestFactory(new BufferingClientHttpRequestFactory(
ClientHttpRequestFactories.get(ClientHttpRequestFactorySettings.DEFAULTS)))
ClientHttpRequestFactoryBuilder.detect().build()))
.requestInterceptor((request, body, execution) -> {
logRequest(request, body);
ClientHttpResponse response = execution.execute(request, body);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,13 @@
package com.learning.ai.llmragwithspringai;

import com.learning.ai.llmragwithspringai.config.ContainersConfig;
import org.springframework.boot.SpringApplication;
import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
import org.springframework.context.annotation.Bean;
import org.testcontainers.containers.PostgreSQLContainer;
import org.testcontainers.utility.DockerImageName;

@TestConfiguration(proxyBeanMethods = false)
public class TestLlmRagWithSpringAiApplication {

@Bean
@ServiceConnection
PostgreSQLContainer<?> pgvectorContainer() {
return new PostgreSQLContainer<>(DockerImageName.parse("pgvector/pgvector:pg17"));
}

public static void main(String[] args) {
SpringApplication.from(LlmRagWithSpringAiApplication::main)
.with(TestLlmRagWithSpringAiApplication.class)
.with(ContainersConfig.class)
.run(args);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,9 @@

import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.learning.ai.llmragwithspringai.TestLlmRagWithSpringAiApplication;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.web.servlet.MockMvc;

@SpringBootTest(
webEnvironment = RANDOM_PORT,
classes = {TestLlmRagWithSpringAiApplication.class})
@AutoConfigureMockMvc
public abstract class AbstractIntegrationTest {

@Autowired
protected MockMvc mockMvc;

@Autowired
protected ObjectMapper objectMapper;
}
classes = {ContainersConfig.class})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

TestQueryController needs MockMvc configuration after base class changes

The removal of MockMvc from AbstractIntegrationTest will break TestQueryController as it relies on the mockMvc field. The test needs to be updated with its own MockMvc configuration.

  • Add @AutoConfigureMockMvc to TestQueryController
  • Add @Autowired MockMvc field declaration
🔗 Analysis chain

Verify ContainersConfig implementation and its impact on tests

The test configuration has been moved to ContainersConfig. Let's verify its implementation and ensure all necessary container configurations are present.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify ContainersConfig implementation and its usage

# Check ContainersConfig implementation
echo "Checking ContainersConfig implementation:"
rg -A 10 "class ContainersConfig" 

# Check if any tests might be broken by this change
echo "Checking tests that extend AbstractIntegrationTest:"
rg "extends\s+AbstractIntegrationTest"

Length of output: 1899


Script:

#!/bin/bash
# Check if the dependent tests have their own MockMvc setup
echo "Checking MockMvc usage in dependent tests:"
rg -A 5 "MockMvc|@AutoConfigureMockMvc" \
    "rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java" \
    "rag/rag-springai-openai-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java" \
    "embeddingstores/pgvector-langchain4j/src/test/java/com/learning/ai/controller/TestQueryController.java"

# Check test class configurations
echo -e "\nChecking test class configurations:"
rg -B 2 -A 2 "class.*extends AbstractIntegrationTest" \
    "rag/rag-springai-ollama-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java" \
    "rag/rag-springai-openai-llm/src/test/java/com/learning/ai/llmragwithspringai/LlmRagWithSpringAiApplicationIntTest.java" \
    "embeddingstores/pgvector-langchain4j/src/test/java/com/learning/ai/controller/TestQueryController.java"

Length of output: 4347


Script:

#!/bin/bash
# Check complete TestQueryController implementation for MockMvc setup
echo "Checking TestQueryController implementation:"
cat "embeddingstores/pgvector-langchain4j/src/test/java/com/learning/ai/controller/TestQueryController.java"

# Check if there's a separate test configuration in the same package
echo -e "\nChecking for additional test configurations:"
fd -e java -p "Test.*Config|.*TestConfig" "embeddingstores/pgvector-langchain4j/src/test"

Length of output: 1925

public abstract class AbstractIntegrationTest {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.learning.ai.llmragwithspringai.config;

import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
import org.springframework.context.annotation.Bean;
import org.testcontainers.containers.PostgreSQLContainer;
import org.testcontainers.utility.DockerImageName;

@TestConfiguration(proxyBeanMethods = false)
public class ContainersConfig {

@Bean
@ServiceConnection
PostgreSQLContainer<?> pgvectorContainer() {
return new PostgreSQLContainer<>(DockerImageName.parse("pgvector/pgvector:pg17"));
}
}
Loading