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

src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java

@@ -131,6 +131,19 @@ public void execute(
                 }
                 if (cacheState == INITIALIZED || skipCache) {
                     result = cacheController.findCachedBuild(session, project, mojoExecutions, skipCache);
+
+                    // Capture validation-time properties for all mojos to ensure consistent property reading
+                    // at the same lifecycle point for all builds (eliminates Maven 4 injection timing issues)
+                    // Always capture when cacheState is INITIALIZED since we may need to save
+                    if (cacheState == INITIALIZED) {
+                        Map<String, MojoExecutionEvent> validationTimeEvents =
+                                captureValidationTimeProperties(session, project, mojoExecutions);
+                        result = CacheResult.rebuilded(result, validationTimeEvents);
+                        LOGGER.debug(
+                                "Captured validation-time properties for {} mojos in project {}",
+                                validationTimeEvents.size(),
+                                projectName);
+                    }
                 }
             } else {
                 LOGGER.info("Cache is disabled on project level for {}", projectName);
@@ -163,8 +176,16 @@ public void execute(
                                 .isEmpty()) {
                     LOGGER.info("Cache storing is skipped since there was no \"clean\" phase.");
                 } else {
-                    final Map<String, MojoExecutionEvent> executionEvents = mojoListener.getProjectExecutions(project);
-                    cacheController.save(result, mojoExecutions, executionEvents);
+                    // Validation-time events must exist for cache storage
+                    // If they don't exist, this indicates a bug in the capture logic
+                    if (result.getValidationTimeEvents() == null || result.getValidationTimeEvents().isEmpty()) {
+                        throw new AssertionError(
+                                "Validation-time properties not captured for project " + projectName
+                                        + ". This is a bug - validation-time capture should always succeed when saving to cache.");
+                    }
+                    LOGGER.debug(
+                            "Using validation-time properties for cache storage (consistent lifecycle point)");
+                    cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());
                 }
             }
 
@@ -434,6 +455,63 @@ private static String normalizedPath(Path path, Path baseDirPath) {
         return normalizedPath;
     }
 
