Repository navigation
Get rid of org id and download - #1384
hamazaspavetisyan wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 1377-drop-library-xml #1384 +/- ##
=========================================================
+ Coverage 45.88% 45.95% +0.06%
=========================================================
Files 58 58
Lines 5067 5055 -12
Branches 2588 2586 -2
=========================================================
- Hits 2325 2323 -2
+ Misses 1015 1007 -8
+ Partials 1727 1725 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
I think there is some misunderstanding with download id. My impression was that support for it had to be added to OPDS library format, instead of dropping it from |
The downloadId was introduced to address specific needs in kiwix-desktop. Adding it to OPDS is not a good idea, as the OPDS library is intended to be generic. Instead, kiwix-desktop can use other methods to identify locally downloaded files. Or you see other uses cases of it ? |
That can be a solution but it has to be implemented first before ditching the previous approach. |
Just did it |
e17e3fd to
cad1f01
Compare
cf061ea to
2f2ad9b
Compare
origId was only ever set by the library.xml reader, which is gone (#1377), and the OPDS format has no representation for it. Removed Book::getOrigId() and Book::setOrigId(). Library::getBookAttributeCounts() and getBooksLanguagesWithCounts() skipped books with an origId. As no book can have one anymore, they now simply count every book; their results are unchanged.
downloadId was only persisted through library.xml, which is gone (#1377), and the OPDS format has no representation for it. Nothing in libkiwix uses it. Removed Book::getDownloadId() and Book::setDownloadId(). kiwix-desktop uses these methods to link a book to its running download and needs to keep track of that itself.
cad1f01 to
8411ec2
Compare
Follow-up to #1377 (#1383). Based on the
1377-drop-library-xmlbranch; please review/merge that PR first. Only the last two commits belong to this PR.With
library.xmlgone, twoBookfields can no longer be stored anywhere: OPDS has no representation for them, and adding one was ruled out in #1377. This PR removes them from the API for libkiwix 15.Removed public API
Book::getOrigId(),Book::setOrigId()library.xmlreader ever setorigId; it was not used by any known client (FeedFormat).Book::getDownloadId(),Book::setDownloadId()library.xml.Behavior changes
None.
Library::getBookAttributeCounts()andLibrary::getBooksLanguagesWithCounts()used to skip books with anorigId. Since no book can have one anymore, they now count every book, and their results are unchanged. These functions backgetBooksCategories(),getBooksCreators(),getBooksPublishers(),getBooksLanguages()and the/catalog/v2/categoriesand/catalog/v2/languagesfeeds.Impact
getDownloadId()/setDownloadId()13 times to link a book to its running download. Since Droplibrary.xmlsupport in libkiwix 15 (OPDS becomes the only library file format) #1377, that link is no longer saved across restarts anyway. Issue: Keep track of in-progress downloads without relying onBook::downloadIdin the library file kiwix-desktop#1538. It has to be fixed before kiwix-desktop moves to libkiwix 15.Commits
origIdchecks insrc/library.cpp.Testing
All 24 test suites pass on macOS. No test change was needed: once #1377 is in, no test uses either field.