developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#279 closed bug (fixed)

Paramètres de requete JSON-RPC

Reported by: Julien Owned by: laurentj
Priority: high Milestone: Jelix 1.0beta3.1
Component: jelix:core request Version: 1.0 beta3
Severity: major Keywords:
Cc: Blocked By:
Blocking: Documentation needed:
Hosting Provider: Php version:

Description

Bonjour,

avec la beta 3, il n'est plus possible de récupérer les paramètres d'une requête JSON-RPC individuellement.

$myparam = $this->param('myparam') ne fonctionne plus, et il faut faire

$params = $this->param('params'); $myparam = $params\['myparam'\];

Ce n'est pas spécialement gênant, mais c'était assez pratique de pouvoir récupérer les paramètres individuellement, et de pouvoir utiliser les méthodes intParam, boolParam, etc...

Donc question : reste-t-on comme dans la beta3, ou y a-t-il un intéret à revenir au fonctionnement précédent ?

PS : c'est juste une observation, pas une critique sur le nouveau fonctionnement. Je dois juste savoir si je dois changer le code de mes controleurs JSON-RPC. J'ai mis un priorité et sévérité élevée car pour le moment les applis ne fonctionnent plus alors que c'était le cas sur beta2 si on récupérait les paramètres individuellement.

Change History (6)

comment:1 follow-up: Changed 13 years ago by laurentj

  • Component changed from jelix to jelix:core request
  • Owner set to laurentj

J'ai oublié d'indiquer ce changement d'API dans le changelog (mais je viens de le mettre à jour).

Donc oui, maintenant, les paramètres RPC sont dans le paramêtre "param" (changeset/565). Un petit peu d'explication : en json-rpc et xml-rpc, les paramètres peuvent être n'importe quoi : un nombre,une chaine, une liste etc, donc pas forcément un tableau associatif/objet.

Donc pour les actions qui recevait un paramêtre RPC simple genre une chaine, il fallait faire $this->param('params') ('params' étant un nom donné par jelix). Mais si on recevait un tableau associatif, le tableau associatif était fusionné avec les autres paramètres jelix. Du coup la récupération des paramètres n'était pas cohérent d'un cas à l'autre. Cependant, c'est vrai que c'etait assez pratique. Alors que faire ?

Peut-être faire un mix des deux ? On met les paramètres à la fois dans une variable params, et dans le tableau des paramètres ? Cependant, il reste aussi le problème des parametres RPC qui ont pour nom "module" et "action" -> ils seraient écrasé par Jelix dans le tableau des paramètres jelix.

Vois-tu une autre solution ?

comment:2 in reply to: ↑ 1 Changed 13 years ago by Julien

Replying to laurentj:

J'ai oublié d'indiquer ce changement d'API dans le changelog (mais je viens de le mettre à jour).

Donc oui, maintenant, les paramètres RPC sont dans le paramêtre "param" (changeset/565). Un petit peu d'explication : en json-rpc et xml-rpc, les paramètres peuvent être n'importe quoi : un nombre,une chaine, une liste etc, donc pas forcément un tableau associatif/objet.

tout à fait exact. Ce qui selon moi penche en faveur de l'ancien système, où l'on peut récupérer chaque paramètre avec la méthode param, mais aussi intParam, boolParam, etc....

Donc pour les actions qui recevait un paramêtre RPC simple genre une chaine, il fallait faire $this->param('params') ('params' étant un nom donné par jelix).

Ok. Personnellement j'avais toujours fait en sorte (en JS) que la propriété params soit un objet avec une propriété pour chaque paramère :

var req = {
  id : 12,
  method : 'module~test',
  params : {
    name: 'toto',
    age: 17
  }
};

donc je pouvais faire, en PHP

$name = $this->param('name');
$age = $this->intParam('age');

ce qui me semble bien, pratique, et cohérent par rapport au principe de récupération des paramètres que l'on utilise dans les requêtes classic.

Mais si on recevait un tableau associatif, le tableau associatif était fusionné avec les autres paramètres jelix. Du coup la récupération des paramètres n'était pas cohérent d'un cas à l'autre.

Sauf erreur de ma part, les seuls paramètres "système" qui sont définis dans jJsonRpcRequest sont module et action, et donc les seuls sujet à écrasement.

Il suffit peut-être de bien indiquer qu'on ne peut pas spécifier, en JS :

req.params.module = 'titi';
req.params.action = 'toto';

et au pire, la requête sera invalide car le développeur aura fait une erreur (renvoyer une réponse JSON-RPC avec un message d'erreur qui indiquerai qu'il n'est pas possible de nommer un paramètre module ou action).

bien entendu, si, en JS :

req.params.pages_ids = [1,5,9,3];

alors en PHP :

$ids = $this->param('pages_ids');

$ids est alors un tableau, mais ce tableau ne doit en aucun fusionner avec le $params global. Je sais pas si je suis clair ;)

