-
Notifications
You must be signed in to change notification settings - Fork 230
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
ISO Support for Time Grains #1739
Changes from 10 commits
1ecbbb4
89da649
7ee3969
dfa4dd8
5529c7b
3412eb1
d2c7d90
348fea2
abeae82
cc3ccc1
ae3658f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
/* | ||
* Copyright 2020, Yahoo Inc. | ||
* Licensed under the Apache License, Version 2.0 | ||
* See LICENSE file in project root for terms. | ||
*/ | ||
package com.yahoo.elide.datastores.aggregation.timegrains; | ||
|
||
import java.sql.Timestamp; | ||
import java.text.ParseException; | ||
import java.text.SimpleDateFormat; | ||
|
||
/** Interface for ISO timegrain Support. */ | ||
public interface TimeGrainFormatter { | ||
|
||
String ISO_FORMAT = "yyyy-MM-dd'T'HH:mm:ss'Z'"; | ||
SimpleDateFormat ISO_FORMATTER = new SimpleDateFormat(ISO_FORMAT); | ||
|
||
static Timestamp formatDateString(SimpleDateFormat formatter, String val) throws ParseException { | ||
try { | ||
return new Timestamp(formatter.parse((String) val).getTime()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't need to cast val here as a String. |
||
} catch (ParseException pe) { | ||
return new Timestamp(ISO_FORMATTER.parse((String) val).getTime()); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
||
import com.yahoo.elide.core.utils.coerce.converters.Serde; | ||
import com.yahoo.elide.datastores.aggregation.timegrains.Day; | ||
import org.junit.jupiter.api.Test; | ||
|
@@ -17,6 +18,7 @@ | |
|
||
public class DaySerdeTest { | ||
private SimpleDateFormat formatter = new SimpleDateFormat("yyyy-MM-dd"); | ||
private SimpleDateFormat isoFormatter = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ"); | ||
|
||
@Test | ||
public void testDateSerialize() throws ParseException { | ||
|
@@ -59,4 +61,14 @@ public void testDeserializeDateInvalidFormat() throws ParseException { | |
serde.deserialize(dateInString); | ||
}); | ||
} | ||
|
||
@Test | ||
public void testISODateString() throws ParseException { | ||
String dateInString = "2020-01-01T00:00:00-0500"; | ||
Day expectedDate = new Day(formatter.parse(dateInString)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be isoFormatter instead of formatter. Also might be a good idea to update dateInString to have different hour minutes. |
||
String actual = "2020-01-01"; | ||
Serde serde = new Day.DaySerde(); | ||
Object actualDate = serde.deserialize(actual); | ||
assertEquals(expectedDate, actualDate); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look correct - why do we need to parse and then format, and then parse again? It looks like we are doing unnecessary work here.