fix: ignore applied migration checksums via config #67
Conversation
📝 WalkthroughWalkthroughThis pull request updates the Lombok dependency to version 1.18.36, refactors Flyway configuration into a centralized helper method, and introduces a configurable flag to control Flyway applied-migration checksum validation behavior via a new application property. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
|
@drtechie i have raised PR for fix: ignore applied migration checksums via config |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/resources/application-local.properties (1)
1-19: Consider documenting that credentials are example values.The hardcoded credentials (
user/password) should be documented as placeholder values for local development. Consider adding a comment indicating developers should customize these or use environment variable overrides.💡 Suggested documentation addition
Add a comment at the top of the file:
+# Local development configuration +# Replace user/password with your local database credentials or use environment variables + spring.datasource.dbiemr.url=jdbc:mysql://localhost:3306/dbiemr spring.datasource.dbiemr.username=user spring.datasource.dbiemr.password=password
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
pom.xmlsrc/main/java/com/db/piramalswasthya/config/FlywayConfig.javasrc/main/resources/application-local.propertiessrc/main/resources/application.properties
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java (2)
src/main/java/com/db/piramalswasthya/config/DatasourceConfig.java (1)
Configuration(10-37)src/main/java/com/db/piramalswasthya/config/JdbcTemplateConfig.java (1)
Configuration(10-32)
🔇 Additional comments (4)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java (2)
15-16: LGTM: Property binding with sensible default.The @value annotation correctly binds to the new property with a safe default of
false(strict validation).
31-49: Excellent refactoring to eliminate duplication.The centralized
createFlywayhelper eliminates repetitive configuration code across all four Flyway beans. The delegation is clean and maintainable.src/main/resources/application.properties (1)
52-52: LGTM: Safe production default.Setting the default to
falseensures strict checksum validation in production environments, which is the correct security posture.src/main/resources/application-local.properties (1)
21-21: Appropriate setting for local development.Setting
ignore-applied-migration-checksum=truefor local development makes sense, as it allows developers to iterate on migrations without checksum validation failures.
| <dependency> | ||
| <groupId>org.projectlombok</groupId> | ||
| <artifactId>lombok</artifactId> | ||
| <version>1.18.36</version> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest Lombok version from Maven Central
echo "=== Latest Lombok Version ==="
curl -s 'https://search.maven.org/solrsearch/select?q=g:org.projectlombok+AND+a:lombok&rows=1&wt=json' | jq -r '.response.docs[0].latestVersion'
# Check for security advisories via GitHub
echo "=== Security Advisories ==="
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: MAVEN, package: "org.projectlombok:lombok") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}' 2>&1 || echo "GitHub GraphQL query failed or no vulnerabilities found"Repository: PSMRI/AMRIT-DB
Length of output: 173
Update Lombok to version 1.18.38.
Version 1.18.36 is two minor versions behind the latest available version (1.18.38). No security vulnerabilities were found for the current version, but upgrading to 1.18.38 is recommended to maintain currency with the latest stable release.
🤖 Prompt for AI Agents
In pom.xml around line 55, the Lombok dependency version is pinned to 1.18.36;
update the <version> element to 1.18.38 to bring the dependency to the latest
stable release, then rebuild (mvn clean verify or mvn -U) to refresh the
dependency cache and ensure the project compiles with the new Lombok version.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
We manage environments with property files in environments folder.
This file should not be committed.
|
Thanks for the review @drtechie! Local Properties: Got it. I'll remove application-local.properties from the PR. I'll move the necessary local configs to my own local environment and ensure they stay out of the repo. Flyway Pattern: I also saw the bot's catch regarding the *:applied syntax. I'm looking into the correct pattern for the Community version now. I'll push an update shortly with these fixes! |
Updated ignoreMigrationPatterns to fix invalid pattern syntax for Flyway configuration.
|
Thanks for the review @drtechie. I've removed the application-local.properties file as requested. I also updated the Flyway pattern in FlywayConfig.java to use a valid status (*:Ignored) to fix the issue flagged by CodeRabbit. Let me know if everything looks good now! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main/java/com/db/piramalswasthya/config/FlywayConfig.java:
- Around line 22-35: The createFlyway method uses Flyway.
ignoreMigrationPatterns which is a Teams-only API and also does not suppress
checksum validation; remove the ignoreMigrationPatterns("*:Ignored") call in
createFlyway and instead disable validation for migrations in the Community
edition by calling configuration.validateOnMigrate(false) or perform
Flyway.repair() at startup where appropriate; update the conditional that checks
ignoreAppliedMigrationChecksum to set validateOnMigrate(false) (or trigger a
repair flow) so the app can start when local checksums differ without relying on
Teams-only methods.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java (2)
src/main/java/com/db/piramalswasthya/config/DatasourceConfig.java (1)
Configuration(10-37)src/main/java/com/db/piramalswasthya/config/JdbcTemplateConfig.java (1)
Configuration(10-32)
🔇 Additional comments (3)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java (3)
6-6: LGTM!The new imports are appropriate for the added functionality.
Also applies to: 8-8
15-16: LGTM!The configuration field with a safe default of
falseensures strict validation in production environments unless explicitly overridden.
37-55: Excellent refactoring!The consolidation of Flyway bean configuration into the centralized
createFlywayhelper eliminates code duplication and ensures consistent configuration across all four database sources.
|
@saurabh24thakur can you also explain how you have tested this? Thanks. |
|
Please check the open comments. Also, I can see the local properties file still in changed files. |
|
@saurabh24thakur any updates? |
|
|
@drtechie |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/db/piramalswasthya/config/FlywayConfig.java`:
- Around line 28-32: You correctly replaced Teams-only ignoreMigrationPatterns
with configuration.validateOnMigrate(false), but ensure production environments
do not disable validation: update the logic around
ignoreAppliedMigrationChecksum/validateOnMigrate so that
configuration.validateOnMigrate(false) is only applied for non-production
contexts (e.g., local/dev profiles or an explicit opt-in flag) and keep
configuration.validateOnMigrate(true) for production; locate the conditional
using ignoreAppliedMigrationChecksum and the configuration.validateOnMigrate
call to add environment/profile or explicit config checks before flipping
validation.
🧹 Nitpick comments (1)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java (1)
18-21: Outdated comment references removed implementation.The Javadoc mentions "valid ignoreMigrationPatterns status" but the code now uses
validateOnMigrate(false). Update the comment to reflect the current implementation.Suggested comment update
/** * Helper method to create a Flyway instance with centralized configuration. - * Updated to use valid ignoreMigrationPatterns status. + * When ignoreAppliedMigrationChecksum is enabled, validation is disabled + * to allow the app to start even if checksums for applied migrations differ. */
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java (2)
src/main/java/com/db/piramalswasthya/config/DatasourceConfig.java (1)
Configuration(10-37)src/main/java/com/db/piramalswasthya/config/JdbcTemplateConfig.java (1)
Configuration(10-32)
🔇 Additional comments (2)
src/main/java/com/db/piramalswasthya/config/FlywayConfig.java (2)
15-16: LGTM! Safe default configuration.The property defaults to
falsewhich maintains strict checksum validation in production. The property naming follows Spring conventions (kebab-case in properties, camelCase in Java).
37-55: Good refactoring - DRY principle applied.The bean methods are cleanly refactored to use the centralized helper. The pattern is consistent with
JdbcTemplateConfigandDatasourceConfig, and the migration paths follow a logical convention.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| if (ignoreAppliedMigrationChecksum) { | ||
| // Updated to use validateOnMigrate(false) which is supported in Community Edition | ||
| // This disables validation of applied migrations, allowing the app to start even if checksums change | ||
| configuration.validateOnMigrate(false); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Flyway validateOnMigrate false what does it disable
💡 Result:
Setting flyway.validateOnMigrate=false disables Flyway's automatic validation step when you run migrate — i.e., Flyway will not check applied vs. available migrations (checksums/CRC, changed/removed files, missing or out-of-sequence migrations, filename/validation rules) before applying new migrations, so migrate will proceed without failing on validation errors. Defaults to true. [1][2]
Good fix for Community edition compatibility.
The implementation now correctly uses validateOnMigrate(false) which is available in the Flyway Community edition, addressing the previous concern about ignoreMigrationPatterns being a Teams-only feature.
Note: Be aware that validateOnMigrate(false) disables validation checks including:
- Missing migrations (files deleted after being applied)
- Out-of-order migrations
- Checksum validation for previously applied migrations
This is acceptable for local development but ensure production always uses the default (validateOnMigrate(true) for validation enabled).
🤖 Prompt for AI Agents
In `@src/main/java/com/db/piramalswasthya/config/FlywayConfig.java` around lines
28 - 32, You correctly replaced Teams-only ignoreMigrationPatterns with
configuration.validateOnMigrate(false), but ensure production environments do
not disable validation: update the logic around
ignoreAppliedMigrationChecksum/validateOnMigrate so that
configuration.validateOnMigrate(false) is only applied for non-production
contexts (e.g., local/dev profiles or an explicit opt-in flag) and keep
configuration.validateOnMigrate(true) for production; locate the conditional
using ignoreAppliedMigrationChecksum and the configuration.validateOnMigrate
call to add environment/profile or explicit config checks before flipping
validation.
There was a problem hiding this comment.
@saurabh24thakur what does this mean? Are we ignoring checksum validation only, or other things too (Missing and out-of-order migrations)?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
@saurabh24thakur |
2 participants
📋 Description
JIRA ID: N/A
This PR addresses the issue where the application fails to start if the checksum of an already applied Flyway migration file changes. This is a common scenario during development or when minor non-functional changes (like comments) are added to existing migrations.
Changes:
amrit.flyway.ignore-applied-migration-checksum.flyway.ignoreMigrationPatterns("*:applied")when this property is set totrue.falsein application.properties (safe for production).true) in application-local.properties for a smoother local development experience.lombokversion in pom.xml to1.18.36to support building with newer JDK versions (e.g., JDK 24).✅ Type of Change
ℹ️ Additional Information
Verification:
ignoreMigrationPatternswhen the property is active.Configuration:
amrit.flyway.ignore-applied-migration-checksum=false(Strict validation)amrit.flyway.ignore-applied-migration-checksum=true(Ignores checksums for applied migrations)Summary by CodeRabbit
Release Notes
Dependencies
Configuration
amrit.flyway.ignore-applied-migration-checksumfor Flyway migration validation (default: false)✏️ Tip: You can customize this high-level summary in your review settings.