[TAM-2029] wrong ttl alert notification fixed

[SoftFx]

No description provided.

[SoftFx]

@claude review it

[github-actions]

PR Review: [TAM-2029] wrong ttl alert notification fixed

File: src/server/HSMServer.Core/Model/Storages/ValueStorages/ValuesStorage.cs

The intent of this PR — preventing timeout values from polluting the value cache and triggering incorrect TTL alert notifications — is correct. However, the implementation introduces a critical regression and leaves a secondary bug unaddressed.

---

🔴 Critical Bug: Double Enqueue

The PR adds an unconditional _cache.Enqueue(value) before the if (_lastValue is null || value.Time >= _lastValue.Time) block, but does not remove the existing _cache.Enqueue(value) inside that block. As a result, every non-timeout value whose Time >= _lastValue.Time — which is the normal case for all freshly received sensor data — gets enqueued twice.

Current code after the PR (lines 85–99):

_cache.Enqueue(value);              // ← added by this PR

if (_cache.Count > CacheSize)
    _cache.TryDequeue(out _);

if (_lastValue is null || value.Time >= _lastValue.Time)
{
    _lastValue = value;
    _to = value.Time;

    _cache.Enqueue(value);          // ← was already here, not removed
    if (_cache.Count > CacheSize)
        _cache.TryDequeue(out _);
}

Impact:

• GetValues(int count) and GetValues(DateTime from, DateTime to) will return duplicate entries for the latest value.
• The effective cache capacity is halved for the common case: two slots are consumed per value instead of one, causing older values to be evicted sooner than intended.
• LastDbValue (_cache.LastOrDefault()) may return a duplicate reference rather than the actual last distinct value.
• TryChangeLastValue dequeues only one entry, leaving the duplicate in the cache.

Fix: Remove the _cache.Enqueue(value) / TryDequeue pair from inside the if block (lines 95–98), keeping only the unconditional pair added by this PR.

---

🟡 Secondary Bug: Stale Timeout Values Fall Through

The guard condition is:

if (value.IsTimeout && (_lastTimeout is null || _lastTimeout.ReceivingTime < value.ReceivingTime))
{
    _lastTimeout = value;
    return;
}

When value.IsTimeout == true but the condition is false (i.e., the incoming timeout is older than the already-stored _lastTimeout), the return is not reached. The stale timeout value then:

1. Gets enqueued into _cache — a timeout value in the data cache is semantically wrong.
2. May update _lastValue and _to if its Time is newer than the current _lastValue.Time.

In the original code this was also a latent issue (the stale timeout could update _lastValue via the else if), but the PR now additionally caches it, making the impact worse.

Suggested fix: Move the return outside the inner condition so all timeout values exit early:

if (value.IsTimeout)
{
    if (_lastTimeout is null || _lastTimeout.ReceivingTime < value.ReceivingTime)
        _lastTimeout = value;
    return;
}

---

✅ What the PR Gets Right

• The early return for newer timeout values correctly prevents them from entering _cache and from updating _lastValue/_to, which is the root cause described in the ticket.
• IsLastEmptyOrTimeout correctly detects when the last meaningful value is a timeout, and this property is unaffected by the change.

---

Summary

Issue	Severity	Type
Double enqueue of normal values	Critical	Regression introduced by this PR
Stale timeout values cached and updating _lastValue	Medium	Pre-existing + worsened by PR

The double-enqueue bug should be fixed before merging. The stale-timeout issue should be addressed in the same change to fully resolve the TTL alert problem.