This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch backport-12083-to-4.0.x
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/backport-12083-to-4.0.x by 
this push:
     new b0238fa2b0 Extract shared session infrastructure and improve property 
collection
b0238fa2b0 is described below

commit b0238fa2b00d2e2c0e51a7ed032bf6222636bf31
Author: Guillaume Nodet <[email protected]>
AuthorDate: Tue May 26 15:58:41 2026 +0200

    Extract shared session infrastructure and improve property collection
    
    Move Maven session management (getSession, createMaven4Session,
    TransporterFactoryConfig, temp directory helpers, buildEffectiveModel)
    from PluginUpgradeStrategy into AbstractUpgradeStrategy to enable
    reuse across strategies.
    
    Simplify CompatibilityFixStrategy's collectAllDefinedProperties to scan
    all pomMap entries via DOM, including profile properties. Add tests for
    property inheritance across parent/grandparent POM hierarchies and
    partial undefined property detection.
    
    Co-Authored-By: Claude Opus 4.6 <[email protected]>
---
 .../mvnup/goals/AbstractUpgradeStrategy.java       | 136 ++++++++++
 .../mvnup/goals/CompatibilityFixStrategy.java      |  31 ++-
 .../invoker/mvnup/goals/PluginUpgradeStrategy.java | 201 +--------------
 .../mvnup/goals/CompatibilityFixStrategyTest.java  | 273 +++++++++++++++++++++
 4 files changed, 426 insertions(+), 215 deletions(-)

diff --git 
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeStrategy.java
 
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeStrategy.java
index 1b2cc1de72..3828e3f03a 100644
--- 
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeStrategy.java
+++ 
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/AbstractUpgradeStrategy.java
@@ -18,9 +18,13 @@
  */
 package org.apache.maven.cling.invoker.mvnup.goals;
 
+import java.io.File;
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -28,8 +32,27 @@
 import eu.maveniverse.domtrip.Element;
 import eu.maveniverse.domtrip.maven.Coordinates;
 import eu.maveniverse.domtrip.maven.MavenPomElements;
+import org.apache.maven.api.RemoteRepository;
+import org.apache.maven.api.Session;
 import org.apache.maven.api.cli.mvnup.UpgradeOptions;
+import org.apache.maven.api.di.Named;
+import org.apache.maven.api.di.Provides;
+import org.apache.maven.api.model.Repository;
+import org.apache.maven.api.model.RepositoryPolicy;
+import org.apache.maven.api.services.ModelBuilder;
+import org.apache.maven.api.services.ModelBuilderRequest;
+import org.apache.maven.api.services.ModelBuilderResult;
+import org.apache.maven.api.services.RepositoryFactory;
+import org.apache.maven.api.services.Sources;
 import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
+import org.apache.maven.impl.standalone.ApiRunner;
+import org.codehaus.plexus.components.secdispatcher.Dispatcher;
+import 
org.codehaus.plexus.components.secdispatcher.internal.dispatchers.LegacyDispatcher;
+import org.eclipse.aether.spi.connector.transport.TransporterFactory;
+import org.eclipse.aether.spi.connector.transport.http.ChecksumExtractor;
+import org.eclipse.aether.spi.io.PathProcessor;
+import org.eclipse.aether.transport.file.FileTransporterFactory;
+import org.eclipse.aether.transport.jdk.JdkTransporterFactory;
 
 import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.PARENT;
 
