r6891 - dumbhippo/trunk/client/common/ddm

[commits mugshot org]

Author: otaylor
Date: 2007-11-12 11:16:10 -0600 (Mon, 12 Nov 2007)
New Revision: 6891

Modified:
   dumbhippo/trunk/client/common/ddm/ddm-data-model.c
   dumbhippo/trunk/client/common/ddm/ddm-data-resource-internal.h
   dumbhippo/trunk/client/common/ddm/ddm-data-resource.c
   dumbhippo/trunk/client/common/ddm/test-rules.c
Log:
* Handle cleaning up rules properly on reconnection (ddm_data_model_reset())
* Simple test of ddm_data_model_reset()
* Fix bug with reset() with outstanding changed properties
* Fix bug with adding resources instead of properties to resource->referencing_rule_properties


Modified: dumbhippo/trunk/client/common/ddm/ddm-data-model.c
===================================================================
--- dumbhippo/trunk/client/common/ddm/ddm-data-model.c	2007-11-09 23:06:59 UTC (rev 6890)
+++ dumbhippo/trunk/client/common/ddm/ddm-data-model.c	2007-11-12 17:16:10 UTC (rev 6891)
@@ -71,10 +71,6 @@
 static void
 ddm_data_model_init(DDMDataModel *model)
 {
-    model->resources = g_hash_table_new_full(g_str_hash, g_str_equal,
-                                             NULL,
-                                             (GDestroyNotify)ddm_data_resource_unref);
-
     /* rules_by_target and rules_by_source together own the reference to the rule.
      * We consider the reference owned by rules_by_target
      */
@@ -85,7 +81,13 @@
                                                    (GDestroyNotify)g_free,
                                                    (GDestroyNotify)g_slist_free);
     
-    model->changed_resources = g_hash_table_new(g_direct_hash, NULL);
+    model->resources = g_hash_table_new_full(g_str_hash, g_str_equal,
+                                             NULL,
+                                             (GDestroyNotify)ddm_data_resource_unref);
+    model->changed_resources = g_hash_table_new_full(g_direct_hash, NULL,
+                                                     NULL,
+                                                     (GDestroyNotify)ddm_data_resource_unref);
+
     model->work_items = g_queue_new();
 
     model->max_answered_query_serial = -1;
@@ -521,18 +523,24 @@
 {
     DDMDataResource *resource = value;
 
-    if (ddm_data_resource_is_local(resource)) {
-        _ddm_data_resource_reset(resource);
-        return FALSE;
-    } else {
-        return TRUE;
-    }
+    return _ddm_data_resource_reset(resource);
 }
 
+static gboolean
+model_reset_changed_resource_foreach (gpointer key,
+                                      gpointer value,
+                                      gpointer data)
+{
+    DDMDataResource *resource = value;
+
+    return !ddm_data_resource_is_local(resource);
+}
+
 void
 ddm_data_model_reset (DDMDataModel *model)
 {
     g_hash_table_foreach_remove(model->resources, model_reset_foreach, NULL);
+    g_hash_table_foreach_remove(model->changed_resources, model_reset_changed_resource_foreach, NULL);
 
     if (model->global_resource != NULL && !ddm_data_resource_is_local(model->global_resource)) {
         ddm_data_resource_unref(model->global_resource);
@@ -569,7 +577,7 @@
                              DDMDataResource *resource)
 {
     if (g_hash_table_lookup(model->changed_resources, resource) == NULL) {
-        g_hash_table_insert(model->changed_resources, resource, resource);
+        g_hash_table_insert(model->changed_resources, resource, ddm_data_resource_ref(resource));
     }
 
     ddm_data_model_schedule_flush(model);

Modified: dumbhippo/trunk/client/common/ddm/ddm-data-resource-internal.h
===================================================================
--- dumbhippo/trunk/client/common/ddm/ddm-data-resource-internal.h	2007-11-09 23:06:59 UTC (rev 6890)
+++ dumbhippo/trunk/client/common/ddm/ddm-data-resource-internal.h	2007-11-12 17:16:10 UTC (rev 6891)
@@ -14,7 +14,10 @@
                                          const char      *class_id,
                                          gboolean         local);
 
-void _ddm_data_resource_reset (DDMDataResource *resource);
+/* Called on reconnection to the backend server. returns TRUE if the resource
+ * should be removed from the resource table.
+ */
+gboolean _ddm_data_resource_reset (DDMDataResource *resource);
 
 GSList *_ddm_data_resource_get_default_properties (DDMDataResource *resource);
 

Modified: dumbhippo/trunk/client/common/ddm/ddm-data-resource.c
===================================================================
--- dumbhippo/trunk/client/common/ddm/ddm-data-resource.c	2007-11-09 23:06:59 UTC (rev 6890)
+++ dumbhippo/trunk/client/common/ddm/ddm-data-resource.c	2007-11-12 17:16:10 UTC (rev 6891)
@@ -79,6 +79,9 @@
 
 };
 
+static void property_remove_rule_source(DDMDataProperty *property,
+                                        DDMDataResource *source);
+
 GQuark
 ddm_data_error_quark (void)
 {
@@ -275,6 +278,21 @@
 }
 
 static gboolean
