noi-techpark / bdp-commons

Open Data Hub time series data collectors (legacy)
GNU Affero General Public License v3.0
2 stars 12 forks source link

As STA I would like that the real-time availability data of my bike parking structures ("bike boxen") is integrated and make then available in the Open Data Hub, so that I can show this information on the suedtirolmobil channels #602

Closed rcavaliere closed 1 year ago

rcavaliere commented 1 year ago

To do list:

Technical information (shared separately)

Specification integration document: 230308_SpecificheIntegrazione_NOI_v1.4.pdf

clezag commented 1 year ago

I've started to implement the data collector, but there are some problems with the authentication mechanism. The authentication scheme looks very much like Oauth 2 (parameter names, response), but it does not actually conform to the specification:

The token endpoint /connect/token does not accept client credentials via HTTP basic auth, it only accepts them as urlencoded parameters. Using basic auth results in a HTTP 400 error.

In the client_credentials flow, credentials shoud be provided (and accepted) as basic authentication header, while only the grant_type is passed as a an urlencoded param (see the RFC). This is also how tools like Swagger or Spring implement it.

While we could implement a workaround for this issue, it prevents us from using standard libraries such as Spring to handle authentication. I imagine that other consumers of the API will have the same problem, so I suggest passing this feedback on to the API developers.

rcavaliere commented 1 year ago

@clezag thanks for the feedback, issue shared with the Data Provider. Let's wait that they fix this issue...

clezag commented 1 year ago

@rcavaliere The Data Provider has resolved the issue. We can proceed with defining and implementing the data mapping.

rcavaliere commented 1 year ago

@clezag based on the information currently available, my proposal is to integrate this real-time data stream as described in the attached document. Let me know if we need to discuss details together. 230214_SpecificheIntegrazione_NOI_v1.1.pdf

clezag commented 1 year ago

@rcavaliere it's up in testing, you can take a look at the data in the test DB.

rcavaliere commented 1 year ago

@clezag I have tested the Data Collector, nice work! There are some little points to be adjusted:

The updated version of the specification document is available here. 230224_SpecificheIntegrazione_NOI_v1.2.pdf

In general, I have some concerns about the data quality, the naming of certain types and the meaning of certain fields. I will contact the Data Provider for more explanations. In the meantime I would fix these points.

clezag commented 1 year ago

@rcavaliere thanks for the feedback. I've implemented the requested changes, the build is available in testing.

The data in measurementstring(history) is still there from early testing where data types were not configured correctly. The current build should not write to those tables anymore.

rcavaliere commented 1 year ago

@clezag thanks for your work, everything looks fine

I have made a little modification to the specification document based on the feedback of the Data Provider (see attached file), basically:

230227_SpecificheIntegrazione_NOI_v1.3.pdf

Can you please make this final adjustment? Then we should wait that the Data Providers puts everything in its production environment...

clezag commented 1 year ago

@rcavaliere thank you! I've made the change in testing.

rcavaliere commented 1 year ago

@clezag good! Can you please delete all previous data stored in the testing environment and then start the Data Collector again? I would like to test the Data Collector and the modified API of the Data Provider in a more "clean" way

clezag commented 1 year ago

@rcavaliere I've deleted the old data and updated the data collector with the new field names (on our side the data type names remain the same). One thing I've noticed is that the station/place/metadata.type field is still always 0 instead of 1 or 2 like specified in the requirements and API documentation.

rcavaliere commented 1 year ago

@clezag you are right, I have requested to Bicincittà the meaning of this value. Another question: why did you use two different types for the type "state" associated to BikeParking and BikeParkingBay? In theory, we can always use the same type

clezag commented 1 year ago

@rcavaliere sorry, probably missed that because I've implemented the station level "state" measurement only after your feedback. I've changed it so that now both use the same "usageState" data type specified in the requirements doc.

You will continue to see the outdated data with type "state" until we clean up the DB once again.

rcavaliere commented 1 year ago

