Skip to content

Commit

Permalink
Issue #268: More improvements in constructor selection for signals
Browse files Browse the repository at this point in the history
- Improved detection of constructors when compatible types are used
  instead of equal types (e.g. CharSequence and String)
- Improved handling of primitive and wrapper array type (int[] /
  Integer[])
  • Loading branch information
hypfvieh committed Sep 15, 2024
1 parent 10becbd commit 63acbf2
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 66 deletions.
43 changes: 16 additions & 27 deletions dbus-java-core/src/main/java/org/freedesktop/dbus/ArrayFrob.java
Original file line number Diff line number Diff line change
@@ -1,46 +1,34 @@
package org.freedesktop.dbus;

import org.freedesktop.dbus.utils.PrimitiveUtils;
import org.slf4j.LoggerFactory;

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

public final class ArrayFrob {
private static final Map<Class<?>, Class<?>> PRIMITIVE_TO_WRAPPER = new ConcurrentHashMap<>();
private static final Map<Class<?>, Class<?>> WRAPPER_TO_PRIMITIVE = new ConcurrentHashMap<>();
static {
PRIMITIVE_TO_WRAPPER.put(Boolean.TYPE, Boolean.class);
PRIMITIVE_TO_WRAPPER.put(Byte.TYPE, Byte.class);
PRIMITIVE_TO_WRAPPER.put(Short.TYPE, Short.class);
PRIMITIVE_TO_WRAPPER.put(Character.TYPE, Character.class);
PRIMITIVE_TO_WRAPPER.put(Integer.TYPE, Integer.class);
PRIMITIVE_TO_WRAPPER.put(Long.TYPE, Long.class);
PRIMITIVE_TO_WRAPPER.put(Float.TYPE, Float.class);
PRIMITIVE_TO_WRAPPER.put(Double.TYPE, Double.class);
WRAPPER_TO_PRIMITIVE.put(Boolean.class, Boolean.TYPE);
WRAPPER_TO_PRIMITIVE.put(Byte.class, Byte.TYPE);
WRAPPER_TO_PRIMITIVE.put(Short.class, Short.TYPE);
WRAPPER_TO_PRIMITIVE.put(Character.class, Character.TYPE);
WRAPPER_TO_PRIMITIVE.put(Integer.class, Integer.TYPE);
WRAPPER_TO_PRIMITIVE.put(Long.class, Long.TYPE);
WRAPPER_TO_PRIMITIVE.put(Float.class, Float.TYPE);
WRAPPER_TO_PRIMITIVE.put(Double.class, Double.TYPE);
}

private ArrayFrob() {
}

/**
* @deprecated use {@link PrimitiveUtils#getPrimitiveToWrapperTypes()}
* @return Map with primitive type and wrapper types
*/
@Deprecated(since = "5.1.1 - 2024-09-15")
public static Map<Class<?>, Class<?>> getPrimitiveToWrapperTypes() {
return Collections.unmodifiableMap(PRIMITIVE_TO_WRAPPER);
return PrimitiveUtils.getPrimitiveToWrapperTypes();
}

/**
* @deprecated use {@link PrimitiveUtils#getWrapperToPrimitiveTypes()}
* @return Map with wrapper type and primitive type
*/
@Deprecated(since = "5.1.1 - 2024-09-15")
public static Map<Class<?>, Class<?>> getWrapperToPrimitiveTypes() {
return Collections.unmodifiableMap(WRAPPER_TO_PRIMITIVE);
return PrimitiveUtils.getWrapperToPrimitiveTypes();
}

@SuppressWarnings("unchecked")
Expand All @@ -50,7 +38,7 @@ public static <T> T[] wrap(Object _o) throws IllegalArgumentException {
throw new IllegalArgumentException("Not an array");
}
Class<? extends Object> cc = ac.getComponentType();
Class<? extends Object> ncc = PRIMITIVE_TO_WRAPPER.get(cc);
Class<? extends Object> ncc = PrimitiveUtils.getPrimitiveToWrapperTypes().get(cc);
if (null == ncc) {
throw new IllegalArgumentException("Not a primitive type");
}
Expand All @@ -65,7 +53,7 @@ public static <T> T[] wrap(Object _o) throws IllegalArgumentException {
public static <T> Object unwrap(T[] _ns) throws IllegalArgumentException {
Class<? extends T[]> ac = (Class<? extends T[]>) _ns.getClass();
Class<T> cc = (Class<T>) ac.getComponentType();
Class<? extends Object> ncc = WRAPPER_TO_PRIMITIVE.get(cc);
Class<? extends Object> ncc = PrimitiveUtils.getWrapperToPrimitiveTypes().get(cc);
if (null == ncc) {
throw new IllegalArgumentException("Not a wrapper type");
}
Expand Down Expand Up @@ -177,4 +165,5 @@ public static Object[] type(Object[] _old, Class<Object> _c) {
System.arraycopy(_old, 0, ns, 0, ns.length);
return ns;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.freedesktop.dbus.types.*;
import org.freedesktop.dbus.utils.Hexdump;
import org.freedesktop.dbus.utils.LoggingHelper;
import org.freedesktop.dbus.utils.PrimitiveUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -1370,41 +1371,56 @@ private Object[] extractArgs(List<Type[]> _constructorArgs) throws DBusException
* @return List of ConstructorArgType
*/
static List<ConstructorArgType> usesPrimitives(List<Type[]> _constructorArgs, List<Type> _dataType) {
Logger logger = LoggerFactory.getLogger(Message.class);
OUTER: for (Type[] ptype : _constructorArgs) {
if (ptype.length == _dataType.size()) {
List<ConstructorArgType> argTypes = new ArrayList<>();

for (int i = 0; i < ptype.length; i++) {
logger.trace(">>>>>> Comparing {} with {}", ptype[i], _dataType.get(i));
// this is a list type and an array should be used
if (ptype[i] instanceof Class<?> clz && clz.isArray()
if (ptype[i] instanceof Class<?> constructorClz && constructorClz.isArray()
&& _dataType.get(i) instanceof ParameterizedType pt
&& pt.getRawType() == List.class
&& pt.getActualTypeArguments() != null
&& pt.getActualTypeArguments().length > 0
&& pt.getActualTypeArguments()[0] instanceof Class<?> tClz) {

argTypes.add(tClz.isPrimitive() ? ConstructorArgType.PRIMITIVE_ARRAY : ConstructorArgType.ARRAY);
&& pt.getActualTypeArguments()[0] instanceof Class<?> sigExpectedClz) {

logger.trace("Found List type when array was required, trying to find proper array type");
if (PrimitiveUtils.isCompatiblePrimitiveOrWrapper(constructorClz.getComponentType(), sigExpectedClz)) {
ConstructorArgType type = constructorClz.getComponentType().isPrimitive() ? ConstructorArgType.PRIMITIVE_ARRAY : ConstructorArgType.ARRAY;
logger.trace("Selecting {} for parameter {} <=> {}", type, constructorClz.getComponentType(), sigExpectedClz);
argTypes.add(type);
} else {
logger.trace("List uses a different type than required. Found {}, required {}", constructorClz.getComponentType(), sigExpectedClz);
continue OUTER;
}
} else if (ptype[i] instanceof Class<?> clz
&& _dataType.get(i) instanceof ParameterizedType pt
&& clz == pt.getRawType()
&& clz.isAssignableFrom((Class<?>) pt.getRawType())
&& Collection.class.isAssignableFrom(clz)) {

logger.trace("Found compatible collection type: {} <=> {}", clz, pt.getRawType());
// the constructor wants some sort of collection
argTypes.add(ConstructorArgType.COLLECTION);

} else if (ptype[i] instanceof Class<?> clz
&& _dataType.get(i) instanceof Class<?> tClz
&& clz != tClz) {
} else if (ptype[i] instanceof Class<?> constructorClz
&& _dataType.get(i) instanceof Class<?> sigExpectedClz
&& !sigExpectedClz.isAssignableFrom(constructorClz)
&& !PrimitiveUtils.isCompatiblePrimitiveOrWrapper(constructorClz, sigExpectedClz)
) {
logger.trace("Constructor data type mismatch, must be wrong constructor ({} != {})", constructorClz, sigExpectedClz);
// constructor class type does not match, must be wrong constructor, try next
continue OUTER;
} else {
// not a list/array and type matches, no conversion needed
logger.trace("Type {} is not an array type, skipping", ptype[i]);
argTypes.add(ConstructorArgType.NOT_ARRAY_TYPE);
}
}
return argTypes;
}
}
logger.trace("No matching constructor arguments found");
return List.of();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package org.freedesktop.dbus.utils;

import java.util.Collections;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

/**
* Utility class containing methods dealing with object and primitive class types.
* @since 5.1.1 - 2024-09-15
* @author hypfvieh
*/
public final class PrimitiveUtils {
private static final Map<Class<?>, Class<?>> PRIMITIVE_TO_WRAPPER = Collections.unmodifiableMap(
new ConcurrentHashMap<>(
Map.of(
Boolean.TYPE, Boolean.class,
Byte.TYPE, Byte.class,
Short.TYPE, Short.class,
Character.TYPE, Character.class,
Integer.TYPE, Integer.class,
Long.TYPE, Long.class,
Float.TYPE, Float.class,
Double.TYPE, Double.class)
)
);

private static final Map<Class<?>, Class<?>> WRAPPER_TO_PRIMITIVE = Collections.unmodifiableMap(
new ConcurrentHashMap<>(
Map.of(
Boolean.class, Boolean.TYPE,
Byte.class, Byte.TYPE,
Short.class, Short.TYPE,
Character.class, Character.TYPE,
Integer.class, Integer.TYPE,
Long.class, Long.TYPE,
Float.class, Float.TYPE,
Double.class, Double.TYPE)
)
);

private PrimitiveUtils() {

}

/**
* Map with all primitives and the corresponding wrapper type.
* @return unmodifiable map
*/
public static Map<Class<?>, Class<?>> getPrimitiveToWrapperTypes() {
return Collections.unmodifiableMap(PRIMITIVE_TO_WRAPPER);
}

/**
* Map with all wrapper types and the corresponding primitive.
* @return unmodifiable map
*/
public static Map<Class<?>, Class<?>> getWrapperToPrimitiveTypes() {
return Collections.unmodifiableMap(WRAPPER_TO_PRIMITIVE);
}

/**
* Check if the given classes are equal or are equal wrapper types (e.g. byte == Byte).
*
* @param _clz1
* @param _clz2
* @return true if classes equal or both of compatible (wrapper) types,
* false otherwise or if any parameter is null
*/
public static boolean isCompatiblePrimitiveOrWrapper(Class<?> _clz1, Class<?> _clz2) {
if (_clz1 == null || _clz2 == null) {
return false;
}

if (_clz1 == _clz2) {
return true;
} else if (_clz1.isPrimitive() && _clz2.isPrimitive() && _clz1 == _clz2) {
return true;
} else if (_clz1.isPrimitive()) {
Class<?> wrappedType = PRIMITIVE_TO_WRAPPER.get(_clz1);
return wrappedType == _clz2;
} else if (_clz2.isPrimitive()) {
Class<?> wrappedType = PRIMITIVE_TO_WRAPPER.get(_clz2);
return wrappedType == _clz1;
}

return false;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@
import org.freedesktop.dbus.messages.Message.ConstructorArgType;
import org.freedesktop.dbus.test.AbstractBaseTest;
import org.freedesktop.dbus.types.DBusListType;
import org.freedesktop.dbus.types.DBusMapType;
import org.freedesktop.dbus.types.UInt32;
import org.freedesktop.dbus.types.Variant;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.lang.reflect.Type;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;

public class MessageTest extends AbstractBaseTest {
Expand Down Expand Up @@ -57,6 +60,10 @@ public void testReadMessageHeader() throws Exception {

static Stream<ParameterData> parameterSource() {
return Stream.of(
new ParameterData("Complex constructor", List.of(new Type[] {long.class, String.class, byte[].class, String.class, Map.class}, new Type[] {String.class}),
List.of(Long.class, String.class, new DBusListType(Byte.class), String.class, new DBusMapType(CharSequence.class, Variant.class)),
List.of(ConstructorArgType.NOT_ARRAY_TYPE, ConstructorArgType.NOT_ARRAY_TYPE, ConstructorArgType.PRIMITIVE_ARRAY,
ConstructorArgType.NOT_ARRAY_TYPE, ConstructorArgType.NOT_ARRAY_TYPE)),
new ParameterData("Byte array constructor",
List.of(new Type[] {Byte[].class, String.class}, new Type[] {Integer.class, String.class}),
List.of(new DBusListType(Byte.class), String.class),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package org.freedesktop.dbus.utils;

import org.freedesktop.dbus.test.AbstractBaseTest;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;

import java.util.stream.Stream;

public class PrimitiveUtilsTest extends AbstractBaseTest {

static Stream<PrimitiveWrapperTestData> createComparePrimitiveWrapperData() {
return Stream.of(
new PrimitiveWrapperTestData("Equal types", Boolean.class, Boolean.class, true),
new PrimitiveWrapperTestData("Equal primitve types", boolean.class, boolean.class, true),
new PrimitiveWrapperTestData("Equal object and primitive", Boolean.class, boolean.class, true),
new PrimitiveWrapperTestData("Equal primitive and object", Boolean.class, boolean.class, true),
new PrimitiveWrapperTestData("Unequal types", Boolean.class, String.class, false),
new PrimitiveWrapperTestData("Unequal primitive types", int.class, long.class, false),
new PrimitiveWrapperTestData("Unequal primitive and object", boolean.class, String.class, false),
new PrimitiveWrapperTestData("Unequal object and primitive", Byte.class, int.class, false)
);
}

@ParameterizedTest(name = "{0}")
@MethodSource("createComparePrimitiveWrapperData")
void testComparePrimitiveWrapper(PrimitiveWrapperTestData _testData) {
assertEquals(_testData.expected(), PrimitiveUtils.isCompatiblePrimitiveOrWrapper(_testData.clz1(), _testData.clz2()));
}

record PrimitiveWrapperTestData(String name, Class<?> clz1, Class<?> clz2, boolean expected) {
@Override
public String toString() {
return name;
}
}
}
29 changes: 0 additions & 29 deletions dbus-java-tests/src/test/java/sample/issue/Issue268Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,19 +82,6 @@ class MessageReceivedV2 extends DBusSignal {
private final String message;
private final Map<String, Variant<?>> extras;

public MessageReceivedV2(
String _objectpath, long _timestamp,
String _sender, String _groupId,
String _message, Map<String, Variant<?>> _extras) throws DBusException {

super(_objectpath, _timestamp, _sender, _groupId, _message, _extras);
this.timestamp = _timestamp;
this.sender = _sender;
this.groupId = _groupId.getBytes();
this.message = _message;
this.extras = _extras;
}

public MessageReceivedV2(
String _objectpath, long _timestamp,
String _sender, byte[] _groupId,
Expand All @@ -108,22 +95,6 @@ public MessageReceivedV2(
this.extras = _extras;
}

public MessageReceivedV2(
String _objectpath, long _timestamp,
String _sender, List<Byte> _groupId,
String _message, Map<String, Variant<?>> _extras) throws DBusException {

super(_objectpath, _timestamp, _sender, _groupId, _message, _extras);
this.timestamp = _timestamp;
this.sender = _sender;
this.groupId = new byte[_groupId.size()];
for (int i = 0; i < groupId.length; i++) {
groupId[i] = _groupId.get(i);
}
this.message = _message;
this.extras = _extras;
}

public long getTimestamp() {
return timestamp;
}
Expand Down
2 changes: 1 addition & 1 deletion dbus-java-tests/src/test/resources/logback-test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@
<appender-ref ref="STDOUT" />
</root>

</configuration>
</configuration>

0 comments on commit 63acbf2

Please sign in to comment.