Generate TimestampAttribute when scaffolding row version properties

[GOVINSAGA]

Fixes #34609

Scaffolding a SQL Server rowversion column with --data-annotations produced no [Timestamp] attribute, leaving the configuration as fluent API in the generated DbContext.

Before:

// Entity file — no [Timestamp]
public byte[] Version { get; set; }

// Context file — fluent config remains even in data-annotations mode
entity.Property(e => e.Version)
    .IsRowVersion()
    .IsConcurrencyToken();

After: [Timestamp] on the property, both fluent calls omitted.

Cause

ScaffoldingModelExtensions.GetDataAnnotations(IProperty, …) had no TimestampAttribute case, so the rowversion facets stayed in GetFluentApiCalls, which emitted .IsRowVersion() and .IsConcurrencyToken() without IsHandledByDataAnnotations — meaning CSharpDbContextGenerator never filtered them out.

Change

A single shared predicate drives both halves, so the emit and suppress conditions can't drift:

private static bool IsRowVersion(IProperty property)
    => property.IsConcurrencyToken
        && property.ValueGenerated == ValueGenerated.OnAddOrUpdate;

• GetDataAnnotations yields [Timestamp] when it holds, placed with the other core-model attributes.
• GetFluentApiCalls sets IsHandledByDataAnnotations on both the ValueGenerated and IsConcurrencyToken fragments. Both need suppressing — .IsRowVersion() already implies the concurrency token, so the previous output was redundant.

IsHandledByDataAnnotations is only consulted when UseDataAnnotations is on, so fluent-mode output is unchanged.

Round-trip

TimestampAttributeConvention sets exactly ValueGenerated.OnAddOrUpdate + IsConcurrencyToken(true) — identical to IsRowVersion() — so no model state is lost. The new test asserts this through the TestAsync model callback, which recompiles the generated code and rebuilds the model, rather than only string-matching.

No [Column(TypeName = "rowversion")] interaction: the SQL Server scaffolding mapping for rowversion is inferred, so HasColumnType is never set.

Tests

Three tests, all run against the full EFCore.Design.Tests suite (1273 passed, 0 failed), with no baseline changes required:

1. TimestampAttribute_is_generated_for_row_version_property — attribute emitted, both fluent calls absent, model round-trips. Verified failing before the change.
2. IsRowVersion_is_still_generated_when_not_using_data_annotations — fluent mode unaffected.
3. IsConcurrencyToken_is_still_generated_for_non_row_version_property_using_data_annotations — guards against over-suppression. Verified this fails if the predicate is widened to IsConcurrencyToken alone.

Behavioral change

Re-scaffolding an existing database with --data-annotations now produces [Timestamp] where it previously produced two fluent calls. The resulting model is unchanged and scaffolded code is regenerated by definition, but flagging it explicitly per the contributing guidelines.

Open questions from the issue thread

Raised in #34609 (comment) and still open — happy to adjust:

1. Type guard — I did not restrict to byte[], keeping the condition the exact inverse of the fluent call being suppressed. Both TimestampAttributeConvention and IsRowVersion() are type-agnostic, and in-box only SQL Server rowversion sets the ConcurrencyToken scaffolding annotation. Can add a byte[] guard if you'd prefer to be conservative for third-party providers.
2. [ConcurrencyCheck] — plain concurrency tokens have the same gap; left out to keep this to one behavior change. Can fold in if wanted.

---

• I've read the guidelines for contributing
• Comment posted on the issue describing the approach; maintainer approval still pending. Opening the PR now so the code is available alongside the discussion — happy to rework or close it if the approach isn't what you want.
• The code builds and tests pass locally
• Commit message follows the required format
• Tests for the changes have been added
• Code follows the same patterns and style as existing code in this repo

[Copilot]

Pull request overview

Updates EF Core design-time scaffolding so SQL Server rowversion/timestamp properties generate a [Timestamp] data annotation when --data-annotations is used, and ensures the corresponding fluent API calls are suppressed in that mode while remaining unchanged in fluent-only generation.

Changes:

• Emit [Timestamp] for properties detected as row-version (concurrency token + ValueGenerated.OnAddOrUpdate).
• Mark the generated .IsRowVersion() and .IsConcurrencyToken() fluent fragments as handled-by-data-annotations for row-version properties, so they’re filtered out when UseDataAnnotations is enabled.
• Add regression tests covering data-annotation emission + fluent suppression, plus guard tests to ensure fluent mode and non-rowversion concurrency tokens remain unaffected.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
test/EFCore.Design.Tests/Scaffolding/Internal/CSharpEntityTypeGeneratorTest.cs	Adds a test asserting [Timestamp] is emitted on a row-version property and fluent config is omitted in data-annotations mode, with model round-trip verification.
test/EFCore.Design.Tests/Scaffolding/Internal/CSharpDbContextGeneratorTest.cs	Adds tests ensuring fluent .IsRowVersion() remains in non-data-annotations mode, and that non-rowversion concurrency tokens still generate .IsConcurrencyToken() (and not [Timestamp]).
src/EFCore.Design/Extensions/ScaffoldingModelExtensions.cs	Implements row-version detection, emits TimestampAttribute, and aligns fluent suppression via IsHandledByDataAnnotations on relevant fragments.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.