Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ColumnReader NullPointerException when finding a setSomething() method that is not a property mutator #917

Merged
merged 16 commits into from
Dec 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,20 @@

package io.spine.server.entity.storage;

import com.google.common.collect.ImmutableSet;
import io.spine.server.entity.Entity;

import java.beans.BeanInfo;
import java.beans.IntrospectionException;
import java.beans.Introspector;
import java.beans.PropertyDescriptor;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collection;
import java.util.Objects;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Lists.newLinkedList;
import static io.spine.server.entity.storage.Methods.getAnnotatedVersion;
import static io.spine.util.Exceptions.newIllegalStateException;
Expand Down Expand Up @@ -63,7 +67,8 @@ private ColumnReader(Class<? extends Entity> entityClass) {
* <p>The reader can be further used to {@linkplain ColumnReader#readColumns() obtain}
* {@linkplain EntityColumn entity columns} for the given class.
*
* @param entityClass {@link Entity} class for which to create the instance
* @param entityClass
* {@link Entity} class for which to create the instance
* @return new instance of {@code ColumnReader} for the specified class
*/
static ColumnReader forClass(Class<? extends Entity> entityClass) {
Expand All @@ -80,8 +85,10 @@ static ColumnReader forClass(Class<? extends Entity> entityClass) {
* <p>If the check for correctness fails, throws {@link IllegalStateException}.
*
* @return a {@code Collection} of {@link EntityColumn} corresponded to entity class
* @throws IllegalStateException if entity column definitions are incorrect
* @throws IllegalStateException
* if entity column definitions are incorrect
*/
@SuppressWarnings("ConstantConditions")
Collection<EntityColumn> readColumns() {
BeanInfo entityDescriptor;
try {
Expand All @@ -90,28 +97,31 @@ Collection<EntityColumn> readColumns() {
throw new IllegalStateException(e);
}

Collection<EntityColumn> entityColumns = newLinkedList();
for (PropertyDescriptor property : entityDescriptor.getPropertyDescriptors()) {
Method getter = property.getReadMethod();
boolean isEntityColumn = getAnnotatedVersion(getter).isPresent();
if (isEntityColumn) {
EntityColumn column = EntityColumn.from(getter);
entityColumns.add(column);
}
}

checkRepeatedColumnNames(entityColumns);
return entityColumns;
PropertyDescriptor[] propertyDescriptors = entityDescriptor.getPropertyDescriptors();
ImmutableSet<EntityColumn> columns = Arrays
.stream(propertyDescriptors)
.map(PropertyDescriptor::getReadMethod)
.filter(Objects::nonNull)
.filter(ColumnReader::isAnnotated)
.map(EntityColumn::from)
.collect(toImmutableSet());
checkRepeatedColumnNames(columns);
return columns;
}

private static boolean isAnnotated(Method method) {
return getAnnotatedVersion(method).isPresent();
}

/**
* Ensures that the specified columns have no repeated names.
*
* <p>If the check fails, throws {@link IllegalStateException}.
*
* @param columns the columns to check
* @throws IllegalStateException if columns contain repeated names
* @param columns
* the columns to check
* @throws IllegalStateException
* if columns contain repeated names
*/
private void checkRepeatedColumnNames(Iterable<EntityColumn> columns) {
Collection<String> checkedNames = newLinkedList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import com.google.common.testing.NullPointerTester;
import com.google.common.testing.NullPointerTester.Visibility;
import io.spine.server.entity.storage.given.ColumnsTestEnv.EntityWithASetterButNoGetter;
import io.spine.server.entity.storage.given.ColumnsTestEnv.EntityWithColumnFromInterface;
import io.spine.server.entity.storage.given.ColumnsTestEnv.EntityWithManyGetters;
import io.spine.server.entity.storage.given.ColumnsTestEnv.EntityWithManyGettersDescendant;
Expand Down Expand Up @@ -114,6 +115,14 @@ void fromImplementedInterface() {
}
}

@Test
@DisplayName("not confuse a setter method with a property mutator")
void testSetterDeclaringEntity() {
ColumnReader columnReader = forClass(EntityWithASetterButNoGetter.class);
Collection<EntityColumn> entityColumns = columnReader.readColumns();
assertThat(entityColumns).isEmpty();
}

@Nested
@DisplayName("when extracting columns, ignore")
class Ignore {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,28 @@ public int getValue() {
}
}

/**
* An entity type which declares a {@linkplain #setSecretNumber(Integer) mutator method},
* however doesn't declare a respective accessor method.
*
* <p>{@code ColumnReader} should not get confused and assume that the mutator method is
* a property, and, therefore, a potential column.
*/
@SuppressWarnings("unused") // Reflective access
public static class EntityWithASetterButNoGetter extends AbstractEntity<String, Any> {

private Integer secretNumber;

protected EntityWithASetterButNoGetter(String id) {
super(id);
}

@SuppressWarnings("WeakerAccess") // Required for a test
public void setSecretNumber(Integer secretNumber) {
this.secretNumber = secretNumber;
}
}

@SuppressWarnings("unused") // Reflective access
public static class EntityWithManyGetters extends AbstractEntity<String, Any> {

Expand Down