Skip to content

Map [MinLength]/[MaxLength] on dictionary properties to minProperties / maxProperties#3922

Merged
martincostello merged 8 commits into
domaindrivendev:masterfrom
KitKeen:fix/minlength-dictionary-minproperties
Jun 16, 2026
Merged

Map [MinLength]/[MaxLength] on dictionary properties to minProperties / maxProperties#3922
martincostello merged 8 commits into
domaindrivendev:masterfrom
KitKeen:fix/minlength-dictionary-minproperties

Conversation

@KitKeen

@KitKeen KitKeen commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #3915.

Problem

Applying [MinLength(1)] to a property typed as a dictionary (IDictionary<,>, IReadOnlyDictionary<,>, etc.) produces "minLength": 1 on the generated schema:

public class MyModel
{
    [Required]
    [MinLength(1)]
    public IReadOnlyDictionary<string, string?> Values { get; init; }
}

"minLength" on an object schema is invalid per the OpenAPI 3.x spec — it's defined only for string schemas. Linters such as vacuum flag this (the original reporter hit it exactly that way). The equivalent constraint for object schemas is "minProperties".

Fix

OpenApiSchemaExtensions.Apply{Min,Max,}LengthAttribute already branches on schema.Type to pick MinItems / MaxItems for arrays. This PR adds a parallel branch for JsonSchemaTypes.Object so dictionary schemas get MinProperties / MaxProperties instead of the meaningless MinLength / MaxLength.

No changes to the MinLengthRouteConstraint / MaxLengthRouteConstraint handlers — route parameters are primitives and cannot be dictionaries, so the existing Array/otherwise split is correct there.

Before / after

public class MyModel
{
    [Required]
    [MinLength(1)]
    public IReadOnlyDictionary<string, string?> Values { get; init; }
}

Before:

"MyModel": {
  "type": "object",
  "required": [ "values" ],
  "properties": {
    "values": {
      "type": "object",
      "additionalProperties": { "type": "string", "nullable": true },
      "minLength": 1
    }
  }
}

After:

"MyModel": {
  "type": "object",
  "required": [ "values" ],
  "properties": {
    "values": {
      "type": "object",
      "additionalProperties": { "type": "string", "nullable": true },
      "minProperties": 1
    }
  }
}

Tests

Added to OpenApiSchemaExtensionsTests:

  • MinLength on dictionary schema → minProperties
  • MaxLength on dictionary schema → maxProperties
  • Length(min,max) on dictionary schema → min+maxProperties
  • Regression guards: MinLength on string schema still maps to minLength; on array schema still maps to minItems.

Scope

  • No public API changes.
  • No dependency changes.
  • Behavior change is limited to schemas whose Type includes Object — previously those would receive MinLength/MaxLength, which was invalid per the OpenAPI spec, so any consumer relying on the old output was already producing non-conforming documents.

Checklist

  • Bug fix includes a test that fails without the fix.
  • No public API surface change.
  • Builds & tests pass locally. (Local SDK is 8.0; repo requires 10.0 — relying on CI.)

KitKeen added 2 commits April 24, 2026 09:25
…erties / maxProperties

Fixes domaindrivendev#3915. OpenAPI defines minLength/maxLength only for string schemas;
for object schemas (including dictionaries) the equivalent constraints
are minProperties/maxProperties. Before this change, applying
[MinLength(1)] to a property typed as IReadOnlyDictionary<TKey, TValue>
produced "minLength": 1 on the schema, which is invalid per the spec and
is flagged by linters such as vacuum.

The three attribute handlers (MinLength, MaxLength, Length) already
switch to MinItems/MaxItems when the schema is an Array. Extend the same
branching to recognise Object schemas and emit MinProperties/MaxProperties
instead of MinLength/MaxLength.

Route constraint handlers (MinLengthRouteConstraint, MaxLengthRouteConstraint)
are intentionally left alone: route parameters are primitives and cannot
be dictionaries, so the existing two-branch array/string logic is correct
there.

Tests added in OpenApiSchemaExtensionsTests:
- MinLength on dictionary → minProperties
- MaxLength on dictionary → maxProperties
- Length on dictionary → min+maxProperties
- Regression guards: MinLength on string still maps to minLength, on array still maps to minItems.
Added:
- End-to-end coverage via shared TypeWithValidationAttributes fixture —
  DictionaryWithMinMaxLength ([MinLength(1),MaxLength(3)]) and
  DictionaryWithLength ([Length(1,3)]) properties, asserted by both
  JsonSerializerSchemaGenerator and NewtonsoftSchemaGenerator tests.
  Reproduces the exact scenario from the issue (dictionary property on a
  model class) rather than only exercising ApplyValidationAttributes in
  isolation.
- Enum-keyed dictionary shape — Object schema with known Properties and
  AdditionalPropertiesAllowed=false (see CreateDictionarySchema). MinLength
  must still route to MinProperties here.
- Nullable dictionary shape — Type = Object | Null. HasFlag(Object) must
  still route to MinProperties.
