Proposition: des vrais Controllers pour les routes /api et /BazaR

[srosset81]

Situation actuelle

Maintenant que, grâce au travail de @seballot, nous pouvoir avoir des actions sous formes de classes, une nouvelle classe BazarAction a été créée.

Comme cette action est très complexe, gérant le rendu d'une vingtaine de pages différents selon la query string (à noter que les fonctions baz_gestion_formulaire et baz_gestion_listes gèrent elles-même des sous-pages), il va être difficile de tout mettre à l'intérieur de la classe BazarAction.

Du coup je pense créer des services qui auront pour unique but de faire des rendus de pages (au final via Twig, mais dans un premier temps ce pourra être via Squelette ou même en renvoyant directement le HTML, le système de Sebastian permettant de gérer tous les cas de figures).

Ces services seraient de sortes de Controllers (au sens de Symfony) nommés en fonction du type de pages rendues:

• EntryController -> gestion des fiches
• FormController -> gestion des formulaires
• ListController -> gestion des listes

La classe BazarAction servirait donc uniquement à appeler la bonne méthode du bon Controller, en fonction de la query string.

Je pense cependant qu'il pourrait y avoir une manière plus élégante et plus lisible de gérer ces Controllers. Continuez à lire...

Le cas des nouvelle pages API

Récemment @mrflos a introduit une nouvelle manière de gérer les APIs, dans le but que chaque extension puisse facilement proposer ses propre APIs JSON. Pour cela il a réservé le mot-clé api: si l'on va sur http://localhost:81/?api, au lieu d'afficher une page wiki normale, on tombe sur une page spéciale. Ensuite si on va sur http://localhost:81/?api/fiche/1, cela va afficher un JSON avec toutes les fiches du formulaire N°1.

Au niveau du code, cela demande de gérer la page api différemment. Dès que ce mot-clé est repéré, c'est le nouvel ApiService qui prend le contrôle du rendu (les handlers, par exemple, ne sont plus pris en compte). Ce service regarde si un fichier .api.php existe dans les extensions, pour voir s'il faut charger d'autres types d'APIs.

Ca marche bien, mais on est encore dans une manière de faire un peu à l'ancienne par rapport à ce qu'on est en train de mettre en place.

Une alternative plus propre serait de gérer ces APIs de la même manière que Symfony gère les Controllers, c'est-à-dire avec des annotations qui permettent de définir directement au-dessus de la méthode la route qui appelle la méthode:

namespace YesWiki\Bazar\Controller;

class BazarApiController extends AbstractController
{
    /**
     * @Route("/api/fiche/{id}")
     */
    public function displayEntry($id): Response
    {
        // Return Twig template, or Squelette, or plain HTML string
        return $this->render(...);
    }
}

Définir BazaR comme une page spéciale

Actuellement toutes l'administration du Bazar est gérée via une action {{ bazar }} qui est insérée à l'installation de YesWiki dans la page http://localhost:81/?BazaR. Mais ne pourrions nous pas simplement définir cette page BazaR comme une page spéciale, à la même manière que la page api ?

Au lieu d'avoir une énorme BazarAction qui propose une vingtaine de rendus différents en fonction de la query string, on pourrait ainsi créer des Controllers qui appelerait les méthodes en fonction de la route définie:

namespace YesWiki\Bazar\Controller;

class EntryController extends AbstractController
{
    /**
     * @Route("/BazaR/entry/create/{formId}")
     */
    public function createEntry($formId): Response
    {
        // Return Twig template, or Squelette, or plain HTML string
        return $this->render(...);
    }
}

En fait, la gestion serait exactement la même que pour la page spéciale api, il n'y aurait rien à changer si ce n'est de définir BazaR comme une page spéciale.

Au lieu d'avoir des URLs type http://localhost:81/?BazaR&vue=saisir&action=saisir_fiche&id=2, on pourrait proposer des URLs plus clean type http://localhost:81/?BazaR/entry/create/2. Cela dit, on pourrait garder une redirection de l'ancien URL vers la nouvelle URL, si la rétrocompatibilité est importante.

Permettre aux extensions de définir leurs propres pages spéciales

Si l'on veut, et sans beaucoup plus d'efforts, on pourrait en fait permettre de facilement ajouter des pages spéciales, sans que cela nécessite de gérer des cas particuliers.