+reset_resource_rule_source_foreach(gpointer     data,
+                                   gpointer     user_data)
+{
+    DDMDataResource *source = data;
+    DDMDataProperty *property = user_data;
+
+    if (!source->local) {
+        source->referencing_rule_properties = g_slist_remove(source->referencing_rule_properties, property);
+        return TRUE;
+    } else {
+        return FALSE;
+    }
+}
+
+static gboolean
 reset_property_foreach(gpointer    data,
                        gpointer    user_data)
 {
@@ -283,6 +301,8 @@
     if (DDM_DATA_BASE(property->value.type) != DDM_DATA_RESOURCE)
         return FALSE;
 
+    property->rule_sources = slist_foreach_remove(property->rule_sources, reset_resource_rule_source_foreach, property);
+
     if (DDM_DATA_IS_LIST(property->value.type)) {
         property->value.u.list = slist_foreach_remove(property->value.u.list, reset_resource_value_foreach, NULL);
         reset_resource_value_foreach(data, user_data);
@@ -297,25 +317,57 @@
     }
 }
 
-void
+gboolean
 _ddm_data_resource_reset (DDMDataResource *resource)
 {
-    g_return_if_fail(resource != NULL);
-    g_return_if_fail(resource->local);
+    g_return_val_if_fail(resource != NULL, FALSE);
 
-    resource->properties = slist_foreach_remove(resource->properties, reset_property_foreach, NULL);
+    if (resource->local) {
+        /* For a local resource, we need to go through the properties, see which properties
+         * reference remote resources, and remove those property values.
+         */
+        
+        resource->properties = slist_foreach_remove(resource->properties, reset_property_foreach, NULL);
+        
+        if (resource->requested_fetch != NULL) {
+            ddm_data_fetch_unref(resource->requested_fetch);
+            resource->requested_fetch = NULL;
+        }
+        
+        if (resource->received_fetch != NULL) {
+            ddm_data_fetch_unref(resource->received_fetch);
+            resource->received_fetch = NULL;
+        }
 
-    if (resource->requested_fetch != NULL) {
-        ddm_data_fetch_unref(resource->requested_fetch);
-        resource->requested_fetch = NULL;
+        resource->requested_serial = -1;
+
+        return FALSE;
+        
+    } else {
+        /* For a remote resource, we remove the resource entirely from the resource table.
+         * after unlinking it from properties that reference it and that it references.
+         */
+        
+        GSList *l;
+
+        for (l = resource->properties; l; l = l->next) {
+            DDMDataProperty *property = l->data;
+            GSList *ll;
+
+            for (ll = property->rule_sources; ll; ll = ll->next) {
+                DDMDataResource *source = ll->data;
+
+                source->referencing_rule_properties = g_slist_remove(source->referencing_rule_properties, property);
+            }
+        }
+
+        while (resource->referencing_rule_properties) {
+            DDMDataProperty *property = resource->referencing_rule_properties->data;
+            property_remove_rule_source(property, resource);
+        }
+        
+        return TRUE;
     }
-    
-    if (resource->received_fetch != NULL) {
-        ddm_data_fetch_unref(resource->received_fetch);
-        resource->received_fetch = NULL;
-    }
-
-    resource->requested_serial = -1;    
 }
 
 DDMDataResource *
@@ -337,6 +389,11 @@
 
     resource->refcount--;
     if (resource->refcount == 0) {
+        if (resource->referencing_rule_properties != NULL) {
+            g_warning("Freeing resource '%s' that is still referenced", resource->resource_id);
+            g_slist_free(resource->referencing_rule_properties);
+        }
+        
         g_free(resource->resource_id);
         g_free(resource->class_id);
 
@@ -1664,7 +1721,7 @@
                     source->resource_id, property->resource->resource_id,
                     property->qname->uri, property->qname->name);
             
-            source->referencing_rule_properties = g_slist_prepend(source->referencing_rule_properties, property->resource);
+            source->referencing_rule_properties = g_slist_prepend(source->referencing_rule_properties, property);
             property_update_value_from_rule_sources(property);
             
             return;
@@ -1691,7 +1748,7 @@
                 property->rule_sources = l->next;
             g_slist_free1(l);
 
-            source->referencing_rule_properties = g_slist_remove(source->referencing_rule_properties, property->resource);
+            source->referencing_rule_properties = g_slist_remove(source->referencing_rule_properties, property);
             property_update_value_from_rule_sources(property);
             
             g_debug("Removing rule source %s from %s:%s#%s",

Modified: dumbhippo/trunk/client/common/ddm/test-rules.c
===================================================================
--- dumbhippo/trunk/client/common/ddm/test-rules.c	2007-11-09 23:06:59 UTC (rev 6890)
+++ dumbhippo/trunk/client/common/ddm/test-rules.c	2007-11-12 17:16:10 UTC (rev 6891)
@@ -91,6 +91,22 @@
 
     g_assert(g_slist_length(aimBuddies) == 1);
     g_assert(aimBuddies->data == buddy1);
+
+    /* Now reset the model as if reconnecting to a remote server, and check that
+     * it cleaned up references to remote resources;x
+     */
+
+    ddm_data_model_reset(model);
     
+    ddm_data_resource_get(buddy1,
+                          "user", DDM_DATA_RESOURCE, &user,
+                          NULL);
+    g_assert(user == NULL);
+
+    /* Flush to make sure that we have nothing queued up from the reset that will
+     * cause problems
+     */
+    test_flush();
+
     return 0;
 }