propershark / shark

An event publisher for realtime transit information.
3 stars 0 forks source link

Embed station object in route objects #5

Closed elliottwilliams closed 8 years ago

elliottwilliams commented 8 years ago

When proper loads a route view, it'll need (at the very least) names of stations on that route to populate the station list. But I can see future uses for other station data (e.g. geo coordinates)

Could shark always pass a hashed Stations in Route#stations? We should make sure there's a reasonable code path for including all or some of an object's associated objects

faultyserver commented 8 years ago

I think the conclusion we reached here was that (at least for routes) a "reduced hash" would get embedded. That is, a station identifier like stations.BUS337SE would get replaced with { "identifier": "stations.BUS337SE", "name": "That station over there" }. The reason for this is to reduce the size of the message, since routes can easily have hundreds of stations, and a full embed would mean thousands of attributes getting passed around for each route.

For vehicles, however, the full route and station objects can be embedded, because each vehicle only has one route and two station references.

elliottwilliams commented 8 years ago

Is there a way to sometimes get fully embedded objects at the root level? In envisioning proper's usage, I think I'd prefer reduced station objects on agency.routes and full embeded objects on Station#update.

faultyserver commented 8 years ago

Yes, by "embedding" I'm just referring to objects within other objects, not objects at the top level. That includes station objects inside of Routes, but not Station objects themselves. So, all of the agency. topics will be full objects, and all update events will include full objects, but the stations inside of a Route update will only include identifier and name.

Thinking about it, it would probably be useful to include latitude and longitude in station embeds, as well.

elliottwilliams commented 8 years ago

Makes sense. From what I see there's no point other than Route#stations where a list of objects is specified, so I think it makes sense to only reduce there.

elliottwilliams commented 8 years ago

Side note, but I am expecting {"stop_code": "stations.BUS337SE", "name": "That station over there"}, instead of having the identifier key. If only a subset of an object's keys is what you send, I can parse it as a that object without having to write additional parser code.

faultyserver commented 8 years ago

That's definitely possible.

However, if we're going for the whole generic thing (where it should work for other agencies than just CityBus), it'd be nice to not have that hard coded.

The way I'm planning on setting this up involves reading Station.identifying_attribute (on the class) and station.identifier (on the instance), meaning the name of the attribute is configurable, but guaranteed to be one of Station's attributes.

elliottwilliams commented 8 years ago

That makes sense, always feel free to stop me when I'm not thinking generic. I was just making a typealias Identifier on models so this fits right into place.

elliottwilliams commented 8 years ago

Another question involves vehicle.last_station, vehicle.next_station, vehicle.route: will these be full embeds, just identifiers, or similarly stubbed?

faultyserver commented 8 years ago

I snuck it in earlier with "For vehicles, however, the full route and station objects can be embedded, because each vehicle only has one route and two station references.". I probably should have named the attributes explicitly.

vehicle.last_station, vehicle.next_station, and vehicle.route will be full-object embeds, since each vehicle only has those three references, rather than hundreds or more for each Route.

Thinking about it, though, it would be much simpler and smaller to do a similar reduction on vehicle.route. Not including path and stations will make route embeds similar in size to station embeds, and I can't see a reason that they would need to be included (If users want more information about the route, you could call meta.last_event on that route.

elliottwilliams commented 8 years ago

it would be much simpler and smaller to do a similar reduction on vehicle.route

Agreed. If we want to define a convention for when to embed vs reduce, I'd say: "Fully embed the object requested, and any one-to-one relations from that object. Reduce one-to-many relations from that object to stub objects that include at least the identifier of the related object."

faultyserver commented 8 years ago

Per discussion from Jul 13th, vehicles should embed a list of next_stations rather than just one.

This allows vehicles to know where it is going further in advance than just having one station does. The advantages of this lie mainly in clients that want to do predictions.

For example, a client wants to show the route that a vehicle is traveling on. It can get the full list of stations by subscribing to the route object from the identifier that the vehicle provides, and simplify this to a loop with conditional stations (e.g., stations that are only visited after 5pm). To know if a conditional station should be displayed as active, rather than relying on currently-unavailable schedule information, the client could read the upcoming stations and see if that condition station is being approached.

Having multiple next stations in this example allows the client to update earlier, making the switch less obvious and less confusing to the user (it will likely not be where they are looking). If only one next station was provided, the client would not be able to update until the vehicle was already on its way to the conditional station, which would cause a jump in state from the old path to the new.

elliottwilliams commented 8 years ago

Exactly. Did we decide to include n stations in next_stations (where n is the furthest away a station can be and still receive approaches), or is next_stations going to have the entire remaining list of stops along the route?

faultyserver commented 8 years ago

It will be n stations, as determined by maximum_distance_in_stops. The default for this is 3, but is configurable to any positive number.

faultyserver commented 8 years ago

Per #10, next_stations is no longer a property that will be embedded, since Vehicle events have been removed from Station topics (See "WAMP Events and RPCs" wiki page).

faultyserver commented 8 years ago

Ignore my previous comment. #10 has no relation to next_stations. It is still useful as a way to determine the state of a conditional station on a Route, and should be included.

faultyserver commented 8 years ago

Closed by #18, because object embedding is now happening.

Opening a new issue for next_stations, as it is a separate concept, and is not currently implemented.