Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions lore-server/src/grpc/thinclient/v1/revision_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ pub async fn handler(
let signature =
resolve_signature(&repository, query.into(), history_step_size, acceleration)
.await?;
// An existing branch with no revisions resolves its latest to the
// zero hash. Surface that as NotFound rather than proceeding into
// state/metadata loads that fail as Internal.
if signature.is_zero() {
return Err(Status::not_found("Branch has no revisions"));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is only the default branch that can have a zero revision - and in that case I would argue that it should early respond with a successful response carrying that zero information

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

}
debug!({REVISION} = %signature, "Loading revision info");

let revision = load_revision(&repository, signature).await?;
Expand Down Expand Up @@ -675,6 +681,79 @@ mod test {
.await;
}

#[tokio::test]
async fn empty_branch_returns_not_found() {
let repository = random::<RepositoryId>();
let (immutable_store, mutable_store, execution) =
test_store_create().await.expect("Failed to create stores");

// A branch that exists but has no revisions resolves its latest to
// the zero hash; the handler must answer NotFound, not Internal
// (regression: empty repos surfaced "file not found: metadata key").
Box::pin(LORE_CONTEXT.scope(execution, async move {
let repository_context = Arc::new(RepositoryContext::new_server_context(
immutable_store.clone(),
mutable_store.clone(),
repository,
));
let write_token = get_write_token();
let branch_id = BranchId::from(uuid::Uuid::now_v7());
branch::create(
repository_context,
&write_token,
branch_id,
"empty-branch",
branch::default_category(),
"creator",
1,
vec![],
false,
false,
)
.await
.expect("create branch");

let err = handler(
make_request(
repository,
Query::Identifier(model_v1::RevisionIdentifier {
branch_id: branch_id.into(),
number: 0,
}),
),
immutable_store,
mutable_store,
DEFAULT_HISTORY_STEP_SIZE,
RevisionListAcceleration::default(),
)
.await
.expect_err("empty branch should fail");
assert_eq!(err.code(), tonic::Code::NotFound);
}))
.await;
}

#[tokio::test]
async fn zero_signature_returns_not_found() {
let repository = random::<RepositoryId>();
let (immutable_store, mutable_store, execution) =
test_store_create().await.expect("Failed to create stores");

Box::pin(LORE_CONTEXT.scope(execution, async move {
let err = handler(
make_request(repository, Query::Signature(Hash::default().into())),
immutable_store,
mutable_store,
DEFAULT_HISTORY_STEP_SIZE,
RevisionListAcceleration::default(),
)
.await
.expect_err("zero signature should fail");
assert_eq!(err.code(), tonic::Code::NotFound);
}))
.await;
}

#[tokio::test]
async fn unknown_signature_returns_not_found() {
let repository = random::<RepositoryId>();
Expand Down
Loading