fix: Use schema instead of metadata.schema for replacement check

Recently added replacement check incorrectly uses metadata.schema
and metadata.name to key on the document -- but it should be schema
and metadata.name, the combination of which uniquely defines a
document.

Change-Id: I6cd1679ad41be38cb78d65ce2763e60f7da390d2

8 files changed, 38 insertions, 41 deletions

diff --git a/deckhand/engine/_replacement.py b/deckhand/engine/_replacement.py
index a420aad..d9bf3d7 100644
--- a/deckhand/engine/_replacement.py
+++ b/deckhand/engine/_replacement.py
@@ -107,16 +107,14 @@ def check_replacement_is_false_uniqueness(
     if not document.is_control:
         document_identifier = (
             document['metadata']['name'],
-            document['metadata']['schema']
+            document['schema']
         )
         if document_identifier in non_replacement_documents:
             error_message = (
-                'Documents with the same name and schema existing in '
-                'different layers without any of them having '
-                '`replacement = true` cannot exist as Deckhand will '
-                'arbitrarily select any of them for processing and none are '
-                'distinguishable from one another because none are a '
-                'parent or child or a replacement document.')
+                'More than one document with the same name and schema was '
+                'found, but none has `replacement: true`. Ensure that only 2 '
+                'documents exist for each replacement and that one has '
+                '`replacement: true` and the other `replacement: false`.')
             raise errors.InvalidDocumentReplacement(
                 schema=document.schema, name=document.name,
                 layer=document.layer, reason=error_message)
diff --git a/deckhand/errors.py b/deckhand/errors.py
index 54a30cf..9d31724 100644
--- a/deckhand/errors.py
+++ b/deckhand/errors.py
@@ -160,7 +160,7 @@ class DeckhandException(Exception):
     a 'msg_fmt' property. That msg_fmt will get printf'd
     with the keyword arguments provided to the constructor.
     """
-    msg_fmt = "An unknown exception occurred."
+    msg_fmt = "An unknown exception occurred"
 
     def __init__(self, message=None, code=500, **kwargs):
         kwargs.setdefault('code', code)
@@ -195,7 +195,7 @@ class InvalidDocumentFormat(DeckhandException):
 
     **Troubleshoot:**
     """
-    msg_fmt = ("The provided documents failed schema validation.")
+    msg_fmt = ("The provided documents failed schema validation")
     code = 400
 
 
@@ -210,7 +210,7 @@ class InvalidDocumentLayer(DeckhandException):
     msg_fmt = ("Invalid layer '%(document_layer)s' for document "
                "[%(document_schema)s] %(document_name)s was not found in "
                "layerOrder: %(layer_order)s for provided LayeringPolicy: "
-               "%(layering_policy_name)s.")
+               "%(layering_policy_name)s")
     code = 400
 
 
@@ -234,7 +234,7 @@ class IndeterminateDocumentParent(DeckhandException):
     **Troubleshoot:**
     """
     msg_fmt = ("Too many parent documents found for document [%(schema)s, "
-               "%(layer)s] %(name)s. Found: %(found)s. Expected: 1.")
+               "%(layer)s] %(name)s. Found: %(found)s. Expected: 1")
     code = 400
 
 
@@ -246,7 +246,7 @@ class SubstitutionDependencyCycle(DeckhandException):
     * Check that there is no two-way substitution dependency between documents.
     """
     msg_fmt = ('Cannot determine substitution order as a dependency '
-               'cycle exists for the following documents: %(cycle)s.')
+               'cycle exists for the following documents: %(cycle)s')
     code = 400
 
 
@@ -266,8 +266,7 @@ class MissingDocumentKey(DeckhandException):
     msg_fmt = ("Missing action path in %(action)s needed for layering from "
                "either the data section of the parent [%(parent_schema)s, "
                "%(parent_layer)s] %(parent_name)s or child [%(child_schema)s, "
-               "%(child_layer)s] %(child_name)s "
-               "document.")
+               "%(child_layer)s] %(child_name)s document")
     code = 400
 
 
@@ -283,7 +282,7 @@ class MissingDocumentPattern(DeckhandException):
     msg_fmt = ("The destination document's `data` section is missing the "
                "pattern %(pattern)s specified under "
                "`substitutions.dest.pattern` at path %(jsonpath)s, specified "
-               "under `substitutions.dest.path`.")
+               "under `substitutions.dest.path`")
     code = 400
 
 
@@ -308,7 +307,7 @@ class UnsupportedActionMethod(DeckhandException):
 
     **Troubleshoot:**
     """
-    msg_fmt = ("Method in %(actions)s is invalid for document %(document)s.")
+    msg_fmt = ("Method in %(actions)s is invalid for document %(document)s")
     code = 400
 
 
@@ -318,7 +317,7 @@ class RevisionTagBadFormat(DeckhandException):
     **Troubleshoot:**
     """
     msg_fmt = ("The requested tag data %(data)s must either be null or "
-               "dictionary.")
+               "dictionary")
     code = 400
 
 
