From d01b55f27d2b8be7b772e25da02416aa4e2ede4d Mon Sep 17 00:00:00 2001 From: Charlton Austin Date: Thu, 13 Jul 2017 15:30:07 -0400 Subject: [PATCH 1/2] refactor(endpoints/api/repoemail): added in pre_oci model ### Description of Changes this is so we can abstract away the data interface [TESTING->locally with docker compose] Issue: https://coreosdev.atlassian.net/browse/QUAY-626 ## Reviewer Checklist - [ ] It works! - [ ] Comments provide sufficient explanations for the next contributor - [ ] Tests cover changes and corner cases - [ ] Follows Quay syntax patterns and format --- endpoints/api/repoemail.py | 23 ++---- endpoints/api/repoemail_models_interface.py | 44 ++++++++++ endpoints/api/repoemail_models_pre_oci.py | 26 ++++++ .../api/test/test_repoemail_models_pre_oci.py | 81 +++++++++++++++++++ ..._pre_oci.py => test_tag_models_pre_oci.py} | 0 5 files changed, 158 insertions(+), 16 deletions(-) create mode 100644 endpoints/api/repoemail_models_interface.py create mode 100644 endpoints/api/repoemail_models_pre_oci.py create mode 100644 endpoints/api/test/test_repoemail_models_pre_oci.py rename endpoints/api/test/{test_models_pre_oci.py => test_tag_models_pre_oci.py} (100%) diff --git a/endpoints/api/repoemail.py b/endpoints/api/repoemail.py index 4b0a33b51..fee9aed51 100644 --- a/endpoints/api/repoemail.py +++ b/endpoints/api/repoemail.py @@ -7,9 +7,9 @@ from flask import request, abort from endpoints.api import (resource, nickname, require_repo_admin, RepositoryParamResource, log_action, validate_json_request, internal_only, path_param, show_if) +from endpoints.api.repoemail_models_pre_oci import pre_oci_model as model from endpoints.exception import NotFound from app import tf -from data import model from data.database import db from util.useremails import send_repo_authorization_email @@ -19,15 +19,6 @@ import features logger = logging.getLogger(__name__) -def record_view(record): - return { - 'email': record.email, - 'repository': record.repository.name, - 'namespace': record.repository.namespace_user.username, - 'confirmed': record.confirmed - } - - @internal_only @resource('/v1/repository//authorizedemail/') @show_if(features.MAILING) @@ -39,11 +30,11 @@ class RepositoryAuthorizedEmail(RepositoryParamResource): @nickname('checkRepoEmailAuthorized') def get(self, namespace, repository, email): """ Checks to see if the given e-mail address is authorized on this repository. """ - record = model.repository.get_email_authorized_for_repo(namespace, repository, email) + record = model.get_email_authorized_for_repo(namespace, repository, email) if not record: abort(404) - return record_view(record) + return record.to_dict() @require_repo_admin @@ -52,12 +43,12 @@ class RepositoryAuthorizedEmail(RepositoryParamResource): """ Starts the authorization process for an e-mail address on a repository. """ with tf(db): - record = model.repository.get_email_authorized_for_repo(namespace, repository, email) + record = model.get_email_authorized_for_repo(namespace, repository, email) if record and record.confirmed: - return record_view(record) + return record.to_dict() if not record: - record = model.repository.create_email_authorization_for_repo(namespace, repository, email) + record = model.create_email_authorization_for_repo(namespace, repository, email) send_repo_authorization_email(namespace, repository, email, record.code) - return record_view(record) + return record.to_dict() diff --git a/endpoints/api/repoemail_models_interface.py b/endpoints/api/repoemail_models_interface.py new file mode 100644 index 000000000..145209548 --- /dev/null +++ b/endpoints/api/repoemail_models_interface.py @@ -0,0 +1,44 @@ +from abc import ABCMeta, abstractmethod +from collections import namedtuple + +from six import add_metaclass + + +class RepositoryAuthorizedEmail( + namedtuple('RepositoryAuthorizedEmail', ['email', 'repository_name', 'namespace_name', 'confirmed', 'code',])): + """ + Tag represents a name to an image. + :type email: string + :type repository_name: string + :type namespace_name: string + :type confirmed: boolean + :type code: string + """ + + def to_dict(self): + return { + 'email': self.email, + 'repository': self.repository_name, + 'namespace': self.namespace_name, + 'confirmed': self.confirmed, + 'code': self.code + } + + +@add_metaclass(ABCMeta) +class RepoEmailDataInterface(object): + """ + Interface that represents all data store interactions required by a Repo Email. + """ + + @abstractmethod + def get_email_authorized_for_repo(self, namespace_name, repository_name, email): + """ + Returns a RepositoryAuthorizedEmail if available else None + """ + + @abstractmethod + def create_email_authorization_for_repo(self, namespace_name, repository_name, email): + """ + Returns the newly created repository authorized email. + """ diff --git a/endpoints/api/repoemail_models_pre_oci.py b/endpoints/api/repoemail_models_pre_oci.py new file mode 100644 index 000000000..36577b977 --- /dev/null +++ b/endpoints/api/repoemail_models_pre_oci.py @@ -0,0 +1,26 @@ +from data import model +from endpoints.api.repoemail_models_interface import RepoEmailDataInterface, RepositoryAuthorizedEmail + + +def _return_none_or_data(func, namespace_name, repository_name, email): + data = func(namespace_name, repository_name, email) + if data is None: + return data + return RepositoryAuthorizedEmail(email, repository_name, namespace_name, data.confirmed, data.code) + + +class PreOCIModel(RepoEmailDataInterface): + """ + PreOCIModel implements the data model for the Repo Email using a database schema + before it was changed to support the OCI specification. + """ + + def get_email_authorized_for_repo(self, namespace_name, repository_name, email): + return _return_none_or_data(model.repository.get_email_authorized_for_repo, namespace_name, repository_name, email) + + def create_email_authorization_for_repo(self, namespace_name, repository_name, email): + return _return_none_or_data(model.repository.create_email_authorization_for_repo, namespace_name, repository_name, + email) + + +pre_oci_model = PreOCIModel() diff --git a/endpoints/api/test/test_repoemail_models_pre_oci.py b/endpoints/api/test/test_repoemail_models_pre_oci.py new file mode 100644 index 000000000..fa37a4c90 --- /dev/null +++ b/endpoints/api/test/test_repoemail_models_pre_oci.py @@ -0,0 +1,81 @@ +import pytest +from mock import Mock + +import util +from data import model +from endpoints.api.repoemail_models_interface import RepositoryAuthorizedEmail +from endpoints.api.repoemail_models_pre_oci import pre_oci_model + + +@pytest.fixture +def get_monkeypatch(monkeypatch): + return monkeypatch + + +def return_none(name, repo, email): + return None + + +def get_return_mock(mock): + def return_mock(name, repo, email): + return mock + + return return_mock + + +def test_get_email_authorized_for_repo(get_monkeypatch): + mock = Mock() + + get_monkeypatch.setattr(model.repository, 'get_email_authorized_for_repo', mock) + + pre_oci_model.get_email_authorized_for_repo('namespace_name', 'repository_name', 'email') + + mock.assert_called_once_with('namespace_name', 'repository_name', 'email') + + +def test_get_email_authorized_for_repo_return_none(get_monkeypatch): + get_monkeypatch.setattr(model.repository, 'get_email_authorized_for_repo', return_none) + + repo = pre_oci_model.get_email_authorized_for_repo('namespace_name', 'repository_name', 'email') + + assert repo is None + + +def test_get_email_authorized_for_repo_return_repo(get_monkeypatch): + mock = Mock(confirmed=True, code='code') + get_monkeypatch.setattr(model.repository, 'get_email_authorized_for_repo', get_return_mock(mock)) + + actual = pre_oci_model.get_email_authorized_for_repo('namespace_name', 'repository_name', 'email') + + assert actual == RepositoryAuthorizedEmail('email', 'repository_name', 'namespace_name', True, 'code') + + +def test_create_email_authorization_for_repo(get_monkeypatch): + mock = Mock() + get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', mock) + + pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', 'email') + + mock.assert_called_once_with('namespace_name', 'repository_name', 'email') + + +def test_create_email_authorization_for_repo_return_none(get_monkeypatch): + get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', return_none) + + assert pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', 'email') is None + + +def test_create_email_authorization_for_repo_return_mock(get_monkeypatch): + mock = Mock() + get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', get_return_mock(mock)) + + assert pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', 'email') is not None + + +def test_create_email_authorization_for_repo_return_value(get_monkeypatch): + mock = Mock(confirmed=False, code='code') + + get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', get_return_mock(mock)) + + actual = pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', 'email') + assert actual == RepositoryAuthorizedEmail('email', 'repository_name', 'namespace_name', False, 'code') diff --git a/endpoints/api/test/test_models_pre_oci.py b/endpoints/api/test/test_tag_models_pre_oci.py similarity index 100% rename from endpoints/api/test/test_models_pre_oci.py rename to endpoints/api/test/test_tag_models_pre_oci.py From bbe9b033d0e9a88362a981f6e10c3c7d78c13dc5 Mon Sep 17 00:00:00 2001 From: Charlton Austin Date: Thu, 13 Jul 2017 15:34:48 -0400 Subject: [PATCH 2/2] style(endpoints/api/repoemail): ran yapf ### Description of Changes Issue: https://coreosdev.atlassian.net/browse/QUAY-626 ## Reviewer Checklist - [ ] It works! - [ ] Comments provide sufficient explanations for the next contributor - [ ] Tests cover changes and corner cases - [ ] Follows Quay syntax patterns and format --- endpoints/api/repoemail.py | 6 ++--- endpoints/api/repoemail_models_interface.py | 8 ++++++- endpoints/api/repoemail_models_pre_oci.py | 10 ++++---- .../api/test/test_repoemail_models_pre_oci.py | 24 ++++++++++++------- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/endpoints/api/repoemail.py b/endpoints/api/repoemail.py index fee9aed51..3edccb4cc 100644 --- a/endpoints/api/repoemail.py +++ b/endpoints/api/repoemail.py @@ -5,8 +5,7 @@ import logging from flask import request, abort from endpoints.api import (resource, nickname, require_repo_admin, RepositoryParamResource, - log_action, validate_json_request, internal_only, - path_param, show_if) + log_action, validate_json_request, internal_only, path_param, show_if) from endpoints.api.repoemail_models_pre_oci import pre_oci_model as model from endpoints.exception import NotFound from app import tf @@ -15,7 +14,6 @@ from util.useremails import send_repo_authorization_email import features - logger = logging.getLogger(__name__) @@ -26,6 +24,7 @@ logger = logging.getLogger(__name__) @path_param('email', 'The e-mail address') class RepositoryAuthorizedEmail(RepositoryParamResource): """ Resource for checking and authorizing e-mail addresses to receive repo notifications. """ + @require_repo_admin @nickname('checkRepoEmailAuthorized') def get(self, namespace, repository, email): @@ -36,7 +35,6 @@ class RepositoryAuthorizedEmail(RepositoryParamResource): return record.to_dict() - @require_repo_admin @nickname('sendAuthorizeRepoEmail') def post(self, namespace, repository, email): diff --git a/endpoints/api/repoemail_models_interface.py b/endpoints/api/repoemail_models_interface.py index 145209548..2aae7ab9c 100644 --- a/endpoints/api/repoemail_models_interface.py +++ b/endpoints/api/repoemail_models_interface.py @@ -5,7 +5,13 @@ from six import add_metaclass class RepositoryAuthorizedEmail( - namedtuple('RepositoryAuthorizedEmail', ['email', 'repository_name', 'namespace_name', 'confirmed', 'code',])): + namedtuple('RepositoryAuthorizedEmail', [ + 'email', + 'repository_name', + 'namespace_name', + 'confirmed', + 'code', + ])): """ Tag represents a name to an image. :type email: string diff --git a/endpoints/api/repoemail_models_pre_oci.py b/endpoints/api/repoemail_models_pre_oci.py index 36577b977..80a65c995 100644 --- a/endpoints/api/repoemail_models_pre_oci.py +++ b/endpoints/api/repoemail_models_pre_oci.py @@ -6,7 +6,8 @@ def _return_none_or_data(func, namespace_name, repository_name, email): data = func(namespace_name, repository_name, email) if data is None: return data - return RepositoryAuthorizedEmail(email, repository_name, namespace_name, data.confirmed, data.code) + return RepositoryAuthorizedEmail(email, repository_name, namespace_name, data.confirmed, + data.code) class PreOCIModel(RepoEmailDataInterface): @@ -16,11 +17,12 @@ class PreOCIModel(RepoEmailDataInterface): """ def get_email_authorized_for_repo(self, namespace_name, repository_name, email): - return _return_none_or_data(model.repository.get_email_authorized_for_repo, namespace_name, repository_name, email) + return _return_none_or_data(model.repository.get_email_authorized_for_repo, namespace_name, + repository_name, email) def create_email_authorization_for_repo(self, namespace_name, repository_name, email): - return _return_none_or_data(model.repository.create_email_authorization_for_repo, namespace_name, repository_name, - email) + return _return_none_or_data(model.repository.create_email_authorization_for_repo, + namespace_name, repository_name, email) pre_oci_model = PreOCIModel() diff --git a/endpoints/api/test/test_repoemail_models_pre_oci.py b/endpoints/api/test/test_repoemail_models_pre_oci.py index fa37a4c90..7c8de8226 100644 --- a/endpoints/api/test/test_repoemail_models_pre_oci.py +++ b/endpoints/api/test/test_repoemail_models_pre_oci.py @@ -45,9 +45,11 @@ def test_get_email_authorized_for_repo_return_repo(get_monkeypatch): mock = Mock(confirmed=True, code='code') get_monkeypatch.setattr(model.repository, 'get_email_authorized_for_repo', get_return_mock(mock)) - actual = pre_oci_model.get_email_authorized_for_repo('namespace_name', 'repository_name', 'email') + actual = pre_oci_model.get_email_authorized_for_repo('namespace_name', 'repository_name', + 'email') - assert actual == RepositoryAuthorizedEmail('email', 'repository_name', 'namespace_name', True, 'code') + assert actual == RepositoryAuthorizedEmail('email', 'repository_name', 'namespace_name', True, + 'code') def test_create_email_authorization_for_repo(get_monkeypatch): @@ -62,20 +64,26 @@ def test_create_email_authorization_for_repo(get_monkeypatch): def test_create_email_authorization_for_repo_return_none(get_monkeypatch): get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', return_none) - assert pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', 'email') is None + assert pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', + 'email') is None def test_create_email_authorization_for_repo_return_mock(get_monkeypatch): mock = Mock() - get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', get_return_mock(mock)) + get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', + get_return_mock(mock)) - assert pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', 'email') is not None + assert pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', + 'email') is not None def test_create_email_authorization_for_repo_return_value(get_monkeypatch): mock = Mock(confirmed=False, code='code') - get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', get_return_mock(mock)) + get_monkeypatch.setattr(model.repository, 'create_email_authorization_for_repo', + get_return_mock(mock)) - actual = pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', 'email') - assert actual == RepositoryAuthorizedEmail('email', 'repository_name', 'namespace_name', False, 'code') + actual = pre_oci_model.create_email_authorization_for_repo('namespace_name', 'repository_name', + 'email') + assert actual == RepositoryAuthorizedEmail('email', 'repository_name', 'namespace_name', False, + 'code')