2012-10-01 07:03:23

by Srinivasa Ragavan

[permalink] [raw]
Subject: [PATCH] client: Add Message.MarkAsRead and Message.MarkAsDeleted implementation and documentation.

From: Srinivasa Ragavan <[email protected]>

---
client/map.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
client/transfer.c | 3 +-
doc/client-api.txt | 11 +++++
3 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/client/map.c b/client/map.c
index 4f07fcb..a08d272 100644
--- a/client/map.c
+++ b/client/map.c
@@ -26,7 +26,9 @@

#include <errno.h>
#include <string.h>
+#include <fcntl.h>
#include <glib.h>
+#include <glib/gstdio.h>
#include <gdbus.h>

#include "dbus.h"
@@ -36,6 +38,8 @@
#include "transfer.h"
#include "session.h"
#include "driver.h"
+#include "src/map_ap.h"
+#include "gobex/gobex-apparam.h"

#define OBEX_MAS_UUID \
"\xBB\x58\x2B\x40\x42\x0C\x11\xDB\xB0\xDE\x08\x00\x20\x0C\x9A\x66"
@@ -46,6 +50,10 @@
#define ERROR_INTERFACE "org.bluez.obex.Error"
#define MAS_UUID "00001132-0000-1000-8000-00805f9b34fb"

