Cleanup, update dependencies

[ibauersachs]

Cleanups, test with Java 17

Needs

• Various cleanups jitsi-xmpp-extensions#55
• Update dependencies, remove deprecations jicoco#157
• Update dependencies, cleanup deprecations jitsi-videobridge#1760

[bgrozev]

Can you merge master please?

[bgrozev]

I'm tempted to update jitsi-xmpp-extensions to set org.jitsi.xmpp.extensions.jitsimeet.VideoMutedExtension#NAMESPACE to jabber:client. The element isn't used anywhere before the above change.

This makes sense.

Edit: the other option would be to update the client to send a proper namespace. The code is relatively new (just one week Nov. 9, 2021), so I guess there's not a lot of client base yet.

I don't follow. The code in the client has been there for a while. However, I'm pretty sure the client's XML parser/code ignores namespaces so it wouldn't be a breaking change. Still, I'd rather not run the risk of sometthing getting out of sync just for fix a namespace.

[ibauersachs]

I'm tempted to update jitsi-xmpp-extensions to set org.jitsi.xmpp.extensions.jitsimeet.VideoMutedExtension#NAMESPACE to jabber:client. The element isn't used anywhere before the above change.

This makes sense.

Edit: the other option would be to update the client to send a proper namespace. The code is relatively new (just one week Nov. 9, 2021), so I guess there's not a lot of client base yet.

I don't follow.

The video muted extension is only parsed since that date above in Jicofo, so I assumed it's new in the client too.

The code in the client has been there for a while. However, I'm pretty sure the client's XML parser/code ignores namespaces so it wouldn't be a breaking change.

The client would still need to add the namespace, so it would still be breaking change.

Still, I'd rather not run the risk of sometthing getting out of sync just for fix a namespace.

Well, part of the mess in jitsi-xmpp-extensions is the lack of setting a proper namespace to the extensions in the first place.

[bgrozev]

The client would still need to add the namespace, so it would still be breaking change.

The code in jicofo is not actively used yet. It's behind a config flag, waiting for changes in the client.

[bgrozev]

LGTM, but can you resolve the conflicts in the JVB PR so the tests can run (the test output is very misleading, it contains successful torture tests results from a previous run, in reality the job failed before even trying to build the projects).

[ibauersachs]

JVB is rebased, I'm not sure what I need to look at in those log outputs.

[JonathanLennox]

I'm slightly concerned that the on-the-wire XML namespaces of the types you split (RtcpMuxPacketExtension and CandidatePacketExtension) will now end up getting encoded whereas previously they weren't. I don't think this will cause problems for the client (which as I understand it ignores namespaces) but I'm concerned this might make our XMPP messaging bigger, increasing load on the Prosody server (this is already a bottleneck for large conferences). Can you take a look at the jicofo-xmpp logs before and after this change and see if there's any problem there?

[JonathanLennox]

JVB is rebased, I'm not sure what I need to look at in those log outputs.

Auto-merging pom.xml
CONFLICT (content): Merge conflict in pom.xml
Auto-merging jvb/src/main/kotlin/org/jitsi/videobridge/transport/ice/IceTransport.kt

[bgrozev]

Reading AbstractPacketExtension.toXml it looks like it will not encode the namespace when it matches the parent (this now happens in XmlStringBuilder actually).

[bgrozev]

2021-11-19 20:16:05.174 SEVERE: [22] org.jivesoftware.smack.AbstractXMPPConnection.lambda$invokeStanzaCollectorsAndNotifyRecvListeners$8: Exception in packet listener
java.lang.IllegalStateException: Extension element is not of expected class 'org.jitsi.xmpp.extensions.jitsimeet.VideoMutedExtension', because there is no according extension element provider registered with ProviderManager for {jabber:client}videomuted. WARNING: This indicates a serious problem with your Smack setup, probably causing Smack not being able to properly initialize itself.
	at org.jivesoftware.smack.util.XmppElementUtil.castOrThrow(XmppElementUtil.java:111)
	at org.jivesoftware.smack.packet.StanzaView.getExtension(StanzaView.java:98)
	at org.jitsi.impl.protocol.xmpp.ChatMemberImpl.processPresence(ChatMemberImpl.java:300)
	at org.jitsi.impl.protocol.xmpp.ChatRoomImpl.processOtherPresence(ChatRoomImpl.java:782)
	at org.jitsi.impl.protocol.xmpp.ChatRoomImpl.processPresence(ChatRoomImpl.java:842)
	at org.jivesoftware.smackx.muc.MultiUserChat$3.processStanza(MultiUserChat.java:294)
	at org.jivesoftware.smack.AbstractXMPPConnection.lambda$invokeStanzaCollectorsAndNotifyRecvListeners$8(AbstractXMPPConnection.java:1626)
	at org.jivesoftware.smack.AsyncButOrdered$Handler.run(AsyncButOrdered.java:151)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

