From 46ddab0b94c3c05b0a6fdb4c39757844059e0fdb Mon Sep 17 00:00:00 2001 From: David M Date: Sun, 22 Jun 2025 14:45:16 +0200 Subject: [PATCH 1/5] Improvements for InterfaceCodeGenerator --- .../utils/generator/ClassBuilderInfo.java | 6 +++- .../generator/InterfaceCodeGenerator.java | 18 ++++++----- .../utils/generator/StructTreeBuilder.java | 32 ++++++++++++------- .../generator/StructTreeBuilderTest.java | 2 +- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/ClassBuilderInfo.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/ClassBuilderInfo.java index f90c05c92..6ad65df50 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/ClassBuilderInfo.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/ClassBuilderInfo.java @@ -498,7 +498,7 @@ public static class MemberOrArgument { /** Name of member/field. */ private final String name; /** Type of member/field (e.g. String, int...). */ - private final String type; + private String type; /** True to force this member to be final, false otherwise. */ private final boolean finalArg; /** List of classes/types or placeholders put into diamond operators to use as generics. */ @@ -529,6 +529,10 @@ public String getType() { return type; } + public void setType(String _type) { + type = _type; + } + public boolean isFinalArg() { return finalArg; } diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java index 0965f47ab..067cae21d 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java @@ -514,18 +514,20 @@ private String createTuple(List _outputArgs, String _className */ private String buildStructClass(String _dbusTypeStr, String _structName, ClassBuilderInfo _packageName, List _structClasses) throws DBusException { String structFqcn = _packageName.getPackageName() + "." + Util.upperCaseFirstChar(_structName); - String structName = _structName; - if (generatedStructClassNames.contains(structFqcn)) { - while (generatedStructClassNames.contains(structFqcn)) { - structFqcn += "Struct"; - structName += "Struct"; - } - } - String structClassName = new StructTreeBuilder(argumentPrefix).buildStructClasses(_dbusTypeStr, structName, _packageName, _structClasses); + String structClassName = new StructTreeBuilder(argumentPrefix).buildStructClasses(_dbusTypeStr, () -> getNextStructFqcn(structFqcn), _packageName, _structClasses); generatedStructClassNames.add(structFqcn); return structClassName; } + String getNextStructFqcn(String _structFqcn) { + String structFqcn = _structFqcn; + while (generatedStructClassNames.contains(structFqcn)) { + structFqcn += "Struct"; + } + generatedStructClassNames.add(structFqcn); + return structFqcn; + } + /** * Creates all files in the given map with the given content in the given directory. * diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/StructTreeBuilder.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/StructTreeBuilder.java index cf293c04e..0d1f84272 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/StructTreeBuilder.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/StructTreeBuilder.java @@ -15,6 +15,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.function.Supplier; /** * Helper to create a DBus struct class. @@ -49,14 +50,14 @@ public class StructTreeBuilder { * This may lead to classes with names like FooStructStructStruct (FooStruct->(InnerStruct->InnerInnerStruct)). * * @param _dbusSig dbus Type string - * @param _structName name the struct should have + * @param _structFqcnGenerator generator to determine name of struct * @param _clzBldr class builder with the class where the struct was first seen * @param _generatedClasses a list, this will contain additional struct classes created, if any. Should never be null! * * @return Struct class name or Collection type name * @throws DBusException on DBus Error */ - public String buildStructClasses(String _dbusSig, String _structName, ClassBuilderInfo _clzBldr, List _generatedClasses) throws DBusException { + public String buildStructClasses(String _dbusSig, Supplier _structFqcnGenerator, ClassBuilderInfo _clzBldr, List _generatedClasses) throws DBusException { if (Util.isBlank(_dbusSig) || _generatedClasses == null) { return null; @@ -70,10 +71,13 @@ public String buildStructClasses(String _dbusSig, String _structName, ClassBuild structTree = structTree.get(0).getSubType(); } + String rootStructName = _structFqcnGenerator.get(); + rootStructName = rootStructName.substring(rootStructName.lastIndexOf('.') + 1); + int cnt = 0; for (StructTree treeItem : structTree) { ClassBuilderInfo root = new ClassBuilderInfo(argumentPrefix); - root.setClassName(Util.upperCaseFirstChar(_structName)); + root.setClassName(Util.upperCaseFirstChar(rootStructName)); root.setPackageName(_clzBldr.getPackageName()); root.setExtendClass(Struct.class.getName()); root.setClassType(ClassType.CLASS); @@ -88,11 +92,11 @@ public String buildStructClasses(String _dbusSig, String _structName, ClassBuild } if (!treeItem.getSubType().isEmpty()) { - createNested(treeItem.getSubType(), root, _generatedClasses); + createNested(treeItem.getSubType(), _structFqcnGenerator, root, _generatedClasses); } } - return parentType == null ? _clzBldr.getPackageName() + "." + Util.upperCaseFirstChar(_structName) : parentType; + return parentType == null ? _clzBldr.getPackageName() + "." + Util.upperCaseFirstChar(rootStructName) : parentType; } @@ -100,12 +104,13 @@ public String buildStructClasses(String _dbusSig, String _structName, ClassBuild * Create nested Struct class. * * @param _list List of struct tree elements + * @param _structFqcnGenerator generators for the FQCN of the struct class * @param _root root class of this struct (maybe other struct) * @param _classes a list, this will contain additional struct classes created, if any. Should never be null! * * @return last created struct or null */ - private ClassBuilderInfo createNested(List _list, ClassBuilderInfo _root, List _classes) { + private ClassBuilderInfo createNested(List _list, Supplier _structFqcnGenerator, ClassBuilderInfo _root, List _classes) { int position = 0; ClassBuilderInfo root = _root; @@ -113,6 +118,7 @@ private ClassBuilderInfo createNested(List _list, ClassBuilderInfo _ ClassConstructor classConstructor = new ClassConstructor(); for (StructTree inTree : _list) { + MemberOrArgument member = new MemberOrArgument("member" + position, inTree.getDataType().getName(), true); member.getAnnotations().add("@Position(" + position + ")"); @@ -122,13 +128,17 @@ private ClassBuilderInfo createNested(List _list, ClassBuilderInfo _ if (Struct.class.isAssignableFrom(inTree.getDataType())) { ClassBuilderInfo temp = new ClassBuilderInfo(argumentPrefix); - temp.setClassName(Util.upperCaseFirstChar(_root.getClassName()) + "Struct"); + + String structName = _structFqcnGenerator.get(); + structName = structName.substring(structName.lastIndexOf('.') + 1); + + temp.setClassName(Util.upperCaseFirstChar(structName)); temp.setPackageName(_root.getPackageName()); temp.setExtendClass(Struct.class.getName()); temp.setClassType(ClassType.CLASS); - classConstructor.getArguments().add(new MemberOrArgument(constructorArg, inTree.getDataType().getName())); - - createNested(inTree.getSubType(), temp, _classes); + classConstructor.getArguments().add(new MemberOrArgument(constructorArg, temp.getClassName())); + member.setType(temp.getClassName()); + createNested(inTree.getSubType(), _structFqcnGenerator, temp, _classes); _classes.add(temp); retval = temp; @@ -137,7 +147,7 @@ private ClassBuilderInfo createNested(List _list, ClassBuilderInfo _ temp.setClassName(root.getClassName()); temp.setPackageName(root.getPackageName()); - ClassBuilderInfo x = createNested(inTree.getSubType(), temp, _classes); + ClassBuilderInfo x = createNested(inTree.getSubType(), _structFqcnGenerator, temp, _classes); if (x != null) { member.getGenerics().add(x.getClassName()); } else { diff --git a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/StructTreeBuilderTest.java b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/StructTreeBuilderTest.java index 606fdf5b6..a9da5dd65 100644 --- a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/StructTreeBuilderTest.java +++ b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/StructTreeBuilderTest.java @@ -20,7 +20,7 @@ void testCreateStructContainingStruct() throws DBusException { List generated = new ArrayList<>(); new StructTreeBuilder() - .buildStructClasses(dbusTypeStr, "UnitTestStruct", classBuilderInfo, generated); + .buildStructClasses(dbusTypeStr, () -> "UnitTestStruct", classBuilderInfo, generated); assertEquals(2, generated.size()); assertEquals("UnitTestStruct", generated.get(0).getClassName()); From 1d4d4674a28c57919f5cf55bc58b212e8aa05c56 Mon Sep 17 00:00:00 2001 From: David M Date: Sun, 22 Jun 2025 16:58:32 +0200 Subject: [PATCH 2/5] Issue #285: Improved creation of Tuple classes in InterfaceCodeGenerator --- .../main/java/org/freedesktop/dbus/Tuple.java | 20 +++++- .../java/org/freedesktop/dbus/utils/Util.java | 21 ++++++ .../utils/generator/ClassBuilderInfo.java | 31 +++------ .../generator/InterfaceCodeGenerator.java | 57 +++++++++++++--- .../dbus/utils/generator/TypeConverter.java | 68 +++++++++++++------ .../generator/InterfaceCodeGeneratorTest.java | 28 ++++++-- 6 files changed, 166 insertions(+), 59 deletions(-) diff --git a/dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java b/dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java index 43b1af8c3..5bf72bdbe 100644 --- a/dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java +++ b/dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java @@ -2,13 +2,27 @@ /** * This class should be extended to create Tuples. - * + *

* Any such class may be used as the return type for a method * which returns multiple values. - * + *

* All fields in the Tuple which you wish to be serialized and sent to the - * remote method should be annotated with the org.freedesktop.dbus.Position + * remote method should be annotated with the {@link org.freedesktop.dbus.Position Position} * annotation, in the order they should appear to DBus. + *

+ * A Tuple should have generic type parameters, otherwise deserialization may fail. + *

+ * Example: + * 
+ * public class MyTuple<String, Integer> extends Tuple {
+ *      @Position(0)
+ *      private final String firstValue;
+ *      @Position(1)
+ *      private final int secondValue;
+ *
+ *      // constructor/getter/setter omitted for brevity
+ * }
+ *
*/ public abstract class Tuple extends Container { } diff --git a/dbus-java-core/src/main/java/org/freedesktop/dbus/utils/Util.java b/dbus-java-core/src/main/java/org/freedesktop/dbus/utils/Util.java index ead50a59b..d8f06c767 100644 --- a/dbus-java-core/src/main/java/org/freedesktop/dbus/utils/Util.java +++ b/dbus-java-core/src/main/java/org/freedesktop/dbus/utils/Util.java @@ -715,4 +715,25 @@ public static Constructor getConstructor(Class _cl return null; } } + + /** + * Extracts the class name from a fully qualified class name (FQCN). + *

+ * If the FQCN is {@code null} or empty, {@code null} will be returned. + * If the FQCN does not contain any dots, the FQCN itself will be returned. + * + * @param _fqcn fully qualified class name + * @return class name or null if input was null/empty + */ + public static String extractClassNameFromFqcn(String _fqcn) { + if (Util.isBlank(_fqcn)) { + return null; + } + int lastDot = _fqcn.lastIndexOf('.'); + if (lastDot < 0) { + return _fqcn; + } + return _fqcn.substring(lastDot + 1); + } + } diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/ClassBuilderInfo.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/ClassBuilderInfo.java index 6ad65df50..ad283dccc 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/ClassBuilderInfo.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/ClassBuilderInfo.java @@ -35,6 +35,8 @@ public class ClassBuilderInfo { /** Constructors for this class. */ private final List constructors = new ArrayList<>(); + private final List generics = new ArrayList<>(); + /** Prefix to prepend to method/constructor arguments. */ private final String argumentPrefix; @@ -134,6 +136,10 @@ public List getConstructors() { return constructors; } + public List getGenerics() { + return generics; + } + /** * Create the Java source for the class information provided. * @@ -191,6 +197,9 @@ private List createClassFileContent(boolean _staticClass, Set _o String bgn = classIndent + "public " + (_staticClass ? "static " : "") + (getClassType() == ClassType.INTERFACE ? "interface" : "class"); bgn += " " + getClassName(); + if (!getGenerics().isEmpty()) { + bgn += "<" + String.join(", ", getGenerics()) + ">"; + } if (getExtendClass() != null) { Set lImports = getImportsForType(getExtendClass()); getImports().addAll(lImports); @@ -198,7 +207,7 @@ private List createClassFileContent(boolean _staticClass, Set _o bgn += " extends " + getSimpleTypeClasses(getExtendClass()); } if (!getImplementedInterfaces().isEmpty()) { - bgn += " implements " + getImplementedInterfaces().stream().map(ClassBuilderInfo::getClassName).collect(Collectors.joining(", ")); + bgn += " implements " + getImplementedInterfaces().stream().map(Util::extractClassNameFromFqcn).collect(Collectors.joining(", ")); // add classes import if implements-arguments are not a java.lang. classes getImports().addAll(getImplementedInterfaces().stream().filter(s -> !s.startsWith("java.lang.")).toList()); } @@ -310,24 +319,6 @@ public String toString() { + extendClass + "]"; } - /** - * Extract the class name from a given FQCN (fully qualified classname). - * - * @param _fqcn fqcn to analyze - * @return classname, null if input was null - */ - static String getClassName(String _fqcn) { - if (_fqcn == null) { - return null; - } - - String clzzName = _fqcn; - if (clzzName.contains(".")) { - clzzName = clzzName.substring(clzzName.lastIndexOf('.') + 1); - } - return clzzName; - } - /** * Simplify class names in the type. Please go to unit tests for usage examples. * @@ -343,7 +334,7 @@ static String getSimpleTypeClasses(String _type) { Matcher matcher = compile.matcher(_type); while (matcher.find()) { String match = matcher.group(); - matcher.appendReplacement(sb, getClassName(match)); + matcher.appendReplacement(sb, Util.extractClassNameFromFqcn(match)); } matcher.appendTail(sb); return sb.toString(); diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java index 067cae21d..403c85d6a 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java @@ -327,7 +327,22 @@ private List extractMethods(Element _methodElement, ClassBuild String resultType; if (outputArgs.size() > 1) { // multi-value return logger.debug("Found method with multiple return values: {}", methodElementName); - resultType = createTuple(outputArgs, methodElementName + "Tuple", _clzBldr, additionalClasses, dbusOutputArgTypes); + List genericTypes = new ArrayList<>(); + + resultType = createTuple(outputArgs, methodElementName + "Tuple", _clzBldr, additionalClasses, dbusOutputArgTypes, genericTypes); + + genericTypes.stream() + .flatMap(e -> ClassBuilderInfo.getImportsForType(e).stream()) + .forEach(_clzBldr.getImports()::add); + + _clzBldr.getImports().add(resultType); + + resultType = Util.extractClassNameFromFqcn(resultType); + List returnGenerics = genericTypes.stream() + .map(TypeConverter::convertJavaPrimitiveToBoxed) + .map(ClassBuilderInfo::getSimpleTypeClasses) + .toList(); + resultType += "<" + String.join(", ", returnGenerics) + ">"; } else { logger.debug("Found method with arguments: {}({})", methodElementName, inputArgs); resultType = outputArgs.isEmpty() ? "void" : outputArgs.get(0).getFullType(new HashSet<>()); @@ -401,7 +416,7 @@ private List extractProperties(Element _propertyElement, Class ClassBuilderInfo propertyTypeRef = null; String origType = null; if (!isComplex) { - clzzName = ClassBuilderInfo.getClassName(type); + clzzName = Util.extractClassNameFromFqcn(type); } else { origType = type; type = TypeRef.class.getName() + "<" + type + ">"; @@ -464,7 +479,8 @@ private List extractProperties(Element _propertyElement, Class * @param _dbusOutputArgTypes Dbus argument names and data types * @return FQCN of the newly created tuple based class */ - private String createTuple(List _outputArgs, String _className, ClassBuilderInfo _parentClzBldr, List _additionalClasses, List _dbusOutputArgTypes) { + private String createTuple(List _outputArgs, String _className, + ClassBuilderInfo _parentClzBldr, List _additionalClasses, List _dbusOutputArgTypes, List _genericTypes) { if (_outputArgs == null || _outputArgs.isEmpty() || _additionalClasses == null) { return null; } @@ -478,17 +494,17 @@ private String createTuple(List _outputArgs, String _className info.getImports().add(Position.class.getName()); } - ArrayList cnstrctArgs = new ArrayList<>(); + List cnstrctArgs = new ArrayList<>(); + Map genericTypes = new LinkedHashMap<>(); + int position = 0; for (MemberOrArgument entry : _outputArgs) { - entry.getAnnotations().add("@Position(" + position++ + ")"); - cnstrctArgs.add(new MemberOrArgument(entry.getName(), entry.getType())); - } + String genericName = findNextGenericName(genericTypes.keySet()); - for (String outputType : _dbusOutputArgTypes) { - for (String part : outputType.replace(" ", "").replace(">", "").split("<|,")) { - info.getImports().add(part); - } + genericTypes.put(genericName, entry.getType()); + entry.getAnnotations().add("@Position(" + position++ + ")"); + entry.setType(genericName); + cnstrctArgs.add(new MemberOrArgument(entry.getName(), genericName)); } ClassConstructor cnstrct = new ClassConstructor(); @@ -496,11 +512,30 @@ private String createTuple(List _outputArgs, String _className info.getConstructors().add(cnstrct); info.getMembers().addAll(_outputArgs); + info.getGenerics().addAll(genericTypes.keySet()); + _additionalClasses.add(info); + _genericTypes.addAll(genericTypes.values()); return info.getFqcn(); } + /** + * Tries to find next available generic name for a Tuple class. + * @param _used Set of already used names + * @return next available name + * @throws IllegalStateException if no name could be found + */ + static String findNextGenericName(Set _used) { + for (char c = 'A'; c <= 'Z'; c++) { + String name = String.valueOf(c); + if (!_used.contains(name)) { + return name; + } + } + throw new IllegalStateException("Unable to find a generic name for Tuple class"); + } + /** * Creates a class for a DBus Struct-Object. * diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/TypeConverter.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/TypeConverter.java index 855d7f4c3..2660d6961 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/TypeConverter.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/TypeConverter.java @@ -21,14 +21,34 @@ */ public final class TypeConverter { - private static final Map CLASS_MAP = new HashMap<>(); - static { - CLASS_MAP.put("java.lang.CharSequence", "String"); - CLASS_MAP.put("java.util.List", "List"); - CLASS_MAP.put("java.util.Set", "Set"); - CLASS_MAP.put("java.util.Map", "Map"); - CLASS_MAP.put(Variant.class.getName(), "Variant"); - } + private static final Map CLASS_MAP = Map.of( + "java.lang.CharSequence", "String", + "java.util.List", "List", + "java.util.Set", "Set", + "java.util.Map", "Map", + Variant.class.getName(), "Variant"); + + private static final Map PRIMITIVE_TO_BOXED = Map.of( + "byte", "Byte", + "short", "Short", + "int", "Integer", + "long", "Long", + "float", "Float", + "double", "Double", + "boolean", "Boolean", + "char", "Character" + ); + + private static final Map BOXED_TO_PRIMITIVE = Map.of( + "Byte", "byte", + "Short", "short", + "Integer", "int", + "Long", "long", + "Float", "float", + "Double", "double", + "Boolean", "boolean", + "Character", "char" + ); private TypeConverter() {} @@ -114,17 +134,27 @@ public static String convertJavaType(String _fqcn, boolean _usePrimitives) { * @param _clazzName class name of boxed type * @return primitive or original input */ - private static String convertJavaBoxedTypeToPrimitive(String _clazzName) { - return switch (_clazzName) { - case "Boolean" -> "boolean"; - case "Integer" -> "int"; - case "Long" -> "long"; - case "Double" -> "double"; - case "Float" -> "float"; - case "Byte" -> "byte"; - case "Char" -> "char"; - default -> _clazzName; - }; + public static String convertJavaBoxedTypeToPrimitive(String _clazzName) { + return BOXED_TO_PRIMITIVE.getOrDefault(_clazzName, _clazzName); + } + + /** + * Converts certain primitives to boxed types. + * + * @param _primitiveName class name of primitve type + * @return boxed type or original input + */ + public static String convertJavaPrimitiveToBoxed(String _primitiveName) { + return PRIMITIVE_TO_BOXED.getOrDefault(_primitiveName, _primitiveName); + } + + /** + * Checks if the given class name is a primitive type. + * @param _clazzName + * @return true if primitive, false otherwise + */ + public static boolean isPrimitive(String _clazzName) { + return PRIMITIVE_TO_BOXED.containsKey(_clazzName); } /** diff --git a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java index c929d15ba..53f5f00a4 100644 --- a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java +++ b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java @@ -1,22 +1,23 @@ package org.freedesktop.dbus.utils.generator; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; import org.freedesktop.dbus.annotations.DBusInterfaceName; import org.freedesktop.dbus.utils.Util; import org.junit.jupiter.api.Test; -import org.junit.platform.commons.util.StringUtils; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import java.io.File; import java.util.Map; +import java.util.Set; +import java.util.stream.Stream; class InterfaceCodeGeneratorTest { static InterfaceCodeGenerator loadDBusXmlFile(File _inputFile, String _objectPath, String _busName) { - if (!StringUtils.isBlank(_busName)) { + if (!Util.isBlank(_busName)) { String introspectionData = Util.readFileToString(_inputFile); return new InterfaceCodeGenerator(false, introspectionData, _objectPath, _busName, null, false, null); @@ -108,4 +109,19 @@ void testCreateStructNames() throws Exception { .noneMatch(s -> s.contains("import PresetUnitFilesChangesStruct")), "Did not expect an import for a class of same package"); } + + @ParameterizedTest + @MethodSource("createFindGenericNameData") + void testFindGenericName(Set _existingNames, String _expectedName) { + assertEquals(_expectedName, InterfaceCodeGenerator.findNextGenericName(_existingNames)); + } + + static Stream createFindGenericNameData() { + return Stream.of( + Arguments.of(Set.of("A", "B", "C"), "D"), + Arguments.of(Set.of("A", "B", "C", "D"), "E"), + Arguments.of(Set.of("A", "B", "C", "D", "E"), "F"), + Arguments.of(Set.of("A", "B", "C", "D", "E", "F"), "G") + ); + } } From 500e82ad843693aed61faa5b44d904717f8c0a84 Mon Sep 17 00:00:00 2001 From: David M Date: Mon, 23 Jun 2025 11:50:26 +0200 Subject: [PATCH 3/5] Updated unit tests --- .../generator/InterfaceCodeGenerator.java | 12 +---- .../utils/generator/StructTreeBuilder.java | 47 ++++++++++++------- .../generator/InterfaceCodeGeneratorTest.java | 34 +++++++------- .../generator/StructTreeBuilderTest.java | 2 +- 4 files changed, 52 insertions(+), 43 deletions(-) diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java index 403c85d6a..27db46988 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java @@ -549,20 +549,12 @@ static String findNextGenericName(Set _used) { */ private String buildStructClass(String _dbusTypeStr, String _structName, ClassBuilderInfo _packageName, List _structClasses) throws DBusException { String structFqcn = _packageName.getPackageName() + "." + Util.upperCaseFirstChar(_structName); - String structClassName = new StructTreeBuilder(argumentPrefix).buildStructClasses(_dbusTypeStr, () -> getNextStructFqcn(structFqcn), _packageName, _structClasses); + String structClassName = new StructTreeBuilder(argumentPrefix, generatedStructClassNames) + .buildStructClasses(_dbusTypeStr, structFqcn, _packageName, _structClasses); generatedStructClassNames.add(structFqcn); return structClassName; } - String getNextStructFqcn(String _structFqcn) { - String structFqcn = _structFqcn; - while (generatedStructClassNames.contains(structFqcn)) { - structFqcn += "Struct"; - } - generatedStructClassNames.add(structFqcn); - return structFqcn; - } - /** * Creates all files in the given map with the given content in the given directory. * diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/StructTreeBuilder.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/StructTreeBuilder.java index 0d1f84272..136ebad73 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/StructTreeBuilder.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/StructTreeBuilder.java @@ -11,11 +11,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Map; -import java.util.function.Supplier; +import java.util.*; /** * Helper to create a DBus struct class. @@ -28,13 +24,15 @@ public class StructTreeBuilder { private final String argumentPrefix; + private final Set generatedStructClassNames; - StructTreeBuilder(String _argumentPrefix) { + StructTreeBuilder(String _argumentPrefix, Set _generatedStructClassNames) { argumentPrefix = _argumentPrefix; + generatedStructClassNames = _generatedStructClassNames; } StructTreeBuilder() { - this(null); + this(null, new HashSet<>()); } /** @@ -50,14 +48,14 @@ public class StructTreeBuilder { * This may lead to classes with names like FooStructStructStruct (FooStruct->(InnerStruct->InnerInnerStruct)). * * @param _dbusSig dbus Type string - * @param _structFqcnGenerator generator to determine name of struct + * @param _structBaseFqcn FQCN including base class name to use for the created struct class * @param _clzBldr class builder with the class where the struct was first seen * @param _generatedClasses a list, this will contain additional struct classes created, if any. Should never be null! * * @return Struct class name or Collection type name * @throws DBusException on DBus Error */ - public String buildStructClasses(String _dbusSig, Supplier _structFqcnGenerator, ClassBuilderInfo _clzBldr, List _generatedClasses) throws DBusException { + public String buildStructClasses(String _dbusSig, String _structBaseFqcn, ClassBuilderInfo _clzBldr, List _generatedClasses) throws DBusException { if (Util.isBlank(_dbusSig) || _generatedClasses == null) { return null; @@ -71,7 +69,7 @@ public String buildStructClasses(String _dbusSig, Supplier _structFqcnGe structTree = structTree.get(0).getSubType(); } - String rootStructName = _structFqcnGenerator.get(); + String rootStructName = findNextStructFqcn(_structBaseFqcn, generatedStructClassNames); rootStructName = rootStructName.substring(rootStructName.lastIndexOf('.') + 1); int cnt = 0; @@ -92,7 +90,7 @@ public String buildStructClasses(String _dbusSig, Supplier _structFqcnGe } if (!treeItem.getSubType().isEmpty()) { - createNested(treeItem.getSubType(), _structFqcnGenerator, root, _generatedClasses); + createNested(treeItem.getSubType(), _structBaseFqcn, root, _generatedClasses); } } @@ -100,17 +98,34 @@ public String buildStructClasses(String _dbusSig, Supplier _structFqcnGe } + /** + * Find next available struct FQCN. + * If the given FQCN is already in use, it will append "Struct" until a free name is found. + * + * @param _structFqcn base FQCN to use + * @param _generatedStructClassNames set of already generated struct class names + * @return next available FQCN + */ + static String findNextStructFqcn(String _structFqcn, Set _generatedStructClassNames) { + String structFqcn = _structFqcn; + while (_generatedStructClassNames.contains(structFqcn)) { + structFqcn += "Struct"; + } + _generatedStructClassNames.add(structFqcn); + return structFqcn; + } + /** * Create nested Struct class. * * @param _list List of struct tree elements - * @param _structFqcnGenerator generators for the FQCN of the struct class + * @param _structFqcnBase base classname of created struct class * @param _root root class of this struct (maybe other struct) * @param _classes a list, this will contain additional struct classes created, if any. Should never be null! * * @return last created struct or null */ - private ClassBuilderInfo createNested(List _list, Supplier _structFqcnGenerator, ClassBuilderInfo _root, List _classes) { + private ClassBuilderInfo createNested(List _list, String _structFqcnBase, ClassBuilderInfo _root, List _classes) { int position = 0; ClassBuilderInfo root = _root; @@ -129,7 +144,7 @@ private ClassBuilderInfo createNested(List _list, Supplier _ if (Struct.class.isAssignableFrom(inTree.getDataType())) { ClassBuilderInfo temp = new ClassBuilderInfo(argumentPrefix); - String structName = _structFqcnGenerator.get(); + String structName = findNextStructFqcn(_structFqcnBase, generatedStructClassNames); structName = structName.substring(structName.lastIndexOf('.') + 1); temp.setClassName(Util.upperCaseFirstChar(structName)); @@ -138,7 +153,7 @@ private ClassBuilderInfo createNested(List _list, Supplier _ temp.setClassType(ClassType.CLASS); classConstructor.getArguments().add(new MemberOrArgument(constructorArg, temp.getClassName())); member.setType(temp.getClassName()); - createNested(inTree.getSubType(), _structFqcnGenerator, temp, _classes); + createNested(inTree.getSubType(), _structFqcnBase, temp, _classes); _classes.add(temp); retval = temp; @@ -147,7 +162,7 @@ private ClassBuilderInfo createNested(List _list, Supplier _ temp.setClassName(root.getClassName()); temp.setPackageName(root.getPackageName()); - ClassBuilderInfo x = createNested(inTree.getSubType(), _structFqcnGenerator, temp, _classes); + ClassBuilderInfo x = createNested(inTree.getSubType(), _structFqcnBase, temp, _classes); if (x != null) { member.getGenerics().add(x.getClassName()); } else { diff --git a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java index 53f5f00a4..97ff8c8da 100644 --- a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java +++ b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java @@ -29,20 +29,22 @@ static InterfaceCodeGenerator loadDBusXmlFile(File _inputFile, String _objectPat return null; } - @Test - void testCreateSelectedFirewallInterfaces() throws Exception { + @ParameterizedTest(name = "{0}") + @MethodSource("createTestData") + void testExtractData(String _description, File _input, String _dbusPath, String _filter, int _expected) throws Exception { InterfaceCodeGenerator ci2 = loadDBusXmlFile( - new File("src/test/resources/CreateInterface/firewall/org.fedoraproject.FirewallD1.xml"), "/org/fedoraproject/FirewallD1", "org.fedoraproject.FirewallD1"); + _input, _dbusPath, _filter); Map analyze = ci2.analyze(true); - assertEquals(9, analyze.size()); + assertEquals(_expected, analyze.size()); } - @Test - void testCreateAllFirewallInterfaces() throws Exception { - InterfaceCodeGenerator ci2 = loadDBusXmlFile( - new File("src/test/resources/CreateInterface/firewall/org.fedoraproject.FirewallD1.xml"), "/org/fedoraproject/FirewallD1", "*"); - Map analyze = ci2.analyze(true); - assertEquals(20, analyze.size()); + static Stream createTestData() { + return Stream.of( + Arguments.of("Test Extract All", new File("src/test/resources/CreateInterface/firewall/org.fedoraproject.FirewallD1.xml"), + "/org/fedoraproject/FirewallD1", "*", 23), + Arguments.of("Test Extract Selected", new File("src/test/resources/CreateInterface/firewall/org.fedoraproject.FirewallD1.xml"), + "/org/fedoraproject/FirewallD1", "org.fedoraproject.FirewallD1", 9) + ); } @Test @@ -110,18 +112,18 @@ void testCreateStructNames() throws Exception { "Did not expect an import for a class of same package"); } - @ParameterizedTest + @ParameterizedTest(name = "{0}") @MethodSource("createFindGenericNameData") - void testFindGenericName(Set _existingNames, String _expectedName) { + void testFindGenericName(String _description, Set _existingNames, String _expectedName) { assertEquals(_expectedName, InterfaceCodeGenerator.findNextGenericName(_existingNames)); } static Stream createFindGenericNameData() { return Stream.of( - Arguments.of(Set.of("A", "B", "C"), "D"), - Arguments.of(Set.of("A", "B", "C", "D"), "E"), - Arguments.of(Set.of("A", "B", "C", "D", "E"), "F"), - Arguments.of(Set.of("A", "B", "C", "D", "E", "F"), "G") + Arguments.of("ABC -> D", Set.of("A", "B", "C"), "D"), + Arguments.of("ABCD -> E", Set.of("A", "B", "C", "D"), "E"), + Arguments.of("ABCDE -> F", Set.of("A", "B", "C", "D", "E"), "F"), + Arguments.of("ABCDEF -> G", Set.of("A", "B", "C", "D", "E", "F"), "G") ); } } diff --git a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/StructTreeBuilderTest.java b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/StructTreeBuilderTest.java index a9da5dd65..606fdf5b6 100644 --- a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/StructTreeBuilderTest.java +++ b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/StructTreeBuilderTest.java @@ -20,7 +20,7 @@ void testCreateStructContainingStruct() throws DBusException { List generated = new ArrayList<>(); new StructTreeBuilder() - .buildStructClasses(dbusTypeStr, () -> "UnitTestStruct", classBuilderInfo, generated); + .buildStructClasses(dbusTypeStr, "UnitTestStruct", classBuilderInfo, generated); assertEquals(2, generated.size()); assertEquals("UnitTestStruct", generated.get(0).getClassName()); From 028c581899de400586a945cd4d846ae765e7ba08 Mon Sep 17 00:00:00 2001 From: David M Date: Mon, 23 Jun 2025 12:16:55 +0200 Subject: [PATCH 4/5] Issue #285: Create empty signal classes --- README.md | 1 + .../generator/InterfaceCodeGenerator.java | 53 +- .../generator/InterfaceCodeGeneratorTest.java | 8 +- .../mutter/org.gnome.Mutter.DisplayConfig.xml | 459 ++++++++++++++++++ 4 files changed, 488 insertions(+), 33 deletions(-) create mode 100644 dbus-java-utils/src/test/resources/CreateInterface/mutter/org.gnome.Mutter.DisplayConfig.xml diff --git a/README.md b/README.md index 4fad07975..a757487bf 100644 --- a/README.md +++ b/README.md @@ -133,6 +133,7 @@ The library will remain open source and MIT licensed and can still be used, fork - the `AbstractConnection.addSigHandler(DBusMatchRule, SigHandler)` is now `public` and can be used to register arbitrary rules - the new implementation supports additional MatchRules as defined by DBus Specification (except eavesdrop) - Extended `EmbeddedDBusDaemon` to properly support MatchRules + - Improved `InterfaceCodeGenerator` to properly create Tuple classes and create empty signal classes as well ##### Changes in 5.1.1 (2025-03-14): - Added new Helper class `VariantBuilder` to allow creating Variants which contain Maps or Collections without messing with the required DBus type arguments diff --git a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java index 27db46988..46f2c1700 100644 --- a/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java +++ b/dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java @@ -186,7 +186,7 @@ private Map extractAll(Element _ife) throws IOException, DBusExcep switch (element.getTagName().toLowerCase()) { case "method" -> additionalClasses.addAll(extractMethods(element, interfaceClass)); case "property" -> additionalClasses.addAll(extractProperties(element, interfaceClass)); - case "signal" -> additionalClasses.addAll(extractSignals(element, interfaceClass)); + case "signal" -> extractSignals(element, interfaceClass); } } @@ -203,18 +203,11 @@ private Map extractAll(Element _ife) throws IOException, DBusExcep * * @param _signalElement signal xml element * @param _clzBldr {@link ClassBuilderInfo} object - * @return List of {@link ClassBuilderInfo} which have been created (maybe empty, never null) * * @throws IOException on IO Error * @throws DBusException on DBus Error */ - private List extractSignals(Element _signalElement, ClassBuilderInfo _clzBldr) throws IOException, DBusException { - List additionalClasses = new ArrayList<>(); - - if (!_signalElement.hasChildNodes()) { // signal without any input/output?! - logger.warn("Signal without any input/output arguments. These are not supported yet, please open a ticket at github!"); - return additionalClasses; - } + private void extractSignals(Element _signalElement, ClassBuilderInfo _clzBldr) throws IOException, DBusException { String className = _signalElement.getAttribute("name"); if (className.contains(".")) { @@ -229,42 +222,42 @@ private List extractSignals(Element _signalElement, ClassBuild innerClass.setClassName(className); _clzBldr.getInnerClasses().add(innerClass); + List argsList = new ArrayList<>(); + + if (!_signalElement.hasChildNodes()) { // signal without any input/output?! + logger.info("Signal without any input/output arguments. Creating empty signal class: {}", innerClass.getFqcn()); + } else { + List signalArgs = XmlUtil.convertToElementList(XmlUtil.applyXpathExpressionToDocument("arg", _signalElement)); - List signalArgs = XmlUtil.convertToElementList(XmlUtil.applyXpathExpressionToDocument("arg", _signalElement)); + Map args = new LinkedHashMap<>(); - Map args = new LinkedHashMap<>(); + int unknownArgCnt = 0; + for (Element argElm : signalArgs) { + String argType = TypeConverter.getJavaTypeFromDBusType(argElm.getAttribute("type"), _clzBldr.getImports()); + String argName = Util.snakeToCamelCase(argElm.getAttribute("name")); + if (Util.isBlank(argName)) { + argName = "arg" + unknownArgCnt; + unknownArgCnt++; + } + args.put(argName, TypeConverter.getProperJavaClass(argType, _clzBldr.getImports())); + } - int unknownArgCnt = 0; - for (Element argElm : signalArgs) { - String argType = TypeConverter.getJavaTypeFromDBusType(argElm.getAttribute("type"), _clzBldr.getImports()); - String argName = Util.snakeToCamelCase(argElm.getAttribute("name")); - if (Util.isBlank(argName)) { - argName = "arg" + unknownArgCnt; - unknownArgCnt++; + for (Entry argEntry : args.entrySet()) { + innerClass.getMembers().add(new MemberOrArgument(argEntry.getKey(), argEntry.getValue(), true)); + argsList.add(new MemberOrArgument(argEntry.getKey(), argEntry.getValue(), false)); } - args.put(argName, TypeConverter.getProperJavaClass(argType, _clzBldr.getImports())); - } - for (Entry argEntry : args.entrySet()) { - innerClass.getMembers().add(new MemberOrArgument(argEntry.getKey(), argEntry.getValue(), true)); } ClassConstructor classConstructor = new ClassBuilderInfo.ClassConstructor(); - List argsList = new ArrayList<>(); - for (Entry e : args.entrySet()) { - argsList.add(new MemberOrArgument("_" + e.getKey(), e.getValue(), false)); - } - classConstructor.getArguments().addAll(argsList); classConstructor.getThrowArguments().add(DBusException.class.getSimpleName()); - classConstructor.getSuperArguments().add(new MemberOrArgument("_path", "String", false)); + classConstructor.getSuperArguments().add(new MemberOrArgument("path", "String", false)); classConstructor.getSuperArguments().addAll(argsList); innerClass.getConstructors().add(classConstructor); - - return additionalClasses; } /** diff --git a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java index 97ff8c8da..4246fc326 100644 --- a/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java +++ b/dbus-java-utils/src/test/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGeneratorTest.java @@ -40,10 +40,12 @@ void testExtractData(String _description, File _input, String _dbusPath, String static Stream createTestData() { return Stream.of( - Arguments.of("Test Extract All", new File("src/test/resources/CreateInterface/firewall/org.fedoraproject.FirewallD1.xml"), + Arguments.of("FirewallD1: Test Extract All", new File("src/test/resources/CreateInterface/firewall/org.fedoraproject.FirewallD1.xml"), "/org/fedoraproject/FirewallD1", "*", 23), - Arguments.of("Test Extract Selected", new File("src/test/resources/CreateInterface/firewall/org.fedoraproject.FirewallD1.xml"), - "/org/fedoraproject/FirewallD1", "org.fedoraproject.FirewallD1", 9) + Arguments.of("FirewallD1: Test Extract Selected", new File("src/test/resources/CreateInterface/firewall/org.fedoraproject.FirewallD1.xml"), + "/org/fedoraproject/FirewallD1", "org.fedoraproject.FirewallD1", 9), + Arguments.of("DisplayConfig: Test Extract All", new File("src/test/resources/CreateInterface/mutter/org.gnome.Mutter.DisplayConfig.xml"), + "/org/gnome/Mutter", "org.gnome.Mutter.DisplayConfig", 16) ); } diff --git a/dbus-java-utils/src/test/resources/CreateInterface/mutter/org.gnome.Mutter.DisplayConfig.xml b/dbus-java-utils/src/test/resources/CreateInterface/mutter/org.gnome.Mutter.DisplayConfig.xml new file mode 100644 index 000000000..30a8e97e2 --- /dev/null +++ b/dbus-java-utils/src/test/resources/CreateInterface/mutter/org.gnome.Mutter.DisplayConfig.xml @@ -0,0 +1,459 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 3b9c90ed76efe5616ff951d7e359ad958eb1e82f Mon Sep 17 00:00:00 2001 From: David M Date: Mon, 23 Jun 2025 12:20:07 +0200 Subject: [PATCH 5/5] Updated javadoc --- .../src/main/java/org/freedesktop/dbus/Tuple.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java b/dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java index 5bf72bdbe..7def81562 100644 --- a/dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java +++ b/dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java @@ -12,17 +12,23 @@ *

* A Tuple should have generic type parameters, otherwise deserialization may fail. *

- * Example: + * Example class: * 
- * public class MyTuple<String, Integer> extends Tuple {
+ * public class MyTuple<A, B> extends Tuple {
  *      @Position(0)
- *      private final String firstValue;
+ *      private final A firstValue;
  *      @Position(1)
- *      private final int secondValue;
+ *      private final B secondValue;
  *
  *      // constructor/getter/setter omitted for brevity
  * }
  *
+ * Example usage: + * 
+ * public SampleDbusInterface extends DBusInterface {
+ *     MyTuple<String, Integer> getMyTuple();
+ * }
+ *
*/ public abstract class Tuple extends Container { }