Skip to content

Commit fda621a

Browse files
committed
Address PR #395 review feedback
- Split run-on sentence in AssertionError message - Use HashMap import instead of fully qualified name - Move Mojo variable declaration into try block (nested try-finally) - Return defensive copy from getValidationTimeEvents() for encapsulation
1 parent 6830324 commit fda621a

File tree

2 files changed

+15
-13
lines changed

2 files changed

+15
-13
lines changed

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
import java.io.File;
2626
import java.nio.file.Path;
27+
import java.util.HashMap;
2728
import java.util.HashSet;
2829
import java.util.List;
2930
import java.util.Map;
@@ -182,7 +183,7 @@ public void execute(
182183
|| result.getValidationTimeEvents().isEmpty()) {
183184
throw new AssertionError(
184185
"Validation-time properties not captured for project " + projectName
185-
+ ". This is a bug - validation-time capture should always succeed when saving to cache.");
186+
+ ". This is a bug. Validation-time capture should always succeed when saving to cache.");
186187
}
187188
LOGGER.debug("Using validation-time properties for cache storage (consistent lifecycle point)");
188189
cacheController.save(result, mojoExecutions, result.getValidationTimeEvents());
@@ -468,7 +469,7 @@ private static String normalizedPath(Path path, Path baseDirPath) {
468469
*/
469470
private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
470471
MavenSession session, MavenProject project, List<MojoExecution> mojoExecutions) {
471-
Map<String, MojoExecutionEvent> validationTimeEvents = new java.util.HashMap<>();
472+
Map<String, MojoExecutionEvent> validationTimeEvents = new HashMap<>();
472473

473474
for (MojoExecution mojoExecution : mojoExecutions) {
474475
// Skip mojos that don't execute or are in clean phase
@@ -477,19 +478,22 @@ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
477478
continue;
478479
}
479480

480-
Mojo mojo = null;
481481
try {
482482
mojoExecutionScope.enter();
483483
mojoExecutionScope.seed(MavenProject.class, project);
484484
mojoExecutionScope.seed(MojoExecution.class, mojoExecution);
485485

486-
mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
487-
MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
488-
validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
486+
Mojo mojo = mavenPluginManager.getConfiguredMojo(Mojo.class, session, mojoExecution);
487+
try {
488+
MojoExecutionEvent event = new MojoExecutionEvent(session, project, mojoExecution, mojo);
489+
validationTimeEvents.put(mojoExecutionKey(mojoExecution), event);
489490

490-
LOGGER.debug(
491-
"Captured validation-time properties for {}",
492-
mojoExecution.getMojoDescriptor().getFullGoalName());
491+
LOGGER.debug(
492+
"Captured validation-time properties for {}",
493+
mojoExecution.getMojoDescriptor().getFullGoalName());
494+
} finally {
495+
mavenPluginManager.releaseMojo(mojo, mojoExecution);
496+
}
493497

494498
} catch (PluginConfigurationException | PluginContainerException e) {
495499
LOGGER.warn(
@@ -502,9 +506,6 @@ private Map<String, MojoExecutionEvent> captureValidationTimeProperties(
502506
} catch (MojoExecutionException e) {
503507
LOGGER.debug("Error exiting mojo execution scope: {}", e.getMessage());
504508
}
505-
if (mojo != null) {
506-
mavenPluginManager.releaseMojo(mojo, mojoExecution);
507-
}
508509
}
509510
}
510511

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package org.apache.maven.buildcache;
2020

21+
import java.util.HashMap;
2122
import java.util.Map;
2223

2324
import org.apache.maven.buildcache.xml.Build;
@@ -150,6 +151,6 @@ public boolean isFinal() {
150151
}
151152

152153
public Map<String, MojoExecutionEvent> getValidationTimeEvents() {
153-
return validationTimeEvents;
154+
return validationTimeEvents != null ? new HashMap<>(validationTimeEvents) : null;
154155
}
155156
}

0 commit comments

Comments
 (0)