Modify

Opened 5 years ago

Closed 13 months ago

Last modified 6 months ago

#14176 closed enhancement (fixed)

Use Java 8 Date API (JSR 310): replace Date by Instant?

Reported by: Don-vip Owned by: simon04
Priority: normal Milestone: 21.05
Component: Core Version:
Keywords: date instant java8 Cc: simon04

Description

Follow-up from #13376:

Left to be done/discussed: replace 257 occurrences of Date by Instant, or leave it as it is :)

Given the number of deprecated methods in Date it's almost if the class is now deprecated itself. I'd say we can change everything that does not impact plugin API.

Compatibility will is a major concern since methods like AbstractPrimitive#getTimestamp, AbstractPrimitive#setTimestamp, Note#getCreatedAt, Note#getClosedAt all take/return Dates.

Attachments (1)

14176-notes.patch (16.4 KB) - added by simon04 14 months ago.

Download all attachments as: .zip

Change History (40)

comment:1 Changed 14 months ago by simon04

In 17653/josm:

see #14176 - DateUtils.parseInstant

Changed 14 months ago by simon04

Attachment: 14176-notes.patch added

comment:2 Changed 14 months ago by simon04

Changing the Note and NoteComment data objects might be a good starting point, since notes are not used by plugins -> attachment:14176-notes.patch

comment:3 Changed 14 months ago by Don-vip

Coverity found an issue:

*** CID 1451346:  Null pointer dereferences  (NULL_RETURNS)
/src/org/openstreetmap/josm/tools/date/DateUtils.java: 142 in org.openstreetmap.josm.tools.date.DateUtils.parseInstant(java.lang.String)()
136                 if (d != null)
137                     return d.toInstant();
138             }
139     
140             try {
141                 // slow path for fractional seconds different from millisecond precision
>>>     CID 1451346:  Null pointer dereferences  (NULL_RETURNS)
>>>     Calling a method on null object "java.time.ZonedDateTime.parse(str)".
142                 return ZonedDateTime.parse(str).toInstant();
143             } catch (IllegalArgumentException | DateTimeParseException ex) {
144                 throw new UncheckedParseException("The date string (" + str + ") could not be parsed.", ex);
145             }
146         }
147     

comment:4 Changed 14 months ago by simon04

This looks like a false-positive to me.

java.time.ZonedDateTime#parse(java.lang.CharSequence) clearly states Returns: the parsed zoned date-time, not null.

After stepping through the called methods, I found java.time.temporal.TemporalAccessor#query and java.time.chrono.ChronoLocalDateTime#query which return null for other parameters not used here.

comment:5 Changed 14 months ago by Don-vip

OK. No tool is perfect :)

comment:6 Changed 14 months ago by simon04

In 17712/josm:

see #14176 - Migrate Note/NoteComment to Instant

comment:7 Changed 13 months ago by simon04

In 17715/josm:

see #14176 - Migrate GPX to Instant

comment:8 Changed 13 months ago by simon04

In 17716/josm:

see #14176 - Migrate UserInfo to Instant

comment:9 Changed 13 months ago by simon04

In 17717/josm:

see #14176 - Migrate Changeset to Instant

comment:10 Changed 13 months ago by simon04

In 17718/josm:

see #14176 - Utils.getDurationString in GpxLayer.formatTimespan

comment:11 Changed 13 months ago by simon04

In 17719/josm:

see #14176 - Migrate AnimationExtensionManager to LocalDate

comment:12 Changed 13 months ago by simon04

In 17720/josm:

see #14176 - Migrate ChangesetDiscussionComment to Instant

comment:13 Changed 13 months ago by simon04

In 17721/josm:

see #14176 - Fix BookmarkList.ChangesetBookmark

https://errorprone.info/bugpattern/FormatString

comment:14 Changed 13 months ago by simon04

In 17722/josm:

see #14176 - Fix Instant writing in GpsWriter

comment:15 Changed 13 months ago by simon04

In 17723/josm:

see #14176 - Fix Instant writing in GpsWriterTest

comment:16 Changed 13 months ago by simon04

In 35732/osm:

see #14176 - reverter: adapt ChangesetReverter for r17717

comment:17 Changed 13 months ago by simon04

In 17726/josm:

see #14176 - Migrate AnimationExtensionManager to LocalDate (ZoneId)

https://errorprone.info/bugpattern/JavaTimeDefaultTimeZone

comment:18 Changed 13 months ago by Don-vip

Another issue reported by Coverity:

*** CID 1452256:    (NULL_RETURNS)
/src/org/openstreetmap/josm/data/gpx/GpxData.java: 298 in org.openstreetmap.josm.data.gpx.GpxData$GpxTrackSegmentSpan.<init>(org.openstreetmap.josm.data.gpx.WayPoint, org.openstreetmap.josm.data.gpx.WayPoint)()
292             private final WayPoint firstWp;
293             private final WayPoint lastWp;
294     
295             GpxTrackSegmentSpan(WayPoint a, WayPoint b) {
296                 Instant at = a.getInstant();
297                 Instant bt = b.getInstant();
>>>     CID 1452256:    (NULL_RETURNS)
>>>     Dereferencing a pointer that might be "null" "at" when calling "isBefore".
298                 inv = bt.isBefore(at);
299                 if (inv) {
300                     firstWp = b;
301                     firstTime = bt;
302                     lastWp = a;
303                     lastTime = at;
/src/org/openstreetmap/josm/data/gpx/GpxData.java: 298 in org.openstreetmap.josm.data.gpx.GpxData$GpxTrackSegmentSpan.<init>(org.openstreetmap.josm.data.gpx.WayPoint, org.openstreetmap.josm.data.gpx.WayPoint)()
292             private final WayPoint firstWp;
293             private final WayPoint lastWp;
294     
295             GpxTrackSegmentSpan(WayPoint a, WayPoint b) {
296                 Instant at = a.getInstant();
297                 Instant bt = b.getInstant();
>>>     CID 1452256:    (NULL_RETURNS)
>>>     Calling a method on null object "bt".
298                 inv = bt.isBefore(at);
299                 if (inv) {
300                     firstWp = b;
301                     firstTime = bt;
302                     lastWp = a;
303                     lastTime = at;

comment:19 Changed 13 months ago by simon04

In 17748/josm:

see #14176 - Migrate GPX to Instant

comment:20 Changed 13 months ago by simon04

In 17749/josm:

see #14176 - Migrate OsmPrimitive to Instant

comment:21 Changed 13 months ago by skyper

This ticket is mentioned in the changelog for milestone 21.04. Is it fixed or is there anything left to do?

comment:22 Changed 13 months ago by simon04

Thanks for double checking / asking. It's ongoing work (maybe half way through)...

comment:23 Changed 13 months ago by simon04

In 17838/josm:

see #14176 - Migrate HistoryOsmPrimitive to Instant

comment:24 Changed 13 months ago by simon04

In 17839/josm:

see #14176 - RtkLibPosReader: parse timestamps using DateUtils

comment:25 Changed 13 months ago by simon04

In 17840/josm:

see #14176 - Migrate ExceptionUtil to Instant

comment:26 Changed 13 months ago by simon04

In 17841/josm:

see #14176 - Migrate various classes to Instant

comment:27 Changed 13 months ago by simon04

In 17842/josm:

see #14176 - Migrate DateFilterPanel to Instant

comment:28 Changed 13 months ago by simon04

In 17843/josm:

see #14176 - Migrate ChangesetQuery to Instant

comment:29 Changed 13 months ago by simon04

Owner: changed from team to simon04
Status: newassigned

comment:30 Changed 13 months ago by simon04

Milestone: 21.05
Resolution: fixed
Status: assignedclosed

comment:31 Changed 13 months ago by simon04

In 17844/josm:

see #14176 - Fix GpxLayerTest

comment:32 Changed 13 months ago by simon04

In 17845/josm:

see #14176 - Introduce class Interval

comment:33 in reply to:  31 Changed 13 months ago by GerdP

Replying to simon04:

In 17844/josm:

see #14176 - Fix GpxLayerTest

Why do you change the test instead of the tested code here?

comment:34 Changed 13 months ago by simon04

Because I intentionally modified this code in r17840. I could have been more clear on that in the commit message...

comment:35 Changed 12 months ago by simon04

In 17911/josm:

see #14176 - Deprecate DateUtils.newIsoDateTimeFormat

comment:36 Changed 12 months ago by simon04

In 17912/josm:

see #14176 - Test DateUtils.getDateTimeFormatter

comment:37 Changed 10 months ago by Don-vip

In 18013/josm:

see #14176 - remove deprecated DateUtils methods

comment:38 Changed 6 months ago by GerdP

In 35848/osm:

see #14176 - Migrate HistoryOsmPrimitive to Instant

  • replace deprecated code

comment:39 Changed 6 months ago by GerdP

In 35850/osm:

see #14176 - Migrate OsmPrimitive to Instant

  • adapt deprecated code

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain simon04.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.