Au chargement, YesWiki va regarder tous les Controllers définis soit dans le coeur, soit dans les extensions. S'il détecte par exemple que l'extension lms a défini, via un Controller, une route /LmsAdmin, alors lorsque l'utilisateur ira sur http://localhost:81/?LmsAdmin, il ne chargera pas de page Wiki, mais appelera la méthode correspondante du Controller.

Cela pourrait être fait sans problème de performance, grâce au cache Symfony.

Cela dit, si on veut éviter la multiplication des pages spéciales, on peut aussi décider de limiter cela en définissant dans le coeur les pages spéciales autorisées (api, BazaR...). Mais je pense que pour certaines extensions, ça pourrait être très utile d'avoir leurs propres pages spéciales.

Qu'en pensez-vous @mrflos @seballot @acheype @J9rem @oncletom ? On en discute vendredi ? ;-)

[seballot]

Wesh ! Je plussoie pour les Controller aka page spéciale :) Y compris le fait de laisser chaque extension créer des Controller Ca permet d'éviter de faire des handlers genre "update" ou "action-builder" qui sont utilisé en mode chelou genre "tu mets /update sur n'importe quelle page du wiki et ça fait la même chose : ?Pageprincipale/update == ?MyPage/update

Juste pour Bazar sous forme controller, je crois qu'on quand même besoin de garder certaines actions non? si on veut incorporer un formulaire à l'intérieur d'une page par example? Soit on garde ta première proposition : une action Bazar qui appelle des Controller, soit une manière automatique de mapper un Controller sur une action, avec des annotation pourquoi pas, à creuser...

namespace YesWiki\Bazar\Controller;

class EntryController extends AbstractController
{
    /**
     * @Route("/BazaR/entry/create/{formId}")
     * @Action("bazar", id="$formId", vue="saisie")
     */
    public function createEntry($formId): Response
    {
        // Return Twig template, or Squelette, or plain HTML string
        return $this->render(...);
    }
}

[srosset81]

@seballot Je parle juste de l'action bazar qui ne sert, je crois, qu'aux opérations d'administration. Il y a quelque chose à garder en action là-dedans tu crois ?

[J9rem]

Je me dis que ce serait bon de garder un peu de rétrocompatibilité pendant quelques mois, le temps que tout le monde puisse mettre à jour ses extensions perso, .... Sinon, ça risque de faire ce que je fais déjà, ne plus mettre à jour YesWiki Doryphore de peur de tout casser alors que tout fonctionnait avant.

Il est toujours possible de configurer un message d'alerte qui s'affiche dans le footer indiquant l'usage de fonctions dépréciées -> contacter le développeur. ce message pourrait être masqué grâce à une variable dans wakka.config.php.

Je suis d'accord avec l'intérêt du refacto mais si la rétrocompatibilité n'est pas maintenue quelques temps, nous risquons de donner une mauvaise image à YesWiki. Les usagers non experts verront apparaître plein de bugs et changeront d'outils de peur d'avoir de nouveaux bugs.

[srosset81]

La reteocompatibilite peut être totale si on l'estime nécessaire 👍

[J9rem]

Je parle juste de l'action bazar qui ne sert, je crois, qu'aux opérations d'administration. Il y a quelque chose à garder en action là-dedans tu crois ?

Je vois aussi qu'il est possible de personnaliser la page BazaR actuellement alors que si ça devient un handler spécifique comme api il ne sera plus possible de le personnaliser.

Je reste plutôt sur le fonctionnement une page BazaR avec une action bazar.

[mrflos]

Yep, je trouve l'idée de pages réservées bonne, mais en effet, il faut être sûr de pouvoir continuer a utiliser les actions dans une page.
{{bazar}} est utilisée aussi pour des affichage de listes avec filtres, pour afficher les formulaires, et ca doit pouvoir se mettre plusieurs fois dans une page.

[srosset81]

@J9rem Cela pourrait devenir un paramètre de wakkaconfig

[mrflos]

je suis d'accord avec @J9rem qu'actuellement les modifications cassent beaucoup trop l'existant, je comprend l'envie de sortir la meilleure archi possible, mais je pense qu'il faut rester sur l'objectif d'avoir un système propre pour les templates de champs bazar, stabiliser l'éditeur de formulaire, avant de creuser les parties Controller.

