elharo commented on code in PR #389:
URL:
https://github.com/apache/maven-build-cache-extension/pull/389#discussion_r4388376423
##########
src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java:
##########
@@ -253,27 +254,181 @@ public boolean isLogAllProperties(MojoExecution
mojoExecution) {
}
private GoalReconciliation findReconciliationConfig(MojoExecution
mojoExecution) {
- if (cacheConfig.getExecutionControl() == null) {
+ if (mojoExecution == null) {
return null;
}
- final ExecutionControl executionControl =
cacheConfig.getExecutionControl();
- if (executionControl.getReconcile() == null) {
+ final String goal = mojoExecution.getGoal();
+ final Plugin plugin = mojoExecution.getPlugin();
+
+ if (plugin == null) {
return null;
}
- final List<GoalReconciliation> reconciliation =
- executionControl.getReconcile().getPlugins();
+ // First check explicit configuration
+ if (cacheConfig.getExecutionControl() != null
+ && cacheConfig.getExecutionControl().getReconcile() != null) {
+ List<GoalReconciliation> explicitConfigs =
+
cacheConfig.getExecutionControl().getReconcile().getPlugins();
+ for (GoalReconciliation config : explicitConfigs) {
+ if (isPluginMatch(plugin, config) && Strings.CS.equals(goal,
config.getGoal())) {
+ // Validate explicit config against parameter definitions
(with version)
+ validateReconciliationConfig(config, plugin);
+ return config;
+ }
+ }
+ }
- for (GoalReconciliation goalReconciliationConfig : reconciliation) {
- final String goal = mojoExecution.getGoal();
+ // Auto-generate from parameter definitions (track all cache-key
parameters)
+ GoalReconciliation autoGenerated =
generateReconciliationFromParameters(plugin, goal);
+ if (autoGenerated != null) {
+ LOGGER.debug(
+ "Auto-generated reconciliation config for {}:{} with {}
cache-key parameters",
+ plugin.getArtifactId(),
+ goal,
+ autoGenerated.getReconciles() != null
+ ? autoGenerated.getReconciles().size()
+ : 0);
+ }
+ return autoGenerated;
+ }
- if (isPluginMatch(mojoExecution.getPlugin(),
goalReconciliationConfig)
- && Strings.CS.equals(goal,
goalReconciliationConfig.getGoal())) {
- return goalReconciliationConfig;
+ /**
+ * Validates a single reconciliation config against plugin parameter
definitions.
+ * Uses plugin version to load the appropriate parameter definition.
+ */
+ private void validateReconciliationConfig(GoalReconciliation config,
Plugin plugin) {
+ String artifactId = config.getArtifactId();
+ String goal = config.getGoal();
+ String pluginVersion = plugin.getVersion();
+
+ // Load parameter definition for this plugin with version
+ PluginParameterDefinition pluginDef = parameterLoader.load(artifactId,
pluginVersion);
+
+ if (pluginDef == null) {
+ LOGGER.warn(
+ "No parameter definition found for plugin {}:{} version
{}. "
+ + "Cannot validate reconciliation configuration. "
+ + "Consider adding a parameter definition file to
plugin-parameters/{}.xml",
+ artifactId,
+ goal,
+ pluginVersion != null ? pluginVersion : "unknown",
+ artifactId);
+ return;
+ }
+
+ // Get goal definition
+ PluginParameterDefinition.GoalParameterDefinition goalDef =
pluginDef.getGoal(goal);
+ if (goalDef == null) {
+ LOGGER.warn(
+ "Goal '{}' not found in parameter definition for plugin {}
version {}. "
+ + "Cannot validate reconciliation configuration.",
+ goal,
+ artifactId,
+ pluginVersion != null ? pluginVersion : "unknown");
+ return;
+ }
+
+ // Validate each tracked property
+ List<TrackedProperty> properties = config.getReconciles();
+
+ for (TrackedProperty property : properties) {
+ String propertyName = property.getPropertyName();
+
+ if (!goalDef.hasParameter(propertyName)) {
+ LOGGER.warn(
+ "Unknown parameter '{}' in reconciliation config for
{}:{} version {}. "
+ + "This may indicate a plugin version mismatch
or renamed parameter. "
+ + "Consider updating parameter definition or
removing from reconciliation.",
+ propertyName,
+ artifactId,
+ goal,
+ pluginVersion != null ? pluginVersion : "unknown");
+ } else {
+ PluginParameterDefinition.ParameterDefinition paramDef =
goalDef.getParameter(propertyName);
+ if (!paramDef.isCacheKey()) {
+ LOGGER.warn(
+ "Parameter '{}' in reconciliation config for {}:{}
is not marked as a cache key. "
+ + "Changing it will not invalidate the
cache according to the plugin definition.",
+ propertyName,
+ artifactId,
+ goal);
+ }
}
}
- return null;
+ }
+
+ /**
+ * Auto-generates a reconciliation config by tracking all cache-key
parameters
+ * from the plugin parameter definition.
+ * This provides automatic default tracking for any plugin with parameter
definitions.
+ *
+ * @param plugin The plugin to generate config for
+ * @param goal The goal name
+ * @return Auto-generated config tracking all cache-key parameters, or
null if no parameter definition exists
+ */
+ private GoalReconciliation generateReconciliationFromParameters(Plugin
plugin, String goal) {
+ String artifactId = plugin.getArtifactId();
+ String pluginVersion = plugin.getVersion();
+
+ LOGGER.debug(
+ "Attempting to auto-generate reconciliation config for {}:{}
version {}",
+ artifactId,
+ goal,
+ pluginVersion);
+
+ // Load parameter definition for this plugin
+ PluginParameterDefinition pluginDef = parameterLoader.load(artifactId,
pluginVersion);
+ if (pluginDef == null) {
+ LOGGER.debug("No parameter definition found for {}:{}",
artifactId, pluginVersion);
+ return null;
+ }
+
+ // Get goal definition
+ PluginParameterDefinition.GoalParameterDefinition goalDef =
pluginDef.getGoal(goal);
Review Comment:
goalDefinition (avoid abbreviations, I first read this as default)
##########
src/main/java/org/apache/maven/buildcache/xml/PluginParameterDefinition.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.xml;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Represents the complete parameter definition for a Maven plugin loaded from
XML.
+ * Contains all goals and their parameters with cache-key metadata.
+ */
+public class PluginParameterDefinition {
Review Comment:
does this all need to be public?
##########
src/main/java/org/apache/maven/buildcache/xml/PluginParameterLoader.java:
##########
@@ -0,0 +1,278 @@
+/*
+ * 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.xml;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import java.io.InputStream;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import
org.apache.maven.buildcache.xml.PluginParameterDefinition.GoalParameterDefinition;
+import
org.apache.maven.buildcache.xml.PluginParameterDefinition.ParameterDefinition;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.NodeList;
+
+/**
+ * Loads plugin parameter definitions from classpath resources.
+ * Definitions are stored in
src/main/resources/plugin-parameters/{artifactId}.xml
+ */
+public class PluginParameterLoader {
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PluginParameterLoader.class);
+ private static final String PARAMETER_DIR = "plugin-parameters/";
+
+ private final Map<String, PluginParameterDefinition> definitions = new
ConcurrentHashMap<>();
+
+ /**
+ * Load parameter definitions for a plugin by artifact ID only (no version
matching)
+ */
+ public PluginParameterDefinition load(String artifactId) {
+ return load(artifactId, null);
+ }
+
+ /**
+ * Load parameter definitions for a plugin by artifact ID and version.
+ * If version is provided, finds the best matching definition (highest
minVersion <= actual version).
+ * If version is null, returns any definition for the artifactId.
+ */
+ public PluginParameterDefinition load(String artifactId, String
pluginVersion) {
+ String cacheKey = artifactId + (pluginVersion != null ? ":" +
pluginVersion : "");
+
+ if (definitions.containsKey(cacheKey)) {
+ return definitions.get(cacheKey);
+ }
+
+ String resourcePath = PARAMETER_DIR + artifactId + ".xml";
+ InputStream is =
getClass().getClassLoader().getResourceAsStream(resourcePath);
+
+ if (is == null) {
+ LOGGER.debug("No parameter definition found for plugin: {}",
artifactId);
+ return null;
+ }
+
+ try (InputStream inputStream = is) {
+ java.util.List<PluginParameterDefinition> allDefinitions =
parseDefinitions(inputStream, artifactId);
+
+ PluginParameterDefinition bestMatch =
findBestMatch(allDefinitions, pluginVersion);
+
+ if (bestMatch != null) {
+ definitions.put(cacheKey, bestMatch);
+ LOGGER.debug(
+ "Loaded parameter definition for {}:{} (minVersion:
{}): {} goals, {} total parameters",
+ artifactId,
+ pluginVersion != null ? pluginVersion : "any",
+ bestMatch.getMinVersion() != null ?
bestMatch.getMinVersion() : "none",
+ bestMatch.getGoals().size(),
+ bestMatch.getGoals().values().stream()
+ .mapToInt(g -> g.getParameters().size())
+ .sum());
+ }
+
+ return bestMatch;
+ } catch (Exception e) {
+ LOGGER.warn("Failed to load parameter definition for {}: {}",
artifactId, e.getMessage(), e);
+ return null;
+ }
+ }
+
+ /**
+ * Find the best matching definition for a plugin version.
+ * Returns the definition with the highest minVersion that is <=
pluginVersion.
+ * If pluginVersion is null, returns the first definition (or the one
without minVersion).
+ */
+ private PluginParameterDefinition findBestMatch(
+ java.util.List<PluginParameterDefinition> definitions, String
pluginVersion) {
+ if (definitions.isEmpty()) {
+ return null;
+ }
+
+ if (pluginVersion == null) {
+ // No version specified, prefer definition without minVersion,
otherwise return first
+ return definitions.stream()
+ .filter(d -> d.getMinVersion() == null)
+ .findFirst()
+ .orElse(definitions.get(0));
+ }
+
+ // Find highest minVersion that's <= pluginVersion
+ PluginParameterDefinition bestMatch = null;
+ String bestMinVersion = null;
+
+ for (PluginParameterDefinition def : definitions) {
Review Comment:
def --> definition
##########
src/main/java/org/apache/maven/buildcache/xml/PluginParameterLoader.java:
##########
@@ -0,0 +1,278 @@
+/*
+ * 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.xml;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import java.io.InputStream;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import
org.apache.maven.buildcache.xml.PluginParameterDefinition.GoalParameterDefinition;
+import
org.apache.maven.buildcache.xml.PluginParameterDefinition.ParameterDefinition;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.NodeList;
+
+/**
+ * Loads plugin parameter definitions from classpath resources.
+ * Definitions are stored in
src/main/resources/plugin-parameters/{artifactId}.xml
+ */
+public class PluginParameterLoader {
Review Comment:
verify this really has to be public
##########
src/main/java/org/apache/maven/buildcache/xml/CacheConfigImpl.java:
##########
@@ -253,27 +254,181 @@ public boolean isLogAllProperties(MojoExecution
mojoExecution) {
}
private GoalReconciliation findReconciliationConfig(MojoExecution
mojoExecution) {
- if (cacheConfig.getExecutionControl() == null) {
+ if (mojoExecution == null) {
return null;
}
- final ExecutionControl executionControl =
cacheConfig.getExecutionControl();
- if (executionControl.getReconcile() == null) {
+ final String goal = mojoExecution.getGoal();
+ final Plugin plugin = mojoExecution.getPlugin();
+
+ if (plugin == null) {
return null;
}
- final List<GoalReconciliation> reconciliation =
- executionControl.getReconcile().getPlugins();
+ // First check explicit configuration
+ if (cacheConfig.getExecutionControl() != null
+ && cacheConfig.getExecutionControl().getReconcile() != null) {
+ List<GoalReconciliation> explicitConfigs =
+
cacheConfig.getExecutionControl().getReconcile().getPlugins();
+ for (GoalReconciliation config : explicitConfigs) {
+ if (isPluginMatch(plugin, config) && Strings.CS.equals(goal,
config.getGoal())) {
+ // Validate explicit config against parameter definitions
(with version)
+ validateReconciliationConfig(config, plugin);
+ return config;
+ }
+ }
+ }
- for (GoalReconciliation goalReconciliationConfig : reconciliation) {
- final String goal = mojoExecution.getGoal();
+ // Auto-generate from parameter definitions (track all cache-key
parameters)
+ GoalReconciliation autoGenerated =
generateReconciliationFromParameters(plugin, goal);
+ if (autoGenerated != null) {
+ LOGGER.debug(
+ "Auto-generated reconciliation config for {}:{} with {}
cache-key parameters",
+ plugin.getArtifactId(),
+ goal,
+ autoGenerated.getReconciles() != null
+ ? autoGenerated.getReconciles().size()
+ : 0);
+ }
+ return autoGenerated;
+ }
- if (isPluginMatch(mojoExecution.getPlugin(),
goalReconciliationConfig)
- && Strings.CS.equals(goal,
goalReconciliationConfig.getGoal())) {
- return goalReconciliationConfig;
+ /**
+ * Validates a single reconciliation config against plugin parameter
definitions.
+ * Uses plugin version to load the appropriate parameter definition.
+ */
+ private void validateReconciliationConfig(GoalReconciliation config,
Plugin plugin) {
+ String artifactId = config.getArtifactId();
+ String goal = config.getGoal();
+ String pluginVersion = plugin.getVersion();
+
+ // Load parameter definition for this plugin with version
+ PluginParameterDefinition pluginDef = parameterLoader.load(artifactId,
pluginVersion);
+
+ if (pluginDef == null) {
+ LOGGER.warn(
+ "No parameter definition found for plugin {}:{} version
{}. "
+ + "Cannot validate reconciliation configuration. "
+ + "Consider adding a parameter definition file to
plugin-parameters/{}.xml",
+ artifactId,
+ goal,
+ pluginVersion != null ? pluginVersion : "unknown",
+ artifactId);
+ return;
+ }
+
+ // Get goal definition
+ PluginParameterDefinition.GoalParameterDefinition goalDef =
pluginDef.getGoal(goal);
+ if (goalDef == null) {
+ LOGGER.warn(
+ "Goal '{}' not found in parameter definition for plugin {}
version {}. "
+ + "Cannot validate reconciliation configuration.",
+ goal,
+ artifactId,
+ pluginVersion != null ? pluginVersion : "unknown");
+ return;
+ }
+
+ // Validate each tracked property
+ List<TrackedProperty> properties = config.getReconciles();
+
+ for (TrackedProperty property : properties) {
+ String propertyName = property.getPropertyName();
+
+ if (!goalDef.hasParameter(propertyName)) {
+ LOGGER.warn(
+ "Unknown parameter '{}' in reconciliation config for
{}:{} version {}. "
+ + "This may indicate a plugin version mismatch
or renamed parameter. "
+ + "Consider updating parameter definition or
removing from reconciliation.",
+ propertyName,
+ artifactId,
+ goal,
+ pluginVersion != null ? pluginVersion : "unknown");
+ } else {
+ PluginParameterDefinition.ParameterDefinition paramDef =
goalDef.getParameter(propertyName);
+ if (!paramDef.isCacheKey()) {
+ LOGGER.warn(
+ "Parameter '{}' in reconciliation config for {}:{}
is not marked as a cache key. "
+ + "Changing it will not invalidate the
cache according to the plugin definition.",
+ propertyName,
+ artifactId,
+ goal);
+ }
}
}
- return null;
+ }
+
+ /**
+ * Auto-generates a reconciliation config by tracking all cache-key
parameters
+ * from the plugin parameter definition.
+ * This provides automatic default tracking for any plugin with parameter
definitions.
+ *
+ * @param plugin The plugin to generate config for
+ * @param goal The goal name
+ * @return Auto-generated config tracking all cache-key parameters, or
null if no parameter definition exists
+ */
+ private GoalReconciliation generateReconciliationFromParameters(Plugin
plugin, String goal) {
+ String artifactId = plugin.getArtifactId();
+ String pluginVersion = plugin.getVersion();
+
+ LOGGER.debug(
+ "Attempting to auto-generate reconciliation config for {}:{}
version {}",
+ artifactId,
+ goal,
+ pluginVersion);
+
+ // Load parameter definition for this plugin
+ PluginParameterDefinition pluginDef = parameterLoader.load(artifactId,
pluginVersion);
Review Comment:
pluginDefinition
##########
src/main/java/org/apache/maven/buildcache/xml/PluginParameterLoader.java:
##########
@@ -0,0 +1,278 @@
+/*
+ * 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.xml;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+
+import java.io.InputStream;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+import
org.apache.maven.buildcache.xml.PluginParameterDefinition.GoalParameterDefinition;
+import
org.apache.maven.buildcache.xml.PluginParameterDefinition.ParameterDefinition;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.NodeList;
+
+/**
+ * Loads plugin parameter definitions from classpath resources.
+ * Definitions are stored in
src/main/resources/plugin-parameters/{artifactId}.xml
+ */
+public class PluginParameterLoader {
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(PluginParameterLoader.class);
+ private static final String PARAMETER_DIR = "plugin-parameters/";
+
+ private final Map<String, PluginParameterDefinition> definitions = new
ConcurrentHashMap<>();
+
+ /**
+ * Load parameter definitions for a plugin by artifact ID only (no version
matching)
+ */
+ public PluginParameterDefinition load(String artifactId) {
+ return load(artifactId, null);
+ }
+
+ /**
+ * Load parameter definitions for a plugin by artifact ID and version.
+ * If version is provided, finds the best matching definition (highest
minVersion <= actual version).
+ * If version is null, returns any definition for the artifactId.
+ */
+ public PluginParameterDefinition load(String artifactId, String
pluginVersion) {
+ String cacheKey = artifactId + (pluginVersion != null ? ":" +
pluginVersion : "");
+
+ if (definitions.containsKey(cacheKey)) {
+ return definitions.get(cacheKey);
+ }
+
+ String resourcePath = PARAMETER_DIR + artifactId + ".xml";
+ InputStream is =
getClass().getClassLoader().getResourceAsStream(resourcePath);
+
+ if (is == null) {
+ LOGGER.debug("No parameter definition found for plugin: {}",
artifactId);
+ return null;
+ }
+
+ try (InputStream inputStream = is) {
+ java.util.List<PluginParameterDefinition> allDefinitions =
parseDefinitions(inputStream, artifactId);
+
+ PluginParameterDefinition bestMatch =
findBestMatch(allDefinitions, pluginVersion);
+
+ if (bestMatch != null) {
+ definitions.put(cacheKey, bestMatch);
+ LOGGER.debug(
+ "Loaded parameter definition for {}:{} (minVersion:
{}): {} goals, {} total parameters",
+ artifactId,
+ pluginVersion != null ? pluginVersion : "any",
+ bestMatch.getMinVersion() != null ?
bestMatch.getMinVersion() : "none",
+ bestMatch.getGoals().size(),
+ bestMatch.getGoals().values().stream()
+ .mapToInt(g -> g.getParameters().size())
+ .sum());
+ }
+
+ return bestMatch;
+ } catch (Exception e) {
Review Comment:
be more specific about exceptions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
