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

[cowwoc]

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 Add ignorePattern attribute to TrackedProperty for array filtering #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 Maven Build Cache Extension incorrectly handles --module-version compiler arguments for Java modules, causing cache invalidation on every build despite no source code changes #375
• Alternative to Add ignorePattern attribute to TrackedProperty for array filtering #391
• Related to Maven 4 JPMS module compilation

Migration

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

[elharo]

need to run mvn spotless:apply

[cowwoc]

@elharo Done. Try now.

[cowwoc]

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

[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?

[elharo]

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

[cowwoc]

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

[cowwoc]

@elharo @olamy I've rebased the PR. Please check if the build passes now.

If it does, please consider merging since the PR is already marked as approved.

[cowwoc]

This PR is approved but unmerged. Can someone please merge it?