From 5e14a283b67940526a20c365b827d8090a552bad Mon Sep 17 00:00:00 2001
From: mauropasse <mauropasse@hotmail.com>
Date: Wed, 6 Mar 2024 13:45:58 +0000
Subject: [PATCH] Modify rclcpp_action::GoalUUID hashing algorithm (#2441)

* Add unit tests for hashing rclcpp_action::GoalUUID's

* Use the FNV-1a hash algorithm for Goal UUID

Signed-off-by: Mauro Passerino <mpasserino@irobot.com>
---
 rclcpp_action/include/rclcpp_action/types.hpp | 15 ++++-----
 rclcpp_action/test/test_types.cpp             | 33 +++++++++++++++++++
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/rclcpp_action/include/rclcpp_action/types.hpp b/rclcpp_action/include/rclcpp_action/types.hpp
index 829b6cd2c1..cf359dfaa9 100644
--- a/rclcpp_action/include/rclcpp_action/types.hpp
+++ b/rclcpp_action/include/rclcpp_action/types.hpp
@@ -69,14 +69,13 @@ struct hash<rclcpp_action::GoalUUID>
 {
   size_t operator()(const rclcpp_action::GoalUUID & uuid) const noexcept
   {
-    // TODO(sloretz) Use someone else's hash function and cite it
-    size_t result = 0;
-    for (size_t i = 0; i < uuid.size(); ++i) {
-      for (size_t b = 0; b < sizeof(size_t); ++b) {
-        size_t part = uuid[i];
-        part <<= CHAR_BIT * b;
-        result ^= part;
-      }
+    // Using the FNV-1a hash algorithm
+    constexpr size_t FNV_prime = 1099511628211u;
+    size_t result = 14695981039346656037u;
+
+    for (const auto & byte : uuid) {
+      result ^= byte;
+      result *= FNV_prime;
     }
     return result;
   }
diff --git a/rclcpp_action/test/test_types.cpp b/rclcpp_action/test/test_types.cpp
index 4922c52930..619a4f7899 100644
--- a/rclcpp_action/test/test_types.cpp
+++ b/rclcpp_action/test/test_types.cpp
@@ -15,6 +15,7 @@
 #include <gtest/gtest.h>
 
 #include <limits>
+#include <random>
 #include "rclcpp_action/types.hpp"
 
 TEST(TestActionTypes, goal_uuid_to_string) {
@@ -59,3 +60,35 @@ TEST(TestActionTypes, rcl_action_goal_info_to_goal_uuid) {
     EXPECT_EQ(goal_info.goal_id.uuid[i], goal_id[i]);
   }
 }
+
+TEST(TestActionTypes, goal_uuid_to_hashed_uuid_random) {
+  // Use std::random_device to seed the generator of goal IDs.
+  std::random_device rd;
+  std::independent_bits_engine<
+    std::default_random_engine, 8, decltype(rd())> random_bytes_generator(rd());
+
+  std::vector<size_t> hashed_guuids;
+  constexpr size_t iterations = 1000;
+
+  for (size_t i = 0; i < iterations; i++) {
+    rclcpp_action::GoalUUID goal_id;
+
+    // Generate random bytes for each element of the array
+    for (auto & element : goal_id) {
+      element = static_cast<uint8_t>(random_bytes_generator());
+    }
+
+    size_t new_hashed_guuid = std::hash<rclcpp_action::GoalUUID>()(goal_id);
+
+    // Search for any prevoius hashed goal_id with the same value
+    for (auto prev_hashed_guuid : hashed_guuids) {
+      EXPECT_NE(prev_hashed_guuid, new_hashed_guuid);
+      if (prev_hashed_guuid == new_hashed_guuid) {
+        // Fail before the first occurrence of a collision
+        GTEST_FAIL();
+      }
+    }
+
+    hashed_guuids.push_back(new_hashed_guuid);
+  }
+}