@@ -338,7 +337,7 @@ class SubstitutionSourceDataNotFound(DeckhandException):
         "%(src_path)s in source document [%(src_schema)s, %(src_layer)s] "
         "%(src_name)s which is referenced by destination document "
         "[%(dest_schema)s, %(dest_layer)s] %(dest_name)s under its "
-        "`metadata.substitutions`.")
+        "`metadata.substitutions`")
     code = 400
 
 
@@ -352,7 +351,7 @@ class EncryptionSourceNotFound(DeckhandException):
     msg_fmt = (
         "Required encryption source reference could not be resolved into a "
         "secret because it was not found among encryption sources. Ref: "
-        "%(secret_ref)s. Referenced by: [%(schema)s, %(layer)s] %(name)s.")
+        "%(secret_ref)s. Referenced by: [%(schema)s, %(layer)s] %(name)s")
     code = 400  # Indicates bad data was passed in, causing a lookup to fail.
 
 
@@ -362,7 +361,7 @@ class DocumentNotFound(DeckhandException):
     **Troubleshoot:**
     """
     msg_fmt = ("The requested document using filters: %(filters)s was not "
-               "found.")
+               "found")
     code = 404
 
 
@@ -371,7 +370,7 @@ class RevisionNotFound(DeckhandException):
 
     **Troubleshoot:**
     """
-    msg_fmt = "The requested revision=%(revision_id)s was not found."
+    msg_fmt = "The requested revision=%(revision_id)s was not found"
     code = 404
 
 
@@ -381,7 +380,7 @@ class RevisionTagNotFound(DeckhandException):
     **Troubleshoot:**
     """
     msg_fmt = ("The requested tag '%(tag)s' for revision %(revision)s was "
-               "not found.")
+               "not found")
     code = 404
 
 
@@ -392,7 +391,7 @@ class ValidationNotFound(DeckhandException):
     """
     msg_fmt = ("The requested validation entry %(entry_id)s was not found "
                "for validation name %(validation_name)s and revision ID "
-               "%(revision_id)s.")
+               "%(revision_id)s")
     code = 404
 
 
@@ -403,7 +402,7 @@ class DuplicateDocumentExists(DeckhandException):
     **Troubleshoot:**
     """
     msg_fmt = ("Document [%(schema)s, %(layer)s] %(name)s already exists in "
-               "bucket: %(bucket)s.")
+               "bucket: %(bucket)s")
     code = 409
 
 
@@ -416,7 +415,7 @@ class SingletonDocumentConflict(DeckhandException):
     msg_fmt = ("A singleton document [%(schema)s, %(layer)s] %(name)s already "
                "exists in the system. The new document(s) %(conflict)s cannot "
                "be created. To create a document with a new name, delete the "
-               "current one first.")
+               "current one first")
     code = 409
 
 
@@ -425,7 +424,7 @@ class LayeringPolicyNotFound(DeckhandException):
 
     **Troubleshoot:**
     """
-    msg_fmt = ("Required LayeringPolicy was not found for layering.")
+    msg_fmt = ("Required LayeringPolicy was not found for layering")
     code = 409
 
 
@@ -440,7 +439,7 @@ class SubstitutionSourceNotFound(DeckhandException):
     msg_fmt = (
         "Required substitution source document [%(src_schema)s] %(src_name)s "
         "was not found, yet is referenced by [%(document_schema)s] "
-        "%(document_name)s.")
+        "%(document_name)s")
     code = 409
 
 