- Symmetry regression guards: MaxLength/Length on string map to
  MinLength/MaxLength, on array map to MinItems/MaxItems (previously only
  MinLength had explicit guards).

Fixture updates are picked up by both Newtonsoft and JsonSerializer
generator test suites because the shared TestSupport fixture drives both.

@martincostello martincostello left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please make some test changes to that the OpenAPI snapshots demonstrate the required changes to the generated OpenAPI schemas.

KitKeen added 2 commits April 25, 2026 11:12
- Replace em-dashes with hyphens in OpenApiSchemaExtensionsTests.cs
- Rename fixture property DictionaryWithMinMaxLength to
  IReadOnlyDictionaryWithMinMaxLength so the name matches its actual
  IReadOnlyDictionary<,> declared type
- Add JSON-output test that serializes the schema for
  TypeWithValidationAttributes/TypeWithValidationAttributesViaMetadataType
  to OpenAPI 3.0 JSON and asserts the dictionary properties surface as
  minProperties/maxProperties in the produced OpenAPI document
…ttribute

Per review: 'Length' as a noun does not describe a dictionary (which has a
property/item count, not a length). The Attribute suffix makes it explicit
that the property exists to test the [Length] attribute mapping rather than
a conceptual 'length' of the dictionary.
Per follow-up review: in TypeWithValidationAttributesViaMetadataType the
outer class is a bare property list (attributes live on the inner
MetadataType class), so names that referenced attributes were misleading.

Renamed to neutral, type-focused names that work in both fixture variants:
- IReadOnlyDictionaryWithMinMaxLength -> BoundedReadOnlyDictionary
- DictionaryWithLengthAttribute       -> BoundedDictionary

The bounded prefix describes the test purpose (a dictionary whose size is
constrained); the readonly/mutable distinction reflects the actual type
under test. No attribute presence is claimed by the property name.
…ictionaries

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov

codecov Bot commented May 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.05%. Comparing base (d22870e) to head (54c1cee).
⚠️ Report is 61 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3922   +/-   ##
=======================================
  Coverage   95.04%   95.05%           
=======================================
  Files         111      111           
  Lines        3958     3965    +7     
  Branches      801      804    +3     
=======================================
+ Hits         3762     3769    +7     
  Misses        196      196           
Flag Coverage Δ
Linux 95.05% <100.00%> (+<0.01%) ⬆️
Windows 95.05% <100.00%> (+<0.01%) ⬆️
macOS 95.05% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@martincostello

Copy link
Copy Markdown
Collaborator

Tests are failing.

The Verify rework only added baselines under snapshots/10_0/, causing
GenerateSchema_BoundedDictionaries_UsesMinAndMaxProperties to fail on
net8.0 and net9.0. Schemas are framework-agnostic, so the snapshots
match the existing net10.0 baselines verbatim.
@KitKeen

KitKeen commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

@martincostello pushed 2d8e31e - the snapshot rework (743061c) only added baselines under snapshots/10_0/, so the test failed on net8.0/9.0. Now mirrored to 8_0/ and 9_0/ (schemas are framework-agnostic; verified against the other snapshots in this repo). Could you re-approve the workflows? Thanks!

The Theory parameter Type type was serialized into snapshot filenames via .UseParameters(type), producing names like ..._type=Swashbuckle.AspNetCore.TestSupport.TypeWithValidationAttributesViaMetadataType.verified.txt. Combined with the Windows runner checkout root and snapshots/<fw>/ path, the total exceeded the 260-character MAX_PATH limit and git checkout failed with "Filename too long" on windows-latest while macos/ubuntu passed.

Replace with .UseTextForParameters(type.Name) to drop the namespace prefix from the filename and rename the 6 existing snapshots accordingly. Content of the snapshots is unchanged.
@KitKeen

KitKeen commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

Pushed 54c1cee. The earlier fix in 2d8e31e (copying snapshots to 8_0/ and 9_0/) brought macos-latest and ubuntu-latest green, but windows-latest kept failing with git checkout errors of the form error: unable to create file ... : Filename too long. The cause was the Theory parameter Type type being serialized into the snapshot filename via .UseParameters(type), producing names like ..._type=Swashbuckle.AspNetCore.TestSupport.TypeWithValidationAttributesViaMetadataType.verified.txt. Combined with the Windows runner checkout root and the snapshots/<fw>/ segment the total exceeded the 260-character MAX_PATH limit.

This commit replaces .UseParameters(type) with .UseTextForParameters(type.Name) so only the short type name (without the Swashbuckle.AspNetCore.TestSupport. namespace) appears in the filename, and renames the six existing snapshots accordingly. Snapshot contents are unchanged.

@martincostello martincostello added this to the v10.2.2 milestone Jun 16, 2026
@martincostello martincostello enabled auto-merge (squash) June 16, 2026 13:48
@martincostello martincostello merged commit 48aba08 into domaindrivendev:master Jun 16, 2026
14 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for your contribution @KitKeen - the changes from this pull request have been published as part of version 10.2.2 📦, which is now available from NuGet.org 🚀

This was referenced Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: MinLength attribute on a dictionary property generates invalid minLength constraint

2 participants