[telepathy-glib/master] borrow_interface_by_id: fail cleanly if invalidated

[Will Thompson]

If a proxy is invalidated, and then has a new interface nailed onto it
with add_interface_by_id, calling borrow_interface_by_id wound up
passing NULL to dbus_g_connection_get_connection and other nasty things
before telling you the proxy's invalidated.

add_interface_by_id should not let you do this, but rearranging
borrow_interface_by_id makes more sense anyway.
---
 telepathy-glib/proxy.c           |   21 ++++++------
 tests/dbus/Makefile.am           |    4 ++
 tests/dbus/invalidate-add-call.c |   65 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+), 11 deletions(-)
 create mode 100644 tests/dbus/invalidate-add-call.c

diff --git a/telepathy-glib/proxy.c b/telepathy-glib/proxy.c
index dce1629..081603c 100644
--- a/telepathy-glib/proxy.c
+++ b/telepathy-glib/proxy.c
@@ -300,6 +300,13 @@ tp_proxy_borrow_interface_by_id (TpProxy *self,
 {
   gpointer dgproxy;
 
+  if (self->invalidated != NULL)
+    {
+      g_set_error (error, self->invalidated->domain, self->invalidated->code,
+          "%s", self->invalidated->message);
+      return NULL;
+    }
+
   if (!tp_dbus_check_valid_interface_name (g_quark_to_string (interface),
         error))
       return NULL;
@@ -331,17 +338,9 @@ tp_proxy_borrow_interface_by_id (TpProxy *self,
       return dgproxy;
     }
 
-  if (self->invalidated != NULL)
-    {
-      g_set_error (error, self->invalidated->domain, self->invalidated->code,
-          "%s", self->invalidated->message);
-    }
-  else
-    {
-      g_set_error (error, TP_DBUS_ERRORS, TP_DBUS_ERROR_NO_INTERFACE,
-          "Object %s does not have interface %s",
-          self->object_path, g_quark_to_string (interface));
-    }
+  g_set_error (error, TP_DBUS_ERRORS, TP_DBUS_ERROR_NO_INTERFACE,
+      "Object %s does not have interface %s",
+      self->object_path, g_quark_to_string (interface));
 
   return NULL;
 }
diff --git a/tests/dbus/Makefile.am b/tests/dbus/Makefile.am
index 049d550..f01648a 100644
--- a/tests/dbus/Makefile.am
+++ b/tests/dbus/Makefile.am
@@ -23,6 +23,7 @@ noinst_PROGRAMS = \
     test-get-interface-after-invalidate \
     test-group-mixin \
     test-handle-set \
+    test-invalidate-add-call \
     test-invalidated-while-invoking-signals \
     test-message-mixin \
     test-params-cm \
@@ -93,6 +94,9 @@ test_group_mixin_SOURCES = group-mixin.c
 
 test_handle_set_SOURCES = handle-set.c
 
+test_invalidate_add_call_SOURCES = \
+    invalidate-add-call.c
+
 test_invalidated_while_invoking_signals_SOURCES = \
     invalidated-while-invoking-signals.c
 
diff --git a/tests/dbus/invalidate-add-call.c b/tests/dbus/invalidate-add-call.c
new file mode 100644
index 0000000..3d5056b
--- /dev/null
+++ b/tests/dbus/invalidate-add-call.c
@@ -0,0 +1,65 @@
+#include <glib-object.h>
+#include <dbus/dbus.h>
+#include <dbus/dbus-glib.h>
+#include <dbus/dbus-glib-lowlevel.h>
+
+#include <telepathy-glib/dbus.h>
+#include <telepathy-glib/debug.h>
+#include <telepathy-glib/errors.h>
+#include <telepathy-glib/interfaces.h>
+#include <telepathy-glib/proxy.h>
+#include <telepathy-glib/proxy-subclass.h>
+#include <telepathy-glib/util.h>
+
+#include "_gen/svc.h"
+#include "tests/lib/myassert.h"
+
+int
+main (int argc, char **argv)
+{
+  DBusGConnection *bus;
+  TpProxy *proxy;
+  GValue *value;
+  GError e = { TP_ERRORS, TP_ERROR_NOT_AVAILABLE, "gabba gabba hey" };
+  GError *error = NULL;
+
+  tp_debug_set_flags ("all");
+  g_type_init ();
+  bus = tp_get_bus ();
+
+  /* Open a D-Bus connection to myself */
+  proxy = TP_PROXY (g_object_new (TP_TYPE_PROXY,
+      "dbus-connection", bus,
+      "bus-name",
+          dbus_bus_get_unique_name (dbus_g_connection_get_connection (bus)),
+      "object-path", "/",
+      NULL));
+
+  MYASSERT (tp_proxy_has_interface (proxy, "org.freedesktop.DBus.Properties"),
+      "");
+
+  /* Invalidate it */
+  tp_proxy_invalidate (proxy, &e);
+
+  MYASSERT (!tp_proxy_has_interface (proxy, "org.freedesktop.DBus.Properties"),
+      "");
+
+  /* Now forcibly re-add the Properties interface... */
+  tp_proxy_add_interface_by_id (proxy, TP_IFACE_QUARK_DBUS_PROPERTIES);
+
+  MYASSERT (tp_proxy_has_interface (proxy, "org.freedesktop.DBus.Properties"),
+      "");
+
+  /* ...and try to call a method on it, which should fail immediately with the
+   * given invalidation reason.
+   */
+  MYASSERT (!tp_cli_dbus_properties_run_get (proxy, -1,
+        "com.example.WithProperties", "ReadOnly", &value, &error, NULL), "");
+  MYASSERT_SAME_ERROR (error, &e);
+
+  g_error_free (error);
+
+  g_object_unref (proxy);
+
+  return 0;
+}
-- 
1.5.6.5