fix(releases): Revert pagination refactor

Revert 29df3e8 (and also d62d2ec, which contains tests for 29df3e8). These changes, which meant to simplify our pagination logic, appear to have caused several closely-related regressions in Sentry CLI version 2.31.1.

Fixes GH-2053
May fix (potentially, need to verify) GH-2052, GH-2054, and GH-2055

src/api/mod.rs

@@ -51,6 +51,7 @@ use crate::utils::retry::{get_default_backoff, DurationAsMilliseconds};
 use crate::utils::sourcemaps::get_sourcemap_reference_from_headers;
 use crate::utils::ui::{capitalize_string, make_byte_progress_bar};
 
+use self::pagination::Pagination;
 use connection_manager::CurlConnectionManager;
 use encoding::{PathArg, QueryArg};
 use errors::{ApiError, ApiErrorKind, ApiResult, SentryError};
@@ -533,9 +534,9 @@ impl<'a> AuthenticatedApi<'a> {
                 }
             }
 
-            let next = resp.next_pagination_cursor();
+            let pagination = resp.pagination();
             rv.extend(resp.convert::<Vec<Artifact>>()?);
-            if let Some(next) = next {
+            if let Some(next) = pagination.into_next_cursor() {
                 cursor = next;
             } else {
                 break;
@@ -1134,9 +1135,9 @@ impl<'a> AuthenticatedApi<'a> {
                     break;
                 }
             }
-            let next = resp.next_pagination_cursor();
+            let pagination = resp.pagination();
             rv.extend(resp.convert::<Vec<Organization>>()?);
-            if let Some(next) = next {
+            if let Some(next) = pagination.into_next_cursor() {
                 cursor = next;
             } else {
                 break;
@@ -1178,9 +1179,9 @@ impl<'a> AuthenticatedApi<'a> {
                     break;
                 }
             }
-            let next = resp.next_pagination_cursor();
+            let pagination = resp.pagination();
             rv.extend(resp.convert::<Vec<Monitor>>()?);
-            if let Some(next) = next {
+            if let Some(next) = pagination.into_next_cursor() {
                 cursor = next;
             } else {
                 break;
@@ -1206,9 +1207,9 @@ impl<'a> AuthenticatedApi<'a> {
                     break;
                 }
             }
-            let next = resp.next_pagination_cursor();
+            let pagination = resp.pagination();
             rv.extend(resp.convert::<Vec<Project>>()?);
-            if let Some(next) = next {
+            if let Some(next) = pagination.into_next_cursor() {
                 cursor = next;
             } else {
                 break;
@@ -1246,14 +1247,14 @@ impl<'a> AuthenticatedApi<'a> {
                 }
             }
 
-            let next = resp.next_pagination_cursor();
+            let pagination = resp.pagination();
             rv.extend(resp.convert::<Vec<ProcessedEvent>>()?);
 
             if requests_no == max_pages {
                 break;
             }
 
-            if let Some(next) = next {
+            if let Some(next) = pagination.into_next_cursor() {
                 cursor = next;
             } else {
                 break;
@@ -1272,7 +1273,7 @@ impl<'a> AuthenticatedApi<'a> {
         query: Option<String>,
     ) -> ApiResult<Vec<Issue>> {
         let mut rv = vec![];
-        let mut cursor = String::from("");
+        let mut cursor = "".to_string();
         let mut requests_no = 0;
 
         let url = if let Some(query) = query {
@@ -1299,14 +1300,14 @@ impl<'a> AuthenticatedApi<'a> {
                 }
             }
 
-            let next = resp.next_pagination_cursor();
+            let pagination = resp.pagination();
             rv.extend(resp.convert::<Vec<Issue>>()?);
 
             if requests_no == max_pages {
                 break;
             }
 
-            if let Some(next) = next {
+            if let Some(next) = pagination.into_next_cursor() {
                 cursor = next;
             } else {
                 break;
@@ -1330,12 +1331,13 @@ impl<'a> AuthenticatedApi<'a> {
             if resp.status() == 404 {
                 break;
             } else {
-                if let Some(next) = resp.next_pagination_cursor() {
+                let pagination = resp.pagination();
+                rv.extend(resp.convert::<Vec<Repo>>()?);
+                if let Some(next) = pagination.into_next_cursor() {
                     cursor = next;
                 } else {
                     break;
                 }
-                rv.extend(resp.convert::<Vec<Repo>>()?);
             }
         }
         Ok(rv)
@@ -1968,10 +1970,11 @@ impl ApiResponse {
         None
     }
 
-    fn next_pagination_cursor(&self) -> Option<String> {
+    /// Returns the pagination info
+    pub fn pagination(&self) -> Pagination {
         self.get_header("link")
-            .and_then(pagination::next_cursor)
-            .map(String::from)
+            .and_then(|x| x.parse().ok())
+            .unwrap_or_default()
     }
 
     /// Returns true if the response is JSON.

src/api/pagination.rs

@@ -1,46 +1,42 @@
+use std::str::FromStr;
+
 use crate::utils::http;
 
-pub(super) fn next_cursor(value: &str) -> Option<&str> {
-    http::parse_link_header(value)
-        .iter()
-        .rev() // Reversing is necessary for backwards compatibility with a previous implementation
-        .find(|item| item.get("rel") == Some(&"next"))
-        .and_then::<&str, _>(|item| {
-            if item.get("results") == Some(&"true") {
-                Some(item.get("cursor").unwrap_or(&""))
-            } else {
-                None
-            }
-        })
+#[derive(Debug, Clone)]
+pub struct Link {
+    results: bool,
+    cursor: String,
 }
 
-#[cfg(test)]
-mod tests {
-    use super::*;
+#[derive(Debug, Default, Clone)]
+pub struct Pagination {
+    next: Option<Link>,
+}
 
-    #[test]
-    fn test_pagination_empty_string() {
-        let result = next_cursor("");
-        assert_eq!(result, None);
+impl Pagination {
+    pub fn into_next_cursor(self) -> Option<String> {
+        self.next
+            .and_then(|x| if x.results { Some(x.cursor) } else { None })
     }
+}
 
-    #[test]
-    fn test_pagination_with_next() {
-        let result = next_cursor(
-            "<https://sentry.io/api/0/organizations/sentry/releases/?&cursor=100:-1:1>; \
-            rel=\"previous\"; results=\"false\"; cursor=\"100:-1:1\", \
-            <https://sentry.io/api/0/organizations/sentry/releases/?&cursor=100:1:0>; \
-            rel=\"next\"; results=\"true\"; cursor=\"100:1:0\"",
-        );
-        assert_eq!(result, Some("100:1:0"));
-    }
+impl FromStr for Pagination {
+    type Err = ();
+
+    fn from_str(s: &str) -> Result<Pagination, ()> {
+        let mut rv = Pagination::default();
+        for item in http::parse_link_header(s) {
+            let target = match item.get("rel") {
+                Some(&"next") => &mut rv.next,
+                _ => continue,
+            };
+
+            *target = Some(Link {
+                results: item.get("results") == Some(&"true"),
+                cursor: (*item.get("cursor").unwrap_or(&"")).to_string(),
+            });
+        }
 
-    #[test]
-    fn test_pagination_without_next() {
-        let result = next_cursor(
-            "<https://sentry.io/api/0/organizations/sentry/releases/?&cursor=100:-1:1>; \
-            rel=\"previous\"; results=\"false\"; cursor=\"100:-1:1\"",
-        );
-        assert_eq!(result, None);
+        Ok(rv)
     }
 }