diff --git a/rust/rhgitaly/proto/ref.proto b/rust/rhgitaly/proto/ref.proto index 8d9351c773c2100de4bfaffaa2164b3600e91829_cnVzdC9yaGdpdGFseS9wcm90by9yZWYucHJvdG8=..27a7fd0ea4aaa0834c3ad50198bf8a658721c031_cnVzdC9yaGdpdGFseS9wcm90by9yZWYucHJvdG8= 100644 --- a/rust/rhgitaly/proto/ref.proto +++ b/rust/rhgitaly/proto/ref.proto @@ -26,6 +26,15 @@ }; } + // FindTag looks up a tag by its name and returns it to the caller if it exists. This RPC supports + // both lightweight and annotated tags. Note: this RPC returns an `Internal` error if the tag was + // not found. + rpc FindTag(FindTagRequest) returns (FindTagResponse) { + option (op_type) = { + op: ACCESSOR + }; + } + // RefExists checks if the specified reference exists. The reference must be fully qualified. rpc RefExists(RefExistsRequest) returns (RefExistsResponse) { option (op_type) = { diff --git a/rust/rhgitaly/src/errors.rs b/rust/rhgitaly/src/errors.rs index 8d9351c773c2100de4bfaffaa2164b3600e91829_cnVzdC9yaGdpdGFseS9zcmMvZXJyb3JzLnJz..27a7fd0ea4aaa0834c3ad50198bf8a658721c031_cnVzdC9yaGdpdGFseS9zcmMvZXJyb3JzLnJz 100644 --- a/rust/rhgitaly/src/errors.rs +++ b/rust/rhgitaly/src/errors.rs @@ -90,7 +90,7 @@ /// conversion to numeric value to the small conveniences for the callers. pub mod error_messages { pub use crate::gitaly::path_error::ErrorType as PathErrorType; - use crate::gitaly::{PathError, ResolveRevisionError}; + use crate::gitaly::{PathError, ReferenceNotFoundError, ResolveRevisionError}; pub trait FromPathError: Sized { fn from_path_error(err: PathError) -> Self; @@ -110,6 +110,14 @@ Self::from_resolve_revision_error(ResolveRevisionError { revision }) } } + + pub trait FromReferenceNotFoundError: Sized { + fn from_reference_not_found_error(err: ReferenceNotFoundError) -> Self; + + fn reference_not_found_error(reference_name: Vec<u8>) -> Self { + Self::from_reference_not_found_error(ReferenceNotFoundError { reference_name }) + } + } } pub use error_messages::*; diff --git a/rust/rhgitaly/src/service/ref.rs b/rust/rhgitaly/src/service/ref.rs index 8d9351c773c2100de4bfaffaa2164b3600e91829_cnVzdC9yaGdpdGFseS9zcmMvc2VydmljZS9yZWYucnM=..27a7fd0ea4aaa0834c3ad50198bf8a658721c031_cnVzdC9yaGdpdGFseS9zcmMvc2VydmljZS9yZWYucnM= 100644 --- a/rust/rhgitaly/src/service/ref.rs +++ b/rust/rhgitaly/src/service/ref.rs @@ -6,6 +6,6 @@ use std::fmt::{Debug, Formatter}; use std::string::String; use std::sync::Arc; -use tonic::{metadata::Ascii, metadata::MetadataValue, Request, Response, Status}; +use tonic::{metadata::Ascii, metadata::MetadataValue, Code, Request, Response, Status}; use tracing::{field, info, instrument}; @@ -10,3 +10,5 @@ use tracing::{field, info, instrument}; +use hg::revlog::RevlogError; + use crate::config::Config; @@ -12,3 +14,4 @@ use crate::config::Config; +use crate::errors::{status_with_structured_error, FromReferenceNotFoundError}; use crate::gitaly::ref_service_server::{RefService, RefServiceServer}; use crate::gitaly::{ @@ -13,5 +16,6 @@ use crate::gitaly::ref_service_server::{RefService, RefServiceServer}; use crate::gitaly::{ - FindDefaultBranchNameRequest, FindDefaultBranchNameResponse, RefExistsRequest, - RefExistsResponse, + find_tag_error, FindDefaultBranchNameRequest, FindDefaultBranchNameResponse, FindTagError, + FindTagRequest, FindTagResponse, RefExistsRequest, RefExistsResponse, ReferenceNotFoundError, + Repository, Tag, }; @@ -17,3 +21,2 @@ }; -use crate::gitlab::gitlab_branch_ref; use crate::gitlab::revision::{existing_default_gitlab_branch, map_full_ref, RefError}; @@ -19,3 +22,6 @@ use crate::gitlab::revision::{existing_default_gitlab_branch, map_full_ref, RefError}; +use crate::gitlab::state::{lookup_typed_ref_as_node, stream_gitlab_tags}; +use crate::gitlab::{gitlab_branch_ref, gitlab_tag_ref}; +use crate::message; use crate::metadata::correlation_id; use crate::repository::{default_repo_spec_error_status, repo_store_vfs}; @@ -20,8 +26,9 @@ use crate::metadata::correlation_id; use crate::repository::{default_repo_spec_error_status, repo_store_vfs}; +use crate::repository::{load_repo_and_then, RequestWithRepo}; #[derive(Debug)] pub struct RefServiceImpl { config: Arc<Config>, } @@ -22,9 +29,23 @@ #[derive(Debug)] pub struct RefServiceImpl { config: Arc<Config>, } +impl RequestWithRepo for FindTagRequest { + fn repository_ref(&self) -> Option<&Repository> { + self.repository.as_ref() + } +} + +impl FromReferenceNotFoundError for FindTagError { + fn from_reference_not_found_error(err: ReferenceNotFoundError) -> Self { + FindTagError { + error: Some(find_tag_error::Error::TagNotFound(err)), + } + } +} + #[tonic::async_trait] impl RefService for RefServiceImpl { async fn find_default_branch_name( @@ -45,6 +66,14 @@ self.inner_ref_exists(inner, correlation_id(&metadata)) .await } + async fn find_tag( + &self, + request: Request<FindTagRequest>, + ) -> Result<Response<FindTagResponse>, Status> { + let (metadata, _ext, inner) = request.into_parts(); + + self.inner_find_tag(inner, correlation_id(&metadata)).await + } } struct RefExistsTracingRequest<'a>(&'a RefExistsRequest); @@ -58,6 +87,22 @@ } } +struct FindTagTracingRequest<'a>(&'a FindTagRequest); + +impl Debug for FindTagTracingRequest<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RefExistRequest") + .field("repository", &self.0.repository) + .field("tag_name", &String::from_utf8_lossy(&self.0.tag_name)) + .finish() + } +} + +impl prost::Name for FindTagError { + const NAME: &'static str = "FindTagError"; + const PACKAGE: &'static str = "gitaly"; +} + impl RefServiceImpl { #[instrument(name = "find_default_branch_name", skip(self))] async fn inner_find_default_branch_name( @@ -109,6 +154,73 @@ Ok(Response::new(RefExistsResponse { value })) } + + #[instrument(name="find_tag", + skip(self, request), + fields(request=field::debug(FindTagTracingRequest(&request))))] + async fn inner_find_tag( + &self, + request: FindTagRequest, + correlation_id: Option<&MetadataValue<Ascii>>, + ) -> Result<Response<FindTagResponse>, Status> { + info!("Processing"); + + let store_vfs = repo_store_vfs(&self.config, &request.repository) + .await + .map_err(default_repo_spec_error_status)?; + + let tag_name = request.tag_name.clone(); + let commit = + match lookup_typed_ref_as_node(stream_gitlab_tags(&store_vfs).await?, &tag_name) + .await + .map_err(|e| Status::internal(format!("GitLab state file error: {:?}", e)))? + { + None => { + return Err(status_with_structured_error( + Code::NotFound, + "tag does not exist", + FindTagError::reference_not_found_error(gitlab_tag_ref(&tag_name)), + )); + } + Some(node) => { + // TODO totally duplicated from FindCommit. Find a way to make a helper! + load_repo_and_then( + self.config.clone(), + request, + default_repo_spec_error_status, + move |_req, repo| { + let cl = repo.changelog().map_err(|e| { + Status::internal(format!("Could not open changelog: {:?}", e)) + })?; + + match message::commit_for_node_prefix(&cl, node.into()) { + // TODO discuss upstream: InvalidRevision is incorrect wording: + // it does represent both invalid input (working dir Node), + // and not found node prefixes. The latter case justifies our + // choice. + // Note that at this stage we cannot tell apart NodePrefix due to + // caller passing a hash from those read in state files (would be an + // inconsistency for them not to resolve). + Err(RevlogError::InvalidRevision) => Ok(None), + Err(e) => { + Err(Status::internal(format!("Repository corruption {:?}", e))) + } + Ok(r) => Ok(r), + } + }, + ) + .await + } + } + .unwrap(); // TODO unwrap + Ok(Response::new(FindTagResponse { + tag: Some(Tag { + name: tag_name, + target_commit: commit, + ..Default::default() + }), + })) + } } /// Takes care of boilerplate that would instead be in the startup sequence. diff --git a/tests_with_gitaly/test_ref.py b/tests_with_gitaly/test_ref.py index 8d9351c773c2100de4bfaffaa2164b3600e91829_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZWYucHk=..27a7fd0ea4aaa0834c3ad50198bf8a658721c031_dGVzdHNfd2l0aF9naXRhbHkvdGVzdF9yZWYucHk= 100644 --- a/tests_with_gitaly/test_ref.py +++ b/tests_with_gitaly/test_ref.py @@ -232,8 +232,9 @@ assert_compare(10, sort_by=sort_by) -def test_find_tag(gitaly_comparison): - fixture = gitaly_comparison +@parametrize('hg_server', ('hgitaly', 'rhgitaly')) +def test_find_tag(gitaly_rhgitaly_comparison, hg_server): + fixture = gitaly_rhgitaly_comparison hg_wrapper = fixture.hg_repo_wrapper hg_wrapper.commit_file('foo') @@ -250,6 +251,7 @@ resp.tag.id = '' rpc_helper = fixture.rpc_helper( + hg_server=hg_server, stub_cls=RefServiceStub, method_name='FindTag', response_sha_attrs=['tag.target_commit.id'],