Produits Entr'ouvert » Passerelle

Je n'avais pas saisi qu'il fallait aussi en chercher.
Du coup le endpoint renvoit dans le json dans 'text' le contenu en base64?

J'ai fait le endpoint pour déposer mais je n'ai pas encore les tests. Est ce que je mets déjà un patch ou j'attends de faire aussi la récupération?

Le fichier views.py vide peut être laissé de côté.

cmis_client plutôt que cmisClient.

username plutôt que login.

typo dans plateform.

Dans cmislib le content type peut être laissé à None, rendre celui-ci optionnel dans le connecteur ?

Retourner quelque chose pour dire que ça a marché ou que ça a échoué (mauvais json en entrée, chemin non existant, etc.)

Idéalement au moins un test.

Une seule migration nécessaire (initial). Eviter aussi de mettre trop de choses à l'intérieur d'un try/except. Courage :)

Ça me semble ok ; quelque test et c'est ok.

Manque un entête avec la licence dans les fichiers + une dépendance à python-cmslib dans debian/control.

body = ast.literal_eval(response.body)

Ici aussi, json.load.

Aussi il faudrait limiter l'accès à cette API (perm="can_upload_file", genre).

De mon point de vue ça manque de contrôle des entrées ici:

        data = json.loads(request.body)
        title = data['filename']
        path = data['path'].encode('utf-8')
        content = base64.b64decode(data['content'])
        content_type = data['contentType']

Si le contenu n'est pas du JSON ou si une des clé manque ou si la clé content n'est pas du base64.

• dans settings, ne pas retirer le commentaire "# backoffice templates and static" avant gadjo
• il faudrait renvoyer du 400 quand les données en entrée sont mauvaises
• les help_text sont inutiles
• je ne suis pas très fan des « except Exception as e: » en général, qui vont empecher de remonter des alertes "bas niveau" quand il y aura un vrai problème
• la fonction cmis_connection() a un nom un peu bizarre, parce qu'elle revoie en fait un repository (et pas une "connection")
• dans les tests tu pourrais ajouter quelques plantages que tu as prévus dans le code, en envoyant des mauvais fake_file
• il ne faut pas construire l'url avec reverse, mais en dur, comme ça si on la change un jour par mégarde, le test plantera (regression)

Par ailleurs est-on sûr de toujours vouloir envoyer dans defaultRepository et pas ailleurs (bon, c'est un autre sujet, je ne connais pas bien le cadre d'utilisation de ce connecteur) ?

Thomas Noël a écrit :

Je dirais que:

• il faudrait renvoyer du 400 quand les données en entrée sont mauvaises

non pas de 400, que du 'err': x on a dit, on laisse les erreurs HTTPs à Django.

Arf : « il faudrait renvoyer du 400 quand les données en entrée sont mauvaises » en fait non, je me trompe encore, excuses.

Ce sont nginx ou les middleware de Django qui renvoient les erreurs HTTP : le code de nos connecteurs doit toujours renvoyer 200, mais avec une err (raise ApiError(message)) le cas échéant.

A-t-on vraiment besoin de urls.py/views.py ?

Ça ne permet pas l'affichage du détails?
Ou c'est géré en amont si tu as un whatever-details.html ?

C'est géré via :

    url(r'^(?P<connector>[\w,-]+)/(?P<slug>[\w,-]+)/$',
        GenericConnectorView.as_view(), name='view-connector'),

Il faut ajouter ça à ton .gitconfig, comme ça quand tu regardes ton patch avec "git show" avant de l'envoyer, il y aura en rouge les espaces de fin de ligne, à supprimer.

[color]
  branch = auto
  diff = auto
  interactive = auto
  status = auto

La migration annonce une dépendance sur 0003_endpointcheckping qui n'existe pas encore.

Il faut aussi ajouter PASSERELLE_APP_CMIS_ENABLED = True au settings.py.

Il faut modifier le _detail.html pour définir un @{% block description %}, pour que le mot de passe n'apparaisse pas. Alternativement, pouvoir marquer ces champs d'une manière ou d'une autre et faire respecter ça à service_view.html.

Prendre en compte les derniers commentaires, et les modifs récentes dans passerelle.

instance publique d'Alfresco pour test CMIS: https://www.alfresco.com/cmis

Voilà j'ai remis le patch avec un bémol sur mes tests.
J'ai galéré à retrouver le endpoint pour cmis sur alfresco, d'abord sur l'instance publique puis ensuite en local (superbe doc). La lib python cmislib se connecte à partir d'une certaine url (pour l'instant je n'ai vu que des exemples pour alfresco mais bon). C'est plus ou moins une sorte de wsdl (cmisatom).

Mais ça fonctionne, j'ai créé un connecteur avec AccessRight, ApiUser et j'ai pu poser mon fichier.

Mais quand je lance les tests, j'ai un

