From 308933ad9f74199cfde6694db9af410475fcd405 Mon Sep 17 00:00:00 2001
From: mmd-osm <mmd.osm@gmail.com>
Date: Fri, 3 Jan 2025 23:03:22 +0100
Subject: [PATCH] Introduce psql_array_ids_to_vector

---
 .../backend/apidb/common_pgsql_selection.hpp  |   4 -
 include/cgimap/backend/apidb/utils.hpp        |  11 ++
 src/backend/apidb/common_pgsql_selection.cpp  | 171 ++++--------------
 src/backend/apidb/utils.cpp                   | 113 ++++++++++++
 test/test_apidb_backend_nodes.cpp             |  38 ++++
 5 files changed, 194 insertions(+), 143 deletions(-)

diff --git a/include/cgimap/backend/apidb/common_pgsql_selection.hpp b/include/cgimap/backend/apidb/common_pgsql_selection.hpp
index b3617954..9955d5e0 100644
--- a/include/cgimap/backend/apidb/common_pgsql_selection.hpp
+++ b/include/cgimap/backend/apidb/common_pgsql_selection.hpp
@@ -51,9 +51,5 @@ void extract_changesets(
   const std::chrono::system_clock::time_point &now,
   bool include_changeset_discussions);
 
-// parses psql array based on specs given
-// https://www.postgresql.org/docs/current/static/arrays.html#ARRAYS-IO
-std::vector<std::string> psql_array_to_vector(std::string_view str);
-std::vector<std::string> psql_array_to_vector(const pqxx::field& field);
 
 #endif /* CGIMAP_BACKEND_APIDB_COMMON_PGSQL_SELECTION_HPP */
diff --git a/include/cgimap/backend/apidb/utils.hpp b/include/cgimap/backend/apidb/utils.hpp
index 1adc492a..ecf8f30a 100644
--- a/include/cgimap/backend/apidb/utils.hpp
+++ b/include/cgimap/backend/apidb/utils.hpp
@@ -19,4 +19,15 @@
  */
 void check_postgres_version(pqxx::connection_base &conn);
 
+// parses psql array based on specs given
+// https://www.postgresql.org/docs/current/static/arrays.html#ARRAYS-IO
+std::vector<std::string> psql_array_to_vector(std::string_view str, int size_hint = 0);
+std::vector<std::string> psql_array_to_vector(const pqxx::field& field, int size_hint = 0);
+
+template <typename T>
+std::vector<T> psql_array_ids_to_vector(const pqxx::field& field);
+
+template <typename T>
+std::vector<T> psql_array_ids_to_vector(std::string_view str);
+
 #endif /* CGIMAP_BACKEND_APIDB_UTILS_HPP */
diff --git a/src/backend/apidb/common_pgsql_selection.cpp b/src/backend/apidb/common_pgsql_selection.cpp
index bb091dd3..7c1fc2bf 100644
--- a/src/backend/apidb/common_pgsql_selection.cpp
+++ b/src/backend/apidb/common_pgsql_selection.cpp
@@ -12,8 +12,6 @@
 #include "cgimap/backend/apidb/utils.hpp"
 
 #include <chrono>
