Bug: Entries::count() memo is not keyed by $source/$col, so Limiter's cache_limit silently fails to apply

[sadmansakibnadvi]

Describe the bug

Entries::count() memoizes its result on the singleton, but the memo is not keyed by the $source/$col arguments. The first call in a request therefore poisons every later call that uses a different $col, and Limiter's per-campaign entry cap silently stops applying.

https://github.com/WPDevelopers/notificationx/blob/master/includes/Admin/Entries.php#L47-L58

public function count($source, $col = 'source'){
    if(empty($this->count)){                       // <-- memo ignores $source and $col
        $this->count = Database::get_instance()->get_source_count(Database::$table_entries, $col, [$col => $source]);
    }
    if(!empty($this->count[$source])){
        return $this->count[$source];
    }
    elseif(!empty($source)){
        return 0;                                  // <-- silent wrong answer
    }
    return $this->count;
}

Limiter::remove() calls it with $col = 'nx_id':

https://github.com/WPDevelopers/notificationx/blob/master/includes/Core/Limiter.php#L23-L35

public function remove($nx_id, $new) {
    $count = Entries::get_instance()->count($nx_id, 'nx_id');
    $limit = Settings::get_instance()->get('settings.cache_limit', 100);
    ...
    if ($new + $count > $limit) {
        $overflow = ($new + $count) - $limit;
        Entries::get_instance()->delete_entries($nx_id, $overflow);
    }
}

If anything earlier in the same request already called count($source) with the default $col = 'source', $this->count holds an array keyed by source slug. The subsequent lookup $this->count[$nx_id] then misses, falls into the elseif, and returns 0.

With $count = 0, the condition $new + 0 > 100 is false for any normal batch, so no overflow is ever computed and cache_limit is not enforced.

Steps to reproduce

Within a single request:

1. Trigger any code path that calls Entries::count($source) with the default $col (source-keyed).
2. Then save a campaign whose extension backfills entries, so update_notifications() → Limiter::remove($nx_id, $n) runs.
3. Compare the resulting row count for that nx_id in {$prefix}nx_entries against settings.cache_limit.

Expected: entries for the campaign are capped at cache_limit (default 100).
Actual: the cap is not applied; the table grows past the limit. Whether it applies depends on what else ran first in the request, so the behaviour is inconsistent between requests.

Suggested fix

Key the memo by both arguments:

public function count($source, $col = 'source'){
    $cache_key = $col . ':' . $source;
    if (!isset($this->count[$cache_key])) {
        $result = Database::get_instance()->get_source_count(Database::$table_entries, $col, [$col => $source]);
        $this->count[$cache_key] = !empty($result[$source]) ? $result[$source] : 0;
    }
    return $this->count[$cache_key];
}

Note the current signature is overloaded — it returns an int when $source is non-empty and the whole array otherwise. Worth checking callers before tightening that.

Related

While in Limiter, the eviction query has no ORDER BY:

https://github.com/WPDevelopers/notificationx/blob/master/includes/Core/Database.php#L246-L253

DELETE FROM ... WHERE nx_id = X LIMIT n deletes an arbitrary n rows rather than the oldest n. So when the cap does apply, it can evict recent entries and keep stale ones. Happy to split that into its own issue if preferred.

Impact

Low severity but unbounded growth: nx_entries can exceed cache_limit indefinitely on busy sites, and the inconsistency makes it hard to reproduce. The missing ORDER BY additionally means capping can remove the wrong entries.

Environment

Found on master @ 55f62f10.