Skip to content

Introduce Converter in junit-platform-commons #4219

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

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -21,6 +21,7 @@
import org.apiguardian.api.API;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.params.converter.DefaultArgumentConverter;
import org.junit.platform.commons.support.conversion.TypeDescriptor;
import org.junit.platform.commons.util.ClassUtils;
import org.junit.platform.commons.util.Preconditions;

Expand All @@ -46,7 +47,7 @@ public static DefaultArgumentsAccessor create(ExtensionContext context, int invo
Preconditions.notNull(classLoader, "ClassLoader must not be null");

BiFunction<Object, Class<?>, Object> converter = (source, targetType) -> new DefaultArgumentConverter(context) //
.convert(source, targetType, classLoader);
.convert(source, TypeDescriptor.forType(targetType), classLoader);
return new DefaultArgumentsAccessor(converter, invocationIndex, arguments);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.junit.jupiter.params.support.FieldContext;
import org.junit.platform.commons.support.conversion.ConversionException;
import org.junit.platform.commons.support.conversion.ConversionSupport;
import org.junit.platform.commons.support.conversion.TypeDescriptor;
import org.junit.platform.commons.util.ReflectionUtils;

/**
Expand All @@ -42,7 +43,7 @@
* {@link File}, {@link BigDecimal}, {@link BigInteger}, {@link Currency},
* {@link Locale}, {@link URI}, {@link URL}, {@link UUID}, etc.
*
* <p>If the source and target types are identical the source object will not
* <p>If the source and target types are identical, the source object will not
* be modified.
*
* @since 5.0
Expand Down Expand Up @@ -81,55 +82,49 @@ public DefaultArgumentConverter(ExtensionContext context) {

@Override
public final Object convert(Object source, ParameterContext context) {
Class<?> targetType = context.getParameter().getType();
ClassLoader classLoader = getClassLoader(context.getDeclaringExecutable().getDeclaringClass());
return convert(source, targetType, classLoader);
return convert(source, TypeDescriptor.forParameter(context.getParameter()), classLoader);
}

@Override
public final Object convert(Object source, FieldContext context) throws ArgumentConversionException {
Class<?> targetType = context.getField().getType();
ClassLoader classLoader = getClassLoader(context.getField().getDeclaringClass());
return convert(source, targetType, classLoader);
return convert(source, TypeDescriptor.forField(context.getField()), classLoader);
}

public final Object convert(Object source, Class<?> targetType, ClassLoader classLoader) {
public final Object convert(Object source, TypeDescriptor targetType, ClassLoader classLoader) {
if (source == null) {
if (targetType.isPrimitive()) {
throw new ArgumentConversionException(
"Cannot convert null to primitive value of type " + targetType.getTypeName());
"Cannot convert null to primitive value of type " + targetType.getType().getTypeName());
}
return null;
}

if (ReflectionUtils.isAssignableTo(source, targetType)) {
if (ReflectionUtils.isAssignableTo(source, targetType.getType())) {
return source;
}

if (source instanceof String) {
if (targetType == Locale.class && getLocaleConversionFormat() == LocaleConversionFormat.BCP_47) {
return Locale.forLanguageTag((String) source);
}

try {
return convert((String) source, targetType, classLoader);
}
catch (ConversionException ex) {
throw new ArgumentConversionException(ex.getMessage(), ex);
}
if (source instanceof String //
&& targetType.getType() == Locale.class //
&& getLocaleConversionFormat() == LocaleConversionFormat.BCP_47) {
return Locale.forLanguageTag((String) source);
}

throw new ArgumentConversionException(
String.format("No built-in converter for source type %s and target type %s",
source.getClass().getTypeName(), targetType.getTypeName()));
try {
return delegateConversion(source, targetType, classLoader);
}
catch (ConversionException ex) {
throw new ArgumentConversionException(ex.getMessage(), ex);
}
}

private LocaleConversionFormat getLocaleConversionFormat() {
return context.getConfigurationParameter(DEFAULT_LOCALE_CONVERSION_FORMAT_PROPERTY_NAME, TRANSFORMER) //
.orElse(LocaleConversionFormat.BCP_47);
}

Object convert(String source, Class<?> targetType, ClassLoader classLoader) {
Object delegateConversion(Object source, TypeDescriptor targetType, ClassLoader classLoader) {
return ConversionSupport.convert(source, targetType, classLoader);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@

package org.junit.platform.commons.support.conversion;

import static java.util.Arrays.asList;
import static java.util.Collections.unmodifiableList;
import static org.apiguardian.api.API.Status.DEPRECATED;
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
import static org.junit.platform.commons.util.ReflectionUtils.getWrapperType;

import java.util.List;
import java.util.Optional;
import java.util.ServiceLoader;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

import org.apiguardian.api.API;
import org.junit.platform.commons.util.ClassLoaderUtils;
Expand All @@ -30,17 +29,6 @@
@API(status = EXPERIMENTAL, since = "1.11")
public final class ConversionSupport {

private static final List<StringToObjectConverter> stringToObjectConverters = unmodifiableList(asList( //
new StringToBooleanConverter(), //
new StringToCharacterConverter(), //
new StringToNumberConverter(), //
new StringToClassConverter(), //
new StringToEnumConverter(), //
new StringToJavaTimeConverter(), //
new StringToCommonJavaTypesConverter(), //
new FallbackStringToObjectConverter() //
));

private ConversionSupport() {
/* no-op */
}
Expand All @@ -49,43 +37,6 @@ private ConversionSupport() {
* Convert the supplied source {@code String} into an instance of the specified
* target type.
*
* <p>If the target type is {@code String}, the source {@code String} will not
* be modified.
*
* <p>Some forms of conversion require a {@link ClassLoader}. If none is
* provided, the {@linkplain ClassLoaderUtils#getDefaultClassLoader() default
* ClassLoader} will be used.
*
* <p>This method is able to convert strings into primitive types and their
* corresponding wrapper types ({@link Boolean}, {@link Character}, {@link Byte},
* {@link Short}, {@link Integer}, {@link Long}, {@link Float}, and
* {@link Double}), enum constants, date and time types from the
* {@code java.time} package, as well as common Java types such as {@link Class},
* {@link java.io.File}, {@link java.nio.file.Path}, {@link java.nio.charset.Charset},
* {@link java.math.BigDecimal}, {@link java.math.BigInteger},
* {@link java.util.Currency}, {@link java.util.Locale}, {@link java.util.UUID},
* {@link java.net.URI}, and {@link java.net.URL}.
*
* <p>If the target type is not covered by any of the above, a convention-based
* conversion strategy will be used to convert the source {@code String} into the
* given target type by invoking a static factory method or factory constructor
* defined in the target type. The search algorithm used in this strategy is
* outlined below.
*
* <h4>Search Algorithm</h4>
*
* <ol>
* <li>Search for a single, non-private static factory method in the target
* type that converts from a String to the target type. Use the factory method
* if present.</li>
* <li>Search for a single, non-private constructor in the target type that
* accepts a String. Use the constructor if present.</li>
* </ol>
*
* <p>If multiple suitable factory methods are discovered they will be ignored.
* If neither a single factory method nor a single constructor is found, the
* convention-based conversion strategy will not apply.
*
* @param source the source {@code String} to convert; may be {@code null}
* but only if the target type is a reference type
* @param targetType the target type the source should be converted into;
Expand All @@ -97,48 +48,50 @@ private ConversionSupport() {
* type is a reference type
*
* @since 1.11
* @see DefaultConverter
* @deprecated Use {@link #convert(Object, TypeDescriptor, ClassLoader)} instead.
*/
@SuppressWarnings("unchecked")
@Deprecated
@API(status = DEPRECATED, since = "5.13")
public static <T> T convert(String source, Class<T> targetType, ClassLoader classLoader) {
if (source == null) {
if (targetType.isPrimitive()) {
throw new ConversionException(
"Cannot convert null to primitive value of type " + targetType.getTypeName());
}
return null;
}
return convert(source, TypeDescriptor.forType(targetType), getClassLoader(classLoader));
Copy link
Contributor Author

@scordio scordio May 11, 2025

Choose a reason for hiding this comment

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

I wonder if the deprecated convert should delegate to the other convert method or should invoke DefaultConverter directly, skipping the new service loader logic. For now, I went with the former.

}

if (String.class.equals(targetType)) {
return (T) source;
}
/**
* Convert the supplied source object into an instance of the specified
* target type.
*
* @param source the source object to convert; may be {@code null}
* but only if the target type is a reference type
* @param targetType the target type the source should be converted into;
* never {@code null}
* @param classLoader the {@code ClassLoader} to use; may be {@code null} to
* use the default {@code ClassLoader}
* @param <T> the type of the target
* @return the converted object; may be {@code null} but only if the target
* type is a reference type
*
* @since 1.13
*/
@API(status = EXPERIMENTAL, since = "1.13")
@SuppressWarnings("unchecked")
public static <T> T convert(Object source, TypeDescriptor targetType, ClassLoader classLoader) {
ClassLoader classLoaderToUse = getClassLoader(classLoader);
ServiceLoader<Converter> serviceLoader = ServiceLoader.load(Converter.class, classLoaderToUse);

Class<?> targetTypeToUse = toWrapperType(targetType);
Optional<StringToObjectConverter> converter = stringToObjectConverters.stream().filter(
candidate -> candidate.canConvertTo(targetTypeToUse)).findFirst();
if (converter.isPresent()) {
try {
ClassLoader classLoaderToUse = classLoader != null ? classLoader
: ClassLoaderUtils.getDefaultClassLoader();
return (T) converter.get().convert(source, targetTypeToUse, classLoaderToUse);
}
catch (Exception ex) {
if (ex instanceof ConversionException) {
// simply rethrow it
throw (ConversionException) ex;
}
// else
throw new ConversionException(
String.format("Failed to convert String \"%s\" to type %s", source, targetType.getTypeName()), ex);
}
}
Converter converter = Stream.concat( //
StreamSupport.stream(serviceLoader.spliterator(), false), //
Stream.of(DefaultConverter.INSTANCE)) //
.filter(candidate -> candidate.canConvert(source, targetType)) //
.findFirst() //
.orElseThrow(() -> new ConversionException("No registered or built-in converter for source '" + source
+ "' and target type " + targetType.getType().getTypeName()));

throw new ConversionException(
"No built-in converter for source type java.lang.String and target type " + targetType.getTypeName());
return (T) converter.convert(source, targetType, classLoaderToUse);
}

private static Class<?> toWrapperType(Class<?> targetType) {
Class<?> wrapperType = getWrapperType(targetType);
return wrapperType != null ? wrapperType : targetType;
private static ClassLoader getClassLoader(ClassLoader classLoader) {
return classLoader != null ? classLoader : ClassLoaderUtils.getDefaultClassLoader();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright 2015-2025 the original author or authors.
*
* All rights reserved. This program and the accompanying materials are
* made available under the terms of the Eclipse Public License v2.0 which
* accompanies this distribution and is available at
*
* https://www.eclipse.org/legal/epl-v20.html
*/

package org.junit.platform.commons.support.conversion;

import static org.apiguardian.api.API.Status.EXPERIMENTAL;

import org.apiguardian.api.API;

/**
* {@code Converter} is an abstraction that allows an input object to
* be converted to an instance of a different class.
*
* <p>Implementations are loaded via the {@link java.util.ServiceLoader} and must
* follow the service provider requirements. They should not make any assumptions
* regarding when they are instantiated or how often they are called. Since
* instances may potentially be cached and called from different threads, they
* should be thread-safe.
*
* <p>Extend {@link TypedConverter} if your implementation always converts
* from a given source type into a given target type and does not need access to
* the {@link ClassLoader} to perform the conversion.
*
* @since 1.13
* @see ConversionSupport
* @see TypedConverter
*/
@API(status = EXPERIMENTAL, since = "1.13")
public interface Converter {
Copy link
Member

Choose a reason for hiding this comment

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

Didn't we decide to make this generic as in Converter<S, T>?

Copy link
Member

Choose a reason for hiding this comment

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

I guess for that to make sense, we'd have to parameterize TypeDescriptor as well.

@scordio @sbrannen WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same in mind but wasn't entirely confident about this direction yesterday, especially before refactoring StringToObjectConversion.

But now the refactoring is done so I can sketch it and see what happens 🙂


/**
* Determine if the supplied source object can be converted into an instance
* of the specified target type.
*
* @param source the source object to convert; may be {@code null} but only
* if the target type is a reference type
* @param targetType the descriptor of the type the source should be converted into;
* never {@code null}
* @return {@code true} if the supplied source can be converted
*/
boolean canConvert(Object source, TypeDescriptor targetType);
Copy link
Contributor Author

@scordio scordio May 11, 2025

Choose a reason for hiding this comment

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

Initially, this was:

Suggested change
boolean canConvert(Object source, TypeDescriptor targetType);
boolean canConvert(Object source, TypeDescriptor targetType, ClassLoader classLoader);

However, I couldn't see the benefit of the ClassLoader parameter here, so I deleted it. Let me know if you see some use cases that could benefit from it.

Plus, I'm not entirely sure if this should rather be:

Suggested change
boolean canConvert(Object source, TypeDescriptor targetType);
boolean canConvert(TypeDescriptor sourceType, TypeDescriptor targetType);

The way Converter is currently integrated into ConversionSupport makes it easy to provide the source instance, but I wonder if this will be flexible enough with a bit of foresight.

Copy link
Member

@marcphilipp marcphilipp May 12, 2025

Choose a reason for hiding this comment

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

I think we should pass an additional TypeDescriptor sourceType to both canConvert and convert.


/**
* Convert the supplied source object into an instance of the specified
* target type.
*
* <p>This method will only be invoked if {@link #canConvert(Object, TypeDescriptor)}
* returned {@code true} for the same target type.
*
* @param source the source object to convert; may be {@code null} but only
* if the target type is a reference type
* @param targetType the descriptor of the type the source should be converted into;
* never {@code null}
* @param classLoader the {@code ClassLoader} to use; never {@code null}
* @return the converted object; may be {@code null} but only if the target
* type is a reference type
* @throws ConversionException if an error occurs during the conversion
*/
Object convert(Object source, TypeDescriptor targetType, ClassLoader classLoader) throws ConversionException;

}
Loading
Loading