diff --git a/docs/source/API.rst b/docs/source/API.rst index 40955d52..b17c0ef9 100644 --- a/docs/source/API.rst +++ b/docs/source/API.rst @@ -139,7 +139,7 @@ progress, this POST should be rejected with a 409 error. Query Parameters '''''''''''''''' -- bufferMode=append|replace\|\ **rejectOnContents** +- buffermode=append|replace\|\ **rejectOnContents** Indicates how the existing Shipyard Buffer should be handled. By default, Shipyard will reject the POST if contents already exist in the Shipyard Buffer. @@ -161,13 +161,21 @@ Responses - The response headers will include a Location indicating the GET endpoint to retrieve the configDocs +400 Bad Request + When: + + - The request is missing a message body, attempting to create a collection + with no contents. + - The request has no new/changed contents for the collection. + - The request is missing a Content-Length header. + 409 Conflict A condition in the system is blocking this document ingestion - If a commitconfigdocs POST is in progress. - - If any collections exist in the Shipyard Buffer unless bufferMode=replace - or bufferMode=append. - - If bufferMode=append, and the collection being posted is already in the + - If any collections exist in the Shipyard Buffer unless buffermode=replace + or buffermode=append. + - If buffermode=append, and the collection being posted is already in the Shipyard Buffer GET /v1.0/configdocs/{collection_id} diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py index bda7007b..c70545bc 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py @@ -22,7 +22,7 @@ from shipyard_airflow.control.configdocs import configdocs_helper from shipyard_airflow.control.api_lock import (api_lock, ApiLockType) from shipyard_airflow.control.base import BaseResource from shipyard_airflow.control.configdocs.configdocs_helper import ( - BufferMode, ConfigdocsHelper) + ConfigdocsHelper) from shipyard_airflow.errors import ApiError CONF = cfg.CONF @@ -55,13 +55,33 @@ class ConfigDocsResource(BaseResource): """ Ingests a collection of documents """ - document_data = req.stream.read(req.content_length or 0) + content_length = req.content_length or 0 + if (content_length == 0): + raise ApiError( + title=('Content-Length is a required header'), + description='Content Length is 0 or not specified', + status=falcon.HTTP_400, + error_list=[{ + 'message': ( + 'The Content-Length specified is 0 or not set. Check ' + 'that a valid payload is included with this request ' + 'and that your client is properly including a ' + 'Content-Length header. Note that a newline character ' + 'in a prior header can trigger subsequent headers to ' + 'be ignored and trigger this failure.') + }], + retry=False, ) + document_data = req.stream.read(content_length) + + buffer_mode = req.get_param('buffermode') + helper = ConfigdocsHelper(req.context) validations = self.post_collection( helper=helper, collection_id=collection_id, document_data=document_data, - buffer_mode_param=req.params.get('buffermode')) + buffer_mode_param=buffer_mode) + resp.location = '/api/v1.0/configdocs/{}'.format(collection_id) resp.body = self.to_json(validations) resp.status = falcon.HTTP_201 @@ -105,15 +125,28 @@ class ConfigDocsResource(BaseResource): """ Ingest the collection after checking preconditions """ - if buffer_mode_param is None: - buffer_mode = BufferMode.REJECTONCONTENTS - else: - buffer_mode = ConfigdocsHelper.get_buffer_mode(buffer_mode_param) + buffer_mode = ConfigdocsHelper.get_buffer_mode(buffer_mode_param) if helper.is_buffer_valid_for_bucket(collection_id, buffer_mode): buffer_revision = helper.add_collection(collection_id, document_data) - return helper.get_deckhand_validation_status(buffer_revision) + if helper.is_collection_in_buffer(collection_id): + return helper.get_deckhand_validation_status(buffer_revision) + else: + raise ApiError( + title=('Collection {} not added to Shipyard ' + 'buffer'.format(collection_id)), + description='Collection empty or resulted in no revision', + status=falcon.HTTP_400, + error_list=[{ + 'message': ( + 'Empty collections are not supported. After ' + 'processing, the collection {} added no new ' + 'revision, and has been rejected as invalid ' + 'input'.format(collection_id)) + }], + retry=False, + ) else: raise ApiError( title='Invalid collection specified for buffer', @@ -125,7 +158,8 @@ class ConfigDocsResource(BaseResource): 'Setting a different buffermode may ' 'provide the desired functionality') }], - retry=False, ) + retry=False, + ) class CommitConfigDocsResource(BaseResource): diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py b/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py index 8fb3ae12..a160b110 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py @@ -79,9 +79,12 @@ class ConfigdocsHelper(object): self.revision_dict = None @staticmethod - def get_buffer_mode(buffermode_string): - """ - Checks the buffer mode for valid values. + def get_buffer_mode(buffermode_string=None): + """Checks the buffer mode for valid values. + + :param buffermode_string: the string matching a valid buffer mode + Returns Buffermode.REJECTONTENTS value if the input is not specified + or is an invalid value. """ if buffermode_string: try: diff --git a/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_api.py b/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_api.py index 5dab3b09..0434d3ea 100644 --- a/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_api.py +++ b/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_api.py @@ -49,9 +49,20 @@ class TestConfigDocsResource(): @patch.object(ConfigDocsResource, 'post_collection', common.str_responder) @mock.patch.object(ApiLock, 'release') @mock.patch.object(ApiLock, 'acquire') - def test_on_post(self, mock_acquire, mock_release, api_client): + def test_on_post_empty(self, mock_acquire, mock_release, api_client): result = api_client.simulate_post( "/api/v1.0/configdocs/coll1", headers=common.AUTH_HEADERS) + assert result.status_code == 400 + assert 'Content-Length specified is 0 or not set' in result.text + + @patch.object(ConfigDocsResource, 'post_collection', common.str_responder) + @mock.patch.object(ApiLock, 'release') + @mock.patch.object(ApiLock, 'acquire') + def test_on_post(self, mock_acquire, mock_release, api_client): + headers = {'Content-Length': '1'} + headers.update(common.AUTH_HEADERS) + result = api_client.simulate_post( + "/api/v1.0/configdocs/coll1", headers=headers, body='A') assert result.status_code == 201 @patch.object(ConfigDocsResource, 'get_collection', common.str_responder) @@ -86,6 +97,8 @@ class TestConfigDocsResource(): mock_method.assert_called_once_with('buffer', 'apples') + @patch.object(ConfigdocsHelper, 'is_collection_in_buffer', + lambda x, y: True) def test_post_collection(self): """ Tests the post collection method of the ConfigdocsResource @@ -106,17 +119,53 @@ class TestConfigDocsResource(): mock_method.assert_called_once_with(collection_id, document_data) - with pytest.raises(ApiError): + @patch.object(ConfigdocsHelper, 'is_collection_in_buffer', + lambda x, y: True) + @patch.object(ConfigdocsHelper, 'is_buffer_valid_for_bucket', + lambda x, y, z: False) + def test_post_collection_not_valid_for_buffer(self): + """ + Tests the post collection method of the ConfigdocsResource + """ + helper = None + collection_id = 'trees' + document_data = 'lots of info' + with pytest.raises(ApiError) as apie: cdr = ConfigDocsResource() helper = ConfigdocsHelper(CTX) # not valid for bucket - helper.is_buffer_valid_for_bucket = lambda a, b: False helper.get_deckhand_validation_status = ( lambda a: ConfigdocsHelper._format_validations_to_status([], 0) ) cdr.post_collection(helper=helper, collection_id=collection_id, document_data=document_data) + assert apie.value.status == '409 Conflict' + + @patch.object(ConfigdocsHelper, 'is_collection_in_buffer', + lambda x, y: False) + @patch.object(ConfigdocsHelper, 'is_buffer_valid_for_bucket', + lambda x, y, z: True) + def test_post_collection_not_added(self): + """ + Tests the post collection method of the ConfigdocsResource + """ + helper = None + collection_id = 'trees' + document_data = 'lots of info' + with patch.object(ConfigdocsHelper, 'add_collection') as mock_method: + cdr = ConfigDocsResource() + helper = ConfigdocsHelper(CTX) + helper.get_deckhand_validation_status = ( + lambda a: ConfigdocsHelper._format_validations_to_status([], 0) + ) + with pytest.raises(ApiError) as apie: + cdr.post_collection(helper=helper, + collection_id=collection_id, + document_data=document_data) + + assert apie.value.status == '400 Bad Request' + mock_method.assert_called_once_with(collection_id, document_data) class TestCommitConfigDocsResource(): @@ -124,7 +173,6 @@ class TestCommitConfigDocsResource(): @mock.patch.object(ApiLock, 'acquire') def test_on_post(self, mock_acquire, mock_release, api_client): queries = ["", "force=true", "dryrun=true"] - ccdr = CommitConfigDocsResource() with patch.object( CommitConfigDocsResource, 'commit_configdocs', return_value={} ) as mock_method: