From 0c169ecfeaa9dbc5f16e658894655f71df20c0b8 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 10 Oct 2024 12:10:41 -0600 Subject: [PATCH] Polish Update Tests Made more robust by verifying fewer implementation details and relying more on expected outcomes Issue gh-446 --- .../ldap/core/LdapTemplateTests.java | 74 +++++++++++-------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/core/src/test/java/org/springframework/ldap/core/LdapTemplateTests.java b/core/src/test/java/org/springframework/ldap/core/LdapTemplateTests.java index 1f9eee7f5..213b497fc 100644 --- a/core/src/test/java/org/springframework/ldap/core/LdapTemplateTests.java +++ b/core/src/test/java/org/springframework/ldap/core/LdapTemplateTests.java @@ -23,6 +23,7 @@ import javax.naming.Name; import javax.naming.NamingEnumeration; import javax.naming.NamingException; +import javax.naming.directory.Attribute; import javax.naming.directory.BasicAttributes; import javax.naming.directory.DirContext; import javax.naming.directory.ModificationItem; @@ -60,6 +61,7 @@ import static org.mockito.BDDMockito.never; import static org.mockito.BDDMockito.times; import static org.mockito.BDDMockito.verify; +import static org.mockito.BDDMockito.willAnswer; import static org.mockito.BDDMockito.willDoNothing; import static org.mockito.BDDMockito.willThrow; @@ -1151,56 +1153,68 @@ public void testCreateWithNoIdAvailableThrows() throws NamingException { @Test public void testUpdateWithIdSpecified() throws NamingException { - given(this.contextSourceMock.getReadOnlyContext()).willReturn(this.dirContextMock); - given(this.contextSourceMock.getReadWriteContext()).willReturn(this.dirContextMock); + MockDirContext dirContext = new MockDirContext(); LdapName expectedName = LdapUtils.newLdapName("ou=someOu"); - - ModificationItem[] expectedModificationItems = new ModificationItem[0]; - DirContextOperations ctxMock = mock(DirContextOperations.class); - given(ctxMock.getDn()).willReturn(expectedName); - given(ctxMock.isUpdateMode()).willReturn(true); - given(ctxMock.getModificationItems()).willReturn(expectedModificationItems); - + dirContext.bind(expectedName, TestDirContextAdapters.forUpdate(expectedName)); Object expectedObject = new Object(); + Attribute added = TestNameAwareAttributes.attribute("someName", "someValue"); + ModificationItem[] expectedModificationItems = { TestModificationItems.add(added) }; + + ArgumentCaptor entryCaptor = ArgumentCaptor.forClass(LdapDataEntry.class); + given(this.contextSourceMock.getReadOnlyContext()).willReturn(dirContext); + given(this.contextSourceMock.getReadWriteContext()).willReturn(dirContext); given(this.odmMock.getId(expectedObject)).willReturn(expectedName); given(this.odmMock.getCalculatedId(expectedObject)).willReturn(null); - - given(this.dirContextMock.lookup(expectedName)).willReturn(ctxMock); + given(this.odmMock.manageClass(Object.class)).willReturn(new String[0]); + willAnswer((invocation) -> { + LdapDataEntry entry = invocation.getArgument(1); + entry.addAttributeValue(added.getID(), added.get()); + return null; + }).given(this.odmMock).mapToLdapDataEntry(eq(expectedObject), entryCaptor.capture()); this.tested.update(expectedObject); - verify(this.odmMock, never()).setId(expectedObject, expectedName); - verify(this.odmMock).mapToLdapDataEntry(expectedObject, ctxMock); - verify(this.dirContextMock).modifyAttributes(expectedName, expectedModificationItems); - - verify(this.dirContextMock, times(2)).close(); + verify(this.odmMock, never()).setId(any(), any()); + DirContextOperations operations = (DirContextOperations) entryCaptor.getValue(); + assertEqualModificationItems(operations.getModificationItems(), expectedModificationItems); + assertThat(dirContext.isClosed()).isTrue(); + assertThat(dirContext.getAttributes(expectedName).size()).isEqualTo(2); } @Test public void testUpdateWithIdCalculated() throws NamingException { - given(this.contextSourceMock.getReadOnlyContext()).willReturn(this.dirContextMock); - given(this.contextSourceMock.getReadWriteContext()).willReturn(this.dirContextMock); + MockDirContext dirContext = new MockDirContext(); LdapName expectedName = LdapUtils.newLdapName("ou=someOu"); - - ModificationItem[] expectedModificationItems = new ModificationItem[0]; - DirContextOperations ctxMock = mock(DirContextOperations.class); - given(ctxMock.getDn()).willReturn(expectedName); - given(ctxMock.isUpdateMode()).willReturn(true); - given(ctxMock.getModificationItems()).willReturn(expectedModificationItems); - + dirContext.bind(expectedName, TestDirContextAdapters.forUpdate(expectedName)); Object expectedObject = new Object(); + Attribute added = TestNameAwareAttributes.attribute("someName", "someValue"); + ModificationItem[] expectedModificationItems = { TestModificationItems.add(added) }; + + ArgumentCaptor entryCaptor = ArgumentCaptor.forClass(DirContextOperations.class); + given(this.contextSourceMock.getReadOnlyContext()).willReturn(dirContext); + given(this.contextSourceMock.getReadWriteContext()).willReturn(dirContext); given(this.odmMock.getId(expectedObject)).willReturn(null); given(this.odmMock.getCalculatedId(expectedObject)).willReturn(expectedName); - - given(this.dirContextMock.lookup(expectedName)).willReturn(ctxMock); + given(this.odmMock.manageClass(Object.class)).willReturn(new String[0]); + willAnswer((invocation) -> { + LdapDataEntry entry = invocation.getArgument(1); + entry.addAttributeValue(added.getID(), added.get()); + return null; + }).given(this.odmMock).mapToLdapDataEntry(eq(expectedObject), entryCaptor.capture()); this.tested.update(expectedObject); verify(this.odmMock).setId(expectedObject, expectedName); - verify(this.odmMock).mapToLdapDataEntry(expectedObject, ctxMock); - verify(this.dirContextMock).modifyAttributes(expectedName, expectedModificationItems); + assertEqualModificationItems(entryCaptor.getValue().getModificationItems(), expectedModificationItems); + assertThat(dirContext.isClosed()).isTrue(); + assertThat(dirContext.getAttributes(expectedName).size()).isEqualTo(2); + } - verify(this.dirContextMock, times(2)).close(); + private static void assertEqualModificationItems(ModificationItem[] actual, ModificationItem[] expected) { + assertThat(actual).hasSize(expected.length); + for (int i = 0; i < actual.length; i++) { + TestModificationItems.assertEquals(actual[i], expected[i]); + } } @Test