credentials: use more ducks

Instead of making everyone inherit from our credential types, use an
interface with two attributes, which makes the C code much shorter and
simpler.

pygit2/credentials.py

@@ -26,10 +26,9 @@
 # Boston, MA 02110-1301, USA.
 
 # Import from pygit2
-from _pygit2 import CredUsernamePassword, CredSshKey
+from _pygit2 import GIT_CREDTYPE_USERPASS_PLAINTEXT, GIT_CREDTYPE_SSH_KEY
-from _pygit2 import GIT_CREDTYPE_USERPASS_PLAINTEXT
 
-class UserPass(CredUsernamePassword):
+class UserPass:
     """Username/Password credentials
 
     This is an object suitable for passing to a remote's credentials
@@ -37,10 +36,22 @@ class UserPass(CredUsernamePassword):
 
     """
 
+    def __init__(self, username, password):
+        self._username = username
+        self._password = password
+
+    @property
+    def credential_type(self):
+        return GIT_CREDTYPE_USERPASS_PLAINTEXT
+
+    @property
+    def credential_tuple(self):
+        return (self._username, self._password)
+
     def __call__(self, _url, _username, _allowed):
         return self
 
-class Keypair(CredSshKey):
+class Keypair:
     """SSH key pair credentials
 
     This is an object suitable for passing to a remote's credentials
@@ -48,5 +59,19 @@ class Keypair(CredSshKey):
 
     """
 
+    def __init__(self, username, pubkey, privkey, passphrase):
+        self._username = username
+        self._pubkey = pubkey
+        self._privkey = privkey
+        self._passphrase = passphrase
+
+    @property
+    def credential_type(self):
+        return GIT_CREDTYPE_SSH_KEY
+
+    @property
+    def credential_tuple(self):
+        return (self._username, self._pubkey, self._privkey, self._passphrase)
+
     def __call__(self, _url, _username, _allowed):
         return self

src/credentials.c

