wscalls: unflatten payload when calling webservice (#66916) #1204
Loading…
Reference in New Issue
No description provided.
Delete Branch "wip/66916-wscall-apply-unflatten-payload"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -51,0 +69,4 @@
def split_key(key):
def map_key(x):
if x.isnumeric():
Dans w.c.s. on a misc.is_ascii_digit() pour faire le is_number() de passerelle, c'est nécessaire parce que :
@ -51,0 +75,4 @@
return x.replace('%s%s' % (separator, separator), separator)
return x
return [map_key(x) for x in re.split(r'(?<!%s)%s(?!%s)' % (separator, separator, separator), key)]
On ne va pas rendre le séparateur paramétrable, la fonction sera plus lisible sans ça.
(ça vaut quand même le coup d'expliciter l'objectif de la regex).
J'ai viré la regex car il n'y a pas besoin de l'utiliser pour découper la chaîne.
@ -410,1 +497,4 @@
},
hint=_(
'Parameters names could be separated by / character to create object or array payload,'
' ex: parameter named "element/child" with value "value" will be sent as {"element": {"child": "value"}.'
reprendre/traduire la version avancée de l'explication donnée dans le ticket.
a056d7b8c0
toc94efb7d1c
@ -158,0 +235,4 @@
unflattened_data = unflatten_data(post_data)
except UnflattenDataException as e:
get_publisher().record_error(
exception=e, context='[WSCALL]', notify=notify_on_errors, record=record_on_errors
Ici il faudrait faire en sorte que le message affiché soit explicite, pour comprendre ce qui a planté (que ce n'est pas l'appel mais la création de l'appel), genre :
Appel webservice impossible, le corps de la requête n'a pas pu être construit, impossible de désapplatir les clés (erreur: ...)
Ok.
@ -292,0 +314,4 @@
wscall.store()
try:
wscall.call()
except Exception:
Plutôt utiliser un with pytest.raises le plus restrictif possible
Au final je ne léve pas d'exception mais plutôt enregistre l'erreur les logs.
@ -292,0 +316,4 @@
wscall.call()
except Exception:
pass
assert http_requests.count() == 1
faire plutôt un http_requests.empty() avant de lancer le wscall, afin de faire un == 0 ici
Ok, merci.
@ -576,0 +599,4 @@
assert payload == {'foo': ['first', 'second'], 'bar': 'example'}
assert http_requests.count() == 1
try:
même remarque sur le with pytest.raise et le http_requests.empty()
c94efb7d1c
tofe7cbb1a7d
@ -301,0 +310,4 @@
'"element": {"child": "value"}. If the subkey, i.e. "child", is an integer it will '
'become a list index and two elements "element/0", "element/1" containing "value1" '
'and "value2" will generate the payload "element": ["value1", "value2"]. '
'It is possible to combine the two types, for example "element/0/key1" to generate '
Comme j'ai eu du support à faire ce matin sur le sujet, on pourrait ajouter un petit bout qui dit qu'en cas de element/0, element/1, le 0 est obligatoire. Genre ajouter un "index 0 is mandatory" :
If the subkey, i.e. "child", is an integer it will become a list index and two elements "element/0", "element/1" containing "value1" and "value2" will generate the payload "element": ["value1", "value2"] (note: index 0 is mandatory).
Bon, si c'est jugé trop lourdingue tu oublies, on essayera de faire préciser ça dans une doc, qui contiendra des exemples.
On n'est pas à quelques mots près. J'ai rajouté la remarque sur les indexes.
fe7cbb1a7d
tod4e1d9c0ac
WIP: wscalls: unflatten payload when calling webservice (#66916)to wscalls: unflatten payload when calling webservice (#66916)@ -51,0 +61,4 @@
into:
{"a": {"b": [{"x": "1234"}]}}
"""
Et tout à coup je me demande si on ne veut pas, dès maintenant, prévoir la possibilité d'échappement ? Genre « // » ?
C'est déjà géré, par cette partie :
il faudrait donc y ajouter un commentaire pour expliciter.
Ok, j'ajoute un commentaire et un test.
@ -155,3 +233,3 @@
payload = {}
with get_publisher().complex_data():
for key, value in post_data.items():
for key, value in unflatten_keys(post_data).items():
J'ai l'impression d'une erreur ici, il faut plutôt faire le compute sur les clés d'origine (avec les /) et ensuite seulement, faire le unflatten... non ?
Je dois rater quelque chose mais je ne saisis pas la différence entre l'appel avant ou après le
compute
.J'ai modifié le test pour montrer que cela fonctionne avec le unflatten avant le calcul des valeurs.
Par exemple si tu mets « 'foo/2': '{{ form_name }}', » dans ton test, ça va envoyer '{{ form_name }}' dans la liste foo, sans faire le compute:
ça ne marche pas:
d4e1d9c0ac
to57c3babdc9
57c3babdc9
to6db003b1c8
6db003b1c8
tod60cacb833
Et pour moi il est vraiment important d'avoir la fenêtre d'explicitation à l'admin de ce que vont donner ses clés.
@ -51,0 +66,4 @@
if not isinstance(d, dict) or not d: # unflattening an empty dict has no sense
return d
# ok d is a dict
Ce commentaire n'apporte pas grand chose.
@ -51,0 +77,4 @@
return x.replace('//', '/')
return x
return [map_key(x) for x in re.split(r'(?<!/)/(?!/)', key)]
Tout le monde ne lit pas les regex facilement, un commentaire pour l'expliciter serait utile.
@ -51,0 +83,4 @@
try:
keys.sort()
except TypeError:
# sorting fail means that there is mix between lists and dicts
a mix
@ -51,0 +84,4 @@
keys.sort()
except TypeError:
# sorting fail means that there is mix between lists and dicts
raise UnflattenKeysException(_('incorrect elements order'))
Le commentaire dit qu'e c'est parce qu'il y a un mélange mais l'exception dit que c'est parce qu'il y a un problème d'ordre ?
Corrigé le message de l'exception.
@ -51,0 +91,4 @@
key, tail = path[i], path[i + 1 :]
if not tail: # end of path, set thevalue
set the value
@ -51,0 +99,4 @@
d.append(value)
else:
assert isinstance(d, dict)
d[key] = value
Je serais pour ici avoir un "return # end recursion", et enchainer la suite un nouveau d'indentation plus bas (retirer le else de la ligne suivante).
Cela parce qu'à la lecture de la fonction je me suis perdu dans l'indentation et avec l'appel set_path() qui la termine je ne voyais pas comment la récursion s'arrêtait.
@ -51,0 +107,4 @@
assert isinstance(d, list)
if len(d) < key:
raise UnflattenKeysException(
_('incomplete array before %s in %s') % ('/'.join(map(str, path[: i + 1])), orig_key)
C'est possible de réécrire le map sous forme de list comprehension ? (str(x) for x in in path[...])
@ -51,0 +118,4 @@
set_path(path, orig_key, new, value, i + 1)
# Is the first level an array or a dict ?
if isinstance(keys[0][0][0], int):
Possibilité d'étendre le commentaire avec un rappel de ce que contiendrait keys ici ?
Bonne remarque.
J'ai complété le test pour couvrir les 2 cas.
d60cacb833
tob4a00ae80e
b4a00ae80e
to09659122ea
wscalls: unflatten payload when calling webservice (#66916)to WIP: wscalls: unflatten payload when calling webservice (#66916)09659122ea
toa016545d48
7dd0b39585
to236fdd8f49
236fdd8f49
toce66482478
ce66482478
to4df0449045
Voici un aperçu du preview du payload.
@ -213,6 +216,7 @@ class RootDirectory(Directory):
'fargo',
'static',
'actions',
('preview-json-payload', 'preview_json_payload'),
Déplace plutôt ça sous api/, dans api.py, et il pourrait y avoir une vérification que l'appelant peut aller dans le backoffice (pour rester simple, ne pas entrer dans les permissions d'accès aux workflows/appels webservice).
@ -303,0 +335,4 @@
r += htmltext('<div class="payload-preview">')
try:
dump = json.dumps(unflatten_keys(payload), ensure_ascii=False, indent=2)
r += htmltext('<pre>%s</pre>') % dump.replace('"{{', '{{').replace('}}"', '}}')
En pratique on pourrait aussi très bien avoir des {% ou des {{ qui ne sont pas en début de chaine; il y a peut-être moyen de gérer ces cas aussi, en imaginant que sur chaque ligne, s'il y a une chaine en valeur, si elle est détectée comme étant un gabarit, alors retirer les " de la valeur.
Ça pourrait aussi se combiner en mettant ces valeurs dans un
<span class=...>
, pour les identifier comme des endroits qui seront transformés.S'ils ne sont pas en début de chaîne alors il me semble ok de laisser les guillemets car le résultat sera une chaîne de caractères avec les valeurs des gabarits entourés de texte. Non ?
Oui, c'était davantage pour l'uniformité (d'où la partie css dessous), histoire que "test {{form_var_test}}" et "{{form_var_test}}" et "{{form_var_test}}{% if 1 %}hello{% endif %}" apparaissent pareil; mais ok si tu veux zappons ça pour le moment, juste gérer {% %} en début/fin de chaine en plus, alors.
Je suis parti pour parcourrir le payload et produire un HTML englobant les elements dans des "<span...>".
@ -303,0 +337,4 @@
dump = json.dumps(unflatten_keys(payload), ensure_ascii=False, indent=2)
r += htmltext('<pre>%s</pre>') % dump.replace('"{{', '{{').replace('}}"', '}}')
except UnflattenKeysException as e:
r += htmltext('<div class="errornotice"><p>%s</p><p>%s<code>%s</code></p></div>') % (
Les exceptions sont traduites/lisibles, en voyant la capture d'écran je me suis dit que c'était dommage de les afficher dans un
<code>
.4df0449045
toe9766d6c10
e9766d6c10
to8885b19d77
8885b19d77
toa3d05511b7
WIP: wscalls: unflatten payload when calling webservice (#66916)to wscalls: unflatten payload when calling webservice (#66916)Des mini-commentaires d'apparence, puis ça sera ok je pense.
@ -3134,0 +3135,4 @@
span.payload-preview {
&--value {
font-family: Monospace;
}
monospace sans majuscule, et je la verrais plutôt sur le bloc entier, là ça fait étrange d'avoir les clés et les valeurs avec une police différente. Pour la lisiiblité aussi j'y ajouterais line-height: 150%.
Et pour distinguer ici, je jouerais plutôt sur le fond; on n'a pas de couleur légère pastelle dans gadjo, ici je trouve qu'un jaune léger, #ffa, donne correctement.
Ok, appliqué.
Avec mes yeux pétés je trouve que le jaune #ffa est un peu trop violent, je l'ai rendu un peu plus pâle: #ffc.
@ -3134,0 +3141,4 @@
display: block;
}
&--item-separator::after {
content: '\a';
Un petit commentaire à côté pour dire que c'est pour forcer un retour à la ligne ?
rajouté
@ -88,6 +88,7 @@ def i18n_js(request):
'close': _('Close'),
'email_domain_suggest': _('Did you want to write'),
'email_domain_fix': _('Apply fix'),
'preview_json_payload': _('Preview produced payload'),
Preview payload structure ?
En effet, c'est mieux.
a3d05511b7
toe84240387d
e84240387d
to5dcfd59175
Branche mise à jour.
Exemples de rendu.
Ah j'ai du me planter dans ma description des classes, je pensais au jaune uniquement pour les endroits qui ont du "gabarit complexe".
5dcfd59175
to28f48f4ed4
D'acc. Corrigé.
Joilijoli (les captures).
28f48f4ed4
to18be525b2e