Contradiction dans la manière de renseigner le champ fields

[alexisthethe]

reférence : https://github.com/before-interop/common/blob/main/common/utils/fields.openapi.yaml#L21

le champ est défini comme array[string], mais le commentaire dit qu'il faut le renseigner avec des virgules.

Je propose de garder uniquement 1 seule façon de rensegner ce champ qui convienne à tout le monde pour ne pas implémenter du superflu et réduire les possibilités de bugs.

Je propose de conserver le format array[string]

[ggrebert]

En effet, il y a une erreur. Je préfèrerais conserver la séparation par , car c'est la norme utilisée par TMF

TMF630_REST_API_Design_Guidelines_Part_1_R14.5.1.pdf

[alexisthethe]

je trouve cela moins pratique.. pas vous ?

[ggrebert]

TMF n'est pas toujours au top....

[alexisthethe]

A-t-on réellement un intérêt à se forcer à suivre TMF ? ou ne peut-on pas simplement considérer TMF comme un exemple de design d'API mais duquel on peut dériver si on le juge utile ?

[alexisthethe]

(mon avis personnel est plutôt sur la 2e option tu t'en doute ^^ mais je ne sais pas quelle est la vision des autres opérateurs ou des autres protocoles interop)

[ggrebert]

Honnêtement, ça change pas grand chose:

faire un tableau est plus sexy par contre, ça augmente le nombre de caractères dans l'url (cf https://github.com/before-interop/malfacon/issues/53).

De plus, il n'est pas possible qu'un attribut contienne le caractère , donc 0 impact.

Le principal problème avec cette modification, c'est qu'elle est dans common et qu'elle est cassante. Donc il faudrait aussi faire le changement pour le protocole AnomalieAdresse et ça, je suis par trop pour car celui-ci a été validé complètement.

[ggrebert]

Hello @alexisthethe

On peut proposer un compromis: Autoriser les 2 solutions. D'ailleurs, en regardant le POC que j'avais fait, j'avais déjà implémenter cette phylosophie: https://github.com/ggrebert/arcep-poc-tmf/blob/main/src/main/java/fr/arcep/tmf/util/TMFilter.java#L52-L83

On pourrait simplement mettre à jour la doc pour mieux expliquer les possibilités. Et l'avantage serait de garder la compatibilité avec AnomalieAdresse.

Qu'en penses tu ?

[ggrebert]

En relisant la doc

On peut constater qu'elle autorise déjà les 2 solutions.

@alexisthethe : peut-on clore cette issue ?

[ggrebert]

@alexisthethe up ?

[alexisthethe]

Justement, c'est ça qui me dérange. Qu'on doive implémenter 2 méthodes. Il faut en choisir une des deux.

[ggrebert]

C'est une fonction de 4 lignes et ça simplifie la vie à tous le monde. Et surtout on ne modifie ni le protocole Malfacon ni AnoAdresse

Exemple en Python:

from flask import Flask
from flask import jsonify
from flask import request

app = Flask(__name__)

@app.route("/")
def home():
    fields = clean_fields(request.args.getlist('fields'))
    return jsonify({"fields": fields})

def clean_fields(fields: list) -> list:
    unique_fields = set()
    for field in fields:
        unique_fields.update(field.split(","))

    return [f.strip() for f in unique_fields if f.strip()]

Exemple en Java:

public static List<String> cleanFields(List<String> fields) {
        var uniqueFields = new HashSet<String>();
        fields.forEach(field -> uniqueFields.addAll(List.of(field.split(","))));
        return uniqueFields.stream()
                .map(String::trim)
                .filter(s -> !s.isEmpty())
                .collect(Collectors.toList());
}

[alexisthethe]

Ce n'est pas une question de code, c'est plutôt pour éviter si possible de complexifier l'API pour rien. Je trouve que c'est une mauvaise pratique de cumuler les différentes possibilités pour 1 même action lorsqu'on définit un protocole.

J'attends l'avis des autres opérateurs sur la question.

[richard-olvera]

on ne garde que la solution avec fields avec l'array : fields=id&fields=name&...