diff --git a/.github/workflows/quick.yml b/.github/workflows/quick.yml index f527cbb1..a86d8d9e 100644 --- a/.github/workflows/quick.yml +++ b/.github/workflows/quick.yml @@ -23,6 +23,9 @@ jobs: - name: Block retired managed-library code and contracts run: python3 tests/test_v2_no_managed_library.py + - name: Run cheap C-source ACL and Branch retry contracts + run: python3 -m unittest discover -s tests -p 'test_cf_*.py' + - name: Validate only changed source shell: bash run: | diff --git a/common/cf-v2-acl.c b/common/cf-v2-acl.c index 6ddd412f..983eae29 100644 --- a/common/cf-v2-acl.c +++ b/common/cf-v2-acl.c @@ -3,9 +3,13 @@ #include "seafile-session.h" #include "repo-mgr.h" +/* Final ACL proof must use the (repo_id, path_hash, ...) PRIMARY prefix. + * The old OR/IN VirtualRepo query chose acl_shared_subject, scanning and + * X-locking unrelated libraries under REPEATABLE READ. No new index needed. */ static const char *query = - "SELECT 1 FROM cf_dir_acl WHERE repo_id=? OR repo_id IN " - "(SELECT origin_repo FROM VirtualRepo WHERE repo_id=?) LIMIT 1"; + "SELECT 1 FROM cf_dir_acl FORCE INDEX (PRIMARY) WHERE repo_id=? LIMIT 1"; +static const char *origin_query = + "SELECT origin_repo FROM VirtualRepo WHERE repo_id=?"; static gboolean found(SeafDBRow *row, void *data) { @@ -13,22 +17,57 @@ static gboolean found(SeafDBRow *row, void *data) return FALSE; } -static int check(SeafDBTrans *trans, struct _SeafileSession *session, const char *repo_id) +static gboolean origin_found(SeafDBRow *row, void *data) +{ + const char *origin = seaf_db_row_get_column_text(row, 0); + *(char **)data = g_strdup(origin); + return FALSE; +} + +static int check_scope(SeafDBTrans *trans, struct _SeafileSession *session, + const char *repo_id) { - if (!session || !session->db || !repo_id || - seaf_db_type(session->db) != SEAF_DB_TYPE_MYSQL) return -1; gboolean protected = FALSE; /* A prior barrier read may have established a REPEATABLE READ snapshot. * Use a locking/current read after Branch, never that older snapshot. */ char *current_query = trans ? g_strconcat(query, " FOR UPDATE", NULL) : NULL; int count = trans ? seaf_db_trans_foreach_selected_row(trans, current_query, found, &protected, - 2, "string", repo_id, "string", repo_id) : + 1, "string", repo_id) : seaf_db_statement_foreach_row(session->db, query, found, &protected, - 2, "string", repo_id, "string", repo_id); + 1, "string", repo_id); g_free(current_query); return count < 0 ? -1 : protected ? -2 : 0; } +static int check(SeafDBTrans *trans, struct _SeafileSession *session, const char *repo_id) +{ + if (!session || !session->db || !repo_id || + seaf_db_type(session->db) != SEAF_DB_TYPE_MYSQL) return -1; + + int result = check_scope(trans, session, repo_id); + if (result != 0) return result; + + /* Preserve CE VirtualRepo origin qualification without combining two + * repository keys in a single optimizer-dependent ACL locking scan. + * VirtualRepo.repo_id already has a unique index. Do not infer an + * unprotected scope from a failed or malformed origin lookup. */ + char *origin = NULL; + int count = trans ? seaf_db_trans_foreach_selected_row(trans, origin_query, + origin_found, &origin, 1, "string", repo_id) : + seaf_db_statement_foreach_row(session->db, origin_query, + origin_found, &origin, 1, "string", repo_id); + if (count < 0 || count > 1 || + (count == 1 && (!origin || strlen(origin) != 36 || + strcmp(origin, repo_id) == 0))) { + g_free(origin); + return -1; + } + if (count == 1) + result = check_scope(trans, session, origin); + g_free(origin); + return result; +} + /* Native transfer and Hub ACL mutations share per-library advisory locks. * Keep the dedicated connection until synchronous publication completes: an * empty ACL snapshot alone would race a newly installed source restriction. */ diff --git a/tests/test_cf_acl_lock_scope.py b/tests/test_cf_acl_lock_scope.py new file mode 100644 index 00000000..89cce944 --- /dev/null +++ b/tests/test_cf_acl_lock_scope.py @@ -0,0 +1,39 @@ +"""Cheap source contract for repository-scoped ACL final locking reads. + +Not a replacement for the disposable MariaDB EXPLAIN/lock-wait regression: +verify zero-rule and virtual-origin cases with concurrent ACL publication. +""" +from pathlib import Path +import unittest + +ROOT = Path(__file__).resolve().parents[1] + + +class ACLFinalLockScope(unittest.TestCase): + def test_exact_library_pk_range_replaces_cross_library_or_scan(self): + source = (ROOT / "common/cf-v2-acl.c").read_text(encoding="utf-8") + part = source.split("static const char *query =", 1)[1].split( + "/* Native transfer and Hub ACL mutations", 1)[0] + self.assertIn("FROM cf_dir_acl FORCE INDEX (PRIMARY) WHERE repo_id=? LIMIT 1", part) + self.assertNotIn("OR repo_id IN", part) + self.assertIn('g_strconcat(query, " FOR UPDATE", NULL)', part) + + def test_virtual_origin_and_sql_failure_still_fail_closed(self): + source = (ROOT / "common/cf-v2-acl.c").read_text(encoding="utf-8") + part = source.split("static int check(", 1)[1].split( + "/* Native transfer and Hub ACL mutations", 1)[0] + self.assertIn("SELECT origin_repo FROM VirtualRepo WHERE repo_id=?", source) + self.assertIn("if (count < 0 || count > 1", part) + self.assertIn("result = check_scope(trans, session, origin)", part) + self.assertIn("return -1;", part) + + def test_lock_order_and_acl_writer_contract_unchanged(self): + branch = (ROOT / "common/branch-mgr.c").read_text(encoding="utf-8") + update = branch.split("test_and_update_branch (", 1)[1] + self.assertLess(update.index("AND repo_id=? FOR UPDATE"), + update.index("cf_v2_acl_final(")) + self.assertIn("cf_v2_acl_final_scoped (", update) + + +if __name__ == "__main__": + unittest.main()