Rework

oxidecomputer · Apr 10, 2024 · 264a361 · 264a361
1 parent ce83ba7
commit 264a361
Showing 1 changed file with 35 additions and 40 deletions.
diff --git a/github/src/lib.rs b/github/src/lib.rs
@@ -586,7 +586,6 @@ impl Client {
         message: Message,
         media_type: crate::utils::MediaType,
         authentication: crate::auth::AuthenticationConstraint,
-        skip_cache: bool,
     ) -> ClientResult<(Option<crate::utils::NextLink>, crate::Response<Out>)>
     where
         Out: serde::de::DeserializeOwned + 'static + Send,
@@ -597,18 +596,34 @@ impl Client {
             .await?;
 
         #[cfg(feature = "httpcache")]
-        let req = {
+        let (req, cache_out) = {
             let mut req = self
                 .make_request(method.clone(), uri, message, media_type, authentication)
                 .await?;
 
-            if method == http::Method::GET && !skip_cache {
-                if let Ok(etag) = self.http_cache.lookup_etag(&uri) {
-                    req = req.header(http::header::IF_NONE_MATCH, etag);
+            let cache_out = if method == http::Method::GET {
+                let cache_out = if let Ok(body) = self.http_cache.lookup_body(&uri) {
+                    if let Ok(out) = serde_json::from_str::<Out>(&body) {
+                        Some(out)
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                };
+
+                if cache_out.is_some() {
+                    if let Ok(etag) = self.http_cache.lookup_etag(&uri) {
+                        req = req.header(http::header::IF_NONE_MATCH, etag);
+                    }
                 }
-            }
 
-            req
+                cache_out
+            } else {
+                None
+            };
+
+            (req, cache_out)
         };
 
         let response = req.send().await?;
@@ -665,35 +680,17 @@ impl Client {
                     // header when cargo builds with --cfg feature="httpcache"
                     #[cfg(feature = "httpcache")]
                     {
-                        if let Some(body) = self.http_cache.lookup_body(&uri) {
-                            if let Ok(out) = serde_json::from_str::<Out>(&body) {
-                                let link = match next_link {
-                                    Some(next_link) => Ok(Some(next_link)),
-                                    None => self
-                                        .http_cache
-                                        .lookup_next_link(&uri)
-                                        .map(|next_link| next_link.map(crate::utils::NextLink)),
-                                };
-                                link.map(|link| (link, Response::new(status, headers, out)))
-                            } else {
-                                self.request(
-                                    method,
-                                    uri,
-                                    message,
-                                    media_type,
-                                    authentication,
-                                    true,
-                                )
-                            }
+                        if let Some(out) = cache_out {
+                            let link = match next_link {
+                                Some(next_link) => Ok(Some(next_link)),
+                                None => self
+                                    .http_cache
+                                    .lookup_next_link(&uri)
+                                    .map(|next_link| next_link.map(crate::utils::NextLink)),
+                            };
+                            link.map(|link| (link, Response::new(status, headers, out)))
                         } else {
-                            self.request(
-                                method,
-                                uri,
-                                message,
-                                media_type,
-                                authentication,
-                                true,
-                            )
+                            unreachable!("GitHub returned a 304 for a request without an etag: {:?} {}", method, uri);
                         }
                     }
                     #[cfg(not(feature = "httpcache"))]
@@ -758,7 +755,7 @@ impl Client {
         D: serde::de::DeserializeOwned + 'static + Send,
     {
         let (_, r) = self
-            .request(method, uri, message, media_type, authentication, false)
+            .request(method, uri, message, media_type, authentication)
             .await?;
         Ok(r)
     }
@@ -813,8 +810,7 @@ impl Client {
             uri,
             Message::default(),
             crate::utils::MediaType::Json,
-            crate::auth::AuthenticationConstraint::Unconstrained,
-            false,
+            crate::auth::AuthenticationConstraint::Unconstrained
         )
         .await
     }
@@ -831,8 +827,7 @@ impl Client {
             url.as_str(),
             Message::default(),
             crate::utils::MediaType::Json,
-            crate::auth::AuthenticationConstraint::Unconstrained,
-            false,
+            crate::auth::AuthenticationConstraint::Unconstrained
         )
         .await
     }