Projet

Général

Profil

Bug #60351

API - CommaSeparatedStringField & entier

Ajouté par Lauréline Guérin il y a environ 2 ans. Mis à jour il y a environ 2 ans.

Statut:
Fermé
Priorité:
Normal
Catégorie:
-
Version cible:
-
Début:
06 janvier 2022
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Si on reçoit un entier, ça pète:

chrono ERROR agendas-venissieux-test.demarches.sitiv.fr 192.168.66.24 - r:7F21049FA518 Internal Server Error: /api/agenda/epj-charreard-max-barel-atelier-sociocultur
                                                Traceback (most recent call last):
                                                  File "/usr/lib/python3/dist-packages/django/core/handlers/exception.py", line 34, in inner
                                                    response = get_response(request)
                                                  File "/usr/lib/python3/dist-packages/django/core/handlers/base.py", line 115, in _get_response
                                                    response = self.process_exception_by_middleware(e, request)
                                                  File "/usr/lib/python3/dist-packages/django/core/handlers/base.py", line 113, in _get_response
                                                    response = wrapped_callback(request, *callback_args, **callback_kwargs)
                                                  File "/usr/lib/python3/dist-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
                                                    return view_func(*args, **kwargs)
                                                  File "/usr/lib/python3/dist-packages/django/views/generic/base.py", line 71, in view
                                                    return self.dispatch(request, *args, **kwargs)
                                                  File "/usr/lib/python3/dist-packages/rest_framework/views.py", line 495, in dispatch
                                                    response = self.handle_exception(exc)
                                                  File "/usr/lib/python3/dist-packages/rest_framework/views.py", line 455, in handle_exception
                                                    self.raise_uncaught_exception(exc)
                                                  File "/usr/lib/python3/dist-packages/rest_framework/views.py", line 492, in dispatch
                                                    response = handler(request, *args, **kwargs)
                                                  File "/usr/lib/python3/dist-packages/chrono/api/views.py", line 2163, in post
                                                    if not serializer.is_valid():
                                                  File "/usr/lib/python3/dist-packages/rest_framework/serializers.py", line 236, in is_valid
                                                    self._validated_data = self.run_validation(self.initial_data)
                                                  File "/usr/lib/python3/dist-packages/rest_framework/serializers.py", line 434, in run_validation
                                                    value = self.to_internal_value(data)
                                                  File "/usr/lib/python3/dist-packages/rest_framework/serializers.py", line 488, in to_internal_value
                                                    validated_value = field.run_validation(primitive_value)
                                                  File "/usr/lib/python3/dist-packages/rest_framework/fields.py", line 536, in run_validation
                                                    value = self.to_internal_value(data)
                                                  File "/usr/lib/python3/dist-packages/chrono/api/serializers.py", line 33, in to_internal_value
                                                    data = [s.strip() for s in data.split(',') if s.strip()]
                                                AttributeError: 'int' object has no attribute 'split'


Fichiers