@@ -1,225 +0,0 @@
-/*
- * Copyright 2010-2014 The pygit2 contributors
- *
- * This file is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License, version 2,
- * as published by the Free Software Foundation.
- *
- * In addition to the permissions in the GNU General Public License,
- * the authors give you unlimited permission to link the compiled
- * version of this file into combinations with other programs,
- * and to distribute those combinations without any restriction
- * coming from the use of this file.  (The General Public License
- * restrictions do apply in other respects; for example, they cover
- * modification of the file, and distribution when not linked into
- * a combined executable.)
- *
- * This file is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; see the file COPYING.  If not, write to
- * the Free Software Foundation, 51 Franklin Street, Fifth Floor,
- * Boston, MA 02110-1301, USA.
- */
-
-#define PY_SSIZE_T_CLEAN
-#include <Python.h>
-#include <structmember.h>
-#include "error.h"
-#include "types.h"
-#include "utils.h"
-#include "oid.h"
-#include "refspec.h"
-#include "remote.h"
-
-int
-CredUsernamePassword_init(CredUsernamePassword *self, PyObject *args, PyObject *kwds)
-{
-    char *username, *password;
-
-    if (kwds && PyDict_Size(kwds) > 0) {
-        PyErr_SetString(PyExc_TypeError, "CredUsernamePassword takes no keyword arguments");
-        return -1;
-    }
-
-    if (!PyArg_ParseTuple(args, "ss", &username, &password))
-        return -1;
-
-    self->parent.credtype = GIT_CREDTYPE_USERPASS_PLAINTEXT;
-
-    self->username = strdup(username);
-    if (!self->username) {
-        PyErr_NoMemory();
-        return -1;
-    }
-
-    self->password = strdup(password);
-    if (!self->password) {
-        free(self->username);
-        PyErr_NoMemory();
-        return -1;
-    }
-
-    return 0;
-}
-
-void
-CredUsernamePassword_dealloc(CredUsernamePassword *self)
-{
-    free(self->username);
-    free(self->password);
-
-    Py_TYPE(self)->tp_free(self);
-}
-
-PyMemberDef CredUsernamePassword_members[] = {
-    MEMBER(CredUsernamePassword, username, T_STRING, "username"),
-    MEMBER(CredUsernamePassword, password, T_STRING, "password"),
-    {NULL},
-};
-
-PyDoc_STRVAR(CredUsernamePassword__doc__,
-  "Credential type for username/password combination");
-
-PyTypeObject CredUsernamePasswordType = {
-    PyVarObject_HEAD_INIT(NULL, 0)
-    "_pygit2.CredUsernamePassword",            /* tp_name           */
-    sizeof(CredUsernamePassword),              /* tp_basicsize      */
-    0,                                         /* tp_itemsize       */
-    (destructor)CredUsernamePassword_dealloc,  /* tp_dealloc        */
-    0,                                         /* tp_print          */
-    0,                                         /* tp_getattr        */
-    0,                                         /* tp_setattr        */
-    0,                                         /* tp_compare        */
-    0,                                         /* tp_repr           */
-    0,                                         /* tp_as_number      */
-    0,                                         /* tp_as_sequence    */
-    0,                                         /* tp_as_mapping     */
-    0,                                         /* tp_hash           */
-    0,                                         /* tp_call           */
-    0,                                         /* tp_str            */
-    0,                                         /* tp_getattro       */
-    0,                                         /* tp_setattro       */
-    0,                                         /* tp_as_buffer      */
-    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,  /* tp_flags          */
-    CredUsernamePassword__doc__,               /* tp_doc            */
-    0,                                         /* tp_traverse       */
-    0,                                         /* tp_clear          */
-    0,                                         /* tp_richcompare    */
-    0,                                         /* tp_weaklistoffset */
-    0,                                         /* tp_iter           */
-    0,                                         /* tp_iternext       */
-    0,                                         /* tp_methods        */
-    CredUsernamePassword_members,              /* tp_members        */
-    0,                                         /* tp_getset         */
-    0,                                         /* tp_base           */
-    0,                                         /* tp_dict           */
-    0,                                         /* tp_descr_get      */
-    0,                                         /* tp_descr_set      */
-    0,                                         /* tp_dictoffset     */
-    (initproc)CredUsernamePassword_init,       /* tp_init           */
-    0,                                         /* tp_alloc          */
-    0,                                         /* tp_new            */
-};
-
-int
-CredSshKey_init(CredSshKey *self, PyObject *args, PyObject *kwds)
-{
-    char *username, *pubkey, *privkey, *passphrase;
-
-    if (kwds && PyDict_Size(kwds) > 0) {
-        PyErr_SetString(PyExc_TypeError, "CredSshKey takes no keyword arguments");
-        return -1;
-    }
-
-    if (!PyArg_ParseTuple(args, "ssss", &username, &pubkey,
-                                        &privkey, &passphrase))
-        return -1;
-
-    self->parent.credtype = GIT_CREDTYPE_SSH_KEY;
-    self->username = self->pubkey = self->privkey = self->passphrase = NULL;
-
-    self->username = strdup(username);
-    self->pubkey = strdup(pubkey);
-    self->privkey = strdup(privkey);
-    self->passphrase = strdup(passphrase);
-
-    if (!(self->username && self->pubkey && self->privkey && self->passphrase))
-        goto on_oom;
-
-    return 0;
-
-  on_oom:
-    free(self->username);
-    free(self->pubkey);
-    free(self->privkey);
-    free(self->passphrase);
-    PyErr_NoMemory();
-    return -1;
-}
-
-void
-CredSshKey_dealloc(CredSshKey *self)
-{
-    free(self->username);
-    free(self->pubkey);
-    free(self->privkey);
-    free(self->passphrase);
-
-    Py_TYPE(self)->tp_free(self);
-}
-
-PyMemberDef CredSshKey_members[] = {
-    MEMBER(CredSshKey, username, T_STRING, "username"),
-    MEMBER(CredSshKey, pubkey, T_STRING, "pubkey"),
-    MEMBER(CredSshKey, privkey, T_STRING, "privkey"),
-    MEMBER(CredSshKey, passphrase, T_STRING, "passphrase"),
-    {NULL},
-};
-
-PyDoc_STRVAR(CredSshKey__doc__,
-  "Credential type for an SSH keypair");
-
-PyTypeObject CredSshKeyType = {
-    PyVarObject_HEAD_INIT(NULL, 0)
-    "_pygit2.CredSshKey",                      /* tp_name           */
-    sizeof(CredSshKey),                        /* tp_basicsize      */
-    0,                                         /* tp_itemsize       */
-    (destructor)CredSshKey_dealloc,            /* tp_dealloc        */
-    0,                                         /* tp_print          */
-    0,                                         /* tp_getattr        */
-    0,                                         /* tp_setattr        */
-    0,                                         /* tp_compare        */
-    0,                                         /* tp_repr           */
-    0,                                         /* tp_as_number      */
-    0,                                         /* tp_as_sequence    */
-    0,                                         /* tp_as_mapping     */
-    0,                                         /* tp_hash           */
-    0,                                         /* tp_call           */
-    0,                                         /* tp_str            */
-    0,                                         /* tp_getattro       */
-    0,                                         /* tp_setattro       */
-    0,                                         /* tp_as_buffer      */
-    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,  /* tp_flags          */
-    CredSshKey__doc__,                         /* tp_doc            */
-    0,                                         /* tp_traverse       */
-    0,                                         /* tp_clear          */
-    0,                                         /* tp_richcompare    */
-    0,                                         /* tp_weaklistoffset */
-    0,                                         /* tp_iter           */
-    0,                                         /* tp_iternext       */
-    0,                                         /* tp_methods        */
-    CredSshKey_members,                        /* tp_members        */
-    0,                                         /* tp_getset         */
-    0,                                         /* tp_base           */
-    0,                                         /* tp_dict           */
-    0,                                         /* tp_descr_get      */
-    0,                                         /* tp_descr_set      */
-    0,                                         /* tp_dictoffset     */
-    (initproc)CredSshKey_init,                 /* tp_init           */
-    0,                                         /* tp_alloc          */
-    0,                                         /* tp_new            */
-};

src/pygit2.c

@@ -74,8 +74,6 @@ extern PyTypeObject RefLogEntryType;
 extern PyTypeObject BranchType;
 extern PyTypeObject SignatureType;
 extern PyTypeObject RemoteType;
-extern PyTypeObject CredUsernamePasswordType;
-extern PyTypeObject CredSshKeyType;
 extern PyTypeObject RefspecType;
 extern PyTypeObject TransferProgressType;
 extern PyTypeObject NoteType;
@@ -460,13 +458,9 @@ moduleinit(PyObject* m)
 
     /* Remotes */
     INIT_TYPE(RemoteType, NULL, NULL)
-    INIT_TYPE(CredUsernamePasswordType, NULL, PyType_GenericNew)
-    INIT_TYPE(CredSshKeyType, NULL, PyType_GenericNew)
     INIT_TYPE(RefspecType, NULL, NULL)
     INIT_TYPE(TransferProgressType, NULL, NULL)
     ADD_TYPE(m, Remote)
-    ADD_TYPE(m, CredUsernamePassword)
-    ADD_TYPE(m, CredSshKey)
     ADD_TYPE(m, Refspec)
     ADD_TYPE(m, TransferProgress)
     /* Direction for the refspec */

src/remote.c

@@ -39,8 +39,6 @@
 extern PyObject *GitError;
 extern PyTypeObject RepositoryType;
 extern PyTypeObject TransferProgressType;
-extern PyTypeObject CredUsernamePasswordType;
-extern PyTypeObject CredSshKeyType;
 
 PyObject *
 wrap_transfer_progress(const git_transfer_progress *stats)

src/types.h

@@ -207,25 +207,6 @@ typedef struct {
     PyObject *update_tips;
 } Remote;
 
