Repository navigation
Feature/lua cache write - #1080
Conversation
…butes and trackables
…d and has the full freedom
Inside opensak.transaction() the columns OpenSAK maintains (distance, bearing, log/waypoint/trackable counts, log dates) are no longer recalculated after every write. The touched caches are collected and recalculated once, just before the commit. This covers sql_write() and the ORM writes (update, insert, insert_/update_<child>). opensak.resync() recalculates them on demand inside a transaction. Codes of caches added by SQL INSERT are still normalised immediately, so the cache can be found by its code inside the transaction. A rolled-back transaction or savepoint leaves nothing pending, and a failed recalculation undoes the whole transaction. Outside a transaction, every write is still recalculated before its own commit. 2000 insert_log calls on one cache in a transaction: 4.5 s -> 1.8 s.
- New feature flag lua-macro-write (beta builds only): update, insert, insert_*/update_*, sql_write, transaction, in_transaction and resync are nil without it; set_corrected/clear_corrected then change the database without asking, as before. The Settings list of approved databases is hidden too. - Docs steer authors to update/insert/insert_<x>/update_<x> first and present sql_write as the tool for bulk work; gated functions are marked beta. - opensak.macro_db() checks write access to the macro's folder through the folder-permission model (OpenSAK-Org#978), prints the path when it creates the file, and a read-only macro database gives a clear error. - Test fixture: reset_flags restores features.json before reloading, so patched flag values no longer leak into later tests.
opensak.macro_db() built the file name from the called macro's folder and the caller's chunk name, so a macro started with opensak.run_macro() opened <caller>.db3 in the callee's folder (OpenSAK-Org#1039). - Name the file after _macro_path (switched by run_macro()); fall back to base_dir / chunk_name for a top-level run without a macro path. - In a called macro, macro_db() detaches the caller's file and attaches the callee's own as `macro`; the caller's file is attached again when the called macro returns, also after an error (the callee's error wins over a failed re-attach). - Until the callee calls macro_db(), `macro` still means the caller's file (DETACH is impossible inside opensak.transaction()). - WritableDatabase.detach_macro_db(); macro_db() docs updated. - Test: caller and helper in lib/, normal and failed return.
…#1034) Before a macro's first write, OpenSAK now asks whether this macro may change the OpenSAK databases (Deny / Until OpenSAK closes / Always). "Always" is stored by macro file path (macros.write_always) and covers all databases; macros still only write the active one. Editing the macro keeps the approval. - Unsaved macros get Deny / Until OpenSAK closes only. - Macros started with run_macro() are asked about on their own. - set_corrected/clear_corrected use the same approval. - Settings → Folder permissions lists approved macros by name (path in tooltip), removable. - MacroHost.approve_database_write(name, path) is now approve_macro_write(macro). - New dialog texts in all languages; docs and tests updated.
|
Thanks @nagisml — a lot of work in here: the write API, per-macro approval, the I ran the full unit-test suite on top of the stack (#1084 included): 6165 passed. One test, One design question: "Always" approval is stored by macro file path. If a user later replaces that file with a different version, e.g. an update downloaded from the forum, the new code silently inherits write access. That's exactly the "macros of unknown origin" scenario the concept doc (§9) cares about. Could we store a hash of the macro's content with the approval, and ask again when the file has changed outside OpenSAK's own editor? Edits made in OpenSAK's editor could keep the approval as now. Cosmetic: the history has a "rebase to v1.22.0-beta.4" commit and a couple of typos ("comapre"). Not blocking. Happy to merge this as the first of the stack once the approval question is settled — or as a follow-up issue if you prefer. |
Root bypasses POSIX file permissions, so chmod(S_IREAD) can't make the file read-only and the write-rejection assertions fail. Skip the test when euid == 0 (reported in PR review).
|
Hi @AgreeDK Thanks for the review and the approval to bring it in. Next would be the companion DB let's discuss how to go further on that. Related to the fedback
On the approval by macro version e.g. by hash. Sure this is possible but will mean an approval on each new macro version. Which may bring "complains".
I propose not to do it by now but think about it before removing the beta flag Sorry for the typos but I assume you squash merge so it does not matter |
|
Thanks @nagisml — agreed on the approval key. I said the file path was fine for now in #1034, and re-approving every macro update would annoy users. Let's revisit it before On squash: I normally use "Create a merge commit" to keep history, and for this stack it actually matters. I tested it, and squash-merging #1080 makes #1083 and #1084 conflict, because they still carry #1080's original commits. So I'll merge the Lua stack with merge commits, in order #1080 → #1083 → #1084. The typos are no problem. Companion DB: happy to discuss — could you start with a short proposal (data model, how it touches GSAK import, Lua, filters and views, migration) in a Discussion? Then we can agree on the design before any code. |
2 participants
Lua macros: write API for caches and their child rows (beta, behind
lua-macro-write)Follows the discussion in #1034. Lets Lua macros change the active database, gated behind the new feature flag
lua-macro-write(on in prerelease builds, off in releases). With the flag off, all write functions arenil, soif opensak.update thencan check for them.What's new
opensak.update(code, fields)andopensak.insert{}. Each field is checked (type, range, date format) with a clear error. Fields that OpenSAK maintains itself (code, distance, bearing, counts,last_log_date, …) are protected.opensak.logs/waypoints/attributes/trackables(code)read them,opensak.insert_<x>(code, fields)adds one andopensak.update_<x>(id, fields)changes one. Cache counts and log dates are updated automatically.opensak.sql_write()runs one INSERT or UPDATE on the writable tables. Protected columns are refused, and so are DELETE and SQLBEGIN/COMMIT. A raw INSERT only needs the DB schema: database defaults for NOT NULL columns, so a raw SQL INSERT only needs the real data #1033 required columns.opensak.transaction(fn)andopensak.in_transaction(). Derived columns are recalculated once, just before the commit, andopensak.resync()recalculates them on demand.opensak.sql().opensak.macro_db(): a per-macro SQLite file (<macro>.db3next to the macro), attached as schemamacro, with full read/write access. It needs folder permission. A macro started withrun_macro()gets its ownmacro_db(), and the caller's is re-attached on return (Allow starting other macros from within a macro #1039).macro_db()file OpenSAK has opened is recorded (macros.databasesin opensak.json) and snapshotted intomacro_databases/of each backup set, folder and zip alike. The manifest gets amacro_databaseslist; older versions ignore it, so the manifest format stays 1. A missing file is left out silently. A damaged one is left out with a warning and never fails the backup. Safety sets (settings only) leave them out.Approval model (per macro, as agreed with AgreeDK in #1034)
Before a macro's first change, OpenSAK asks: Deny / Until OpenSAK closes / Always.
run_macro()) needs its own approval.set_corrected()/clear_corrected()use the same approval.opensak --run-macro, Run a macro in the running OpenSAK instance from the command line (--run-macro) #1013) execute inside the open OpenSAK and get the same prompt in the OpenSAK window. Always is shared with the macro window through the resolved path.Docs
sql_write().docs/architecture/backup.md: themacro_databases/folder and manifest key.docs/macros/api.mdand the editor files are not committed; the Macro API docs workflow regenerates them after the merge.Tests
New:
test_macro_cache_write,test_macro_child_write,test_macro_own_db,test_macro_transaction,test_macro_temp_tables,test_macro_resync,test_macro_permissions,test_backup_macro_db_942, plus flag tests. Full unit suite locally: 5972 passed, 108 skipped.Not in this PR (follow-ups from #1034)
opensak.attach()) as a controlled replacement for SQLATTACH#1074