In doing my final review of Huijuns change to add P-Asserted-Identity
(which by the way looks very good now), I noticed that there was no unit
test that actually tested that the proxy would indeed challenge a local
requester correctly. While I'm sure that you've tested this in a system
test setup, a unit test for it would be a quicker way to detect any
regression.
Since writing such a test would require knowing how to create a
test-specific credentials database, I thought it would be easier if I
just wrote the test...
Looking further, I realized that it's a little harder than that, because
the SipRouter::proxyMessage method sends responses internally in two
places (the challenge, and a reject for unsupported options), so there
is no opportunity for the unit test to look at the response message.
My original reason for creating the proxyMessage method (as opposed to
just putting all that logic directly into the SipRouter::handleMessage
method) was specifically that it be easier to test - all receiving and
sending of messages would take place in the caller, and proxyMessage
would just make decisions. That way, the tests can (and do) create
requests and pass them in and see what the proxy would do with them, but
never need to do any real I/O (which ducks all sorts of issues in
creating the test environment). Then later I went and broke that when I
added the Proxy-Require checks (sigh).
So... I propose to make a small change to the method signature from:
/// Modify the message as needed to be proxied
bool proxyMessage(SipMessage& sipRequest);
///< @returns true if message should be sent, false if not
to:
/// Action to be taken by the caller of proxyAction method
typedef enum
{
SendRequest,
SendResponse,
DoNothing
} ProxyAction;
/// Examine a request to be proxied, and either modify it for forwarding or
create a response.
ProxyAction proxyMessage(SipMessage& request, /**< request to be proxied
* iff return is
SendRequest */
SipMessage& response /**< response to be sent
* iff return is
SendResponse */
);
/**< the caller must pass either request or response
* to SipUserAgent::send as directed by the return code
*/
and then change the internals of proxyMessage so that when it wants to
send a response, it writes it into the message provided by the caller
and sets the return code appropriately; it would then never call
SipUserAgent::send. This will make expanding the unit test coverage
much easier.
Comments?
_______________________________________________
sipx-dev mailing list
[email protected]
List Archive: http://list.sipfoundry.org/archive/sipx-dev
Unsubscribe: http://list.sipfoundry.org/mailman/listinfo/sipx-dev
