Fix returning container-typed properties

lib/dbus/data.rb

@@ -165,6 +165,9 @@ def self.fixed?
 
     # Represents integers
     class Int < Fixed
+      # @!method self.range
+      # @return [Range] the full range of allowed values
+
       # @param value [::Integer,DBus::Data::Int]
       # @raise RangeError
       def initialize(value)
@@ -174,10 +177,6 @@ def initialize(value)
 
         super(value)
       end
-
-      def self.range
-        raise NotImplementedError, "Abstract"
-      end
     end
 
     # Byte.
@@ -542,7 +541,7 @@ def self.from_typed(value, member_types:)
           Data.make_typed(member_type, i)
         end
 
-        new(items) # initialize(::Array<Data::Base>)
+        new(items, member_type: member_type) # initialize(::Array<Data::Base>)
       end
 
       # FIXME: should Data::Array be mutable?
@@ -603,8 +602,6 @@ def self.from_typed(value, member_types:)
         # TODO: validation
         raise unless value.size == member_types.size
 
-        @member_types = member_types
-
         items = member_types.zip(value).map do |item_type, item|
           Data.make_typed(item_type, item)
         end
@@ -643,14 +640,20 @@ def self.from_typed(value, member_types:) # rubocop:disable Lint/UnusedMethodArg
         # assert member_types.empty?
 
         # decide on type of value
-        new(value)
+        new(value, member_type: nil)
       end
 
-      # Note that for Variants type=="v",
+      # @return [Type]
+      def self.type
+        # memoize
+        @type ||= Type.new(type_code).freeze
+      end
+
+      # Note that for Variants type.to_s=="v",
       # for the specific see {Variant#member_type}
       # @return [Type] the exact type of this value
       def type
-        "v"
+        self.class.type
       end
 
       # @return [Type]
@@ -708,6 +711,22 @@ def self.from_items(value, mode:, member_types:) # rubocop:disable Lint/UnusedMe
         value
       end
 
+      # @param value [::Object] (#size, #each)
+      # @param member_types [::Array<Type>]
+      # @return [DictEntry]
+      def self.from_typed(value, member_types:)
+        # assert member_types.size == 2
+        # TODO: duplicated from Struct. Inherit/delegate?
+        # TODO: validation
+        raise unless value.size == member_types.size
+
+        items = member_types.zip(value).map do |item_type, item|
+          Data.make_typed(item_type, item)
+        end
+
+        new(items, member_types: member_types) # initialize(::Array<Data::Base>)
+      end
+
       def initialize(value, member_types:)
         @member_types = member_types
         @type = nil

lib/dbus/marshall.rb

@@ -102,6 +102,8 @@ def do_parse(signature, mode: :plain)
         packet = data_class.from_raw(value, mode: mode)
       elsif data_class.basic?
         size = aligned_read_value(data_class.size_class)
+        # @raw_msg.align(data_class.alignment)
+        # ^ is not necessary because we've just read a suitably-aligned *size*
         value = @raw_msg.read(size)
         nul = @raw_msg.read(1)
         if nul != "\u0000"
@@ -250,7 +252,7 @@ def append(type, val)
         when Type::ARRAY
           append_array(type.child, val)
         when Type::STRUCT, Type::DICT_ENTRY
-          val = val.value if val.is_a?(Data::Struct)
+          val = val.value if val.is_a?(Data::Struct) || val.is_a?(Data::DictEntry)
           unless val.is_a?(Array) || val.is_a?(Struct)
             type_name = Type::TYPE_MAPPING[type.sigtype].first
             raise TypeException, "#{type_name} expects an Array or Struct, seen #{val.class}"

spec/data_spec.rb

@@ -183,6 +183,14 @@
 
       include_examples "constructor accepts plain or typed values", good
       include_examples "constructor rejects values from this list", bad
+
+      describe ".alignment" do
+        # this overly specific test avoids a redundant alignment call
+        # in the production code
+        it "returns the correct value" do
+          expect(described_class.alignment).to eq 4
+        end
+      end
     end
 
     describe DBus::Data::ObjectPath do
@@ -198,6 +206,14 @@
 
       include_examples "constructor accepts plain or typed values", good
       include_examples "constructor rejects values from this list", bad
+
+      describe ".alignment" do
+        # this overly specific test avoids a redundant alignment call
+        # in the production code
+        it "returns the correct value" do
+          expect(described_class.alignment).to eq 4
+        end
+      end
     end
 
     describe DBus::Data::Signature do
