Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ecb33fd
Add Cache_Prefix trait for object cache group invalidation
donnapep Feb 26, 2026
c27bf77
Add is_cache_enabled() to Progress_Storage_Settings
donnapep Feb 26, 2026
2e32f0a
Add inline caching to student progress repositories
donnapep Feb 26, 2026
4e2524c
Add inline caching to quiz submission repositories
donnapep Feb 26, 2026
43c447b
Add sentinel property, thundering herd fix, and @since tags to Cache_…
donnapep Feb 26, 2026
360fc91
Memoize is_cache_enabled() to prevent mid-request instability
donnapep Feb 26, 2026
3aad620
Harden caching in student progress repositories
donnapep Feb 26, 2026
5fc5d01
Harden caching in quiz submission repositories
donnapep Feb 26, 2026
9827d82
Add missing cache tests and harden test teardown
donnapep Feb 26, 2026
eedf0ba
Address second code review findings for HPPS caching
donnapep Feb 26, 2026
bea908b
Add changelog entry for HPPS inline caching
donnapep Feb 26, 2026
b3887c7
Fix Psalm errors: remove unnecessary sentinel in array-returning methods
donnapep Feb 26, 2026
e8a934d
Restore has() filters and fix CacheDisabled test assertions
donnapep Feb 26, 2026
e656c4c
Use $$next-version$$ placeholder for @since tags
donnapep Feb 27, 2026
22a8f33
Extract get_prefix_key() to remove string duplication in Cache_Prefix…
donnapep Feb 27, 2026
86fb93a
Remove redundant sensei_ prefix from cache prefix key
donnapep Feb 27, 2026
e3e9440
Add assertion messages to tests with multiple assertions
donnapep Feb 27, 2026
f48cec8
Move $cache_enabled property to top of class with other declarations
donnapep Feb 27, 2026
c892aca
Use lightweight COUNT query in tables-based progress has() methods
donnapep Mar 2, 2026
323cf60
Remove redundant has() cache test and group tests by method
donnapep Mar 2, 2026
c7f4517
Update changelog entry
donnapep Mar 2, 2026
30dee5b
Use mocked wpdb in cache read tests to prove DB is skipped
donnapep Mar 2, 2026
d6311ab
Strip space from microtime() output in cache prefix to avoid invalid …
donnapep Mar 3, 2026
d9a3889
Add (int) cast to apply_filters calls in submission repositories
donnapep Mar 3, 2026
c0e8871
Extract get_cache_key() method in tables-based repositories
donnapep Mar 3, 2026
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
2 changes: 2 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ Composer production dependencies that may conflict with other plugins are scoped

- **CRITICAL: WordPress filters persist between test cases.** Always remove filters added during a test in `tearDown()` or the test will leak state into other tests.
- **Use `assertSame()` over `assertEquals()`** — strict type + equality comparison.
- **Use assertion messages when a test has multiple assertions** — pass the `$message` parameter to differentiate which assertion failed (e.g., `self::assertSame( 1, $id, 'ID should be 1' )`).
- **Text domain is `sensei-lms`** (not `sensei`). All user-facing strings MUST use this.
- **Never concatenate translatable strings** — use `sprintf()` with placeholders.
- **Never use `extract()`, `eval()`, or `create_function()`.**
Expand All @@ -45,6 +46,7 @@ Composer production dependencies that may conflict with other plugins are scoped
- **Branch naming**: `type/description` — e.g. `fix/course-average-query`, `add/show-tailored-course-outline`, `feature/ai-make-quiz`
- **PRs**: Must reference an issue (`Resolves #123`), include testing instructions, and follow `.github/PULL_REQUEST_TEMPLATE.md`.
- **Changelogs**: Every user-facing change MUST have a changelog entry before opening a PR. Run `npm run changelog` (entries stored in `changelog/`).
- Use the `$$next-version$$` placeholder for the `@since` parameter.

## Boundaries

Expand Down
4 changes: 4 additions & 0 deletions changelog/add-hpps-inline-caching
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: added

Add object caching to HPPS (High-Performance Progress Storage) repositories
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