0002-api-improve-recurrence_days-serialiser-on-create-eve.patch (3,1 ko) 0002-api-improve-recurrence_days-serialiser-on-create-eve.patch Nicolas Roche (absent jusqu'au 3 avril), 27 janvier 2022 17:06
0001-test-add-a-recurent-event-providing-int-as-recurrent.patch (2,54 ko) 0001-test-add-a-recurent-event-providing-int-as-recurrent.patch Nicolas Roche (absent jusqu'au 3 avril), 27 janvier 2022 17:06
0002-api-accept-int-on-recurrence_days-serialiser-60351.patch (2,63 ko) 0002-api-accept-int-on-recurrence_days-serialiser-60351.patch Nicolas Roche (absent jusqu'au 3 avril), 02 février 2022 19:10
0001-api-use-StringOrListField-for-recurrence_days-serial.patch (3,35 ko) 0001-api-use-StringOrListField-for-recurrence_days-serial.patch Nicolas Roche (absent jusqu'au 3 avril), 02 février 2022 19:10
0002-api-accept-int-on-StringOrListField-serialiser-60351.patch (2,58 ko) 0002-api-accept-int-on-StringOrListField-serialiser-60351.patch Nicolas Roche (absent jusqu'au 3 avril), 03 février 2022 08:48
0001-api-use-StringOrListField-for-recurrence_days-serial.patch (3,56 ko) 0001-api-use-StringOrListField-for-recurrence_days-serial.patch Nicolas Roche (absent jusqu'au 3 avril), 03 février 2022 08:48
0001-tests-change-post-patch-to-pass-json-to-event-view-6.patch (11,2 ko) 0001-tests-change-post-patch-to-pass-json-to-event-view-6.patch Nicolas Roche (absent jusqu'au 3 avril), 14 février 2022 17:38
0002-api-use-StringOrListField-for-recurrence_days-serial.patch (3,04 ko) 0002-api-use-StringOrListField-for-recurrence_days-serial.patch Nicolas Roche (absent jusqu'au 3 avril), 14 février 2022 17:38

Révisions associées

Révision e532eefc (diff)
Ajouté par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

tests: change post/patch to pass json to event view (#60351)

Révision ac41d09c (diff)
Ajouté par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

api: use StringOrListField for recurrence_days serialiser (#60351)

Historique

#2

Mis à jour par Valentin Deniaud il y a environ 2 ans

Note, plutôt que de chercher à réparer CommaSeparatedStringField, il faut le remplacer par StringOrListField et ensuite réparer si ça ne marche toujours pas.

La raison c'est #59813, w.c.s. est capable de passer les données POST sous forme de liste, donc il faut permettre d'exploiter ça. CommaSeparatedStringField devrait être réservé aux listes passées en paramètre d'URL, donc toujours des chaînes de caractères et jamais des vraies listes.

#3

Mis à jour par Lauréline Guérin il y a environ 2 ans

(note: c'était sur le endpoint de création d'event)

#4

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

  • Assigné à mis à Nicolas Roche (absent jusqu'au 3 avril)

(note: c'était sur le endpoint de création d'event)

oups, c'est pour moi

dans le ticket lié

le paramètre recurrence_days ... renvoyait un entier

Pour l'instant je n'arrive pas à reproduire via les tests.

#5

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

Bug reproduit en utilisant une expression python via wcs, ex: int(2).

0001: Pour être sûr que le pasage à StringOrListField ne casse rien.
0002: Remplacer par StringOrListField et ensuite réparer.

#6

Mis à jour par Valentin Deniaud il y a environ 2 ans

Je comprends pas

        if isinstance(data, list) and len(data) == 1:
            data = data[0]

dans 0002 ?

#7

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

L'idée c'est de continuer à gérer les "CommaSeparatedStringField" dans le champs POST,
pour éviter la régression sur ce test : tests/api/test_event.py::test_add_event_serialiser

        params = {
            'start_datetime': '2022-01-27 16:00',
            'places': 12,
            'recurrence_days': '0, 3, 5',
        }
        resp = app.post(api_url, params=params)

le serializer reçoit la liste ['0, 3, 5'], et il passe ses éléments
au validateur enfant (serializers.IntegerField) qui reçoit la chaîne '0, 3, 5'.

class StringOrListField(serializers.ListField):
    def to_internal_value(self, data):
        data_initial = data
        if isinstance(data, list) and len(data) == 1:
            data = data[0]
        if isinstance(data, str):
            data = [s.strip() for s in data.split(',') if s.strip()]
        import pdb; pdb.set_trace()
>       pasglop = super().to_internal_value(data_initial)
        return super().to_internal_value(data)

(Pdb) data_initial
['0, 3, 5']
(Pdb) n
rest_framework.exceptions.ValidationError: {0: [ErrorDetail(string='A valid integer is required.', code='invalid')]}
(Pdb) d
(Pdb) d
rest_framework/fields.py(1688)run_child_validation()
(Pdb) self.child.run_validation('0, 3, 5')
*** rest_framework.exceptions.ValidationError: [ErrorDetail(string='A valid integer is required.', code='invalid')]

(je n'ai pas compris pourquoi l'on reçoit une liste ici mais j'avoue que je n'ai pas encore testé pour voir ce que l'on reçoit de wcs et si ce sont en fait ce ne serait pas plutôt les tests qui sont faux)

#8

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

  • Statut changé de Solution proposée à En cours

Sauf que ça crashera si wcs envoie une (vraie) liste avec 1 seul entier.

#9

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

Quand wcs envoie la chaine '0, 3, 5', elle est reçu telle qu'elle par le serializer, mais ce n'est pas le cas avec une simple requête request (sans Content-type': application/json dans les headers comme le fait wcs) ou curl.
Je garde donc ma rustine, mais en la précisant pour qu'elle n'attrape que les tableaux contenant une unique chaîne de caractères.

0001: remplacer par StringOrListField et ensuite réparer
0002: ne plus planter sur la trace ci-dessus (et accepter l'entier).

#11

Mis à jour par Valentin Deniaud il y a environ 2 ans

Nicolas Roche a écrit :

Quand wcs envoie la chaine '0, 3, 5', elle est reçu telle qu'elle par le serializer, mais ce n'est pas le cas avec une simple requête request (sans Content-type': application/json dans les headers comme le fait wcs) ou curl.
Je garde donc ma rustine, mais en la précisant pour qu'elle n'attrape que les tableaux contenant une unique chaîne de caractères.

Je pense qu'il ne faut pas jouer à inventer des problèmes dans les tests, et ne pas garder cette rustine.

On doit valider la liste ou la chaîne de caractère qui nous est passée ; on ne doit pas accepter de liste de liste cheloue.

pour éviter la régression sur ce test : tests/api/test_event.py::test_add_event_serialiser

On peut considérer qu'ici c'est le test qui est faux, et qu'il faut utiliser app.post_json ce qui devrait lui permettre de passer.


Désolé en tout cas d'avoir immédiatement embrouillé le ticket en demandant un changement de serializer, tu as toujours la possibilité de ragequit en tapant genre

@@ -32,2 +32,4 @@ class CommaSeparatedStringField(serializers.ListField):
     def to_internal_value(self, data):
+        if not isinstance(data, str):
+            raise ValidationError('input must be a string')
         data = [s.strip() for s in data.split(',') if s.strip()]

ce qui aurait été le bon fix simple si je n'avais rien dit.

#12

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

On peut considérer qu'ici c'est le test qui est faux, et qu'il faut utiliser app.post_json ce qui devrait lui permettre de passer.

En effet, merci pour cette indication.

#13

Mis à jour par Valentin Deniaud il y a environ 2 ans

  • Statut changé de Solution proposée à Solution validée

Nickel

#14

Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 2 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit ac41d09c94716e154cacaad5c5fb7207a39158d1
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Wed Feb 2 18:53:31 2022 +0100

    api: use StringOrListField for recurrence_days serialiser (#60351)

commit e532eefc0e11ef9ab48965b87cd81c5610cf6891
Author: Nicolas ROCHE <nroche@entrouvert.com>
Date:   Mon Feb 14 17:19:03 2022 +0100

    tests: change post/patch to pass json to event view (#60351)
#15

Mis à jour par Transition automatique il y a environ 2 ans

  • Statut changé de Résolu (à déployer) à Solution déployée
#16

Mis à jour par Transition automatique il y a presque 2 ans

Automatic expiration

Formats disponibles : Atom PDF