@@ -449,7 +448,7 @@ class PolicyNotAuthorized(DeckhandException):
 
     **Troubleshoot:**
     """
-    msg_fmt = "Policy doesn't allow %(action)s to be performed."
+    msg_fmt = "Policy doesn't allow %(action)s to be performed"
     code = 403
 
 
diff --git a/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml b/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml
index d984a1c..da032d0 100644
--- a/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml
+++ b/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml
@@ -161,7 +161,7 @@ tests:
       $.[0].details.messageList[0].level: Error
       $.[0].details.messageList[0].name: D002
       $.[0].kind: Status
-      $.[0].message: The provided documents failed schema validation.
+      $.[0].message: The provided documents failed schema validation
       $.[0].reason: Validation
       $.[0].status: Failure
 
diff --git a/deckhand/tests/unit/control/test_errors.py b/deckhand/tests/unit/control/test_errors.py
index ed18407..538973a 100644
--- a/deckhand/tests/unit/control/test_errors.py
+++ b/deckhand/tests/unit/control/test_errors.py
@@ -158,7 +158,7 @@ class TestValidationMessageFormatting(test_base.BaseControllerTest):
                     }
                 ]
             },
-            'message': 'The provided documents failed schema validation.',
+            'message': 'The provided documents failed schema validation',
             'metadata': {}
         }
         body = yaml.safe_load(resp.text)
@@ -224,7 +224,7 @@ class TestValidationMessageFormatting(test_base.BaseControllerTest):
                     }
                 ]
             },
-            'message': 'The provided documents failed schema validation.',
+            'message': 'The provided documents failed schema validation',
             'metadata': {}
         }
         body = yaml.safe_load(resp.text)
diff --git a/deckhand/tests/unit/control/test_validations_controller.py b/deckhand/tests/unit/control/test_validations_controller.py
index 05c94d4..15bcb7c 100644
--- a/deckhand/tests/unit/control/test_validations_controller.py
+++ b/deckhand/tests/unit/control/test_validations_controller.py
@@ -348,7 +348,7 @@ class TestValidationsController(BaseValidationsControllerTest):
                                        VALIDATION_FAILURE_RESULT)
         self.assertEqual(201, resp.status_code)
         expected_error = ('The requested validation entry 5 was not found for '
-                          'validation name %s and revision ID %d.' % (
+                          'validation name %s and revision ID %d' % (
                               validation_name, revision_id))
 
         resp = self.app.simulate_get(
diff --git a/deckhand/tests/unit/db/test_documents_negative.py b/deckhand/tests/unit/db/test_documents_negative.py
index 1633a13..dcd5bb9 100644
--- a/deckhand/tests/unit/db/test_documents_negative.py
+++ b/deckhand/tests/unit/db/test_documents_negative.py
@@ -61,7 +61,7 @@ class TestDocumentsNegative(base.DeckhandWithDBTestCase):
 
         # Verify that the document cannot be created in another bucket.
         alt_bucket_name = test_utils.rand_name('bucket')
-        error_re = r"^Document .* already exists in bucket: %s.$" % bucket_name
+        error_re = r"^Document .* already exists in bucket: %s$" % bucket_name
         self.assertRaisesRegex(
             errors.DuplicateDocumentExists, error_re, self.create_documents,
             alt_bucket_name, payload)
diff --git a/deckhand/tests/unit/db/test_revisions.py b/deckhand/tests/unit/db/test_revisions.py
index 34419ba..e26c0f9 100644
--- a/deckhand/tests/unit/db/test_revisions.py
+++ b/deckhand/tests/unit/db/test_revisions.py
@@ -126,7 +126,7 @@ class TestRevisions(base.DeckhandWithDBTestCase):
 
         # Validate that all revisions were deleted.
         for revision_id in all_revision_ids:
-            error_re = (r'^The requested revision=%s was not found.$'
+            error_re = (r'^The requested revision=%s was not found$'
                         % revision_id)
             self.assertRaisesRegex(errors.RevisionNotFound, error_re,
                                    self.show_revision, revision_id)
@@ -135,7 +135,7 @@ class TestRevisions(base.DeckhandWithDBTestCase):
         for doc in created_documents:
             filters = {'id': doc['id']}
             error_re = (r'^The requested document using filters: %s was not '
-                        'found.$' % filters)
+                        'found$' % filters)
             self.assertRaisesRegex(errors.DocumentNotFound, error_re,
                                    self.show_document, **filters)
 
diff --git a/deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py b/deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py
index 7f73a94..2515f0c 100644
--- a/deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py
+++ b/deckhand/tests/unit/engine/test_document_layering_and_replacement_negative.py
@@ -139,9 +139,9 @@ class TestDocumentLayeringReplacementNegative(
                                 self._test_layering, documents)
 
     def test_replacement_true_with_parent_replacement_true_raises_exc(self):
-        """Validate that when documents have the same `metadata.name` and
-        `metadata.schema` existing in different layers without any of them
-        having `replacement = true` raises an exception
+        """Validate that when documents have the same ``metadata.name`` and
+        ``schema`` existing in different layers without any of them
+        having ``replacement = true`` raises an exception
         """
         doc_factory = factories.DocumentFactory(2, [1, 1])
         documents = doc_factory.gen_test({})
@@ -154,8 +154,8 @@ class TestDocumentLayeringReplacementNegative(
                 document['metadata']['layeringDefinition'].pop(
                     'parentSelector')
 
-        error_re = (r'Documents with the same name and schema existing in '
-                    'different layers without any of them having '
-                    '`replacement = true` cannot exist.*')
+        error_re = (
+            r'More than one document with the same name and schema was found, '
+            'but none has `replacement: true`.*')
         self.assertRaisesRegexp(errors.InvalidDocumentReplacement, error_re,
                                 self._test_layering, documents)