improve comments in bucket helper

src/model/helper/bucket.rs

@@ -19,31 +19,29 @@ impl<'a> BucketHelper<'a> {
 	// ================
 	//      Local functions to find buckets FAST.
 	//      This is only for the fast path in API requests.
-	//      They do not conserve the read-after-write guarantee.
+	//      They do not provide the read-after-write guarantee
+	//      when used in conjunction with other operations that
+	//      modify buckets and bucket aliases.
 	// ================
 
-	/// Return bucket ID corresponding to global bucket name.
+	/// Return bucket corresponding to global bucket name, if it exists
+	/// (and is not a tombstone entry).
 	///
 	/// The name can be of two forms:
 	/// 1. A global bucket alias
 	/// 2. The full ID of a bucket encoded in hex
 	///
-	/// This will not do any network interaction to check the alias table,
-	/// it will only check the local copy of the table.
-	/// As a consequence, it does not conserve read-after-write guarantees.
+	/// Note that there is no possible ambiguity between the two forms,
+	/// as the maximum length of a bucket name is 63 characters, and the full
+	/// hex id is 64 chars long.
+	///
+	/// This will not do any network interaction to check the alias and
+	/// bucket tables, it will only check the local copy of the table.
+	/// As a consequence, it does not provide read-after-write guarantees.
 	pub fn resolve_global_bucket_fast(
 		&self,
 		bucket_name: &String,
 	) -> Result<Option<Bucket>, GarageError> {
-		// Bucket names in Garage are aliases, true bucket identifiers
-		// are 32-byte UUIDs. This function resolves bucket names into
-		// their full identifier by looking up in the bucket_alias_table.
-		// This function also allows buckets to be identified by their
-		// full UUID (hex-encoded). Here, if the name to be resolved is a
-		// hex string of the correct length, it is directly parsed as a bucket
-		// identifier which is returned. There is no risk of this conflicting
-		// with an actual bucket name: bucket names are max 63 chars long by
-		// the AWS spec, and hex-encoded UUIDs are 64 chars long.
 		let hexbucket = hex::decode(bucket_name.as_str())
 			.ok()
 			.and_then(|by| Uuid::try_from(&by));
@@ -68,19 +66,20 @@ impl<'a> BucketHelper<'a> {
 			.filter(|x| !x.state.is_deleted()))
 	}
 
-	/// Return bucket ID corresponding to a bucket name from the perspective of
-	/// a given access key.
+	/// Return bucket corresponding to a bucket name from the perspective of
+	/// a given access key, if it exists (and is not a tombstone entry).
 	///
 	/// The name can be of three forms:
 	/// 1. A global bucket alias
 	/// 2. A local bucket alias
 	/// 3. The full ID of a bucket encoded in hex
 	///
-	/// This will not do any network interaction to check the alias table,
-	/// it will only check the local copy of the table.
-	/// As a consequence, it does not conserve read-after-write guarantees.
+	/// This will not do any network interaction, it will only check the local
+	/// copy of the bucket and global alias table. It will also resolve local
+	/// aliases directly using the data provided in the `api_key` parameter.
+	/// As a consequence, it does not provide read-after-write guarantees.
 	///
-	/// This function transforms non-existing buckets in a NoSuchBucket error.
+	/// In case no such bucket is found, this function returns a NoSuchBucket error.
 	#[allow(clippy::ptr_arg)]
 	pub fn resolve_bucket_fast(
 		&self,
@@ -109,8 +108,8 @@ impl<'a> BucketHelper<'a> {
 	//      for admin operations.
 	// ================
 
-	/// See resolve_global_bucket_fast,
-	/// but this one does a quorum read to ensure consistency
+	/// This is the same as `resolve_global_bucket_fast`,
+	/// except that it does quorum reads to ensure consistency.
 	pub async fn resolve_global_bucket(
 		&self,
 		bucket_name: &String,
@@ -141,8 +140,14 @@ impl<'a> BucketHelper<'a> {
 			.filter(|x| !x.state.is_deleted()))
 	}
 
-	/// See resolve_bucket_fast, but this one does a quorum read to ensure consistency.
-	/// Also, this function does not return a HelperError::NoSuchBucket if bucket is absent.
+	/// Return bucket corresponding to a bucket name from the perspective of
+	/// a given access key, if it exists (and is not a tombstone entry).
+	///
+	/// This is the same as `resolve_bucket_fast`, with the following differences:
+	///
+	/// - this function does quorum reads to ensure consistency.
+	/// - this function fetches the Key entry from the key table to ensure up-to-date data
+	/// - this function returns None if the bucket is not found, instead of HelperError::NoSuchBucket
 	#[allow(clippy::ptr_arg)]
 	pub async fn resolve_bucket(
 		&self,
@@ -176,7 +181,7 @@ impl<'a> BucketHelper<'a> {
 	/// Returns a Bucket if it is present in bucket table,
 	/// even if it is in deleted state. Querying a non-existing
 	/// bucket ID returns an internal error.
-	pub async fn get_internal_bucket(&self, bucket_id: Uuid) -> Result<Bucket, Error> {
+	pub(crate) async fn get_internal_bucket(&self, bucket_id: Uuid) -> Result<Bucket, Error> {
 		Ok(self
 			.0
 			.bucket_table