Migrate CEL policy engine to JDBI and simplify code structure#1906
Migrate CEL policy engine to JDBI and simplify code structure#1906
Conversation
68af673 to
edf4229
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates the CEL-based policy evaluation engine’s persistence and data-loading layer to JDBI, removing legacy JDO/JDBC mixes and simplifying how policy evaluation data is queried and mapped into protobuf models.
Changes:
- Introduces a new JDBI-based query DAO (
CelPolicyQueryDao) and replaces the prior SQLObject DAO + JDO query manager approach. - Replaces reflection/annotation-based field mapping and projection classes with a central field-mapping registry and direct protobuf row mappers.
- Refactors policy violation reconciliation and notification emission to run fully through JDBI (including new notification subject querying).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyEngine.java | Refactors CEL policy evaluation to use the new JDBI query DAO, simplified script compilation flow, and JDBI-based notification emission. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/CelVulnerabilityPolicyEvaluator.java | Switches required-field loading from the removed SQLObject DAO to CelPolicyQueryDao and updates logging style. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/CelCommonPolicyLibrary.java | Routes “direct dependency” checks through CelPolicyQueryDao and removes duplicated SQL helper. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyQueryDao.java | New JDBI DAO consolidating queries for projects, components, licenses, vulnerabilities, applicable policies, and violation reconciliation. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyFieldMappingRegistry.java | New registry mapping proto field names to SQL expressions, used to build SELECT lists consistently. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyLicenseRowMapper.java | New row mapper mapping license rows (and aggregated groups) into policy protobuf License. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyComponentRowMapper.java | Extends component-to-proto mapping with additional hash fields; class made final. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyProjectRowMapper.java | Simplifies optional column mapping via maybeSet and adjusts JSON property parsing to be more robust. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyVulnerabilityRowMapper.java | Updates risk-score handling to include CVSS v4 fields and makes helper method static; class made final. |
| apiserver/src/main/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDao.java | Adds a JDBI query method to fetch notification subjects for newly created policy violations. |
| apiserver/src/main/java/org/dependencytrack/policy/EvalProjectPoliciesActivity.java | Ensures MDC project UUID is set around policy evaluation invocation. |
| apiserver/src/main/java/org/dependencytrack/util/NotificationUtil.java | Removes legacy JDO-based policy violation notification analysis utility. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/persistence/CelPolicyDao.java | Removes the legacy SQLObject DAO previously used to load required proto fields. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/CelPolicyQueryManager.java | Removes the legacy JDO/JDBC-based query manager in favor of JDBI. |
| apiserver/src/main/java/org/dependencytrack/policy/cel/mapping/* | Removes projection/annotation/reflection mapping infrastructure now superseded by the field registry + row mappers. |
| apiserver/src/test/java/org/dependencytrack/policy/cel/persistence/CelPolicyDaoTest.java | Updates tests to use CelPolicyQueryDao and extends JSON expectations for newly mapped fields. |
| apiserver/src/test/java/org/dependencytrack/policy/cel/CelPolicyEngineTest.java | Updates evaluation invocation in a test to align with the refactored engine behavior. |
| apiserver/src/test/java/org/dependencytrack/policy/cel/mapping/FieldMappingUtilTest.java | Removes tests that validated the removed reflection/annotation-based mapping utility. |
Comments suppressed due to low confidence (1)
apiserver/src/main/java/org/dependencytrack/persistence/jdbi/NotificationSubjectDao.java:21
com.google.protobuf.util.Timestampsis imported but not used anywhere in this file, which will fail compilation if unused-import checks are enabled. Please remove the unused import.
import org.dependencytrack.notification.proto.v1.Component;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
edf4229 to
54e777b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
54e777b to
a960b18
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a960b18 to
96736c3
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
96736c3 to
4ed9365
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The engine pre-dates the introduction of JDBI and contained a wild mix of JDO and JDBC API usage. Now it only uses JDBI. Also: * Replaces the indirection of projection classes with direct mapping to Proto objects. * Replaces `@MappedField` annotations with a simple field mapping registry. * Unifies SQL code style. * Cleans up duplicated code. * Replaces in-memory policy violation reconciliation with a single upsert-and-delete query. Signed-off-by: nscuro <nscuro@protonmail.com>
4ed9365 to
e62f2a2
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Migrates CEL policy engine to JDBI and simplify code structure.
The engine pre-dates the introduction of JDBI and contained a wild mix of JDO and JDBC API usage. Now it only uses JDBI.
Also:
@MappedFieldannotations with a simple field mapping registry.Addressed Issue
N/A
Additional Details
N/A
Checklist
This PR fixes a defect, and I have provided tests to verify that the fix is effectiveThis PR implements an enhancement, and I have provided tests to verify that it works as intendedThis PR introduces changes to the database model, and I have updated the migration changelog accordinglyThis PR introduces new or alters existing behavior, and I have updated the documentation accordingly