-#include <charconv>
-
 
 namespace {
 
@@ -209,32 +207,14 @@ std::optional<T> extract_optional(const pqxx_field &f) {
   tags.reserve(keys.size());
 
   for (std::size_t i = 0; i < keys.size(); i++)
-    tags.emplace_back(keys[i], values[i]);
+    tags.emplace_back(std::move(keys[i]), std::move(values[i]));
 
   return tags;
 }
 
-[[nodiscard]] nodes_t extract_nodes(const pqxx_tuple &row, const way_extra_columns& col) {
-
-  nodes_t nodes;
-
-  auto ids = psql_array_to_vector(row[col.node_ids_col]);
-
-  nodes.reserve(ids.size());
-
-  for (const auto & id : ids) {
-    osm_nwr_id_t node{};
-
-    auto [_, ec] = std::from_chars(id.data(), id.data() + id.size(), node);
+[[nodiscard]] nodes_t extract_way_nodes(const pqxx_tuple &row, const way_extra_columns& col) {
 
-    if (ec != std::errc()) {
-     throw std::runtime_error("Node id conversion to integer failed");
-    }
-
-    nodes.push_back(node);
-  }
-
-  return nodes;
+  return psql_array_ids_to_vector<osm_nwr_id_t>(row[col.node_ids_col]);
 }
 
 element_type type_from_name(const char *name) {
@@ -269,9 +249,9 @@ element_type type_from_name(const char *name) {
 
   members_t members;
 
-  auto types = psql_array_to_vector(row[col.member_types_col]);
-  auto ids   = psql_array_to_vector(row[col.member_ids_col]);
-  auto roles = psql_array_to_vector(row[col.member_roles_col]);
+  auto ids   = psql_array_ids_to_vector<osm_nwr_id_t>(row[col.member_ids_col]);
+  auto types = psql_array_to_vector(row[col.member_types_col], ids.size());
+  auto roles = psql_array_to_vector(row[col.member_roles_col], ids.size());
 
   if (types.size() != ids.size() ||
       ids.size() != roles.size()) {
@@ -281,19 +261,8 @@ element_type type_from_name(const char *name) {
   members.reserve(ids.size());
 
   for (std::size_t i=0; i<ids.size(); i++) {
-    member_info member;
-    member.type = type_from_name(types[i].c_str());
-
-    {
-      auto [_, ec] = std::from_chars(ids[i].data(), ids[i].data() + ids[i].size(), member.ref);
-
-      if (ec != std::errc()) {
-       throw std::runtime_error("Member ref conversion to integer failed");
-      }
-    }
-
-    member.role = roles[i];
-    members.push_back(member);
+    element_type member_type = type_from_name(types[i].c_str());
+    members.emplace_back(member_type, ids[i], std::move(roles[i]));
   }
 
   return members;
@@ -303,11 +272,11 @@ element_type type_from_name(const char *name) {
 
   comments_t comments;
 
-  auto id           = psql_array_to_vector(row[col.comment_id_col]);
-  auto author_id    = psql_array_to_vector(row[col.comment_author_id_col]);
-  auto display_name = psql_array_to_vector(row[col.comment_display_name_col]);
-  auto body         = psql_array_to_vector(row[col.comment_body_col]);
-  auto created_at   = psql_array_to_vector(row[col.comment_created_at_col]);
+  auto id           = psql_array_ids_to_vector<osm_changeset_comment_id_t>(row[col.comment_id_col]);
+  auto author_id    = psql_array_ids_to_vector<osm_user_id_t>(row[col.comment_author_id_col]);
+  auto display_name = psql_array_to_vector(row[col.comment_display_name_col], id.size());
+  auto body         = psql_array_to_vector(row[col.comment_body_col], id.size());
+  auto created_at   = psql_array_to_vector(row[col.comment_created_at_col], id.size());
 
   if (id.size() != author_id.size() ||
       author_id.size() != display_name.size() ||
@@ -319,29 +288,11 @@ element_type type_from_name(const char *name) {
   comments.reserve(id.size());
 
   for (std::size_t i=0; i<id.size(); i++) {
-
-    changeset_comment_info comment;
-
-    {
-      auto [_, ec] = std::from_chars(id[i].data(), id[i].data() + id[i].size(), comment.id);
-
-      if (ec != std::errc()) {
-       throw std::runtime_error("Comment id conversion to integer failed");
-      }
-    }
-
-    {
-      auto [_, ec] = std::from_chars(author_id[i].data(), author_id[i].data() + author_id[i].size(), comment.author_id);
-
-      if (ec != std::errc()) {
-       throw std::runtime_error("Author id conversion to integer failed");
-      }
-    }
-
-    comment.author_display_name = display_name[i];
-    comment.body = body[i];
-    comment.created_at = created_at[i];
-    comments.push_back(comment);
+    comments.emplace_back(id[i],
+                          author_id[i],
+                          std::move(body[i]),
+                          std::move(created_at[i]),
+                          std::move(display_name[i]));
   }
 
   return comments;
@@ -352,12 +303,11 @@ struct node {
   using extra_columns = node_extra_columns;
 
   struct extra_info {
-    double lon;
-    double lat;
-    inline void extract(const pqxx_tuple &row, const extra_columns& col) {
-      lon = double(row[col.longitude_col].as<int64_t>()) / (global_settings::get_scale());
-      lat = double(row[col.latitude_col].as<int64_t>()) / (global_settings::get_scale());
-    }
+    const double lon;
+    const double lat;
+    extra_info(const pqxx_tuple &row, const extra_columns& col) :
+      lon(double(row[col.longitude_col].as<int64_t>()) / global_settings::get_scale()),
+      lat(double(row[col.latitude_col].as<int64_t>()) / global_settings::get_scale()) {}
   };
   static inline void write(
     output_formatter &formatter, const element_info &elem,
@@ -370,16 +320,15 @@ struct way {
   using extra_columns = way_extra_columns;
 
   struct extra_info {
-    nodes_t nodes;
-    inline void extract(const pqxx_tuple &row, const extra_columns& col) {
-      nodes = extract_nodes(row, col);
-    }
+    const nodes_t way_nodes;
+    extra_info(const pqxx_tuple &row, const extra_columns& col) :
+      way_nodes(extract_way_nodes(row, col)) {}
   };
 
   static inline void write(
     output_formatter &formatter, const element_info &elem,
     const extra_info &extra, const tags_t &tags) {
-    formatter.write_way(elem, extra.nodes, tags);
+    formatter.write_way(elem, extra.way_nodes, tags);
   }
 };
 
@@ -387,10 +336,10 @@ struct relation {
   using extra_columns = relation_extra_columns;
 
   struct extra_info {
-    members_t members;
-    inline void extract(const pqxx_tuple &row, const extra_columns& col) {
-      members = extract_members(row, col);
-    }
+    const members_t members;
+    extra_info(const pqxx_tuple &row, const extra_columns& col) :
+      members(extract_members(row, col)) {}
+
   };
 
   static inline void write(
@@ -411,9 +360,8 @@ void extract(
   const tag_columns tag_cols(rows);
 
   for (const auto &row : rows) {
-    typename T::extra_info extra{};
+    typename T::extra_info extra(row, extra_cols);
     auto elem = extract_elem(row, cc, elem_cols);
-    extra.extract(row, extra_cols);
     auto tags = extract_tags(row, tag_cols);
     if (notify)
       notify(elem);     // let callback function know about a new element we're processing
@@ -466,59 +414,4 @@ void extract_changesets(
   }
 }
 
-std::vector<std::string> psql_array_to_vector(const pqxx::field& field) {
-  return psql_array_to_vector(std::string_view(field.c_str(), field.size()));
-}
 
-std::vector<std::string> psql_array_to_vector(std::string_view str) {
-  std::vector<std::string> strs;
-  std::string value;
-  bool quotedValue = false;
-  bool escaped = false;
-  bool write = false;
-
-  if (str == "{NULL}" || str.empty())
-    return strs;
-
-  const auto str_size = str.size();
-  for (unsigned int i = 1; i < str_size; i++) {
-    if (str[i] == ',') {
-      if (quotedValue) {
-        value += ',';
-      } else {
-        write = true;
-      }
-    } else if (str[i] == '"') {
-      if (escaped) {
-        value += '"';
-        escaped = false;
-      } else if (quotedValue) {
-        quotedValue = false;
-      } else {
-        quotedValue = true;
-      }
-    } else if (str[i] == '\\') {
-      if (escaped) {
-        value += '\\';
-        escaped = false;
-      } else {
-        escaped = true;
-      }
-    } else if (str[i] == '}') {
-      if (quotedValue) {
-        value += '}';
-      } else {
-        write = true;
-      }
-    } else {
-      value += str[i];
-    }
-
-    if (write) {
-      strs.push_back(value);
-      value.clear();
-      write = false;
-    }
-  }
-  return strs;
-}
diff --git a/src/backend/apidb/utils.cpp b/src/backend/apidb/utils.cpp
index e1554ae1..e50b42bd 100644
--- a/src/backend/apidb/utils.cpp
+++ b/src/backend/apidb/utils.cpp
@@ -7,6 +7,13 @@
  * For a full list of authors see the git log.
  */
 
+#include <charconv>
+#include <functional>
+#include <pqxx/pqxx>
+#include <string_view>
+#include <string>
+#include <vector>
+
 #include "cgimap/backend/apidb/utils.hpp"
 
 void check_postgres_version(pqxx::connection_base &conn) {
@@ -16,3 +23,109 @@ void check_postgres_version(pqxx::connection_base &conn) {
       + std::to_string(version));
   }
 }
+
+std::vector<std::string> psql_array_to_vector(const pqxx::field& field, int size_hint) {
+  return psql_array_to_vector(std::string_view(field.c_str(), field.size()), size_hint);
+}
+
+std::vector<std::string> psql_array_to_vector(std::string_view str, int size_hint) {
+  std::vector<std::string> strs;
+  std::string value;
+  bool quotedValue = false;
+  bool escaped = false;
+  bool write = false;
+
+  if (size_hint > 0)
+    strs.reserve(size_hint);
+
+  if (str == "{NULL}" || str.empty())
+    return strs;
+
+  const auto str_size = str.size();
+  for (unsigned int i = 1; i < str_size; i++) {
+    if (str[i] == ',') {
+      if (quotedValue) {
+        value += ',';
+      } else {
+        write = true;
+      }
+    } else if (str[i] == '"') {
+      if (escaped) {
+        value += '"';
+        escaped = false;
+      } else if (quotedValue) {
+        quotedValue = false;
+      } else {
+        quotedValue = true;
+      }
+    } else if (str[i] == '\\') {
+      if (escaped) {
+        value += '\\';
+        escaped = false;
+      } else {
+        escaped = true;
+      }
+    } else if (str[i] == '}') {
+      if (quotedValue) {
+        value += '}';
+      } else {
+        write = true;
+      }
+    } else {
+      value += str[i];
+    }
+
+    if (write) {
+      strs.emplace_back(std::move(value));
+      value.clear();
+      write = false;
+    }
+  }
+  return strs;
+}
+
+template <typename T>
+std::vector<T> psql_array_ids_to_vector(const pqxx::field& field) {
+  return psql_array_ids_to_vector<T>(std::string_view(field.c_str(), field.size()));
+}
+
+template <typename T>
+std::vector<T> psql_array_ids_to_vector(std::string_view str) {
+  std::vector<T> ids;
+  int start_offset = 1;
+
+  if (str == "{NULL}" || str.empty())
+    return ids;
+
+  const auto str_size = str.size();
+
+  for (unsigned int i = 1; i < str_size; i++) {
+    if (str[i] == ',' || str[i] == '}') {
+      T id;
+
+      auto [_, ec] = std::from_chars(str.data() + start_offset, str.data() + i, id);
+
+      if (ec != std::errc()) {
+       throw std::runtime_error("Conversion to integer failed");
+      }
+      ids.emplace_back(id);
+      start_offset = i + 1;
+    }
+  }
+  return ids;
+}
+
+
+template std::vector<int32_t> psql_array_ids_to_vector(const pqxx::field& field);
+template std::vector<int64_t> psql_array_ids_to_vector(const pqxx::field& field);
+
+template std::vector<uint32_t> psql_array_ids_to_vector(const pqxx::field& field);
+template std::vector<uint64_t> psql_array_ids_to_vector(const pqxx::field& field);
+
+template std::vector<int32_t> psql_array_ids_to_vector(std::string_view str);
+template std::vector<int64_t> psql_array_ids_to_vector(std::string_view str);
+
+template std::vector<uint32_t> psql_array_ids_to_vector(std::string_view str);
+template std::vector<uint64_t> psql_array_ids_to_vector(std::string_view str);
+
+
diff --git a/test/test_apidb_backend_nodes.cpp b/test/test_apidb_backend_nodes.cpp
index a19cb3e8..de731a1c 100644
--- a/test/test_apidb_backend_nodes.cpp
+++ b/test/test_apidb_backend_nodes.cpp
@@ -239,6 +239,44 @@ TEST_CASE("test_psql_array_to_vector", "[nodb]") {
   }
 }
 
+TEST_CASE("psql_array_ids_to_vector", "[nodb]") {
+
+  std::string test;
+  std::vector<int64_t> actual_values;
+  std::vector<int64_t> values;
+
+  SECTION("NULL") {
+    test = "{NULL}";
+    values = psql_array_ids_to_vector<int64_t>(test);
+    REQUIRE (values == actual_values);
+  }
+
+  SECTION("Empty string") {
+    test = "";
+    values = psql_array_ids_to_vector<int64_t>(test);
+    REQUIRE (values == actual_values);
+  }
+
+  SECTION("One value") {
+    test = "{1}";
+    values = psql_array_ids_to_vector<int64_t>(test);
+    actual_values = {1};
+    REQUIRE (values == actual_values);
+  }
+
+  SECTION("Two values") {
+    test = "{1,-2}";
+    values = psql_array_ids_to_vector<int64_t>(test);
+    actual_values = {1, -2};
+    REQUIRE (values == actual_values);
+  }
+
+  SECTION("Invalid string") {
+    test = "{1,}";
+    REQUIRE_THROWS_AS(psql_array_ids_to_vector<int64_t>(test), std::runtime_error);
+  }
+}
+
 int main(int argc, char *argv[]) {
   Catch::Session session;