feat: Allow setting OriginID when sending a message

[selfhoster1312]

Following some discussion in #226, i also believe it makes sense to allow the client to define the origin-id sent by go-xmpp in order to keep track of replies.

I'm not sure if setting id/origin-id to the same thing is the way you want to go. Suggestions welcome.

[poVoq]

Maybe related? https://xmpp.org/extensions/xep-0359.html

[mdosch]

IIRC go-xmpp creates UUIDv7 IDs as those are unique but sortable which is to prefer according to community common sense. If we let the user set arbitrary IDs we might end up with a mishmash. I don't know why it was chosen to return the bytes written and if anyone is using this.
As the version is 0.x there is not really a promise for stable APIs but I usually don't want to break backwards compatibility but this might be a rare case where it makes sense.

I think in general it would make sense to return xmpp.Chat as there might also be other stuff generated in the future that might not be of interest today.

[mdosch]

I made a POC commit for the Send() function there: 1089118

[selfhoster1312]

there might also be other stuff generated in the future

Not sure what you have in mind, but allowing post-processing introspection is indeed a good thing. Especially since i believe go-xmpp doesn't debug log outgoing messages.

The Send POC looks great! My PR wanted minimal breaking change, but like you said, it actually makes sense to break the existing API in this case :-)

Slightly off-topic on the topic of send: am i wrong to assume that writing to stanzaWriter such as in Send is a non-blocking operation that will never stall? i.e. should i place xmpp sends behind a dedicated goroutine? We're trying to eliminate blocking operations and deadlocks in matterbridge.

[kousu]

I'm not sure if setting id/origin-id to the same thing is the way you want to go. Suggestions welcome.

In matterbridge-org/matterbridge#134 's current form it's necessary to work with the <message>'s ID because it that feature needs a common ID space to keep track of which message is what and origin-id is not set by all clients (Dino, for example). Our (me + @sh4sh's) latest verison supports defaulting ID to OriginID which defaults to a UUID. I like the idea of 1089118 a lot, I can definitely work with that instead of making them writable, but in any case I need to read clientMessage.ID.

[selfhoster1312]

origin-id is not set by all clients (Dino, for example)

I don't think this is the case. dino definitely sets origin-id (there's even old issues from 2018 about that feature on their github). And if a client doesn't set origin-id, it just cannot support message replies. message id field is only guaranteed unique on a C2S level… for a message federated with a third party (private message or MUC) only origin-id should be used.

To my knowledge, the only reason to use the ID in a client request is more or less to follow iq responses.

I like the idea of 1089118 a lot, I can definitely work with that instead

Please do, as it seems to be the way the go-xmpp maintainer has chosen :)

[mdosch]

IIRC go-xmpp creates UUIDv7 IDs as those are unique but sortable which is to prefer according to community common sense. If we let the user set arbitrary IDs we might end up with a mishmash. I don't know why it was chosen to return the bytes written and if anyone is using this. As the version is 0.x there is not really a promise for stable APIs but I usually don't want to break backwards compatibility but this might be a rare case where it makes sense.

I think in general it would make sense to return xmpp.Chat as there might also be other stuff generated in the future that might not be of interest today.

@mattn What do you think?
I made a POC of a possible solution in 1089118
It could also be extended to return the number of bytes written, just in case someone used this before.