Reviewed headers logic and documentation

joffrey-bion / krossbow

A Kotlin multiplatform coroutine-based STOMP client over websockets, with built-in conversions.

MIT License

189 stars 14 forks source link

Reviewed headers logic and documentation #520

Closed freynder closed 2 months ago

freynder commented 2 months ago

Added documentation regarding StompHeader property delegates.
Changed SubscriptionHeader so it is always in a consistent state when constructed through the secondary constructor. This assures that the header will have an id when constructed this way. This eliminates the need to check and correct during subscribe.

freynder commented 2 months ago

In preparation of more important changes to create the stomp server functionality, I have been reviewing the code in more detail and added some documentation + suggestion for change.

freynder commented 2 months ago

Just read about https://github.com/joffrey-bion/krossbow/issues/518 which is much more comprehensive. Will close for now.

joffrey-bion commented 2 months ago

@freynder I was about to comment just that :)

However, the point about the subscription ID header is a good one. The reason why it was like this is from the time when subscriptions where proper flows, where each collection needed to generate a new subscription ID. Since the subscribe() function became a hybrid (suspend + returning a Flow), it's actually not necessary anymore to delay the ID generation. So it's a fair remark. I'll take that into account in #518 too