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