[gnome-keyring] secret-store: Update the Created and Modified properties correctly

[Stefan Walter <stefw src gnome org>]

commit c90a1cca64b2cf0da8e4b38baa905434beff5e72
Author: Stef Walter <stefw gnome org>
Date:   Tue Mar 5 12:43:48 2013 +0100

    secret-store: Update the Created and Modified properties correctly
    
     * Set the Modified property before committing transactions
     * Set the Created property when items and collections are created
     * Add tests for this functionality
    
    https://bugzilla.gnome.org/show_bug.cgi?id=695052

 daemon/dbus/tests/Makefile.am                  |    1 +
 daemon/dbus/tests/test-secret-items.c          |  201 ++++++++++++++++++++++++
 pkcs11/secret-store/gkm-secret-collection.c    |    2 +
 pkcs11/secret-store/gkm-secret-item.c          |    8 +-
 pkcs11/secret-store/gkm-secret-object.c        |   52 ++++++-
 pkcs11/secret-store/gkm-secret-object.h        |    5 +-
 pkcs11/secret-store/tests/test-secret-object.c |    8 +-
 7 files changed, 266 insertions(+), 11 deletions(-)
---
diff --git a/daemon/dbus/tests/Makefile.am b/daemon/dbus/tests/Makefile.am
index 8f082f0..68a7af2 100644
--- a/daemon/dbus/tests/Makefile.am
+++ b/daemon/dbus/tests/Makefile.am
@@ -28,6 +28,7 @@ LDADD =  \
 TEST_PROGS = \
        test-secret-util \
        test-secret-search \
+       test-secret-items \
        test-secret-signals
 
 check_PROGRAMS = $(TEST_PROGS)
