From fa277f69b14446b47a3dfa63ed7fc1a8a16ef7f8 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Thu, 9 Jul 2026 13:09:34 +0100 Subject: Include type in existingEntities cache Fixes the issues of entities having the same value (hash) but occurring in different places. This is most often when the client states the virtual host name as the referrer. --- src/ingestor.cpp | 14 ++++++++------ src/ingestor.hpp | 2 +- src/logTypes.hpp | 9 +++++++-- test/test-ingest.cpp | 14 ++++++++++++++ 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/ingestor.cpp b/src/ingestor.cpp index 41dd24f..ebff00b 100644 --- a/src/ingestor.cpp +++ b/src/ingestor.cpp @@ -53,9 +53,11 @@ namespace WebStat { operator()(const std::string_view value) const { return { - .hash = makeHash(value), + .key = { + .hash = makeHash(value), + .type = Type, + }, .id = std::nullopt, - .type = Type, .value = value, }; } @@ -331,7 +333,7 @@ namespace WebStat { } constexpr auto ENTITY_IDS = std::views::transform([](auto && value) { - return std::make_pair(value->hash, *value->id); + return std::tie(value->key, *value->id); }); Ingestor::Job::Result @@ -610,7 +612,7 @@ namespace WebStat { { auto lockedEntities = existingEntities.shared(); for (const auto entity : entities) { - if (auto existing = lockedEntities->find(entity->hash); existing != lockedEntities->end()) { + if (auto existing = lockedEntities->find(entity->key); existing != lockedEntities->end()) { entity->id = existing->second; } } @@ -644,7 +646,7 @@ namespace WebStat { }}; assert(!entity.id); - const auto & [typeName, onInsert] = ENTITY_TYPE_VALUES[std::to_underlying(entity.type)]; + const auto & [typeName, onInsert] = ENTITY_TYPE_VALUES[std::to_underlying(entity.key.type)]; bool entityNullDetail = true; std::tie(entity.id, entityNullDetail) = insert(dbconn, SQL::ENTITY_INSERT, SQL::ENTITY_INSERT_OPTS, entity.value, typeName); @@ -671,7 +673,7 @@ namespace WebStat { void Ingestor::onNewUserAgent(const Entity & entity) const { - const auto & [entityHash, entityId, type, value] = entity; + const auto & [_, entityId, value] = entity; auto curlOp = curlGetUserAgentDetail(*entityId, value, settings.userAgentAPI.c_str()); { std::lock_guard curlOperationsLock {curlOperationsMutex}; diff --git a/src/ingestor.hpp b/src/ingestor.hpp index e250072..7082dc0 100644 --- a/src/ingestor.hpp +++ b/src/ingestor.hpp @@ -101,7 +101,7 @@ namespace WebStat { DB::ConnectionPoolPtr dbpool; mutable Stats stats {}; - ThreadSafeT> existingEntities; + ThreadSafeT> existingEntities; LineBatch queuedLines, processingLines; bool terminated = false; diff --git a/src/logTypes.hpp b/src/logTypes.hpp index 7f0473e..d1bda3f 100644 --- a/src/logTypes.hpp +++ b/src/logTypes.hpp @@ -38,10 +38,15 @@ namespace WebStat { using EntityId = int32_t; using EntityHash = std::array; - struct Entity { + struct EntityKey { EntityHash hash; - std::optional id; EntityType type; + constexpr auto operator<=>(const EntityKey &) const noexcept = default; + }; + + struct Entity { + EntityKey key; + std::optional id; std::string_view value; }; } diff --git a/test/test-ingest.cpp b/test/test-ingest.cpp index a9966a5..aba6a66 100644 --- a/test/test-ingest.cpp +++ b/test/test-ingest.cpp @@ -248,6 +248,8 @@ constexpr std::string_view LOGLINE1 = R"LOG(git.randomdan.homeip.net 98.82.40.168 1755561576768318 GET "/repo/gentoobrowse-api/commit/gentoobrowse-api/unittests/fixtures/756569aa764177340726dd3d40b41d89b11b20c7/app-crypt/pdfcrack/Manifest" "?h=gentoobrowse-api-0.9.1&id=a2ed3fd30333721accd4b697bfcb6cc4165c7714" HTTP/1.1 200 1884 107791 "-" "Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko; compatible; Amazonbot/0.1; +https://developer.amazon.com/support/amazonbot) Chrome/119.0.6045.214 Safari/537.36" "test/plain")LOG"; constexpr std::string_view LOGLINE2 = R"LOG(www.randomdan.homeip.net 43.128.84.166 1755561575973204 GET "/app-dicts/myspell-et/Manifest" "" HTTP/1.1 200 312 10369 "https://google.com" "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36" "image/png")LOG"; +constexpr std::string_view LOGLINE_DUPES + = R"LOG(www.randomdan.homeip.net 43.128.84.166 1755561575973204 GET "/app-dicts/myspell-et/Manifest" "" HTTP/1.1 200 312 10369 "www.randomdan.homeip.net" "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36" "image/png")LOG"; BOOST_TEST_DECORATOR(*boost::unit_test::depends_on("QuotedStringsGood")) BOOST_TEST_DECORATOR(*boost::unit_test::depends_on("QueryStringsGood")) @@ -379,6 +381,18 @@ BOOST_DATA_TEST_CASE(StoreLogLine, BOOST_CHECK_EQUAL(existingEntities->size(), 5); } +BOOST_AUTO_TEST_CASE(StoreLogLines_WithDuplicateOfDifferentType, *boost::unit_test::depends_on("I/StoreLogLine")) +{ + const std::vector lines {std::string {LOGLINE1}, std::string {LOGLINE2}, std::string {LOGLINE_DUPES}}; + ingestLogLines(DB::MockDatabase::openConnectionTo("webstat").get(), lines); + BOOST_CHECK_EQUAL(stats.linesRead, 0); + BOOST_CHECK_EQUAL(stats.linesParsed, 3); + BOOST_CHECK_EQUAL(stats.linesParseFailed, 0); + BOOST_CHECK_EQUAL(stats.logsInserted, 3); + BOOST_CHECK_EQUAL(stats.entitiesInserted, 11); + BOOST_CHECK_EQUAL(existingEntities->size(), 11); +} + BOOST_AUTO_TEST_CASE(StoreLog, *boost::unit_test::depends_on("I/StoreLogLine")) { WebStat::LogFile log {"/tmp/store-log-fixture.log", 10}; -- cgit v1.3