Database: Update design for database entries

I changed primary_key() into a function since it is only called once,
and there is no point in using more mmemory than I need to.  I also
created a basic unit test for everything that database entries are
supposed to do.

Signed-off-by: Anna Schumaker <schumaker.anna@gmail.com>

DESIGN

@@ -189,6 +189,48 @@ On-disk files:
 
 
 
+Database Entry:
+	The database entry class is a base class used for storing data inside
+	a database (below).  The valid flag will be managed by the database
+	itself, and should be initialized to false.
+
+- DatabaseEntry:
+	class DatabaseEntry {
+	public:
+		bool valid;
+
+		DatabaseEntry();
+		virtual void ~DatabaseEntry() = 0;
+		virtual std::string primary_key() = 0;
+
+		virtual void write(File &) = 0;
+		virtual void read(File &) = 0;
+		virtual void print() = 0;
+	}
+
+- API:
+	DatabaseEntry :: DatabaseEntry():
+		Set valid = false.
+
+	std::string DatabaseEntry :: primary_key();
+		This function should return a unique string representing this
+		DatabaseEntry instance, which will be used to prevent
+		duplicates in a database.  This string is not expected to
+		change once a DatabaseEntry has been initialized.
+
+	void DatabaseEntry :: write(File &);
+		This function is called to write a specific DatabaseEntry to
+		file.
+
+	void DatabaseEntry :: read(File &);
+		This function is called to read a DatabaseEntry from a file.
+
+	void DatabaseEntry :: print();
+		This function will only exist if CONFIG_TEST is enabled and is
+		called to print a DatabaseEntry to console.
+
+
+
 Database: (lib/database.cpp)
 	Ocarina 5.x created a different save file format for each type of
 	data that needed to be stored (preferences, library paths, playlists).
@@ -210,20 +252,6 @@ Database: (lib/database.cpp)
 	include/database.hpp, which will be included by database.h.  Any
 	function not relying on a template can be written in lib/database.cpp.
 
