Remove redundancy.

Author: PSUdaemon

include/proxy/ParentConsistentHash.h

@@ -48,8 +48,6 @@ class ParentConsistentHash : public ParentSelectionStrategy
   uint64_t                           hash_seed0;
   uint64_t                           hash_seed1;
 
-  std::unique_ptr<ATSHash64> createHashInstance(ParentHashAlgorithm algo, uint64_t seed0, uint64_t seed1);
-
 public:
   static const int PRIMARY   = 0;
   static const int SECONDARY = 1;

include/proxy/ParentSelection.h

@@ -35,11 +35,13 @@
 #include "proxy/ControlMatcher.h"
 #include "records/RecProcess.h"
 #include "tscore/ConsistentHash.h"
+#include "tscore/Hash.h"
 #include "tscore/Tokenizer.h"
 #include "tscore/ink_apidefs.h"
 #include "proxy/HostStatus.h"
 
 #include <algorithm>
+#include <memory>
 #include <vector>
 
 #define MAX_PARENTS           64
@@ -450,6 +452,10 @@ struct ParentConfig {
 // Helper Functions
 ParentRecord *createDefaultParent(char *val);
 
+// Hash utility functions
+ParentHashAlgorithm        parseHashAlgorithm(std::string_view name);
+std::unique_ptr<ATSHash64> createHashInstance(ParentHashAlgorithm algo, uint64_t seed0, uint64_t seed1);
+
 // Unit Test Functions
 void show_result(ParentResult *aParentResult);
 void br(HttpRequestData *h, const char *os_hostname, sockaddr const *dest_ip = nullptr); // short for build request

src/proxy/ParentConsistentHash.cc

@@ -30,20 +30,6 @@ namespace
 DbgCtl dbg_ctl_parent_select{"parent_select"};
 }
 
-std::unique_ptr<ATSHash64>
-ParentConsistentHash::createHashInstance(ParentHashAlgorithm algo, uint64_t seed0, uint64_t seed1)
-{
-  switch (algo) {
-  case ParentHashAlgorithm::SIPHASH24:
-    return std::make_unique<ATSHash64Sip24>(seed0, seed1);
-  case ParentHashAlgorithm::SIPHASH13:
-    return std::make_unique<ATSHash64Sip13>(seed0, seed1);
-  default:
-    Warning("Unknown hash algorithm %d, using SipHash-2-4", static_cast<int>(algo));
-    return std::make_unique<ATSHash64Sip24>(seed0, seed1);
-  }
-}
-
 ParentConsistentHash::ParentConsistentHash(ParentRecord *parent_record)
 {
   int i;

src/proxy/ParentSelection.cc

@@ -69,6 +69,20 @@ parseHashAlgorithm(std::string_view name)
   }
 }
 
