Refactor/transform into one module - #513
Conversation
tests diffusion and gestion OK, swagger OK, call of http://localhost:8080/geo/departements and http://localhost:8080/datasets/list OK
…or not at this step, all tests aure OK
…-impl at this step, all tests are OK +delete /old
…pl into magma-impl
…pointsUtils in magma-commons-infra
…utomatically recognized as source root avoid manual action : clic droit sur target/generated-sources/openapi/src/main/java → Mark Directory as → Generated Sources Root
…ause of 2 test databases
+ add a directory for all config test containers classes + add expected json for gestion container tests
whole tests are OK
nsenave
left a comment
nsenave
left a comment
There was a problem hiding this comment.
👍
quelques remarques / questions / commentaires au passage mais le refacto a l'air très propre 👏
| @@ -6,17 +6,16 @@ | |||
| <parent> | |||
| <groupId>fr.insee.rmes</groupId> | |||
| <artifactId>magma-parent</artifactId> | |||
| <version>magma-refactored-1.0-rc0</version> <!-- Hérite de la propriété du parent --> | |||
| <version>2.0.0-SNAPSHOT</version> | |||
There was a problem hiding this comment.
depuis une une certaine version de maven (~3.5+) on peut mettre en commun la version du projet pour tous les modules avec la property revision https://maven.apache.org/guides/mini/guide-maven-ci-friendly.html
There was a problem hiding this comment.
mais question : doit-on avoir le même numéro pour <revision> (présente dans le pom parent - version Maven) et VERSION (présente dans la classe OpenApiService créée suite à cette PR - reflète la version du contrat d'API REST exposé aux consommateurs) ?
There was a problem hiding this comment.
Oui les deux devraient porter la même version, avec la logique semver
There was a problem hiding this comment.
Rien d'important à signaler : quelques remarques ça et là. Je m'interroge juste sur l'utilité du module magma-app désormais
=> GTan : c'est le module de démarrage, pourquoi ne pas le garder ?
Sinon dernière remarque en passant : dans fr.insee.rmes.magma.queryexecutor.QueryExecutor#parseAskResponse , l'objet JsonMapper est un objet fait pour être réutilisatbe et thread safe : il devrait être transformé en un attribut de la classe
=> GTan : fait
| </configuration> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> |
There was a problem hiding this comment.
On ne retrouve plus le yaml de la spec open-api par dépendance transitive sur les resources ?
There was a problem hiding this comment.
explication IA : normalement, quand un module dépend d'un autre (ici magma-fusion-oas est déclaré en dépendance ligne 17-19), les ressources de ce module sont accessibles par dépendance transitive sur le classpath, sans avoir besoin de les extraire manuellement. Pourquoi c'est fait ainsi malgré tout ? Le openapi-generator-maven-plugin attend un paramètre inputSpec qui est un chemin fichier, pas une ressource classpath. Il ne peut pas lire le yaml directement depuis le classpath d'une dépendance. D'où l'étape explicite d'extraction avec unpack-dependencies.
En résumé : c'est un contournement technique nécessaire parce que le plugin OpenAPI Generator ne supporte pas la lecture de specs depuis le classpath — il faut un fichier physique sur disque.
sachant qu'on a jackson dans le projet, on adapte pour se passer de la dépendance org.json
3 participants
No description provided.