Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 4 additions & 1 deletion syncserver/src/server/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,10 @@ async fn put_bso() {
let bytes = test_endpoint_with_body(
http::Method::PUT,
"/1.5/42/storage/bookmarks/wibble",
json!(BsoBody::default()),
json!(BsoBody {
payload: Some("wibble".to_string()),
..Default::default()
}),
)
.await;
let result: PutBso = serde_json::from_slice(&bytes).expect("Could not get result in put_bso");
Expand Down
21 changes: 10 additions & 11 deletions syncstorage-db/src/tests/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1225,25 +1225,24 @@ async fn put_bso_sortindex_only_preserves_payload_link() -> Result<(), DbError>
.await
}

/// A metadata-only write (ttl/sortindex, no payload/link) to a *new* BSO
/// creates it with an empty inline payload — matching historical Sync
/// behavior — rather than a NULL payload that the read-path check would reject.
/// A metadata-only write (ttl/sortindex, no payload/link) to a *new* BSO is rejected.
#[cfg(feature = "spanner")]
#[tokio::test]
async fn put_bso_metadata_only_create_defaults_empty_payload() -> Result<(), DbError> {
async fn put_bso_metadata_only_create_rejected() -> Result<(), DbError> {
with_test_transaction(None, async |db: &mut dyn Db<Error = DbError>| {
let uid = *UID;
let coll = "clients";
let bid = "b0";

// No prior row: payload and payload_link both absent.
db.put_bso(pbso(uid, coll, bid, None, Some(3), Some(DEFAULT_BSO_TTL)))
.await?;

let got = db.get_bso(gbso(uid, coll, bid)).await?.unwrap();
assert_eq!(got.payload, "");
assert_eq!(got.payload_link, None);
assert_eq!(got.sortindex, Some(3));
let err = db
.put_bso(pbso(uid, coll, bid, None, Some(3), Some(DEFAULT_BSO_TTL)))
.await
.unwrap_err();
assert!(
err.to_string().contains("payload and payload_link"),
"unexpected error: {err}"
);
Ok(())
})
.await
Expand Down
21 changes: 17 additions & 4 deletions syncstorage-spanner/src/db/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ impl SpannerDb {
) -> DbResult<()> {
validate_payload_exclusive(bso.payload.as_ref(), bso.payload_link.as_ref())?;

let is_metadata_update = bso.payload.is_none() && bso.payload_link.is_none();
let has_payload_or_sortindex =
bso.payload.is_some() || bso.payload_link.is_some() || bso.sortindex.is_some();

Expand Down Expand Up @@ -655,15 +656,27 @@ impl SpannerDb {
AND fxa_kid = @fxa_kid
AND collection_id = @collection_id
AND bso_id = @bso_id
) AS existing ON TRUE"
) AS existing ON TRUE
THEN RETURN WITH ACTION AS action bso_id"
);

self.sql(&sql)
let mut result = self
.sql(&sql)
.await?
.params(sqlparams)
.param_types(sqlparam_types)
.execute_dml(&self.conn)
.await?;
.execute(&self.conn)?;
let is_insert = result
.one_or_none()
.await?
.is_some_and(|row| row[1].get_string_value() == "INSERT");

if is_insert && is_metadata_update {
return Err(DbError::integrity(
"a BSO write cannot leave both payload and payload_link empty".to_owned(),
));
}

Ok(())
}

Expand Down
13 changes: 3 additions & 10 deletions tools/integration_tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -1376,27 +1376,20 @@ def test_update_of_ttl_without_sending_data(st_ctx):
bso = {"payload": "x", "ttl": 1}
retry_put_json(app, root + "/storage/xxx_col2/TEST1", bso)
retry_put_json(app, root + "/storage/xxx_col2/TEST2", bso)
# Before those expire, update ttl on one that exists
# and on one that does not.
# Before those expire, update ttl.
time.sleep(0.2)
bso = {"ttl": 10}
retry_put_json(app, root + "/storage/xxx_col2/TEST2", bso)
retry_put_json(app, root + "/storage/xxx_col2/TEST3", bso)
# Update some other field on TEST1, which should leave ttl untouched.
bso = {"sortindex": 3}
retry_put_json(app, root + "/storage/xxx_col2/TEST1", bso)
# If we wait, TEST1 should expire but the others should not.
# If we wait, TEST1 should expire but TEST2 should not.
time.sleep(0.8)
items = app.get(root + "/storage/xxx_col2?full=1").json
items = dict((item["id"], item) for item in items)
assert sorted(list(items.keys())) == ["TEST2", "TEST3"]
assert sorted(list(items.keys())) == ["TEST2"]
# The existing item should have retained its payload.
# The new item should have got a default payload of empty string.
assert items["TEST2"]["payload"] == "x"
assert items["TEST3"]["payload"] == ""
ts2 = items["TEST2"]["modified"]
ts3 = items["TEST3"]["modified"]
assert ts2 < ts3


def test_bulk_update_of_ttls_without_sending_data(st_ctx):
Expand Down