On Tue, Mar 22, 2011 at 23:55, Peter Saint-Andre <[email protected]> wrote: > Hi Niklas, > > What do you mean by "relaying" here? If you're just talking about > delivery of stanzas address to a full JID, then that is standard XMPP > functionality.
"Relaying" here refers to finding the right receiver for a given stanza. And I agree, you have to properly check against the XMPP spec to get things right. I find the flow of stanzas to be a bit complicated at times, in the sense that I need to (re)check the RFCs and don't know inituitively what needs to be done. But since the XMPP spec is very well written, I figure it out eventually. Thanks, Peter, for writing those great specs! We owe you a [beverage of your choice]! Bernd > > Peter > > On 3/22/11 4:43 PM, Niklas Gustavsson wrote: >> Bernd, >> >> could you please review this commit. I'm not very experienced with how >> IQ stanza relaying is supposed to work. >> >> /niklas >> >> On Tue, Mar 22, 2011 at 11:40 PM, <[email protected]> wrote: >>> Author: ngn >>> Date: Tue Mar 22 22:40:49 2011 >>> New Revision: 1084393 >>> >>> URL: http://svn.apache.org/viewvc?rev=1084393&view=rev >>> Log: >>> Implement relaying of IQ stanzas and disco IQ stanzas to a user with a full >>> JID provided (VYSPER-278) >>> >>> Modified: >>> >>> mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/delivery/inbound/DeliveringInternalInboundStanzaRelay.java >>> >>> mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandler.java >>> >>> mina/vysper/trunk/server/core/src/test/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandlerTestCase.java >>> >>> Modified: >>> mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/delivery/inbound/DeliveringInternalInboundStanzaRelay.java >>> URL: >>> http://svn.apache.org/viewvc/mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/delivery/inbound/DeliveringInternalInboundStanzaRelay.java?rev=1084393&r1=1084392&r2=1084393&view=diff >>> ============================================================================== >>> --- >>> mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/delivery/inbound/DeliveringInternalInboundStanzaRelay.java >>> (original) >>> +++ >>> mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/delivery/inbound/DeliveringInternalInboundStanzaRelay.java >>> Tue Mar 22 22:40:49 2011 >>> @@ -232,7 +232,7 @@ public class DeliveringInternalInboundSt >>> } >>> } else if (IQStanza.isOfType(stanza)) { >>> // TODO handle on behalf of the user/client >>> - throw new RuntimeException("inbound IQ not yet handled"); >>> + return relayToBestSessions(false); >>> } >>> >>> return relayNotPossible(); >>> >>> Modified: >>> mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandler.java >>> URL: >>> http://svn.apache.org/viewvc/mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandler.java?rev=1084393&r1=1084392&r2=1084393&view=diff >>> ============================================================================== >>> --- >>> mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandler.java >>> (original) >>> +++ >>> mina/vysper/trunk/server/core/src/main/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandler.java >>> Tue Mar 22 22:40:49 2011 >>> @@ -25,6 +25,8 @@ import org.apache.vysper.compliance.Spec >>> import org.apache.vysper.compliance.SpecCompliant; >>> import org.apache.vysper.xml.fragment.XMLElement; >>> import org.apache.vysper.xmpp.addressing.Entity; >>> +import org.apache.vysper.xmpp.delivery.failure.DeliveryException; >>> +import >>> org.apache.vysper.xmpp.delivery.failure.ReturnErrorToSenderFailureStrategy; >>> import org.apache.vysper.xmpp.modules.core.base.handler.DefaultIQHandler; >>> import >>> org.apache.vysper.xmpp.modules.servicediscovery.collection.ServiceCollector; >>> import >>> org.apache.vysper.xmpp.modules.servicediscovery.collection.ServiceDiscoveryRequestListenerRegistry; >>> @@ -58,7 +60,8 @@ public class DiscoInfoIQHandler extends >>> >>> @Override >>> protected boolean verifyInnerElement(Stanza stanza) { >>> - return verifyInnerElementWorker(stanza, "query") && >>> verifyInnerNamespace(stanza, NamespaceURIs.XEP0030_SERVICE_DISCOVERY_INFO); >>> + return verifyInnerElementWorker(stanza, "query") >>> + && verifyInnerNamespace(stanza, >>> NamespaceURIs.XEP0030_SERVICE_DISCOVERY_INFO); >>> } >>> >>> @Override >>> @@ -78,8 +81,8 @@ public class DiscoInfoIQHandler extends >>> } >>> >>> if (serviceCollector == null) { >>> - return >>> ServerErrorResponses.getStanzaError(StanzaErrorCondition.INTERNAL_SERVER_ERROR, >>> - stanza, StanzaErrorType.CANCEL, "cannot retrieve >>> IQ-get-info result from internal components", >>> + return >>> ServerErrorResponses.getStanzaError(StanzaErrorCondition.INTERNAL_SERVER_ERROR, >>> stanza, >>> + StanzaErrorType.CANCEL, "cannot retrieve IQ-get-info >>> result from internal components", >>> getErrorLanguage(serverRuntimeContext, sessionContext), >>> null); >>> } >>> >>> @@ -109,25 +112,30 @@ public class DiscoInfoIQHandler extends >>> List<InfoElement> elements = null; >>> try { >>> Entity from = stanza.getFrom(); >>> - if (from == null) from = sessionContext.getInitiatingEntity(); >>> + if (from == null) >>> + from = sessionContext.getInitiatingEntity(); >>> if (isServerInfoRequest) { >>> - elements = serviceCollector.processServerInfoRequest(new >>> InfoRequest(from, to, node, stanza >>> - .getID())); >>> + elements = serviceCollector.processServerInfoRequest(new >>> InfoRequest(from, to, node, stanza.getID())); >>> } else if (isComponentInfoRequest) { >>> - elements = >>> serviceCollector.processComponentInfoRequest(new InfoRequest(from, to, node, >>> - stanza.getID())); >>> + elements = serviceCollector >>> + .processComponentInfoRequest(new InfoRequest(from, >>> to, node, stanza.getID())); >>> } else { >>> - elements = serviceCollector.processInfoRequest(new >>> InfoRequest(from, to, node, stanza >>> - .getID())); >>> + // "When an entity sends a disco#info request to a bare >>> JID (<[email protected]>) hosted by a server, >>> + // the server itself MUST reply on behalf of the hosted >>> account, either with an IQ-error or an IQ-result" >>> + if (to.isResourceSet()) { >>> + relayOrWrite(stanza, serverRuntimeContext, >>> sessionContext); >>> + return null; >>> + } else { >>> + elements = serviceCollector.processInfoRequest(new >>> InfoRequest(from, to, node, stanza.getID())); >>> + } >>> } >>> } catch (ServiceDiscoveryRequestException e) { >>> // the request yields an error >>> StanzaErrorCondition stanzaErrorCondition = >>> e.getErrorCondition(); >>> if (stanzaErrorCondition == null) >>> stanzaErrorCondition = >>> StanzaErrorCondition.INTERNAL_SERVER_ERROR; >>> - return >>> ServerErrorResponses.getStanzaError(stanzaErrorCondition, stanza, >>> - StanzaErrorType.CANCEL, "disco info request failed.", >>> - getErrorLanguage(serverRuntimeContext, >>> sessionContext), null); >>> + return >>> ServerErrorResponses.getStanzaError(stanzaErrorCondition, stanza, >>> StanzaErrorType.CANCEL, >>> + "disco info request failed.", >>> getErrorLanguage(serverRuntimeContext, sessionContext), null); >>> } >>> >>> //TODO check that elementSet contains at least one identity element >>> and on feature element! >>> @@ -146,4 +154,36 @@ public class DiscoInfoIQHandler extends >>> >>> return stanzaBuilder.build(); >>> } >>> + >>> + @Override >>> + protected Stanza handleResult(IQStanza stanza, ServerRuntimeContext >>> serverRuntimeContext, >>> + SessionContext sessionContext) { >>> + >>> + if (stanza.getTo().isResourceSet()) { >>> + relayOrWrite(stanza, serverRuntimeContext, sessionContext); >>> + return null; >>> + } else { >>> + return super.handleResult(stanza, serverRuntimeContext, >>> sessionContext); >>> + } >>> + } >>> + >>> + private void relayOrWrite(IQStanza stanza, ServerRuntimeContext >>> serverRuntimeContext, SessionContext sessionContext) { >>> + boolean isOutbound = >>> !sessionContext.getInitiatingEntity().equals(stanza.getTo().getBareJID()); >>> + if (isOutbound) { >>> + try { >>> + Entity from = stanza.getFrom(); >>> + if (from == null) { >>> + from = sessionContext.getInitiatingEntity(); >>> + } >>> + Stanza forward = StanzaBuilder.createForwardStanza(stanza, >>> from, null); >>> + >>> + >>> serverRuntimeContext.getStanzaRelay().relay(stanza.getTo(), forward, >>> + new >>> ReturnErrorToSenderFailureStrategy(serverRuntimeContext.getStanzaRelay())); >>> + } catch (DeliveryException e) { >>> + logger.warn("relaying IQ failed", e); >>> + } >>> + } else { >>> + sessionContext.getResponseWriter().write(stanza); >>> + } >>> + } >>> } >>> >>> Modified: >>> mina/vysper/trunk/server/core/src/test/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandlerTestCase.java >>> URL: >>> http://svn.apache.org/viewvc/mina/vysper/trunk/server/core/src/test/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandlerTestCase.java?rev=1084393&r1=1084392&r2=1084393&view=diff >>> ============================================================================== >>> --- >>> mina/vysper/trunk/server/core/src/test/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandlerTestCase.java >>> (original) >>> +++ >>> mina/vysper/trunk/server/core/src/test/java/org/apache/vysper/xmpp/modules/servicediscovery/handler/DiscoInfoIQHandlerTestCase.java >>> Tue Mar 22 22:40:49 2011 >>> @@ -27,6 +27,8 @@ import junit.framework.Assert; >>> import org.apache.vysper.StanzaAssert; >>> import org.apache.vysper.xmpp.addressing.Entity; >>> import org.apache.vysper.xmpp.addressing.EntityImpl; >>> +import org.apache.vysper.xmpp.delivery.StanzaRelay; >>> +import org.apache.vysper.xmpp.delivery.failure.DeliveryFailureStrategy; >>> import >>> org.apache.vysper.xmpp.modules.servicediscovery.collection.ServiceCollector; >>> import >>> org.apache.vysper.xmpp.modules.servicediscovery.collection.ServiceDiscoveryRequestListenerRegistry; >>> import org.apache.vysper.xmpp.modules.servicediscovery.management.Feature; >>> @@ -44,6 +46,7 @@ import org.apache.vysper.xmpp.stanza.Sta >>> import org.apache.vysper.xmpp.stanza.StanzaBuilder; >>> import org.apache.vysper.xmpp.stanza.StanzaErrorCondition; >>> import org.apache.vysper.xmpp.stanza.XMPPCoreStanza; >>> +import org.apache.vysper.xmpp.writer.StanzaWriter; >>> import org.junit.Before; >>> import org.junit.Test; >>> import org.mockito.Mockito; >>> @@ -56,10 +59,13 @@ public class DiscoInfoIQHandlerTestCase >>> private static final Entity SERVER = >>> EntityImpl.parseUnchecked("vysper.org"); >>> private static final Entity COMPONENT = >>> EntityImpl.parseUnchecked("comp.vysper.org"); >>> private static final Entity USER = >>> EntityImpl.parseUnchecked("[email protected]"); >>> + private static final Entity USER_WITH_RESOURCE = >>> EntityImpl.parseUnchecked("[email protected]/res1"); >>> >>> - private ServerRuntimeContext serverRuntimeContext = >>> Mockito.mock(ServerRuntimeContext.class); >>> - private SessionContext sessionContext = >>> Mockito.mock(SessionContext.class); >>> - private ServiceCollector serviceCollector = >>> Mockito.mock(ServiceCollector.class); >>> + private ServerRuntimeContext serverRuntimeContext = >>> mock(ServerRuntimeContext.class); >>> + private SessionContext sessionContext = mock(SessionContext.class); >>> + private ServiceCollector serviceCollector = >>> mock(ServiceCollector.class); >>> + private StanzaRelay stanzaRelay = mock(StanzaRelay.class); >>> + private StanzaWriter stanzaWriter = mock(StanzaWriter.class); >>> >>> private IQStanza stanza = (IQStanza) IQStanza.getWrapper(buildStanza()); >>> >>> @@ -148,6 +154,8 @@ public class DiscoInfoIQHandlerTestCase >>> .thenReturn(serviceCollector); >>> >>> when(serverRuntimeContext.getServerEnitity()).thenReturn(SERVER); >>> + >>> when(serverRuntimeContext.getStanzaRelay()).thenReturn(stanzaRelay); >>> + when(sessionContext.getResponseWriter()).thenReturn(stanzaWriter); >>> } >>> >>> @Test >>> @@ -186,6 +194,32 @@ public class DiscoInfoIQHandlerTestCase >>> } >>> >>> @Test >>> + public void handleGetToUserWithResource() throws Exception { >>> + when(sessionContext.getInitiatingEntity()).thenReturn(FROM); >>> + >>> + IQStanza stanza = createRequest(USER_WITH_RESOURCE); >>> + >>> + Stanza response = handler.handleGet(stanza, serverRuntimeContext, >>> sessionContext); >>> + >>> + Assert.assertNull(response); >>> + >>> + verify(stanzaRelay).relay(eq(USER_WITH_RESOURCE), eq(stanza), >>> any(DeliveryFailureStrategy.class)); >>> + } >>> + >>> + @Test >>> + public void handleGetToUserWithResourceInbound() throws Exception { >>> + when(sessionContext.getInitiatingEntity()).thenReturn(USER); >>> + >>> + IQStanza stanza = createRequest(USER_WITH_RESOURCE); >>> + >>> + Stanza response = handler.handleGet(stanza, serverRuntimeContext, >>> sessionContext); >>> + >>> + Assert.assertNull(response); >>> + >>> + verify(stanzaWriter).write(stanza); >>> + } >>> + >>> + @Test >>> public void handleGetToNonExistingComponent() throws Exception { >>> IQStanza stanza = createRequest(COMPONENT); >>> >>> @@ -253,6 +287,45 @@ public class DiscoInfoIQHandlerTestCase >>> StanzaAssert.assertEquals(expected, response); >>> } >>> >>> + @Test >>> + public void handleResultToUserWithResource() throws Exception { >>> + when(sessionContext.getInitiatingEntity()).thenReturn(FROM); >>> + >>> + IQStanza stanza = createRequest(USER_WITH_RESOURCE, >>> IQStanzaType.RESULT); >>> + >>> + Stanza response = handler.handleResult(stanza, >>> serverRuntimeContext, sessionContext); >>> + >>> + Assert.assertNull(response); >>> + >>> + verify(stanzaRelay).relay(eq(USER_WITH_RESOURCE), eq(stanza), >>> any(DeliveryFailureStrategy.class)); >>> + } >>> + >>> + @Test >>> + public void handleResultToUserWithResourceInbound() throws Exception { >>> + when(sessionContext.getInitiatingEntity()).thenReturn(USER); >>> + >>> + IQStanza stanza = createRequest(USER_WITH_RESOURCE, >>> IQStanzaType.RESULT); >>> + >>> + Stanza response = handler.handleResult(stanza, >>> serverRuntimeContext, sessionContext); >>> + >>> + Assert.assertNull(response); >>> + >>> + verify(stanzaWriter).write(stanza); >>> + } >>> + >>> + @Test >>> + public void handleResultToUser() throws Exception { >>> + when(sessionContext.getInitiatingEntity()).thenReturn(FROM); >>> + >>> + IQStanza stanza = createRequest(USER, IQStanzaType.RESULT); >>> + >>> + Stanza response = handler.handleResult(stanza, >>> serverRuntimeContext, sessionContext); >>> + >>> + Stanza expected = createErrorResponse(SERVER, >>> "feature-not-implemented"); >>> + >>> + StanzaAssert.assertEquals(expected, response); >>> + } >>> + >>> private Stanza createErrorResponse(Entity from, String error) { >>> Stanza expected = StanzaBuilder.createIQStanza(from, FROM, >>> IQStanzaType.ERROR, "id1") >>> .startInnerElement("query", >>> NamespaceURIs.XEP0030_SERVICE_DISCOVERY_INFO) >>> @@ -266,7 +339,11 @@ public class DiscoInfoIQHandlerTestCase >>> } >>> >>> private IQStanza createRequest(Entity to) { >>> - IQStanza stanza = (IQStanza) >>> XMPPCoreStanza.getWrapper(StanzaBuilder.createIQStanza(FROM, to, >>> IQStanzaType.GET, "id1") >>> + return createRequest(to, IQStanzaType.GET); >>> + } >>> + >>> + private IQStanza createRequest(Entity to, IQStanzaType type) { >>> + IQStanza stanza = (IQStanza) >>> XMPPCoreStanza.getWrapper(StanzaBuilder.createIQStanza(FROM, to, type, >>> "id1") >>> .startInnerElement("query", >>> NamespaceURIs.XEP0030_SERVICE_DISCOVERY_INFO) >>> .addAttribute("node", "n") >>> .build()); >>> >>> >>> > >
