From 6ac57812f85752faa755264b90d38d180eb85552 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 9 Jan 2025 09:47:36 -0700 Subject: [PATCH] Add Observation Tests Issue gh-991 --- .../support/ObservationContextSource.java | 13 ++- .../ObservationContextSourceTests.java | 90 +++++++++++++++++-- 2 files changed, 92 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/springframework/ldap/core/support/ObservationContextSource.java b/core/src/main/java/org/springframework/ldap/core/support/ObservationContextSource.java index bb11cd46f..4c9dfdaac 100644 --- a/core/src/main/java/org/springframework/ldap/core/support/ObservationContextSource.java +++ b/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); diff --git a/core/src/test/java/org/springframework/ldap/core/support/ObservationContextSourceTests.java b/core/src/test/java/org/springframework/ldap/core/support/ObservationContextSourceTests.java index 4c3f79324..4a6371027 100644 --- a/core/src/test/java/org/springframework/ldap/core/support/ObservationContextSourceTests.java +++ b/core/src/test/java/org/springframework/ldap/core/support/ObservationContextSourceTests.java @@ -20,22 +20,32 @@ 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() @@ -43,9 +53,75 @@ public void dirContextGetAttributesWhenObservingThenObserves() throws Exception // @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(); @@ -53,7 +129,7 @@ private final class TestContextSource extends AbstractContextSource { @Override protected DirContext getDirContextInstance(Hashtable environment) throws NamingException { - return mock(DirContext.class); + return this.ldapContext ? mock(LdapContext.class) : mock(DirContext.class); } }