diff --git a/daemon/dbus/tests/test-secret-items.c b/daemon/dbus/tests/test-secret-items.c
new file mode 100644
index 0000000..95b2e56
--- /dev/null
+++ b/daemon/dbus/tests/test-secret-items.c
@@ -0,0 +1,201 @@
+/* -*- Mode: C; indent-tabs-mode: t; c-basic-offset: 8; tab-width: 8 -*- */
+/* test-secret-items.c: Test secret items
+
+   Copyright (C) 2013 Red Hat Inc
+
+   The Gnome Keyring Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Library General Public License as
+   published by the Free Software Foundation; either version 2 of the
+   License, or (at your option) any later version.
+
+   The Gnome Keyring Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Library General Public License for more details.
+
+   You should have received a copy of the GNU Library General Public
+   License along with the Gnome Library; see the file COPYING.LIB.  If not,
+   write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.
+
+   Author: Stef Walter <stefw gnome org>
+*/
+
+#include "config.h"
+
+#include "test-service.h"
+
+#include "gkd-secret-types.h"
+
+#include "egg/egg-testing.h"
+
+#include <gcr/gcr-base.h>
+
+#include <glib.h>
+#include <glib/gstdio.h>
+#include <gio/gio.h>
+
+#include <fcntl.h>
+
+typedef struct {
+       TestService service;
+} Test;
+
+static void
+setup (Test *test,
+       gconstpointer unused)
+{
+       GVariant *retval;
+       GError *error = NULL;
+
+       test_service_setup (&test->service);
+
+       /* Unlock the test collection */
+       retval = g_dbus_connection_call_sync (test->service.connection,
+                                             test->service.bus_name,
+                                             SECRET_SERVICE_PATH,
+                                             INTERNAL_SERVICE_INTERFACE,
+                                             "UnlockWithMasterPassword",
+                                             g_variant_new ("(o@(oayays))",
+                                                            "/org/freedesktop/secrets/collection/test",
+                                                            test_service_build_secret (&test->service, 
"booo")),
+                                             G_VARIANT_TYPE ("()"),
+                                             G_DBUS_CALL_FLAGS_NO_AUTO_START,
+                                             -1, NULL, &error);
+       g_assert_no_error (error);
+       g_variant_unref (retval);
+}
+
+static void
+teardown (Test *test,
+          gconstpointer unused)
+{
+       test_service_teardown (&test->service);
+}
+
+static GVariant *
+get_all_properties (Test *test,
+                    const gchar *path,
+                    const gchar *interface)
+{
+       GVariant *retval;
+       GVariant *props;
+       GError *error = NULL;
+
+       retval = g_dbus_connection_call_sync (test->service.connection,
+                                             test->service.bus_name,
+                                             path, "org.freedesktop.DBus.Properties",
+                                             "GetAll", g_variant_new ("(s)", interface),
+                                             G_VARIANT_TYPE ("(a{sv})"),
+                                             G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error);
+       g_assert_no_error (error);
+
+       g_variant_get (retval, "(@a{sv})", &props);
+       g_variant_unref (retval);
+
+       return props;
+}
+
+static void
+set_property (Test *test,
+              const gchar *path,
+              const gchar *interface,
+              const gchar *property,
+              GVariant *value)
+{
+       GVariant *retval;
+       GError *error = NULL;
+
+       retval = g_dbus_connection_call_sync (test->service.connection,
+                                             test->service.bus_name,
+                                             path, "org.freedesktop.DBus.Properties",
+                                             "Set", g_variant_new ("(ssv)", interface, property, value),
+                                             G_VARIANT_TYPE ("()"),
+                                             G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error);
+       g_assert_no_error (error);
+       g_variant_unref (retval);
+}
+
+static void
+test_created_modified_properties (Test *test,
+                                  gconstpointer unused)
+{
+       GVariantBuilder builder;
+       GError *error = NULL;
+       GVariant *retval;
+       gchar *item;
+       gchar *prompt;
+       guint64 created;
+       guint64 modified;
+       GVariant *props;
+
+       g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
+       g_variant_builder_add (&builder, "{sv}", SECRET_ITEM_INTERFACE ".Label", g_variant_new_string ("The 
Label"));
+       props = g_variant_builder_end (&builder);
+
+       retval = g_dbus_connection_call_sync (test->service.connection,
+                                             test->service.bus_name,
+                                             "/org/freedesktop/secrets/collection/test",
+                                             SECRET_COLLECTION_INTERFACE,
+                                             "CreateItem",
+                                             g_variant_new ("(@a{sv}@(oayays)b)", props,
+                                                            test_service_build_secret (&test->service, "the 
secret"), TRUE),
+                                             G_VARIANT_TYPE ("(oo)"),
+                                             G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, NULL, &error);
+       g_assert_no_error (error);
+
+       g_variant_get (retval, "(oo)", &item, &prompt);
+       g_assert_cmpstr (prompt, ==, "/");
+       g_assert_cmpstr (item, !=, "/");
+       g_variant_unref (retval);
+       g_free (prompt);
+
+       props = get_all_properties (test, item, SECRET_ITEM_INTERFACE);
+       if (!g_variant_lookup (props, "Created", "t", &created))
+               g_assert_not_reached ();
+       if (!g_variant_lookup (props, "Modified", "t", &modified))
+               g_assert_not_reached ();
+       g_variant_unref (props);
+
+       /* Created and modified within the last 10 seconds */
+       g_assert_cmpuint (created, >, (g_get_real_time () / G_TIME_SPAN_SECOND) - 10);
+       g_assert_cmpuint (modified, >, (g_get_real_time () / G_TIME_SPAN_SECOND) - 10);
+       g_assert_cmpuint (created, ==, modified);
+
+       if (!g_test_thorough ())
+               return;
+
+       /* Unfortunately have to wait 1.25 seconds here */
+       g_usleep (G_TIME_SPAN_SECOND + (G_TIME_SPAN_SECOND / 4));
+
+       /* Now modify the item */
+       set_property (test, item, SECRET_ITEM_INTERFACE, "Label", g_variant_new_string ("New Label"));
+
+       /* Check the properties again */
+       props = get_all_properties (test, item, SECRET_ITEM_INTERFACE);
+       if (!g_variant_lookup (props, "Created", "t", &created))
+               g_assert_not_reached ();
+       if (!g_variant_lookup (props, "Modified", "t", &modified))
+               g_assert_not_reached ();
+       g_variant_unref (props);
+
+       /* Modified should have changed */
+       g_assert_cmpuint (modified, >, (g_get_real_time () / G_TIME_SPAN_SECOND) - 10);
+       g_assert_cmpuint (created, !=, modified);
+
+       g_free (item);
+}
+
+int
+main (int argc, char **argv)
+{
+#if !GLIB_CHECK_VERSION(2,35,0)
+       g_type_init ();
+#endif
+       g_test_init (&argc, &argv, NULL);
+
+       g_test_add ("/secret-item/created-modified-properties", Test, NULL,
+                   setup, test_created_modified_properties, teardown);
+
+       return egg_tests_run_with_loop ();
+}
diff --git a/pkcs11/secret-store/gkm-secret-collection.c b/pkcs11/secret-store/gkm-secret-collection.c
index cd5df63..6e5bb95 100644
--- a/pkcs11/secret-store/gkm-secret-collection.c
+++ b/pkcs11/secret-store/gkm-secret-collection.c
@@ -334,6 +334,7 @@ factory_create_collection (GkmSession *session, GkmTransaction *transaction,
                                   "label", label,
                                   NULL);
 
+       gkm_secret_object_mark_created (GKM_SECRET_OBJECT (collection));
        g_free (identifier);
        g_free (label);
 
@@ -795,6 +796,7 @@ gkm_secret_collection_create_item (GkmSecretCollection *self, GkmTransaction *tr
 
        g_free (identifier);
        add_item (self, transaction, item);
+       gkm_secret_object_mark_created (GKM_SECRET_OBJECT (item));
        g_object_unref (item);
        return item;
 }
diff --git a/pkcs11/secret-store/gkm-secret-item.c b/pkcs11/secret-store/gkm-secret-item.c
index d7cbdb3..a2a92e6 100644
--- a/pkcs11/secret-store/gkm-secret-item.c
+++ b/pkcs11/secret-store/gkm-secret-item.c
@@ -67,7 +67,6 @@ complete_set_schema (GkmTransaction *transaction, GObject *obj, gpointer user_da
        } else {
                gkm_object_notify_attribute (GKM_OBJECT (obj), CKA_G_SCHEMA);
                g_object_notify (G_OBJECT (obj), "schema");
-               gkm_secret_object_was_modified (GKM_SECRET_OBJECT (self));
                g_free (old_schema);
        }
 
@@ -81,6 +80,7 @@ begin_set_schema (GkmSecretItem *self, GkmTransaction *transaction, gchar *schem
        g_assert (!gkm_transaction_get_failed (transaction));
 
        if (self->schema != schema) {
+               gkm_secret_object_begin_modified (GKM_SECRET_OBJECT (self), transaction);
                gkm_transaction_add (transaction, self, complete_set_schema, self->schema);
                self->schema = schema;
        }
@@ -89,11 +89,8 @@ begin_set_schema (GkmSecretItem *self, GkmTransaction *transaction, gchar *schem
 static gboolean
 complete_set_secret (GkmTransaction *transaction, GObject *obj, gpointer user_data)
 {
-       GkmSecretItem *self = GKM_SECRET_ITEM (obj);
-
        if (!gkm_transaction_get_failed (transaction)) {
                gkm_object_notify_attribute (GKM_OBJECT (obj), CKA_VALUE);
-               gkm_secret_object_was_modified (GKM_SECRET_OBJECT (self));
        }
 
        return TRUE;
@@ -112,7 +109,6 @@ complete_set_fields (GkmTransaction *transaction, GObject *obj, gpointer user_da
        } else {
                gkm_object_notify_attribute (GKM_OBJECT (obj), CKA_G_FIELDS);
                g_object_notify (G_OBJECT (obj), "fields");
-               gkm_secret_object_was_modified (GKM_SECRET_OBJECT (self));
                if (old_fields)
                        g_hash_table_unref (old_fields);
        }
@@ -126,6 +122,7 @@ begin_set_fields (GkmSecretItem *self, GkmTransaction *transaction, GHashTable *
        g_assert (GKM_IS_SECRET_OBJECT (self));
        g_assert (!gkm_transaction_get_failed (transaction));
 
+       gkm_secret_object_begin_modified (GKM_SECRET_OBJECT (self), transaction);
        gkm_transaction_add (transaction, self, complete_set_fields, self->fields);
        self->fields = fields;
 }
@@ -278,6 +275,7 @@ gkm_secret_item_real_set_attribute (GkmObject *base, GkmSession *session,
                gkm_secret_data_set_transacted (sdata, transaction, identifier, secret);
                g_object_unref (secret);
                g_object_unref (sdata);
+               gkm_secret_object_begin_modified (GKM_SECRET_OBJECT (self), transaction);
                if (!gkm_transaction_get_failed (transaction))
                        gkm_transaction_add (transaction, self, complete_set_secret, NULL);
                return;
diff --git a/pkcs11/secret-store/gkm-secret-object.c b/pkcs11/secret-store/gkm-secret-object.c
index 2c71719..580dfa6 100644
--- a/pkcs11/secret-store/gkm-secret-object.c
+++ b/pkcs11/secret-store/gkm-secret-object.c
@@ -64,7 +64,6 @@ complete_set_label (GkmTransaction *transaction, GObject *obj, gpointer user_dat
        } else {
                gkm_object_notify_attribute (GKM_OBJECT (obj), CKA_LABEL);
                g_object_notify (G_OBJECT (obj), "label");
-               gkm_secret_object_was_modified (self);
                g_free (old_label);
        }
 
@@ -77,6 +76,7 @@ begin_set_label (GkmSecretObject *self, GkmTransaction *transaction, gchar *labe
        g_assert (GKM_IS_SECRET_OBJECT (self));
        g_assert (!gkm_transaction_get_failed (transaction));
 
+       gkm_secret_object_begin_modified (GKM_SECRET_OBJECT (self), transaction);
        gkm_transaction_add (transaction, self, complete_set_label, self->pv->label);
        self->pv->label = label;
 }
@@ -361,11 +361,30 @@ gkm_secret_object_get_created (GkmSecretObject *self)
 void
 gkm_secret_object_set_created (GkmSecretObject *self, glong when)
 {
+       GTimeVal tv;
+
        g_return_if_fail (GKM_IS_SECRET_OBJECT (self));
+
+       if (when < 0) {
+               g_get_current_time (&tv);
+               when = tv.tv_sec;
+       }
+
        self->pv->created = when;
        g_object_notify (G_OBJECT (self), "created");
 }
 
+void
+gkm_secret_object_mark_created (GkmSecretObject *self)
+{
+       GTimeVal tv;
+
+       g_return_if_fail (GKM_IS_SECRET_OBJECT (self));
+
+       g_get_current_time (&tv);
+       gkm_secret_object_set_created (self, tv.tv_sec);
+}
+
 glong
 gkm_secret_object_get_modified (GkmSecretObject *self)
 {
@@ -381,13 +400,38 @@ gkm_secret_object_set_modified (GkmSecretObject *self, glong when)
        g_object_notify (G_OBJECT (self), "modified");
 }
 
+static gboolean
+complete_set_modified (GkmTransaction *transaction,
+                       GObject *obj,
+                       gpointer user_data)
+{
+       GkmSecretObject *self = GKM_SECRET_OBJECT (obj);
+       glong *old_modified = user_data;
+
+       if (gkm_transaction_get_failed (transaction)) {
+               self->pv->modified = *old_modified;
+
+       } else {
+               gkm_object_notify_attribute (GKM_OBJECT (obj), CKA_G_MODIFIED);
+               g_object_notify (G_OBJECT (obj), "modified");
+       }
+
+       g_free (old_modified);
+       return TRUE;
+}
+
 void
-gkm_secret_object_was_modified (GkmSecretObject *self)
+gkm_secret_object_begin_modified (GkmSecretObject *self,
+                                  GkmTransaction *transaction)
 {
        GTimeVal tv;
-       g_return_if_fail (GKM_IS_SECRET_OBJECT (self));
+
+       g_return_if_fail (!gkm_transaction_get_failed (transaction));
+       gkm_transaction_add (transaction, self, complete_set_modified,
+                            g_memdup (&self->pv->modified, sizeof (gulong)));
+
        g_get_current_time (&tv);
-       gkm_secret_object_set_modified (self, tv.tv_sec);
+       self->pv->modified = tv.tv_sec;
 }
 
 gboolean
diff --git a/pkcs11/secret-store/gkm-secret-object.h b/pkcs11/secret-store/gkm-secret-object.h
index 9cd458a..1dc361c 100644
--- a/pkcs11/secret-store/gkm-secret-object.h
+++ b/pkcs11/secret-store/gkm-secret-object.h
@@ -64,12 +64,15 @@ glong                gkm_secret_object_get_created     (GkmSecretObject *self);
 void                 gkm_secret_object_set_created     (GkmSecretObject *self,
                                                         glong value);
 
+void                 gkm_secret_object_mark_created    (GkmSecretObject *self);
+
 glong                gkm_secret_object_get_modified    (GkmSecretObject *self);
 
 void                 gkm_secret_object_set_modified    (GkmSecretObject *self,
                                                         glong value);
 
-void                 gkm_secret_object_was_modified    (GkmSecretObject *self);
+void                 gkm_secret_object_begin_modified  (GkmSecretObject *self,
+                                                        GkmTransaction *transaction);
 
 gboolean             gkm_secret_object_is_locked       (GkmSecretObject *self,
                                                         GkmSession *session);
diff --git a/pkcs11/secret-store/tests/test-secret-object.c b/pkcs11/secret-store/tests/test-secret-object.c
index be33ff5..a916387 100644
--- a/pkcs11/secret-store/tests/test-secret-object.c
+++ b/pkcs11/secret-store/tests/test-secret-object.c
@@ -129,9 +129,15 @@ test_modified_prop (Test *test, gconstpointer unused)
 static void
 test_was_modified (Test *test, gconstpointer unused)
 {
+       GkmTransaction *transaction;
        GTimeVal tv;
+
        g_get_current_time (&tv);
-       gkm_secret_object_was_modified (test->object);
+
+       transaction = gkm_transaction_new ();
+       gkm_secret_object_begin_modified (test->object, transaction);
+       g_assert_cmpuint (gkm_transaction_complete_and_unref (transaction), ==, CKR_OK);
+
        g_assert (tv.tv_sec == gkm_secret_object_get_modified (test->object));
 }