[srosset81]

Yep, je trouve l'idée de pages réservées bonne, mais en effet, il faut être sûr de pouvoir continuer a utiliser les actions dans une page. {{bazar}} est utilisée aussi pour des affichage de listes avec filtres, pour afficher les formulaires, et ca doit pouvoir se mettre plusieurs fois dans une page.

Ah OK je me rappelais pas que {{ bazar }} pouvait être utilisé à l'intérieur de pages. Du coup il suffirait d'appeler la méthode correspondante du Controller (qui serait aussi un service) à partir de l'action, qui resterait donc pour tout ce qui est pertinent.

je suis d'accord avec @J9rem qu'actuellement les modifications cassent beaucoup trop l'existant, je comprend l'envie de sortir la meilleure archi possible, mais je pense qu'il faut rester sur l'objectif d'avoir un système propre pour les templates de champs bazar, stabiliser l'éditeur de formulaire, avant de creuser les parties Controller.

On en reparle cet après-midi, mais si on veut avancer sur Bazar, il va falloir traiter ces questions d'affichages.

[seballot]

actuellement les modifications cassent beaucoup trop l'existant

De ce que je comprend, la seule que pourrait casser cette utilisation des controller est si quelqu'un a modifié sa page BazaR (qui par défault vaut {{bazar showexportbuttons="1"}})

Faudrait demander à la communauté si quelqu'un a déjà fait ça, mais ça me semble marginal non?

[J9rem]

Personnellement, même si c'est plus compliqué, je préfère donc que nous concevions quelque chose qui détecte si la page BazaR a été modifiée et qui alors ne change rien au comportement habituel.

Nous ne pouvons pas savoir à l'avance si quelqu'un l'a modifié même en sollicitant la communauté. Un utilisateur YesWiki n'est pas forcément un concepteur YesWiki.

Désolé, j'y tiens à cette rétrocompatibilité totale mais temporaire. Car j'ai vraiment la sensation que si on ne le fait pas, ça risque de vraiment donner une mauvaise image de YesWiki.

[seballot]

Et juste une autre idée, plutot que de garder des mots clés réservé (api, bazar) c'est pas plus simple de faire des urls normales genre monwiki.fr/bazar plutot que monwiki.fr/?BazaR ?

[srosset81]

Et juste une autre idée, plutot que de garder des mots clés réservé (api, bazar) c'est pas plus simple de faire des urls normales genre monwiki.fr/bazar plutot que monwiki.fr/?BazaR ?

