Campus-Android icon indicating copy to clipboard operation
Campus-Android copied to clipboard

Migration to eat api

Open Liqs-v2 opened this issue 3 years ago • 29 comments

Issue

resolves: #1370

Why this is useful for all students

Migrating to the latest, still maintained API for getting cafeteria data ensures this component will keep functioning for the forseeable future.

Known issues

  1. CafeteriaCard is not being displayed in the landing
    • This is happening, because menus are no longer being downloaded in bulk at application startup and instead on-the-fly on a by cafeteria basis. This leads to the problem that on the home screen intially, there are no menus stored in the RoomDB yet and a download for the default location needs to be initiated to fix this. A first fix is already in the codebase, however I was unable to verify it, because of issue 2.
  2. Cafeteria menus are no longer downloading
    • Previously this was already working, now I am getting a rather cryptic error message telling me that the response was invalid/unable to be fetched via HTTP (for the error messsage see below, this occured on multiple devices and network connections).
    • 2022-06-15 16:06:12.712 14801-14876/de.tum.in.tumcampus E/AuthenticationManager$$ExternalSyntheticLambda1: javax.net.ssl.SSLHandshakeException: Chain validation failed javax.net.ssl.SSLHandshakeException: Chain validation failed at com.android.org.conscrypt.SSLUtils.toSSLHandshakeException(SSLUtils.java:362) at com.android.org.conscrypt.ConscryptEngine.convertException(ConscryptEngine.java:1134) at com.android.org.conscrypt.ConscryptEngine.readPlaintextData(ConscryptEngine.java:1089) at com.android.org.conscrypt.ConscryptEngine.unwrap(ConscryptEngine.java:876) at com.android.org.conscrypt.ConscryptEngine.unwrap(ConscryptEngine.java:747) at com.android.org.conscrypt.ConscryptEngine.unwrap(ConscryptEngine.java:712) at com.android.org.conscrypt.ConscryptEngineSocket$SSLInputStream.processDataFromSocket(ConscryptEngineSocket.java:849) at com.android.org.conscrypt.ConscryptEngineSocket$SSLInputStream.access$100(ConscryptEngineSocket.java:722) at com.android.org.conscrypt.ConscryptEngineSocket.doHandshake(ConscryptEngineSocket.java:238) at com.android.org.conscrypt.ConscryptEngineSocket.startHandshake(ConscryptEngineSocket.java:217) at okhttp3.internal.connection.RealConnection.connectTls(RealConnection.kt:379) at okhttp3.internal.connection.RealConnection.establishProtocol(RealConnection.kt:337) at okhttp3.internal.connection.RealConnection.connect(RealConnection.kt:209) at okhttp3.internal.connection.ExchangeFinder.findConnection(ExchangeFinder.kt:226) at okhttp3.internal.connection.ExchangeFinder.findHealthyConnection(ExchangeFinder.kt:106) at okhttp3.internal.connection.ExchangeFinder.find(ExchangeFinder.kt:74) at okhttp3.internal.connection.RealCall.initExchange$okhttp(RealCall.kt:255) at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.kt:32) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.kt:95) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:83) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:76) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at de.tum.in.tumcampusapp.api.app.ChaosMonkeyInterceptor.intercept(ChaosMonkeyInterceptor.kt:27) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at de.tum.in.tumcampusapp.api.app.ApiHelper.lambda$getDeviceInterceptor$2(ApiHelper.java:97) at de.tum.in.tumcampusapp.api.app.ApiHelper.$r8$lambda$Ed6Z45oC4dmTmrD4C58bWLoLAoM(Unknown Source:0) at de.tum.in.tumcampusapp.api.app.ApiHelper$$ExternalSyntheticLambda1.intercept(Unknown Source:4) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at de.tum.in.tumcampusapp.api.app.ApiHelper.lambda$disableGzip$1(ApiHelper.java:72) at de.tum.in.tumcampusapp.api.app.ApiHelper.$r8$lambda$OUC4myTOllc53YNAK_DOFdZp_xU(Unknown Source:0) at de.tum.in.tumcampusapp.api.app.ApiHelper$$ExternalSyntheticLambda2.intercept(Unknown Source:0) at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109) at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:201) at okhttp3.internal.connection.RealCall.execute(RealCall.kt:154) at retrofit2.OkHttpCall.execute(OkHttpCall.java:204) at retrofit2.adapter.rxjava2.CallExecuteObservable.subscribeActual(CallExecuteObservable.java:46) at io.reactivex.Observable.subscribe(Observable.java:12267) at retrofit2.adapter.rxjava2.BodyObservable.subscribeActual(BodyObservable.java:35) at io.reactivex.Observable.subscribe(Observable.java:12267) at io.reactivex.internal.operators.observable.ObservableFlatMap$MergeObserver.subscribeInner(ObservableFlatMap.java:165) 2022-06-15 16:06:12.712 14801-14876/de.tum.in.tumcampus E/AuthenticationManager$$ExternalSyntheticLambda1: at io.reactivex.internal.operators.observable.ObservableFlatMap$MergeObserver.onNext(ObservableFlatMap.java:139) at io.reactivex.internal.operators.observable.ObservableDoOnEach$DoOnEachObserver.onNext(ObservableDoOnEach.java:101) at io.reactivex.internal.operators.observable.ObservableSubscribeOn$SubscribeOnObserver.onNext(ObservableSubscribeOn.java:58) at io.reactivex.internal.operators.observable.ObservableFilter$FilterObserver.onNext(ObservableFilter.java:52) at io.reactivex.internal.operators.observable.ObservableScalarXMap$ScalarDisposable.run(ObservableScalarXMap.java:248) at io.reactivex.internal.operators.observable.ObservableJust.subscribeActual(ObservableJust.java:35) at io.reactivex.Observable.subscribe(Observable.java:12267) at io.reactivex.internal.operators.observable.ObservableFilter.subscribeActual(ObservableFilter.java:30) at io.reactivex.Observable.subscribe(Observable.java:12267) at io.reactivex.internal.operators.observable.ObservableSubscribeOn$SubscribeTask.run(ObservableSubscribeOn.java:96) at io.reactivex.Scheduler$DisposeTask.run(Scheduler.java:578) at io.reactivex.internal.schedulers.ScheduledRunnable.run(ScheduledRunnable.java:66) at io.reactivex.internal.schedulers.ScheduledRunnable.call(ScheduledRunnable.java:57) at java.util.concurrent.FutureTask.run(FutureTask.java:266) at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:301) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641) at java.lang.Thread.run(Thread.java:923) Caused by: java.security.cert.CertificateException: Chain validation failed at com.android.org.conscrypt.TrustManagerImpl.verifyChain(TrustManagerImpl.java:724) at com.android.org.conscrypt.TrustManagerImpl.checkTrustedRecursive(TrustManagerImpl.java:554) at com.android.org.conscrypt.TrustManagerImpl.checkTrustedRecursive(TrustManagerImpl.java:575) at com.android.org.conscrypt.TrustManagerImpl.checkTrustedRecursive(TrustManagerImpl.java:620) at com.android.org.conscrypt.TrustManagerImpl.checkTrusted(TrustManagerImpl.java:510) at com.android.org.conscrypt.TrustManagerImpl.checkTrusted(TrustManagerImpl.java:428) at com.android.org.conscrypt.TrustManagerImpl.getTrustedChainForServer(TrustManagerImpl.java:356) at android.security.net.config.NetworkSecurityTrustManager.checkServerTrusted(NetworkSecurityTrustManager.java:94) at android.security.net.config.RootTrustManager.checkServerTrusted(RootTrustManager.java:90) at com.android.org.conscrypt.ConscryptEngineSocket$2.checkServerTrusted(ConscryptEngineSocket.java:161) at com.android.org.conscrypt.Platform.checkServerTrusted(Platform.java:250) at com.android.org.conscrypt.ConscryptEngine.verifyCertificateChain(ConscryptEngine.java:1644) at com.android.org.conscrypt.NativeCrypto.ENGINE_SSL_read_direct(Native Method) at com.android.org.conscrypt.NativeSsl.readDirectByteBuffer(NativeSsl.java:568) at com.android.org.conscrypt.ConscryptEngine.readPlaintextDataDirect(ConscryptEngine.java:1095) at com.android.org.conscrypt.ConscryptEngine.readPlaintextData(ConscryptEngine.java:1079) ... 58 more Caused by: java.security.cert.CertPathValidatorException: Response is unreliable: its validity interval is out-of-date at sun.security.provider.certpath.PKIXMasterCertPathValidator.validate(PKIXMasterCertPathValidator.java:135) at sun.security.provider.certpath.PKIXCertPathValidator.validate(PKIXCertPathValidator.java:222) at sun.security.provider.certpath.PKIXCertPathValidator.validate(PKIXCertPathValidator.java:140) at sun.security.provider.certpath.PKIXCertPathValidator.engineValidate(PKIXCertPathValidator.java:79) at java.security.cert.CertPathValidator.validate(CertPathValidator.java:301) at com.android.org.conscrypt.TrustManagerImpl.verifyChain(TrustManagerImpl.java:720) ... 73 more 2022-06-15 16:06:12.713 14801-14876/de.tum.in.tumcampus E/AuthenticationManager$$ExternalSyntheticLambda1: Caused by: java.security.cert.CertPathValidatorException: Response is unreliable: its validity interval is out-of-date at sun.security.provider.certpath.OCSPResponse.verify(OCSPResponse.java:619) at sun.security.provider.certpath.RevocationChecker.checkOCSP(RevocationChecker.java:709) at sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:363) at sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:337) at sun.security.provider.certpath.PKIXMasterCertPathValidator.validate(PKIXMasterCertPathValidator.java:125) ... 78 more Suppressed: java.security.cert.CertPathValidatorException: Unable to determine revocation status due to network error at sun.security.provider.certpath.RevocationChecker.checkCRLs(RevocationChecker.java:562) at sun.security.provider.certpath.RevocationChecker.checkCRLs(RevocationChecker.java:465) at sun.security.provider.certpath.RevocationChecker.check(RevocationChecker.java:394) ... 80 more Caused by: sun.security.provider.certpath.PKIX$CertStoreTypeException: java.io.IOException: Cleartext HTTP traffic to crl4.digicert.com not permitted at sun.security.provider.certpath.URICertStore.engineGetCRLs(URICertStore.java:430) at java.security.cert.CertStore.getCRLs(CertStore.java:190) at sun.security.provider.certpath.DistributionPointFetcher.getCRL(DistributionPointFetcher.java:245) at sun.security.provider.certpath.DistributionPointFetcher.getCRLs(DistributionPointFetcher.java:189) at sun.security.provider.certpath.DistributionPointFetcher.getCRLs(DistributionPointFetcher.java:121) at sun.security.provider.certpath.RevocationChecker.checkCRLs(RevocationChecker.java:552) ... 82 more Caused by: java.io.IOException: Cleartext HTTP traffic to crl4.digicert.com not permitted at com.android.okhttp.HttpHandler$CleartextURLFilter.checkURLPermitted(HttpHandler.java:127) at com.android.okhttp.internal.huc.HttpURLConnectionImpl.execute(HttpURLConnectionImpl.java:462) at com.android.okhttp.internal.huc.HttpURLConnectionImpl.getResponse(HttpURLConnectionImpl.java:411) at com.android.okhttp.internal.huc.HttpURLConnectionImpl.getInputStream(HttpURLConnectionImpl.java:248) at sun.security.provider.certpath.URICertStore.engineGetCRLs(URICertStore.java:396) ... 87 more
  3. Cafeteria Pricing is not displayed next to dishes
    • As a result of the migration this section was broken and still needs updating (Fetching, storing somehow and updating frontend code)
  4. FavoriteDish functionality no longer works after migration
    • As a result of the migration this section was broken and still needs updating (Fetching, storing somehow and updating frontend code)
  5. Location table and its API are outdated
    • Because the eat-API uses new ids for cafeterias (and also uses slugs) the Location table, that is also referencing cafeteria locations needs to be updated.
    • Unfortunately this data is provided by an API that still provides the outdated ids and I was thus unable to migrate this table as part of this PR.

