Stop a finished push deleting the edit made while it was in flight, and pull a newly synced type from the start

- The outbox coalesces a new save into the row a push already read, under a new operation id, but
  Complete, Revise and Park found the row by sequence alone. The acknowledgement of the earlier state
  deleted the row holding the later one and the edit was gone without a word. They now take the
  PendingOperation and act only while the row still holds it; a later edit is rebased onto the version
  just accepted, so it pushes cleanly instead of conflicting with its own earlier save.
- A pull filtered by type moves the cursor past changes of types it did not ask for, so a build that
  added a type (SpelunkKey, ObjectStore) never received the items of that type made before it.
  sync_state.pulled_types records the list a cursor was read under, and a different list starts the
  pull over once. Migration AddSyncStatePulledTypes.
This commit is contained in:
jaap-jan committed 2026-10-02 13:16:47 +02:00
1 parent 03c8e8589a
commit f332d9a56f
12 files changed
+757 -31

No files matched your search

+11
View File
@@ -350,6 +350,17 @@ internal sealed class SyncStateRow
/// decide which edit wins. /// decide which edit wins.
/// </remarks> /// </remarks>
public long ServerTimeSkewMs { get; set; } public long ServerTimeSkewMs { get; set; }
/// <summary>
/// The item types <see cref="Cursor"/> was read under, as this build names them; null before this was kept.
/// </summary>
/// <remarks>
/// A pull asks for the types this build can apply, and the cursor it gets back is past every change it
/// read — including the changes of types it did not ask for. So a build that adds a type to that list
/// and carries on from the old cursor never sees the items of that type made before it arrived. Kept
/// beside the cursor so a change of list starts the pull over; see <c>SyncEngine.PullAsync</c>.
/// </remarks>
public string? PulledTypes { get; set; }
} }
/// <summary>Something the merge had to override, or an item that could not be processed.</summary> /// <summary>Something the merge had to override, or an item that could not be processed.</summary>
@@ -0,0 +1,469 @@
// <auto-generated />
using System;
using DodoSSH.Client.Storage;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Migrations;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
#nullable disable
namespace DodoSSH.Client.Storage.Migrations
{
[DbContext(typeof(ClientCacheContext))]
[Migration("20261002104920_AddSyncStatePulledTypes")]
partial class AddSyncStatePulledTypes
{
/// <inheritdoc />
protected override void BuildTargetModel(ModelBuilder modelBuilder)
{
#pragma warning disable 612, 618
modelBuilder.HasAnnotation("ProductVersion", "10.0.10");
modelBuilder.Entity("DodoSSH.Client.Storage.CachedItemRow", b =>
{
b.Property<Guid>("VaultId")
.HasColumnType("TEXT")
.HasColumnName("vault_id");
b.Property<int>("EntityType")
.HasColumnType("INTEGER")
.HasColumnName("entity_type");
b.Property<Guid>("EntityId")
.HasColumnType("TEXT")
.HasColumnName("entity_id");
b.Property<byte>("AadVersion")
.HasColumnType("INTEGER")
.HasColumnName("aad_version");
b.Property<long>("ChangeSequence")
.HasColumnType("INTEGER")
.HasColumnName("change_sequence");
b.Property<Guid?>("DataKeyId")
.HasColumnType("TEXT")
.HasColumnName("data_key_id");
b.Property<bool>("IsDeleted")
.HasColumnType("INTEGER")
.HasColumnName("is_deleted");
b.Property<uint>("KeyGeneration")
.HasColumnType("INTEGER")
.HasColumnName("key_generation");
b.Property<byte[]>("Payload")
.HasColumnType("BLOB")
.HasColumnName("payload");
b.Property<byte[]>("ProtectedFields")
.HasColumnType("BLOB")
.HasColumnName("protected_fields");
b.Property<long>("UpdatedAtUtc")
.HasColumnType("INTEGER")
.HasColumnName("updated_at_utc");
b.Property<int>("Version")
.HasColumnType("INTEGER")
.HasColumnName("version");
b.Property<byte[]>("WrappedDataKey")
.HasColumnType("BLOB")
.HasColumnName("wrapped_data_key");
b.HasKey("VaultId", "EntityType", "EntityId")
.HasName("pk_item");
b.HasIndex("VaultId", "ChangeSequence")
.HasDatabaseName("ix_item_vault_id_change_sequence");
b.HasIndex("VaultId", "EntityType")
.HasDatabaseName("ix_item_vault_id_entity_type");
b.ToTable("item", (string)null);
});
modelBuilder.Entity("DodoSSH.Client.Storage.CachedVaultKeyWrapRow", b =>
{
b.Property<Guid>("VaultId")
.HasColumnType("TEXT")
.HasColumnName("vault_id");
b.Property<uint>("KeyGeneration")
.HasColumnType("INTEGER")
.HasColumnName("key_generation");
b.Property<byte[]>("WrappedKey")
.IsRequired()
.HasColumnType("BLOB")
.HasColumnName("wrapped_key");
b.HasKey("VaultId", "KeyGeneration")
.HasName("pk_vault_key_wrap");
b.ToTable("vault_key_wrap", (string)null);
});
modelBuilder.Entity("DodoSSH.Client.Storage.CachedVaultRow", b =>
{
b.Property<Guid>("VaultId")
.HasColumnType("TEXT")
.HasColumnName("vault_id");
b.Property<bool>("Hidden")
.HasColumnType("INTEGER")
.HasColumnName("hidden");
b.Property<bool>("IsPersonal")
.HasColumnType("INTEGER")
.HasColumnName("is_personal");
b.Property<uint>("KeyGeneration")
.HasColumnType("INTEGER")
.HasColumnName("key_generation");
b.Property<string>("Name")
.IsRequired()
.HasColumnType("TEXT")
.HasColumnName("name");
b.Property<int>("Permissions")
.HasColumnType("INTEGER")
.HasColumnName("permissions");
b.Property<bool>("RekeyRequired")
.HasColumnType("INTEGER")
.HasColumnName("rekey_required");
b.Property<Guid?>("TeamId")
.HasColumnType("TEXT")
.HasColumnName("team_id");
b.Property<long>("UpdatedAtUtc")
.HasColumnType("INTEGER")
.HasColumnName("updated_at_utc");
b.Property<byte[]>("WrappedVaultKey")
.HasColumnType("BLOB")
.HasColumnName("wrapped_vault_key");
b.HasKey("VaultId")
.HasName("pk_vault");
b.ToTable("vault", (string)null);
});
modelBuilder.Entity("DodoSSH.Client.Storage.ConflictRow", b =>
{
b.Property<Guid>("Id")
.HasColumnType("TEXT")
.HasColumnName("id");
b.Property<bool>("Acknowledged")
.HasColumnType("INTEGER")
.HasColumnName("acknowledged");
b.Property<byte[]>("Detail")
.IsRequired()
.HasColumnType("BLOB")
.HasColumnName("detail");
b.Property<long>("DetectedAtUtc")
.HasColumnType("INTEGER")
.HasColumnName("detected_at_utc");
b.Property<Guid>("EntityId")
.HasColumnType("TEXT")
.HasColumnName("entity_id");
b.Property<int>("EntityType")
.HasColumnType("INTEGER")
.HasColumnName("entity_type");
b.Property<int>("Kind")
.HasColumnType("INTEGER")
.HasColumnName("kind");
b.Property<Guid>("VaultId")
.HasColumnType("TEXT")
.HasColumnName("vault_id");
b.HasKey("Id")
.HasName("pk_conflict");
b.HasIndex("VaultId", "Acknowledged")
.HasDatabaseName("ix_conflict_vault_id_acknowledged");
b.HasIndex("VaultId", "EntityType", "EntityId")
.HasDatabaseName("ix_conflict_vault_id_entity_type_entity_id");
b.ToTable("conflict", (string)null);
});
modelBuilder.Entity("DodoSSH.Client.Storage.OutboxRow", b =>
{
b.Property<long>("Sequence")
.ValueGeneratedOnAdd()
.HasColumnType("INTEGER")
.HasColumnName("sequence");
b.Property<byte>("AadVersion")
.HasColumnType("INTEGER")
.HasColumnName("aad_version");
b.Property<byte?>("AncestorAadVersion")
.HasColumnType("INTEGER")
.HasColumnName("ancestor_aad_version");
b.Property<Guid?>("AncestorDataKeyId")
.HasColumnType("TEXT")
.HasColumnName("ancestor_data_key_id");
b.Property<uint?>("AncestorKeyGeneration")
.HasColumnType("INTEGER")
.HasColumnName("ancestor_key_generation");
b.Property<byte[]>("AncestorPayload")
.HasColumnType("BLOB")
.HasColumnName("ancestor_payload");
b.Property<byte[]>("AncestorProtectedFields")
.HasColumnType("BLOB")
.HasColumnName("ancestor_protected_fields");
b.Property<int?>("AncestorVersion")
.HasColumnType("INTEGER")
.HasColumnName("ancestor_version");
b.Property<byte[]>("AncestorWrappedDataKey")
.HasColumnType("BLOB")
.HasColumnName("ancestor_wrapped_data_key");
b.Property<int>("Attempts")
.HasColumnType("INTEGER")
.HasColumnName("attempts");
b.Property<Guid?>("DataKeyId")
.HasColumnType("TEXT")
.HasColumnName("data_key_id");
b.Property<Guid>("EntityId")
.HasColumnType("TEXT")
.HasColumnName("entity_id");
b.Property<int>("EntityType")
.HasColumnType("INTEGER")
.HasColumnName("entity_type");
b.Property<int?>("ExpectedVersion")
.HasColumnType("INTEGER")
.HasColumnName("expected_version");
b.Property<bool>("IsParked")
.HasColumnType("INTEGER")
.HasColumnName("is_parked");
b.Property<uint>("KeyGeneration")
.HasColumnType("INTEGER")
.HasColumnName("key_generation");
b.Property<string>("LastError")
.HasColumnType("TEXT")
.HasColumnName("last_error");
b.Property<int>("Operation")
.HasColumnType("INTEGER")
.HasColumnName("operation");
b.Property<Guid>("OperationId")
.HasColumnType("TEXT")
.HasColumnName("operation_id");
b.Property<byte[]>("Payload")
.HasColumnType("BLOB")
.HasColumnName("payload");
b.Property<byte[]>("ProtectedFields")
.HasColumnType("BLOB")
.HasColumnName("protected_fields");
b.Property<long>("QueuedAtUtc")
.HasColumnType("INTEGER")
.HasColumnName("queued_at_utc");
b.Property<Guid>("VaultId")
.HasColumnType("TEXT")
.HasColumnName("vault_id");
b.Property<byte[]>("WrappedDataKey")
.HasColumnType("BLOB")
.HasColumnName("wrapped_data_key");
b.HasKey("Sequence")
.HasName("pk_outbox");
b.HasIndex("OperationId")
.IsUnique()
.HasDatabaseName("ix_outbox_operation_id");
b.HasIndex("VaultId", "EntityType", "EntityId")
.IsUnique()
.HasDatabaseName("ix_outbox_vault_id_entity_type_entity_id");
b.HasIndex("VaultId", "IsParked", "Sequence")
.HasDatabaseName("ix_outbox_vault_id_is_parked_sequence");
b.ToTable("outbox", (string)null);
});
modelBuilder.Entity("DodoSSH.Client.Storage.RememberedSignInRow", b =>
{
b.Property<int>("Id")
.HasColumnType("INTEGER")
.HasColumnName("id");
b.Property<byte[]>("SealedRefreshToken")
.IsRequired()
.HasColumnType("BLOB")
.HasColumnName("sealed_refresh_token");
b.Property<long>("UpdatedAtUtc")
.HasColumnType("INTEGER")
.HasColumnName("updated_at_utc");
b.HasKey("Id")
.HasName("pk_remembered_sign_in");
b.ToTable("remembered_sign_in", null, t =>
{
t.HasCheckConstraint("ck_remembered_sign_in_singleton", "id = 1");
});
});
modelBuilder.Entity("DodoSSH.Client.Storage.SyncStateRow", b =>
{
b.Property<Guid>("VaultId")
.HasColumnType("TEXT")
.HasColumnName("vault_id");
b.Property<string>("Cursor")
.HasColumnType("TEXT")
.HasColumnName("cursor");
b.Property<uint>("KeyGeneration")
.HasColumnType("INTEGER")
.HasColumnName("key_generation");
b.Property<long?>("LastPulledAtUtc")
.HasColumnType("INTEGER")
.HasColumnName("last_pulled_at_utc");
b.Property<long?>("LastPushedAtUtc")
.HasColumnType("INTEGER")
.HasColumnName("last_pushed_at_utc");
b.Property<string>("PulledTypes")
.HasColumnType("TEXT")
.HasColumnName("pulled_types");
b.Property<long>("ServerTimeSkewMs")
.HasColumnType("INTEGER")
.HasColumnName("server_time_skew_ms");
b.HasKey("VaultId")
.HasName("pk_sync_state");
b.ToTable("sync_state", (string)null);
});
modelBuilder.Entity("DodoSSH.Client.Storage.UnlockMaterialRow", b =>
{
b.Property<int>("Id")
.HasColumnType("INTEGER")
.HasColumnName("id");
b.Property<Guid?>("DeviceId")
.HasColumnType("TEXT")
.HasColumnName("device_id");
b.Property<byte[]>("DeviceWrappedPrivateKey")
.HasColumnType("BLOB")
.HasColumnName("device_wrapped_private_key");
b.Property<string>("DisplayName")
.HasColumnType("TEXT")
.HasColumnName("display_name");
b.Property<string>("Email")
.HasColumnType("TEXT")
.HasColumnName("email");
b.Property<string>("Issuer")
.IsRequired()
.HasColumnType("TEXT")
.HasColumnName("issuer");
b.Property<string>("KdfAlgorithm")
.IsRequired()
.HasColumnType("TEXT")
.HasColumnName("kdf_algorithm");
b.Property<int>("KdfMemoryKibibytes")
.HasColumnType("INTEGER")
.HasColumnName("kdf_memory_kibibytes");
b.Property<int>("KdfParallelism")
.HasColumnType("INTEGER")
.HasColumnName("kdf_parallelism");
b.Property<int>("KdfPasses")
.HasColumnType("INTEGER")
.HasColumnName("kdf_passes");
b.Property<byte[]>("KdfSalt")
.IsRequired()
.HasColumnType("BLOB")
.HasColumnName("kdf_salt");
b.Property<uint>("KeyGeneration")
.HasColumnType("INTEGER")
.HasColumnName("key_generation");
b.Property<string>("ServerUrl")
.IsRequired()
.HasColumnType("TEXT")
.HasColumnName("server_url");
b.Property<string>("Subject")
.IsRequired()
.HasColumnType("TEXT")
.HasColumnName("subject");
b.Property<long>("UpdatedAtUtc")
.HasColumnType("INTEGER")
.HasColumnName("updated_at_utc");
b.Property<Guid>("UserId")
.HasColumnType("TEXT")
.HasColumnName("user_id");
b.Property<byte[]>("WrappedPrivateKey")
.IsRequired()
.HasColumnType("BLOB")
.HasColumnName("wrapped_private_key");
b.HasKey("Id")
.HasName("pk_unlock_material");
b.ToTable("unlock_material", null, t =>
{
t.HasCheckConstraint("ck_unlock_material_singleton", "id = 1");
});
});
#pragma warning restore 612, 618
}
}
}
@@ -0,0 +1,28 @@
using Microsoft.EntityFrameworkCore.Migrations;
#nullable disable
namespace DodoSSH.Client.Storage.Migrations
{
/// <inheritdoc />
public partial class AddSyncStatePulledTypes : Migration
{
/// <inheritdoc />
protected override void Up(MigrationBuilder migrationBuilder)
{
migrationBuilder.AddColumn<string>(
name: "pulled_types",
table: "sync_state",
type: "TEXT",
nullable: true);
}
/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
migrationBuilder.DropColumn(
name: "pulled_types",
table: "sync_state");
}
}
}
@@ -362,6 +362,10 @@ namespace DodoSSH.Client.Storage.Migrations
.HasColumnType("INTEGER") .HasColumnType("INTEGER")
.HasColumnName("last_pushed_at_utc"); .HasColumnName("last_pushed_at_utc");
b.Property<string>("PulledTypes")
.HasColumnType("TEXT")
.HasColumnName("pulled_types");
b.Property<long>("ServerTimeSkewMs") b.Property<long>("ServerTimeSkewMs")
.HasColumnType("INTEGER") .HasColumnType("INTEGER")
.HasColumnName("server_time_skew_ms"); .HasColumnName("server_time_skew_ms");
+31 -10
View File
@@ -44,6 +44,14 @@ public sealed record QueuedChange(
/// ancestor that is this client's own earlier edit. That merge finds no disagreement, so it converges /// ancestor that is this client's own earlier edit. That merge finds no disagreement, so it converges
/// on the newest state with nothing for the user to arbitrate. /// on the newest state with nothing for the user to arbitrate.
/// </para> /// </para>
/// <para>
/// ◆ <b>Which is only true if an answer is applied to the operation it answers.</b> A push reads a row,
/// sends it, and acts on the reply some time later; a user who saves in between coalesces a new state into
/// that same row. So <see cref="CompleteAsync"/>, <see cref="ReviseAsync"/> and <see cref="ParkAsync"/>
/// take the operation that was sent and do nothing when the row has since become a different one. Keyed by
/// sequence alone, an acknowledgement of the earlier state deleted the row holding the later one, and the
/// edit was gone with nothing to say so.
/// </para>
/// </remarks> /// </remarks>
public sealed class OutboxStore( public sealed class OutboxStore(
IDbContextFactory<ClientCacheContext> contexts, IDbContextFactory<ClientCacheContext> contexts,
@@ -116,7 +124,7 @@ public sealed class OutboxStore(
/// </para> /// </para>
/// </remarks> /// </remarks>
public async Task<PendingOperation?> ReviseAsync( public async Task<PendingOperation?> ReviseAsync(
long sequence, PendingOperation revising,
SyncOperation operation, SyncOperation operation,
int? expectedVersion, int? expectedVersion,
EncryptedPayload? payload, EncryptedPayload? payload,
@@ -127,12 +135,18 @@ public sealed class OutboxStore(
var context = contexts.CreateDbContext(); var context = contexts.CreateDbContext();
await using var scope = context.ConfigureAwait(false); await using var scope = context.ConfigureAwait(false);
ArgumentNullException.ThrowIfNull(revising);
var row = await context.Set<OutboxRow>() var row = await context.Set<OutboxRow>()
.SingleOrDefaultAsync(r => r.Sequence == sequence, cancellationToken) .SingleOrDefaultAsync(
r => r.Sequence == revising.Sequence && r.OperationId == revising.OperationId,
cancellationToken)
.ConfigureAwait(false); .ConfigureAwait(false);
if (row is null) if (row is null)
{ {
// Completed, or replaced by a newer edit since it was read. Either way the merge was of a state
// that is no longer the one queued, and the next push will meet the server with the current one.
return null; return null;
} }
@@ -233,16 +247,19 @@ public sealed class OutboxStore(
/// <summary>Records that an operation has been sent, so a repeated failure can be noticed.</summary> /// <summary>Records that an operation has been sent, so a repeated failure can be noticed.</summary>
public Task MarkDispatchedAsync(long sequence, CancellationToken cancellationToken) => public Task MarkDispatchedAsync(long sequence, CancellationToken cancellationToken) =>
UpdateAsync(sequence, row => row.Attempts++, cancellationToken); UpdateAsync(sequence, operationId: null, row => row.Attempts++, cancellationToken);
/// <summary>Removes an operation the server accepted.</summary> /// <summary>Removes an operation the server accepted, unless a newer edit has replaced it since.</summary>
public async Task<bool> CompleteAsync(long sequence, CancellationToken cancellationToken) /// <returns>Whether a row was removed.</returns>
public async Task<bool> CompleteAsync(PendingOperation operation, CancellationToken cancellationToken)
{ {
ArgumentNullException.ThrowIfNull(operation);
var context = contexts.CreateDbContext(); var context = contexts.CreateDbContext();
await using var scope = context.ConfigureAwait(false); await using var scope = context.ConfigureAwait(false);
var removed = await context.Set<OutboxRow>() var removed = await context.Set<OutboxRow>()
.Where(r => r.Sequence == sequence) .Where(r => r.Sequence == operation.Sequence && r.OperationId == operation.OperationId)
.ExecuteDeleteAsync(cancellationToken) .ExecuteDeleteAsync(cancellationToken)
.ConfigureAwait(false); .ConfigureAwait(false);
@@ -257,9 +274,10 @@ public sealed class OutboxStore(
/// invalid, or the caller no longer having permission. Retrying either would spin forever and, far /// invalid, or the caller no longer having permission. Retrying either would spin forever and, far
/// worse, would block every change queued behind it in a vault the user can still write to. /// worse, would block every change queued behind it in a vault the user can still write to.
/// </remarks> /// </remarks>
public Task ParkAsync(long sequence, string reason, CancellationToken cancellationToken) => public Task ParkAsync(PendingOperation operation, string reason, CancellationToken cancellationToken) =>
UpdateAsync( UpdateAsync(
sequence, operation.Sequence,
operation.OperationId,
row => row =>
{ {
row.IsParked = true; row.IsParked = true;
@@ -269,7 +287,7 @@ public sealed class OutboxStore(
/// <summary>Records a transient failure without parking the operation.</summary> /// <summary>Records a transient failure without parking the operation.</summary>
public Task RecordFailureAsync(long sequence, string reason, CancellationToken cancellationToken) => public Task RecordFailureAsync(long sequence, string reason, CancellationToken cancellationToken) =>
UpdateAsync(sequence, row => row.LastError = reason, cancellationToken); UpdateAsync(sequence, operationId: null, row => row.LastError = reason, cancellationToken);
private static Task<OutboxRow?> FindRowAsync( private static Task<OutboxRow?> FindRowAsync(
ClientCacheContext context, ClientCacheContext context,
@@ -302,6 +320,7 @@ public sealed class OutboxStore(
private async Task UpdateAsync( private async Task UpdateAsync(
long sequence, long sequence,
Guid? operationId,
Action<OutboxRow> mutate, Action<OutboxRow> mutate,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
@@ -309,7 +328,9 @@ public sealed class OutboxStore(
await using var scope = context.ConfigureAwait(false); await using var scope = context.ConfigureAwait(false);
var row = await context.Set<OutboxRow>() var row = await context.Set<OutboxRow>()
.SingleOrDefaultAsync(r => r.Sequence == sequence, cancellationToken) .SingleOrDefaultAsync(
r => r.Sequence == sequence && (operationId == null || r.OperationId == operationId),
cancellationToken)
.ConfigureAwait(false); .ConfigureAwait(false);
if (row is null) if (row is null)
+3 -1
View File
@@ -183,13 +183,15 @@ public sealed record PendingOperation(
/// <param name="LastPulledAt">When the last pull completed.</param> /// <param name="LastPulledAt">When the last pull completed.</param>
/// <param name="LastPushedAt">When the last push completed.</param> /// <param name="LastPushedAt">When the last push completed.</param>
/// <param name="ServerTimeSkewMs">Observed clock difference, recorded and never acted on.</param> /// <param name="ServerTimeSkewMs">Observed clock difference, recorded and never acted on.</param>
/// <param name="PulledTypes">The item types the cursor was read under; see <c>SyncStateRow.PulledTypes</c>.</param>
public sealed record StoredSyncState( public sealed record StoredSyncState(
Guid VaultId, Guid VaultId,
string? Cursor, string? Cursor,
uint KeyGeneration, uint KeyGeneration,
DateTimeOffset? LastPulledAt = null, DateTimeOffset? LastPulledAt = null,
DateTimeOffset? LastPushedAt = null, DateTimeOffset? LastPushedAt = null,
long ServerTimeSkewMs = 0); long ServerTimeSkewMs = 0,
string? PulledTypes = null);
/// <summary>Something the merge overrode, or an item that could not be processed.</summary> /// <summary>Something the merge overrode, or an item that could not be processed.</summary>
/// <param name="Id">This record's own id, which its sealed detail is bound to.</param> /// <param name="Id">This record's own id, which its sealed detail is bound to.</param>
+3 -1
View File
@@ -46,7 +46,8 @@ public sealed class SyncStateStore(IDbContextFactory<ClientCacheContext> context
row.KeyGeneration, row.KeyGeneration,
row.LastPulledAtUtc, row.LastPulledAtUtc,
row.LastPushedAtUtc, row.LastPushedAtUtc,
row.ServerTimeSkewMs); row.ServerTimeSkewMs,
row.PulledTypes);
} }
/// <summary>Records a vault's position.</summary> /// <summary>Records a vault's position.</summary>
@@ -72,6 +73,7 @@ public sealed class SyncStateStore(IDbContextFactory<ClientCacheContext> context
row.LastPulledAtUtc = state.LastPulledAt; row.LastPulledAtUtc = state.LastPulledAt;
row.LastPushedAtUtc = state.LastPushedAt; row.LastPushedAtUtc = state.LastPushedAt;
row.ServerTimeSkewMs = state.ServerTimeSkewMs; row.ServerTimeSkewMs = state.ServerTimeSkewMs;
row.PulledTypes = state.PulledTypes;
await context.SaveChangesAsync(cancellationToken).ConfigureAwait(false); await context.SaveChangesAsync(cancellationToken).ConfigureAwait(false);
} }
+8 -8
View File
@@ -120,7 +120,7 @@ internal sealed class ItemReconciler<TSecret>(
{ {
return pending.Operation == SyncOperation.Delete return pending.Operation == SyncOperation.Delete
// Both sides deleted it. Nothing to arbitrate and nothing to tell the user. // Both sides deleted it. Nothing to arbitrate and nothing to tell the user.
? outbox.CompleteAsync(pending.Sequence, cancellationToken) ? outbox.CompleteAsync(pending, cancellationToken)
: ResurrectAsync(vaultId, remote, pending, report, cancellationToken); : ResurrectAsync(vaultId, remote, pending, report, cancellationToken);
} }
@@ -166,7 +166,7 @@ internal sealed class ItemReconciler<TSecret>(
} }
await outbox.ReviseAsync( await outbox.ReviseAsync(
pending.Sequence, pending,
SyncOperation.Upsert, SyncOperation.Upsert,
expectedVersion: null, expectedVersion: null,
kind.Seal(local.Secret, vaultKey.Span, pending.EntityId, generation, itemVersion: 1), kind.Seal(local.Secret, vaultKey.Span, pending.EntityId, generation, itemVersion: 1),
@@ -212,7 +212,7 @@ internal sealed class ItemReconciler<TSecret>(
{ {
// Field for field the same item: this is our own create coming back. Nothing to do but stop // Field for field the same item: this is our own create coming back. Nothing to do but stop
// trying to send it again. // trying to send it again.
await outbox.CompleteAsync(pending.Sequence, cancellationToken).ConfigureAwait(false); await outbox.CompleteAsync(pending, cancellationToken).ConfigureAwait(false);
return; return;
} }
@@ -362,7 +362,7 @@ internal sealed class ItemReconciler<TSecret>(
cancellationToken).ConfigureAwait(false); cancellationToken).ConfigureAwait(false);
// Now the tombstone can stand. // Now the tombstone can stand.
await outbox.CompleteAsync(pending.Sequence, cancellationToken).ConfigureAwait(false); await outbox.CompleteAsync(pending, cancellationToken).ConfigureAwait(false);
await conflicts.RecordAsync( await conflicts.RecordAsync(
vaultId, vaultId,
@@ -385,7 +385,7 @@ internal sealed class ItemReconciler<TSecret>(
SyncReportBuilder report, SyncReportBuilder report,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
await outbox.CompleteAsync(pending.Sequence, cancellationToken).ConfigureAwait(false); await outbox.CompleteAsync(pending, cancellationToken).ConfigureAwait(false);
await conflicts.RecordAsync( await conflicts.RecordAsync(
vaultId, vaultId,
@@ -413,7 +413,7 @@ internal sealed class ItemReconciler<TSecret>(
var nextVersion = SyncVersions.NextVersion(remote.Version); var nextVersion = SyncVersions.NextVersion(remote.Version);
await outbox.ReviseAsync( await outbox.ReviseAsync(
pending.Sequence, pending,
SyncOperation.Upsert, SyncOperation.Upsert,
expectedVersion: remote.Version, expectedVersion: remote.Version,
kind.Seal(secret, vaultKey.Span, remote.EntityId, generation, nextVersion), kind.Seal(secret, vaultKey.Span, remote.EntityId, generation, nextVersion),
@@ -486,7 +486,7 @@ internal sealed class ItemReconciler<TSecret>(
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
await outbox.ParkAsync( await outbox.ParkAsync(
pending.Sequence, pending,
$"Written by a newer version of DodoSSH; update before editing this {kind.Noun}.", $"Written by a newer version of DodoSSH; update before editing this {kind.Noun}.",
cancellationToken).ConfigureAwait(false); cancellationToken).ConfigureAwait(false);
@@ -512,7 +512,7 @@ internal sealed class ItemReconciler<TSecret>(
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
await outbox.ParkAsync( await outbox.ParkAsync(
pending.Sequence, pending,
$"The local or the server copy of this {kind.Noun} could not be decrypted.", $"The local or the server copy of this {kind.Noun} could not be decrypted.",
cancellationToken).ConfigureAwait(false); cancellationToken).ConfigureAwait(false);
+99 -5
View File
@@ -127,7 +127,7 @@ public sealed class SyncEngine
SyncReportBuilder report, SyncReportBuilder report,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
var state = await syncState.ReadAsync(vaultId, cancellationToken).ConfigureAwait(false); var state = await ReadStateAsync(vaultId, report, cancellationToken).ConfigureAwait(false);
// At most one restart per pull. A server that rejects the cursor it has just issued is not // At most one restart per pull. A server that rejects the cursor it has just issued is not
// telling this client anything it can act on, and replaying the whole log against it would turn // telling this client anything it can act on, and replaying the whole log against it would turn
@@ -245,6 +245,26 @@ public sealed class SyncEngine
/// the beginning as well, rather than meeting the same refusal again. /// the beginning as well, rather than meeting the same refusal again.
/// </para> /// </para>
/// </remarks> /// </remarks>
/// <summary>Where the pull should carry on from, which is the start when the types it reads have changed.</summary>
/// <remarks>
/// A cursor read under a different list of types is past changes of the types it did not ask for, so an
/// upgrade that adds one — SpelunkKey, ObjectStore — would never receive the items of that type made
/// before it. Starting over once is the only way to see them; applying is idempotent, which is what makes
/// the restart for a refused cursor safe as well. A state from before the list was kept has none and
/// starts over once for the same reason: it may already have missed some.
/// </remarks>
private async Task<StoredSyncState> ReadStateAsync(
Guid vaultId,
SyncReportBuilder report,
CancellationToken cancellationToken)
{
var state = await syncState.ReadAsync(vaultId, cancellationToken).ConfigureAwait(false);
return state.Cursor is not null && !string.Equals(state.PulledTypes, PulledTypes, StringComparison.Ordinal)
? await StartOverAsync(state, report, cancellationToken).ConfigureAwait(false)
: state;
}
private async Task<StoredSyncState> StartOverAsync( private async Task<StoredSyncState> StartOverAsync(
StoredSyncState state, StoredSyncState state,
SyncReportBuilder report, SyncReportBuilder report,
@@ -282,9 +302,14 @@ public sealed class SyncEngine
KeyGeneration = response.CurrentKeyGeneration, KeyGeneration = response.CurrentKeyGeneration,
LastPulledAt = now, LastPulledAt = now,
ServerTimeSkewMs = skew, ServerTimeSkewMs = skew,
PulledTypes = PulledTypes,
}; };
} }
/// <summary>The types this build pulls, in a form a stored state can be compared against.</summary>
private static string PulledTypes { get; } =
string.Join(',', ItemKinds.SyncedTypes.Select(type => (int)type).Order());
private async Task ApplyAsync( private async Task ApplyAsync(
Guid vaultId, Guid vaultId,
SyncChange change, SyncChange change,
@@ -446,7 +471,7 @@ public sealed class SyncEngine
} }
await outbox.ReviseAsync( await outbox.ReviseAsync(
operation.Sequence, operation,
operation.Operation, operation.Operation,
operation.ExpectedVersion, operation.ExpectedVersion,
resealed, resealed,
@@ -551,7 +576,76 @@ public sealed class SyncEngine
cancellationToken).ConfigureAwait(false); cancellationToken).ConfigureAwait(false);
} }
await outbox.CompleteAsync(operation.Sequence, cancellationToken).ConfigureAwait(false); var completed = await outbox.CompleteAsync(operation, cancellationToken).ConfigureAwait(false);
if (!completed && canMirror && !isDelete)
{
await RebaseOntoAcceptedAsync(vaultId, operation, version, cancellationToken).ConfigureAwait(false);
}
}
/// <summary>
/// Moves an edit made while <paramref name="accepted"/> was in flight onto the version that push produced.
/// </summary>
/// <remarks>
/// <para>
/// The user saved again between the push being read and its answer arriving, so the row now holds a
/// later state branched from the same base as the one the server just took. Left alone it is still the
/// user's edit and still goes — but it goes expecting the old version, comes back <c>Conflict</c>, and is
/// merged against an ancestor that predates the save the server already has, which makes every field
/// changed twice look like a disagreement with somebody else. Its true parent is the state just accepted,
/// so that becomes its ancestor and expected version, and its payload is re-sealed at the version it
/// will now produce, because the AAD binds that.
/// </para>
/// <para>
/// Only for a row still branched from the same base. One that is not has been revised by something else
/// since, and the ordinary conflict path is the one that knows what to do with it; so is a row whose
/// payload this session cannot re-seal.
/// </para>
/// </remarks>
private async Task RebaseOntoAcceptedAsync(
Guid vaultId,
PendingOperation accepted,
int version,
CancellationToken cancellationToken)
{
var later = await outbox
.FindAsync(vaultId, accepted.EntityType, accepted.EntityId, cancellationToken)
.ConfigureAwait(false);
if (later is null || later.ExpectedVersion != accepted.ExpectedVersion || accepted.Payload is null)
{
return;
}
EncryptedPayload? payload = null;
if (later.Payload is { } queued)
{
payload = PayloadReseal.TryReseal(
keyring,
vaultId,
later.EntityType,
later.EntityId,
queued,
openAtVersion: SyncVersions.NextVersion(later.ExpectedVersion),
sealAtVersion: SyncVersions.NextVersion(version));
if (payload is null)
{
return;
}
}
await outbox.ReviseAsync(
later,
later.Operation,
version,
payload,
later.Fields,
new StoredAncestor(version, accepted.Payload, accepted.Fields),
cancellationToken)
.ConfigureAwait(false);
} }
private async Task<bool> ResolveAsync( private async Task<bool> ResolveAsync(
@@ -617,7 +711,7 @@ public sealed class SyncEngine
if (operation.Operation == SyncOperation.Delete) if (operation.Operation == SyncOperation.Delete)
{ {
// Nothing there to delete, so the intent is already satisfied. // Nothing there to delete, so the intent is already satisfied.
await outbox.CompleteAsync(operation.Sequence, cancellationToken).ConfigureAwait(false); await outbox.CompleteAsync(operation, cancellationToken).ConfigureAwait(false);
return false; return false;
} }
@@ -645,7 +739,7 @@ public sealed class SyncEngine
SyncReportBuilder report, SyncReportBuilder report,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
await outbox.ParkAsync(operation.Sequence, reason, cancellationToken).ConfigureAwait(false); await outbox.ParkAsync(operation, reason, cancellationToken).ConfigureAwait(false);
await conflicts.RecordAsync( await conflicts.RecordAsync(
vaultId, vaultId,
@@ -327,7 +327,7 @@ internal sealed class VaultItemRepository<TSecret>(
if (pending is not null && NeverReachedTheServer(pending)) if (pending is not null && NeverReachedTheServer(pending))
{ {
await outbox.CompleteAsync(pending.Sequence, cancellationToken).ConfigureAwait(false); await outbox.CompleteAsync(pending, cancellationToken).ConfigureAwait(false);
// Recorded even though nothing goes to the server. Somebody created an item and then removed it, // Recorded even though nothing goes to the server. Somebody created an item and then removed it,
// which is two things they did — and a log that showed only the create would describe a keychain // which is two things they did — and a log that showed only the create would describe a keychain
@@ -173,7 +173,7 @@ public sealed class OutboxStoreTests : IAsyncLifetime
var merged = new StoredAncestor(4, Payload(seed: 20), new SyncPlaintextFields()); var merged = new StoredAncestor(4, Payload(seed: 20), new SyncPlaintextFields());
var revised = await harness.Outbox.ReviseAsync( var revised = await harness.Outbox.ReviseAsync(
queued.Sequence, queued,
SyncOperation.Upsert, SyncOperation.Upsert,
expectedVersion: 4, expectedVersion: 4,
Payload(seed: 30), Payload(seed: 30),
@@ -199,7 +199,7 @@ public sealed class OutboxStoreTests : IAsyncLifetime
var queued = await harness.Outbox.QueueAsync(Change(parked), Token); var queued = await harness.Outbox.QueueAsync(Change(parked), Token);
await harness.Outbox.QueueAsync(Change(healthy), Token); await harness.Outbox.QueueAsync(Change(healthy), Token);
await harness.Outbox.ParkAsync(queued.Sequence, "Entity type not supported.", Token); await harness.Outbox.ParkAsync(queued, "Entity type not supported.", Token);
var pending = await harness.Outbox.TakeAsync(VaultId, 10, Token); var pending = await harness.Outbox.TakeAsync(VaultId, 10, Token);
pending.ShouldHaveSingleItem().EntityId.ShouldBe(healthy); pending.ShouldHaveSingleItem().EntityId.ShouldBe(healthy);
@@ -216,7 +216,7 @@ public sealed class OutboxStoreTests : IAsyncLifetime
var entityId = Guid.CreateVersion7(); var entityId = Guid.CreateVersion7();
var queued = await harness.Outbox.QueueAsync(Change(entityId), Token); var queued = await harness.Outbox.QueueAsync(Change(entityId), Token);
await harness.Outbox.ParkAsync(queued.Sequence, "nope", Token); await harness.Outbox.ParkAsync(queued, "nope", Token);
var requeued = await harness.Outbox.QueueAsync(Change(entityId, seed: 5), Token); var requeued = await harness.Outbox.QueueAsync(Change(entityId, seed: 5), Token);
@@ -230,11 +230,42 @@ public sealed class OutboxStoreTests : IAsyncLifetime
{ {
var queued = await harness.Outbox.QueueAsync(Change(Guid.CreateVersion7()), Token); var queued = await harness.Outbox.QueueAsync(Change(Guid.CreateVersion7()), Token);
(await harness.Outbox.CompleteAsync(queued.Sequence, Token)).ShouldBeTrue(); (await harness.Outbox.CompleteAsync(queued, Token)).ShouldBeTrue();
(await harness.Outbox.TakeAsync(VaultId, 10, Token)).ShouldBeEmpty(); (await harness.Outbox.TakeAsync(VaultId, 10, Token)).ShouldBeEmpty();
// Idempotent: a drain that retries after a crash must not fail on an already-cleared row. // Idempotent: a drain that retries after a crash must not fail on an already-cleared row.
(await harness.Outbox.CompleteAsync(queued.Sequence, Token)).ShouldBeFalse(); (await harness.Outbox.CompleteAsync(queued, Token)).ShouldBeFalse();
}
/// <summary>
/// An answer to an operation that has since been replaced by a newer edit applies to nothing.
/// </summary>
/// <remarks>
/// The newer edit coalesced into the same row, so the sequence still matches; only the operation id says
/// the row is no longer the one that was sent. Completing, revising or parking it would act on the newer
/// edit on the strength of an answer about the older one.
/// </remarks>
[Fact]
public async Task AnAnswerToAReplacedOperation_LeavesTheNewerEditAlone()
{
var entityId = Guid.CreateVersion7();
var sent = await harness.Outbox.QueueAsync(Change(entityId, seed: 1), Token);
var newer = await harness.Outbox.QueueAsync(Change(entityId, seed: 2), Token);
newer.Sequence.ShouldBe(sent.Sequence, "the newer edit coalesces into the same row");
(await harness.Outbox.CompleteAsync(sent, Token)).ShouldBeFalse();
await harness.Outbox.ParkAsync(sent, "refused", Token);
(await harness.Outbox.ReviseAsync(
sent, sent.Operation, sent.ExpectedVersion, sent.Payload, sent.Fields, sent.Ancestor, Token))
.ShouldBeNull();
var left = (await harness.Outbox.TakeAsync(VaultId, 10, Token)).ShouldHaveSingleItem();
left.OperationId.ShouldBe(newer.OperationId);
left.IsParked.ShouldBeFalse();
} }
[Fact] [Fact]
@@ -231,4 +231,68 @@ public sealed class SyncEngineTests
async () => await harness.First.Hosts.ListAsync( async () => await harness.First.Hosts.ListAsync(
unknown, TestContext.Current.CancellationToken)); unknown, TestContext.Current.CancellationToken));
} }
/// <summary>
/// A save made while the previous save of the same host is on its way to the server survives that push
/// being acknowledged, and reaches the server as the newer state rather than as a conflict.
/// </summary>
/// <remarks>
/// The outbox coalesces the second save into the row the push read. Acknowledging by sequence alone
/// deleted that row — the newer state with it — and the server and both machines kept the older one.
/// </remarks>
[Fact]
public async Task ASaveMadeWhileAPushIsInFlight_IsNotLostWhenThatPushIsAcknowledged()
{
using var harness = await CreateAsync();
var id = await harness.First.CreateAsync(Host("prod-db", hostname: "first.internal"));
await harness.First.SyncAsync();
await harness.First.UpdateAsync(id, Host("prod-db", hostname: "second.internal"));
harness.Server.OnPush = () => harness.First
.UpdateAsync(id, Host("prod-db", hostname: "third.internal"))
.GetAwaiter()
.GetResult();
await harness.First.SyncAsync();
await harness.First.SyncAsync();
(await harness.First.FindAsync(id)).Secret.Hostname.ShouldBe("third.internal");
(await harness.First.ConflictsAsync()).ShouldBeEmpty();
await harness.Second.SyncAsync();
(await harness.Second.FindAsync(id)).Secret.Hostname.ShouldBe("third.internal");
}
/// <summary>
/// A cursor read under a shorter list of item types starts the pull over, once.
/// </summary>
/// <remarks>
/// The cursor a pull returns is past every change it read, including the changes of types it did not ask
/// for. A build that adds a type to the list — SpelunkKey did, ObjectStore before it — and carried on from
/// that cursor would never receive the items of the new type made before it arrived.
/// </remarks>
[Fact]
public async Task ACursorReadUnderAnotherListOfTypes_StartsThePullOverOnce()
{
using var harness = await CreateAsync();
await harness.First.CreateAsync(Host("prod-db"));
await harness.First.SyncAsync();
var state = await harness.First.SyncState.ReadAsync(VaultId, TestContext.Current.CancellationToken);
state.Cursor.ShouldNotBeNull();
state.PulledTypes.ShouldNotBeNull();
// What an older build, which pulled fewer types, would have left behind.
await harness.First.SyncState.SaveAsync(
state with { PulledTypes = "1" }, TestContext.Current.CancellationToken);
(await harness.First.SyncAsync()).ResyncedFromStart.ShouldBeTrue();
(await harness.First.SyncAsync()).ResyncedFromStart.ShouldBeFalse("the list is recorded with the cursor");
(await harness.First.ListAsync()).Items.ShouldHaveSingleItem();
}
} }