Fix OPENDJ-932: ConnectionFactory timeout setting is applied for persistent...

Matthew Swift

14.06.2013 a11f078db3c6921146373966b9b95fb5c25d4c2c

Fix OPENDJ-932: ConnectionFactory timeout setting is applied for persistent search requests

Skip persistent searches when perform client side request timeout expiration.

 opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/AbstractLDAPFutureResultImpl.java

@@ -22,7 +22,7 @@
 *
 *
 *      Copyright 2009-2010 Sun Microsystems, Inc.
 *      Portions copyright 2011 ForgeRock AS.
 *      Portions copyright 2011-2013 ForgeRock AS.
 */

package com.forgerock.opendj.ldap;
@@ -47,13 +47,9 @@
abstract class AbstractLDAPFutureResultImpl<S extends Result>
        extends AsynchronousFutureResult<S, ResultHandler<? super S>>
        implements IntermediateResponseHandler {

    private final Connection connection;

    private final int requestID;

    private IntermediateResponseHandler intermediateResponseHandler;

    private volatile long timestamp;

    AbstractLDAPFutureResultImpl(final int requestID,
@@ -136,4 +132,16 @@
        timestamp = System.currentTimeMillis();
    }

    /**
     * Returns {@code true} if this request should be canceled once the timeout
     * period expires. The default implementation is to return {@code true}
     * which will be appropriate for nearly all requests, the one obvious
     * exception being persistent searches.
     *
     * @return {@code true} if this request should be canceled once the timeout
     *         period expires.
     */
    boolean checkForTimeout() {
        return true;
    }
}

 opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java

