From 1359c81cd796a4691f43365a939e14e94470ac9c Mon Sep 17 00:00:00 2001 From: Mario Milas Date: Thu, 11 Jan 2018 07:58:55 +0100 Subject: [PATCH 1/7] Substitute loopback with request matching strategy The current loopback implementation was limited in the sense that it wasn't open to mutation of request matching, causing a SipPhone client to potentially miss an incoming request which did not fit either the current Request URI check or the To check by equality. This commit introduces a concept of a request matching strategy processing, which is evaluated against the incoming request. This (collection of) strateg(y/ies) provides a determination mechanism of acceptance of request into the SipSession. Thus a user may provide extensions to the SipSession on which inbound requests may be accepted without the need to mutate any parameters. To enable this behavior, some parameters accessors have been moved from SipPhone to its parent, the SipSession. The request matching mechanism deprecates the loopback accessors. However, the loopback behavior has been retrofitted with the new matching strategy mechanism (tests included for proof). --- .../java/org/cafesip/sipunit/SipPhone.java | 22 -- .../java/org/cafesip/sipunit/SipSession.java | 283 +++++++++--------- .../matching/RequestMatchingStrategy.java | 109 +++++++ .../matching/RequestUriMatchingStrategy.java | 25 ++ .../sipunit/matching/ToMatchingStrategy.java | 31 ++ .../test/misc/TestRequestMatching.java | 283 ++++++++++++++++++ 6 files changed, 595 insertions(+), 158 deletions(-) create mode 100644 src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java create mode 100644 src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java create mode 100644 src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java create mode 100644 src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java diff --git a/src/main/java/org/cafesip/sipunit/SipPhone.java b/src/main/java/org/cafesip/sipunit/SipPhone.java index ed834a085f..ee53ff7786 100644 --- a/src/main/java/org/cafesip/sipunit/SipPhone.java +++ b/src/main/java/org/cafesip/sipunit/SipPhone.java @@ -1101,18 +1101,6 @@ protected CallIdHeader getNewCallIdHeader() return id; } - /** - * The method getContactInfo() returns the contact information currently in effect for this user - * agent. This may be the value associated with the last registration attempt or as defaulted to - * user@host if no registration has occurred. Or, if the setPublicAddress() has been called on - * this object, the returned value will reflect the most recent call to setPublicAddress(). - * - * @return The SipContact object currently in effect for this user agent - */ - public SipContact getContactInfo() { - return contactInfo; - } - /** * This method is the same as getContactInfo(). * @@ -1133,16 +1121,6 @@ protected void updateContactInfo(ContactHeader hdr) { } } - /** - * Gets the user Address for this SipPhone. This is the same address used in the - * "from" header field. - * - * @return Returns the javax.sip.address.Address for this SipPhone (UA). - */ - public Address getAddress() { - return myAddress; - } - /** * Gets the request sent at the last successful registration. * diff --git a/src/main/java/org/cafesip/sipunit/SipSession.java b/src/main/java/org/cafesip/sipunit/SipSession.java index d2b83e9fad..f12adb6ed5 100644 --- a/src/main/java/org/cafesip/sipunit/SipSession.java +++ b/src/main/java/org/cafesip/sipunit/SipSession.java @@ -18,47 +18,23 @@ package org.cafesip.sipunit; import gov.nist.javax.sip.header.ParameterNames; +import org.cafesip.sipunit.matching.RequestMatchingStrategy; +import org.cafesip.sipunit.matching.RequestUriMatchingStrategy; +import org.cafesip.sipunit.matching.ToMatchingStrategy; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.sip.ClientTransaction; -import javax.sip.Dialog; -import javax.sip.DialogTerminatedEvent; -import javax.sip.IOExceptionEvent; -import javax.sip.InvalidArgumentException; -import javax.sip.RequestEvent; -import javax.sip.ResponseEvent; -import javax.sip.ServerTransaction; -import javax.sip.SipListener; -import javax.sip.TimeoutEvent; -import javax.sip.TransactionAlreadyExistsException; -import javax.sip.TransactionState; -import javax.sip.TransactionTerminatedEvent; +import javax.sip.*; import javax.sip.address.Address; import javax.sip.address.AddressFactory; import javax.sip.address.SipURI; import javax.sip.address.URI; -import javax.sip.header.AuthorizationHeader; -import javax.sip.header.ContactHeader; -import javax.sip.header.ContentTypeHeader; -import javax.sip.header.ExpiresHeader; -import javax.sip.header.Header; -import javax.sip.header.ProxyAuthenticateHeader; -import javax.sip.header.ToHeader; -import javax.sip.header.ViaHeader; -import javax.sip.header.WWWAuthenticateHeader; +import javax.sip.header.*; import javax.sip.message.Message; import javax.sip.message.Request; import javax.sip.message.Response; import java.text.ParseException; -import java.util.ArrayList; -import java.util.EventObject; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.StringTokenizer; +import java.util.*; /** * Methods of this class provide the test program with low-level access to a SIP session. Instead of @@ -166,8 +142,6 @@ public class SipSession implements SipListener, SipActionObject { protected Object contactLock = new Object(); - protected String myDisplayName; - protected String proxyHost; protected String proxyProto; @@ -186,11 +160,21 @@ public class SipSession implements SipListener, SipActionObject { private BlockObject respBlock = new BlockObject(); + /** + * key = String request method, value = ArrayList of RequestListener + */ private Map> requestListeners = new HashMap<>(); - // key = String request method, value = ArrayList of RequestListener - - private boolean loopback; + /** + * Request matching strategies determine if an incoming {@link Request} will be accepted for this client after receiving + * the request through the stack. This class is initialized with {@link org.cafesip.sipunit.matching.RequestUriMatchingStrategy}}. + * The user of this library may add additional matching strategies in order to accept a certain request which has + * been formed in different ways depending on the SIP back end configuration. + * + * @see SipSession#isLoopback() + * @see SipSession#setLoopback(boolean) + */ + private final List requestMatchingStrategies = Collections.synchronizedList(new ArrayList()); private boolean supportRegisterRequests; @@ -251,6 +235,9 @@ protected SipSession(SipStack stack, String proxyHost, String proxyProto, int pr viaHeaders = new ArrayList<>(1); viaHeaders.add(via_header); + // Initialize the default request matching strategies + requestMatchingStrategies.add(new RequestUriMatchingStrategy(this)); + // finally, register with the sip stack parent.registerListener(this); } @@ -407,23 +394,13 @@ public boolean setPublicAddress(String host, int port) { */ public void processRequest(RequestEvent request) { Request req_msg = request.getRequest(); - ToHeader to = (ToHeader) req_msg.getHeader(ToHeader.NAME); SipContact my_contact_info = new SipContact(); synchronized (contactLock) { my_contact_info.setContactHeader((ContactHeader) (contactInfo.getContactHeader().clone())); } - // Is it for me? Check: Request-URI = my contact address (I may not be - // the original 'To' party, also there may be multiple devices for one - // "me" address of record) - // If no match, check 'To' = me - // (so that local messaging without proxy still works) - but ONLY IF - // setLoopback() has been called - LOG.trace("request received !"); - LOG.trace(" me ('To' check) = {}", me); - LOG.trace(" my local contact info ('Request URI' check) = {}", my_contact_info.getURI()); LOG.trace(" {}" , req_msg.toString()); if (req_msg.getMethod().equalsIgnoreCase(SipRequest.REGISTER)) { @@ -441,17 +418,11 @@ public void processRequest(RequestEvent request) { } } } - } else if (destMatch((SipURI) my_contact_info.getContactHeader().getAddress().getURI(), - (SipURI) req_msg.getRequestURI()) == false) { - if (!loopback) { - LOG.trace(" skipping 'To' check, we're not loopback (see setLoopback())"); - return; - } - - // check 'To' for a match - if (to.getAddress().getURI().toString().equals(me) == false) { - return; - } + } else if (requestMatches(req_msg)) { + LOG.trace(" incoming request match found, proceeding with processing"); + } else { + LOG.trace(" no match found for incoming request, skipping processing"); + return; } if (req_msg.getMethod().equalsIgnoreCase(Request.OPTIONS)) { @@ -497,6 +468,29 @@ public void processRequest(RequestEvent request) { } } + /** + * Is it for me? By default check: + * + * Request-URI = my contact address (I may not be the original 'To' party, also there may be multiple devices for one + * "me" address of record) + * + * If no match, check 'To' = me (so that local messaging without proxy still works) - but ONLY IF setLoopback() + * has been called + * + * @param request The request being tested for a match with the available strategies + * @return If the request matches (true) or not (false) + */ + private boolean requestMatches(Request request) { + boolean requestMatches = false; + synchronized (requestMatchingStrategies) { + for (RequestMatchingStrategy strategy : requestMatchingStrategies) { + // If we find a match, then the other strategies will not execute + requestMatches = requestMatches || strategy.isRequestMatching(request); + } + } + return requestMatches; + } + /** * FOR INTERNAL USE ONLY. Not to be used by a test program. */ @@ -574,84 +568,6 @@ public void processTimeout(TimeoutEvent timeout) { } } - protected static boolean destMatch(SipURI uri1, SipURI uri2) { - if (uri1.getScheme().equalsIgnoreCase(uri2.getScheme())) { - if (uri1.getUser() != null) { - if (uri2.getUser() == null) { - return false; - } - - if (uri1.getUser().equals(uri2.getUser()) == false) { - return false; - } - - if (uri1.getUserPassword() != null) { - if (uri2.getUserPassword() == null) { - return false; - } - - if (uri1.getUserPassword().equals(uri2.getUserPassword()) == false) { - return false; - } - } else if (uri2.getUserPassword() != null) { - return false; - } - } else if (uri2.getUser() != null) { - return false; - } - - if (uri1.getHost().equalsIgnoreCase(uri2.getHost()) == false) { - return false; - } - - if (uri1.toString().indexOf(uri1.getHost() + ':') != -1) { - if (uri2.toString().indexOf(uri2.getHost() + ':') == -1) { - return false; - } - - if (uri1.getPort() != uri2.getPort()) { - return false; - } - } else if (uri2.toString().indexOf(uri2.getHost() + ':') != -1) { - return false; - } - - // FOR A FULL URI-EQUAL CHECK, add the following: - /* - * if (uri1.getTransportParam() != null) { if (uri2.getTransportParam() == null) { return - * false; } - * - * if (uri1.getTransportParam().equals(uri2.getTransportParam()) == false) { return false; } } - * else if (uri2.getTransportParam() != null) { return false; } - * - * if (uri1.getTTLParam() != -1) { if (uri2.getTTLParam() == -1) { return false; } - * - * if (uri1.getTTLParam() != uri2.getTTLParam()) { return false; } } else if - * (uri2.getTTLParam() != -1) { return false; } - * - * if (uri1.getMethodParam() != null) { if (uri2.getMethodParam() == null) { return false; } - * - * if (uri1.getMethodParam().equals(uri2.getMethodParam()) == false) { return false; } } else - * if (uri2.getMethodParam() != null) { return false; } / next - incorporate the following - * remaining checks: - * - * URI uri-parameter components are compared as follows: - Any uri-parameter appearing in both - * URIs must match. - A user, ttl, or method uri-parameter appearing in only one URI never - * matches, even if it contains the default value. - A URI that includes an maddr parameter - * will not match a URI that contains no maddr parameter. - All other uri-parameters appearing - * in only one URI are ignored when comparing the URIs. - * - * o URI header components are never ignored. Any present header component MUST be present in - * both URIs and match for the URIs to match. The matching rules are defined for each header - * field in Section 20. - */ - - return true; - } - - return false; - } - /** * This sendUnidirectionalRequest() method sends out a request message with no response expected. * A Request object is constructed from the string parameter passed in. @@ -1958,9 +1874,20 @@ public String getStackAddress() { /** * @return Returns the loopback. See setLoopback(). + * @see SipSession#setLoopback(boolean) + * @deprecated Replaced by {@link SipSession#getRequestMatchingStrategies()} */ + @Deprecated public boolean isLoopback() { - return loopback; + synchronized (requestMatchingStrategies) { + for (RequestMatchingStrategy requestMatchingStrategy : requestMatchingStrategies) { + if (requestMatchingStrategy.getClass().equals(ToMatchingStrategy.class)) { + return true; + } + } + } + + return false; } /** @@ -1970,11 +1897,95 @@ public boolean isLoopback() { * the 'To' header matches even if the Request URI doesn't - so that local messaging tests without * proxy still work. This is for direct UA-UA testing convenience. This should not be the default, * however. + *

+ * If set to false, it will remove any existing {@link ToMatchingStrategy} in the request matching list. If this is the + * only strategy in the strategy list before removal, the strategy list will be set to default, i.e. be reset with the + * default {@link RequestUriMatchingStrategy}. * * @param loopback The loopback to set. + * @deprecated Replaced by {@link SipSession#setRequestMatchingStrategies(Collection)} */ + @Deprecated public void setLoopback(boolean loopback) { - this.loopback = loopback; + if (loopback) { + addToMatching(); + } else { + removeToMatching(); + + if (requestMatchingStrategies.isEmpty()) { + LOG.info("Request matching strategies empty, setting default strategy to " + RequestUriMatchingStrategy.class.getName()); + requestMatchingStrategies.add(new RequestUriMatchingStrategy(this)); + } + } + } + + @Deprecated + private void addToMatching() { + if (!isLoopback()) { + requestMatchingStrategies.add(new ToMatchingStrategy(this)); + } + } + + @Deprecated + private void removeToMatching() { + synchronized (requestMatchingStrategies) { + Iterator it = requestMatchingStrategies.iterator(); + + while (it.hasNext()) { + RequestMatchingStrategy current = it.next(); + + if (current.getClass().equals(ToMatchingStrategy.class)) { + it.remove(); + } + } + } + } + + /** + * @return A list of matching strategies for incoming requests which this instance uses to determine + * if a request will be accepted or not. Mutating this list WILL NOT affect currently configured strategies in this + * instance. + */ + public List getRequestMatchingStrategies() { + return new ArrayList<>(requestMatchingStrategies); + } + + /** + * Sets request matching strategies which will be used by this instance to determine if a request will be accepted. + * The new strategy collection must not be empty. + * + * @param requestMatchingStrategies New collection of request matching strategies + * @throws IllegalArgumentException If the new collection is empty + */ + public void setRequestMatchingStrategies(Collection requestMatchingStrategies) { + if (requestMatchingStrategies.size() == 0) { + throw new IllegalArgumentException("Cannot set an empty request matching strategy"); + } + + this.requestMatchingStrategies.clear(); + this.requestMatchingStrategies.addAll(requestMatchingStrategies); + } + + /** + * The method getContactInfo() returns the contact information currently in effect for this user + * agent. This may be the value associated with the last registration attempt or as defaulted to + * user@host if no registration has occurred. Or, if the setPublicAddress() has been called on + * this object, the returned value will reflect the most recent call to setPublicAddress(). + * + * @return The SipContact object currently in effect for this user agent + */ + public SipContact getContactInfo() { + return contactInfo; + } + + /** + * Gets the user Address for this user agent. This is the same address used in the + * "from" header field. + * + * @return Returns the javax.sip.address.Address for this user agent. + */ + public Address getAddress() { + return myAddress; } public void processIOException(IOExceptionEvent arg0) { diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java new file mode 100644 index 0000000000..887d16dd5b --- /dev/null +++ b/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java @@ -0,0 +1,109 @@ +package org.cafesip.sipunit.matching; + +import org.cafesip.sipunit.SipSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.sip.RequestEvent; +import javax.sip.address.SipURI; +import javax.sip.message.Request; + +/** + * The request matching strategy is used within every {@link SipSession} which is bound to process an incoming request. + * In order to provide multiple ways of accepting a request from various sources, we introduce this type to provide an + * unified way to determine if a request fits a criterion for it to be accepted and stored within a session. + *

+ * Created by TELES AG on 09/01/2018. + * + * @see SipSession#processRequest(RequestEvent) + */ +public abstract class RequestMatchingStrategy { + + protected static final Logger LOG = LoggerFactory.getLogger(RequestMatchingStrategy.class); + + protected final SipSession managedSession; + + public RequestMatchingStrategy(SipSession managedSession){ + this.managedSession = managedSession; + } + + public abstract boolean isRequestMatching(Request request); + + protected static boolean destMatch(SipURI uri1, SipURI uri2) { + if (uri1.getScheme().equalsIgnoreCase(uri2.getScheme())) { + if (uri1.getUser() != null) { + if (uri2.getUser() == null) { + return false; + } + + if (uri1.getUser().equals(uri2.getUser()) == false) { + return false; + } + + if (uri1.getUserPassword() != null) { + if (uri2.getUserPassword() == null) { + return false; + } + + if (uri1.getUserPassword().equals(uri2.getUserPassword()) == false) { + return false; + } + } else if (uri2.getUserPassword() != null) { + return false; + } + } else if (uri2.getUser() != null) { + return false; + } + + if (uri1.getHost().equalsIgnoreCase(uri2.getHost()) == false) { + return false; + } + + if (uri1.toString().indexOf(uri1.getHost() + ':') != -1) { + if (uri2.toString().indexOf(uri2.getHost() + ':') == -1) { + return false; + } + + if (uri1.getPort() != uri2.getPort()) { + return false; + } + } else if (uri2.toString().indexOf(uri2.getHost() + ':') != -1) { + return false; + } + + // FOR A FULL URI-EQUAL CHECK, add the following: + /* + * if (uri1.getTransportParam() != null) { if (uri2.getTransportParam() == null) { return + * false; } + * + * if (uri1.getTransportParam().equals(uri2.getTransportParam()) == false) { return false; } } + * else if (uri2.getTransportParam() != null) { return false; } + * + * if (uri1.getTTLParam() != -1) { if (uri2.getTTLParam() == -1) { return false; } + * + * if (uri1.getTTLParam() != uri2.getTTLParam()) { return false; } } else if + * (uri2.getTTLParam() != -1) { return false; } + * + * if (uri1.getMethodParam() != null) { if (uri2.getMethodParam() == null) { return false; } + * + * if (uri1.getMethodParam().equals(uri2.getMethodParam()) == false) { return false; } } else + * if (uri2.getMethodParam() != null) { return false; } / next - incorporate the following + * remaining checks: + * + * URI uri-parameter components are compared as follows: - Any uri-parameter appearing in both + * URIs must match. - A user, ttl, or method uri-parameter appearing in only one URI never + * matches, even if it contains the default value. - A URI that includes an maddr parameter + * will not match a URI that contains no maddr parameter. - All other uri-parameters appearing + * in only one URI are ignored when comparing the URIs. + * + * o URI header components are never ignored. Any present header component MUST be present in + * both URIs and match for the URIs to match. The matching rules are defined for each header + * field in Section 20. + */ + + return true; + } + + return false; + } +} diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java new file mode 100644 index 0000000000..6f10c73722 --- /dev/null +++ b/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java @@ -0,0 +1,25 @@ +package org.cafesip.sipunit.matching; + +import org.cafesip.sipunit.SipSession; + +import javax.sip.address.SipURI; +import javax.sip.message.Request; + +/** + * Determines if the request is viable for processing base on the {@link Request#getRequestURI()} of the received {@link Request} + *

+ * Created by TELES AG on 09/01/2018. + */ +public final class RequestUriMatchingStrategy extends RequestMatchingStrategy { + public RequestUriMatchingStrategy(SipSession managedSession) { + super(managedSession); + } + + @Override + public boolean isRequestMatching(Request request) { + LOG.trace("my local contact info ('Request URI' check) = {}", managedSession.getContactInfo().getURI()); + + return destMatch((SipURI) managedSession.getContactInfo().getContactHeader().getAddress().getURI(), + (SipURI) request.getRequestURI()); + } +} diff --git a/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java new file mode 100644 index 0000000000..fac2914b21 --- /dev/null +++ b/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java @@ -0,0 +1,31 @@ +package org.cafesip.sipunit.matching; + +import org.cafesip.sipunit.SipSession; + +import javax.sip.RequestEvent; +import javax.sip.header.ToHeader; +import javax.sip.message.Request; + +/** + * Determines if the request is viable for processing base on the {@link ToHeader} of the received {@link Request} in + * {@link SipSession#processRequest(RequestEvent)} + *

+ * Created by TELES AG on 09/01/2018. + */ +public final class ToMatchingStrategy extends RequestMatchingStrategy { + public ToMatchingStrategy(SipSession managedSession) { + super(managedSession); + } + + @Override + public boolean isRequestMatching(Request request) { + ToHeader to = (ToHeader) request.getHeader(ToHeader.NAME); + + String me = managedSession.getAddress().getURI().toString(); + String expected = to.getAddress().getURI().toString(); + + LOG.trace("me ('To' check) = {}", me); + + return expected.equals(me); + } +} diff --git a/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java new file mode 100644 index 0000000000..918cab2ac7 --- /dev/null +++ b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java @@ -0,0 +1,283 @@ +package org.cafesip.sipunit.test.misc; + +import org.cafesip.sipunit.SipPhone; +import org.cafesip.sipunit.SipSession; +import org.cafesip.sipunit.SipStack; +import org.cafesip.sipunit.matching.RequestMatchingStrategy; +import org.cafesip.sipunit.matching.RequestUriMatchingStrategy; +import org.cafesip.sipunit.matching.ToMatchingStrategy; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.sip.RequestEvent; +import javax.sip.message.Request; +import javax.sip.message.Response; +import java.text.ParseException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Properties; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.cafesip.sipunit.SipAssert.assertLastOperationSuccess; +import static org.cafesip.sipunit.SipAssert.awaitStackDispose; +import static org.junit.Assert.*; + +/** + * Test behavior of matching depending on the configuration of {@link RequestMatchingStrategy} in {@link SipSession}. + * + * Provides backwards compatibility testing with the removed loopback attribute which has been replaced by the tested + * strategy approach. + * + * Created by TELES AG on 10/01/2018. + */ +@SuppressWarnings("deprecation") +public class TestRequestMatching { + + private static final Logger LOG = LoggerFactory.getLogger(TestRequestMatching.class); + + private static final String LOCALHOST = "127.0.0.1"; + + private SipStack sipStackA; + private SipPhone ua; + private final int uaPort = 5081; + private final String uaProtocol = "UDP"; + private final String uaContact = "sip:clientA@127.0.0.1:" + uaPort; + + private SipStack sipStackB; + private SipPhone ub; + private final int ubPort = 5082; + private final String ubProtocol = "UDP"; + private final String ubContact = "sip:clientB@127.0.0.1:" + ubPort; + + private SipStack sipStackProxy; + private SipPhone proxy; + private final int proxyPort = 5080; + private final String proxyProtocol = "UDP"; + private final String proxyContact = "sip:proxy@127.0.0.1:5080"; + private final Properties uaProperties = new Properties(); + private final Properties proxyProperties = new Properties(); + private final ExecutorService executorService = Executors.newCachedThreadPool(); + + private ProxyMock proxyMock; + + @Before + public void setup() throws Exception { + uaProperties.setProperty("javax.sip.IP_ADDRESS", LOCALHOST); + uaProperties.setProperty("javax.sip.STACK_NAME", "testAgentA"); + uaProperties.setProperty("gov.nist.javax.sip.TRACE_LEVEL", "16"); + uaProperties.setProperty("gov.nist.javax.sip.DEBUG_LOG", "testAgentA_debug.txt"); + uaProperties.setProperty("gov.nist.javax.sip.SERVER_LOG", "testAgentA_log.txt"); + uaProperties.setProperty("gov.nist.javax.sip.READ_TIMEOUT", "1000"); + uaProperties.setProperty("gov.nist.javax.sip.CACHE_SERVER_CONNECTIONS", "false"); + uaProperties.setProperty("gov.nist.javax.sip.PASS_INVITE_NON_2XX_ACK_TO_LISTENER", "true"); + + sipStackA = new SipStack(uaProtocol, uaPort, uaProperties); + ua = sipStackA.createSipPhone(LOCALHOST, proxyProtocol, proxyPort, uaContact); + + uaProperties.setProperty("javax.sip.IP_ADDRESS", LOCALHOST); + uaProperties.setProperty("javax.sip.STACK_NAME", "testAgentB"); + uaProperties.setProperty("gov.nist.javax.sip.TRACE_LEVEL", "16"); + uaProperties.setProperty("gov.nist.javax.sip.DEBUG_LOG", "testAgentB_debug.txt"); + uaProperties.setProperty("gov.nist.javax.sip.SERVER_LOG", "testAgentB_log.txt"); + uaProperties.setProperty("gov.nist.javax.sip.READ_TIMEOUT", "1000"); + uaProperties.setProperty("gov.nist.javax.sip.CACHE_SERVER_CONNECTIONS", "false"); + uaProperties.setProperty("gov.nist.javax.sip.PASS_INVITE_NON_2XX_ACK_TO_LISTENER", "true"); + + sipStackB = new SipStack(ubProtocol, ubPort, uaProperties); + ub = sipStackB.createSipPhone(LOCALHOST, proxyProtocol, proxyPort, ubContact); + + proxyProperties.setProperty("javax.sip.STACK_NAME", "testProxy"); + proxyProperties.setProperty("gov.nist.javax.sip.TRACE_LEVEL", "16"); + proxyProperties.setProperty("gov.nist.javax.sip.DEBUG_LOG", "testProxy_debug.txt"); + proxyProperties.setProperty("gov.nist.javax.sip.SERVER_LOG", "testProxy_log.txt"); + proxyProperties.setProperty("gov.nist.javax.sip.READ_TIMEOUT", "1000"); + proxyProperties.setProperty("gov.nist.javax.sip.CACHE_SERVER_CONNECTIONS", "false"); + proxyProperties.setProperty("gov.nist.javax.sip.PASS_INVITE_NON_2XX_ACK_TO_LISTENER", "true"); + proxyProperties.setProperty("javax.sip.IP_ADDRESS", LOCALHOST); + + sipStackProxy = new SipStack(proxyProtocol, proxyPort, proxyProperties); + proxy = sipStackProxy.createSipPhone(proxyContact); + + proxyMock = new ProxyMock(); + executorService.submit(proxyMock); + } + + /** + * Release the sipStack and a user agent for the test. + */ + @After + public void tearDown() { + // Shutdown current mock + proxyMock.setRunning(false); + executorService.shutdown(); + + ua.unregister(uaContact, 1000); + ua.dispose(); + awaitStackDispose(sipStackA); + + ub.unregister(ubContact, 1000); + ub.dispose(); + awaitStackDispose(sipStackB); + + proxy.dispose(); + awaitStackDispose(sipStackProxy); + } + + @Test + public void testStrategiesMutation() { + // Attempt mutating the list obtained through the getter + // Should have 2 default matching strategies + List requestMatchingStrategies = ua.getRequestMatchingStrategies(); + assertEquals(1, requestMatchingStrategies.size()); + + requestMatchingStrategies.clear(); + assertEquals(1, ua.getRequestMatchingStrategies().size()); + + // Create a dummy strategy + RequestMatchingStrategy newStrategy = new RequestMatchingStrategy(ua) { + @Override + public boolean isRequestMatching(Request request) { + return true; + } + }; + + // Mutate strategies through the setter + ua.setRequestMatchingStrategies(Arrays.asList(newStrategy)); + List newMatchingStrategies = ua.getRequestMatchingStrategies(); + + assertEquals(1, newMatchingStrategies.size()); + assertEquals(newStrategy, newMatchingStrategies.get(0)); + } + + /** + * If the strategy list has only {@link ToMatchingStrategy} and this strategy is removed through + * {@link SipSession#setLoopback(boolean)}, the strategy list should be reset with the default + * {@link RequestUriMatchingStrategy}. + */ + @Test + public void testRemoveOnlyToStrategy() { + ua.setRequestMatchingStrategies(Arrays.asList(new ToMatchingStrategy(ua))); + assertTrue(ua.isLoopback()); + + ua.setLoopback(false); + assertEquals(1, ua.getRequestMatchingStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, ua.getRequestMatchingStrategies().get(0).getClass()); + } + + /** + * The session object should not allow for no request strategies to be present for request processing + */ + @Test(expected = IllegalArgumentException.class) + public void testStrategiesMutationEmpty() { + ua.setRequestMatchingStrategies(Collections.emptyList()); + } + + /** + * Backwards compatibility check for isLoopback after the addition of request matching strategies + *

+ * Is loopback should be true when both the {@link RequestUriMatchingStrategy} and {@link ToMatchingStrategy} + * are set in the strategies of a {@link SipSession} object, and false if only the {@link RequestUriMatchingStrategy} + * is set in the strategies. (does not check existence of other strategies) + */ + @Test + public void testIsLoopbackSetting() { + // Default is false + assertFalse(ua.isLoopback()); + assertEquals(1, ua.getRequestMatchingStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, ua.getRequestMatchingStrategies().get(0).getClass()); + + ua.setLoopback(true); + assertTrue(ua.isLoopback()); + assertEquals(2, ua.getRequestMatchingStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, ua.getRequestMatchingStrategies().get(0).getClass()); + assertEquals(ToMatchingStrategy.class, ua.getRequestMatchingStrategies().get(1).getClass()); + + // Add a new strategy and set loopback to false to check that this strategy will not be deleted + List strategies = ua.getRequestMatchingStrategies(); + RequestMatchingStrategy additionalStrategy = new RequestMatchingStrategy(ua) { + @Override + public boolean isRequestMatching(Request request) { + return false; + } + }; + strategies.add(additionalStrategy); + + ua.setRequestMatchingStrategies(strategies); + + ua.setLoopback(false); + assertEquals(2, ua.getRequestMatchingStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, ua.getRequestMatchingStrategies().get(0).getClass()); + assertEquals(additionalStrategy, ua.getRequestMatchingStrategies().get(1)); + } + + @Test + public void testToMatching() { + testMatchingStrategy(new ToMatchingStrategy(ub)); + } + + @Test + public void testRequestUriMatching() { + testMatchingStrategy(new RequestUriMatchingStrategy(ub)); + } + + private void testMatchingStrategy(RequestMatchingStrategy requestMatchingStrategy) { + ub.setRequestMatchingStrategies(Arrays.asList(requestMatchingStrategy)); + + assertTrue(ua.register("userA", "test1", uaContact, 4890, 1000000)); + assertLastOperationSuccess("user a registration - " + ua.format(), ua); + + assertTrue(ub.register("userB", "test2", ubContact, 4890, 1000000)); + assertLastOperationSuccess("user b registration - " + ub.format(), ub); + + assertTrue(ub.listenRequestMessage()); + + ua.makeCall(ubContact, ub.getPublicAddress() + "/" + ubProtocol); + assertLastOperationSuccess("user a sendRequest - " + ua.format(), ua); + + ub.waitRequest(TimeUnit.SECONDS.toMillis(10)); + assertLastOperationSuccess("user b receive request - " + ub.format(), ub); + } + + private class ProxyMock implements Runnable { + + private final AtomicBoolean isRunning = new AtomicBoolean(); + + public ProxyMock() { + proxy.setSupportRegisterRequests(true); + assertTrue(proxy.listenRequestMessage()); + + setRunning(false); + } + + @Override + public void run() { + setRunning(true); + + while (isRunning.get()) { + RequestEvent requestEvent = proxy.waitRequest(1000); + if (requestEvent == null) { + continue; + } + + try { + Response response = proxy.getParent().getMessageFactory().createResponse(200, requestEvent.getRequest()); + proxy.sendReply(requestEvent, response); + } catch (ParseException e) { + e.printStackTrace(); + fail("Parsing of input request failed"); + } + } + } + + public void setRunning(boolean runStatus) { + isRunning.set(runStatus); + } + } +} From 140b7ea7f3837f60b9b6fe598a181128a576b78f Mon Sep 17 00:00:00 2001 From: Mario Milas Date: Thu, 11 Jan 2018 09:45:42 +0100 Subject: [PATCH 2/7] Refactor request matching logic into a separate object In order to follow extensibility guidelines defined in the Gang Of Four book on Design Patterns, we refactor the request matching mechanism to increase the cohesion of the SipSession object (which just queries the matching and handles that the matching is up to its business logic). The business logic of the strategy handling themselves is handled now by the request matcher, which will determine if a strategy will be added, removed or contained. To reduce coupling of the matching strategy to the SipSession, we move the sip session to be an argument of the strategy determination. This way we can represent and initialize each handler as a functional interface (for Java 8), reducing code footprint while still delegating the responsibility of request attribution to a session to the strategy itself. --- .../java/org/cafesip/sipunit/SipSession.java | 101 ++-------- .../sipunit/matching/RequestMatcher.java | 177 ++++++++++++++++++ .../matching/RequestMatchingStrategy.java | 20 +- .../matching/RequestUriMatchingStrategy.java | 10 +- .../sipunit/matching/ToMatchingStrategy.java | 8 +- .../test/misc/TestRequestMatching.java | 105 ++++++----- 6 files changed, 271 insertions(+), 150 deletions(-) create mode 100644 src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java diff --git a/src/main/java/org/cafesip/sipunit/SipSession.java b/src/main/java/org/cafesip/sipunit/SipSession.java index f12adb6ed5..848d259cee 100644 --- a/src/main/java/org/cafesip/sipunit/SipSession.java +++ b/src/main/java/org/cafesip/sipunit/SipSession.java @@ -18,8 +18,8 @@ package org.cafesip.sipunit; import gov.nist.javax.sip.header.ParameterNames; +import org.cafesip.sipunit.matching.RequestMatcher; import org.cafesip.sipunit.matching.RequestMatchingStrategy; -import org.cafesip.sipunit.matching.RequestUriMatchingStrategy; import org.cafesip.sipunit.matching.ToMatchingStrategy; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -165,16 +165,7 @@ public class SipSession implements SipListener, SipActionObject { */ private Map> requestListeners = new HashMap<>(); - /** - * Request matching strategies determine if an incoming {@link Request} will be accepted for this client after receiving - * the request through the stack. This class is initialized with {@link org.cafesip.sipunit.matching.RequestUriMatchingStrategy}}. - * The user of this library may add additional matching strategies in order to accept a certain request which has - * been formed in different ways depending on the SIP back end configuration. - * - * @see SipSession#isLoopback() - * @see SipSession#setLoopback(boolean) - */ - private final List requestMatchingStrategies = Collections.synchronizedList(new ArrayList()); + private final RequestMatcher requestMatcher = new RequestMatcher(); private boolean supportRegisterRequests; @@ -235,9 +226,6 @@ protected SipSession(SipStack stack, String proxyHost, String proxyProto, int pr viaHeaders = new ArrayList<>(1); viaHeaders.add(via_header); - // Initialize the default request matching strategies - requestMatchingStrategies.add(new RequestUriMatchingStrategy(this)); - // finally, register with the sip stack parent.registerListener(this); } @@ -481,14 +469,7 @@ public void processRequest(RequestEvent request) { * @return If the request matches (true) or not (false) */ private boolean requestMatches(Request request) { - boolean requestMatches = false; - synchronized (requestMatchingStrategies) { - for (RequestMatchingStrategy strategy : requestMatchingStrategies) { - // If we find a match, then the other strategies will not execute - requestMatches = requestMatches || strategy.isRequestMatching(request); - } - } - return requestMatches; + return requestMatcher.requestMatches(request, this); } /** @@ -1875,19 +1856,11 @@ public String getStackAddress() { /** * @return Returns the loopback. See setLoopback(). * @see SipSession#setLoopback(boolean) - * @deprecated Replaced by {@link SipSession#getRequestMatchingStrategies()} + * @deprecated Replaced by {@link RequestMatcher} behavior */ @Deprecated public boolean isLoopback() { - synchronized (requestMatchingStrategies) { - for (RequestMatchingStrategy requestMatchingStrategy : requestMatchingStrategies) { - if (requestMatchingStrategy.getClass().equals(ToMatchingStrategy.class)) { - return true; - } - } - } - - return false; + return requestMatcher.contains(ToMatchingStrategy.class); } /** @@ -1898,72 +1871,28 @@ public boolean isLoopback() { * proxy still work. This is for direct UA-UA testing convenience. This should not be the default, * however. *

- * If set to false, it will remove any existing {@link ToMatchingStrategy} in the request matching list. If this is the - * only strategy in the strategy list before removal, the strategy list will be set to default, i.e. be reset with the - * default {@link RequestUriMatchingStrategy}. + * If set to false, it will remove any existing {@link ToMatchingStrategy} in the request matching list. * * @param loopback The loopback to set. - * @deprecated Replaced by {@link SipSession#setRequestMatchingStrategies(Collection)} + * @deprecated Replaced by {@link RequestMatcher} behavior + * + * @see RequestMatcher#add(RequestMatchingStrategy) + * @see RequestMatcher#remove(Class) */ @Deprecated public void setLoopback(boolean loopback) { if (loopback) { - addToMatching(); + requestMatcher.add(new ToMatchingStrategy()); } else { - removeToMatching(); - - if (requestMatchingStrategies.isEmpty()) { - LOG.info("Request matching strategies empty, setting default strategy to " + RequestUriMatchingStrategy.class.getName()); - requestMatchingStrategies.add(new RequestUriMatchingStrategy(this)); - } - } - } - - @Deprecated - private void addToMatching() { - if (!isLoopback()) { - requestMatchingStrategies.add(new ToMatchingStrategy(this)); + requestMatcher.remove(ToMatchingStrategy.class); } } - @Deprecated - private void removeToMatching() { - synchronized (requestMatchingStrategies) { - Iterator it = requestMatchingStrategies.iterator(); - - while (it.hasNext()) { - RequestMatchingStrategy current = it.next(); - - if (current.getClass().equals(ToMatchingStrategy.class)) { - it.remove(); - } - } - } - } - - /** - * @return A list of matching strategies for incoming requests which this instance uses to determine - * if a request will be accepted or not. Mutating this list WILL NOT affect currently configured strategies in this - * instance. - */ - public List getRequestMatchingStrategies() { - return new ArrayList<>(requestMatchingStrategies); - } - /** - * Sets request matching strategies which will be used by this instance to determine if a request will be accepted. - * The new strategy collection must not be empty. - * - * @param requestMatchingStrategies New collection of request matching strategies - * @throws IllegalArgumentException If the new collection is empty + * @return The request matcher which governs the acceptance of inbound requests in this session */ - public void setRequestMatchingStrategies(Collection requestMatchingStrategies) { - if (requestMatchingStrategies.size() == 0) { - throw new IllegalArgumentException("Cannot set an empty request matching strategy"); - } - - this.requestMatchingStrategies.clear(); - this.requestMatchingStrategies.addAll(requestMatchingStrategies); + public final RequestMatcher getRequestMatcher() { + return requestMatcher; } /** diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java b/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java new file mode 100644 index 0000000000..342b6e935e --- /dev/null +++ b/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java @@ -0,0 +1,177 @@ +package org.cafesip.sipunit.matching; + +import org.cafesip.sipunit.SipSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.sip.message.Request; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; + +/** + * This class takes care that the inbound requests received by a governing {@link SipSession} are tested against a + * configurable list of strategies. The {@link RequestMatchingStrategy} instances which this class manages are + * in charge of testing an inbound SIP {@link Request} and providing feedback if the receiver should accept and process + * the received request. + *

+ * The request matcher additionally manages that the request matching strategies do not produce side effects when being + * mutated, and provides concurrent access to the strategy handling mechanism. The default strategy which is provided by + * this class on instantiation is {@link RequestUriMatchingStrategy}. + *

+ * Created by TELES AG on 11/01/2018. + * + * @see RequestMatchingStrategy + */ +public class RequestMatcher { + + private static final Logger LOG = LoggerFactory.getLogger(RequestMatcher.class); + + /** + * Request matching strategies determine if an incoming {@link Request} will be accepted for this client after receiving + * the request through the stack. This class is initialized with {@link org.cafesip.sipunit.matching.RequestUriMatchingStrategy}}. + * The user of this library may add additional matching strategies in order to accept a certain request which has + * been formed in different ways depending on the SIP backend configuration. + */ + private final List requestMatchingStrategies = Collections.synchronizedList(new ArrayList()); + + /** + * Initialize this matcher with the default {@link RequestUriMatchingStrategy} + */ + public RequestMatcher() { + setDefaultStrategy(); + } + + /** + * Initialize this matcher with the provided default strategies + * + * @param initialStrategies Initial strategies which should be added to the matcher + * @see RequestMatcher#add(RequestMatchingStrategy) + */ + public RequestMatcher(List initialStrategies) { + for (RequestMatchingStrategy requestMatchingStrategy : initialStrategies) { + add(requestMatchingStrategy); + } + + setDefaultStrategy(); + } + + /** + * Run all configured strategies in this matcher and determine if the request matches any configured criterion. + * + * @param request The request being tested for a match with the available strategies + * @param sipSession The governing object which received the request + * @return If the request matches (true) or not (false) + */ + public boolean requestMatches(final Request request, final SipSession sipSession) { + boolean requestMatches = false; + synchronized (requestMatchingStrategies) { + for (RequestMatchingStrategy strategy : requestMatchingStrategies) { + // If we find a match, then the other strategies will not execute + requestMatches = requestMatches || strategy.isRequestMatching(request, sipSession); + } + } + return requestMatches; + } + + /** + * Add the strategy to be used in request processing in this matches + * + * @param requestMatchingStrategy The added request matching strategy + * @return If the request matching strategy was successfully added to the list of strategies used by this matcher + */ + public boolean add(RequestMatchingStrategy requestMatchingStrategy) { + // TODO: check if strategy is forced to have only one instance + return requestMatchingStrategies.add(requestMatchingStrategy); + } + + /** + * Removes the instance of the specified request matching strategy + * + * @param requestMatchingStrategy The strategy that needs to be removed by its reference in the request matcher + * @return True if the specified instance of the searched strategy has been removed, false otherwise + * @see List#remove(Object) + */ + public boolean remove(RequestMatchingStrategy requestMatchingStrategy) { + return requestMatchingStrategies.remove(requestMatchingStrategy); + } + + /** + * Removes any existing {@link RequestMatchingStrategy} defined by the searched class in the request matching list. + * If this is the only strategy in the strategy list before removal, the strategy list will be set to default, + * i.e. be reset with the default {@link RequestUriMatchingStrategy}. + * + * @param searchedClass The class that needs to be removed by its type in the request matcher + * @return True if any instance of the searched strategy has been removed, false otherwise + */ + public boolean remove(Class searchedClass) { + boolean isRemoved = false; + + synchronized (requestMatchingStrategies) { + Iterator it = requestMatchingStrategies.iterator(); + + while (it.hasNext()) { + RequestMatchingStrategy current = it.next(); + + if (current.getClass().equals(searchedClass)) { + isRemoved = true; + it.remove(); + } + } + + setDefaultStrategy(); + } + + return isRemoved; + } + + /** + * Check if any existing {@link RequestMatchingStrategy} defined by the searched class is present in the matching + * strategy list. + * + * @param searchedClass The class whose direct instances will be searched for in the request matcher + * @return True if any instance of the searched strategy has been found, false otherwise + */ + public boolean contains(Class searchedClass) { + synchronized (requestMatchingStrategies) { + for (RequestMatchingStrategy requestMatchingStrategy : requestMatchingStrategies) { + if (requestMatchingStrategy.getClass().equals(searchedClass)) { + return true; + } + } + } + + return false; + } + + /** + * Check if the {@link RequestMatchingStrategy} is added to the matcher (by reference). + * + * @return True if the specified instance of the searched strategy has been found, false otherwise + * @see List#contains(Object) + */ + public boolean contains(RequestMatchingStrategy requestMatchingStrategy) { + return requestMatchingStrategies.contains(requestMatchingStrategy); + } + + /** + * @return A list of matching strategies for incoming requests which this instance uses to determine + * if a request will be accepted or not. This list is unmodifiable and will be updated with each change. + */ + public List getRequestMatchingStrategies() { + return Collections.unmodifiableList(requestMatchingStrategies); + } + + /** + * Sets the default {@link RequestUriMatchingStrategy} if the available strategy list is empty. + */ + private void setDefaultStrategy() { + synchronized (requestMatchingStrategies) { + if (requestMatchingStrategies.isEmpty()) { + LOG.info("Request matching strategies empty, setting default strategy to " + RequestUriMatchingStrategy.class.getName()); + add(new RequestUriMatchingStrategy()); + } + } + } +} diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java index 887d16dd5b..6c2610341a 100644 --- a/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java +++ b/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java @@ -21,15 +21,17 @@ public abstract class RequestMatchingStrategy { protected static final Logger LOG = LoggerFactory.getLogger(RequestMatchingStrategy.class); - protected final SipSession managedSession; - - public RequestMatchingStrategy(SipSession managedSession){ - this.managedSession = managedSession; - } - - public abstract boolean isRequestMatching(Request request); - - protected static boolean destMatch(SipURI uri1, SipURI uri2) { + /** + * Determines if the inbound request matches the criterion defined by this matching strategy. + * + * @param request The inbound request + * @param sipSession The governing SipSession which received the request through its {@link org.cafesip.sipunit.SipStack} + * + * @return True if the request matches the defined criterion, false otherwise + */ + public abstract boolean isRequestMatching(final Request request, final SipSession sipSession); + + protected static boolean isSipUriEquals(SipURI uri1, SipURI uri2) { if (uri1.getScheme().equalsIgnoreCase(uri2.getScheme())) { if (uri1.getUser() != null) { if (uri2.getUser() == null) { diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java index 6f10c73722..110748763d 100644 --- a/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java +++ b/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java @@ -11,15 +11,13 @@ * Created by TELES AG on 09/01/2018. */ public final class RequestUriMatchingStrategy extends RequestMatchingStrategy { - public RequestUriMatchingStrategy(SipSession managedSession) { - super(managedSession); - } @Override - public boolean isRequestMatching(Request request) { - LOG.trace("my local contact info ('Request URI' check) = {}", managedSession.getContactInfo().getURI()); + public boolean isRequestMatching(Request request, SipSession sipSession) { + LOG.trace("my local contact info ('Request URI' check) = {}", sipSession.getContactInfo().getURI()); - return destMatch((SipURI) managedSession.getContactInfo().getContactHeader().getAddress().getURI(), + return isSipUriEquals((SipURI) sipSession.getContactInfo().getContactHeader().getAddress().getURI(), (SipURI) request.getRequestURI()); } + } diff --git a/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java index fac2914b21..0f7d7aae87 100644 --- a/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java +++ b/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java @@ -13,19 +13,17 @@ * Created by TELES AG on 09/01/2018. */ public final class ToMatchingStrategy extends RequestMatchingStrategy { - public ToMatchingStrategy(SipSession managedSession) { - super(managedSession); - } @Override - public boolean isRequestMatching(Request request) { + public boolean isRequestMatching(Request request, SipSession sipSession) { ToHeader to = (ToHeader) request.getHeader(ToHeader.NAME); - String me = managedSession.getAddress().getURI().toString(); + String me = sipSession.getAddress().getURI().toString(); String expected = to.getAddress().getURI().toString(); LOG.trace("me ('To' check) = {}", me); return expected.equals(me); } + } diff --git a/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java index 918cab2ac7..a937d3e032 100644 --- a/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java +++ b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java @@ -3,6 +3,7 @@ import org.cafesip.sipunit.SipPhone; import org.cafesip.sipunit.SipSession; import org.cafesip.sipunit.SipStack; +import org.cafesip.sipunit.matching.RequestMatcher; import org.cafesip.sipunit.matching.RequestMatchingStrategy; import org.cafesip.sipunit.matching.RequestUriMatchingStrategy; import org.cafesip.sipunit.matching.ToMatchingStrategy; @@ -16,8 +17,6 @@ import javax.sip.message.Request; import javax.sip.message.Response; import java.text.ParseException; -import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.Properties; import java.util.concurrent.ExecutorService; @@ -31,10 +30,10 @@ /** * Test behavior of matching depending on the configuration of {@link RequestMatchingStrategy} in {@link SipSession}. - * + *

* Provides backwards compatibility testing with the removed loopback attribute which has been replaced by the tested * strategy approach. - * + *

* Created by TELES AG on 10/01/2018. */ @SuppressWarnings("deprecation") @@ -130,30 +129,45 @@ public void tearDown() { awaitStackDispose(sipStackProxy); } + /** + * The request matcher should not allow any mutation because of the immutable list + */ + @Test(expected = UnsupportedOperationException.class) + public void testGetStrategiesMutation() { + ua.getRequestMatcher().getRequestMatchingStrategies().clear(); + } + @Test public void testStrategiesMutation() { + final RequestMatcher requestMatcher = new RequestMatcher(); + // Attempt mutating the list obtained through the getter // Should have 2 default matching strategies - List requestMatchingStrategies = ua.getRequestMatchingStrategies(); + List requestMatchingStrategies = requestMatcher.getRequestMatchingStrategies(); assertEquals(1, requestMatchingStrategies.size()); - requestMatchingStrategies.clear(); - assertEquals(1, ua.getRequestMatchingStrategies().size()); - // Create a dummy strategy - RequestMatchingStrategy newStrategy = new RequestMatchingStrategy(ua) { + RequestMatchingStrategy newStrategy = new RequestMatchingStrategy() { @Override - public boolean isRequestMatching(Request request) { + public boolean isRequestMatching(Request request, SipSession sipSession) { return true; } }; - // Mutate strategies through the setter - ua.setRequestMatchingStrategies(Arrays.asList(newStrategy)); - List newMatchingStrategies = ua.getRequestMatchingStrategies(); + // Mutate strategies through the accessor + requestMatcher.add(newStrategy); + // Returned list should be updated + assertEquals(2, requestMatchingStrategies.size()); + + List newMatchingStrategies = requestMatcher.getRequestMatchingStrategies(); + assertEquals(2, newMatchingStrategies.size()); + assertEquals(newStrategy, newMatchingStrategies.get(1)); + + assertTrue(requestMatcher.contains(newStrategy)); + assertTrue(requestMatcher.contains(newStrategy.getClass())); - assertEquals(1, newMatchingStrategies.size()); - assertEquals(newStrategy, newMatchingStrategies.get(0)); + // Default strategy + assertTrue(requestMatcher.contains(RequestUriMatchingStrategy.class)); } /** @@ -163,20 +177,17 @@ public boolean isRequestMatching(Request request) { */ @Test public void testRemoveOnlyToStrategy() { - ua.setRequestMatchingStrategies(Arrays.asList(new ToMatchingStrategy(ua))); + final RequestMatcher requestMatcher = ua.getRequestMatcher(); + requestMatcher.add(new ToMatchingStrategy()); assertTrue(ua.isLoopback()); - ua.setLoopback(false); - assertEquals(1, ua.getRequestMatchingStrategies().size()); - assertEquals(RequestUriMatchingStrategy.class, ua.getRequestMatchingStrategies().get(0).getClass()); - } + requestMatcher.remove(RequestUriMatchingStrategy.class); + assertEquals(1, requestMatcher.getRequestMatchingStrategies().size()); + assertEquals(ToMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); - /** - * The session object should not allow for no request strategies to be present for request processing - */ - @Test(expected = IllegalArgumentException.class) - public void testStrategiesMutationEmpty() { - ua.setRequestMatchingStrategies(Collections.emptyList()); + ua.setLoopback(false); + assertEquals(1, requestMatcher.getRequestMatchingStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); } /** @@ -188,47 +199,53 @@ public void testStrategiesMutationEmpty() { */ @Test public void testIsLoopbackSetting() { + final RequestMatcher requestMatcher = ua.getRequestMatcher(); + // Default is false assertFalse(ua.isLoopback()); - assertEquals(1, ua.getRequestMatchingStrategies().size()); - assertEquals(RequestUriMatchingStrategy.class, ua.getRequestMatchingStrategies().get(0).getClass()); + assertEquals(1, requestMatcher.getRequestMatchingStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); ua.setLoopback(true); assertTrue(ua.isLoopback()); - assertEquals(2, ua.getRequestMatchingStrategies().size()); - assertEquals(RequestUriMatchingStrategy.class, ua.getRequestMatchingStrategies().get(0).getClass()); - assertEquals(ToMatchingStrategy.class, ua.getRequestMatchingStrategies().get(1).getClass()); + assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); + assertEquals(ToMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(1).getClass()); // Add a new strategy and set loopback to false to check that this strategy will not be deleted - List strategies = ua.getRequestMatchingStrategies(); - RequestMatchingStrategy additionalStrategy = new RequestMatchingStrategy(ua) { + RequestMatchingStrategy additionalStrategy = new RequestMatchingStrategy() { @Override - public boolean isRequestMatching(Request request) { - return false; + public boolean isRequestMatching(Request request, SipSession sipSession) { + return true; } }; - strategies.add(additionalStrategy); - - ua.setRequestMatchingStrategies(strategies); + requestMatcher.add(additionalStrategy); ua.setLoopback(false); - assertEquals(2, ua.getRequestMatchingStrategies().size()); - assertEquals(RequestUriMatchingStrategy.class, ua.getRequestMatchingStrategies().get(0).getClass()); - assertEquals(additionalStrategy, ua.getRequestMatchingStrategies().get(1)); + assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); + assertEquals(additionalStrategy, requestMatcher.getRequestMatchingStrategies().get(1)); } @Test public void testToMatching() { - testMatchingStrategy(new ToMatchingStrategy(ub)); + testMatchingStrategy(new ToMatchingStrategy()); } @Test public void testRequestUriMatching() { - testMatchingStrategy(new RequestUriMatchingStrategy(ub)); + testMatchingStrategy(new RequestUriMatchingStrategy()); } private void testMatchingStrategy(RequestMatchingStrategy requestMatchingStrategy) { - ub.setRequestMatchingStrategies(Arrays.asList(requestMatchingStrategy)); + final RequestMatcher requestMatcher = ub.getRequestMatcher(); + + List initialStrategies = requestMatcher.getRequestMatchingStrategies(); + + requestMatcher.add(requestMatchingStrategy); + for (RequestMatchingStrategy initialStrategy : initialStrategies) { + requestMatcher.remove(initialStrategy); + } assertTrue(ua.register("userA", "test1", uaContact, 4890, 1000000)); assertLastOperationSuccess("user a registration - " + ua.format(), ua); From e9faf25526b4abd2eeb8b43c6b8139797f473fc4 Mon Sep 17 00:00:00 2001 From: Mario Milas Date: Thu, 11 Jan 2018 10:52:02 +0100 Subject: [PATCH 3/7] Add discrimination for singleton-type strategies The To and Request URI matching strategies are now treated as singleton strategies, permitting only one instance in the matcher. This is to improve performance and remove bugs when adding multiple same-typed strategies. If a strategy wants to be instantiated multiple times (which is default), it has to return the corresponding boolean parameter. This may be applicable in cases where a SipUnit client wants to, for example, accept multiple numbers in the To header using a same strategy class. --- .../sipunit/matching/RequestMatcher.java | 29 +++++++++---- .../matching/RequestMatchingStrategy.java | 41 ++++++++++++++++--- .../matching/RequestUriMatchingStrategy.java | 4 ++ .../sipunit/matching/ToMatchingStrategy.java | 6 ++- .../test/misc/TestRequestMatching.java | 38 +++++++++++++++++ 5 files changed, 102 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java b/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java index 342b6e935e..7feafb958c 100644 --- a/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java +++ b/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java @@ -5,10 +5,7 @@ import org.slf4j.LoggerFactory; import javax.sip.message.Request; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; +import java.util.*; /** * This class takes care that the inbound requests received by a governing {@link SipSession} are tested against a @@ -76,14 +73,20 @@ public boolean requestMatches(final Request request, final SipSession sipSession } /** - * Add the strategy to be used in request processing in this matches + * Add the strategy to be used in request processing in this matches. If the strategy is not permitted to add multiple + * instances and an existing instance of the same strategy class is present in the matcher, it will not be added. + * Otherwise, the strategy will be added per the collection add behavior contract. * * @param requestMatchingStrategy The added request matching strategy * @return If the request matching strategy was successfully added to the list of strategies used by this matcher + * @see List#add(Object) */ public boolean add(RequestMatchingStrategy requestMatchingStrategy) { - // TODO: check if strategy is forced to have only one instance - return requestMatchingStrategies.add(requestMatchingStrategy); + synchronized (requestMatchingStrategies) { + boolean permittedToAddAdditionalInstances = requestMatchingStrategy.multipleInstanceAllowed() || + !contains(requestMatchingStrategy.getClass()); + return permittedToAddAdditionalInstances && requestMatchingStrategies.add(requestMatchingStrategy); + } } /** @@ -94,7 +97,12 @@ public boolean add(RequestMatchingStrategy requestMatchingStrategy) { * @see List#remove(Object) */ public boolean remove(RequestMatchingStrategy requestMatchingStrategy) { - return requestMatchingStrategies.remove(requestMatchingStrategy); + synchronized (requestMatchingStrategies){ + boolean isRemoved = requestMatchingStrategies.remove(requestMatchingStrategy); + setDefaultStrategy(); + + return isRemoved; + } } /** @@ -157,7 +165,10 @@ public boolean contains(RequestMatchingStrategy requestMatchingStrategy) { /** * @return A list of matching strategies for incoming requests which this instance uses to determine - * if a request will be accepted or not. This list is unmodifiable and will be updated with each change. + * if a request will be accepted or not. This list is unmodifiable and synchronized and will be updated + * with each change. + * + * @see Collections#synchronizedCollection(Collection) */ public List getRequestMatchingStrategies() { return Collections.unmodifiableList(requestMatchingStrategies); diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java index 6c2610341a..34a98027a8 100644 --- a/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java +++ b/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java @@ -15,21 +15,32 @@ *

* Created by TELES AG on 09/01/2018. * + * @see RequestMatcher * @see SipSession#processRequest(RequestEvent) */ public abstract class RequestMatchingStrategy { protected static final Logger LOG = LoggerFactory.getLogger(RequestMatchingStrategy.class); + private final boolean multipleInstanceAllowed; + /** - * Determines if the inbound request matches the criterion defined by this matching strategy. - * - * @param request The inbound request - * @param sipSession The governing SipSession which received the request through its {@link org.cafesip.sipunit.SipStack} + * Initialize this strategy with multiple instances of this class allowed to be present in the matcher + */ + public RequestMatchingStrategy() { + this(true); + } + + /** + * Initialize this strategy with option of multiple instances of this class to be present in the matcher * - * @return True if the request matches the defined criterion, false otherwise + * @param multipleInstanceAllowed If set to true, the matcher will allow multiple instances of this class. Otherwise, + * any additional instances will not be added to the matcher, and will be treated as + * a localized singleton. */ - public abstract boolean isRequestMatching(final Request request, final SipSession sipSession); + public RequestMatchingStrategy(boolean multipleInstanceAllowed) { + this.multipleInstanceAllowed = multipleInstanceAllowed; + } protected static boolean isSipUriEquals(SipURI uri1, SipURI uri2) { if (uri1.getScheme().equalsIgnoreCase(uri2.getScheme())) { @@ -108,4 +119,22 @@ protected static boolean isSipUriEquals(SipURI uri1, SipURI uri2) { return false; } + + /** + * @return If true, the matcher will allow multiple instances of this class. Otherwise, + * any additional instances will not be added to the matcher, and will be treated as + * a localized singleton. + */ + public final boolean multipleInstanceAllowed() { + return multipleInstanceAllowed; + } + + /** + * Determines if the inbound request matches the criterion defined by this matching strategy. + * + * @param request The inbound request + * @param sipSession The governing SipSession which received the request through its {@link org.cafesip.sipunit.SipStack} + * @return True if the request matches the defined criterion, false otherwise + */ + public abstract boolean isRequestMatching(final Request request, final SipSession sipSession); } diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java index 110748763d..54ba68a027 100644 --- a/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java +++ b/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java @@ -12,6 +12,10 @@ */ public final class RequestUriMatchingStrategy extends RequestMatchingStrategy { + public RequestUriMatchingStrategy(){ + super(false); + } + @Override public boolean isRequestMatching(Request request, SipSession sipSession) { LOG.trace("my local contact info ('Request URI' check) = {}", sipSession.getContactInfo().getURI()); diff --git a/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java index 0f7d7aae87..f859d9277a 100644 --- a/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java +++ b/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java @@ -14,6 +14,10 @@ */ public final class ToMatchingStrategy extends RequestMatchingStrategy { + public ToMatchingStrategy(){ + super(false); + } + @Override public boolean isRequestMatching(Request request, SipSession sipSession) { ToHeader to = (ToHeader) request.getHeader(ToHeader.NAME); @@ -25,5 +29,5 @@ public boolean isRequestMatching(Request request, SipSession sipSession) { return expected.equals(me); } - + } diff --git a/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java index a937d3e032..89cadec404 100644 --- a/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java +++ b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java @@ -190,6 +190,44 @@ public void testRemoveOnlyToStrategy() { assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); } + @Test + public void testMultipleInstancesAllowed() { + RequestMatchingStrategy toStrategy = new ToMatchingStrategy(); + RequestMatchingStrategy requestUriStrategy = new RequestUriMatchingStrategy(); + + assertFalse(toStrategy.multipleInstanceAllowed()); + assertFalse(requestUriStrategy.multipleInstanceAllowed()); + + RequestMatchingStrategy multipleInstancesStrategy = new RequestMatchingStrategy() { + @Override + public boolean isRequestMatching(Request request, SipSession sipSession) { + return false; + } + }; + + final RequestMatcher requestMatcher = new RequestMatcher(); + assertEquals(1, requestMatcher.getRequestMatchingStrategies().size()); + assertTrue(requestMatcher.contains(requestUriStrategy.getClass())); + + assertTrue(requestMatcher.add(toStrategy)); + assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); + + // Already has this strategies by now + assertFalse(requestMatcher.add(requestUriStrategy)); + assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); + + assertFalse(requestMatcher.add(toStrategy)); + assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); + + // Add a multiple instance strategy + assertTrue(requestMatcher.add(multipleInstancesStrategy)); + assertEquals(3, requestMatcher.getRequestMatchingStrategies().size()); + + assertTrue(requestMatcher.add(multipleInstancesStrategy)); + assertEquals(4, requestMatcher.getRequestMatchingStrategies().size()); + } + + /** * Backwards compatibility check for isLoopback after the addition of request matching strategies *

From 1dc04e02d00b1898b7ed16320ec0e40cfa9520da Mon Sep 17 00:00:00 2001 From: Mario Milas Date: Thu, 11 Jan 2018 10:52:02 +0100 Subject: [PATCH 4/7] Fix javadoc error --- src/main/java/org/cafesip/sipunit/SipSession.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/cafesip/sipunit/SipSession.java b/src/main/java/org/cafesip/sipunit/SipSession.java index 848d259cee..347991d90a 100644 --- a/src/main/java/org/cafesip/sipunit/SipSession.java +++ b/src/main/java/org/cafesip/sipunit/SipSession.java @@ -1870,7 +1870,7 @@ public boolean isLoopback() { * the 'To' header matches even if the Request URI doesn't - so that local messaging tests without * proxy still work. This is for direct UA-UA testing convenience. This should not be the default, * however. - *

+ *

* If set to false, it will remove any existing {@link ToMatchingStrategy} in the request matching list. * * @param loopback The loopback to set. From 4775086c67c687e57eb04156af5903347771de72 Mon Sep 17 00:00:00 2001 From: Mario Milas Date: Thu, 11 Jan 2018 15:11:50 +0100 Subject: [PATCH 5/7] Fix indentation in request matching trace --- src/main/java/org/cafesip/sipunit/SipSession.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/cafesip/sipunit/SipSession.java b/src/main/java/org/cafesip/sipunit/SipSession.java index 347991d90a..e28d4391f7 100644 --- a/src/main/java/org/cafesip/sipunit/SipSession.java +++ b/src/main/java/org/cafesip/sipunit/SipSession.java @@ -407,9 +407,9 @@ public void processRequest(RequestEvent request) { } } } else if (requestMatches(req_msg)) { - LOG.trace(" incoming request match found, proceeding with processing"); + LOG.trace("incoming request match found, proceeding with processing"); } else { - LOG.trace(" no match found for incoming request, skipping processing"); + LOG.trace("no match found for incoming request, skipping processing"); return; } From 657541d0125f167d8d062156ab565c778952ec4e Mon Sep 17 00:00:00 2001 From: Mario Milas Date: Fri, 12 Jan 2018 09:21:18 +0100 Subject: [PATCH 6/7] Add request processor This commit introduces a concept of a request processing strategy, which is evaluated against the incoming request. This (collection of) strateg(y/ies) provides a determination mechanism of a request being processed by a SipSession or its children. Thus a user may provide extensions to the SipSession on which inbound requests may be accepted without the need to mutate any parameters, or request event processing mechanisms in SipPhone This is a derivation of the request matcher, allowing for a more flexible implementation. (cherry picked from commit 30dbcee) --- .../processing/RequestProcessingResult.java | 38 +++ .../processing/RequestProcessingStrategy.java | 62 +++++ .../sipunit/processing/RequestProcessor.java | 216 ++++++++++++++++++ .../test/misc/TestRequestProcessing.java | 171 ++++++++++++++ 4 files changed, 487 insertions(+) create mode 100644 src/main/java/org/cafesip/sipunit/processing/RequestProcessingResult.java create mode 100644 src/main/java/org/cafesip/sipunit/processing/RequestProcessingStrategy.java create mode 100644 src/main/java/org/cafesip/sipunit/processing/RequestProcessor.java create mode 100644 src/test/java/org/cafesip/sipunit/test/misc/TestRequestProcessing.java diff --git a/src/main/java/org/cafesip/sipunit/processing/RequestProcessingResult.java b/src/main/java/org/cafesip/sipunit/processing/RequestProcessingResult.java new file mode 100644 index 0000000000..f548a44164 --- /dev/null +++ b/src/main/java/org/cafesip/sipunit/processing/RequestProcessingResult.java @@ -0,0 +1,38 @@ +package org.cafesip.sipunit.processing; + +/** + * Contains provisional data on whether a request event was accepted by any {@link RequestProcessingStrategy} and if + * that processing step was successful. + *

+ * Created by TELES AG on 15/01/2018. + * + * @see RequestProcessor + */ +public class RequestProcessingResult { + + private final boolean isProcessed; + private final boolean isSuccessful; + + /** + * @param isProcessed If the input was accepted and processed + * @param isSuccessful If the processing result was successful + */ + public RequestProcessingResult(boolean isProcessed, boolean isSuccessful) { + this.isProcessed = isProcessed; + this.isSuccessful = isSuccessful; + } + + /** + * @return True if any strategy was able to accept the input of the request processor + */ + public boolean isProcessed() { + return isProcessed; + } + + /** + * @return True if the accepting strategy was able to successfully process the input of the request processor + */ + public boolean isSuccessful() { + return isSuccessful; + } +} diff --git a/src/main/java/org/cafesip/sipunit/processing/RequestProcessingStrategy.java b/src/main/java/org/cafesip/sipunit/processing/RequestProcessingStrategy.java new file mode 100644 index 0000000000..5654440e13 --- /dev/null +++ b/src/main/java/org/cafesip/sipunit/processing/RequestProcessingStrategy.java @@ -0,0 +1,62 @@ +package org.cafesip.sipunit.processing; + +import org.cafesip.sipunit.SipSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.sip.RequestEvent; +import javax.sip.SipListener; + +/** + * The request matching strategy is used within every {@link SipListener} subclass which is bound to process an incoming + * request. + * In order to provide multiple ways of accepting a request from various sources, we introduce this type to provide an + * unified way to determine if a request fits a criterion for it to be accepted and processed within a session. + *

+ * Created by TELES AG on 12/01/2018. + * + * @see RequestProcessor + * @see SipSession#processRequest(RequestEvent) + */ +public abstract class RequestProcessingStrategy { + + protected static final Logger LOG = LoggerFactory.getLogger(RequestProcessingStrategy.class); + + private final boolean multipleInstanceAllowed; + + /** + * Initialize this strategy with multiple instances of this class allowed to be present in {@link RequestProcessor} + */ + public RequestProcessingStrategy() { + this(true); + } + + /** + * Initialize this strategy with option of multiple instances of this class to be present in {@link RequestProcessor} + * + * @param multipleInstanceAllowed If set to true, the processor will allow multiple instances of this class. Otherwise, + * any additional instances will not be added to the processor, and will be treated as + * a localized singleton. + */ + public RequestProcessingStrategy(boolean multipleInstanceAllowed) { + this.multipleInstanceAllowed = multipleInstanceAllowed; + } + + /** + * @return If true, the matcher will allow multiple instances of this class. Otherwise, any additional instances + * will not be added to the processor, and will be treated as a localized singleton. + */ + public final boolean multipleInstanceAllowed() { + return multipleInstanceAllowed; + } + + /** + * Determines if the inbound request is processed according to the criterion defined by this strategy. + * + * @param requestEvent The inbound request event + * @param receiver The governing receiver handler which received the request through its {@link org.cafesip.sipunit.SipStack} + * @return The result of the processing. A request may be accepted, but it may not be successful. If the request is accepted, + * no other strategies in the governing processor will execute. + */ + public abstract RequestProcessingResult processRequestEvent(final RequestEvent requestEvent, final ReceiverType receiver); +} diff --git a/src/main/java/org/cafesip/sipunit/processing/RequestProcessor.java b/src/main/java/org/cafesip/sipunit/processing/RequestProcessor.java new file mode 100644 index 0000000000..d819686df1 --- /dev/null +++ b/src/main/java/org/cafesip/sipunit/processing/RequestProcessor.java @@ -0,0 +1,216 @@ +package org.cafesip.sipunit.processing; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.sip.RequestEvent; +import javax.sip.SipListener; +import javax.sip.message.Request; +import java.util.*; + +/** + * This class takes care that the inbound requests received by a governing {@link SipListener} subclass are tested + * against a configurable list of available strategies. The {@link RequestProcessingStrategy} instances which this class + * manages are in charge of testing an inbound SIP {@link Request} and providing feedback if the strategy was able to + * process the request successfully. The semantics of the (result of) processing is at the discretion of the implementing + * strategy. + *

+ * The request processor additionally manages that the request processing strategies do not produce side effects when being + * mutated, and provides concurrent access to the strategy handling mechanism. The default strategies which are provided by + * this class on instantiation are specified with the dedicated constructor. + *

+ * Created by TELES AG on 12/01/2018. + * + * @see RequestProcessingStrategy + */ +public class RequestProcessor { + + protected static final Logger LOG = LoggerFactory.getLogger(RequestProcessor.class); + + /** + * Request processing strategies process an incoming {@link Request} for the governing client after receiving + * the request through the stack. This class is initialized with a subset of initial strategies. + * The user of this library may add additional processing strategies in order to add additional processing options. + */ + private final List> availableStrategies = + Collections.synchronizedList(new ArrayList>()); + + /** + * Initialize this instance with the provided initial strategies + * + * @param initialStrategies Initial strategies which should be added to the instance + * @see RequestProcessor#add(RequestProcessingStrategy) + */ + public RequestProcessor(final RequestProcessingStrategy... initialStrategies) { + this(Arrays.asList(initialStrategies)); + } + + /** + * Initialize this instance with the provided initial strategies + * + * @param initialStrategies Initial strategies which should be added to the instance + * @see RequestProcessor#add(RequestProcessingStrategy) + */ + public RequestProcessor(final List> initialStrategies) { + synchronized (availableStrategies) { + for (RequestProcessingStrategy strategy : initialStrategies) { + add(strategy); + } + } + } + + /** + * Run all configured strategies in this matcher and determine if the request matches any configured criterion. The + * processor will execute every strategy in the available strategy list until the first strategy reports that the + * processing was successful. + * + * @param requestEvent The request being tested for a processing success with the available strategies + * @param receiver The governing object which received the request + * @return Result denoting if the request has been processed by any configured strategy, and if it was processed + * successfully + */ + public RequestProcessingResult processRequestEvent(final RequestEvent requestEvent, final ReceiverType receiver) { + RequestProcessingResult requestProcessingResult = new RequestProcessingResult(false, false); + + synchronized (availableStrategies) { + Iterator> iterator = availableStrategies.iterator(); + + // If we find a match, then the other strategies will not execute + while (!requestProcessingResult.isProcessed() && iterator.hasNext()) { + RequestProcessingStrategy strategy = iterator.next(); + requestProcessingResult = strategy.processRequestEvent(requestEvent, receiver); + + if(requestProcessingResult == null){ + LOG.warn("Request processing strategy " + strategy.getClass().getName() + " returned null"); + + requestProcessingResult = new RequestProcessingResult(false, false); + } + } + } + return requestProcessingResult; + } + + /** + * Add the strategy to be used in request processing in this processor. If the strategy is not permitted to add multiple + * instances and an existing instance of the same strategy class is present in the processor, it will not be added. + * Otherwise, the strategy will be added per the collection add behavior contract. + * + * @param requestProcessingStrategy The added request processing strategy + * @return If the request processing strategy was successfully added to the list of strategies used by this class + * @see List#add(Object) + */ + public boolean add(RequestProcessingStrategy requestProcessingStrategy) { + assertNotNull(requestProcessingStrategy); + + synchronized (availableStrategies) { + boolean permittedToAddAdditionalInstances = requestProcessingStrategy.multipleInstanceAllowed() || + !contains(requestProcessingStrategy.getClass()); + return permittedToAddAdditionalInstances && availableStrategies.add(requestProcessingStrategy); + } + } + + /** + * Removes the instance of the specified request processing strategy + * + * @param requestProcessingStrategy The strategy that needs to be removed by its reference in the request processor + * @return True if the specified instance of the searched strategy has been removed, false otherwise + * @see List#remove(Object) + */ + public boolean remove(RequestProcessingStrategy requestProcessingStrategy) { + assertNotNull(requestProcessingStrategy); + + synchronized (availableStrategies) { + if (availableStrategies.contains(requestProcessingStrategy) && availableStrategies.size() == 1) { + throw new IllegalArgumentException("Cannot remove only remaining strategy"); + } + + return availableStrategies.remove(requestProcessingStrategy); + } + } + + /** + * Removes any existing {@link RequestProcessingStrategy} defined by the searched class in the strategy list. + * If this is the only strategy in the strategy list before removal, the strategy list will be set to default, + * i.e. be reset with the default configured strategies. + * + * @param searchedClass The class that needs to be removed by its type in the request processor + * @return True if any instance of the searched strategy has been removed, false otherwise + */ + public boolean remove(Class> searchedClass) { + assertNotNull(searchedClass); + boolean isRemoved = false; + + synchronized (availableStrategies) { + Iterator> it = availableStrategies.iterator(); + + while (it.hasNext()) { + RequestProcessingStrategy current = it.next(); + + if (current.getClass().equals(searchedClass)) { + if (availableStrategies.size() == 1) { + throw new IllegalArgumentException("Cannot remove only remaining strategy"); + } + + isRemoved = true; + it.remove(); + } + } + } + + return isRemoved; + } + + /** + * Check if any existing {@link RequestProcessingStrategy} defined by the searched class is present in the configured + * strategy list. + * + * @param searchedClass The class whose direct instances will be searched for in the request class + * @return True if any instance of the searched strategy has been found, false otherwise + */ + public boolean contains(Class searchedClass) { + assertNotNull(searchedClass); + + synchronized (availableStrategies) { + for (RequestProcessingStrategy requestProcessingStrategy : availableStrategies) { + if (requestProcessingStrategy.getClass().equals(searchedClass)) { + return true; + } + } + } + + return false; + } + + /** + * Check if the {@link RequestProcessingStrategy} is added to this instance (by reference). + * + * @return True if the specified instance of the searched strategy has been found, false otherwise + * @see List#contains(Object) + */ + public boolean contains(RequestProcessingStrategy requestProcessingStrategy) { + assertNotNull(requestProcessingStrategy); + + return availableStrategies.contains(requestProcessingStrategy); + } + + /** + * @return A list of available strategies for incoming requests which this instance uses to process the inbound + * request. This list is unmodifiable and synchronized and will be updated with each change. + * @see Collections#synchronizedCollection(Collection) + */ + public List> getAvailableStrategies() { + return Collections.unmodifiableList(availableStrategies); + } + + private void assertNotNull(RequestProcessingStrategy requestProcessingStrategy) { + if(requestProcessingStrategy == null){ + throw new IllegalArgumentException("Request processing strategy may not be null"); + } + } + + private void assertNotNull(Class searchedClass) { + if(searchedClass == null){ + throw new IllegalArgumentException("Request processing strategy class may not be null"); + } + } +} diff --git a/src/test/java/org/cafesip/sipunit/test/misc/TestRequestProcessing.java b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestProcessing.java new file mode 100644 index 0000000000..8d87adfd2c --- /dev/null +++ b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestProcessing.java @@ -0,0 +1,171 @@ +package org.cafesip.sipunit.test.misc; + +import org.cafesip.sipunit.SipSession; +import org.cafesip.sipunit.processing.RequestProcessingResult; +import org.cafesip.sipunit.processing.RequestProcessingStrategy; +import org.cafesip.sipunit.processing.RequestProcessor; +import org.junit.Before; +import org.junit.Test; + +import javax.sip.RequestEvent; +import java.util.List; + +import static org.junit.Assert.*; + +/** + * Test behavior of processing processing on the configuration of {@link org.cafesip.sipunit.processing.RequestProcessingStrategy}. + *

+ * Created by TELES AG on 12/01/2018. + */ +public class TestRequestProcessing { + + private RequestProcessingStrategy defaultRequestProcessingStrategy = new RequestProcessingStrategy(false) { + @Override + public RequestProcessingResult processRequestEvent(RequestEvent requestEvent, SipSession receiver) { + return new RequestProcessingResult(false, false); + } + }; + + private RequestProcessor requestProcessor; + + @Before + public void setUp() { + requestProcessor = new RequestProcessor(defaultRequestProcessingStrategy); + } + + @Test(expected = IllegalArgumentException.class) + public void testShouldNotInitializeWithNull() { + new RequestProcessor<>((RequestProcessingStrategy) null); + } + + /** + * The request processor should not allow any mutation because of the immutable list + */ + @Test(expected = UnsupportedOperationException.class) + public void testGetStrategiesMutation() { + requestProcessor.getAvailableStrategies().clear(); + } + + /** + * If the strategy list has only the default strategy and this strategy is attempted to be removed, the processor + * should throw an exception to prevent side effects + */ + @Test(expected = IllegalArgumentException.class) + public void testRemoveLastStrategyClass() { + requestProcessor.remove(defaultRequestProcessingStrategy.getClass()); + } + + /** + * If the strategy list has only the default strategy and this strategy is attempted to be removed, the processor + * should throw an exception to prevent side effects + */ + @Test(expected = IllegalArgumentException.class) + public void testRemoveLastStrategyInstance() { + requestProcessor.remove(defaultRequestProcessingStrategy); + } + + @Test(expected = IllegalArgumentException.class) + public void testShouldNotAddNull() { + requestProcessor.add(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testShouldNotRemoveNullInstance() { + requestProcessor.remove((RequestProcessingStrategy) null); + } + + @Test(expected = IllegalArgumentException.class) + public void testShouldNotRemoveNullClass() { + requestProcessor.remove((Class) null); + } + + @Test(expected = IllegalArgumentException.class) + public void testShouldNotSearchForNullInstance() { + requestProcessor.contains((RequestProcessingStrategy) null); + } + + @Test(expected = IllegalArgumentException.class) + public void testShouldNotSearchForNullClass() { + requestProcessor.contains((Class) null); + } + + @Test + public void testStrategiesMutation() { + // Attempt mutating the list obtained through the getter + // Should have 1 default processing strategies + List requestProcessingStrategies = requestProcessor.getAvailableStrategies(); + assertEquals(1, requestProcessingStrategies.size()); + + // Create a dummy strategy + RequestProcessingStrategy newStrategy = createMockStrategy(); + + // Mutate strategies through the accessor + requestProcessor.add(newStrategy); + // Returned list should be updated + assertEquals(2, requestProcessingStrategies.size()); + + List newMatchingStrategies = requestProcessor.getAvailableStrategies(); + assertEquals(2, newMatchingStrategies.size()); + assertEquals(newStrategy, newMatchingStrategies.get(1)); + + assertTrue(requestProcessor.contains(newStrategy)); + assertTrue(requestProcessor.contains(newStrategy.getClass())); + + // Default strategy + assertTrue(requestProcessor.contains(defaultRequestProcessingStrategy.getClass())); + } + + @Test + public void testMultipleInstancesAllowed() { + RequestProcessingStrategy multipleInstancesStrategy = createMockStrategy(); + + assertFalse(defaultRequestProcessingStrategy.multipleInstanceAllowed()); + assertTrue(multipleInstancesStrategy.multipleInstanceAllowed()); + + assertEquals(1, requestProcessor.getAvailableStrategies().size()); + assertTrue(requestProcessor.contains(defaultRequestProcessingStrategy.getClass())); + + assertTrue(requestProcessor.add(multipleInstancesStrategy)); + assertEquals(2, requestProcessor.getAvailableStrategies().size()); + + // Already has this strategy by now + assertFalse(requestProcessor.add(defaultRequestProcessingStrategy)); + assertEquals(2, requestProcessor.getAvailableStrategies().size()); + + // Add a multiple instance strategy + assertTrue(requestProcessor.add(multipleInstancesStrategy)); + assertEquals(3, requestProcessor.getAvailableStrategies().size()); + + assertTrue(requestProcessor.add(multipleInstancesStrategy)); + assertEquals(4, requestProcessor.getAvailableStrategies().size()); + } + + @Test + public void testShouldConvertNullResultToFailed() { + RequestProcessingStrategy nullStrategy = new RequestProcessingStrategy() { + @Override + public RequestProcessingResult processRequestEvent(RequestEvent requestEvent, SipSession receiver) { + return null; + } + }; + requestProcessor.add(nullStrategy); + + RequestProcessingResult result = requestProcessor.processRequestEvent(new RequestEvent(this, null, null, null), null); + assertNotNull(result); + assertFalse(result.isProcessed()); + assertFalse(result.isSuccessful()); + } + + private RequestProcessingStrategy createMockStrategy() { + return createMockStrategy(true); + } + + private RequestProcessingStrategy createMockStrategy(boolean multipleInstancesAllowed) { + return new RequestProcessingStrategy(multipleInstancesAllowed) { + @Override + public RequestProcessingResult processRequestEvent(RequestEvent requestEvent, SipSession receiver) { + return new RequestProcessingResult(false, false); + } + }; + } +} From 1f7e87b8fb857db1d91776c59ea291fcf151fc84 Mon Sep 17 00:00:00 2001 From: Mario Milas Date: Tue, 16 Jan 2018 10:30:22 +0100 Subject: [PATCH 7/7] Adapt request matching to go through the request processor --- .../java/org/cafesip/sipunit/SipSession.java | 41 ++-- .../sipunit/matching/RequestMatcher.java | 188 ------------------ .../matching/RequestUriMatchingStrategy.java | 27 --- .../sipunit/matching/ToMatchingStrategy.java | 33 --- .../matching/RequestUriMatchingStrategy.java} | 62 ++---- .../matching/ToMatchingStrategy.java | 38 ++++ .../test/misc/TestRequestMatching.java | 155 ++++----------- 7 files changed, 120 insertions(+), 424 deletions(-) delete mode 100644 src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java delete mode 100644 src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java delete mode 100644 src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java rename src/main/java/org/cafesip/sipunit/{matching/RequestMatchingStrategy.java => processing/matching/RequestUriMatchingStrategy.java} (55%) create mode 100644 src/main/java/org/cafesip/sipunit/processing/matching/ToMatchingStrategy.java diff --git a/src/main/java/org/cafesip/sipunit/SipSession.java b/src/main/java/org/cafesip/sipunit/SipSession.java index e28d4391f7..27427a2131 100644 --- a/src/main/java/org/cafesip/sipunit/SipSession.java +++ b/src/main/java/org/cafesip/sipunit/SipSession.java @@ -18,9 +18,11 @@ package org.cafesip.sipunit; import gov.nist.javax.sip.header.ParameterNames; -import org.cafesip.sipunit.matching.RequestMatcher; -import org.cafesip.sipunit.matching.RequestMatchingStrategy; -import org.cafesip.sipunit.matching.ToMatchingStrategy; +import org.cafesip.sipunit.processing.RequestProcessingResult; +import org.cafesip.sipunit.processing.RequestProcessingStrategy; +import org.cafesip.sipunit.processing.RequestProcessor; +import org.cafesip.sipunit.processing.matching.RequestUriMatchingStrategy; +import org.cafesip.sipunit.processing.matching.ToMatchingStrategy; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -165,7 +167,14 @@ public class SipSession implements SipListener, SipActionObject { */ private Map> requestListeners = new HashMap<>(); - private final RequestMatcher requestMatcher = new RequestMatcher(); + /** + * Request matching processor. By default, matches incoming request according to their request URI, but provides + * extensibility in request processing methods by adding additional {@link RequestProcessingStrategy} + * strategies. + */ + private final RequestProcessor requestMatcher = new RequestProcessor<>( + new RequestUriMatchingStrategy() + ); private boolean supportRegisterRequests; @@ -406,7 +415,7 @@ public void processRequest(RequestEvent request) { } } } - } else if (requestMatches(req_msg)) { + } else if (requestMatches(request)) { LOG.trace("incoming request match found, proceeding with processing"); } else { LOG.trace("no match found for incoming request, skipping processing"); @@ -465,11 +474,12 @@ public void processRequest(RequestEvent request) { * If no match, check 'To' = me (so that local messaging without proxy still works) - but ONLY IF setLoopback() * has been called * - * @param request The request being tested for a match with the available strategies + * @param requestEvent The request being tested for a match with the available strategies * @return If the request matches (true) or not (false) */ - private boolean requestMatches(Request request) { - return requestMatcher.requestMatches(request, this); + private boolean requestMatches(RequestEvent requestEvent) { + RequestProcessingResult result = requestMatcher.processRequestEvent(requestEvent, this); + return result.isProcessed() && result.isSuccessful(); } /** @@ -1856,7 +1866,7 @@ public String getStackAddress() { /** * @return Returns the loopback. See setLoopback(). * @see SipSession#setLoopback(boolean) - * @deprecated Replaced by {@link RequestMatcher} behavior + * @deprecated Replaced by {@link RequestProcessor} behavior */ @Deprecated public boolean isLoopback() { @@ -1874,16 +1884,21 @@ public boolean isLoopback() { * If set to false, it will remove any existing {@link ToMatchingStrategy} in the request matching list. * * @param loopback The loopback to set. - * @deprecated Replaced by {@link RequestMatcher} behavior + * @deprecated Replaced by {@link RequestProcessor} behavior * - * @see RequestMatcher#add(RequestMatchingStrategy) - * @see RequestMatcher#remove(Class) + * @see RequestProcessor#add(RequestProcessingStrategy) + * @see RequestProcessor#remove(Class) */ @Deprecated public void setLoopback(boolean loopback) { if (loopback) { requestMatcher.add(new ToMatchingStrategy()); } else { + // Safeguard against side effects caused by removing the request uri matching strategy + if (requestMatcher.getAvailableStrategies().size() == 1 && requestMatcher.contains(ToMatchingStrategy.class)) { + requestMatcher.add(new RequestUriMatchingStrategy()); + } + requestMatcher.remove(ToMatchingStrategy.class); } } @@ -1891,7 +1906,7 @@ public void setLoopback(boolean loopback) { /** * @return The request matcher which governs the acceptance of inbound requests in this session */ - public final RequestMatcher getRequestMatcher() { + public final RequestProcessor getRequestMatcher() { return requestMatcher; } diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java b/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java deleted file mode 100644 index 7feafb958c..0000000000 --- a/src/main/java/org/cafesip/sipunit/matching/RequestMatcher.java +++ /dev/null @@ -1,188 +0,0 @@ -package org.cafesip.sipunit.matching; - -import org.cafesip.sipunit.SipSession; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.sip.message.Request; -import java.util.*; - -/** - * This class takes care that the inbound requests received by a governing {@link SipSession} are tested against a - * configurable list of strategies. The {@link RequestMatchingStrategy} instances which this class manages are - * in charge of testing an inbound SIP {@link Request} and providing feedback if the receiver should accept and process - * the received request. - *

- * The request matcher additionally manages that the request matching strategies do not produce side effects when being - * mutated, and provides concurrent access to the strategy handling mechanism. The default strategy which is provided by - * this class on instantiation is {@link RequestUriMatchingStrategy}. - *

- * Created by TELES AG on 11/01/2018. - * - * @see RequestMatchingStrategy - */ -public class RequestMatcher { - - private static final Logger LOG = LoggerFactory.getLogger(RequestMatcher.class); - - /** - * Request matching strategies determine if an incoming {@link Request} will be accepted for this client after receiving - * the request through the stack. This class is initialized with {@link org.cafesip.sipunit.matching.RequestUriMatchingStrategy}}. - * The user of this library may add additional matching strategies in order to accept a certain request which has - * been formed in different ways depending on the SIP backend configuration. - */ - private final List requestMatchingStrategies = Collections.synchronizedList(new ArrayList()); - - /** - * Initialize this matcher with the default {@link RequestUriMatchingStrategy} - */ - public RequestMatcher() { - setDefaultStrategy(); - } - - /** - * Initialize this matcher with the provided default strategies - * - * @param initialStrategies Initial strategies which should be added to the matcher - * @see RequestMatcher#add(RequestMatchingStrategy) - */ - public RequestMatcher(List initialStrategies) { - for (RequestMatchingStrategy requestMatchingStrategy : initialStrategies) { - add(requestMatchingStrategy); - } - - setDefaultStrategy(); - } - - /** - * Run all configured strategies in this matcher and determine if the request matches any configured criterion. - * - * @param request The request being tested for a match with the available strategies - * @param sipSession The governing object which received the request - * @return If the request matches (true) or not (false) - */ - public boolean requestMatches(final Request request, final SipSession sipSession) { - boolean requestMatches = false; - synchronized (requestMatchingStrategies) { - for (RequestMatchingStrategy strategy : requestMatchingStrategies) { - // If we find a match, then the other strategies will not execute - requestMatches = requestMatches || strategy.isRequestMatching(request, sipSession); - } - } - return requestMatches; - } - - /** - * Add the strategy to be used in request processing in this matches. If the strategy is not permitted to add multiple - * instances and an existing instance of the same strategy class is present in the matcher, it will not be added. - * Otherwise, the strategy will be added per the collection add behavior contract. - * - * @param requestMatchingStrategy The added request matching strategy - * @return If the request matching strategy was successfully added to the list of strategies used by this matcher - * @see List#add(Object) - */ - public boolean add(RequestMatchingStrategy requestMatchingStrategy) { - synchronized (requestMatchingStrategies) { - boolean permittedToAddAdditionalInstances = requestMatchingStrategy.multipleInstanceAllowed() || - !contains(requestMatchingStrategy.getClass()); - return permittedToAddAdditionalInstances && requestMatchingStrategies.add(requestMatchingStrategy); - } - } - - /** - * Removes the instance of the specified request matching strategy - * - * @param requestMatchingStrategy The strategy that needs to be removed by its reference in the request matcher - * @return True if the specified instance of the searched strategy has been removed, false otherwise - * @see List#remove(Object) - */ - public boolean remove(RequestMatchingStrategy requestMatchingStrategy) { - synchronized (requestMatchingStrategies){ - boolean isRemoved = requestMatchingStrategies.remove(requestMatchingStrategy); - setDefaultStrategy(); - - return isRemoved; - } - } - - /** - * Removes any existing {@link RequestMatchingStrategy} defined by the searched class in the request matching list. - * If this is the only strategy in the strategy list before removal, the strategy list will be set to default, - * i.e. be reset with the default {@link RequestUriMatchingStrategy}. - * - * @param searchedClass The class that needs to be removed by its type in the request matcher - * @return True if any instance of the searched strategy has been removed, false otherwise - */ - public boolean remove(Class searchedClass) { - boolean isRemoved = false; - - synchronized (requestMatchingStrategies) { - Iterator it = requestMatchingStrategies.iterator(); - - while (it.hasNext()) { - RequestMatchingStrategy current = it.next(); - - if (current.getClass().equals(searchedClass)) { - isRemoved = true; - it.remove(); - } - } - - setDefaultStrategy(); - } - - return isRemoved; - } - - /** - * Check if any existing {@link RequestMatchingStrategy} defined by the searched class is present in the matching - * strategy list. - * - * @param searchedClass The class whose direct instances will be searched for in the request matcher - * @return True if any instance of the searched strategy has been found, false otherwise - */ - public boolean contains(Class searchedClass) { - synchronized (requestMatchingStrategies) { - for (RequestMatchingStrategy requestMatchingStrategy : requestMatchingStrategies) { - if (requestMatchingStrategy.getClass().equals(searchedClass)) { - return true; - } - } - } - - return false; - } - - /** - * Check if the {@link RequestMatchingStrategy} is added to the matcher (by reference). - * - * @return True if the specified instance of the searched strategy has been found, false otherwise - * @see List#contains(Object) - */ - public boolean contains(RequestMatchingStrategy requestMatchingStrategy) { - return requestMatchingStrategies.contains(requestMatchingStrategy); - } - - /** - * @return A list of matching strategies for incoming requests which this instance uses to determine - * if a request will be accepted or not. This list is unmodifiable and synchronized and will be updated - * with each change. - * - * @see Collections#synchronizedCollection(Collection) - */ - public List getRequestMatchingStrategies() { - return Collections.unmodifiableList(requestMatchingStrategies); - } - - /** - * Sets the default {@link RequestUriMatchingStrategy} if the available strategy list is empty. - */ - private void setDefaultStrategy() { - synchronized (requestMatchingStrategies) { - if (requestMatchingStrategies.isEmpty()) { - LOG.info("Request matching strategies empty, setting default strategy to " + RequestUriMatchingStrategy.class.getName()); - add(new RequestUriMatchingStrategy()); - } - } - } -} diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java deleted file mode 100644 index 54ba68a027..0000000000 --- a/src/main/java/org/cafesip/sipunit/matching/RequestUriMatchingStrategy.java +++ /dev/null @@ -1,27 +0,0 @@ -package org.cafesip.sipunit.matching; - -import org.cafesip.sipunit.SipSession; - -import javax.sip.address.SipURI; -import javax.sip.message.Request; - -/** - * Determines if the request is viable for processing base on the {@link Request#getRequestURI()} of the received {@link Request} - *

- * Created by TELES AG on 09/01/2018. - */ -public final class RequestUriMatchingStrategy extends RequestMatchingStrategy { - - public RequestUriMatchingStrategy(){ - super(false); - } - - @Override - public boolean isRequestMatching(Request request, SipSession sipSession) { - LOG.trace("my local contact info ('Request URI' check) = {}", sipSession.getContactInfo().getURI()); - - return isSipUriEquals((SipURI) sipSession.getContactInfo().getContactHeader().getAddress().getURI(), - (SipURI) request.getRequestURI()); - } - -} diff --git a/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java deleted file mode 100644 index f859d9277a..0000000000 --- a/src/main/java/org/cafesip/sipunit/matching/ToMatchingStrategy.java +++ /dev/null @@ -1,33 +0,0 @@ -package org.cafesip.sipunit.matching; - -import org.cafesip.sipunit.SipSession; - -import javax.sip.RequestEvent; -import javax.sip.header.ToHeader; -import javax.sip.message.Request; - -/** - * Determines if the request is viable for processing base on the {@link ToHeader} of the received {@link Request} in - * {@link SipSession#processRequest(RequestEvent)} - *

- * Created by TELES AG on 09/01/2018. - */ -public final class ToMatchingStrategy extends RequestMatchingStrategy { - - public ToMatchingStrategy(){ - super(false); - } - - @Override - public boolean isRequestMatching(Request request, SipSession sipSession) { - ToHeader to = (ToHeader) request.getHeader(ToHeader.NAME); - - String me = sipSession.getAddress().getURI().toString(); - String expected = to.getAddress().getURI().toString(); - - LOG.trace("me ('To' check) = {}", me); - - return expected.equals(me); - } - -} diff --git a/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/processing/matching/RequestUriMatchingStrategy.java similarity index 55% rename from src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java rename to src/main/java/org/cafesip/sipunit/processing/matching/RequestUriMatchingStrategy.java index 34a98027a8..200513ffd6 100644 --- a/src/main/java/org/cafesip/sipunit/matching/RequestMatchingStrategy.java +++ b/src/main/java/org/cafesip/sipunit/processing/matching/RequestUriMatchingStrategy.java @@ -1,48 +1,36 @@ -package org.cafesip.sipunit.matching; +package org.cafesip.sipunit.processing.matching; import org.cafesip.sipunit.SipSession; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.cafesip.sipunit.processing.RequestProcessingResult; +import org.cafesip.sipunit.processing.RequestProcessingStrategy; import javax.sip.RequestEvent; import javax.sip.address.SipURI; import javax.sip.message.Request; /** - * The request matching strategy is used within every {@link SipSession} which is bound to process an incoming request. - * In order to provide multiple ways of accepting a request from various sources, we introduce this type to provide an - * unified way to determine if a request fits a criterion for it to be accepted and stored within a session. + * Determines if the request is viable for processing base on the {@link Request#getRequestURI()} of the received {@link Request} *

* Created by TELES AG on 09/01/2018. - * - * @see RequestMatcher - * @see SipSession#processRequest(RequestEvent) */ -public abstract class RequestMatchingStrategy { +public final class RequestUriMatchingStrategy extends RequestProcessingStrategy { - protected static final Logger LOG = LoggerFactory.getLogger(RequestMatchingStrategy.class); + public RequestUriMatchingStrategy(){ + super(false); + } - private final boolean multipleInstanceAllowed; + @Override + public RequestProcessingResult processRequestEvent(RequestEvent requestEvent, SipSession receiver) { + LOG.trace("my local contact info ('Request URI' check) = {}", receiver.getContactInfo().getURI()); + Request request = requestEvent.getRequest(); - /** - * Initialize this strategy with multiple instances of this class allowed to be present in the matcher - */ - public RequestMatchingStrategy() { - this(true); - } + boolean isRequestMatching = isSipUriEquals((SipURI) receiver.getContactInfo().getContactHeader().getAddress().getURI(), + (SipURI) request.getRequestURI()); - /** - * Initialize this strategy with option of multiple instances of this class to be present in the matcher - * - * @param multipleInstanceAllowed If set to true, the matcher will allow multiple instances of this class. Otherwise, - * any additional instances will not be added to the matcher, and will be treated as - * a localized singleton. - */ - public RequestMatchingStrategy(boolean multipleInstanceAllowed) { - this.multipleInstanceAllowed = multipleInstanceAllowed; + return new RequestProcessingResult(isRequestMatching, isRequestMatching); } - protected static boolean isSipUriEquals(SipURI uri1, SipURI uri2) { + private static boolean isSipUriEquals(SipURI uri1, SipURI uri2) { if (uri1.getScheme().equalsIgnoreCase(uri2.getScheme())) { if (uri1.getUser() != null) { if (uri2.getUser() == null) { @@ -119,22 +107,4 @@ protected static boolean isSipUriEquals(SipURI uri1, SipURI uri2) { return false; } - - /** - * @return If true, the matcher will allow multiple instances of this class. Otherwise, - * any additional instances will not be added to the matcher, and will be treated as - * a localized singleton. - */ - public final boolean multipleInstanceAllowed() { - return multipleInstanceAllowed; - } - - /** - * Determines if the inbound request matches the criterion defined by this matching strategy. - * - * @param request The inbound request - * @param sipSession The governing SipSession which received the request through its {@link org.cafesip.sipunit.SipStack} - * @return True if the request matches the defined criterion, false otherwise - */ - public abstract boolean isRequestMatching(final Request request, final SipSession sipSession); } diff --git a/src/main/java/org/cafesip/sipunit/processing/matching/ToMatchingStrategy.java b/src/main/java/org/cafesip/sipunit/processing/matching/ToMatchingStrategy.java new file mode 100644 index 0000000000..5c31c41f88 --- /dev/null +++ b/src/main/java/org/cafesip/sipunit/processing/matching/ToMatchingStrategy.java @@ -0,0 +1,38 @@ +package org.cafesip.sipunit.processing.matching; + +import org.cafesip.sipunit.SipSession; +import org.cafesip.sipunit.processing.RequestProcessingResult; +import org.cafesip.sipunit.processing.RequestProcessingStrategy; + +import javax.sip.RequestEvent; +import javax.sip.header.ToHeader; +import javax.sip.message.Request; + +/** + * Determines if the request is viable for processing base on the {@link ToHeader} of the received {@link Request} in + * {@link SipSession#processRequest(RequestEvent)} + *

+ * Created by TELES AG on 09/01/2018. + */ +public final class ToMatchingStrategy extends RequestProcessingStrategy { + + public ToMatchingStrategy(){ + super(false); + } + + @Override + public RequestProcessingResult processRequestEvent(RequestEvent requestEvent, SipSession receiver) { + Request request = requestEvent.getRequest(); + ToHeader to = (ToHeader) request.getHeader(ToHeader.NAME); + + String me = receiver.getAddress().getURI().toString(); + String expected = to.getAddress().getURI().toString(); + + LOG.trace("me ('To' check) = {}", me); + + boolean isMatching = expected.equals(me); + RequestProcessingResult result = new RequestProcessingResult(isMatching, isMatching); + + return result; + } +} diff --git a/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java index 89cadec404..74106b261c 100644 --- a/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java +++ b/src/test/java/org/cafesip/sipunit/test/misc/TestRequestMatching.java @@ -3,10 +3,11 @@ import org.cafesip.sipunit.SipPhone; import org.cafesip.sipunit.SipSession; import org.cafesip.sipunit.SipStack; -import org.cafesip.sipunit.matching.RequestMatcher; -import org.cafesip.sipunit.matching.RequestMatchingStrategy; -import org.cafesip.sipunit.matching.RequestUriMatchingStrategy; -import org.cafesip.sipunit.matching.ToMatchingStrategy; +import org.cafesip.sipunit.processing.RequestProcessingResult; +import org.cafesip.sipunit.processing.RequestProcessingStrategy; +import org.cafesip.sipunit.processing.RequestProcessor; +import org.cafesip.sipunit.processing.matching.RequestUriMatchingStrategy; +import org.cafesip.sipunit.processing.matching.ToMatchingStrategy; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -14,7 +15,7 @@ import org.slf4j.LoggerFactory; import javax.sip.RequestEvent; -import javax.sip.message.Request; +import javax.sip.SipListener; import javax.sip.message.Response; import java.text.ParseException; import java.util.List; @@ -29,7 +30,8 @@ import static org.junit.Assert.*; /** - * Test behavior of matching depending on the configuration of {@link RequestMatchingStrategy} in {@link SipSession}. + * Test behavior of matching depending on the configuration of {@link RequestProcessingStrategy} for request matching + * in {@link SipSession}. *

* Provides backwards compatibility testing with the removed loopback attribute which has been replaced by the tested * strategy approach. @@ -42,28 +44,24 @@ public class TestRequestMatching { private static final Logger LOG = LoggerFactory.getLogger(TestRequestMatching.class); private static final String LOCALHOST = "127.0.0.1"; - - private SipStack sipStackA; - private SipPhone ua; private final int uaPort = 5081; private final String uaProtocol = "UDP"; private final String uaContact = "sip:clientA@127.0.0.1:" + uaPort; - - private SipStack sipStackB; - private SipPhone ub; private final int ubPort = 5082; private final String ubProtocol = "UDP"; private final String ubContact = "sip:clientB@127.0.0.1:" + ubPort; - - private SipStack sipStackProxy; - private SipPhone proxy; private final int proxyPort = 5080; private final String proxyProtocol = "UDP"; private final String proxyContact = "sip:proxy@127.0.0.1:5080"; private final Properties uaProperties = new Properties(); private final Properties proxyProperties = new Properties(); private final ExecutorService executorService = Executors.newCachedThreadPool(); - + private SipStack sipStackA; + private SipPhone ua; + private SipStack sipStackB; + private SipPhone ub; + private SipStack sipStackProxy; + private SipPhone proxy; private ProxyMock proxyMock; @Before @@ -129,47 +127,6 @@ public void tearDown() { awaitStackDispose(sipStackProxy); } - /** - * The request matcher should not allow any mutation because of the immutable list - */ - @Test(expected = UnsupportedOperationException.class) - public void testGetStrategiesMutation() { - ua.getRequestMatcher().getRequestMatchingStrategies().clear(); - } - - @Test - public void testStrategiesMutation() { - final RequestMatcher requestMatcher = new RequestMatcher(); - - // Attempt mutating the list obtained through the getter - // Should have 2 default matching strategies - List requestMatchingStrategies = requestMatcher.getRequestMatchingStrategies(); - assertEquals(1, requestMatchingStrategies.size()); - - // Create a dummy strategy - RequestMatchingStrategy newStrategy = new RequestMatchingStrategy() { - @Override - public boolean isRequestMatching(Request request, SipSession sipSession) { - return true; - } - }; - - // Mutate strategies through the accessor - requestMatcher.add(newStrategy); - // Returned list should be updated - assertEquals(2, requestMatchingStrategies.size()); - - List newMatchingStrategies = requestMatcher.getRequestMatchingStrategies(); - assertEquals(2, newMatchingStrategies.size()); - assertEquals(newStrategy, newMatchingStrategies.get(1)); - - assertTrue(requestMatcher.contains(newStrategy)); - assertTrue(requestMatcher.contains(newStrategy.getClass())); - - // Default strategy - assertTrue(requestMatcher.contains(RequestUriMatchingStrategy.class)); - } - /** * If the strategy list has only {@link ToMatchingStrategy} and this strategy is removed through * {@link SipSession#setLoopback(boolean)}, the strategy list should be reset with the default @@ -177,57 +134,19 @@ public boolean isRequestMatching(Request request, SipSession sipSession) { */ @Test public void testRemoveOnlyToStrategy() { - final RequestMatcher requestMatcher = ua.getRequestMatcher(); + final RequestProcessor requestMatcher = ua.getRequestMatcher(); requestMatcher.add(new ToMatchingStrategy()); assertTrue(ua.isLoopback()); requestMatcher.remove(RequestUriMatchingStrategy.class); - assertEquals(1, requestMatcher.getRequestMatchingStrategies().size()); - assertEquals(ToMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); + assertEquals(1, requestMatcher.getAvailableStrategies().size()); + assertEquals(ToMatchingStrategy.class, requestMatcher.getAvailableStrategies().get(0).getClass()); ua.setLoopback(false); - assertEquals(1, requestMatcher.getRequestMatchingStrategies().size()); - assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); - } - - @Test - public void testMultipleInstancesAllowed() { - RequestMatchingStrategy toStrategy = new ToMatchingStrategy(); - RequestMatchingStrategy requestUriStrategy = new RequestUriMatchingStrategy(); - - assertFalse(toStrategy.multipleInstanceAllowed()); - assertFalse(requestUriStrategy.multipleInstanceAllowed()); - - RequestMatchingStrategy multipleInstancesStrategy = new RequestMatchingStrategy() { - @Override - public boolean isRequestMatching(Request request, SipSession sipSession) { - return false; - } - }; - - final RequestMatcher requestMatcher = new RequestMatcher(); - assertEquals(1, requestMatcher.getRequestMatchingStrategies().size()); - assertTrue(requestMatcher.contains(requestUriStrategy.getClass())); - - assertTrue(requestMatcher.add(toStrategy)); - assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); - - // Already has this strategies by now - assertFalse(requestMatcher.add(requestUriStrategy)); - assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); - - assertFalse(requestMatcher.add(toStrategy)); - assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); - - // Add a multiple instance strategy - assertTrue(requestMatcher.add(multipleInstancesStrategy)); - assertEquals(3, requestMatcher.getRequestMatchingStrategies().size()); - - assertTrue(requestMatcher.add(multipleInstancesStrategy)); - assertEquals(4, requestMatcher.getRequestMatchingStrategies().size()); + assertEquals(1, requestMatcher.getAvailableStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getAvailableStrategies().get(0).getClass()); } - /** * Backwards compatibility check for isLoopback after the addition of request matching strategies *

@@ -237,32 +156,32 @@ public boolean isRequestMatching(Request request, SipSession sipSession) { */ @Test public void testIsLoopbackSetting() { - final RequestMatcher requestMatcher = ua.getRequestMatcher(); + final RequestProcessor requestMatcher = ua.getRequestMatcher(); // Default is false assertFalse(ua.isLoopback()); - assertEquals(1, requestMatcher.getRequestMatchingStrategies().size()); - assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); + assertEquals(1, requestMatcher.getAvailableStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getAvailableStrategies().get(0).getClass()); ua.setLoopback(true); assertTrue(ua.isLoopback()); - assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); - assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); - assertEquals(ToMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(1).getClass()); + assertEquals(2, requestMatcher.getAvailableStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getAvailableStrategies().get(0).getClass()); + assertEquals(ToMatchingStrategy.class, requestMatcher.getAvailableStrategies().get(1).getClass()); // Add a new strategy and set loopback to false to check that this strategy will not be deleted - RequestMatchingStrategy additionalStrategy = new RequestMatchingStrategy() { + RequestProcessingStrategy additionalStrategy = new RequestProcessingStrategy() { @Override - public boolean isRequestMatching(Request request, SipSession sipSession) { - return true; + public RequestProcessingResult processRequestEvent(RequestEvent requestEvent, SipListener receiver) { + return new RequestProcessingResult(false, false); } }; requestMatcher.add(additionalStrategy); ua.setLoopback(false); - assertEquals(2, requestMatcher.getRequestMatchingStrategies().size()); - assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getRequestMatchingStrategies().get(0).getClass()); - assertEquals(additionalStrategy, requestMatcher.getRequestMatchingStrategies().get(1)); + assertEquals(2, requestMatcher.getAvailableStrategies().size()); + assertEquals(RequestUriMatchingStrategy.class, requestMatcher.getAvailableStrategies().get(0).getClass()); + assertEquals(additionalStrategy, requestMatcher.getAvailableStrategies().get(1)); } @Test @@ -275,14 +194,16 @@ public void testRequestUriMatching() { testMatchingStrategy(new RequestUriMatchingStrategy()); } - private void testMatchingStrategy(RequestMatchingStrategy requestMatchingStrategy) { - final RequestMatcher requestMatcher = ub.getRequestMatcher(); + private void testMatchingStrategy(RequestProcessingStrategy requestMatchingStrategy) { + final RequestProcessor requestMatcher = ub.getRequestMatcher(); - List initialStrategies = requestMatcher.getRequestMatchingStrategies(); + List> initialStrategies = requestMatcher.getAvailableStrategies(); requestMatcher.add(requestMatchingStrategy); - for (RequestMatchingStrategy initialStrategy : initialStrategies) { - requestMatcher.remove(initialStrategy); + if (initialStrategies.size() > 1) { + for (RequestProcessingStrategy initialStrategy : initialStrategies) { + requestMatcher.remove(initialStrategy); + } } assertTrue(ua.register("userA", "test1", uaContact, 4890, 1000000));