Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
135 changes: 135 additions & 0 deletions Daqifi.Desktop/Exporter/LoggingSessionSampleSource.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
using System.Runtime.CompilerServices;
using Daqifi.Core.Logging.Export;
using Daqifi.Desktop.Channel;
using Daqifi.Desktop.Logger;
using Microsoft.EntityFrameworkCore;

namespace Daqifi.Desktop.Exporter;

/// <summary>
/// Adapts a desktop <see cref="LoggingSession"/> to the core
/// <see cref="ISampleSource"/> seam consumed by <see cref="CsvExporter"/>.
/// Supports two modes: an EF Core path backed by <see cref="LoggingContext"/>
/// (production) and an in-memory path over a pre-populated
/// <see cref="LoggingSession.DataSamples"/> collection (used by tests).
/// </summary>
public sealed class LoggingSessionSampleSource : ISampleSource
{
#region Private Fields
private readonly LoggingSession _session;
private readonly IDbContextFactory<LoggingContext> _contextFactory;
private readonly ICollection<DataSample> _inMemorySamples;
private IReadOnlyList<ChannelDescriptor> _channelsCache;
private int? _countCache;
#endregion

#region Constructors
public LoggingSessionSampleSource(LoggingSession session, IDbContextFactory<LoggingContext> contextFactory)
{
_session = session ?? throw new ArgumentNullException(nameof(session));
_contextFactory = contextFactory ?? throw new ArgumentNullException(nameof(contextFactory));
}

public LoggingSessionSampleSource(LoggingSession session, ICollection<DataSample> inMemorySamples)
{
_session = session ?? throw new ArgumentNullException(nameof(session));
_inMemorySamples = inMemorySamples ?? throw new ArgumentNullException(nameof(inMemorySamples));
}
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
#endregion

#region ISampleSource
public IReadOnlyList<ChannelDescriptor> GetChannels()
{
if (_channelsCache != null)
{
return _channelsCache;
}

if (_inMemorySamples != null)
{
_channelsCache = _inMemorySamples
.GroupBy(s => new { s.DeviceName, s.DeviceSerialNo, s.ChannelName })
.Select(g => new ChannelDescriptor(
g.Key.DeviceName,
g.Key.DeviceSerialNo,
g.Key.ChannelName,
g.First().Type))
.OrderBy(c => c.DeviceName)
.ThenBy(c => c.DeviceSerialNo)
.ThenBy(c => c.ChannelName)
.ToList();
return _channelsCache;
}

using var context = _contextFactory.CreateDbContext();
context.ChangeTracker.AutoDetectChangesEnabled = false;

_channelsCache = context.Samples
.AsNoTracking()
.Where(s => s.LoggingSessionID == _session.ID)
.Select(s => new { s.DeviceName, s.DeviceSerialNo, s.ChannelName, s.Type })
.Distinct()
.OrderBy(s => s.DeviceName)
.ThenBy(s => s.DeviceSerialNo)
.ThenBy(s => s.ChannelName)
.AsEnumerable()
.Select(s => new ChannelDescriptor(s.DeviceName, s.DeviceSerialNo, s.ChannelName, s.Type))
.ToList();
return _channelsCache;
}

public ValueTask<int> GetSampleCountAsync(CancellationToken cancellationToken = default)
{
if (_countCache.HasValue)
{
return new ValueTask<int>(_countCache.Value);
}

if (_inMemorySamples != null)
{
_countCache = _inMemorySamples.Count;
return new ValueTask<int>(_countCache.Value);
}

using var context = _contextFactory.CreateDbContext();
context.ChangeTracker.AutoDetectChangesEnabled = false;
_countCache = context.Samples
.AsNoTracking()
.Count(s => s.LoggingSessionID == _session.ID);
return new ValueTask<int>(_countCache.Value);
}

public async IAsyncEnumerable<SampleRow> StreamSamples([EnumeratorCancellation] CancellationToken cancellationToken = default)
{
if (_inMemorySamples != null)
{
foreach (var s in _inMemorySamples.OrderBy(d => d.TimestampTicks))
{
cancellationToken.ThrowIfCancellationRequested();
yield return new SampleRow(
s.TimestampTicks,
$"{s.DeviceName}:{s.DeviceSerialNo}:{s.ChannelName}",
s.Value);
}
Comment on lines +141 to +145
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Action required

3. Invalid tick handling lost 🐞 Bug ≡ Correctness

The adapter now streams raw TimestampTicks into the core CsvExporter without any in-repo
invalid-tick labeling/guard, so the previous "INVALID(ticks)" behavior is no longer enforced by
desktop code. This breaks the repo’s existing test that requires invalid ticks to be exported as
"INVALID(...)" (and not throw) unless the core exporter independently reproduces that behavior.
Agent Prompt
### Issue description
Desktop previously guaranteed that out-of-range/invalid `TimestampTicks` would not crash export and would be written as an `INVALID(<ticks>)` token. After delegating formatting to core, the desktop layer no longer enforces this behavior, but the repo still contains a test that requires it.

### Issue Context
`LoggingSessionSampleSource` emits raw ticks for every sample row; there is no adapter-side sanitization or fallback formatting.

### Fix Focus Areas
- Daqifi.Desktop/Exporter/LoggingSessionSampleSource.cs[102-133]
- Daqifi.Desktop/Exporter/OptimizedLoggingSessionExporter.cs[46-169]
- Daqifi.Desktop.Test/Exporter/OptimizedExporterValidationTests.cs[162-209]

### What to change
- Ensure invalid tick values (negative, zero if considered invalid, > `DateTime.MaxValue.Ticks`) do not cause export to fail.
- Preserve the existing contract that invalid timestamps are labeled as `INVALID(<ticks>)` in absolute-time exports. Options include:
  - Adding/using a core option/hook (if available) to format invalid ticks, or
  - Adding a desktop-side fallback path that mirrors the legacy invalid-timestamp formatting for absolute time when invalid ticks are detected, so the existing test continues to pass.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Disagree — Daqifi.Core 0.21.0's CsvExporter already enforces this. Per its public XML docs:

CsvExporter.FormatTimestamp: Formats a tick value as an absolute ISO 8601 string or relative seconds string. Ticks that are out of the valid DateTime range are rendered as INVALID({ticks}) in both modes.

The desktop test OptimizedExporter_InvalidTimestamps_DoesNotThrowAndMarksInvalidRows will pass because Core honors the contract — duplicating the guard in the adapter would be redundant. (This was actually called out as gotcha #3 in the parent issue: the adapter just streams raw ticks; Core handles formatting including the invalid case.)

If Core ever regressed on this contract, the test would fail and we'd catch it in CI — that's the right place to enforce it.

yield break;
}

await using var context = _contextFactory.CreateDbContext();
context.ChangeTracker.AutoDetectChangesEnabled = false;

var query = context.Samples
.AsNoTracking()
.Where(s => s.LoggingSessionID == _session.ID)
.OrderBy(s => s.TimestampTicks)
.Select(s => new { s.TimestampTicks, s.DeviceName, s.DeviceSerialNo, s.ChannelName, s.Value });

await foreach (var s in query.AsAsyncEnumerable().WithCancellation(cancellationToken))
{
yield return new SampleRow(
s.TimestampTicks,
$"{s.DeviceName}:{s.DeviceSerialNo}:{s.ChannelName}",
s.Value);
}
}
#endregion
}
Loading
Loading