Add a null-check to the ids argument in the stopPlaces Transmodel API endpoint.

[eibakke]

Summary

This makes the stopPlaces API endpoint in the Transmodel API return an error if a null value is passed for the ids argument. This is in line with the behavior today, but today the error happens because the response is too large. Failing early is preferable.

Issue

#6543

Unit tests

Tested by running locally. There are no unit tests at this level.

[codecov]

Codecov Report

Attention: Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 70.25%. Comparing base (c2355db) to head (430ddd2).
Report is 73 commits behind head on dev-2.x.

Files with missing lines	Patch %	Lines
...anner/apis/transmodel/TransmodelGraphQLSchema.java	16.66%	5 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #6556      +/-   ##
=============================================
- Coverage      70.25%   70.25%   -0.01%     
+ Complexity     18387    18382       -5     
=============================================
  Files           2088     2088              
  Lines          77425    77408      -17     
  Branches        7843     7840       -3     
=============================================
- Hits           54395    54380      -15     
+ Misses         20258    20257       -1     
+ Partials        2772     2771       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[habrahamsson-skanetrafiken]

It looks to me like the original intention behind these endpoints was to allow users to fetch all stopPlaces/quays/serviceJourneys. Both the fact that the ids parameter is optional and that the documentation says Get all stopPlaces signals to a user that they can actually use this endpoint to fetch all stopPlaces.

I can see that this will be a problem in a big deployment because the response might get very large. When I tested on our setup it doesn't seem to be a big issue since we only have some thousands of stop places.

At skånetrafiken we are always providing ids in these endpoints so this won't break anything on our side. But this might break something for someone running a smaller instance that is using these endpoints to fetch all stopPlaces/quays/serviceJourneys. What do you think about that risk?

[eibakke]

Well spotted, thank you! I missed that documentation. Our longer term goal is to provide a type of pagination that will still let people fetch all stopPlaces safely. I'm unsure if we should block this change in the mean time. Given we don't know how many provide this Transmodel API it's hard to know how many smaller deployments we would break. Maybe a question for the developer meeting.

[vpaturet]

We discussed this during the developer meeting and agreed that we should aim for system stability and that breaking this specific use case (dumping all stop places / quays / service journeys) is acceptable.
Indeed the documentation on the end point should be updated.

[habrahamsson-skanetrafiken]

I'm fine with this if you update the documentation of the endpoint

[vpaturet]

If there is a need to provide an API endpoint to retrieve all quays or all stop places, that should be implemented as a paginated query.