Add Observation Tests

Issue gh-991

Author: jzheaux

core/src/main/java/org/springframework/ldap/core/support/ObservationContextSource.java

@@ -40,6 +40,7 @@
 import javax.naming.ldap.LdapContext;
 import javax.naming.ldap.LdapName;
 
+import com.mysema.commons.lang.Assert;
 import io.micrometer.common.KeyValue;
 import io.micrometer.common.KeyValues;
 import io.micrometer.observation.Observation;
@@ -69,9 +70,13 @@ public final class ObservationContextSource implements BaseLdapPathContextSource
 
 	private final ObservationRegistry registry;
 
-	public ObservationContextSource(BaseLdapPathContextSource contextSource, ObservationRegistry registry) {
+	public ObservationContextSource(BaseLdapPathContextSource contextSource, ObservationRegistry observationRegistry) {
+		Assert.notNull(contextSource, "contextSource cannot be null");
+		Assert.notNull(observationRegistry, "observationRegistry cannot be null");
+		Assert.isFalse(contextSource instanceof ObservationContextSource,
+				"contextSource is already wrapped in an ObservationContextSource");
 		this.contextSource = contextSource;
-		this.registry = registry;
+		this.registry = observationRegistry;
 		this.builder = DirContextOperationObservationContext.withContextSource(this.contextSource);
 	}
 
@@ -91,8 +96,8 @@ public DirContext getContext(String principal, String credentials) throws Naming
 	}
 
 	private DirContext wrapDirContext(DirContext delegate) {
-		if (delegate instanceof DirContextOperations operations) {
-			return operations;
+		if (delegate instanceof DirContextOperations) {
+			return delegate;
 		}
 		if (delegate instanceof LdapContext ldap) {
 			return new ObservationLdapContext(this.builder, ldap, this.registry);

core/src/test/java/org/springframework/ldap/core/support/ObservationContextSourceTests.java

@@ -20,40 +20,116 @@
 
 import javax.naming.NamingException;
 import javax.naming.directory.DirContext;
+import javax.naming.ldap.LdapContext;
+import javax.naming.ldap.StartTlsRequest;
 
 import io.micrometer.observation.tck.TestObservationRegistry;
 import io.micrometer.observation.tck.TestObservationRegistryAssert;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
+import org.springframework.ldap.core.DirContextOperations;
+
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 
+/**
+ * Tests for {@link ObservationContextSource}
+ */
 public class ObservationContextSourceTests {
 
 	private final TestObservationRegistry registry = TestObservationRegistry.create();
 
+	private final ObservationContextSource contextSource = new ObservationContextSource(new TestContextSource(),
+			this.registry);
+
 	@Test
-	public void dirContextGetAttributesWhenObservingThenObserves() throws Exception {
-		TestContextSource observed = new TestContextSource();
-		ObservationContextSource observing = new ObservationContextSource(observed, this.registry);
-		observing.getReadOnlyContext().getAttributes("ou=user,ou=people");
+	void dirContextGetAttributesWhenObservingThenObserves() throws Exception {
+		this.contextSource.getReadOnlyContext().getAttributes("ou=user,ou=people");
 		// @formatter:off
 		TestObservationRegistryAssert.assertThat(this.registry)
 			.hasObservationWithNameEqualTo("spring.ldap.dir.context.operations").that()
 			.hasContextualNameEqualTo("perform get.attributes");
 		// @formatter:on
 	}
 
-	private final class TestContextSource extends AbstractContextSource {
+	@Test
+	void dirContextGetAttributesByNameWhenObservingThenObserves() throws Exception {
+		this.contextSource.getReadOnlyContext().getAttributes("ou=user,ou=people", new String[] { "id" });
+		// @formatter:off
+		TestObservationRegistryAssert.assertThat(this.registry)
+			.hasObservationWithNameEqualTo("spring.ldap.dir.context.operations").that()
+			.hasContextualNameEqualTo("perform get.attributes")
+			.hasHighCardinalityKeyValue("attribute.ids", "[id]");
+		// @formatter:on
+	}
+
+	@Test
+	void dirContextRenameWhenObservingThenObserves() throws Exception {
+		this.contextSource.getReadOnlyContext().rename("ou=user,ou=people", "ou=carrot,ou=people");
+		// @formatter:off
+		TestObservationRegistryAssert.assertThat(this.registry)
+			.hasObservationWithNameEqualTo("spring.ldap.dir.context.operations").that()
+			.hasContextualNameEqualTo("perform rename");
+		// @formatter:on
+	}
+
+	@Test
+	void dirContextWhenDirContextOperationsThenDoesNotObserve() throws Exception {
+		BaseLdapPathContextSource contextSource = mock(BaseLdapPathContextSource.class);
+		ObservationContextSource observing = new ObservationContextSource(contextSource, this.registry);
+		DirContextOperations operations = mock(DirContextOperations.class);
+		given(contextSource.getReadOnlyContext()).willReturn(operations);
+		observing.getReadOnlyContext().getAttributes("ou=user,ou=people");
+		// @formatter:off
+		TestObservationRegistryAssert.assertThat(this.registry)
+			.hasNumberOfObservationsEqualTo(0);
+		// @formatter:on
+	}
+
+	@Test
+	void ldapContextWhenExtendedOperationThenObserves() throws Exception {
+		ObservationContextSource observing = new ObservationContextSource(new TestContextSource(true), this.registry);
+		((LdapContext) observing.getReadOnlyContext()).extendedOperation(new StartTlsRequest());
+		// @formatter:off
+		TestObservationRegistryAssert.assertThat(this.registry)
+			.hasObservationWithNameEqualTo("spring.ldap.dir.context.operations").that()
+			.hasContextualNameEqualTo("perform extended.operation");
+		// @formatter:on
+	}
+
+	@Test
+	void constructorWhenObservationContextSourceThenIllegalArgument() {
+		assertThatExceptionOfType(IllegalArgumentException.class)
+			.isThrownBy(() -> new ObservationContextSource(this.contextSource, this.registry));
+	}
+
+	@Test
+	void constructorWhenNullParametersThenIllegalArgumment() {
+		assertThatExceptionOfType(IllegalArgumentException.class)
+			.isThrownBy(() -> new ObservationContextSource(null, this.registry));
+		assertThatExceptionOfType(IllegalArgumentException.class)
+			.isThrownBy(() -> new ObservationContextSource(new TestContextSource(), null));
+	}
+
+	private static final class TestContextSource extends AbstractContextSource {
+
+		private final boolean ldapContext;
 
 		TestContextSource() {
+			this(false);
+		}
+
+		TestContextSource(boolean ldapContext) {
+			this.ldapContext = ldapContext;
 			setUrls(new String[] { "ldap://localhost:1234" });
 			setBase("dc=example,dc=org");
 			afterPropertiesSet();
 		}
 
 		@Override
 		protected DirContext getDirContextInstance(Hashtable<String, Object> environment) throws NamingException {
-			return mock(DirContext.class);
+			return this.ldapContext ? mock(LdapContext.class) : mock(DirContext.class);
 		}
 
 	}