Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Montée de version de l'ensemble des librairies #554

Closed
MaelREBOUX opened this issue Mar 8, 2021 · 22 comments
Closed

Montée de version de l'ensemble des librairies #554

MaelREBOUX opened this issue Mar 8, 2021 · 22 comments
Assignees
Milestone

Comments

@MaelREBOUX
Copy link
Member

à cause de #548 il a été retenu de faire une montée de version de Geotools.

@MaelREBOUX MaelREBOUX added this to the v 1.10 milestone Mar 8, 2021
@pierrejego pierrejego self-assigned this Mar 10, 2021
@catmorales
Copy link

Nous avons un problème avec le passage en version 20.1.1. de georchestra.

2.17.5-georchestra
Révision Git
27792f1f5e8971670a2647663aeeaa64ff7f2f5a
Build Date
24-Feb-2021 10:29
Version de GeoTools
23.5 (rev 00e6fc8f7ea8135bad4512fa2cc36da15e0dcfcc)

@jusabatier
Copy link
Collaborator

On en est où sur cette issue ? C'est en cours où pas encore ?

Sur notre nouvelle plateforme j'ai un souci de compat avec JAVA11, je suspecte fortement que ça vienne de la version de GeoTools utilisée par cadastrapp :

ERROR /getImageBordereau - nouser - norole -o.a.c.i.AbstractFaultChainInitiatorObserver - An unexpected error occurred during error handling. No further error processing will occur.
org.apache.cxf.interceptor.Fault: Could not initialize class org.geotools.data.wfs.internal.v1_x.StrictWFS_1_x_Strategy

@pierrejego
Copy link
Member

Oui j'ai commencé une branche issue-554, mais j'ai un soucis de config swagger et j'ai pas fini les tests.
Je merge teste et merge ta PR, je remets à jour la branche 554 avec le master et je finis les tests.
La PR devrait être bonne pour demain soir

@landryb
Copy link
Member

landryb commented Aug 17, 2021

demain soir ok mais de quel mois ? ;)

@pierrejego
Copy link
Member

pierrejego commented Aug 17, 2021 via email

@pierrejego
Copy link
Member

pierrejego commented Sep 14, 2021

Nouvelle branche https://github.com/georchestra/cadastrapp/tree/issue-554-clean lié à cette anomalie. Avec montée générale des librairies, passage de CXF à Spring MVC et simplification de la gestion des droits utilisateurs/roles/org en utilisant le framework MDC.
Ajout de Swagger pour faciliter l'utilisation du serveur sans client, ou les tests une fois l'API mise en place

Test en cours avant PR

@pierrejego
Copy link
Member

pierrejego commented Sep 17, 2021

Test des services

  • bordereau-parcellaire-controller
    En local problème d'uri ne permettant pas d'avoir l'image a retester deployé
  • co-proprietaire-controller
    • exportCoProprietaireByParcelles
    • exportLotsAsCSV
    • exportLotsAsPDF
    • getCoProprietaire
    • getCoProprietaireList
  • commune-controller
  • configuration-controller
  • csv-export-controller
  • datadir-controller -> a tester déployé pas de datadir en jetty local
  • demande-controller
  • habitation-controller
  • image-parcelle-controller
  • info-bulle-controller
    • getInfoBulle
    • getInfobulleParcelle
    • getInfoUniteFonciere
  • parcelle-controller
    • exportParcellesAsCSV
    • fromParcellesFile
    • fromProprietairesFiles
    • getDnuplaList
    • getParcelle
    • getParcelle (POST) -> nom a changer a terme a passer en restfull /parcelle
  • proprietaire-controller
    • exportProprietaireByParcelles
    • getProprietaire
    • getProprietairesByInfoParcelles -> comportement étrange de la recherche. Les paramètres commune section et numero devrait être obligatoire. Je fais la modification
    • getProprietairesByParcelles
  • releve-propriete-controller
    • createReleveCSVPropriete
    • createRelevePDFPropriete
  • request-information-controller
    • checkRequestLimitation
    • saveInformationRequest
  • request-pdf-controller
    • printPDFRequest
  • section-controller
  • unite-cadastrale-controller
  • voie-controller
  • batiment-controller
    • getBatiments
    • getBatimentsByParcelle

Test limitation accès -(utilisation de modHeaders)

  • limitation geographique par roles
  • CNIL level
  • Limitation geographique par id organisme
  • Presence du username
  • Vérification du format de log après modification du logback

