[DAT-20351] Migrate to Java 21 and update dependencies in pom to the latest version (plus Junit4 to Jupiter migration)#275
Conversation
…latest version (plus Junit4 to Jupiter migration) (#268) Original PR with all comments and descriptions -> #268 * [DAT-20351] - update to java21 and Groovy 4, all tests are passed except of liquibase.changelog.filter.ShouldRunChangeSetFilterTest.should_decline_not_changed_changeset_when_has_run_on_change. We need to figure out why behaviour of RunChangesetFilter changed during this upgrade * [DAT-20351] - fixup ShouldRunChangeSetFilterTest * [DAT-20351] - fixup integration tests, minor clean up *.groovy files * [DAT-20351] - update snakeyaml 1.17 -> 2.4, minor *.java tests clean up * [DAT-20351] - improved coverage reporting to see dependencies, licenses and coverage info ran either [mvn -U clean install site] or [mvn -U clean verify site] * [DAT-20351] - post review changes * [DAT-20351] - post review changes - second round * [DAT-20351] - post review changes - third round * [DAT-20351] - one more 'AfterEach' move up
… to pom (as we don't generate main JAR as we don't have src module) and add phases to generate test JAR and attach surefire plugin to run during test (because of packaging type pom) (#277) * [DAT-20351] - update to java21 and Groovy 4, all tests are passed except of liquibase.changelog.filter.ShouldRunChangeSetFilterTest.should_decline_not_changed_changeset_when_has_run_on_change. We need to figure out why behaviour of RunChangesetFilter changed during this upgrade * [DAT-20351] - fixup ShouldRunChangeSetFilterTest * [DAT-20351] - fixup integration tests, minor clean up *.groovy files * [DAT-20351] - update snakeyaml 1.17 -> 2.4, minor *.java tests clean up * [DAT-20351] - improved coverage reporting to see dependencies, licenses and coverage info ran either [mvn -U clean install site] or [mvn -U clean verify site] * [DAT-20351] - post review changes * [DAT-20351] - post review changes - second round * [DAT-20351] - post review changes - third round * [DAT-20351] - one more 'AfterEach' move up * [DAT-20627] - try to remove maven source plugin config from parent plugin management to avoid issue on CI/CD * [DAT-20627] - move source plugin configuration from core to parent level * [DAT-20627] - try to update maven assembly plugin * [DAT-20627] - this one is working locally! * [DAT-20627] - return the latest version of resource plugin, as it was downgraded by mistake (only source plugin should downgraded a bit) * [DAT-20682] Change packaging type to pom (as we don't generate main JAR as we don't have src module) and add phases to generate test JAR and attach surefire plugin to run during test (because of packaging type pom) (#279) * [DAT-20682] Change packaging type to pom (as we don't generate main JAR as we don't have src module) and add phases to generate test JAR and attach surefire plugin to run during test (because of packaging type pom) * [DAT-20682] Add comments into POM file --------- Co-authored-by: Andrii Tytarchuk <27773495+andrulban@users.noreply.github.com>
… needed any more (#280)
… tests for Sonar Cloud (#282) * [DAT-21138] - 1) Extended code point limit for Yaml 2) introduce unit tests for Sonar Cloud * [DAT-21391] - post-code review changes
- Add continue-on-error to security report step to prevent timeout failures from breaking the workflow Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @andrulban's task in 2m 38s —— View job PR Review CompleteTodo List:
Review SummaryLGTM - No significant issues found in this Java 21 migration and dependency update PR. Key Changes ReviewedProduction Code Changes:
Build & Dependencies:
Test Coverage:
No bugs, security vulnerabilities, or logic errors detected in the changed code. |
* [DAT-21063] - preserve catalog name case only
|
Claude finished @v-petrovych's task in 1m 21s —— View job PR Review CompleteTodo List:
Review SummaryLGTM - No significant issues found in this Java 21 migration and dependency update PR. Files ReviewedProduction Code Changes:
Build & Dependencies (pom.xml):
Test Coverage:
No bugs, security vulnerabilities, or logic errors detected. |
… logic (#287) The driverUsesSpFkeys() method in ForeignKeySnapshotGenerator had a flawed semver comparison that caused FK referential action rules (ON DELETE/ON UPDATE) to be misread after upgrading the MSSQL JDBC driver from 12.4.0 to 13.2.1 (driver package 1.0.66 → 1.0.72). Root cause: The old condition `driverMajorVersion >= 6 && driverMinorVersion >= 3` incorrectly evaluates for any driver with major > 6 but minor < 3. For driver 13.2.1: `13 >= 6 && 2 >= 3` → false, causing the method to wrongly return true (thinks driver uses sp_fkeys convention). This meant when the fastFetch() path called JDBC getImportedKeys(), the returned standard JDBC constant 3 (importedKeyNoAction) was interpreted using sp_fkeys convention where 3 = importedKeySetDefault. Meanwhile the bulkFetch() path used Liquibase's custom MSSQL SQL (ObjectProperty/CnstIsDeleteCascade) which only returns 0 or 1, accidentally producing the correct result. The mismatch between these two paths caused diff to report phantom FK changes (e.g. "deleteRule changed from importedKeyNoAction to importedKeySetDefault") even when both databases had identical NO ACTION rules, resulting in non-empty diffChangeLogs. Observed failure: Liquibase-CLI-testing repo, customer_schema_mssql_phytelmaster.feature:113 expected an empty diffChangeLog but got changesets dropping and recreating FK PopulationRuleCodeSet_PopulationRuleCodeSetConfiguration_FK1. Fix: Change the version comparison to properly handle semver: `driverMajorVersion > 6 || (driverMajorVersion == 6 && driverMinorVersion >= 3)` Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
|
Claude finished @andrulban's task in 1m 3s —— View job Todo List:
Review SummaryLGTM - The latest commit (c79221f) contains a critical bug fix with correct implementation. Change ReviewedForeignKeySnapshotGenerator.java:282 - Fixed flawed semver comparison in MSSQL JDBC driver version check Old condition (buggy): if (driverMajorVersion >= 6 && driverMinorVersion >= 3)New condition (correct): if (driverMajorVersion > 6 || (driverMajorVersion == 6 && driverMinorVersion >= 3))Why this fix is correct:
No security vulnerabilities, logic errors, or other issues detected. |
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
3 participants
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.
No description provided.