Add Context Router with Middleware Support

[akfaiz]

Add Context Router with Middleware Support

Description

This PR introduces a context-aware router extension (paho/extensions/routercontext/) that provides topic-based message routing with middleware support. Handlers receive context.Context for cancellation and timeout control.

⚠️ Note: This is an experimental extension, not part of core. Not covered by semantic versioning and support is limited.

Key Features

• Context-Aware Handlers: Handlers receive context.Context along with the message
• Middleware Support: Wrap handlers for logging, metrics, error handling, etc.
• MQTT Wildcards: Full support for + (single-level) and # (multi-level) patterns
• Thread-Safe: Uses sync.RWMutex for concurrent access
• Default Handler: Route unmatched topics to a default handler

Changes

Extension (paho/extensions/routercontext/):

• Router struct with handler registration and routing
• Handler and Middleware types
• Topic matching with MQTT wildcard support
• Default handler and debug logging support

Examples (autopaho/examples/routercontext/):

• Complete working example with autopaho integration
• Middleware implementations: logger and panic recoverer
• Comprehensive README with usage guide

Tests (paho/extensions/routercontext/routercontext_test.go):

• 23 unit tests covering all functionality
• 97.1% code coverage
• Tests for handler routing, middleware ordering, wildcards, and concurrency

Documentation:

• Inline code comments in all files
• README in extension with API reference and best practices
• README in examples with integration guide
• Stability warnings on both READMEs

Usage

router := routercontext.NewRouter()
router.Use(middleware.Logger, middleware.Recoverer)

router.RegisterHandler("sensors/#", func(ctx context.Context, p *paho.Publish) {
    slog.InfoContext(ctx, "Message received", "topic", p.Topic)
})

router.DefaultHandler(func(ctx context.Context, p *paho.Publish) {
    slog.InfoContext(ctx, "Unmatched", "topic", p.Topic)
})

Testing

• ✅ All 23 tests passing
• ✅ 97.1% code coverage
• ✅ All existing tests pass without modification

Closes

closes #286

ECA Compliance: Commits are signed with an ECA'd email address.

[MattBrittan]

I've added a few comments to your code. My main point here is that I don't think this should require changes in client (because client.Router is depreciated so you should not try to duplicate this).

Having reviewed this I'm still a bit uncertain about it's utility - it adds quite a bit of code that could be fairly quickly handled in OnPublishReceived (without the need for the added overhead). My initial thought is that this library should only provide a basic router that will meet the majority of needs (to help users get started and provide an example) - users with more complex requirements can use OnPublishReceived to do whatever they need.

Note that my reluctance here is not really related to your code. Experience from the V3 client indicates that we should try to keep the client as small as possible (we need to support anything added so want to limit additions to stuff that most users need, whist providing ways for other users to meet their unique needs themselves). The reason for this approach is that if I accept this PR the next request may well be to add support for something like #71 (which is a valid request but not common enough to be worth adding here - we cannot be all things to all users).

I will be very interested in feedback from others reading this PR because othere may see this as an essential addition (it's not something that I would use myself).

Currently my feeling is that it might be more appropriate to:

• Add a readme to paho/extensions & autopaho/extensions stating that items in this folder are not part of the core library, are not covered by semantic versioning, may be broken by updates eleswhere, support will be limited etc.
• Move this PR into those folder (after removing the changes to client.go etc).
• Possibly reference it in the main README (I do need to update that anyway)

[MattBrittan]

Router Router notes that it's depreciated so I'm not keen to see an additional router added here (the idea is that users utilise OnPublishReceived (the README notes "ClientConfig.Router will be removed in a future release"). There are a few reasons for this change, but the main one is that its difficult for us (library authors) to predict how this will be used (and example being routing based on Subscription identifier - see #168 for some discussion on this).

[MattBrittan]

Can you please explain the utility of this? cancel will be called when Route completes atc which time any called routines should have completed (so I think that just using context.Background might make more sense?). Sorry - it may just be that I don't understand your aim here?

[akfaiz]

Agree, will be changed to use context.Background only, before i think we can get base context from mqtt client, like when mqtt shutdown and has long running process at handler, so it can cancel with some timeout.

[MattBrittan]

If there are multiple handlers then the middleware will be called multiple times and I wonder if that is what a user would expect? (guess it depends on what the middleware does).

[akfaiz]

Yes it depend on what middleware, i think middleware like for recoverer it expected to called multiple times when processing message at handler, because if we has panic when processing mqtt packet it can make application crash.

[akfaiz]

Hi @MattBrittan , thanks for your feedback, already update this pr, also for extensions is only at paho, not on autopaho, at autopaho only for example use it.

[nickajacks1]

@MattBrittan here's my 2 cents.

The StandardRouter type itself is useful enough to where I basically use it as the only handler right now. The topic matching is useful. That said, I haven't tried to (or needed to (yet)) extend the StandardRouter for further purposes. I think your instincts are right that people's use cases will be varied enough that it's probably wiser to pursue extensibility over thorough functionality. To that end, it might be better to focus on having composable building blocks. One way that would make it easier for people to implement their own router-like constructs would be to make the route matching logic exported. The StandardRouter doesn't actually do a whole lot more than loop through the registered handlers, check if they match, then call all the handlers for said subscription. By having access to the match function, it would be pretty easy to implement your own router (such as one that passes along a context).

One microcosm of the varied use cases is that the fact that this router uses context.Background means that I wouldn't be able to use it effectively for my case. My applications always pass in a single parent context from the main function for cancellation purposes. This implementation does not provide that, but maybe that's perfectly fine for the author's case. Even though I need to pass context along to a lot of my handlers, I wouldn't be able to use this router to accomplish it. For middleware, it would be perfectly fine for me to simply add additional handlers to the front of the OnPublishReceived chain, but for this author, that might not be good enough.

Just brainstorming on my own a bit here, but a good way to nudge users into the right direction might be to build out a combination of documentation and examples to show various ways to accomplish different cases. I think mosquitto does this well for its plugin API. There are a lot of examples that show what you can accomplish, and it's a good justification to avoid adding too many bells and whistles to the API.

As far as accepting this Router, I would say that's really up to you on how you want to handle extensions. If this were my project, I might request that this be moved to be included only as an example, or be added to some contrib repo.
As a user of this project, what I really wouldn't like is if extensions started to add extra dependencies to go.mod that I don't need. That's not the case here, but as you allude, it's something of a slippery slope.

[akfaiz]

I agree that using context.Background() inside Route is not ideal, since it prevents applications from passing their own parent context (for example, one derived from OS signals).
To improve this, we can provide a new RouteContext method that accepts a context, and let the existing Route method simply fall back to context.Background() for backward compatibility.

Proposed API:

// Backward-compatible entrypoint
func (r *Router) Route(pb *packets.Publish) {
    r.RouteContext(context.Background(), pb)
}

// New context-aware version
func (r *Router) RouteContext(ctx context.Context, pb *packets.Publish) {
    // routing logic using the provided ctx
}

This allows applications to pass their own parent context (e.g., from signal.NotifyContext), while keeping the existing Route signature working as before.

[MattBrittan]

@nickajacks1 thanks very much for your thoughts (as a maintainer I ofter second guess myself when responding to PR's like this so do really appreciate another view).

One way that would make it easier for people to implement their own router-like constructs would be to make the route matching logic exported

The challange with this is that once we make something public we then cannot change it without a new major version (well technically this only applies once we hit V1). As such I've been fairly reluctant to make things public unless there is a compelling reason (have written some fairly nasty workarounds to avoid breaking compatability in the V3 client!). I believe the standard approach here is to recommend that users copy the relevant code (40 odd lines in this case) and I think thats generally workable? I don't really have a feel for how many users need a modified router (suepect it's fairly low and their requirements are fairly diverse). Always love seeing the unexpected ways users utilise the library!.

One microcosm of the varied use cases is that the fact that this router uses context.Background means that I wouldn't be able to use it effectively for my case.

This was one of my concerns - everyone has different requirements and I don't think there is a one size fits all option. Personally I'd probably just wrap the handlers :-).

build out a combination of documentation and examples to show various ways to accomplish different cases.

Would be very happy to see submisions like this :-).

If this were my project, I might request that this be moved to be included only as an example, or be added to some contrib repo.

This was pretty much my feeling. However the challange is the split between paho and autopaho (don't really want an autopaho example to import a paho example) - this is why I thought the extension folder might be a better option. I did consider suggesting a contrib repo but don't think usage is currently big enough to justify this (trying to keep admin to a minimum!) and feel that specifically calling out that this is not covered by SEMVER should be OK for now.

Fully agree with regards to extra dependencies (would be unlikely to accept PR's that add additional production dependencies).

[MattBrittan]

// Backward-compatible entrypoint

I would not really worry about this. The Router interface really dates back to the legacy Router in ClientConfig. With the move to OnPublishReceived I don't see much benefit in adhering to this (and suspect that most end users would want to push in their own context).

Other that that I think that this change would make your PR easier to use/understand. I think quite a few devs think of Context as mostly being mostly about cancellation (I very rarely use it to pass values) so having the router create the context did feel a bit wrong. Making it user created also avoids confusion about it including cancellation signals from the library (something mentioned in issue #286, but I was never clear what context would be passed in that case, so think this approach is better).

[MattBrittan]

Really just the race condition (others are copied from the current router).

[MattBrittan]

Bug (duplicated from main router) - we only have an RLock so should not be modifying aliases. Will raise a separate issue for this (noting it as this was picked up whilst reviewing this PR)

[MattBrittan]

Data race? callCount will be incremented from multiple goroutines (Route only holds a read lock so believe there is a race).

[MattBrittan]

This code (from original) seems likely to cause confusion because it resolves the alias but the message passed to the handler (m) will still have the alias. I'll log a separate issue re this as its an issue in the existing router too.

[MattBrittan]

Apologies for the massive delay on this. I think including this as an example makes sense. I think there is a data race in the test so if you can resolve that I'll accept the PR.