ZEP-30 : Elaboration de l'API des forums

Désolé mais je reste catégorique. Tu me décris précisément le mécanisme des permissions et ceci doit être géré par le code de réponse HTTP.

Absolument pas. Désolé Andr0 mais là je ne suis pas du tout d'accord.

C'est à peu près comme dire : il est inutile d'afficher les permissions sur les fichiers, si le mec l'ouvre en écriture et essaie de le sauvegarder, il va se faire jeter de toute façon.

Non, les erreurs HTTP sont là pour remonter des ERREURS du client. C'est une façon propre d'indiquer quel type d'erreur il a faite. Et accessoirement, cela permet de protéger le serveur (essentiellement l'erreur 400). Et oui, cela décrit, côté serveur, la gestion des permissions sur une ressource. Simplement, mets-toi deux secondes à la place d'un client.

Je veux afficher un topic, avec le bouton "éditer" à droite de la page, (appli mobile, web, …). Comment je détermine si le bouton doit être affiché ? Je fais une requête bidon de modification et je regarde si elle passe ? Tu bouffes complètement inutilement des ressources côté client et côté serveur (et un round-trip réseau qui ne ser à rien). Et si la requête passe, tu as potentiellement des effets de bord complètement indésirables (date de dernière modification changée, donc une indication fonctionnelle pour… une simple raison bassement technique : ça n'a pas de sens).

Les codes d'erreur HTTP permettent de gérer proprement les permissions côté serveur, et d'indiquer DES ERREURS je me répète, pas de donner une indication. L'API doit se mettre au service des besoins des clients. Là, clairement, il y a une gestion de l'affichage différente suivant qui visionne un sujet, un message, etc. C'est à l'API de fournir cette information au client. Pas au client de :

• la deviner (si les règles de modification changent, ça fait deux endroits où la changer)

• tester pour savoir

La façon la plus propre de faire ça au sens REST du terme (encore mieux qu'un flag, je dirais), c'est d'indiquer, soit dans les headers soit en payload de la requête les URL de modifications d'une ressource si, et seulement si, l'utilisateur en possède les droits.

PS : et non, ce n'est certainement pas de la pollution d'information ou du bruit. C'est une information essentielle pour tout client.

PPS : Exemple ? L'API Github :

https://developer.github.com/v3/repos/

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11

[
  {
    "id": 1296269,
    /* ... */
    "permissions": {
      "admin": false,
      "push": false,
      "pull": true
    }
  }
]

Javier: Je n'ai rien contre prévenir la possibilité d'informer en réponse si on peut éditer ou non la ressource courante, je suis contre renseigner cette information dans le résultat de la requête (le JSON que nous renvoyons). Je ne veux pas d'un attribue can_edit partout qui va polluer le résultat.

Relis mon message, je suis à 100% POUR une façon d'indiquer les permissions dans la réponse. Qu'ils s'agisse d'un attribut ou d'un header ou de n'importe quoi.

En l'occurrence, si le système de permission est complexe (exemple de Github ci-dessus), un header ne suffira certainement pas, il faut l'indiquer dans la charge utile. Et je me répète c'est une information FONCTIONNELLE qui ne pollue donc pas du tout la charge utile puisqu'elle a du sens fonctionnellement pour l'utilisateur de l'API.

J'avais répondu après l'édition de ton message précédent avec ton exemple GitHub. C'est sans doute moi qui est dans le faux alors allons sur un bloc permissions sur toutes les ressources …

Là où tu as raison c'est qu'il faut faire extrêmement attention aux informations renvoyées aux clients (il faut systématiquement se poser la question de leur intérêt fonctionnel).

En l'occurrence, qu'est-ce-que le client va en faire ? Est-ce-que ça a un sens fonctionnel ? Est-ce-que l'API en tire partie directement ?

Là, oui. Clairement les clients vont orienter leur présentation de la ressource différemment en fonction de cette information. (c'est le cas pour Github aussi).

GitHub a une approche différente pour les messages par exemple. Ils n'ont pas mis de "can_edit" en se disant que de toute façon seul l'auteur du message peut l'éditer, que c'est une règle fonctionnelle gravée dans le marbre et qu'il n'y a aucun intérêt à l'indiquer. Ici c'est plus limite, parce que le staff peut également éditer des messages… Et potentiellement les groupes d'utilisateurs pourraient changer, etc. Du coup, cette information a réellement de la valeur.

Si je résume les derniers débats, nous partons sur ces modifications à rajouter à la spec :

Routes

* : Il faut être authentifié

** : Il faut appartenir au staff

Autres

Karma

Les messages disposeront d'un objet karma avec toutes les informations relatifs au +1/-1. Dans un premier temps, il n'y figurera que le nombre de like et dislike mais si retirons l'anonymisation des +1/-1, nous y ajouterons la liste des utilisateurs pour tous les likes et dislikes.

v1 :

1
2
3
4
5
6
7

{
  "karma": {
    "like": 42,
    "dislike": 0,
    "status": "liked" # status: none | liked
  }
}

v2 :

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18

{
  "karma": {
    "like": {
      "count": 42,
      "result": [
        { ... },
        { ... },
        ...
      ]
    }
    "dislike": {
      "count": 0,
      "result": [
      ]
    },
    "status": "liked" # status: none | liked
  }
}

Permissions

Toutes les ressources retournent l'objet un objet permissions pour spécifier si le staff, un utilisateur connecté ou un auteur peut ou non édité la ressource demandée.

1
2
3
4
5
6
7
8

{
  ...
  "permissions": {
    "staff": true,
    "auteur": true,
    "membre": false
  }
}

Je vais peut-être dire une bêtise, mais pourquoi conserver le pk du forum dans les adresses liées aux sujets et messages autres que /api/forums/:forum/sujets (Lister tous les sujets d'un forum ou Créer un sujet dans un forum) ?

Ne pas le mettre éviterait d'avoir à gérer des redirections 301 quand un sujet est déplacé vers un autre forum, et laisse la place à une éventuelle évolution qui rajouterait de nouveaux niveaux de hiérarchie (comme des sous-forums), qui exigerait sinon de refondre toutes les adresses concernées.

J'ai bien pensé au fil d'Ariane qui permet de remonter vers le forum où se trouve le sujet, mais si j'ai bien compris le cours sur REST, une telle information correspond plutôt à la partie HATEOAS (où aller ensuite ?) et devrait être renvoyée par l'API et non connue à l'avance par le client.

Dans le même ordre d'idée, pourquoi les catégories sont-elles directement à la racine de l'API ? Il existe aussi des catégories pour les tutos et articles, et elles n'ont rien à voir avec celles des forums. J'ai l'impression qu'il faudrait plutôt avoir des adresses /api/forums/categories à la manière de /api/forums/tags.

Concernant la liste des votant (dis)like, je me rappelle que dans un sujet il avait été dit (pour le site en tout cas) que pour des raisons de performances cette infos ne serai pas chargée de base mais récupérée en ajax lorsqu'un utilisateur survolerai les (dis)likes.

Du coup ne faudrait-il pas envisager la même chose pour l'API ?

Rien n'est débile !

Comme dit un peu avant, je me suis inspiré des routes des issues d'un repo de GitHub mais, et je n'y avais pas pensé, il n'est pas possible de déplacer une issue d'un repo à un autre comme il est possible de le faire pour un sujet d'un forum à un autre. Du coup, ce que tu dis est tout à fait pertinent !

Je cite la raison pour laquelle j'avais renseigné le forum dans l'URL des sujets et des messages :

Ca me fait dire qu'un sujet n'est pas tant que ça identifiable par son forum. J'opterais donc pour des routes du types :

• /categories
• /forums
• /sujets
• /sujets/messages

Sur le site, il n'y a aucune redirection faite et rien n'est prévu dans le back-end pour rajouter des niveaux. Ce sont 2 faux problèmes mais qui n'ont que peu d'importance.

Effectivement, nous avons un problème et je ne parviens pas à trouver une solution satisfaisante. Malheureusement, ce que tu me proposes n'est pas une bonne solution pour 2 raisons :

1. /api/forums/categories/ signifierait que tu veux toutes les catégories dans tous les forums, contrairement à /api/forums/tags/ qui signifie bien tous les tags dans tous les forums.
2. Même si nous décidons que le premier point n'est pas important (difficilement concevable), il faut rester cohérent avec les autres réelles et donc avoir /api/forums/forums/ et /api/forums/sujets/. C'est vraiment pas beau.

C'est un peu chiant. Je vais y réfléchir mais je suis open à toutes les propositions !

Première petite remarque, comment pouvons-nous savoir que nous aurons des problèmes de performance pour quelque chose qui n'existe pas ? oO

Mais surtout, je ne vois pas bien le lien. L'API n'est pas le site Web. Ce qui pourrait poser des problèmes de performance sur le site (dans l'hypothèse où nous en aurions), c'est tu veux afficher tous les messages d'un sujet et tu charges directement toutes les personnes qui ont like/dislike ces messages. Hors, dans une API, tu demandes la ressource sans aucun contexte. Après, ça sera au site d'utiliser l'API (ou une vue designé pour supporter l'Ajax) pour charger ces données à la demande (en l'occurrence, au survol des likes/dislikes). Avec l'API, tu survoles rien du tout. Tu fais juste tes requêtes et tu veux avoir en retour les données que tu as demandé.

La requête Ajax du site utilisera l'API pour faire sa requête. Puis, comment pouvons-nous avoir des problèmes de performance pour

Concernant le problème de performance c'était de l'anticipation du problème.

Et sa paraît logique, lorsque l'on fait un /api/forums/:forum/sujets/:sujet (GET sur un sujet) on récupère tous les messages de celui-ci ainsi qu'une jointure sur les différents participants pour afficher le pseudo/avatar.

Si on doit également pour chaque message faire une jointure pour l'ensemble des personnes ayant mis un (dis)like sa sera forcément plus lourd.

Je crois que lorsque l'on récupère un sujet, on doit récupérer l'ensemble des messages, les infos sur les auteurs ainsi que le total de (dis)like mais ne pas récupérer les utilisateurs ayant (dis)liker. Pour moi il faudrait une route du style /api/forum/like/:id_message

De base, les ressources externes ne sont que des identifiants. Il faut explicitement dire qu'on veut "étendre" un objet (?expand=...) pour les afficher. Donc, quand on demandera la liste des utilisateurs des likes/dislikes, on aura que les identifiants par défaut.

D'accord… mais pour avoir cette liste il aura déjà fallut faire une jointure par message pour l'obtenir.

Moi je souligne sa juste pour limiter le coût d'une requête sur l'API, comme l'a dis javier, il faut déterminer si l'informations que l'on renvoie est pertinente. Hors ce n'est pas une informations qui sera utile systématiquement (qui va afficher la liste des votant de tous les messages par défaut ?)

D'accord… mais pour avoir cette liste il aura déjà fallut faire une jointure par message pour l'obtenir.

Je ne suis pas un expert Django mais je ne pense pas non. L'identifiant est l'information sauvegardé dans la table, pas besoin de requête pour y accéder.

Moi je souligne sa juste pour limiter le coût d'une requête sur l'API, comme l'a dis javier, il faut déterminer si l'informations que l'on renvoie est pertinente. Hors ce n'est pas une informations qui sera utile systématiquement (qui va afficher la liste des votant de tous les messages par défaut ?)

Aucune idée mais ce que font les gens avec une API, ça dépend de l'imagination de chacun. Tu pourrais très bien avoir une application qui fait des stats sur ce genre de données. Des idées, il peut y en avoir plein.

Ceci étant dit, cette information là n'existe même pas et personne n'est disponible pour la développer avant un moment donc c'est aussi un faux problème.

L'information existe… elle est quelque par dans le modèle, c'est simplement que sur le site elle n'est pas exploitée pour être affichée (mais exploitée rien que pour savoir si l'utilisateur à ou non déjà voté pour ce message).

Oui mais c'est une liste… donc dans une table différente que celle des messages; donc jointure.

L'information existe… elle est quelque par dans le modèle, c'est simplement que sur le site elle n'est pas exploitée pour être affichée (mais exploitée rien que pour savoir si l'utilisateur à ou non déjà voté pour ce message).

Je peux te dire avec certitude que, pour l'instant, c'est qu'un simple compteur. Rien n'est prévu et développé ni dans le back, ni dans le front.

Si, c'est développé : y'a deux modèles déjà utilisés CommentLike et CommentDislike. Par contre je ne sais pas si ces modèles ont les bons indexes pour supporter l'API.

Le compteur des (dis)like est dé-normalisé on est bien d'accord. Mais la liste en elle-même existe bien, je re-cite le message que j'ai cité juste au dessus:

Oe sauf qu'évidement le code à évoluer depuis c'est de la ligne 252 à 277 qu'il faut regarder.

Ouaip, j'en ai discuté avec sandhose et artragis. Je n'avais pas vu que le lien pointait vers la classe juste en dessous et j'avais encore moins vue que les choses étaient faites comme ça …

Sachant que nous statuons toujours sur le fait de rendre anonyme les anciens likes/dislikes ou pas et même de savoir si on veut cette fonctionnalité dans le site ou non, ça me semble plus sage d'écarter cette information de l'API pour l'instant. Nous pourrons l'intégrer quand la décision sera prise définitivement et mise en oeuvre sur le site.

Bien, aux dernières nouvelles, il ne restait qu'un seul point bloquant avant d'arriver à une spec finale, l'URL des catégories. Pour rappel, il existe des catégories pour des forums, des articles et des tutoriels. Ces catégories ne partagent pas toujours la même classe techniquement et donc, ne sauvegardent pas les mêmes données (ou différemment).

Alors comment faire ?

J'ai 2 propositions :

La première renseigne d'abord le module avant la ressource. Lister toutes les catégories d'un forum ressemblerait à /forum/categories, pour les articles /articles/categories/ et pour les tutoriels /tutoriels/categories/. Mais toutes les autres URLs ne renseignent pas le module, elles indiquent la ressource voulue directement, plus précisément le chemin à travers toutes les ressources.

Cette solution séduit par son côté simple mais elle ne s'intègre pas aux autres URLs. Toutes les URLs de l'API à travers tous les modules existants (membre et mp) ne renseignent jamais le module dans leur URL. Elle ferait alors de cette solution un cas particulier non naturel pour l'utilisateur final.

La seconde demande du travail. Aujourd'hui, les forums utilisent techniquement des catégories différentes des articles et des tutoriels. Un pré-travail doit être appliqué pour unifier ces catégories et n'avoir plus que un seul modèle parent pour toutes ces ressources. Category deviendrait la catégorie parente. Les forums et les contenus (articles et tutoriels) disposeraient de leurs sous classes à ce modèle.

A partir de ce refactoring, l'URL /api/categories/ prendrait du sens. Elle listerait toutes les catégories de tous les contenus. /api/categories/?type=forum listerait toutes les catégories des forums et de même pour les autres contenus.

Je préfère la seconde solution mais j'aimerais vos avis.

PS : Merci à pierre_24 et sandhose qui ont fait évolué mes idées de départ sur IRC.

Tu dis que pour les autres modules on ne préfixe pas les urls par le nom du module, le problème ne se pose finalement que maintenant car il y a un conflit ici sur les catégories.

Ne risquons-nous pas lors de la réalisation d'un autre module d'API de rencontrer une même incompatibilité du même style ?

N'avons-nous pas intérêt de préfixer systématiquement toutes les url de l'API par le nom du module ?

Nous parlons ici des ressources au premier niveau ; c'est à dire "membres", "mps", "sujets", "forums", "catégories", "tutoriels" et "articles". Les sous ressources sont spécialisées par l'une de ces ressources au premier niveau (aucun risque de conflit sur ces sous-ressources). S'il y a un conflit dans les ressources du premier niveau, c'est une indication comme quoi il y a un problème dans l'organisation de nos ressources et l'information du module est purement technique, elle n'apporte rien à l'URL, voire la pollue (comme je l'explique dans mon message précédent).

Je pense que nous avons tout intérêt à unifier les catégories.