Skip to content
Draft
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
2 changes: 1 addition & 1 deletion src/Couchbase/Client/Transactions/AttemptContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1561,7 +1561,7 @@ await RepeatUntilSuccessOrThrow(async () =>

await _testHooks.BeforeDocCommitted(this, sm.Doc.Id).CAF();
var (updatedCas, mutationToken) = await _docs
.UnstageInsertOrReplace(sm.Doc.Collection, sm.Doc.Id, cas, content, insertMode, sm.Flags ?? new Flags(), sm.Expiry).CAF();
.UnstageInsertOrReplace(sm.Doc.Collection, sm.Doc.Id, cas, content, insertMode, sm.Flags ?? Flags.JsonCommonFlags, sm.Expiry).CAF();
Logger.LogInformation(
"Unstaged mutation successfully on {redactedId}, attempt={attemptId}, insertMode={insertMode}, ambiguityResolutionMode={ambiguityResolutionMode}, preCas={cas}, postCas={updatedCas}",
Redactor.UserData(sm.Doc.FullyQualifiedId),
Expand Down
10 changes: 8 additions & 2 deletions src/Couchbase/Client/Transactions/Cleanup/Cleaner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Couchbase.Client.Transactions.DataAccess;
using Couchbase.Client.Transactions.DataModel;
using Couchbase.Client.Transactions.Error;
using Couchbase.Client.Transactions.Internal;
using Couchbase.Client.Transactions.Internal.Test;
using Couchbase.Client.Transactions.LogUtil;
using Couchbase.Client.Transactions.Support;
Expand Down Expand Up @@ -230,11 +231,14 @@ private async Task UnstageInsertOrRemove<T>(DocumentLookupResult op, T content,
var coll = op.DocumentCollection;
if (op.IsDeleted)
{
// InsertAsync has no flags option; the persisted flags are whatever the transcoder's
// GetFormat returns. Pin them to the staged flags (txn.aux.uf) — we did not stage
// this doc, so re-deriving from the transcoder would not preserve the user's flags.
await coll.InsertAsync(op.Id, content,
options =>
{
options.Durability(durabilityLevel)
.Transcoder(op.StagedContent?.Transcoder);
.Transcoder(new FixedFlagsTranscoder(op.StagedContent!.Transcoder, op.StagedContent.Flags));
if (op.Expiry.HasValue) options.Expiry(op.Expiry.Value.RemainingTtl());
}).CAF();
}
Expand All @@ -246,7 +250,9 @@ await coll.MutateInAsync(op.Id, specs =>
{
opts.Durability(durabilityLevel)
.Transcoder(op.StagedContent?.Transcoder)
.Flags(op.StagedContent!.Transcoder.GetFormat(content))
// Use the flags recorded at staging time (txn.aux.uf), not flags
// re-derived from this cleaner's transcoder — we did not stage this doc.
.Flags(op.StagedContent!.Flags)
.Timeout(_keyValueTimeout)
.Cas(op.Cas)
.PreserveTtl(coll.Scope.Bucket.SupportsCollections);
Expand Down
50 changes: 37 additions & 13 deletions src/Couchbase/Client/Transactions/DataAccess/DocumentRepository.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#nullable enable
using System;
using System.Buffers.Binary;
using System.Collections.Generic;
using System.Threading.Tasks;
using Couchbase.Core;
Expand Down Expand Up @@ -177,10 +176,13 @@ public bool SupportsReplaceBodyWithXattr(ICouchbaseCollection collection)
// if bucket doesn't support ReplaceBodyWithXattr (and ReviveDocument)
if (insertMode)
{
// InsertAsync has no flags option; the persisted flags are whatever the transcoder's
// GetFormat returns. Pin them to the staged flags so the committed doc keeps them.
ITypeTranscoder insertTranscoder = isBinary
? new RawBinaryTranscoder()
: _jsonUserDataTranscoder;
var opts = new InsertOptions().Defaults(_durability, _keyValueTimeout)
.Transcoder(isBinary
? new RawBinaryTranscoder()
: _jsonUserDataTranscoder);
.Transcoder(new FixedFlagsTranscoder(insertTranscoder, flags));
if (expiry.HasValue)
{
opts.Expiry(expiry.Value.RemainingTtl());
Expand Down Expand Up @@ -289,6 +291,12 @@ internal static async Task<DocumentLookupResult> LookupDocumentAsync(ICouchbaseC
var docMeta = lookupInResult.ContentAs<DocumentMetadata>(docMetaIndex);
int dataIdx = stagedDataIndex; // just need a default

// Deserialize the transaction xattr once; reused below for the staged user-flags
// (txn.aux.uf) and stored on the result.
var txnXattrs = lookupInResult.Exists(txnIndex)
? lookupInResult.ContentAs<TransactionXattrs>(txnIndex)
: null;

// Determine the appropriate transcoder for wrapping the document content.
// This is separate from the lookup transcoder used above.
// Note: We use separate transcoders for staged vs unstaged content because:
Expand Down Expand Up @@ -340,8 +348,12 @@ internal static async Task<DocumentLookupResult> LookupDocumentAsync(ICouchbaseC
? new LookupInContentAsWrapper(lookupInResult, fullDocIndex.Value, unstagedContentTranscoder)
: null;

// Staged content carries the user flags recorded in txn.aux.uf when it was staged,
// NOT the live document body's flags. This matters when committing content we didn't
// stage (lost-transaction cleanup) or when resolving ambiguity from a re-read doc.
var stagedContent = lookupInResult.Exists(dataIdx)
? new LookupInContentAsWrapper(lookupInResult, dataIdx, stagedContentTranscoder)
? new LookupInContentAsWrapper(lookupInResult, dataIdx, stagedContentTranscoder,
flagsOverride: ParseStagedUserFlags(txnXattrs))
: null;

var result = new DocumentLookupResult(docId,
Expand All @@ -351,12 +363,27 @@ internal static async Task<DocumentLookupResult> LookupDocumentAsync(ICouchbaseC
docMeta,
collection);

if (lookupInResult.Exists(txnIndex))
result.TransactionXattrs = txnXattrs;

return result;
}

/// <summary>
/// Reconstruct the user flags recorded in <c>txn.aux.uf</c> at staging time. Falls back to
/// JSON common flags when the field is absent (e.g. staged by an older/other SDK that did
/// not record it — such content was always JSON), mirroring Java's
/// <c>stagedUserFlags().orElse(CodecFlags.JSON_COMMON_FLAGS)</c>.
/// </summary>
internal static Flags ParseStagedUserFlags(TransactionXattrs? txnXattrs)
{
if (txnXattrs?.AuxiliaryData is { ValueKind: JsonValueKind.Object } aux
&& aux.TryGetProperty("uf", out var ufElement)
&& ufElement.TryGetUInt32(out var uf))
{
result.TransactionXattrs = lookupInResult.ContentAs<TransactionXattrs>(txnIndex);
return Flags.FromUInt32(uf);
Comment on lines +379 to +383

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.

True! But also all other sdks wrote it in the big-endian. We must match other sdk, so we have to change it. the only risk is if we are cleaning a txn written by us from before this change. The much, much bigger risk is we don't change and therefore remain broken with respect to all other sdks, and cannot interoperate.

A better debate than making the fix would be when to make it. Do we put this in now, or do we wait for a minor and call it out. The reality is that this only really burns us if there are lost transactions (or concurrently transactions from an older sdk) that we need to commit, and this is really rare.

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.

Perhaps create a Jira ticket do this in a minor release?

}

return result;
return Flags.JsonCommonFlags;
}

private MutateInOptions GetMutateInOptions(StoreSemantics storeSemantics, ICouchbaseCollection collection) =>
Expand Down Expand Up @@ -414,11 +441,8 @@ private List<MutateInSpec> CreateMutationSpecs(IAtrRepository atr, string opType
specs.Add(MutateInSpec.Upsert(TransactionFields.StagedData, rawJsonElement,
createPath: true, isXattr: true));
}
// convert flags to a uint
Span<byte> span = stackalloc byte[4];
content.Flags.Write(span);
var flagCompact = BinaryPrimitives.ReadUInt32LittleEndian(span);
// now add it
// record the user flags (reconstructed on read by Flags.FromUInt32)
var flagCompact = content.Flags.ToUInt32();
specs.Add(MutateInSpec.Upsert(TransactionFields.UserFlags, flagCompact,
createPath: true, isXattr: true));
break;
Expand Down
51 changes: 51 additions & 0 deletions src/Couchbase/Client/Transactions/Internal/FixedFlagsTranscoder.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#nullable enable
using System;
using System.IO;
using Couchbase.Core.IO.Operations;
using Couchbase.Core.IO.Serializers;
using Couchbase.Core.IO.Transcoders;

namespace Couchbase.Client.Transactions.Internal
{
/// <summary>
/// A transcoder decorator that pins the flags written to the document, delegating the actual
/// byte encoding/decoding to an inner transcoder.
/// <para>
/// On a .NET mutation the persisted flags are always <c>Transcoder.GetFormat(content)</c>
/// (see <c>OperationBase&lt;T&gt;.WriteExtras</c>) — there is no per-operation flags override,
/// which is why <c>InsertOptions</c> exposes none. When committing content we did not stage
/// (lost-transaction cleanup) or via the legacy insert path, we must persist the user flags
/// recorded at staging time (<c>txn.aux.uf</c>) rather than flags re-derived from the content.
/// This wrapper makes <see cref="GetFormat{T}"/> report those staged flags so they land on the
/// document, while <see cref="Encode{T}"/>/<see cref="Decode{T}"/> behave exactly as the inner
/// transcoder. It is the .NET analogue of Java passing <c>stagedUserFlags</c> straight to the
/// insert request.
/// </para>
/// </summary>
internal sealed class FixedFlagsTranscoder : ITypeTranscoder
{
private readonly ITypeTranscoder _inner;
private readonly Flags _flags;

public FixedFlagsTranscoder(ITypeTranscoder inner, Flags flags)
{
_inner = inner ?? throw new ArgumentNullException(nameof(inner));
_flags = flags;
}

/// <summary>Always reports the fixed (staged) flags, ignoring the content's runtime type.</summary>
public Flags GetFormat<T>(T value) => _flags;

public void Encode<T>(Stream stream, T value, Flags flags, OpCode opcode) =>
_inner.Encode(stream, value, flags, opcode);

public T? Decode<T>(ReadOnlyMemory<byte> buffer, Flags flags, OpCode opcode) =>
_inner.Decode<T>(buffer, flags, opcode);

public ITypeSerializer? Serializer
{
get => _inner.Serializer;
set => _inner.Serializer = value;
}
}
}
10 changes: 6 additions & 4 deletions src/Couchbase/Client/Transactions/Internal/IContentAsWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,18 @@ internal class LookupInContentAsWrapper : IContentAsWrapper

public bool IsBinary { get; init; }

public LookupInContentAsWrapper(ILookupInResult lookupInResult, int specIndex, ITypeTranscoder? transcoder = null)
public LookupInContentAsWrapper(ILookupInResult lookupInResult, int specIndex, ITypeTranscoder? transcoder = null, Flags? flagsOverride = null)
{
_lookupInResult = lookupInResult;
if (lookupInResult is not ILookupInResultInternal res)
{
throw new InvalidArgumentException("lookupInResult is not a LookupInResult");
}
// NOTE: this Flags isn't necessarily what we want to use for the flags if this specIndex
// becomes the document body.
Flags = res.Flags;
// res.Flags is the flags of the top-level (live) document body. That is correct for
// pre-transaction (unstaged) content, but wrong for staged content: the staged user
// flags are recorded separately in txn.aux.uf. Callers wrapping staged content pass
// flagsOverride so this wrapper carries the flags the content was actually staged with.
Flags = flagsOverride ?? res.Flags;
_specIndex = specIndex;
IsBinary = (res.Specs[specIndex].PathFlags & SubdocPathFlags.BinaryValue) != 0;
Transcoder = transcoder ?? new JsonTranscoder();
Expand Down
41 changes: 41 additions & 0 deletions src/Couchbase/Core/IO/Operations/Flags.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Buffers.Binary;
using System.Runtime.InteropServices;
using Couchbase.Core.IO.Converters;

Expand All @@ -15,6 +16,19 @@ public struct Flags

internal const int Size = 4; // we explicitly serialize into 4 bytes

/// <summary>
/// The canonical common flags for JSON content (a JSON object). Used as a fallback
/// when a persisted flags value is unavailable (e.g. a staged transaction mutation
/// written by an older/other SDK that did not record <c>txn.aux.uf</c>). Mirrors what
/// <see cref="Transcoders.JsonTranscoder.GetFormat{T}"/> emits for an object body.
/// </summary>
Comment thread
jeffrymorris marked this conversation as resolved.
internal static Flags JsonCommonFlags => new()
{
DataFormat = DataFormat.Json,
Compression = Compression.None,
TypeCode = TypeCode.Object
};

/// <summary>
/// Read flags from a buffer. The buffer must be at least 4 bytes long.
/// </summary>
Expand Down Expand Up @@ -51,6 +65,33 @@ internal readonly void Write(Span<byte> buffer)
ByteConverter.FromUInt16((ushort)TypeCode, buffer.Slice(2));
}

/// <summary>
/// Encode these flags as the 32-bit "user flags" integer used to persist them in a document
/// xattr (e.g. the transaction <c>txn.aux.uf</c> staged user-flags field). Uses network byte
/// order (big-endian) so the common-flags/format nibble lands in the top byte, matching the
/// Common Flags SDK RFC and the other SDKs — i.e. <c>(ToUInt32() >> 24) &amp; 0xF</c> is the
/// document format. <see cref="Write"/> already emits that byte first, so a big-endian read
/// places it in the most-significant byte.
/// </summary>
internal readonly uint ToUInt32()
{
Span<byte> span = stackalloc byte[Size];
Write(span);
return BinaryPrimitives.ReadUInt32BigEndian(span);
}

/// <summary>
/// The exact inverse of <see cref="ToUInt32"/>: reconstruct flags from a persisted network
/// byte order (big-endian) 32-bit user-flags value. Round-trips byte-for-byte with
/// <see cref="ToUInt32"/>.
/// </summary>
internal static Flags FromUInt32(uint value)
{
Span<byte> span = stackalloc byte[Size];
BinaryPrimitives.WriteUInt32BigEndian(span, value);
return Read(span);
}

private static void ThrowArgumentException()
{
throw new ArgumentException("buffer must be at least 4 bytes.");
Expand Down
55 changes: 55 additions & 0 deletions tests/Couchbase.UnitTests/Core/IO/Operations/FlagsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,60 @@ public void Write_ValidBuffer_ExpectedResult(byte[] expectedOutput, DataFormat d
}

#endregion

#region ToUInt32 / FromUInt32

[Theory]
[InlineData(DataFormat.Json, TypeCode.Object)]
[InlineData(DataFormat.Json, TypeCode.String)]
[InlineData(DataFormat.Binary, TypeCode.Object)]
[InlineData(DataFormat.Private, TypeCode.String)]
[InlineData(DataFormat.String, TypeCode.String)]
public void ToUInt32_FromUInt32_RoundTrips(DataFormat dataFormat, TypeCode typeCode)
{
// Arrange

var flags = new Flags
{
DataFormat = dataFormat,
Compression = Couchbase.Core.IO.Operations.Compression.None,
TypeCode = typeCode
};

// Act

var roundTripped = Flags.FromUInt32(flags.ToUInt32());

// Assert

Assert.Equal(flags.DataFormat, roundTripped.DataFormat);
Assert.Equal(flags.Compression, roundTripped.Compression);
Assert.Equal(flags.TypeCode, roundTripped.TypeCode);
}

[Fact]
public void ToUInt32_UsesNetworkByteOrder_CommonFlagsInTopByte()
{
// JSON + Object writes bytes [0x02, 0x00, 0x00, 0x01]; read big-endian => 0x02000001.
// The common-flags/format nibble must be in the top byte so cross-SDK readers get it
// via (uf >> 24) & 0xF — matching Java's CodecFlags (format << 24).
var flags = Flags.JsonCommonFlags;

Assert.Equal(0x02000001u, flags.ToUInt32());
Assert.Equal((uint)DataFormat.Json, flags.ToUInt32() >> 24);
Assert.Equal(DataFormat.Json, Flags.FromUInt32(0x02000001u).DataFormat);
}

[Fact]
public void JsonCommonFlags_IsJsonObject()
{
var flags = Flags.JsonCommonFlags;

Assert.Equal(DataFormat.Json, flags.DataFormat);
Assert.Equal(Couchbase.Core.IO.Operations.Compression.None, flags.Compression);
Assert.Equal(TypeCode.Object, flags.TypeCode);
}

#endregion
}
}
Loading
Loading