Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,27 @@ impl FeatureSetRepository for SqliteFeatureSetRepository {
anyhow::bail!("Cannot delete builtin FeatureSet: {}", id);
}

// Soft delete
conn.execute(
// Soft delete, and drop every reference to it in the same transaction.
// FeatureSets are soft-deleted (`is_deleted = 1`), so the FK
// `ON DELETE CASCADE` on the junction / grants never fires — a
// workspace binding or client grant would keep pointing at a
// FeatureSet that `get()` now reports as missing ("Feature set not
// found"). Prune those references explicitly so the deletion is fully
// reflected.
let tx = conn.unchecked_transaction()?;
tx.execute(
"UPDATE feature_sets SET is_deleted = 1, updated_at = datetime('now') WHERE id = ?",
params![id],
)?;
tx.execute(
"DELETE FROM workspace_binding_feature_sets WHERE feature_set_id = ?",
params![id],
)?;
tx.execute(
"DELETE FROM client_grants WHERE feature_set_id = ?",
params![id],
)?;
tx.commit()?;

Ok(())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,20 @@ impl SqliteWorkspaceBindingRepository {
let placeholders = std::iter::repeat_n("?", binding_ids.len())
.collect::<Vec<_>>()
.join(", ");
// Join `feature_sets` and skip soft-deleted ones: deleting a FeatureSet
// is a soft delete (`is_deleted = 1`), so the FK `ON DELETE CASCADE` on
// this junction never fires and the binding would otherwise keep
// pointing at a FeatureSet that `get()` now reports as missing
// ("Feature set not found"). Filtering here keeps a binding's
// `feature_set_ids` consistent with what actually resolves — and fixes
// bindings that were orphaned before delete-time cleanup existed.
let sql = format!(
"SELECT binding_id, feature_set_id
FROM workspace_binding_feature_sets
WHERE binding_id IN ({placeholders})
ORDER BY binding_id, sort_order, feature_set_id"
"SELECT wbfs.binding_id, wbfs.feature_set_id
FROM workspace_binding_feature_sets wbfs
JOIN feature_sets fs ON fs.id = wbfs.feature_set_id
WHERE wbfs.binding_id IN ({placeholders})
AND fs.is_deleted = 0
ORDER BY wbfs.binding_id, wbfs.sort_order, wbfs.feature_set_id"
);
let mut stmt = conn.prepare(&sql)?;
let params_dyn: Vec<&dyn rusqlite::ToSql> = binding_ids
Expand Down Expand Up @@ -376,6 +385,60 @@ mod tests {
assert_eq!(after.feature_set_ids, vec![fs_id2]);
}

async fn soft_delete_fs(db: &Arc<Mutex<Database>>, fs_id: &str) {
let guard = db.lock().await;
guard
.connection()
.execute(
"UPDATE feature_sets SET is_deleted = 1 WHERE id = ?",
params![fs_id],
)
.unwrap();
}

#[tokio::test]
async fn test_soft_deleted_feature_set_is_dropped_from_binding() {
// Regression: a FeatureSet is soft-deleted, so the junction's FK
// ON DELETE CASCADE never fires. The binding must NOT keep reporting
// the deleted FS (which `get()` now treats as missing → "Feature set
// not found"); only the live one(s) come back.
let (repo, space_id, fs_id1) = fixture().await;
let db = repo.db.clone();
let fs_id2 = add_fs(&db, space_id, "second").await;

let root = if cfg!(windows) { "d:\\del" } else { "/del" };
let binding =
WorkspaceBinding::new_multi(root, space_id, vec![fs_id1.clone(), fs_id2.clone()]);
repo.create(&binding).await.unwrap();
assert_eq!(
repo.get(&binding.id)
.await
.unwrap()
.unwrap()
.feature_set_ids,
vec![fs_id1.clone(), fs_id2.clone()]
);

soft_delete_fs(&db, &fs_id1).await;

// get(), list() and find_exact_for_roots() all drop the deleted FS.
let got = repo.get(&binding.id).await.unwrap().unwrap();
assert_eq!(got.feature_set_ids, vec![fs_id2.clone()]);
let listed = repo.list().await.unwrap();
assert_eq!(listed[0].feature_set_ids, vec![fs_id2.clone()]);
let matched = repo
.find_exact_for_roots(&[root.to_string()])
.await
.unwrap()
.unwrap();
assert_eq!(matched.feature_set_ids, vec![fs_id2.clone()]);

// Deleting the remaining FS leaves an (empty) binding, not an error.
soft_delete_fs(&db, &fs_id2).await;
let empty = repo.get(&binding.id).await.unwrap().unwrap();
assert!(empty.feature_set_ids.is_empty());
}

#[tokio::test]
async fn test_create_allows_empty_fs_list() {
// An empty feature_set_ids is a valid "no Space tools" mapping — the
Expand Down
28 changes: 28 additions & 0 deletions tests/rust/tests/integration/feature_set_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,34 @@ async fn binding_routes_to_its_target_space_and_fs() {
assert_eq!(r.feature_set_ids, vec![f.fs_a_id]);
}

#[tokio::test]
async fn deleting_a_bound_feature_set_drops_it_and_resolution_survives() {
// Repro of the "Feature set not found" report: a folder mapped to two
// FeatureSets; deleting one must drop it from the binding (FeatureSets are
// soft-deleted, so the FK ON DELETE CASCADE can't fire) and resolution must
// keep working — routing to the survivor, not erroring on the missing one.
let f = Fixture::new().await;
let binding = WorkspaceBinding::new_multi(
normalize_workspace_root(test_root()),
f.space_id,
vec![f.fs_a_id.clone(), f.fs_b_id.clone()],
);
f.binding_repo.create(&binding).await.unwrap();

f.fs_repo.delete(&f.fs_a_id).await.unwrap();

// The binding no longer references the deleted FS...
let reloaded = f.binding_repo.get(&binding.id).await.unwrap().unwrap();
assert_eq!(reloaded.feature_set_ids, vec![f.fs_b_id.clone()]);

// ...and the resolver routes via the binding to just the survivor.
f.session_roots.set("s", [test_root()]);
f.session_roots.set_roots_capable("s", true);
let r = f.resolver.resolve(Some("s"), None).await.unwrap();
assert_eq!(r.source, ResolutionSource::WorkspaceBinding);
assert_eq!(r.feature_set_ids, vec![f.fs_b_id.clone()]);
}

#[tokio::test]
async fn no_inheritance_child_of_bound_parent_falls_back_to_default() {
// Inheritance is intentionally NOT supported: a session whose reported root
Expand Down
Loading