Feature/dry run config - #1785
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
![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
🤖 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
`@Java/validation/src/main/java/com/lantanagroup/link/validation/services/RubricExecutionService.java`:
- Around line 113-114: Update the log.info call in the dry-run recording flow to
sanitize version.getRubricId(), version.getSemver(), and
out.resultEntity().getStatus() before passing them as logger arguments, using
the existing logging-sanitization utility or pattern.
In
`@Java/validation/src/test/java/com/lantanagroup/link/validation/services/RubricRegistryServiceTest.java`:
- Around line 94-103: Extend RubricRegistryServiceTest with a focused publish
test for an ACCEPTABLE dryRunStatus whose dryRunCompletedAt remains null.
Configure the draft version and required dry-run gate, assert publish throws
RubricDryRunRequiredException, and verify no lifecycle event is written, reusing
the existing event-verification setup and symbols.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 52b2a622-d9a6-4c77-bf45-db6ec859848e
📒 Files selected for processing (17)
Java/validation/src/main/java/com/lantanagroup/link/validation/configs/RubricDryRunConfig.javaJava/validation/src/main/java/com/lantanagroup/link/validation/controllers/RubricController.javaJava/validation/src/main/java/com/lantanagroup/link/validation/controllers/RubricExceptionHandler.javaJava/validation/src/main/java/com/lantanagroup/link/validation/controllers/ValidationController.javaJava/validation/src/main/java/com/lantanagroup/link/validation/entities/RubricVersion.javaJava/validation/src/main/java/com/lantanagroup/link/validation/exceptions/RubricDryRunRequiredException.javaJava/validation/src/main/java/com/lantanagroup/link/validation/models/RubricVersionDetailDto.javaJava/validation/src/main/java/com/lantanagroup/link/validation/models/RubricVersionSummaryDto.javaJava/validation/src/main/java/com/lantanagroup/link/validation/repositories/RubricVersionRepository.javaJava/validation/src/main/java/com/lantanagroup/link/validation/services/RubricExecutionService.javaJava/validation/src/main/java/com/lantanagroup/link/validation/services/RubricRegistryService.javaJava/validation/src/main/resources/application.ymlJava/validation/src/main/resources/database/migrations/U20260804__Rubric_version_dry_run.sqlJava/validation/src/main/resources/database/migrations/V20260804__Rubric_version_dry_run.sqlJava/validation/src/test/java/com/lantanagroup/link/validation/controllers/RubricControllerTest.javaJava/validation/src/test/java/com/lantanagroup/link/validation/services/RubricExecutionServiceTest.javaJava/validation/src/test/java/com/lantanagroup/link/validation/services/RubricRegistryServiceTest.java
| log.info("Recorded dry run for rubric {} v{}: {}", | ||
| version.getRubricId(), version.getSemver(), out.resultEntity().getStatus()); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sanitize all new logger arguments.
Line 113 logs rubric ID, semantic version, and result status without sanitization. Sanitize each value before passing it to log.info.
Proposed fix
log.info("Recorded dry run for rubric {} v{}: {}",
- version.getRubricId(), version.getSemver(), out.resultEntity().getStatus());
+ LogUtils.sanitize(version.getRubricId()),
+ LogUtils.sanitize(version.getSemver()),
+ LogUtils.sanitize(out.resultEntity().getStatus().name()));As per path instructions, “All logging message arguments must be sanitized before they are passed to logger methods to avoid static code scanning security/vulnerability findings.”
🤖 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
`@Java/validation/src/main/java/com/lantanagroup/link/validation/services/RubricExecutionService.java`
around lines 113 - 114, Update the log.info call in the dry-run recording flow
to sanitize version.getRubricId(), version.getSemver(), and
out.resultEntity().getStatus() before passing them as logger arguments, using
the existing logging-sanitization utility or pattern.
Source: Path instructions
| @Test | ||
| @DisplayName("dry-run gate on + no completed dry run -> publish blocked") | ||
| void publish_dryRunRequiredButNotCompleted() { | ||
| dryRunConfig.setRequiredForPublish(true); | ||
| stubVersion(draftVersion()); | ||
|
|
||
| assertThatThrownBy(() -> service().publish("piqi.core", "1.0.0", "qa")) | ||
| .isInstanceOf(RubricDryRunRequiredException.class) | ||
| .hasMessageContaining("no dry run has been completed"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test an acceptable status without a completion time.
publish requires dryRunCompletedAt and an acceptable dryRunStatus. This test only covers both values being absent. Add a focused JUnit test where dryRunStatus is ACCEPTABLE and dryRunCompletedAt is null. Assert that publishing throws RubricDryRunRequiredException and does not write a lifecycle event.
As per path instructions, “If/Else or Switch/Case blocks are introduced or modified — ensure each branch has a corresponding unit test.”
🤖 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
`@Java/validation/src/test/java/com/lantanagroup/link/validation/services/RubricRegistryServiceTest.java`
around lines 94 - 103, Extend RubricRegistryServiceTest with a focused publish
test for an ACCEPTABLE dryRunStatus whose dryRunCompletedAt remains null.
Configure the draft version and required dry-run gate, assert publish throws
RubricDryRunRequiredException, and verify no lifecycle event is written, reusing
the existing event-verification setup and symbols.
Source: Path instructions
…ace, soft-delete superseded checks, and record a new lifecycle event; PUBLISHED and RETIRED versions stay immutable
…eject duplicate check definitions within a rubric
…into the base governance migration
…e audit trail stays intact
2 participants
🛠️ Description of Changes
Please provide a high-level overview of the changes included in this PR.
🧪 Testing Performed
Please describe the testing that was performed on the changes included in this PR.
🧑🔬 Unit Testing
📓 Documentation Updated
Please update any relevant sections in the project documentation that were impacted by the changes in the PR.
Summary by CodeRabbit