+    /**
+     * Captures plugin properties at validation time for all mojo executions.
+     * This ensures properties are read at the same lifecycle point for all builds,
+     * eliminating timing mismatches caused by Maven 4's auto-injection of properties
+     * like --module-version during execution.
+     *
+     * @param session Maven session
+     * @param project Current project
+     * @param mojoExecutions List of mojo executions to capture properties for
+     * @return Map of execution key to MojoExecutionEvent captured at validation time
+     */
+    private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
+            MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions) {
+        Map<String, MojoExecutionEvent> validationTimeEvents = new java.util.HashMap<>();
+
+        for (MojoExecution mojoExecution : mojoExecutions) {
+            // Skip mojos that don't execute or are in clean phase
+            if (mojoExecution.getLifecyclePhase() == null
+                    || !lifecyclePhasesHelper.isLaterPhaseThanClean(mojoExecution.getLifecyclePhase())) {
+                continue;
+            }
+
+            Mojo mojo = null;
+            try {
+                mojoExecutionScope.enter();
+                mojoExecutionScope.seed(MavenProject.class, project);
+                mojoExecutionScope.seed(MojoExecution.class, mojoExecution);
+
+                mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
+                MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
+                validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
+
+                LOGGER.debug(
+                        "Captured validation-time properties for {}",
+                        mojoExecution.getMojoDescriptor().getFullGoalName());
+
+            } catch (PluginConfigurationException | PluginContainerException e) {
+                LOGGER.warn(
+                        "Cannot capture validation-time properties for {}: {}",
+                        mojoExecution.getMojoDescriptor().getFullGoalName(),
+                        e.getMessage());
+            } finally {
+                try {
+                    mojoExecutionScope.exit();
+                } catch (MojoExecutionException e) {
+                    LOGGER.debug("Error exiting mojo execution scope: {}", e.getMessage());
+                }
+                if (mojo != null) {
+                    mavenPluginManager.releaseMojo(mojo, mojoExecution);
+                }
+            }
+        }
+
+        LOGGER.debug("Captured validation-time properties for {} mojos", validationTimeEvents.size());
+        return validationTimeEvents;
+    }
+
     private enum CacheRestorationStatus {
         SUCCESS,
         FAILURE,

src/main/java/org/apache/maven/buildcache/CacheResult.java

@@ -18,8 +18,11 @@
  */
 package org.apache.maven.buildcache;
 
+import java.util.Map;
+
 import org.apache.maven.buildcache.xml.Build;
 import org.apache.maven.buildcache.xml.CacheSource;
+import org.apache.maven.execution.MojoExecutionEvent;
 
 import static java.util.Objects.requireNonNull;
 
@@ -31,49 +34,84 @@ public class CacheResult {
     private final RestoreStatus status;
     private final Build build;
     private final CacheContext context;
+    private final Map<String, MojoExecutionEvent> validationTimeEvents;
 
-    private CacheResult(RestoreStatus status, Build build, CacheContext context) {
+    private CacheResult(RestoreStatus status, Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
         this.status = requireNonNull(status);
         this.build = build;
         this.context = context;
+        this.validationTimeEvents = validationTimeEvents;
     }
 
     public static CacheResult empty(CacheContext context) {
         requireNonNull(context);
-        return new CacheResult(RestoreStatus.EMPTY, null, context);
+        return new CacheResult(RestoreStatus.EMPTY, null, context, null);
+    }
+
+    public static CacheResult empty(CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+        requireNonNull(context);
+        return new CacheResult(RestoreStatus.EMPTY, null, context, validationTimeEvents);
     }
 
     public static CacheResult empty() {
-        return new CacheResult(RestoreStatus.EMPTY, null, null);
+        return new CacheResult(RestoreStatus.EMPTY, null, null, null);
     }
 
     public static CacheResult failure(Build build, CacheContext context) {
         requireNonNull(build);
         requireNonNull(context);
-        return new CacheResult(RestoreStatus.FAILURE, build, context);
+        return new CacheResult(RestoreStatus.FAILURE, build, context, null);
+    }
+
+    public static CacheResult failure(Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+        requireNonNull(build);
+        requireNonNull(context);
+        return new CacheResult(RestoreStatus.FAILURE, build, context, validationTimeEvents);
     }
 
     public static CacheResult success(Build build, CacheContext context) {
         requireNonNull(build);
         requireNonNull(context);
-        return new CacheResult(RestoreStatus.SUCCESS, build, context);
+        return new CacheResult(RestoreStatus.SUCCESS, build, context, null);
+    }
+
+    public static CacheResult success(Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+        requireNonNull(build);
+        requireNonNull(context);
+        return new CacheResult(RestoreStatus.SUCCESS, build, context, validationTimeEvents);
     }
 
     public static CacheResult partialSuccess(Build build, CacheContext context) {
         requireNonNull(build);
         requireNonNull(context);
-        return new CacheResult(RestoreStatus.PARTIAL, build, context);
+        return new CacheResult(RestoreStatus.PARTIAL, build, context, null);
+    }
+
+    public static CacheResult partialSuccess(Build build, CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+        requireNonNull(build);
+        requireNonNull(context);
+        return new CacheResult(RestoreStatus.PARTIAL, build, context, validationTimeEvents);
     }
 
     public static CacheResult failure(CacheContext context) {
         requireNonNull(context);
-        return new CacheResult(RestoreStatus.FAILURE, null, context);
+        return new CacheResult(RestoreStatus.FAILURE, null, context, null);
+    }
+
+    public static CacheResult failure(CacheContext context, Map<String, MojoExecutionEvent> validationTimeEvents) {
+        requireNonNull(context);
+        return new CacheResult(RestoreStatus.FAILURE, null, context, validationTimeEvents);
     }
 
     public static CacheResult rebuilded(CacheResult orig, Build build) {
         requireNonNull(orig);
         requireNonNull(build);
-        return new CacheResult(orig.status, build, orig.context);
+        return new CacheResult(orig.status, build, orig.context, orig.validationTimeEvents);
+    }
+
+    public static CacheResult rebuilded(CacheResult orig, Map<String, MojoExecutionEvent> validationTimeEvents) {
+        requireNonNull(orig);
+        return new CacheResult(orig.status, orig.build, orig.context, validationTimeEvents);
     }
 
     public boolean isSuccess() {
@@ -103,4 +141,8 @@ public RestoreStatus getStatus() {
     public boolean isFinal() {
         return build != null && build.getDto().is_final();
     }
+
+    public Map<String, MojoExecutionEvent> getValidationTimeEvents() {
+        return validationTimeEvents;
+    }
 }

src/test/java/org/apache/maven/buildcache/its/ExplicitModuleVersionTest.java

@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.buildcache.its;
+
+import org.apache.maven.buildcache.its.junit.IntegrationTest;
+import org.apache.maven.it.VerificationException;
+import org.apache.maven.it.Verifier;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Integration test for JPMS module compilation with explicit moduleVersion configuration.
+ *
+ * <p>This test verifies that the validation-time property capture approach works correctly
+ * when the moduleVersion is explicitly configured in the POM. Unlike Maven 4's auto-injection
+ * scenario, this configuration is present at validation time, so there's no timing mismatch.
+ * However, validation-time capture should still work correctly.
+ *
+ * <p>This test verifies:
+ * <ol>
+ *   <li>First build creates cache entry with validation-time properties</li>
+ *   <li>Second build restores from cache successfully</li>
+ *   <li>Explicit configuration is captured correctly at validation time</li>
+ * </ol>
+ */
+@IntegrationTest("src/test/projects/explicit-module-version")
+class ExplicitModuleVersionTest {
+
+    /**
+     * Verifies that JPMS module compilation with explicit moduleVersion works with cache restoration.
+     * This tests that validation-time capture works correctly when moduleVersion is explicitly
+     * configured in the POM (no Maven 4 auto-injection needed).
+     *
+     * @param verifier Maven verifier for running builds
+     * @throws VerificationException if verification fails
+     */
+    @Test
+    void testExplicitModuleVersionCacheRestoration(Verifier verifier) throws VerificationException {
+        verifier.setAutoclean(false);
+
+        // First build - should create cache entry with validation-time properties
+        verifier.setLogFileName("../log-build-1.txt");
+        verifier.executeGoal("clean");
+        verifier.executeGoal("compile");
+        verifier.verifyErrorFreeLog();
+
+        // Verify compilation succeeded
+        verifier.verifyFilePresent("target/classes/module-info.class");
+        verifier.verifyFilePresent(
+                "target/classes/org/apache/maven/caching/test/explicit/ExplicitVersionModule.class");
+
+        // Second build - should restore from cache
+        verifier.setLogFileName("../log-build-2.txt");
+        verifier.executeGoal("clean");
+        verifier.executeGoal("compile");
+        verifier.verifyErrorFreeLog();
+
+        // Verify cache was used (not rebuilt)
+        verifier.verifyTextInLog(
+                "Found cached build, restoring org.apache.maven.caching.test.explicit:explicit-module-version from cache");
+
+        // Verify compilation was skipped (restored from cache)
+        verifier.verifyTextInLog("Skipping plugin execution (cached): compiler:compile");
+
+        // Verify output files were restored from cache
+        verifier.verifyFilePresent("target/classes/module-info.class");
+        verifier.verifyFilePresent(
+                "target/classes/org/apache/maven/caching/test/explicit/ExplicitVersionModule.class");
+    }
+}

src/test/java/org/apache/maven/buildcache/its/Maven4JpmsModuleTest.java

@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.buildcache.its;
+
+import org.apache.maven.buildcache.its.junit.IntegrationTest;
+import org.apache.maven.it.VerificationException;
+import org.apache.maven.it.Verifier;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Integration test for Maven 4 JPMS module compilation with automatic --module-version injection.
+ *
+ * <p>This test verifies that issue #375 is fixed by the validation-time property capture approach.
+ * Maven 4 automatically injects {@code --module-version ${project.version}} into compiler arguments
+ * during execution. Without the fix, this creates a timing mismatch:
+ * <ul>
+ *   <li>First build: Properties captured during execution (WITH injection)</li>
+ *   <li>Second build: Properties captured during validation (WITHOUT injection yet)</li>
+ *   <li>Result: Cache invalidation due to parameter mismatch</li>
+ * </ul>
+ *
+ * <p>The fix captures properties at validation time for ALL builds, ensuring consistent reading
+ * at the same lifecycle point. This test verifies:
+ * <ol>
+ *   <li>First build creates cache entry</li>
+ *   <li>Second build restores from cache (NO cache invalidation)</li>
+ *   <li>NO {@code ignorePattern} configuration required</li>
+ * </ol>
+ */
+@IntegrationTest("src/test/projects/maven4-jpms-module")
+class Maven4JpmsModuleTest {
+
+    /**
+     * Verifies that Maven 4 JPMS module compilation works with cache restoration.
+     * Maven 4 auto-injects {@code --module-version} during compilation, but the
+     * validation-time capture approach ensures this doesn't cause cache invalidation.
+     *
+     * @param verifier Maven verifier for running builds
+     * @throws VerificationException if verification fails
+     */
+    @Test
+    void testMaven4JpmsModuleCacheRestoration(Verifier verifier) throws VerificationException {
+        verifier.setAutoclean(false);
+
+        // First build - should create cache entry with validation-time properties
+        verifier.setLogFileName("../log-build-1.txt");
+        verifier.executeGoal("clean");
+        verifier.executeGoal("compile");
+        verifier.verifyErrorFreeLog();
+
+        // Verify compilation succeeded
+        verifier.verifyFilePresent("target/classes/module-info.class");
+        verifier.verifyFilePresent(
+                "target/classes/org/apache/maven/caching/test/maven4/HelloMaven4.class");
+
+        // Second build - should restore from cache WITHOUT invalidation
+        // This is the critical test: validation-time properties should match stored properties
+        verifier.setLogFileName("../log-build-2.txt");
+        verifier.executeGoal("clean");
+        verifier.executeGoal("compile");
+        verifier.verifyErrorFreeLog();
+
+        // Verify cache was used (not rebuilt) - this proves the fix works!
+        verifier.verifyTextInLog(
+                "Found cached build, restoring org.apache.maven.caching.test.maven4:maven4-jpms-module from cache");
+
+        // Verify compilation was skipped (restored from cache)
+        verifier.verifyTextInLog("Skipping plugin execution (cached): compiler:compile");
+
+        // Verify output files were restored from cache
+        verifier.verifyFilePresent("target/classes/module-info.class");
+        verifier.verifyFilePresent(
+                "target/classes/org/apache/maven/caching/test/maven4/HelloMaven4.class");
+    }
+}