Cela obligerai à utiliser des fichiers .htaccess, ce que Florian voulait éviter je pense (et je suis d'accord avec lui). Ou alors il faudrait créer un répertoire pour chaque page spéciale, ce qui n'est pas top non plus.

[acheype]

J'étais un peu dans le gaz ces derniers jours et bien occupé avec d'autres trucs, mais j'arrive enfin à y voir un peu plus clair par rapport à ta proposition Seb.

On est tous d'accord sur le fait que ca serait un plus que d'avoir ce type de controleurs, et que se recalquer sur ce qui se fait avec les routes de Symphony est carrément pertinent.

Je pense aussi y voir plus clair dans ce qui me gênait depuis le début. Actuellement le soucis c'est que bazar est un vrai fourre-tout. Tu proposes d'avoir des services Controller sachant qu'on a déjà des contrôleurs avec les classes XXXAction, le but étant que le contrôleurs premiers délèguent aux Controllers seconds (EntryController, FormController, ListController) car le pauvre contrôleur est dépassé avec tout ce bazar ;)

Donc le premier truc, c'est qu'on introduit un autre type de contrôleur pour pallier à une organisation qui est merdique depuis le départ. Il ne devrait pas y avoir une action bazar qui fait à la fois de la consultation, de la saisie et parfois même le café... S'il on était parti de zéro, on aurait pensé naturellement dès le départ à faire plusieurs actions : une pour la saisie, une autre pour la consultation, d'autres petites pour afficher des interfaces admin spécifiques, etc. Il me semble donc qu'on resterait dans la logique actuelle si on redéfinissait une action à chaque fois et que celles-ci soit appelées directement dans le twig de l'action {{bazar}} (par exemple {{bazarsaisir ...}} ou son équivalent dans la langue de Shakespeare). Cela permettrait d'avoir à chaque fois qu'un seul contrôleur qui gère moins de code, et au niveau utilisateur, ça a l'avantage d'avoir des actions plus simples à utiliser (tout en gardant la rétrocompatibilité {{bazar}} et avec des paramètres plus sympas (genre bannir les XXX="1").

Le deuxième est que si on introduit avec ces contrôleurs les routes à la Symphony pour gérer l'action {{bazar}}, cela fait qu'on aura pour cette version Doryphore, la cohabitation de deux logiques différents pour le rendu de page : la classique où tout est géré par l'index.php avec des paramètres et une tout autre pour bazar. Il faudra certainement des bidouilles pour passer dans le cas de l'action {{bazar}} passer sur cette logique de Controller Symphony et même au niveau utilisateurs, on se retrouvera alors avec des urls différentes... Pour moi, ce n'est pas juste introduire un nouveau type de contrôleur pour pallier à la complexité de l'action {{bazar}} mais c'est ajouter un nouveau système de navigation qui a intérêt à se penser dès le départ global pour remplacer l'actuel. Il aurait tout intérêt à être davantage réfléchi et mis en place globalement dans une prochaine itération.

J'ai l'impression qu'on était bien divisé hier face à cette question de rajouter ces contrôleurs à la Symphony dès cette itération. Je me demande si on ne pourrait s'accorder davantage sur ce compromis : l'introduire pour l'instant uniquement pour la page spéciale API. Les urls étant déjà de ce type, cela ne changera aucune logique existante et ça nous permettra d'apprendre de cette première étape avant de nous lancer dans des changements plus critiques. Pour un sprint qui est axé finalisation/consolidation de l'existant, ça serait déjà pas mal. Mais c'est sûr que ça serait cool d'avoir à terme de belles urls (ou juste de vraies urls ;)).

[seballot]

Bim le pavé :)

S'il on était parti de zéro, on aurait pensé naturellement dès le départ à faire plusieurs actions : une pour la saisie, une autre pour la consultation

