Enhance the DBus interface for peerd

dbus/property.cc

Index: dbus/property.cc
diff --git a/dbus/property.cc b/dbus/property.cc
index 9475a0728a6cca7e4de360caa10333593a4bda44..84e87a3feae861d0a1c8d11ec8e1489bfc01325b 100644
--- a/dbus/property.cc
+++ b/dbus/property.cc
@@ -478,6 +478,105 @@ void Property<std::vector<uint8> >::AppendSetValueToWriter(
   writer->CloseContainer(&variant_writer);
 }
 
+//
+// Property<std::map<std::string, std::string> > specialization.

[hashimoto, 2015/02/05 09:51:12]

nit: No need for space in "> >".
The same goes for other places.

[dtapuska, 2015/02/05 19:51:17]

Done.

+//
+
+template <>
+bool Property<std::map<std::string, std::string>>::PopValueFromReader(
+    MessageReader* reader) {
+  MessageReader array_reader(NULL);

[hashimoto, 2015/02/05 09:51:12]

nit: Please move this line to the line before PopArray() to avoid unnecessary
initialization on error cases.
(or if you prefer less lines of code, you can keep this line here while merging
two "if"s below by ||.)
The same goes for other parts.

[dtapuska, 2015/02/05 19:51:17]

Done.

+  MessageReader variant_reader(NULL);
+  if (!reader->PopVariant(&variant_reader))
+    return false;
+  if (!variant_reader.PopArray(&array_reader))
+    return false;
+  value_.clear();
+  while (array_reader.HasMoreData()) {
+    dbus::MessageReader dict_entry_reader(NULL);
+    if (!array_reader.PopDictEntry(&dict_entry_reader))
+      return false;
+    std::string key;
+    std::string value;
+    if (!dict_entry_reader.PopString(&key) ||
+        !dict_entry_reader.PopString(&value))
+      return false;
+    value_.emplace(std::move(key), std::move(value));

[hashimoto, 2015/02/05 09:51:12]

move is not allowed yet. http://chromium-cpp.appspot.com/

[dtapuska, 2015/02/05 19:51:17]

Done.

+  }
+  return true;
+}
+
+template <>
+void Property<std::map<std::string, std::string>>::AppendSetValueToWriter(
+    MessageWriter* writer) {
+  MessageWriter variant_writer(NULL);
+  MessageWriter dict_writer(NULL);
+  writer->OpenVariant("a{ss}", &variant_writer);
+  variant_writer.OpenArray("{ss}", &dict_writer);
+  for (const auto& pair : set_value_) {
+    dbus::MessageWriter entry_writer(NULL);
+    dict_writer.OpenDictEntry(&entry_writer);
+    entry_writer.AppendString(pair.first);
+    entry_writer.AppendString(pair.second);
+    dict_writer.CloseContainer(&entry_writer);
+  }
+  variant_writer.CloseContainer(&dict_writer);
+  writer->CloseContainer(&variant_writer);
+}
+
+//
+// Propertystd::vector<std::tuple<std::vector<uint8_t>, uint16_t> > >

[hashimoto, 2015/02/05 09:51:12]

nit: Property<std::vector<std::tuple<std::vector<uint8_t>, uint16_t>>>

[dtapuska, 2015/02/05 19:51:17]

Done.

+// specialization.
+//
+
+template <>
+bool Property<std::vector<std::tuple<std::vector<uint8_t>, uint16_t>>>::
+    PopValueFromReader(MessageReader* reader) {
+  MessageReader array_reader(NULL);
+  MessageReader variant_reader(NULL);
+  if (!reader->PopVariant(&variant_reader))
+    return false;
+  if (!variant_reader.PopArray(&array_reader))
+    return false;
+
+  value_.clear();
+  while (array_reader.HasMoreData()) {
+    dbus::MessageReader struct_reader(NULL);
+    if (!array_reader.PopStruct(&struct_reader))
+      return false;
+
+    std::tuple<std::vector<uint8_t>, uint16_t> entry;
+    const uint8* bytes = NULL;
+    size_t length = 0;
+    if (!struct_reader.PopArrayOfBytes(&bytes, &length))
+      return false;
+    std::get<0>(entry).assign(bytes, bytes + length);
+    if (!struct_reader.PopUint16(&std::get<1>(entry)))
+      return false;
+    value_.push_back(std::move(entry));
+  }
+  return true;
+}
+
+template <>
+void Property<std::vector<std::tuple<std::vector<uint8_t>, uint16_t>>>::
+    AppendSetValueToWriter(MessageWriter* writer) {
+  MessageWriter variant_writer(NULL);
+  MessageWriter array_writer(NULL);
+  writer->OpenVariant("a(ayq)", &variant_writer);
+  variant_writer.OpenArray("a(ayq)", &array_writer);
+  for (const auto& pair : set_value_) {
+    dbus::MessageWriter struct_writer(nullptr);
+    array_writer.OpenStruct(&struct_writer);
+    struct_writer.AppendArrayOfBytes(std::get<0>(pair).data(),
+                                     std::get<0>(pair).size());
+    struct_writer.AppendUint16(std::get<1>(pair));
+    array_writer.CloseContainer(&struct_writer);
+  }
+  variant_writer.CloseContainer(&array_writer);
+  writer->CloseContainer(&variant_writer);
+}

[satorux1, 2015/02/05 08:20:15]

The new functions have lots of logic. Could you add unit tests for these?

+
 template class Property<uint8>;
 template class Property<bool>;
 template class Property<int16>;
@@ -492,5 +591,8 @@ template class Property<ObjectPath>;
 template class Property<std::vector<std::string> >;
 template class Property<std::vector<ObjectPath> >;
 template class Property<std::vector<uint8> >;
+template class Property<std::map<std::string, std::string>>;
+template class Property<
+    std::vector<std::tuple<std::vector<uint8_t>, uint16_t>>>;
 
 }  // namespace dbus