Sort cinemas and DKDM recipients correctly using the collator (#2950).

10 files changed, 74 insertions, 26 deletions

diff --git a/src/lib/cinema_list.cc b/src/lib/cinema_list.cc
index 4ee538efe..8673f9c8b 100644
--- a/src/lib/cinema_list.cc
+++ b/src/lib/cinema_list.cc
@@ -244,7 +244,7 @@ cinemas_from_result(SQLiteStatement& statement)
 vector<pair<CinemaID, Cinema>>
 CinemaList::cinemas() const
 {
-	SQLiteStatement statement(_db, _cinemas.select("ORDER BY name ASC"));
+	SQLiteStatement statement(_db, _cinemas.select("ORDER BY name COLLATE unicode ASC"));
 	return cinemas_from_result(statement);
 }
 
diff --git a/src/lib/collator.cc b/src/lib/collator.cc
index 8de1857ab..21e89fd78 100644
--- a/src/lib/collator.cc
+++ b/src/lib/collator.cc
@@ -27,6 +27,7 @@
 #include <unicode/utypes.h>
 #include <unicode/usearch.h>
 #include <unicode/ustring.h>
+#include <fmt/format.h>
 #include <boost/scoped_array.hpp>
 #include <cstring>
 #include <vector>
@@ -36,10 +37,14 @@ using std::string;
 using std::vector;
 
 
-Collator::Collator(char const* locale)
+Collator::Collator()
 {
 	UErrorCode status = U_ZERO_ERROR;
-	_collator = ucol_open(locale, &status);
+#ifdef DCPOMATIC_POSIX
+	_collator = ucol_open("POSIX", &status);
+#else
+	_collator = ucol_open(nullptr, &status);
+#endif
 	if (_collator) {
 		ucol_setAttribute(_collator, UCOL_NORMALIZATION_MODE, UCOL_ON, &status);
 		/* Ignore case and character encoding (and probably some other things) */
@@ -72,9 +77,12 @@ int
 Collator::compare (string const& utf8_a, string const& utf8_b) const
 {
 	if (_collator) {
-		auto utf16_a = utf8_to_utf16(utf8_a);
-		auto utf16_b = utf8_to_utf16(utf8_b);
-		return ucol_strcoll(_collator, utf16_a.data(), -1, utf16_b.data(), -1);
+		UErrorCode error = U_ZERO_ERROR;
+		auto const result = ucol_strcollUTF8(_collator, utf8_a.data(), -1, utf8_b.data(), -1, &error);
+		if (error != U_ZERO_ERROR) {
+			throw std::runtime_error(fmt::format("Failed to compare strings ({})", static_cast<int>(error)));
+		}
+		return result;
 	} else {
 		return strcoll(utf8_a.c_str(), utf8_b.c_str());
 	}
diff --git a/src/lib/collator.h b/src/lib/collator.h
index 3cbb40748..1768c43c0 100644
--- a/src/lib/collator.h
+++ b/src/lib/collator.h
@@ -32,7 +32,7 @@ struct UCollator;
 class Collator
 {
 public:
-	Collator(char const* locale = nullptr);
+	Collator();
 	~Collator();
 
 	Collator(Collator const &) = delete;
diff --git a/src/lib/dkdm_recipient_list.cc b/src/lib/dkdm_recipient_list.cc
index c57ade437..5b8a373f6 100644
--- a/src/lib/dkdm_recipient_list.cc
+++ b/src/lib/dkdm_recipient_list.cc
@@ -179,7 +179,7 @@ dkdm_recipients_from_result(SQLiteStatement& statement)
 vector<std::pair<DKDMRecipientID, DKDMRecipient>>
 DKDMRecipientList::dkdm_recipients() const
 {
-	SQLiteStatement statement(_db, _dkdm_recipients.select("ORDER BY name ASC"));
+	SQLiteStatement statement(_db, _dkdm_recipients.select("ORDER BY name COLLATE unicode ASC"));
 	return dkdm_recipients_from_result(statement);
 }
 
diff --git a/src/lib/sqlite_database.cc b/src/lib/sqlite_database.cc
index f0270129b..e1ad15143 100644
--- a/src/lib/sqlite_database.cc
+++ b/src/lib/sqlite_database.cc
@@ -24,6 +24,20 @@
 #include <sqlite3.h>
 
 
+using std::string;
+
+
+static
+int collator_compare(void* context, int length_a, const void* string_a, int length_b, const void* string_b)
+{
+	auto collator = reinterpret_cast<Collator*>(context);
+	return collator->compare(
+		string(reinterpret_cast<char const*>(string_a), length_a),
+		string(reinterpret_cast<char const*>(string_b), length_b)
+	);
+}
+
+
 SQLiteDatabase::SQLiteDatabase(boost::filesystem::path path)
 {
 #ifdef DCPOMATIC_WINDOWS
@@ -36,6 +50,11 @@ SQLiteDatabase::SQLiteDatabase(boost::filesystem::path path)
 	}
 
 	sqlite3_busy_timeout(_db, 500);
+
+	rc = sqlite3_create_collation(_db, "unicode", SQLITE_UTF8, &_collator, collator_compare);
+	if (rc != SQLITE_OK) {
+		throw std::runtime_error("Could not set SQLite database collation");
+	}
 }
 
 
diff --git a/src/lib/sqlite_database.h b/src/lib/sqlite_database.h
index 7858cbf5c..1f459091f 100644
--- a/src/lib/sqlite_database.h
+++ b/src/lib/sqlite_database.h
@@ -23,6 +23,7 @@
 #define DCPOMATIC_SQLITE_DATABASE_H
 
 
+#include "collator.h"
 #include <boost/filesystem.hpp>
 
 struct sqlite3;
@@ -46,6 +47,7 @@ public:
 
 private:
 	sqlite3* _db = nullptr;
+	Collator _collator;
 };
 
 
diff --git a/test/cinema_list_test.cc b/test/cinema_list_test.cc
index 58af7839a..15e832eba 100644
--- a/test/cinema_list_test.cc
+++ b/test/cinema_list_test.cc
@@ -227,16 +227,16 @@ BOOST_AUTO_TEST_CASE(cinemas_list_copy_from_xml_test)
 	BOOST_REQUIRE_EQUAL(cinemas.size(), 3U);
 
 	auto cinema_iter = cinemas.begin();
+	BOOST_CHECK_EQUAL(cinema_iter->second.name, "classy joint");
+	BOOST_CHECK_EQUAL(cinema_iter->second.notes, "Can't stand this place");
+	++cinema_iter;
+
 	BOOST_CHECK_EQUAL(cinema_iter->second.name, "Great");
 	BOOST_CHECK_EQUAL(cinema_iter->second.emails.size(), 1U);
 	BOOST_CHECK_EQUAL(cinema_iter->second.emails[0], "julie@tinyscreen.com");
 	BOOST_CHECK(cinema_iter->second.utc_offset == dcp::UTCOffset(1, 0));
 	++cinema_iter;
 
-	BOOST_CHECK_EQUAL(cinema_iter->second.name, "classy joint");
-	BOOST_CHECK_EQUAL(cinema_iter->second.notes, "Can't stand this place");
-	++cinema_iter;
-
 	BOOST_CHECK_EQUAL(cinema_iter->second.name, "stinking dump");
 	BOOST_CHECK_EQUAL(cinema_iter->second.emails.size(), 2U);
 	BOOST_CHECK_EQUAL(cinema_iter->second.emails[0], "bob@odourscreen.com");
@@ -255,3 +255,22 @@ BOOST_AUTO_TEST_CASE(cinemas_list_copy_from_xml_test)
 	BOOST_CHECK_EQUAL(screen_iter->second.recipient()->subject_dn_qualifier(), "CVsuuv9eYsQZSl8U4fDpvOmzZhI=");
 }
 
+
+BOOST_AUTO_TEST_CASE(cinemas_list_sort_test)
+{
+	auto const db = setup("cinemas_list_sort_test");
+
+	CinemaList cinemas(db);
+	cinemas.add_cinema({"Ŝpecial", { "foo@bar.com" }, "", dcp::UTCOffset()});
+	cinemas.add_cinema({"Ţest", { "foo@bar.com" }, "", dcp::UTCOffset()});
+	cinemas.add_cinema({"Name", { "foo@bar.com" }, "", dcp::UTCOffset()});
+	cinemas.add_cinema({"ÄBC", { "foo@bar.com" }, "", dcp::UTCOffset()});
+
+	auto sorted = cinemas.cinemas();
+	BOOST_REQUIRE_EQUAL(sorted.size(), 4U);
+	BOOST_CHECK_EQUAL(sorted[0].second.name, "ÄBC");
+	BOOST_CHECK_EQUAL(sorted[1].second.name, "Name");
+	BOOST_CHECK_EQUAL(sorted[2].second.name, "Ŝpecial");
+	BOOST_CHECK_EQUAL(sorted[3].second.name, "Ţest");
+}
+
diff --git a/test/collator_test.cc b/test/collator_test.cc
index 035de9919..792a11182 100644
--- a/test/collator_test.cc
+++ b/test/collator_test.cc
@@ -27,7 +27,7 @@
 
 BOOST_AUTO_TEST_CASE(collator_compare_works_and_ignores_case)
 {
-	Collator collator("en");
+	Collator collator;
 
 #if 0
 	// Print out available locales
@@ -51,7 +51,7 @@ BOOST_AUTO_TEST_CASE(collator_compare_works_and_ignores_case)
 
 BOOST_AUTO_TEST_CASE(collator_search_works_and_ignores_case)
 {
-	Collator collator("en");
+	Collator collator;
 
 	BOOST_CHECK(collator.find("outh", "With filthy mouths, and bad attitudes"));
 	BOOST_CHECK(collator.find("with", "With filthy mouths, and bad attitudes"));
diff --git a/test/config_test.cc b/test/config_test.cc
index 8278f55a9..a588691c6 100644
--- a/test/config_test.cc
+++ b/test/config_test.cc
@@ -309,8 +309,8 @@ BOOST_AUTO_TEST_CASE(read_cinemas_xml_and_write_sqlite)
 		/* The detailed creation of sqlite3 from XML is tested in cinema_list_test.cc */
 		auto cinemas = test.cinemas();
 		BOOST_REQUIRE_EQUAL(cinemas.size(), 3U);
-		BOOST_CHECK_EQUAL(cinemas[0].second.name, "Great");
-		BOOST_CHECK_EQUAL(cinemas[1].second.name, "classy joint");
+		BOOST_CHECK_EQUAL(cinemas[0].second.name, "classy joint");
+		BOOST_CHECK_EQUAL(cinemas[1].second.name, "Great");
 		BOOST_CHECK_EQUAL(cinemas[2].second.name, "stinking dump");
 
 		/* Add another recipient to the sqlite */
@@ -326,10 +326,10 @@ BOOST_AUTO_TEST_CASE(read_cinemas_xml_and_write_sqlite)
 
 		auto cinemas = test.cinemas();
 		BOOST_REQUIRE_EQUAL(cinemas.size(), 4U);
-		BOOST_CHECK_EQUAL(cinemas[0].second.name, "Great");
-		BOOST_CHECK_EQUAL(cinemas[1].second.name, "The ol' 1-seater");
-		BOOST_CHECK_EQUAL(cinemas[2].second.name, "classy joint");
-		BOOST_CHECK_EQUAL(cinemas[3].second.name, "stinking dump");
+		BOOST_CHECK_EQUAL(cinemas[0].second.name, "classy joint");
+		BOOST_CHECK_EQUAL(cinemas[1].second.name, "Great");
+		BOOST_CHECK_EQUAL(cinemas[2].second.name, "stinking dump");
+		BOOST_CHECK_EQUAL(cinemas[3].second.name, "The ol' 1-seater");
 	}
 }
 
@@ -445,8 +445,8 @@ BOOST_AUTO_TEST_CASE(load_config_from_zip_with_only_xml_current)
 	CinemaList cinema_list(cinemas_file);
 	auto cinemas = cinema_list.cinemas();
 	BOOST_REQUIRE_EQUAL(cinemas.size(), 3U);
-	BOOST_CHECK_EQUAL(cinemas[0].second.name, "Great");
-	BOOST_CHECK_EQUAL(cinemas[1].second.name, "classy joint");
+	BOOST_CHECK_EQUAL(cinemas[0].second.name, "classy joint");
+	BOOST_CHECK_EQUAL(cinemas[1].second.name, "Great");
 	BOOST_CHECK_EQUAL(cinemas[2].second.name, "stinking dump");
 }
 
@@ -480,8 +480,8 @@ BOOST_AUTO_TEST_CASE(load_config_from_zip_with_only_xml_zip)
 	CinemaList cinema_list("build/test/hide/it/here/cinemas.sqlite3");
 	auto cinemas = cinema_list.cinemas();
 	BOOST_REQUIRE_EQUAL(cinemas.size(), 3U);
-	BOOST_CHECK_EQUAL(cinemas[0].second.name, "Great");
-	BOOST_CHECK_EQUAL(cinemas[1].second.name, "classy joint");
+	BOOST_CHECK_EQUAL(cinemas[0].second.name, "classy joint");
+	BOOST_CHECK_EQUAL(cinemas[1].second.name, "Great");
 	BOOST_CHECK_EQUAL(cinemas[2].second.name, "stinking dump");
 }
 
diff --git a/test/kdm_cli_test.cc b/test/kdm_cli_test.cc
index f8c85cb5a..85fd61c62 100644
--- a/test/kdm_cli_test.cc
+++ b/test/kdm_cli_test.cc
@@ -260,8 +260,8 @@ BOOST_AUTO_TEST_CASE(kdm_cli_specify_cinemas_file)
 	BOOST_CHECK(!error);
 
 	BOOST_REQUIRE_EQUAL(output.size(), 3U);
-	BOOST_CHECK_EQUAL(output[0], "Great (julie@tinyscreen.com)");
-	BOOST_CHECK_EQUAL(output[1], "classy joint ()");
+	BOOST_CHECK_EQUAL(output[0], "classy joint ()");
+	BOOST_CHECK_EQUAL(output[1], "Great (julie@tinyscreen.com)");
 	BOOST_CHECK_EQUAL(output[2], "stinking dump (bob@odourscreen.com, alice@whiff.com)");
 }