diff --git a/src/db/db/dbPropertiesRepository.cc b/src/db/db/dbPropertiesRepository.cc index d01d068f7..862c2e061 100644 --- a/src/db/db/dbPropertiesRepository.cc +++ b/src/db/db/dbPropertiesRepository.cc @@ -360,7 +360,7 @@ PropertiesRepository::properties_id (const PropertiesSet &props) pid = db::properties_id_type (&new_props); for (auto nv = props.begin (); nv != props.end (); ++nv) { m_properties_by_name_table [nv->first].insert (pid); - m_properties_by_name_table [nv->second].insert (pid); + m_properties_by_value_table [nv->second].insert (pid); } changed = true; @@ -432,6 +432,19 @@ PropertiesRepository::properties_ids_by_name (db::property_names_id_type name_id } } +PropertiesRepository::properties_id_set +PropertiesRepository::properties_ids_by_value (db::property_values_id_type value_id) const +{ + tl::MutexLocker locker (&m_lock); + + auto vi = m_properties_by_value_table.find (value_id); + if (vi == m_properties_by_value_table.end ()) { + return properties_id_set (); + } else { + return vi->second; + } +} + PropertiesRepository::properties_id_set PropertiesRepository::properties_ids_by_name_value (db::property_names_id_type name_id, db::property_values_id_type value_id) const { @@ -448,7 +461,27 @@ PropertiesRepository::properties_ids_by_name_value (db::property_names_id_type n } properties_id_set result; - std::set_intersection (vi->second.begin (), vi->second.end (), ni->second.begin (), ni->second.end (), std::inserter (result, result.begin ())); + + // find the property sets in the intersection of ni->second and vi->second that contain the + // combination of name and value. + + for (auto n = ni->second.begin (); n != ni->second.end (); ++n) { + + auto vv = vi->second.find (*n); + if (vv != vi->second.end ()) { + + const db::PropertiesSet &props = db::properties (*n); + for (auto p = props.find (name_id); p != props.end () && p->first == name_id; ++p) { + if (p->second == value_id) { + result.insert (*n); + break; + } + } + + } + + } + return result; } diff --git a/src/db/db/dbPropertiesRepository.h b/src/db/db/dbPropertiesRepository.h index 880c85c94..8775b90dc 100644 --- a/src/db/db/dbPropertiesRepository.h +++ b/src/db/db/dbPropertiesRepository.h @@ -251,22 +251,11 @@ public: } /** - * @brief Non-const iterator (begin) - * - * This iterator delivers key/value pairs in the ID form. - * The order is basically undefined. + * @brief Finds and entry with the given name ID */ - non_const_iterator begin_non_const () + iterator find (db::property_names_id_type name_id) const { - return m_map.begin (); - } - - /** - * @brief Non-const iterator (end) - */ - non_const_iterator end_non_const () - { - return m_map.end (); + return m_map.find (name_id); } private: @@ -397,6 +386,14 @@ public: */ properties_id_set properties_ids_by_name (db::property_names_id_type name) const; + /** + * @brief Lookup a table of properties id's by a value + * + * For a given name, this method returns a set of property IDs + * of property sets that contain the given value. + */ + properties_id_set properties_ids_by_value (db::property_values_id_type value) const; + /** * @brief Collect memory statistics */ diff --git a/src/db/unit_tests/dbPropertiesRepositoryTests.cc b/src/db/unit_tests/dbPropertiesRepositoryTests.cc index 434980e4a..efda0831c 100644 --- a/src/db/unit_tests/dbPropertiesRepositoryTests.cc +++ b/src/db/unit_tests/dbPropertiesRepositoryTests.cc @@ -333,3 +333,121 @@ TEST(PropertiesTranslator) EXPECT_EQ (db::prop2string (t (prop2)), "{1=>102}"); EXPECT_EQ (db::prop2string (t (prop3)), "{}"); } + +static std::string ps2s (const db::PropertiesRepository::properties_id_set &ps) +{ + std::string res; + for (auto p = ps.begin (); p != ps.end (); ++p) { + if (! res.empty ()) { + res += ","; + } + res += db::properties (*p).to_dict_var ().to_string (); + } + return res; +} + +TEST(PropertyIdsByNameAndValue) +{ + db::PropertiesRepository rp; + + db::PropertiesSet ps; + ps.insert_by_id (rp.prop_name_id (1), rp.prop_value_id ("A")); + + // 1=>"A" + db::properties_id_type pid1 = rp.properties_id (ps); + + ps.insert_by_id (rp.prop_name_id (2), rp.prop_value_id ("A")); + + // 1=>"A", 2=>"A" + db::properties_id_type pid2 = rp.properties_id (ps); + + ps.clear (); + ps.insert_by_id (rp.prop_name_id (2), rp.prop_value_id ("B")); + + // 2=>"B" + db::properties_id_type pid3 = rp.properties_id (ps); + + ps.insert_by_id (rp.prop_name_id (2), rp.prop_value_id ("C")); + + // 2=>"B",2=>"C" + db::properties_id_type pid4 = rp.properties_id (ps); + + ps.insert_by_id (rp.prop_name_id (3), rp.prop_value_id ("C")); + + // 2=>"B",2=>"C",3=>"C" + db::properties_id_type pid5 = rp.properties_id (ps); + + db::PropertiesRepository::properties_id_set res, ref; + + res = rp.properties_ids_by_name (rp.prop_name_id (1)); + ref.clear (); + ref.insert (pid1); + ref.insert (pid2); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_name (rp.prop_name_id (2)); + ref.clear (); + ref.insert (pid2); + ref.insert (pid3); + ref.insert (pid4); + ref.insert (pid5); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_name (rp.prop_name_id (3)); + ref.clear (); + ref.insert (pid5); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_value (rp.prop_value_id ("A")); + ref.clear (); + ref.insert (pid1); + ref.insert (pid2); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_value (rp.prop_value_id ("B")); + ref.clear (); + ref.insert (pid3); + ref.insert (pid4); + ref.insert (pid5); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_value (rp.prop_value_id ("C")); + ref.clear (); + ref.insert (pid4); + ref.insert (pid5); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_name_value (rp.prop_name_id (1), rp.prop_value_id ("A")); + ref.clear (); + ref.insert (pid1); + ref.insert (pid2); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_name_value (rp.prop_name_id (1), rp.prop_value_id ("B")); + ref.clear (); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_name_value (rp.prop_name_id (2), rp.prop_value_id ("A")); + ref.clear (); + ref.insert (pid2); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_name_value (rp.prop_name_id (2), rp.prop_value_id ("B")); + ref.clear (); + ref.insert (pid3); + ref.insert (pid4); + ref.insert (pid5); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_name_value (rp.prop_name_id (2), rp.prop_value_id ("C")); + ref.clear (); + ref.insert (pid4); + ref.insert (pid5); + EXPECT_EQ (ps2s (res), ps2s (ref)); + + res = rp.properties_ids_by_name_value (rp.prop_name_id (3), rp.prop_value_id ("C")); + ref.clear (); + ref.insert (pid5); + EXPECT_EQ (ps2s (res), ps2s (ref)); +} + diff --git a/src/laybasic/laybasic/layParsedLayerSource.cc b/src/laybasic/laybasic/layParsedLayerSource.cc index ce8a853a8..03d52863b 100644 --- a/src/laybasic/laybasic/layParsedLayerSource.cc +++ b/src/laybasic/laybasic/layParsedLayerSource.cc @@ -353,7 +353,7 @@ public: bool selection (std::set &ids) const { - const db::PropertiesRepository::properties_id_set &idv = db::PropertiesRepository::instance ().properties_ids_by_name_value (db::property_names_id (m_name), db::property_values_id (m_value)); + db::PropertiesRepository::properties_id_set idv = db::PropertiesRepository::instance ().properties_ids_by_name_value (db::property_names_id (m_name), db::property_values_id (m_value)); ids.insert (idv.begin (), idv.end ()); return ! m_equal; diff --git a/src/laybasic/unit_tests/layParsedLayerSource.cc b/src/laybasic/unit_tests/layParsedLayerSourceTests.cc similarity index 93% rename from src/laybasic/unit_tests/layParsedLayerSource.cc rename to src/laybasic/unit_tests/layParsedLayerSourceTests.cc index d283a6f6c..08a237c40 100644 --- a/src/laybasic/unit_tests/layParsedLayerSource.cc +++ b/src/laybasic/unit_tests/layParsedLayerSourceTests.cc @@ -26,6 +26,32 @@ #include "dbCell.h" #include "tlUnitTest.h" +namespace { + +/** + * @brief Installs a temporary repository instance for testing + * + * By using a temp instance, we do not disturb other tests. + */ +class TempPropertiesRepository +{ +public: + TempPropertiesRepository () + { + db::PropertiesRepository::replace_instance_temporarily (&m_temp); + } + + ~TempPropertiesRepository () + { + db::PropertiesRepository::replace_instance_temporarily (0); + } + +private: + db::PropertiesRepository m_temp; +}; + +} + TEST (1) { lay::ParsedLayerSource ps1 (1, 2, -1); @@ -165,6 +191,10 @@ TEST (4) TEST (5) { + // Use a temporary singleton properties repo, so we have better control + // over the results of property selectors. + TempPropertiesRepository tmp_prop_repo; + lay::ParsedLayerSource ps0 ("@2"); lay::ParsedLayerSource ps1 ("@2 [ X == #2 ]"); @@ -279,8 +309,8 @@ TEST (5) inv = ps2.property_selector ().matching (ids); EXPECT_EQ (inv, false); EXPECT_EQ (ids.size (), size_t (2)); - EXPECT_EQ (*ids.begin (), id1); - EXPECT_EQ (*(++ids.begin ()), id4); + EXPECT_EQ (ids.find (id1) != ids.end (), true); + EXPECT_EQ (ids.find (id4) != ids.end (), true); ps.clear (); ps.insert (tl::Variant ("X"), tl::Variant (2l)); @@ -291,27 +321,27 @@ TEST (5) inv = ps2.property_selector ().matching (ids); EXPECT_EQ (inv, false); EXPECT_EQ (ids.size (), size_t (3)); - EXPECT_EQ (*ids.begin (), id1); - EXPECT_EQ (*(++ids.begin ()), id4); - EXPECT_EQ (*(++(++ids.begin ())), id5); + EXPECT_EQ (ids.find (id1) != ids.end (), true); + EXPECT_EQ (ids.find (id4) != ids.end (), true); + EXPECT_EQ (ids.find (id5) != ids.end (), true); EXPECT_EQ (ps2a.property_selector ().check (id5), false); ids.clear (); inv = ps2a.property_selector ().matching (ids); EXPECT_EQ (inv, true); EXPECT_EQ (ids.size (), size_t (3)); - EXPECT_EQ (*ids.begin (), id1); - EXPECT_EQ (*(++ids.begin ()), id4); - EXPECT_EQ (*(++(++ids.begin ())), id5); + EXPECT_EQ (ids.find (id1) != ids.end (), true); + EXPECT_EQ (ids.find (id4) != ids.end (), true); + EXPECT_EQ (ids.find (id5) != ids.end (), true); EXPECT_EQ (ps2b.property_selector ().check (id5), false); ids.clear (); inv = ps2b.property_selector ().matching (ids); EXPECT_EQ (inv, true); EXPECT_EQ (ids.size (), size_t (3)); - EXPECT_EQ (*ids.begin (), id1); - EXPECT_EQ (*(++ids.begin ()), id4); - EXPECT_EQ (*(++(++ids.begin ())), id5); + EXPECT_EQ (ids.find (id1) != ids.end (), true); + EXPECT_EQ (ids.find (id4) != ids.end (), true); + EXPECT_EQ (ids.find (id5) != ids.end (), true); ps.clear (); @@ -321,9 +351,9 @@ TEST (5) inv = ps2.property_selector ().matching (ids); EXPECT_EQ (inv, false); EXPECT_EQ (ids.size (), size_t (3)); - EXPECT_EQ (*ids.begin (), id1); - EXPECT_EQ (*(++ids.begin ()), id4); - EXPECT_EQ (*(++(++ids.begin ())), id5); + EXPECT_EQ (ids.find (id1) != ids.end (), true); + EXPECT_EQ (ids.find (id4) != ids.end (), true); + EXPECT_EQ (ids.find (id5) != ids.end (), true); EXPECT_EQ (ps0.property_selector ().check (id6), true); ids.clear (); diff --git a/src/laybasic/unit_tests/unit_tests.pro b/src/laybasic/unit_tests/unit_tests.pro index d0132ce06..7bcc32f3a 100644 --- a/src/laybasic/unit_tests/unit_tests.pro +++ b/src/laybasic/unit_tests/unit_tests.pro @@ -12,7 +12,7 @@ SOURCES = \ layBitmapsToImage.cc \ layLayerProperties.cc \ layMarginTests.cc \ - layParsedLayerSource.cc \ + layParsedLayerSourceTests.cc \ layRenderer.cc \ layAbstractMenuTests.cc \ layTextInfoTests.cc \