gnodet-bot commented on code in PR #13542: URL: https://github.com/apache/maven/pull/13542#discussion_r4323951787
########## api/maven-api-core/src/main/java/org/apache/maven/api/BuildInvocation.java: ########## @@ -0,0 +1,238 @@ +/* + * 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.api; + +import java.nio.file.Path; +import java.util.List; +import java.util.Map; + +import org.apache.maven.api.annotations.Experimental; +import org.apache.maven.api.annotations.Immutable; +import org.apache.maven.api.annotations.Nonnull; + +/** + * Describes the invocation context of a Maven build: the goals, flags, properties, + * and platform settings that were active when the build started. + * + * <p>An instance is available via {@link Session#buildInvocation()} during the build, + * and is also recorded in the structured build report for post-mortem analysis and + * reproducibility. + * + * <h2>What is captured</h2> + * <ul> + * <li>Goals and lifecycle phases requested ({@link #goals()})</li> + * <li>User properties passed via {@code -Dkey=value} ({@link #userProperties()}), + * with sensitive keys redacted — see {@link #userProperties()} for the denylist</li> + * <li>A curated subset of system properties relevant to reproducibility + * ({@link #systemInfo()}): OS name/arch/version, Java vendor and VM name/version, + * Maven home, and available processors</li> + * <li>Local repository path ({@link #localRepository()})</li> + * <li>Explicitly activated or deactivated profiles ({@link #activeProfiles()})</li> + * <li>Selected projects ({@link #selectedProjects()}) and excluded projects + * ({@link #excludedProjects()})</li> + * <li>Make behavior ({@link #makeBehavior()}): upstream/downstream dependency building</li> + * <li>Resume-from ({@link #resumeFrom()}) and resume mode ({@link #resume()})</li> + * <li>Recursive mode ({@link #recursive()})</li> + * <li>Reactor failure behavior ({@link #reactorFailureBehavior()})</li> + * <li>Offline mode ({@link #offline()}) and snapshot update policy + * ({@link #updateSnapshots()})</li> + * <li>Degree of concurrency ({@link #threads()})</li> + * <li>Transfer-progress suppression ({@link #noTransferProgress()}) and batch mode ({@link #batchMode()})</li> + * </ul> + * + * <h2>What is not yet captured</h2> + * <p>The following information is not currently available through the Maven 4 API + * and therefore cannot be recorded here. It may be added in future versions as + * the API evolves: + * <ul> + * <li><b>Raw command-line arguments</b> ({@code args[]}): the CLI bootstrap layer + * does not surface these through {@code MavenExecutionRequest} or the session</li> + * <li><b>Implicitly activated profiles</b>: profiles activated by OS, JDK, or + * property conditions rather than by explicit {@code -P} flag; computing + * these requires per-project activation and is not available at session level</li> + * </ul> + * + * @since 4.1.0 + * @see Session#buildInvocation() + */ +@Experimental +@Immutable +public interface BuildInvocation { + + /** + * The goals or lifecycle phases that were requested on the command line. + * + * @return the requested goals/phases, never {@code null} + */ + @Nonnull + List<String> goals(); + + /** + * User properties passed via {@code -Dkey=value} on the command line or via + * {@code --define}. + * + * <p>Sensitive keys are redacted and replaced with {@code "***"}. A key is + * considered sensitive if its lower-case form contains any of: + * {@code password}, {@code passwd}, {@code secret}, {@code token}, + * {@code apikey}, {@code api_key}, {@code credential}, {@code passphrase}. + * + * @return the user properties, never {@code null}; values of sensitive keys + * are replaced with {@code "***"} + */ + @Nonnull + Map<String, String> userProperties(); + + /** + * A curated subset of system properties capturing platform and JVM identity, + * relevant for build reproducibility analysis. + * + * <p>The following keys are included when present: + * {@code os.name}, {@code os.arch}, {@code os.version}, + * {@code java.vendor}, {@code java.vm.name}, {@code java.vm.version}, + * {@code maven.home}, {@code user.home}, {@code user.name}, + * and {@code available.processors} (from {@code Runtime.getRuntime()}). + * + * <p><b>Privacy note:</b> {@code user.home} and {@code user.name} reflect the OS + * user identity and may be considered personally identifiable information (PII) + * in shared build environments, CI logs, or published build reports. Callers + * should consider redacting these values before persisting or transmitting + * build invocation data externally. + * + * <p>The full {@code System.getProperties()} map is intentionally not captured: + * it is large, mostly irrelevant, and may contain sensitive values. + * + * @return the curated system info map, never {@code null} + */ + @Nonnull + Map<String, String> systemInfo(); + + /** + * The path to the local repository used for this build. + * + * @return the local repository path, never {@code null} + */ + @Nonnull + Path localRepository(); + + /** + * Profiles explicitly activated ({@code -P profileId}) or deactivated + * ({@code -P !profileId}) on the command line. + * + * <p>This list reflects only explicit {@code -P} selections. Profiles activated + * implicitly by OS, JDK, or property conditions are not included here — see + * the class-level javadoc for details. + * + * @return the explicitly selected profiles, never {@code null}; may be empty + */ + @Nonnull + List<String> activeProfiles(); + + /** + * Projects explicitly selected via {@code -pl} / {@code --projects}. + * + * @return the selected project selectors (e.g. {@code ":my-module"}), + * never {@code null}; empty for a full reactor build + */ + @Nonnull + List<String> selectedProjects(); + + /** + * Projects explicitly excluded via {@code -pl !module} / {@code --projects !module}. + * + * @return the excluded project selectors, never {@code null}; empty if no exclusions + */ + @Nonnull + List<String> excludedProjects(); + + /** + * The reactor make behavior ({@code -am} / {@code -amd}), controlling whether + * upstream and/or downstream dependencies of selected projects are also built. + * + * @return the make behavior, or {@code null} if not specified + */ + MakeBehavior makeBehavior(); + + /** + * The project to resume from, as specified via {@code -rf} / {@code --resume-from}. + * + * @return the resume-from selector, or {@code null} if not specified + */ Review Comment: 💡 **`@Nullable` annotation missing on `makeBehavior()`** The Javadoc says "or `null` if not specified", but the method has no `@Nullable` annotation. Static null-analysis tools (IntelliJ, SpotBugs, Error Prone) will treat the return as non-null and flag any null-check as redundant — exactly the wrong signal for a method that legally returns `null`. ```suggestion @Nullable MakeBehavior makeBehavior(); ``` Add `@Nullable` (from `org.apache.maven.api.annotations`) to match the documented contract. Note that `@Nonnull` on the other accessors is already imported. ########## impl/maven-core/src/test/java/org/apache/maven/internal/build/DefaultBuildInvocationTest.java: ########## @@ -0,0 +1,395 @@ +/* + * 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.internal.build; + +import java.io.File; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.Properties; + +import org.apache.maven.api.BuildInvocation; +import org.apache.maven.api.MakeBehavior; +import org.apache.maven.api.ReactorFailureBehavior; +import org.apache.maven.execution.DefaultMavenExecutionRequest; +import org.apache.maven.execution.MavenExecutionRequest; +import org.apache.maven.execution.MavenSession; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class DefaultBuildInvocationTest { + + private static MavenExecutionRequest newRequest() { + MavenExecutionRequest request = new DefaultMavenExecutionRequest(); + request.setLocalRepositoryPath(new File("/tmp/test-repo")); + return request; + } + + @Test + void fromBasicRequest() { + MavenExecutionRequest request = new DefaultMavenExecutionRequest(); + request.setGoals(List.of("clean", "install")); + request.setOffline(true); + request.setUpdateSnapshots(true); + request.setInteractiveMode(false); + request.setReactorFailureBehavior(MavenExecutionRequest.REACTOR_FAIL_AT_END); + request.setLocalRepositoryPath(new File("/tmp/repo")); + + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertEquals(List.of("clean", "install"), inv.goals()); + assertTrue(inv.offline()); + assertTrue(inv.updateSnapshots()); + assertTrue(inv.batchMode()); + assertEquals(ReactorFailureBehavior.FAIL_AT_END, inv.reactorFailureBehavior()); + assertEquals(Path.of("/tmp/repo"), inv.localRepository()); + assertTrue(inv.recursive()); + assertFalse(inv.resume()); + assertNull(inv.makeBehavior()); + assertTrue(inv.excludedProjects().isEmpty()); + } + + @Test + void fromNullGoals() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertNotNull(inv.goals()); + assertTrue(inv.goals().isEmpty()); + } + + @Test + void fromNullActiveProfiles() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertNotNull(inv.activeProfiles()); + assertTrue(inv.activeProfiles().isEmpty()); + } + + @Test + void fromNullSelectedProjects() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertNotNull(inv.selectedProjects()); + assertTrue(inv.selectedProjects().isEmpty()); + } + + @Test + void fromNullLocalRepositoryPathThrows() { + MavenExecutionRequest request = new DefaultMavenExecutionRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + assertThrows(IllegalStateException.class, () -> DefaultBuildInvocation.from(session)); + } + + @Test + void fromNullReactorFailureBehaviorDefaultsToFailFast() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertEquals(ReactorFailureBehavior.FAIL_FAST, inv.reactorFailureBehavior()); + } + + @Test + void resumeFrom() { + MavenExecutionRequest request = newRequest(); + request.setResumeFrom(":my-module"); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertEquals(":my-module", inv.resumeFrom()); + } + + @Test + void resumeFromNull() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertNull(inv.resumeFrom()); + } + + @Test + void noTransferProgress() { + MavenExecutionRequest request = newRequest(); + request.setNoTransferProgress(true); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertTrue(inv.noTransferProgress()); + } + + @Test + void sensitiveKeyRedaction() { + MavenExecutionRequest request = newRequest(); + Properties userProps = new Properties(); + userProps.setProperty("my.password", "secret123"); + userProps.setProperty("api_key", "key123"); + userProps.setProperty("auth.token", "tok123"); + userProps.setProperty("safe.property", "visible"); + request.setUserProperties(userProps); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertEquals("***", inv.userProperties().get("my.password")); + assertEquals("***", inv.userProperties().get("api_key")); + assertEquals("***", inv.userProperties().get("auth.token")); + assertEquals("visible", inv.userProperties().get("safe.property")); + } + + @ParameterizedTest + @ValueSource( + strings = { + "password", + "PASSWORD", + "my.passwd", + "db_secret", + "auth_token", + "APIKEY", + "my.api_key", + "credential", + "ssh.passphrase" + }) + void sensitiveKeyFragments(String key) { + assertTrue(DefaultBuildInvocation.isSensitiveKey(key)); + } + + @ParameterizedTest + @ValueSource(strings = {"safe.property", "skip.tests", "maven.compiler.source", "project.build.outputDirectory"}) + void nonSensitiveKeys(String key) { + assertFalse(DefaultBuildInvocation.isSensitiveKey(key)); + } + + @Test + void systemInfoCapturesExpectedKeys() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + Map<String, String> sysInfo = inv.systemInfo(); + assertNotNull(sysInfo.get("os.name")); + assertNotNull(sysInfo.get("os.arch")); + assertNotNull(sysInfo.get("java.vm.version")); + assertNotNull(sysInfo.get("available.processors")); + } + + @Test + void collectionsAreImmutable() { + DefaultBuildInvocation inv = new DefaultBuildInvocation( + List.of("compile"), + Map.of("key", "val"), + Map.of("os.name", "Linux"), + Path.of("/tmp/repo"), + List.of("profile1"), + List.of(":module1"), + List.of(":excluded"), + null, + null, + ReactorFailureBehavior.FAIL_FAST, + false, + false, + false, + false, + true, + false, + 1); + + assertThrows(UnsupportedOperationException.class, () -> inv.goals().add("test")); + assertThrows( + UnsupportedOperationException.class, () -> inv.userProperties().put("new", "val")); + assertThrows( + UnsupportedOperationException.class, () -> inv.activeProfiles().add("p2")); + assertThrows( + UnsupportedOperationException.class, + () -> inv.selectedProjects().add(":m2")); + assertThrows( + UnsupportedOperationException.class, + () -> inv.excludedProjects().add(":m3")); + assertThrows(UnsupportedOperationException.class, () -> inv.systemInfo().put("k", "v")); + } + + @Test + void reactorFailureBehaviorValues() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + request.setReactorFailureBehavior(MavenExecutionRequest.REACTOR_FAIL_FAST); + assertEquals( + ReactorFailureBehavior.FAIL_FAST, + DefaultBuildInvocation.from(session).reactorFailureBehavior()); + + request.setReactorFailureBehavior(MavenExecutionRequest.REACTOR_FAIL_AT_END); + assertEquals( + ReactorFailureBehavior.FAIL_AT_END, + DefaultBuildInvocation.from(session).reactorFailureBehavior()); + + request.setReactorFailureBehavior(MavenExecutionRequest.REACTOR_FAIL_NEVER); + assertEquals( + ReactorFailureBehavior.FAIL_NEVER, + DefaultBuildInvocation.from(session).reactorFailureBehavior()); + } + + @Test + void unrecognizedReactorFailureBehaviorDefaultsToFailFast() { + MavenExecutionRequest request = newRequest(); + request.setReactorFailureBehavior("UNKNOWN_VALUE"); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertEquals(ReactorFailureBehavior.FAIL_FAST, inv.reactorFailureBehavior()); + } + + @Test + void threadsDefault() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertEquals(1, inv.threads()); + } + + @Test + void threadsExplicit() { + MavenExecutionRequest request = newRequest(); + request.setDegreeOfConcurrency(4); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertEquals(4, inv.threads()); + } + + @Test + void excludedProjects() { + MavenExecutionRequest request = newRequest(); + request.setExcludedProjects(List.of(":slow-module", ":flaky-module")); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + BuildInvocation inv = DefaultBuildInvocation.from(session); + + assertEquals(2, inv.excludedProjects().size()); + assertTrue(inv.excludedProjects().contains(":slow-module")); + assertTrue(inv.excludedProjects().contains(":flaky-module")); + } + + @Test + void makeBehaviorUpstream() { + MavenExecutionRequest request = newRequest(); + request.setMakeBehavior(MavenExecutionRequest.REACTOR_MAKE_UPSTREAM); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + assertEquals(MakeBehavior.UPSTREAM, DefaultBuildInvocation.from(session).makeBehavior()); + } + + @Test + void makeBehaviorDownstream() { + MavenExecutionRequest request = newRequest(); + request.setMakeBehavior(MavenExecutionRequest.REACTOR_MAKE_DOWNSTREAM); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + assertEquals( + MakeBehavior.DOWNSTREAM, DefaultBuildInvocation.from(session).makeBehavior()); + } + + @Test + void makeBehaviorBoth() { + MavenExecutionRequest request = newRequest(); + request.setMakeBehavior(MavenExecutionRequest.REACTOR_MAKE_BOTH); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + assertEquals(MakeBehavior.BOTH, DefaultBuildInvocation.from(session).makeBehavior()); + } + + @Test + void makeBehaviorNull() { + MavenExecutionRequest request = newRequest(); + MavenSession session = mock(MavenSession.class); + when(session.getRequest()).thenReturn(request); + + assertNull(DefaultBuildInvocation.from(session).makeBehavior()); + } + Review Comment: 💡 **Missing test: unknown make behavior string returns `null`** `makeBehaviorNull()` covers the case where `request.getMakeBehavior()` is `null`. But `DefaultBuildInvocation.from()` has a second null-return path: when `getMakeBehavior()` returns an **unrecognized string**, `MakeBehavior.fromString()` throws `IllegalArgumentException`, the catch block swallows it and logs at `DEBUG`, and `makeBehavior` stays `null`. Add a companion test: ```java @Test void makeBehaviorUnknownStringReturnsNull() { MavenExecutionRequest request = newRequest(); request.setMakeBehavior("invalid-value"); MavenSession session = mock(MavenSession.class); when(session.getRequest()).thenReturn(request); assertNull(DefaultBuildInvocation.from(session).makeBehavior()); } ``` -- 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]
