Skip to content

Commit

Permalink
[MNG-8447] Lossy ProblemCollector (#1994)
Browse files Browse the repository at this point in the history
Historically (from Maven3) "problem collection" (in various scenarios, like building effective settings and toolchains, to building models) were done by passing around `List<Problem>` structure. This proved quite ineffective, as in case of (really) huge projects with quite big count of problems reported choked Maven (but also produced unusable output). We are aware of projects producing 3 million warnings! Dumping all of them onto console/log makes really no sense.

This PR changes Maven that by default it reports "problems" (settings building, toolchains building, or model building) as one liner warnings: how much and where were problems collected. This produces much more less overwhelming output than happens on master. User can control "build errors" using `-e`, so `mvn -e` will dump errors on screen/log.

The new `org.apache.maven.api.services.ProblemCollector<P>` class, that is somewhat "drop in" replacement for `List`, but with huge difference: it is "lossy", in a way, it maintains counters precisely, but actual problem instances are simply dropped after threshold `maven.builder.maxProblems` is surpassed (default 100). See `org.apache.maven.api.Constants#MAVEN_BUILDER_MAX_PROBLEMS`.

Rules:
* on problem addition counters are always updated
* if below threshold, store the problem (and will be reported later to the user)
* if above threshold, try to drop one problem with severity lower than currently reported problem, if succeeded, store the current problem, otherwise drop it

---

https://issues.apache.org/jira/browse/MNG-8447
  • Loading branch information
cstamas authored Dec 19, 2024
1 parent 5599934 commit 0222aff
Show file tree
Hide file tree
Showing 31 changed files with 652 additions and 555 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@
*/
package org.apache.maven.api.services;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;

import org.apache.maven.api.annotations.Experimental;

/**
Expand All @@ -32,14 +28,14 @@
@Experimental
public abstract class MavenBuilderException extends MavenException {

private final List<BuilderProblem> problems;
private final ProblemCollector<BuilderProblem> problems;

public MavenBuilderException(String message, Throwable cause) {
super(message, cause);
problems = List.of();
problems = ProblemCollector.empty();
}

public MavenBuilderException(String message, List<BuilderProblem> problems) {
public MavenBuilderException(String message, ProblemCollector<BuilderProblem> problems) {
super(buildMessage(message, problems), null);
this.problems = problems;
}
Expand All @@ -49,20 +45,16 @@ public MavenBuilderException(String message, List<BuilderProblem> problems) {
* and then a list is built. These exceptions are usually thrown in "fatal" cases (and usually prevent Maven
* from starting), and these exceptions may end up very early on output.
*/
protected static String buildMessage(String message, List<BuilderProblem> problems) {
protected static String buildMessage(String message, ProblemCollector<BuilderProblem> problems) {
StringBuilder msg = new StringBuilder(message);
ArrayList<BuilderProblem> sorted = new ArrayList<>(problems);
sorted.sort(Comparator.comparing(BuilderProblem::getSeverity));
for (BuilderProblem problem : sorted) {
msg.append("\n * ")
.append(problem.getSeverity().name())
.append(": ")
.append(problem.getMessage());
}
problems.problems().forEach(problem -> msg.append("\n * ")
.append(problem.getSeverity().name())
.append(": ")
.append(problem.getMessage()));
return msg.toString();
}

public List<BuilderProblem> getProblems() {
public ProblemCollector<BuilderProblem> getProblemCollector() {
return problems;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
package org.apache.maven.api.services;

import java.io.Serial;
import java.util.Collections;
import java.util.List;

import org.apache.maven.api.annotations.Experimental;

Expand Down Expand Up @@ -82,10 +80,10 @@ public String getModelId() {
*
* @return The problems that caused this exception, never {@code null}.
*/
public List<ModelProblem> getProblems() {
public ProblemCollector<ModelProblem> getProblemCollector() {
if (result == null) {
return Collections.emptyList();
return ProblemCollector.empty();
}
return Collections.unmodifiableList(result.getProblems());
return result.getProblemCollector();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@ public interface ModelBuilderResult {
List<Profile> getActiveExternalProfiles();

/**
* Gets the problems that were encountered during the project building.
* Gets the problem collector that collected problems encountered during the project building.
*
* @return the problems that were encountered during the project building, can be empty but never {@code null}
* @return the problem collector that collected problems encountered during the project building
*/
@Nonnull
List<ModelProblem> getProblems();
ProblemCollector<ModelProblem> getProblemCollector();

/**
* Gets the children of this result.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
*/
package org.apache.maven.api.services;

import java.util.List;

import org.apache.maven.api.model.InputLocation;
import org.apache.maven.api.model.Model;

Expand All @@ -33,15 +31,15 @@
*/
public interface ModelProblemCollector {

/**
* The collected problems.
* @return a list of model problems encountered, never {@code null}
*/
List<ModelProblem> getProblems();
ProblemCollector<ModelProblem> getProblemCollector();

boolean hasErrors();
default boolean hasErrors() {
return getProblemCollector().hasErrorProblems();
}

boolean hasFatalErrors();
default boolean hasFatalErrors() {
return getProblemCollector().hasFatalProblems();
}

default void add(BuilderProblem.Severity severity, ModelProblem.Version version, String message) {
add(severity, version, message, null, null);
Expand All @@ -64,7 +62,9 @@ void add(
InputLocation location,
Exception exception);

void add(ModelProblem problem);
default void add(ModelProblem problem) {
getProblemCollector().reportProblem(problem);
}

ModelBuilderException newModelBuilderException();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
/*
* 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.services;

import java.util.Arrays;
import java.util.Comparator;
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.LongAdder;
import java.util.stream.Stream;

import org.apache.maven.api.Constants;
import org.apache.maven.api.ProtoSession;
import org.apache.maven.api.annotations.Experimental;
import org.apache.maven.api.annotations.Nonnull;
import org.apache.maven.api.annotations.Nullable;

import static java.util.Objects.requireNonNull;

/**
* Collects problems that were encountered during project building.
*
* @param <P> The type of the problem.
* @since 4.0.0
*/
@Experimental
public interface ProblemCollector<P extends BuilderProblem> {
/**
* Returns {@code true} if there is at least one problem collected with severity equal or more severe than
* {@link org.apache.maven.api.services.BuilderProblem.Severity#WARNING}. This check is logically equivalent
* to "is there any problem reported?", given warning is the lowest severity.
*/
default boolean hasWarningProblems() {
return hasProblemsFor(BuilderProblem.Severity.WARNING);
}

/**
* Returns {@code true} if there is at least one problem collected with severity equal or more severe than
* {@link org.apache.maven.api.services.BuilderProblem.Severity#ERROR}.
*/
default boolean hasErrorProblems() {
return hasProblemsFor(BuilderProblem.Severity.ERROR);
}

/**
* Returns {@code true} if there is at least one problem collected with severity equal or more severe than
* {@link org.apache.maven.api.services.BuilderProblem.Severity#FATAL}.
*/
default boolean hasFatalProblems() {
return hasProblemsFor(BuilderProblem.Severity.FATAL);
}

/**
* Returns {@code true} if there is at least one problem collected with severity equal or more severe than
* passed in severity.
*/
default boolean hasProblemsFor(BuilderProblem.Severity severity) {
requireNonNull(severity, "severity");
for (BuilderProblem.Severity s : BuilderProblem.Severity.values()) {
if (s.ordinal() <= severity.ordinal() && problemsReportedFor(s) > 0) {
return true;
}
}
return false;
}

/**
* Returns total count of problems reported.
*/
default int totalProblemsReported() {
return problemsReportedFor(BuilderProblem.Severity.values());
}

/**
* Returns count of problems reported for given severities.
*/
int problemsReportedFor(BuilderProblem.Severity... severities);

/**
* Returns {@code true} if reported problem count exceeded allowed count, and issues were lost. When this
* method returns {@code true}, it means that element count of stream returned by method {@link #problems()}
* and the counter returned by {@link #totalProblemsReported()} are not equal (latter is bigger than former).
*/
boolean problemsOverflow();

/**
* Reports a problem: always maintains the counters, but whether problem is preserved in memory, depends on
* implementation and its configuration.
*
* @return {@code true} if passed problem is preserved by this call.
*/
boolean reportProblem(P problem);

/**
* Returns all reported and preserved problems ordered by severity in decreasing order. Note: counters and
* element count in this stream does not have to be equal.
*/
@Nonnull
default Stream<P> problems() {
Stream<P> result = Stream.empty();
for (BuilderProblem.Severity severity : BuilderProblem.Severity.values()) {
result = Stream.concat(result, problems(severity));
}
return result;
}

/**
* Returns all reported and preserved problems for given severity. Note: counters and element count in this
* stream does not have to be equal.
*/
@Nonnull
Stream<P> problems(BuilderProblem.Severity severity);

/**
* Creates "empty" problem collector.
*/
@Nonnull
static <P extends BuilderProblem> ProblemCollector<P> empty() {
return new ProblemCollector<>() {
@Override
public boolean problemsOverflow() {
return false;
}

@Override
public int problemsReportedFor(BuilderProblem.Severity... severities) {
return 0;
}

@Override
public boolean reportProblem(P problem) {
throw new IllegalStateException("empty problem collector");
}

@Override
public Stream<P> problems(BuilderProblem.Severity severity) {
return Stream.empty();
}
};
}

/**
* Creates new instance of problem collector.
*/
@Nonnull
static <P extends BuilderProblem> ProblemCollector<P> create(@Nullable ProtoSession protoSession) {
if (protoSession != null
&& protoSession.getUserProperties().containsKey(Constants.MAVEN_BUILDER_MAX_PROBLEMS)) {
return new Impl<>(
Integer.parseInt(protoSession.getUserProperties().get(Constants.MAVEN_BUILDER_MAX_PROBLEMS)));
} else {
return create(100);
}
}

/**
* Creates new instance of problem collector. Visible for testing only.
*/
@Nonnull
static <P extends BuilderProblem> ProblemCollector<P> create(int maxCountLimit) {
return new Impl<>(maxCountLimit);
}

class Impl<P extends BuilderProblem> implements ProblemCollector<P> {

private final int maxCountLimit;
private final AtomicInteger totalCount;
private final ConcurrentMap<BuilderProblem.Severity, LongAdder> counters;
private final ConcurrentMap<BuilderProblem.Severity, List<P>> problems;

private static final List<BuilderProblem.Severity> REVERSED_ORDER = Arrays.stream(
BuilderProblem.Severity.values())
.sorted(Comparator.reverseOrder())
.toList();

private Impl(int maxCountLimit) {
if (maxCountLimit < 0) {
throw new IllegalArgumentException("maxCountLimit must be non-negative");
}
this.maxCountLimit = maxCountLimit;
this.totalCount = new AtomicInteger();
this.counters = new ConcurrentHashMap<>();
this.problems = new ConcurrentHashMap<>();
}

@Override
public int problemsReportedFor(BuilderProblem.Severity... severity) {
int result = 0;
for (BuilderProblem.Severity s : severity) {
result += getCounter(s).intValue();
}
return result;
}

@Override
public boolean problemsOverflow() {
return totalCount.get() > maxCountLimit;
}

@Override
public boolean reportProblem(P problem) {
requireNonNull(problem, "problem");
int currentCount = totalCount.incrementAndGet();
getCounter(problem.getSeverity()).increment();
if (currentCount <= maxCountLimit || dropProblemWithLowerSeverity(problem.getSeverity())) {
getProblems(problem.getSeverity()).add(problem);
return true;
}
return false;
}

@Override
public Stream<P> problems(BuilderProblem.Severity severity) {
requireNonNull(severity, "severity");
return getProblems(severity).stream();
}

private LongAdder getCounter(BuilderProblem.Severity severity) {
return counters.computeIfAbsent(severity, k -> new LongAdder());
}

private List<P> getProblems(BuilderProblem.Severity severity) {
return problems.computeIfAbsent(severity, k -> new CopyOnWriteArrayList<>());
}

private boolean dropProblemWithLowerSeverity(BuilderProblem.Severity severity) {
for (BuilderProblem.Severity s : REVERSED_ORDER) {
if (s.ordinal() > severity.ordinal()) {
List<P> problems = getProblems(s);
while (!problems.isEmpty()) {
try {
return problems.remove(0) != null;
} catch (IndexOutOfBoundsException e) {
// empty, continue
}
}
}
}
return false;
}
}
}
Loading

0 comments on commit 0222aff

Please sign in to comment.