Merge pull request CenterForOpenScience#10475 from cslzchen/feature/e…

…ng-4897-tests-addon

[ENG-4897] Unit tests for Boa add-on + improve Boa add-on

addons/base/exceptions.py

@@ -1,21 +1,27 @@
-"""
-Custom exceptions for add-ons.
-"""
-
 class AddonError(Exception):
     pass
 
+
 class InvalidFolderError(AddonError):
     pass
 
+
 class InvalidAuthError(AddonError):
     pass
 
+
 class HookError(AddonError):
     pass
 
+
 class QueryError(AddonError):
     pass
 
+
 class DoesNotExist(AddonError):
     pass
+
+
+class NotApplicableError(AddonError):
+    """This exception is used by non-storage and/or non-oauth add-ons when they don't need or have certain features."""
+    pass

addons/base/serializer.py

@@ -141,7 +141,7 @@ class StorageAddonSerializer(OAuthAddonSerializer):
     REQUIRED_URLS = ('auth', 'importAuth', 'folders', 'files', 'config', 'deauthorize', 'accounts')
 
     @abc.abstractmethod
-    def credentials_are_valid(self, user_settings):
+    def credentials_are_valid(self, user_settings, client=None):
         pass
 
     @abc.abstractmethod
@@ -154,7 +154,7 @@ def serialize_settings(self, node_settings, current_user, client=None):
         current_user_settings = current_user.get_addon(self.addon_short_name)
         user_is_owner = user_settings is not None and user_settings.owner == current_user
 
