From aa63d2ff06be2f790102da3ffd8d24193c4f86f7 Mon Sep 17 00:00:00 2001 From: Elizabeth Esswein Date: Mon, 29 Jan 2024 15:44:24 -0500 Subject: [PATCH] standardize exceptions --- SpiffWorkflow/bpmn/parser/event_parsers.py | 20 +++++++++++++++---- SpiffWorkflow/bpmn/parser/util.py | 5 ++--- .../serializer/{migration => }/exceptions.py | 0 .../bpmn/serializer/migration/version_1_2.py | 2 +- SpiffWorkflow/operators.py | 3 ++- SpiffWorkflow/serializer/dict.py | 6 ------ SpiffWorkflow/serializer/prettyxml.py | 3 +-- SpiffWorkflow/specs/AcquireMutex.py | 1 - SpiffWorkflow/specs/Choose.py | 3 --- SpiffWorkflow/specs/ExclusiveChoice.py | 1 - SpiffWorkflow/specs/Execute.py | 2 -- SpiffWorkflow/specs/Gate.py | 3 --- SpiffWorkflow/specs/MultiChoice.py | 1 - SpiffWorkflow/specs/ReleaseMutex.py | 1 - SpiffWorkflow/specs/SubWorkflow.py | 2 -- SpiffWorkflow/specs/ThreadMerge.py | 1 - SpiffWorkflow/specs/Transform.py | 2 -- SpiffWorkflow/specs/Trigger.py | 4 ---- SpiffWorkflow/specs/base.py | 3 --- SpiffWorkflow/spiff/parser/event_parsers.py | 20 +++++++++++++++---- SpiffWorkflow/task.py | 4 ---- SpiffWorkflow/workflow.py | 1 - .../bpmn/serializer/VersionMigrationTest.py | 2 +- 23 files changed, 39 insertions(+), 51 deletions(-) rename SpiffWorkflow/bpmn/serializer/{migration => }/exceptions.py (100%) diff --git a/SpiffWorkflow/bpmn/parser/event_parsers.py b/SpiffWorkflow/bpmn/parser/event_parsers.py index aa856f47..0f2f197d 100644 --- a/SpiffWorkflow/bpmn/parser/event_parsers.py +++ b/SpiffWorkflow/bpmn/parser/event_parsers.py @@ -89,7 +89,10 @@ def parse_error_event(self, error_event): """Parse the errorEventDefinition node and return an instance of ErrorEventDefinition.""" error_ref = error_event.get('errorRef') if error_ref: - error = one(self.doc_xpath('.//bpmn:error[@id="%s"]' % error_ref)) + try: + error = one(self.doc_xpath('.//bpmn:error[@id="%s"]' % error_ref)) + except Exception: + self.raise_validation_exception('Expected an error node', node=error_event) error_code = error.get('errorCode') name = error.get('name') else: @@ -101,7 +104,10 @@ def parse_escalation_event(self, escalation_event): escalation_ref = escalation_event.get('escalationRef') if escalation_ref: - escalation = one(self.doc_xpath('.//bpmn:escalation[@id="%s"]' % escalation_ref)) + try: + escalation = one(self.doc_xpath('.//bpmn:escalation[@id="%s"]' % escalation_ref)) + except Exception: + self.raise_validation_exception('Expected an Escalation node', node=escalation_event) escalation_code = escalation.get('escalationCode') name = escalation.get('name') else: @@ -112,7 +118,10 @@ def parse_message_event(self, message_event): message_ref = message_event.get('messageRef') if message_ref is not None: - message = one(self.doc_xpath('.//bpmn:message[@id="%s"]' % message_ref)) + try: + message = one(self.doc_xpath('.//bpmn:message[@id="%s"]' % message_ref)) + except Exception: + self.raise_validation_exception('Expected a Message node', node=message_event) name = message.get('name') description = self.get_event_description(message_event) correlations = self.get_message_correlations(message_ref) @@ -127,7 +136,10 @@ def parse_signal_event(self, signal_event): signal_ref = signal_event.get('signalRef') if signal_ref: - signal = one(self.doc_xpath('.//bpmn:signal[@id="%s"]' % signal_ref)) + try: + signal = one(self.doc_xpath('.//bpmn:signal[@id="%s"]' % signal_ref)) + except Exception: + self.raise_validation_exception('Expected a Signal node', node=signal_event) name = signal.get('name') else: name = signal_event.getparent().get('name') diff --git a/SpiffWorkflow/bpmn/parser/util.py b/SpiffWorkflow/bpmn/parser/util.py index 0dd876b9..03ddbc5e 100644 --- a/SpiffWorkflow/bpmn/parser/util.py +++ b/SpiffWorkflow/bpmn/parser/util.py @@ -17,7 +17,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA # 02110-1301 USA - BPMN_MODEL_NS = 'http://www.omg.org/spec/BPMN/20100524/MODEL' DIAG_INTERCHANGE_NS = "http://www.omg.org/spec/BPMN/20100524/DI" DIAG_COMMON_NS = "http://www.omg.org/spec/DD/20100524/DC" @@ -34,8 +33,8 @@ def one(nodes, or_none=False): """ if not nodes and or_none: return None - assert len( - nodes) == 1, 'Expected 1 result. Received %d results.' % (len(nodes)) + + assert len(nodes) == 1, 'Expected 1 result. Received %d results.' % (len(nodes)) return nodes[0] diff --git a/SpiffWorkflow/bpmn/serializer/migration/exceptions.py b/SpiffWorkflow/bpmn/serializer/exceptions.py similarity index 100% rename from SpiffWorkflow/bpmn/serializer/migration/exceptions.py rename to SpiffWorkflow/bpmn/serializer/exceptions.py diff --git a/SpiffWorkflow/bpmn/serializer/migration/version_1_2.py b/SpiffWorkflow/bpmn/serializer/migration/version_1_2.py index e3f0146b..90b54f2e 100644 --- a/SpiffWorkflow/bpmn/serializer/migration/version_1_2.py +++ b/SpiffWorkflow/bpmn/serializer/migration/version_1_2.py @@ -22,7 +22,7 @@ from SpiffWorkflow.util.task import TaskState from SpiffWorkflow.bpmn.specs.event_definitions.timer import LOCALTZ -from .exceptions import VersionMigrationError +from ..exceptions import VersionMigrationError def td_to_iso(td): total = td.total_seconds() diff --git a/SpiffWorkflow/operators.py b/SpiffWorkflow/operators.py index b9f43cb6..2071861d 100644 --- a/SpiffWorkflow/operators.py +++ b/SpiffWorkflow/operators.py @@ -139,7 +139,8 @@ def __init__(self, """ if not right_attribute and not right: raise ValueError('require argument: right_attribute or right') - assert left_attribute is not None + if left_attribute is None: + raise ValueError('left attribute is None') self.left_attribute = left_attribute self.right_attribute = right_attribute self.right = right diff --git a/SpiffWorkflow/serializer/dict.py b/SpiffWorkflow/serializer/dict.py index 5574c672..21b0e32a 100644 --- a/SpiffWorkflow/serializer/dict.py +++ b/SpiffWorkflow/serializer/dict.py @@ -348,12 +348,6 @@ def deserialize_simple(self, wf_spec, s_state): self.deserialize_task_spec(wf_spec, s_state, spec=spec) return spec - def deserialize_generic(self, wf_spec, s_state,newclass): - assert isinstance(wf_spec, WorkflowSpec) - spec = newclass(wf_spec, s_state['name']) - self.deserialize_task_spec(wf_spec, s_state, spec=spec) - return spec - def serialize_start_task(self, spec): return self.serialize_task_spec(spec) diff --git a/SpiffWorkflow/serializer/prettyxml.py b/SpiffWorkflow/serializer/prettyxml.py index 8b4410be..a2a70b84 100644 --- a/SpiffWorkflow/serializer/prettyxml.py +++ b/SpiffWorkflow/serializer/prettyxml.py @@ -219,8 +219,7 @@ def deserialize_task_spec(self, workflow, start_node, read_specs): pass elif node.tag == 'description': kwargs['description'] = node.text - elif node.tag == 'successor' \ - or node.tag == 'default-successor': + elif node.tag == 'successor' or node.tag == 'default-successor': if not node.text: self.raise_parser_exception('Empty %s tag' % node.tag) successors.append((None, node.text)) diff --git a/SpiffWorkflow/specs/AcquireMutex.py b/SpiffWorkflow/specs/AcquireMutex.py index 6c3a3c6e..ee85e572 100644 --- a/SpiffWorkflow/specs/AcquireMutex.py +++ b/SpiffWorkflow/specs/AcquireMutex.py @@ -45,7 +45,6 @@ def __init__(self, wf_spec, name, mutex, **kwargs): :type kwargs: dict :param kwargs: See :class:`SpiffWorkflow.specs.TaskSpec`. """ - assert mutex is not None TaskSpec.__init__(self, wf_spec, name, **kwargs) self.mutex = mutex diff --git a/SpiffWorkflow/specs/Choose.py b/SpiffWorkflow/specs/Choose.py index c048c653..34f1e7b9 100644 --- a/SpiffWorkflow/specs/Choose.py +++ b/SpiffWorkflow/specs/Choose.py @@ -48,9 +48,6 @@ def __init__(self, wf_spec, name, context, choice=None, **kwargs): :type kwargs: dict :param kwargs: See :class:`SpiffWorkflow.specs.TaskSpec`. """ - assert wf_spec is not None - assert name is not None - assert context is not None # HACK: inherit from TaskSpec (not Trigger) on purpose. TaskSpec.__init__(self, wf_spec, name, **kwargs) self.context = context diff --git a/SpiffWorkflow/specs/ExclusiveChoice.py b/SpiffWorkflow/specs/ExclusiveChoice.py index 848e3268..f15116b0 100644 --- a/SpiffWorkflow/specs/ExclusiveChoice.py +++ b/SpiffWorkflow/specs/ExclusiveChoice.py @@ -53,7 +53,6 @@ def connect(self, taskspec): :type task_spec: TaskSpec :param task_spec: The following task spec. """ - assert self.default_task_spec is None self.default_task_spec = taskspec.name super().connect(taskspec) diff --git a/SpiffWorkflow/specs/Execute.py b/SpiffWorkflow/specs/Execute.py index 06537abb..18ff0b47 100644 --- a/SpiffWorkflow/specs/Execute.py +++ b/SpiffWorkflow/specs/Execute.py @@ -49,8 +49,6 @@ def __init__(self, wf_spec, name, args=None, **kwargs): :type kwargs: dict :param kwargs: kwargs to pass-through to TaskSpec initializer. """ - assert wf_spec is not None - assert name is not None TaskSpec.__init__(self, wf_spec, name, **kwargs) self.args = args diff --git a/SpiffWorkflow/specs/Gate.py b/SpiffWorkflow/specs/Gate.py index ff2692bf..74d6bf36 100644 --- a/SpiffWorkflow/specs/Gate.py +++ b/SpiffWorkflow/specs/Gate.py @@ -46,9 +46,6 @@ def __init__(self, wf_spec, name, context, **kwargs): :type kwargs: dict :param kwargs: See :class:`SpiffWorkflow.specs.TaskSpec`. """ - assert wf_spec is not None - assert name is not None - assert context is not None TaskSpec.__init__(self, wf_spec, name, **kwargs) self.context = context diff --git a/SpiffWorkflow/specs/MultiChoice.py b/SpiffWorkflow/specs/MultiChoice.py index 0bea89e0..3bf5b946 100644 --- a/SpiffWorkflow/specs/MultiChoice.py +++ b/SpiffWorkflow/specs/MultiChoice.py @@ -59,7 +59,6 @@ def connect_if(self, condition, task_spec): condition -- a condition (Condition) taskspec -- the conditional task spec """ - assert task_spec is not None self._outputs.append(task_spec.name) self.cond_task_specs.append((condition, task_spec.name)) task_spec._connect_notify(self) diff --git a/SpiffWorkflow/specs/ReleaseMutex.py b/SpiffWorkflow/specs/ReleaseMutex.py index c4514f16..f7c34716 100644 --- a/SpiffWorkflow/specs/ReleaseMutex.py +++ b/SpiffWorkflow/specs/ReleaseMutex.py @@ -44,7 +44,6 @@ def __init__(self, wf_spec, name, mutex, **kwargs): :type kwargs: dict :param kwargs: See :class:`SpiffWorkflow.specs.TaskSpec`. """ - assert mutex is not None TaskSpec.__init__(self, wf_spec, name, **kwargs) self.mutex = mutex diff --git a/SpiffWorkflow/specs/SubWorkflow.py b/SpiffWorkflow/specs/SubWorkflow.py index a0acc707..ccf27cac 100644 --- a/SpiffWorkflow/specs/SubWorkflow.py +++ b/SpiffWorkflow/specs/SubWorkflow.py @@ -62,8 +62,6 @@ def __init__(self, :type kwargs: dict :param kwargs: See :class:`SpiffWorkflow.specs.TaskSpec`. """ - assert wf_spec is not None - assert name is not None super(SubWorkflow, self).__init__(wf_spec, name, **kwargs) self.file = None self.in_assign = in_assign is not None and in_assign or [] diff --git a/SpiffWorkflow/specs/ThreadMerge.py b/SpiffWorkflow/specs/ThreadMerge.py index 5973f3b2..8e026790 100644 --- a/SpiffWorkflow/specs/ThreadMerge.py +++ b/SpiffWorkflow/specs/ThreadMerge.py @@ -49,7 +49,6 @@ def __init__(self, :type kwargs: dict :param kwargs: See :class:`SpiffWorkflow.specs.Join`. """ - assert split_task is not None Join.__init__(self, wf_spec, name, split_task, **kwargs) def _start(self, my_task): diff --git a/SpiffWorkflow/specs/Transform.py b/SpiffWorkflow/specs/Transform.py index ccb04f64..585fdcb4 100644 --- a/SpiffWorkflow/specs/Transform.py +++ b/SpiffWorkflow/specs/Transform.py @@ -47,8 +47,6 @@ def __init__(self, wf_spec, name, transforms=None, **kwargs): :type kwargs: dict :param kwargs: See :class:`SpiffWorkflow.specs.TaskSpec`. """ - assert wf_spec is not None - assert name is not None TaskSpec.__init__(self, wf_spec, name, **kwargs) self.transforms = transforms diff --git a/SpiffWorkflow/specs/Trigger.py b/SpiffWorkflow/specs/Trigger.py index 22f89a89..ff4f031e 100644 --- a/SpiffWorkflow/specs/Trigger.py +++ b/SpiffWorkflow/specs/Trigger.py @@ -48,10 +48,6 @@ def __init__(self, wf_spec, name, context, times=1, **kwargs): :type kwargs: dict :param kwargs: See :class:`SpiffWorkflow.specs.TaskSpec`. """ - assert wf_spec is not None - assert name is not None - assert context is not None - assert isinstance(context, list) TaskSpec.__init__(self, wf_spec, name, **kwargs) self.context = context self.times = times diff --git a/SpiffWorkflow/specs/base.py b/SpiffWorkflow/specs/base.py index 8489afa7..822c4c28 100644 --- a/SpiffWorkflow/specs/base.py +++ b/SpiffWorkflow/specs/base.py @@ -87,8 +87,6 @@ def __init__(self, wf_spec, name, **kwargs): :type post_assign: list((str, object)) :param post_assign: a list of name/value pairs """ - assert wf_spec is not None - assert name is not None self._wf_spec = wf_spec self.name = str(name) self.description = kwargs.get('description', None) @@ -279,7 +277,6 @@ def _on_ready(self, my_task): :type my_task: Task :param my_task: The associated task in the task tree. """ - assert my_task is not None self.test() # Assign variables, if so requested. diff --git a/SpiffWorkflow/spiff/parser/event_parsers.py b/SpiffWorkflow/spiff/parser/event_parsers.py index a93dd77d..32ae6a9f 100644 --- a/SpiffWorkflow/spiff/parser/event_parsers.py +++ b/SpiffWorkflow/spiff/parser/event_parsers.py @@ -43,7 +43,10 @@ def parse_message_event(self, message_event): message_ref = message_event.get('messageRef') if message_ref: - message = one(self.doc_xpath('.//bpmn:message[@id="%s"]' % message_ref)) + try: + message = one(self.doc_xpath('.//bpmn:message[@id="%s"]' % message_ref)) + except Exception: + self.raise_validation_error('Expected a Message node', node=message_event) name = message.get('name') extensions = self.parse_extensions(message) correlations = self.get_message_correlations(message_ref) @@ -61,7 +64,10 @@ def parse_signal_event(self, signal_event): """Parse a Spiff signal event""" signal_ref = signal_event.get('signalRef') if signal_ref is not None: - signal = one(self.doc_xpath(f'.//bpmn:signal[@id="{signal_ref}"]')) + try: + signal = one(self.doc_xpath(f'.//bpmn:signal[@id="{signal_ref}"]')) + except Exception: + self.raise_validation_error('Expected a Signal node', node=signal_event) name = signal.get('name') extensions = self.parse_extensions(signal) expression = extensions.get('payloadExpression') @@ -75,7 +81,10 @@ def parse_error_event(self, error_event): """Parse a Spiff error event""" error_ref = error_event.get('errorRef') if error_ref is not None: - error = one(self.doc_xpath(f'.//bpmn:error[@id="{error_ref}"]')) + try: + error = one(self.doc_xpath(f'.//bpmn:error[@id="{error_ref}"]')) + except Exception: + self.raise_validation_error('Expected an Error node', node=error_event) name = error.get('name') code = error.get('errorCode') extensions = self.parse_extensions(error) @@ -90,7 +99,10 @@ def parse_escalation_event(self, escalation_event): """Parse a Spiff error event""" escalation_ref = escalation_event.get('escalationRef') if escalation_ref is not None: - escalation = one(self.doc_xpath(f'.//bpmn:escalation[@id="{escalation_ref}"]')) + try: + escalation = one(self.doc_xpath(f'.//bpmn:escalation[@id="{escalation_ref}"]')) + except Exception: + self.raise_validation_error('Expected an Escalation node', node=escalation_event) name = escalation.get('name') code = escalation.get('escalationCode') extensions = self.parse_extensions(escalation) diff --git a/SpiffWorkflow/task.py b/SpiffWorkflow/task.py index ff33a7e9..07d565d9 100644 --- a/SpiffWorkflow/task.py +++ b/SpiffWorkflow/task.py @@ -71,9 +71,6 @@ def __init__(self, workflow, task_spec, parent=None, state=TaskState.MAYBE, id=N state (`TaskState`): the state of this task (default MAYBE) id: an optional id (defaults to a random UUID) """ - assert workflow is not None - assert task_spec is not None - self.id = id or uuid4() workflow.tasks[self.id] = self self.workflow = workflow @@ -278,7 +275,6 @@ def _sync_children(self, task_specs, state=TaskState.MAYBE): def _child_added_notify(self, child): """Called by another task to let us know that a child was added.""" - assert child is not None self._children.append(child.id) def _drop_children(self, force=False): diff --git a/SpiffWorkflow/workflow.py b/SpiffWorkflow/workflow.py index d8d7d146..90f5c9ea 100644 --- a/SpiffWorkflow/workflow.py +++ b/SpiffWorkflow/workflow.py @@ -50,7 +50,6 @@ def __init__(self, workflow_spec, deserializing=False): workflow_spec (`WorkflowSpec`): the spec that describes this workflow deserializing (bool): whether this workflow is being deserialized """ - assert workflow_spec is not None self.spec = workflow_spec self.data = {} self.locks = {} diff --git a/tests/SpiffWorkflow/bpmn/serializer/VersionMigrationTest.py b/tests/SpiffWorkflow/bpmn/serializer/VersionMigrationTest.py index f8ffd909..1ebed64f 100644 --- a/tests/SpiffWorkflow/bpmn/serializer/VersionMigrationTest.py +++ b/tests/SpiffWorkflow/bpmn/serializer/VersionMigrationTest.py @@ -3,7 +3,7 @@ from SpiffWorkflow import TaskState from SpiffWorkflow.bpmn.script_engine import PythonScriptEngine, TaskDataEnvironment -from SpiffWorkflow.bpmn.serializer.migration.exceptions import VersionMigrationError +from SpiffWorkflow.bpmn.serializer.exceptions import VersionMigrationError from .BaseTestCase import BaseTestCase