Skip to content

Commit

Permalink
Change Lookup::Bypass to Lookup::Skip, add map_dyn method to LookupRe…
Browse files Browse the repository at this point in the history 
…sult, change map(|l| Box::new ... calls to use map_dyn()
  • Loading branch information
@marcus0x62
marcus0x62 committed Mar 4, 2024
1 parent 1e959df commit d560fd3
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 32 deletions.
40 changes: 25 additions & 15 deletions crates/server/src/authority/authority.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::proto::rr::{
Name,
};
use crate::{
authority::{LookupError, MessageRequest, UpdateResult, ZoneType},
authority::{LookupError, LookupObject, MessageRequest, UpdateResult, ZoneType},
proto::rr::{LowerName, RecordSet, RecordType, RrsetRecords},
server::RequestInfo,
};
Expand Down Expand Up @@ -208,22 +208,22 @@ pub enum LookupResult<T, E = LookupError> {
/// The authority did not answer the query and the next authority in the chain should
/// be consulted. Do not use this for any other purpose, in particular, do not use it
/// to represent an empty lookup (use Ok(EmptyLookup) for that.)
Bypass,
Skip,
}

/// The following are a minimal set of methods typically used with Result or Option, and that
/// were used in the server code or test suite prior to when the LookupResult type was created
/// (authority lookup functions previously returned a Result over a Lookup or LookupError type.)
impl<T, E: std::fmt::Display> LookupResult<T, E> {
impl<T: LookupObject + 'static, E: std::fmt::Display> LookupResult<T, E> {
/// Return LookupResult::Ok variant or panic with a custom error message.
pub fn expect(self, msg: &str) -> T {
match self {
Self::Ok(lookup) => lookup,
Self::Err(_e) => {
panic!("LookupResult expect called on LookupResult::Err: {msg}");
}
Self::Bypass => {
panic!("LookupResult expect called on LookupResult::Bypass: {msg}");
Self::Skip => {
panic!("LookupResult expect called on LookupResult::Skip: {msg}");
}
}
}
Expand All @@ -235,8 +235,8 @@ impl<T, E: std::fmt::Display> LookupResult<T, E> {
panic!("LookupResult::expect_err called on LookupResult::Ok value: {msg}");
}
Self::Err(e) => e,
Self::Bypass => {
panic!("LookupResult::expect_err called on LookupResult::Bypass value: {msg}");
Self::Skip => {
panic!("LookupResult::expect_err called on LookupResult::Skip value: {msg}");
}
}
}
Expand All @@ -248,8 +248,8 @@ impl<T, E: std::fmt::Display> LookupResult<T, E> {
Self::Err(e) => {
panic!("LookupResult unwrap called on LookupResult::Err: {e}");
}
Self::Bypass => {
panic!("LookupResult unwrap called on LookupResult::Bypass");
Self::Skip => {
panic!("LookupResult unwrap called on LookupResult::Skip");
}
}
}
Expand All @@ -261,8 +261,8 @@ impl<T, E: std::fmt::Display> LookupResult<T, E> {
panic!("LookupResult::unwrap_err called on LookupResult::Ok value");
}
Self::Err(e) => e,
Self::Bypass => {
panic!("LookupResult::unwrap_err called on LookupResult::Bypass");
Self::Skip => {
panic!("LookupResult::unwrap_err called on LookupResult::Skip");
}
}
}
Expand All @@ -279,22 +279,32 @@ impl<T, E: std::fmt::Display> LookupResult<T, E> {
}

/// Maps LookupResult::Ok(T) to LookupResult::Ok(U), passing LookupResult::Err and
/// LookupResult::Bypass values unchanged.
/// LookupResult::Skip values unchanged.
pub fn map<U, F: FnOnce(T) -> U>(self, op: F) -> LookupResult<U, E> {
match self {
Self::Ok(t) => LookupResult::<U, E>::Ok(op(t)),
Self::Err(e) => LookupResult::<U, E>::Err(e),
Self::Bypass => LookupResult::<U, E>::Bypass,
Self::Skip => LookupResult::<U, E>::Skip,
}
}

/// Maps LookupResult::Ok(T) to LookupResult::Ok(Box<dyn LookupObject>), passing LookupResult::Err and
/// LookupResult::Skip values unchanged.
pub fn map_dyn(self) -> LookupResult<Box<dyn LookupObject>, E> {
match self {
Self::Ok(t) => LookupResult::<Box<dyn LookupObject>, E>::Ok(Box::new(t) as Box<dyn LookupObject>),
Self::Err(e) => LookupResult::<Box<dyn LookupObject>, E>::Err(e),
Self::Skip => LookupResult::<Box<dyn LookupObject>, E>::Skip,
}
}

/// Maps LookupResult::Err(T) to LookupResult::Err(U), passing LookupResult::Ok and
/// LookupResult::Bypass values unchanged.
/// LookupResult::Skip values unchanged.
pub fn map_err<U, F: FnOnce(E) -> U>(self, op: F) -> LookupResult<T, U> {
match self {
Self::Ok(t) => LookupResult::<T, U>::Ok(t),
Self::Err(e) => LookupResult::<T, U>::Err(op(e)),
Self::Bypass => LookupResult::<T, U>::Bypass,
Self::Skip => LookupResult::<T, U>::Skip,
}
}
}
6 changes: 3 additions & 3 deletions crates/server/src/authority/authority_object.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ where
) -> LookupResult<Box<dyn LookupObject>> {
let this = self.as_ref();
let lookup = Authority::lookup(this, name, rtype, lookup_options).await;
lookup.map(|l| Box::new(l) as Box<dyn LookupObject>)
lookup.map_dyn() //(|l| Box::new(l) as Box<dyn LookupObject>)
}