@@ -47,6 +70,8 @@
  */
 public abstract class AbstractUpgradeStrategy implements UpgradeStrategy {
 
+    private Session session;
+
     /**
      * Template method that handles common logging and error handling.
      * Subclasses implement the actual upgrade logic in doApply().
@@ -185,4 +210,115 @@ public static Set<Coordinates> 
computeAllArtifactCoordinates(UpgradeContext cont
         context.info("Computed " + coordinatesByGAV.size() + " unique 
artifact(s) for inference");
         return new HashSet<>(coordinatesByGAV.values());
     }
+
+    protected Session getSession() {
+        if (session == null) {
+            session = createMaven4Session();
+        }
+        return session;
+    }
+
+    private Session createMaven4Session() {
+        Session session = ApiRunner.createSession(injector -> {
+            injector.bindInstance(Dispatcher.class, new LegacyDispatcher());
+            injector.bindImplicit(TransporterFactoryConfig.class);
+        });
+
+        // TODO: we should read settings
+        RemoteRepository central =
+                session.createRemoteRepository(RemoteRepository.CENTRAL_ID, 
"https://repo.maven.apache.org/maven2";);
+        RemoteRepository snapshots = 
session.getService(RepositoryFactory.class)
+                .createRemote(Repository.newBuilder()
+                        .id("apache-snapshots")
+                        
.url("https://repository.apache.org/content/repositories/snapshots/";)
+                        
.releases(RepositoryPolicy.newBuilder().enabled("false").build())
+                        
.snapshots(RepositoryPolicy.newBuilder().enabled("true").build())
+                        .build());
+
+        return session.withRemoteRepositories(List.of(central, snapshots));
+    }
+
+    protected Path createTempProjectStructure(UpgradeContext context, 
Map<Path, Document> pomMap) throws Exception {
+        Path tempDir = Files.createTempDirectory("mvnup-project-");
+        context.debug("Created temp project directory: " + tempDir);
+
+        Path commonRoot = findCommonRoot(pomMap.keySet());
+        context.debug("Common root: " + commonRoot);
+
+        for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
+            Path originalPath = entry.getKey();
+            Document document = entry.getValue();
+
+            Path relativePath = commonRoot.relativize(originalPath);
+            Path tempPomPath = tempDir.resolve(relativePath);
+
+            Files.createDirectories(tempPomPath.getParent());
+            Files.writeString(tempPomPath, document.toXml());
+            context.debug("Wrote POM to temp location: " + tempPomPath);
+        }
+
+        return tempDir;
+    }
+
+    protected Path findCommonRoot(Set<Path> pomPaths) {
+        Path commonRoot = null;
+        for (Path pomPath : pomPaths) {
+            Path parent = pomPath.getParent();
+            if (parent == null) {
+                parent = Path.of(".");
+            }
+            if (commonRoot == null) {
+                commonRoot = parent;
+            } else {
+                while (!parent.startsWith(commonRoot)) {
+                    commonRoot = commonRoot.getParent();
+                    if (commonRoot == null) {
+                        break;
+                    }
+                }
+            }
+        }
+        return commonRoot;
+    }
+
+    protected void cleanupTempDirectory(Path tempDir) {
+        try {
+            Files.walk(tempDir)
+                    .sorted(Comparator.reverseOrder())
+                    .map(Path::toFile)
+                    .forEach(File::delete);
+        } catch (Exception e) {
+            // Best effort cleanup
+        }
+    }
+
+    protected org.apache.maven.api.model.Model buildEffectiveModel(Path 
pomPath) {
+        Session session = getSession();
+        ModelBuilder modelBuilder = session.getService(ModelBuilder.class);
+
+        ModelBuilderRequest request = ModelBuilderRequest.builder()
+                .session(session)
+                .source(Sources.buildSource(pomPath))
+                .requestType(ModelBuilderRequest.RequestType.BUILD_EFFECTIVE)
+                .recursive(false)
+                .build();
+
+        ModelBuilderResult result = modelBuilder.newSession().build(request);
+        return result.getEffectiveModel();
+    }
+
+    static class TransporterFactoryConfig {
+        @Provides
+        @Named(JdkTransporterFactory.NAME)
+        static TransporterFactory jdkTransporterFactory(
+                ChecksumExtractor checksumExtractor, PathProcessor 
pathProcessor) {
+            return new JdkTransporterFactory(checksumExtractor, pathProcessor);
+        }
+
+        @Provides
+        @Named(FileTransporterFactory.NAME)
+        static TransporterFactory fileTransporterFactory() {
+            return new FileTransporterFactory();
+        }
+    }
 }
diff --git 
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
 
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
index d5e55e9e56..59cbf59d5c 100644
--- 
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
+++ 
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategy.java
@@ -124,7 +124,6 @@ public UpgradeResult doApply(UpgradeContext context, 
Map<Path, Document> pomMap)
         Set<Path> modifiedPoms = new HashSet<>();
         Set<Path> errorPoms = new HashSet<>();
 
-        // Collect all properties defined across all project POMs for 
cross-POM analysis
         Set<String> allDefinedProperties = collectAllDefinedProperties(pomMap);
 
         for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
@@ -138,7 +137,6 @@ public UpgradeResult doApply(UpgradeContext context, 
Map<Path, Document> pomMap)
             try {
                 boolean hasIssues = false;
 
-                // Apply all compatibility fixes
                 hasIssues |= 
fixUnsupportedCombineChildrenAttributes(pomDocument, context);
                 hasIssues |= fixUnsupportedCombineSelfAttributes(pomDocument, 
context);
                 hasIssues |= fixDuplicateDependencies(pomDocument, context);
@@ -356,26 +354,25 @@ private boolean fixIncorrectParentRelativePaths(
         return false;
     }
 
-    /**
-     * Collects all property names defined in properties sections across all 
project POMs.
-     */
     private Set<String> collectAllDefinedProperties(Map<Path, Document> 
pomMap) {
         Set<String> properties = new HashSet<>();
+        for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
+            collectPropertiesFromDom(entry.getValue(), properties);
+        }
+        return properties;
+    }
 
