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

Removed JodaTime dependency - migrated to java.time. #133

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Removed JodaTime dependency - migrated to java.time. #133

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3940a8c
Removed JodaTime dependency - migrated to java.time.
Jonarzz Feb 29, 2020
937e2da
Merge branch 'develop' into java_time_instead_of_joda
Apr 27, 2020
59ce930
Fixes after code review - ZonedDateTime used instead of Temporal (wou…
Apr 27, 2020
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
13 changes: 0 additions & 13 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<jackson.databind.version>2.9.10.1</jackson.databind.version>
<jackson.datatype.version>2.9.9</jackson.datatype.version>
<jodatime.version>2.9.7</jodatime.version>
<lombok.version>1.18.10</lombok.version>
<testng.version>6.11</testng.version>
<logback.version>1.2.1</logback.version>
Expand All @@ -30,24 +29,12 @@

<dependencies>

<dependency>
<groupId>joda-time</groupId>
<artifactId>joda-time</artifactId>
<version>${jodatime.version}</version>
</dependency>

<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>${jackson.databind.version}</version>
</dependency>

<dependency>
<groupId>com.fasterxml.jackson.datatype</groupId>
<artifactId>jackson-datatype-joda</artifactId>
<version>${jackson.datatype.version}</version>
</dependency>

<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
Expand Down
13 changes: 7 additions & 6 deletions src/main/java/in/zapr/druid/druidry/granularity/DurationGranularity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@

package in.zapr.druid.druidry.granularity;

import static java.time.format.DateTimeFormatter.ISO_DATE_TIME;

import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;

import org.joda.time.DateTime;

import lombok.EqualsAndHashCode;
import lombok.Getter;

import java.time.temporal.Temporal;

@Getter
@JsonInclude(JsonInclude.Include.NON_NULL)
@EqualsAndHashCode(callSuper = true)
Expand All @@ -34,19 +35,19 @@ public class DurationGranularity extends Granularity {
private final String type;
@JsonProperty("duration")
private long durationInMilliSeconds;
private DateTime origin;
private Temporal origin;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we keep origin as Temporal and user set it to Year, Instant, etc then wouldn't getOrigin fail with UnsupportedTemporalTypeException ? Shouldn't we keep origin as ZonedDateTime ?


public DurationGranularity(long durationInMilliSeconds) {
this(durationInMilliSeconds, null);
}

public DurationGranularity(long durationInMilliSeconds, DateTime origin) {
public DurationGranularity(long durationInMilliSeconds, Temporal origin) {
this.type = DURATION_GRANULARITY_TYPE;
this.durationInMilliSeconds = durationInMilliSeconds;
this.origin = origin;
}

public String getOrigin() {
return origin == null ? null : origin.toDateTimeISO().toString();
return origin == null ? null : ISO_DATE_TIME.format(origin);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use ISO_OFFSET_DATE_TIME formatter instead of ISO_DATE_TIME, as ISO_DATE_TIME formatter includes ZoneId along with offset.
Eg : Consider origin with ZoneId Asia/Kolkata
Then ISO_DATE_TIME.format(origin) will give you 2020-07-12T22:32:11.907+05:30[Asia/Kolkata]
vs origin.toDateTimeISO().toString() will give you 2020-07-12T22:32:11.907+05:30.
I think using ISO_DATE_TIME will break current contract.

}
}
18 changes: 10 additions & 8 deletions src/main/java/in/zapr/druid/druidry/granularity/PeriodGranularity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,22 @@

package in.zapr.druid.druidry.granularity;

import com.fasterxml.jackson.annotation.JsonInclude;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import static java.time.format.DateTimeFormatter.ISO_DATE_TIME;

import com.fasterxml.jackson.annotation.JsonInclude;
import lombok.Builder;
import lombok.EqualsAndHashCode;
import lombok.Getter;

import java.time.ZoneId;
import java.time.ZonedDateTime;

@Builder
@Getter
@JsonInclude(JsonInclude.Include.NON_NULL)
@EqualsAndHashCode(callSuper = true)
public class PeriodGranularity extends Granularity {

private static final String PERIOD_GRANULARITY_TYPE = "period";
private static final String DEFAULT_TIMEZONE = "UTC";

Expand All @@ -40,14 +42,14 @@ public class PeriodGranularity extends Granularity {
private String period;

// TODO: Check if it is alright
private DateTimeZone timeZone;
private DateTime origin;
private ZoneId timeZone;
private ZonedDateTime origin;

public String getOrigin() {
return origin == null ? null : origin.toDateTimeISO().toString();
return origin == null ? null : ISO_DATE_TIME.format(origin);
}

public String getTimeZone() {
return timeZone == null ? DEFAULT_TIMEZONE : timeZone.getID();
return timeZone == null ? DEFAULT_TIMEZONE : timeZone.getId();
}
}
16 changes: 9 additions & 7 deletions src/main/java/in/zapr/druid/druidry/query/config/Interval.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,31 @@
package in.zapr.druid.druidry.query.config;

import com.fasterxml.jackson.annotation.JsonValue;

import org.joda.time.DateTime;

import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NonNull;

import java.time.format.DateTimeFormatter;
import java.time.temporal.TemporalAccessor;

@Getter
@EqualsAndHashCode
public class Interval {

private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'");

private final static String DRUID_INTERVAL_FORMAT = "%s/%s";

private DateTime startTime;
private DateTime endTime;
private TemporalAccessor startTime;
private TemporalAccessor endTime;
Copy link
Contributor

Choose a reason for hiding this comment

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

similar consideration here for startTime and endTime as for origin in DurationGranularity


public Interval(@NonNull DateTime startTime, @NonNull DateTime endTime) {
public Interval(@NonNull TemporalAccessor startTime, @NonNull TemporalAccessor endTime) {
this.startTime = startTime;
this.endTime = endTime;
}

@JsonValue
private String getIntervalAsString() {
return String.format(DRUID_INTERVAL_FORMAT, startTime.toDateTimeISO(), endTime.toDateTimeISO());
return String.format(DRUID_INTERVAL_FORMAT, FORMATTER.format(startTime), FORMATTER.format(endTime));
Copy link
Contributor

Choose a reason for hiding this comment

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

Here also there will be contract issue and library might be giving wrong interval/s than what user intended as we are not including offset which was getting included before with toDateTimeISO().

Eg : Now getIntervalAsString() would be returning 2020-07-12T22:55:15.714Z/2020-07-12T22:55:15.714Z vs previous value 2020-07-12T22:55:15.714+05:30/2020-07-12T22:55:15.714+05:30 with offsets.

}
}
27 changes: 14 additions & 13 deletions src/test/java/in/zapr/druid/druidry/dataSource/QueryDataSourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import in.zapr.druid.druidry.dimension.DruidDimension;
import in.zapr.druid.druidry.dimension.SimpleDimension;
import in.zapr.druid.druidry.granularity.Granularity;
import in.zapr.druid.druidry.granularity.PredefinedGranularity;
import in.zapr.druid.druidry.granularity.SimpleGranularity;
import in.zapr.druid.druidry.query.aggregation.DruidGroupByQuery;
import in.zapr.druid.druidry.query.config.Interval;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand All @@ -29,17 +33,12 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.Temporal;
import java.util.Arrays;
import java.util.Collections;

import in.zapr.druid.druidry.dimension.DruidDimension;
import in.zapr.druid.druidry.dimension.SimpleDimension;
import in.zapr.druid.druidry.granularity.Granularity;
import in.zapr.druid.druidry.granularity.PredefinedGranularity;
import in.zapr.druid.druidry.granularity.SimpleGranularity;
import in.zapr.druid.druidry.query.aggregation.DruidGroupByQuery;
import in.zapr.druid.druidry.query.config.Interval;

public class QueryDataSourceTest {
private static ObjectMapper objectMapper;

Expand All @@ -55,8 +54,10 @@ public void testQueryDataSource() throws JsonProcessingException, JSONException

Granularity granularity = new SimpleGranularity(PredefinedGranularity.ALL);
// Interval
DateTime startTime = new DateTime(2012, 1, 1, 0, 0, 0, DateTimeZone.UTC);
DateTime endTime = new DateTime(2012, 1, 3, 0, 0, 0, DateTimeZone.UTC);
Temporal startTime = ZonedDateTime.of(2012, 1, 1,
0, 0, 0, 0, ZoneOffset.UTC);
Temporal endTime = ZonedDateTime.of(2012, 1, 3,
0, 0, 0, 0, ZoneOffset.UTC);
Interval interval = new Interval(startTime, endTime);

DruidGroupByQuery druidGroupByQuery = DruidGroupByQuery.builder()
Expand Down
17 changes: 9 additions & 8 deletions ...test/java/in/zapr/druid/druidry/extractionFunctions/TimeFormatExtractionFunctionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,22 @@

package in.zapr.druid.druidry.extractionFunctions;

import static java.time.format.DateTimeFormatter.ISO_DATE_TIME;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import in.zapr.druid.druidry.granularity.DurationGranularity;
import org.json.JSONException;
import org.skyscreamer.jsonassert.JSONAssert;
import org.skyscreamer.jsonassert.JSONCompareMode;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.Temporal;
import java.util.Locale;

import in.zapr.druid.druidry.granularity.DurationGranularity;

public class TimeFormatExtractionFunctionTest {

private static ObjectMapper objectMapper;
Expand All @@ -48,7 +49,7 @@ public void testAllFields() throws JsonProcessingException, JSONException {

String timeZone = "America/Montreal";

DateTime originDate = new DateTime(DateTimeZone.UTC);
Temporal originDate = ZonedDateTime.now(ZoneOffset.UTC);

DurationGranularity spec = new DurationGranularity(7200000, originDate);

Expand All @@ -63,8 +64,8 @@ public void testAllFields() throws JsonProcessingException, JSONException {
String actualJSON = objectMapper.writeValueAsString(timeFormatExtractonFunction);

String expectedJSONString = "{\n\"type\" : \"timeFormat\",\n \"format\" : \"dd-MM-yyyy\",\n \"timeZone\" : \"America/Montreal\",\n \"locale\" : \"fr\",\n \"granularity\": {\"type\": \"duration\", \"duration\": 7200000, \"origin\": \"" +
originDate.toDateTimeISO() +
"\"},\n \"asMillis\": true\n\n }";
ISO_DATE_TIME.format(originDate) +
"\"},\n \"asMillis\": true\n\n }";

JSONAssert.assertEquals(expectedJSONString, actualJSON, JSONCompareMode.NON_EXTENSIBLE);
}
Expand Down
25 changes: 12 additions & 13 deletions src/test/java/in/zapr/druid/druidry/filter/IntervalFilterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import in.zapr.druid.druidry.query.config.Interval;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand All @@ -29,11 +27,12 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.Temporal;
import java.util.ArrayList;
import java.util.Arrays;

import in.zapr.druid.druidry.query.config.Interval;

public class IntervalFilterTest {

private static ObjectMapper objectMapper;
Expand Down Expand Up @@ -67,15 +66,15 @@ public void testFields() throws JsonProcessingException, JSONException {
jsonObject.put("dimension", "__time");
jsonObject.put("intervals", intervalJsonArray);

DateTime startTimeInterval1 = new DateTime(2013, 8, 31,
0, 0, 0, DateTimeZone.UTC);
DateTime endTimeInterval1 = new DateTime(2013, 9, 3,
0, 0, 0, DateTimeZone.UTC);
Temporal startTimeInterval1 = ZonedDateTime.of(2013, 8, 31,
0, 0, 0, 0, ZoneOffset.UTC);
Temporal endTimeInterval1 = ZonedDateTime.of(2013, 9, 3,
0, 0, 0, 0, ZoneOffset.UTC);

DateTime startTimeInterval2 = new DateTime(2018, 8, 31,
0, 0, 0, DateTimeZone.UTC);
DateTime endTimeInterval2 = new DateTime(2018, 9, 3,
0, 0, 0, DateTimeZone.UTC);
Temporal startTimeInterval2 = ZonedDateTime.of(2018, 8, 31,
0, 0, 0, 0, ZoneOffset.UTC);
Temporal endTimeInterval2 = ZonedDateTime.of(2018, 9, 3,
0, 0, 0, 0, ZoneOffset.UTC);

Interval interval1 = new Interval(startTimeInterval1, endTimeInterval1);
Interval interval2 = new Interval(startTimeInterval2, endTimeInterval2);
Expand Down
15 changes: 9 additions & 6 deletions src/test/java/in/zapr/druid/druidry/granularity/DurationGranularityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@

package in.zapr.druid.druidry.granularity;

import static java.time.format.DateTimeFormatter.ISO_DATE_TIME;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.json.JSONException;
import org.json.JSONObject;
import org.skyscreamer.jsonassert.JSONAssert;
Expand All @@ -29,6 +28,10 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.temporal.Temporal;

public class DurationGranularityTest {

private static ObjectMapper objectMapper;
Expand All @@ -41,12 +44,12 @@ public void init() {
@Test
public void testAllFields() throws JSONException, JsonProcessingException {

DateTime originDate = new DateTime(DateTimeZone.UTC);
Temporal originDate = ZonedDateTime.now(ZoneOffset.UTC);
DurationGranularity granularity = new DurationGranularity(3141, originDate);
JSONObject jsonObject = new JSONObject();
jsonObject.put("type", "duration");
jsonObject.put("duration", 3141L);
jsonObject.put("origin", originDate);
jsonObject.put("origin", ISO_DATE_TIME.format(originDate));

String actualJSON = objectMapper.writeValueAsString(granularity);
String expectedJSON = jsonObject.toString();
Expand Down Expand Up @@ -85,7 +88,7 @@ public void testEqualsNegative() {
public void testEqualsWithAnotherSubClass() {
SimpleGranularity granularity1 = new SimpleGranularity(PredefinedGranularity.ALL);

DateTime originDate = new DateTime(DateTimeZone.UTC);
Temporal originDate = ZonedDateTime.now(ZoneOffset.UTC);
DurationGranularity granularity2 = new DurationGranularity(3141, originDate);

Assert.assertNotEquals(granularity1, granularity2);
Expand Down
Loading