+#define STATUS_READ 0
+#define STATUS_DELETE 1
+#define FILLER_BYTE 0x30
+
struct map_data {
struct obc_session *session;
DBusMessage *msg;
@@ -72,6 +80,7 @@ struct map_msg {
uint64_t size;
char *status;
uint8_t flags;
+ DBusMessage *msg;
};

struct map_parser {
@@ -284,12 +293,116 @@ fail:
return reply;
}

+static void set_message_flag_cb(struct obc_session *session,
+ struct obc_transfer *transfer,
+ GError *err, void *user_data)
+{
+ struct map_msg *msg = user_data;
+ DBusMessage *reply;
+ char *target_file;
+
+ if (err != NULL) {
+ reply = g_dbus_create_error(msg->msg,
+ ERROR_INTERFACE ".Failed",
+ "%s", err->message);
+ goto done;
+ }
+
+ /* Get rid of the tmp file */
+ target_file = g_build_filename (g_get_tmp_dir(), "bt-msg-status", NULL);
+ g_unlink (target_file);
+ g_free (target_file);
+
+ reply = dbus_message_new_method_return(msg->msg);
+ if (reply == NULL)
+ return;
+
+done:
+ g_dbus_send_message(conn, reply);
+ dbus_message_unref(msg->msg);
+ msg->msg = NULL;
+}
+
+static DBusMessage *map_msg_mark_flag (DBusConnection *connection,
+ DBusMessage *message, int op, void *user_data)
+{
+ struct map_msg *msg = user_data;
+ struct obc_transfer *transfer;
+ char *target_file;
+ gboolean status;
+ GError *err = NULL;
+ DBusMessage *reply;
+ GObexApparam *apparam;
+ guint8 buf[6];
+ gsize len;
+ char contents[2];
+
+ if (dbus_message_get_args(message, NULL,
+ DBUS_TYPE_BOOLEAN, &status,
+ DBUS_TYPE_INVALID) == FALSE)
+ return g_dbus_create_error(message,
+ ERROR_INTERFACE ".InvalidArguments", NULL);
+
+ target_file = g_build_filename (g_get_tmp_dir(), "bt-msg-status", NULL);
+
+ contents[0] = FILLER_BYTE;
+ contents[1] = '\0';
+
+ transfer = obc_transfer_put("x-bt/messageStatus", msg->handle, target_file,
+ contents, sizeof(contents), &err);
+ if (transfer == NULL)
+ goto fail;
+
+ apparam = g_obex_apparam_set_uint8(NULL, MAP_AP_STATUSINDICATOR,
+ op);
+ apparam = g_obex_apparam_set_uint8(apparam, MAP_AP_STATUSVALUE,
+ status);
+ len = g_obex_apparam_encode(apparam, buf, sizeof(buf));
+
+ obc_transfer_set_params(transfer, buf, len);
+
+ g_obex_apparam_free(apparam);
+
+ if (!obc_session_queue(msg->data->session, transfer, set_message_flag_cb, msg, &err))
+ goto fail;
+
+ msg->msg = dbus_message_ref (message);
+
+ return NULL;
+
+fail:
+ reply = g_dbus_create_error(message, ERROR_INTERFACE ".Failed", "%s",
+ err->message);
+ g_free (target_file);
+ g_error_free(err);
+ return reply;
+}
+
+static DBusMessage *map_msg_mark_deleted (DBusConnection *connection,
+ DBusMessage *message, void *user_data)
+{
+ return map_msg_mark_flag (connection, message, STATUS_DELETE, user_data);
+}
+
+static DBusMessage *map_msg_mark_read (DBusConnection *connection,
+ DBusMessage *message, void *user_data)
+{
+ return map_msg_mark_flag (connection, message, STATUS_READ, user_data);
+}
+
static const GDBusMethodTable map_msg_methods[] = {
{ GDBUS_METHOD("Get",
- GDBUS_ARGS({ "targetfile", "s" }),
+ GDBUS_ARGS({ "targetfile", "s" },
+ { "attachment", "b" }),
GDBUS_ARGS({ "transfer", "o" },
{ "properties", "a{sv}" }),
map_msg_get) },
+ { GDBUS_ASYNC_METHOD("MarkAsRead",
+ GDBUS_ARGS({ "read", "b" }), NULL,
+ map_msg_mark_read) },
+ { GDBUS_ASYNC_METHOD("MarkAsDeleted",
+ GDBUS_ARGS({ "deleted", "b" }), NULL,
+ map_msg_mark_deleted) },
{ }
};

diff --git a/client/transfer.c b/client/transfer.c
index e9fabfb..0040f74 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -416,7 +416,7 @@ struct obc_transfer *obc_transfer_put(const char *type, const char *name,
if (contents != NULL) {
ssize_t w;

- if (!transfer_open(transfer, O_RDWR, 0, err))
+ if (!transfer_open(transfer, O_RDWR|O_CREAT|O_TRUNC, 0, err))
goto fail;

w = write(transfer->fd, contents, size);
@@ -432,6 +432,7 @@ struct obc_transfer *obc_transfer_put(const char *type, const char *name,
"Writing all contents to file failed");
goto fail;
}
+ lseek(transfer->fd, 0, SEEK_SET);
} else {
if (!transfer_open(transfer, O_RDONLY, 0, err))
goto fail;
diff --git a/doc/client-api.txt b/doc/client-api.txt
index f447789..aa98399 100644
--- a/doc/client-api.txt
+++ b/doc/client-api.txt
@@ -471,6 +471,17 @@ Methods object, dict Get(string targetfile)
The properties of this transfer are also returned along
with the object path, to avoid a call to GetProperties.

+ void MarkAsRead(boolean read)
+
+ Mark the message as read or unread depending on the *read*
+ flag.
+
+ void MarkAsDeleted(boolean deleted)
+
+ Mark the message as deleted or undeleted depending on the
+ *deleted* flag.
+
+
Transfer hierarchy
==================

--
1.7.10.4



2012-10-01 10:04:01

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] client: Add Message.MarkAsRead and Message.MarkAsDeleted implementation and documentation.

Hi,

On Mon, Oct 1, 2012 at 12:05 PM, Venkateswaran, Srinivasa Ragavan
<[email protected]> wrote:
>> 1. Use properties, so for setting delete/read the application should
>> use SetProperty method and it can get the values with GetProperties
>> and PropertyChanged to signal when the values has changed. (Note this
>> may change if we add support to standard D-Bus Properties interface
>> but the idea is the same just the interface change)
>
> I'll do it as SetProperty ("read/deleted", true/false) . But I don't
> see a support in MAP ('s spec) for getting a message status or being
> notified for message update. If you can point me, I could do this also
> and give a complete patch the Message object.

Yep, it is not in the spec, but it doesn't mean we should not have
support for getting updates asynchronously as signals because
different processes can change the properties and in future if this is
added in the spec we already have support for them, besides it is
consistent with overall use of properties.

>> 2. What is this filler byte and lseek for?
>
> lseek was because, after just we write to the file, the read always
> failed and I thought the position was the end of the file. I just seek
> to the beginning of the file, it worked. I hope I didn't overlook
> something.

Yep, it is actually a bug so could you please have it in a separate patch?

>> 3. We probably need to add support for non-body/apparam only PUT so we
>> can pass NULL to filename. (maybe this is the reason for 2?)
>
> For SetMessageStatus the spec mentioned that Body/End of Body,
> specifying the filler byte 0x30 is mandatory. Quoting from the spec
> below:
> "To avoid PUT with empty Body leading to a 'delete' of the related message these
> headers shall contain a filler byte. The value of this byte shall be
> set to 0x30 (="0")."
>
> If it wasn't for this, I would have updated PUT with support to empty
> body. I assume, this is fine.

Yep, very strange requirement to be honest but I guess the author was
afraid the OBEX parser would trigger delete of the message if no body
was found, anyway sending 0x30 is mandatory so we should include as
you did. Still I think the proper way to support this is to have
obc_transfer_put supporting NULL filename, in that case the caller
need to provide the contents which should be stored in a temporary
which is automatically removed (transfer_open already takes care of
this).


--
Luiz Augusto von Dentz

2012-10-01 09:05:26

by Srinivasa Ragavan

[permalink] [raw]
Subject: Re: [PATCH] client: Add Message.MarkAsRead and Message.MarkAsDeleted implementation and documentation.

Hi,

On Mon, Oct 1, 2012 at 1:52 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Srinivasa,
>
> On Mon, Oct 1, 2012 at 10:14 AM, Venkateswaran, Srinivasa Ragavan
> <[email protected]> wrote:
>> On Mon, Oct 1, 2012 at 12:33 PM,
>> <[email protected]> wrote:
>>> From: Srinivasa Ragavan <[email protected]>
>>>
>>> ---
>>> client/map.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>>> client/transfer.c | 3 +-
>>> doc/client-api.txt | 11 +++++
>>> 3 files changed, 127 insertions(+), 2 deletions(-)
>>
>> Ignore this patch, this has a partial patch I was trying from Luiz.
>> Sorry for the mess, sent an updated one.
>>
>> -Srini.
>
> I just merged some fix last week so you would have to rebase your
> changes, but I got some suggestions:

Oh sorry, I will rebase with my updated patch after fixing the issues
mentioned below.

>
> 1. Use properties, so for setting delete/read the application should
> use SetProperty method and it can get the values with GetProperties
> and PropertyChanged to signal when the values has changed. (Note this
> may change if we add support to standard D-Bus Properties interface
> but the idea is the same just the interface change)

I'll do it as SetProperty ("read/deleted", true/false) . But I don't
see a support in MAP ('s spec) for getting a message status or being
notified for message update. If you can point me, I could do this also
and give a complete patch the Message object.

> 2. What is this filler byte and lseek for?

lseek was because, after just we write to the file, the read always
failed and I thought the position was the end of the file. I just seek
to the beginning of the file, it worked. I hope I didn't overlook
something.

> 3. We probably need to add support for non-body/apparam only PUT so we
> can pass NULL to filename. (maybe this is the reason for 2?)

For SetMessageStatus the spec mentioned that Body/End of Body,
specifying the filler byte 0x30 is mandatory. Quoting from the spec
below:
"To avoid PUT with empty Body leading to a 'delete' of the related message these
headers shall contain a filler byte. The value of this byte shall be
set to 0x30 (="0")."

If it wasn't for this, I would have updated PUT with support to empty
body. I assume, this is fine.

Thanks,
-Srini.

>
> --
> Luiz Augusto von Dentz

2012-10-01 08:22:44

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH] client: Add Message.MarkAsRead and Message.MarkAsDeleted implementation and documentation.

Hi Srinivasa,

On Mon, Oct 1, 2012 at 10:14 AM, Venkateswaran, Srinivasa Ragavan
<[email protected]> wrote:
> On Mon, Oct 1, 2012 at 12:33 PM,
> <[email protected]> wrote:
>> From: Srinivasa Ragavan <[email protected]>
>>
>> ---
>> client/map.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>> client/transfer.c | 3 +-
>> doc/client-api.txt | 11 +++++
>> 3 files changed, 127 insertions(+), 2 deletions(-)
>
> Ignore this patch, this has a partial patch I was trying from Luiz.
> Sorry for the mess, sent an updated one.
>
> -Srini.

I just merged some fix last week so you would have to rebase your
changes, but I got some suggestions:

1. Use properties, so for setting delete/read the application should
use SetProperty method and it can get the values with GetProperties
and PropertyChanged to signal when the values has changed. (Note this
may change if we add support to standard D-Bus Properties interface
but the idea is the same just the interface change)
2. What is this filler byte and lseek for?
3. We probably need to add support for non-body/apparam only PUT so we
can pass NULL to filename. (maybe this is the reason for 2?)

--
Luiz Augusto von Dentz

2012-10-01 07:14:19

by Srinivasa Ragavan

[permalink] [raw]
Subject: Re: [PATCH] client: Add Message.MarkAsRead and Message.MarkAsDeleted implementation and documentation.

On Mon, Oct 1, 2012 at 12:33 PM,
<[email protected]> wrote:
> From: Srinivasa Ragavan <[email protected]>
>
> ---
> client/map.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> client/transfer.c | 3 +-
> doc/client-api.txt | 11 +++++
> 3 files changed, 127 insertions(+), 2 deletions(-)

Ignore this patch, this has a partial patch I was trying from Luiz.
Sorry for the mess, sent an updated one.

-Srini.