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

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

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
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
13 changes: 0 additions & 13 deletions pom.xml
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.4</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
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;

import org.joda.time.DateTime;
import java.time.ZonedDateTime;

import lombok.EqualsAndHashCode;
import lombok.Getter;

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

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

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

public DurationGranularity(long durationInMilliSeconds, DateTime origin) {
public DurationGranularity(long durationInMilliSeconds, ZonedDateTime 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.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,21 @@

import com.fasterxml.jackson.annotation.JsonInclude;

import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import java.time.ZoneId;
import java.time.ZonedDateTime;

import lombok.Builder;
import lombok.EqualsAndHashCode;
import lombok.Getter;

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

@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 +43,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();
}
}
13 changes: 8 additions & 5 deletions src/main/java/in/zapr/druid/druidry/query/config/Interval.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@

import com.fasterxml.jackson.annotation.JsonValue;

import org.joda.time.DateTime;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;

import lombok.EqualsAndHashCode;
import lombok.Getter;
Expand All @@ -28,18 +29,20 @@
@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 ZonedDateTime startTime;
private ZonedDateTime endTime;

public Interval(@NonNull DateTime startTime, @NonNull DateTime endTime) {
public Interval(@NonNull ZonedDateTime startTime, @NonNull ZonedDateTime 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.

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
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.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand All @@ -29,6 +27,8 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

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

Expand All @@ -55,8 +55,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);
ZonedDateTime startTime = ZonedDateTime.of(2012, 1, 1,
0, 0, 0, 0, ZoneOffset.UTC);
ZonedDateTime endTime = ZonedDateTime.of(2012, 1, 3,
0, 0, 0, 0, ZoneOffset.UTC);
Interval interval = new Interval(startTime, endTime);

DruidGroupByQuery druidGroupByQuery = DruidGroupByQuery.builder()
Expand Down
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);
ZonedDateTime 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
20 changes: 10 additions & 10 deletions src/test/java/in/zapr/druid/druidry/filter/IntervalFilterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
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.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand All @@ -29,6 +27,8 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

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

Expand Down Expand Up @@ -67,15 +67,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);
ZonedDateTime startTimeInterval1 = ZonedDateTime.of(2013, 8, 31,
0, 0, 0, 0, ZoneOffset.UTC);
ZonedDateTime 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);
ZonedDateTime startTimeInterval2 = ZonedDateTime.of(2018, 8, 31,
0, 0, 0, 0, ZoneOffset.UTC);
ZonedDateTime 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
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
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 +27,11 @@
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import java.time.ZoneOffset;
import java.time.ZonedDateTime;

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

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);
ZonedDateTime 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);
ZonedDateTime originDate = ZonedDateTime.now(ZoneOffset.UTC);
DurationGranularity granularity2 = new DurationGranularity(3141, originDate);

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