-        for (Document document : pomMap.values()) {
-            Element root = document.root();
-
-            root.childElement(PROPERTIES)
-                    .ifPresent(propsElement ->
-                            propsElement.childElements().forEach(child -> 
properties.add(child.name())));
+    private void collectPropertiesFromDom(Document document, Set<String> 
properties) {
+        Element root = document.root();
 
-            root.childElement(PROFILES).ifPresent(profiles -> 
profiles.childElements(PROFILE)
-                    .forEach(profile -> profile.childElement(PROPERTIES)
-                            .ifPresent(propsElement ->
-                                    propsElement.childElements().forEach(child 
-> properties.add(child.name())))));
-        }
+        root.childElement(PROPERTIES)
+                .ifPresent(propsElement -> 
propsElement.childElements().forEach(child -> properties.add(child.name())));
 
-        return properties;
+        root.childElement(PROFILES)
+                .ifPresent(profiles -> profiles.childElements(PROFILE)
+                        .forEach(profile -> profile.childElement(PROPERTIES)
+                                .ifPresent(propsElement ->
+                                        
propsElement.childElements().forEach(child -> properties.add(child.name())))));
     }
 
     /**
diff --git 
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
 
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
index ac0de5eefb..552ab1c2cf 100644
--- 
a/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
+++ 
b/impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvnup/goals/PluginUpgradeStrategy.java
@@ -18,10 +18,7 @@
  */
 package org.apache.maven.cling.invoker.mvnup.goals;
 
-import java.io.File;
-import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -32,35 +29,17 @@
 import eu.maveniverse.domtrip.Document;
 import eu.maveniverse.domtrip.Editor;
 import eu.maveniverse.domtrip.Element;
-import org.apache.maven.api.RemoteRepository;
-import org.apache.maven.api.Session;
 import org.apache.maven.api.cli.mvnup.UpgradeOptions;
 import org.apache.maven.api.di.Inject;
 import org.apache.maven.api.di.Named;
 import org.apache.maven.api.di.Priority;
-import org.apache.maven.api.di.Provides;
 import org.apache.maven.api.di.Singleton;
 import org.apache.maven.api.model.Build;
 import org.apache.maven.api.model.Model;
 import org.apache.maven.api.model.Parent;
 import org.apache.maven.api.model.Plugin;
 import org.apache.maven.api.model.PluginManagement;
-import org.apache.maven.api.model.Repository;
-import org.apache.maven.api.model.RepositoryPolicy;
-import org.apache.maven.api.services.ModelBuilder;
-import org.apache.maven.api.services.ModelBuilderRequest;
-import org.apache.maven.api.services.ModelBuilderResult;
-import org.apache.maven.api.services.RepositoryFactory;
-import org.apache.maven.api.services.Sources;
 import org.apache.maven.cling.invoker.mvnup.UpgradeContext;
