library: Update the code and unit test

Lots of changes here!  I switched from using track and library ids to
passing pointers, renamed some functions, and made the code much
cleaner.

Signed-off-by: Anna Schumaker <Anna@OcarinaProject.net>

DESIGN

@@ -1063,25 +1063,25 @@ Library:
 		Scan the tagdb track list, and add each track to the library
 		queue.
 
-	Library *library :: add_path(string dir);
+	Library *library :: add(string dir);
 		If dir is not a directory:
 			return NULL
 
 		Add a new path to the tag database, trigger an update, and
 		then return the corresponding Library tag to the caller.
 
-	void library :: del_path(unsigned int lib_id);
+	void library :: remove(Library *library);
 		Invalidate a library_db row and all tracks owned by that path.
-		if lib_id is not valid do nothing.
+		Do not use the library pointer after calling this function.
 
-	void library :: update_path(unsigned int lib_id);
+	void library :: update(Library *library);
 		First, validate all tracks in the given library.
 		Next, trigger an update on the given library.
 
 	void library :: update_all();
 		Update all valid library paths.
 
-	void library :: set_enabled(unsigned int id, bool enabled);
+	void library :: set_enabled(Library *library, bool enabled);
 		Toggle if a library path is enabled or not.  A disabled
 		library path will have its tracks removed from the
 		LibraryQueue.

gui/main.cpp

