Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid xmlstring builder toString #1118

Merged
merged 3 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import edu.umd.cs.findbugs.annotations.*;
import org.jitsi.utils.logging2.*;
import org.jitsi.xmpp.util.*;
import org.jivesoftware.smack.*;
import org.jivesoftware.smack.debugger.*;
import org.jivesoftware.smack.packet.*;
Expand Down Expand Up @@ -66,13 +67,17 @@ public PacketDebugger(XMPPConnection connection, @NonNull String id)
@Override
public void onIncomingStreamElement(TopLevelStreamElement streamElement)
{
logger.debug(() -> "RCV PKT (" + id + "): " + streamElement.toXML());
logger.debug(
() -> "RCV PKT (" + id + "): " + XmlStringBuilderUtil.Companion.toStringOpt(streamElement.toXML())
);
}

@Override
public void onOutgoingStreamElement(TopLevelStreamElement streamElement)
{
logger.debug(() -> "SENT PKT (" + id + "): " + streamElement.toXML());
logger.debug(
() -> "SENT PKT (" + id + "): " + XmlStringBuilderUtil.Companion.toStringOpt(streamElement.toXML())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If your method in jitsi-xmpp-extensions were on Element rather than ExtensionElement this could just use the non-companion flavor of toStringOpt -- is it worth it? (I think it clearly would be if this were Kotlin, but it's less obvious for Java.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this was awkward, but I didn't think it's worth another jitsi-xmpp-extensions update

);
}

// It's fine to do non-atomic as it's only 1 thread doing write operation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.jetbrains.annotations.*;
import org.jitsi.jicofo.xmpp.muc.*;
import org.jitsi.utils.logging2.*;
import org.jitsi.xmpp.util.*;
import org.jivesoftware.smack.*;
import org.jivesoftware.smack.packet.*;
import org.jxmpp.jid.*;
Expand Down Expand Up @@ -303,7 +304,7 @@ protected void processInstanceStatusChanged(@NotNull EntityFullJid jid, @NotNull
instance.status = extension;
}

logger.debug("New presence from " + jid + ": " + extension.toXML());
logger.debug(() -> "New presence from " + jid + ": " + XmlStringBuilderUtil.toStringOpt(extension));
onInstanceStatusChanged(jid, extension);
}

Expand Down
5 changes: 3 additions & 2 deletions jicofo-common/src/main/kotlin/org/jitsi/jicofo/xmpp/Util.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.jitsi.jicofo.xmpp

