Bug 33471 - Implement FileTransfer.FileURI
Summary: Implement FileTransfer.FileURI
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://git.collabora.co.uk/?p=user/ca...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-01-25 07:42 UTC by Guillaume Desmottes
Modified: 2011-02-03 00:49 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Guillaume Desmottes 2011-01-25 07:42:01 UTC
We should have an implementation of it before merging bug #32434.
Comment 2 Will Thompson 2011-01-26 03:08:46 UTC
+          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.
Comment 3 Guillaume Desmottes 2011-01-26 06:11:40 UTC
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.
Comment 4 Will Thompson 2011-01-28 05:48:35 UTC
Looks fine.
Comment 5 Guillaume Desmottes 2011-01-28 06:08:30 UTC
Merged to master. I keep this bug open as we still have to implement it as stable API using tp-glib.
Comment 6 Guillaume Desmottes 2011-01-31 04:15:22 UTC
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.
Comment 7 Guillaume Desmottes 2011-02-02 04:00:08 UTC
tp-glib is out, the only blocker is the review now.
Comment 8 Will Thompson 2011-02-02 09:34:11 UTC
ogogo
Comment 9 Guillaume Desmottes 2011-02-03 00:49:41 UTC
Merged to master; will be in 0.11.7


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.