Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
3 changes: 2 additions & 1 deletion docs/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

- Update onebusaway-gtfs to latest version from OBA project (#2636)
- Remove the coupling to OneBusAway GTFS within OTP's internal model by creating new classes replacing the external classes (#2494)
- Allow itineraries in response to be sorted by duration (#2593)

## 1.3 (2018-08-03)

Expand Down Expand Up @@ -364,4 +365,4 @@ This release was made to consolidate all the development that had occurred with
- and of course, lots of bugfixes

## 0.4.4 (2012-02-06)
Release in anticipation of upcoming merges.
Release in anticipation of upcoming merges.
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,13 @@ public abstract class RoutingResource {
@QueryParam("geoidElevation")
private Boolean geoidElevation;

/**
* 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.
*/
@QueryParam("pathComparator")
private String pathComparator;

/*
* somewhat ugly bug fix: the graphService is only needed here for fetching per-graph time zones.
* this should ideally be done when setting the routing context, but at present departure/
Expand Down Expand Up @@ -600,6 +607,9 @@ protected RoutingRequest buildRequest() throws ParameterException {
if (geoidElevation != null)
request.geoidElevation = geoidElevation;

if (pathComparator != null)
request.pathComparator = pathComparator;

//getLocale function returns defaultLocale if locale is null
request.locale = ResourceBundleSingleton.INSTANCE.getLocale(locale);
return request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,21 @@
import org.opentripplanner.routing.graph.Edge;
import org.opentripplanner.routing.graph.Graph;
import org.opentripplanner.routing.graph.Vertex;
import org.opentripplanner.routing.impl.DurationComparator;
import org.opentripplanner.routing.impl.PathComparator;
import org.opentripplanner.routing.request.BannedStopSet;
import org.opentripplanner.routing.spt.DominanceFunction;
import org.opentripplanner.routing.spt.GraphPath;
import org.opentripplanner.routing.spt.ShortestPathTree;
import org.opentripplanner.util.DateUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -439,6 +444,9 @@ public class RoutingRequest implements Cloneable, Serializable {
/** Whether to apply the ellipsoid->geoid offset to all elevations in the response */
public boolean geoidElevation = false;

/** Which path comparator to use */
public String pathComparator = null;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot any places where pathComparator is set. I think a comment on how this is used(set to somethingother null) should be documented. Is the plan to integrate with some kind of configuration later, routing request parameter, or just fork the code and inject a different strategy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not set explicitly in the code, but if you include configuration in router-config.json, it will get set automatically, e.g.

// router-config.json
{
  "routingDefaults": { 
     "pathComparator":"duration" 
   }
}

This happens via the existing config-setting code:

this.defaultRoutingRequest = scraper.scrape(routingDefaultsNode);

I'll add code to allow this to be set via an API parameter as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the clarification. I think your solution is the best, it might be confusing to have "config" params only.

/** Saves split edge which can be split on origin/destination search
*
* This is used so that TrivialPathException is thrown if origin and destination search would split the same edge
Expand Down Expand Up @@ -634,14 +642,14 @@ public void setOtherThanPreferredRoutesPenalty(int penalty) {
if(penalty < 0) penalty = 0;
this.otherThanPreferredRoutesPenalty = penalty;
}

public void setUnpreferredAgencies(String s) {
if (!s.isEmpty()) {
unpreferredAgencies = new HashSet<>();
Collections.addAll(unpreferredAgencies, s.split(","));
}
}

public void setUnpreferredRoutes(String s) {
if (!s.isEmpty()) {
unpreferredRoutes = RouteMatcher.parse(s);
Expand Down Expand Up @@ -686,7 +694,7 @@ public void setBannedStopsHard(String s) {
bannedStopsHard = StopMatcher.emptyMatcher();
}
}

public void setBannedAgencies(String s) {
if (!s.isEmpty()) {
bannedAgencies = new HashSet<>();
Expand Down Expand Up @@ -747,7 +755,7 @@ public void setDateTime(String date, String time, TimeZone tz) {
setDateTime(dateObject);
}

public int getNumItineraries() {
public int getNumItineraries() {
if (modes.isTransit()) {
return numItineraries;
} else {
Expand Down Expand Up @@ -783,14 +791,14 @@ public void setIntermediatePlacesFromStrings(List<String> intermediates) {
intermediatePlaces.add(GenericLocation.fromOldStyleString(place));
}
}

/** Clears any intermediate places from this request. */
public void clearIntermediatePlaces() {
if (this.intermediatePlaces != null) {
this.intermediatePlaces.clear();
}
}

/**
* Returns true if there are any intermediate places set.
*/
Expand Down Expand Up @@ -892,7 +900,7 @@ public void setRoutingContext(Graph graph, Edge fromBackEdge, Vertex from, Verte
this.rctx = new RoutingContext(this, graph, from, to);
this.rctx.originBackEdge = fromBackEdge;
}

public void setRoutingContext(Graph graph, Vertex from, Vertex to) {
setRoutingContext(graph, null, from, to);
}
Expand Down Expand Up @@ -1279,4 +1287,12 @@ public void canSplitEdge(StreetEdge edge) {
}

}

public Comparator<GraphPath> getPathComparator(boolean compareStartTimes) {
if ("duration".equals(pathComparator)) {
return new DurationComparator();
}
return new PathComparator(compareStartTimes);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* This program is free software: you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public License
as published by the Free Software Foundation, either version 3 of
the License, or (at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */

package org.opentripplanner.routing.impl;

import org.opentripplanner.routing.spt.GraphPath;

import java.util.Comparator;

public class DurationComparator implements Comparator<GraphPath> {

@Override
public int compare(GraphPath o1, GraphPath o2) {
return o1.getDuration() - o2.getDuration();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public List<GraphPath> getPaths(RoutingRequest options) {
LOG.debug("we have {} paths", paths.size());
}
LOG.debug("END SEARCH ({} msec)", System.currentTimeMillis() - searchBeginTime);
Collections.sort(paths, new PathComparator(options.arriveBy));
Collections.sort(paths, options.getPathComparator(options.arriveBy));
return paths;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* This program is free software: you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public License
as published by the Free Software Foundation, either version 3 of
the License, or (at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
package org.opentripplanner.routing.impl;

import org.junit.Test;
import org.opentripplanner.routing.spt.GraphPath;

import java.util.Arrays;
import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class PathComparatorTest {

GraphPath a = mockGraphPath(5, 7);

GraphPath b = mockGraphPath(0, 8);

GraphPath c = mockGraphPath(9, 12);

private List<GraphPath> paths = Arrays.asList(a, b, c);

@Test
public void testPathComparator() {
paths.sort(new PathComparator(false));
assertEquals(paths, Arrays.asList(a, b, c));
}

@Test
public void testPathComparatorArriveBy() {
paths.sort(new PathComparator(true));
assertEquals(paths, Arrays.asList(c, a, b));
}

@Test
public void testDurationComparator() {
paths.sort(new DurationComparator());
assertEquals(paths, Arrays.asList(a, c, b));
}


private GraphPath mockGraphPath(long startTime, long endTime) {
GraphPath path = mock(GraphPath.class);
when(path.getStartTime()).thenReturn(startTime);
when(path.getEndTime()).thenReturn(endTime);
when(path.getDuration()).thenReturn((int) (endTime - startTime));
return path;
}
}