use DateTimeImmutable;
use DateTimeZone;
use Sensei\Internal\Cache_Prefix;
use Sensei\Internal\Services\Progress_Storage_Settings;
use Sensei\Internal\Quiz_Submission\Answer\Models\Answer_Interface;
use Sensei\Internal\Quiz_Submission\Answer\Models\Tables_Based_Answer;
use Sensei\Internal\Quiz_Submission\Submission\Models\Submission_Interface;
Expand All @@ -26,6 +28,17 @@
* @since 4.16.1
*/
class Tables_Based_Answer_Repository implements Answer_Repository_Interface {
use Cache_Prefix;

/**
* Cache group for quiz answers.
*
* @since $$next-version$$
*
* @var string
*/
private const CACHE_GROUP = 'sensei_quiz_answers';

/**
* WordPress database object.
*
Expand Down Expand Up @@ -102,14 +115,20 @@ public function create( Submission_Interface $submission, int $question_id, stri
]
);

return new Tables_Based_Answer(
$answer = new Tables_Based_Answer(
$this->wpdb->insert_id,
$submission_id,
$question_id,
$value,
$current_datetime,
$current_datetime
);

if ( $this->wpdb->insert_id && Progress_Storage_Settings::is_cache_enabled() ) {
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission_id ), self::CACHE_GROUP ), self::CACHE_GROUP );
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache invalidation uses $submission_id as filtered by sensei_quiz_answer_create_submission_id, but get_all() caches under a key derived from sensei_quiz_answer_get_all_submission_id. If those filters ever map IDs differently, create() may fail to invalidate the get_all() cache, causing stale reads. Consider deriving the cache key for invalidation using the same ID-normalization logic as get_all() (e.g., apply the get_all filter or centralize key computation in a helper used by both).

Suggested change
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission_id ), self::CACHE_GROUP ), self::CACHE_GROUP );
/**
* Normalize the submission ID for cache key computation in the same way as get_all().
*
* This must stay in sync with the logic in get_all(), which uses the
* 'sensei_quiz_answer_get_all_submission_id' filter when generating the cache key.
*/
$cache_submission_id = (int) apply_filters( 'sensei_quiz_answer_get_all_submission_id', $submission_id, 'tables' );
wp_cache_delete(
self::get_prefixed_key( $this->get_cache_key( $cache_submission_id ), self::CACHE_GROUP ),
self::CACHE_GROUP
);

Copilot uses AI. Check for mistakes.
}

return $answer;
}

/**
Expand All @@ -135,6 +154,15 @@ public function get_all( int $submission_id ): array {
*/
$submission_id = (int) apply_filters( 'sensei_quiz_answer_get_all_submission_id', $submission_id, 'tables' );

$cache_key = $this->get_cache_key( $submission_id );

if ( Progress_Storage_Settings::is_cache_enabled() ) {
$cached = wp_cache_get( self::get_prefixed_key( $cache_key, self::CACHE_GROUP ), self::CACHE_GROUP );
if ( false !== $cached ) {
return $cached;
}
}

$query = $this->wpdb->prepare(
// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
"SELECT * FROM {$this->get_table_name()} WHERE submission_id = %d",
Expand All @@ -154,6 +182,10 @@ public function get_all( int $submission_id ): array {
);
}

if ( Progress_Storage_Settings::is_cache_enabled() ) {
wp_cache_set( self::get_prefixed_key( $cache_key, self::CACHE_GROUP ), $answers, self::CACHE_GROUP );
}

return $answers;
}

Expand Down Expand Up @@ -187,6 +219,22 @@ public function delete_all( Submission_Interface $submission ): void {
'%d',
]
);

if ( Progress_Storage_Settings::is_cache_enabled() ) {
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission_id ), self::CACHE_GROUP ), self::CACHE_GROUP );
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete_all() invalidates the cache key using the ID filtered by sensei_quiz_answer_delete_all_submission_id, but get_all() caches under a key derived from sensei_quiz_answer_get_all_submission_id. If those filters diverge, the cached answers list may not be invalidated correctly. Suggest computing the invalidation key using the same ID transformation/key helper as get_all() so cache keys stay consistent across read/write paths.

Suggested change
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission_id ), self::CACHE_GROUP ), self::CACHE_GROUP );
// Use the same ID transformation as get_all() when computing the cache key.
$cache_submission_id = (int) apply_filters( 'sensei_quiz_answer_get_all_submission_id', $submission->get_id(), 'tables' );
wp_cache_delete(
self::get_prefixed_key( $this->get_cache_key( $cache_submission_id ), self::CACHE_GROUP ),
self::CACHE_GROUP
);

Copilot uses AI. Check for mistakes.
}
}

/**
* Get the cache key for quiz answers by submission.
*
* @since $$next-version$$
*
* @param int $submission_id The submission ID.
* @return string The cache key.
*/
private function get_cache_key( int $submission_id ): string {
return (string) $submission_id;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
exit;
}

use Sensei\Internal\Cache_Prefix;
use Sensei\Internal\Services\Progress_Storage_Settings;
use Sensei\Internal\Quiz_Submission\Answer\Models\Answer_Interface;
use Sensei\Internal\Quiz_Submission\Grade\Models\Tables_Based_Grade;
use Sensei\Internal\Quiz_Submission\Grade\Models\Grade_Interface;
Expand All @@ -25,6 +27,17 @@
* @since 4.16.1
*/
class Tables_Based_Grade_Repository implements Grade_Repository_Interface {
use Cache_Prefix;

/**
* Cache group for quiz grades.
*
* @since $$next-version$$
*
* @var string
*/
private const CACHE_GROUP = 'sensei_quiz_grades';

/**
* WordPress database object.
*
Expand Down Expand Up @@ -105,7 +118,7 @@ public function create( Submission_Interface $submission, Answer_Interface $answ
]
);

return new Tables_Based_Grade(
$grade = new Tables_Based_Grade(
$this->wpdb->insert_id,
$answer_id,
$question_id,
Expand All @@ -114,6 +127,12 @@ public function create( Submission_Interface $submission, Answer_Interface $answ
$current_date,
$current_date
);

if ( $this->wpdb->insert_id && Progress_Storage_Settings::is_cache_enabled() ) {
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission->get_id() ), self::CACHE_GROUP ), self::CACHE_GROUP );
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create() invalidates the get_all() cache using $submission->get_id() directly, but get_all() caches under a key derived from sensei_quiz_grade_get_all_submission_id. If that filter maps submission IDs (e.g., translation/aliasing), cache invalidation can miss and serve stale grades. Consider generating the invalidation key using the same ID normalization as get_all() (apply the same filter or centralize cache-key computation).

Suggested change
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission->get_id() ), self::CACHE_GROUP ), self::CACHE_GROUP );
$submission_id_for_cache = apply_filters( 'sensei_quiz_grade_get_all_submission_id', $submission->get_id() );
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission_id_for_cache ), self::CACHE_GROUP ), self::CACHE_GROUP );

Copilot uses AI. Check for mistakes.
}

return $grade;
}

/**
Expand All @@ -139,8 +158,21 @@ public function get_all( int $submission_id ): array {
*/
$submission_id = (int) apply_filters( 'sensei_quiz_grade_get_all_submission_id', $submission_id, 'tables' );

$cache_key = $this->get_cache_key( $submission_id );

if ( Progress_Storage_Settings::is_cache_enabled() ) {
$cached = wp_cache_get( self::get_prefixed_key( $cache_key, self::CACHE_GROUP ), self::CACHE_GROUP );
if ( false !== $cached ) {
return $cached;
}
}

$answer_ids = $this->get_answer_ids_by_submission_id( $submission_id );
if ( empty( $answer_ids ) ) {
if ( Progress_Storage_Settings::is_cache_enabled() ) {
wp_cache_set( self::get_prefixed_key( $cache_key, self::CACHE_GROUP ), array(), self::CACHE_GROUP );
}

return [];
}

Expand All @@ -162,6 +194,10 @@ public function get_all( int $submission_id ): array {
);
}

if ( Progress_Storage_Settings::is_cache_enabled() ) {
wp_cache_set( self::get_prefixed_key( $cache_key, self::CACHE_GROUP ), $grades, self::CACHE_GROUP );
}

return $grades;
}

Expand All @@ -177,6 +213,10 @@ public function save_many( Submission_Interface $submission, array $grades ): vo
foreach ( $grades as $grade ) {
$this->save( $grade );
}

if ( Progress_Storage_Settings::is_cache_enabled() ) {
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission->get_id() ), self::CACHE_GROUP ), self::CACHE_GROUP );
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