@@ -28,6 +28,7 @@ const std::string share_file(const std::string &file)
 Gtk::Window *ocarina_init(int *argc, char ***argv)
 {
 	Gtk::Window *window = setup_gui();
+	tagdb :: init();
 	audio::init(argc, argv);
 	deck::init();
 	library::init();

include/callback.h

@@ -20,7 +20,6 @@ struct Callbacks {
 	void (*on_pq_removed)(Queue *);
 
 	/* Library callbacks */
-	void (*on_library_add)(unsigned int, Library *);
 	void (*on_library_update)(unsigned int, Library *);
 	void (*on_library_track_add)(unsigned int);
 	void (*on_library_track_del)(unsigned int);

include/library.h

@@ -4,31 +4,23 @@
 #ifndef OCARINA_LIBRARY_H
 #define OCARINA_LIBRARY_H
 
+#include <queue.h>
 #include <tags.h>
 #include <string>
 
 namespace library
 {
 
-	enum DB_Type {
-		DB_ALBUM,
-		DB_ARTIST,
-		DB_GENRE,
-		DB_LIBRARY,
-		DB_TRACK,
-	};
-
-
 	void init();
-	void add_path(const std::string &);
-	void del_path(unsigned int);
-	void update_path(unsigned int);
+
+	Library *add(const std::string &);
+	void remove(Library *);
+	void update(Library *);
 	void update_all();
-	void set_enabled(unsigned int, bool);
-#ifdef CONFIG_TEST
-	void print_db(DB_Type);
-	void reset();
-#endif /* CONFIG_TEST */
+
+	void set_enabled(Library *, bool);
+	Queue *get_queue();
+
 };
 
 #endif /* OCARINA_LIBRARY_H */

include/queue.h

@@ -44,8 +44,8 @@ public:
 	void write(File &);
 	void read(File &);
 
-	void set_flag(queue_flags);
-	void unset_flag(queue_flags);
+	virtual void set_flag(queue_flags);
+	virtual void unset_flag(queue_flags);
 	bool has_flag(queue_flags);
 
 	virtual unsigned int add(Track *);
@@ -58,7 +58,7 @@ public:
 	const std::string size_str();
 	const std::string length_str();
 
-	void sort(sort_t, bool);
+	virtual void sort(sort_t, bool);
 	Track *operator[](unsigned int);
 	void track_selected(unsigned int);
 };

lib/callback.cpp

@@ -22,7 +22,6 @@ static struct Callbacks callbacks = {
 	.on_pq_created			= no_op,
 	.on_pq_removed			= no_op,
 
-	.on_library_add			= no_op,
 	.on_library_update		= no_op,
 	.on_library_track_add		= no_op,
 	.on_library_track_del		= no_op,

lib/library.cpp

@@ -1,47 +1,99 @@
 /*
  * Copyright 2013 (c) Anna Schumaker.
  */
-#include <callback.h>
-#include <filter.h>
 #include <idle.h>
 #include <library.h>
-#include <print.h>
 
 #include <glib.h>
-#include <sstream>
-#include <time.h>
 
 
-/*
- * Internal library functions
- */
+class LibraryQueue : public Queue {
+private:
+	File f;
+
+public:
+
+	LibraryQueue() : Queue(Q_ENABLED | Q_REPEAT), f("library.q")
+	{
+		Queue :: sort(SORT_ARTIST, true);
+		Queue :: sort(SORT_YEAR,  false);
+		Queue :: sort(SORT_TRACK, false);
+	}
+
+	void save()
+	{
+		std::vector<struct sort_info>::iterator it;
+
+		f.open(OPEN_WRITE);
+		f << _flags << " " << _sort_order.size();
+		for (it = _sort_order.begin(); it != _sort_order.end(); it++)
+			f << " " << it->field << " " << it->ascending;
+		f << std::endl;
+		f.close();
+	}
+
+	void load()
+	{
+		unsigned int field;
+		bool ascending;
+		unsigned int n;
+
+		if (!f.open(OPEN_READ))
+			return;
+
+		f >> _flags >> n;
+		for (unsigned int i = 0; i < n; i++) {
+			f >> field >> ascending;
+			Queue :: sort((sort_t)field, (i == 0) ? true : false);
+			if (ascending == false)
+				Queue :: sort((sort_t)field, false);
+		}
+	}
+
+	void   set_flag(queue_flags f) { Queue ::   set_flag(f); save(); }
+	void unset_flag(queue_flags f) { Queue :: unset_flag(f); save(); }
+
+	void sort(sort_t field, bool ascending)
+	{
+		Queue :: sort(field, ascending);
+		save();
+	};
+
+};
+
+static LibraryQueue library_q;
+
 struct scan_info {
 	Library *library;
 	std :: string path;
 };
-static void do_scan_path(struct scan_info &);
 
-static void read_tags(const std::string &path, Library *library)
-{
-	Track *track = tagdb :: add_track(path, library);
-	get_callbacks()->on_library_track_add(track->id);
-}
+static void scan_path(struct scan_info &);
+
+
+
+/*
+ * Scanning functions are here
+ */
 
 static void process_path(Library *library, const std :: string &dir,
 			 const std :: string &name)
 {
-	struct scan_info scan;
-	std :: string path = dir + "/" + name;
+	struct scan_info scan = {
+		.library = library,
+		.path = dir + "/" + name,
+	};
 
-	if (g_file_test(path.c_str(), G_FILE_TEST_IS_DIR) == true) {
-		scan.library = library;
-		scan.path = path;
-		idle :: schedule (do_scan_path, scan);
-	} else
-		read_tags(path, library);
+	if (g_file_test(scan.path.c_str(), G_FILE_TEST_IS_DIR) == true)
+		idle :: schedule (scan_path, scan);
+	else {
+		Track *track = tagdb :: add_track(scan.path, library);
+		if (track)
+			library_q.add(track);
+	}
 }
 
-static void do_scan_path(struct scan_info &scan)
+static void scan_path(struct scan_info &scan)
 {
 	GDir *dir;
 	const char *name;
@@ -57,143 +109,110 @@ static void do_scan_path(struct scan_info &scan)
 	}
 
 	tagdb :: commit();
-	get_callbacks()->on_library_update(scan.library->id, scan.library);
 }
 
-static void do_validate_library(unsigned int &lib_id)
+static void validate_library(Library *&library)
 {
-	std :: string path;
-	Database<Track> *db = &tagdb :: get_track_db();
+	Track *track;
 	Database<Track>::iterator it;
+	Database<Track> *db = &tagdb :: get_track_db();
 
 	for (it = db->begin(); it != db->end(); it = db->next(it)) {
-		Track *track = *it;
-		if (track->library->id != lib_id)
+		track = *it;
+		if (track->library != library)
 			continue;
 
 		if (g_file_test(track->path().c_str(), G_FILE_TEST_EXISTS) == false) {
-			dprint("Removing file: %s\n", track->path().c_str());
+			library_q.del(track);
 			tagdb :: remove_track(track->id);
 		}
 	}
-
-	get_callbacks()->on_library_update(lib_id, tagdb :: lookup_library(lib_id));
-}
-
-static void do_update_library(unsigned int lib_id)
-{
-	Library *library = tagdb :: lookup_library(lib_id);
-	struct scan_info scan = { library, library->root_path };
-	idle :: schedule(do_validate_library, lib_id);
-	idle :: schedule(do_scan_path, scan);
 }
 
 
 
 /*
- * API used by the GUI begins here
+ * External API begins here
  */
 
 void library :: init()
 {
-	tagdb :: init();
-
-	Database<Track> *db = &tagdb :: get_track_db();
 	Database<Track>::iterator it;
+	Database<Track> *db = &tagdb :: get_track_db();
+
+	library_q.load();
 
 	for (it = db->begin(); it != db->end(); it = db->next(it)) {
 		if ((*it)->library->enabled)
-			get_callbacks()->on_library_track_add((*it)->id);
+			library_q.add(*it);
 	}
-
-	Database<Library> *ldb = &tagdb :: get_library_db();
-	Database<Library>::iterator l_it;
-
-	for (l_it = ldb->begin(); l_it != ldb->end(); l_it = ldb->next(l_it))
-		get_callbacks()->on_library_add((*l_it)->id, *l_it);
 }
 
-void library :: add_path(const std::string &dir)
+Library *library :: add(const std::string &dir)
 {
+	Library *library = NULL;
+
 	if (g_file_test(dir.c_str(), G_FILE_TEST_IS_DIR) == false)
-		throw -E_INVAL;
+		return library;
 
-	Library *library = tagdb :: add_library(dir);
-	if (!library)
-		return;
-
-	get_callbacks()->on_library_add(library->id, library);
-	update_path(library->id);
-}
-
-void library :: del_path(unsigned int id)
-{
-	Database<Track> *db = &tagdb :: get_track_db();
-	Database<Track>::iterator it;
-
-	for (it = db->begin(); it != db->end(); it = db->next(it)) {
-		if ((*it)->library->id == id)
-			get_callbacks()->on_library_track_del((*it)->id);
-	}
-
-	tagdb :: remove_library(id);
-	tagdb :: commit();
-}
-
-void library :: update_path(unsigned int id)
-{
-	Library *library = tagdb :: lookup_library(id);
+	library = tagdb :: add_library(dir);
 	if (library)
-		do_update_library(library->id);
+		update(library);
+	return library;
+}
+
+void library :: remove(Library *library)
+{
+	if (library) {
+		set_enabled(library, false);
+		tagdb :: remove_library(library->id);
+	}
+}
+
+void library :: update(Library *library)
+{
+	struct scan_info scan = {
+		.library = library,
+	};
+
+	if (library) {
+		scan.path = library->root_path;
+		idle :: schedule(validate_library, library);
+		idle :: schedule(scan_path, scan);
+	}
 }
 
 void library :: update_all()
 {
-	Database<Library> *db = &tagdb :: get_library_db();
 	Database<Library>::iterator it;
+	Database<Library> *db = &tagdb :: get_library_db();
 
 	for (it = db->begin(); it != db->end(); it = db->next(it))
-		update_path((*it)->id);
+		update(*it);
 }
 
-void library :: set_enabled(unsigned int id, bool enabled)
+void library :: set_enabled(Library *library, bool enabled)
 {
-	Library *library = tagdb :: lookup_library(id);
+	Database<Track>::iterator it;
+	Database<Track> *db = &(tagdb :: get_track_db());
+
+	if (!library || (library->enabled == enabled))
+		return;
 
 	library->enabled = enabled;
 	tagdb :: commit_library();
 
-	Database<Track> *db = &(tagdb :: get_track_db());
-	Database<Track>::iterator it;
-
 	for (it = db->begin(); it != db->end(); it = db->next(it)) {
-		if ((*it)->library->id == id) {
+		if ((*it)->library == library) {
 			if (enabled)
-				get_callbacks()->on_library_track_add((*it)->id);
+				library_q.add(*it);
 			else
-				get_callbacks()->on_library_track_del((*it)->id);
+				library_q.del(*it);
 		}
 	}
 }
 
-#ifdef CONFIG_TEST
-void library :: print_db(DB_Type type)
+Queue *library :: get_queue()
 {
-	switch (type) {
-	case DB_ALBUM:
-		break;
-	case DB_ARTIST:
-		break;
-	case DB_GENRE:
-		break;
-	case DB_LIBRARY:
-		break;
-	case DB_TRACK:
-		break;
-	}
+	return &library_q;
 }
-
-void library :: reset()
-{
-}
-#endif /* CONFIG_TEST */

tests/.gitignore

@@ -7,4 +7,5 @@ idle
 tags
 random
 queue
+library
 playlist

tests/Sconscript

@@ -18,6 +18,7 @@ tests = [
 	("tags.cpp",     True,  [], [ "taglib" ]),
 	("random.cpp",   False, [ "random.cpp" ], []),
 	("queue.cpp",    True,  [ "callback.cpp", "random.cpp" ], []),
+	("library.cpp",  True,  [ "idle.cpp" ], []),
 	("playlist.cpp", True,  [], []),
 
 ]

tests/gen_library.sh

@@ -0,0 +1,26 @@
+#!/bin/bash
+#
+# Copyright 2013 (c) Anna Schumaker.
+#
+# Generate a test library in /tmp
+#
+
+# $1 - File, $2 - Directory number
+function tag_file()
+{
+	artist="Artist $2"
+	album="Album $2"
+	let date=2008+$2
+	vorbiscomment -w $1 -t "ARTIST=$artist" -t "ALBUM=$album" -t "DATE=$date"
+}
+
+
+mkdir -p /tmp/ocarina/dir{1..5}
+
+for i in $(seq 5); do
+	cp tests/Music/* /tmp/ocarina/dir$i/
+
+	for f in /tmp/ocarina/dir$i/*; do
+		tag_file $f $i
+	done
+done

tests/library.cpp

@@ -1,7 +1,119 @@
 /*
  * Copyright 2013 (c) Anna Schumaker.
  */
-#include <callback.h>
+#include <idle.h>
+#include <library.h>
+#include "test.h"
+
+static Queue *Q_NULL = NULL;
+static Library *LIB_NULL = NULL;
+
+static void test_init()
+{
+	Queue *q = library :: get_queue();
+
+	test_not_equal(q, Q_NULL);
+	test_equal(q->has_flag(Q_ENABLED), true);
+	test_equal(q->has_flag(Q_REPEAT),  true);
+
+	tagdb :: init();
+	library :: init();
+
+	test_equal(q->size(), (unsigned)24);
+}
+
+static void test_enable()
+{
+	Queue *q = library :: get_queue();
+	Library *library = tagdb :: lookup_library(0);
+
+	library :: set_enabled(LIB_NULL, true);
+	test_equal(q->size(), (unsigned)24);
+
+	library :: set_enabled(library, false);
+	test_equal(q->size(), (unsigned)0);
+
+	library :: set_enabled(library, true);
+	test_equal(q->size(), (unsigned)24);
+
+	library :: set_enabled(library, true);
+	test_equal(q->size(), (unsigned)24);
+
+	library :: set_enabled(library, false);
+	test_equal(q->size(), (unsigned)0);
+
+	library :: set_enabled(library, true);
+	test_equal(q->size(), (unsigned)24);
+}
+
+static void test_remove()
+{
+	Queue *q = library :: get_queue();
+	Library *library = tagdb :: lookup_library(0);
+
+	library :: remove(LIB_NULL);
+	test_equal(q->size(), (unsigned)24);
+
+	library :: remove(library);
+	test_equal(q->size(), (unsigned)0);
+
+	library :: remove(library);
+	test_equal(q->size(), (unsigned)0);
+}
+
+static void test_add()
+{
+	Queue *q = library :: get_queue();
+
+	test :: gen_library();
+	library :: add("/tmp/ocarina/");
+
+	test_equal(q->size(), (unsigned)0);
+	test_equal(idle :: run_task(), true);
+	test_equal(q->size(), (unsigned)0);
+
+	for (unsigned int i = 0; i < 6; i++) {
+		test_equal(idle :: run_task(), (i < 5) ? true : false);
+		test_equal(q->size(), i * 7);
+	}
+}
+
+static void test_update()
+{
+	Queue *q = library :: get_queue();
+	test :: rm_library_dirs();
+
+	library :: update_all();
+	test_equal(idle :: run_task(), true);
+	test_equal(q->size(), (unsigned)21);
+
+	for (unsigned int i = 0; i < 4; i++)
+		test_equal(idle :: run_task(), (i < 3) ? true : false);
+	test_equal(q->size(), (unsigned)21);
+
+
+	test :: gen_library();
+
+	library :: update_all();
+	test_equal(idle :: run_task(), true);
+	test_equal(q->size(), (unsigned)21);
+
+	for (unsigned int i = 0; i < 6; i++)
+		test_equal(idle :: run_task(), (i < 5) ? true : false);
+	test_equal(q->size(), (unsigned)35);
+}
+
+int main(int argc, char **argv)
+{
+	test :: cp_library();
+
+	run_test("Library Init Test", test_init);
+	run_test("Library Enable and Disable Test", test_enable);
+	run_test("Library Delete Path Test", test_remove);
+	run_test("Library Add Path Test", test_add);
+	run_test("Library Update Path Test", test_update);
+}
+/*#include <callback.h>
 #include <idle.h>
 #include <library.h>
 #include <print.h>
@@ -130,27 +242,27 @@ void test_print_dbs(const std::string &test)
 	print("Test %s\n", test.c_str());
 	library :: print_db(library :: DB_GENRE);
 	print("\n");
-}
+}*/
 
 
 /* Add paths library that SHOULD fail */
-void test_0()
+/*void test_0()
 {
 	test_add_dir("0a", "/tmp/library/error", false);
 	test_add_dir("0b", "/tmp/library/file", false);
 	print("\n");
-}
+}*/
 
 /* Simple library path operations */
-void test_1()
+/*void test_1()
 {
 	test_add_dir("1a", "/tmp/library/0", true);
 	test_del_dir("1b", 0);
 	print("\n");
-}
+}*/
 
 /* Test multiple paths */
-void test_2()
+/*void test_2()
 {
 	library :: reset();
 	test_add_dir("2a", "/tmp/library/0", true);
@@ -161,10 +273,10 @@ void test_2()
 	test_del_dir("2e", 0);
 	test_del_dir("2f", 2);
 	print("\n");
-}
+}*/
 
 /* Test load and save of library db */
-void test_3()
+/*void test_3()
 {
 	library :: reset();
 	test_add_dir("3a", "/tmp/library/0", true);
@@ -179,10 +291,10 @@ void test_3()
 	library :: init();
 	library :: print_db(library :: DB_LIBRARY);
 	print("\n");
-}
+}*/
 
 /* Test scanning a single path */
-void test_4()
+/*void test_4()
 {
 	library :: reset();
 	test_add_dir("4a", "/tmp/library/0", true);
@@ -195,32 +307,32 @@ void test_4()
 	print("\n");
 	library :: print_db(library :: DB_TRACK);
 	print("\n");
-}
+}*/
 
 /* Test lookup() */
-void test_5()
+/*void test_5()
 {
 	library :: reset();
-
+*/
 	/* Lookup on empty DB */
-	test_lookup("5a", 0, false);
-	test_add_dir("5b", "/tmp/library/0", true);
+//	test_lookup("5a", 0, false);
+//	test_add_dir("5b", "/tmp/library/0", true);
 	/* Lookup on DB[0] */
-	test_lookup("5c", 0, true);
+//	test_lookup("5c", 0, true);
 	/* Lookup on DB[10] */
-	test_lookup("5d", 42, true);
+//	test_lookup("5d", 42, true);
 	/* Lookup beyond db */
-	test_lookup("5e", 100000, false);
+//	test_lookup("5e", 100000, false);
 
 	/* Lookup path id = 0 */
-	test_path_lookup("5f", 0, true);
+//	test_path_lookup("5f", 0, true);
 	/* Lookup path id that doesn't exist */
-	test_path_lookup("5g", 1, false);
-	print("\n");
-}
+//	test_path_lookup("5g", 1, false);
+//	print("\n");
+//}
 
 /* Test validation code */
-void test_6()
+/*void test_6()
 {
 	library :: reset();
 
@@ -249,10 +361,10 @@ void test_6()
 	run_idle_tasks();
 	library :: print_db(library :: DB_TRACK);
 	print("\n");
-}
+}*/
 
 /* Test importing Ocarina 5.11 libraries */
-void test_7()
+/*void test_7()
 {
 	library :: reset();
 
@@ -263,10 +375,10 @@ void test_7()
 	run_idle_tasks();
 	library :: print_db(library :: DB_LIBRARY);
 	print("\n");
-}
+}*/
 
 /* Test disabling libraries */
-void test_8()
+/*void test_8()
 {
 	library :: reset();
 
@@ -302,4 +414,4 @@ int main(int argc, char **argv)
 	test_7();
 	test_8();
 	return 0;
-}
+}*/

tests/library/0

@@ -1 +0,0 @@
-1

tests/library/Sconscript

@@ -1,6 +0,0 @@
-#!/usr/bin/python
-Import("Test", "CONFIG")
-
-CONFIG.LIBRARY = True
-
-Test("library", "library.cpp")

tests/library/gen_library.sh

@@ -1,86 +0,0 @@
-#!/bin/bash
-#
-# Copyright 2013 (c) Anna Schumaker.
-#
-# Generate a test library in /tmp
-#
-
-declare -A genres
-genres["Album 0"]=Test
-genres["Album 1"]=Trial
-genres["Album 2"]=Tryout
-
-declare -A dates
-dates["Album 0"]=2011
-dates["Album 1"]=2012
-dates["Album 2"]=2013
-
-#
-# gen_tracks() $library $artist $album
-#
-function gen_tracks()
-{
-	library="library/$1"
-	artist="Artist $2"
-	album="Album $3"
-
-	mkdir -p "/tmp/$library/$artist/$album"
-
-	for i in $(seq 10); do
-		track="Track $i"
-		let remainder=$i%4
-		out="/tmp/$library/$artist/$album/$i - $track.ogg"
-
-		if [ -f "$out" ]; then
-			continue
-		fi
-
-		case $remainder in
-		0) OGG="1.ogg"   ;;
-		1) OGG="10.ogg"  ;;
-		2) OGG="60.ogg"  ;;
-		3) OGG="600.ogg" ;;
-		esac
-
-		vorbiscomment -a -q -t "ARTIST=$artist" -t "ALBUM=$album" \
-			      -t "GENRE=${genres[$album]}" -t "DATE=${dates[$album]}" \
-			      -t "TRACKNUMBER=$i" -t "TITLE=$track" "tests/library/$OGG" \
-			      "/tmp/$library/$artist/$album/$i - $track.ogg"
-	done
-}
-
-#
-# gen_albums() $library $artist
-#
-function gen_albums()
-{
-	for i in $(seq 0 2); do
-		gen_tracks $1 $2 $i
-	done
-}
-
-#
-# gen_artists() $library
-#
-function gen_artists()
-{
-	let begin=$1*5
-	let end=$begin+4
-
-	for i in $(seq $begin $end); do
-		gen_albums $1 $i
-	done
-}
-
-for i in $(seq 0 4); do
-	echo "Generating library: $i"
-	gen_artists $i
-done
-
-touch /tmp/library/file
-
-##
-# Set up legacy library files
-#
-mkdir -p $HOME/.ocarina-test/library/
-cp tests/library/0 tests/library/1 tests/library/2 tests/library/3 $HOME/.ocarina-test/library/

tests/test.h

@@ -131,6 +131,16 @@ namespace test
 		std::string cmd = "cp -r tests/Playlist/* " + data_dir();
 		system(cmd.c_str());
 	}
+
+	void gen_library()
+	{
+		system("tests/gen_library.sh");
+	}
+
+	void rm_library_dirs()
+	{
+		system("rm -r /tmp/ocarina/dir2 /tmp/ocarina/dir4");
+	}
 }
 
 #define run_test(name, func, ...)	\