@clezag based on the final feedback on Bicincittà, I would suggest a final modification, i.e. to not save the field stationPlaces/type for the bike parking bays in case they are referred to a bike sharing station. This should be saved only it is a bike parking station. See revised version of the specification

230308_SpecificheIntegrazione_NOI_v1.4.pdf

clezag commented 1 year ago

@rcavaliere New build is up in testing, containing both your requested change and the serviceType calls detailed below:

Looking through the data and revisiting the postman file provided, I've noticed a bug/inconsistency in the API. We currently don't have any type = 2 (parking) stations in the DB, and the reason is the following:

When interrogating /resources/stations, which returns a list of stations for a city, an additional parameter serviceType can be provided. I assume that this doesn't corresponds to the field station.type, but is just 1 = sharing, 2 = parking.

Currently it behaves as follows:

If I search with

I would expect the webservice to return all stations of any type if I don't provide any serviceType parameter to the call. This would both be logically consistent, and save API consumers a second webservice call.

I've just now implemented two separate calls (so we can proceed with testing) with type 1 and type 2 for every city, but we should probably signal this inconsistency to the data provider, since they seem open to feedback.

Another thing: the /resources/services returns multiple services, BicInCittà is only one of them. I currently load all cities of all services. Is this OK? Asking because we called the origin "bicincitta"

rcavaliere commented 1 year ago

@clezag thanks for the feedback. Good that you have fixed this in that way, but the Data Provider should fix this - I will write them again. Let's keep the origin "Bicincittà", in their platform they several services for different clients. We will see if they then will share with us all data when we go into production...

rcavaliere commented 1 year ago

Data Provider should activate their back-end soon. @clezag, testing activities could be planned in the next sprint

rcavaliere commented 1 year ago

@clezag @dulvui the Data Provider has unexpectedly changed their API, which is something bad. This was however done to accomplish other users of their API, which is needed to implement the entire customer journey on suedtirolmobil. In any case, I have seen the changes and are in my point of view quite minor. They also introduced multilinguism which is something more. Please find attached the reviewed version of the integration specifications, both in track change and in clean mode. Let me know if you have doubts!

230630_SpecificheIntegrazione_NOI_v2.1.pdf 230630_SpecificheIntegrazione_NOI_v2.1-clean.pdf

dulvui commented 1 year ago

@rcavaliere Live on testing now BikeParking stations: https://mobility.api.opendatahub.testingmachine.eu/v2/flat,node/BikeParking/ data: https://mobility.api.opendatahub.testingmachine.eu/v2/flat,node/BikeParking/*/latest?where=sactive.eq.true

BikeParkingBay stations: https://mobility.api.opendatahub.testingmachine.eu/v2/flat,node/BikeParkingBay/ data: https://mobility.api.opendatahub.testingmachine.eu/v2/flat,node/BikeParkingBay/*/latest?where=sactive.eq.true

rcavaliere commented 1 year ago

@dulvui looks good. I will test this before sharing our APIs with STA, which wants to immediately start with the integration on their suedtirolmobil channels

rcavaliere commented 1 year ago

@dulvui I am not sure that the data we currently have stored is the one that now this new API is providing. In particular for the metadata of the bike parking station we expect different values. Can you clean the test DB with the data we have and start the Data Collector again?

dulvui commented 1 year ago

@rcavaliere Database is clean now and I made some fixes for the metadata. Let me know if you still find problems

rcavaliere commented 1 year ago

@dulvui thanks a lot. I have checked the data entering, I have some questions:

dulvui commented 1 year ago

@rcavaliere Now I added the multilingual fields names and addresses in the BikeParking metadata. Also the mapping should now be correct.

rcavaliere commented 1 year ago

@dulvui thanks a lot, all the comments above are now solved. I will now share the testing API of the Open Data Hub with STA and their supplier, so that they can start the integration. I will ask them if they prefer that we put everything already in production, so that they don't have to change the URL of the requests anymore. I will let you know!

clezag commented 1 year ago

@rcavaliere , @dulvui I've modified the datatype names as requested and put the data collector (and related ninja API changes) into production