Open nfaugout-lucca opened 6 years ago
lucienbertin
commented
6 years ago Je sais pas si c'est ici le bon endroit pour en parler mais qu'en est-il du format api ? est-ce qu'il est iso api/v3 ou ca implemente un standard tierce (swagger ou autre)
rducom
commented
6 years ago Au vu de l'impact en terme de surface d'api, c'est plutot RFC pour v3 :) Je rajouterais volontier le chantier "rapprochement vers OpenAPI/swagger", qui accéssoirement est annoncé dans la roadmap aspnet core 2.1
rducom
commented
6 years ago on est synchro @lucienbertin 😄
nfaugout-lucca
commented
6 years ago Oui, c'est bien que vous parliez de ça.
8. Format d'exposition des API
Soit on garde le format API v3 actuel, en complément d'un format Swagger ou autre, soit on part sur un de ces standard et on abandonne complètement notre format.
Même si je sais qu'un développeur préfère utiliser un outil plutôt que de s'en forger un lui-même, je ne suis pas hyper chaud pour abandonner le contrôle sur notre format. Donc je serais plus pour un mix custom/swagger par ex. Ca permet d'y aller en douceur, et si on voit via les premiers projets à basculer en Swagger que c'est vraiment swag, ben on convertira tous les autres.
lucienbertin
commented
6 years ago on peut en tout cas essayer au7 maximum de respecter la norme openAPI sans integrer de code tiers d'implementation, on pourra s'interfacer avec enormement d'outils tiers sans integrer de code tiers
lucienbertin
commented
6 years ago en plus ca en jete de dire
oui on a un framework rest OpenAPI RDD btw fyi
Poltuu
commented
6 years ago Cache HTTP: -> Très très bonne idée le fait de supporter le cache http -> on avait parlé de simplement virer la partie données liées, avec notamment suppression des includes ? Ca règle tout ces problèmes ET offre le cache HTTP ;) -> Je préfère d'ailleurs cela à ton approche "serialization", pas hyper safe à mon goût (càd difficile à implémenter correctement/garantir que le name ne change pas par exemple/maintenir).
Query web/domain -> gros point, je pense que c'est le chantier refacto le plus simple et le plus impactant pour les apis.
PostedData -> cet objet a fait son temps, et ne suffit plus à plusieurs encornures. Je suis pour une révision globale des besoins liés à cet objet et l'écriture d'un nouveau format. A mon avis, une légère surcouche JToken peut suffir grandement. Il faut, comme raph le signale, bien garder les perfs en vue sur cet objet. -> Même les ICandidate, je suis pas encore totalement vendu ;)
PropertySelector -> absoluement chaud pour un vrai refacto. J'avais d'ailleurs proposé un nouvel objet dans ma PR RDD V2 https://github.com/LuccaSA/RestDrivenDomain/pull/37/files(cf ISelectorsTree.cs et companie) -> tu proposes un changement de mdèle, en passant d'un arbre à une liste. C'est en effet grandement simplificateur. Il faut trouver le bon équilibre, puisqu'on peut avoir besoin de choses telles que .Contains(u=>u.Department), alors que seul existe u=>u.Department.Head, mais c'est à regarder -> virer les sous entités simplifie aussi GRANDEMENT ce champ
SubCollection -> je suis pour l'uniformisation des urls, le choix que tu proposes à d'ailleurs du sens pour moi -> uniformiser/automatiser au niveau du controlelr les routes -> collection est un peu plus chaud, mais ce fait aussi. -> si on vire les sous-entité ... ;)
Principal Claims -> spéciale dédicace à raph -> c'est, il faut bien le dire, sans doute la voie vers laquelle ce projet doit se diriger, je le pense aussi
Contraintes -> c'est pas tellement moins yen a mieux c'est, mais c'est surtout qu'en l'état, elle sont artificielles. -> il ne faut définir comme contrainte que ce que le système a absolument besoin, càd le strict minimum -> et découper en morceaux les interfaces
Swagger ->en fait, il faut que les controllers supportent swagger, mais ya quasi rien à implémenter que je sache non ? Comme lucien dit
lucienbertin
commented
6 years ago @poltuu en fait au lieu de se dire "oui mais comment on fait pour formatter les retours d'api dans le cas de bugublugublug", on regarde si la norme specifie quelque chose et on fait comme ca
lucienbertin
commented
6 years ago genre pour le point 5 SubCollection & Commands, qu'en dit la doc ?
rducom
commented
6 years ago @nfaugout ou pourrait ouvrir une issue "rfc" pour chacun des points (certaines existent déjà) et poursuivre le détail de chaque dessus.
nfaugout-lucca
commented
6 years ago J'ai créé 1 Issue par point, cette Issue permettra de centraliser tout ça.
Poltuu
commented
6 years ago au final, on n'a fait que la 6 et la 7 :/
Je vous propose de discuter ici des changements que j'aimerais faire à l'occasion d'une v2.2 de RDD.
La v2.1 de RDD, toujours en rc, est utilisée en prod dans Lucca.Auth, et donne satisfaction. Il manque des trucs à droite à gauche, mais globalement ça va. On peut continuer à traiter les Issues dans cette version là.
Pour RDD v2.2, j'aimerais faire quelques gros changements (peut-être RDD v3 en fait..), que je vous liste ici.
1. Cache HTTP
Si on veut utiliser le cache HTTP, il faut que nos API renvoie des entités sans enfant, uniquement avec des FK vers les entités liées.
Techniquement, y'a rien à changer dans le Domain, càd qu'il doit continuer à travailler sur des grappes d'entités complètes. C'est dans la couche Web que, même si on a les enfants sous la main, on coupe le lien au moment de la sérialisation en ne renvoyant que les (id, name, url) de ces sous objets.
Du coup, on peut très bien supporter les 2 approches dans la même version de RDD, càd avoir 2 sérialieurs :
Le fait de supporter les 2 et de gérer ça au niveau Web permet de ne pas remettre massivement en cause RDD.Domain, et donc finalement de pouvoir changer d'avis, si jamais un premier POC sur un projet cobaye s'avère catastrophique.
2. Découpage de Query en Web vs Domain
On sépare les concerns sur l'objet Query, la discussion est déjà engagée dans https://github.com/LuccaSA/RestDrivenDomain/issues/83
L'idée c'est de conserver cet objet, qui permet de structurer une "demande" au niveau d'une Collection, tout en enlevant 2 catégories d'éléments :
3. Virer PostedData au profit de ICandidate
Dans Lucca, l'objet de PostedData était de pouvoir décorer un JSON entrant (POST, PUT), pour que le Domain puisse valider la req (est-ce que telle propriété est présente, etc...)
Romain a fait une version alternative qui utilise directement les JToken de Json.Net, c'est mieux, mais ça manque de contrôle sur le côté introspectif de l'objet posté.
Le mini POC https://github.com/LuccaSA/RestDrivenDomain/pull/93 vise à proposer un embryon de ce que pourrait être ce concept de ICandidate.
L'idée c'est que la couche web prend le JSON entrant et en fait un ICandidate, avec un .Value qui correspond à la désérialisation brute vers T sur JSON. Intéressant pour une Collection basique qui enregistrera tel quel l'objet après avoir joué la Validation basique.
Et des accesseurs, genre .HasProperty(..) qui prend une expression et permet de savoir si le JSON contient telle ou telle propriété, et ainsi pouvoir aiguiller le code métier en fonction.
En gros on aura 2 types de Collections :
4. Linéariser PropertySelector
PropertySelector est un wrapper d'Expression pour indiquer qu'on sélectionne une propriété assez profonde dans une lignée d'objets (ex : u => u.Department.Members.Select(m => m.Name))
Interroger l'expression directement en quête de savoir si elle contient tel ou tel accesseur est délicat. C'est l'objet de PropertySelector.
Le pb c'est qu'au lieu de ne contenir qu'une seule Expression, il en contient N, et ça nuit à sa simplicité je trouve.
J'aimerais donc notamment que Fields se repose sur une collection de PropertySelector plutôt qu'un seul PropertySelector !
Ca permet de simplifié PropertySelector et de l'utiliser pour d'autres usages par la suite, notamment dans les Candidates.
5. SubCollection & Commands
J'aimerais ici qu'on s'attaque à une problématique récurrente, celle des sous API, ou sous Collection.
PUT /api/users/123/friends/2ici on veut mettre à jour le 2ème Friend du User 123Est-ce que le code qui gère cet appel est dans la UsersCollection ? Ou dans une UserFriendsCollection ? Si oui, quelle relation entre ces 2 collections ?
On voit un pattern se dessiner, c'est qu'à chaque fois qu'on est dans ce genre de cas, on est sur 1 seul objet principal (ici le User 123), et sur un seul ou une collection de sous objets (les Friends).
On pourrait donc créer un concept de SubCollection (notamment pour isoler le code dédié aux Friends de celui dédié aux users directement, et qui se trouve dans la UsersCollection), et indiquer qu'elle dépend d'une Collection ou d'un TEntity "parent".
Ainsi, le
UpdateByIdAsync( TKey id )deviendrait unUpdateByIdAsync( TParent parent, TKey id ), càd qu'on fournirait à la SubCollection l'entité parent correspondant à l'appel, et les données entrantes notamment l'ID de la sous entité concernée.On pourrait mettre ce code sur la classe TParent directement, en mode DDD puriste, mais ce n'est pas pratique vis-à-vis de l'injection de dépendance, car du coup on va devoir mettre des dépendances sur l'entité parent, ce qu'on veut tous éviter j'ai l'impression. Et ce serait également contraire à SRP, qui dit qu'une entité ne doit pas "trop" en faire.
D'une sous entité à une commande il n'y a qu'un pas.
POST /api/users/123/move, un move étant un déménagement, avec une adresse, une date prévisionnelle, etc..Ici on est dans le même cas de figure, et si on considère que les commandes sont des entités à part entière, alors le cas revient au cas précédent = pas de différence entre un Friend et un Move.
6. Principal & Claims
.NET ayant fait d'énorme progrès dans la représentation des droits de l'utilisateur connecté, on pourrait utiliser les Claims et le concept de Principal M$ en lieu et place de notre IPrincipal maison avec ses permissions.
Y'a du boulot de recherche car il faut sélectionner la bonne approche (celle avec les Role me semble la plus adapter à notre besoin), et voir si elle couvre tous les cas d'usage.
Ceci déboucherait sur l'utilisation de MiddleWare d'authentification et d'authorization standard.
7. Relâcher les contraintes
On me demande régulièrement de mettre moins de contraintes sur les TEntity. On a déjà fait une partie du boulot, mais il reste des cas en effet où on se demande pourquoi on doit implémenter tel ou tel propriété.
Sur EntityBase, on pourrait déjà virer Id, Name, car de toute façon ils sont déjà imposés par IEntityBase en get seulement, ce qui semble mieux adapter à la plupart des situations.
Sur les Repo ou les Collection, on pourrait imaginer des Repo ou des Collection de type non-EntityBase. Il faudrait alors faire la distinction entre un type qui a une API racine (ex : User), et un type qui n'a qu'une sous API (ex : Move ou Friend), et qui n'a sans doute pas besoin d'être un EntityBase.
Reste à voir les modalité de ce changement.
Est-ce que vous voyez d'autres trucs que vous aimeriez voir changer ? Ou des trucs qui ne sont pas dans RDD et que vous aimeriez ajouter ?
N'hésitez pas à référencer chaque point numéroté ci-dessus dans vos propositions, et mettre des numéros sur les nouveaux trucs que vous proposez, ça permet de suivre la discussion.
Une fois qu'on arrivera à un consensus sur ce qu'on fait / ce qu'on ne fait pas, on pourra lancer des Issues individuelles détaillées de chaque point, puis des PR associées.