-
Notifications
You must be signed in to change notification settings - Fork 89
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
base: develop
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
|
@@ -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; | ||
|
@@ -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)); | ||
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. 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 Eg : Now |
||
} | ||
} |
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.
I think we should use
ISO_OFFSET_DATE_TIME
formatter instead ofISO_DATE_TIME
, asISO_DATE_TIME
formatter includes ZoneId along with offset.Eg : Consider
origin
with ZoneIdAsia/Kolkata
Then
ISO_DATE_TIME.format(origin)
will give you2020-07-12T22:32:11.907+05:30[Asia/Kolkata]
vs
origin.toDateTimeISO().toString()
will give you2020-07-12T22:32:11.907+05:30
.I think using
ISO_DATE_TIME
will break current contract.