VDVde / OJP

Open API for distributed journey planning. CEN/TS 17118:2017.
https://www.vdv.de/open-journey-planner.aspx
22 stars 12 forks source link

Handling of MultiPointType: urgent answer required #442

Closed ue71603 closed 5 months ago

ue71603 commented 5 months ago

Austria noted:

Made MultiPointType a required parameter in OJPMultiPointTripRequest. #353 Auf Basis dieser Definition (die jedenfalls Sinn macht, wenngleich die von einem passiven Knoten unterstützten MultiPointTypes ohnehin vorab abgeklärt werden müssen) ist der Parameter MultiPointType in Chapter 8.6.4.5 MultiPointTripParamStructure in Table 151Description of MultiPointTripParamStructure als mandatory angeführt.

Allerdings sind die Params in denen sich der MultiPointType versteckt laut Chapter 8.6.4.3 OJPMultiPointTripRequestStructure und Table 149 Description of OJPMultiPointTripRequestStructure optional.

The parameter IS optional. and marked that way. In TripRequest "anyPoint" is the the default value that should be implemented.

In the response it is noted, that it "should" be returned. In my view this might be enough. Different opinions?

Urgent answers needed. image

Andrés proposal:

I agree with the Austrian comment, there is indeed an incoherence and it can be found in the code and the documentation. Correcting it would be simple: setting the cardinality of the Params element in MultiPointTripRequest to 1:1 (instead of 0:1). The currently existing inconsistency shows that a certain parameter (MultiPointType) was intended to be a required (non-optional) parameter, but this is not fully implemented / guaranteed by the code. P.S. Before applying the change - should we check and discuss whether having it a required parameter is really the right decision - a previous solution (CR #98, still mentioned in the changelog and misleading/superfluous if a required parameter) was to have a default value (anyPoint)? Depending on the decision, I think the changlog entry should be removed.

ue71603 commented 5 months ago

Stefan/Matthias: Let's also have a default in the response. And update the Changelog (that it is optional with default value)

herlitze commented 5 months ago

@ue71603 Do I understand you correctly that you want to keep the OJPMultiPointTripRequest.Params optional and also want to keep OJPMultiPointTripRequest.Params.MultiPointType required? The changes you are suggesting are adapting the documentation accordingly and adding a default value to the response (OJPMultiPointTripDelivery.MultiPointType)? This would be fine for me.

ue71603 commented 5 months ago

@herlitze that is exactly what I intend to do...

ue71603 commented 5 months ago

addressed in https://github.com/VDVde/OJP/pull/443

sgrossberndt commented 1 month ago

@ue71603 Do I understand you correctly that you want to keep the OJPMultiPointTripRequest.Params optional and also want to keep OJPMultiPointTripRequest.Params.MultiPointType required? The changes you are suggesting are adapting the documentation accordingly and adding a default value to the response (OJPMultiPointTripDelivery.MultiPointType)? This would be fine for me.

In the current state OJPMultiPointTripRequest.Params.MultiPointType no longer is required. Is that how it is supposed to be? @herlitze @ue71603

herlitze commented 1 month ago

@sgrossberndt I don't remember any reason for this and I didn't find any reason in the comments either. @ue71603 Was this done intentionally?