http://git.collabora.co.uk/?p=user/cassidy/telepathy-gabble;a=shortlog;h=refs/heads/ft-uri-33471

+          props = tp_dbus_properties_mixin_make_properties_hash (object,
+              TP_IFACE_CHANNEL, "ChannelType",
+              TP_IFACE_CHANNEL, "Interfaces",
+              TP_IFACE_CHANNEL, "TargetHandle",
+              TP_IFACE_CHANNEL, "TargetID",
+              TP_IFACE_CHANNEL, "TargetHandleType",
+              TP_IFACE_CHANNEL, "Requested",
+              TP_IFACE_CHANNEL, "InitiatorHandle",
+              TP_IFACE_CHANNEL, "InitiatorID",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "State",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "ContentType",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "Filename",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "Size",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "ContentHashType",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "ContentHash",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "Description",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "Date",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "AvailableSocketTypes",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "TransferredBytes",
+              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "InitialOffset",
+              GABBLE_IFACE_CHANNEL_TYPE_FILETRANSFER_FUTURE, "FileCollection",
+              NULL);
+
+              /* FileURI is immutable only for outgoing transfers */
+              if (self->priv->initiator == base_conn->self_handle)
+                {
+                  tp_dbus_properties_mixin_fill_properties_hash (object, props,
+                      GABBLE_IFACE_CHANNEL_TYPE_FILETRANSFER_FUTURE, "FileURI",
+                      NULL);
+                }
+
+              g_value_take_boxed (value, props);
+        }

The indentation is wrong.


+        assert props[cs.FT_FILE_URI] == self.file.uri

assertEquals. You could change all the assertions in that section while you're at it.

The test should also test not specifying the URI, in both directions.


+  if (self->priv->file_uri != NULL)
+    {
+      g_set_error (error, TP_ERRORS, TP_ERROR_NOT_AVAILABLE,
+          "FileURI has already be set");
+      return FALSE;
+    }

These should all be InvalidArgument. NotAvailable is temporary.


+        # Check it has the right value now
+        uri = ft_props.Get(cs.CHANNEL_TYPE_FILE_TRANSFER + '.FUTURE', 'FileURI')
+        assertEquals(self.file.uri, uri)
+

You should also test setting it after it's been set, and after the file has been accepted.

+        g_assert (self->priv->file_uri == NULL); /* construct only */

I think this comment is a bit misleading, but okay.

I did the FileURI -> URI renaming dance.

(In reply to comment #2)
> +          props = tp_dbus_properties_mixin_make_properties_hash (object,
> +              TP_IFACE_CHANNEL, "ChannelType",
> +              TP_IFACE_CHANNEL, "Interfaces",
> +              TP_IFACE_CHANNEL, "TargetHandle",
> +              TP_IFACE_CHANNEL, "TargetID",
> +              TP_IFACE_CHANNEL, "TargetHandleType",
> +              TP_IFACE_CHANNEL, "Requested",
> +              TP_IFACE_CHANNEL, "InitiatorHandle",
> +              TP_IFACE_CHANNEL, "InitiatorID",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "State",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "ContentType",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "Filename",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "Size",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "ContentHashType",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "ContentHash",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "Description",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "Date",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "AvailableSocketTypes",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "TransferredBytes",
> +              TP_IFACE_CHANNEL_TYPE_FILE_TRANSFER, "InitialOffset",
> +              GABBLE_IFACE_CHANNEL_TYPE_FILETRANSFER_FUTURE, "FileCollection",
> +              NULL);
> +
> +              /* FileURI is immutable only for outgoing transfers */
> +              if (self->priv->initiator == base_conn->self_handle)
> +                {
> +                  tp_dbus_properties_mixin_fill_properties_hash (object,
> props,
> +                      GABBLE_IFACE_CHANNEL_TYPE_FILETRANSFER_FUTURE,
> "FileURI",
> +                      NULL);
> +                }
> +
> +              g_value_take_boxed (value, props);
> +        }
> 
> The indentation is wrong.

Ooops; fixed.

> +        assert props[cs.FT_FILE_URI] == self.file.uri
> assertEquals. You could change all the assertions in that section while you're
> at it.

done. (each time I use assertEquals I wonder myself why it's
assertEquals(expected, value) and not the other way around...)

> The test should also test not specifying the URI, in both directions.

done

> +  if (self->priv->file_uri != NULL)
> +    {
> +      g_set_error (error, TP_ERRORS, TP_ERROR_NOT_AVAILABLE,
> +          "FileURI has already be set");
> +      return FALSE;
> +    }
> 
> These should all be InvalidArgument. NotAvailable is temporary.

fixed.

> +        # Check it has the right value now
> +        uri = ft_props.Get(cs.CHANNEL_TYPE_FILE_TRANSFER + '.FUTURE',
> 'FileURI')
> +        assertEquals(self.file.uri, uri)
> +
> 
> You should also test setting it after it's been set, and after the file has
> been accepted.

done.

Looks fine.

Merged to master. I keep this bug open as we still have to implement it as stable API using tp-glib.

Here we go: http://git.collabora.co.uk/?p=user/cassidy/telepathy-gabble;a=shortlog;h=refs/heads/file-uri-33471

This is blocked by having a tp-glib release.

tp-glib is out, the only blocker is the review now.

ogogo

Merged to master; will be in 0.11.7