-typedef struct {
-    PyObject_HEAD
-    git_credtype_t credtype;
-} Cred;
-
-typedef struct {
-    Cred parent;
-    char *username;
-    char *password;
-} CredUsernamePassword;
-
-typedef struct {
-    Cred parent;
-    char *username;
-    char *pubkey;
-    char *privkey;
-    char *passphrase;
-} CredSshKey;
-
 /* git_refspec */
 typedef struct {
     PyObject_HEAD

src/utils.c

@@ -31,8 +31,6 @@
 #include "utils.h"
 
 extern PyTypeObject ReferenceType;
-extern PyTypeObject CredUsernamePasswordType;
-extern PyTypeObject CredSshKeyType;
 
 /**
  * py_str_to_c_str() returns a newly allocated C string holding the string
@@ -159,42 +157,62 @@ on_error:
 static int
 py_cred_to_git_cred(git_cred **out, PyObject *py_cred, unsigned int allowed)
 {
-    Cred *base_cred;
+    PyObject *py_type, *py_tuple;
-    int err;
+    long type;
+    int err = -1;
 
-    if (!PyObject_TypeCheck(py_cred, &CredUsernamePasswordType) &&
+    py_type = PyObject_GetAttrString(py_cred, "credential_type");
-        !PyObject_TypeCheck(py_cred, &CredSshKeyType)) {
+    py_tuple = PyObject_GetAttrString(py_cred, "credential_tuple");
-        PyErr_SetString(PyExc_TypeError, "unkown credential type");
+
-        return -1;
+    if (!py_type || !py_tuple) {
+        printf("py_type %p, py_tuple %p\n", py_type, py_tuple);
+        PyErr_SetString(PyExc_TypeError, "credential doesn't implement the interface");
+        goto cleanup;
     }
 
-    base_cred = (Cred *) py_cred;
+    if (!PyLong_Check(py_type)) {
+        PyErr_SetString(PyExc_TypeError, "credential type is not a long");
+        goto cleanup;
+    }
+
+    type = PyLong_AsLong(py_type);
 
     /* Sanity check, make sure we're given credentials we can use */
-    if (!(allowed & base_cred->credtype)) {
+    if (!(allowed & type)) {
         PyErr_SetString(PyExc_TypeError, "invalid credential type");
-        return -1;
+        goto cleanup;
     }
 
-    switch (base_cred->credtype) {
+    switch (type) {
     case GIT_CREDTYPE_USERPASS_PLAINTEXT:
     {
-        CredUsernamePassword *cred = (CredUsernamePassword *) base_cred;
+        const char *username, *password;
-        err = git_cred_userpass_plaintext_new(out, cred->username, cred->password);
+
+        if (!PyArg_ParseTuple(py_tuple, "ss", &username, &password))
+            goto cleanup;
+
+        err = git_cred_userpass_plaintext_new(out, username, password);
         break;
     }
     case GIT_CREDTYPE_SSH_KEY:
     {
-        CredSshKey *cred = (CredSshKey *) base_cred;
+        const char *username, *pubkey, *privkey, *passphrase;
-        err = git_cred_ssh_key_new(out, cred->username, cred->pubkey, cred->privkey, cred->passphrase);
+
+        if (!PyArg_ParseTuple(py_tuple, "ssss", &username, &pubkey, &privkey, &passphrase))
+            goto cleanup;
+
+        err = git_cred_ssh_key_new(out, username, pubkey, privkey, passphrase);
         break;
     }
     default:
         PyErr_SetString(PyExc_TypeError, "unsupported credential type");
-        err = -1;
         break;
     }
 
+cleanup:
+    Py_XDECREF(py_type);
+    Py_XDECREF(py_tuple);
+
     return err;
 }
 

test/test_credentials.py

@@ -30,7 +30,6 @@
 
 import unittest
 import pygit2
-from pygit2 import CredUsernamePassword, CredSshKey
 from pygit2 import GIT_CREDTYPE_USERPASS_PLAINTEXT
 from pygit2 import UserPass, Keypair
 from . import utils
@@ -49,9 +48,8 @@ class CredentialCreateTest(utils.NoRepoTestCase):
         username = "git"
         password = "sekkrit"
 
-        cred = CredUsernamePassword(username, password)
+        cred = UserPass(username, password)
-        self.assertEqual(username, cred.username)
+        self.assertEqual((username, password), cred.credential_tuple)
-        self.assertEqual(password, cred.password)
 
     def test_ssh_key(self):
         username = "git"
@@ -59,11 +57,8 @@ class CredentialCreateTest(utils.NoRepoTestCase):
         privkey = "id_rsa"
         passphrase = "bad wolf"
 
-        cred = CredSshKey(username, pubkey, privkey, passphrase)
+        cred = Keypair(username, pubkey, privkey, passphrase)
-        self.assertEqual(username, cred.username)
+        self.assertEqual((username, pubkey, privkey, passphrase), cred.credential_tuple)
-        self.assertEqual(pubkey, cred.pubkey)
-        self.assertEqual(privkey, cred.privkey)
-        self.assertEqual(passphrase, cred.passphrase)
 
 class CredentialCallback(utils.RepoTestCase):
     def test_callback(self):
@@ -79,7 +74,7 @@ class CredentialCallback(utils.RepoTestCase):
     def test_bad_cred_type(self):
         def credentials_cb(url, username, allowed):
             self.assertTrue(allowed & GIT_CREDTYPE_USERPASS_PLAINTEXT)
-            return CredSshKey("git", "foo.pub", "foo", "sekkrit")
+            return Keypair("git", "foo.pub", "foo", "sekkrit")
 
         remote = self.repo.create_remote("github", "https://github.com/github/github")
         remote.credentials = credentials_cb