-- DatabaseEntry:
-	class DatabaseEntry { /* let database modify valid flag */
-	public:
-		const std::string primary_key;
-		bool valid;
-
-		DatabaseEntry(const std::string &);
-		virtual void write(File &) = 0;
-		virtual void read(File &) = 0;
-		virtual void print() = 0;
-	};
-
-	File << <CHILD_CLASS_DATA>
-
 - IndexEntry:
 	class IndexEntry : public DatabaseEntry {
 	public:

gui/gui.cpp

@@ -74,7 +74,7 @@ static void on_track_loaded(library :: Song &song)
 	Gtk::Label *duration = get_widget<Gtk::Label>("o_total_time");
 
 	set_label_text(title, "xx-large", song.track->title);
-	set_label_text(artist, "x-large", "By: " + song.artist->primary_key);
+	set_label_text(artist, "x-large", "By: " + song.artist->name);
 	set_label_text(album, "x-large", "From: " + song.album->name);
 	duration->set_text(song.track->length_str);
 

gui/model.cpp

@@ -146,13 +146,13 @@ void PlayqueueModel::get_value_str(struct library::Song &song, int column,
 		specific.set(song.track->length_str);
 		break;
 	case 3:
-		specific.set(song.artist->primary_key);
+		specific.set(song.artist->name);
 		break;
 	case 4:
 		specific.set(song.album->name);
 		break;
 	case 6:
-		specific.set(song.genre->primary_key);
+		specific.set(song.genre->name);
 		break;
 	case 8:
 		if (song.track->play_count == 0)
@@ -165,7 +165,7 @@ void PlayqueueModel::get_value_str(struct library::Song &song, int column,
 		}
 		break;
 	case 9:
-		specific.set(Glib::Markup::escape_text(song.track->primary_key));
+		specific.set(Glib::Markup::escape_text(song.track->full_path));
 	}
 
 	value.init(Glib::Value<std::string>::value_type());

include/database.h

@@ -14,10 +14,10 @@
 
 class DatabaseEntry {
 public:
-	std::string primary_key;
 	bool valid;
 
 	DatabaseEntry();
+	virtual std::string primary_key() = 0;
 	virtual void write(File &) = 0;
 	virtual void read(File &) = 0;
 #ifdef CONFIG_TEST
@@ -28,11 +28,13 @@ public:
 
 class IndexEntry : public DatabaseEntry {
 public:
+	std::string key;
 	std::set<unsigned int> values;
 
 	IndexEntry();
 	IndexEntry(const std::string &, unsigned int);
 	~IndexEntry();
+	std::string primary_key();
 	void write(File &);
 	void read(File &);
 #ifdef CONFIG_TEST

include/database.hpp

@@ -60,7 +60,7 @@ void Database<T> :: load()
 		file >> db[i].valid;
 		if (db[i].valid == true) {
 			db[i].read(file);
-			keys.insert(std::pair<std::string, unsigned int>(db[i].primary_key, i));
+			keys.insert(std::pair<std::string, unsigned int>(db[i].primary_key(), i));
 			_size++;
 		}
 	}
@@ -108,7 +108,7 @@ unsigned int Database<T> :: insert(T val)
 	unsigned int id;
 	typename std::map<const std::string, unsigned int>::iterator it;
 
-	it = keys.find(val.primary_key);
+	it = keys.find(val.primary_key());
 	if (it != keys.end())
 		return it->second;
 
@@ -117,7 +117,7 @@ unsigned int Database<T> :: insert(T val)
 	 */
 	id = db.size();
 	db.push_back(val);
-	keys.insert(std::pair<const std::string, unsigned int>(val.primary_key, id));
+	keys.insert(std::pair<const std::string, unsigned int>(val.primary_key(), id));
 	db[id].valid = true;
 	_size++;
 	return id;
@@ -126,7 +126,7 @@ unsigned int Database<T> :: insert(T val)
 template <class T>
 void Database<T> :: remove(unsigned int id)
 {
-	keys.erase(db[id].primary_key);
+	keys.erase(db[id].primary_key());
 	db[id].valid = false;
 	_size--;
 }

include/library.h

@@ -27,11 +27,13 @@ namespace library
 	class AGInfo : public DatabaseEntry {
 	public:
 		DB_Type db_type;
+		std :: string name;
 		std :: string key_lower;
 
 		AGInfo();
 		AGInfo(DB_Type, TagLib :: Tag *);
 		AGInfo(DB_Type, const std::string &);
+		std::string primary_key();
 		void read(File &);
 		void write(File &);
 #ifdef CONFIG_TEST
@@ -50,6 +52,7 @@ namespace library
 		Album();
 		Album(TagLib :: Tag *, unsigned int);
 		Album(const std::string &, unsigned int, unsigned int);
+		std::string primary_key();
 		void read(File &);
 		void write(File &);
 #ifdef CONFIG_TEST
@@ -66,6 +69,7 @@ namespace library
 
 		Library();
 		Library(const std::string &, bool);
+		std::string primary_key();
 		void read(File &);
 		void write(File &);
 #ifdef CONFIG_TEST
@@ -92,6 +96,7 @@ namespace library
 		std :: string title_lower;
 		std :: string length_str;
 		std :: string filepath;
+		std :: string full_path;
 
 		Track();
 		Track(TagLib :: Tag *, TagLib :: AudioProperties *,
@@ -99,6 +104,7 @@ namespace library
 			unsigned int, const std :: string &);
 		Track(ImportData *, unsigned int, unsigned int,
 			unsigned int, unsigned int);
+		std::string primary_key();
 		void read(File &);
 		void write(File &);
 #ifdef CONFIG_TEST

lib/audio.cpp

@@ -29,7 +29,7 @@ static void parse_error(GstMessage *error)
 
 	library :: lookup(cur_trackid, &song);
 	gst_message_parse_error(error, &err, &debug);
-	g_print("Error playing file: %s\n", song.track->primary_key.c_str());
+	g_print("Error playing file: %s\n", song.track->primary_key().c_str());
 	g_print("Error: %s\n", err->message);
 	g_error_free(err);
 	g_free(debug);

lib/database.cpp

@@ -5,7 +5,7 @@
 
 
 DatabaseEntry :: DatabaseEntry()
-	       : primary_key(""), valid(false)
+	       : valid(false)
 {
 }
 
@@ -15,9 +15,9 @@ IndexEntry :: IndexEntry()
 {
 }
 
-IndexEntry :: IndexEntry(const std::string &key, unsigned int v)
+IndexEntry :: IndexEntry(const std::string &k, unsigned int v)
 {
-	primary_key = key;
+	key = k;
 	insert(v);
 }
 
@@ -25,10 +25,15 @@ IndexEntry :: ~IndexEntry()
 {
 }
 
+std::string IndexEntry :: primary_key()
+{
+	return key;
+}
+
 void IndexEntry :: write(File &f)
 {
 	std::set<unsigned int>::iterator it;
-	f << primary_key << std::endl << values.size() << " ";
+	f << key << std::endl << values.size() << " ";
 	for (it = values.begin(); it != values.end(); it++)
 		f << *it << " ";
 }
@@ -37,7 +42,7 @@ void IndexEntry :: read(File &f)
 {
 	unsigned int num, val;
 
-	f >> primary_key >> num;
+	f >> key >> num;
 	for (unsigned int i = 0; i < num; i++) {
 		f >> val;
 		values.insert(val);

lib/library.cpp

@@ -41,44 +41,48 @@ library :: AGInfo :: AGInfo(DB_Type type, TagLib :: Tag *tag)
 	 : db_type(type)
 {
 	if (db_type == DB_ARTIST)
-		primary_key = tag->artist().stripWhiteSpace().to8Bit(true);
+		name = tag->artist().stripWhiteSpace().to8Bit(true);
 	else if (db_type == DB_GENRE)
-		primary_key = tag->genre().stripWhiteSpace().to8Bit(true);
+		name = tag->genre().stripWhiteSpace().to8Bit(true);
 	else
 		throw -E_INVAL;
 
-	key_lower = filter :: to_lowercase(primary_key);
+	key_lower = filter :: to_lowercase(name);
 }
 
 library :: AGInfo :: AGInfo(DB_Type type, const std::string &str)
 	 : db_type(type)
 {
 	if ((db_type == DB_ARTIST) || (db_type == DB_GENRE)) {
-		primary_key = str;
+		name = str;
-		key_lower = filter :: to_lowercase(primary_key);
+		key_lower = filter :: to_lowercase(name);
 	} else
 		throw -E_INVAL;
+}
 
+std::string library :: AGInfo :: primary_key()
+{
+	return name;
 }
 
 void library :: AGInfo :: read(File &f)
 {
-	primary_key = f.getline();
+	name = f.getline();
-	key_lower = filter :: to_lowercase(primary_key);
+	key_lower = filter :: to_lowercase(name);
 }
 
 void library :: AGInfo :: write(File &f)
 {
-	f << primary_key;
+	f << name;
 }
 
 #ifdef CONFIG_TEST
 void library :: AGInfo :: print()
 {
 	if (db_type == DB_ARTIST)
-		:: print("Artist: %s", primary_key.c_str());
+		:: print("Artist: %s", name.c_str());
 	else
-		:: print("Genre: %s", primary_key.c_str());
+		:: print("Genre: %s", name.c_str());
 }
 #endif /* CONFIG_TEST */
 
@@ -97,19 +101,20 @@ library :: Album :: Album(TagLib :: Tag *tag, unsigned int artist)
 	 : name(tag->album().stripWhiteSpace().to8Bit(true)),
 	   year(tag->year()), artist_id(artist)
 {
-	std::stringstream ss;
-	ss << artist_id << "." << name << "." << year;
-	primary_key = ss.str();
 	name_lower = filter :: to_lowercase(name);
 }
 
 library :: Album :: Album(const std::string &str, unsigned int yr, unsigned int artist)
 	 : name(str), year(yr), artist_id(artist)
+{
+	name_lower = filter :: to_lowercase(name);
+}
+
+std::string library :: Album :: primary_key()
 {
 	std::stringstream ss;
 	ss << artist_id << "." << name << "." << year;
-	primary_key = ss.str();
+	return ss.str();
-	name_lower = filter :: to_lowercase(name);
 }
 
 void library :: Album :: read(File &f)
@@ -127,7 +132,7 @@ void library :: Album :: write(File &f)
 #ifdef CONFIG_TEST
 void library :: Album :: print()
 {
-	:: print("Album: %s (%u) by %s", name.c_str(), year, artist_db[artist_id].primary_key.c_str());
+	:: print("Album: %s (%u) by %s", name.c_str(), year, artist_db[artist_id].name.c_str());
 }
 #endif /* CONFIG_TEST */
 
@@ -145,14 +150,17 @@ library :: Library :: Library()
 library :: Library :: Library(const std::string &path, bool is_enabled)
 	 : root_path(path), size(0), enabled(is_enabled)
 {
-	primary_key = root_path;
+}
+
+std::string library :: Library :: primary_key()
+{
+	return root_path;
 }
 
 void library :: Library :: read(File &f)
 {
 	f >> enabled;
 	root_path = f.getline();
-	primary_key = root_path;
 	size = 0;
 }
 
@@ -195,7 +203,7 @@ library :: Track :: Track(TagLib :: Tag *tag, TagLib :: AudioProperties *audio,
 	std::stringstream ss;
 	unsigned int minutes, seconds;
 
-	primary_key = path;
+	full_path = path;
 	filepath = path.substr(library_db[library_id].root_path.size() + 1);
 	title_lower = filter :: to_lowercase(title);
 
@@ -219,8 +227,8 @@ library :: Track :: Track(struct ImportData *data, unsigned int lib,
 	std::stringstream ss;
 	unsigned int minutes, seconds;
 
-	primary_key = data->filepath;
+	full_path = data->filepath;
-	filepath = primary_key.substr(library_db[library_id].root_path.size() + 1);
+	filepath = full_path.substr(library_db[library_id].root_path.size() + 1);
 	title_lower = filter :: to_lowercase(title);
 
 	minutes = length / 60;
@@ -232,6 +240,11 @@ library :: Track :: Track(struct ImportData *data, unsigned int lib,
 	length_str = ss.str();
 }
 
+std::string library :: Track :: primary_key()
+{
+	return full_path;
+}
+
 void library :: Track :: read(File &f)
 {
 	f >> library_id >> artist_id >> album_id >> genre_id;
@@ -241,7 +254,7 @@ void library :: Track :: read(File &f)
 	title = f.getline();
 	filepath = f.getline();
 	title_lower = filter :: to_lowercase(title);
-	primary_key = library_db[library_id].root_path + "/" + filepath;
+	full_path = library_db[library_id].root_path + "/" + filepath;
 	library_db[library_id].size++;
 }
 
@@ -258,10 +271,10 @@ void library :: Track :: write(File &f)
 void library :: Track :: print()
 {
 	:: print("%u. %s by %s from %s (%u)\n", track, title.c_str(),
-			artist_db[artist_id].primary_key.c_str(),
+			artist_db[artist_id].name.c_str(),
 			album_db[album_id].name.c_str(), album_db[album_id].year);
 	:: print("	Genre: %s, Length: %u (seconds)\n",
-			genre_db[genre_id].primary_key.c_str(), length);
+			genre_db[genre_id].name.c_str(), length);
 	:: print("	Play count: %u, last played %u/%u/%u\n", play_count,
 			last_day, last_month, last_year);
 	:: print("	%s", filepath.c_str());
@@ -302,7 +315,7 @@ static void read_tags(unsigned int lib_id, const std :: string &path)
 					artist_id, album_id, genre_id, path));
 	library_db[lib_id].size++;
 
-	filter::add(artist_db[artist_id].primary_key, track_id);
+	filter::add(artist_db[artist_id].name, track_id);
 	filter::add(album_db[album_id].name, track_id);
 	filter::add(track_db[track_id].title, track_id);
 	get_callbacks()->on_library_track_add(track_id);
@@ -419,7 +432,7 @@ static void do_import_track(File &f, unsigned int lib_id)
 					album_id, genre_id));
 	library_db[lib_id].size++;
 
-	filter::add(artist_db[artist_id].primary_key, track_id);
+	filter::add(artist_db[artist_id].name, track_id);
 	filter::add(album_db[album_id].name, track_id);
 	filter::add(track_db[track_id].title, track_id);
 	get_callbacks()->on_library_track_add(track_id);
@@ -482,7 +495,7 @@ void library :: init()
 	track_db.load();
 
 	for (i = track_db.first(); i < track_db.num_rows(); i = track_db.next(i)) {
-		filter::add(artist_db[track_db[i].artist_id].primary_key, i);
+		filter::add(artist_db[track_db[i].artist_id].name, i);
 		filter::add(album_db[track_db[i].album_id].name, i);
 		filter::add(track_db[i].title, i);
 

tests/Sconscript

@@ -8,7 +8,7 @@ if sys.argv.count("tests") > 0:
 
 src = SConscript("src/Sconscript")
 
-tests = [ "version", "file" ]
+tests = [ "version", "file", "db_entry" ]
 #scripts = [ "print", "file", "database", "index", "filter", "idle", "playlist",
 #            "library", "playqueue", "deck", "audio", "gui" ]
 

tests/db_entry

@@ -0,0 +1,65 @@
+#!/bin/bash
+# Copyright 2014 (c) Anna Schumaker
+
+. $(dirname $0)/_functions
+
+function test_entry
+{
+	test_equal "./src/db_entry.run $1 $2" "$3"
+}
+
+function test_print
+{
+	test_entry $1 $2 "Value: $1 Key: $2 Valid: 0"
+}
+
+function test_primary_key
+{
+	test_entry "-p $1" $2 "Primary key: $2"
+}
+
+function test_write
+{
+	rm $DATA_DIR/db_entry.txt 2>/dev/null || true
+	./src/db_entry.run -w $1 $2
+	test_equal "tail -1 $DATA_DIR/db_entry.txt" "$1 $2"
+}
+
+function test_read
+{
+	rm $DATA_DIR/db_entry.txt 2>/dev/null || true
+	echo 0 > $DATA_DIR/db_entry.txt
+	echo $1 $2 >> $DATA_DIR/db_entry.txt
+	test_entry "-r $1" $2 "Value: $1 Key: $2 Valid: 0"
+}
+
+
+new_test "Database Entry Print Test"
+test_print 1 1
+test_print 1 2
+test_print 2 1
+test_print 2 2
+
+
+echo
+new_test "Database Entry Primary Key Test"
+test_primary_key 1 1
+test_primary_key 1 2
+test_primary_key 2 1
+test_primary_key 2 2
+
+
+echo
+new_test "Database Entry Write Test"
+test_write 1 1
+test_write 1 2
+test_write 2 1
+test_write 2 2
+
+
+echo
+new_test "Database Entry Read Test"
+test_read 1 1
+test_read 1 2
+test_read 2 1
+test_read 2 2

tests/src/db_entry.cpp

@@ -0,0 +1,108 @@
+/*
+ * Copyright 2014 (c) Anna Schumaker.
+ * Test a DatabaseEntry
+ */
+
+#include <database.h>
+#include <file.h>
+#include <print.h>
+
+#include <string>
+#include <stdlib.h>
+#include <unistd.h>
+
+enum action_t { PRIMARY_KEY, PRINT, READ, WRITE };
+
+class IntEntry : public DatabaseEntry {
+public:
+	unsigned int val;
+	std::string key;
+
+	IntEntry(unsigned int, const std::string &);
+	std::string primary_key();
+	void write(File &);
+	void read(File &);
+	void print();
+};
+
+IntEntry :: IntEntry(unsigned int i, const std::string &s)
+{
+	val = i;
+	key = s;
+}
+
+std::string IntEntry :: primary_key()
+{
+	return key;
+}
+
+void IntEntry :: write(File &f)
+{
+	f << val << " " << key;
+}
+
+void IntEntry :: read(File &f)
+{
+	f >> val >> key;
+}
+
+void IntEntry :: print()
+{
+	:: print("Value: %u Key: %s Valid: %d\n", val, key.c_str(), valid);
+}
+
+
+
+int main(int argc, char **argv)
+{
+	action_t action = PRINT;
+	char c;
+
+	while ((c = getopt(argc, argv, "prw")) != -1) {
+		switch (c) {
+		case 'p':
+			action = PRIMARY_KEY;
+			break;
+		case 'r':
+			action = READ;
+			break;
+		case 'w':
+			action = WRITE;
+			break;
+		}
+	}
+
+	if (optind >= argc) {
+		print("ERROR: Not enough arguments\n");
+		return 1;
+	}
+
+	unsigned int i  = atoi(argv[optind++]);
+	std::string key = argv[optind];
+
+
+	File f("db_entry.txt", FILE_TYPE_DATA);
+	IntEntry ient(i, key);
+
+	switch (action) {
+	case PRIMARY_KEY:
+		print("Primary key: %s\n", ient.primary_key().c_str());
+		break;
+	case PRINT:
+		ient.print();
+		break;
+	case READ:
+		f.open(OPEN_READ);
+		ient.read(f);
+		ient.print();
+		f.close();
+		break;
+	case WRITE:
+		f.open(OPEN_WRITE);
+		ient.write(f);
+		f.close();
+		break;
+	}
+
+	return 0;
+}