-        valid_credentials = self.credentials_are_valid(user_settings, client)
+        valid_credentials = self.credentials_are_valid(user_settings, client=client)
 
         result = {
             'userIsOwner': user_is_owner,

addons/base/tests/base.py

@@ -1,9 +1,6 @@
-from nose.tools import *  # noqa (PEP8 asserts)
+from django.conf import settings as django_settings
 
 from framework.auth import Auth
-
-from django.conf import settings
-
 from osf_tests.factories import AuthUserFactory, ProjectFactory
 
 
@@ -28,31 +25,34 @@ class AddonTestCase(object):
 
     """
     DISABLE_OUTGOING_CONNECTIONS = True
-    DB_NAME = getattr(settings, 'TEST_DB_ADDON_NAME', 'osf_addon')
+    DB_NAME = getattr(django_settings, 'TEST_DB_ADDON_NAME', 'osf_addon')
     ADDON_SHORT_NAME = None
     OWNERS = ['user', 'node']
     NODE_USER_FIELD = 'user_settings'
 
+    def __init__(self, *args, **kwargs):
+        super(AddonTestCase,self).__init__(*args, **kwargs)
+        self.node_settings = None
+        self.project = None
+        self.user = None
+        self.user_settings = None
+
     # Optional overrides
-    def create_user(self):
+    @staticmethod
+    def create_user():
         return AuthUserFactory.build()
 
     def create_project(self):
         return ProjectFactory(creator=self.user)
 
     def set_user_settings(self, settings):
-        """Make any necessary modifications to the user settings object,
-        e.g. setting access_token.
-
-        """
         raise NotImplementedError('Must define set_user_settings(self, settings) method')
 
     def set_node_settings(self, settings):
         raise NotImplementedError('Must define set_node_settings(self, settings) method')
 
     def create_user_settings(self):
         """Initialize user settings object if requested by `self.OWNERS`.
-
         """
         if 'user' not in self.OWNERS:
             return
@@ -64,9 +64,7 @@ def create_user_settings(self):
 
     def create_node_settings(self):
         """Initialize node settings object if requested by `self.OWNERS`,
-        additionally linking to user settings if requested by
-        `self.NODE_USER_FIELD`.
-
+        additionally linking to user settings if requested by `self.NODE_USER_FIELD`.
         """
         if 'node' not in self.OWNERS:
             return
@@ -86,19 +84,26 @@ def setUp(self):
         if not self.ADDON_SHORT_NAME:
             raise ValueError('Must define ADDON_SHORT_NAME in the test class.')
         self.user.save()
-
         self.project = self.create_project()
         self.project.save()
-
         self.create_user_settings()
         self.create_node_settings()
 
+
 class OAuthAddonTestCaseMixin(object):
 
     @property
     def ExternalAccountFactory(self):
         raise NotImplementedError()
 
+    def __init__(self, *args, **kwargs):
+        super(OAuthAddonTestCaseMixin, self).__init__(*args, **kwargs)
+        self.auth = None
+        self.external_account = None
+        self.project = None
+        self.user_settings = None
+        self.user = None
+
     def set_user_settings(self, settings):
         self.external_account = self.ExternalAccountFactory()
         self.external_account.save()

addons/base/tests/views.py

@@ -1,20 +1,27 @@
-from rest_framework import status as http_status
-from future.moves.urllib.parse import urlparse, urljoin, parse_qs
-
+from future.moves.urllib.parse import urlparse, parse_qs
 import mock
+from nose.tools import *  # noqa
 import responses
+from rest_framework import status as http_status
+
 from addons.base.tests.base import OAuthAddonTestCaseMixin
 from framework.auth import Auth
 from framework.exceptions import HTTPError
-from nose.tools import (assert_equal, assert_false, assert_in, assert_is_none,
-                        assert_not_equal, assert_raises, assert_true)
 from osf_tests.factories import AuthUserFactory, ProjectFactory
 from osf.utils import permissions
 from website.util import api_url_for, web_url_for
 
 
 class OAuthAddonAuthViewsTestCaseMixin(OAuthAddonTestCaseMixin):
 
+    @property
+    def ADDON_SHORT_NAME(self):
+        raise NotImplementedError()
+
+    @property
+    def ExternalAccountFactory(self):
+        raise NotImplementedError()
+
     @property
     def Provider(self):
         raise NotImplementedError()
@@ -68,8 +75,21 @@ def test_delete_external_account_not_owner(self):
         res = self.app.delete(url, auth=other_user.auth, expect_errors=True)
         assert_equal(res.status_code, http_status.HTTP_403_FORBIDDEN)
 
+
 class OAuthAddonConfigViewsTestCaseMixin(OAuthAddonTestCaseMixin):
 
+    def __init__(self, *args, **kwargs):
+        super(OAuthAddonConfigViewsTestCaseMixin,self).__init__(*args, **kwargs)
+        self.node_settings = None
+
+    @property
+    def ADDON_SHORT_NAME(self):
+        raise NotImplementedError()
+
+    @property
+    def ExternalAccountFactory(self):
+        raise NotImplementedError()
+
     @property
     def folder(self):
         raise NotImplementedError("This test suite must expose a 'folder' property.")
@@ -218,8 +238,35 @@ def test_deauthorize_node(self):
         last_log = self.project.logs.latest()
         assert_equal(last_log.action, '{0}_node_deauthorized'.format(self.ADDON_SHORT_NAME))
 
+
 class OAuthCitationAddonConfigViewsTestCaseMixin(OAuthAddonConfigViewsTestCaseMixin):
 
+    def __init__(self, *args, **kwargs):
+        super(OAuthAddonConfigViewsTestCaseMixin,self).__init__(*args, **kwargs)
+        self.mock_verify = None
+        self.node_settings = None
+        self.provider = None
+
+    @property
+    def ADDON_SHORT_NAME(self):
+        raise NotImplementedError()
+
+    @property
+    def ExternalAccountFactory(self):
+        raise NotImplementedError()
+
+    @property
+    def folder(self):
+        raise NotImplementedError()
+
+    @property
+    def Serializer(self):
+        raise NotImplementedError()
+
+    @property
+    def client(self):
+        raise NotImplementedError()
+
     @property
     def citationsProvider(self):
         raise NotImplementedError()

addons/boa/apps.py

@@ -1,12 +1,11 @@
 import os
+
 from addons.base.apps import BaseAddonAppConfig
 from addons.boa.settings import MAX_UPLOAD_SIZE
 
 HERE = os.path.dirname(os.path.abspath(__file__))
-TEMPLATE_PATH = os.path.join(
-    HERE,
-    'templates'
-)
+TEMPLATE_PATH = os.path.join(HERE, 'templates')
+
 
 class BoaAddonAppConfig(BaseAddonAppConfig):
 

addons/boa/models.py

@@ -1,29 +1,23 @@
-# -*- coding: utf-8 -*-
-import logging
-
-from addons.base.models import (BaseOAuthNodeSettings, BaseOAuthUserSettings,
-                                BaseStorageAddon)
 from django.db import models
-from framework.auth import Auth
+
+from addons.base.exceptions import NotApplicableError
+from addons.base.models import BaseOAuthNodeSettings, BaseOAuthUserSettings, BaseStorageAddon
 from addons.boa.serializer import BoaSerializer
 from addons.boa.settings import DEFAULT_HOSTS
+from framework.auth import Auth
 from osf.models.external import BasicAuthProviderMixin
-# from website.util import api_v2_url
-logger = logging.getLogger(__name__)
 
 
 class BoaProvider(BasicAuthProviderMixin):
-    """An alternative to `ExternalProvider` not tied to OAuth"""
+    """Boa provider, an alternative to `ExternalProvider` which is not tied to OAuth"""
 
     name = 'Boa'
     short_name = 'boa'
 
     def __init__(self, account=None, host=None, username=None, password=None):
         if username:
             username = username.lower()
-        return super(BoaProvider, self).__init__(
-            account=account, host=host, username=username, password=password
-        )
+        super(BoaProvider, self).__init__(account=account, host=host, username=username, password=password)
 
     def __repr__(self):
         return '<{name}: {status}>'.format(
@@ -33,6 +27,7 @@ def __repr__(self):
 
 
 class UserSettings(BaseOAuthUserSettings):
+
     oauth_provider = BoaProvider
     serializer = BoaSerializer
 
@@ -43,13 +38,12 @@ def to_json(self, user):
 
 
 class NodeSettings(BaseOAuthNodeSettings, BaseStorageAddon):
+
     oauth_provider = BoaProvider
     serializer = BoaSerializer
 
     folder_id = models.TextField(blank=True, null=True)
-    user_settings = models.ForeignKey(
-        UserSettings, null=True, blank=True, on_delete=models.CASCADE
-    )
+    user_settings = models.ForeignKey(UserSettings, null=True, blank=True, on_delete=models.CASCADE)
 
     _api = None
 
@@ -67,7 +61,9 @@ def folder_path(self):
     def folder_name(self):
         return self.folder_id
 
-    # def set_folder(self, folder, auth=None):  # NOTE: no for Boa
+    def set_folder(self, folder, auth=None):
+        """Not applicable to remote computing add-on."""
+        raise NotApplicableError
 
     def fetch_folder_name(self):
         if self.folder_id == '/':
@@ -85,16 +81,16 @@ def deauthorize(self, auth=None, add_log=True):
         self.clear_auth()  # Also performs a .save()
 
     def serialize_waterbutler_credentials(self):
-        # required by superclass, not actually used
-        pass
+        """Not applicable to remote computing add-on."""
+        raise NotApplicableError
 
     def serialize_waterbutler_settings(self):
-        # required by superclass, not actually used
-        pass
+        """Not applicable to remote computing add-on."""
+        raise NotApplicableError
 
     def create_waterbutler_log(self, *args, **kwargs):
-        # required by superclass, not actually used
-        pass
+        """Not applicable to remote computing add-on."""
+        raise NotApplicableError
 
     def after_delete(self, user):
         self.deauthorize(Auth(user=user), add_log=True)
@@ -104,4 +100,6 @@ def on_delete(self):
         self.deauthorize(add_log=False)
         self.save()
 
-    # def get_folders(self, **kwargs):  # NOTE: no for boa
+    def get_folders(self, **kwargs):
+        """Not applicable to remote computing add-on."""
+        raise NotApplicableError