feat(docker): add docker with postgresql#17
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds Docker-based PostgreSQL database support to the Spring Boot application. It introduces database infrastructure with Spring Data JDBC, schema initialization, environment-based configuration, and Maven-orchestrated Docker Compose integration for both local development and testing.
Changes:
- Added Docker Compose configuration with PostgreSQL 15 Alpine container
- Enabled Spring Data JDBC with database initialization via schema.sql
- Configured environment variable management using java-dotenv library
- Set up Maven plugins for integration testing with docker-compose-maven-plugin
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docker-compose.yml | PostgreSQL service configuration with health checks, volume mounts, and port mapping to 5433 |
| .env.template | Template for database connection environment variables |
| src/main/resources/application.properties | Database datasource configuration and SQL initialization settings |
| src/main/resources/schema.sql | PostgreSQL schema definition for user table |
| src/main/java/com/xpeho/spring_boot_java_random_user/SpringBootJavaRandomUserApplication.java | Added dotenv library integration to load environment variables at startup |
| src/test/resources/application-test.properties.template | Template for test environment variables |
| src/test/java/com/xpeho/spring_boot_java_random_user/SpringBootJavaRandomUserApplicationTests.java | Added test profile activation and removed unused import |
| pom.xml | Enabled Spring Data JDBC dependencies, added java-dotenv, configured maven-surefire to skip, maven-failsafe for integration tests, and docker-compose-maven-plugin |
| .gitignore | Added entries to ignore .env and application-test.properties files |
| README.md | Added documentation (though with significant structural issues) |
Comments suppressed due to low confidence (13)
pom.xml:64
- Adding the java-dotenv dependency is unnecessary for Spring Boot applications. Spring Boot 2.4+ has native support for .env files without requiring an external library. This adds an extra dependency to maintain and the implementation in SpringBootJavaRandomUserApplication.java loads variables as system properties, which has security implications.
Instead, use Spring Boot's native .env support by adding to application.properties:
spring.config.import=optional:file:.env[.properties]
This eliminates the need for the java-dotenv dependency entirely, follows Spring Boot conventions, and is more secure as it doesn't expose environment variables as system properties.
<dependency>
<groupId>io.github.cdimascio</groupId>
<artifactId>java-dotenv</artifactId>
<version>5.2.2</version>
</dependency>
src/main/resources/application.properties:18
- Setting spring.sql.init.continue-on-error=true can mask serious database initialization problems. When set to true, SQL errors during schema initialization are logged but don't prevent application startup. This can lead to:
- Application starting with an incomplete or incorrect schema
- Runtime errors that are harder to diagnose than startup failures
- Data corruption if schema is partially applied
This setting should typically be false (the default) to fail fast on schema errors. If continue-on-error is truly needed (e.g., for idempotent scripts), it should be:
- Documented why it's necessary
- Paired with comprehensive schema validation on startup
- Set only for specific environments via Spring profiles
spring.sql.init.continue-on-error=true
README.md:44
- The README documentation claims tests use "Testcontainers to spin up ephemeral Docker containers" (line 42), but the actual implementation uses docker-compose-maven-plugin with a docker-compose.yml file. These are different approaches:
- Testcontainers: Programmatic container management in Java test code
- docker-compose-maven-plugin: Maven lifecycle integration with docker-compose
Additionally, the pom.xml has commented out testcontainers-postgresql dependency (lines 91-95), and no Testcontainers code exists in the test classes. The documentation should accurately describe the docker-compose-maven-plugin approach being used, or the implementation should be changed to actually use Testcontainers.
Tests use **Testcontainers** to spin up ephemeral Docker containers for external dependencies (e.g. PostgreSQL).
> **Prerequisite for tests**: Docker must be installed and running.
pom.xml:109
- Skipping all unit tests by setting surefire skip to true is not a best practice. This configuration means:
- No unit tests will run during the standard test phase
- All tests (including unit tests) are forced to run as integration tests via failsafe
- This defeats the purpose of separating fast unit tests from slower integration tests
- Build feedback is slower since integration tests typically take longer
Consider either:
- Keep surefire enabled and use naming conventions to separate unit (*Test.java) and integration tests (*IT.java)
- Or if all tests truly require the database, this is acceptable but should be documented
- Use separate test source directories for integration tests (src/integration-test/java) if truly all tests need Docker
The current configuration makes it impossible to run fast unit tests in isolation, which impacts developer productivity.
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<skip>true</skip>
</configuration>
src/main/resources/application.properties:18
- Setting spring.sql.init.mode=always can cause issues in production environments. This setting will attempt to run schema.sql on every application startup, which:
- Will fail in production if tables already exist (despite continue-on-error=true, errors are still logged)
- Can cause race conditions in multi-instance deployments
- Is not suitable for managing schema changes over time
Consider using:
- spring.sql.init.mode=never for production (default)
- Use a proper database migration tool like Flyway or Liquibase for schema management
- Or set this conditionally via a Spring profile (e.g., only for local development)
For now, at minimum document that this configuration is for development only and should be overridden in production environments.
spring.sql.init.mode=always
spring.sql.init.schema-locations=classpath:schema.sql
spring.sql.init.data-locations=
spring.sql.init.platform=postgresql
spring.sql.init.continue-on-error=true
docker-compose.yml:1
- The docker-compose file uses version '3.8', but this version specification is now obsolete. Docker Compose V2 (released 2020+) ignores the version field, and Docker has officially deprecated it as of 2023. The version field is only relevant for legacy Compose V1.
Consider either:
- Remove the version field entirely (recommended for modern Docker Compose)
- Update to version '3.9' if backward compatibility with older Compose V1 is needed
However, since this is a new file, removing the version field is the recommended approach to follow current Docker Compose best practices.
version: '3.8'
README.md:97
- The port number documented here (5432) is inconsistent with the actual configuration. The docker-compose.yml maps PostgreSQL to host port 5433 (line 12: "5433:5432"), and both .env.template and application-test.properties.template specify POSTGRES_PORT=5433. The documentation should be updated to use port 5433 to match the actual configuration.
spring.datasource.url=jdbc:postgresql://localhost:5432/<database_name>
src/main/java/com/xpeho/spring_boot_java_random_user/SpringBootJavaRandomUserApplication.java:15
- Loading environment variables from .env file and setting them as system properties in the main application class is problematic for several reasons:
- Security: System properties are visible to all code and can be logged/exposed
- Test isolation: This runs for every application start, including tests, potentially causing conflicts
- Production concerns: In production environments, system properties or actual environment variables are preferred over .env files
- Spring Boot convention: Spring Boot already supports .env files natively via spring.config.import=optional:file:.env[] in Spring Boot 2.4+
Consider using Spring Boot's native .env support by adding to application.properties:
spring.config.import=optional:file:.env[.properties]
This is more secure, follows Spring conventions, and doesn't pollute system properties.
Dotenv.configure()
.ignoreIfMissing()
.load()
.entries()
.forEach(entry -> System.setProperty(entry.getKey(), entry.getValue()));
src/main/resources/schema.sql:14
- Using "user" as a table name requires quoting in PostgreSQL because "user" is a reserved keyword. While the quotes are present in the DROP and CREATE statements here, this will require quotes in all future SQL queries and can cause issues with:
- ORM frameworks that may not automatically quote reserved words
- Manual queries where developers forget to quote the table name
- Cross-database compatibility issues
Consider renaming the table to something like "users", "app_user", or "random_user" to avoid this reserved keyword issue. This is a better long-term approach than relying on quotes everywhere.
DROP TABLE IF EXISTS "user";
CREATE TABLE IF NOT EXISTS "user"
(
id SERIAL PRIMARY KEY,
gender VARCHAR(20),
firstname VARCHAR(100),
lastname VARCHAR(100),
civility VARCHAR(20),
email VARCHAR(255),
phone VARCHAR(50),
picture VARCHAR(500),
nationality VARCHAR(10)
);
pom.xml:152
- The docker-compose-maven-plugin is configured with removeVolumes=false, which means PostgreSQL data volumes persist between test runs. This can cause:
- Test pollution - data from previous test runs affecting current tests
- False test passes/failures due to leftover data
- Difficulty reproducing test failures locally
For integration tests, removeVolumes should typically be true to ensure clean state for each test run. Consider changing to:
removeVolumes=true
This ensures each test execution starts with a fresh database, improving test reliability and reproducibility.
<removeVolumes>false</removeVolumes>
README.md:118
- The prerequisites list "PostgreSQL running" as a requirement, but with the docker-compose setup in this PR, users don't need PostgreSQL installed locally - they need Docker and Docker Compose instead. The prerequisite should be updated to:
- Docker and Docker Compose (for running PostgreSQL via docker-compose.yml)
Alternatively, clarify that PostgreSQL can be run either:
- Via Docker Compose (recommended, using included docker-compose.yml)
- Or as a standalone installation
This is especially important since the docker-compose.yml is set up to handle PostgreSQL automatically.
- **PostgreSQL** running (if data persistence is enabled)
pom.xml:153
- The docker-compose-maven-plugin configuration lacks a startup wait strategy. While the docker-compose.yml defines a healthcheck for PostgreSQL, the plugin doesn't wait for services to be healthy before proceeding to run tests. This can cause test failures due to race conditions where tests start before PostgreSQL is ready.
Add startup wait configuration to ensure PostgreSQL is ready:
Consider adding:
- awaitHealthy: true (if the plugin supports it)
- or add a build-helper-maven-plugin execution to wait for port 5433 to be available
- or add explicit sleep/retry logic in test setup
Without this, tests may fail intermittently, especially in CI/CD environments.
<configuration>
<composeFile>${project.basedir}/docker-compose.yml</composeFile>
<detachedMode>true</detachedMode>
<removeVolumes>false</removeVolumes>
</configuration>
README.md:128
- The PR adds .env.template and application-test.properties.template files that developers need to manually copy and configure, but there's no documentation explaining:
- That developers need to copy .env.template to .env and fill in values
- That developers need to copy application-test.properties.template to application-test.properties
- What values to use for local development (e.g., example values beyond "your_postgres_user")
- The relationship between these files and docker-compose
Add a section to the README explaining the initial setup steps, such as:
- Copy .env.template to .env
- Customize the values in .env (or provide working defaults)
- Explain when application-test.properties is needed vs using the main profile
## ⚙️ Configuration
The main configuration file is located at:
src/main/resources/application.properties
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (17)
README.md:236
- Duplicate content detected: The same header sections and content appear multiple times throughout the README. For example, "## 🛠️ Tech Stack" appears at both line 147 and line 157, "## 📋 Prerequisites" at lines 122 and 172, "## ⚙️ Configuration" at lines 130 and 180, and the entire "🚀 Running the project" section is duplicated. This creates confusion and makes maintenance difficult. Remove all duplicate sections and keep only one instance of each.
## 📋 Prerequisites
- **Java 25** (or compatible version)
- **Maven** (or use the included `./mvnw` wrapper)
- **PostgreSQL** running (if data persistence is enabled)
---
## ⚙️ Configuration
The main configuration file is located at:
src/main/resources/application.properties
| Technology | Version |
|----------------------|-----------------|
| Java | 25 |
| Spring Boot | 4.0.3 |
| Spring Web MVC | (managed by Boot) |
| Spring Actuator | (managed by Boot) |
| springdoc-openapi | 3.0.1 |
| PostgreSQL (driver) | (managed by Boot) |
| Testcontainers | (managed by Boot) |
| Maven | Wrapper included |
## 🛠️ Tech Stack
Spring Boot Java project that exposes a REST API consuming the public [Random User](https://randomuser.me/) API.
The interactive API documentation is available via **Swagger UI / OpenAPI**.
# spring_boot_java_random_user
Spring Boot Java project that exposes a REST API consuming the public [Random User](https://randomuser.me/) API.
The interactive API documentation is available via **Swagger UI / OpenAPI**.
---
## 🛠️ Tech Stack
| Technology | Version |
|----------------------|-----------------|
| Java | 25 |
| Spring Boot | 4.0.3 |
| Spring Web MVC | (managed by Boot) |
| Spring Actuator | (managed by Boot) |
| springdoc-openapi | 3.0.1 |
| PostgreSQL (driver) | (managed by Boot) |
| Testcontainers | (managed by Boot) |
| Maven | Wrapper included |
---
## 📋 Prerequisites
- **Java 25** (or compatible version)
- **Maven** (or use the included `./mvnw` wrapper)
- **PostgreSQL** running (if data persistence is enabled)
---
## ⚙️ Configuration
The main configuration file is located at:
src/main/resources/application.properties
### Properties to configure
```properties
spring.application.name=spring_boot_java_random_user
# Swagger UI — available at /api
springdoc.swagger-ui.path=/api
# PostgreSQL datasource (add these if you use persistence)
spring.datasource.url=jdbc:postgresql://localhost:5432/<database_name>
spring.datasource.username=<username>
spring.datasource.password=<password>
spring.datasource.driver-class-name=org.postgresql.Driver
⚠️ Common startup error:Failed to configure a DataSource: 'url' attribute is not specified and no embedded datasource could be configured. Reason: Failed to determine a suitable driver classThis error occurs because the PostgreSQL driver is present in the dependencies but the datasource URL is not configured.
Fix: either add thespring.datasource.*properties above, or exclude the DataSource auto-configuration if no database is needed:spring.autoconfigure.exclude=org.springframework.boot.autoconfigure.jdbc.DataSourceAutoConfiguration
🚀 Running the project
With the Maven Wrapper (recommended)
./mvnw spring-boot:runWith Maven installed
mvn spring-boot:runBuild and run the JAR
./mvnw clean package
java -jar target/spring_boot_java_random_user-0.0.1-SNAPSHOT.jar**docker-compose.yml:17**
* The healthcheck command uses an environment variable ${POSTGRES_USER} in the shell command, but this may not be properly substituted at runtime within the CMD-SHELL context. In Docker healthchecks, environment variable substitution inside the shell command array may not work as expected. Consider either using a hardcoded value that matches the environment variable, or using the 'postgres' default user since that's what will typically be configured. Alternatively, use the format: test: ["CMD-SHELL", "pg_isready -U postgres"] or ensure the POSTGRES_USER is consistently set.
test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER}"]
**src/main/resources/schema.sql:13**
* According to the linked issue #6, the schema should include a field for 'picture.medium' (stored as 'picture'). The current schema uses 'picture' which aligns with the requirement. However, the issue mentions that 'nat' should be included, but the schema uses 'nationality' instead. While this is semantically correct, it's worth verifying that the mapping from the Random User API field 'nat' to the database column 'nationality' is handled correctly in the application code. Consider adding a comment in the schema file documenting the mapping from API fields to database columns.
nationality VARCHAR(10)
**README.md:94**
* The section header "## Running PostgreSQL with Docker" at line 88 appears incomplete and is followed by content that starts mid-sentence. The actual instructions for running PostgreSQL with Docker are missing - only the step to copy the .env template is shown. This section should include complete instructions on how to start the docker-compose services (e.g., "docker-compose up -d") and verify the database is running.
Running PostgreSQL with Docker
-
Copy the environment template and adjust values as needed:
cp .env.template .env # then edit .env to set database name, user, password, etc.
**.env.template:4**
* Environment variable placeholders (your_postgres_user, your_postgres_password, your_postgres_db) should be replaced with more descriptive example values to guide users. Consider using values like "postgres", "secure_password_here", and "xpeho_db" as examples. The current placeholders might be confusing since they need to be replaced. Also consider adding comments explaining that these values should be kept secret and never committed to version control.
POSTGRES_USER=your_postgres_user
POSTGRES_PASSWORD=your_postgres_password
POSTGRES_DB=your_postgres_db
POSTGRES_PORT=5433
**src/main/resources/schema.sql:14**
* Missing indexes on the 'user' table. While the table has a primary key on 'id', there are no indexes on commonly queried fields. Consider adding indexes on frequently accessed columns such as 'email' (likely to be unique and queried often) and potentially 'nationality' if filtering by nationality is a common operation. This will improve query performance as the table grows. For example: CREATE INDEX IF NOT EXISTS idx_user_email ON "user"(email);
DROP TABLE IF EXISTS "user";
CREATE TABLE IF NOT EXISTS "user"
(
id SERIAL PRIMARY KEY,
gender VARCHAR(20),
firstname VARCHAR(100),
lastname VARCHAR(100),
civility VARCHAR(20),
email VARCHAR(255),
phone VARCHAR(50),
picture VARCHAR(500),
nationality VARCHAR(10)
);
**src/main/resources/application.properties:16**
* The spring.sql.init.data-locations property is set to an empty value. According to the linked issue #6, there should be consideration for a data.sql file for initial data loading (shown commented out in the example docker-compose). If initial data loading is intentionally disabled, this is fine. However, if data initialization is planned for future implementation, consider adding a comment explaining this, or set it to classpath:data.sql with a corresponding (possibly empty) data.sql file to make the structure clear.
spring.sql.init.data-locations=
**pom.xml:109**
* The maven-surefire-plugin is configured to skip all tests (skip=true). This means unit tests will never run during the normal test phase. While integration tests are handled by maven-failsafe-plugin, completely skipping unit tests is not a best practice. If the intention is to run all tests as integration tests, this should be clearly documented, or consider restructuring to keep unit tests separate from integration tests. This configuration may cause developers to miss running fast unit tests during development.
<configuration>
<skip>true</skip>
</configuration>
**pom.xml:153**
* The docker-compose-maven-plugin starts Docker containers in the pre-integration-test phase but the configuration may cause issues if tests fail or the build is interrupted. The removeVolumes is set to false, which means data will persist between test runs and could lead to test pollution. Consider setting removeVolumes to true for a clean test environment on each run, especially since this appears to be for testing purposes based on the integration with maven-failsafe-plugin. Also consider adding a build configuration to handle .env file loading before docker-compose starts.
<configuration>
<composeFile>${project.basedir}/docker-compose.yml</composeFile>
<detachedMode>true</detachedMode>
<removeVolumes>false</removeVolumes>
</configuration>
**src/main/resources/application.properties:18**
* The spring.sql.init.continue-on-error=true setting will silently ignore database initialization errors. This can hide critical problems during schema setup, making it difficult to diagnose issues when the database schema fails to initialize correctly. Consider setting this to false or removing this line entirely to ensure database initialization failures are caught early. If there are specific known errors that need to be ignored, document why this is necessary.
spring.sql.init.continue-on-error=true
**src/main/java/com/xpeho/spring_boot_java_random_user/SpringBootJavaRandomUserApplication.java:15**
* Loading environment variables from .env file in the main application method and setting them as system properties can cause issues in production environments. This approach means production deployments would need a .env file, which is not a typical deployment pattern. A better approach would be to: 1) Use Spring's `@PropertySource` or ConfigurationProperties to load environment-specific configuration, 2) Only load the .env file in development/local environments using Spring profiles, or 3) Use Spring Boot's built-in support for .env files (available in newer versions). Consider refactoring to use Spring's native configuration mechanisms instead of manually loading and setting system properties.
Dotenv.configure()
.ignoreIfMissing()
.load()
.entries()
.forEach(entry -> System.setProperty(entry.getKey(), entry.getValue()));
**src/test/resources/application-test.properties.template:4**
* The test properties template uses the same placeholder names (your_postgres_user, etc.) as the main .env.template, but tests should typically use separate database credentials and potentially a different database to avoid conflicts. Consider using test-specific values like "postgres_test", "test_password", "xpeho_test_db", or add comments clarifying that tests should use a separate database instance. Additionally, this template includes POSTGRES_PORT which may not be necessary for tests if they use Testcontainers, as indicated in the README.
POSTGRES_USER=your_postgres_user
POSTGRES_PASSWORD=your_postgres_password
POSTGRES_DB=your_postgres_db
POSTGRES_PORT=5433
**pom.xml:64**
* The java-dotenv dependency (version 5.2.2) is being added to handle .env file loading. However, Spring Boot 3.1+ has built-in support for .env files without requiring an external library. Since this project uses Spring Boot 4.0.3, the built-in .env support should be available. Consider removing this dependency and using Spring Boot's native .env file support instead, which would simplify the codebase by removing the manual environment variable loading code in the main application class. This would also follow Spring Boot conventions more closely.
<dependency>
<groupId>io.github.cdimascio</groupId>
<artifactId>java-dotenv</artifactId>
<version>5.2.2</version>
</dependency>
**README.md:105**
* The port configuration is inconsistent between the README documentation and the actual application properties. The README at line 105 shows port 5432 (jdbc:postgresql://localhost:5432/), but the docker-compose.yml maps the database to port 5433, and the .env.template specifies POSTGRES_PORT=5433. The README should be updated to use port 5433 to match the actual configuration, or use the environment variable placeholder like ${POSTGRES_PORT}.
spring.datasource.url=jdbc:postgresql://localhost:5432/<database_name>
**README.md:5**
* The "✅ Todo" section at line 347 shows issue #6 (Add PostgreSQL database with docker) as unchecked, but at line 5 the same issue is marked as completed [x]. Since this PR is implementing issue #6, both instances should be marked as completed for consistency, or the duplicated section should be removed entirely.
**src/main/resources/schema.sql:12**
* The column size for VARCHAR fields may not be optimal. The 'picture' column is set to VARCHAR(500), but URLs can potentially exceed this length. Modern image CDN URLs, especially with query parameters and signed tokens, can be quite long. Consider using VARCHAR(1000) or TEXT for the picture column to avoid truncation errors. Additionally, the 'email' column at VARCHAR(255) should be sufficient, but validate this against your expected data from the Random User API.
picture VARCHAR(500),
**src/main/resources/application.properties:10**
* The database configuration references environment variables (${POSTGRES_USER}, ${POSTGRES_PASSWORD}, ${POSTGRES_DB}, ${POSTGRES_PORT}) which is good practice. However, there's no fallback or validation if these environment variables are not set. The application will fail to start with unclear error messages if the .env file is missing or incomplete. Consider adding spring.config.import=optional:file:.env to explicitly handle .env file loading with Spring Boot's native support, or add clear comments documenting that these environment variables must be set before starting the application.
spring.datasource.url=jdbc:postgresql://localhost:${POSTGRES_PORT}/${POSTGRES_DB}
spring.datasource.username=${POSTGRES_USER}
spring.datasource.password=${POSTGRES_PASSWORD}
spring.datasource.driver-class-name=org.postgresql.Driver
</details>
---
💡 <a href="/XPEHO/spring_boot_java_random_user/new/main?filename=.github/instructions/*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
MayuriXx
force-pushed
the
feat/docker
branch
from
7049a0b to
cf23c0d
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (8)
pom.xml:159
docker-compose-maven-pluginis wired into the Maven lifecycle for everyverify, which makesmvn verifyrequire Docker even when someone only wants to run fast/unit tests or build artifacts. Consider gating this behind a Maven profile (e.g.,-Pdocker-it) or supporting a skip flag so CI/local builds can opt out cleanly.
<plugin>
<groupId>com.dkanejs.maven.plugins</groupId>
<artifactId>docker-compose-maven-plugin</artifactId>
<version>4.0.0</version>
<executions>
<execution>
<id>docker-compose-up</id>
<phase>pre-integration-test</phase>
<goals>
<goal>up</goal>
</goals>
</execution>
<execution>
<id>docker-compose-down</id>
<phase>post-integration-test</phase>
<goals>
<goal>down</goal>
</goals>
</execution>
</executions>
<configuration>
<composeFile>${project.basedir}/docker-compose.yml</composeFile>
<detachedMode>true</detachedMode>
<removeVolumes>false</removeVolumes>
</configuration>
</plugin>
README.md:162
- README says the Sonar workflow requires
POSTGRES_*secrets, but the current.github/workflows/sonar.yamlstill expects anAPPLICATION_TEST_PROPERTIESsecret and writes it intosrc/test/resources/application-test.properties. Either update the workflow to match this new approach, or adjust the README so it reflects what CI actually uses.
### Required GitHub Secrets
| Secret | Description |
|---|---|
| `SONAR_TOKEN` | SonarQube authentication token |
| `SONAR_HOST_URL` | SonarQube instance URL |
| `POSTGRES_USER` | PostgreSQL user |
| `POSTGRES_PASSWORD` | PostgreSQL password |
| `POSTGRES_DB` | Database name |
| `POSTGRES_PORT` | PostgreSQL port |
src/main/resources/application.properties:18
spring.sql.init.continue-on-error=truewill silently ignore SQL initialization failures, which can mask schema problems and lead to the app starting with a partially initialized database. Prefer leaving this false (default) and makingschema.sqlidempotent instead.
spring.sql.init.continue-on-error=true
docker-compose.yml:13
- Port mapping is hard-coded to
5433:5432, but the rest of the setup usesPOSTGRES_PORT(templates + Spring config). If someone changesPOSTGRES_PORT, Docker will still publish 5433 and the app will fail to connect. Use${POSTGRES_PORT}:5432(with a default) to keep Compose and Spring in sync.
ports:
- "5433:5432"
volumes:
src/main/resources/application.properties:15
spring.sql.init.mode=alwaysforces schema initialization on every startup in every environment. That’s convenient locally, but risky for production deployments and makes startup dependent on SQL scripts. Consider scoping this to a local/dev profile (and/or using Flyway/Liquibase) so production doesn’t auto-run init scripts.
spring.sql.init.mode=always
spring.sql.init.schema-locations=classpath:schema.sql
src/main/resources/application.properties:17
spring.sql.init.data-locations=is set to an empty value. This can lead to confusing behavior (empty location entry / warnings) and isn’t needed if you don’t want data initialization. Remove this property or point it to an actual script (e.g.,classpath:data.sql) when needed.
spring.sql.init.schema-locations=classpath:schema.sql
spring.sql.init.data-locations=
spring.sql.init.platform=postgresql
src/main/resources/schema.sql:4
- Using the table name
useris risky in PostgreSQL becauseUSERis a keyword; it can force quoting in generated SQL and cause subtle issues with ORMs/query builders. Prefer a non-keyword name likeusersorapp_userto avoid needing quotes everywhere.
CREATE TABLE IF NOT EXISTS "user"
(
src/main/java/com/xpeho/spring_boot_java_random_user/SpringBootJavaRandomUserApplication.java:16
- This loads
.envvalues and unconditionally writes them into JVM system properties, which can override already-provided environment/system properties (e.g., in CI/production) if a.envfile exists. Consider only setting a property when it isn’t already defined, or load dotenv into Spring via configuration (spring.config.import/ profile-based config) rather than mutating global system properties.
Dotenv.configure()
.ignoreIfMissing()
.load()
.entries()
.forEach(entry -> System.setProperty(entry.getKey(), entry.getValue()));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
profotoce59
reviewed
Feb 27, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (11)
pom.xml:158
- The docker-compose Maven plugin runs during
pre-integration-testbut the build doesn’t supply thePOSTGRES_*variables required bydocker-compose.yml. Unless the CI job exports them (or Compose has defaults),mvn verifywill fail when the plugin executes. Consider wiring these values through the plugin configuration / a Maven profile, or make the Compose file self-contained via defaults.
<configuration>
<composeFile>${project.basedir}/docker-compose.yml</composeFile>
<detachedMode>true</detachedMode>
<removeVolumes>false</removeVolumes>
</configuration>
src/main/resources/schema.sql:4
- Using the table name
user(even quoted in DDL) is problematic in PostgreSQL becauseUSERis a reserved keyword; it typically forces quoting in all generated queries and can break ORM/JDBC defaults. Prefer a non-reserved table name (e.g.,usersorapp_user) and adjust mappings accordingly.
CREATE TABLE IF NOT EXISTS "user"
(
src/main/resources/application.properties:10
- Datasource properties rely on
${POSTGRES_*}placeholders with no defaults; if.envisn't present (andDotenv.ignoreIfMissing()is enabled), the app will fail to start due to unresolved placeholders. Consider adding safe defaults in the placeholders (or providing a profile that disables datasource auto-config when DB isn't configured).
spring.datasource.url=jdbc:postgresql://localhost:${POSTGRES_PORT}/${POSTGRES_DB}
spring.datasource.username=${POSTGRES_USER}
spring.datasource.password=${POSTGRES_PASSWORD}
spring.datasource.driver-class-name=org.postgresql.Driver
src/main/java/com/xpeho/spring_boot_java_random_user/SpringBootJavaRandomUserApplication.java:15
- Loading
.envand then copying all entries into JVM system properties can unintentionally override values passed via-D/Spring config and may propagate unrelated environment keys intoSystem.getProperties(). Consider only mapping the requiredPOSTGRES_*keys and avoid overwriting existing system properties (or integrate via a SpringPropertySource).
Dotenv.configure()
.ignoreIfMissing()
.load()
.entries()
.forEach(entry -> System.setProperty(entry.getKey(), entry.getValue()));
docker-compose.yml:10
docker-compose.ymlrequiresPOSTGRES_USER/POSTGRES_PASSWORD/POSTGRES_DBfor container startup, but there are no defaults in the Compose file.docker compose up(and the Maven docker-compose plugin) will fail unless these are exported or provided via a checked-in env file. Consider adding Compose defaults (or ensuring CI exports them explicitly).
environment:
POSTGRES_USER: ${POSTGRES_USER}
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}
POSTGRES_DB: ${POSTGRES_DB}
README.md:20
- README states PostgreSQL version
15, butdocker-compose.ymlpinspostgres:17.9-alpine. Align the documentation with the actual image tag (or change the image tag) to avoid confusion and environment drift.
| PostgreSQL | 15 |
| dotenv-java | 5.2.2 |
| Docker / Docker Compose | - |
README.md:157
- The “Required GitHub Secrets” Markdown table is malformed (missing the
|---|---|separator row and several rows are missing the trailing|), so it won’t render as a table. Fix the table syntax so the secrets list is readable.
| Secret | Description |
| `APPLICATION_TEST_PROPERTIES` (Base64-encoded `application-test.properties` content)
| `SONAR_TOKEN` | SonarQube authentication token |
pom.xml:114
- Disabling Surefire (
<skip>true</skip>) makesmvn testrun zero tests and breaks the standard Maven split between unit tests (Surefire) and integration tests (Failsafe). Prefer keeping Surefire enabled for unit tests and configuring Failsafe for integration tests only (e.g., naming*IT.java) so local/CI workflows remain conventional.
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<skip>true</skip>
</configuration>
docker-compose.yml:12
docker-compose.ymlhard-codes the host port mapping to5433:5432, but the app reads the host port from${POSTGRES_PORT}. If someone changesPOSTGRES_PORTin.env, the app and container ports will diverge. Use Compose variable substitution for the mapping (e.g., host port from${POSTGRES_PORT}with a default).
ports:
- "5433:5432"
docker-compose.yml:18
- The healthcheck uses
pg_isready -U ${POSTGRES_USER}; ifPOSTGRES_USERisn't set, the healthcheck command will be invalid and may keep the container unhealthy. Provide a default forPOSTGRES_USERin Compose (or hard-code the healthcheck user to match the default user).
healthcheck:
test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER}"]
interval: 10s
src/main/resources/application.properties:18
spring.sql.init.mode=alwayscombined withspring.sql.init.continue-on-error=truewill apply schema on every startup and can hide real init failures. Consider scoping SQL init to a specific profile/environment and keepcontinue-on-errordisabled so schema problems fail fast.
spring.sql.init.mode=always
spring.sql.init.schema-locations=classpath:schema.sql
spring.sql.init.data-locations=
spring.sql.init.platform=postgresql
spring.sql.init.continue-on-error=true
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (7)
src/main/resources/application.properties:10
- Datasource placeholders don’t provide defaults (e.g.
${POSTGRES_PORT}), so the application will fail to start with “Could not resolve placeholder …” unless all variables are provided. Consider adding sensible defaults (e.g. port/db/user) or failing fast with a clearer message; right now this also makesDotenv.configure().ignoreIfMissing()misleading.
spring.datasource.url=jdbc:postgresql://localhost:${POSTGRES_PORT}/${POSTGRES_DB}
spring.datasource.username=${POSTGRES_USER}
spring.datasource.password=${POSTGRES_PASSWORD}
spring.datasource.driver-class-name=org.postgresql.Driver
src/main/java/com/xpeho/spring_boot_java_random_user/SpringBootJavaRandomUserApplication.java:15
- This loads all
.enventries into JVM system properties unconditionally, overwriting any values already provided via-D...or the host environment. To avoid surprising overrides, only set the property when it’s missing (or limit to the specific keys your app uses).
Dotenv.configure()
.ignoreIfMissing()
.load()
.entries()
.forEach(entry -> System.setProperty(entry.getKey(), entry.getValue()));
README.md:162
- The “Required GitHub Secrets” table is malformed (rows are missing the closing
|and there’s no separator row), so it won’t render as a table in Markdown. Fix the table formatting so each row has two columns and include the|---|---|separator.
| Secret | Description |
| `APPLICATION_TEST_PROPERTIES` (Base64-encoded `application-test.properties` content)
| `SONAR_TOKEN` | SonarQube authentication token |
| `SONAR_HOST_URL` | SonarQube instance URL |
| `POSTGRES_USER` | PostgreSQL user |
| `POSTGRES_PASSWORD` | PostgreSQL password |
| `POSTGRES_DB` | Database name |
| `POSTGRES_PORT` | PostgreSQL port |
README.md:20
- README lists PostgreSQL version as 15, but the provided
docker-compose.ymlimage tag is 17.x. Update the README’s Tech Stack entry (or the compose image) so the version users run matches what’s documented.
| Spring Actuator | (managed by Boot) |
| springdoc-openapi | 3.0.1 |
| PostgreSQL | 15 |
| dotenv-java | 5.2.2 |
| Docker / Docker Compose | - |
src/main/resources/application.properties:18
spring.sql.init.continue-on-error=truecan mask real schema/init failures and let the app start with a partially initialized database. It’s safer to remove this or set it tofalse(and handle migrations explicitly) so startup fails when schema init fails.
# Spring Data JDBC - Initialize database
spring.sql.init.mode=always
spring.sql.init.schema-locations=classpath:schema.sql
spring.sql.init.data-locations=
spring.sql.init.platform=postgresql
spring.sql.init.continue-on-error=true
docker-compose.yml:10
docker-compose.ymlrelies on${POSTGRES_USER},${POSTGRES_PASSWORD}, and${POSTGRES_DB}without defaults.docker-compose upwill fail if these variables aren’t exported or if no.envis present. Consider adding defaults (e.g.${POSTGRES_USER:-postgres}) or hardcoding local-dev credentials in compose to make onboarding smoother.
environment:
POSTGRES_USER: ${POSTGRES_USER}
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}
POSTGRES_DB: ${POSTGRES_DB}
docker-compose.yml:6
- PostgreSQL version is inconsistent: README states PostgreSQL 15, but
docker-compose.ymlusespostgres:17.9-alpine. Align the documented version with the Docker image tag to avoid confusion and compatibility surprises.
image: postgres:17.9-alpine
container_name: xpeho_postgres
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Romain Vanhee <romain.vanhee@yrycom.com>
|
profotoce59
approved these changes
Mar 2, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (6)
src/main/java/com/xpeho/spring_boot_java_random_user/SpringBootJavaRandomUserApplication.java:16
- Loading
.envby force-setting JVM system properties can unintentionally override values already provided via real environment variables / JVM args, and it also makes secrets available throughSystem.getProperties(). Consider only setting properties when they are absent, and/or scoping dotenv loading to local/dev runs (not production).
Dotenv.configure()
.ignoreIfMissing()
.load()
.entries()
.forEach(entry -> System.setProperty(entry.getKey(), entry.getValue()));
pom.xml:125
- The build now globally skips Surefire (
<skip>true</skip>), somvn testwon’t run any tests and all*Test(s)classes are pushed into Failsafe/integration-test. This makes the default Maven lifecycle surprising and will force future unit tests to run in the integration-test phase (and potentially require Docker). Prefer keeping Surefire enabled for unit tests and using a dedicated naming pattern for integration tests (e.g.,*IT).
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<skip>true</skip>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<configuration>
<includes>
<include>**/*Tests.java</include>
<include>**/*Test.java</include>
</includes>
</configuration>
docker-compose.yml:16
postgres_datais a persistent named volume. With the schema mounted as an init script in/docker-entrypoint-initdb.d, schema initialization only runs the first time the volume is created; subsequentupruns won’t apply updates. Either document runningdocker compose down -vwhen schema changes, or make the test/dev compose setup use ephemeral storage.
volumes:
- postgres_data:/var/lib/postgresql/data
- ./src/main/resources/schema.sql:/docker-entrypoint-initdb.d/01-schema.sql
healthcheck:
README.md:20
- The Tech Stack section states PostgreSQL version
15, butdocker-compose.ymlpinspostgres:17.9-alpine. Align the documented version with what’s actually used (or update the image tag to match the intended supported version).
| springdoc-openapi | 3.0.1 |
| PostgreSQL | 15 |
| dotenv-java | 5.2.2 |
| Docker / Docker Compose | - |
src/main/resources/schema.sql:4
- Using a table named
useris problematic in PostgreSQL becauseUSERis a reserved keyword and can require identifier quoting in generated SQL. Prefer a non-reserved table name (e.g.,users) to avoid query/ORM/dialect issues.
DROP TABLE IF EXISTS "user";
CREATE TABLE IF NOT EXISTS "user"
(
README.md:162
- The “Required GitHub Secrets” markdown table is malformed (missing the separator row and pipe delimiters for the
APPLICATION_TEST_PROPERTIESrow), so it won’t render as a table. Please fix the table formatting so the secrets list is readable.
### Required GitHub Secrets
| Secret | Description |
| `APPLICATION_TEST_PROPERTIES` (Base64-encoded `application-test.properties` content)
| `SONAR_TOKEN` | SonarQube authentication token |
| `SONAR_HOST_URL` | SonarQube instance URL |
| `POSTGRES_USER` | PostgreSQL user |
| `POSTGRES_PASSWORD` | PostgreSQL password |
| `POSTGRES_DB` | Database name |
| `POSTGRES_PORT` | PostgreSQL port |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#6