I would suggest to tackle issue 1+2 in a single PR or potentially add the fix to this one and 3-5 as separate PRs.

Caveat

There probably are some performance issues with the downloading of the menus, that could potentially be moved into a seperate thread and done asynchronously. I have not implemented it in this way so far, due to my inexperience in Android and Kotlin development.

Screenrecordings

Migrated version (before issue 2 started occuring)

eat-api_migration

Liqs-v2 avatar Jan 12 '22 08:01 Liqs-v2

This is still a very rough draft with lots missing, however I would still appreciate some feedback on the direction I have taken with changing the CafeteriaMenu table as well as the migration class that was added. As potential changes needed here are rather easy to do now, but will be increasingly more work as go through with migrating more of the codebase.

@joschahenningsen @kordianbruck

Liqs-v2 avatar Jan 12 '22 08:01 Liqs-v2

This looks great already! You're definitely going in the right direction. The only concern I have is the request size: all.json is 3.5MB (130KB gzipped) whereas the old API is just 205 KB (15 gzipped). Loading this payload in the ubahn on my way to Garching sounds painful :D Also, all.json will grow indefinitely.

This is a little more work but I think it would be best to load only the weeks that are needed right now (perhaps with a week or two padding).

joschahenningsen avatar Jan 12 '22 08:01 joschahenningsen

