Merge pull request #286 from hypfvieh/issue-285

Issue 285

Author: hypfvieh

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

dbus-java-core/src/main/java/org/freedesktop/dbus/Tuple.java

@@ -2,13 +2,33 @@
 
 /**
  * This class should be extended to create Tuples.
- *
+ * <p>
  * Any such class may be used as the return type for a method
  * which returns multiple values.
- *
+ * </p><p>
  * 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.
+ * </p><p>
+ * A Tuple should have generic type parameters, otherwise deserialization may fail.
+ * </p>
+ * Example class:
+ * <pre>
+ * public class MyTuple&lt;A, B&gt; extends Tuple {
+ *      &#64;Position(0)
+ *      private final A firstValue;
+ *      &#64;Position(1)
+ *      private final B secondValue;
+ *
+ *      // constructor/getter/setter omitted for brevity
+ * }
+ * </pre>
+ * Example usage:
+ * <pre>
+ * public SampleDbusInterface extends DBusInterface {
+ *     MyTuple&lt;String, Integer&gt; getMyTuple();
+ * }
+ * </pre>
  */
 public abstract class Tuple extends Container {
 }

dbus-java-core/src/main/java/org/freedesktop/dbus/utils/Util.java

@@ -715,4 +715,25 @@ public static <C> Constructor<? extends C> getConstructor(Class<? extends C> _cl
             return null;
         }
     }
+
+    /**
+     * Extracts the class name from a fully qualified class name (FQCN).
+     * <p>
+     * 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);
+    }
+
 }

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<ClassConstructor> constructors          = new ArrayList<>();
 
+    private final List<String>           generics              = new ArrayList<>();
+
     /** Prefix to prepend to method/constructor arguments. */
     private final String                 argumentPrefix;
 
@@ -134,6 +136,10 @@ public List<ClassConstructor> getConstructors() {
         return constructors;
     }
 
+    public List<String> getGenerics() {
+        return generics;
+    }
+
     /**
      * Create the Java source for the class information provided.
      *
@@ -191,14 +197,17 @@ private List<String> createClassFileContent(boolean _staticClass, Set<String> _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<String> lImports = getImportsForType(getExtendClass());
             getImports().addAll(lImports);
             allImports.addAll(lImports);
             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();
@@ -498,7 +489,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 +520,10 @@ public String getType() {
             return type;
         }
 
+        public void setType(String _type) {
+            type = _type;
+        }
+
         public boolean isFinalArg() {
             return finalArg;
         }

dbus-java-utils/src/main/java/org/freedesktop/dbus/utils/generator/InterfaceCodeGenerator.java

@@ -186,7 +186,7 @@ private Map<File, String> 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<File, String> 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<ClassBuilderInfo> extractSignals(Element _signalElement, ClassBuilderInfo _clzBldr) throws IOException, DBusException {
-        List<ClassBuilderInfo> 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<ClassBuilderInfo> extractSignals(Element _signalElement, ClassBuild
         innerClass.setClassName(className);
 
         _clzBldr.getInnerClasses().add(innerClass);
+        List<MemberOrArgument> 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<Element> signalArgs = XmlUtil.convertToElementList(XmlUtil.applyXpathExpressionToDocument("arg", _signalElement));
 
-        List<Element> signalArgs = XmlUtil.convertToElementList(XmlUtil.applyXpathExpressionToDocument("arg", _signalElement));
+            Map<String, String> args = new LinkedHashMap<>();
 
-        Map<String, String> 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<String, String> 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<String, String> argEntry : args.entrySet()) {
-            innerClass.getMembers().add(new MemberOrArgument(argEntry.getKey(), argEntry.getValue(), true));
         }
 
         ClassConstructor classConstructor = new ClassBuilderInfo.ClassConstructor();
 
-        List<MemberOrArgument> argsList = new ArrayList<>();
-        for (Entry<String, String> 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;
     }
 
     /**
@@ -327,7 +320,22 @@ private List<ClassBuilderInfo> 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<String> 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<String> 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 +409,7 @@ private List<ClassBuilderInfo> 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 +472,8 @@ private List<ClassBuilderInfo> 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<MemberOrArgument> _outputArgs, String _className, ClassBuilderInfo _parentClzBldr, List<ClassBuilderInfo> _additionalClasses, List<String> _dbusOutputArgTypes) {
+    private String createTuple(List<MemberOrArgument> _outputArgs, String _className,
+        ClassBuilderInfo _parentClzBldr, List<ClassBuilderInfo> _additionalClasses, List<String> _dbusOutputArgTypes, List<String> _genericTypes) {
         if (_outputArgs == null || _outputArgs.isEmpty() || _additionalClasses == null) {
             return null;
         }
@@ -478,29 +487,48 @@ private String createTuple(List<MemberOrArgument> _outputArgs, String _className
             info.getImports().add(Position.class.getName());
         }
 
-        ArrayList<MemberOrArgument> cnstrctArgs = new ArrayList<>();
+        List<MemberOrArgument> cnstrctArgs = new ArrayList<>();
+        Map<String, String> 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();
         cnstrct.getArguments().addAll(cnstrctArgs);
 
         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<String> _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.
      *
@@ -514,14 +542,8 @@ private String createTuple(List<MemberOrArgument> _outputArgs, String _className
      */
     private String buildStructClass(String _dbusTypeStr, String _structName, ClassBuilderInfo _packageName, List<ClassBuilderInfo> _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, generatedStructClassNames)
+            .buildStructClasses(_dbusTypeStr, structFqcn, _packageName, _structClasses);
         generatedStructClassNames.add(structFqcn);
         return structClassName;
     }