Skip to content

Conversation

@csikb
Copy link
Contributor

@csikb csikb commented Oct 5, 2025

Summary by CodeRabbit

  • Refactor
    • Client auth simplified: username/password removed; requests use a Bearer token in the Authorization header.
  • Documentation
    • Integration guide updated to describe Testcontainers-driven, Spring-managed startup/teardown and service lifecycle.
  • Tests
    • Integration tests migrated to Testcontainers with dynamic runtime properties and added container/JUnit5 test dependencies.
  • Chores
    • .gitignore expanded to cover common IDE/editor files.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 5, 2025

Walkthrough

Removed constructor parameters from DefaultFeignConfig; migrated integration tests to Testcontainers with Docker Compose via a new TestContainerConfiguration and DynamicPropertySource; added Testcontainers/JUnit dependencies; updated integration README to document the change; expanded .gitignore with IDE entries.

Changes

Cohort / File(s) Summary
Feign config refactor
client/src/main/kotlin/hu/bsstudio/bssweb/config/DefaultFeignConfig.kt
Converted DefaultFeignConfig from a primary-constructor class with username and password parameters to a parameterless class; retained the RequestInterceptor bean that sets Authorization: Bearer <token>.
Integration test infrastructure
integration/src/integrationTest/kotlin/.../TestContainerConfiguration.kt, integration/src/integrationTest/kotlin/.../IntegrationTest.kt
Added TestContainerConfiguration Spring TestConfiguration providing a DockerComposeContainer bean and a @DynamicPropertySource that registers datasource, Flyway, and bss.client.url properties from the compose services; updated IntegrationTest to include TestContainerConfiguration and removed the previous @TestPropertySource.
Integration docs and build
integration/README.md, integration/build.gradle.kts
README rewritten to describe switch from manual Docker Compose to Testcontainers managed via Spring DI and lifecycle; build.gradle.kts updated to add integrationTest dependencies (spring-boot-testcontainers, testcontainers, junit-jupiter).
Repository ignores
.gitignore
Added VSCode and JetBrains/IDE related ignore rules and templates.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant J as JUnit
  participant S as Spring Test Context
  participant TC as Testcontainers (DockerComposeContainer)
  participant APP as Application under test
  participant PG as Postgres container
  participant WM as WireMock/OIDC

  J->>TC: Start docker-compose services (compose files, --profile app)
  TC-->>PG: Start Postgres (mapped host/port)
  TC-->>WM: Start WireMock/OIDC (mapped host/ports)
  TC->>S: Provide runtime endpoints/ports via @DynamicPropertySource
  S->>APP: Initialize Spring context with injected datasource/client properties
  J->>APP: Run integration tests
  APP-->>PG: DB operations (per-test cleanup)
  J->>TC: Stop containers after tests
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately describes the primary change of integrating Testcontainers to automate Docker Compose management in integration tests, aligning directly with the modifications to test configuration, dependencies, and documentation in this pull request.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch integration-testcontainers

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Oct 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (905158b) to head (c5ba578).

Additional details and impacted files
@@             Coverage Diff             @@
##                main      #495   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity       234       234           
===========================================
  Files             40        40           
  Lines            487       487           
===========================================
  Hits             487       487           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
client/src/main/kotlin/hu/bsstudio/bssweb/config/DefaultFeignConfig.kt (2)

4-4: Remove unused import.

The @Value annotation is imported but not used anywhere in the file.

Apply this diff to remove the unused import:

-import org.springframework.beans.factory.annotation.Value

10-14: Critical: Replace hardcoded token with configuration-based injection.

The hardcoded string "Bearer token" is not a valid authentication token and will fail in any real authentication scenario. This appears to be production code (located in src/main), so it should properly inject the actual token value from configuration.

Based on the presence of the unused @Value import, it appears the token should be injected from application properties. Apply this diff to inject the token properly:

 @Configuration
