From c845849da0b98b1fad93be5aae40a9242ef23050 Mon Sep 17 00:00:00 2001 From: Colin Kuskie Date: Mon, 7 Dec 2009 13:07:46 -0800 Subject: [PATCH] Fix bug #11286: Tell the difference between end of Workflow, and unable to load WorkflowActivity. Tests are added for new methods. The changes to Workflow::Instance->run are peripherally tested in Workflow Activity tests. --- docs/changelog/7.x.x.txt | 1 + lib/WebGUI/Workflow.pm | 47 ++++++++++++++++++++++++++++++--- lib/WebGUI/Workflow/Instance.pm | 43 ++++++++++++++++++++++++------ t/Workflow.t | 24 ++++++++++++++++- t/Workflow/Instance.t | 9 ++----- 5 files changed, 105 insertions(+), 19 deletions(-) diff --git a/docs/changelog/7.x.x.txt b/docs/changelog/7.x.x.txt index a90db0522..8c2edff3b 100644 --- a/docs/changelog/7.x.x.txt +++ b/docs/changelog/7.x.x.txt @@ -1,6 +1,7 @@ 7.8.8 - fixed #11289: Gallery with pending version tag causes search engine indexer to puke. - fixed #11292: Search function limited to onje search? + - fixed #11286: Workflow Instance deleted when reaching an Activity that fails to load 7.8.7 - fixed #11278: Wrong test for Template::Toolkit in testEnvironment.pl diff --git a/lib/WebGUI/Workflow.pm b/lib/WebGUI/Workflow.pm index 36003eab2..4232fdf63 100644 --- a/lib/WebGUI/Workflow.pm +++ b/lib/WebGUI/Workflow.pm @@ -314,7 +314,8 @@ sub getList { =head2 getNextActivity ( [ activityId ] ) -Returns the next activity in the workflow after the activity specified. If no activity id is specified, then the first workflow will be returned. +Returns the next activity in the workflow after the activity specified. +If no activity id is specified, then the first workflow will be returned. =head3 activityId @@ -323,13 +324,53 @@ The unique id of an activity in this workflow. =cut sub getNextActivity { - my $self = shift; + my $self = shift; + my $activityId = shift; + my $id = $self->getNextActivityId($activityId); + return $self->getActivity($id); +} + + +#------------------------------------------------------------------- + +=head2 getNextActivityId ( [ activityId ] ) + +Returns the ID of the next activity in the workflow after the activity specified. +If no activity id is specified, then the first workflow will be returned. + +=head3 activityId + +The unique id of an activity in this workflow. + +=cut + +sub getNextActivityId { + my $self = shift; my $activityId = shift; my ($sequenceNumber) = $self->session->db->quickArray("select sequenceNumber from WorkflowActivity where activityId=?", [$activityId]); $sequenceNumber++; my ($id) = $self->session->db->quickArray("select activityId from WorkflowActivity where workflowId=? and sequenceNumber>=? order by sequenceNumber", [$self->getId, $sequenceNumber]); - return $self->getActivity($id); + return $id; +} + + +#------------------------------------------------------------------- + +=head2 hasNextActivity ( [ activityId ] ) + +Returns true if there is an activity after the specified activity. +If no activity id is specified, then the first workflow will be returned. + +=head3 activityId + +The unique id of an activity in this workflow. + +=cut + +sub hasNextActivity { + my $self = shift; + return $self->getNextActivityId(@_) ? 1 : 0; } diff --git a/lib/WebGUI/Workflow/Instance.pm b/lib/WebGUI/Workflow/Instance.pm index a53b99db3..a0df11343 100644 --- a/lib/WebGUI/Workflow/Instance.pm +++ b/lib/WebGUI/Workflow/Instance.pm @@ -260,6 +260,21 @@ sub getWorkflow { #------------------------------------------------------------------- +=head2 hasNextActivity ( ) + +Returns true if the instance has a workflow activity after the current one. + +=cut + +sub hasNextActivity { + my $self = shift; + my $workflow = $self->getWorkflow; + return undef unless defined $workflow; + return $workflow->hasNextActivity($self->get("currentActivityId")); +} + +#------------------------------------------------------------------- + =head2 new ( session, instanceId, [isNew] ) Constructor. @@ -315,8 +330,9 @@ B>NOTE:> You should normally never run this method. The workflow engine will use =cut sub run { - my $self = shift; + my $self = shift; my $workflow = $self->getWorkflow; + my $session = $self->session; unless (defined $workflow) { $self->set({lastStatus=>"undefined"}, 1); return "undefined"; @@ -326,7 +342,7 @@ sub run { return "disabled"; } if ($workflow->isSerial) { - my ($firstId) = $self->session->db->quickArray( + my ($firstId) = $session->db->quickArray( "select instanceId from WorkflowInstance where workflowId=? order by runningSince", [$workflow->getId] ); @@ -335,22 +351,33 @@ sub run { return "waiting"; } } + ##Undef if returned if there is an error, or if there is not a next activity. + ##Use hasNextActivity to tell the difference and handle the cases differently. + if (! $self->hasNextActivity) { + $self->delete(1); + return "done"; + } my $activity = $self->getNextActivity; unless (defined $activity) { - $self->delete(1); - return "done"; + $session->errorHandler->error( + sprintf q{Unable to load Workflow Activity for activity after id %s in workflow %s}, + $self->get('currentActivityId'), + $workflow->getId + ); + $self->set({lastStatus=>"error"}, 1); + return "error"; } - $self->session->errorHandler->info("Running workflow activity ".$activity->getId.", which is a ".(ref $activity).", for instance ".$self->getId."."); + $session->errorHandler->info("Running workflow activity ".$activity->getId.", which is a ".(ref $activity).", for instance ".$self->getId."."); my $object = eval { $self->getObject }; if ( my $e = WebGUI::Error::ObjectNotFound->caught ) { - $self->session->log->warn( + $session->log->warn( q{The object for this workflow does not exist. Type: } . $self->get('className') . q{, ID: } . $e->id ); $self->delete(1); return "done"; } elsif ($@) { - $self->session->errorHandler->error( + $session->errorHandler->error( q{Error on workflow instance '} . $self->getId . q{': }. $@ ); $self->set({lastStatus=>"error"}, 1); @@ -359,7 +386,7 @@ sub run { my $status = eval { $activity->execute($object, $self) }; if ($@) { - $self->session->errorHandler->error("Caught exception executing workflow activity ".$activity->getId." for instance ".$self->getId." which reported ".$@); + $session->errorHandler->error("Caught exception executing workflow activity ".$activity->getId." for instance ".$self->getId." which reported ".$@); $self->set({lastStatus=>"error"}, 1); return "error"; } diff --git a/t/Workflow.t b/t/Workflow.t index eabf3767f..dd75e5d47 100644 --- a/t/Workflow.t +++ b/t/Workflow.t @@ -16,7 +16,7 @@ use WebGUI::Session; use WebGUI::Workflow; use WebGUI::Workflow::Cron; use WebGUI::Utility qw/isIn/; -use Test::More tests => 67; # increment this value for each test you create +use Test::More tests => 75; # increment this value for each test you create use Test::Deep; my $session = WebGUI::Test->session; @@ -144,6 +144,28 @@ my $decayKarma = $wf3->addActivity('WebGUI::Workflow::Activity::DecayKarma'); my $cleanTemp = $wf3->addActivity('WebGUI::Workflow::Activity::CleanTempStorage'); my $oldTrash = $wf3->addActivity('WebGUI::Workflow::Activity::PurgeOldTrash'); +##################################################################### +# +# getNextActivityId +# +##################################################################### + +is $wf3->getNextActivityId($decayKarma->getId), $cleanTemp->getId, 'getNextActivityId: first activity to second'; +is $wf3->getNextActivityId($cleanTemp->getId), $oldTrash->getId, '... second activity to third'; +is $wf3->getNextActivityId($oldTrash->getId), undef, '... last returns undef'; +is $wf3->getNextActivityId(), $decayKarma->getId, '... with no activityId, returns the first'; + +##################################################################### +# +# hasNextActivity +# +##################################################################### + +ok $wf3->hasNextActivity($decayKarma->getId), 'hasNextActivityId: first activity to second'; +ok $wf3->hasNextActivity($cleanTemp->getId), '... second activity to third'; +ok $wf3->hasNextActivity(), '... with no activityId, returns the first'; +ok !$wf3->hasNextActivity($oldTrash->getId), '... last returns undef'; + ##################################################################### # # Activity tests, promote, demote, reorder, accessing, deleting diff --git a/t/Workflow/Instance.t b/t/Workflow/Instance.t index 11876dd46..e4a2928b3 100644 --- a/t/Workflow/Instance.t +++ b/t/Workflow/Instance.t @@ -69,6 +69,7 @@ my $wf = WebGUI::Workflow->create( } ); isa_ok($wf, 'WebGUI::Workflow', 'workflow created for test'); +addToCleanup($wf); # create an instance of $wfId my $properties = { @@ -179,6 +180,7 @@ my $wf2 = WebGUI::Workflow->create( type => 'None', } ); +addToCleanup($wf2); my $wf2Instance = WebGUI::Workflow::Instance->create($session, {workflowId => $wf2->getId}); cmp_deeply($wf2Instance->get('parameters'), {}, 'get returns {} for parameters when there are no parameters stored'); @@ -222,10 +224,3 @@ cmp_deeply($wf2Instance->get('parameters'), {}, 'get returns {} for parameters w is $wf3Instance->getObject, $return; } 'getObject is able to retrieve correct object'; } - -#---------------------------------------------------------------------------- -# Cleanup -END { - $wf->delete; ##Deleting a Workflow deletes its instances, too. - $wf2->delete; -}