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

ENG-5818vPersistOnly added optionals #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
35 changes: 33 additions & 2 deletions src/main/net/sf/persist/AnnotationTableMapping.java
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public AnnotationTableMapping(final Class<?> objectClass) {
if (annotation != null && annotation.autoGenerated()) {
autoGeneratedColumnsTemp.add(columnName);
}

}

if (primaryKeysList.isEmpty()) {
Expand All @@ -115,6 +116,8 @@ public AnnotationTableMapping(final Class<?> objectClass) {
notAutoGeneratedColumns = toArray(notAutoGeneratedColumnsList);
autoGeneratedColumns = toArray(autoGeneratedColumnsList);

// TODO: implement optional

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment


// assemble sql blocks to be used by crud sql statements

final String allColumns = join(columns, "", ",");
Expand Down Expand Up @@ -197,13 +200,27 @@ public Map<String, Method> getSettersMap() {
@Override
public Method getGetterForColumn(final String columnName) {
final String fieldName = columnsMap.get(columnName.toLowerCase(Locale.ENGLISH));
return gettersMap.get(fieldName);
if (fieldName == null) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do the if check log + catch a guaranteed NPE if the if check is true?
Wrap fieldName in Optional<>, do an "if present" check and throw an exception if it isn't. otherwise, return as normal. (e.g. no log needed, no try catch)

ENGINE_LOG.warn("Could not find field name corresponding to column name [" + columnName + "]");
}
try {
return gettersMap.get(fieldName);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed what you were trying to do here, wrap in an optional and throw a different persist exception for the inner map.
love the foresight for error cases!

} catch (NullPointerException e) {
throw new PersistException("Could not find getter for columnn with field name [" + fieldName + "]");
}
}

@Override
public Method getSetterForColumn(final String columnName) {
final String fieldName = columnsMap.get(columnName.toLowerCase(Locale.ENGLISH));
return settersMap.get(fieldName);
if (fieldName == null) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

ENGINE_LOG.warn("Could not find field name corresponding to column name [" + columnName + "]");
}
try {
return settersMap.get(fieldName);
} catch (NullPointerException e) {
throw new PersistException("Could not find setter for columnn with field name [" + fieldName + "]");
}
}

public String getSelectSql() {
Expand All @@ -226,6 +243,20 @@ public String getDeleteSql() {
return deleteSql;
}

@Override
public Class<?> getOptionalSubType(final String columnName) {
final String fieldName = columnsMap.get(columnName.toLowerCase(Locale.ENGLISH));
if (fieldName == null) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

ENGINE_LOG.warn("Could not find field name corresponding to column name [" + columnName + "]");
}
try {
return annotationsMap.get(fieldName).optionalSubType();
} catch (NullPointerException e) {
throw new PersistException(
"Could not find optional subtype for columnn with field name [" + fieldName + "]");
}
}

// ---------- helpers ----------

private static String[] toArray(final List<String> list) {
Expand Down
11 changes: 7 additions & 4 deletions src/main/net/sf/persist/Mapping.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ public abstract class Mapping {

public abstract Method getSetterForColumn(String columnName);

public abstract Class<?> getOptionalSubType(String columnName);

// ---------- utility methods ----------

/**
Expand Down Expand Up @@ -49,7 +51,7 @@ protected static Map[] getFieldsMaps(final Class<?> objectClass) {

// create map with all getters and setters

final Map<String, Method[]> allMethods = new HashMap<String, Method[]>();
final Map<String, Method[]> allMethods = new HashMap<>();
for (Method method : methods) {
final String name = method.getName();

Expand Down Expand Up @@ -86,9 +88,10 @@ protected static Map[] getFieldsMaps(final Class<?> objectClass) {
// a field is only taken into consideration if it has a getter and a
// setter

final Map<String, net.sf.persist.annotations.Column> annotationsMap = new HashMap<String, net.sf.persist.annotations.Column>();
final Map<String, Method> gettersMap = new HashMap<String, Method>();
final Map<String, Method> settersMap = new HashMap<String, Method>();
final Map<String, net.sf.persist.annotations.Column> annotationsMap = new HashMap<>();
final Map<String, Method> gettersMap = new HashMap<>();
final Map<String, Method> settersMap = new HashMap<>();
final Map<String, Class<?>> optionalSubTypeMap = new HashMap<>();

for (String suffix : allMethods.keySet()) {

Expand Down
27 changes: 24 additions & 3 deletions src/main/net/sf/persist/NoTableMapping.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ public class NoTableMapping extends Mapping {
// map possible column names to field names
private final Map<String, String> columnsMap;

private final Map<String, net.sf.persist.annotations.Column> annotationsMap;

public NoTableMapping(Class objectClass, NameGuesser nameGuesser) {

checkAnnotation(objectClass);
Expand All @@ -37,7 +39,7 @@ public NoTableMapping(Class objectClass, NameGuesser nameGuesser) {

// get the list of annotations, getters and setters
Map[] fieldsMaps = Mapping.getFieldsMaps(objectClass);
final Map<String, net.sf.persist.annotations.Column> annotationsMap = fieldsMaps[0];
annotationsMap = fieldsMaps[0];
gettersMap = fieldsMaps[1];
settersMap = fieldsMaps[2];

Expand Down Expand Up @@ -114,7 +116,11 @@ public String getFieldNameForColumn(String columnName) {
@Override
public Method getSetterForColumn(String columnName) {
String fieldName = getFieldNameForColumn(columnName);
return settersMap.get(fieldName);
try {
return settersMap.get(fieldName);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

} catch (NullPointerException e) {
throw new PersistException("Could not find setter for columnn with field name [" + fieldName + "]");
}
}

/**
Expand All @@ -126,7 +132,22 @@ public Method getSetterForColumn(String columnName) {
@Override
public Method getGetterForColumn(String columnName) {
String fieldName = getFieldNameForColumn(columnName);
return gettersMap.get(fieldName);
try {
return gettersMap.get(fieldName);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

} catch (NullPointerException e) {
throw new PersistException("Could not find getter for columnn with field name [" + fieldName + "]");
}
}

@Override
public Class<?> getOptionalSubType(String columnName) {
String fieldName = getFieldNameForColumn(columnName);
try {
return annotationsMap.get(fieldName).optionalSubType();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

} catch (NullPointerException e) {
throw new PersistException(
"Could not find optional subtype for columnn with field name [" + fieldName + "]");
}
}

/**
Expand Down
73 changes: 54 additions & 19 deletions src/main/net/sf/persist/Persist.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

Expand Down Expand Up @@ -68,8 +69,8 @@ public final class Persist {
private PreparedStatement lastPreparedStatement = null;
private boolean closePreparedStatementsAfterRead = true;

private static ConcurrentMap<String, ConcurrentMap<Class, Mapping>> mappingCaches = new ConcurrentHashMap<String, ConcurrentMap<Class, Mapping>>();
private static ConcurrentMap<String, NameGuesser> nameGuessers = new ConcurrentHashMap<String, NameGuesser>();
private static ConcurrentMap<String, ConcurrentMap<Class, Mapping>> mappingCaches = new ConcurrentHashMap<>();
private static ConcurrentMap<String, NameGuesser> nameGuessers = new ConcurrentHashMap<>();

private static final String DEFAULT_CACHE = "default cache";

Expand Down Expand Up @@ -142,7 +143,7 @@ public Persist(String cacheName, Connection connection, ObjectFactory objectFact
}

public static void flushMappings() {
mappingCaches = new ConcurrentHashMap<String, ConcurrentMap<Class, Mapping>>();
mappingCaches = new ConcurrentHashMap<>();
}

// ---------- name guesser ----------
Expand Down Expand Up @@ -211,7 +212,7 @@ public Mapping getMapping(final Class objectClass) {
cacheName = DEFAULT_CACHE;
}

ConcurrentMap<Class, Mapping> mappingCache = new ConcurrentHashMap<Class, Mapping>();
ConcurrentMap<Class, Mapping> mappingCache = new ConcurrentHashMap<>();
ConcurrentMap<Class, Mapping> previousMappingCache = mappingCaches.putIfAbsent(cacheName, mappingCache);
if (previousMappingCache != null) {
mappingCache = previousMappingCache;
Expand Down Expand Up @@ -487,7 +488,12 @@ public static void setParameters(final PreparedStatement stmt, final Object[] pa

for (int i = 1; i <= parameters.length; i++) {

final Object parameter = parameters[i - 1];
Object parameter = parameters[i - 1];

if (parameter != null && parameter.getClass() == Optional.class) {
Optional<?> instanceOfParameter = (Optional<?>) parameter;
parameter = instanceOfParameter.orElse(null);
}

if (parameter == null) {

Expand Down Expand Up @@ -712,7 +718,7 @@ private static boolean isNativeType(final Class type) {
* for a null value from the {@link java.sql.ResultSet}.
*
* @param resultSet {@link java.sql.ResultSet} (positioned in the row to be
* processed)
* processed)
* @param column column index in the result set (starting with 1)
* @param type {@link java.lang.Class} of the object to be returned
*
Expand All @@ -725,7 +731,6 @@ public static <T> T getValueFromResultSet(final ResultSet resultSet, final int c
Object value;

try {

if (type == boolean.class) {
value = resultSet.getBoolean(column);
} else if (type == Boolean.class) {
Expand Down Expand Up @@ -817,6 +822,15 @@ public static <T> T getValueFromResultSet(final ResultSet resultSet, final int c
return (T)value;
}

public static Optional<?> getValueFromResultSetOptional(final ResultSet resultSet, final int column,
final Class<?> optionalSubType) {
if (getValueFromResultSet(resultSet, column, optionalSubType) == null) {
return Optional.empty();
} else {
return Optional.of(getValueFromResultSet(resultSet, column, optionalSubType));
}
}

/**
* Reads a column from the current row in the provided
* {@link java.sql.ResultSet} and return a value correspondent to the SQL
Expand Down Expand Up @@ -1031,8 +1045,8 @@ public <T> T loadObject(final Class<T> objectClass, final ResultSet resultSet) t
}

// for beans
// TODO optional

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment?

else {

final Mapping mapping = getMapping(objectClass);

try {
Expand All @@ -1046,15 +1060,34 @@ public <T> T loadObject(final Class<T> objectClass, final ResultSet resultSet) t
}

for (int i = 1; i <= resultSetMetaData.getColumnCount(); i++) {

final String columnName = resultSetMetaData.getColumnLabel(i).toLowerCase();
final Method setter = mapping.getSetterForColumn(columnName);

if (setter == null) {
PARAMETERS_LOG.warn("Column [" + columnName
+ "] from result set does not have a mapping to a field in [" + objectClass.getName() + "]");
} else {

final Class<?> type = setter.getParameterTypes()[0];
final Object value = getValueFromResultSet(resultSet, i, type);
final Class<?> optionalSubType = mapping.getOptionalSubType(columnName);

// figure out these exception texts

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remov e comment?

if ((type == Optional.class) && (optionalSubType == Void.class)) {
throw new PersistException(
"Column [" + columnName + "] was Optional but optionalSubType was Void.class");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's wrong with the void class?

} else if ((type != Optional.class) && (optionalSubType != Void.class)) {
PARAMETERS_LOG.error("Column type was not Optional but optionalSubType was not Void.class");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is if someone populates "optionalSubType" but doesn't use optional as their main... okay.

}

Object value;

if (type == Optional.class) {
value = getValueFromResultSetOptional(resultSet, i, optionalSubType);
} else {
value = getValueFromResultSet(resultSet, i, type);
}


try {
setter.invoke(ret, value);
Expand Down Expand Up @@ -1085,7 +1118,7 @@ public <T> T loadObject(final Class<T> objectClass, final ResultSet resultSet) t
*/
public static Map<String, Object> loadMap(final ResultSet resultSet) throws SQLException {

final Map<String, Object> ret = new LinkedHashMap<String, Object>();
final Map<String, Object> ret = new LinkedHashMap<>();
final ResultSetMetaData resultSetMetaData = resultSet.getMetaData();

for (int i = 1; i <= resultSetMetaData.getColumnCount(); i++) {
Expand Down Expand Up @@ -1391,7 +1424,7 @@ public <T> T read(final Class<T> objectClass, final String sql) {
*
* @since 1.0
*/
public <T> T read(final Class<T> objectClass, final String sql, final Object...parameters) {
public <T> T read(final Class<T> objectClass, final String sql, final Object... parameters) {
final PreparedStatement stmt = getPreparedStatement(sql);
return read(objectClass, stmt, parameters);
}
Expand All @@ -1409,7 +1442,7 @@ public <T> T read(final Class<T> objectClass, final String sql, final Object...p
*
* @since 1.0
*/
public <T> T read(final Class<T> objectClass, final PreparedStatement statement, final Object...parameters) {
public <T> T read(final Class<T> objectClass, final PreparedStatement statement, final Object... parameters) {
try {
setParameters(statement, parameters);
final ResultSet resultSet = statement.executeQuery();
Expand Down Expand Up @@ -1463,7 +1496,7 @@ public <T> T read(final Class<T> objectClass, final ResultSet resultSet) {
*
* @since 1.0
*/
public <T> T readByPrimaryKey(final Class<T> objectClass, final Object...primaryKeyValues) {
public <T> T readByPrimaryKey(final Class<T> objectClass, final Object... primaryKeyValues) {
final AnnotationTableMapping mapping = getTableMapping(objectClass, "readByPrimaryKey()");
final String sql = mapping.getSelectSql();
return read(objectClass, sql, primaryKeyValues);
Expand All @@ -1487,7 +1520,7 @@ public <T> List<T> readList(final Class<T> objectClass, final ResultSet resultSe
begin = System.currentTimeMillis();
}

final List<T> ret = new ArrayList<T>();
final List<T> ret = new ArrayList<>();
try {
while (resultSet.next()) {
ret.add(loadObject(objectClass, resultSet));
Expand Down Expand Up @@ -1540,7 +1573,7 @@ public <T> List<T> readList(final Class<T> objectClass, final PreparedStatement
*
* @since 1.0
*/
public <T> List<T> readList(final Class<T> objectClass, final String sql, final Object...parameters) {
public <T> List<T> readList(final Class<T> objectClass, final String sql, final Object... parameters) {
final PreparedStatement stmt = getPreparedStatement(sql);
try {
return readList(objectClass, stmt, parameters);
Expand Down Expand Up @@ -1588,14 +1621,15 @@ public <T> List<T> readList(final Class<T> objectClass) {
* <em>Passed ResultSet and PreparedStatement will be closed when ResultSetIterator is.</em>
* @since 1.0
*/
public <T> ResultSetIterator<T> readIterator(final Class<T> objectClass, final ResultSet resultSet, final PreparedStatement preparedStatement) {
public <T> ResultSetIterator<T> readIterator(final Class<T> objectClass, final ResultSet resultSet,
final PreparedStatement preparedStatement) {

long begin = 0;
if (PROFILING_LOG.isDebugEnabled()) {
begin = System.currentTimeMillis();
}

final ResultSetIterator<T> i = new ObjectResultSetIterator<T>(this, objectClass, resultSet, preparedStatement);
final ResultSetIterator<T> i = new ObjectResultSetIterator<>(this, objectClass, resultSet, preparedStatement);

if (PROFILING_LOG.isDebugEnabled()) {
final long end = System.currentTimeMillis();
Expand Down Expand Up @@ -1801,7 +1835,7 @@ public List<Map<String, Object>> readMapList(final ResultSet resultSet) {
begin = System.currentTimeMillis();
}

final List<Map<String, Object>> ret = new ArrayList<Map<String, Object>>();
final List<Map<String, Object>> ret = new ArrayList<>();
try {
while (resultSet.next()) {
ret.add(loadMap(resultSet));
Expand Down Expand Up @@ -2004,7 +2038,8 @@ static class ObjectResultSetIterator<T> implements ResultSetIterator<T> {
private final PreparedStatement preparedStatement;
private boolean hasNext = false;

ObjectResultSetIterator(final Persist persist, final Class<T> objectClass, final ResultSet resultSet, final PreparedStatement preparedStatement) {
ObjectResultSetIterator(final Persist persist, final Class<T> objectClass, final ResultSet resultSet,
final PreparedStatement preparedStatement) {
this.persist = persist;
this.objectClass = objectClass;
this.resultSet = resultSet;
Expand Down
6 changes: 6 additions & 0 deletions src/main/net/sf/persist/annotations/Column.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,10 @@
* and reading by primary key.
*/
boolean primaryKey() default false;

/**
* If the getter/setter use optional values, the subtype must be explicitly stated due to Java's generic
* subtype handling.
*/
Class<?> optionalSubType() default Void.class;
}