refactor!: stop taking a callback for test-only uses

Author: djc

gix-protocol/src/fetch/refmap/init.rs

@@ -80,7 +80,6 @@ impl RefMap {
         let remote_refs = crate::ls_refs(
             transport,
             capabilities,
-            |_capabilities| Ok(crate::ls_refs::RefsAction::Continue),
             push_prefix_arguments(prefix_from_spec_as_filter_on_remote, &all_refspecs),
             &mut progress,
             trace_packetlines,

gix-protocol/src/ls_refs.rs

@@ -29,18 +29,6 @@ mod error {
 #[cfg(any(feature = "blocking-client", feature = "async-client"))]
 pub use error::Error;
 
-/// What to do after preparing ls-refs in [`ls_refs()`][crate::ls_refs()].
-#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
-pub enum RefsAction {
-    /// Continue by sending a 'ls-refs' command.
-    Continue,
-    /// Skip 'ls-refs' entirely.
-    ///
-    /// This is useful if the `ref-in-want` capability is taken advantage of. When fetching, one must must then send
-    /// `want-ref`s during the negotiation phase.
-    Skip,
-}
-
 #[cfg(any(feature = "blocking-client", feature = "async-client"))]
 pub(crate) mod function {
     use std::borrow::Cow;
@@ -50,14 +38,14 @@ pub(crate) mod function {
     use gix_transport::client::Capabilities;
     use maybe_async::maybe_async;
 
-    use super::{Error, RefsAction};
+    use super::Error;
     #[cfg(feature = "async-client")]
     use crate::transport::client::async_io::{Transport, TransportV2Ext};
     #[cfg(feature = "blocking-client")]
     use crate::transport::client::blocking_io::{Transport, TransportV2Ext};
     use crate::{
         handshake::{refs::from_v2_refs, Ref},
-        indicate_end_of_interaction, Command,
+        Command,
     };
 
     /// Invoke a ls-refs V2 command on `transport`, which requires a prior handshake that yielded
@@ -70,7 +58,6 @@ pub(crate) mod function {
     pub async fn ls_refs(
         mut transport: impl Transport,
         capabilities: &Capabilities,
-        prepare_ls_refs: impl FnOnce(&Capabilities) -> std::io::Result<RefsAction>,
         extra_args: Vec<BString>,
         progress: &mut impl Progress,
         trace: bool,
@@ -90,37 +77,22 @@ pub(crate) mod function {
         }
 
         ls_args.extend(extra_args);
-        let refs = match prepare_ls_refs(capabilities) {
-            Ok(RefsAction::Skip) => Vec::new(),
-            Ok(RefsAction::Continue) => {
-                ls_refs.validate_argument_prefixes(
-                    gix_transport::Protocol::V2,
-                    capabilities,
-                    &ls_args,
-                    &ls_features,
-                )?;
+        ls_refs.validate_argument_prefixes(gix_transport::Protocol::V2, capabilities, &ls_args, &ls_features)?;
 
-                progress.step();
-                progress.set_name("list refs".into());
-                let mut remote_refs = transport
-                    .invoke(
-                        ls_refs.as_str(),
-                        ls_features.into_iter(),
-                        if ls_args.is_empty() {
-                            None
-                        } else {
-                            Some(ls_args.into_iter())
-                        },
-                        trace,
-                    )
-                    .await?;
-                from_v2_refs(&mut remote_refs).await?
-            }
-            Err(err) => {
-                indicate_end_of_interaction(transport, trace).await?;
-                return Err(err.into());
-            }
-        };
-        Ok(refs)
+        progress.step();
+        progress.set_name("list refs".into());
+        let mut remote_refs = transport
+            .invoke(
+                ls_refs.as_str(),
+                ls_features.into_iter(),
+                if ls_args.is_empty() {
+                    None
+                } else {
+                    Some(ls_args.into_iter())
+                },
+                trace,
+            )
+            .await?;
+        Ok(from_v2_refs(&mut remote_refs).await?)
     }
 }

gix-protocol/tests/protocol/fetch/_impl.rs

@@ -1,3 +1,15 @@
+/// What to do after preparing ls-refs.
+#[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)]
+pub enum RefsAction {
+    /// Continue by sending a 'ls-refs' command.
+    Continue,
+    /// Skip 'ls-refs' entirely.
+    ///
+    /// This is useful if the `ref-in-want` capability is taken advantage of. When fetching, one
+    /// must then send `want-ref`s during the negotiation phase.
+    Skip,
+}
+
 mod fetch_fn {
     use std::borrow::Cow;
 
@@ -13,7 +25,7 @@ mod fetch_fn {
     use gix_transport::client::blocking_io::{ExtendedBufRead, HandleProgress, Transport};
     use maybe_async::maybe_async;
 
-    use super::{Action, Delegate};
+    use super::{Action, Delegate, RefsAction};
     use crate::fetch::Error;
 
     /// A way to indicate how to treat the connection underlying the transport, potentially allowing to reuse it.
@@ -91,18 +103,24 @@ mod fetch_fn {
         let agent = gix_protocol::agent(agent);
         let refs = match refs {
             Some(refs) => refs,
-            None => {
-                gix_protocol::ls_refs(
-                    &mut transport,
-                    &capabilities,
-                    |a| delegate.prepare_ls_refs(a),
-                    Vec::new(),
-                    &mut progress,
-                    trace,
-                    ("agent", Some(Cow::Owned(agent.clone()))),
-                )
-                .await?
-            }
+            None => match delegate.action() {
+                Ok(RefsAction::Skip) => Vec::new(),
+                Ok(RefsAction::Continue) => {
+                    gix_protocol::ls_refs(
+                        &mut transport,
+                        &capabilities,
+                        Vec::new(),
+                        &mut progress,
+                        trace,
+                        ("agent", Some(Cow::Owned(agent.clone()))),
+                    )
+                    .await?
+                }
+                Err(err) => {
+                    indicate_end_of_interaction(transport, trace).await?;
+                    return Err(err.into());
+                }
+            },
         };
 
         let fetch = Command::Fetch;
@@ -199,10 +217,11 @@ mod delegate {
     use gix_protocol::{
         fetch::{Arguments, Response},
         handshake::Ref,
-        ls_refs::{self, RefsAction},
     };
     use gix_transport::client::Capabilities;
 
+    use super::RefsAction;
+
     /// Defines what to do next after certain [`Delegate`] operations.
     #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)]
     pub enum Action {
@@ -231,10 +250,10 @@ mod delegate {
         /// Note that some arguments are preset based on typical use, and `features` are preset to maximize options.
         /// The `server` capabilities can be used to see which additional capabilities the server supports as per the handshake which happened prior.
         ///
-        /// If the delegate returns [`ls_refs::Action::Skip`], no `ls-refs` command is sent to the server.
+        /// If the delegate returns [`RefsAction::Skip`], no `ls-refs` command is sent to the server.
         ///
         /// Note that this is called only if we are using protocol version 2.
-        fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result<RefsAction> {
+        fn action(&mut self) -> std::io::Result<RefsAction> {
             Ok(RefsAction::Continue)
         }
 
@@ -299,8 +318,8 @@ mod delegate {
             self.deref().handshake_extra_parameters()
         }
 
-        fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result<RefsAction> {
-            self.deref_mut().prepare_ls_refs(_server)
+        fn action(&mut self) -> io::Result<RefsAction> {
+            self.deref_mut().action()
         }
 
         fn prepare_fetch(
@@ -328,8 +347,8 @@ mod delegate {
             self.deref().handshake_extra_parameters()
         }
 
-        fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result<RefsAction> {
-            self.deref_mut().prepare_ls_refs(_server)
+        fn action(&mut self) -> io::Result<RefsAction> {
+            self.deref_mut().action()
         }
 
         fn prepare_fetch(

gix-protocol/tests/protocol/fetch/mod.rs

@@ -4,14 +4,13 @@ use bstr::{BString, ByteSlice};
 use gix_protocol::{
     fetch::{Arguments, Response},
     handshake,
-    ls_refs::{self, RefsAction},
 };
 use gix_transport::client::Capabilities;
 
 use crate::fixture_bytes;
 
 pub(super) mod _impl;
-use _impl::{Action, DelegateBlocking};
+use _impl::{Action, DelegateBlocking, RefsAction};
 
 mod error {
     use std::io;
@@ -103,7 +102,7 @@ pub struct CloneRefInWantDelegate {
 }
 
 impl DelegateBlocking for CloneRefInWantDelegate {
-    fn prepare_ls_refs(&mut self, _server: &Capabilities) -> io::Result<RefsAction> {
+    fn action(&mut self) -> io::Result<RefsAction> {
         Ok(RefsAction::Skip)
     }
 
@@ -142,7 +141,7 @@ impl DelegateBlocking for LsRemoteDelegate {
     fn handshake_extra_parameters(&self) -> Vec<(String, Option<String>)> {
         vec![("value-only".into(), None), ("key".into(), Some("value".into()))]
     }
-    fn prepare_ls_refs(&mut self, _server: &Capabilities) -> std::io::Result<RefsAction> {
+    fn action(&mut self) -> std::io::Result<RefsAction> {
         match self.abort_with.take() {
             Some(err) => Err(err),
             None => Ok(RefsAction::Continue),

gix-protocol/tests/protocol/fetch/v2.rs

@@ -1,6 +1,6 @@
 use bstr::ByteSlice;
 use gix_features::progress;
-use gix_protocol::{handshake, ls_refs};
+use gix_protocol::handshake;
 use gix_transport::Protocol;
 
 use crate::fetch::{
@@ -143,7 +143,7 @@ async fn ls_remote_abort_in_prep_ls_refs() -> crate::Result {
         b"0044git-upload-pack does/not/matter\x00\x00version=2\x00value-only\x00key=value\x000000".as_bstr()
     );
     match err {
-        Error::LsRefs(ls_refs::Error::Io(err)) => {
+        Error::Io(err) => {
             assert_eq!(err.kind(), std::io::ErrorKind::Other);
             assert_eq!(err.get_ref().expect("other error").to_string(), "hello world");
         }