-
Notifications
You must be signed in to change notification settings - Fork 3k
Support sealed parameter implementation hints #12117
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,9 @@ | |||||||||||||||||||
| */ | ||||||||||||||||||||
| package org.apache.maven.configuration.internal; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||||||
| import java.util.Comparator; | ||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||
| import java.util.concurrent.ConcurrentHashMap; | ||||||||||||||||||||
| import java.util.concurrent.ConcurrentMap; | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -87,7 +90,7 @@ public Object fromConfiguration( | |||||||||||||||||||
| return value; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| final Class<?> implType = getClassForImplementationHint(type, configuration, loader); | ||||||||||||||||||||
| final Class<?> implType = resolveClassForImplementationHint(type, configuration, loader); | ||||||||||||||||||||
| if (null == value && implType.isInterface() && configuration.getChildCount() == 0) { | ||||||||||||||||||||
| return null; // nothing to process | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
@@ -108,6 +111,56 @@ public Object fromConfiguration( | |||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private Class<?> resolveClassForImplementationHint( | ||||||||||||||||||||
| final Class<?> type, final PlexusConfiguration configuration, final ClassLoader loader) | ||||||||||||||||||||
| throws ComponentConfigurationException { | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| return super.getClassForImplementationHint(type, configuration, loader); | ||||||||||||||||||||
| } catch (final ComponentConfigurationException e) { | ||||||||||||||||||||
| if (type == null || !type.isSealed()) { | ||||||||||||||||||||
| throw e; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| final String implementation = configuration.getAttribute("implementation"); | ||||||||||||||||||||
| if (implementation == null || implementation.isEmpty()) { | ||||||||||||||||||||
| throw e; | ||||||||||||||||||||
| } | ||||||||||||||||||||
| return getPermittedSubclass(type, implementation, configuration, e); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private Class<?> getPermittedSubclass( | ||||||||||||||||||||
| final Class<?> type, | ||||||||||||||||||||
| final String implementation, | ||||||||||||||||||||
| final PlexusConfiguration configuration, | ||||||||||||||||||||
| final ComponentConfigurationException cause) | ||||||||||||||||||||
| throws ComponentConfigurationException { | ||||||||||||||||||||
| final List<Class<?>> matches = new ArrayList<>(); | ||||||||||||||||||||
| for (Class<?> permittedSubclass : type.getPermittedSubclasses()) { | ||||||||||||||||||||
| if (implementation.equals(permittedSubclass.getName()) | ||||||||||||||||||||
| || implementation.equals(permittedSubclass.getCanonicalName()) | ||||||||||||||||||||
| || implementation.equals(permittedSubclass.getSimpleName())) { | ||||||||||||||||||||
| matches.add(permittedSubclass); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (matches.size() == 1) { | ||||||||||||||||||||
| return matches.get(0); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| matches.sort(Comparator.comparing(Class::getName)); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (matches.isEmpty()) { | ||||||||||||||||||||
| throw new ComponentConfigurationException( | ||||||||||||||||||||
| configuration, | ||||||||||||||||||||
| "Cannot find permitted subclass '" + implementation + "' for sealed type " + type.getName(), | ||||||||||||||||||||
| cause); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| throw new ComponentConfigurationException( | ||||||||||||||||||||
| configuration, | ||||||||||||||||||||
| "Implementation hint '" + implementation + "' is ambiguous for sealed type " + type.getName() + ": " | ||||||||||||||||||||
| + matches.stream().map(Class::getName).toList()); | ||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: the "not found" path includes
Suggested change
|
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public void processConfiguration( | ||||||||||||||||||||
| final ConverterLookup lookup, | ||||||||||||||||||||
| final Object bean, | ||||||||||||||||||||
|
|
@@ -122,7 +175,7 @@ public void processConfiguration( | |||||||||||||||||||
| final String propertyName = fromXML(element.getName()); | ||||||||||||||||||||
| Class<?> valueType; | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| valueType = getClassForImplementationHint(null, element, loader); | ||||||||||||||||||||
| valueType = resolveClassForImplementationHint(null, element, loader); | ||||||||||||||||||||
| } catch (final ComponentConfigurationException e) { | ||||||||||||||||||||
| valueType = null; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,6 +31,8 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.junit.jupiter.api.Test; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static org.junit.jupiter.api.Assertions.assertInstanceOf; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+34
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -108,8 +110,72 @@ void testChildConfigurationElement() throws BeanConfigurationException { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertEquals(new File("test"), bean.file); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void testSealedTypeImplementationHintCanUseSimpleName() throws BeanConfigurationException { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SealedBean bean = new SealedBean(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Xpp3Dom config = toConfig("<artifact implementation=\"LocalArtifact\"><name>local</name></artifact>"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DefaultBeanConfigurationRequest request = new DefaultBeanConfigurationRequest(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request.setBean(bean).setConfiguration(config); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configurator.configureBean(request); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LocalArtifact artifact = assertInstanceOf(LocalArtifact.class, bean.artifact); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertEquals("local", artifact.name); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void testSealedTypeImplementationHintCanStillUseClassName() throws BeanConfigurationException { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SealedBean bean = new SealedBean(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Xpp3Dom config = toConfig("<artifact implementation=\"" + RemoteArtifact.class.getName() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| + "\"><url>https://example.invalid/artifact</url></artifact>"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DefaultBeanConfigurationRequest request = new DefaultBeanConfigurationRequest(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request.setBean(bean).setConfiguration(config); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| configurator.configureBean(request); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| RemoteArtifact artifact = assertInstanceOf(RemoteArtifact.class, bean.artifact); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertEquals("https://example.invalid/artifact", artifact.url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @Test | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| void testSealedTypeImplementationHintMustMatchPermittedSubclass() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SealedBean bean = new SealedBean(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Xpp3Dom config = toConfig("<artifact implementation=\"MissingArtifact\"><name>missing</name></artifact>"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DefaultBeanConfigurationRequest request = new DefaultBeanConfigurationRequest(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| request.setBean(bean).setConfiguration(config); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BeanConfigurationException e = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertThrows(BeanConfigurationException.class, () -> configurator.configureBean(request)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assertEquals( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Cannot find permitted subclass 'MissingArtifact' for sealed type " + SealedArtifact.class.getName(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| e.getMessage()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| static class SomeBean { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| File file; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| static class SealedBean { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SealedArtifact artifact; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public sealed interface SealedArtifact permits LocalArtifact, RemoteArtifact {} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static final class LocalArtifact implements SealedArtifact { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String name; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| public static final class RemoteArtifact implements SealedArtifact { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String url; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+173
to
213
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a test for the ambiguity code path (two permitted subclasses sharing the same simple name). This also requires adding the helper types:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| /* | ||
| * 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.it; | ||
|
|
||
| import java.io.File; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| /** | ||
| * This is a test set for <a href="https://github.com/apache/maven/issues/11378">GH-11378</a>. | ||
| */ | ||
| public class MavenITgh11378SealedParameterConfigTest extends AbstractMavenIntegrationTestCase { | ||
|
|
||
| @Test | ||
| public void testSealedParameterImplementationCanUseSimpleName() throws Exception { | ||
| File testDir = extractResources("/gh-11378-sealed-parameter-config"); | ||
|
|
||
| Verifier parentVerifier = newVerifier(testDir.getAbsolutePath(), false); | ||
| parentVerifier.addCliArgument("-N"); | ||
| parentVerifier.addCliArgument("install"); | ||
| parentVerifier.execute(); | ||
| parentVerifier.verifyErrorFreeLog(); | ||
|
|
||
| File pluginDir = new File(testDir, "plugin"); | ||
| Verifier pluginVerifier = newVerifier(pluginDir.getAbsolutePath(), false); | ||
| pluginVerifier.getSystemProperties().put("maven.compiler.source", "17"); | ||
| pluginVerifier.getSystemProperties().put("maven.compiler.target", "17"); | ||
| pluginVerifier.getSystemProperties().put("maven.compiler.release", "17"); | ||
| pluginVerifier.addCliArgument("install"); | ||
| pluginVerifier.execute(); | ||
| pluginVerifier.verifyErrorFreeLog(); | ||
|
|
||
| File consumerDir = new File(testDir, "consumer"); | ||
| Verifier verifier = newVerifier(consumerDir.getAbsolutePath(), false); | ||
| verifier.addCliArgument("test:test-goal"); | ||
| verifier.execute(); | ||
| verifier.verifyErrorFreeLog(); | ||
| verifier.verifyTextInLog("Configured sealed artifact: local"); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <parent> | ||
| <groupId>org.apache.maven.its.gh11378</groupId> | ||
| <artifactId>test-project</artifactId> | ||
| <version>0.0.1-SNAPSHOT</version> | ||
| </parent> | ||
|
|
||
| <artifactId>consumer</artifactId> | ||
|
|
||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.apache.maven.its.gh11378</groupId> | ||
| <artifactId>test-plugin</artifactId> | ||
| <version>0.0.1-SNAPSHOT</version> | ||
| <configuration> | ||
| <artifact implementation="LocalArtifact"> | ||
| <name>local</name> | ||
| </artifact> | ||
| </configuration> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <parent> | ||
| <groupId>org.apache.maven.its.gh11378</groupId> | ||
| <artifactId>test-project</artifactId> | ||
| <version>0.0.1-SNAPSHOT</version> | ||
| </parent> | ||
|
|
||
| <artifactId>test-plugin</artifactId> | ||
| <packaging>maven-plugin</packaging> | ||
|
|
||
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <maven.version>4.1.0-SNAPSHOT</maven.version> | ||
| <maven-plugin-plugin.version>4.0.0-beta-1</maven-plugin-plugin.version> | ||
| <maven.compiler.release>17</maven.compiler.release> | ||
| </properties> | ||
|
|
||
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.apache.maven</groupId> | ||
| <artifactId>maven-api-core</artifactId> | ||
| <version>${maven.version}</version> | ||
| <scope>provided</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.maven</groupId> | ||
| <artifactId>maven-api-di</artifactId> | ||
| <version>${maven.version}</version> | ||
| <scope>provided</scope> | ||
| </dependency> | ||
| </dependencies> | ||
|
|
||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-plugin-plugin</artifactId> | ||
| <version>${maven-plugin-plugin.version}</version> | ||
| <configuration> | ||
| <goalPrefix>test</goalPrefix> | ||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <id>default-descriptor</id> | ||
| <goals> | ||
| <goal>descriptor</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| /* | ||
| * 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.its.gh11378; | ||
|
|
||
| import org.apache.maven.api.di.Inject; | ||
| import org.apache.maven.api.plugin.Log; | ||
| import org.apache.maven.api.plugin.annotations.Mojo; | ||
| import org.apache.maven.api.plugin.annotations.Parameter; | ||
|
|
||
| @Mojo(name = "test-goal") | ||
| public class TestMojo implements org.apache.maven.api.plugin.Mojo { | ||
|
|
||
| @Inject | ||
| private Log log; | ||
|
|
||
| @Parameter | ||
| private Artifact artifact; | ||
|
|
||
| @Override | ||
| public void execute() { | ||
| if (!(artifact instanceof LocalArtifact)) { | ||
| throw new IllegalStateException("Expected LocalArtifact but got " + artifact); | ||
| } | ||
| LocalArtifact localArtifact = (LocalArtifact) artifact; | ||
| if (!"local".equals(localArtifact.name)) { | ||
| throw new IllegalStateException("Expected artifact name 'local' but got '" + localArtifact.name + "'"); | ||
| } | ||
| log.info("Configured sealed artifact: " + localArtifact.name); | ||
| } | ||
|
|
||
| public sealed interface Artifact permits LocalArtifact, RemoteArtifact {} | ||
|
|
||
| public static final class LocalArtifact implements Artifact { | ||
|
|
||
| public String name; | ||
| } | ||
|
|
||
| public static final class RemoteArtifact implements Artifact { | ||
|
|
||
| public String url; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| <groupId>org.apache.maven.its.gh11378</groupId> | ||
| <artifactId>test-project</artifactId> | ||
| <version>0.0.1-SNAPSHOT</version> | ||
| <packaging>pom</packaging> | ||
|
|
||
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| </properties> | ||
|
|
||
| <modules> | ||
| <module>plugin</module> | ||
| <module>consumer</module> | ||
| </modules> | ||
| </project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: sorting before the
isEmpty()check is harmless but reads oddly. MovingisEmpty()first makes the intent clearer: