Report not-found when AddChildVersion would succeed. (#48)

Previously, if there was no latest version (the latest version is nil),
this would return GONE when in fact AddChildVersion would succeed.
This commit is contained in:
Dustin J. Mitchell 2024-10-30 20:14:44 -04:00 committed by GitHub
parent 526b37f6e5
commit 1d6abd00ec
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 49 additions and 47 deletions

View file

@ -131,20 +131,39 @@ mod test {
#[actix_rt::test]
async fn test_version_not_found_and_gone() {
let client_id = Uuid::new_v4();
let parent_version_id = Uuid::new_v4();
let test_version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
// create the client, but not the version
// create the client and a single version.
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_id, Uuid::new_v4()).unwrap();
txn.add_version(client_id, test_version_id, NIL_VERSION_ID, b"vers".to_vec())
.unwrap();
}
let server = Server::new(Default::default(), storage);
let app = App::new().configure(|sc| server.config(sc));
let app = test::init_service(app).await;
// the child of an unknown parent_version_id is GONE
let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
// the child of the nil version is the added version
let uri = format!("/v1/client/get-child-version/{}", NIL_VERSION_ID);
let req = test::TestRequest::get()
.uri(&uri)
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&app, req).await;
assert_eq!(resp.status(), StatusCode::OK);
assert_eq!(
resp.headers().get("X-Version-Id").unwrap(),
&test_version_id.to_string(),
);
assert_eq!(
resp.headers().get("X-Parent-Version-Id").unwrap(),
&NIL_VERSION_ID.to_string(),
);
// the child of an unknown parent_version_id is GONE.
let uri = format!("/v1/client/get-child-version/{}", Uuid::new_v4());
let req = test::TestRequest::get()
.uri(&uri)
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
@ -154,10 +173,9 @@ mod test {
assert_eq!(resp.headers().get("X-Version-Id"), None);
assert_eq!(resp.headers().get("X-Parent-Version-Id"), None);
// but the child of the nil parent_version_id is NOT FOUND, since
// there is no snapshot. The tests in crate::server test more
// The child of the latest version is NOT_FOUND. The tests in crate::server test more
// corner cases.
let uri = format!("/v1/client/get-child-version/{}", NIL_VERSION_ID);
let uri = format!("/v1/client/get-child-version/{}", test_version_id);
let req = test::TestRequest::get()
.uri(&uri)
.append_header((CLIENT_ID_HEADER, client_id.to_string()))

View file

@ -73,25 +73,17 @@ pub(crate) fn get_child_version<'a>(
});
}
// If the requested parentVersionId is the nil UUID ..
if parent_version_id == NIL_VERSION_ID {
return Ok(match client.snapshot {
// ..and snapshotVersionId is nil, the response is _not-found_ (the client has no
// versions).
None => GetVersionResult::NotFound,
// ..and snapshotVersionId is not nil, the response is _gone_ (the first version has
// been deleted).
Some(_) => GetVersionResult::Gone,
});
}
// If a version with versionId equal to the requested parentVersionId exists, the response is _not-found_ (the client is up-to-date)
if txn.get_version(client_id, parent_version_id)?.is_some() {
return Ok(GetVersionResult::NotFound);
}
// Otherwise, the response is _gone_ (the requested version has been deleted).
Ok(GetVersionResult::Gone)
// Return NotFound if an AddVersion with this parent_version_id would succeed, and otherwise
// return Gone.
//
// AddVersion will succeed if either
// - the requested parent version is the latest version; or
// - there is no latest version, meaning there are no versions stored for this client
Ok(if client.latest_version_id == parent_version_id || client.latest_version_id == NIL_VERSION_ID {
GetVersionResult::NotFound
} else {
GetVersionResult::Gone
})
}
/// Response to add_version
@ -349,7 +341,7 @@ mod test {
}
#[test]
fn get_child_version_not_found_initial() -> anyhow::Result<()> {
fn get_child_version_not_found_initial_nil() -> anyhow::Result<()> {
init_logging();
let storage = InMemoryStorage::new();
@ -357,7 +349,7 @@ mod test {
let client_id = Uuid::new_v4();
txn.new_client(client_id, NIL_VERSION_ID)?;
// when no snapshot exists, the first version is NotFound
// when no latest version exists, the first version is NotFound
let client = txn.get_client(client_id)?.unwrap();
assert_eq!(
get_child_version(
@ -373,25 +365,17 @@ mod test {
}
#[test]
fn get_child_version_gone_initial() -> anyhow::Result<()> {
fn get_child_version_not_found_initial_continuing() -> anyhow::Result<()> {
init_logging();
let storage = InMemoryStorage::new();
let mut txn = storage.txn()?;
let client_id = Uuid::new_v4();
txn.new_client(client_id, Uuid::new_v4())?;
txn.set_snapshot(
client_id,
Snapshot {
version_id: Uuid::new_v4(),
versions_since: 0,
timestamp: Utc.ymd(2001, 9, 9).and_hms(1, 46, 40),
},
vec![1, 2, 3],
)?;
txn.new_client(client_id, NIL_VERSION_ID)?;
// when a snapshot exists, the first version is GONE
// when no latest version exists, _any_ child version is NOT_FOUND. This allows syncs to
// start to a new server even if the client already has been uploading to another service.
let client = txn.get_client(client_id)?.unwrap();
assert_eq!(
get_child_version(
@ -399,9 +383,9 @@ mod test {
&ServerConfig::default(),
client_id,
client,
NIL_VERSION_ID
Uuid::new_v4(),
)?,
GetVersionResult::Gone
GetVersionResult::NotFound
);
Ok(())
}
@ -434,17 +418,17 @@ mod test {
}
#[test]
fn get_child_version_gone() -> anyhow::Result<()> {
fn get_child_version_gone_not_latest() -> anyhow::Result<()> {
init_logging();
let storage = InMemoryStorage::new();
let mut txn = storage.txn()?;
let client_id = Uuid::new_v4();
// make up a parent version id, but neither that version
// nor its child exists (presumed to have been deleted)
// Add a parent version, but not the requested parent version
let parent_version_id = Uuid::new_v4();
txn.new_client(client_id, Uuid::new_v4())?;
txn.new_client(client_id, parent_version_id)?;
txn.add_version(client_id, parent_version_id, NIL_VERSION_ID, vec![])?;
let client = txn.get_client(client_id)?.unwrap();
assert_eq!(
@ -453,7 +437,7 @@ mod test {
&ServerConfig::default(),
client_id,
client,
parent_version_id
Uuid::new_v4(),
)?,
GetVersionResult::Gone
);