gnodet-bot commented on code in PR #13542:
URL: https://github.com/apache/maven/pull/13542#discussion_r4294570705
##########
impl/maven-testing/src/main/java/org/apache/maven/testing/plugin/stubs/SessionStub.java:
##########
@@ -107,6 +108,11 @@ public Settings getSettings() {
return settings;
}
+ @Override
+ public BuildInvocation buildInvocation() {
+ return null;
Review Comment:
🚫 **[HIGH — still unaddressed]** `SessionStub.buildInvocation()` returns
`null` but `Session.buildInvocation()` is annotated `@Nonnull`. Any test code
that calls `session.buildInvocation()` and trusts the annotation (e.g. passes
the result directly to a method expecting non-null) will get an NPE.
Either throw `UnsupportedOperationException` (consistent with `ApiRunner`'s
standalone stub), or return a minimal test-only `BuildInvocation` built from a
factory helper.
```suggestion
public BuildInvocation buildInvocation() {
throw new UnsupportedOperationException("buildInvocation() is not
available in SessionStub");
}
```
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultSession.java:
##########
@@ -172,6 +175,19 @@ public int getDegreeOfConcurrency() {
return getMavenSession().getRequest().getDegreeOfConcurrency();
}
+ @Nonnull
+ @Override
+ public BuildInvocation buildInvocation() {
+ if (buildInvocation == null) {
+ synchronized (this) {
Review Comment:
⚠️ **[HIGH] `synchronized(this)` exposes the intrinsic lock externally**
Using `synchronized(this)` in DCL is correct for the double-check idiom
(combined with `volatile`), but it exposes the lock — any caller can do
`synchronized(session) { ... }` and deadlock the build. Prefer a dedicated
private lock object:
```suggestion
synchronized (buildInvocationLock) {
```
Add `private final Object buildInvocationLock = new Object();` as a field
alongside `buildInvocation`.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/BuildInvocation.java:
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.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 resume-from
+ * ({@link #resumeFrom()})</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>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 string, never {@code null}
+ */
+ @Nonnull
+ String localRepository();
Review Comment:
⚠️ **[MEDIUM] `localRepository()` should return `Path`, not `String`**
The Maven 4 API consistently uses `java.nio.file.Path` for filesystem paths
(see e.g. `Session.getRootDirectory()`, `LocalRepository`). Returning a raw
`String` forces every caller to parse it back into a `Path`, and the implicit
assumption of an absolute path is undocumented.
Change to:
```suggestion
@Nonnull
Path localRepository();
```
The implementation in `DefaultBuildInvocation.from()` can use
`request.getLocalRepositoryPath().toPath()` directly.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/BuildInvocation.java:
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.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 resume-from
+ * ({@link #resumeFrom()})</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()}).
Review Comment:
⚠️ **[MEDIUM] `systemInfo()` captures `user.home` and `user.name` without a
PII/privacy caveat in the Javadoc**
`user.home` contains the absolute home directory path (e.g. `/home/alice`)
and `user.name` is the OS login name. Both are PII. The class-level Javadoc
already mentions that `userProperties()` redacts sensitive keys —
`systemInfo()` needs a similar note, especially since this data ends up in the
structured build report which may be shared or stored.
Add to the Javadoc:
```
* <p><b>Privacy note:</b> {@code user.home} and {@code user.name} are
personal
* identifiers. Consumers persisting or sharing the build report should
consider
* redacting these fields.
```
##########
api/maven-api-core/src/main/java/org/apache/maven/api/Session.java:
##########
@@ -46,6 +46,16 @@
@ThreadSafe
public interface Session extends ProtoSession {
+ /**
+ * Returns the invocation context of this build: the goals, flags,
properties,
+ * and platform settings that were active when the session started.
+ *
+ * @return the build invocation, never {@code null}
+ * @since 4.1.0
+ */
+ @Nonnull
+ BuildInvocation buildInvocation();
Review Comment:
🚫 **[HIGH — still unaddressed]** `buildInvocation()` is added to the
`Session` interface without a `default` implementation. Any external
implementor of `Session` (embedders, test stubs in downstream projects, plugin
testing frameworks) will get a compile error when upgrading to 4.1.0 — a
binary- and source-incompatible API change.
Either add a `default` implementation that throws
`UnsupportedOperationException`, or — if you want it truly abstract — document
it explicitly in the class-level Javadoc as a breaking change requiring all
implementors to implement it. The latter is rarely acceptable in a 4.x minor
for a public API interface.
```suggestion
@Nonnull
default BuildInvocation buildInvocation() {
throw new UnsupportedOperationException("buildInvocation() not
implemented");
}
```
##########
impl/maven-core/src/main/java/org/apache/maven/execution/MavenExecutionRequest.java:
##########
@@ -279,6 +279,11 @@ public interface MavenExecutionRequest {
boolean isInteractiveMode();
+ // Transfer progress
+ MavenExecutionRequest setNoTransferProgress(boolean noTransferProgress);
Review Comment:
⚠️ **[MEDIUM] `setNoTransferProgress` / `isNoTransferProgress` are added to
`MavenExecutionRequest` without `default` implementations**
`MavenExecutionRequest` is a public API interface. Adding abstract methods
without defaults breaks all existing implementations (embedders, plugin test
frameworks, etc.) at compile time when they upgrade.
In Maven 4's compatibility layer this is especially risky since
`MavenExecutionRequest` is part of the deprecated compat bridge that third
parties still implement.
Add defaults:
```suggestion
default MavenExecutionRequest setNoTransferProgress(boolean
noTransferProgress) {
return this;
}
default boolean isNoTransferProgress() {
return false;
}
```
##########
api/maven-api-core/src/main/java/org/apache/maven/api/BuildInvocation.java:
##########
@@ -0,0 +1,203 @@
+/*
+ * 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.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 resume-from
+ * ({@link #resumeFrom()})</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>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 string, never {@code null}
+ */
+ @Nonnull
+ String 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();
+
+ /**
+ * The project to resume from, as specified via {@code -rf} / {@code
--resume-from}.
+ *
+ * @return the resume-from selector, or {@code null} if not specified
+ */
+ String resumeFrom();
+
+ /**
+ * The reactor failure behavior, corresponding to the {@code -ff} / {@code
-fae} /
+ * {@code -fn} flags.
+ *
+ * <p>Possible values (matching {@code MavenExecutionRequest} constants):
+ * <ul>
+ * <li>{@code "FAIL_FAST"} — stop at first failure ({@code -ff},
default)</li>
+ * <li>{@code "FAIL_AT_END"} — build all, report failures at end ({@code
-fae})</li>
+ * <li>{@code "FAIL_NEVER"} — always exit with success ({@code -fn})</li>
+ * </ul>
+ *
+ * @return the failure behavior string, never {@code null}
+ */
+ @Nonnull
+ String reactorFailureBehavior();
Review Comment:
⚠️ **[MEDIUM] `reactorFailureBehavior()` should return an enum or a typed
constant, not a raw `String`**
The three values (`FAIL_FAST`, `FAIL_AT_END`, `FAIL_NEVER`) are already
defined as string constants on `MavenExecutionRequest`, but that's an internal
class. Exposing raw strings in the public API forces callers to compare strings
against undocumented constants, and makes typos silent.
Consider either:
1. An enum in `maven-api-core` (preferred): `ReactorFailureBehavior {
FAIL_FAST, FAIL_AT_END, FAIL_NEVER }`
2. Or typed constants on `BuildInvocation` itself: `String FAIL_FAST =
"FAIL_FAST";` etc. (weaker, but backward-compatible to add later)
For a new API surface in 4.1.0 there is no backward-compat reason to start
with a raw String.
##########
impl/maven-core/src/main/java/org/apache/maven/internal/build/DefaultBuildInvocation.java:
##########
@@ -0,0 +1,170 @@
+/*
+ * 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.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+
+import org.apache.maven.api.BuildInvocation;
+import org.apache.maven.execution.MavenSession;
+
+/**
+ * Internal immutable implementation of {@link BuildInvocation}.
+ *
+ * @param goals requested goals/phases
+ * @param userProperties user properties with sensitive values redacted
+ * @param systemInfo curated system properties for reproducibility
+ * @param localRepository local repository path
+ * @param activeProfiles explicitly activated profiles
+ * @param selectedProjects projects selected via {@code -pl}
+ * @param resumeFrom resume-from selector, or {@code null}
+ * @param reactorFailureBehavior reactor failure behavior
+ * @param offline whether offline mode was active
+ * @param updateSnapshots whether snapshot updates were forced
+ * @param noTransferProgress whether transfer progress was suppressed
+ * @param batchMode whether batch mode was active
+ * @param threads degree of concurrency
+ */
+public record DefaultBuildInvocation(
+ List<String> goals,
+ Map<String, String> userProperties,
+ Map<String, String> systemInfo,
+ String localRepository,
+ List<String> activeProfiles,
+ List<String> selectedProjects,
+ String resumeFrom,
+ String reactorFailureBehavior,
+ boolean offline,
+ boolean updateSnapshots,
+ boolean noTransferProgress,
+ boolean batchMode,
+ int threads)
+ implements BuildInvocation {
+
+ private static final Set<String> SENSITIVE_KEY_FRAGMENTS =
+ Set.of("password", "passwd", "secret", "token", "apikey",
"api_key", "credential", "passphrase");
+
+ @Override
+ public List<String> goals() {
+ return List.copyOf(goals);
Review Comment:
⚠️ **[MEDIUM] Record accessor overrides allocate a defensive copy on every
call**
The record compiler already generates a canonical accessor that returns the
component directly (the record field is `List<String> goals`). Overriding it
with `return List.copyOf(goals)` allocates a new collection on every `goals()`
call, defeating the immutability guarantee that the record component itself
could provide.
The right approach is to enforce immutability at construction time — once in
the compact constructor or `from()` — and then expose the field directly:
```suggestion
@Override
public List<String> goals() {
return goals;
}
```
In the compact constructor (or `from()`), wrap once:
```java
goals = List.copyOf(goals);
userProperties = Map.copyOf(userProperties);
systemInfo = Map.copyOf(systemInfo);
activeProfiles = List.copyOf(activeProfiles);
selectedProjects = List.copyOf(selectedProjects);
```
This removes the O(n) copy per accessor call and is the idiomatic Java
record pattern for immutable collections.
--
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]
