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.

Sign up for GitHub

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

Jump to bottom

Fix coerce to date #308

Merged
merged 1 commit into from
Aug 17, 2016
Merged

Fix coerce to date #308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions elide-core/src/main/java/com/yahoo/elide/utils/coerce/CoerceUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@

import com.yahoo.elide.core.exceptions.InvalidAttributeException;
import com.yahoo.elide.core.exceptions.InvalidValueException;
import com.yahoo.elide.utils.coerce.converters.EpochToDateConverter;
import com.yahoo.elide.utils.coerce.converters.FromMapConverter;
import com.yahoo.elide.utils.coerce.converters.ToEnumConverter;
import org.apache.commons.beanutils.BeanUtilsBean;
import org.apache.commons.beanutils.ConversionException;
import org.apache.commons.beanutils.ConvertUtils;
import org.apache.commons.beanutils.ConvertUtilsBean;
import org.apache.commons.beanutils.Converter;
import org.apache.commons.lang3.ClassUtils;

import java.util.Date;
import java.util.Map;

/**
Expand All @@ -24,6 +27,7 @@ public class CoerceUtil {

private static final ToEnumConverter TO_ENUM_CONVERTER = new ToEnumConverter();
private static final FromMapConverter FROM_MAP_CONVERTER = new FromMapConverter();
private static final EpochToDateConverter EPOCH_TO_DATE_CONVERTER = new EpochToDateConverter();

//static block for setup and registering new converters
static {
Expand Down Expand Up @@ -70,6 +74,9 @@ public Converter lookup(Class<?> sourceType, Class<?> targetType) {
return TO_ENUM_CONVERTER;
} else if (Map.class.isAssignableFrom(sourceType)) {
return FROM_MAP_CONVERTER;
} else if ((String.class.isAssignableFrom(sourceType) || Long.class.isAssignableFrom(sourceType))
Copy link
Contributor

@Koka Koka Sep 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@xiaoyao1991 @DeathByTape I've found a bug here. If you try to create entity with date field and set value to 1970-01-01 (timestamp is -14400000 in my time zone) it will be automactically converted into Integer object, not Long inside elide because the value is too small and it seems like the smallest allowed number type is selected on conversion. Then it will miss this if branch and wrong date converter will be chosen.

&& ClassUtils.isAssignable(targetType, Date.class)) {
return EPOCH_TO_DATE_CONVERTER;
} else {
return super.lookup(sourceType, targetType);
}
Expand Down
55 changes: 55 additions & 0 deletions elide-core/src/main/java/com/yahoo/elide/utils/coerce/converters/EpochToDateConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2016, Yahoo Inc.
* Licensed under the Apache License, Version 2.0
* See LICENSE file in project root for terms.
*/
package com.yahoo.elide.utils.coerce.converters;

import com.yahoo.elide.core.exceptions.InvalidAttributeException;

import org.apache.commons.beanutils.Converter;
import org.apache.commons.lang3.ClassUtils;

import java.sql.Time;
import java.sql.Timestamp;
import java.util.Date;

/**
* Convert epoch(in string or long) to Date
*/
public class EpochToDateConverter implements Converter {

@Override
public <T> T convert(Class<T> cls, Object value) {
try {
if (ClassUtils.isAssignable(value.getClass(), String.class)) {
return stringToDate(cls, (String) value);
} else if (ClassUtils.isAssignable(value.getClass(), Long.class, true)) {
return longToDate(cls, (Long) value);
} else {
throw new UnsupportedOperationException(value.getClass().getSimpleName() + " is not a valid epoch");
}
} catch (IndexOutOfBoundsException | ReflectiveOperationException
| UnsupportedOperationException | IllegalArgumentException e) {
throw new InvalidAttributeException("Unknown " + cls.getSimpleName() + " value " + value, e);
}
}

private static <T> T longToDate(Class<T> cls, Long epoch) throws ReflectiveOperationException {
if (ClassUtils.isAssignable(cls, java.sql.Date.class)) {
return (T) new java.sql.Date(epoch);
} else if (ClassUtils.isAssignable(cls, Timestamp.class)) {
return (T) new Timestamp(epoch);
} else if (ClassUtils.isAssignable(cls, Time.class)) {
return (T) new Time(epoch);
} else if (ClassUtils.isAssignable(cls, Date.class)) {
return (T) new Date(epoch);
} else {
throw new UnsupportedOperationException("Cannot convert to " + cls.getSimpleName());
}
}

private static <T> T stringToDate(Class<T> cls, String epoch) throws ReflectiveOperationException {
return longToDate(cls, Long.parseLong(epoch));
}
}
36 changes: 28 additions & 8 deletions elide-core/src/test/java/com/yahoo/elide/utils/coerce/CoerceUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,23 @@
*/
package com.yahoo.elide.utils.coerce;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;

import com.yahoo.elide.core.exceptions.InvalidValueException;

import org.testng.annotations.Test;

import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
import lombok.NoArgsConstructor;
import org.testng.annotations.Test;

import java.sql.Time;
import java.sql.Timestamp;
import java.util.Date;
import java.util.LinkedHashMap;
import java.util.Map;

import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;

public class CoerceUtilTest {

public enum Seasons { WINTER, SPRING }
Expand All @@ -39,7 +44,7 @@ public void testNoConversions() throws Exception {
"coerce returns value if value is null");

assertEquals((Object) CoerceUtil.coerce(1, int.class), 1,
"coerce returns value if value is assignable to target");
"coerce returns value if value is assignable to target");
}

@Test
Expand All @@ -53,13 +58,13 @@ public void testToEnumConversion() throws Exception {
public void testBasicConversion() throws Exception {

assertEquals(CoerceUtil.coerce(1, String.class), "1",
"coerce converts int to String");
"coerce converts int to String");

assertEquals(CoerceUtil.coerce("1", Long.class), Long.valueOf("1"),
"coerce converts String to Long");
"coerce converts String to Long");

assertEquals((Object) CoerceUtil.coerce(1.0, int.class), 1,
"coerce converts float to int");
"coerce converts float to int");
}

@Test(expectedExceptions = InvalidValueException.class)
Expand Down Expand Up @@ -103,4 +108,19 @@ public void testConversionFailure() throws Exception {
public void testNullConversion() throws Exception {
assertNull(CoerceUtil.coerce(null, String.class));
}

@Test
public void testStringToDate() throws Exception {
Date date = CoerceUtil.coerce("1", Date.class);
assertEquals(date, new Date(1));

java.sql.Date date1 = CoerceUtil.coerce("1", java.sql.Date.class);
assertEquals(date1, new java.sql.Date(1));

Timestamp timestamp = CoerceUtil.coerce("1", Timestamp.class);
assertEquals(timestamp, new Timestamp(1));

Time time = CoerceUtil.coerce("1", Time.class);
assertEquals(time, new Time(1));
}
}