Improve sqlite iterator interface (ros2#33)

* ros2GH-84 Cleanup: conform to ROS2 naming conventions

* ros2GH-84 Allow only a single non-end iterator for query results

Reason: multiple iterators for the same query result would interfere
with each other as a single result row can only be retrieved
once. These side effects would be very surprising and are therefore
forbidden.

* ros2GH-84 Do not use noexcept for gcc < 6

rosbag2_storage_default_plugins/include/rosbag2_storage_default_plugins/sqlite/sqlite_statement_wrapper.hpp

@@ -69,6 +69,12 @@ class ROSBAG2_STORAGE_DEFAULT_PLUGINS_PUBLIC SqliteStatementWrapper
         }
       }
 
+      Iterator(const Iterator &) = delete;
+      Iterator & operator=(const Iterator &) = delete;
+
+      Iterator(Iterator &&) = default;
+      Iterator & operator=(Iterator &&) = default;
+
       Iterator & operator++()
       {
         if (next_row_idx_ != POSITION_END) {
@@ -102,11 +108,11 @@ class ROSBAG2_STORAGE_DEFAULT_PLUGINS_PUBLIC SqliteStatementWrapper
         return row;
       }
 
-      bool operator==(Iterator other) const
+      bool operator==(const Iterator & other) const
       {
         return statement_ == other.statement_ && next_row_idx_ == other.next_row_idx_;
       }
-      bool operator!=(Iterator other) const
+      bool operator!=(const Iterator & other) const
       {
         return !(*this == other);
       }
@@ -140,11 +146,12 @@ class ROSBAG2_STORAGE_DEFAULT_PLUGINS_PUBLIC SqliteStatementWrapper
     };
 
     explicit QueryResult(std::shared_ptr<SqliteStatementWrapper> statement)
-    : statement_(statement)
+    : statement_(statement), is_already_accessed_(false)
     {}
 
     Iterator begin()
     {
+      try_access_data();
       return Iterator(statement_, 0);
     }
     Iterator end()
@@ -158,7 +165,16 @@ class ROSBAG2_STORAGE_DEFAULT_PLUGINS_PUBLIC SqliteStatementWrapper
     }
 
 private:
+    void try_access_data()
+    {
+      if (is_already_accessed_) {
+        throw SqliteException("Only one iterator per query result is supported!");
+      }
+      is_already_accessed_ = true;
+    }
+
     std::shared_ptr<SqliteStatementWrapper> statement_;
+    bool is_already_accessed_;
   };
 
   std::shared_ptr<SqliteStatementWrapper> execute_and_reset();
@@ -177,7 +193,7 @@ class ROSBAG2_STORAGE_DEFAULT_PLUGINS_PUBLIC SqliteStatementWrapper
 
 private:
   bool step();
-  bool isQueryOk(int return_code);
+  bool is_query_ok(int return_code);
 
   void obtain_column_value(size_t index, int & value) const;
   void obtain_column_value(size_t index, rcutils_time_point_value_t & value) const;

rosbag2_storage_default_plugins/src/rosbag2_storage_default_plugins/sqlite/sqlite_statement_wrapper.cpp

@@ -51,14 +51,14 @@ SqliteStatementWrapper::~SqliteStatementWrapper()
 std::shared_ptr<SqliteStatementWrapper> SqliteStatementWrapper::execute_and_reset()
 {
   int return_code = sqlite3_step(statement_);
-  if (!isQueryOk(return_code)) {
+  if (!is_query_ok(return_code)) {
     throw SqliteException("Error processing SQLite statement. Return code: " +
             std::to_string(return_code));
   }
   return reset();
 }
 
-bool SqliteStatementWrapper::isQueryOk(int return_code)
+bool SqliteStatementWrapper::is_query_ok(int return_code)
 {
   return return_code == SQLITE_OK || return_code == SQLITE_DONE || return_code == SQLITE_ROW;
 }

rosbag2_storage_default_plugins/test/rosbag2_storage_default_plugins/sqlite/sqlite_wrapper_integration_test.cpp

@@ -40,7 +40,8 @@ class SqliteWrapperTestFixture : public StorageTestFixture
 TEST_F(SqliteWrapperTestFixture, querying_empty_adhoc_results_gives_empty_query_result) {
   auto result = db_.prepare_statement("SELECT 1 WHERE 1=2;")->execute_query<int>();
 
-  ASSERT_THAT(result.begin(), Eq(result.end()));
+  // ASSERT_THAT cannot be used as the iterators can only be moved and not copied.
+  ASSERT_TRUE(result.begin() == result.end());
 }
 
 TEST_F(SqliteWrapperTestFixture, querying_adhoc_results_with_normal_data_gives_content) {
@@ -93,6 +94,7 @@ TEST_F(SqliteWrapperTestFixture, all_result_rows_are_available) {
   ASSERT_THAT(row_value, Eq(4));
 
   query->reset();
+  result = query->execute_query<int>();
 
   // iterator access
   row_value = 1;
@@ -104,6 +106,16 @@ TEST_F(SqliteWrapperTestFixture, all_result_rows_are_available) {
   ASSERT_THAT(row_value, Eq(4));
 }
 
+TEST_F(SqliteWrapperTestFixture, only_a_single_iterator_is_allowed_per_result) {
+  auto result = db_.prepare_statement("SELECT 1;")->execute_query<int>();
+
+  auto row = result.begin();
+
+  EXPECT_THROW(result.get_single_line(), rosbag2_storage_plugins::SqliteException);
+  EXPECT_THROW(result.begin(), rosbag2_storage_plugins::SqliteException);
+  EXPECT_NO_THROW(result.end());
+}
+
 TEST_F(SqliteWrapperTestFixture, ros_specific_types_are_supported_for_reading_and_writing) {
   db_.prepare_statement("CREATE TABLE test (timestamp INTEGER, data BLOB);")->execute_and_reset();
   rcutils_time_point_value_t time = 1099511627783;