refactor!: model MQTT scalar-or-schema values as a type hierarchy by bielu · Pull Request #50 · ByteBardOrg/AsyncAPI.NET

bielu · 2026-10-17T17:41:51Z

Problem

Since #45, these MQTT fields can be an integer/string or a schema, but each was exposed twice: a union object with nullable Integer/LongInteger/Schema members, plus a scalar accessor kept for backward compatibility:

Property	Union property
`MQTTServerBinding.SessionExpiryInterval` (`int?`)	`SessionExpiryIntervalValue`
`MQTTServerBinding.MaximumPacketSize` (`int?`)	`MaximumPacketSizeValue`
`MQTTOperationBinding.MessageExpiryInterval` (`int?`)	`MessageExpiryIntervalValue`
`MQTTMessageBinding.ResponseTopic` (`string`)	`ResponseTopicValue`

The int? accessors returned null for schema values and for values above Int32.MaxValue. MQTT allows up to 4294967295, and a 0xFFFFFFFF session expiry means "never expires", so it couldn't be told apart from "not set". Consumers also had to null-check members of the union to find out which alternative was present.

Change

Following review feedback, the unions are now a type hierarchy (same approach as AsyncApiAvroSchema):

MQTTIntegerOrSchema (abstract) → MQTTInteger (long Value) | MQTTIntegerSchema (AsyncApiJsonSchema Schema)
MQTTStringOrSchema (abstract) → MQTTString (string Value) | MQTTStringSchema (AsyncApiJsonSchema Schema)
Implicit conversions from long/string/AsyncApiJsonSchema to the base, and from each subtype back to its underlying value.
The scalar accessors and the *Value properties are removed. The four properties above now expose the union type directly, as MQTT5ServerBinding.SessionExpiryInterval already did.

binding.SessionExpiryInterval = 4294967295;              // MQTTInteger
binding.MaximumPacketSize = new AsyncApiJsonSchema { … }; // MQTTIntegerSchema

if (binding.SessionExpiryInterval is MQTTInteger interval) { long seconds = interval; }

Breaking change

v3.0.1 shipped these four properties as int?/string. Assignments still compile through the implicit conversions. Code that reads them as int?/string has to switch to type checks. The FromInteger/FromLong/FromSchema/FromString factories and the Integer/LongInteger members (unreleased, from #45) are replaced by the subtype constructors.

The full suite passes on net8.0/net9.0/net10.0.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- MQTT server, operation, and message bindings now support integer-or-schema and string-or-schema values directly, including schema-based response topics and expiry or packet-size settings.
- Integer and string values can be assigned directly to these properties.
Changes
- MQTT binding property types have changed. Existing integrations may need updates to use the new value types; the separate *Value properties are no longer available.

coderabbitai · 2026-10-17T17:42:00Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 9a7783a2-9559-4694-bf9e-302bd5aa11e8

📥 Commits

Reviewing files that changed from the base of the PR and between 70e73db and 5cd41b1.

📒 Files selected for processing: 6

src/ByteBard.AsyncAPI.Bindings/MQTT/MQTTMessageBinding.cs
src/ByteBard.AsyncAPI.Bindings/MQTT/MQTTOperationBinding.cs
src/ByteBard.AsyncAPI.Bindings/MQTT/MQTTServerBinding.cs
src/ByteBard.AsyncAPI.Bindings/MQTT/MQTTValueUnions.cs
test/ByteBard.AsyncAPI.Tests/Bindings/MQTT/MQTTBindingVariants_Should.cs
test/ByteBard.AsyncAPI.Tests/Bindings/MQTT5/MQTT5Bindings_Should.cs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

MQTT binding properties now expose integer-or-schema and string-or-schema values directly. The union types provide scalar and schema alternatives, and binding parsing, serialization, and tests use the updated properties.

Changes: MQTT binding value unions

Layer / File(s)	Summary
Define scalar and schema union alternatives `src/ByteBard.AsyncAPI.Bindings/MQTT/MQTTValueUnions.cs`	`MQTTIntegerOrSchema` and `MQTTStringOrSchema` are abstract unions with scalar and schema alternatives, implicit conversions, parsing, and version-aware serialization.
Use union values in MQTT bindings `src/ByteBard.AsyncAPI.Bindings/MQTT/MQTTServerBinding.cs`, `src/ByteBard.AsyncAPI.Bindings/MQTT/MQTTOperationBinding.cs`, `src/ByteBard.AsyncAPI.Bindings/MQTT/MQTTMessageBinding.cs`	Server, operation, and message properties expose union values directly. Parsing assigns parsed unions, and serialization delegates to each value’s serializer.
Validate direct union properties `test/ByteBard.AsyncAPI.Tests/Bindings/MQTT/MQTTBindingVariants_Should.cs`, `test/ByteBard.AsyncAPI.Tests/Bindings/MQTT5/MQTT5Bindings_Should.cs`	Tests use the direct properties and check scalar values, schemas, parsing, serialization, and round trips.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: visualbean

Merge Risk: ⚪ Minimal · up to 5cd41

No concrete issue remains that should block merging. The public property changes are intentional, though the reported test results have not been independently verified here.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 27 functions across 6 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the main change: refactoring MQTT scalar-or-schema values into a type hierarchy.
Description check		The description is detailed and explains the problem, implementation, breaking API changes, examples, and test status. It does not use the template headings or include a specific related issue entry, …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description is detailed and explains the problem, implementation, breaking API changes, examples, and test status. It does not use the template headings or include a specific related issue entry, but the required change information is substantially present.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

VisualBean

I've never been a fan of these 'scalar or object' types.
Honestly this should be changed to a null object type thing and be given a hierachy.
New type, 2 subtype with implicit converters to their actual type. So consumer can use regular type checks rather than null validation.

I did something similar in other places.

Edit: they already have these types.. why the hell do they have the integers as well.

I'll dig further into the code

VisualBean · 2026-10-17T18:06:08Z

God I love Claude sometimes

…olete MQTTServerBinding.SessionExpiryInterval, MaximumPacketSize and MQTTOperationBinding.MessageExpiryInterval are int? views over the MQTTIntegerOrSchema values. They return null when the value is a schema or above Int32.MaxValue, which MQTT allows (up to 4294967295; for the session expiry interval 0xFFFFFFFF means "never expires"). Callers cannot tell that apart from "not set". Keep the properties for compatibility, but document the behaviour and mark them [Obsolete] pointing to the *Value properties. Tests now use the *Value properties. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

MQTTIntegerOrSchema and MQTTStringOrSchema are now abstract bases with MQTTInteger/MQTTIntegerSchema and MQTTString/MQTTStringSchema subtypes, so callers use type checks instead of null checks. Implicit conversions from long/string/AsyncApiJsonSchema keep assignments such as SessionExpiryInterval = 600 compiling, and each subtype converts back to its underlying value. The lossy int?/string accessors and the *Value properties are removed; SessionExpiryInterval, MaximumPacketSize, MessageExpiryInterval and ResponseTopic now expose the union type directly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

bielu · 2026-10-17T18:17:50Z

@VisualBean i have love/hate relation with claude, as it makes me faster in delivery of packages, but cognitive load is much higher 🙄

VisualBean · 2026-10-17T18:55:30Z

@VisualBean i have love/hate relation with claude, as it makes me faster in delivery of packages, but cognitive load is much higher 🙄

And thank god for that. Only means you still care for the result.

It makes a lot of things faster, but also makes mistakes like a junior 😅 its all trade-offs 😁.
But for something pre-defined like this type-hierachy its almost a one-shot prompt to greatness

bielu · 2026-10-17T19:04:37Z

@VisualBean yeah my approach is to treat it as working with junior developer 😂

VisualBean · 2026-10-17T19:08:46Z

@VisualBean yeah my approach is to treat it as working with junior developer 😂

100% same for me. Extreme programming style + a lot of swearing 😂.

VisualBean · 2026-10-21T09:29:49Z

            var schemaBinding = new MQTT5ServerBinding
            {
-                SessionExpiryInterval = MQTTIntegerOrSchema.FromSchema(new AsyncApiJsonSchema { Type = SchemaType.Integer }),
+                SessionExpiryInterval = new MQTTIntegerSchema(new AsyncApiJsonSchema { Type = SchemaType.Integer }),


Wasnt there an implicit operator for the jsonschema as well?

Yes — MQTTIntegerOrSchema has an implicit conversion from AsyncApiJsonSchema (which also covers AsyncApiJsonSchemaReference). Updated the tests to use it instead of wrapping in MQTTIntegerSchema explicitly in 5cd41b1.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

bielu marked this pull request as ready for review October 17, 2026 17:45

bielu requested a review from VisualBean as a code owner October 17, 2026 17:45

VisualBean reviewed Oct 17, 2026

View reviewed changes

bielu changed the title ~~fix: mark MQTT int accessors that hide large and schema values as obsolete~~ refactor!: model MQTT scalar-or-schema values as a type hierarchy Oct 17, 2026

bielu and others added 2 commits October 17, 2026 19:14

bielu force-pushed the fix/mqtt-int-accessors-obsolete branch from 617306c to ccdb44e Compare October 17, 2026 18:14

bielu mentioned this pull request Oct 17, 2026

refactor!: model Solace values and StringOrStringList as type hierarchies #55

Merged

VisualBean reviewed Oct 21, 2026

View reviewed changes

test: use implicit schema conversion for MQTT5 sessionExpiryInterval

5cd41b1

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

VisualBean merged commit f86120b into ByteBardOrg:vnext Oct 22, 2026
8 checks passed

Navigation Menu

Repository navigation

refactor!: model MQTT scalar-or-schema values as a type hierarchy - #50

refactor!: model MQTT scalar-or-schema values as a type hierarchy#50
VisualBean merged 3 commits into
ByteBardOrg:vnextfrom
bielu:fix/mqtt-int-accessors-obsolete

bielu commented Oct 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 17, 2026 •

edited

Loading

Uh oh!

VisualBean left a comment •

edited

Loading

Uh oh!

VisualBean commented Oct 17, 2026

Uh oh!

bielu commented Oct 17, 2026 •

edited

Loading

Uh oh!

VisualBean commented Oct 17, 2026

Uh oh!

bielu commented Oct 17, 2026

Uh oh!

VisualBean commented Oct 17, 2026

Uh oh!

VisualBean Oct 21, 2026

Uh oh!

bielu Oct 22, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bielu commented Oct 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Breaking change

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VisualBean left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VisualBean commented Oct 17, 2026

Uh oh!

bielu commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VisualBean commented Oct 17, 2026

Uh oh!

bielu commented Oct 17, 2026

Uh oh!

VisualBean commented Oct 17, 2026

Uh oh!

VisualBean Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

bielu Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bielu commented Oct 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 17, 2026 •

edited

Loading

VisualBean left a comment •

edited

Loading

bielu commented Oct 17, 2026 •

edited

Loading