Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ under the License.
<model>src/main/mdo/build-cache-diff.mdo</model>
<model>src/main/mdo/build-cache-report.mdo</model>
</models>
<version>1.2.0</version>
<version>1.3.0</version>
</configuration>
<executions>
<execution>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@
import javax.inject.Inject;
import javax.inject.Named;

import java.io.File;
import java.nio.file.Path;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.apache.commons.lang3.ArrayUtils;
import org.apache.commons.lang3.Strings;
import org.apache.maven.SessionScoped;
import org.apache.maven.buildcache.artifact.ArtifactRestorationReport;
Expand Down Expand Up @@ -333,7 +331,8 @@ private boolean verifyCacheConsistency(
final CompletedExecution completedExecution = cachedBuild.findMojoExecutionInfo(cacheCandidate);
final String fullGoalName = cacheCandidate.getMojoDescriptor().getFullGoalName();

if (completedExecution != null && !isParamsMatched(project, cacheCandidate, mojo, completedExecution)) {
if (completedExecution != null
&& !isParamsMatched(project, session, cacheCandidate, mojo, completedExecution)) {
LOGGER.info(
"Mojo cached parameters mismatch with actual, forcing full project build. Mojo: {}",
fullGoalName);
Expand Down Expand Up @@ -367,7 +366,11 @@ private boolean verifyCacheConsistency(
}

boolean isParamsMatched(
MavenProject project, MojoExecution mojoExecution, Mojo mojo, CompletedExecution completedExecution) {
MavenProject project,
MavenSession session,
MojoExecution mojoExecution,
Mojo mojo,
CompletedExecution completedExecution) {
List<TrackedProperty> tracked = cacheConfig.getTrackedProperties(mojoExecution);

for (TrackedProperty trackedProperty : tracked) {
Expand All @@ -380,20 +383,14 @@ boolean isParamsMatched(

final String currentValue;
try {
Object value = ReflectionUtils.getValueIncludingSuperclasses(propertyName, mojo);

if (value instanceof File) {
Path baseDirPath = project.getBasedir().toPath();
Path path = ((File) value).toPath();
currentValue = normalizedPath(path, baseDirPath);
} else if (value instanceof Path) {
Path baseDirPath = project.getBasedir().toPath();
currentValue = normalizedPath(((Path) value), baseDirPath);
} else if (value != null && value.getClass().isArray()) {
currentValue = ArrayUtils.toString(value);
Object value;
if (trackedProperty.getExpression() != null) {
value = DtoUtils.interpolateExpression(trackedProperty.getExpression(), session, mojoExecution);
} else {
currentValue = String.valueOf(value);
value = ReflectionUtils.getValueIncludingSuperclasses(propertyName, mojo);
}
Path baseDirPath = project.getBasedir().toPath();
currentValue = DtoUtils.normalizeValue(value, baseDirPath);
} catch (IllegalAccessException e) {
LOGGER.error("Cannot extract plugin property {} from mojo {}", propertyName, mojo, e);
return false;
Expand All @@ -419,33 +416,6 @@ boolean isParamsMatched(
return true;
}

/**
* Best effort to normalize paths from Mojo fields.
* - all absolute paths under project root to be relativized for portability
* - redundant '..' and '.' to be removed to have consistent views on all paths
* - all relative paths are considered portable and should not be touched
* - absolute paths outside of project directory could not be deterministically
* relativized and not touched
*/
private static String normalizedPath(Path path, Path baseDirPath) {
boolean isProjectSubdir = path.isAbsolute() && path.startsWith(baseDirPath);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(
"normalizedPath isProjectSubdir {} path '{}' - baseDirPath '{}', path.isAbsolute() {}, path.startsWith(baseDirPath) {}",
isProjectSubdir,
path,
baseDirPath,
path.isAbsolute(),
path.startsWith(baseDirPath));
}
Path preparedPath = isProjectSubdir ? baseDirPath.relativize(path) : path;
String normalizedPath = preparedPath.normalize().toString();
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("normalizedPath '{}' - {} return {}", path, baseDirPath, normalizedPath);
}
return normalizedPath;
}

private enum CacheRestorationStatus {
SUCCESS,
FAILURE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ private void recordMojoProperties(CompletedExecution execution, MojoExecutionEve
final Object mojo = executionEvent.getMojo();

final File baseDir = executionEvent.getProject().getBasedir();
final String baseDirPath = FilenameUtils.normalizeNoEndSeparator(baseDir.getAbsolutePath()) + File.separator;
final Path baseDirPath = baseDir.toPath();

final List<Parameter> parameters = mojoExecution.getMojoDescriptor().getParameters();
for (Parameter parameter : parameters) {
Expand Down Expand Up @@ -730,6 +730,19 @@ private void recordMojoProperties(CompletedExecution execution, MojoExecutionEve
}
}
}
// add properties with expressions
for (TrackedProperty trackedProperty : trackedProperties) {
if (trackedProperty.getExpression() != null) {
String propertyName = trackedProperty.getPropertyName();
if (!isExcluded(propertyName, logAll, noLogProperties, forceLogProperties)) {
Object value = DtoUtils.interpolateExpression(
trackedProperty.getExpression(),
executionEvent.getSession(),
executionEvent.getExecution());
DtoUtils.addProperty(execution, propertyName, value, baseDirPath, true);
}
}
}
}

private static Method getGetter(String fieldName, Class<?> clazz) {
Expand Down
75 changes: 70 additions & 5 deletions src/main/java/org/apache/maven/buildcache/xml/DtoUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@

import javax.annotation.Nonnull;

import java.io.File;
import java.nio.file.FileSystems;
import java.nio.file.Path;
import java.util.List;

import org.apache.commons.lang3.ArrayUtils;
Expand All @@ -30,7 +33,10 @@
import org.apache.maven.buildcache.xml.build.DigestItem;
import org.apache.maven.buildcache.xml.build.PropertyValue;
import org.apache.maven.buildcache.xml.config.TrackedProperty;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.model.Dependency;
import org.apache.maven.plugin.MojoExecution;
import org.apache.maven.plugin.PluginParameterExpressionEvaluator;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -108,15 +114,21 @@ public static Dependency createDependency(Artifact artifact) {
return dependency;
}

/**
* @deprecated use {@link #addProperty(CompletedExecution, String, Object, Path, boolean)}
*/
@Deprecated
public static void addProperty(
CompletedExecution execution, String propertyName, Object value, String baseDirPath, boolean tracked) {
addProperty(execution, propertyName, value, FileSystems.getDefault().getPath(baseDirPath), tracked);
}

public static void addProperty(
CompletedExecution execution, String propertyName, Object value, Path baseDirPath, boolean tracked) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. Keep the old method as a delegate to the new method and deprecate it with reference to use this method

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done that, because technically it is true, however all usages all the method have been replaced. Who are potential users of the old method?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done that, because technically it is true, however all usages all the method have been replaced. Who are potential users of the old method?

https://www.hyrumslaw.com/ 🙈

final PropertyValue valueType = new PropertyValue();
valueType.setName(propertyName);
if (value != null && value.getClass().isArray()) {
value = ArrayUtils.toString(value);
}
final String valueText = String.valueOf(value);
valueType.setValue(Strings.CS.remove(valueText, baseDirPath));
final String valueText = normalizeValue(value, baseDirPath);
valueType.setValue(valueText);
valueType.setTracked(tracked);
execution.addProperty(valueType);
}
Expand Down Expand Up @@ -173,4 +185,57 @@ public static Artifact copy(Artifact artifact) {
copy.setFileSize(artifact.getFileSize());
return copy;
}

public static String normalizeValue(Object value, Path baseDirPath) {
final String currentValue;
if (value instanceof File) {
Path path = ((File) value).toPath();
currentValue = normalizedPath(path, baseDirPath);
} else if (value instanceof Path) {
currentValue = normalizedPath(((Path) value), baseDirPath);
} else if (value != null && value.getClass().isArray()) {
currentValue = ArrayUtils.toString(value);
} else {
currentValue = String.valueOf(value);
}
return currentValue;
}

/**
* Best effort to normalize paths from Mojo fields.
* - all absolute paths under project root to be relativized for portability
* - redundant '..' and '.' to be removed to have consistent views on all paths
* - all relative paths are considered portable and should not be touched
* - absolute paths outside of project directory could not be deterministically
* relativized and not touched
*/
private static String normalizedPath(Path path, Path baseDirPath) {
boolean isProjectSubdir = path.isAbsolute() && path.startsWith(baseDirPath);
if (LOGGER.isDebugEnabled()) {
LOGGER.debug(
"normalizedPath isProjectSubdir {} path '{}' - baseDirPath '{}', path.isAbsolute() {},"
+ " path.startsWith(baseDirPath) {}",
isProjectSubdir,
path,
baseDirPath,
path.isAbsolute(),
path.startsWith(baseDirPath));
}
Path preparedPath = isProjectSubdir ? baseDirPath.relativize(path) : path;
String normalizedPath = preparedPath.normalize().toString();
if (LOGGER.isDebugEnabled()) {
LOGGER.debug("normalizedPath '{}' - {} return {}", path, baseDirPath, normalizedPath);
}
return normalizedPath;
}

public static Object interpolateExpression(String expression, MavenSession session, MojoExecution execution) {
try {
PluginParameterExpressionEvaluator evaluator = new PluginParameterExpressionEvaluator(session, execution);
return evaluator.evaluate(expression);
} catch (Exception e) {
LOGGER.warn("Cannot interpolate expression '{}': {}", expression, e.getMessage(), e);
return expression; // return the expression as is if interpolation fails
}
}
}
13 changes: 13 additions & 0 deletions src/main/mdo/build-cache-config.mdo
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,13 @@ under the License.
</xs:documentation>
</xs:annotation>
</xs:attribute>
<xs:attribute name="expression" type="xs:string">
<xs:annotation>
<xs:documentation>
Interpolated expression to use as cached value.
</xs:documentation>
</xs:annotation>
</xs:attribute>
</xs:extension>
</xs:simpleContent>
</xs:complexType>
Expand All @@ -1465,6 +1472,11 @@ under the License.
<name>defaultValue</name>
<type>String</type>
</field>
<field xml.attribute="true">
<name>expression</name>
<type>String</type>
<description>Interpolated expression to use as cached value.</description>
</field>
</fields>
</class>
</classes>
Expand Down Expand Up @@ -1497,3 +1509,4 @@ under the License.
-->

</model>

15 changes: 8 additions & 7 deletions src/site/resources/maven-build-cache-config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
See the License for the specific language governing permissions and
limitations under the License.
-->
<cache xmlns="http://maven.apache.org/BUILD-CACHE-CONFIG/1.2.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/BUILD-CACHE-CONFIG/1.2.0 https://maven.apache.org/xsd/build-cache-config-1.2.0.xsd">
<cache xmlns="http://maven.apache.org/BUILD-CACHE-CONFIG/1.3.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/BUILD-CACHE-CONFIG/1.3.0 https://maven.apache.org/xsd/build-cache-config-1.3.0.xsd">

<!--
Template Maven build cache configuration
Expand Down Expand Up @@ -105,11 +105,6 @@
<goal>install</goal>
</goals>
</goalsList>
<goalsList artifactId="maven-deploy-plugin">
<goals>
<goal>deploy</goal>
</goals>
</goalsList>
<goalsList artifactId="bb-sdk-codegen">
<goals>
<goal>deploy-local</goal>
Expand Down Expand Up @@ -149,6 +144,12 @@
<nolog propertyName="redundantProperty2"/>
</nologs>
</plugin>
<plugin artifactId="maven-deploy-plugin" goal="deploy">
<reconciles>
<!-- Deploy cached artifact when project version changes -->
<reconcile propertyName="project.version" expression="${project.version}"/>
</reconciles>
</plugin>
</plugins>
</reconcile>
</executionControl>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.apache.maven.buildcache.xml.build.CompletedExecution;
import org.apache.maven.buildcache.xml.build.PropertyValue;
import org.apache.maven.buildcache.xml.config.TrackedProperty;
import org.apache.maven.execution.MavenSession;
import org.apache.maven.execution.scope.internal.MojoExecutionScope;
import org.apache.maven.plugin.MavenPluginManager;
import org.apache.maven.plugin.MojoExecution;
Expand All @@ -54,6 +55,7 @@ class ParametersMatchingTest {
private MojoExecution executionMock;
private CompletedExecution cacheRecordMock;
private CacheConfig cacheConfigMock;
private MavenSession sessionMock;

@BeforeEach
void setUp() {
Expand All @@ -69,6 +71,7 @@ void setUp() {
projectMock = mock(MavenProject.class);
executionMock = mock(MojoExecution.class);
cacheRecordMock = mock(CompletedExecution.class);
sessionMock = mock(MavenSession.class);
}

@Test
Expand Down Expand Up @@ -106,7 +109,7 @@ void testBasicParamsMatching() {
Arrays.asList("a", "b", "c"),
new String[] {"c", "d", "e"});

assertTrue(strategy.isParamsMatched(projectMock, executionMock, testMojo, cacheRecordMock));
assertTrue(strategy.isParamsMatched(projectMock, sessionMock, executionMock, testMojo, cacheRecordMock));
}

@Test
Expand All @@ -133,7 +136,7 @@ void testSkipValue() {
testMojo.setAnyObject("true");

assertTrue(
strategy.isParamsMatched(projectMock, executionMock, testMojo, cacheRecordMock),
strategy.isParamsMatched(projectMock, sessionMock, executionMock, testMojo, cacheRecordMock),
"If property set to 'skipValue' mismatch could be ignored because cached build"
+ " is more complete than requested build");
}
Expand Down Expand Up @@ -162,7 +165,7 @@ void testDefaultValue() {
testMojo.setAnyObject("defaultValue");

assertTrue(
strategy.isParamsMatched(projectMock, executionMock, testMojo, cacheRecordMock),
strategy.isParamsMatched(projectMock, sessionMock, executionMock, testMojo, cacheRecordMock),
"If property has defaultValue it must be matched even if cache record has no this field");
}

Expand All @@ -188,7 +191,7 @@ void testMismatch() {
TestMojo testMojo = new TestMojo();
testMojo.setAnyObject("2");

assertFalse(strategy.isParamsMatched(projectMock, executionMock, testMojo, cacheRecordMock));
assertFalse(strategy.isParamsMatched(projectMock, sessionMock, executionMock, testMojo, cacheRecordMock));
}

@NotNull
Expand Down
Loading