E   RuntimeError: Conflicting 'cmisconnector_users' models in application 'cmis': <class 'passerelle.apps.cmis.models.CmisConnector_users'> and <class 'cmis.models.CmisConnector_users'>

Pour tester sur la plateforme en ligne, il faut déclarer le connecteur comme ceci:

CmisConnector.objects.create(cmis_endpoint='https://cmis.alfresco.com/alfresco/cmisatom', username='admin', password='admin')

Dans le payload qui doit ressembler à ça :

fake_file = {
    'filename': 'test.txt',
    'path': '/Partagé',
    'content': base64.b64encode('fake file content.'),
    'contentType': 'text/plain'
}

Si le path est à '', j'ai fait en sorte que ça prenne le Folder racine du dépot, donc ça mettra le fichier dedans (la valeur Partagé c'est sur mon local, j'ai un dossier Partagé). Comme on a pas accès au dossiers directement, c'est plus simple pour tester (je suis aller regarder avec cmis ce qu'il y avait sur le repo).

Je me demandais si du coup les tests je les faisais dessus, mais il faudrait supprimer le fichier à chaque fois (et je ne sais pas si cmislib le permet, je vais regarder) et évidemment, si la plateforme disparaît ou tombe, tous les tests foirent. Mais bon

E RuntimeError: Conflicting 'cmisconnector_users' models in application 'cmis': <class 'passerelle.apps.cmis.models.CmisConnector_users'> and <class 'cmis.models.CmisConnector_users'>

C'est parce que d'un côté tu as passerelle.apps.cmis et de l'autre juste cmis. Ma recommandation et d'utiliser passerelle.apps.cmis. partout.

Je me demandais si du coup les tests je les faisais dessus, mais il faudrait supprimer le fichier à chaque fois (et je ne sais pas si cmislib le permet, je vais regarder) et évidemment, si la plateforme disparaît ou tombe, tous les tests foirent. Mais bon

Les tests doivent fonctionner hors ligne, utiliser Mock.

    cmis_endpoint = models.CharField(max_length=250, verbose_name=_('CMIS endpoint'),
                                     help_text=_('URL of the CMIS endpoint'))

Pourquoi pas URLField ?

raise APIError('CmisError : Invalid JSON string sent.')

et les autres; plutôt faire une exception CMISError(APIError) que préfixer les messages avec CmisError. Cela évitera aussi un commentaire sur l'espace en trop devant les :.

repo_name = kwargs.get('repo', False)

Pourquoi "False" ?

        if repo_name:
            for repo in cmis_client.getRepositories():
                if repo_name == repo['repositoryName']:
                    return cmis_client.getRepository(repo['repositoryId'])
        return cmis_client.defaultRepository

Le comportement quand on spécifié un repo_name et qu'il n'existe pas est mauvais : il ne faut pas (surtout silencieusement) retomber sur le dépôt par défaut.

{% block description %}
{% endblock %}

Noter pourquoi on refuse d'afficher la description automatique. (parce qu'on ne veut pas afficher le mot de passe) Mais le vrai truc ce serait d'afficher la description, sans le mot de passe. Ça pourrait être fait en gardant le block description natif mais en ajoutant un get_description_fields dans le connecteur.

    conn = CmisConnector.objects.create(cmis_endpoint='http://cmis_endpoint.com/cmis', username='admin',

Pour les tests, il faut utiliser example.{net,com,org}, pas inventer des domaines.

{% blocktrans %}
The API currently doesn't support all parameters and is limited to the JSON
format.
{% endblocktrans %}

Me semble un copié/collé sans rapport.

Juste parce que ça fait moche dans mon vim, c'est deux lignes avant un classe ou une fonction de top niveau (dans les tests) et une ligne avant une méthode, sauf en début de classe.

Application de  cmis: add cmis connector to upload file (#12876)
.git/rebase-apply/patch:323: new blank line at EOF.
+
warning: 1 ligne a ajouté des erreurs d'espace.

Il manque dans cmisconnector_detail.html l'info pointant l'endpoint existant (url, méthode HTTP, description du json attendu, etc.).

{% block endpoints %}

<ul>

  <li>

    <h4>{% trans 'Upload file' %}</h4>

    {% url "generic-endpoint" connector="cmis" slug=object.slug endpoint="upload_file" as upload_file %}

    <p> <strong>POST</strong> <a href="{{upload_file}}">{{upload_file}}</a></p>

    <p>

     data_send = {

       'filename': 'test.txt',

       'path': '/a/path',

       'content': 'ZmlsZSBjb250ZW50' (base64),

       'contentType': 'text/plain'

    }

    </p>

  </li>

</ul>

{% endblock %}

Il faut utiliser le tag "pre" et pas "p" sinon tu on ne verra pas la formatage de ton exemple de JSON.

Voilà, relecture et correction, http://git.entrouvert.org/passerelle.git/log/?h=wip/relecture-ticket-cmis-12876 . Les tests passent.

Ok, envoie.