Cependant, c'est vrai que c'etait assez pratique. Alors que faire ?

Peut-être faire un mix des deux ? On met les paramètres à la fois dans une variable params, et dans le tableau des paramètres ?

oui, comme dans l'ancien système : si la propriété JS n'est pas une structure (tableau, objet), alors sa valeur va directement dans $this->params\['params'\].

Sinon, $this->params prend la valeur du tableau associatif décodé par JSON.

Je pense personnellement que de toute façon, la propriété JS params devrait toujours être donnée sous forme d'objet, pour faciliter l'extensibilité, car on peut rajouter des paramètres facilement :

req.params.newparam = 'titi';

mais bon, la solution de 'détection' du type de $params permet de ne pas imposer cette méthodologie.

Cependant, il reste aussi le problème des parametres RPC qui ont pour nom "module" et "action" -> ils seraient écrasé par Jelix dans le tableau des paramètres jelix.

oui, voir ci-dessus l'émission d'un message d'erreur, je pense que ce serait le + cool.

Qu'en penses-tu ? Je veux bien réaliser le patch si OK.

comment:3 follow-up: Changed 13 years ago by laurentj

  • Status changed from new to assigned

donc je pouvais faire, en PHP $name = $this->param('name'); $age = $this->intParam('age'); ce qui me semble bien, pratique, et cohérent par rapport au principe de récupération des paramètres que l'on utilise dans les requêtes classic.

Exact.

et au pire, la requête sera invalide car le développeur aura fait une erreur (renvoyer une réponse JSON-RPC avec un message d'erreur qui indiquerai qu'il n'est pas possible de nommer un paramètre module ou action).

Il faudrait, en effet,

$ids est alors un tableau, mais ce tableau ne doit en aucun fusionner avec le $params global.

Oui, bien entendu.

Je pense personnellement que de toute façon, la propriété JS params devrait toujours être donnée sous forme d'objet, pour faciliter l'extensibilité, car on peut rajouter des paramètres facilement

Certes, mais le protocole json-rpc ne spécifiant pas qu'il faille systématiquement donner un tableau associatif/objet en paramètre, on doit tenir compte de ça dans jelix :-)

Je veux bien réaliser le patch si OK.

Le patch, on l'a déjà là, mais à l'envers : changeset/565 ;-) Cependant, je pense qu'il faut mettre aussi les parametres dans params (donc supprimer le else) pour garder une cohérence, pour qu'on est finalement le choix de la façon dont on récupère :

   if(is_array($requestobj['params'])) 
        $this->params = $requestobj['params']; 

   $this->params['params'] = $requestobj['params']; 

Je vais changer ça dans le trunk.

comment:4 in reply to: ↑ 3 Changed 13 years ago by Julien

Replying to laurentj:

Cependant, je pense qu'il faut mettre aussi les parametres dans params (donc supprimer le else) pour garder une cohérence, pour qu'on est finalement le choix de la façon dont on récupère :

   if(is_array($requestobj['params'])) 
        $this->params = $requestobj['params']; 

   $this->params['params'] = $requestobj['params']; 

Pourquoi ne veux-tu pas conserver le else ? Parce que là on va doubler les paramètres dans la mémoire, donc on grève un peu les perfs. Enfin je pense (par exemple si on échange des contenus d'articles comme dans dragon-cms).

Si comme moi le développeur passe une structure (ce qui n'est pas une obligation et nous en tenons compte via le else), alors je ne pense pas qu'il s'amusera à faire :

$params = $this->param('params');
$myparam = $params\['myparam'\];

Je me trompe peut-être, mais je ne vois pas vraiment de cas où il peut être intéressant de faire $this->param('params') à part si params est une valeur simple (chaine, nombre, bool).

Cependant, si tu penses que la duplication des paramètres n'est pas trop pénalisante niveau perf, je suis ta solution.

Je vais changer ça dans le trunk.

Ok Julien

comment:5 follow-up: Changed 13 years ago by laurentj

  • Milestone set to Jelix 1.0beta3.1
  • Resolution set to fixed
  • Status changed from assigned to closed

Je ne pense pas que ce soit pénalisant, dans la mesure où je pense que dans la plupart des cas, le volume des données ne sera pas énorme.

Corrigé dans le trunk et dans une nouvelle branche 1.0beta3.x.

comment:6 in reply to: ↑ 5 Changed 13 years ago by Julien

Replying to laurentj:

Je ne pense pas que ce soit pénalisant, dans la mesure où je pense que dans la plupart des cas, le volume des données ne sera pas énorme.

Corrigé dans le trunk et dans une nouvelle branche 1.0beta3.x.

Impeccable ;) Merci

Note: See TracTickets for help on using tickets.