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

diff --git a/src/lib/cinema_list.cc b/src/lib/cinema_list.cc
index 4ee538efe442f54f1834e56c756cfe73e04f66fd..8673f9c8bfb0a31092fe28ad962cdd3cd13c51ac 100644 (file)
--- 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 8de1857abeb6c24c988f9e5b2536be94b67b2c6a..21e89fd78b3496a06cf8f80606d830fddfe77840 100644 (file)
--- 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 3cbb407487b7dd7fa0a18406456fc9f43baa7f64..1768c43c09595d3ef880b79b67549343a7ea5c28 100644 (file)
--- 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 c57ade437df120a7c171312a44ceaef218b609c3..5b8a373f684631ce6e329abb41cf663905644d42 100644 (file)
--- 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 f0270129b12c46cdb5a24169b061cbb2bd9b8df0..e1ad151438af21108fa9721ce445861177d2d34b 100644 (file)
--- 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 7858cbf5c6a6deef8780cfd9492b5b97fe5210db..1f459091f5649daff622796bb85028b638f81e2c 100644 (file)
--- 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 58af7839a09786779c2d5aefe00c9b38feb47b34..15e832eba13e29c59e56395c42e4868f183f1bae 100644 (file)
--- 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 035de9919ffa0e5f20d66243907d56b490d5d3c0..792a1118269b119666fcbfa0b1e2f22507744f16 100644 (file)
--- 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 8278f55a952282516eec364b386040b661214696..a588691c6d93fdccb1d58c71f2bf2343c1e7a0c8 100644 (file)
--- 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 f8c85cb5a01eb7670931ef0bc838661536564c28..85fd61c629d969e8908bbe4066d59b9fb50334a5 100644 (file)
--- 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)");
 }