Add partial support for XEP-0249 Direct MUC Invitations #535

MF1-MS · 2022-08-01T16:30:27Z

This MR adds in partial support for XEP-0249 Direct MUC Invitations

It exposes a method for a MUC to invite a user to the room, and adds a listener to the MultiUserChat listener to inform users of direct invitations they have received.

A new UT class is added to test this function.

I found I could not build Smack without updating bouncy castle to 1.71 so have done so in this MR. Please advise how I would build without doing this if this is not desired.

Flowdalic

Thanks for your contribution. I have left some remarks inline.

I think the most important issue that this is currently somehow entangled with some arbitrary MultiUserChat instance, when its business logic should really go into a new class DirectMucInviteManager.

build.gradle

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java

Flowdalic · 2022-08-03T13:06:25Z

I found I could not build Smack without updating bouncy castle to 1.71 so have done so in this MR. Please advise how I would build without doing this if this is not desired.

Bumping BC is fine. Since you give not details why you couldn't build without updating, I can't really help you there.

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java

Flowdalic · 2022-08-07T15:11:48Z

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java

+                    final MUCUser.Invite mucInvite = null;
+                    final EntityJid from = message.getFrom().asEntityJidIfPossible();
+                    if (from == null) {
+                        LOGGER.warning("Group Chat Invitation from non entity JID: '" + message.toXML() + "'");


I am not super happy about that, as I believe it is likely that components, i.e., domain bare JIDs, could also send directed invites. I think we could change the signature of InvitationsLIstener to use just Jid for inviter.

I'm not sure I agree. The end user the invitation is from should be the same regardless of if they're using an XEP-0249 direct invitation or a mediated XEP-0045 invitation. If we are asserting a user must be an entity jid in the XEP-0045 case then I think it's very reasonable to assert the same here.

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java

smack-extensions/src/test/java/org/jivesoftware/smackx/muc/MultiUserChatTest.java

MF1-MS · 2022-10-10T16:24:42Z

OK sorry about the delay, that's amended now. I can squash the commits and amend the message if you're happy, but leaving as is for now for a cleaner diff.

Are you happy with my other PRs too?:
#537
#538

Flowdalic

I am not sure if you re-invented org.jivesoftware.util.Protocol with InvokeDirectlyResponder in your tests (or similar already existing classes of Smack's test fixtures). Please check that you don't invent the wheel again, I wouldn't be surprised if Smack wouldn't already provide what you need for the tests. Unfortunately, I had no time to dig into this further.

build.gradle

smack-core/src/testFixtures/java/org/jivesoftware/smack/test/util/WaitForPacketListener.java

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChat.java

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/packet/GroupChatInvitation.java

smack-extensions/src/test/java/org/jivesoftware/smackx/muc/MultiUserChatTest.java

MF1-MS · 2023-02-17T15:38:33Z

I am not sure if you re-invented org.jivesoftware.util.Protocol with InvokeDirectlyResponder in your tests (or similar already existing classes of Smack's test fixtures). Please check that you don't invent the wheel again, I wouldn't be surprised if Smack wouldn't already provide what you need for the tests. Unfortunately, I had no time to dig into this further.

I think these are suitably disparate. org.jivesoftware.util.Protocol (which seems to have an out of data docstring, but I found good usage examples in the Socks5 tests) seems focussed on enabling dialogues where Smack will be expecting responses to its requests, and verifying the requests Smack sent. The sent stanza we're verifying with InvokeDirectlyResponder is generated by Smack due to it conceptually receiving a message.

InvokeDirectlyResponder simply builds off the DummyConnection test fixture, capturing an outgoing stanza and verifying it.

MF1-MS · 2023-02-17T16:12:17Z

I can squash the commits and amend the message if you're happy, but leaving as is for now for a cleaner diff.

Are you happy with my other PRs too?:
#537
#538

Flowdalic

LGTM, only one minor thing, then this is ready to go.

Please

squash the commits into one

Add an table row for xep249 to

Smack/smack-java8-full/src/main/java/org/jivesoftware/smackx/package-info.java

Lines 348 to 361 in f6c85d9

    
            * <tr> 
        
            * <td>Roster Versioning</td> 
        
            * <td><a href="https://xmpp.org/extensions/xep-0237.html">XEP-0237</a></td> 
        
            * <td></td> 
        
            * <td>Efficient roster synchronization.</td> 
        
            * </tr> 
        
            * <tr> 
        
            * <td>Message Carbons</td> 
        
            * <td><a href="https://xmpp.org/extensions/xep-0280.html">XEP-0280</a></td> 
        
            * <td>{@link org.jivesoftware.smackx.carbons}</td> 
        
            * <td>Keep all IM clients for a user engaged in a conversation, by carbon-copy outbound messages to all interested 
        
            * resources.</td> 
        
            * </tr> 
        
            * <tr>

rebase on the current master
mention "Fixes SMACK-932" in the commit message's body. Also

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java

MF1-MS · 2023-04-21T16:50:50Z

Should be good to merge now :)

Flowdalic · 2023-04-23T10:36:24Z

The commit message's subject currently reads "Fixes SMACK-932.". That is not really helpful if I look at a commit log only consisting of the commit subject.

Exposes a method for a MUC to invite a user to the room, and adds a listener to the MultiUserChat listener to inform users of direct invitations they have received. Fixes SMACK-932.

MF1-MS · 2023-04-24T09:49:21Z

Ok, commit message re-arranged.

Flowdalic requested changes Aug 3, 2022

View reviewed changes

Flowdalic requested changes Aug 7, 2022

View reviewed changes

MF1-MS requested a review from Flowdalic August 16, 2022 09:17

Flowdalic requested changes Nov 30, 2022

View reviewed changes

Flowdalic requested changes Mar 18, 2023

View reviewed changes

smack-extensions/src/main/java/org/jivesoftware/smackx/muc/MultiUserChatManager.java Outdated Show resolved Hide resolved

MF1-MS force-pushed the mf1-ms/xep_0249_support branch from e03c02a to 75c6663 Compare April 21, 2023 16:49

MF1-MS requested a review from Flowdalic April 21, 2023 16:50

Add partial support for XEP-0249 Direct MUC Invitations.

9e9c233

Exposes a method for a MUC to invite a user to the room, and adds a listener to the MultiUserChat listener to inform users of direct invitations they have received. Fixes SMACK-932.

MF1-MS force-pushed the mf1-ms/xep_0249_support branch from 75c6663 to 9e9c233 Compare April 24, 2023 09:48

Flowdalic changed the title ~~Xep 0249 support~~ Add partial support for XEP-0249 Direct MUC Invitations. Apr 24, 2023

Flowdalic changed the title ~~Add partial support for XEP-0249 Direct MUC Invitations.~~ Add partial support for XEP-0249 Direct MUC Invitations Apr 24, 2023

Flowdalic added this pull request to the merge queue Apr 24, 2023

Merged via the queue into igniterealtime:master with commit 50eb948 Apr 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add partial support for XEP-0249 Direct MUC Invitations #535

Add partial support for XEP-0249 Direct MUC Invitations #535

MF1-MS commented Aug 1, 2022

Flowdalic left a comment

Flowdalic commented Aug 3, 2022

Flowdalic Aug 7, 2022

MF1-MS Aug 12, 2022

MF1-MS commented Oct 10, 2022

Flowdalic left a comment

MF1-MS commented Feb 17, 2023

MF1-MS commented Feb 17, 2023

Flowdalic left a comment

MF1-MS commented Apr 21, 2023

Flowdalic commented Apr 23, 2023

MF1-MS commented Apr 24, 2023

	* <tr>
	* <td>Roster Versioning</td>
	* <td><a href="https://xmpp.org/extensions/xep-0237.html">XEP-0237</a></td>
	* <td></td>
	* <td>Efficient roster synchronization.</td>
	* </tr>
	* <tr>
	* <td>Message Carbons</td>
	* <td><a href="https://xmpp.org/extensions/xep-0280.html">XEP-0280</a></td>
	* <td>{@link org.jivesoftware.smackx.carbons}</td>
	* <td>Keep all IM clients for a user engaged in a conversation, by carbon-copy outbound messages to all interested
	* resources.</td>
	* </tr>
	* <tr>

Add partial support for XEP-0249 Direct MUC Invitations #535

Add partial support for XEP-0249 Direct MUC Invitations #535

Conversation

MF1-MS commented Aug 1, 2022

Flowdalic left a comment

Choose a reason for hiding this comment

Flowdalic commented Aug 3, 2022

Flowdalic Aug 7, 2022

Choose a reason for hiding this comment

MF1-MS Aug 12, 2022

Choose a reason for hiding this comment

MF1-MS commented Oct 10, 2022

Flowdalic left a comment

Choose a reason for hiding this comment

MF1-MS commented Feb 17, 2023

MF1-MS commented Feb 17, 2023

Flowdalic left a comment

Choose a reason for hiding this comment

MF1-MS commented Apr 21, 2023

Flowdalic commented Apr 23, 2023

MF1-MS commented Apr 24, 2023