import org.jitsi.utils.logging2.LoggerImpl
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.AbstractXMPPConnection
import org.jivesoftware.smack.SmackException
import org.jivesoftware.smack.XMPPConnection
Expand Down Expand Up @@ -61,9 +62,9 @@ fun parseJidFromClientProxyJid(
fun XMPPConnection.tryToSendStanza(stanza: Stanza) = try {
sendStanza(stanza)
} catch (e: SmackException.NotConnectedException) {
logger.error("No connection - unable to send packet: " + stanza.toXML(), e)
logger.error("No connection - unable to send packet: ${stanza.toXML().toStringOpt()}", e)
} catch (e: InterruptedException) {
logger.error("Failed to send packet: " + stanza.toXML().toString(), e)
logger.error("Failed to send packet: ${stanza.toXML().toStringOpt()}", e)
}

@Throws(SmackException.NotConnectedException::class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import org.jitsi.xmpp.extensions.jingle.JinglePacketFactory
import org.jitsi.xmpp.extensions.jingle.Reason
import org.jitsi.xmpp.extensions.jingle.RtpDescriptionPacketExtension
import org.jitsi.xmpp.extensions.jitsimeet.JsonMessageExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.AbstractXMPPConnection
import org.jivesoftware.smack.SmackException
import org.jivesoftware.smack.packet.ExtensionElement
Expand Down Expand Up @@ -143,7 +144,7 @@ class JingleSession(
val response = if (error == null) {
IQ.createResultIQ(iq)
} else {
logger.info("Returning error: request=${iq.toXML()}, error=${error.toXML()} ")
logger.info("Returning error: request=${iq.toStringOpt()}, error=${error.toStringOpt()} ")
IQ.createErrorResponse(iq, error)
}
connection.tryToSendStanza(response)
Expand Down Expand Up @@ -207,7 +208,7 @@ class JingleSession(
return if (response?.type == IQ.Type.result) {
true
} else {
logger.error("Unexpected response to transport-replace: ${response?.toXML()}")
logger.error("Unexpected response to transport-replace: ${response?.toStringOpt()}")
false
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import org.jitsi.utils.event.EventEmitter
import org.jitsi.utils.event.SyncEventEmitter
import org.jitsi.utils.logging2.createLogger
import org.jitsi.utils.observableWhenChanged
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.PresenceListener
import org.jivesoftware.smack.SmackException
import org.jivesoftware.smack.XMPPConnection
Expand Down Expand Up @@ -309,7 +310,7 @@ class ChatRoomImpl(
try {
val reply = xmppProvider.xmppConnection.sendIqAndGetResponse(admin)
if (reply == null || reply.type != IQ.Type.result) {
logger.warn("Failed to grant ownership: ${reply?.toXML() ?: "timeout"}")
logger.warn("Failed to grant ownership: ${reply?.toString() ?: "timeout"}")
}
} catch (e: SmackException.NotConnectedException) {
logger.warn("Failed to grant ownership: XMPP disconnected")
Expand Down Expand Up @@ -505,15 +506,15 @@ class ChatRoomImpl(
*/
override fun processPresence(presence: Presence?) {
if (presence == null || presence.error != null) {
logger.warn("Unable to handle packet: ${presence?.toXML()}")
logger.warn("Unable to handle packet: ${presence?.toXML()?.toStringOpt()}")
return
}
logger.trace { "Presence received ${presence.toXML()}" }
logger.trace { "Presence received ${presence.toXML().toStringOpt()}" }

// Should never happen, but log if something is broken
val myOccupantJid = this.myOccupantJid
if (myOccupantJid == null) {
logger.error("Processing presence when myOccupantJid is not set: ${presence.toXML()}")
logger.error("Processing presence when myOccupantJid is not set: ${presence.toXML().toStringOpt()}")
}
if (myOccupantJid != null && myOccupantJid.equals(presence.from)) {
processOwnPresence(presence)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.jitsi.utils.logging2.*;
import org.jitsi.xmpp.extensions.health.*;

import org.jitsi.xmpp.util.*;
import org.jivesoftware.smack.*;
import org.jivesoftware.smack.packet.*;

Expand Down Expand Up @@ -236,13 +237,16 @@ protected void doHealthCheck()
|| StanzaError.Condition.service_unavailable.equals(condition))
{
// Health check failure
logger.warn("Health check failed for: " + bridge + ": " + error.toXML().toString());
logger.warn(
"Health check failed for: " + bridge + ": " + XmlStringBuilderUtil.toStringOpt(error)
);
listener.healthCheckFailed(bridge.getJid());
}
else
{
logger.error(
"Unexpected error returned by the bridge: " + bridge + ", err: " + response.toXML());
"Unexpected error returned by the bridge: " + bridge + ", err: "
+ XmlStringBuilderUtil.toStringOpt(response));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import org.jitsi.xmpp.extensions.colibri2.Transport
import org.jitsi.xmpp.extensions.jingle.DtlsFingerprintPacketExtension
import org.jitsi.xmpp.extensions.jingle.ExtmapAllowMixedPacketExtension
import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.StanzaCollector
import org.jivesoftware.smack.packet.ErrorIQ
import org.jivesoftware.smack.packet.IQ
Expand Down Expand Up @@ -110,7 +111,7 @@ class Colibri2Session(
participant.medias.forEach { endpoint.addMedia(it) }
request.addEndpoint(endpoint.build())

logger.trace { "Sending allocation request for ${participant.id}: ${request.build().toXML()}" }
logger.trace { "Sending allocation request for ${participant.id}: ${request.build().toStringOpt()}" }
created = true
return xmppConnection.createStanzaCollectorAndSend(request.build())
}
Expand Down Expand Up @@ -259,7 +260,7 @@ class Colibri2Session(
relayId: String
) {
logger.info("Setting relay transport for $relayId")
logger.debug { "Setting relay transport for $relayId: ${transport.toXML()}" }
logger.debug { "Setting relay transport for $relayId: ${transport.toStringOpt()}" }
relays[relayId]?.setTransport(transport)
?: throw IllegalStateException("Relay $relayId doesn't exist (bridge=${this.relayId}")
}
Expand Down Expand Up @@ -292,10 +293,10 @@ class Colibri2Session(
* failure.
*/
private fun sendRequest(iq: IQ, name: String) {
logger.debug { "Sending $name request: ${iq.toXML()}" }
logger.debug { "Sending $name request: ${iq.toStringOpt()}" }
xmppConnection.sendIqAndHandleResponseAsync(iq) {
when (it) {
is ConferenceModifiedIQ -> logger.debug { "Received $name response: ${it.toXML()}" }
is ConferenceModifiedIQ -> logger.debug { "Received $name response: ${it.toStringOpt()}" }
null -> logger.info("$name request timed out. Ignoring.")
else -> {
if (it is ErrorIQ) {
Expand All @@ -313,14 +314,14 @@ class Colibri2Session(
val reInvite = reason == Colibri2Error.Reason.UNKNOWN_ENDPOINT && endpointId != null
if (reInvite) {
logger.warn(
"Endpoint [$endpointId] is not found, session failed: ${it.toXML()}, " +
"request was: ${iq.toXML()}"
"Endpoint [$endpointId] is not found, session failed: ${it.toStringOpt()}, " +
"request was: ${iq.toStringOpt()}"
)
colibriSessionManager.endpointFailed(endpointId!!)
return@sendIqAndHandleResponseAsync
}
}
logger.error("Received error response for $name, session failed: ${it.toXML()}")
logger.error("Received error response for $name, session failed: ${it.toStringOpt()}")
colibriSessionManager.sessionFailed(this@Colibri2Session)
}
}
Expand Down Expand Up @@ -367,22 +368,22 @@ class Colibri2Session(
/** Send a request to allocate a new relay, and submit a task to wait for a response. */
internal fun start(initialParticipants: List<ParticipantInfo>) {
val request = buildCreateRelayRequest(initialParticipants)
logger.trace { "Sending create relay: ${request.toXML()}" }
logger.trace { "Sending create relay: ${request.toStringOpt()}" }

xmppConnection.sendIqAndHandleResponseAsync(request) { response ->
// Wait for a response to the relay allocation request. When a response is received, parse the contained
// transport and forward it to the associated [Relay] for the remote side via [colibriSessionManager]
logger.trace { "Received response: ${response?.toXML()}" }
logger.trace { "Received response: ${response?.toStringOpt()}" }
if (response !is ConferenceModifiedIQ) {
logger.error("Received error: ${response?.toXML() ?: "timeout"}")
logger.error("Received error: ${response?.toStringOpt() ?: "timeout"}")
colibriSessionManager.sessionFailed(this@Colibri2Session)
return@sendIqAndHandleResponseAsync
}

// TODO: We just assume that the response has a single [Colibri2Relay].
val transport = response.relays.firstOrNull()?.transport
?: run {
logger.error("No transport in response: ${response.toXML()}")
logger.error("No transport in response: ${response.toStringOpt()}")
colibriSessionManager.sessionFailed(this@Colibri2Session)
return@sendIqAndHandleResponseAsync
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import org.jitsi.xmpp.extensions.colibri2.Colibri2Error
import org.jitsi.xmpp.extensions.colibri2.ConferenceModifiedIQ
import org.jitsi.xmpp.extensions.colibri2.InitialLastN
import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.AbstractXMPPConnection
import org.jivesoftware.smack.StanzaCollector
import org.jivesoftware.smack.packet.ErrorIQ
Expand Down Expand Up @@ -359,7 +360,7 @@ class ColibriV2SessionManager(
val response: IQ?
try {
response = stanzaCollector.nextResult()
logger.trace { "Received response: ${response?.toXML()}" }
logger.trace { "Received response: ${response?.toStringOpt()}" }
} finally {
stanzaCollector.cancel()
}
Expand Down Expand Up @@ -435,13 +436,13 @@ class ColibriV2SessionManager(
Colibri2Error.ELEMENT,
Colibri2Error.NAMESPACE
)?.reason
logger.info("Received error response: ${response.toXML()}")
logger.info("Received error response: ${response.toStringOpt()}")
when (response.error?.condition) {
bad_request -> {
// Most probably we sent a bad request.
// If we flag the bridge as non-operational we may disrupt other conferences.
// If we trigger a re-invite we may cause the same error repeating.
throw ColibriAllocationFailedException("Bad request: ${response.error?.toXML()?.toString()}", false)
throw ColibriAllocationFailedException("Bad request: ${response.error?.toStringOpt()}", false)
}
item_not_found -> {
if (reason == Colibri2Error.Reason.CONFERENCE_NOT_FOUND) {
Expand All @@ -460,7 +461,7 @@ class ColibriV2SessionManager(
if (reason == null) {
// An error NOT coming from the bridge.
throw ColibriAllocationFailedException(
"XMPP error: ${response.error?.toXML()}",
"XMPP error: ${response.error?.toStringOpt()}",
true
)
} else if (reason == Colibri2Error.Reason.CONFERENCE_ALREADY_EXISTS) {
Expand All @@ -473,7 +474,7 @@ class ColibriV2SessionManager(
// we can't expire a conference without listing its individual endpoints and we think there
// were none.
// We remove the bridge from the conference (expiring it) and re-invite the participants.
throw ColibriAllocationFailedException("Colibri error: ${response.error?.toXML()}", true)
throw ColibriAllocationFailedException("Colibri error: ${response.error?.toStringOpt()}", true)
}
}
service_unavailable -> {
Expand All @@ -489,7 +490,7 @@ class ColibriV2SessionManager(
}
else -> {
session.bridge.isOperational = false
throw ColibriAllocationFailedException("Error: ${response.error?.toXML()}", true)
throw ColibriAllocationFailedException("Error: ${response.error?.toStringOpt()}", true)
}
}
}
Expand Down Expand Up @@ -618,7 +619,7 @@ class ColibriV2SessionManager(
relayId: String
) {
logger.info("Received transport from $session for relay $relayId")
logger.debug { "Received transport from $session for relay $relayId: ${transport.toXML()}" }
logger.debug { "Received transport from $session for relay $relayId: ${transport.toStringOpt()}" }
synchronized(syncRoot) {
// It's possible a new session was started for the same bridge.
if (!sessions.containsKey(session.bridge.relayId) || sessions[session.bridge.relayId] != session) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.jitsi.xmpp.extensions.jingle.JingleUtils;
import org.jitsi.xmpp.extensions.jitsimeet.*;
import org.jitsi.utils.logging2.*;
import org.jitsi.xmpp.util.*;
import org.jivesoftware.smack.*;
import org.jivesoftware.smack.packet.*;
import org.jxmpp.jid.*;
Expand Down Expand Up @@ -187,7 +188,9 @@ private void doRun()
}
else
{
logger.warn("Failed to convert ContentPacketExtension to Media: " + content.toXML());
logger.warn(
"Failed to convert ContentPacketExtension to Media: "
+ XmlStringBuilderUtil.toStringOpt(content));
}
});
// This makes the bridge signal its private host candidates. We enable them for backend components, because
Expand Down
12 changes: 6 additions & 6 deletions jicofo/src/main/java/org/jitsi/jicofo/jibri/JibriSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.jitsi.xmpp.extensions.jibri.JibriIq.*;
import org.jetbrains.annotations.*;
import org.jitsi.utils.logging2.*;
import org.jitsi.xmpp.util.*;
import org.jivesoftware.smack.*;
import org.jivesoftware.smack.packet.*;
import org.jxmpp.jid.*;
Expand Down Expand Up @@ -351,7 +352,7 @@ synchronized public void stop(Jid initiator)
stopRequest.setAction(JibriIq.Action.STOP);
stopRequest.setSessionId(this.sessionId);

logger.info("Trying to stop: " + stopRequest.toXML());
logger.info("Trying to stop: " + XmlStringBuilderUtil.toStringOpt(stopRequest));

SmackFuture<IQ, Exception> future =
jibriDetector.getXmppConnection().sendIqRequestAsync(stopRequest, 60000);
Expand All @@ -366,7 +367,7 @@ synchronized public void stop(Jid initiator)
{
logger.error(
"Unexpected response to stop iq: "
+ (stanza != null ? stanza.toXML() : "null"));
+ (stanza != null ? XmlStringBuilderUtil.toStringOpt(stanza) : "null"));

JibriIq error = new JibriIq();

Expand Down Expand Up @@ -428,7 +429,7 @@ private void processJibriIqFromJibri(JibriIq iq)
JibriIq.Status status = iq.getStatus();
if (!JibriIq.Status.UNDEFINED.equals(status))
{
logger.info("Updating status from JIBRI: " + iq.toXML() + " for " + roomName);
logger.info("Updating status from JIBRI: " + XmlStringBuilderUtil.toStringOpt(iq) + " for " + roomName);

handleJibriStatusUpdate(iq.getFrom(), status, iq.getFailureReason(), iq.getShouldRetry());
}
Expand Down Expand Up @@ -535,7 +536,7 @@ private void sendJibriStartIq(final Jid jibriJid)
}
if (!(reply instanceof JibriIq))
{
logger.error("Unexpected response to start request: " + reply.toXML());
logger.error("Unexpected response to start request: " + XmlStringBuilderUtil.toStringOpt(reply));

throw new StartException.UnexpectedResponse();
}
Expand All @@ -549,8 +550,7 @@ private void sendJibriStartIq(final Jid jibriJid)
if (!isPendingResponse(jibriIq))
{
logger.error(
"Unexpected status received in response to the start IQ: "
+ jibriIq.toXML());
"Unexpected status received in response to the start IQ: " + XmlStringBuilderUtil.toStringOpt(jibriIq));

throw new StartException.UnexpectedResponse();
}
Expand Down
Loading
Loading