2020-09-27, 22:44
I just put in a pull request with this on github, but I'm very new to github and not sure I did that correctly so I wanted to follow up here. Please advise me if I did something wrong, I may have another few bugfixes to contribute and want to make sure I'm going through the proper process.
Note that 1 test in the test suite ("make test") is failing for me right now: that test fails with or without my patch and appears completely unrelated.
Currently, if the query code needs to apply limits manually it ignores a limitStart which is greater than limitEnd. So if there are only 10 movies, and I request movies 11-20 sorted by dateadded, then it returns movies 1-10 again.
Note that 1 test in the test suite ("make test") is failing for me right now: that test fails with or without my patch and appears completely unrelated.
Currently, if the query code needs to apply limits manually it ignores a limitStart which is greater than limitEnd. So if there are only 10 movies, and I request movies 11-20 sorted by dateadded, then it returns movies 1-10 again.
- This can obviously break paginated interfaces that use the JSON API to query for results
- It's a bad incoherence based on sorting: requesting movies 11-20 with no sort uses the database limits, so correctly returns 0 items. But the same request sorted by dateadded returns 10 results.
Quote:diff --git a/xbmc/utils/SortUtils.cpp b/xbmc/utils/SortUtils.cpp
index 840e69ec06..b5c4b3cdfc 100644
--- a/xbmc/utils/SortUtils.cpp
+++ b/xbmc/utils/SortUtils.cpp
@@ -965,6 +965,11 @@ void SortUtils::Sort(SortBy sortBy, SortOrder sortOrder, SortAttribute attribute
items.erase(items.begin(), items.begin() + limitStart);
limitEnd -= limitStart;
}
+ else if (limitStart > 0)
+ {
+ items.erase(items.begin(), items.end());
+ limitEnd = 0;
+ }
if (limitEnd > 0 && (size_t)limitEnd < items.size())
items.erase(items.begin() + limitEnd, items.end());
}
@@ -1004,6 +1009,11 @@ void SortUtils::Sort(SortBy sortBy, SortOrder sortOrder, SortAttribute attribute
items.erase(items.begin(), items.begin() + limitStart);
limitEnd -= limitStart;
}
+ else if (limitStart > 0)
+ {
+ items.erase(items.begin(), items.end());
+ limitEnd = 0;
+ }
if (limitEnd > 0 && (size_t)limitEnd < items.size())
items.erase(items.begin() + limitEnd, items.end());
}