Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

Commit

Permalink
Fixing get, update, delete call when non existent importer is provided.
Browse files Browse the repository at this point in the history 
closes #750
https://pulp.plan.io/issues/750
  • Loading branch information
@ipanova
ipanova committed Jun 30, 2015
1 parent 1f99019 commit 0d9c8b0
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 42 deletions.
49 changes: 32 additions & 17 deletions server/pulp/server/webservices/views/repositories.py
View file
Expand Up @@ -121,6 +121,35 @@ def _process_repos(repos, details, importers, distributors):
return repos


def _validate_repo_importer_existance(repo_id, importer_id):
"""
Validates if the specified repo_id and importer_id are valid.
If not MissingResource exception is raised.
:param repo_id: id of the repo
:type repo_id: str
:param importer_id: id of the importer
:type importer_id: str
:return: key-value pairs describing the importer in use
:rtype: dict
:raises pulp_exceptions.MissingResource: if repo or importer cannot be found
"""

repo_query_manager = manager_factory.repo_query_manager()
repo = repo_query_manager.find_by_id(repo_id)
if repo is None:
raise pulp_exceptions.MissingResource(repo_id=repo_id)
importer_manager = manager_factory.repo_importer_manager()
try:
importer = importer_manager.get_importer(repo_id)
if importer['id'] != importer_id:
raise pulp_exceptions.MissingResource(importer_id=importer_id)
except pulp_exceptions.MissingResource:
raise pulp_exceptions.MissingResource(importer_id=importer_id)
return importer


class ReposView(View):
"""
View for all repos.
Expand Down Expand Up @@ -441,12 +470,7 @@ def get(self, request, repo_id, importer_id):
:rtype : django.http.HttpResponse
:raises pulp_exceptions.MissingResource: if importer_id does not match importer for repo
"""

importer_manager = manager_factory.repo_importer_manager()
importer = importer_manager.get_importer(repo_id)
if importer['id'] != importer_id:
raise pulp_exceptions.MissingResource(importer_id=importer_id)

importer = _validate_repo_importer_existance(repo_id, importer_id)
serializer = serializers.ImporterSerializer(importer)
return generate_json_response_with_pulp_encoder(serializer.data)

Expand All @@ -465,12 +489,7 @@ def delete(self, request, repo_id, importer_id):
:raises pulp_exceptions.MissingResource: if importer cannot be found for this repo
:raises pulp_exceptions.OperationPostponed: to dispatch a task to delete the importer
"""

importer_manager = manager_factory.repo_importer_manager()
importer = importer_manager.get_importer(repo_id)
if importer['id'] != importer_id:
raise pulp_exceptions.MissingResource(importer_id=importer_id)

_validate_repo_importer_existance(repo_id, importer_id)
task_tags = [tags.resource_tag(tags.RESOURCE_REPOSITORY_TYPE, repo_id),
tags.resource_tag(tags.RESOURCE_REPOSITORY_IMPORTER_TYPE, importer_id),
tags.action_tag('delete_importer')]
Expand All @@ -496,11 +515,7 @@ def put(self, request, repo_id, importer_id):
:raises pulp_exceptions.OperationPostponed: dispatch a task
"""

importer_manager = manager_factory.repo_importer_manager()
importer = importer_manager.get_importer(repo_id)
if importer['id'] != importer_id:
raise pulp_exceptions.MissingResource(importer_id=importer_id)

_validate_repo_importer_existance(repo_id, importer_id)
importer_config = request.body_as_json.get('importer_config', None)

if importer_config is None:
Expand Down
136 changes: 111 additions & 25 deletions server/test/unit/server/webservices/views/test_repositories.py
View file
Expand Up @@ -135,6 +135,96 @@ def test_no_last_added_or_removed(self, mock_to_utc, mock_8601):
self.assertFalse(mock_8601.called)


class TestValidateRepoImporterExistance(unittest.TestCase):
"""
Tests validation of provided repo id and importer id.
"""

@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test__validate_repo_importer_existance(self, mock_factory):
"""
Test validation repo importer.
"""

mock_q_manager = mock_factory.repo_query_manager.return_value
mock_q_manager.find_by_id.return_value = {'mock_repo': 'some_repo'}
mock_imp_manager = mock_factory.repo_importer_manager.return_value
mock_imp_manager.get_importer.return_value = {'id': 'some_importer'}

importer = repositories._validate_repo_importer_existance('some_repo', 'some_importer')
self.assertEquals(importer, {'id': 'some_importer'})
mock_q_manager.find_by_id.assert_called_once_with('some_repo')
mock_imp_manager.get_importer.assert_called_once_with('some_repo')