Good point, will look into that.

Liqs-v2 avatar Jan 12 '22 09:01 Liqs-v2

So it seems like the all.json from the eat-api already only contains the current week, the two past and two upcoming weeks in terms of data it provides. It seems like it just contains a lot more data in general.

For example

  • labels: New field containing an array of dish labels (typically allergens) for each dish
  • price: Is now provided for each dish instead of once and in a standalone field

all.json contains about 1000 more dishes than the old api. Old API: ~1500 entries vs Eat-API: ~2900 entries, which is what could be attributed to the 2 week padding that you suggested, as the old API has no backwards padding and only provides the next two weeks from the current date onwards. I will check in with the Eat-API people and ask whether Eat-API all.json always provides a 2 week padding or if this is some behaviour that one would better not rely on.

all_ref.json could be an alternative, as its backwards padding is only 1 day (ie yesterday) and apart from that only provides future menus. It however still is 1.9 MB.

Liqs-v2 avatar Jan 12 '22 10:01 Liqs-v2

@joschahenningsen Okay, so all.json apparently does not have a specified padding. I would propose using all_ref.json as that is already very close to what you are suggesting.

Sadly that is still a lot larger than the old API, but since it is already only using 2 days padding for the future and one day backwards, I dont really know how to reduce it even further.

Potentially only using 1 week could shave off another couple hundred KB, but would also be a lot more work to implement.

