JPA Configuration - #3
Open
ramonjardon wants to merge 16 commits into
Open
Conversation
JPA Configuration
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR establishes production infrastructure and runtime configuration for a Spring Boot application. It adds Docker multi-stage builds and ChangesProduction Infrastructure and Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Update codeql configuration
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
src/main/java/dev/ramonjardon/portfolio/infrastructure/config/TimeZoneConfig.java (1)
1-10:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftComponent may not be discovered by component scanning.
This class has the same component scanning issue as
AsyncConfigandJacksonConfig. The class is indev.ramonjardon.portfolio.infrastructure.config, which is outside the default scan scope of the@SpringBootApplicationindev.ramonjardon.portfolio.backend.Without explicit configuration, this component will not be registered, and the JVM default timezone will not be set to UTC.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/TimeZoneConfig.java` around lines 1 - 10, TimeZoneConfig (class TimeZoneConfig in package dev.ramonjardon.portfolio.infrastructure.config) is outside the default component-scan root and may not be registered; ensure the component is discovered by either moving TimeZoneConfig into a package scanned by your `@SpringBootApplication` (e.g., under dev.ramonjardon.portfolio.backend or its subpackages) or by adding explicit scanning to your application (use `@ComponentScan` with the package dev.ramonjardon.portfolio.infrastructure.config on the main application class) so the `@Component` and its ContextRefreshedEvent listener run and set the JVM timezone to UTC.src/main/java/dev/ramonjardon/portfolio/infrastructure/config/JacksonConfig.java (1)
1-18:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftConfiguration class may not be discovered by component scanning.
This class has the same component scanning issue as
AsyncConfig. The class is indev.ramonjardon.portfolio.infrastructure.config, which is outside the default scan scope of the@SpringBootApplicationindev.ramonjardon.portfolio.backend.Without explicit configuration, this
@PrimaryJsonMapper.Builderbean will not be registered, and the custom Jackson configuration (non-recycling pool, enum handling, customizers) will not take effect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/JacksonConfig.java` around lines 1 - 18, JacksonConfig (the `@Configuration` class) lives outside the application's default component-scan so its `@Primary` JsonMapper.Builder bean never gets registered; fix by either moving JacksonConfig into a package under the main `@SpringBootApplication` base package (so it’s discovered), or explicitly register it from the main app by adding an `@Import`(JacksonConfig.class) or by configuring component scanning to include dev.ramonjardon.portfolio.infrastructure.config; ensure the same fix is applied for AsyncConfig if applicable so the custom JsonMapper.Builder and related customizers are actually registered.
🧹 Nitpick comments (4)
Dockerfile (1)
21-21: Pin the runtime base image to an immutable digest
DockerfileusesFROM public.ecr.aws/chainguard/wolfi-base:latestfor the runtime stage, and the mutable:latesttag can change contents over time, hurting reproducibility and supply-chain control.Update it to a pinned digest, e.g.
public.ecr.aws/chainguard/wolfi-base@sha256:<digest>.FROM public.ecr.aws/chainguard/wolfi-base:latest🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` at line 21, Replace the mutable tag for the runtime base image with an immutable digest: locate the FROM line that references public.ecr.aws/chainguard/wolfi-base:latest and change it to the digest-pinned form public.ecr.aws/chainguard/wolfi-base@sha256:<digest>; fetch the correct sha256 digest from the Chainguard/ECR registry (or your artifact repository) for the exact image you intend to pin and update the Dockerfile accordingly so the runtime stage uses the pinned digest instead of :latest.src/main/java/dev/ramonjardon/portfolio/infrastructure/config/TimeZoneConfig.java (1)
12-15: 💤 Low valueDocument JVM-wide side effect and consider single-execution guard.
TimeZone.setDefault()is a JVM-wide operation that affects all threads and libraries. While setting it to UTC is a good practice, be aware:
- Scope: This affects the entire JVM, not just Spring-managed beans
- Multiple invocations:
ContextRefreshedEventcan fire multiple times if there are parent-child application contexts (e.g., with Spring MVC or Spring Boot tests). While re-setting to UTC is harmless, it's inefficient.Consider adding a guard to ensure single execution:
private static final AtomicBoolean initialized = new AtomicBoolean(false); `@EventListener`(ContextRefreshedEvent.class) public void onApplicationEvent() { if (initialized.compareAndSet(false, true)) { TimeZone.setDefault(TimeZone.getTimeZone("UTC")); } }Alternatively, use
@PostConstructor set the timezone in themainmethod before starting Spring.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/TimeZoneConfig.java` around lines 12 - 15, The onApplicationEvent method in TimeZoneConfig calls TimeZone.setDefault JVM-wide on every ContextRefreshedEvent; add a single-execution guard (e.g., a private static AtomicBoolean initialized) and only call TimeZone.setDefault(TimeZone.getTimeZone("UTC")) when initialized.compareAndSet(false,true) returns true, or move the TimeZone.setDefault call out of the event listener into a `@PostConstruct` or the application's main method to ensure the JVM-wide side effect is documented and executed once.src/main/java/dev/ramonjardon/portfolio/infrastructure/config/AsyncConfig.java (1)
42-48: 💤 Low valueClarify timeout handling configuration.
The code sets
defaultTimeouton line 44 and registers aTimeoutCallableProcessingInterceptoron line 47. The interceptor is instantiated without passing an explicit timeout value, which may lead to confusion:
- Does the interceptor use the default timeout set on line 44?
- Or does it use its own internal default?
If the interceptor should use the same timeout, consider passing it explicitly:
configurer.registerCallableInterceptors( new TimeoutCallableProcessingInterceptor(asyncTimeout));If the current approach is intentional, consider adding a comment explaining the timeout handling strategy.
Also, the comment on line 45 is in Spanish.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/AsyncConfig.java` around lines 42 - 48, The configureAsyncSupport method sets the default timeout via setDefaultTimeout(asyncTimeout) but registers a TimeoutCallableProcessingInterceptor without an explicit timeout which may be ambiguous; either pass asyncTimeout into the interceptor (registerCallableInterceptors(new TimeoutCallableProcessingInterceptor(asyncTimeout))) so TimeoutCallableProcessingInterceptor and setDefaultTimeout use the same value, or add a concise comment in configureAsyncSupport explaining that the interceptor intentionally relies on the container default timeout; also replace the Spanish comment on the executor line with an English comment (or remove it) for consistency.src/main/resources/application.yaml (1)
9-16: Hikari sizing config is fine; JDBC URL/credentials are injected via compose env vars.
src/main/resources/application.yamldoesn’t setspring.datasource.url/spring.datasource.username/spring.datasource.password, butcompose.ymldefinesSPRING_DATASOURCE_URL,SPRING_DATASOURCE_USERNAME, andSPRING_DATASOURCE_PASSWORD(so JDBC config won’t be missing as long as those env vars are supplied). Consider documenting these required env vars and clarifying the Spanish inline comment# Par obligatorio de provider_disables_autocommit, which is currently attached to thehikari.maximum-pool-sizeline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/application.yaml` around lines 9 - 16, Add explicit documentation for the required JDBC env vars by adding a comment block near the datasource config that lists SPRING_DATASOURCE_URL, SPRING_DATASOURCE_USERNAME, and SPRING_DATASOURCE_PASSWORD as required environment variables; also move or reword the Spanish inline comment currently attached to datasource.hikari.maximum-pool-size (the text "# Par obligatorio de provider_disables_autocommit") so it is on its own line above the hikari block and clarified/translated (e.g., note that provider_disables_autocommit is required) to avoid being misinterpreted as part of the maximum-pool-size setting; update references to the YAML keys datasource.hikari.maximum-pool-size and the spring.datasource URL/username/password keys to reflect the documentation change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitignore:
- Line 34: Current .gitignore only ignores the literal ".env", which still
allows variants like ".env.local" or ".env.prod" to be committed; update the
.gitignore to ignore common env variants by adding broader patterns such as
".env*", ".env.*" and specific common names (e.g., ".env.local",
".env.production", ".env.test") so all environment file variants are excluded
from commits and accidental secret leaks are prevented.
In `@compose.yml`:
- Line 6: The compose.yml uses POSTGRES_DB for POSTGRES_USER
(POSTGRES_USER=${POSTGRES_DB}), which breaks when the DB name and DB user
differ; update the docker-compose service envs to use a dedicated POSTGRES_USER
variable (e.g., POSTGRES_USER=${POSTGRES_USER}) and ensure you add/define
POSTGRES_USER in your environment or .env so the DB user is supplied separately
from POSTGRES_DB; update both occurrences that currently reference POSTGRES_DB
for the username.
In
`@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/AsyncConfig.java`:
- Around line 19-31: AsyncConfig has a fragile circular dependency: the
constructor `@Lazy-injects` the same bean created by applicationTaskExecutor(),
producing a proxy stored in executor and used in configureAsyncSupport; replace
the constructor injection with a non-constructor approach to break and clarify
the cycle—either change to `@Autowired` `@Lazy` field injection of AsyncTaskExecutor
executor or inject ObjectProvider<AsyncTaskExecutor> (call
getIfAvailable()/getObject() inside configureAsyncSupport) so the bean creation
and post-processing are decoupled; also convert the Spanish comments above
executor to English to improve team readability.
- Around line 1-17: AsyncConfig is outside the default component-scan rooted at
PortfolioBackendApplication (package dev.ramonjardon.portfolio.backend) so
Spring may not load its beans; fix by either (a) moving the AsyncConfig class
into a package under dev.ramonjardon.portfolio.backend, (b) adding explicit
scanning to the main application class (PortfolioBackendApplication) via
scanBasePackages on `@SpringBootApplication` to include
"dev.ramonjardon.portfolio.infrastructure" or (c) importing the configuration
directly by adding `@Import`(AsyncConfig.class) to PortfolioBackendApplication;
pick one approach and apply it so the AsyncConfig configuration and its beans
are discovered at startup.
In
`@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/JacksonConfig.java`:
- Around line 20-43: The jsonMapperBuilder method should explicitly document
that EnumFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL (from
tools.jackson.databind.cfg.EnumFeature) is the correct Jackson 3 constant to
preserve current behavior and must remain enabled on the JsonMapper.Builder;
also add a short inline comment next to JsonRecyclerPools.nonRecyclingPool()
explaining the intentional trade-off (non-recycling to avoid object reuse issues
and potential threading/pool-related bugs at the cost of allocation overhead) so
future readers understand why a non-recycling pool is required when building the
JsonFactory.
In `@src/main/resources/application.yaml`:
- Around line 84-86: The YAML indentation for the app.async.timeout property is
incorrect; fix the indentation so "timeout" is nested under "async" with
consistent two-space indentation (i.e., ensure the key path app -> async ->
timeout uses the same two-space indent level as other entries under "async");
update the "timeout" line accordingly to match the file's indentation style.
- Around line 18-46: Update the Hibernate connection properties: use the
Hibernate 6 key hibernate.connection.provider_disables_autocommit (keep or
remove/set to false based on whether your connection pool actually hands out
connections with autocommit=false) and replace the incorrect
connection.handling_mode entry with the Hibernate 6 property
hibernate.connection.acquisition_mode, setting it to a valid value such as
AS_NEEDED or IMMEDIATELY; ensure provider_disables_autocommit is only true when
your pool guarantees autocommit=false to avoid SQL running outside transactions.
- Around line 51-74: The YAML contains invalid/deprecated Jackson 3 keys: keep
the correct datatype keys (datatype.datetime.write-dates-as-timestamps and
write-durations-as-timestamps) and time-zone/date-format, but remove
spring.jackson.deserialization.fail-on-ignored-properties (it’s not a valid
Spring property) and stop relying on default-property-inclusion in YAML; instead
implement the behavior in code by providing a JsonMapper/ObjectMapper
customization bean (e.g., a `@Configuration` that registers a JsonMapper or
Jackson2ObjectMapperBuilderCustomizer named jacksonCustomizer) that sets
DeserializationFeature.FAIL_ON_IGNORED_PROPERTIES = false if needed and
configures default property inclusion via
JsonMapper.setDefaultPropertyInclusion(JsonInclude.Value) so inclusion and
ignored-property behavior work with Jackson 3.
---
Duplicate comments:
In
`@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/JacksonConfig.java`:
- Around line 1-18: JacksonConfig (the `@Configuration` class) lives outside the
application's default component-scan so its `@Primary` JsonMapper.Builder bean
never gets registered; fix by either moving JacksonConfig into a package under
the main `@SpringBootApplication` base package (so it’s discovered), or explicitly
register it from the main app by adding an `@Import`(JacksonConfig.class) or by
configuring component scanning to include
dev.ramonjardon.portfolio.infrastructure.config; ensure the same fix is applied
for AsyncConfig if applicable so the custom JsonMapper.Builder and related
customizers are actually registered.
In
`@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/TimeZoneConfig.java`:
- Around line 1-10: TimeZoneConfig (class TimeZoneConfig in package
dev.ramonjardon.portfolio.infrastructure.config) is outside the default
component-scan root and may not be registered; ensure the component is
discovered by either moving TimeZoneConfig into a package scanned by your
`@SpringBootApplication` (e.g., under dev.ramonjardon.portfolio.backend or its
subpackages) or by adding explicit scanning to your application (use
`@ComponentScan` with the package dev.ramonjardon.portfolio.infrastructure.config
on the main application class) so the `@Component` and its ContextRefreshedEvent
listener run and set the JVM timezone to UTC.
---
Nitpick comments:
In `@Dockerfile`:
- Line 21: Replace the mutable tag for the runtime base image with an immutable
digest: locate the FROM line that references
public.ecr.aws/chainguard/wolfi-base:latest and change it to the digest-pinned
form public.ecr.aws/chainguard/wolfi-base@sha256:<digest>; fetch the correct
sha256 digest from the Chainguard/ECR registry (or your artifact repository) for
the exact image you intend to pin and update the Dockerfile accordingly so the
runtime stage uses the pinned digest instead of :latest.
In
`@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/AsyncConfig.java`:
- Around line 42-48: The configureAsyncSupport method sets the default timeout
via setDefaultTimeout(asyncTimeout) but registers a
TimeoutCallableProcessingInterceptor without an explicit timeout which may be
ambiguous; either pass asyncTimeout into the interceptor
(registerCallableInterceptors(new
TimeoutCallableProcessingInterceptor(asyncTimeout))) so
TimeoutCallableProcessingInterceptor and setDefaultTimeout use the same value,
or add a concise comment in configureAsyncSupport explaining that the
interceptor intentionally relies on the container default timeout; also replace
the Spanish comment on the executor line with an English comment (or remove it)
for consistency.
In
`@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/TimeZoneConfig.java`:
- Around line 12-15: The onApplicationEvent method in TimeZoneConfig calls
TimeZone.setDefault JVM-wide on every ContextRefreshedEvent; add a
single-execution guard (e.g., a private static AtomicBoolean initialized) and
only call TimeZone.setDefault(TimeZone.getTimeZone("UTC")) when
initialized.compareAndSet(false,true) returns true, or move the
TimeZone.setDefault call out of the event listener into a `@PostConstruct` or the
application's main method to ensure the JVM-wide side effect is documented and
executed once.
In `@src/main/resources/application.yaml`:
- Around line 9-16: Add explicit documentation for the required JDBC env vars by
adding a comment block near the datasource config that lists
SPRING_DATASOURCE_URL, SPRING_DATASOURCE_USERNAME, and
SPRING_DATASOURCE_PASSWORD as required environment variables; also move or
reword the Spanish inline comment currently attached to
datasource.hikari.maximum-pool-size (the text "# Par obligatorio de
provider_disables_autocommit") so it is on its own line above the hikari block
and clarified/translated (e.g., note that provider_disables_autocommit is
required) to avoid being misinterpreted as part of the maximum-pool-size
setting; update references to the YAML keys datasource.hikari.maximum-pool-size
and the spring.datasource URL/username/password keys to reflect the
documentation change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7460a83c-46dd-4a80-a76d-53fdede8ff8e
📒 Files selected for processing (9)
.dockerignore.gitignoreDockerfilecompose.ymlpom.xmlsrc/main/java/dev/ramonjardon/portfolio/infrastructure/config/AsyncConfig.javasrc/main/java/dev/ramonjardon/portfolio/infrastructure/config/JacksonConfig.javasrc/main/java/dev/ramonjardon/portfolio/infrastructure/config/TimeZoneConfig.javasrc/main/resources/application.yaml
Comment on lines
+1
to
+17
| package dev.ramonjardon.portfolio.infrastructure.config; | ||
|
|
||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.ThreadFactory; | ||
|
|
||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.context.annotation.Lazy; | ||
| import org.springframework.core.task.AsyncTaskExecutor; | ||
| import org.springframework.core.task.support.TaskExecutorAdapter; | ||
| import org.springframework.web.context.request.async.TimeoutCallableProcessingInterceptor; | ||
| import org.springframework.web.servlet.config.annotation.AsyncSupportConfigurer; | ||
| import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; | ||
|
|
||
| @Configuration(proxyBeanMethods=false) | ||
| public class AsyncConfig implements WebMvcConfigurer{ |
There was a problem hiding this comment.
Configuration class may not be discovered by component scanning.
The main application class (PortfolioBackendApplication) is in package dev.ramonjardon.portfolio.backend, which means Spring's default component scanning will only discover components under that package and its sub-packages. This AsyncConfig class is in dev.ramonjardon.portfolio.infrastructure.config, which is outside the scan scope.
Without explicit configuration (e.g., @ComponentScan or scanBasePackages on @SpringBootApplication), this configuration will not be loaded, and the async executor and WebMvcConfigurer customizations will not be applied.
Run the following script to check if there's explicit component scan configuration:
#!/bin/bash
# Description: Check if PortfolioBackendApplication or any other config explicitly scans the infrastructure package
# Search for ComponentScan or scanBasePackages annotations
rg -n -A 3 -B 3 '`@ComponentScan`|scanBasePackages' --type=java
# Also check if there's an `@Import` that includes this config
rg -n '`@Import`.*AsyncConfig' --type=java🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/AsyncConfig.java`
around lines 1 - 17, AsyncConfig is outside the default component-scan rooted at
PortfolioBackendApplication (package dev.ramonjardon.portfolio.backend) so
Spring may not load its beans; fix by either (a) moving the AsyncConfig class
into a package under dev.ramonjardon.portfolio.backend, (b) adding explicit
scanning to the main application class (PortfolioBackendApplication) via
scanBasePackages on `@SpringBootApplication` to include
"dev.ramonjardon.portfolio.infrastructure" or (c) importing the configuration
directly by adding `@Import`(AsyncConfig.class) to PortfolioBackendApplication;
pick one approach and apply it so the AsyncConfig configuration and its beans
are discovered at startup.
Comment on lines
+20
to
+43
| @Bean | ||
| @Scope(ConfigurableBeanFactory.SCOPE_PROTOTYPE) | ||
| @Primary | ||
| public JsonMapper.Builder jsonMapperBuilder( | ||
| List<JsonMapperBuilderCustomizer> customizers) { | ||
|
|
||
| // JsonFactory con el pool configurado debe pasarse | ||
| // en el constructor del builder, no después | ||
| JsonFactory factory = JsonFactory.builder() | ||
| .recyclerPool(JsonRecyclerPools.nonRecyclingPool()) | ||
| .build(); | ||
|
|
||
| // Creamos el builder con la factory personalizada | ||
| JsonMapper.Builder builder = JsonMapper.builder(factory); | ||
| builder.enable(EnumFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL); | ||
| // Aplicamos manualmente todos los customizers de Boot 4 | ||
| // (los que leen application.yml, registran módulos, etc.) | ||
| // Esto es lo que hace JacksonAutoConfiguration internamente | ||
| for (JsonMapperBuilderCustomizer customizer : customizers) { | ||
| customizer.customize(builder); | ||
| } | ||
|
|
||
| return builder; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the correct way to configure READ_UNKNOWN_ENUM_VALUES_AS_NULL in Jackson 3 (tools.jackson.databind)?
💡 Result:
In Jackson 3, READ_UNKNOWN_ENUM_VALUES_AS_NULL has been moved from DeserializationFeature to EnumFeature [1][2]. To configure it, you must use the EnumFeature enumeration located in the tools.jackson.databind.cfg package [1][3]. You can enable this feature on an ObjectMapper or ObjectReader using the with(EnumFeature...) method [4]. Example usage: ObjectMapper mapper = new ObjectMapper; mapper.enable(EnumFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL); // Or via ObjectReader for specific deserialization calls ObjectReader reader = mapper.readerFor(MyEnum.class).with(EnumFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL); Key points to remember: - This feature is disabled by default [1]. - When enabled, unknown enum values are parsed as null instead of throwing an exception [1]. - It has lower precedence than READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE [1]. If both are enabled, the latter takes precedence [1]. - This feature may not work as expected when enum values are used as keys in an EnumMap or values in an EnumSet, as these data structures do not support null values [1].
Citations:
- 1: https://javadoc.io/static/tools.jackson.core/jackson-databind/3.0.0-rc3/tools.jackson.databind/tools/jackson/databind/cfg/EnumFeature.html
- 2: Move Enum-related
DeserializationFeatures intoEnumFeature(3.0) FasterXML/jackson-databind#5079 - 3: https://docs.openrewrite.org/recipes/java/jackson/upgradejackson_2_3_relocatedfeatureconstants
- 4: FasterXML/jackson-databind@05f2f2c
JacksonConfig: keep correct Jackson 3 enum feature + clarify nonRecyclingPool intent (lines 20-43)
builder.enable(EnumFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL);uses the correct Jackson 3 constant (it moved fromDeserializationFeaturetotools.jackson.databind.cfg.EnumFeature).JsonRecyclerPools.nonRecyclingPool()needs a brief comment explaining the intentional trade-off/why non-recycling is required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/dev/ramonjardon/portfolio/infrastructure/config/JacksonConfig.java`
around lines 20 - 43, The jsonMapperBuilder method should explicitly document
that EnumFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL (from
tools.jackson.databind.cfg.EnumFeature) is the correct Jackson 3 constant to
preserve current behavior and must remain enabled on the JsonMapper.Builder;
also add a short inline comment next to JsonRecyclerPools.nonRecyclingPool()
explaining the intentional trade-off (non-recycling to avoid object reuse issues
and potential threading/pool-related bugs at the cost of allocation overhead) so
future readers understand why a non-recycling pool is required when building the
JsonFactory.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Fix coderabbit manul
Fix the Errors detected by coderabbit
JPA Configuration and Jackdon optimize by graalvm
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/codeql.yml (1)
96-104:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Manual build step exits with error, breaking the workflow.
The workflow sets
build-mode: manualat line 55, but the manual build step (lines 96–104) is still a placeholder that exits withexit 1. This causes the workflow to fail before CodeQL analysis runs, completely breaking security scanning.Replace the placeholder with actual Maven build commands.
🔨 Proposed fix
- name: Run manual build steps if: matrix.build-mode == 'manual' shell: bash run: | - echo 'If you are using a "manual" build mode for one or more of the' \ - 'languages you are analyzing, replace this with the commands to build' \ - 'your code, for example:' - echo ' make bootstrap' - echo ' make release' - exit 1 + ./mvnw clean compile -DskipTests -B🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codeql.yml around lines 96 - 104, The workflow currently leaves the manual build placeholder (when matrix.build-mode == 'manual') as a run block that echoes messages and calls exit 1, which aborts the job; replace that placeholder run block with actual Maven build commands (e.g., a non-interactive Maven build such as a batch-mode package/verify with tests skipped or not as needed) so the step completes successfully and CodeQL can run; locate the conditional using matrix.build-mode == 'manual' and update the run script under that condition to invoke Maven (mvn -B ... appropriate goals) instead of exiting.
♻️ Duplicate comments (1)
src/main/java/dev/ramonjardon/portfolio/backend/infrastructure/config/JacksonConfig.java (1)
25-27: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDocument the nonRecyclingPool choice.
The
nonRecyclingPool()configuration needs an inline comment explaining the intentional trade-off. Given the PR objectives mention GraalVM optimization, this is likely chosen for native image compatibility and to avoid object-reuse threading issues, at the cost of allocation overhead.📝 Suggested documentation
JsonFactory factory = JsonFactory.builder() + // Non-recycling pool for GraalVM native image compatibility and thread safety. + // Trade-off: allocation overhead vs. avoiding object reuse/pooling issues. .recyclerPool(JsonRecyclerPools.nonRecyclingPool()) .build();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/ramonjardon/portfolio/backend/infrastructure/config/JacksonConfig.java` around lines 25 - 27, Add an inline comment next to the JsonFactory.builder() call explaining why JsonRecyclerPools.nonRecyclingPool() was chosen: note the intentional trade-off for GraalVM native-image compatibility and to avoid problematic object-reuse/threading issues in native images (at the cost of higher allocation overhead), so future maintainers understand this is deliberate; update the comment in JacksonConfig.java adjacent to the JsonFactory / JsonRecyclerPools.nonRecyclingPool() usage.
🧹 Nitpick comments (2)
.github/workflows/codeql.yml (1)
73-75: 💤 Low valueOptional: Use English for consistency.
The comment on line 73 is in Spanish while the rest of the workflow uses English. Consider translating for consistency.
📝 Proposed consistency fix
- # 2. 🔑 PERMISOS: Asegura que el entorno de GitHub pueda ejecutar tu script de Maven + # Grant execute permission so GitHub Actions can run the Maven wrapper - name: Grant execute permission for mvnw run: chmod +x mvnw🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/codeql.yml around lines 73 - 75, Translate the Spanish comment/step title to English for consistency: rename the workflow step currently titled "Grant execute permission for mvnw" (which is the step that runs the command `chmod +x mvnw`) or update its surrounding comment text from Spanish to English so the entire .github/workflows/codeql.yml file uses English consistently.src/main/java/dev/ramonjardon/portfolio/backend/infrastructure/config/JacksonConfig.java (1)
33-40: ⚡ Quick winUse English for code comments.
The inline comments are in Spanish, which reduces maintainability for international collaboration. Please translate to English.
🌐 Suggested translation
- // serializationInclusion() eliminado en Jackson 3 + // serializationInclusion() was removed in Jackson 3, use changeDefaultPropertyInclusion instead builder.changeDefaultPropertyInclusion(incl -> incl.withValueInclusion(JsonInclude.Include.NON_NULL) .withContentInclusion(JsonInclude.Include.NON_NULL) ); - // Evita notación científica en BigDecimal: 1234.50 en vez de 1.23450E+3 - // Coste: cero en serialización, solo afecta al formato de salida + // Avoids scientific notation for BigDecimal: 1234.50 instead of 1.23450E+3 + // Cost: zero overhead, only affects output format builder.enable(StreamWriteFeature.WRITE_BIGDECIMAL_AS_PLAIN);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/ramonjardon/portfolio/backend/infrastructure/config/JacksonConfig.java` around lines 33 - 40, Update the Spanish inline comments in JacksonConfig.java to English: change the comment above builder.changeDefaultPropertyInclusion(...) to explain that serializationInclusion() was removed in Jackson 3 and that the code sets non-null property and content inclusion, and change the comment above builder.enable(StreamWriteFeature.WRITE_BIGDECIMAL_AS_PLAIN) to explain it prevents scientific notation for BigDecimal (e.g., outputs 1234.50 instead of 1.23450E+3) and that it incurs no runtime cost in serialization; keep references to builder.changeDefaultPropertyInclusion and builder.enable(StreamWriteFeature.WRITE_BIGDECIMAL_AS_PLAIN) so reviewers can locate the exact lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/codeql.yml:
- Around line 66-71: The workflow step labeled "Set up JDK 25" currently uses
the mutable tag actions/setup-java@v4; replace that mutable tag with the
specific commit SHA for the latest v4 release to pin the action (i.e., change
the uses value from actions/setup-java@v4 to actions/setup-java@<commit-sha>),
and verify the correct SHA from the actions/setup-java GitHub releases page
before committing.
---
Outside diff comments:
In @.github/workflows/codeql.yml:
- Around line 96-104: The workflow currently leaves the manual build placeholder
(when matrix.build-mode == 'manual') as a run block that echoes messages and
calls exit 1, which aborts the job; replace that placeholder run block with
actual Maven build commands (e.g., a non-interactive Maven build such as a
batch-mode package/verify with tests skipped or not as needed) so the step
completes successfully and CodeQL can run; locate the conditional using
matrix.build-mode == 'manual' and update the run script under that condition to
invoke Maven (mvn -B ... appropriate goals) instead of exiting.
---
Duplicate comments:
In
`@src/main/java/dev/ramonjardon/portfolio/backend/infrastructure/config/JacksonConfig.java`:
- Around line 25-27: Add an inline comment next to the JsonFactory.builder()
call explaining why JsonRecyclerPools.nonRecyclingPool() was chosen: note the
intentional trade-off for GraalVM native-image compatibility and to avoid
problematic object-reuse/threading issues in native images (at the cost of
higher allocation overhead), so future maintainers understand this is
deliberate; update the comment in JacksonConfig.java adjacent to the JsonFactory
/ JsonRecyclerPools.nonRecyclingPool() usage.
---
Nitpick comments:
In @.github/workflows/codeql.yml:
- Around line 73-75: Translate the Spanish comment/step title to English for
consistency: rename the workflow step currently titled "Grant execute permission
for mvnw" (which is the step that runs the command `chmod +x mvnw`) or update
its surrounding comment text from Spanish to English so the entire
.github/workflows/codeql.yml file uses English consistently.
In
`@src/main/java/dev/ramonjardon/portfolio/backend/infrastructure/config/JacksonConfig.java`:
- Around line 33-40: Update the Spanish inline comments in JacksonConfig.java to
English: change the comment above builder.changeDefaultPropertyInclusion(...) to
explain that serializationInclusion() was removed in Jackson 3 and that the code
sets non-null property and content inclusion, and change the comment above
builder.enable(StreamWriteFeature.WRITE_BIGDECIMAL_AS_PLAIN) to explain it
prevents scientific notation for BigDecimal (e.g., outputs 1234.50 instead of
1.23450E+3) and that it incurs no runtime cost in serialization; keep references
to builder.changeDefaultPropertyInclusion and
builder.enable(StreamWriteFeature.WRITE_BIGDECIMAL_AS_PLAIN) so reviewers can
locate the exact lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1de1f302-c546-4cc3-9555-c2d43a2a9c89
📒 Files selected for processing (3)
.github/workflows/codeql.ymlsrc/main/java/dev/ramonjardon/portfolio/backend/infrastructure/config/JacksonConfig.javasrc/main/resources/application.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/resources/application.yaml
Fix Manual execution in CodeQL
JPA, Liquibase Configuration
Configure Action with semgresp and action with sonar and codecov
Fix sonar issue avoid secrets in run
…n/portfolio-backend into feature/1_project_scaffolding
Edit workflow to deploy in two phase generate dockerfile local
Owner
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Fix error with shas
Config initial test with test containers
|
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
1 participant
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.
JPA Configuration
Summary by CodeRabbit