Does VideoMutedExtension need to be registered somewhere?

[ibauersachs]

Does VideoMutedExtension need to be registered somewhere?

Yes, in the ProviderManager of Smack. I'll push an update.

[ibauersachs]

JVB is rebased, I'm not sure what I need to look at in those log outputs.

Auto-merging pom.xml
CONFLICT (content): Merge conflict in pom.xml
Auto-merging jvb/src/main/kotlin/org/jitsi/videobridge/transport/ice/IceTransport.kt

Was that from on old test before I rebased on Boris' ice4j cleanup?

[JonathanLennox]

Reading AbstractPacketExtension.toXml it looks like it will not encode the namespace when it matches the parent (this now happens in XmlStringBuilder actually).

Ok - it looks like both Jingle and Colibri use the urn:xmpp:jingle:transports:ice-udp:1 namespace for <transport> objects, so copying CandidatePacketExtensions between them is still safe even as the extensions themselves get namespaces.

[ibauersachs]

Seems there's some broken stuff left :-(

• Something with the presence extension/filter @JonathanLennox already flagged in Update dependencies, remove deprecations jicoco#157
• An unparsable stanza, but I don't know which one yet

[JonathanLennox]

Was that from on old test before I rebased on Boris' ice4j cleanup?

No, I think it's just the pom? Sorry, the other merge notification was probably a red herring.

[bgrozev]

LGTM, just the jitsi-videobridge version remains to be fixed.

[codecov]

Codecov Report

Merging #832 (5bfcb5c) into master (b6396a9) will increase coverage by 1.18%.
The diff coverage is 31.81%.

@@             Coverage Diff              @@
##             master     #832      +/-   ##
============================================
+ Coverage     44.65%   45.84%   +1.18%     
- Complexity      762      767       +5     
============================================
  Files           111      112       +1     
  Lines          6628     6908     +280     
  Branches        941      954      +13     
============================================
+ Hits           2960     3167     +207     
- Misses         3252     3317      +65     
- Partials        416      424       +8     

Impacted Files	Coverage Δ
...a/org/jitsi/impl/protocol/xmpp/ChatMemberImpl.java	0.00% <0.00%> (ø)
...ava/org/jitsi/impl/protocol/xmpp/ChatRoomImpl.java	0.00% <0.00%> (ø)
.../java/org/jitsi/jicofo/conference/Participant.java	59.42% <0.00%> (+0.67%)	⬆️
...ence/UnsupportedFeatureConfigurationException.java	0.00% <0.00%> (ø)
...va/org/jitsi/jicofo/jigasi/TranscriberManager.java	20.83% <0.00%> (ø)
...g/jitsi/protocol/xmpp/util/TransportSignaling.java	0.00% <0.00%> (ø)
...jicofo/conference/ParticipantChannelAllocator.java	43.16% <50.00%> (-0.64%)	⬇️
...c/main/java/org/jitsi/jicofo/bridge/JvbDoctor.java	25.74% <100.00%> (ø)
...c/main/java/org/jitsi/jicofo/xmpp/BaseBrewery.java	39.32% <100.00%> (ø)
...in/kotlin/org/jitsi/jicofo/xmpp/JigasiIqHandler.kt	78.76% <100.00%> (+0.66%)	⬆️
... and 53 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b6396a9...5bfcb5c. Read the comment docs.

[bgrozev]

Thank you for your work, Ingo!