@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test__validate_repo_importer_existance_invalid_repo(self, mock_factory):
"""
Test validation when provided repo is invalid.
"""

mock_q_manager = mock_factory.repo_query_manager.return_value
mock_q_manager.find_by_id.return_value = None

try:
repositories._validate_repo_importer_existance('invalid_repo', 'some_importer')
except pulp_exceptions.MissingResource, response:
pass
else:
raise AssertionError("MissingResource should be raised if importer does not exist")

self.assertEqual(response.http_status_code, 404)
self.assertTrue(response.error_code is error_codes.PLP0009)
mock_q_manager.find_by_id.assert_called_once_with('invalid_repo')

@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test__validate_repo_importer_existance_no_importer(self, mock_factory):
"""
Test validation when provided importer is invalid.
And repo has no importers in general.
"""

mock_q_manager = mock_factory.repo_query_manager.return_value
mock_q_manager.find_by_id.return_value = {'mock_repo': 'some_repo'}
mock_imp_manager = mock_factory.repo_importer_manager.return_value
mock_imp_manager.get_importer.side_effect = pulp_exceptions.MissingResource

try:
repositories._validate_repo_importer_existance('some_repo', 'invalid_importer')
except pulp_exceptions.MissingResource, response:
pass
else:
raise AssertionError("MissingResource should be raised if importer does not exist")

self.assertEqual(response.http_status_code, 404)
self.assertTrue(response.error_code is error_codes.PLP0009)
mock_q_manager.find_by_id.assert_called_once_with('some_repo')
mock_imp_manager.get_importer.assert_called_once_with('some_repo')

@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test__validate_repo_importer_existance_invalid_importer(self, mock_factory):
"""
Test validation when provided importer is invalid, i.e. it is different
from valid one.
"""

mock_q_manager = mock_factory.repo_query_manager.return_value
mock_q_manager.find_by_id.return_value = {'mock_repo': 'some_repo'}
mock_imp_manager = mock_factory.repo_importer_manager.return_value
mock_imp_manager.get_importer.return_value = {'id': 'some_importer'}

try:
repositories._validate_repo_importer_existance('some_repo', 'different_importer')
except pulp_exceptions.MissingResource, response:
pass
else:
raise AssertionError("MissingResource should be raised if importer does not exist")

self.assertEqual(response.http_status_code, 404)
self.assertTrue(response.error_code is error_codes.PLP0009)
mock_q_manager.find_by_id.assert_called_once_with('some_repo')
mock_imp_manager.get_importer.assert_called_once_with('some_repo')


class TestReposView(unittest.TestCase):
"""
Tests for ReposView.
Expand Down Expand Up @@ -847,15 +937,15 @@ class TestRepoImporterResourceView(unittest.TestCase):
new=assert_auth_READ())
@mock.patch(
'pulp.server.webservices.views.repositories.generate_json_response_with_pulp_encoder')
@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test_get_importer(self, mock_factory, mock_resp):
@mock.patch('pulp.server.webservices.views.repositories._validate_repo_importer_existance')
def test_get_importer(self, mock_validate, mock_resp):
"""
Get an importer for a repository.
"""

mock_request = mock.MagicMock()
mock_importer = {"id": "mock_importer", 'repo_id': 'mock-repo'}
mock_factory.repo_importer_manager.return_value.get_importer.return_value = mock_importer
mock_validate.return_value = mock_importer

repo_importer = RepoImporterResourceView()
response = repo_importer.get(mock_request, 'mock_repo', 'mock_importer')
Expand All @@ -867,15 +957,13 @@ def test_get_importer(self, mock_factory, mock_resp):

@mock.patch('pulp.server.webservices.views.decorators._verify_auth',
new=assert_auth_READ())
@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test_get_nonexisting_importer(self, mock_factory):
@mock.patch('pulp.server.webservices.views.repositories._validate_repo_importer_existance')
def test_get_nonexisting_importer(self, mock_validate):
"""
Try to get an importer that doesn't exist.
"""

mock_request = mock.MagicMock()
mock_importer = {"id": "mock_importer"}
mock_factory.repo_importer_manager.return_value.get_importer.return_value = mock_importer
mock_validate.side_effect = pulp_exceptions.MissingResource

repo_importer = RepoImporterResourceView()
try:
Expand All @@ -892,15 +980,15 @@ def test_get_nonexisting_importer(self, mock_factory):
new=assert_auth_DELETE())
@mock.patch('pulp.server.webservices.views.repositories.repo_importer_manager.remove_importer')
@mock.patch('pulp.server.webservices.views.repositories.tags')
@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test_delete_importer(self, mock_factory, mock_tags, mock_remove_importer):
@mock.patch('pulp.server.webservices.views.repositories._validate_repo_importer_existance')
def test_delete_importer(self, mock_validate, mock_tags, mock_remove_importer):
"""
Diassociate an importer from a repository.
"""

mock_request = mock.MagicMock()
mock_importer = {"id": "mock_importer"}
mock_factory.repo_importer_manager.return_value.get_importer.return_value = mock_importer
mock_validate.return_value = mock_importer
mock_task = [mock_tags.resource_tag(), mock_tags.resource_tag(), mock_tags.action_tag()]

repo_importer = RepoImporterResourceView()
Expand All @@ -918,15 +1006,14 @@ def test_delete_importer(self, mock_factory, mock_tags, mock_remove_importer):

@mock.patch('pulp.server.webservices.views.decorators._verify_auth',
new=assert_auth_DELETE())
@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test_delete_nonexisting_importer(self, mock_factory):
@mock.patch('pulp.server.webservices.views.repositories._validate_repo_importer_existance')
def test_delete_nonexisting_importer(self, mock_validate):
"""
Attpempt diassociate an importer that is not associated to a repository.
"""

mock_request = mock.MagicMock()
mock_importer = {"id": "mock_importer"}
mock_factory.repo_importer_manager.return_value.get_importer.return_value = mock_importer
mock_validate.side_effect = pulp_exceptions.MissingResource

repo_importer = RepoImporterResourceView()
try:
Expand All @@ -945,16 +1032,16 @@ def test_delete_nonexisting_importer(self, mock_factory):
@mock.patch(
'pulp.server.webservices.views.repositories.repo_importer_manager.update_importer_config')
@mock.patch('pulp.server.webservices.views.repositories.tags')
@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test_put_update_importer(self, mock_factory, mock_tags, mock_update_importer):
@mock.patch('pulp.server.webservices.views.repositories._validate_repo_importer_existance')
def test_put_update_importer(self, mock_validate, mock_tags, mock_update_importer):
"""
Update an importer with all required params.
"""

mock_request = mock.MagicMock()
mock_request.body = json.dumps({'importer_config': 'test'})
mock_importer = {"id": "mock_importer"}
mock_factory.repo_importer_manager.return_value.get_importer.return_value = mock_importer
mock_validate.return_value = mock_importer
mock_task = [mock_tags.resource_tag(), mock_tags.resource_tag(), mock_tags.action_tag()]

repo_importer = RepoImporterResourceView()
Expand All @@ -973,16 +1060,15 @@ def test_put_update_importer(self, mock_factory, mock_tags, mock_update_importer

@mock.patch('pulp.server.webservices.views.decorators._verify_auth',
new=assert_auth_UPDATE())
@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test_put_bad_importer_id(self, mock_factory):
@mock.patch('pulp.server.webservices.views.repositories._validate_repo_importer_existance')
def test_put_bad_importer_id(self, mock_validate):
"""
Update an importer with invalid importer_id.
"""

mock_request = mock.MagicMock()
mock_request.body = json.dumps({'importer_config': 'test'})
mock_importer = {"id": "mock_importer"}
mock_factory.repo_importer_manager.return_value.get_importer.return_value = mock_importer
mock_validate.side_effect = pulp_exceptions.MissingResource

repo_importer = RepoImporterResourceView()
try:
Expand All @@ -998,16 +1084,16 @@ def test_put_bad_importer_id(self, mock_factory):

@mock.patch('pulp.server.webservices.views.decorators._verify_auth',
new=assert_auth_UPDATE())
@mock.patch('pulp.server.webservices.views.repositories.manager_factory')
def test_put_no_importer_conf(self, mock_factory):
@mock.patch('pulp.server.webservices.views.repositories._validate_repo_importer_existance')
def test_put_no_importer_conf(self, mock_validate):
"""
Update an importer with the importer config missing from the request body.
"""

mock_request = mock.MagicMock()
mock_request.body = json.dumps({'not_importer_config': 'will fail'})
mock_importer = {"id": "mock_importer"}
mock_factory.repo_importer_manager.return_value.get_importer.return_value = mock_importer
mock_validate.return_value = mock_importer

repo_importer = RepoImporterResourceView()
try:
Expand Down

0 comments on commit 0d9c8b0

Please sign in to comment.