+std::unique_ptr<ATSHash64>
+createHashInstance(ParentHashAlgorithm algo, uint64_t seed0, uint64_t seed1)
+{
+  switch (algo) {
+  case ParentHashAlgorithm::SIPHASH24:
+    return std::make_unique<ATSHash64Sip24>(seed0, seed1);
+  case ParentHashAlgorithm::SIPHASH13:
+    return std::make_unique<ATSHash64Sip13>(seed0, seed1);
+  default:
+    Warning("Unknown hash algorithm %d, using SipHash-2-4", static_cast<int>(algo));
+    return std::make_unique<ATSHash64Sip24>(seed0, seed1);
+  }
+}
+
 ParentSelectionPolicy::ParentSelectionPolicy()
 {
   int32_t retry_time     = 0;

src/proxy/http/remap/NextHopConsistentHash.cc

@@ -27,6 +27,7 @@
 #include "iocore/utils/Machine.h"
 #include "tsutil/YamlCfg.h"
 #include "proxy/http/remap/NextHopConsistentHash.h"
+#include "proxy/ParentSelection.h"
 
 // hash_key strings.
 constexpr std::string_view hash_key_url           = "url";
@@ -57,16 +58,10 @@ std::shared_ptr<HostRecord>
 NextHopConsistentHash::chashLookup(const std::shared_ptr<ATSConsistentHash> &ring, uint32_t cur_ring, ParentResult &result,
                                    HttpRequestData &request_info, bool *wrapped, uint64_t sm_id)
 {
-  uint64_t hash_key = 0;
-  // Create hash instance based on configured algorithm and seeds
-  std::unique_ptr<ATSHash64> hash;
-  if (hash_algorithm == "siphash13") {
-    hash = std::make_unique<ATSHash64Sip13>(hash_seed0, hash_seed1);
-  } else {
-    hash = std::make_unique<ATSHash64Sip24>(hash_seed0, hash_seed1);
-  }
-  HostRecord            *host_rec = nullptr;
-  ATSConsistentHashIter *iter     = &result.chashIter[cur_ring];
+  uint64_t                   hash_key = 0;
+  std::unique_ptr<ATSHash64> hash     = createHashInstance(parseHashAlgorithm(hash_algorithm), hash_seed0, hash_seed1);
+  HostRecord                *host_rec = nullptr;
+  ATSConsistentHashIter     *iter     = &result.chashIter[cur_ring];
 
   if (result.chash_init[cur_ring] == false) {
     hash_key                    = getHashKey(sm_id, request_info, hash.get());
@@ -196,13 +191,7 @@ NextHopConsistentHash::NextHopConsistentHash(const std::string_view name, const
                                 "', this strategy will be ignored.");
   }
 
-  // Create the hash instance based on configured algorithm and seeds
-  if (hash_algorithm == "siphash13") {
-    hash = std::make_unique<ATSHash64Sip13>(hash_seed0, hash_seed1);
-  } else {
-    // Default to siphash24
-    hash = std::make_unique<ATSHash64Sip24>(hash_seed0, hash_seed1);
-  }
+  hash = createHashInstance(parseHashAlgorithm(hash_algorithm), hash_seed0, hash_seed1);
 
   // load up the hash rings.
   for (uint32_t i = 0; i < groups; i++) {

src/proxy/http/remap/unit-tests/CMakeLists.txt

@@ -201,8 +201,13 @@ target_compile_definitions(
 target_include_directories(test_NextHopStrategyFactory PRIVATE ${PROJECT_SOURCE_DIR}/tests/include)
 
 target_link_libraries(
-  test_NextHopStrategyFactory PRIVATE Catch2::Catch2WithMain ts::hdrs ts::inkutils tscore libswoc::libswoc
-                                      yaml-cpp::yaml-cpp
+  test_NextHopStrategyFactory
+  PRIVATE Catch2::Catch2WithMain
+          ts::hdrs
+          ts::inkutils
+          tscore
+          libswoc::libswoc
+          yaml-cpp::yaml-cpp
 )
 
 add_catch2_test(NAME test_NextHopStrategyFactory COMMAND $<TARGET_FILE:test_NextHopStrategyFactory>)

src/proxy/http/remap/unit-tests/nexthop_test_stubs.cc

@@ -209,3 +209,30 @@ HostStatus::setHostStatus(const std::string_view host, TSHostStatus status, unsi
   NH_Dbg(DbgCtl{"next_hop"}, "setting host status for '%.*s' to %s", static_cast<int>(host.size()), host.data(),
          HostStatusNames[status]);
 }
+
+// Stub implementations for Parent Selection hash utilities
+#include "proxy/ParentSelection.h"
+#include "tscore/Hash.h"
+
+ParentHashAlgorithm
+parseHashAlgorithm(std::string_view name)
+{
+  if (name == "siphash13") {
+    return ParentHashAlgorithm::SIPHASH13;
+  } else {
+    return ParentHashAlgorithm::SIPHASH24;
+  }
+}
+
+std::unique_ptr<ATSHash64>
+createHashInstance(ParentHashAlgorithm algo, uint64_t seed0, uint64_t seed1)
+{
+  switch (algo) {
+  case ParentHashAlgorithm::SIPHASH24:
+    return std::make_unique<ATSHash64Sip24>(seed0, seed1);
+  case ParentHashAlgorithm::SIPHASH13:
+    return std::make_unique<ATSHash64Sip13>(seed0, seed1);
+  default:
+    return std::make_unique<ATSHash64Sip24>(seed0, seed1);
+  }
+}