Skip to content

Commit 57a76d8

Browse files
committed
Update PR description to clarify validation-time only storage
- Clarified that we store ONLY validation-time values (not both) - Removed fallback logic from code examples - Added table showing what gets stored vs. logged - Fixed alternatives section - removed contradiction - Changed exception type to AssertionError for programming bugs
1 parent 895fa2e commit 57a76d8

File tree

1 file changed

+31
-23
lines changed

1 file changed

+31
-23
lines changed

PR_DESCRIPTION.md

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -49,48 +49,47 @@ The problem: **validation reads BEFORE injection, storage reads AFTER injection*
4949

5050
## Solution
5151

52-
Capture properties at **validation time** for ALL builds (even when no cache exists). This ensures both validation and storage read at the same lifecycle point.
52+
Capture properties at **validation time** for ALL builds (even when no cache exists). Store ONLY validation-time values in the cache. This ensures both validation and storage read at the same lifecycle point.
5353

5454
### Implementation
5555

5656
1. **Modified `CacheResult`** to store validation-time mojo events
5757
2. **Modified `BuildCacheMojosExecutionStrategy`** to capture properties immediately after `findCachedBuild()`
58-
3. **Modified `save()`** to use validation-time events instead of execution-time events
58+
3. **Modified `save()`** to store ONLY validation-time events (fails with `AssertionError` if missing)
5959

6060
### Key Code Changes
6161

6262
```java
6363
// In BuildCacheMojosExecutionStrategy.execute():
6464
result = cacheController.findCachedBuild(session, project, mojoExecutions, skipCache);
6565

66-
// NEW: Capture validation-time properties for ALL mojos
67-
Map<String, MojoExecutionEvent> validationTimeEvents =
68-
captureValidationTimeProperties(session, project, mojoExecutions);
69-
70-
// Store in result
71-
result = CacheResult.rebuilded(result, validationTimeEvents);
66+
// NEW: Always capture validation-time properties when cache is enabled
67+
if (cacheState == INITIALIZED) {
68+
Map<String, MojoExecutionEvent> validationTimeEvents =
69+
captureValidationTimeProperties(session, project, mojoExecutions);
70+
result = CacheResult.rebuilded(result, validationTimeEvents);
71+
}
7272
```
7373

7474
```java
7575
// In save():
76-
// Use validation-time events instead of execution-time events
77-
Map<String, MojoExecutionEvent> propertyEvents =
78-
cacheResult.getValidationTimeEvents() != null
79-
? cacheResult.getValidationTimeEvents() // Use validation-time
80-
: executionEvents; // Fallback to execution-time
81-
82-
List<CompletedExecution> completedExecution =
83-
buildExecutionInfo(mojoExecutions, propertyEvents);
76+
// Validation-time events MUST exist - no fallback to execution-time
77+
if (result.getValidationTimeEvents() == null || result.getValidationTimeEvents().isEmpty()) {
78+
throw new AssertionError("Validation-time properties not captured - this is a bug");
79+
}
80+
cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());
8481
```
8582

83+
**Note:** Execution-time values are still captured by `MojoParametersListener` for logging/debugging during the build, but are NOT stored in the cache.
84+
8685
### New Timeline (Both Builds)
8786

8887
```
8988
First Build:
9089
1. findCachedBuild() → no cache
9190
2. captureValidationTimeProperties() → reads WITHOUT injection
92-
3. Mojos execute → Maven 4 injects (but we don't use these values)
93-
4. save() uses validation-time properties → stores WITHOUT injection
91+
3. Mojos execute → Maven 4 injects (logged but not stored)
92+
4. save() stores ONLY validation-time properties → WITHOUT injection
9493
9594
Second Build:
9695
1. findCachedBuild() → cache found
@@ -99,6 +98,15 @@ Second Build:
9998
4. Compares to first build → BOTH without injection → MATCH!
10099
```
101100

101+
### What Gets Stored vs. Logged
102+
103+
| Data | Stored in Cache | Available in Logs |
104+
|------|-----------------|-------------------|
105+
| Validation-time properties (no injection) | ✅ Yes | ✅ Yes |
106+
| Execution-time properties (with injection) | ❌ No | ✅ Yes |
107+
108+
This approach ensures cache consistency while preserving debugging information in build logs.
109+
102110
## Benefits Over PR #391
103111

104112
| Aspect | PR #391 (`ignorePattern`) | This PR (Validation-Time Capture) |
@@ -132,11 +140,11 @@ All tests verify:
132140

133141
## Alternative Approaches Considered
134142

135-
1. **Store validation-time values only** ❌ - Would lose auditability of what actually executed
136-
2. **Delay validation until execution** ❌ - Would lose early cache-miss detection performance
137-
3. **Pattern-based filtering (PR #391)** ❌ - Treats symptom, not root cause
138-
4. **Maven core changes** ❌ - Outside our control
139-
5. **Validation-time capture (this PR)** ✅ - Fixes root cause, no configuration needed
143+
1. **Delay validation until execution** ❌ - Would lose early cache-miss detection performance benefit
144+
2. **Pattern-based filtering (PR #391)** ❌ - Treats symptom (value mismatch) instead of root cause (timing mismatch)
145+
3. **Maven core changes** ❌ - Outside our control, would take years to deploy
146+
4. **Store both validation-time and execution-time** ❌ - Unnecessary complexity, doesn't solve the consistency problem
147+
5. **Validation-time capture + storage (this PR)** ✅ - Fixes root cause, no configuration needed, preserves logging for debugging
140148

141149
## Related Issues
142150

0 commit comments

Comments
 (0)