Skip to content

Fixed title indexing of items with title identical to the item's path #968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kelson42 merged 6 commits into from
Apr 16, 2025
Merged

Fixed title indexing of items with title identical to the item's path #968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aab6afe
Fixed a typo in unittest name
veloman-yunkan Apr 11, 2025
86e4c98
Introduced helper function makeHtmlItem()
veloman-yunkan Apr 11, 2025
9bc4e2e
New unit test Suggestion.titleEdgeCases
veloman-yunkan Apr 11, 2025
97e19c5
Fixed title indexing of items with title == path
veloman-yunkan Apr 11, 2025
52a8998
Dropped zim::writer::Dirent::getRealTitle()
veloman-yunkan Apr 11, 2025
8d202f8
PathTitleTinyString::getTitle() lost its parameter
veloman-yunkan Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/writer/_dirent.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ namespace zim
{ }

NS getNamespace() const { return static_cast<NS>(_ns); }
std::string getTitle() const { return pathTitle.getTitle(false); }
std::string getRealTitle() const { return pathTitle.getTitle(true); }
std::string getTitle() const { return pathTitle.getTitle(); }
std::string getPath() const { return pathTitle.getPath(); }

uint32_t getVersion() const { return version; }
Expand Down
8 changes: 2 additions & 6 deletions src/writer/tinyString.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,13 @@ namespace zim
}
return std::string(m_data);
}
std::string getTitle(bool storedOnly) const {
std::string getTitle() const {
if (m_size == 0) {
return std::string();
}
auto title_start = std::strlen(m_data) + 1;
if (title_start == m_size) {
if (storedOnly) {
return std::string(); // return empty title
} else {
return std::string(m_data); // return the path as a title
}
return std::string(m_data); // return the path as a title
} else {
return std::string(m_data+title_start, m_size-title_start);
}
Expand Down
2 changes: 1 addition & 1 deletion src/writer/xapianHandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ DirentHandler::ContentProviders XapianHandler::getContentProviders() const {
}

void XapianHandler::indexTitle(Dirent* dirent) {
auto title = dirent->getRealTitle();
auto title = dirent->getTitle();
if (title.empty()) {
return;
}
Expand Down
54 changes: 49 additions & 5 deletions test/suggestion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ TEST(Suggestion, singleTermOrder) {
);
}

TEST(Suggestion, caseDiacriticsAndHomogrpaphsHandling) {
TEST(Suggestion, caseDiacriticsAndHomographsHandling) {
std::vector<std::string> titles = {
"nonberlin",
"simply berlin",
Expand Down Expand Up @@ -667,16 +667,18 @@ TEST(Suggestion, reuseSearcher) {
ASSERT_EQ(count, 3);
}

std::shared_ptr<TestItem> makeHtmlItem(std::string path, std::string title) {
return std::make_shared<TestItem>(path, "text/html", title);
}

TEST(Suggestion, CJK) {
TempZimArchive tza("testZim");
zim::writer::Creator creator;
creator.configIndexing(true, "zh");
creator.startZimCreation(tza.getPath());

auto item1 = std::make_shared<TestItem>("testPath1", "text/html", "平方");
auto item2 = std::make_shared<TestItem>("testPath2", "text/html", "平方根");
creator.addItem(item1);
creator.addItem(item2);
creator.addItem(makeHtmlItem("testPath1", "平方"));
creator.addItem(makeHtmlItem("testPath2", "平方根"));

creator.addMetadata("Title", "Test zim");
creator.finishZimCreation();
Expand All @@ -692,4 +694,46 @@ TEST(Suggestion, CJK) {
);
}

TEST(Suggestion, titleEdgeCases) {
TempZimArchive tza("testZim");
zim::writer::Creator creator;
creator.configIndexing(true, "en");
creator.startZimCreation(tza.getPath());

// Title identical to path
creator.addItem(makeHtmlItem("About", "About"));

// Title differing from path in case only
creator.addItem(makeHtmlItem("Trout", "trout"));

// No title
creator.addItem(makeHtmlItem("Without", ""));

// Non edge cases
creator.addItem(makeHtmlItem("Stout", "About Rex Stout"));
creator.addItem(makeHtmlItem("Hangout", "Without a trout"));

creator.addMetadata("Title", "Test zim");
creator.finishZimCreation();

zim::Archive archive(tza.getPath());
EXPECT_SUGGESTION_RESULTS(archive, "abo",
"About",
"About Rex Stout",
);

EXPECT_SUGGESTION_RESULTS(archive, "witho",
"Without", // this is a path rather than a title
"Without a trout",
);

EXPECT_SUGGESTION_RESULTS(archive, "tro",
"trout",
"Without a trout",
);

EXPECT_SUGGESTION_RESULTS(archive, "hang"
/* nothing */
);
}
} // unnamed namespace
18 changes: 6 additions & 12 deletions test/tinyString.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ TEST(PathTitleTinyString, none)
ASSERT_EQ((std::string)s, "");
ASSERT_EQ(s, TinyString());
ASSERT_EQ(s.getPath(), "");
ASSERT_EQ(s.getTitle(false), "");
ASSERT_EQ(s.getTitle(true), "");
ASSERT_EQ(s.getTitle(), "");
}

TEST(PathTitleTinyString, empty)
Expand All @@ -95,8 +94,7 @@ TEST(PathTitleTinyString, empty)
ASSERT_EQ(s.size(), 1U);
ASSERT_EQ((std::string)s, std::string("", 1));
ASSERT_EQ(s.getPath(), "");
ASSERT_EQ(s.getTitle(false), "");
ASSERT_EQ(s.getTitle(true), "");
ASSERT_EQ(s.getTitle(), "");
}

TEST(PathTitleTinyString, no_title)
Expand All @@ -107,8 +105,7 @@ TEST(PathTitleTinyString, no_title)
ASSERT_EQ(s.size(), 4U);
ASSERT_EQ((std::string)s, std::string("FOO\0", 4));
ASSERT_EQ(s.getPath(), "FOO");
ASSERT_EQ(s.getTitle(false), "FOO");
ASSERT_EQ(s.getTitle(true), "");
ASSERT_EQ(s.getTitle(), "FOO");
}

TEST(PathTitleTinyString, no_path)
Expand All @@ -119,8 +116,7 @@ TEST(PathTitleTinyString, no_path)
ASSERT_EQ(s.size(), 4U);
ASSERT_EQ((std::string)s, std::string("\0BAR", 4));
ASSERT_EQ(s.getPath(), "");
ASSERT_EQ(s.getTitle(false), "BAR");
ASSERT_EQ(s.getTitle(true), "BAR");
ASSERT_EQ(s.getTitle(), "BAR");
}

TEST(PathTitleTinyString, path_title)
Expand All @@ -131,8 +127,7 @@ TEST(PathTitleTinyString, path_title)
ASSERT_EQ(s.size(), 7U);
ASSERT_EQ((std::string)s, std::string("FOO\0BAR", 7));
ASSERT_EQ(s.getPath(), "FOO");
ASSERT_EQ(s.getTitle(false), "BAR");
ASSERT_EQ(s.getTitle(true), "BAR");
ASSERT_EQ(s.getTitle(), "BAR");
}

TEST(PathTitleTinyString, equal_path_title)
Expand All @@ -143,7 +138,6 @@ TEST(PathTitleTinyString, equal_path_title)
ASSERT_EQ(s.size(), 4U);
ASSERT_EQ((std::string)s, std::string("FOO\0", 4));
ASSERT_EQ(s.getPath(), "FOO");
ASSERT_EQ(s.getTitle(false), "FOO");
ASSERT_EQ(s.getTitle(true), "");
ASSERT_EQ(s.getTitle(), "FOO");
}
} // namespace
Loading