Skip to content

Exception usage cleanup #1910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 16, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* 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.io.Serial;

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

/**
* Base class for all maven exceptions you want to deal with.
*
* @since 4.0.0
*/
@Experimental
public class MavenCheckedException extends Exception {

@Serial
private static final long serialVersionUID = 5687063382274793735L;

public MavenCheckedException() {}

public MavenCheckedException(String message) {
super(message);
}

public MavenCheckedException(String message, Throwable cause) {
super(message, cause);
}

public MavenCheckedException(Throwable cause) {
super(cause);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,5 @@ interface ModelBuilderSession {
ModelBuilderResult build(ModelBuilderRequest request) throws ModelBuilderException;
}

Model buildRawModel(ModelBuilderRequest request);
Model buildRawModel(ModelBuilderRequest request) throws ModelBuilderException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
* @since 4.0.0
*/
@Experimental
public class ModelBuilderException extends MavenException {
public class ModelBuilderException extends MavenCheckedException {

@Serial
private static final long serialVersionUID = -1865447022070650896L;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public interface ProjectBuilder extends Service {
* @throws IllegalArgumentException if an argument is {@code null} or invalid
*/
@Nonnull
ProjectBuilderResult build(ProjectBuilderRequest request);
ProjectBuilderResult build(ProjectBuilderRequest request) throws ProjectBuilderException;

/**
* Creates a {@link org.apache.maven.api.Project} from a POM file.
Expand All @@ -52,7 +52,8 @@ public interface ProjectBuilder extends Service {
* @see #build(ProjectBuilderRequest)
*/
@Nonnull
default ProjectBuilderResult build(@Nonnull Session session, @Nonnull Source source) {
default ProjectBuilderResult build(@Nonnull Session session, @Nonnull Source source)
throws ProjectBuilderException {
return build(ProjectBuilderRequest.build(session, source));
}

Expand All @@ -66,7 +67,7 @@ default ProjectBuilderResult build(@Nonnull Session session, @Nonnull Source sou
* @see #build(ProjectBuilderRequest)
*/
@Nonnull
default ProjectBuilderResult build(@Nonnull Session session, @Nonnull Path path) {
default ProjectBuilderResult build(@Nonnull Session session, @Nonnull Path path) throws ProjectBuilderException {
return build(ProjectBuilderRequest.build(session, path));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* @since 4.0.0
*/
@Experimental
public class ProjectBuilderException extends MavenException {
public class ProjectBuilderException extends MavenCheckedException {

@Serial
private static final long serialVersionUID = -7629871850875943799L;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import java.nio.file.Path;

import org.apache.maven.api.model.Model;
import org.apache.maven.model.building.ModelBuildingException;
import org.apache.maven.api.services.ModelBuilderException;
import org.apache.maven.project.MavenProject;
import org.eclipse.aether.RepositorySystemSession;

Expand All @@ -35,5 +35,5 @@
interface ConsumerPomBuilder {

Model build(RepositorySystemSession session, MavenProject project, Path src)
throws ModelBuildingException, IOException, XMLStreamException;
throws ModelBuilderException, IOException, XMLStreamException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@

import org.apache.maven.api.feature.Features;
import org.apache.maven.api.model.Model;
import org.apache.maven.api.services.ModelBuilderException;
import org.apache.maven.internal.transformation.ConsumerPomArtifactTransformer;
import org.apache.maven.model.building.ModelBuildingException;
import org.apache.maven.model.v4.MavenStaxWriter;
import org.apache.maven.project.MavenProject;
import org.apache.maven.project.artifact.ProjectArtifact;
Expand Down Expand Up @@ -112,7 +112,7 @@ TransformedArtifact createConsumerPomArtifact(
}

void transform(MavenProject project, RepositorySystemSession session, Path src, Path tgt)
throws ModelBuildingException, XMLStreamException, IOException {
throws ModelBuilderException, XMLStreamException, IOException {
Model model = builder.build(session, project, src);
write(model, tgt);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;

import org.apache.maven.api.services.ModelBuilderException;
import org.apache.maven.artifact.DefaultArtifact;
import org.apache.maven.internal.transformation.TransformationFailedException;
import org.apache.maven.model.building.ModelBuildingException;
import org.apache.maven.project.MavenProject;
import org.eclipse.aether.RepositorySystemSession;

Expand Down Expand Up @@ -97,13 +97,12 @@ public synchronized File getFile() {
return null;
}
return target.toFile();
} catch (IOException | NoSuchAlgorithmException | XMLStreamException | ModelBuildingException e) {
} catch (IOException | NoSuchAlgorithmException | XMLStreamException | ModelBuilderException e) {
throw new TransformationFailedException(e);
}
}

private String mayUpdate()
throws IOException, NoSuchAlgorithmException, XMLStreamException, ModelBuildingException {
private String mayUpdate() throws IOException, NoSuchAlgorithmException, XMLStreamException, ModelBuilderException {
String result;
Path src = sourcePathProvider.get();
if (src == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.apache.maven.api.services.DependencyResolver;
import org.apache.maven.api.services.DependencyResolverResult;
import org.apache.maven.api.services.ProjectBuilder;
import org.apache.maven.api.services.ProjectBuilderException;
import org.apache.maven.api.services.ProjectBuilderRequest;
import org.apache.maven.api.services.SettingsBuilder;
import org.apache.maven.artifact.handler.manager.ArtifactHandlerManager;
Expand Down Expand Up @@ -140,7 +141,7 @@ void setup() {
sessionScope.seed(InternalMavenSession.class, InternalMavenSession.from(this.session));
}

private Project project(Artifact artifact) {
private Project project(Artifact artifact) throws ProjectBuilderException {
return session.getService(ProjectBuilder.class)
.build(ProjectBuilderRequest.builder()
.session(session)
Expand All @@ -165,7 +166,7 @@ void testCreateAndResolveArtifact() {
}

@Test
void testBuildProject() {
void testBuildProject() throws ProjectBuilderException {
Artifact artifact = session.createArtifact("org.codehaus.plexus", "plexus-utils", "1.4.5", "pom");

Project project = project(artifact);
Expand Down Expand Up @@ -229,7 +230,7 @@ void testMetadataGeneratorFactory() throws ComponentLookupException {
}

@Test
void testProjectDependencies() {
void testProjectDependencies() throws ProjectBuilderException {
Artifact pom = session.createArtifact("org.codehaus.plexus", "plexus-container-default", "1.0-alpha-32", "pom");

Project project = project(pom);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
*/
package org.apache.maven.api.services.model;

import org.apache.maven.api.services.MavenException;
import org.apache.maven.api.services.MavenCheckedException;

/**
* Signals an error when resolving the path to an external model.
*
*/
public class ModelResolverException extends MavenException {
public class ModelResolverException extends MavenCheckedException {

/**
* The group id of the unresolvable model.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,7 @@ Model readParent(Model childModel) throws ModelBuilderException {
return parentModel;
}

private Model resolveParent(Model childModel) {
private Model resolveParent(Model childModel) throws ModelBuilderException {
Model parentModel = null;
if (isBuildRequest()) {
parentModel = readParentLocally(childModel);
Expand All @@ -906,7 +906,7 @@ private Model resolveParent(Model childModel) {
}

private Model readParentLocally(Model childModel) throws ModelBuilderException {
ModelSource candidateSource = null;
ModelSource candidateSource;

Parent parent = childModel.getParent();
String parentPath = parent.getRelativePath();
Expand Down Expand Up @@ -1493,7 +1493,7 @@ private Model doReadRawModel() throws ModelBuilderException {
/**
* Reads the request source's parent.
*/
Model readAsParentModel() {
Model readAsParentModel() throws ModelBuilderException {
return cache(request.getSource(), PARENT, this::doReadAsParentModel);
}

Expand Down Expand Up @@ -1664,7 +1664,7 @@ private Model doLoadDependencyManagement(
importSource = modelResolver.resolveModel(
request.getSession(), repositories, dependency, new AtomicReference<>());
}
} catch (ModelBuilderException e) {
} catch (ModelBuilderException | ModelResolverException e) {
StringBuilder buffer = new StringBuilder(256);
buffer.append("Non-resolvable import POM");
if (!containsCoordinates(e.getMessage(), groupId, artifactId, version)) {
Expand Down Expand Up @@ -1719,7 +1719,8 @@ private Model doLoadDependencyManagement(
return importModel;
}

ModelSource resolveReactorModel(String groupId, String artifactId, String version) {
ModelSource resolveReactorModel(String groupId, String artifactId, String version)
throws ModelBuilderException {
Set<ModelSource> sources = mappedSources.get(new GAKey(groupId, artifactId));
if (sources != null) {
for (ModelSource source : sources) {
Expand All @@ -1733,12 +1734,50 @@ ModelSource resolveReactorModel(String groupId, String artifactId, String versio
return null;
}

private <T> T cache(String groupId, String artifactId, String version, String tag, Supplier<T> supplier) {
return cache.computeIfAbsent(groupId, artifactId, version, tag, supplier);
@FunctionalInterface
private interface ThrowingSupplier<T, E extends Exception> {
T get() throws E;
}

private static class ThrowingSupplierWrapper<T, E extends Exception> implements Supplier<T> {
private final ThrowingSupplier<T, E> throwingSupplier;

private ThrowingSupplierWrapper(ThrowingSupplier<T, E> throwingSupplier) {
this.throwingSupplier = throwingSupplier;
}

@Override
public T get() {
try {
return throwingSupplier.get();
} catch (Exception e) {
throw new RuntimeException(e);
}
}

private static <T, E extends Exception> T process(
ThrowingSupplier<T, E> throwingSupplier, Function<Supplier<T>, T> consumer) throws E {
try {
return consumer.apply(new ThrowingSupplierWrapper<>(throwingSupplier));
} catch (RuntimeException e) {
if (e.getClass().equals(RuntimeException.class) && e.getCause() != null) {
throw (E) e.getCause();
} else {
throw e;
}
}
}
}

private <T, E extends Exception> T cache(
String groupId, String artifactId, String version, String tag, ThrowingSupplier<T, E> supplier)
throws E {
return ThrowingSupplierWrapper.process(
supplier, s -> cache.computeIfAbsent(groupId, artifactId, version, tag, s));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly the kind of boiler plate code I wanted to avoid using runtime exceptions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most of this undone, we need to deal with one single exception only

Copy link
Member Author

@cstamas cstamas Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, the latest push is quite sensible. Also, have to add "boilerplate code wanted to avoid" vs all the changes revealed just by the compiler, as exception went from unchecked to checked... which one is worth more for you?


private <T> T cache(Source source, String tag, Supplier<T> supplier) {
return cache.computeIfAbsent(source, tag, supplier);
private <T, E extends Exception> T cache(Source source, String tag, ThrowingSupplier<T, E> supplier) throws E {
return ThrowingSupplierWrapper.process(supplier, s -> cache.computeIfAbsent(source, tag, s));
}

boolean isBuildRequest() {
Expand Down Expand Up @@ -1831,7 +1870,7 @@ private static List<String> getSubprojects(Model activated) {
public Model buildRawModel(ModelBuilderRequest request) throws ModelBuilderException {
ModelBuilderSessionState build = new ModelBuilderSessionState(request);
Model model = build.readRawModel();
if (((ModelProblemCollector) build).hasErrors()) {
if (build.hasErrors()) {
throw build.newModelBuilderException();
}
return model;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.apache.maven.api.PathScope;
import org.apache.maven.api.Session;
import org.apache.maven.api.services.ModelBuilder;
import org.apache.maven.api.services.ModelBuilderException;
import org.apache.maven.api.services.ModelBuilderRequest;
import org.apache.maven.api.services.ModelBuilderResult;
import org.apache.maven.api.services.ModelSource;
Expand All @@ -39,7 +40,7 @@
class TestApiStandalone {

@Test
void testStandalone() {
void testStandalone() throws ModelBuilderException {
Session session = ApiRunner.createSession();

ModelBuilder builder = session.getService(ModelBuilder.class);
Expand Down