maven-build-cache-extension icon indicating copy to clipboard operation
maven-build-cache-extension copied to clipboard

Fix #375: Capture plugin properties at validation time to eliminate timing mismatch

Open cowwoc opened this issue 5 months ago • 1 comments

Problem

Maven 4 automatically injects --module-version ${project.version} into compiler arguments during execution, but the build cache validates properties before execution. This creates a timing mismatch that causes cache invalidation:

  • First build: No cache exists → properties captured during/after execution (WITH Maven 4 injection)
  • Second build: Cache exists → properties captured during validation (WITHOUT injection yet) → mismatch → cache invalidated

Root Cause Analysis

The cache extension currently reads plugin properties at different lifecycle points for different builds:

First Build Timeline:
  1. findCachedBuild() → no cache found
  2. Mojos execute → Maven 4 injects --module-version
  3. beforeMojoExecution() fires → captures properties WITH injection
  4. save() stores properties from execution events

Second Build Timeline:
  1. findCachedBuild() → cache found
  2. verifyCacheConsistency() → creates mojo via getConfiguredMojo()
  3. Reads properties → WITHOUT injection (too early in lifecycle)
  4. Compares to first build's stored values (WITH injection)
  5. MISMATCH → cache invalidated!

The problem: validation reads BEFORE injection, storage reads AFTER injection.

Solution

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.

Implementation

  1. Modified CacheResult to store validation-time mojo events
  2. Modified BuildCacheMojosExecutionStrategy to capture properties immediately after findCachedBuild()
  3. Modified save() to store ONLY validation-time events (fails with AssertionError if missing)

Key Code Changes

// In BuildCacheMojosExecutionStrategy.execute():
result = cacheController.findCachedBuild(session, project, mojoExecutions, skipCache);

// NEW: Always capture validation-time properties when cache is enabled
if (cacheState == INITIALIZED) {
    Map<String, MojoExecutionEvent> validationTimeEvents =
        captureValidationTimeProperties(session, project, mojoExecutions);
    result = CacheResult.rebuilded(result, validationTimeEvents);
}
// In save():
// Validation-time events MUST exist - no fallback to execution-time
if (result.getValidationTimeEvents() == null || result.getValidationTimeEvents().isEmpty()) {
    throw new AssertionError("Validation-time properties not captured - this is a bug");
}
cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());

Note: Execution-time values are still captured by MojoParametersListener for logging/debugging during the build, but are NOT stored in the cache.

New Timeline (Both Builds)

First Build:
  1. findCachedBuild() → no cache
  2. captureValidationTimeProperties() → reads WITHOUT injection
  3. Mojos execute → Maven 4 injects (logged but not stored)
  4. save() stores ONLY validation-time properties → WITHOUT injection

Second Build:
  1. findCachedBuild() → cache found
  2. verifyCacheConsistency() → reads WITHOUT injection
  3. captureValidationTimeProperties() → reads WITHOUT injection
  4. Compares to first build → BOTH without injection → MATCH!

What Gets Stored vs. Logged

Data Stored in Cache Available in Logs
Validation-time properties (no injection) ✅ Yes ✅ Yes
Execution-time properties (with injection) ❌ No ✅ Yes

This approach ensures cache consistency while preserving debugging information in build logs.

Benefits Over PR #391

Aspect PR #391 (ignorePattern) This PR (Validation-Time Capture)
Approach Workaround: Filter mismatched values Fix: Eliminate timing mismatch
Configuration Required None needed
Flexibility Pattern must match version format Format-agnostic
Maintenance Patterns need updates No maintenance
Scope Only fixes known patterns Fixes ALL auto-injected properties

Testing

Created comprehensive integration tests:

  1. Maven4JpmsModuleTest - JPMS module with Maven 4 auto-injection of --module-version
  2. ExplicitModuleVersionTest - JPMS module with explicit moduleVersion configuration
  3. NonJpmsProjectTest - Regular Java project without JPMS (ensures no regression)
  4. MultiModuleJpmsTest - Multi-module project with mixed JPMS and non-JPMS modules

All tests verify:

  • ✅ First build creates cache
  • ✅ Second build restores from cache (NO cache invalidation)
  • ✅ NO ignorePattern configuration required

Backward Compatibility

  • Fully backward compatible
  • Existing caches remain valid
  • No configuration changes required
  • ignorePattern still works but is no longer necessary for Maven 4 injection

Alternative Approaches Considered

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

Related Issues

  • Fixes #375
  • Alternative to #391
  • Related to Maven 4 JPMS module compilation

Migration

No migration needed. This change is transparent to users. Existing projects will benefit automatically.

cowwoc avatar Oct 12 '25 07:10 cowwoc

@elharo Done. Try now.

cowwoc avatar Oct 29 '25 18:10 cowwoc

@elharo I've pushed a commit that should fix the latest build failures. Try now.

cowwoc avatar Dec 01 '25 20:12 cowwoc

It looks like the latest build errors are not related to this PR. Can it be merged? Do you need to do something else to fix the build?

cowwoc avatar Dec 03 '25 16:12 cowwoc

Absolutely nothing should be merged on a failing build. A failing build blocks everything.

elharo avatar Dec 03 '25 20:12 elharo

@elharo Agreed, but is anyone taking a look at the failing build?

cowwoc avatar Dec 05 '25 12:12 cowwoc