DATACMNS-867 - Less Optional for auditing subsystem.

Removed Optional<Object> parameters in favor of plain Object and clients that avoid calling methods for absent values in the first place.
This commit is contained in:
Oliver Gierke
2017-03-15 15:26:49 +01:00
parent 5f7d1ae2f2
commit 082c4157d1
13 changed files with 101 additions and 125 deletions

View File

@@ -38,16 +38,16 @@ public class AuditingHandlerUnitTests {
AuditingHandler handler;
AuditorAware<AuditedUser> auditorAware;
Optional<AuditedUser> user;
AuditedUser user;
@Before
public void setUp() {
handler = getHandler();
user = Optional.of(new AuditedUser());
user = new AuditedUser();
auditorAware = mock(AuditorAware.class);
when(auditorAware.getCurrentAuditor()).thenReturn(user);
when(auditorAware.getCurrentAuditor()).thenReturn(Optional.of(user));
}
protected AuditingHandler getHandler() {
@@ -62,14 +62,11 @@ public class AuditingHandlerUnitTests {
handler.markCreated(user);
assertThat(user).hasValueSatisfying(it -> {
assertThat(user.getCreatedDate()).isPresent();
assertThat(user.getLastModifiedDate()).isPresent();
assertThat(it.getCreatedDate()).isPresent();
assertThat(it.getLastModifiedDate()).isPresent();
assertThat(it.getCreatedBy()).isNotPresent();
assertThat(it.getLastModifiedBy()).isNotPresent();
});
assertThat(user.getCreatedBy()).isNotPresent();
assertThat(user.getLastModifiedBy()).isNotPresent();
}
/**
@@ -82,14 +79,11 @@ public class AuditingHandlerUnitTests {
handler.markCreated(user);
assertThat(user).hasValueSatisfying(it -> {
assertThat(user.getCreatedDate()).isPresent();
assertThat(user.getLastModifiedDate()).isPresent();
assertThat(it.getCreatedDate()).isPresent();
assertThat(it.getLastModifiedDate()).isPresent();
assertThat(it.getCreatedBy()).isPresent();
assertThat(it.getLastModifiedBy()).isPresent();
});
assertThat(user.getCreatedBy()).isPresent();
assertThat(user.getLastModifiedBy()).isPresent();
verify(auditorAware).getCurrentAuditor();
}
@@ -104,14 +98,11 @@ public class AuditingHandlerUnitTests {
handler.setModifyOnCreation(false);
handler.markCreated(user);
assertThat(user).hasValueSatisfying(it -> {
assertThat(user.getCreatedDate()).isPresent();
assertThat(user.getCreatedBy()).isPresent();
assertThat(it.getCreatedDate()).isPresent();
assertThat(it.getCreatedBy()).isPresent();
assertThat(it.getLastModifiedBy()).isNotPresent();
assertThat(it.getLastModifiedDate()).isNotPresent();
});
assertThat(user.getLastModifiedBy()).isNotPresent();
assertThat(user.getLastModifiedDate()).isNotPresent();
verify(auditorAware).getCurrentAuditor();
}
@@ -125,19 +116,14 @@ public class AuditingHandlerUnitTests {
AuditedUser audited = new AuditedUser();
audited.id = 1L;
user = Optional.of(audited);
handler.setAuditorAware(auditorAware);
handler.markModified(user);
handler.markModified(audited);
assertThat(user).hasValueSatisfying(it -> {
assertThat(audited.getCreatedBy()).isNotPresent();
assertThat(audited.getCreatedDate()).isNotPresent();
assertThat(it.getCreatedBy()).isNotPresent();
assertThat(it.getCreatedDate()).isNotPresent();
assertThat(it.getLastModifiedBy()).isPresent();
assertThat(it.getLastModifiedDate()).isPresent();
});
assertThat(audited.getLastModifiedBy()).isPresent();
assertThat(audited.getLastModifiedDate()).isPresent();
verify(auditorAware).getCurrentAuditor();
}
@@ -149,14 +135,11 @@ public class AuditingHandlerUnitTests {
handler.setAuditorAware(auditorAware);
handler.markCreated(user);
assertThat(user).hasValueSatisfying(it -> {
assertThat(user.getCreatedBy()).isPresent();
assertThat(user.getCreatedDate()).isNotPresent();
assertThat(it.getCreatedBy()).isPresent();
assertThat(it.getCreatedDate()).isNotPresent();
assertThat(it.getLastModifiedBy()).isPresent();
assertThat(it.getLastModifiedDate()).isNotPresent();
});
assertThat(user.getLastModifiedBy()).isPresent();
assertThat(user.getLastModifiedDate()).isNotPresent();
}
@Test // DATAJPA-9

View File

@@ -36,26 +36,28 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests {
DefaultAuditableBeanWrapperFactory factory = new DefaultAuditableBeanWrapperFactory();
@Test
public void returnsEmptyForEmptySource() {
assertThat(factory.getBeanWrapperFor(Optional.empty())).isNotPresent();
@Test(expected = IllegalArgumentException.class)
public void rejectsNullSource() {
factory.getBeanWrapperFor(null);
}
@Test
public void returnsAuditableInterfaceBeanWrapperForAuditable() {
assertThat(factory.getBeanWrapperFor(Optional.of(new AuditedUser()))).hasValueSatisfying(it -> assertThat(it).isInstanceOf(AuditableInterfaceBeanWrapper.class));
assertThat(factory.getBeanWrapperFor(new AuditedUser()))
.hasValueSatisfying(it -> assertThat(it).isInstanceOf(AuditableInterfaceBeanWrapper.class));
}
@Test
public void returnsReflectionAuditingBeanWrapperForNonAuditableButAnnotated() {
assertThat(factory.getBeanWrapperFor(Optional.of(new AnnotatedUser()))).hasValueSatisfying(it -> assertThat(it).isInstanceOf(ReflectionAuditingBeanWrapper.class));
assertThat(factory.getBeanWrapperFor(new AnnotatedUser()))
.hasValueSatisfying(it -> assertThat(it).isInstanceOf(ReflectionAuditingBeanWrapper.class));
}
@Test
public void returnsEmptyForNonAuditableType() {
assertThat(factory.getBeanWrapperFor(Optional.of(new Object()))).isNotPresent();
assertThat(factory.getBeanWrapperFor(new Object())).isNotPresent();
}
@Test // DATACMNS-643
@@ -64,7 +66,7 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests {
Jsr310ThreeTenBpAuditedUser user = new Jsr310ThreeTenBpAuditedUser();
Instant instant = Instant.now();
Optional<AuditableBeanWrapper> wrapper = factory.getBeanWrapperFor(Optional.of(user));
Optional<AuditableBeanWrapper> wrapper = factory.getBeanWrapperFor(user);
assertThat(wrapper).hasValueSatisfying(it -> {
@@ -83,7 +85,7 @@ public class DefaultAuditableBeanWrapperFactoryUnitTests {
Jsr310ThreeTenBpAuditedUser user = new Jsr310ThreeTenBpAuditedUser();
ZonedDateTime zonedDateTime = ZonedDateTime.now();
Optional<AuditableBeanWrapper> wrapper = factory.getBeanWrapperFor(Optional.of(user));
Optional<AuditableBeanWrapper> wrapper = factory.getBeanWrapperFor(user);
assertThat(wrapper).hasValueSatisfying(it -> it.setLastModifiedDate(Optional.of(zonedDateTime)));
}

View File

@@ -15,8 +15,8 @@
*/
package org.springframework.data.auditing;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.Mockito.*;
import java.util.Collections;
import java.util.Optional;
@@ -25,9 +25,7 @@ import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.runners.MockitoJUnitRunner;
import org.springframework.data.annotation.Id;
import org.springframework.data.mapping.context.MappingContext;
import org.springframework.data.mapping.context.PersistentEntities;
import org.springframework.data.mapping.context.SampleMappingContext;
@@ -57,7 +55,7 @@ public class IsNewAwareAuditingHandlerUnitTests extends AuditingHandlerUnitTests
AuditedUser user = new AuditedUser();
getHandler().markAudited(Optional.of(user));
getHandler().markAudited(user);
assertThat(user.createdDate).isNotNull();
assertThat(user.modifiedDate).isNotNull();
@@ -69,7 +67,7 @@ public class IsNewAwareAuditingHandlerUnitTests extends AuditingHandlerUnitTests
AuditedUser user = new AuditedUser();
user.id = 1L;
getHandler().markAudited(Optional.of(user));
getHandler().markAudited(user);
assertThat(user.createdDate).isNull();
assertThat(user.modifiedDate).isNotNull();

View File

@@ -20,7 +20,6 @@ import static org.assertj.core.api.Assertions.*;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
@@ -54,16 +53,16 @@ public class MappingContextIsNewStrategyFactoryUnitTests {
IsNewStrategy strategy = factory.getIsNewStrategy(VersionedEntity.class);
Optional<VersionedEntity> entity = Optional.of(new VersionedEntity());
VersionedEntity entity = new VersionedEntity();
assertThat(strategy.isNew(entity)).isTrue();
entity.get().id = 1L;
entity.id = 1L;
assertThat(strategy.isNew(entity)).isTrue();
entity.get().version = 0L;
entity.version = 0L;
assertThat(strategy.isNew(entity)).isTrue();
entity.get().version = 1L;
entity.version = 1L;
assertThat(strategy.isNew(entity)).isFalse();
}
@@ -72,13 +71,13 @@ public class MappingContextIsNewStrategyFactoryUnitTests {
IsNewStrategy strategy = factory.getIsNewStrategy(VersionedEntity.class);
Optional<VersionedEntity> entity = Optional.of(new VersionedEntity());
VersionedEntity entity = new VersionedEntity();
assertThat(strategy.isNew(entity)).isTrue();
entity.get().id = 1L;
entity.id = 1L;
assertThat(strategy.isNew(entity)).isTrue();
entity.get().version = 1L;
entity.version = 1L;
assertThat(strategy.isNew(entity)).isFalse();
}
@@ -87,10 +86,10 @@ public class MappingContextIsNewStrategyFactoryUnitTests {
IsNewStrategy strategy = factory.getIsNewStrategy(Entity.class);
Optional<Entity> entity = Optional.of(new Entity());
Entity entity = new Entity();
assertThat(strategy.isNew(entity)).isTrue();
entity.get().id = 1L;
entity.id = 1L;
assertThat(strategy.isNew(entity)).isFalse();
}

View File

@@ -17,8 +17,6 @@ package org.springframework.data.support;
import static org.assertj.core.api.Assertions.*;
import java.util.Optional;
import org.junit.Test;
import org.springframework.data.domain.Persistable;
@@ -34,16 +32,16 @@ public class PersistableIsNewStrategyUnitTests {
@Test
public void invokesPersistableIsNewForTest() {
Optional<PersistableEntity> entity = Optional.of(new PersistableEntity());
PersistableEntity entity = new PersistableEntity();
assertThat(strategy.isNew(entity)).isTrue();
entity.get().isNew = false;
entity.isNew = false;
assertThat(strategy.isNew(entity)).isFalse();
}
@Test(expected = IllegalArgumentException.class)
public void rejectsNonPersistableEntity() {
strategy.isNew(Optional.of(new Object()));
strategy.isNew(new Object());
}
@SuppressWarnings("serial")