@@ -555,7 +555,7 @@
        if (timeout > 0) {
            for (final int requestID : pendingRequests.keySet()) {
                final AbstractLDAPFutureResultImpl<?> future = pendingRequests.get(requestID);
                if (future != null) {
                if (future != null && future.checkForTimeout()) {
                    final long diff = (future.getTimestamp() + timeout) - currentTime;
                    if (diff <= 0 && pendingRequests.remove(requestID) != null) {
                        DEBUG_LOG.fine("Cancelling expired future result: " + future);

 opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPSearchFutureResultImpl.java

@@ -22,7 +22,7 @@
 *
 *
 *      Copyright 2009-2010 Sun Microsystems, Inc.
 *      Portions copyright 2011 ForgeRock AS.
 *      Portions copyright 2011-2013 ForgeRock AS.
 */

package com.forgerock.opendj.ldap;
@@ -31,6 +31,7 @@
import org.forgerock.opendj.ldap.IntermediateResponseHandler;
import org.forgerock.opendj.ldap.ResultCode;
import org.forgerock.opendj.ldap.SearchResultHandler;
import org.forgerock.opendj.ldap.controls.PersistentSearchRequestControl;
import org.forgerock.opendj.ldap.requests.SearchRequest;
import org.forgerock.opendj.ldap.responses.Responses;
import org.forgerock.opendj.ldap.responses.Result;
@@ -42,10 +43,9 @@
 */
final class LDAPSearchFutureResultImpl extends AbstractLDAPFutureResultImpl<Result> implements
        SearchResultHandler {

    private SearchResultHandler searchResultHandler;

    private final SearchRequest request;
    private final boolean isPersistentSearch;

    LDAPSearchFutureResultImpl(final int requestID, final SearchRequest request,
            final SearchResultHandler resultHandler,
@@ -54,6 +54,7 @@
        super(requestID, resultHandler, intermediateResponseHandler, connection);
        this.request = request;
        this.searchResultHandler = resultHandler;
        this.isPersistentSearch = request.containsControl(PersistentSearchRequestControl.OID);
    }

    public boolean handleEntry(final SearchResultEntry entry) {
@@ -103,13 +104,18 @@
        return request;
    }

    /**
     * {@inheritDoc}
     */
    @Override
    Result newErrorResult(final ResultCode resultCode, final String diagnosticMessage,
            final Throwable cause) {
        return Responses.newResult(resultCode).setDiagnosticMessage(diagnosticMessage).setCause(
                cause);
    }

    /**
     * Persistent searches should not time out.
     */
    @Override
    boolean checkForTimeout() {
        return !isPersistentSearch;
    }
}

 opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/LDAPConnectionTestCase.java

New file
@@ -0,0 +1,125 @@
/*
 * CDDL HEADER START
 *
 * The contents of this file are subject to the terms of the
 * Common Development and Distribution License, Version 1.0 only
 * (the "License").  You may not use this file except in compliance
 * with the License.
 *
 * You can obtain a copy of the license at legal-notices/CDDLv1_0.txt
 * or http://forgerock.org/license/CDDLv1.0.html.
 * See the License for the specific language governing permissions
 * and limitations under the License.
 *
 * When distributing Covered Code, include this CDDL HEADER in each
 * file and include the License file at legal-notices/CDDLv1_0.txt.
 * If applicable, add the following below this CDDL HEADER, with the
 * fields enclosed by brackets "[]" replaced with your own identifying
 * information:
 *      Portions Copyright [yyyy] [name of copyright owner]
 *
 * CDDL HEADER END
 *
 *
 *      Copyright 2013 ForgeRock AS.
 */

package com.forgerock.opendj.ldap;

import static org.fest.assertions.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyZeroInteractions;

import java.net.SocketAddress;
import java.util.concurrent.TimeUnit;

import org.forgerock.opendj.ldap.Connections;
import org.forgerock.opendj.ldap.ErrorResultException;
import org.forgerock.opendj.ldap.LDAPConnectionFactory;
import org.forgerock.opendj.ldap.LDAPListener;
import org.forgerock.opendj.ldap.LDAPOptions;
import org.forgerock.opendj.ldap.RequestHandler;
import org.forgerock.opendj.ldap.ResultCode;
import org.forgerock.opendj.ldap.SearchResultHandler;
import org.forgerock.opendj.ldap.SearchScope;
import org.forgerock.opendj.ldap.TestCaseUtils;
import org.forgerock.opendj.ldap.TimeoutResultException;
import org.forgerock.opendj.ldap.controls.PersistentSearchRequestControl;
import org.forgerock.opendj.ldap.requests.Requests;
import org.forgerock.opendj.ldap.requests.SearchRequest;
import org.mockito.ArgumentCaptor;
import org.testng.annotations.Test;

/**
 * Tests LDAP connection implementation class.
 */
@SuppressWarnings("javadoc")
public class LDAPConnectionTestCase extends LDAPTestCase {

    /**
     * Tests that a normal request is subject to client side timeout checking.
     */
    @Test
    public void testRequestTimeout() throws Exception {
        doTestRequestTimeout(false);
    }

    /**
     * Tests that a persistent search request is not subject to client side
     * timeout checking.
     */
    @Test
    public void testRequestTimeoutPersistentSearch() throws Exception {
        doTestRequestTimeout(true);
    }

    private void doTestRequestTimeout(boolean isPersistentSearch) throws Exception {
        SocketAddress address = TestCaseUtils.findFreeSocketAddress();

        /*
         * Use a mock server implementation which will ignore incoming requests
         * and leave the client waiting forever for a response.
         */
        @SuppressWarnings("unchecked")
        LDAPListener listener =
                new LDAPListener(address, Connections
                        .newServerConnectionFactory(mock(RequestHandler.class)));

        /*
         * Use a very long time out in order to prevent the timeout thread from
         * triggering the timeout.
         */
        LDAPConnectionFactory factory =
                new LDAPConnectionFactory(address, new LDAPOptions().setTimeout(100,
                        TimeUnit.SECONDS));
        LDAPConnection connection = (LDAPConnection) factory.getConnection();
        try {
            SearchRequest request =
                    Requests.newSearchRequest("dc=test", SearchScope.BASE_OBJECT, "(objectClass=*)");
            if (isPersistentSearch) {
                request.addControl(PersistentSearchRequestControl.newControl(true, true, true));
            }
            SearchResultHandler handler = mock(SearchResultHandler.class);
            connection.searchAsync(request, null, handler);

            // Pass in a time which is guaranteed to trigger expiration.
            connection.cancelExpiredRequests(System.currentTimeMillis() + 1000000);
            if (isPersistentSearch) {
                verifyZeroInteractions(handler);
            } else {
                ArgumentCaptor<ErrorResultException> arg =
                        ArgumentCaptor.forClass(ErrorResultException.class);
                verify(handler).handleErrorResult(arg.capture());
                assertThat(arg.getValue()).isInstanceOf(TimeoutResultException.class);
                assertThat(arg.getValue().getResult().getResultCode()).isEqualTo(
                        ResultCode.CLIENT_SIDE_TIMEOUT);
            }
        } finally {
            connection.close();
            listener.close();
            factory.close();
        }
    }

}