/// Using the specified query, perform a lookup against this zone.
Expand All @@ -186,7 +186,7 @@ where
let this = self.as_ref();
debug!("performing {} on {}", request_info.query, this.origin());
let lookup = Authority::search(this, request_info, lookup_options).await;
lookup.map(|l| Box::new(l) as Box<dyn LookupObject>)
lookup.map_dyn() //(|l| Box::new(l) as Box<dyn LookupObject>)
}

/// Return the NSEC records based on the given name
Expand All @@ -202,7 +202,7 @@ where
lookup_options: LookupOptions,
) -> LookupResult<Box<dyn LookupObject>> {
let lookup = Authority::get_nsec_records(self.as_ref(), name, lookup_options).await;
lookup.map(|l| Box::new(l) as Box<dyn LookupObject>)
lookup.map_dyn() //(|l| Box::new(l) as Box<dyn LookupObject>)
}
}

Expand Down
28 changes: 14 additions & 14 deletions crates/server/src/authority/catalog.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ impl Catalog {

match result {
// The current authority in the chain did not handle the request, so we need to try the next one, if any.
LookupResult::Bypass => {
LookupResult::Skip => {
debug!("catalog::lookup::authority did not handle request.");
panic!("Correct implementation is part of the chained recursor PR.");
}
Expand Down Expand Up @@ -461,7 +461,7 @@ async fn lookup<'a, R: ResponseHandler + Unpin>(
}
}
LookupResult::Err(e) => LookupResult::Err(e),
LookupResult::Bypass => LookupResult::Bypass,
LookupResult::Skip => LookupResult::Skip,
}
}

Expand Down Expand Up @@ -517,9 +517,9 @@ async fn build_response(
let result = authority.search(request_info, lookup_options).await;

// Abort only if the authority declined to handle the request.
if let LookupResult::Bypass = result {
if let LookupResult::Skip = result {
trace!("build_response: Aborting search on None");
return LookupResult::Bypass;
return LookupResult::Skip;
}

#[allow(deprecated)]
Expand Down Expand Up @@ -580,8 +580,8 @@ async fn send_authoritative_response(
};
None
}
LookupResult::Bypass => {
debug!("Unexpected lookup bypass");
LookupResult::Skip => {
debug!("Unexpected lookup skip");
None
}
};
Expand All @@ -597,8 +597,8 @@ async fn send_authoritative_response(
warn!("ns_lookup errored: {}", e);
(None, None)
}
LookupResult::Bypass => {
warn!("ns_lookup unexpected bypass");
LookupResult::Skip => {
warn!("ns_lookup unexpected skip");
(None, None)
}
}
Expand All @@ -618,8 +618,8 @@ async fn send_authoritative_response(
warn!("failed to lookup nsecs: {}", e);
None
}
LookupResult::Bypass => {
warn!("Unexpected lookup bypass");
LookupResult::Skip => {
warn!("Unexpected lookup skip");
None
}
}
Expand All @@ -633,8 +633,8 @@ async fn send_authoritative_response(
warn!("failed to lookup soa: {}", e);
(nsecs, None)
}
LookupResult::Bypass => {
warn!("Unexpected lookup bypass");
LookupResult::Skip => {
warn!("Unexpected lookup skip");
(None, None)
}
}
Expand Down Expand Up @@ -688,8 +688,8 @@ async fn send_forwarded_response(
debug!("error resolving: {}", e);
Box::new(EmptyLookup)
}
LookupResult::Bypass => {
info!("Unexpected lookup bypass");
LookupResult::Skip => {
info!("Unexpected lookup skip");
Box::new(EmptyLookup)
}
LookupResult::Ok(rsp) => rsp,
Expand Down

0 comments on commit d560fd3

Please sign in to comment.