-import org.apache.maven.impl.standalone.ApiRunner;
-import org.codehaus.plexus.components.secdispatcher.Dispatcher;
-import 
org.codehaus.plexus.components.secdispatcher.internal.dispatchers.LegacyDispatcher;
-import org.eclipse.aether.spi.connector.transport.TransporterFactory;
-import org.eclipse.aether.spi.connector.transport.http.ChecksumExtractor;
-import org.eclipse.aether.spi.io.PathProcessor;
-import org.eclipse.aether.transport.file.FileTransporterFactory;
-import org.eclipse.aether.transport.jdk.JdkTransporterFactory;
 
 import static 
eu.maveniverse.domtrip.maven.MavenPomElements.Elements.ARTIFACT_ID;
 import static eu.maveniverse.domtrip.maven.MavenPomElements.Elements.BUILD;
@@ -130,8 +109,6 @@ public class PluginUpgradeStrategy extends 
AbstractUpgradeStrategy {
             "1.4",
             "Versions before 1.4 use a removed DependencyGraphBuilder API 
incompatible with Maven 4"));
 
-    private Session session;
-
     @Inject
     public PluginUpgradeStrategy() {}
 
@@ -486,104 +463,6 @@ public static List<PluginUpgrade> getPluginUpgrades() {
         return PLUGIN_UPGRADES;
     }
 
-    /**
-     * Gets or creates the cached Maven 4 session.
-     */
-    private Session getSession() {
-        if (session == null) {
-            session = createMaven4Session();
-        }
-        return session;
-    }
-
-    /**
-     * Creates a new Maven 4 session for effective POM computation.
-     */
-    private Session createMaven4Session() {
-        Session session = ApiRunner.createSession(injector -> {
-            injector.bindInstance(Dispatcher.class, new LegacyDispatcher());
-            injector.bindImplicit(TransporterFactoryConfig.class);
-        });
-
-        // Configure repositories
-        // TODO: we should read settings
-        RemoteRepository central =
-                session.createRemoteRepository(RemoteRepository.CENTRAL_ID, 
"https://repo.maven.apache.org/maven2";);
-        RemoteRepository snapshots = 
session.getService(RepositoryFactory.class)
-                .createRemote(Repository.newBuilder()
-                        .id("apache-snapshots")
-                        
.url("https://repository.apache.org/content/repositories/snapshots/";)
-                        
.releases(RepositoryPolicy.newBuilder().enabled("false").build())
-                        
.snapshots(RepositoryPolicy.newBuilder().enabled("true").build())
-                        .build());
-
-        return session.withRemoteRepositories(List.of(central, snapshots));
-    }
-
-    /**
-     * Creates a temporary project structure with all POMs written to preserve 
relative paths.
-     * This allows Maven 4 API to properly resolve the project hierarchy.
-     */
-    private Path createTempProjectStructure(UpgradeContext context, Map<Path, 
Document> pomMap) throws Exception {
-        Path tempDir = Files.createTempDirectory("mvnup-project-");
-        context.debug("Created temp project directory: " + tempDir);
-
-        // Find the common root of all POM paths to preserve relative structure
-        Path commonRoot = findCommonRoot(pomMap.keySet());
-        context.debug("Common root: " + commonRoot);
-
-        // Write each POM to the temp directory, preserving relative structure
-        for (Map.Entry<Path, Document> entry : pomMap.entrySet()) {
-            Path originalPath = entry.getKey();
-            Document document = entry.getValue();
-
-            // Calculate the relative path from common root
-            Path relativePath = commonRoot.relativize(originalPath);
-            Path tempPomPath = tempDir.resolve(relativePath);
-
-            // Ensure parent directories exist
-            Files.createDirectories(tempPomPath.getParent());
-
-            // Write POM to temp location
-            writePomToFile(document, tempPomPath);
-            context.debug("Wrote POM to temp location: " + tempPomPath);
-        }
-
-        return tempDir;
-    }
-
-    /**
-     * Finds the common root directory of all POM paths.
-     */
-    private Path findCommonRoot(Set<Path> pomPaths) {
-        Path commonRoot = null;
-        for (Path pomPath : pomPaths) {
-            Path parent = pomPath.getParent();
-            if (parent == null) {
-                parent = Path.of(".");
-            }
-            if (commonRoot == null) {
-                commonRoot = parent;
-            } else {
-                // Find common ancestor
-                while (!parent.startsWith(commonRoot)) {
-                    commonRoot = commonRoot.getParent();
-                    if (commonRoot == null) {
-                        break;
-                    }
-                }
-            }
-        }
-        return commonRoot;
-    }
-
-    /**
-     * Writes a Document to a file using the same format as the existing 
codebase.
-     */
-    private void writePomToFile(Document document, Path filePath) throws 
Exception {
-        Files.writeString(filePath, document.toXml());
-    }
-
     /**
      * Analyzes plugins using effective models built from the temp directory.
      * Returns a map of POM path to the set of plugin keys that need 
management.
@@ -637,28 +516,9 @@ private Map<String, PluginUpgrade> 
getPluginUpgradesAsMap() {
                         upgrade -> upgrade.groupId() + ":" + 
upgrade.artifactId(), upgrade -> upgrade));
     }
 
-    /**
-     * Analyzes the effective model for a single POM to find plugins that need 
upgrades.
-     */
     private Set<String> analyzeEffectiveModelForPlugins(
             UpgradeContext context, Path tempPomPath, Map<String, 
PluginUpgrade> pluginUpgrades) {
-
-        // Use the cached Maven 4 session
-        Session session = getSession();
-        ModelBuilder modelBuilder = session.getService(ModelBuilder.class);
-
-        // Build effective model
-        ModelBuilderRequest request = ModelBuilderRequest.builder()
-                .session(session)
-                .source(Sources.buildSource(tempPomPath))
-                .requestType(ModelBuilderRequest.RequestType.BUILD_EFFECTIVE)
-                .recursive(false) // We only want this POM, not its modules
-                .build();
-
-        ModelBuilderResult result = modelBuilder.newSession().build(request);
-        Model effectiveModel = result.getEffectiveModel();
-
-        // Analyze plugins from effective model
+        Model effectiveModel = buildEffectiveModel(tempPomPath);
         return analyzePluginsFromEffectiveModel(context, effectiveModel, 
pluginUpgrades);
     }
 
@@ -729,19 +589,7 @@ private String getPluginKey(Plugin plugin) {
     private Path findLastLocalParentForPluginManagement(
             UpgradeContext context, Path tempPomPath, Map<Path, Document> 
pomMap, Path tempDir, Path commonRoot) {
 
-        // Build effective model to get parent information
-        Session session = getSession();
-        ModelBuilder modelBuilder = session.getService(ModelBuilder.class);
-
-        ModelBuilderRequest request = ModelBuilderRequest.builder()
-                .session(session)
-                .source(Sources.buildSource(tempPomPath))
-                .requestType(ModelBuilderRequest.RequestType.BUILD_EFFECTIVE)
-                .recursive(false)
-                .build();
-
-        ModelBuilderResult result = modelBuilder.newSession().build(request);
-        Model effectiveModel = result.getEffectiveModel();
+        Model effectiveModel = buildEffectiveModel(tempPomPath);
 
         // Convert the temp path back to the original path
         Path relativePath = tempDir.relativize(tempPomPath);
@@ -761,17 +609,8 @@ private Path findLastLocalParentForPluginManagement(
                 // Parent is local, so it becomes our new candidate
                 lastLocalParent = parentPath;
 
-                // Load the parent model to continue walking up
                 Path parentTempPath = 
tempDir.resolve(commonRoot.relativize(parentPath));
-                ModelBuilderRequest parentRequest = 
ModelBuilderRequest.builder()
-                        .session(session)
-                        .source(Sources.buildSource(parentTempPath))
-                        
.requestType(ModelBuilderRequest.RequestType.BUILD_EFFECTIVE)
-                        .recursive(false)
-                        .build();
-
-                ModelBuilderResult parentResult = 
modelBuilder.newSession().build(parentRequest);
-                currentModel = parentResult.getEffectiveModel();
+                currentModel = buildEffectiveModel(parentTempPath);
             } else {
                 // Parent is external, stop here
                 break;
@@ -896,20 +735,6 @@ private void addPluginManagementEntryFromUpgrade(
                 + upgrade.minVersion() + " (found through effective model 
analysis)");
     }
 
-    /**
-     * Cleans up the temporary directory.
-     */
-    private void cleanupTempDirectory(Path tempDir) {
-        try {
-            Files.walk(tempDir)
-                    .sorted(Comparator.reverseOrder())
-                    .map(Path::toFile)
-                    .forEach(File::delete);
-        } catch (Exception e) {
-            // Best effort cleanup - don't fail the whole operation
-        }
-    }
-
     /**
      * Holds plugin upgrade information for Maven 4 compatibility.
      * This class contains the minimum version requirements for plugins
@@ -938,24 +763,4 @@ public static class PluginUpgradeInfo {
             this.minVersion = minVersion;
         }
     }
-
-    /**
-     * DI configuration that registers transporter factories for the 
standalone Maven session.
-     * Uses {@code @Provides} methods so the factories are properly registered 
as named beans
-     * and feed into the {@code Map<String, TransporterFactory>} used by the 
TransporterProvider.
-     */
-    static class TransporterFactoryConfig {
-        @Provides
-        @Named(JdkTransporterFactory.NAME)
-        static TransporterFactory jdkTransporterFactory(
-                ChecksumExtractor checksumExtractor, PathProcessor 
pathProcessor) {
-            return new JdkTransporterFactory(checksumExtractor, pathProcessor);
-        }
-
-        @Provides
-        @Named(FileTransporterFactory.NAME)
-        static TransporterFactory fileTransporterFactory() {
-            return new FileTransporterFactory();
-        }
-    }
 }
diff --git 
a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
 
b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
index 15ea063016..00dacbb1c1 100644
--- 
a/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
+++ 
b/impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnup/goals/CompatibilityFixStrategyTest.java
@@ -18,6 +18,7 @@
  */
 package org.apache.maven.cling.invoker.mvnup.goals;
 
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Map;
@@ -32,6 +33,7 @@
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -58,6 +60,10 @@ private UpgradeContext createMockContext() {
         return TestUtils.createMockContext();
     }
 
+    private UpgradeContext createMockContext(Path workingDirectory) {
+        return TestUtils.createMockContext(workingDirectory);
+    }
+
     private UpgradeContext createMockContext(UpgradeOptions options) {
         return TestUtils.createMockContext(options);
     }
@@ -648,6 +654,273 @@ void shouldRecognizePropertyFromOtherPom() throws 
Exception {
         }
     }
 
+    @Nested
+    @DisplayName("Undefined Property Expression Fixes with Effective Model")
+    class UndefinedPropertyEffectiveModelTests {
+
+        @TempDir
+        Path tempDir;
+
+        @Test
+        @DisplayName("should recognize property inherited from external parent 
via relativePath")
+        void shouldRecognizePropertyFromExternalParent() throws Exception {
+            String parentPom = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <project xmlns="http://maven.apache.org/POM/4.0.0";
+                         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+                    <modelVersion>4.0.0</modelVersion>
+                    <groupId>com.example</groupId>
+                    <artifactId>external-parent</artifactId>
+                    <version>1.0.0</version>
+                    <packaging>pom</packaging>
+                    <properties>
+                        <guava.version>32.1.3-jre</guava.version>
+                    </properties>
+                </project>
+                """;
+
+            String childPom = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <project xmlns="http://maven.apache.org/POM/4.0.0";
+                         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+                    <modelVersion>4.0.0</modelVersion>
+                    <parent>
+                        <groupId>com.example</groupId>
+                        <artifactId>external-parent</artifactId>
+                        <version>1.0.0</version>
+                        <relativePath>../pom.xml</relativePath>
+                    </parent>
+                    <artifactId>child</artifactId>
+                    <dependencies>
+                        <dependency>
+                            <groupId>com.google.guava</groupId>
+                            <artifactId>guava</artifactId>
+                            <version>${guava.version}</version>
+                        </dependency>
+                    </dependencies>
+                </project>
+                """;
+
+            Path parentPomPath = tempDir.resolve("pom.xml");
+            Path childDir = Files.createDirectories(tempDir.resolve("child"));
+            Path childPomPath = childDir.resolve("pom.xml");
+
+            Files.writeString(parentPomPath, parentPom);
+            Files.writeString(childPomPath, childPom);
+
+            Document parentDoc = Document.of(parentPom);
+            Document childDoc = Document.of(childPom);
+            Map<Path, Document> pomMap = Map.of(
+                    parentPomPath, parentDoc,
+                    childPomPath, childDoc);
+
+            UpgradeContext context = createMockContext(tempDir);
+            strategy.doApply(context, pomMap);
+
+            Element root = childDoc.root();
+            Element deps = DomUtils.findChildElement(root, "dependencies");
+            assertEquals(
+                    1,
+                    deps.childElements("dependency").count(),
+                    "Dependency should not be commented out when property is 
inherited from external parent");
+        }
+
+        @Test
+        @DisplayName("should comment out dependency when property is not in 
parent either")
+        void shouldCommentOutWhenPropertyNotInParent() throws Exception {
+            String parentPom = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <project xmlns="http://maven.apache.org/POM/4.0.0";
+                         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+                    <modelVersion>4.0.0</modelVersion>
+                    <groupId>com.example</groupId>
+                    <artifactId>external-parent</artifactId>
+                    <version>1.0.0</version>
+                    <packaging>pom</packaging>
+                </project>
+                """;
+
+            String childPom = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <project xmlns="http://maven.apache.org/POM/4.0.0";
+                         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+                    <modelVersion>4.0.0</modelVersion>
+                    <parent>
+                        <groupId>com.example</groupId>
+                        <artifactId>external-parent</artifactId>
+                        <version>1.0.0</version>
+                        <relativePath>../pom.xml</relativePath>
+                    </parent>
+                    <artifactId>child</artifactId>
+                    <dependencies>
+                        <dependency>
+                            <groupId>com.google.guava</groupId>
+                            <artifactId>guava</artifactId>
+                            <version>${undefined.prop}</version>
+                        </dependency>
+                    </dependencies>
+                </project>
+                """;
+
+            Path parentPomPath = tempDir.resolve("pom.xml");
+            Path childDir = Files.createDirectories(tempDir.resolve("child"));
+            Path childPomPath = childDir.resolve("pom.xml");
+
+            Files.writeString(parentPomPath, parentPom);
+            Files.writeString(childPomPath, childPom);
+
+            Document childDoc = Document.of(childPom);
+            Map<Path, Document> pomMap = Map.of(childPomPath, childDoc);
+
+            UpgradeContext context = createMockContext(tempDir);
+            strategy.doApply(context, pomMap);
+
+            String xml = DomUtils.toXml(childDoc);
+            assertTrue(xml.contains("mvnup: commented out"), "Should contain 
comment-out marker");
+        }
+
+        @Test
+        @DisplayName("should handle partial undefined - only comment out 
dependency with undefined property")
+        void shouldHandlePartialUndefined() throws Exception {
+            String pomXml = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <project xmlns="http://maven.apache.org/POM/4.0.0";
+                         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+                    <modelVersion>4.0.0</modelVersion>
+                    <groupId>com.example</groupId>
+                    <artifactId>test</artifactId>
+                    <version>1.0.0</version>
+                    <properties>
+                        <commons.version>3.14.0</commons.version>
+                    </properties>
+                    <dependencies>
+                        <dependency>
+                            <groupId>org.apache.commons</groupId>
+                            <artifactId>commons-lang3</artifactId>
+                            <version>${commons.version}</version>
+                        </dependency>
+                        <dependency>
+                            <groupId>com.google.guava</groupId>
+                            <artifactId>guava</artifactId>
+                            <version>${undefined.version}</version>
+                        </dependency>
+                    </dependencies>
+                </project>
+                """;
+
+            Path pomPath = tempDir.resolve("pom.xml");
+            Files.writeString(pomPath, pomXml);
+
+            Document document = Document.of(pomXml);
+            Map<Path, Document> pomMap = Map.of(pomPath, document);
+
+            UpgradeContext context = createMockContext(tempDir);
+            strategy.doApply(context, pomMap);
+
+            Element root = document.root();
+            Element deps = DomUtils.findChildElement(root, "dependencies");
+            assertEquals(1, deps.childElements("dependency").count(), "Only 
defined-property dependency should remain");
+
+            String xml = DomUtils.toXml(document);
+            assertTrue(xml.contains("mvnup: commented out"), "Should contain 
comment-out marker");
+            assertTrue(xml.contains("'undefined.version'"), "Should mention 
the undefined property in comment");
+            assertFalse(xml.contains("'commons.version'"), "Should not flag 
the defined property as undefined");
+        }
+
+        @Test
+        @DisplayName("should recognize property from grandparent POM")
+        void shouldRecognizePropertyFromGrandparent() throws Exception {
+            String grandparentPom = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <project xmlns="http://maven.apache.org/POM/4.0.0";
+                         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+                    <modelVersion>4.0.0</modelVersion>
+                    <groupId>com.example</groupId>
+                    <artifactId>grandparent</artifactId>
+                    <version>1.0.0</version>
+                    <packaging>pom</packaging>
+                    <properties>
+                        <guava.version>32.1.3-jre</guava.version>
+                    </properties>
+                </project>
+                """;
+
+            String parentPom = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <project xmlns="http://maven.apache.org/POM/4.0.0";
+                         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+                    <modelVersion>4.0.0</modelVersion>
+                    <parent>
+                        <groupId>com.example</groupId>
+                        <artifactId>grandparent</artifactId>
+                        <version>1.0.0</version>
+                        <relativePath>../pom.xml</relativePath>
+                    </parent>
+                    <artifactId>parent</artifactId>
+                    <packaging>pom</packaging>
+                </project>
+                """;
+
+            String childPom = """
+                <?xml version="1.0" encoding="UTF-8"?>
+                <project xmlns="http://maven.apache.org/POM/4.0.0";
+                         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+                         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+                    <modelVersion>4.0.0</modelVersion>
+                    <parent>
+                        <groupId>com.example</groupId>
+                        <artifactId>parent</artifactId>
+                        <version>1.0.0</version>
+                        <relativePath>../parent/pom.xml</relativePath>
+                    </parent>
+                    <artifactId>child</artifactId>
+                    <dependencies>
+                        <dependency>
+                            <groupId>com.google.guava</groupId>
+                            <artifactId>guava</artifactId>
+                            <version>${guava.version}</version>
+                        </dependency>
+                    </dependencies>
+                </project>
+                """;
+
+            Path grandparentPath = tempDir.resolve("pom.xml");
+            Path parentDir = 
Files.createDirectories(tempDir.resolve("parent"));
+            Path parentPath = parentDir.resolve("pom.xml");
+            Path childDir = Files.createDirectories(tempDir.resolve("child"));
+            Path childPomPath = childDir.resolve("pom.xml");
+
+            Files.writeString(grandparentPath, grandparentPom);
+            Files.writeString(parentPath, parentPom);
+            Files.writeString(childPomPath, childPom);
+
+            Document grandparentDoc = Document.of(grandparentPom);
+            Document parentDoc = Document.of(parentPom);
+            Document childDoc = Document.of(childPom);
+            Map<Path, Document> pomMap = Map.of(
+                    grandparentPath, grandparentDoc,
+                    parentPath, parentDoc,
+                    childPomPath, childDoc);
+
+            UpgradeContext context = createMockContext(tempDir);
+            strategy.doApply(context, pomMap);
+
+            Element root = childDoc.root();
+            Element deps = DomUtils.findChildElement(root, "dependencies");
+            assertEquals(
+                    1,
+                    deps.childElements("dependency").count(),
+                    "Dependency should not be commented out when property is 
inherited from grandparent");
+        }
+    }
+
     @Nested
     @DisplayName("Strategy Description")
     class StrategyDescriptionTests {


Reply via email to