Liqs-v2 avatar Jan 13 '22 14:01 Liqs-v2

Ahh thanks for clearing that up @Liqs-v2

The IOS app seems to load the canteen-specific week when needed: https://github.com/TUM-Dev/Campus-iOS/blob/38a3606ed3d043305724b4bb44a127dfe3afda2f/TUM%20Campus%20App/Cafeteria/MealPlanTableViewController.swift#L40 (e.g. https://tum-dev.github.io/eat-api/mensa-garching/2022/02.json, which is just 70kb (4 gzipped))

If that's too much work, all_ref should do as well though.

joschahenningsen avatar Jan 13 '22 14:01 joschahenningsen

Alright I ll take a look at it. This will probably be more difficult to do though, because the old API used to just load everything, meaning this will be more of a rewrite.

Liqs-v2 avatar Jan 13 '22 14:01 Liqs-v2

But yea I agree loading for a canteen when needed is a better way of handling this.

Liqs-v2 avatar Jan 13 '22 14:01 Liqs-v2

Alright I ll take a look at it. This will probably be more difficult to do though, because the old API used to just load everything, meaning this will be more of a rewrite.

Lmk if there's any way I can assist here :)

joschahenningsen avatar Jan 13 '22 14:01 joschahenningsen

Seems like using all_ref might be a problem: https://github.com/TUM-Dev/eat-api/issues/108

Side note: I opened https://github.com/TUM-Dev/eat-api/issues/109

joschahenningsen avatar Jan 13 '22 14:01 joschahenningsen

Actually there is a thing that I need help with. The Eat-API uses a cafeteria slug as id (e.g: mensa-garching). That means I would need to migrate the Cafeteria table too, since it currently uses an integer cafeteriaId from the old API. However the endpoint this table gets its data from is also outdated then. But I do not have access and or dont know where to find the code for that to change it. So I would be very grateful if u could point me to the repo and give me access or help me update the API. Thanks :)

Liqs-v2 avatar Jan 13 '22 15:01 Liqs-v2

Sure thing! The old api deployed is not open source, that's why we currently rebuild the backend over at https://github.com/TUM-Dev/Campus-Backend/ I can take care of a new endpoint that returns the right cafeteria ids, or we schedule a meeting and I show you around the backend if you want to implement this yourself :)

Once I got #1423 to compile it should be fairly easy to implement a new endpoint.

joschahenningsen avatar Jan 13 '22 15:01 joschahenningsen

Lets have a meeting sometime during the weekend, preferably Saturday. Always good to know how to do that for the future :)

Liqs-v2 avatar Jan 13 '22 15:01 Liqs-v2

Sure thing! The old api deployed is not open source, that's why we currently rebuild the backend over at https://github.com/TUM-Dev/Campus-Backend/ I can take care of a new endpoint that returns the right cafeteria ids, or we schedule a meeting and I show you around the backend if you want to implement this yourself :)

Once I got #1423 to compile it should be fairly easy to implement a new endpoint.

Alright, I guess that means the only real way of doing it is rewriting to do lazy fetching.

Liqs-v2 avatar Jan 13 '22 15:01 Liqs-v2

