Bug 55108 - GVariant-based tp_protocol_new, and test coverage
Summary: GVariant-based tp_protocol_new, and test coverage
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks: 30422
  Show dependency treegraph
 
Reported: 2012-09-19 15:24 UTC by Simon McVittie
Modified: 2014-03-03 13:40 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
test-protocol-objects: factor out check_tp_protocol() (4.75 KB, patch)
2014-02-28 16:01 UTC, Guillaume Desmottes
Details | Splinter Review
test creating a TpProtocol by passing its immutable props (1.47 KB, patch)
2014-02-28 16:02 UTC, Guillaume Desmottes
Details | Splinter Review
protocol: add TpProtocol:protocol-properties-vardict (4.62 KB, patch)
2014-02-28 16:02 UTC, Guillaume Desmottes
Details | Splinter Review
add tp_protocol_new_vardict() (4.04 KB, patch)
2014-02-28 16:02 UTC, Guillaume Desmottes
Details | Splinter Review

Description Simon McVittie 2012-09-19 15:24:19 UTC
+++ This bug was initially created as a clone of Bug #30422 +++

tp_protocol_new() takes a dbus-glib GHashTable. We should have a version that takes a G_VARIANT_TYPE_VARDICT, taking ownership if it is floating.

The implementation is pretty easy, but there's no regression test, so I haven't verified that this actually works or anything. We need a regression test for tp_protocol_new() with non-NULL immutable_properties first.

/**
 * tp_protocol_new_vardict:
 * @dbus: proxy for the D-Bus daemon; may not be %NULL
 * @cm_name: the connection manager name (such as "gabble")
 * @protocol_name: the protocol name (such as "jabber")
 * @immutable_properties: the immutable D-Bus properties for this protocol
 * @error: used to indicate the error if %NULL is returned
 *
 * Create a new protocol proxy.
 *
 * If @immutable_properties is a floating reference, this function will
 * take ownership of it, much like g_variant_ref_sink(). See documentation of
 * that function for details.
 *
 * Returns: a new protocol proxy, or %NULL on invalid arguments
 *
 * Since: 0.UNRELEASED
 */
TpProtocol *
tp_protocol_new_vardict (TpDBusDaemon *dbus,
    const gchar *cm_name,
    const gchar *protocol_name,
    GVariant *immutable_properties,
    GError **error)
{
  GHashTable *hash;
  TpProtocol *ret;

  g_return_val_if_fail (g_variant_is_of_type (immutable_properties,
        G_VARIANT_TYPE_VARDICT), NULL);

  g_variant_ref_sink (immutable_properties);
  hash = _tp_asv_from_vardict (immutable_properties);
  ret = tp_protocol_new (dbus, cm_name, protocol_name, hash, error);
  g_hash_table_unref (hash);
  g_variant_unref (immutable_properties);
  return ret;
}
Comment 1 Guillaume Desmottes 2014-02-28 16:01:47 UTC
Created attachment 94896 [details] [review]
test-protocol-objects: factor out check_tp_protocol()
Comment 2 Guillaume Desmottes 2014-02-28 16:02:02 UTC
Created attachment 94897 [details] [review]
test creating a TpProtocol by passing its immutable props
Comment 3 Guillaume Desmottes 2014-02-28 16:02:13 UTC
Created attachment 94898 [details] [review]
protocol: add TpProtocol:protocol-properties-vardict
Comment 4 Guillaume Desmottes 2014-02-28 16:02:25 UTC
Created attachment 94899 [details] [review]
add tp_protocol_new_vardict()
Comment 5 Guillaume Desmottes 2014-02-28 16:02:37 UTC
Those patches are for master.
Comment 6 Simon McVittie 2014-02-28 16:06:14 UTC
Comment on attachment 94898 [details] [review]
protocol: add TpProtocol:protocol-properties-vardict

Review of attachment 94898 [details] [review]:
-----------------------------------------------------------------

::: telepathy-glib/protocol.c
@@ +2387,5 @@
> + */
> +GVariant *
> +tp_protocol_dup_immutable_properties (TpProtocol *self)
> +{
> +  return _tp_asv_to_vardict (self->priv->protocol_properties);

This will need sinking when you merge this to next.
Comment 7 Simon McVittie 2014-02-28 16:07:04 UTC
All fine for master.
Comment 8 Guillaume Desmottes 2014-03-03 10:17:32 UTC
(In reply to comment #7)
> All fine for master.

Merged to master.
Comment 9 Guillaume Desmottes 2014-03-03 13:40:42 UTC
(In reply to comment #6)
> Comment on attachment 94898 [details] [review] [review]
> protocol: add TpProtocol:protocol-properties-vardict
> 
> Review of attachment 94898 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: telepathy-glib/protocol.c
> @@ +2387,5 @@
> > + */
> > +GVariant *
> > +tp_protocol_dup_immutable_properties (TpProtocol *self)
> > +{
> > +  return _tp_asv_to_vardict (self->priv->protocol_properties);
> 
> This will need sinking when you merge this to next.

Done that.

Merged to next as well.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.