Moi je pense qu'on aurait pensé naturellement à faire des routes plutot que des actions :) Le côté "tout est une action" est attrayant en première lecture, mais dans la pratique je trouve ça bancal de devoir créer des pages wiki pour y mettre juste une action, c'est galère à maintenir, galère à comprendre au début, ça peut créer des problèmes (genre un utilisateur edite la page sans comprendre ce qu'il fait)

Mais ok pour utiliser le Controller juste pour l'api pour l'instant, et pour faire des sous action bazar appelées par l'action bazar principale !

Zoubs !

[acheype]

à vous de voir (et à Seb principalement), l'important pour moi était de vous en faire part avant de ne plus être dispo :)

[srosset81]

Donc le premier truc, c'est qu'on introduit un autre type de contrôleur pour pallier à une organisation qui est merdique depuis le départ. Il ne devrait pas y avoir une action bazar qui fait à la fois de la consultation, de la saisie et parfois même le café... S'il on était parti de zéro, on aurait pensé naturellement dès le départ à faire plusieurs actions : une pour la saisie, une autre pour la consultation, d'autres petites pour afficher des interfaces admin spécifiques, etc.

L'action {{ bazar }} a la particularité de dépendre non seulement des paramètres qu'on lui passe, mais aussi de la query string du navigateur. On sort du principe d'encapsulation qui est propre à la programmation OO, et cela rend la lisibilité de l'action extrêment compliquée.

Je ne vois pas l'intérêt qu'il y aurait à séparer cette action en sous-actions. Il faudrait alors créer, en plus de la page ?BazaR, les pages ?BazarSaisie, ?BazarAjoutList, etc, bref une page wiki par sous-action, créée automatiquement au moment de l'installation (une vingtaine de pages en tout). De plus, il faudrait empêcher que l'utilisateur modifie ces pages, sinon ça casserait tout le code. Ce serait tout bonnement impossible à maintenir, et je ne vois pas ce que toutes ces pages prédéfinies apporterait à l'utilisateur, mis à part de la confusion (sans parler non plus de la complication que ça engendrerait pour assurer la rétro-compatibilité).

Si tu veux faire une proposition pour des sous-actions Bazar, je te propose de faire une issue dédiée, en décrivant précisément tout ce que cela impliquerait.

Le deuxième est que si on introduit avec ces contrôleurs les routes à la Symphony pour gérer l'action {{bazar}}, cela fait qu'on aura pour cette version Doryphore, la cohabitation de deux logiques différents pour le rendu de page : la classique où tout est géré par l'index.php avec des paramètres et une tout autre pour bazar. Il faudra certainement des bidouilles pour passer dans le cas de l'action {{bazar}} passer sur cette logique de Controller Symphony et même au niveau utilisateurs, on se retrouvera alors avec des urls différentes...

La bidouille dont tu parles existe déjà pour la route ?api. L'idée c'est justement de faire un truc propre.

Et pour finir je reviens sur ce point:

Tu proposes d'avoir des services Controller sachant qu'on a déjà des contrôleurs avec les classes XXXAction

Je ne vois pas en quoi les actions sont des Controllers, mis à part pour l'action {{ bazar }} qui dépend de la query string ? Pour l'utilisateur, une action permet d'ajouter des fonctionnalités spéciales à l'intérieur d'une page wiki. Dans son nouveau formulaire, Sebastian les appelle d'ailleurs "Composants", ce qui me semble plus juste (mais bon, on ne va pas tout renommer non plus !)

Il me semble que l'objectif de ce refactoring c'est notamment de rendre le code plus intelligible pour les développeurs, en enlevant les "hacks" qui rendent la lecture du code difficile. L'action {{ bazar }} est pour moi clairement un hack, et c'est pour ça qu'elle est si difficile à lire et à maintenir. Mettre en place des Controllers permet de faire proprement une chose qui est, en réalité, déjà en place pour le nouveau système d'API et pour l'action Bazar. On n'ajoute pas donc de nouvelles fonctionnalités, on se contente de clarifier un besoin qui existe déjà.

[srosset81]

Le côté "tout est une action" est attrayant en première lecture, mais dans la pratique je trouve ça bancal de devoir créer des pages wiki pour y mettre juste une action, c'est galère à maintenir, galère à comprendre au début, ça peut créer des problèmes (genre un utilisateur edite la page sans comprendre ce qu'il fait)

Bien résumé !

[mrflos]

En tout cas, dans les règles de l'art ou pas, c'est important que l'on puisse avoir les principaux composants des bases de données : formulaire de saisie, recherche, liste avec vues, toujours disponibles quand on édite une page comme un composant a ajouter à la page, c'est quand même l’intérêt fondamental de ces actions.
Il y a plein de cas ou l'on va utiliser le même formulaire de saisie avec des look différents, des langues différentes, incrusté dans une onepage, avec un texte custom avant et/ou apres l'action, simplifié graphiquement pour les usagers par du code css custom, les vues à incruster dans les iframes avec des css custom, etc,...
Cela ne pourrait pas marcher avec une url d’accès unique par le controler...

[srosset81]

Oui bien sûr, il n'a jamais été question de supprimer ces "composants" Bazar :-)

[acheype]

Ok si tu penses que ce chantier est important pour ce refactoring et si ça te va d'être sur ces différents fronts à la fois.

Du coup, je précise quelques points, il y a eu des trucs qui ne devaient pas être clairs :

Je ne vois pas en quoi les actions sont des Controllers, mis à part pour l'action {{ bazar }} qui dépend de la query string ? Pour l'utilisateur, une action permet d'ajouter des fonctionnalités spéciales à l'intérieur d'une page wiki.

Je commence par là car c'est une question de vocabulaire et ça me paraît important qu'on s'entende sur ces concepts. Là je parlais de Contrôleur au sens MVC. Avant nous avions tout dans la Vue (M + V + C), aujourd'hui tout dépend de comment on utilise les classes XXXAction, on peut très bien mettre le M + C dans cette classe mais si on utilise des services pour charger/mettre à jour nos modèles, on a bien la séparation des responsabilités : M dans les services, V dans le template twig et C dans la classe XXXAction.

Et avec cette nouvelle organisation que vous nous avez amené (merci), je ne pense pas qu'il y ait besoin de rajouter un autre niveau de contrôleur pour les actions... par contre au niveau des handlers... (cf plus loin).

