Changeset 14456 in josm for trunk/src/org/openstreetmap/josm/data/gpx/WayPoint.java

Timestamp:

2018-11-27T21:40:10+01:00 (5 years ago)

Author:

Don-vip

Message:

fix #16995 - de-duplicate storage of timestamp within WayPoint and refactor some methods, added documentation, added some robustness against legacy code (will also log a warning if detected). Patch by cmuelle8, modified

File:

• trunk/src/org/openstreetmap/josm/data/gpx/WayPoint.java (modified) (14 diffs)

trunk/src/org/openstreetmap/josm/data/gpx/WayPoint.java

@@ -5 +5 @@
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Objects;
@@ -14 +15 @@
 import org.openstreetmap.josm.data.projection.Projecting;
 import org.openstreetmap.josm.tools.Logging;
+import org.openstreetmap.josm.tools.date.DateUtils;
 import org.openstreetmap.josm.tools.template_engine.TemplateEngineDataProvider;
 
@@ -23 +25 @@
 
     /**
-     * The seconds (not milliseconds!) since 1970-01-01 00:00 UTC
-     */
-    public double time;
-    /**
      * The color to draw the segment before this point in
      * @see #drawLine
      */
     public Color customColoring;
+
     /**
      * <code>true</code> indicates that the line before this point should be drawn
      */
     public boolean drawLine;
+
     /**
      * The direction of the line before this point. Used as cache to speed up drawing. Should not be relied on.
@@ -40 +40 @@
     public int dir;
 
+    /*
+     * We "inline" lat/lon, rather than using a LatLon internally => reduces memory overhead. Relevant
+     * because a lot of GPX waypoints are created when GPS tracks are downloaded from the OSM server.
+     */
+    private final double lat;
+    private final double lon;
+
+    /*
+     * internal cache of projected coordinates
+     */
+    private double east = Double.NaN;
+    private double north = Double.NaN;
+    private Object eastNorthCacheKey;
+
     /**
      * Constructs a new {@code WayPoint} from an existing one.
+     *
+     * Except for PT_TIME attribute, all attribute objects are shallow copied.
+     * This means modification of attr objects will affect original and new {@code WayPoint}.
+     *
      * @param p existing waypoint
      */
     public WayPoint(WayPoint p) {
+        init_attr();
         attr.putAll(p.attr);
+        attr.put(PT_TIME, p.getDate());
         lat = p.lat;
         lon = p.lon;
@@ -51 +71 @@
         north = p.north;
         eastNorthCacheKey = p.eastNorthCacheKey;
-        time = p.time;
         customColoring = p.customColoring;
         drawLine = p.drawLine;
@@ -62 +81 @@
      */
     public WayPoint(LatLon ll) {
+        init_attr();
         lat = ll.lat();
         lon = ll.lon();
     }
 
-    /*
-     * We "inline" lat/lon, rather than usinga LatLon internally => reduces memory overhead. Relevant
-     * because a lot of GPX waypoints are created when GPS tracks are downloaded from the OSM server.
-     */
-    private final double lat;
-    private final double lon;
-
-    /*
-     * internal cache of projected coordinates
-     */
-    private double east = Double.NaN;
-    private double north = Double.NaN;
-    private Object eastNorthCacheKey;
+    /**
+     * Interim to detect legacy code that is not using {@code WayPoint.setTime(x)}
+     * functions, but {@code attr.put(PT_TIME, (String) x)} logic.
+     * To remove mid 2019
+     */
+    private void init_attr() {
+        attr = new HashMap<String, Object>(0) {
+            @Override
+            public Object put(String key, Object value) {
+                Object ret = null;
+                if (key != PT_TIME || (key == PT_TIME && value instanceof Date)) {
+                    ret = super.put(key, value);
+                } else {
+                    if (value instanceof String) {
+                        ret = super.put(PT_TIME, DateUtils.fromString((String) value));
+                        List<String> lastErrorAndWarnings = Logging.getLastErrorAndWarnings();
+                        if (!lastErrorAndWarnings.isEmpty() && !lastErrorAndWarnings.get(0).contains("calling WayPoint.put")) {
+                            StackTraceElement[] e = Thread.currentThread().getStackTrace();
+                            int n = 1;
+                            while (n < e.length && "put".equals(e[n].getMethodName())) {
+                                n++;
+                            }
+                            if (n < e.length) {
+                                Logging.warn("{0}:{1} calling WayPoint.put(PT_TIME, ..) is deprecated. " +
+                                    "Use WayPoint.setTime(..) instead.", e[n].getClassName(), e[n].getMethodName());
+                            }
+                        }
+                    }
+                }
+                return ret;
+            }
+        };
+    }
 
     /**
@@ -126 +166 @@
 
     /**
-     * Sets the {@link #time} field as well as the {@link #PT_TIME} attribute to the specified time.
+     * Sets the {@link #PT_TIME} attribute to the specified time.
      *
      * @param time the time to set
@@ -132 +172 @@
      */
     public void setTime(Date time) {
-        this.time = time.getTime() / 1000.;
-        this.attr.put(PT_TIME, time);
+        setTimeInMillis(time.getTime());
     }
 
@@ -139 +178 @@
      * Convert the time stamp of the waypoint into seconds from the epoch.
      *
-     * @deprecated call {@link #setTimeFromAttribute()} directly if you need this
+     * @deprecated Use {@link #setTime(Date)}, {@link #setTime(long)}, {@link #setTimeInMillis(long)}
      */
     @Deprecated
@@ -147 +186 @@
 
     /**
-     * Sets the {@link #time} field as well as the {@link #PT_TIME} attribute to the specified time.
+     * Sets the {@link #PT_TIME} attribute to the specified time.
      *
      * @param ts seconds from the epoch
@@ -153 +192 @@
      */
     public void setTime(long ts) {
-        setTimeInMillis(ts*1000);
-    }
-
-    /**
-     * Sets the {@link #time} field as well as the {@link #PT_TIME} attribute to the specified time.
+        setTimeInMillis(ts * 1000);
+    }
+
+    /**
+     * Sets the {@link #PT_TIME} attribute to the specified time.
      *
      * @param ts milliseconds from the epoch
@@ -163 +202 @@
      */
     public void setTimeInMillis(long ts) {
-        this.time = ts / 1000.;
-        this.attr.put(PT_TIME, new Date(ts));
-    }
-
-    /**
-     * Convert the time stamp of the waypoint into seconds from the epoch
+        attr.put(PT_TIME, new Date(ts));
+    }
+
+    /**
+     * Convert the time stamp of the waypoint into seconds from the epoch.
      * @return The parsed time if successful, or {@code null}
      * @since 9383
-     */
+     * @deprecated Use {@link #setTime(Date)}, {@link #setTime(long)}, {@link #setTimeInMillis(long)}
+     */
+    @Deprecated
     public Date setTimeFromAttribute() {
-        if (attr.containsKey(PT_TIME)) {
-            final Object obj = get(PT_TIME);
+        Logging.warn("WayPoint.setTimeFromAttribute() is deprecated, please fix calling code");
+        return getDate();
+    }
+
+    @Override
+    public int compareTo(WayPoint w) {
+        return Long.compare(getTimeInMillis(), w.getTimeInMillis());
+    }
+
+    /**
+     * Returns the waypoint time in seconds since the epoch.
+     *
+     * @return the waypoint time
+     */
+    public double getTime() {
+        return getTimeInMillis() / 1000.;
+    }
+
+    /**
+     * Returns the waypoint time in milliseconds since the epoch.
+     *
+     * @return the waypoint time
+     * @since 14456
+     */
+    public long getTimeInMillis() {
+        Date d = getDateImpl();
+        return d == null ? 0 : d.getTime();
+    }
+
+    /**
+     * Returns true if this waypoint has a time.
+     *
+     * @return true if a time is set, false otherwise
+     * @since 14456
+     */
+    public boolean hasDate() {
+        return attr.get(PT_TIME) instanceof Date;
+    }
+
+    /**
+     * Returns the waypoint time Date object.
+     *
+     * @return a copy of the Date object associated with this waypoint
+     * @since 14456
+     */
+    public Date getDate() {
+        return DateUtils.cloneDate(getDateImpl());
+    }
+
+    /**
+     * Returns the waypoint time Date object.
+     *
+     * @return the Date object associated with this waypoint
+     */
+    private Date getDateImpl() {
+        if (attr != null) {
+            final Object obj = attr.get(PT_TIME);
+
             if (obj instanceof Date) {
-                final Date date = (Date) obj;
-                time = date.getTime() / 1000.;
-                return date;
+                return (Date) obj;
             } else if (obj == null) {
                 Logging.info("Waypoint {0} value unset", PT_TIME);
             } else {
                 Logging.warn("Unsupported waypoint {0} value: {1}", PT_TIME, obj);
-                time = 0;
             }
         }
+
         return null;
-    }
-
-    @Override
-    public int compareTo(WayPoint w) {
-        return Double.compare(time, w.time);
-    }
-
-    /**
-     * Returns the waypoint time.
-     * @return the waypoint time
-     */
-    public Date getTime() {
-        return new Date((long) (time * 1000));
     }
 
@@ -228 +309 @@
         temp = Double.doubleToLongBits(lon);
         result = prime * result + (int) (temp ^ (temp >>> 32));
-        temp = Double.doubleToLongBits(time);
+        temp = getTimeInMillis();
         result = prime * result + (int) (temp ^ (temp >>> 32));
         return result;
@@ -242 +323 @@
         return Double.doubleToLongBits(lat) == Double.doubleToLongBits(other.lat)
             && Double.doubleToLongBits(lon) == Double.doubleToLongBits(other.lon)
-            && Double.doubleToLongBits(time) == Double.doubleToLongBits(other.time);
+            && getTimeInMillis() == other.getTimeInMillis();
     }
 }