change return code for invalid data in handlePartialUpdateItem - [merged] #118

Closed benoitdm-oslandia closed 1 year ago

In GitLab by @jmkerloch on Oct 11, 2022, 17:57

_Merges feature/update_return_code_for_invalidjson -> develop

related #44

`api/api.go`

add ErrMsgCollectionRequestBodyRead const for common error string.

`service/util.go`

rename appErrorInternalFmt to appErrorInternal with optional formatting parameter
update appErrorBadRequest function to allow string formatting
rename appErrorNotFoundFmt to appErrorNotFound with optional formatting parameter
remove generic func appErrorMsg : need to use specific function for each error code.

`service/handler.go`

throw error code 400 (Bad request) instead of error code 500 (Internal error) when json content is invalid

In GitLab by @jmkerloch on Oct 11, 2022, 17:57

requested review from @azarz

    errUnMarsh := json.Unmarshal(body, &val)

en fait ici, on récupère un schéma qui peut ne pas contenir les mm info que le GeojsonFeatureData.

marked this merge request as draft

bon départ ! :smile:

Est ce que tu peux ajouter les modifications pour les autres verbes http dans la mm PR stp ?

In GitLab by @jmkerloch on Oct 12, 2022, 08:42

Commented on internal/service/handler.go line 699

Du coup il faut utiliser quel schema ? J'ai utilisé le schema défini dans PartialUpdateTableFeature : https://git.oslandia.net/Client-projects/geoplateforme-ign-pg-featureserv/-/blob/develop/internal/data/catalog_db.go#L298.

Si je ne change pas le schema utilisé, aucune erreur n'est remonté par json.Unmarshal

In GitLab by @jmkerloch on Oct 12, 2022, 08:44

Je voulais faire une MR la plus petite possible, du coup je n'ai touché que à cette partie. Je vais le faire pour toute la fonction handlePartialUpdateItem

Le test sur errUnMarsh ne vérifie que si le contenu est bien correct au niveau json. Le vrai test de validation du schema est fait un peu plus loin en ligne 704

In GitLab by @jmkerloch on Oct 12, 2022, 09:29

Commented on internal/service/handler.go line 699

Le test de validation du schema n'arrive pas en erreur en ligne 704. L'erreur ne se produit que dans PartialUpdateTableFeature.

Je propose de rajouter un json.Unmarshal de api.GeojsonFeatureData avant l'appel à PartialUpdateTableFeature

vu en visio. Voir #63

In GitLab by @jmkerloch on Oct 12, 2022, 10:49

Commented on internal/service/handler.go line 699

changed this line in version 2 of the diff

In GitLab by @jmkerloch on Oct 12, 2022, 10:49

added 1 commit

c644145a - restore test for invalid schema and update return code for bad requests

Compare with previous version

In GitLab by @jmkerloch on Oct 12, 2022, 17:35

added 2 commits

c644145a...57696f50 - 2 commits from branch develop

Compare with previous version

In GitLab by @jmkerloch on Oct 14, 2022, 15:08

added 8 commits

57696f50...c1ddb739 - 8 commits from branch develop

Compare with previous version

In GitLab by @jmkerloch on Oct 14, 2022, 15:45

added 1 commit

40378933 - update return code and remove generic func for app error

Compare with previous version

je pense que ce genre de message doit etre repeté un peu partout dans le code. Est ce que tu peux voir pour trouver les doublons ?

In GitLab by @jmkerloch on Oct 14, 2022, 15:50

Commented on internal/service/util.go line 289

Je ne sais pas quoi mettre comme parametre pour le formattage. Il n'y en avait pas avant du coup j'ai juste mis un string vide.

tu ne mets rien et ca devrait passer

In GitLab by @jmkerloch on Oct 14, 2022, 15:55

Commented on internal/service/util.go line 289

Les tests passent mais j'ai quand même ce message de warning dans VSCode

var err error
github.com/CrunchyData/pg_featureserv/internal/service.appErrorInternal format %v reads arg #1, but call has 0 argsprintf

In GitLab by @jmkerloch on Oct 14, 2022, 15:57

Commented on internal/service/handler.go line 342

changed this line in version 5 of the diff

In GitLab by @jmkerloch on Oct 14, 2022, 15:57

added 1 commit

cc66a2b7 - add ErrMsgCollectionRequestBodyRead for common error messages

Compare with previous version

In GitLab by @jmkerloch on Oct 14, 2022, 15:58

marked this merge request as ready

ahh bah oui: la const api.ErrMsgDataWriteError contient un %v donc tu dois mettre un parametre

In GitLab by @jmkerloch on Oct 14, 2022, 15:59

J'ai fait une passe sur les autres appel à appErrorInternal

In GitLab by @jmkerloch on Oct 14, 2022, 16:04

Commented on internal/service/handler.go line 342

J'ai ajouté la const ErrMsgCollectionRequestBodyRead

ok

hum ... pareil avec celui là ? :innocent:

In GitLab by @jmkerloch on Oct 14, 2022, 16:09

Commented on internal/service/handler.go line 327

OK mais je vais jamais réussi à faire merger ma MR si ca continue :smile:

mais si mais si

In GitLab by @jmkerloch on Oct 14, 2022, 16:11

Commented on internal/service/handler.go line 327

changed this line in version 6 of the diff

In GitLab by @jmkerloch on Oct 14, 2022, 16:11

added 1 commit

a0c52f00 - add api.ErrMsgNoParameters const

Compare with previous version

In GitLab by @jmkerloch on Oct 14, 2022, 16:11

Commented on internal/service/handler.go line 327

done

approved this merge request

In GitLab by @lowzonenose on Oct 14, 2022, 16:20

approved this merge request

In GitLab by @azarz on Oct 14, 2022, 16:36

approved this merge request

In GitLab by @jmkerloch on Oct 17, 2022, 15:58

resolved all threads

benoitdm-oslandia / pg_featureserv