Dans son nouveau formulaire, Sebastian les appelle d'ailleurs "Composants", ce qui me semble plus juste (mais bon, on ne va pas tout renommer non plus !)

Oui, "Composant" est plus explicite et rend bien l'idée qu'il est réutilisable et peut être intégré à n'importe quel endroit sur une page.

Si tu veux faire une proposition pour des sous-actions Bazar, je te propose de faire une issue dédiée, en décrivant précisément tout ce que cela impliquerait.

J'ai créé du coup l'issue #558 . Ca n'aurait en effet pas été adapté de devoir créer une page pour chaque sous-action, mais il n'y en a pas besoin.

La bidouille dont tu parles existe déjà pour la route ?api. L'idée c'est justement de faire un truc propre.

Ca serait chouette d'avoir quelque chose de propre mais il faudrait que la solution choisie s'accorde bien avec la logique déjà existante de YesWiki. Ce qu'amène le contrôleur que tu proposes est l'intégration du système de routage de Symphony, et s'il on veut généraliser ce routage à l'ensemble de Yeswiki, c'est au niveau des handlers qu'il faudrait l'intégrer. Actuellement, le système est très limité, les pages sont aiguillées suivant ce qui est écrit après le ? (ou le paramètre wakka), mais redéfinir le fonctionnement des handlers permettrait d'intégrer globalement le routage de Symphony. En récrivant les handlers show et iframe avec une classe XXXHandler, on pourrait par exemple annoter la méthode run($arguments) avec l'annotation @Route pour permettre d'avoir de jolies urls pour les pages : /mapage?param1=X&param2... et son équivalent /mapage/iframe (à voir ce qui nous paraît le plus sympa). Pareil, plutôt qu'une "page spéciale" on pourrait créer la classe ApiHandler qui gère les adresses qui commencent par /api...

Pour Bazar, il serait aussi possible de créer un BazarHandler pour avoir des urls de type /bazar/entry/create/{formId}, et le lien pourrait se faire entre les actions et les handlers grâce à une fonction à créer : $wiki->include($relativeUrl) appelée depuis BazarAction et qui intègrerait dans la page courante le flux renvoyé par le BazarHandler (il me semble qu'on a actuellement seulement la redirection possible avec $wiki->Redirect($url)). Toutefois, même si je pense que ce lien entre actions et handlers pourra certainement nous être utile, pour Bazar ce me semble plus cohérent de rester dans le concept des composants et que le découpage profite également aux utilisateurs en se voyant offrir des actions plus faciles à utiliser. Par contre, je trouverait super utile d'avoir à terme un handler pour mettre à disposition une api spécifique à Bazar : /api/bazar/...

A voir ce qui vous plaît, ce qui peut-être ajusté/amélioré mais j'ai l'impression qu'on a tout intérêt à intégrer globalement ce système de routage en améliorant les handlers. De la même manière que toi et Sebastian l'avez fait avec les nouveaux services ou le système des WikiPerformables : ajouter de nouvelles facilités, une meilleure organisation tout en suivant la logique déjà existante de YesWiki.

[seballot]

Ouaip bonne idée pour le système de routage associé aux Handler, ça permet de ne pas introduire un autre concept et ça fera pareil !

[srosset81]

Les handlers sont toujours liés à une page wiki indiquée dans la première partie de l'URL (?MaPage/edit). Donc quand tu parles d'un handler pour une url type ?bazar/entry/create/{formId}, tu pars de l'idée que bazar est une page wiki spéciale. Ce qui est précisément le fond de ma proposition.

Comme je l'ai dit plusieurs fois, l'action Bazar a une vingtaine de vues. Il faudrait donc créer une vingtaine de fichiers différents pour les handlers, et comment les nommeraient-on ? Avec un système de controller type Symfony, on pourrait regrouper ces vues dans des classes, selon le type de vue (ListController, ExportController...), ce qui me semble plus élégant.

Au final ça ne change pas grand chose. J'ai vraiment de la peine à discerner une objection, pour parler "GPC", il me semble qu'on est plutôt dans des préférences. Je n'ai pas beaucoup d'énergie à consacrer à YesWiki, je vais donc éviter de reprendre chaque point et plutôt continuer à avancer.