From a11f078db3c6921146373966b9b95fb5c25d4c2c Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Fri, 14 Jun 2013 18:06:16 +0000
Subject: [PATCH] Fix OPENDJ-932: ConnectionFactory timeout setting is applied for persistent search requests
---
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/AbstractLDAPFutureResultImpl.java | 18 ++++-
opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/LDAPConnectionTestCase.java | 125 +++++++++++++++++++++++++++++++++++++++++
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPSearchFutureResultImpl.java | 18 ++++--
opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java | 2
4 files changed, 151 insertions(+), 12 deletions(-)
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/AbstractLDAPFutureResultImpl.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/AbstractLDAPFutureResultImpl.java
index 4aa9c0f..0558a48 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/AbstractLDAPFutureResultImpl.java
+++ b/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;
+ }
}
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java
index dc20e65..0ca678f 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPConnection.java
+++ b/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);
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPSearchFutureResultImpl.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPSearchFutureResultImpl.java
index 2c98e0c..39f2022 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/main/java/com/forgerock/opendj/ldap/LDAPSearchFutureResultImpl.java
+++ b/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;
+ }
}
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/LDAPConnectionTestCase.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/LDAPConnectionTestCase.java
new file mode 100644
index 0000000..5924e23
--- /dev/null
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/LDAPConnectionTestCase.java
@@ -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();
+ }
+ }
+
+}
--
Gitblit v1.10.0