TSetOfMetricMapInitializers throws for unknown types

MRPT · Nov 6, 2023 · cb7fd9c · cb7fd9c
1 parent 5942cf8
commit cb7fd9c
Show file tree

Hide file tree

Showing 5 changed files with 82 additions and 8 deletions.
diff --git a/doc/source/doxygen-docs/changelog.md b/doc/source/doxygen-docs/changelog.md
@@ -2,6 +2,8 @@
 
 # Version 2.11.3: UNRELEASED
 - Changes in libraries:
+  - \ref mrpt_core_grp
+    - Add the `[[nodiscard]]` attribute to all functions returning a value in `<mrpt/core/bits_math.h>`
   - \ref mrpt_maps_grp
     - mrpt::maps::COccupancyGridMap3D::insertObservation() now also handles mrpt::obs::CObservationPointCloud
     - New virtual interface mrpt::maps::NearestNeighborsCapable, implemented in:
@@ -11,8 +13,7 @@
       - mrpt::maps::COccupancyGridMap2D
     - New virtual method mrpt::maps::CMetricMap::boundingBox()
     - mrpt::maps::TMetricMapInitializer now returns `shared_ptr`s instead of plain pointers.
-  - \ref mrpt_core_grp
-    - Add the `[[nodiscard]]` attribute to all functions returning a value in `<mrpt/core/bits_math.h>`
+    - mrpt::maps::TSetOfMetricMapInitializers::loadFromConfigFile() now throws if it finds a `*_count` entry with an unknown map class name.
 
 # Version 2.11.2: Released Oct 25th, 2023
 - Changes in libraries:

diff --git a/libs/config/include/mrpt/config/CConfigFileBase.h b/libs/config/include/mrpt/config/CConfigFileBase.h
@@ -74,10 +74,26 @@ class CConfigFileBase
 	/** Returns a list with all the section names. */
 	virtual void getAllSections(std::vector<std::string>& sections) const = 0;
 
+	/** Returns, by value, a list with all the section names. */
+	std::vector<std::string> sections() const
+	{
+		std::vector<std::string> ret;
+		getAllSections(ret);
+		return ret;
+	}
+
 	/** Returs a list with all the keys into a section */
 	virtual void getAllKeys(
 		const std::string& section, std::vector<std::string>& keys) const = 0;
 
+	/** Returs, by value, a list with all the keys into a section */
+	std::vector<std::string> keys(const std::string& section) const
+	{
+		std::vector<std::string> keys;
+		getAllKeys(section, keys);
+		return keys;
+	}
+
 	/** Checks if a given section exists (name is case insensitive)
 	 * \sa keyExists() */
 	bool sectionExists(const std::string& section_name) const;

diff --git a/libs/config/src/CConfigFileMemory_unittest.cpp b/libs/config/src/CConfigFileMemory_unittest.cpp
@@ -25,11 +25,11 @@ TEST(CConfigFileMemory, readwrite)
 
 TEST(CConfigFileMemory, Sections)
 {
-	std::vector<std::string> sections;
 	mrpt::config::CConfigFileMemory second;
 	second.write("one", "name", "val");
 	second.write("two", "names", "value");
-	second.getAllSections(sections);
+
+	const std::vector<std::string> sections = second.sections();
 	EXPECT_EQ(2U, sections.size());
 	if (sections.size() == 2)
 	{  // avoid potential crash if fails
@@ -40,11 +40,11 @@ TEST(CConfigFileMemory, Sections)
 
 TEST(CConfigFileMemory, Names)
 {
-	std::vector<std::string> names;
 	mrpt::config::CConfigFileMemory third;
 	third.write("sec", "name", "val");
 	third.write("sec", "names", "value");
-	third.getAllKeys("sec", names);
+
+	const std::vector<std::string> names = third.keys("sec");
 	EXPECT_EQ(2U, names.size());
 	if (names.size() == 2)
 	{  // avoid potential crash if fails

diff --git a/libs/maps/src/maps/CMultiMetricMap_unittest.cpp b/libs/maps/src/maps/CMultiMetricMap_unittest.cpp
@@ -8,6 +8,7 @@
    +------------------------------------------------------------------------+ */
 
 #include <gtest/gtest.h>
+#include <mrpt/config/CConfigFileMemory.h>
 #include <mrpt/maps/CMultiMetricMap.h>
 #include <mrpt/maps/COccupancyGridMap2D.h>
 #include <mrpt/maps/CSimplePointsMap.h>
@@ -21,7 +22,9 @@ TEST(CMultiMetricMapTests, isEmpty)
 	}
 }
 
-static mrpt::maps::CMultiMetricMap initializer1()
+namespace
+{
+mrpt::maps::CMultiMetricMap initializer1()
 {
 	mrpt::maps::TSetOfMetricMapInitializers map_inits;
 	{
@@ -42,13 +45,14 @@ static mrpt::maps::CMultiMetricMap initializer1()
 	}
 }
 
-static mrpt::maps::CMultiMetricMap initializer2()
+mrpt::maps::CMultiMetricMap initializer2()
 {
 	mrpt::maps::CMultiMetricMap m;
 	m.maps.push_back(mrpt::maps::COccupancyGridMap2D::Create());
 	m.maps.push_back(mrpt::maps::CSimplePointsMap::Create());
 	return m;
 }
+}  // namespace
 
 TEST(CMultiMetricMapTests, initializers)
 {
@@ -102,3 +106,42 @@ TEST(CMultiMetricMapTests, moveOp)
 
 	EXPECT_EQ(m2.mapByClass<CSimplePointsMap>()->size(), 1U);
 }
+
+TEST(CMultiMetricMapTests, unknownMapType)
+{
+	{
+		const mrpt::config::CConfigFileMemory cfg(R""""(
+[map]
+// Creation of maps:
+occupancyGrid_count=1
+
+[map_occupancyGrid_00_creationOpts]
+min_x=-10
+max_x= 10
+min_y=-10
+max_y= 10
+)"""");
+
+		mrpt::maps::TSetOfMetricMapInitializers map_inits;
+		EXPECT_NO_THROW(map_inits.loadFromConfigFile(cfg, "map"));
+		EXPECT_EQ(map_inits.size(), 1UL);
+	}
+
+	{
+		const mrpt::config::CConfigFileMemory cfg(R""""(
+[map]
+// Creation of maps:
+occupancyGrid_count=1
+myUnregisteredMap_count=1
+
+[map_occupancyGrid_00_creationOpts]
+min_x=-10
+max_x= 10
+min_y=-10
+max_y= 10
+)"""");
+
+		mrpt::maps::TSetOfMetricMapInitializers map_inits;
+		EXPECT_ANY_THROW(map_inits.loadFromConfigFile(cfg, "map"));
+	}
+}
diff --git a/libs/obs/src/TMetricMapInitializer.cpp b/libs/obs/src/TMetricMapInitializer.cpp
@@ -109,6 +109,20 @@ void TSetOfMetricMapInitializers::loadFromConfigFile(
 
 	}  // end for each map kind
 
+	// Check for unknown map types and throw an error:
+	for (const auto& s : ini.keys(sectionName))
+	{
+		auto p = s.find("_count");
+		if (p == std::string::npos) continue;
+		const auto className = s.substr(0, p);
+		if (allMapKinds.count(className) != 0) continue;  // ok, it exists
+		THROW_EXCEPTION_FMT(
+			"Error: found INI section '%s' while parsing "
+			"TSetOfMetricMapInitializers, but there is no such registered "
+			"CMetricMap class '%s'",
+			s.c_str(), className.c_str());
+	}
+
 	MRPT_END
 }