@@ -215,6 +231,14 @@
 
       include_examples "constructor accepts plain or typed values", good
       include_examples "constructor rejects values from this list", bad
+
+      describe ".alignment" do
+        # this overly specific test avoids a redundant alignment call
+        # in the production code
+        it "returns the correct value" do
+          expect(described_class.alignment).to eq 1
+        end
+      end
     end
   end
 
@@ -238,6 +262,13 @@
 
       include_examples "constructor (kwargs) accepts values", good
       include_examples "constructor (kwargs) rejects values", bad
+
+      describe ".from_typed" do
+        it "creates new instance from given object and type" do
+          type = DBus::Type.new("s")
+          expect(described_class.from_typed(["test", "lest"], member_types: [type])).to be_a(described_class)
+        end
+      end
     end
 
     describe DBus::Data::Struct do
@@ -287,9 +318,33 @@
 
       include_examples "constructor (kwargs) accepts values", good
       # include_examples "constructor (kwargs) rejects values", bad
+
+      describe ".from_typed" do
+        it "creates new instance from given object and type" do
+          type = DBus::Type.new("s")
+          expect(described_class.from_typed(["test", "lest"].freeze, member_types: [type, type]))
+            .to be_a(described_class)
+        end
+      end
     end
 
     describe DBus::Data::Variant do
+      describe ".from_typed" do
+        it "creates new instance from given object and type" do
+          type = DBus::Type.new("s")
+          expect(described_class.from_typed("test", member_types: [type])).to be_a(described_class)
+        end
+
+        it "ignores the member_types argument" do
+          type = DBus::Type.new("s")
+          # Base.from_typed is a generic interface with a fixed signature;
+          # So it must offer the member_types parameter, which is misleading
+          # for a Variant
+          value = described_class.from_typed("test", member_types: [type])
+          expect(value.type.to_s).to eq "v"
+          expect(value.member_type.to_s).to eq "s"
+        end
+      end
     end
 
     describe DBus::Data::DictEntry do

spec/property_spec.rb

@@ -52,15 +52,15 @@
 
   it "tests get all" do
     all = @iface.all_properties
-    expect(all.keys.sort).to eq(["MyStruct", "ReadMe", "ReadOrWriteMe"])
+    expect(all.keys.sort).to eq(["MyArray", "MyDict", "MyStruct", "MyVariant", "ReadMe", "ReadOrWriteMe"])
   end
 
   it "tests get all on a V1 object" do
     obj = @svc["/org/ruby/MyInstance"]
     iface = obj["org.ruby.SampleInterface"]
 
     all = iface.all_properties
-    expect(all.keys.sort).to eq(["MyStruct", "ReadMe", "ReadOrWriteMe"])
+    expect(all.keys.sort).to eq(["MyArray", "MyDict", "MyStruct", "MyVariant", "ReadMe", "ReadOrWriteMe"])
   end
 
   it "tests unknown property reading" do
@@ -147,4 +147,29 @@
       expect(reply).to match(/variant\s+struct {\s+string "three"\s+string "strings"\s+string "in a struct"\s+}/)
     end
   end
+
+  context "an array-typed property" do
+    it "gets read as an array" do
+      val = @iface["MyArray"]
+      expect(val).to eq([42, 43])
+    end
+  end
+
+  context "an dict-typed property" do
+    it "gets read as a hash" do
+      val = @iface["MyDict"]
+      expect(val).to eq({
+                          "one" => 1,
+                          "two" => "dva",
+                          "three" => [3, 3, 3]
+                        })
+    end
+  end
+
+  context "a variant-typed property" do
+    it "gets read at all" do
+      val = @iface["MyVariant"]
+      expect(val).to eq([42, 43])
+    end
+  end
 end

spec/service_newapi.rb

@@ -21,6 +21,13 @@ def initialize(path)
     @read_me = "READ ME"
     @read_or_write_me = "READ OR WRITE ME"
     @my_struct = ["three", "strings", "in a struct"].freeze
+    @my_array = [42, 43]
+    @my_dict = {
+      "one" => 1,
+      "two" => "dva",
+      "three" => [3, 3, 3]
+    }
+    @my_variant = @my_array.dup
     @main_loop = nil
   end
 
@@ -101,6 +108,9 @@ def explosive
     dbus_reader :explosive, "s"
 
     dbus_attr_reader :my_struct, "(sss)"
+    dbus_attr_reader :my_array, "aq"
+    dbus_attr_reader :my_dict, "a{sv}"
+    dbus_attr_reader :my_variant, "v"
   end
 
   # closing and reopening the same interface