Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
using System;
using System.Linq;
using System.Threading.Tasks;
using JasperFx.Events;
using JasperFx.Events.Projections;
using Marten;
using Marten.Events;
using Marten.Events.Aggregation;
using Marten.Metadata;
using Marten.Testing.Harness;
using Shouldly;
using Xunit;

namespace EventSourcingTests.Bugs;

/// <summary>
/// <c>Query&lt;T&gt;()</c> throws <see cref="IndexOutOfRangeException"/> ("Ordinal must be
/// between 0 and 1") when a document is BOTH tenant-mapped (the <c>tenant_id</c> metadata
/// column is projected onto a member — via <see cref="ITenanted"/> or
/// <c>.Metadata(m =&gt; m.TenantId.MapTo(...))</c>) AND a projection/aggregate target (so it is
/// optimistic-concurrency-versioned with no <c>[Version]</c> member).
/// </summary>
/// <remarks>
/// Either feature alone is fine (see the two <c>control_*</c> tests), and <c>LoadAsync&lt;T&gt;()</c>
/// is unaffected — only the LINQ path breaks. Likely cause: <c>DocumentStorageDescriptorBuilder.Build</c>
/// adds a version read-binder whenever <c>UseOptimisticConcurrency</c> is set, without the
/// <c>storageStyle != QueryOnly</c> guard that <c>VersionColumn.ShouldSelect</c> applies. The single
/// descriptor is shared across storage styles (<c>ClosedShapeRegistration.BuildProvider</c>), so the
/// <c>QueryOnly</c> selector ends up with one more metadata read-binder than its SELECT projects,
/// shifting every later ordinal so the tenant binder reads past the end of the row.
/// </remarks>
public partial class Bug_4602_query_tenant_mapped_projection_document : BugIntegrationContext
{
[Fact]
public async Task can_query_an_ITenanted_projection_document()
{
StoreOptions(opts =>
{
opts.Events.TenancyStyle = TenancyStyle.Conjoined;
opts.Projections.Add<TenantedProjection>(ProjectionLifecycle.Inline);
});

var id = Guid.NewGuid();
await using (var session = theStore.LightweightSession("tenant-a"))
{
session.Events.StartStream<TenantedDoc>(id, new TenantedCreated(id, "tenanted"));
await session.SaveChangesAsync();
}

await using var query = theStore.QuerySession("tenant-a");

// sanity: the load-by-id path works
(await query.LoadAsync<TenantedDoc>(id)).ShouldNotBeNull();

// bug: throws IndexOutOfRangeException: "Ordinal must be between 0 and 1"
var docs = await query.Query<TenantedDoc>().ToListAsync();
docs.Count.ShouldBe(1);
}

[Fact]
public async Task can_query_a_MapTo_projection_document()
{
StoreOptions(opts =>
{
opts.Events.TenancyStyle = TenancyStyle.Conjoined;
opts.Projections.Add<MappedProjection>(ProjectionLifecycle.Inline);
opts.Schema.For<MappedDoc>()
.MultiTenanted()
.Metadata(m => m.TenantId.MapTo(x => x.Tenant));
});

var id = Guid.NewGuid();
await using (var session = theStore.LightweightSession("tenant-a"))
{
session.Events.StartStream<MappedDoc>(id, new MappedCreated(id, "mapped"));
await session.SaveChangesAsync();
}

await using var query = theStore.QuerySession("tenant-a");

(await query.LoadAsync<MappedDoc>(id)).ShouldNotBeNull();

var docs = await query.Query<MappedDoc>().ToListAsync();
docs.Count.ShouldBe(1);
}

[Fact]
public async Task workaround_IRevisioned_member_realigns_the_select()
{
// Userland escape hatch — and the sharpest confirmation of the root cause.
// A projection target gets UseNumericRevisions=true with no revision member, so the
// QueryOnly SELECT omits mt_version while the shared descriptor still binds it (the bug).
// Implementing IRevisioned gives the revision column an actual member, so
// RevisionColumn.ShouldSelect returns true for QueryOnly too: the SELECT now includes
// mt_version, the column/binder counts realign, and the LINQ path stops throwing.
StoreOptions(opts =>
{
opts.Events.TenancyStyle = TenancyStyle.Conjoined;
opts.Projections.Add<RevisionedProjection>(ProjectionLifecycle.Inline);
opts.Schema.For<RevisionedDoc>()
.MultiTenanted()
.Metadata(m => m.TenantId.MapTo(x => x.Tenant));
});

var id = Guid.NewGuid();
await using (var session = theStore.LightweightSession("tenant-a"))
{
session.Events.StartStream<RevisionedDoc>(id, new RevisionedCreated(id, "revisioned"));
await session.SaveChangesAsync();
}

await using var query = theStore.QuerySession("tenant-a");

(await query.LoadAsync<RevisionedDoc>(id)).ShouldNotBeNull();

var docs = await query.Query<RevisionedDoc>().ToListAsync();
docs.Count.ShouldBe(1);
}

// ---- controls: each feature ALONE already works; kept so a partial fix can't shift the boundary ----

[Fact]
public async Task control_tenant_mapped_but_not_a_projection_target()
{
StoreOptions(opts =>
{
opts.Schema.For<MappedDoc>()
.MultiTenanted()
.Metadata(m => m.TenantId.MapTo(x => x.Tenant));
});

var id = Guid.NewGuid();
await using (var session = theStore.LightweightSession("tenant-a"))
{
session.Store(new MappedDoc { Id = id, Name = "stored" });
await session.SaveChangesAsync();
}

await using var query = theStore.QuerySession("tenant-a");
(await query.Query<MappedDoc>().ToListAsync()).Count.ShouldBe(1);
}

[Fact]
public async Task control_projection_target_but_not_tenant_mapped()
{
StoreOptions(opts =>
{
opts.Events.TenancyStyle = TenancyStyle.Conjoined;
opts.Projections.Add<PlainProjection>(ProjectionLifecycle.Inline);
// Conjoined events require a conjoined projection target, but with no
// tenant member mapped — so there's no DocumentTenantIdBinder, isolating
// the version-binder offset from the tenant read.
opts.Schema.For<PlainDoc>().MultiTenanted();
});

var id = Guid.NewGuid();
await using (var session = theStore.LightweightSession("tenant-a"))
{
session.Events.StartStream<PlainDoc>(id, new PlainCreated(id, "plain"));
await session.SaveChangesAsync();
}

await using var query = theStore.QuerySession("tenant-a");
(await query.Query<PlainDoc>().ToListAsync()).Count.ShouldBe(1);
}

// ---- types ----

// One event type per projection so each read model populates from exactly its own stream.
public record PlainCreated(Guid Id, string Name);
public record MappedCreated(Guid Id, string Name);
public record TenantedCreated(Guid Id, string Name);
public record RevisionedCreated(Guid Id, string Name);

public class PlainDoc
{
public Guid Id { get; set; }
public string Name { get; set; } = "";
}

public class MappedDoc
{
public Guid Id { get; set; }
public string Name { get; set; } = "";
public string Tenant { get; set; } = "";
}

public class TenantedDoc : ITenanted
{
public Guid Id { get; set; }
public string Name { get; set; } = "";
public string? TenantId { get; set; }
}

public class RevisionedDoc : IRevisioned
{
public Guid Id { get; set; }
public string Name { get; set; } = "";
public string Tenant { get; set; } = "";
public int Version { get; set; }
}

public partial class PlainProjection : SingleStreamProjection<PlainDoc, Guid>
{
public static PlainDoc Create(IEvent<PlainCreated> e) => new() { Id = e.Data.Id, Name = e.Data.Name };
}

public partial class MappedProjection : SingleStreamProjection<MappedDoc, Guid>
{
public static MappedDoc Create(IEvent<MappedCreated> e) => new() { Id = e.Data.Id, Name = e.Data.Name };
}

public partial class TenantedProjection : SingleStreamProjection<TenantedDoc, Guid>
{
public static TenantedDoc Create(IEvent<TenantedCreated> e) => new() { Id = e.Data.Id, Name = e.Data.Name };
}

public partial class RevisionedProjection : SingleStreamProjection<RevisionedDoc, Guid>
{
public static RevisionedDoc Create(IEvent<RevisionedCreated> e) => new() { Id = e.Data.Id, Name = e.Data.Name };
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,11 @@ private async System.Threading.Tasks.ValueTask<T> ReadDocumentAsync(DbDataReader

private void ApplyMetadata(DbDataReader reader, T document)
{
// #4602: QueryOnlyReadBinders, not ReadBinders — the QueryOnly SELECT omits
// mt_version when the version/revision column has no member, so its binder
// set is narrower to keep ordinals aligned with the projection.
var ordinal = FirstMetadataColumn;
foreach (var binder in _descriptor.ReadBinders)
foreach (var binder in _descriptor.QueryOnlyReadBinders)
{
binder.Apply(reader, ordinal, document, _session);
ordinal++;
Expand Down
12 changes: 12 additions & 0 deletions src/Marten/Internal/ClosedShape/DocumentStorageDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ internal DocumentStorageDescriptor(
IDocumentMetadataBinder<TDoc>[] clientSideWriteBinders,
IDocumentMetadataBinder<TDoc>[] writeBinders,
IDocumentMetadataBinder<TDoc>[] readBinders,
IDocumentMetadataBinder<TDoc>[] queryOnlyReadBinders,
string upsertSql,
string insertSql,
string updateSql,
Expand All @@ -72,6 +73,7 @@ internal DocumentStorageDescriptor(
ClientSideWriteBinders = clientSideWriteBinders;
WriteBinders = writeBinders;
ReadBinders = readBinders;
QueryOnlyReadBinders = queryOnlyReadBinders;
TableName = tableName;
UpsertSql = upsertSql;
InsertSql = insertSql;
Expand Down Expand Up @@ -148,6 +150,16 @@ internal DocumentStorageDescriptor(
/// </summary>
public IDocumentMetadataBinder<TDoc>[] ReadBinders { get; }

/// <summary>
/// #4602: the read-binder set for the QueryOnly storage style. Identical
/// to <see cref="ReadBinders"/> except it omits the version/revision binder
/// when that column has no annotated member — <c>Version/RevisionColumn.ShouldSelect</c>
/// returns false for QueryOnly in that case, so <c>mt_version</c> is absent
/// from the QueryOnly SELECT and its binders would otherwise be offset by one.
/// Same array instance as <see cref="ReadBinders"/> when nothing is dropped.
/// </summary>
public IDocumentMetadataBinder<TDoc>[] QueryOnlyReadBinders { get; }

/// <summary>
/// Full upsert SQL with <c>?</c> placeholders for client-side
/// parameters and inline literals for server-side ones — e.g.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ public static DocumentStorageDescriptor<TDoc, TId> Build<TDoc, TId>(
var docTypeReadIndex = -1;
DocumentMapping? hierarchyMapping = null;

// #4602: the version/revision column is the one metadata column whose
// presence in the SELECT depends on the storage style — Version/Revision
// ShouldSelect returns false for QueryOnly when there's no annotated
// member (only the non-QueryOnly concurrency-tracking rule keeps it).
// readBinders below matches the non-QueryOnly SELECT (mt_version present);
// when this stays false we drop that binder for the QueryOnly read set so
// the QueryOnly selector's ordinals stay aligned with its narrower SELECT.
var versionInQueryOnlyReadSet = false;

// M11: hierarchical mappings carry mt_doc_type — the discriminator
// SubClassDocumentStorage filters on and the selector reads to
// dispatch deserialization to the right subclass type.
Expand All @@ -65,6 +74,9 @@ public static DocumentStorageDescriptor<TDoc, TId> Build<TDoc, TId>(
{
versionReadIndex = readBinders.Count;
readBinders.Add(revisionBinder);
// Selected by QueryOnly too only when it has a member; otherwise it's
// present here solely for the non-QueryOnly concurrency rule.
versionInQueryOnlyReadSet = mapping.Metadata.Revision.Member is not null;
}
}
else if (mapping.Metadata.Version.Enabled)
Expand All @@ -77,6 +89,7 @@ public static DocumentStorageDescriptor<TDoc, TId> Build<TDoc, TId>(
{
versionReadIndex = readBinders.Count;
readBinders.Add(versionBinder);
versionInQueryOnlyReadSet = mapping.Metadata.Version.Member is not null;
}
}

Expand Down Expand Up @@ -217,6 +230,17 @@ public static DocumentStorageDescriptor<TDoc, TId> Build<TDoc, TId>(

var writeArray = writeBinders.ToArray();
var readArray = readBinders.ToArray();

// #4602: QueryOnly's SELECT omits mt_version when the version/revision
// column has no member, so its read set drops that binder to stay aligned
// with the narrower projection. Same array reference otherwise — no extra
// allocation for the common case.
var queryOnlyReadArray = readArray;
if (versionReadIndex >= 0 && !versionInQueryOnlyReadSet)
{
queryOnlyReadArray = readArray.Where((_, i) => i != versionReadIndex).ToArray();
}

var clientSide = writeArray.Where(b => !b.IsServerSide).ToArray();

var isConjoined = mapping.TenancyStyle == TenancyStyle.Conjoined;
Expand Down Expand Up @@ -251,6 +275,7 @@ public static DocumentStorageDescriptor<TDoc, TId> Build<TDoc, TId>(
clientSideWriteBinders: clientSide,
writeBinders: writeArray,
readBinders: readArray,
queryOnlyReadBinders: queryOnlyReadArray,
upsertSql: upsertSql,
insertSql: insertSql,
updateSql: updateSql,
Expand Down