Spring/spring-data-keyvalue
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

DATAKV-187 - Polishing.

Browse Source 
Remove Serializable ID constraints from factory beans. Replace casts with type.cast(…). Convert anonymous inner classes to lambdas. Remove unused code and casts. Simplify test entities by removing Serializable and using lombok. Letter casing, formatting, Javadoc.

Original pull request: #25.
This commit is contained in:
Mark Paluch
2017-07-21 14:18:33 +02:00
parent d6ee87ace5
commit 14da42ff15
16 changed files with 200 additions and 492 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/test/java/org/springframework/data/keyvalue/Person.java
View File

@@ -15,8 +15,6 @@
*/
package org.springframework.data.keyvalue;
import java.io.Serializable;
import org.springframework.data.annotation.Id;
import org.springframework.util.ObjectUtils;
@@ -26,9 +24,7 @@ import com.querydsl.core.annotations.QueryEntity;
* @author Christoph Strobl
*/
@QueryEntity
public class Person implements Serializable {
private static final long serialVersionUID = 4212763002445358314L;
public class Person {
private @Id String id;
private String firstname;

153
src/test/java/org/springframework/data/keyvalue/core/KeyValueTemplateTests.java
View File

@@ -18,6 +18,9 @@ package org.springframework.data.keyvalue.core;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import lombok.AllArgsConstructor;
import lombok.Data;
import java.io.Serializable;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
@@ -35,11 +38,11 @@ import org.springframework.data.annotation.Persistent;
import org.springframework.data.keyvalue.annotation.KeySpace;
import org.springframework.data.keyvalue.core.query.KeyValueQuery;
import org.springframework.data.map.MapKeyValueAdapter;
import org.springframework.util.ObjectUtils;
/**
* @author Christoph Strobl
* @author Oliver Gierke
* @author Mark Paluch
*/
public class KeyValueTemplateTests {
@@ -212,116 +215,31 @@ public class KeyValueTemplateTests {
assertThat(operations.findAll(ALIASED.getClass()), containsInAnyOrder(ALIASED, SUBCLASS_OF_ALIASED));
}
static class Foo implements Serializable {
private static final long serialVersionUID = -8912754229220128922L;
@Data
@AllArgsConstructor
static class Foo {
String foo;
public Foo(String foo) {
this.foo = foo;
}
public String getFoo() {
return foo;
}
@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(this.foo);
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof Foo)) {
return false;
}
Foo other = (Foo) obj;
return ObjectUtils.nullSafeEquals(this.foo, other.foo);
}
}
static class Bar implements Serializable {
@Data
@AllArgsConstructor
static class Bar {
private static final long serialVersionUID = 196011921826060210L;
String bar;
public Bar(String bar) {
this.bar = bar;
}
public String getBar() {
return bar;
}
@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(this.bar);
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof Bar)) {
return false;
}
Bar other = (Bar) obj;
return ObjectUtils.nullSafeEquals(this.bar, other.bar);
}
}
@Data
static class ClassWithStringId implements Serializable {
private static final long serialVersionUID = -7481030649267602830L;
@Id String id;
String value;
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ObjectUtils.nullSafeHashCode(this.id);
result = prime * result + ObjectUtils.nullSafeHashCode(this.value);
return result;
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof ClassWithStringId)) {
return false;
}
ClassWithStringId other = (ClassWithStringId) obj;
if (!ObjectUtils.nullSafeEquals(this.id, other.id)) {
return false;
}
if (!ObjectUtils.nullSafeEquals(this.value, other.value)) {
return false;
}
return true;
}
}
@ExplicitKeySpace(name = "aliased")
@Data
static class ClassWithTypeAlias implements Serializable {
private static final long serialVersionUID = -5921943364908784571L;
@@ -331,53 +249,6 @@ public class KeyValueTemplateTests {
public ClassWithTypeAlias(String name) {
this.name = name;
}
public String getId() {
return id;
}
public void setId(String id) {
this.id = id;
}
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ObjectUtils.nullSafeHashCode(this.id);
result = prime * result + ObjectUtils.nullSafeHashCode(this.name);
return result;
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof ClassWithTypeAlias)) {
return false;
}
ClassWithTypeAlias other = (ClassWithTypeAlias) obj;
if (!ObjectUtils.nullSafeEquals(this.id, other.id)) {
return false;
}
if (!ObjectUtils.nullSafeEquals(this.name, other.name)) {
return false;
}
return true;
}
}
static class SubclassOfAliasedType extends ClassWithTypeAlias {

136
src/test/java/org/springframework/data/keyvalue/core/KeyValueTemplateUnitTests.java
View File

@@ -20,7 +20,9 @@ import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
import java.io.Serializable;
import lombok.AllArgsConstructor;
import lombok.Data;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -54,12 +56,12 @@ import org.springframework.data.keyvalue.core.event.KeyValueEvent.BeforeGetEvent
import org.springframework.data.keyvalue.core.event.KeyValueEvent.BeforeInsertEvent;
import org.springframework.data.keyvalue.core.event.KeyValueEvent.BeforeUpdateEvent;
import org.springframework.data.keyvalue.core.query.KeyValueQuery;
import org.springframework.util.ObjectUtils;
/**
* @author Christoph Strobl
* @author Thomas Darimont
* @author Oliver Gierke
* @author Mark Paluch
*/
@RunWith(MockitoJUnitRunner.Silent.class)
public class KeyValueTemplateUnitTests {
@@ -179,9 +181,9 @@ public class KeyValueTemplateUnitTests {
verify(adapterMock, times(1)).get("1", Foo.class.getName(), Foo.class);
}
@Test(expected = IllegalArgumentException.class) // DATACMNS-525
@Test(expected = IllegalArgumentException.class) // DATACMNS-525, DATAKV-187
public void findByIdShouldThrowExceptionWhenGivenNullId() {
template.findById((Serializable) null, Foo.class);
template.findById(null, Foo.class);
}
@Test // DATACMNS-525
@@ -383,7 +385,7 @@ public class KeyValueTemplateUnitTests {
verifyZeroInteractions(publisherMock);
}
@Test // DATAKV-91, DATAKV-104
@Test // DATAKV-91, DATAKV-104, DATAKV-187
@SuppressWarnings({ "unchecked", "rawtypes" })
public void shouldPublishBeforeInsertEventCorrectly() {
@@ -396,12 +398,12 @@ public class KeyValueTemplateUnitTests {
verify(publisherMock, times(1)).publishEvent(captor.capture());
verifyNoMoreInteractions(publisherMock);
assertThat(captor.getValue().getKey(), is((Serializable) "1"));
assertThat(captor.getValue().getKey(), is("1"));
assertThat(captor.getValue().getKeyspace(), is(Foo.class.getName()));
assertThat(captor.getValue().getPayload(), is((Object) FOO_ONE));
}
@Test // DATAKV-91, DATAKV-104
@Test // DATAKV-91, DATAKV-104, DATAKV-187
@SuppressWarnings({ "unchecked", "rawtypes" })
public void shouldPublishAfterInsertEventCorrectly() {
@@ -414,12 +416,12 @@ public class KeyValueTemplateUnitTests {
verify(publisherMock, times(1)).publishEvent(captor.capture());
verifyNoMoreInteractions(publisherMock);
assertThat(captor.getValue().getKey(), is((Serializable) "1"));
assertThat(captor.getValue().getKey(), is("1"));
assertThat(captor.getValue().getKeyspace(), is(Foo.class.getName()));
assertThat(captor.getValue().getPayload(), is((Object) FOO_ONE));
}
@Test // DATAKV-91, DATAKV-104
@Test // DATAKV-91, DATAKV-104, DATAKV-187
@SuppressWarnings({ "unchecked", "rawtypes" })
public void shouldPublishBeforeUpdateEventCorrectly() {
@@ -432,12 +434,12 @@ public class KeyValueTemplateUnitTests {
verify(publisherMock, times(1)).publishEvent(captor.capture());
verifyNoMoreInteractions(publisherMock);
assertThat(captor.getValue().getKey(), is((Serializable) "1"));
assertThat(captor.getValue().getKey(), is("1"));
assertThat(captor.getValue().getKeyspace(), is(Foo.class.getName()));
assertThat(captor.getValue().getPayload(), is((Object) FOO_ONE));
}
@Test // DATAKV-91, DATAKV-104
@Test // DATAKV-91, DATAKV-104, DATAKV-187
@SuppressWarnings({ "unchecked", "rawtypes" })
public void shouldPublishAfterUpdateEventCorrectly() {
@@ -450,12 +452,12 @@ public class KeyValueTemplateUnitTests {
verify(publisherMock, times(1)).publishEvent(captor.capture());
verifyNoMoreInteractions(publisherMock);
assertThat(captor.getValue().getKey(), is((Serializable) "1"));
assertThat(captor.getValue().getKey(), is("1"));
assertThat(captor.getValue().getKeyspace(), is(Foo.class.getName()));
assertThat(captor.getValue().getPayload(), is((Object) FOO_ONE));
}
@Test // DATAKV-91, DATAKV-104
@Test // DATAKV-91, DATAKV-104, DATAKV-187
@SuppressWarnings({ "rawtypes", "unchecked" })
public void shouldPublishBeforeDeleteEventCorrectly() {
@@ -468,11 +470,11 @@ public class KeyValueTemplateUnitTests {
verify(publisherMock, times(1)).publishEvent(captor.capture());
verifyNoMoreInteractions(publisherMock);
assertThat(captor.getValue().getKey(), is((Serializable) "1"));
assertThat(captor.getValue().getKey(), is("1"));
assertThat(captor.getValue().getKeyspace(), is(Foo.class.getName()));
}
@Test // DATAKV-91, DATAKV-104
@Test // DATAKV-91, DATAKV-104, DATAKV-187
@SuppressWarnings({ "rawtypes", "unchecked" })
public void shouldPublishAfterDeleteEventCorrectly() {
@@ -486,12 +488,12 @@ public class KeyValueTemplateUnitTests {
verify(publisherMock, times(1)).publishEvent(captor.capture());
verifyNoMoreInteractions(publisherMock);
assertThat(captor.getValue().getKey(), is((Serializable) "1"));
assertThat(captor.getValue().getKey(), is("1"));
assertThat(captor.getValue().getKeyspace(), is(Foo.class.getName()));
assertThat(captor.getValue().getPayload(), is((Object) FOO_ONE));
}
@Test // DATAKV-91, DATAKV-104
@Test // DATAKV-91, DATAKV-104, DATAKV-187
@SuppressWarnings({ "rawtypes", "unchecked" })
public void shouldPublishBeforeGetEventCorrectly() {
@@ -506,7 +508,7 @@ public class KeyValueTemplateUnitTests {
verify(publisherMock, times(1)).publishEvent(captor.capture());
verifyNoMoreInteractions(publisherMock);
assertThat(captor.getValue().getKey(), is((Serializable) "1"));
assertThat(captor.getValue().getKey(), is("1"));
assertThat(captor.getValue().getKeyspace(), is(Foo.class.getName()));
}
@@ -525,12 +527,12 @@ public class KeyValueTemplateUnitTests {
verify(publisherMock, times(1)).publishEvent(captor.capture());
verifyNoMoreInteractions(publisherMock);
assertThat(captor.getValue().getKey(), is((Serializable) "1"));
assertThat(captor.getValue().getKey(), is("1"));
assertThat(captor.getValue().getKeyspace(), is(Foo.class.getName()));
assertThat(captor.getValue().getPayload(), is((Object) FOO_ONE));
}
@Test // DATAKV-91, DATAKV-104
@Test // DATAKV-91, DATAKV-104, DATAKV-187
@SuppressWarnings({ "rawtypes", "unchecked" })
public void shouldPublishDropKeyspaceEventCorrectly() {
@@ -559,108 +561,24 @@ public class KeyValueTemplateUnitTests {
template.setEventTypesToPublish(new HashSet<>(Arrays.asList(events)));
}
@Data
@AllArgsConstructor
static class Foo {
String foo;
public Foo(String foo) {
this.foo = foo;
}
public String getFoo() {
return foo;
}
@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(this.foo);
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof Foo)) {
return false;
}
Foo other = (Foo) obj;
return ObjectUtils.nullSafeEquals(this.foo, other.foo);
}
}
@Data
@AllArgsConstructor
class Bar {
String bar;
public Bar(String bar) {
this.bar = bar;
}
public String getBar() {
return bar;
}
@Override
public int hashCode() {
return ObjectUtils.nullSafeHashCode(this.bar);
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof Bar)) {
return false;
}
Bar other = (Bar) obj;
return ObjectUtils.nullSafeEquals(this.bar, other.bar);
}
}
@Data
static class ClassWithStringId {
@Id String id;
String value;
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ObjectUtils.nullSafeHashCode(this.id);
result = prime * result + ObjectUtils.nullSafeHashCode(this.value);
return result;
}
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
return false;
}
if (!(obj instanceof ClassWithStringId)) {
return false;
}
ClassWithStringId other = (ClassWithStringId) obj;
if (!ObjectUtils.nullSafeEquals(this.id, other.id)) {
return false;
}
if (!ObjectUtils.nullSafeEquals(this.value, other.value)) {
return false;
}
return true;
}
}
}

7
src/test/java/org/springframework/data/keyvalue/core/SpelQueryEngineUnitTests.java
View File

@@ -45,6 +45,7 @@ import org.springframework.expression.spel.support.StandardEvaluationContext;
*
* @author Martin Macko
* @author Oliver Gierke
* @author Mark Paluch
*/
@RunWith(MockitoJUnitRunner.class)
public class SpelQueryEngineUnitTests {
@@ -54,14 +55,14 @@ public class SpelQueryEngineUnitTests {
@Mock KeyValueAdapter adapter;
SpelQueryEngine<KeyValueAdapter> engine;
SpelQueryEngine engine;
Iterable<Person> people = Arrays.asList(BOB_WITH_FIRSTNAME, MIKE_WITHOUT_FIRSTNAME);
@Before
public void setUp() {
engine = new SpelQueryEngine<>();
engine = new SpelQueryEngine();
engine.registerAdapter(adapter);
}
@@ -102,7 +103,7 @@ public class SpelQueryEngineUnitTests {
return new SpelCriteria(creator.createQuery().getCriteria(), new StandardEvaluationContext(args));
}
static interface PersonRepository {
interface PersonRepository {
Person findByFirstname(String firstname);
}

64
src/test/java/org/springframework/data/keyvalue/repository/SimpleKeyValueRepositoryUnitTests.java
View File

@@ -22,7 +22,9 @@ import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.*;
import java.io.Serializable;
import lombok.Data;
import lombok.NoArgsConstructor;
import java.util.Arrays;
import java.util.Optional;
@@ -86,7 +88,7 @@ public class SimpleKeyValueRepositoryUnitTests {
public void multipleSave() {
Foo one = new Foo("one");
Foo two = new Foo("one");
Foo two = new Foo("two");
repo.saveAll(Arrays.asList(one, two));
verify(opsMock, times(1)).insert(eq(one));
@@ -121,25 +123,28 @@ public class SimpleKeyValueRepositoryUnitTests {
}
@Test // DATACMNS-525
@SuppressWarnings("unchecked")
public void findAllIds() {
when(opsMock.findById(any(Serializable.class), any(Class.class))).thenReturn(Optional.empty());
when(opsMock.findById(any(), any(Class.class))).thenReturn(Optional.empty());
repo.findAllById(Arrays.asList("one", "two", "three"));
verify(opsMock, times(3)).findById(anyString(), eq(Foo.class));
}
@Test // DATAKV-186
@SuppressWarnings("unchecked")
public void existsByIdReturnsFalseForEmptyOptional() {
when(opsMock.findById(any(Serializable.class), any(Class.class))).thenReturn(Optional.empty());
when(opsMock.findById(any(), any(Class.class))).thenReturn(Optional.empty());
assertThat(repo.existsById("one"), is(false));
}
@Test // DATAKV-186
@SuppressWarnings("unchecked")
public void existsByIdReturnsTrueWhenOptionalValuePresent() {
when(opsMock.findById(any(Serializable.class), any(Class.class))).thenReturn(Optional.of(new Foo()));
when(opsMock.findById(any(), any(Class.class))).thenReturn(Optional.of(new Foo()));
assertTrue(repo.existsById("one"));
}
@@ -168,6 +173,8 @@ public class SimpleKeyValueRepositoryUnitTests {
verify(opsMock, times(1)).findAll(eq(Foo.class));
}
@Data
@NoArgsConstructor
static class Foo {
private @Id String id;
@@ -175,65 +182,20 @@ public class SimpleKeyValueRepositoryUnitTests {
private String name;
private Bar bar;
public Foo() {
}
public Foo(String name) {
this.name = name;
}
public String getId() {
return id;
}
public void setId(String id) {
this.id = id;
}
public Long getLongValue() {
return longValue;
}
public void setLongValue(Long longValue) {
this.longValue = longValue;
}
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public Bar getBar() {
return bar;
}
public void setBar(Bar bar) {
this.bar = bar;
}
}
@Data
static class Bar {
private String bar;
public String getBar() {
return bar;
}
public void setBar(String bar) {
this.bar = bar;
}
}
@Persistent
static class WithNumericId {
@Id Integer id;
}
}

12
src/test/java/org/springframework/data/keyvalue/repository/support/KeyValueRepositoryFactoryBeanUnitTests.java
View File

@@ -19,8 +19,6 @@ import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
import static org.mockito.Mockito.*;
import java.io.Serializable;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -33,8 +31,9 @@ import org.springframework.data.repository.query.parser.AbstractQueryCreator;
/**
* Unit tests for {@link KeyValueRepositoryFactoryBean}.
*
*
* @author Oliver Gierke
* @author Mark Paluch
*/
public class KeyValueRepositoryFactoryBeanUnitTests {
@@ -44,7 +43,7 @@ public class KeyValueRepositoryFactoryBeanUnitTests {
@Before
public void setUp() {
this.factoryBean = new KeyValueRepositoryFactoryBean<Repository<Object, Serializable>, Object, Serializable>(
this.factoryBean = new KeyValueRepositoryFactoryBean<Repository<Object, Object>, Object, Object>(
SampleRepository.class);
}
@@ -87,8 +86,7 @@ public class KeyValueRepositoryFactoryBeanUnitTests {
Class<? extends AbstractQueryCreator<?, ?>> creatorType = (Class<? extends AbstractQueryCreator<?, ?>>) mock(
AbstractQueryCreator.class).getClass();
Class<? extends RepositoryQuery> queryType = (Class<? extends RepositoryQuery>) mock(KeyValuePartTreeQuery.class)
.getClass();
Class<? extends RepositoryQuery> queryType = mock(KeyValuePartTreeQuery.class).getClass();
factoryBean.setQueryCreator(creatorType);
factoryBean.setKeyValueOperations(mock(KeyValueOperations.class));
@@ -102,5 +100,5 @@ public class KeyValueRepositoryFactoryBeanUnitTests {
factoryBean.setQueryType(null);
}
interface SampleRepository extends Repository<Object, Serializable> {}
interface SampleRepository extends Repository<Object, Object> {}
}