summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-05-10 14:48:25 -0500
committerCraig Jennings <c@cjennings.net>2026-05-10 14:48:25 -0500
commita7071b13d5a3309e28ee03e33acc526d7eaed810 (patch)
treeb79b547a581646ab04eb784f16bec840b1e3c51a
parenteb512124d9d587f9f26a17e9031f30a2cfff9476 (diff)
downloaddotemacs-a7071b13d5a3309e28ee03e33acc526d7eaed810.tar.gz
dotemacs-a7071b13d5a3309e28ee03e33acc526d7eaed810.zip
refactor(org-agenda): migrate to cj-cache helper
Phase 5 step 2 of utility-consolidation. The agenda-files cache (four module-level vars + 35-line build-with-cache function) now delegates to `cj-cache.el'. Behavior preserved: cache-hit logging, "Building..." background message, building-flag cleanup on error. Add `cj/--org-agenda-scan-files' as a pure-ish helper that returns the file list (the slow filesystem walk). `cj/build-org-agenda-list' becomes a thin wrapper that calls `cj/cache-value-or-rebuild' with the scan helper as BUILD-FN and routes the original log lines through :on-hit / :on-build-success. Drop four module-level state vars: - `cj/org-agenda-files-cache' - `cj/org-agenda-files-cache-time' - `cj/org-agenda-files-cache-ttl' - `cj/org-agenda-files-building' Replace with a single `cj/--org-agenda-files-cache' plist held by the helper. Rewrite the existing test file to test wrapper behavior at the contract level (stub the scan helper, verify wrapper outcomes) instead of poking at internal state vars. 8 tests cover first-call builds, second-call uses cache, force-rebuild bypass, TTL expiration, empty scan, building-flag cleanup on success and error, and error propagation.
-rw-r--r--modules/org-agenda-config.el84
-rw-r--r--tests/test-org-agenda-build-list.el352
2 files changed, 130 insertions, 306 deletions
diff --git a/modules/org-agenda-config.el b/modules/org-agenda-config.el
index 9205cf1c..049ece7b 100644
--- a/modules/org-agenda-config.el
+++ b/modules/org-agenda-config.el
@@ -44,6 +44,7 @@
;;; Code:
(require 'user-constants)
(require 'system-lib)
+(require 'cj-cache)
;; Load debug functions if enabled
(when (or (eq cj/debug-modules t)
@@ -82,21 +83,11 @@
#'org-agenda-todo-previousset))))
;; ------------------------ Org Agenda File List Cache -------------------------
-;; Cache agenda file list to avoid expensive directory scanning on every view
+;; Cache agenda file list to avoid expensive directory scanning on every view.
+;; The TTL+building cache lifecycle is provided by `cj-cache.el'.
-(defvar cj/org-agenda-files-cache nil
- "Cached agenda files list to avoid expensive directory scanning.
-Set to nil to invalidate cache.")
-
-(defvar cj/org-agenda-files-cache-time nil
- "Time when agenda files cache was last built.")
-
-(defvar cj/org-agenda-files-cache-ttl 3600
- "Time-to-live for agenda files cache in seconds (default: 1 hour).")
-
-(defvar cj/org-agenda-files-building nil
- "Non-nil when agenda files are being built asynchronously.
-Prevents duplicate builds if user opens agenda before async build completes.")
+(defvar cj/--org-agenda-files-cache (cj/cache-make :ttl 3600)
+ "Cache state for the agenda files list. See `cj-cache.el'.")
;; ------------------------ Add Files To Org Agenda List -----------------------
;; Checks immediate subdirectories of DIRECTORY for todo.org files and adds
@@ -118,6 +109,17 @@ Only checks DIRECTORY/*/todo.org — does not recurse deeper."
;; builds the org agenda list from all agenda targets with caching.
;; agenda targets is the schedule, contacts, project todos,
;; inbox, and org roam projects.
+(defun cj/--org-agenda-scan-files ()
+ "Scan disk for the agenda files list. Pure-ish: no caching, no logging.
+Returns the list to assign to `org-agenda-files'. Slow -- walks
+`projects-dir' for per-project todo.org files."
+ (let ((files (list inbox-file schedule-file gcal-file pcal-file dcal-file)))
+ ;; cj/add-files-to-org-agenda-files-list mutates org-agenda-files; let-bind
+ ;; it for the duration of the helper, then return whatever it produced.
+ (let ((org-agenda-files files))
+ (cj/add-files-to-org-agenda-files-list projects-dir)
+ org-agenda-files)))
+
(defun cj/build-org-agenda-list (&optional force-rebuild)
"Build org-agenda-files list with caching.
@@ -127,43 +129,23 @@ Otherwise, returns cached list if available and not expired.
This function scans projects-dir for todo.org files, so caching
improves performance from several seconds to instant."
(interactive "P")
- ;; Check if we can use cache
- (let ((cache-valid (and cj/org-agenda-files-cache
- cj/org-agenda-files-cache-time
- (not force-rebuild)
- (< (- (float-time) cj/org-agenda-files-cache-time)
- cj/org-agenda-files-cache-ttl))))
- (if cache-valid
- ;; Use cached file list (instant)
- (progn
- (setq org-agenda-files cj/org-agenda-files-cache)
- ;; Always show cache-hit message (interactive or background)
- (cj/log-silently "Using cached agenda files (%d files)"
- (length org-agenda-files)))
- ;; Check if async build is in progress
- (when cj/org-agenda-files-building
- (cj/log-silently "Waiting for background agenda build to complete..."))
- ;; Rebuild from scratch (slow - scans projects directory)
- (unwind-protect
- (progn
- (setq cj/org-agenda-files-building t)
- (let ((start-time (current-time)))
- ;; Reset org-agenda-files to base files
- (setq org-agenda-files (list inbox-file schedule-file gcal-file pcal-file dcal-file))
-
- ;; Check all projects for scheduled tasks
- (cj/add-files-to-org-agenda-files-list projects-dir)
-
- ;; Update cache
- (setq cj/org-agenda-files-cache org-agenda-files)
- (setq cj/org-agenda-files-cache-time (float-time))
-
- ;; Always show completion message (interactive or background)
- (cj/log-silently "Built agenda files: %d files in %.3f sec"
- (length org-agenda-files)
- (- (float-time) (float-time start-time)))))
- ;; Always clear the building flag, even if build fails
- (setq cj/org-agenda-files-building nil)))))
+ (when (cj/cache-building-p cj/--org-agenda-files-cache)
+ (cj/log-silently "Waiting for background agenda build to complete..."))
+ (let* ((start-time (current-time))
+ (files
+ (cj/cache-value-or-rebuild
+ cj/--org-agenda-files-cache
+ #'cj/--org-agenda-scan-files
+ :force-rebuild force-rebuild
+ :on-hit (lambda (v)
+ (cj/log-silently "Using cached agenda files (%d files)"
+ (length v)))
+ :on-build-success
+ (lambda (v)
+ (cj/log-silently "Built agenda files: %d files in %.3f sec"
+ (length v)
+ (- (float-time) (float-time start-time)))))))
+ (setq org-agenda-files files)))
;; Build cache asynchronously after startup to avoid blocking Emacs
(run-with-idle-timer
diff --git a/tests/test-org-agenda-build-list.el b/tests/test-org-agenda-build-list.el
index 8f9da912..f62100ec 100644
--- a/tests/test-org-agenda-build-list.el
+++ b/tests/test-org-agenda-build-list.el
@@ -1,14 +1,17 @@
;;; test-org-agenda-build-list.el --- Tests for cj/build-org-agenda-list -*- lexical-binding: t; -*-
;;; Commentary:
-;; Unit tests for cj/build-org-agenda-list caching logic.
-;; Tests cache behavior, TTL expiration, force rebuild, and async build flag.
+;; Tests for `cj/build-org-agenda-list' and the underlying cache shape.
+;; The wrapper delegates to `cj-cache.el', so these tests exercise the
+;; integration -- they don't reach into the cache plist directly (the
+;; cache primitives have their own tests in test-cj-cache.el). Stubs
+;; `cj/--org-agenda-scan-files' to avoid touching the filesystem.
;;; Code:
(require 'ert)
+(require 'cl-lib)
-;; Add modules to load path
(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory))
;; Stub dependencies before loading the module
@@ -19,286 +22,125 @@
(defvar dcal-file "/tmp/test-dcal.org")
(defvar projects-dir "/tmp/test-projects/")
-;; Now load the actual production module
(require 'org-agenda-config)
-;;; Setup and Teardown
-
-(defun test-org-agenda-setup ()
- "Reset cache and state before each test."
- (setq cj/org-agenda-files-cache nil)
- (setq cj/org-agenda-files-cache-time nil)
- (setq cj/org-agenda-files-building nil)
- (setq org-agenda-files nil))
-
-(defun test-org-agenda-teardown ()
- "Clean up after each test."
- (setq cj/org-agenda-files-cache nil)
- (setq cj/org-agenda-files-cache-time nil)
- (setq cj/org-agenda-files-building nil)
+(defun test-org-agenda--reset ()
+ "Reset cache and `org-agenda-files' between tests."
+ (cj/cache-invalidate cj/--org-agenda-files-cache)
(setq org-agenda-files nil))
;;; Normal Cases
-(ert-deftest test-org-agenda-build-list-normal-first-call-builds-cache ()
- "Test that first call builds cache from scratch.
-
-When cache is empty, function should:
-1. Scan directory for todo.org files
-2. Build agenda files list
-3. Populate cache
-4. Set cache timestamp"
- (test-org-agenda-setup)
+(ert-deftest test-org-agenda-build-list-first-call-builds-and-populates ()
+ "Normal: first call calls the scan helper and assigns to `org-agenda-files'."
+ (test-org-agenda--reset)
(unwind-protect
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir)
- (setq org-agenda-files
- (append '("/tmp/project/todo.org") org-agenda-files)))))
-
- ;; Before call: cache empty
- (should (null cj/org-agenda-files-cache))
- (should (null cj/org-agenda-files-cache-time))
-
- ;; Build agenda files
- (cj/build-org-agenda-list)
-
- ;; After call: cache populated
- (should cj/org-agenda-files-cache)
- (should cj/org-agenda-files-cache-time)
- (should org-agenda-files)
-
- ;; Cache matches org-agenda-files
- (should (equal cj/org-agenda-files-cache org-agenda-files))
-
- ;; Contains base files (inbox, schedule, gcal, pcal, dcal) plus project files
- (should (>= (length org-agenda-files) 3)))
- (test-org-agenda-teardown)))
-
-(ert-deftest test-org-agenda-build-list-normal-second-call-uses-cache ()
- "Test that second call uses cache instead of rebuilding.
+ (let ((scan-calls 0))
+ (cl-letf (((symbol-function 'cj/--org-agenda-scan-files)
+ (lambda ()
+ (cl-incf scan-calls)
+ '("/a.org" "/b.org" "/c.org"))))
+ (cj/build-org-agenda-list)
+ (should (= 1 scan-calls))
+ (should (equal '("/a.org" "/b.org" "/c.org") org-agenda-files))))
+ (test-org-agenda--reset)))
-When cache is valid (not expired):
-1. Should NOT scan directories again
-2. Should restore files from cache
-3. Should NOT update cache timestamp"
- (test-org-agenda-setup)
+(ert-deftest test-org-agenda-build-list-second-call-uses-cache ()
+ "Normal: a valid cache means the second call doesn't call the scan helper."
+ (test-org-agenda--reset)
(unwind-protect
- (let ((scan-count 0))
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir)
- (setq scan-count (1+ scan-count))
- (setq org-agenda-files
- (append '("/tmp/project/todo.org") org-agenda-files)))))
-
- ;; First call: builds cache
+ (let ((scan-calls 0))
+ (cl-letf (((symbol-function 'cj/--org-agenda-scan-files)
+ (lambda ()
+ (cl-incf scan-calls)
+ '("/a.org"))))
(cj/build-org-agenda-list)
- (should (= scan-count 1))
-
- (let ((cached-time cj/org-agenda-files-cache-time)
- (cached-files cj/org-agenda-files-cache))
-
- ;; Second call: uses cache
- (cj/build-org-agenda-list)
-
- ;; Scan count unchanged (cache hit)
- (should (= scan-count 1))
-
- ;; Cache unchanged
- (should (equal cj/org-agenda-files-cache-time cached-time))
- (should (equal cj/org-agenda-files-cache cached-files)))))
- (test-org-agenda-teardown)))
-
-(ert-deftest test-org-agenda-build-list-normal-force-rebuild-bypasses-cache ()
- "Test that force-rebuild parameter bypasses cache.
+ (cj/build-org-agenda-list)
+ (should (= 1 scan-calls))))
+ (test-org-agenda--reset)))
-When force-rebuild is non-nil:
-1. Should ignore valid cache
-2. Should rebuild from scratch
-3. Should update cache with new data"
- (test-org-agenda-setup)
+(ert-deftest test-org-agenda-build-list-force-rebuild-bypasses-cache ()
+ "Normal: FORCE-REBUILD calls the scan helper even when the cache is valid."
+ (test-org-agenda--reset)
(unwind-protect
- (let ((scan-count 0))
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir)
- (setq scan-count (1+ scan-count))
- (let ((extra (if (> scan-count 1)
- '("/tmp/project/todo.org" "/tmp/project2/todo.org")
- '("/tmp/project/todo.org"))))
- (setq org-agenda-files (append extra org-agenda-files))))))
-
- ;; First call: builds cache
+ (let ((scan-calls 0))
+ (cl-letf (((symbol-function 'cj/--org-agenda-scan-files)
+ (lambda ()
+ (cl-incf scan-calls)
+ '("/a.org"))))
(cj/build-org-agenda-list)
- (let ((initial-count (length org-agenda-files)))
-
- ;; Force rebuild
- (cj/build-org-agenda-list 'force)
-
- ;; Scanned again
- (should (= scan-count 2))
-
- ;; New files include additional project
- (should (> (length org-agenda-files) initial-count)))))
- (test-org-agenda-teardown)))
+ (cj/build-org-agenda-list 'force)
+ (should (= 2 scan-calls))))
+ (test-org-agenda--reset)))
;;; Boundary Cases
-(ert-deftest test-org-agenda-build-list-boundary-cache-expires-after-ttl ()
- "Test that cache expires after TTL period.
-
-When cache timestamp exceeds TTL:
-1. Should rebuild files list
-2. Should update cache timestamp
-3. Should rescan directory"
- (test-org-agenda-setup)
+(ert-deftest test-org-agenda-build-list-cache-expires-after-ttl ()
+ "Boundary: an expired cache rebuilds. Simulated by backdating the
+cache's :time field beyond the TTL."
+ (test-org-agenda--reset)
(unwind-protect
- (let ((scan-count 0))
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir)
- (setq scan-count (1+ scan-count))
- (setq org-agenda-files
- (append '("/tmp/project/todo.org") org-agenda-files)))))
-
- ;; First call: builds cache
+ (let ((scan-calls 0))
+ (cl-letf (((symbol-function 'cj/--org-agenda-scan-files)
+ (lambda ()
+ (cl-incf scan-calls)
+ '("/a.org"))))
(cj/build-org-agenda-list)
- (should (= scan-count 1))
-
- ;; Simulate cache expiration (set time to 2 hours ago)
- (setq cj/org-agenda-files-cache-time
- (- (float-time) (* 2 3600)))
-
- ;; Second call: cache expired, rebuild
+ ;; Expire the cache by moving its timestamp backwards beyond TTL.
+ (let ((ttl (plist-get cj/--org-agenda-files-cache :ttl)))
+ (plist-put cj/--org-agenda-files-cache :time
+ (- (float-time) (1+ ttl))))
(cj/build-org-agenda-list)
-
- ;; Scanned again (cache was expired)
- (should (= scan-count 2))
-
- ;; Cache timestamp updated to current time
- (should (< (- (float-time) cj/org-agenda-files-cache-time) 1))))
- (test-org-agenda-teardown)))
-
-(ert-deftest test-org-agenda-build-list-boundary-empty-directory-creates-minimal-list ()
- "Test behavior when directory contains no todo.org files.
-
-When directory scan returns empty:
-1. Should still create base files (inbox, schedule)
-2. Should not fail or error
-3. Should cache the minimal result"
- (test-org-agenda-setup)
- (unwind-protect
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir) nil))) ; No files added
-
- (cj/build-org-agenda-list)
-
- ;; Should have base files only (inbox, schedule, gcal, pcal, dcal)
- (should (= (length org-agenda-files) 5))
-
- ;; Cache should contain base files
- (should cj/org-agenda-files-cache)
- (should (= (length cj/org-agenda-files-cache) 5)))
- (test-org-agenda-teardown)))
-
-(ert-deftest test-org-agenda-build-list-boundary-building-flag-set-during-build ()
- "Test that building flag is set during build and cleared after.
-
-During build:
-1. Flag should be set to prevent concurrent builds
-2. Flag should clear even if build fails
-3. Flag state should be consistent"
- (test-org-agenda-setup)
+ (should (= 2 scan-calls))))
+ (test-org-agenda--reset)))
+
+(ert-deftest test-org-agenda-build-list-empty-scan-still-assigns ()
+ "Boundary: a scan helper that returns nil does NOT cache (per the
+documented \"nil reads as invalid\" contract). org-agenda-files is
+still assigned correctly."
+ (test-org-agenda--reset)
(unwind-protect
- (let ((flag-during-build nil))
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir)
- ;; Capture flag state during directory scan
- (setq flag-during-build cj/org-agenda-files-building)
- (setq org-agenda-files
- (append '("/tmp/project/todo.org") org-agenda-files)))))
-
- ;; Before build
- (should (null cj/org-agenda-files-building))
-
- ;; Build
+ (let ((scan-calls 0))
+ (cl-letf (((symbol-function 'cj/--org-agenda-scan-files)
+ (lambda () (cl-incf scan-calls) nil)))
(cj/build-org-agenda-list)
+ (should (= 1 scan-calls))
+ (should (null org-agenda-files))
+ ;; Next call rebuilds because nil-value reads as invalid.
+ (cj/build-org-agenda-list)
+ (should (= 2 scan-calls))))
+ (test-org-agenda--reset)))
- ;; Flag was set during build
- (should flag-during-build)
-
- ;; Flag cleared after build
- (should (null cj/org-agenda-files-building))))
- (test-org-agenda-teardown)))
-
-(ert-deftest test-org-agenda-build-list-boundary-building-flag-clears-on-error ()
- "Test that building flag clears even if build errors.
-
-When build encounters error:
-1. Flag should still be cleared (unwind-protect)
-2. Prevents permanently locked state
-3. Next build can proceed"
- (test-org-agenda-setup)
+(ert-deftest test-org-agenda-build-list-building-flag-clears-on-success ()
+ "Boundary: the cache's :building flag is cleared after a successful build."
+ (test-org-agenda--reset)
(unwind-protect
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir)
- (error "Simulated scan failure"))))
-
- ;; Build will error
- (should-error (cj/build-org-agenda-list))
-
- ;; Flag cleared despite error (unwind-protect)
- (should (null cj/org-agenda-files-building)))
- (test-org-agenda-teardown)))
+ (cl-letf (((symbol-function 'cj/--org-agenda-scan-files)
+ (lambda () '("/a.org"))))
+ (cj/build-org-agenda-list)
+ (should-not (cj/cache-building-p cj/--org-agenda-files-cache)))
+ (test-org-agenda--reset)))
;;; Error Cases
-(ert-deftest test-org-agenda-build-list-error-nil-cache-with-old-timestamp ()
- "Test handling of inconsistent state (nil cache but timestamp set).
-
-When cache is nil but timestamp exists:
-1. Should recognize cache as invalid
-2. Should rebuild files list
-3. Should set both cache and timestamp"
- (test-org-agenda-setup)
+(ert-deftest test-org-agenda-build-list-scan-error-propagates ()
+ "Error: a failure inside the scan helper propagates to the caller."
+ (test-org-agenda--reset)
(unwind-protect
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir)
- (setq org-agenda-files
- (append '("/tmp/project/todo.org") org-agenda-files)))))
-
- ;; Set inconsistent state
- (setq cj/org-agenda-files-cache nil)
- (setq cj/org-agenda-files-cache-time (float-time))
-
- ;; Build should recognize invalid state
- (cj/build-org-agenda-list)
-
- ;; Cache now populated
- (should cj/org-agenda-files-cache)
- (should cj/org-agenda-files-cache-time)
- (should org-agenda-files))
- (test-org-agenda-teardown)))
-
-(ert-deftest test-org-agenda-build-list-error-directory-scan-failure-propagates ()
- "Test that directory scan failures propagate as errors.
-
-When cj/add-files-to-org-agenda-files-list errors:
-1. Error should propagate to caller
-2. Cache should not be corrupted
-3. Building flag should clear"
- (test-org-agenda-setup)
+ (cl-letf (((symbol-function 'cj/--org-agenda-scan-files)
+ (lambda () (error "Permission denied"))))
+ (should-error (cj/build-org-agenda-list)))
+ (test-org-agenda--reset)))
+
+(ert-deftest test-org-agenda-build-list-building-flag-clears-on-error ()
+ "Error: the building flag is cleared even when the scan helper signals."
+ (test-org-agenda--reset)
(unwind-protect
- (cl-letf (((symbol-function 'cj/add-files-to-org-agenda-files-list)
- (lambda (_dir)
- (error "Permission denied"))))
-
- ;; Should propagate error
- (should-error (cj/build-org-agenda-list))
-
- ;; Cache not corrupted (still nil)
- (should (null cj/org-agenda-files-cache))
-
- ;; Building flag cleared
- (should (null cj/org-agenda-files-building)))
- (test-org-agenda-teardown)))
+ (cl-letf (((symbol-function 'cj/--org-agenda-scan-files)
+ (lambda () (error "Simulated failure"))))
+ (ignore-errors (cj/build-org-agenda-list))
+ (should-not (cj/cache-building-p cj/--org-agenda-files-cache)))
+ (test-org-agenda--reset)))
(provide 'test-org-agenda-build-list)
;;; test-org-agenda-build-list.el ends here