Saturday works for me, but I think https://tum-dev.github.io/eat-api/canteens.json could be used instead of adapting the API.

joschahenningsen avatar Jan 13 '22 15:01 joschahenningsen

That seems promising indeed, I ll see if I can change the fetching to use this enpoint instead.

Liqs-v2 avatar Jan 13 '22 15:01 Liqs-v2

The switch from an integer cafeteriaId to a string cafeteriaId is causing me some problems, as it comes with a huge cascade of changes throught the entire project and database. Might it be better to introduce some sort of a mapping table for the old cafeteriaId to the new cafeteriaId and leave the components mostly alone?

I am not sure what the best practice is in this case and would appreciate some input. @joschahenningsen

Liqs-v2 avatar Jan 20 '22 17:01 Liqs-v2

@Liqs-v2 What exactly is still needed on this issue? What are concrete tasks I can do to get this PR to a state where this can be merged into master?

CommanderStorm avatar Jun 07 '22 21:06 CommanderStorm

Theres a bug causing the cafeteria card to not be shown which I am still investigating. Apart from that there are some other known issues that should be addressed once this PR is merged. Please be patient for a bit longer.

Liqs-v2 avatar Jun 08 '22 05:06 Liqs-v2

I am sorry If you persieved my message as threatening. It was not meant that way. I just thought that maybe I can help out with one of the remaining issues. xD Sorry..

CommanderStorm avatar Jun 08 '22 08:06 CommanderStorm

I didnt, no worries. All i meant to say is that I think it makes more sense to work on new PRs once this one is merged in, since it is rather old by now. For the other issues seperate PRs would probably be better to keep things small-ish.

Liqs-v2 avatar Jun 08 '22 09:06 Liqs-v2

Alright, so I finally found some time to take another look at this over the past few weeks. I found out why the CafeteriaCard is no longer showing in the Home (Landing) screen and also attempted to implement a fix, but now the Cafeteria component is no longer able to download the eat-api data either. The error seems like some certificate issue, so I was wondering if there were any changes to the eat-api in that regard.

Regardless of this, because I atleast know which section is causing the issue and in the interest of time, I have decided to just put this PR up for review and add this issue to the list of known issues, that I would have put up as a disclaimer anyways.

This means that now would be a good time to jump in and help, if ur still down for that @CommanderStorm :)

Liqs-v2 avatar Jul 02 '22 11:07 Liqs-v2

The file permmissions for gradlew seem to be changed in edf9aca435f4bac9143b0da97f06e8e0a314fda2. 😉 Could you drop the commit to make shure the build (incl klint) runs?

CommanderStorm avatar Jul 04 '22 23:07 CommanderStorm

One of the reasons, why this issue exists you noticed, is that I dont think we currently give feedback to the user, that a menu is not availiable (e.g. mediziner mensa). This looks suspiciously like a certificate error in the log. How should this be handled? Retry 3 times and then show an error message or immedeatly show one?

CommanderStorm avatar Jul 05 '22 00:07 CommanderStorm

The file permmissions for gradlew seem to be changed in edf9aca. 😉 Could you drop the commit to make shure the build (incl klint) runs?

Ah yea, I'll take another look at that. My project was being all weird since I am trying out Windows again since April and I had Linux before, so it didnt really like that.

Liqs-v2 avatar Jul 05 '22 12:07 Liqs-v2

One of the reasons, why this issue exists you noticed, is that I dont think we currently give feedback to the user, that a menu is not availiable (e.g. mediziner mensa). This looks suspiciously like a certificate error in the log. How should this be handled? Retry 3 times and then show an error message or immedeatly show one?

I'll try to see if this error can be reduced to occuring for certain cafeterias and menu availability. But it has been occuring for mensa-garching, which typically does pretty decently with providing menus.

Liqs-v2 avatar Jul 05 '22 12:07 Liqs-v2

But it has been occuring for mensa-garching, which typically does pretty decently with providing menus.

Yep, this is why I said one of the issues ;)

CommanderStorm avatar Jul 05 '22 21:07 CommanderStorm

Any updates on this topic?

COM8 avatar Sep 22 '22 08:09 COM8

No, not really. I tried what @CommanderStorm mentioned, but it didn't work and I also didn't have any other ideas.

I can take another look in the time after my exam and before the semester starts, but quite frankly, my priorities have shifted and I have other commitments at this time and will probably not be contributing much in the coming semester.

Liqs-v2 avatar Sep 22 '22 13:09 Liqs-v2

closing in favor of #1583

CommanderStorm avatar Jun 11 '23 20:06 CommanderStorm