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

Feed Id and Agency Id mapping #2524

Open
chrissekaran opened this Issue Dec 28, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@chrissekaran
Copy link

chrissekaran commented Dec 28, 2017

Expected behavior

The Api's in the IndexAPI.java dont really provide proper insight into the relation between between a FeedId and an AgencyId. The One Bus Away GTFS loader should distinguish between FeedId and AgencyId.
This is because

  1. any GTFS feed can be produced by multiple agencies.
  2. an agency can produce multiple GTFS feeds in concurrence with other Agencies and this relationship should be clear in the API through the URI contexts given in IndexAPI.java

Observed behavior

Observed behaviour is elaborated in the OpenTripPlanner-Dev forum here

Version of OTP used (exact commit hash or JAR name)

Latest as of this date

Data sets in use (links to GTFS and OSM PBF files)

Example GTFS files are from
http://transitfeeds.com/p/association-of-train-operating-companies/284 . (> 10MB)
http://transitfeeds.com/p/citymapper/894

Command line used to start OTP

--build /path-to-router --inMemory

Router config and graph build config JSON

defaults or none

Steps to reproduce the problem

Also described here in request and responses

@johannilsson

This comment has been minimized.

Copy link
Contributor

johannilsson commented Dec 28, 2017

If one agency with the same id would exist in more than one feed, OTP would treat them as separate agencies and not as one single agency.

Since the agency id itself is not scoped with the feed id as other entities is this url structure is used to list agencies for a specific feed; /agencies/{feedId} and to list routes belonging to a specific agency /agencies/{feedId}/{agencyId}/routes changing the url structure of the api has been discussed in #2201.

@chrissekaran

This comment has been minimized.

Copy link

chrissekaran commented Dec 29, 2017

@johannilsson Thanks for the link to the #2201 thread.

Im not sure then if we should have both issues open as long as all concerns are addressed but it seems like a full total solution is required that addresses all concerns. How we do that is really fine with me.

So,

  1. I think it makes sense as you pointed out that all Agency Ids are scoped under the Feed Id and the uri context needs to reflect that. Whether we do that using a FeedScopedId as discussed in the other thread or another way is fine as long as that approach resolves the Agency scope being under the feed scope.

  2. In my google group thread I pointed out the first problem in the first two request/responses. I guess that needs to be resolved regardless of the Agency Id as that just seems like only one FeedId returns information and others a 404
    GET /otp/routers/default/index/feeds/
    [
    "1",
    "2"
    ]

GET /otp/routers/default/index/feeds/1
{
"id": 1,
"publisherName": "XYZ TRANSIT",
"publisherUrl": "http://www.xyz.org",
"lang": "en",
"version": "999-0"
}
..whereas
GET /otp/routers/default/index/feeds/2
FOUR ZERO FOUR

  1. Also could you please elaborate on what is an enityId when you speak of feedId:entityId

johannilsson added a commit to johannilsson/OpenTripPlanner that referenced this issue Dec 29, 2017

Fix issue with loading feed info by feed id
Indexed FeedInfoS is now mapped to the feed id and not the id field of
the FeedInfo.

Resolves opentripplanner#2524
@johannilsson

This comment has been minimized.

Copy link
Contributor

johannilsson commented Dec 29, 2017

  1. I believe the current structure works but it is confusing and should probably be addressed. There has been a discussion on replacing the OBA model with an OTP model and if that work is merged it would probably be a good idea to introduce scoped agencies to this model instead of doing it as I did in the linked issue.
  2. It looks like the feed infos was indexed with FeedInfo#id and not the feed id. I've pushed a patch that fixes this issue.
  3. The entity id is the id of stops, trips, routes etc. If a feed has the id 1 and a stop has the id 100 the id for this stop will become 1:100.
@t2gran

This comment has been minimized.

Copy link
Member

t2gran commented Jan 18, 2018

I agree that the current model i confusing, and changing the AgencyAndId to FeedScopedId would be clearer. I can do that as soon as the split #2494 is merged into master.

However, I am a little puzzled by the idea that ids are changed in OTP. I see OTP as just one component in a chain of components, and if every component change IDs, it would very soon be a mess. Also, I don´t think that OTP clients should need to know about feeds. Hence, feed ids do not belong in resource URLs. At the moment this is not an issue for us, and not a prioritized thing to do, just a though from me.

@abyrd

This comment has been minimized.

Copy link
Member

abyrd commented Jan 18, 2018

Hi @t2gran, can you clarify what you mean by "the idea that ids are changed in OTP"? What kind of IDs, and where are they changed? I agree, ideally no IDs should be changed anywhere. However changes have accumulated from several sources so perhaps some shared rules/guidelines have not emerged.

However, if IDs are not changed, then clients must be aware of feeds, at least as prefixes on IDs, because IDs inside GTFS feeds are not globally unique.

@t2gran

This comment has been minimized.

Copy link
Member

t2gran commented Jan 18, 2018

I am sorry if I was a bit unclear. What I mean by change is that an ID is prefixed with feedId. When looking at OTP as a black box, the IDs that goes inn, are not the same as the one used to access data on the API, the clients need to know the feedId. This is OK if the id was retrieved from OTP, but not if it came from som other component in the ecosystem. The problem of conflicting IDs between feeds should be fixed by prefixing, only when it is a problem (enabled in the build config). This is not a big issue - and we(Entur and Ruter) already worked around it. I was just a little concerned that the feed ID should pollute the URLs also. At Entur and Ruter all IDs are uniq. We also merged all operators into one feed, but might undo that in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment