[rhythmbox] rhythmdb: fix handling of empty strings when looking for sort values

[Jonathan Matthew <jmatthew src gnome org>]

commit 733698890a79bb1df1c56853104d52051e9cdac8
Author: Jonathan Matthew <jonathan d14n org>
Date:   Sun Feb 21 16:18:05 2010 +1000

    rhythmdb: fix handling of empty strings when looking for sort values
    
    If none of the sort properties has a non-empty value, use the display
    property, which is almost certainly going to be empty too.

 rhythmdb/rhythmdb-property-model.c   |    9 ++++-
 tests/test-rhythmdb-property-model.c |   75 ++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletions(-)
---
diff --git a/rhythmdb/rhythmdb-property-model.c b/rhythmdb/rhythmdb-property-model.c
index faee4a5..e50a376 100644
--- a/rhythmdb/rhythmdb-property-model.c
+++ b/rhythmdb/rhythmdb-property-model.c
@@ -730,7 +730,7 @@ update_sort_string (RhythmDBPropertyModel *model,
 	}
 
 	/* if we found one, replace the current sort string */
-	if (newvalue != NULL && (prop->sort_string == NULL || pi < prop->sort_string_from)) {
+	if (newvalue != NULL && newvalue[0] != '\0' && (prop->sort_string == NULL || pi < prop->sort_string_from)) {
 		rb_debug ("replacing current sort string %s with %s (%d -> %d)",
 			  prop->sort_string ? rb_refstring_get (prop->sort_string) : "NULL",
 			  newvalue,
@@ -740,9 +740,16 @@ update_sort_string (RhythmDBPropertyModel *model,
 			rb_refstring_unref (prop->sort_string);
 		}
 		prop->sort_string = rb_refstring_new (newvalue);
+		g_assert (pi < model->priv->sort_propids->len);
 		prop->sort_string_from = pi;
 		return TRUE;
 	}
+
+	/* if we can't find a sort string at all, use the display string as fallback */
+	if (prop->sort_string == NULL) {
+		prop->sort_string = rb_refstring_ref (prop->string);
+	}
+
 	return FALSE;
 }
 
diff --git a/tests/test-rhythmdb-property-model.c b/tests/test-rhythmdb-property-model.c
index c50c416..7905b52 100644
--- a/tests/test-rhythmdb-property-model.c
+++ b/tests/test-rhythmdb-property-model.c
@@ -589,6 +589,80 @@ START_TEST (test_rhythmdb_property_model_sorting)
 }
 END_TEST
 
+/* tests handling of empty strings */
+START_TEST (test_rhythmdb_property_model_empty_strings)
+{
+	RhythmDBQueryModel *model;
+	RhythmDBPropertyModel *propmodel;
+	RhythmDBEntry *a, *b;
+
+	start_test_case ();
+
+	/* setup */
+	model = rhythmdb_query_model_new_empty (db);
+	propmodel = rhythmdb_property_model_new (db, RHYTHMDB_PROP_GENRE);
+	g_object_set (propmodel, "query-model", model, NULL);
+
+	/* create test entries */
+	set_waiting_signal (G_OBJECT (db), "entry_added");
+	a = rhythmdb_entry_new (db, RHYTHMDB_ENTRY_TYPE_IGNORE, "file:///a.ogg");
+	set_entry_string (db, a, RHYTHMDB_PROP_GENRE, "unknown");
+	rhythmdb_commit (db);
+	wait_for_signal ();
+
+	set_waiting_signal (G_OBJECT (db), "entry_added");
+	b = rhythmdb_entry_new (db, RHYTHMDB_ENTRY_TYPE_IGNORE, "file:///b.ogg");
+	set_entry_string (db, b, RHYTHMDB_PROP_GENRE, "something");
+	rhythmdb_commit (db);
+	wait_for_signal ();
+
+	end_step ();
+
+	/* add to model */
+	set_waiting_signal (G_OBJECT (propmodel), "row-inserted");
+	rhythmdb_query_model_add_entry (model, a, -1);
+	wait_for_signal ();
+
+	set_waiting_signal (G_OBJECT (propmodel), "row-inserted");
+	rhythmdb_query_model_add_entry (model, b, -1);
+	wait_for_signal ();
+
+	end_step ();
+
+	/* set to empty string */
+	set_waiting_signal (G_OBJECT (propmodel), "row-inserted");
+	set_entry_string (db, a, RHYTHMDB_PROP_GENRE, "");
+	rhythmdb_commit (db);
+	wait_for_signal ();
+
+	end_step ();
+
+	/* set to non-empty string */
+	set_waiting_signal (G_OBJECT (propmodel), "row-inserted");
+	set_entry_string (db, a, RHYTHMDB_PROP_GENRE, "junk");
+	rhythmdb_commit (db);
+	wait_for_signal ();
+
+	end_step ();
+
+	/* set to empty string again */
+	set_waiting_signal (G_OBJECT (propmodel), "row-inserted");
+	set_entry_string (db, a, RHYTHMDB_PROP_GENRE, "");
+	rhythmdb_commit (db);
+	wait_for_signal ();
+
+	end_step ();
+
+	rhythmdb_entry_delete (db, a);
+	rhythmdb_entry_delete (db, b);
+	rhythmdb_commit (db);
+
+	end_test_case ();
+	g_object_unref (model);
+	g_object_unref (propmodel);
+}
+END_TEST
+
 static Suite *
 rhythmdb_property_model_suite (void)
 {
@@ -609,6 +683,7 @@ rhythmdb_property_model_suite (void)
 
 	/* tests for breakable bug fixes */
 /*	tcase_add_test (tc_bugs, test_hidden_chain_filter);*/
+	tcase_add_test (tc_chain, test_rhythmdb_property_model_empty_strings);
 
 	return s;
 }