Repository: repobee/repobee (python)

Issue:
Generalize the API class to allow for different specializations (e.g. GitHub and GitLab)
There are a few different approaches here. It could be useful to define an abstract base class (like the API was defined before), but that would also make all of the abstract methods required. Maybe defining a custom metaclass that just issues a very specific crash message if any method that is defined for the generalized API, but is not implemented in the specialized API, is called.

I think that whichever approach is taken, that the API should be defined as a plugin.

Pull request:
Add generalized API metaclass
Adds a metaclass defining the platform (e.g. GitHub, GitLab) API behavior required by RepoBee. Should move over to `repobee-plug` once it's stable, so APIs can be implemented as plugins.

Fix #178 

Patch:
diff --git a/repobee/apimeta.py b/repobee/apimeta.py
new file mode 100644
index 0000000..2e9569d
--- /dev/null
+++ b/repobee/apimeta.py
@@ -0,0 +1,131 @@
+"""Metaclass for API implementations.
+
+:py:class:`APIMeta` defines the behavior required of platform API
+implementations, based on the methods in :py:class:`APISpec`. With platform
+API, we mean for example the GitHub REST API, and the GitLab REST API. The
+point is to introduce another layer of indirection such that higher levels of
+RepoBee can use different platforms in a platform-independent way.
+:py:class:`API` is a convenience class so consumers don't have to use the
+metaclass directly.
+
+Any class implementing a platform API should derive from :py:class:`API`. It
+will enforce that all public methods are one of the method defined py
+:py:class:`APISpec`, and give a default implementation (that just raises
+NotImplementedError) for any unimplemented API methods.
+
+.. module:: apimeta
+    :synopsis: Metaclass for API implementations.
+
+.. moduleauthor:: Simon Larsén
+"""
+import inspect
+
+from repobee import exception
+
+
+class APISpec:
+    """Wrapper class for API method stubs."""
+
+    def __init__(self, base_url, token, org_name, user):
+        _not_implemented()
+
+    def ensure_teams_and_members(self, member_lists, permission):
+        _not_implemented()
+
+    def get_teams(self):
+        _not_implemented()
+
+    def create_repos(self, repos):
+        _not_implemented()
+
+    def get_repo_urls(self, master_repo_names, org_name, students):
+        _not_implemented()
+
+    def get_issues(self, repo_names, state, title_regex):
+        _not_implemented()
+
+    def open_issue(self, title, body, repo_names):
+        _not_implemented()
+
+    def close_issue(self, title_regex, repo_names):
+        _not_implemented()
+
+    def add_repos_to_review_teams(self, team_to_repos, issue):
+        _not_implemented()
+
+    def get_review_progress(self, review_team_names, students, title_regex):
+        _not_implemented()
+
+    def delete_teams(self, team_names):
+        _not_implemented()
+
+    @staticmethod
+    def verify_settings(user, org_name, base_url, token, master_org_name):
+        _not_implemented()
+
+
+def _not_implemented():
+    raise NotImplementedError(
+        "The chosen API does not currently support this functionality"
+    )
+
+
+def methods(attrdict):
+    """Return all public methods and __init__ for some class."""
+    return {
+        name: method
+        for name, method in attrdict.items()
+        if callable(method)
+        and not (name.startswith("_") or name == "__init__")
+    }
+
+
+def parameter_names(function):
+    """Extract parameter names (in order) from a function."""
+    return [
+        param.name for param in inspect.signature(function).parameters.values()
+    ]
+
+
+def check_signature(reference, compare):
+    """Check if the compared method matches the reference signature. Currently
+    only checks parameter names and order of parameters.
+    """
+    reference_params = parameter_names(reference)
+    compare_params = parameter_names(compare)
+    if reference_params != compare_params:
+        raise exception.APIImplementationError(
+            "expected method '{}' to have parameters '{}', "
+            "found '{}'".format(
+                reference.__name__, reference_params, compare_params
+            )
+        )
+
+
+class APIMeta(type):
+    """Metaclass for an API implementation. All public methods must be a
+    specified api method, but all api methods do not need to be implemented.
+    Any unimplemented api method will be replaced with a default implementation
+    that simply raises a NotImplementedError.
+    """
+
+    def __new__(cls, name, bases, attrdict):
+        api_methods = methods(APISpec.__dict__)
+        implemented_methods = methods(attrdict)
+        non_api_methods = set(implemented_methods.keys()) - set(
+            api_methods.keys()
+        )
+        if non_api_methods:
+            raise exception.APIImplementationError(
+                "non-API methods may not be public: {}".format(non_api_methods)
+            )
+        for method_name, method in api_methods.items():
+            if method_name in implemented_methods:
+                check_signature(method, implemented_methods[method_name])
+            else:
+                attrdict[method_name] = method
+        return super().__new__(cls, name, bases, attrdict)
+
+
+class API(metaclass=APIMeta):
+    """API base class that all API implementations should inherit from."""
diff --git a/repobee/exception.py b/repobee/exception.py
index bbe4ad9..15e6e06 100644
--- a/repobee/exception.py
+++ b/repobee/exception.py
@@ -28,6 +28,10 @@ class RepoBeeException(Exception):
         return "<{}(msg='{}')>".format(type(self).__name__, str(self.msg))
 
 
+class APIImplementationError(RepoBeeException):
+    """Raise when an API is defined incorrectly."""
+
+
 class ParseError(RepoBeeException):
     """Raise when something goes wrong in parsing."""
 


Test patch:
diff --git a/tests/test_apimeta.py b/tests/test_apimeta.py
new file mode 100644
index 0000000..92f85de
--- /dev/null
+++ b/tests/test_apimeta.py
@@ -0,0 +1,52 @@
+import pytest
+from repobee import apimeta
+from repobee import exception
+
+
+def api_methods():
+    methods = apimeta.methods(apimeta.APISpec.__dict__)
+    assert methods, "there must be api methods"
+    return methods.items()
+
+
+@pytest.mark.parametrize("method", api_methods())
+def test_raises_when_unimplemented_method_called(method):
+    """Test that get_teams method raises NotImplementedError when called if
+    left undefined.
+    """
+
+    class API(apimeta.API):
+        pass
+
+    name, impl = method
+    params = apimeta.parameter_names(impl)
+
+    with pytest.raises(NotImplementedError):
+        m = getattr(API, name)
+        arguments = (None,) * len(params)
+        m(*arguments)
+
+
+def test_raises_when_method_incorrectly_declared():
+    """``get_teams`` takes only a self argument, so defining it with a
+    different argument should raise.
+    """
+
+    with pytest.raises(exception.APIImplementationError):
+
+        class API(apimeta.API):
+            def get_teams(a):
+                pass
+
+
+def test_accepts_correctly_defined_method():
+    """API should accept a correctly defined method, and not alter it in any
+    way.
+    """
+    expected = 42
+
+    class API(apimeta.API):
+        def get_teams(self):
+            return expected
+
+    assert API().get_teams() == expected