-class DefaultFeignConfig {
+class DefaultFeignConfig(
+    @Value("\${bss.client.token:}") private val token: String
+) {
     @Bean
     fun interceptor() =
         RequestInterceptor { template ->
-            template.header("Authorization", "Bearer token")
+            template.header("Authorization", "Bearer $token")
         }
 }

Alternatively, if this configuration is only intended for tests and dynamic properties are injected via Testcontainers, ensure this file is moved to test sources or the token is properly configured through the test property sources.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 905158b and 57d6349.

⛔ Files ignored due to path filters (2)
  • .github/workflows/gradle.yml is excluded by !**/*.yml
  • docker-compose.ci.yml is excluded by !**/*.yml
📒 Files selected for processing (4)
  • client/src/main/kotlin/hu/bsstudio/bssweb/config/DefaultFeignConfig.kt (1 hunks)
  • integration/README.md (1 hunks)
  • integration/build.gradle.kts (1 hunks)
  • integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/IntegrationTest.kt (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build and publish
  • GitHub Check: Integration test
🔇 Additional comments (3)
integration/README.md (1)

7-35: LGTM! Clear and comprehensive documentation.

The documentation effectively explains the migration to Testcontainers, highlighting key benefits like automatic lifecycle management, dynamic port mapping, and test isolation. The structure is clear and helps developers understand what changed and why.

integration/build.gradle.kts (1)

16-18: LGTM! Appropriate dependencies for Testcontainers integration.

The three added dependencies correctly support the Testcontainers-based integration test approach with JUnit 5. Version management appears to be handled by the dependency management convention.

integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/IntegrationTest.kt (1)

41-45: Credentials match docker-compose.yml; verify “private” schema is created by migrations
IntegrationTest.kt uses DB =bss and user/password matching docker-compose.yml. I didn’t find any SQL migrations creating the “private” schema—confirm it’s created (via SQL or programmatic migration) before tests run.

Comment on lines 26 to 31
val compose: DockerComposeContainer<*> =
DockerComposeContainer(File("../docker-compose.yml"), File("../docker-compose.ci.yml"))
.withExposedService("postgres", 5432)
.withExposedService("app", 8080)
.withOptions("--profile app")
.withBuild(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify service names and configuration match docker-compose.yml.

The DockerComposeContainer configuration references service names "postgres" and "app", uses profile "app", and enables image building. Ensure these match your docker-compose.yml configuration to avoid runtime failures.

Run the following script to verify the Docker Compose configuration:


🏁 Script executed:

#!/bin/bash
# Description: Verify docker-compose.yml contains the expected service names and profile.

# Check for postgres service
echo "=== Checking for 'postgres' service ==="
rg -A 5 '^\s*postgres:' ../docker-compose.yml ../docker-compose.ci.yml

# Check for app service
echo -e "\n=== Checking for 'app' service ==="
rg -A 5 '^\s*app:' ../docker-compose.yml ../docker-compose.ci.yml

# Check for app profile usage
echo -e "\n=== Checking for 'app' profile ==="
rg 'profiles:\s*-\s*app' ../docker-compose.yml ../docker-compose.ci.yml

Length of output: 492


🏁 Script executed:

#!/bin/bash
# Locate docker-compose files in the repository
echo "=== Finding docker-compose files ==="
find . -type f -name 'docker-compose*.yml'

Length of output: 81


🏁 Script executed:

#!/bin/bash
# Verify docker-compose services and profile in the root files
echo "=== services section ==="
rg -nP -C3 'services:' docker-compose.yml docker-compose.ci.yml
echo -e "\n=== postgres service ==="
rg -nP -C3 '^\s{2}postgres:' docker-compose.yml docker-compose.ci.yml
echo -e "\n=== app service ==="
rg -nP -C3 '^\s{2}app:' docker-compose.yml docker-compose.ci.yml
echo -e "\n=== app profile ==="
rg -nP 'profiles:\s*-\s*app' docker-compose.yml docker-compose.ci.yml

Length of output: 1436


🏁 Script executed:

#!/bin/bash
echo "=== docker-compose.yml (lines 29-45) ==="
sed -n '29,45p' docker-compose.yml
echo -e "\n=== docker-compose.ci.yml (lines 1-15) ==="
sed -n '1,15p' docker-compose.ci.yml

Length of output: 854


Adjust DockerComposeContainer profile and test schema setup

  • Remove withOptions("--profile app")—no profiles: [app] defined in your compose files.
  • Disabling Flyway means the “private” schema isn’t created by migrations; either enable migrations or point currentSchema to an existing schema.
  • Credentials (user/password) and ports match your compose configuration.
🤖 Prompt for AI Agents
In integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/IntegrationTest.kt
around lines 26 to 31, remove the .withOptions("--profile app") call because no
profiles are defined in the compose files; then ensure the test DB setup matches
the compose: either re-enable Flyway migrations so the "private" schema is
created, or set the datasource/currentSchema used in tests to an existing schema
(e.g., "public"); finally verify the DockerComposeContainer uses the same
service names, exposed ports and credentials (user/password) as defined in
../docker-compose.yml and ../docker-compose.ci.yml and update the container
configuration accordingly.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 5, 2025

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
integration/README.md (1)

1-57: Fix Markdown formatting to resolve spotlessFlexmarkCheck failure.

The pipeline indicates formatting violations in this file.

Run the following command to automatically fix the formatting:

#!/bin/bash
./gradlew spotlessApply
♻️ Duplicate comments (3)
integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/TestContainerConfiguration.kt (3)

18-18: Remove --profile app option as no profiles are defined in compose files.

Per previous review feedback, the --profile app option should be removed because no profiles: [app] are defined in your docker-compose.yml or docker-compose.ci.yml files.

-            .withOptions("--profile app")

30-34: Schema configuration mismatch: Flyway disabled but currentSchema=private.

With spring.flyway.enabled=false (line 34), Flyway migrations won't run, so the "private" schema referenced in the datasource URL (line 30) won't be created. This will cause connection failures.

Either:

  1. Enable Flyway to create the schema: change line 34 to "true"
  2. Use an existing schema: change line 30 to use currentSchema=public
-            "jdbc:postgresql://$postgresHost:$postgresPort/bss?currentSchema=private"
+            "jdbc:postgresql://$postgresHost:$postgresPort/bss?currentSchema=public"

19-19: Consider disabling withBuild for faster test execution.

Enabling withBuild(true) forces Docker to rebuild images before each test run, significantly slowing down execution. Consider making this configurable via an environment variable or disabling it unless you specifically need fresh builds.

-            .withBuild(true)
+            .withBuild(System.getenv("REBUILD_IMAGES")?.toBoolean() ?: false)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 62d73f9 and 7bce671.

⛔ Files ignored due to path filters (1)
  • .vscode/settings.json is excluded by !**/*.json
📒 Files selected for processing (3)
  • integration/README.md (1 hunks)
  • integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/IntegrationTest.kt (1 hunks)
  • integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/TestContainerConfiguration.kt (1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Gradle
integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/TestContainerConfiguration.kt

[error] 1-1: integration:spotlessKotlinCheck failed: format violations detected in TestContainerConfiguration.kt. There are formatting/value assignment style issues in dockerComposeContainer and containerPropertyConfigurer definitions. Run './gradlew spotlessApply' to fix formatting.

integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/IntegrationTest.kt

[error] 1-1: integration:spotlessKotlinCheck failed: format violations detected in IntegrationTest.kt. The file has extraneous blank line and spacing issues near class declaration. Run './gradlew spotlessApply' to fix formatting.

integration/README.md

[error] 1-1: integration:spotlessFlexmarkCheck failed: format violations detected in README.md. Run './gradlew spotlessApply' to fix formatting issues.

)
@SpringJUnitConfig(classes = [TestContainerConfiguration::class, BssFeignConfig::class, DataConfig::class])
open class IntegrationTest {

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove extraneous blank line to fix spotlessKotlinCheck failure.

The pipeline failure indicates formatting violations including an extraneous blank line near the class declaration.

 @SpringJUnitConfig(classes = [TestContainerConfiguration::class, BssFeignConfig::class, DataConfig::class])
-
 open class IntegrationTest {
🤖 Prompt for AI Agents
In integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/IntegrationTest.kt
around line 16, there is an extraneous blank line causing spotlessKotlinCheck to
fail; remove that blank line so the class declaration and surrounding code
follow the project's Kotlin formatting rules (no extra empty lines) and then
re-run the formatter/check to confirm the violation is resolved.

Comment on lines +1 to +37
package hu.bsstudio.bssweb

import org.springframework.boot.test.context.TestConfiguration
import org.springframework.context.annotation.Bean
import org.springframework.test.context.DynamicPropertyRegistry
import org.springframework.test.context.DynamicPropertySource
import org.testcontainers.containers.DockerComposeContainer
import java.io.File

@TestConfiguration
class TestContainerConfiguration {

@Bean
fun dockerComposeContainer(): DockerComposeContainer<*> {
return DockerComposeContainer(File("../docker-compose.yml"), File("../docker-compose.ci.yml"))
.withExposedService("postgres", 5432)
.withExposedService("app", 8080)
.withOptions("--profile app")
.withBuild(true)
}

@DynamicPropertySource
fun containerPropertyConfigurer(registry: DynamicPropertyRegistry, dockerComposeContainer: DockerComposeContainer<*>) {
val postgresHost = dockerComposeContainer.getServiceHost("postgres", 5432)
val postgresPort = dockerComposeContainer.getServicePort("postgres", 5432)
val appHost = dockerComposeContainer.getServiceHost("app", 8080)
val appPort = dockerComposeContainer.getServicePort("app", 8080)

registry.add("spring.datasource.url") {
"jdbc:postgresql://$postgresHost:$postgresPort/bss?currentSchema=private"
}
registry.add("spring.datasource.username") { "user" }
registry.add("spring.datasource.password") { "password" }
registry.add("spring.flyway.enabled") { "false" }
registry.add("bss.client.url") { "http://$appHost:$appPort" }
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix Kotlin formatting to resolve spotlessKotlinCheck failure.

The pipeline indicates formatting violations in this file.

Run the following command to automatically fix the formatting:

#!/bin/bash
./gradlew spotlessApply
🧰 Tools
🪛 GitHub Actions: Gradle

[error] 1-1: integration:spotlessKotlinCheck failed: format violations detected in TestContainerConfiguration.kt. There are formatting/value assignment style issues in dockerComposeContainer and containerPropertyConfigurer definitions. Run './gradlew spotlessApply' to fix formatting.

🤖 Prompt for AI Agents
integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/TestContainerConfiguration.kt
lines 1-37: the file fails spotlessKotlinCheck due to formatting issues; run the
formatter and commit the result to fix the pipeline. Execute ./gradlew
spotlessApply (or use your IDE’s Kotlin reformat) to auto-fix formatting, verify
the file is updated, then add and commit the changed file and push the branch so
CI picks up the formatted code.

Comment on lines +22 to +36
@DynamicPropertySource
fun containerPropertyConfigurer(registry: DynamicPropertyRegistry, dockerComposeContainer: DockerComposeContainer<*>) {
val postgresHost = dockerComposeContainer.getServiceHost("postgres", 5432)
val postgresPort = dockerComposeContainer.getServicePort("postgres", 5432)
val appHost = dockerComposeContainer.getServiceHost("app", 8080)
val appPort = dockerComposeContainer.getServicePort("app", 8080)

registry.add("spring.datasource.url") {
"jdbc:postgresql://$postgresHost:$postgresPort/bss?currentSchema=private"
}
registry.add("spring.datasource.username") { "user" }
registry.add("spring.datasource.password") { "password" }
registry.add("spring.flyway.enabled") { "false" }
registry.add("bss.client.url") { "http://$appHost:$appPort" }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

@DynamicPropertySource cannot inject beans as method parameters.

The @DynamicPropertySource annotation only accepts a single DynamicPropertyRegistry parameter. You cannot inject the dockerComposeContainer bean as a method parameter. This will cause a runtime failure.

To fix this, either:

  1. Make dockerComposeContainer a constructor parameter or field and access it from the property source method
  2. Use a companion object with @JvmStatic for the property source method

Option 1 (Recommended): Use constructor injection

 @TestConfiguration
-class TestContainerConfiguration {
+class TestContainerConfiguration(
+    private val dockerComposeContainer: DockerComposeContainer<*>
+) {
 
-    @Bean
-    fun dockerComposeContainer(): DockerComposeContainer<*> {
-        return DockerComposeContainer(File("../docker-compose.yml"), File("../docker-compose.ci.yml"))
-            .withExposedService("postgres", 5432)
-            .withExposedService("app", 8080)
-            .withOptions("--profile app")
-            .withBuild(true)
-    }
-
     @DynamicPropertySource
-    fun containerPropertyConfigurer(registry: DynamicPropertyRegistry, dockerComposeContainer: DockerComposeContainer<*>) {
+    fun containerPropertyConfigurer(registry: DynamicPropertyRegistry) {
         val postgresHost = dockerComposeContainer.getServiceHost("postgres", 5432)
         val postgresPort = dockerComposeContainer.getServicePort("postgres", 5432)
         val appHost = dockerComposeContainer.getServiceHost("app", 8080)
         val appPort = dockerComposeContainer.getServicePort("app", 8080)
 
         registry.add("spring.datasource.url") {
             "jdbc:postgresql://$postgresHost:$postgresPort/bss?currentSchema=private"
         }
         registry.add("spring.datasource.username") { "user" }
         registry.add("spring.datasource.password") { "password" }
         registry.add("spring.flyway.enabled") { "false" }
         registry.add("bss.client.url") { "http://$appHost:$appPort" }
     }
+
+    companion object {
+        @Bean
+        @JvmStatic
+        fun dockerComposeContainer(): DockerComposeContainer<*> {
+            return DockerComposeContainer(File("../docker-compose.yml"), File("../docker-compose.ci.yml"))
+                .withExposedService("postgres", 5432)
+                .withExposedService("app", 8080)
+                .withOptions("--profile app")
+                .withBuild(true)
+        }
+    }
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
integration/src/integrationTest/kotlin/hu/bsstudio/bssweb/TestContainerConfiguration.kt
around lines 22 to 36, the @DynamicPropertySource method currently declares two
parameters (DynamicPropertyRegistry and dockerComposeContainer) which is
invalid; change the method so it only accepts DynamicPropertyRegistry and obtain
the DockerComposeContainer from a class field (set via constructor or property)
or move the method into a companion object as a @JvmStatic function that
accesses a static container reference; recommended: make dockerComposeContainer
a constructor-injected or top-level field on the test configuration class and
update the property configurer to use that field so the @DynamicPropertySource
method signature is exactly (registry: DynamicPropertyRegistry).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants