nats.rs
nats.rs copied to clipboard
nats-io
Add a subject type
This PR adds a Subject type to the NATS client. It also includes some convenience stuff like a mutable, owned version SubjectBuf, Tokens as an own type, the possibility to iterate a subjects tokens and a matching algorithm.
For the validation algorithm I tried to stick to the docs from nats.io.
The subject type is for now nowhere integrated, i.e. it would only be useful for users. However, piece by piece substituting str with Subject(Buf) should proof beneficial.
This could also be used to solve #221 .
Yes, in my application I don't validate subjects that much either. But frequently use the Subjects to parse messages I receive from subscription with wildcards to check out where it actually came from and subject.tokens().nth(4) is just way simpler than writting some custum parser every time.
I think the ability to pull tokens and such is a good addition, but would not want to force changes to normal publish and subscribe.
Yes, an integration of the Subject API into the crate was my long time goal. However, I was not sure if the work would be accepted so I waited for a positive response. When I'm done with my other PR I'll continue here.
As predicted, incorporating Subject and SubjectBuf into the whole crate is a lot of work. At the moment I'm not done but the work in the crate itself is mostly done.
I reworked Subject and Token into dynamically sized types, i.e. they behave like str or Path now. Sadly, this includes a little bit of unsafe. I'd like to have your feedback on this matter.
ToDo:
- [x] Fix benches, examples and tests
- [x] Reevaluate the uses of
Subject::new_unchecked()andSubjectBuf::new_unchecked() - [x] Write proper documentation for
SubjectandSubjectBuf - [x] DISCUSS: Public API could take
impl TryInto<&Subject>or a bound to another converter trait. This would probably lessen the migration effort for users.
FYI we need two types. One for publish which can not have wildcards (must be literal) and one for subscriptions which can include wildcards.
The current implementation has a contains_wildcards() -> bool isn't that enough?
Possibly, depends on the contract and when they are valid or not and whether or not we want that correctness enforced in a zero abstractions scenario.
Discussed this a little bit offline last week with @Jarema. I think a type for this can be useful, in that we provide a guarantee that once constructed the type is valid but this feels a bit heavy handed with both Subject and SubjectBuf's.
I feel like the sweet spot could be to have a Subject type, that is a validated type. Then we can implement AsSubject/Into<Subject> traits for conversions from &str, String, etc. Basically somewhere in-between this PR and https://github.com/nats-io/nats.rs/pull/295
I think I might take a quick pass at this soon, see if my vague idea is actually any better in practice 🙂
So after long time I have again some time to work on this. So far I got everything to compile again. Next I'll marge main to catch up with recent development (good work by the way +1). Then I'll fix tests to work again. And after all this is done we can start reviewing this giant of a PR...
@caspervonb
Finally, after months of delay this PR is ready for review, CI is passing, and Subject is introduced to the whole crate.
I slimed the added features a bit down:
-
Tokenas an own type is gone -
Subject::sub_subject()is removed
The AsSubject trait was introduced to allow the use of &str in most places of the public API so the doc tests are mostly left unchanged, and the change for users should be minimal.
@MattesWhite hey! We will look into this as soon as we will tackle this topic in async-nats client to have it in both, consistent.