save_many() invalidates the cached get_all() result using $submission->get_id(), but get_all() caches under a key derived from the filtered submission ID (sensei_quiz_grade_get_all_submission_id). If the filter changes the ID, the cache entry being read won’t be the one being deleted. Suggest deriving the cache key for invalidation via the same filter/key helper as get_all().

Suggested change
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission->get_id() ), self::CACHE_GROUP ), self::CACHE_GROUP );
$submission_id = (int) apply_filters( 'sensei_quiz_grade_get_all_submission_id', $submission->get_id(), 'tables' );
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission_id ), self::CACHE_GROUP ), self::CACHE_GROUP );

Copilot uses AI. Check for mistakes.
}
}

/**
Expand Down Expand Up @@ -209,6 +249,10 @@ public function delete_all( Submission_Interface $submission ): void {
$delete_query = 'DELETE FROM ' . $this->get_table_name() . ' WHERE answer_id IN (' . $placeholders . ')';
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$this->wpdb->query( $this->wpdb->prepare( $delete_query, ...$answer_ids ) );

if ( Progress_Storage_Settings::is_cache_enabled() ) {
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission_id ), self::CACHE_GROUP ), self::CACHE_GROUP );
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete_all() deletes the cache entry using the ID filtered by sensei_quiz_grade_delete_all_submission_id, but get_all() reads/writes cache using an ID filtered by sensei_quiz_grade_get_all_submission_id. If those filters return different values, the cached grades list may not be invalidated correctly after deletes. Consider computing invalidation keys using the same ID normalization logic as get_all() (shared helper / apply the same filter) to guarantee consistency.

Suggested change
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $submission_id ), self::CACHE_GROUP ), self::CACHE_GROUP );
$cache_submission_id = (int) apply_filters( 'sensei_quiz_grade_get_all_submission_id', $submission->get_id(), 'tables' );
wp_cache_delete( self::get_prefixed_key( $this->get_cache_key( $cache_submission_id ), self::CACHE_GROUP ), self::CACHE_GROUP );

Copilot uses AI. Check for mistakes.
}
}

/**
Expand Down Expand Up @@ -240,10 +284,23 @@ private function save( Grade_Interface $grade ): void {
);
}

/**
* Get the cache key for quiz grades by submission.
*
* @since $$next-version$$
*
* @param int $submission_id The submission ID.
* @return string The cache key.
*/
private function get_cache_key( int $submission_id ): string {
return (string) $submission_id;
}

/**
* Get all answer IDs for a submission.
*
* @param int $submission_id The submission ID.
* @return array The answer IDs.
*/
private function get_answer_ids_by_submission_id( int $submission_id ): array {
$answers_query = $this->wpdb->prepare(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function create( int $quiz_id, int $user_id, float $final_grade = null ):
* @param {int} $quiz_id The quiz ID.
* @return {int} The quiz ID.
*/
$quiz_id = apply_filters( 'sensei_quiz_submission_create_quiz_id', $quiz_id );
$quiz_id = (int) apply_filters( 'sensei_quiz_submission_create_quiz_id', $quiz_id );

$status_comment = $this->get_status_comment( $quiz_id, $user_id );

Expand Down Expand Up @@ -96,7 +96,7 @@ public function get_or_create( int $quiz_id, int $user_id, float $final_grade =
* @param {int} $quiz_id The quiz ID.
* @return {int} The quiz ID.
*/
$quiz_id = apply_filters( 'sensei_quiz_submission_get_or_create_quiz_id', $quiz_id );
$quiz_id = (int) apply_filters( 'sensei_quiz_submission_get_or_create_quiz_id', $quiz_id );

$submission = $this->get( $quiz_id, $user_id );

Expand Down Expand Up @@ -128,7 +128,7 @@ public function get( int $quiz_id, int $user_id ): ?Submission_Interface {
* @param {int} $quiz_id The quiz ID.
* @return {int} The quiz ID.
*/
$quiz_id = apply_filters( 'sensei_quiz_submission_get_quiz_id', $quiz_id );
$quiz_id = (int) apply_filters( 'sensei_quiz_submission_get_quiz_id', $quiz_id );

$status_comment = $this->get_status_comment( $quiz_id, $user_id );

Expand Down
Loading
Loading