Tests d'Intégration

  • Test appel via addons Mapfishapp -> Sur la PF JDev, moins d'une seconde pour le BP et RP quasi instantané
    - [ ] l'import par lot ne fonctionne plus les opérations fromParcellesFile et fromProprietairesFiles ne comprenne pas le flux venant du formdata en Extjs, j'ai essayé plein de chose, mais rien ne fonctionne je suis à cours d'idée (Pas d'impact pour Mapstore qui n'utilise pas ces opérations)
  • Test appel via module Mapstore -> Sur la PF JDev, Environ 2 secondes pour le BP et RP quasi instantané

@pierrejego pierrejego changed the title Montée de version Geotools Montée de version de l'ensemble des librairies Sep 17, 2021
@pierrejego
Copy link
Member

Reste uniquement les deux services de recherche par import de fichier qui ne fonctionne plus.
En cours de correction.
Tout le reste est opérationnel pour moi.

@pierrejego
Copy link
Member

Démo de la partie Swagger pour faciliter les tests
Cadastrapp-swagger

@pierrejego
Copy link
Member

@MaelREBOUX la rechercher par lot sous forme de fichier ( fromParcellesFile et fromProprietairesFiles ) ne fonctionne pas pour l'addons Mapfishapp. Voir message dans les tests.

Je vais essayer de modifier l'addons pour utiliser la norme et pas la surcouche extjs, mais cela impliquera que les anciennes versions de l'addons ne fonctionneront plus avec la nouvelle version de cadastrapp.

C'est le dernier point avant de proposer la PR.

Tout le reste des tests sont ok avec les deux addons.

En résumé ce qui a été fait :

  • Montée de version de l'ensemble des librairies
  • Passage de JaxRs en SpringMVC (pour un potentiel passage en SpringBoot dans un avenir proche, pour simplifier le déploiement)
  • Le passage en SpringMVC a impliqué une modification de l'ensemble des controllers et interceptors, lors de ces mopdifications, j'ai simplifié la gestion des headers pour les autorisations pour tout mettre dans le contexte MDC.
  • le fichier logback.xml sera aussi à mettre à jour dans le datadir pour prendre en compte la modification des clés MDC
  • ajout de l'outils Swagger-ui pour faciliter les tests
  • passage de la compilation minimum en 1.8 ( compiler en java 11 sur pf de dev)

@landryb
Copy link
Member

landryb commented Sep 20, 2021

Je vais essayer de modifier l'addons pour utiliser la norme et pas la surcouche extjs, mais cela impliquera que les anciennes versions de l'addons ne fonctionneront plus avec la nouvelle version de cadastrapp.

ca on s'en fiche completement, c'est de la responsabilité des admins de maj l'addon mfapp quand on met a jour le backend :)

le fichier logback.xml sera aussi à mettre à jour dans le datadir pour prendre en compte la modification des clés MDC

pr a faire sur https://github.com/georchestra/datadir ? ah ben non en fait, y'a pas cadastrapp dedans :) (y'avait georchestra/datadir#185)

très sympa l'ajout de swagger-ui !!

@catmorales
Copy link

cela impliquera que les anciennes versions de l'addons ne fonctionneront plus avec la nouvelle version de cadastrapp

Ca veut dire qu'il va falloir financer des évolution de cadastrapp dans mapstore et/ou dans mapfishapp, non ? et qu'on ne pourra pas déployer la nouvelle version de cadastrapp avant ?

@pierrejego
Copy link
Member

@catmorales, non en fait, pour le plugins Mapstore c'est bon tout fonctionne. La nouvelle version de l'API cadastrapp c'est transparent.

C'est juste pour l'addons Mapfishapp que ça coince avec le fileUpload de Extjs.
La montée de version Geotools c'est Rennes Métropole qui le finance, tout le reste c'est sur mon temps "Communauté" que c'est fait, donc je vais le faire sans financement pas de soucis. J'ai juste pas la solution toute faite pour l'instant. J'ai essayé deux trois trucs qui ne marchent pas. Il faut que je trouve un moyen de modifier le flux qui est envoyé à l'API ( en plus ça c'est lié au changement Jaxrs vers SpringMVC)

@pierrejego
Copy link
Member

@landryb pour le datadir je pense que c'est lié à #488.

@MaelREBOUX
Copy link
Member Author

