serviceDayLookout: more documentation, refactor to avoid duplicate en…

…tries, remove from API parameter

src/main/java/org/opentripplanner/api/common/RoutingResource.java

@@ -391,10 +391,6 @@ public abstract class RoutingResource {
     @QueryParam("geoidElevation")
     private Boolean geoidElevation;
 
-    /* If trip discovery mode (look out a week) should be used */
-    @QueryParam("serviceDayLookout")
-    private Integer serviceDayLookout;
-
     /**
      * Set the method of sorting itineraries in the response. Right now, the only supported value is "duration";
      * otherwise it uses default sorting. More sorting methods may be added in the future.
@@ -655,9 +651,6 @@ protected RoutingRequest buildRequest() throws ParameterException {
         if (geoidElevation != null)
             request.geoidElevation = geoidElevation;
 
-        if (serviceDayLookout != null)
-            request.setServiceDayLookout(serviceDayLookout);
-
         if (pathComparator != null)
             request.pathComparator = pathComparator;
 

src/main/java/org/opentripplanner/routing/core/RoutingContext.java

@@ -34,12 +34,14 @@
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Collection;
+import java.util.Comparator;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TimeZone;
 
 /**
  * A RoutingContext holds information needed to carry out a search for a particular TraverseOptions, on a specific graph.
@@ -377,23 +379,29 @@ private void setServiceDays() {
             return;
         }
 
-        for (String feedId : graph.getFeedIds()) {
-            for (Agency agency : graph.getAgencies(feedId)) {
-                addIfNotExists(this.serviceDays, new ServiceDay(graph, serviceDate.previous(),
-                        calendarService, agency.getId()));
-                addIfNotExists(this.serviceDays, new ServiceDay(graph, serviceDate, calendarService, agency.getId()));
-                addIfNotExists(this.serviceDays, new ServiceDay(graph, serviceDate.next(),
-                        calendarService, agency.getId()));
-                if (opt.serviceDayLookout > 0) {
-                    ServiceDate sd = serviceDate;
-                    for (int i = 0; i < opt.serviceDayLookout; i++) {
-                        sd = opt.arriveBy ? sd.previous() : sd.next();
-                        addIfNotExists(this.serviceDays, new ServiceDay(graph, sd, calendarService, agency.getId()));
-                    }
-                }
+        // In general, there should be a set of service days (yesterday, today, tomorrow) for every
+        // timezone in the graph. The parameter `serviceDayLookout` allows more service days to be
+        // added to search for future service (or past, if arriveBy=true). The furthest back trips
+        // can begin is 1 service day (e.g. a trip which started yesterday is usable today.) This
+        // does not address the case where a trip started multiple days ago (e.g. a multi-day ferry
+        // trip will not be board-able after day 2).
+        for (TimeZone timeZone : graph.getAllTimeZones()) {
+            // Add today
+            addIfNotExists(this.serviceDays, new ServiceDay(graph, serviceDate, calendarService, timeZone));
+            // Add one day previous (previous in the direction of the transit search, so yesterday if
+            // arriveBy=false and tomorrow if arriveBy=true
+            addIfNotExists(this.serviceDays, new ServiceDay(graph,
+                    opt.arriveBy ? serviceDate.next() : serviceDate.previous(),
+                    calendarService, timeZone));
+            // Add one or more days in the "forward" direction
+            ServiceDate sd = serviceDate;
+            int lookout = Math.max(1, opt.serviceDayLookout);
+            for (int i = 0; i < lookout; i++) {
+                sd = opt.arriveBy ? sd.previous() : sd.next();
+                addIfNotExists(this.serviceDays, new ServiceDay(graph, sd, calendarService, timeZone));
             }
-
         }
+        serviceDays.sort(Comparator.comparing(ServiceDay::getServiceDate));
     }
 
     private static <T> void addIfNotExists(ArrayList<T> list, T item) {

src/main/java/org/opentripplanner/routing/core/RoutingRequest.java

@@ -564,8 +564,17 @@ public class RoutingRequest implements Cloneable, Serializable {
     /** Whether to apply the ellipsoid->geoid offset to all elevations in the response */
     public boolean geoidElevation = false;
 
-    /** How many extra ServiceDays to look out (or back) */
-    public int serviceDayLookout = -1;
+    /**
+     * How many extra ServiceDays to look in the future (or back, if arriveBy=true)
+     *
+     * This parameter allows the configuration of how far, in service days, OTP should look for
+     * transit service when evaluating the next departure (or arrival) at a stop. In some cases,
+     * for example for services which run weekly or monthly, it may make sense to increase this
+     * value. Larger values will increase the search time. This does not affect a case where a
+     * trip starts multiple service days in the past (e.g. a multiday ferry trip will not be
+     * board-able after the 2nd day in the current implementation).
+     */
+    public int serviceDayLookout = 1;
 
     /** Which path comparator to use */
     public String pathComparator = null;

src/main/java/org/opentripplanner/routing/core/ServiceDay.java

@@ -46,6 +46,11 @@ public ServiceDay(Graph graph, ServiceDate serviceDate, CalendarService cs, Stri
         init(graph, cs, timeZone);
     }
 
+    public ServiceDay(Graph graph, ServiceDate serviceDate, CalendarService cs, TimeZone timeZone) {
+        this.serviceDate = new ServiceDate(serviceDate);
+        init(graph, cs, timeZone);
+    }
+
     private void init(Graph graph, CalendarService cs, TimeZone timeZone) {
         Date d = serviceDate.getAsDate(timeZone);
         this.midnight = d.getTime() / 1000;

src/main/java/org/opentripplanner/routing/graph/Graph.java

@@ -931,7 +931,24 @@ public TimeZone getTimeZone() {
         }
         return timeZone;
     }
-
+
+    /**
+     * Return all TimeZones for all agencies in the graph
+     * @return collection of referenced timezones
+     */
+    public Collection<TimeZone> getAllTimeZones() {
+        List<TimeZone> timeZones = new ArrayList<>();
+        for (String feedId : getFeedIds()) {
+            for (Agency agency : getAgencies(feedId)) {
+                TimeZone timeZone = calendarService.getTimeZoneForAgencyId(agency.getId());
+                if (timeZone != null) {
+                    timeZones.add(timeZone);
+                }
+            }
+        }
+        return timeZones;
+    }
+
     /**
      * The timezone is cached by the graph. If you've done something to the graph that has the
      * potential to change the time zone, you should call this to ensure it is reset. 

src/test/java/org/opentripplanner/routing/core/RoutingContextTest.java

@@ -34,6 +34,8 @@ public void testSetServiceDays() throws Exception {
         routingRequest.modes = new TraverseModeSet("WALK,TRANSIT");
 
         when(graph.getTimeZone()).thenReturn(TimeZone.getTimeZone("Europe/Budapest"));
+        when(graph.getAllTimeZones()).thenReturn(Collections.singletonList(
+                TimeZone.getTimeZone("Europe/Budapest")));
         when(graph.getCalendarService()).thenReturn(calendarService);
         when(graph.getFeedIds()).thenReturn(Collections.singletonList("FEED"));
         when(graph.getAgencies(feedId)).thenReturn(Collections.singletonList(agency));