From 0be00b791ffa9e6d073a6626108fa763ac9d62bb Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Sat, 11 Dec 2021 16:34:16 +0400 Subject: [PATCH 1/2] Searcher::add_reader() rejects duplicate readers A O(N) linear search was added to `Searcher::add_reader()` deliberately. This doesn't seem to be an operation that may lead to performance problems. --- src/searcher.cpp | 6 ++++++ test/searcher.cpp | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/searcher.cpp b/src/searcher.cpp index 8fa77c391..51f16a7b3 100644 --- a/src/searcher.cpp +++ b/src/searcher.cpp @@ -94,6 +94,12 @@ bool Searcher::add_reader(Reader* reader) if (!reader->hasFulltextIndex()) { return false; } + + for ( const Reader* const existing_reader : readers ) { + if ( existing_reader->getZimFilePath() == reader->getZimFilePath() ) + return false; + } + this->readers.push_back(reader); return true; } diff --git a/test/searcher.cpp b/test/searcher.cpp index a3569d959..735f4c373 100644 --- a/test/searcher.cpp +++ b/test/searcher.cpp @@ -5,6 +5,20 @@ namespace kiwix { +TEST(Searcher, add_reader) { + Reader reader1("./test/example.zim"); + Reader reader2("./test/example.zim"); + Reader reader3("./test/../test/example.zim"); + + Searcher searcher; + ASSERT_TRUE (searcher.add_reader(&reader1)); + ASSERT_FALSE(searcher.add_reader(&reader1)); + ASSERT_FALSE(searcher.add_reader(&reader2)); + + // equivalence of resolved paths is not checked by Searcher::add_reader + ASSERT_TRUE(searcher.add_reader(&reader3)); +} + TEST(Searcher, search) { Reader reader("./test/example.zim"); @@ -64,4 +78,4 @@ TEST(Searcher, incrementalRange) { ASSERT_EQ(suggCount, 50); } -} \ No newline at end of file +} From 250f46c7f989748332813846631252df7fbb0642 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Mon, 13 Dec 2021 15:47:04 +0400 Subject: [PATCH 2/2] fixup! Searcher::add_reader() rejects duplicate readers --- src/searcher.cpp | 2 +- test/searcher.cpp | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/searcher.cpp b/src/searcher.cpp index 51f16a7b3..7e7403027 100644 --- a/src/searcher.cpp +++ b/src/searcher.cpp @@ -96,7 +96,7 @@ bool Searcher::add_reader(Reader* reader) } for ( const Reader* const existing_reader : readers ) { - if ( existing_reader->getZimFilePath() == reader->getZimFilePath() ) + if ( existing_reader->getZimArchive()->getUuid() == reader->getZimArchive()->getUuid() ) return false; } diff --git a/test/searcher.cpp b/test/searcher.cpp index 735f4c373..5adb0adeb 100644 --- a/test/searcher.cpp +++ b/test/searcher.cpp @@ -14,9 +14,7 @@ TEST(Searcher, add_reader) { ASSERT_TRUE (searcher.add_reader(&reader1)); ASSERT_FALSE(searcher.add_reader(&reader1)); ASSERT_FALSE(searcher.add_reader(&reader2)); - - // equivalence of resolved paths is not checked by Searcher::add_reader - ASSERT_TRUE(searcher.add_reader(&reader3)); + ASSERT_FALSE(searcher.add_reader(&reader3)); } TEST(Searcher, search) {