[MNG-8142] Hidden bug: JDK profile activator throw NumberFormatEx (#1557)

If property `java.version` is in unexpected format, the activator throws `NumberFormatEx`, that in turn, is caught and reported by `DefaultProfileSelector` w/o any cause.

These should be cleanly reported instead: report that `java.version` property is in "unexpected format", and also report why was there are failure to evaluate a property activation.

Note 1: Maven allows `-Djava.version` override (!!!), this is exactly what IT MNG-3746 does, but the `NumberFormatEx` went unnoticed, was swallowed, no cause reported.

Note 2: This bug was revealed by #1555 as it reported the issue, and later "asserted error free log" which was not error-free. Hence, this bug was simply revealed by improved logging on unrelated issue.

---

https://issues.apache.org/jira/browse/MNG-8142
This commit is contained in:
Tamas Cservenak 2024-06-06 18:34:16 +02:00 committed by GitHub
parent 865072025d
commit 200dc02da8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 107 additions and 2 deletions

View File

@ -103,7 +103,8 @@ public class DefaultProfileSelector implements ProfileSelector {
}
} catch (RuntimeException e) {
problems.add(new ModelProblemCollectorRequest(Severity.ERROR, Version.BASE)
.setMessage("Failed to determine activation for profile " + profile.getId())
.setMessage(
"Failed to determine activation for profile " + profile.getId() + ": " + e.getMessage())
.setLocation(profile.getLocation(""))
.setException(e));
return false;

View File

@ -74,7 +74,16 @@ public class JdkVersionProfileActivator implements ProfileActivator {
if (jdk.startsWith("!")) {
return !version.startsWith(jdk.substring(1));
} else if (isRange(jdk)) {
return isInRange(version, getRange(jdk));
try {
return isInRange(version, getRange(jdk));
} catch (NumberFormatException e) {
problems.add(new ModelProblemCollectorRequest(Severity.WARNING, Version.BASE)
.setMessage("Failed to determine JDK activation for profile " + profile.getId()
+ " due invalid JDK version: '" + version + "'")
.setLocation(profile.getLocation(""))
.setException(e));
return false;
}
} else {
return version.startsWith(jdk);
}

View File

@ -0,0 +1,72 @@
/*
* 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.model.profile;
import java.util.Collections;
import java.util.List;
import org.apache.maven.model.Activation;
import org.apache.maven.model.Profile;
import org.apache.maven.model.building.ModelProblemCollector;
import org.apache.maven.model.building.SimpleProblemCollector;
import org.apache.maven.model.profile.activation.ProfileActivator;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
/**
* Tests {@link DefaultProfileSelector}.
*/
public class DefaultProfileSelectorTest {
private Profile newProfile(String id) {
Activation activation = new Activation();
Profile profile = new Profile();
profile.setId(id);
profile.setActivation(activation);
return profile;
}
@Test
public void testThrowingActivator() {
DefaultProfileSelector selector = new DefaultProfileSelector();
selector.addProfileActivator(new ProfileActivator() {
@Override
public boolean isActive(Profile profile, ProfileActivationContext context, ModelProblemCollector problems) {
throw new RuntimeException("BOOM");
}
@Override
public boolean presentInConfig(
Profile profile, ProfileActivationContext context, ModelProblemCollector problems) {
return true;
}
});
List<Profile> profiles = Collections.singletonList(newProfile("one"));
DefaultProfileActivationContext context = new DefaultProfileActivationContext();
SimpleProblemCollector problems = new SimpleProblemCollector();
List<Profile> active = selector.getActiveProfiles(profiles, context, problems);
assertTrue(active.isEmpty());
assertEquals(1, problems.getErrors().size());
assertEquals(
"Failed to determine activation for profile one: BOOM",
problems.getErrors().get(0));
}
}

View File

@ -22,6 +22,8 @@ import java.util.Properties;
import org.apache.maven.model.Activation;
import org.apache.maven.model.Profile;
import org.apache.maven.model.building.SimpleProblemCollector;
import org.apache.maven.model.profile.ProfileActivationContext;
/**
* Tests {@link JdkVersionProfileActivator}.
@ -161,4 +163,25 @@ public class JdkVersionProfileActivatorTest extends AbstractProfileActivatorTest
assertActivation(false, profile, newContext(null, newProperties("1.6.0_09")));
assertActivation(false, profile, newContext(null, newProperties("1.6.0_09-b03")));
}
public void testRubbishJavaVersion() {
Profile profile = newProfile("[1.8,)");
assertActivationWithProblems(profile, newContext(null, newProperties("Pūteketeke")), "invalid JDK version");
assertActivationWithProblems(profile, newContext(null, newProperties("rubbish")), "invalid JDK version");
assertActivationWithProblems(profile, newContext(null, newProperties("1.a.0_09")), "invalid JDK version");
assertActivationWithProblems(profile, newContext(null, newProperties("1.a.2.b")), "invalid JDK version");
}
private void assertActivationWithProblems(
Profile profile, ProfileActivationContext context, String warningContains) {
SimpleProblemCollector problems = new SimpleProblemCollector();
assertEquals(false, activator.isActive(profile, context, problems));
assertEquals(problems.getErrors().toString(), 0, problems.getErrors().size());
assertEquals(
problems.getWarnings().toString(), 1, problems.getWarnings().size());
assertTrue(problems.getWarnings().get(0), problems.getWarnings().get(0).contains(warningContains));
}
}