From 667a538330480cb3502b006071ee928258f0abcf Mon Sep 17 00:00:00 2001 From: Bryan Strassner Date: Wed, 17 Oct 2018 17:33:40 -0500 Subject: [PATCH] Support clearing collections of configdocs Adds an option to create configdocs as an empty colleciton. This is done as an explicit flag (--empty-collection) on the command as opposed to using empty files to prevent accidental emtpy file loads leading to frustration. Since this introduced a new flag value for the CLI, the CLIs using flag values were updated to use the standard is_flag=True instead of the flag_value=True or some other value when a boolean flag is expected. Minor updates to CLI tests due to moving to responses 0.10.2 Depends-On: https://review.openstack.org/#/c/614421/ Change-Id: I489b0e1183335cbfbaa2014c1458a84dadf6bb0b --- doc/source/API.rst | 11 +- doc/source/CLI.rst | 19 ++- .../control/configdocs/configdocs_api.py | 84 ++++++++---- .../control/helpers/configdocs_helper.py | 19 ++- .../shipyard_airflow/test-requirements.txt | 2 +- .../unit/control/test_configdocs_helper.py | 32 ++++- .../api_client/shipyard_api_client.py | 5 + .../shipyard_client/cli/commit/commands.py | 4 +- .../shipyard_client/cli/create/actions.py | 34 +++-- .../shipyard_client/cli/create/commands.py | 120 +++++++++++------- .../shipyard_client/cli/get/commands.py | 16 +-- src/bin/shipyard_client/test-requirements.txt | 2 +- .../unit/cli/commit/test_commit_actions.py | 13 +- .../unit/cli/create/test_create_actions.py | 45 +++++++ .../unit/cli/create/test_create_commands.py | 109 +++++++++++++++- 15 files changed, 389 insertions(+), 126 deletions(-) diff --git a/doc/source/API.rst b/doc/source/API.rst index 1abce103..f463b67f 100644 --- a/doc/source/API.rst +++ b/doc/source/API.rst @@ -158,10 +158,9 @@ Deckhand POST /v1.0/configdocs/{collection_id} ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Ingests a collection of documents. Synchronous. POSTing an empty body -indicates that the specified collection should be deleted when the -Shipyard Buffer is committed. If a POST to the commitconfigdocs is in -progress, this POST should be rejected with a 409 error. +Ingests a collection of documents. Synchronous. If a POST to the +commitconfigdocs is already in progress, this POST should be rejected with a +409 error. .. note:: @@ -183,6 +182,10 @@ Query Parameters - replace: Clear the Shipyard Buffer before adding the specified collection. +- empty-collection: Set to true to indicate that this collection should be + made empty and effectively deleted when the Shipyard Buffer is committed. + If this parameter is specified, the POST body will be ignored. + Responses ''''''''' 201 Created diff --git a/doc/source/CLI.rst b/doc/source/CLI.rst index bca045f9..651ede11 100644 --- a/doc/source/CLI.rst +++ b/doc/source/CLI.rst @@ -293,7 +293,7 @@ or one or more directory options must be specified. shipyard create configdocs - [--append | --replace] + [--append | --replace] [--empty-collection] --filename= (repeatable) | --directory= (repeatable) @@ -308,8 +308,8 @@ or one or more directory options must be specified. .. note:: - Either --filename or --directory must be specified, but both may not be - specified for the same invocation of shipyard. + --filename and/or --directory must be specified unless --empty-collection + is used. The collection to load. @@ -321,17 +321,22 @@ or one or more directory options must be specified. \--replace Clear the shipyard buffer and replace it with the specified contents. +\--empty-collection + Indicate to Shipyard that the named collection should be made empty (contain + no documents). If --empty-collection is specified, the files named by + --filename or --directory will be ignored. + \--filename= The file name to use as the contents of the collection. (repeatable) If any documents specified fail basic validation, all of the documents will - be rejected. Use of filename parameters may not be used in conjunction + be rejected. Use of ``filename`` parameters may not be used in conjunction with the directory parameter. \--directory= A directory containing documents that will be joined and loaded as a - collection. (Repeatable) Any documents that fail basic validation will reject the - whole set. Use of the directory parameter may not be used with the - filename parameter. + collection. (Repeatable) Any documents that fail basic validation will + reject the whole set. Use of the ``directory`` parameter may not be used + with the ``filename`` parameter. \--recurse Recursively search through all directories for sub-directories that 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 2b2f61e5..92c8ac4b 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 @@ -14,6 +14,8 @@ """ Resources representing the configdocs API for shipyard """ +import logging + import falcon from oslo_config import cfg @@ -26,6 +28,7 @@ from shipyard_airflow.control.helpers.configdocs_helper import ( from shipyard_airflow.errors import ApiError CONF = cfg.CONF +LOG = logging.getLogger(__name__) VERSION_VALUES = ['buffer', 'committed', 'last_site_action', @@ -59,23 +62,17 @@ class ConfigDocsResource(BaseResource): """ Ingests a collection of documents """ - 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) + # Determine if this request is clearing the collection's contents. + empty_coll = req.get_param_as_bool('empty-collection') or False + if empty_coll: + document_data = "" + LOG.debug("Collection %s is being emptied", collection_id) + else: + # Note, a newline in a prior header can trigger subsequent + # headers to be "missing" (and hence cause this code to think + # that the content length is missing) + content_length = self.validate_content_length(req.content_length) + document_data = req.stream.read(content_length) buffer_mode = req.get_param('buffermode') @@ -84,7 +81,8 @@ class ConfigDocsResource(BaseResource): helper=helper, collection_id=collection_id, document_data=document_data, - buffer_mode_param=buffer_mode) + buffer_mode_param=buffer_mode, + empty_collection=empty_coll) resp.status = falcon.HTTP_201 if validations and validations['status'] == 'Success': @@ -92,6 +90,30 @@ class ConfigDocsResource(BaseResource): resp.location = '/api/v1.0/configdocs/{}'.format(collection_id) resp.body = self.to_json(validations) + def validate_content_length(self, content_length): + """Validates that the content length header is valid + + :param content_length: the value of the content-length header. + :returns: the validate content length value + """ + content_length = 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. To " + "clear a collection's contents, please specify " + "the query parameter 'empty-collection=true'." + "Otherwise, a non-zero length payload and " + "matching Content-Length header is required to " + "post a collection.") + }], + retry=False, ) + return content_length + @policy.ApiEnforcer(policy.GET_CONFIGDOCS) def on_get(self, req, resp, collection_id): """ @@ -132,7 +154,8 @@ class ConfigDocsResource(BaseResource): helper, collection_id, document_data, - buffer_mode_param=None): + buffer_mode_param=None, + empty_collection=False): """ Ingest the collection after checking preconditions """ @@ -141,23 +164,28 @@ class ConfigDocsResource(BaseResource): if helper.is_buffer_valid_for_bucket(collection_id, buffer_mode): buffer_revision = helper.add_collection(collection_id, document_data) - if helper.is_collection_in_buffer(collection_id): - return helper.get_deckhand_validation_status(buffer_revision) - else: + if not (empty_collection or helper.is_collection_in_buffer( + collection_id)): + # raise an error if adding the collection resulted in no new + # revision (meaning it was unchanged) and we're not explicitly + # clearing the collection raise ApiError( title=('Collection {} not added to Shipyard ' 'buffer'.format(collection_id)), - description='Collection empty or resulted in no revision', + description='Collection created no new 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)) + 'message': + ('The collection {} added no new revision, and has ' + 'been rejected as invalid input. This likely ' + 'means that the collection already exists and ' + 'was reloaded with the same contents'.format( + collection_id)) }], retry=False, ) + else: + return helper.get_deckhand_validation_status(buffer_revision) else: raise ApiError( title='Invalid collection specified for buffer', diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/helpers/configdocs_helper.py b/src/bin/shipyard_airflow/shipyard_airflow/control/helpers/configdocs_helper.py index 19982e0c..749c4ff3 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/helpers/configdocs_helper.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/helpers/configdocs_helper.py @@ -105,8 +105,23 @@ class ConfigdocsHelper(object): return BufferMode.REJECTONCONTENTS def is_buffer_empty(self): - """ Check if the buffer is empty. """ - return self._get_revision(BUFFER) is None + """ Check if the buffer is empty. + + This can occur if there is no buffer revision, or if the buffer + revision is unchanged since the last committed version (or version 0) + """ + if self._get_revision(BUFFER) is None: + return True + # Get the "diff" of the collctions for Buffer vs. Committed (or 0) + collections = self.get_configdocs_status() + # If there are no collections or they are all unmodified, return True + # Deleted, created, or modified means there's something in the buffer. + if not collections: + return True + for c in collections: + if c['new_status'] != 'unmodified': + return False + return True def is_collection_in_buffer(self, collection_id): """ diff --git a/src/bin/shipyard_airflow/test-requirements.txt b/src/bin/shipyard_airflow/test-requirements.txt index 0e8884d6..5ea8b8ff 100644 --- a/src/bin/shipyard_airflow/test-requirements.txt +++ b/src/bin/shipyard_airflow/test-requirements.txt @@ -1,7 +1,7 @@ # Testing pytest==3.4 pytest-cov==2.5.1 -responses==0.8.1 +responses==0.10.2 testfixtures==5.1.1 apache-airflow[crypto,celery,postgres,hive,hdfs,jdbc]==1.10.0 diff --git a/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py b/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py index 4ac2982c..842defd4 100644 --- a/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py +++ b/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py @@ -61,6 +61,8 @@ REV_BUFFER_DICT = { } DIFF_BUFFER_DICT = {'mop': 'unmodified', 'chum': 'created', 'slop': 'deleted'} +UNMOD_BUFFER_DICT = {'mop': 'unmodified', 'chum': 'unmodified'} +EMPTY_BUFFER_DICT = {} ORDERED_VER = ['committed', 'buffer'] REV_NAME_ID = ('committed', 'buffer', 3, 5) @@ -183,21 +185,41 @@ def test_get_buffer_mode(): assert ConfigdocsHelper.get_buffer_mode('hippopotomus') is None -def test_is_buffer_emtpy(): +def test_is_buffer_empty(): """ Test the method to check if the configdocs buffer is empty """ helper = ConfigdocsHelper(CTX) - helper._get_revision_dict = lambda: REV_BUFFER_DICT - assert not helper.is_buffer_empty() + # BUFFER revision is none, short circuit case (no buffer revision) + # buffer is empty. helper._get_revision_dict = lambda: REV_BUFF_EMPTY_DICT assert helper.is_buffer_empty() - helper._get_revision_dict = lambda: REV_NO_COMMIT_DICT + # BUFFER revision is none, also a short circuit case (no revisions at all) + # buffer is empty + helper._get_revision_dict = lambda: REV_EMPTY_DICT + assert helper.is_buffer_empty() + + # BUFFER revision is not none, collections have been modified + # buffer is NOT empty. + helper._get_revision_dict = lambda: REV_BUFFER_DICT + helper.deckhand.get_diff = ( + lambda old_revision_id, new_revision_id: DIFF_BUFFER_DICT) assert not helper.is_buffer_empty() - helper._get_revision_dict = lambda: REV_EMPTY_DICT + # BUFFER revision is not none, all collections unmodified + # buffer is empty. + helper._get_revision_dict = lambda: REV_NO_COMMIT_DICT + helper.deckhand.get_diff = ( + lambda old_revision_id, new_revision_id: UNMOD_BUFFER_DICT) + assert helper.is_buffer_empty() + + # BUFFER revision is not none, no collections listed (deleted, rollback 0) + # buffer is empty. + helper._get_revision_dict = lambda: REV_NO_COMMIT_DICT + helper.deckhand.get_diff = ( + lambda old_revision_id, new_revision_id: EMPTY_BUFFER_DICT) assert helper.is_buffer_empty() diff --git a/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py b/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py index 1b251400..4eb5ac69 100644 --- a/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py +++ b/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py @@ -52,16 +52,21 @@ class ShipyardClient(BaseClient): def post_configdocs(self, collection_id=None, buffer_mode='rejectoncontents', + empty_collection=False, document_data=None): """ Ingests a collection of documents :param str collection_id: identifies a collection of docs.Bucket_id :param str buffermode: append|replace|rejectOnContents + :param empty_collection: True if the collection is empty. Document + data will be ignored if this flag is set to True. Default: False :param str document_data: data in a format understood by Deckhand(YAML) :returns: diff from last committed revision to new revision :rtype: Response object """ query_params = {"buffermode": buffer_mode} + if empty_collection: + query_params['empty-collection'] = True url = ApiPaths.POST_GET_CONFIG.value.format( self.get_endpoint(), collection_id diff --git a/src/bin/shipyard_client/shipyard_client/cli/commit/commands.py b/src/bin/shipyard_client/shipyard_client/cli/commit/commands.py index 554e841d..b1cee04f 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/commit/commands.py +++ b/src/bin/shipyard_client/shipyard_client/cli/commit/commands.py @@ -47,11 +47,11 @@ SHORT_DESC_CONFIGDOCS = ("Attempts to commit the Shipyard Buffer documents, " @click.option( '--force', '-f', - flag_value=True, + is_flag=True, help='Force the commit to occur, even if validations fail.') @click.option( '--dryrun', - flag_value=True, + is_flag=True, help='Retrieve validation status for the contents of the buffer without ' 'committing.') @click.pass_context diff --git a/src/bin/shipyard_client/shipyard_client/cli/create/actions.py b/src/bin/shipyard_client/shipyard_client/cli/create/actions.py index f37f901b..a9a30abe 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/create/actions.py +++ b/src/bin/shipyard_client/shipyard_client/cli/create/actions.py @@ -22,8 +22,8 @@ class CreateAction(CliAction): super().__init__(ctx) self.logger.debug( "CreateAction action initialized with action command " - "%s, parameters %s and allow-intermediate-commits=%s", - action_name, param, allow_intermediate_commits) + "%s, parameters %s and allow-intermediate-commits=%s", action_name, + param, allow_intermediate_commits) self.action_name = action_name self.param = param self.allow_intermediate_commits = allow_intermediate_commits @@ -57,27 +57,34 @@ class CreateAction(CliAction): class CreateConfigdocs(CliAction): """Action to Create Configdocs""" - def __init__(self, ctx, collection, buffer, data, filename): + def __init__(self, ctx, collection, buffer_mode, empty_collection, data, + filenames): """Sets parameters.""" super().__init__(ctx) - self.logger.debug("CreateConfigdocs action initialized with " - "collection=%s,buffer=%s, " - "Processed Files=" % (collection, buffer)) - for file in filename: + self.logger.debug( + "CreateConfigdocs action initialized with collection: %s, " + "buffer mode: %s, empty collection: %s, data length: %s. " + "Processed Files:", collection, buffer_mode, empty_collection, + len(data)) + for file in filenames: self.logger.debug(file) - self.logger.debug("data=%s" % str(data)) self.collection = collection - self.buffer = buffer + self.buffer_mode = buffer_mode + self.empty_collection = empty_collection self.data = data def invoke(self): """Calls API Client and formats response from API Client""" self.logger.debug("Calling API Client post_configdocs.") + + # Only send data payload if not empty_collection + data_to_send = "" if self.empty_collection else self.data + return self.get_api_client().post_configdocs( collection_id=self.collection, - buffer_mode=self.buffer, - document_data=self.data - ) + buffer_mode=self.buffer_mode, + empty_collection=self.empty_collection, + document_data=data_to_send) # Handle 409 with default error handler for cli. cli_handled_err_resp_codes = [409] @@ -94,5 +101,4 @@ class CreateConfigdocs(CliAction): """ outfmt_string = "Configuration documents added.\n{}" return outfmt_string.format( - format_utils.cli_format_status_handler(response) - ) + format_utils.cli_format_status_handler(response)) diff --git a/src/bin/shipyard_client/shipyard_client/cli/create/commands.py b/src/bin/shipyard_client/shipyard_client/cli/create/commands.py index aabce762..6ef0779d 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/create/commands.py +++ b/src/bin/shipyard_client/shipyard_client/cli/create/commands.py @@ -59,7 +59,7 @@ SHORT_DESC_ACTION = ( @click.option( '--allow-intermediate-commits', 'allow_intermediate_commits', - flag_value=True, + is_flag=True, help="Allow site action to go through even though there are prior commits " "that have not been used as part of a site action.") @click.pass_context @@ -82,6 +82,7 @@ DESC_CONFIGDOCS = """ COMMAND: configdocs \n DESCRIPTION: Load documents into the Shipyard Buffer. \n FORMAT: shipyard create configdocs [--append | --replace] +[--empty-collection] [--filename= (repeatable) | --directory=] (repeatable) --recurse\n EXAMPLE: shipyard create configdocs design --append @@ -96,15 +97,16 @@ SHORT_DESC_CONFIGDOCS = "Load documents into the Shipyard Buffer." @click.argument('collection') @click.option( '--append', - flag_value=True, + is_flag=True, help='Add the collection to the Shipyard Buffer. ') @click.option( '--replace', - flag_value=True, + is_flag=True, help='Clear the Shipyard Buffer and replace it with the specified ' 'contents. ') @click.option( '--filename', + 'filenames', multiple=True, type=click.Path(exists=True), help='The file name to use as the contents of the collection. ' @@ -117,59 +119,89 @@ SHORT_DESC_CONFIGDOCS = "Load documents into the Shipyard Buffer." 'a collection. (Repeatable).') @click.option( '--recurse', - flag_value=True, + is_flag=True, help='Recursively search through directories for yaml files.' ) +# The --empty-collection flag is explicit to prevent a user from accidentally +# loading an empty file and deleting things. This requires the user to clearly +# state their intention. +@click.option( + '--empty-collection', + is_flag=True, + help='Creates a version of the specified collection with no contents. ' + 'This option is the method by which a collection can be effectively ' + 'deleted. Any file and directory parameters will be ignored if this ' + 'option is used.' +) @click.pass_context -def create_configdocs(ctx, collection, filename, directory, append, - replace, recurse): +def create_configdocs(ctx, collection, filenames, directory, append, replace, + recurse, empty_collection): if (append and replace): ctx.fail('Either append or replace may be selected but not both') - if (not filename and not directory) or (filename and directory): - ctx.fail('Please specify one or more filenames using ' - '--filename="" OR one or more directories using ' - '--directory=""') if append: - create_buffer = 'append' + buffer_mode = 'append' elif replace: - create_buffer = 'replace' + buffer_mode = 'replace' else: - create_buffer = None + buffer_mode = None - if directory: - for dir in directory: - if recurse: - for path, dirs, files in os.walk(dir): - filename += tuple( - [os.path.join(path, name) for name in files - if name.endswith('.yaml')]) - else: - filename += tuple( - [os.path.join(dir, each) for each in os.listdir(dir) - if each.endswith('.yaml')]) + if empty_collection: + # Use an empty string as the document payload, and indicate no files. + data = "" + filenames = [] + else: + # Validate that appropriate file/directory params were specified. + if (not filenames and not directory) or (filenames and directory): + ctx.fail('Please specify one or more filenames using ' + '--filename="" OR one or more directories ' + 'using --directory=""') + # Scan and parse the input directories and files + if directory: + for _dir in directory: + if recurse: + for path, dirs, files in os.walk(_dir): + filenames += tuple([ + os.path.join(path, name) for name in files + if is_yaml(name) + ]) + else: + filenames += tuple([ + os.path.join(_dir, each) for each in os.listdir(_dir) + if is_yaml(each) + ]) - if not filename: - # None or empty list should raise this error - ctx.fail('The directory does not contain any YAML files. ' - 'Please enter one or more YAML files or a ' - 'directory that contains one or more YAML files.') + if not filenames: + # None or empty list should raise this error + ctx.fail('The directory does not contain any YAML files. ' + 'Please enter one or more YAML files or a ' + 'directory that contains one or more YAML files.') - docs = [] - for file in filename: - with open(file, 'r') as stream: - if file.endswith(".yaml"): - try: - docs += list(yaml.safe_load_all(stream)) - except yaml.YAMLError as exc: - ctx.fail('YAML file {} is invalid because {}' - .format(file, exc)) - else: - ctx.fail('The file {} is not a YAML file. Please enter ' - 'only YAML files.'.format(file)) + docs = [] + for _file in filenames: + with open(_file, 'r') as stream: + if is_yaml(_file): + try: + docs += list(yaml.safe_load_all(stream)) + except yaml.YAMLError as exc: + ctx.fail('YAML file {} is invalid because {}'.format( + _file, exc)) + else: + ctx.fail('The file {} is not a YAML file. Please enter ' + 'only YAML files.'.format(_file)) - data = yaml.safe_dump_all(docs) + data = yaml.safe_dump_all(docs) click.echo( - CreateConfigdocs(ctx, collection, create_buffer, data, filename) - .invoke_and_return_resp()) + CreateConfigdocs( + ctx=ctx, + collection=collection, + buffer_mode=buffer_mode, + empty_collection=empty_collection, + data=data, + filenames=filenames).invoke_and_return_resp()) + + +def is_yaml(filename): + """Test if the filename should be regarded as a yaml file""" + return filename.endswith(".yaml") or filename.endswith(".yml") diff --git a/src/bin/shipyard_client/shipyard_client/cli/get/commands.py b/src/bin/shipyard_client/shipyard_client/cli/get/commands.py index 1bbffa29..69892d09 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/get/commands.py +++ b/src/bin/shipyard_client/shipyard_client/cli/get/commands.py @@ -77,25 +77,25 @@ SHORT_DESC_CONFIGDOCS = ("Retrieve documents loaded into Shipyard, either " @click.option( '--committed', '-c', - flag_value='committed', + is_flag=True, help='Retrieve the documents that have last been committed for this ' 'collection') @click.option( '--buffer', '-b', - flag_value='buffer', + is_flag=True, help='Retrieve the documents that have been loaded into Shipyard since ' 'the prior commit. If no documents have been loaded into the buffer for ' 'this collection, this will return an empty response (default)') @click.option( '--last-site-action', '-l', - flag_value='last_site_action', + is_flag=True, help='Holds the revision information for the most recent site action') @click.option( '--successful-site-action', '-s', - flag_value='successful_site_action', + is_flag=True, help='Holds the revision information for the most recent successfully ' 'executed site action.') @click.option( @@ -150,23 +150,23 @@ SHORT_DESC_RENDEREDCONFIGDOCS = ( @click.option( '--committed', '-c', - flag_value='committed', + is_flag=True, help='Retrieve the documents that have last been committed.') @click.option( '--buffer', '-b', - flag_value='buffer', + is_flag=True, help='Retrieve the documents that have been loaded into Shipyard since the' ' prior commit. (default)') @click.option( '--last-site-action', '-l', - flag_value='last_site_action', + is_flag=True, help='Holds the revision information for the most recent site action') @click.option( '--successful-site-action', '-s', - flag_value='successful_site_action', + is_flag=True, help='Holds the revision information for the most recent successfully ' 'executed site action.') @click.option( diff --git a/src/bin/shipyard_client/test-requirements.txt b/src/bin/shipyard_client/test-requirements.txt index e280508d..32098efb 100644 --- a/src/bin/shipyard_client/test-requirements.txt +++ b/src/bin/shipyard_client/test-requirements.txt @@ -1,7 +1,7 @@ # Testing pytest==3.4 pytest-cov==2.5.1 -responses==0.8.1 +responses==0.10.2 testfixtures==5.1.1 # Linting diff --git a/src/bin/shipyard_client/tests/unit/cli/commit/test_commit_actions.py b/src/bin/shipyard_client/tests/unit/cli/commit/test_commit_actions.py index 079e24a3..aeac1f0f 100644 --- a/src/bin/shipyard_client/tests/unit/cli/commit/test_commit_actions.py +++ b/src/bin/shipyard_client/tests/unit/cli/commit/test_commit_actions.py @@ -19,13 +19,18 @@ from shipyard_client.api_client.base_client import BaseClient from shipyard_client.cli.commit.actions import CommitConfigdocs from tests.unit.cli import stubs +# TODO: refactor these tests to use responses callbacks (or other features) +# so that query parameter passing can be validated. +# moving to responses > 0.8 (e.g. 0.10.2) changed how URLS for responses +# seem to operate. + @responses.activate @mock.patch.object(BaseClient, 'get_endpoint', lambda x: 'http://shiptest') @mock.patch.object(BaseClient, 'get_token', lambda x: 'abc') def test_commit_configdocs(*args): responses.add(responses.POST, - 'http://shiptest/commitconfigdocs?force=false', + 'http://shiptest/commitconfigdocs', body=None, status=200) response = CommitConfigdocs(stubs.StubCliContext(), @@ -44,7 +49,7 @@ def test_commit_configdocs_409(*args): reason='Conflicts reason', code=409) responses.add(responses.POST, - 'http://shiptest/commitconfigdocs?force=false', + 'http://shiptest/commitconfigdocs', body=api_resp, status=409) response = CommitConfigdocs(stubs.StubCliContext(), @@ -65,7 +70,7 @@ def test_commit_configdocs_forced(*args): reason='Conflicts reason', code=200) responses.add(responses.POST, - 'http://shiptest/commitconfigdocs?force=true', + 'http://shiptest/commitconfigdocs', body=api_resp, status=200) response = CommitConfigdocs(stubs.StubCliContext(), @@ -80,7 +85,7 @@ def test_commit_configdocs_forced(*args): @mock.patch.object(BaseClient, 'get_token', lambda x: 'abc') def test_commit_configdocs_dryrun(*args): responses.add(responses.POST, - 'http://shiptest/commitconfigdocs?force=false', + 'http://shiptest/commitconfigdocs', body=None, status=200) response = CommitConfigdocs(stubs.StubCliContext(), diff --git a/src/bin/shipyard_client/tests/unit/cli/create/test_create_actions.py b/src/bin/shipyard_client/tests/unit/cli/create/test_create_actions.py index 5fa71f82..dbbae504 100644 --- a/src/bin/shipyard_client/tests/unit/cli/create/test_create_actions.py +++ b/src/bin/shipyard_client/tests/unit/cli/create/test_create_actions.py @@ -116,6 +116,7 @@ def test_create_configdocs(*args): response = CreateConfigdocs(stubs.StubCliContext(), 'design', 'append', + False, document_data, file_list).invoke_and_return_resp() assert 'Configuration documents added.' @@ -145,6 +146,7 @@ def test_create_configdocs_201_with_val_fails(*args): response = CreateConfigdocs(stubs.StubCliContext(), 'design', 'append', + False, document_data, file_list).invoke_and_return_resp() assert 'Configuration documents added.' in response @@ -175,8 +177,51 @@ def test_create_configdocs_409(*args): response = CreateConfigdocs(stubs.StubCliContext(), 'design', 'append', + False, document_data, file_list).invoke_and_return_resp() assert 'Error: Invalid collection' in response assert 'Reason: Buffermode : append' in response assert 'Buffer is either not...' in response + + +@responses.activate +@mock.patch.object(BaseClient, 'get_endpoint', lambda x: 'http://shiptest') +@mock.patch.object(BaseClient, 'get_token', lambda x: 'abc') +def test_create_configdocs_empty(*args): + def validating_callback(request): + # a request that has empty_collection should have no body. + assert request.body is None + resp_body = stubs.gen_err_resp( + message='Validations succeeded', + sub_error_count=0, + sub_info_count=0, + reason='Validation', + code=200) + return (201, {}, resp_body) + + responses.add_callback( + responses.POST, + 'http://shiptest/configdocs/design', + callback=validating_callback, + content_type='application/json') + + filename = 'tests/unit/cli/create/sample_yaml/sample.yaml' + document_data = yaml.dump_all(filename) + file_list = (filename, ) + + # pass data and empty_collection = True - should init with data, but + # not send the data on invoke + action = CreateConfigdocs( + stubs.StubCliContext(), + collection='design', + buffer_mode='append', + empty_collection=True, + data=document_data, + filenames=file_list) + + assert action.data == document_data + assert action.empty_collection == True + + response = action.invoke_and_return_resp() + assert response.startswith("Configuration documents added.") diff --git a/src/bin/shipyard_client/tests/unit/cli/create/test_create_commands.py b/src/bin/shipyard_client/tests/unit/cli/create/test_create_commands.py index 04649173..f05c29de 100644 --- a/src/bin/shipyard_client/tests/unit/cli/create/test_create_commands.py +++ b/src/bin/shipyard_client/tests/unit/cli/create/test_create_commands.py @@ -66,8 +66,93 @@ def test_create_configdocs(): auth_vars, 'create', 'configdocs', collection, '--' + append, '--filename=' + filename ]) - mock_method.assert_called_once_with(ANY, collection, 'append', - ANY, file_list) + mock_method.assert_called_once_with(ctx=ANY, collection=collection, + buffer_mode='append', empty_collection=False, data=ANY, + filenames=file_list) + + +def test_create_configdocs_empty(): + """test create configdocs with the --empty-collection flag""" + + collection = 'design' + filename = 'tests/unit/cli/create/sample_yaml/sample.yaml' + directory = 'tests/unit/cli/create/sample_yaml' + runner = CliRunner() + tests = [ + { + # replace mode, no file, no data, empty collection + 'kwargs': { + 'buffer_mode': 'replace', + 'empty_collection': True, + 'filenames': [], + 'data': "" + }, + 'args': [ + '--replace', + '--empty-collection', + ], + }, + { + # Append mode, no file, no data, empty collection + 'kwargs': { + 'buffer_mode': 'append', + 'empty_collection': True, + 'filenames': [], + 'data': "" + }, + 'args': [ + '--append', + '--empty-collection', + ], + }, + { + # No buffer mode specified, empty collection + 'kwargs': { + 'buffer_mode': None, + 'empty_collection': True, + 'filenames': [], + 'data': "" + }, + 'args': [ + '--empty-collection', + ], + }, + { + # Filename should be ignored and not passed, empty collection + 'kwargs': { + 'buffer_mode': None, + 'empty_collection': True, + 'filenames': [], + 'data': "" + }, + 'args': [ + '--empty-collection', + '--filename={}'.format(filename) + ], + }, + { + # Directory should be ignored and not passed, empty collection + 'kwargs': { + 'buffer_mode': None, + 'empty_collection': True, + 'filenames': [], + 'data': "" + }, + 'args': [ + '--empty-collection', + '--directory={}'.format(directory) + ], + }, + ] + + for tc in tests: + with patch.object(CreateConfigdocs, '__init__') as mock_method: + runner.invoke(shipyard, [ + auth_vars, 'create', 'configdocs', collection, *tc['args'] + ]) + + mock_method.assert_called_once_with(ctx=ANY, collection=collection, + **tc['kwargs']) def test_create_configdocs_directory(): @@ -82,7 +167,11 @@ def test_create_configdocs_directory(): auth_vars, 'create', 'configdocs', collection, '--' + append, '--directory=' + directory ]) - mock_method.assert_called_once_with(ANY, collection, 'append', ANY, ANY) + # TODO(bryan-strassner) Make this test useful to show directory parsing + # happened. + mock_method.assert_called_once_with(ctx=ANY, collection=collection, + buffer_mode='append', empty_collection=False, data=ANY, + filenames=ANY) def test_create_configdocs_directory_empty(): @@ -114,11 +203,15 @@ def test_create_configdocs_multi_directory(): auth_vars, 'create', 'configdocs', collection, '--' + append, '--directory=' + dir1, '--directory=' + dir2 ]) - mock_method.assert_called_once_with(ANY, collection, 'append', ANY, ANY) + # TODO(bryan-strassner) Make this test useful to show multiple directories + # were actually traversed. + mock_method.assert_called_once_with(ctx=ANY, collection=collection, + buffer_mode='append', empty_collection=False, data=ANY, + filenames=ANY) def test_create_configdocs_multi_directory_recurse(): - """test create configdocs with multiple directories""" + """test create configdocs with multiple directories recursively""" collection = 'design' dir1 = 'tests/unit/cli/create/sample_yaml/' @@ -130,7 +223,11 @@ def test_create_configdocs_multi_directory_recurse(): auth_vars, 'create', 'configdocs', collection, '--' + append, '--directory=' + dir1, '--directory=' + dir2, '--recurse' ]) - mock_method.assert_called_once_with(ANY, collection, 'append', ANY, ANY) + # TODO(bryan-strassner) Make this test useful to show multiple directories + # were actually traversed and recursed. + mock_method.assert_called_once_with(ctx=ANY, collection=collection, + buffer_mode='append', empty_collection=False, data=ANY, + filenames=ANY) def test_create_configdocs_negative():