| 857 |
857 |
|
"idempotent re-confirm must not delete the live object"
|
| 858 |
858 |
|
);
|
| 859 |
859 |
|
}
|
|
860 |
+ |
|
|
861 |
+ |
// ---------------------------------------------------------------------------
|
|
862 |
+ |
// CAS guard pins (test-fuzz Phase 2.2)
|
|
863 |
+ |
//
|
|
864 |
+ |
// The Run #9 tx port sealed every confirm-upload write behind an
|
|
865 |
+ |
// `IS NOT DISTINCT FROM expected_old` compare-and-swap so a confirm that lost a
|
|
866 |
+ |
// concurrent race (or whose target row was deleted/transferred mid-flight)
|
|
867 |
+ |
// matches zero rows and the surrounding tx rolls back — never double-crediting
|
|
868 |
+ |
// storage and never clobbering the live object the winning confirm published.
|
|
869 |
+ |
//
|
|
870 |
+ |
// The handler-level rollback + orphan-queue wiring on that branch can only fire
|
|
871 |
+ |
// under TRUE concurrency (a sequential request always observes its own read, so
|
|
872 |
+ |
// its CAS always matches — see the comment at uploads.rs around the confirm tx).
|
|
873 |
+ |
// What IS deterministic, and what these pin, is the CAS predicate itself: feed
|
|
874 |
+ |
// the db function a stale `expected_old` and assert it (a) reports the lost-race
|
|
875 |
+ |
// outcome and (b) leaves the row untouched. A regression that dropped the guard
|
|
876 |
+ |
// would make the stale write land here.
|
|
877 |
+ |
// ---------------------------------------------------------------------------
|
|
878 |
+ |
|
|
879 |
+ |
#[tokio::test]
|
|
880 |
+ |
async fn update_item_file_cas_guards_against_stale_confirm() {
|
|
881 |
+ |
use makenotwork::db::items::{update_item_file_cas, FileConfirmOutcome};
|
|
882 |
+ |
use makenotwork::db::{ItemId, UserId};
|
|
883 |
+ |
use makenotwork::storage::FileType;
|
|
884 |
+ |
|
|
885 |
+ |
let mut h = TestHarness::with_storage().await;
|
|
886 |
+ |
let (user_id, _project_id, item_id) = setup_creator_with_item(&mut h, 0).await;
|
|
887 |
+ |
let item = ItemId::from(uuid::Uuid::parse_str(&item_id).unwrap());
|
|
888 |
+ |
let owner = UserId::from(uuid::Uuid::parse_str(&user_id).unwrap());
|
|
889 |
+ |
|
|
890 |
+ |
// audio_s3_key is NULL, so the first confirm (expected_old = None) wins.
|
|
891 |
+ |
let r = update_item_file_cas(&h.db, item, owner, FileType::Audio, None, "key_v1", 1000)
|
|
892 |
+ |
.await
|
|
893 |
+ |
.unwrap();
|
|
894 |
+ |
assert!(matches!(r, FileConfirmOutcome::Committed));
|
|
895 |
+ |
|
|
896 |
+ |
// A second confirm that still observed the pre-state (None) loses: the row
|
|
897 |
+ |
// now holds key_v1, so `IS NOT DISTINCT FROM NULL` matches zero rows.
|
|
898 |
+ |
let r = update_item_file_cas(&h.db, item, owner, FileType::Audio, None, "key_v2", 2000)
|
|
899 |
+ |
.await
|
|
900 |
+ |
.unwrap();
|
|
901 |
+ |
assert!(matches!(r, FileConfirmOutcome::LostRace), "stale-None confirm must lose the CAS race");
|
|
902 |
+ |
|
|
903 |
+ |
// The loser left the row untouched — key_v1/1000, not key_v2 (no clobber, no double-credit).
|
|
904 |
+ |
let (k, sz): (Option<String>, Option<i64>) = sqlx::query_as(
|
|
905 |
+ |
"SELECT audio_s3_key, audio_file_size_bytes FROM items WHERE id = $1::uuid",
|
|
906 |
+ |
)
|
|
907 |
+ |
.bind(&item_id)
|
|
908 |
+ |
.fetch_one(&h.db)
|
|
909 |
+ |
.await
|
|
910 |
+ |
.unwrap();
|
|
911 |
+ |
assert_eq!(k.as_deref(), Some("key_v1"));
|
|
912 |
+ |
assert_eq!(sz, Some(1000));
|
|
913 |
+ |
|
|
914 |
+ |
// A correct CAS (expected_old = the current key) commits the swap.
|
|
915 |
+ |
let r = update_item_file_cas(&h.db, item, owner, FileType::Audio, Some("key_v1"), "key_v3", 3000)
|
|
916 |
+ |
.await
|
|
917 |
+ |
.unwrap();
|
|
918 |
+ |
assert!(matches!(r, FileConfirmOutcome::Committed));
|
|
919 |
+ |
|
|
920 |
+ |
// The ownership filter is part of the same predicate: a non-owner never
|
|
921 |
+ |
// matches, even with the correct expected_old.
|
|
922 |
+ |
let stranger = UserId::new();
|
|
923 |
+ |
let r = update_item_file_cas(&h.db, item, stranger, FileType::Audio, Some("key_v3"), "key_v4", 4000)
|
|
924 |
+ |
.await
|
|
925 |
+ |
.unwrap();
|
|
926 |
+ |
assert!(matches!(r, FileConfirmOutcome::LostRace), "a non-owner must not write the item file");
|
|
927 |
+ |
let k: Option<String> = sqlx::query_scalar("SELECT audio_s3_key FROM items WHERE id = $1::uuid")
|
|
928 |
+ |
.bind(&item_id)
|
|
929 |
+ |
.fetch_one(&h.db)
|
|
930 |
+ |
.await
|
|
931 |
+ |
.unwrap();
|
|
932 |
+ |
assert_eq!(k.as_deref(), Some("key_v3"), "the non-owner write must not land");
|
|
933 |
+ |
}
|
|
934 |
+ |
|
|
935 |
+ |
#[tokio::test]
|
|
936 |
+ |
async fn update_version_file_guards_against_stale_confirm() {
|
|
937 |
+ |
use makenotwork::db::versions::update_version_file;
|
|
938 |
+ |
use makenotwork::db::VersionId;
|
|
939 |
+ |
|
|
940 |
+ |
let mut h = TestHarness::with_storage().await;
|
|
941 |
+ |
let (_user, _proj, item_id) = setup_creator_with_item(&mut h, 0).await;
|
|
942 |
+ |
|
|
943 |
+ |
let resp = h
|
|
944 |
+ |
.client
|
|
945 |
+ |
.post_json(
|
|
946 |
+ |
&format!("/api/items/{}/versions", item_id),
|
|
947 |
+ |
&json!({"version_number": "1.0.0"}).to_string(),
|
|
948 |
+ |
)
|
|
949 |
+ |
.await;
|
|
950 |
+ |
assert!(resp.status.is_success(), "create version failed: {}", resp.text);
|
|
951 |
+ |
let v: Value = resp.json();
|
|
952 |
+ |
let version_id = v["id"].as_str().unwrap().to_string();
|
|
953 |
+ |
let vid = VersionId::from(uuid::Uuid::parse_str(&version_id).unwrap());
|
|
954 |
+ |
|
|
955 |
+ |
// s3_key is NULL initially. A confirm carrying a stale non-null expected key
|
|
956 |
+ |
// matches zero rows.
|
|
957 |
+ |
let r = update_version_file(&h.db, vid, Some("not_the_current_key"), "vk_v1", Some(1000), Some("a.zip"))
|
|
958 |
+ |
.await
|
|
959 |
+ |
.unwrap();
|
|
960 |
+ |
assert!(r.is_none(), "a stale expected_old key must match zero rows");
|
|
961 |
+ |
let k: Option<String> = sqlx::query_scalar("SELECT s3_key FROM versions WHERE id = $1::uuid")
|
|
962 |
+ |
.bind(&version_id)
|
|
963 |
+ |
.fetch_one(&h.db)
|
|
964 |
+ |
.await
|
|
965 |
+ |
.unwrap();
|
|
966 |
+ |
assert!(k.is_none(), "the row must stay unwritten after a lost CAS");
|
|
967 |
+ |
|
|
968 |
+ |
// Correct CAS (expected None) commits.
|
|
969 |
+ |
let r = update_version_file(&h.db, vid, None, "vk_v1", Some(1000), Some("a.zip"))
|
|
970 |
+ |
.await
|
|
971 |
+ |
.unwrap();
|
|
972 |
+ |
assert!(r.is_some());
|
|
973 |
+ |
|
|
974 |
+ |
// A replace that still observed the pre-state (None) loses to the committed key.
|
|
975 |
+ |
let r = update_version_file(&h.db, vid, None, "vk_v2", Some(2000), Some("b.zip"))
|
|
976 |
+ |
.await
|
|
977 |
+ |
.unwrap();
|
|
978 |
+ |
assert!(r.is_none(), "stale-None replace must lose once the key is set");
|
|
979 |
+ |
let (k, sz): (Option<String>, Option<i64>) = sqlx::query_as(
|
|
980 |
+ |
"SELECT s3_key, file_size_bytes FROM versions WHERE id = $1::uuid",
|
|
981 |
+ |
)
|
|
982 |
+ |
.bind(&version_id)
|
|
983 |
+ |
.fetch_one(&h.db)
|
|
984 |
+ |
.await
|
|
985 |
+ |
.unwrap();
|
|
986 |
+ |
assert_eq!(k.as_deref(), Some("vk_v1"));
|
|
987 |
+ |
assert_eq!(sz, Some(1000));
|
|
988 |
+ |
}
|
|
989 |
+ |
|
|
990 |
+ |
#[tokio::test]
|
|
991 |
+ |
async fn update_project_cover_cas_guards_against_stale_and_non_owner() {
|
|
992 |
+ |
use makenotwork::db::projects::update_project_cover_cas;
|
|
993 |
+ |
use makenotwork::db::{ProjectId, UserId};
|
|
994 |
+ |
|
|
995 |
+ |
let mut h = TestHarness::with_storage().await;
|
|
996 |
+ |
let (user_id, project_id, _item) = setup_creator_with_item(&mut h, 0).await;
|
|
997 |
+ |
let pid = ProjectId::from(uuid::Uuid::parse_str(&project_id).unwrap());
|
|
998 |
+ |
let owner = UserId::from(uuid::Uuid::parse_str(&user_id).unwrap());
|
|
999 |
+ |
|
|
1000 |
+ |
// cover_image_url is NULL. A stale expected url matches zero rows.
|
|
1001 |
+ |
let ok = update_project_cover_cas(&h.db, pid, owner, Some("stale_url"), "url_v1", 1000)
|
|
1002 |
+ |
.await
|
|
1003 |
+ |
.unwrap();
|
|
1004 |
+ |
assert!(!ok, "a stale expected url must match zero rows");
|
|
1005 |
+ |
|
|
1006 |
+ |
// Correct CAS (expected None) commits.
|
|
1007 |
+ |
let ok = update_project_cover_cas(&h.db, pid, owner, None, "url_v1", 1000).await.unwrap();
|
|
1008 |
+ |
assert!(ok);
|
|
1009 |
+ |
|
|
1010 |
+ |
// Stale-None loses now that the cover is set.
|
|
1011 |
+ |
let ok = update_project_cover_cas(&h.db, pid, owner, None, "url_v2", 2000).await.unwrap();
|
|
1012 |
+ |
assert!(!ok, "stale-None confirm must lose once the cover is set");
|
|
1013 |
+ |
|
|
1014 |
+ |
// A non-owner cannot write, even with the correct expected url.
|
|
1015 |
+ |
let stranger = UserId::new();
|
|
1016 |
+ |
let ok = update_project_cover_cas(&h.db, pid, stranger, Some("url_v1"), "url_v3", 3000)
|
|
1017 |
+ |
.await
|
|
1018 |
+ |
.unwrap();
|
|
1019 |
+ |
assert!(!ok, "a non-owner must not write the project cover");
|
|
1020 |
+ |
|
|
1021 |
+ |
let (url, sz): (Option<String>, Option<i64>) = sqlx::query_as(
|
|
1022 |
+ |
"SELECT cover_image_url, cover_image_size_bytes FROM projects WHERE id = $1::uuid",
|
|
1023 |
+ |
)
|
|
1024 |
+ |
.bind(&project_id)
|
|
1025 |
+ |
.fetch_one(&h.db)
|
|
1026 |
+ |
.await
|
|
1027 |
+ |
.unwrap();
|
|
1028 |
+ |
assert_eq!(url.as_deref(), Some("url_v1"));
|
|
1029 |
+ |
assert_eq!(sz, Some(1000));
|
|
1030 |
+ |
}
|
|
1031 |
+ |
|
|
1032 |
+ |
#[tokio::test]
|
|
1033 |
+ |
async fn update_item_cover_guards_against_stale_confirm() {
|
|
1034 |
+ |
use makenotwork::db::items::update_item_cover;
|
|
1035 |
+ |
use makenotwork::db::{ItemId, UserId};
|
|
1036 |
+ |
|
|
1037 |
+ |
let mut h = TestHarness::with_storage().await;
|
|
1038 |
+ |
let (user_id, _proj, item_id) = setup_creator_with_item(&mut h, 0).await;
|
|
1039 |
+ |
let item = ItemId::from(uuid::Uuid::parse_str(&item_id).unwrap());
|
|
1040 |
+ |
let owner = UserId::from(uuid::Uuid::parse_str(&user_id).unwrap());
|
|
1041 |
+ |
|
|
1042 |
+ |
// cover_s3_key is NULL. A stale expected key matches zero rows.
|
|
1043 |
+ |
let ok = update_item_cover(&h.db, item, owner, Some("stale_key"), "u1", "ck_v1", 1000)
|
|
1044 |
+ |
.await
|
|
1045 |
+ |
.unwrap();
|
|
1046 |
+ |
assert!(!ok, "a stale expected key must match zero rows");
|
|
1047 |
+ |
|
|
1048 |
+ |
// Correct CAS (expected None) commits the cover key + url + size together.
|
|
1049 |
+ |
let ok = update_item_cover(&h.db, item, owner, None, "u1", "ck_v1", 1000).await.unwrap();
|
|
1050 |
+ |
assert!(ok);
|
|
1051 |
+ |
|
|
1052 |
+ |
// Stale-None loses now that the cover key is set.
|
|
1053 |
+ |
let ok = update_item_cover(&h.db, item, owner, None, "u2", "ck_v2", 2000).await.unwrap();
|
|
1054 |
+ |
assert!(!ok, "stale-None confirm must lose once the cover key is set");
|
|
1055 |
+ |
|
|
1056 |
+ |
// A non-owner cannot write, even with the correct expected key.
|
|
1057 |
+ |
let stranger = UserId::new();
|
|
1058 |
+ |
let ok = update_item_cover(&h.db, item, stranger, Some("ck_v1"), "u3", "ck_v3", 3000)
|
|
1059 |
+ |
.await
|
|
1060 |
+ |
.unwrap();
|
|
1061 |
+ |
assert!(!ok, "a non-owner must not write the item cover");
|
|
1062 |
+ |
|
|
1063 |
+ |
let (key, url, sz): (Option<String>, Option<String>, Option<i64>) = sqlx::query_as(
|
|
1064 |
+ |
"SELECT cover_s3_key, cover_image_url, cover_file_size_bytes FROM items WHERE id = $1::uuid",
|
|
1065 |
+ |
)
|
|
1066 |
+ |
.bind(&item_id)
|
|
1067 |
+ |
.fetch_one(&h.db)
|
|
1068 |
+ |
.await
|
|
1069 |
+ |
.unwrap();
|
|
1070 |
+ |
assert_eq!(key.as_deref(), Some("ck_v1"));
|
|
1071 |
+ |
assert_eq!(url.as_deref(), Some("u1"));
|
|
1072 |
+ |
assert_eq!(sz, Some(1000));
|
|
1073 |
+ |
}
|