ministero-salute / it-dgc-verificac19-sdk-android

Digital Covid Certificate SDK
Apache License 2.0
57 stars 30 forks source link

Hilt Error #82

Closed bobbysimon24 closed 1 year ago

bobbysimon24 commented 2 years ago

Buonasera a tutti,

Ho un problema con l'implementazione delle sdk, oggi ho aggiornato diversi componenti di android studio (Es Kotlin e Gradle Version, oltre a qualche implementazione). Dopo averlo fatto mi sono accorto che mi ha cominciato a dare fastidio il componente Hilt che se non sbaglio serve per l'injecting di alcune classi delle SDK. In pratica mi va in errore quando buildo se lascio @HiltAndroidApp sia su VerificaApplication che sulla mia application (MyApplication), e se lo tolgo solo dalla mia crasha quando provo ad avviare un'activity, quindi l'unica soluzione per ora che ha risolto il problema è stato togliere l'annotazione dalla verificaApplication, però non mi sembra giusta come operazione visto che andrebbero lasciate invariate le SDK. Sapete dire dove sbaglio? Vorrei anche capire quale sarebbe l'implementazione giusta, quella che avevo prima andava bene o è giusto che mi vada in errore ora?

Inoltre un altro problema sorto dopo queste operazioni è che mi va in errore per questi campi nel manifest delle SDK, io ho risolto eliminandoli ma come nel problema di prima non mi sembra molto elegante, i campi sono questi: android:icon="@mipmap/ic_launcher" android:label="@string/app_name" android:roundIcon="@mipmap/ic_launcher_round" android:supportsRtl="true" android:theme="@style/AppTheme"

rawmain commented 2 years ago

Ciao @bobbysimon24

Ho un problema con l'implementazione delle sdk, oggi ho aggiornato diversi componenti di android studio (Es Kotlin e Gradle Version, oltre a qualche implementazione). Dopo averlo fatto mi sono accorto che mi ha cominciato a dare fastidio il componente Hilt [...] quindi l'unica soluzione per ora che ha risolto il problema è stato togliere l'annotazione dalla verificaApplication, però non mi sembra giusta come operazione visto che andrebbero lasciate invariate le SDK.

Già solo aggiornando Gradle Version (6.5->7.0.2 in gradle-wrapper.properties / project structure) per upgrade Gradle Plugin (4.1.3->7.0.4 suggerito in event log), hai dovuto apportare ulteriori modifiche di allineamento progetto per ok sync, tra le quali anche un cambio hilt_version (e.g. 2.33-beta->2.40.5).

Ciò comporta comunque questioni lato hiltAggregateDepsRelease, vista la differente validazione d'uso @HiltAndroidApp nelle compilation unit - già a partire da hilt-version 2.35.

Avendo infatti rootannotation = HiltAndroidApp.class per 2 root differenti nella stessa compilation unit (prima per it.ministerodellasalute.verificaC19sdk.VerificaApplication e poi per tua MyApplication), viene registrata la relativa eccezione. Ecco anche perché non viene sollevata, quando appunto procedi temporaneamente a rimuovere l'annotation nel modulo dgc-sdk.

Stando finora alle (mie veloci) prove effettuate, non risulterebbe efficace il solo/semplice override - previo (ri)allineamento dipendenze kotlin related - tramite impostazione dei flag appositi in compile options per disabilitazione controlli/validazioni hilt.

Pur risultando correttamente in log la disabilitazione dei controlli cross/class validation per HiltAndroidApp, l'eccezione non viene comunque bypassata in compilazione. Vedrò nei prossimi giorni eventuali variazioni, apportando diverse combinazioni di modifica/override.

Vorrei anche capire quale sarebbe l'implementazione giusta, quella che avevo prima andava bene o è giusto che mi vada in errore ora?

Se il tuo intento fosse quello di apportare modifiche per una tua app-produzione, rispetto a definizioni correnti version/dependency in whitelabel project/app, allora l'ideale (IMHO) sarebbe mantenere comunque un approccio "keep it simple".

Tradotto, evitare modifiche locali & minimizzare override per quanto concerne DGC-SDK e EU DCC core, operando solo lato definizioni project base e modulo app (come fatto p.es. per cambio targetSdk in app-produzione - ved. issue #67).

Inoltre un altro problema sorto dopo queste operazioni è che mi va in errore per questi campi nel manifest delle SDK, io ho risolto eliminandoli ma come nel problema di prima non mi sembra molto elegante

Avendo cambiato gradle-version + modificato altre definizioni/dipendenze del progetto, hai variato anche la gestione manifest merge, per cui - in assenza p.es. di specifici tools:replace definiti nel manifest app - ci sta che vi sia un'eccezione per duplicati da precedente dichiarazione manifest. Infatti non la rilevi - p.es. commentandoli/rimuovendoli temporaneamente nel manifest dgc-sdk.

bobbysimon24 commented 2 years ago

Grazie @rawmain,

Quindi in pratica se ho capito bene quello che mi hai detto è che è meglio non provare ad aggiornare i componenti dell'app per mantenere un approccio "keep it simple"?

Inoltre mentre sistemavo queste cose mi sono chiesto, ma è normale che il workmanager viene istanziato in VerificaApplication ma che quest'ultima non venga mai eseguita? Perchè io fino ad ora lo istanzio nella mia di application altrimenti non parte il LoadKeysWorker, e anche questo non mi sembra molto corretto come approccio visto che si duplica il codice.

rawmain commented 2 years ago

Ciao @bobbysimon24

Grazie @rawmain

Prego ;).

è meglio non provare ad aggiornare i componenti dell'app per mantenere un approccio "keep it simple"?

Corretto.

D'altronde, proprio in virtù dei vincoli DPCM 12/10/2021 per uso autorizzato SDK in app-produzione, puoi comunque apportare solo modifiche/override di progetto - richieste strettamente dal tuo codice app/UI - che A. non rompano funzionalità / flussi di validazione e B. non richiedano modifiche locali - non ufficiali al codice moduli SDK e Decoder/Core.

Vedasi appunto esempio citato di valutazione/test d'impatto per approccio alle modifiche in issue #67 (innalzamento targetSdk app rispetto a specifiche base di progetto).

è normale che il workmanager viene istanziato in VerificaApplication ma che quest'ultima non venga mai eseguita? Perchè io fino ad ora lo istanzio nella mia di application altrimenti non parte il LoadKeysWorker, e anche questo non mi sembra molto corretto come approccio visto che si duplica il codice.

Operi comunque in modo normale/corretto, visto che è appunto lato codice modulo app che definisci inizializzazione & gestione WorkManager (ved. anche eventuali personalizzazioni per trigger fetch worker & riduzione periodicità base <24h).

Per il resto, vero, ci sono ancora alcuni residui di codice non utilizzato/necessario strict - rimasti a seguito dello scorporo codice, effettuato a Settembre & che ha portato appunto alla distinzione moduli app/UI e SDK.

Già in corso il relativo redundant-code refactoring per prossime versioni SDK (e conseguente adattamento anche di verifier-app) = prime evidenze pub già visibili oggi in feature branches.

Nel frattempo, la loro persistenza in develop branch non impatta comunque build/runtime di test-release.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.