C'est juste pour l'addons Mapfishapp que ça coince avec le fileUpload de Extjs.

si c'est juste ça on peut annoncer que c'est déprécié et masquer l'onglet.
je suis pas certain que ce soit très utilisé.
et on est en phase de migration vers mapstore donc...

@landryb
Copy link
Member

landryb commented Oct 29, 2021

je teste cette branche avec java 11 (cf #593), et pour avoir accès a swagger il faut manuellement donner l'url /cadastrapp/services/ pour que l'UI sur /cadastrapp/swagger-ui.html retrouve ses petits - sinon il y'a une alerte javascript disant qu'il n'arrive pas a trouver ses ressources derriere le proxy ?

au passage, je pense qu'il faut que la doc swagger soit par défaut cachée/filtrée/accessible uniquement au SUPERUSER georchestra (via une conf du sec-proxy ?), car n'ayant pas 200% confiance dans l'API de cadastrapp je ne suis pas certain que ce soit une bonne idée de publier l'API au grand public (meme si l'intention est louable pour multiplier les usages/interactions !!!)

dans les accesslog cadastrapp, je trouve étrange que les réponses (log Send response) n'aient pas les infos user/role/org alors qu'elles sont la pour le log de la requete.. mais je suppose que ca n'est pas propagé en interne:

2021-10-29 16:31:01,225 [http-nio-8280-exec-86] INFO  //cadastrapp/services/v2/api-docs - testadmin - ROLE_SUPERUSER;ROLE_MAPSTORE_ADMIN;ROLE_USER;ROLE_ADMINISTRATOR;ROLE_CAD_CNIL2;ROLE_GN_ADMIN - psc -o.g.c.p.CadastrappInterceptor - Incot
2021-10-29 16:31:01,249 [http-nio-8280-exec-86] INFO  / - nouser - norole - noorg -o.g.c.p.CadastrappInterceptor - Send response

@landryb
Copy link
Member

landryb commented Oct 29, 2021

il y'a encore qqs erreurs dans swagger, cliquer sur l'api pour getBatiments me donne:

Resolver error at paths./getBatiments.get.responses.200.schema.items.$ref
Could not resolve reference: Could not resolve pointer: /definitions/Map«string,object» does not exist in document

pierrejego added a commit that referenced this issue Nov 19, 2021
@pierrejego
Copy link
Member

dans les accesslog cadastrapp, je trouve étrange que les réponses (log Send response) n'aient pas les infos user/role/org alors qu'elles sont la pour le log de la requete.. mais je suppose que ca n'est pas propagé en interne:

Corrigé avec le commit 744ee50

@pierrejego
Copy link
Member

au passage, je pense qu'il faut que la doc swagger soit par défaut cachée/filtrée/accessible uniquement au SUPERUSER georchestra (via une conf du sec-proxy ?), car n'ayant pas 200% confiance dans l'API de cadastrapp je ne suis pas certain que ce soit une bonne idée de publier l'API au grand public (meme si l'intention est louable pour multiplier les usages/interactions !!!)

Les services API appelés sont protégés par les roles de la personne. Si les personnes n'ont pas les droits, il voit le service mais pas de réponse. Pas de changement par rapport à avant, la seule chose que j'ai ajouté c'est l'interface Swagger pour faire des tests, mais si tu as pas les droits pour le service ça renvoie rien. ( Biensur si cadastrapp est bien configuré derrière le security-proxy autrement on peut by-passé en modifiant les headers)

@landryb
Copy link
Member

landryb commented Nov 19, 2021

dans les accesslog cadastrapp, je trouve étrange que les réponses (log Send response) n'aient pas les infos user/role/org alors qu'elles sont la pour le log de la requete.. mais je suppose que ca n'est pas propagé en interne:

Corrigé avec le commit 744ee50

nickel merci :)

@landryb
Copy link
Member

landryb commented Nov 19, 2021

Reste uniquement les deux services de recherche par import de fichier qui ne fonctionne plus.
En cours de correction.

il ne reste plus que ca a corriger avant de merger issue-554-clean ? p-e faire une PR avec la branche ?

@pierrejego
Copy link
Member

Reste uniquement les deux services de recherche par import de fichier qui ne fonctionne plus.
En cours de correction.

il ne reste plus que ca a corriger avant de merger issue-554-clean ? p-e faire une PR avec la branche ?

Je suis dessus là, j'ai fini le code je fini les tests. Je fais une PR juste après

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants