From e6d03252d2a648c39b83b7ea9eb250acbc1298dd Mon Sep 17 00:00:00 2001 From: "Harry T. Vennik" Date: Sun, 6 Aug 2017 19:52:13 +0200 Subject: [PATCH 1/4] Added support for Phobos-style variants Only variants that are limited to contain types supported by ddbus are allowed. --- source/ddbus/conv.d | 56 ++++++++++++++++++++++++++++++++++++---- source/ddbus/exception.d | 13 +++++++--- source/ddbus/util.d | 9 ++++++- 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/source/ddbus/conv.d b/source/ddbus/conv.d index 11683d7..7979667 100644 --- a/source/ddbus/conv.d +++ b/source/ddbus/conv.d @@ -10,6 +10,7 @@ import std.string; import std.typecons; import std.range; import std.traits; +import std.variant : VariantN; void buildIter(TS...)(DBusMessageIter *iter, TS args) if(allCanDBus!TS) { foreach(index, arg; args) { @@ -56,6 +57,19 @@ void buildIter(TS...)(DBusMessageIter *iter, TS args) if(allCanDBus!TS) { dbus_message_iter_close_container(&sub, &entry); } dbus_message_iter_close_container(iter, &sub); + } else static if(isInstanceOf!(VariantN, T)) { + enforce(arg.hasValue, + new InvalidValueException(arg, "dbus:" ~ cast(char) typeCode!T)); + + DBusMessageIter sub; + foreach(AT; T.AllowedTypes) { + if (arg.peek!AT) { + dbus_message_iter_open_container(iter, 'v', typeSig!AT.ptr, &sub); + buildIter(&sub, arg.get!AT); + dbus_message_iter_close_container(iter, &sub); + break; + } + } } else static if(is(T == DBusAny) || is(T == Variant!DBusAny)) { static if(is(T == Variant!DBusAny)) { auto val = arg.data; @@ -171,10 +185,11 @@ T readIter(T)(DBusMessageIter *iter) if (isInstanceOf!(BitFlags, T)) { } T readIter(T)(DBusMessageIter *iter) if (!is(T == enum) && !isInstanceOf!(BitFlags, T) && canDBus!T) { + auto argType = dbus_message_iter_get_arg_type(iter); T ret; static if(!isVariant!T || is(T == Variant!DBusAny)) { - if(dbus_message_iter_get_arg_type(iter) == 'v') { + if(argType == 'v') { DBusMessageIter sub; dbus_message_iter_recurse(iter, &sub); static if(is(T == Variant!DBusAny)) { @@ -188,8 +203,12 @@ T readIter(T)(DBusMessageIter *iter) if (!is(T == enum) && !isInstanceOf!(BitFla return ret; } } - static if(!is(T == DBusAny) && !is(T == Variant!DBusAny)) { - auto argType = dbus_message_iter_get_arg_type(iter); + + static if( + !is(T == DBusAny) + && !is(T == Variant!DBusAny) + && !isInstanceOf!(VariantN, T) + ) { enforce(argType == typeCode!T(), new TypeMismatchException(typeCode!T(), argType)); } @@ -227,6 +246,25 @@ T readIter(T)(DBusMessageIter *iter) if (!is(T == enum) && !isInstanceOf!(BitFla DBusMessageIter sub; dbus_message_iter_recurse(iter, &sub); ret.data = readIter!(VariantType!T)(&sub); + } else static if(isInstanceOf!(VariantN, T)) { + scope const(char)[] argSig = + dbus_message_iter_get_signature(iter).fromStringz(); + scope(exit) + dbus_free(cast(void*) argSig.ptr); + + foreach(AT; T.AllowedTypes) { + // We have to compare the full signature here, not just the typecode. + // Otherwise, in case of container types, we might select the wrong one. + // We would then be calling an incorrect instance of readIter, which would + // probably throw a TypeMismatchException. + if (typeSig!AT == argSig) { + ret = readIter!AT(iter); + break; + } + } + + // If no value is in ret, apparently none of the types matched. + enforce(ret.hasValue, new TypeMismatchException(typeCode!T, argType)); } else static if(isAssociativeArray!T) { DBusMessageIter sub; dbus_message_iter_recurse(iter, &sub); @@ -239,7 +277,7 @@ T readIter(T)(DBusMessageIter *iter) if (!is(T == enum) && !isInstanceOf!(BitFla dbus_message_iter_next(&sub); } } else static if(is(T == DBusAny)) { - ret.type = dbus_message_iter_get_arg_type(iter); + ret.type = argType; ret.explicitVariant = false; if(ret.type == 's') { ret.str = readIter!string(iter); @@ -428,11 +466,16 @@ unittest { import dunit.toolkit; import ddbus.thin; + import std.variant : Algebraic; + enum E : int { a, b, c } enum F : uint { x = 1, y = 2, z = 4 } + alias V = Algebraic!(byte, short, int, long, string); Message msg = Message("org.example.wow", "/wut", "org.test.iface", "meth2"); - msg.build(E.c, 4, 5u, 8u); + V v1 = "hello from variant"; + V v2 = cast(short) 345; + msg.build(E.c, 4, 5u, 8u, v1, v2); DBusMessageIter iter, iter2; dbus_message_iter_init(msg.msg, &iter); @@ -446,5 +489,8 @@ unittest { readIter!F(&iter).assertThrow!InvalidValueException(); readIter!(BitFlags!F)(&iter2).assertThrow!InvalidValueException(); + + readIter!V(&iter).assertEqual(v1); + readIter!short(&iter).assertEqual(v2.get!short); } diff --git a/source/ddbus/exception.d b/source/ddbus/exception.d index a5a840f..fc97d7b 100644 --- a/source/ddbus/exception.d +++ b/source/ddbus/exception.d @@ -48,15 +48,20 @@ class TypeMismatchException : Exception { ) pure nothrow @safe { _expectedType = expectedType; _actualType = actualType; - super("The type of value at the current position in the message does not match the type of value to be read." - ~ " Expected: " ~ cast(char) expectedType ~ ", Got: " ~ cast(char) actualType); + if (expectedType == 'v') { + super("The type of value at the current position in the message is incompatible to the target variant type." + ~ " Type code of the value: " ~ cast(char) actualType); + } else { + super("The type of value at the current position in the message does not match the type of value to be read." + ~ " Expected: " ~ cast(char) expectedType ~ ", Got: " ~ cast(char) actualType); + } } - int expectedType() @property pure const nothrow @nogc { + int expectedType() @property pure const nothrow @safe @nogc { return _expectedType; } - int actualType() @property pure const nothrow @nogc { + int actualType() @property pure const nothrow @safe @nogc { return _actualType; } diff --git a/source/ddbus/util.d b/source/ddbus/util.d index 960624d..7aa55ee 100644 --- a/source/ddbus/util.d +++ b/source/ddbus/util.d @@ -4,6 +4,7 @@ import ddbus.thin; import std.typecons; import std.range; import std.traits; +import std.variant : VariantN; struct DictionaryEntry(K, V) { K key; @@ -62,6 +63,9 @@ template canDBus(T) { enum canDBus = true; } else static if(isVariant!T) { enum canDBus = canDBus!(VariantType!T); + } else static if(isInstanceOf!(VariantN, T)) { + // Phobos-style variants are supported if limited to DBus compatible types. + enum canDBus = (T.AllowedTypes.length > 0) && allCanDBus!(T.AllowedTypes); } else static if(isTuple!T) { enum canDBus = allCanDBus!(T.Types); } else static if(isInputRange!T) { @@ -112,7 +116,7 @@ string typeSig(T)() if(canDBus!T) { return "s"; } else static if(is(T == ObjectPath)) { return "o"; - } else static if(isVariant!T) { + } else static if(isVariant!T || isInstanceOf!(VariantN, T)) { return "v"; } else static if(is(T B == enum)) { return typeSig!B; @@ -212,6 +216,9 @@ unittest { typeSig!(DictionaryEntry!(string, int)[]).assertEqual("a{si}"); // multiple arguments typeSigAll!(int,bool).assertEqual("ib"); + // Phobos-style variants + canDBus!(std.variant.Variant).assertFalse(); + typeSig!(std.variant.Algebraic!(int, double, string)).assertEqual("v"); // type codes typeCode!int().assertEqual(cast(int)('i')); typeCode!bool().assertEqual(cast(int)('b')); From e4186bcc5c5d53ebecb15c77b18f239e2d6ece15 Mon Sep 17 00:00:00 2001 From: "Harry T. Vennik" Date: Mon, 14 Aug 2017 20:25:31 +0200 Subject: [PATCH 2/4] Deprecate ddbus.util.isVariant Closes #22 --- source/ddbus/conv.d | 6 +++--- source/ddbus/util.d | 13 +++++++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/source/ddbus/conv.d b/source/ddbus/conv.d index 7979667..c4891d5 100644 --- a/source/ddbus/conv.d +++ b/source/ddbus/conv.d @@ -122,7 +122,7 @@ void buildIter(TS...)(DBusMessageIter *iter, TS args) if(allCanDBus!TS) { } if(val.explicitVariant) dbus_message_iter_close_container(iter, sub); - } else static if(isVariant!T) { + } else static if(isInstanceOf!(Variant, T)) { DBusMessageIter sub; const(char)* subSig = typeSig!(VariantType!T).toStringz(); dbus_message_iter_open_container(iter, 'v', subSig, &sub); @@ -188,7 +188,7 @@ T readIter(T)(DBusMessageIter *iter) if (!is(T == enum) && !isInstanceOf!(BitFla auto argType = dbus_message_iter_get_arg_type(iter); T ret; - static if(!isVariant!T || is(T == Variant!DBusAny)) { + static if(!isInstanceOf!(Variant, T) || is(T == Variant!DBusAny)) { if(argType == 'v') { DBusMessageIter sub; dbus_message_iter_recurse(iter, &sub); @@ -242,7 +242,7 @@ T readIter(T)(DBusMessageIter *iter) if (!is(T == enum) && !isInstanceOf!(BitFla ret ~= readIter!U(&sub); } } - } else static if(isVariant!T) { + } else static if(isInstanceOf!(Variant, T)) { DBusMessageIter sub; dbus_message_iter_recurse(iter, &sub); ret.data = readIter!(VariantType!T)(&sub); diff --git a/source/ddbus/util.d b/source/ddbus/util.d index 7aa55ee..a381ca0 100644 --- a/source/ddbus/util.d +++ b/source/ddbus/util.d @@ -17,6 +17,15 @@ auto byDictionaryEntries(K, V)(V[K] aa) { return aa.byKeyValue.map!(pair => DictionaryEntry!(K, V)(pair.key, pair.value)); } +/+ + Predicate template indicating whether T is an instance of ddbus.thin.Variant. + + Deprecated: + This template used to be undocumented and user code should not depend on it. + Its meaning became unclear when support for Phobos-style variants was added. + It seemed best to remove it at that point. ++/ +deprecated("Use std.traits.isInstanceOf instead.") template isVariant(T) { static if(isBasicType!T || isInputRange!T) { enum isVariant = false; @@ -61,7 +70,7 @@ template basicDBus(T) { template canDBus(T) { static if(basicDBus!T || is(T == DBusAny)) { enum canDBus = true; - } else static if(isVariant!T) { + } else static if(isInstanceOf!(Variant, T)) { enum canDBus = canDBus!(VariantType!T); } else static if(isInstanceOf!(VariantN, T)) { // Phobos-style variants are supported if limited to DBus compatible types. @@ -116,7 +125,7 @@ string typeSig(T)() if(canDBus!T) { return "s"; } else static if(is(T == ObjectPath)) { return "o"; - } else static if(isVariant!T || isInstanceOf!(VariantN, T)) { + } else static if(isInstanceOf!(Variant, T) || isInstanceOf!(VariantN, T)) { return "v"; } else static if(is(T B == enum)) { return typeSig!B; From ba2c2979d2212b7e3b57883102158e1d3e686b05 Mon Sep 17 00:00:00 2001 From: "Harry T. Vennik" Date: Sat, 19 Aug 2017 14:50:45 +0200 Subject: [PATCH 3/4] Extend unittest in router.d to use recently added types --- source/ddbus/router.d | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/source/ddbus/router.d b/source/ddbus/router.d index e999eca..e107963 100644 --- a/source/ddbus/router.d +++ b/source/ddbus/router.d @@ -194,6 +194,10 @@ void registerRouter(Connection conn, MessageRouter router) { unittest{ import dunit.toolkit; + + import std.typecons : BitFlags; + import std.variant : Algebraic; + auto router = new MessageRouter(); // set up test messages MessagePattern patt = MessagePattern("/root","ca.thume.test","test"); @@ -209,6 +213,24 @@ unittest{ patt = MessagePattern("/troll","ca.thume.tester","wow"); router.setHandler!(void)(patt,{return;}); + patt = MessagePattern("/root/fancy","ca.thume.tester","crazyTest"); + enum F : ushort { a = 1, b = 8, c = 16 } + struct S { byte b; ulong ul; F f; } + router.setHandler!(int)(patt, (Algebraic!(ushort, BitFlags!F, S) v) { + if (v.type is typeid(ushort) || v.type is typeid(BitFlags!F)) { + return v.coerce!int; + } else if (v.type is typeid(S)) { + auto s = v.get!S; + final switch (s.f) { + case F.a: return s.b; + case F.b: return cast(int) s.ul; + case F.c: return cast(int) s.ul + s.b; + } + } + + assert(false); + }); + static assert(!__traits(compiles, { patt = MessagePattern("/root/bar","ca.thume.tester","lolwut"); router.setHandler!(void, DBusAny)(patt,(DBusAny wrongUsage){return;}); @@ -216,10 +238,13 @@ unittest{ // TODO: these tests rely on nondeterministic hash map ordering static string introspectResult = ` -`; +`; router.introspectXML("/root").assertEqual(introspectResult); static string introspectResult2 = ` `; router.introspectXML("/root/foo").assertEqual(introspectResult2); + static string introspectResult3 = ` +`; + router.introspectXML("/root/fancy").assertEqual(introspectResult3); router.introspectXML("/").assertEndsWith(``); } From 63bbbd0a148b1f3ad8e4235339ea2e4a6a8862a2 Mon Sep 17 00:00:00 2001 From: "Harry T. Vennik" Date: Sat, 19 Aug 2017 14:52:38 +0200 Subject: [PATCH 4/4] Fix template bug in type conversion for BitFlags --- source/ddbus/conv.d | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/ddbus/conv.d b/source/ddbus/conv.d index c4891d5..1a85bdd 100644 --- a/source/ddbus/conv.d +++ b/source/ddbus/conv.d @@ -173,7 +173,7 @@ T readIter(T)(DBusMessageIter *iter) if (isInstanceOf!(BitFlags, T)) { alias TemplateArgsOf!T[0] E; alias OriginalType!E B; - B mask = only(EnumMembers!E).fold!((a, b) => a | b); + B mask = only(EnumMembers!E).fold!((a, b) => cast(B) (a | b)); B value = readIter!B(iter); enforce(