[Bps-public-commit] rt-extension-rest2 branch, add-validation-hook-for-status-updates-2, created. 1.09-13-gba9e209

Dianne Skoll dianne at bestpractical.com
Fri Jan 15 17:07:16 EST 2021


The branch, add-validation-hook-for-status-updates-2 has been created
        at  ba9e20908d0f5d1003006c6f3b5e2e91be0b1ed4 (commit)

- Log -----------------------------------------------------------------
commit 884acf270d943752f61a596e0d6a8618a2563ec3
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Mon Jan 4 14:23:07 2021 -0500

    Add system for maintaining validation hooks to RT::Extension::REST2

diff --git a/lib/RT/Extension/REST2.pm b/lib/RT/Extension/REST2.pm
index e11acfb..f5eeaaf 100644
--- a/lib/RT/Extension/REST2.pm
+++ b/lib/RT/Extension/REST2.pm
@@ -1250,6 +1250,53 @@ sub CleanupRequest {
             'DBIx::SearchBuilder::Record::Cachable' => 'FlushCache' ) );
 }
 
+# This hash holds all registered validation hooks.
+our $ValidationHooks = {};
+
+# Acceptable validation hook types
+my $ValidationHookTypes = { update => 1, create => 1 };
+
+# Acceptable validation hook objects
+my $ValidationHookObjects = { 'RT::Ticket' => 1 };
+
+# Register a validation hook for the given update type and object type
+# Returns ($ok, $msg).  On success, $ok will be 1 and $msg irrelevant;
+# on failure, $ok will be 0 and $msg will indicate the nature of
+# the error.
+sub add_validation_hook
+{
+    my ($class, $type, $object, $coderef) = @_;
+
+    # Allow caller to pass in an object or a name like RT::Ticket;
+    $object = ref($object) if ref($object);
+
+    unless (exists($ValidationHookTypes->{$type})) {
+        return(0, "$type is not a valid validation-hook type");
+    }
+    unless (exists($ValidationHookObjects->{$object})) {
+        return (0, "$object is not a valid validation-hook object");
+    }
+
+    push(@{$ValidationHooks->{$type}->{$object}}, $coderef);
+    return (1, '');
+}
+
+# Call all of the validation hooks for the given update type
+# and object type.  Returns (1, "") if they all pass, or (0, "msg") if
+# one of them fails.  Each validation hook is expected to return an
+# ($ok, "msg") indicator.
+sub call_validation_hooks
+{
+    my $class = shift;
+    my $type = shift;
+    my $object = shift;
+    foreach my $hook (@{$ValidationHooks->{$type}->{$object}}) {
+        my ($ok, $msg) = $hook->(@_);
+        return (0, $msg) unless $ok;
+    }
+    return (1, "");
+}
+
 =head1 AUTHOR
 
 Best Practical Solutions, LLC <modules at bestpractical.com>

commit e0aad849ee74491cd49bda32210ba4e14e5e505a
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Mon Jan 4 14:34:40 2021 -0500

    Add validation hooks for ticket update/create/correspond

diff --git a/lib/RT/Extension/REST2/Resource/Message.pm b/lib/RT/Extension/REST2/Resource/Message.pm
index f15d996..edd5c9e 100644
--- a/lib/RT/Extension/REST2/Resource/Message.pm
+++ b/lib/RT/Extension/REST2/Resource/Message.pm
@@ -116,6 +116,12 @@ sub add_message {
         Subject   => $args{Subject},
     );
 
+    # Call validation hooks
+    my ($ok, $errmsg) = RT::Extension::REST2->call_validation_hooks('update', 'RT::Ticket', $self->record, \%args);
+    if (!$ok) {
+        return (\400, $errmsg);
+    }
+
     # Process attachments
     foreach my $attachment (@{$args{Attachments}}) {
         $MIME->attach(
diff --git a/lib/RT/Extension/REST2/Resource/Ticket.pm b/lib/RT/Extension/REST2/Resource/Ticket.pm
index f8cf37a..1b5ad2b 100644
--- a/lib/RT/Extension/REST2/Resource/Ticket.pm
+++ b/lib/RT/Extension/REST2/Resource/Ticket.pm
@@ -12,7 +12,8 @@ with (
         => { -alias => { hypermedia_links => '_default_hypermedia_links' } },
     'RT::Extension::REST2::Resource::Record::Deletable',
     'RT::Extension::REST2::Resource::Record::Writable'
-        => { -alias => { create_record => '_create_record' } },
+        => { -alias => { create_record => '_create_record',
+                         update_record => '_update_record'} },
 );
 
 sub dispatch_rules {
@@ -52,10 +53,29 @@ sub create_record {
         );
     }
 
-    my ($ok, $txn, $msg) = $self->_create_record($data);
+    my ($ok, $txn, $msg);
+
+    # Call validation hooks and return failure if any fail
+    ($ok, $msg) = RT::Extension::REST2->call_validation_hooks('create', 'RT::Ticket', $queue, $data);
+    if (!$ok) {
+        return (\400, $msg);
+    }
+
+    ($ok, $txn, $msg) = $self->_create_record($data);
     return ($ok, $msg);
 }
 
+sub update_record
+{
+    my ($self, $data) = @_;
+
+    my ($ok, $msg) = RT::Extension::REST2->call_validation_hooks('update', 'RT::Ticket', $self->record, $data);
+    if (!$ok) {
+        return (0, $msg);
+    }
+    return $self->_update_record($data);
+}
+
 sub forbidden {
     my $self = shift;
     return 0 unless $self->record->id;

commit 25b4ca9a76b5e1a467442a2b5bcdd8f56a83cbdd
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Mon Jan 4 14:47:05 2021 -0500

    Add unit test to exercise ticket update/create validation hooks

diff --git a/xt/ticket_validate.t b/xt/ticket_validate.t
new file mode 100644
index 0000000..2af3cad
--- /dev/null
+++ b/xt/ticket_validate.t
@@ -0,0 +1,104 @@
+use strict;
+use warnings;
+use RT::Extension::REST2::Test tests => undef;
+use Test::Deep;
+my $mech = RT::Extension::REST2::Test->mech;
+
+my $auth = RT::Extension::REST2::Test->authorization_header;
+my $rest_base_path = '/REST/2.0';
+my $user = RT::Extension::REST2::Test->user;
+
+sub my_create_validator
+{
+    my ($queue, $data) = @_;
+    if ($data->{VALIDATE} && $data->{VALIDATE} eq 'BAD_CREATE!') {
+        return (0, 'Bad data');
+    }
+    return (1, '');
+}
+
+sub my_update_validator
+{
+    my ($ticket, $data) = @_;
+    if ( $data->{'VALIDATE'} and $data->{'VALIDATE'} eq 'BAD_UPDATE!' ) {
+        return (0, 'Bad data');
+    }
+
+    return (1, '');
+}
+
+my ($ok, $msg);
+($ok, $msg) = RT::Extension::REST2->add_validation_hook('create', 'RT::Ticket', sub { return my_create_validator(@_); });
+ok($ok, 'Successfully added ticket-create validation hook');
+($ok, $msg) = RT::Extension::REST2->add_validation_hook('update', 'RT::Ticket', sub { return my_update_validator(@_); });
+ok($ok, 'Successfully added ticket-update validation hook');
+
+($ok, $msg) = RT::Extension::REST2->add_validation_hook('update', 'RT::NOPE', sub { return my_update_validator(@_); });
+ok(!$ok, 'Correctly failed to add validation hook for unknown object type');
+is($msg, 'RT::NOPE is not a valid validation-hook object', '... and failed with the expected error message');
+
+($ok, $msg) = RT::Extension::REST2->add_validation_hook('migrate', 'RT::Ticket', sub { return my_update_validator(@_); });
+ok(!$ok, 'Correctly failed to add validation hook for unknown modification type');
+is($msg, 'migrate is not a valid validation-hook type', '... and failed with the expected error message');
+
+diag 'Check validation on create';
+my ($ticket_url, $ticket_id);
+{
+    $user->PrincipalObj->GrantRight( Right => 'CreateTicket' );
+    my $payload = {
+        Subject => 'Ticket creation using REST',
+        Queue   => 'General',
+        Content => 'Testing ticket creation using REST API.',
+        VALIDATE => 'BAD_CREATE!',
+    };
+
+    my $res = $mech->post_json("$rest_base_path/ticket",
+        $payload,
+        'Authorization' => $auth,
+    );
+    is($res->code, 400, 'Validation returned 400');
+
+    # Do a successful create for later test
+    $payload = {
+        Subject => 'Ticket creation using REST',
+        Queue   => 'General',
+        Content => 'Testing ticket creation using REST API.',
+    };
+
+    $res = $mech->post_json("$rest_base_path/ticket",
+        $payload,
+        'Authorization' => $auth,
+    );
+    is($res->code, 201);
+    ok($ticket_url = $res->header('location'));
+    ok(($ticket_id) = $ticket_url =~ qr[/ticket/(\d+)]);
+}
+
+# Ticket Update
+{
+    {
+        no warnings;
+        *RT::Extension::REST2::Resource::Ticket::validate_hook_before_create = *RT::Extension::REST2::TestValidate::validate_hook_before_create;
+        *RT::Extension::REST2::Resource::Ticket::validate_hook_before_update = *RT::Extension::REST2::TestValidate::validate_hook_before_update;
+    }
+
+    my $payload = {
+        Subject  => 'Ticket update using REST',
+        Priority => 42,
+        VALIDATE => 'BAD_UPDATE!',
+    };
+
+    $user->PrincipalObj->GrantRight( Right => 'ShowTicket' );
+    $user->PrincipalObj->GrantRight( Right => 'ModifyTicket' );
+
+    my $res = $mech->put_json($ticket_url,
+        $payload,
+        'Authorization' => $auth,
+        );
+    is($res->code, 200, 'Validation returned 200');
+    cmp_deeply($mech->json_response, [0, 'Bad data'], "Validation error is indicated by JSON response");
+
+}
+
+
+done_testing;

commit ba9e20908d0f5d1003006c6f3b5e2e91be0b1ed4
Author: Dianne Skoll <dianne at bestpractical.com>
Date:   Fri Jan 15 17:06:28 2021 -0500

    Document the validation hook machinery

diff --git a/lib/RT/Extension/REST2.pm b/lib/RT/Extension/REST2.pm
index f5eeaaf..2823ffd 100644
--- a/lib/RT/Extension/REST2.pm
+++ b/lib/RT/Extension/REST2.pm
@@ -1190,8 +1190,48 @@ that problem.
 The REST API uses the full range of HTTP status codes, and your client should
 handle them appropriately.
 
+=head2 Validation hooks
+
+The REST2 code permits you to add hooks to perform extra validation before
+a create or update call is allowed.  Currently, you can add validation hooks
+for ticket creation or updates.  (These hooks could be added from
+another RT extension, for example.)
+
+To add a validation hook, call
+
+    RT::Extension::REST2->add_validation_hook($type, $objtype, $coderef);
+
+In the above call, $type is the type of action being performed; currently
+it must be one of 'create' or 'update.  $objtype is the type of object
+on which the action is being performed; currently, only 'RT::Ticket'
+is supported.
+
+$coderef is a code reference that will be called by the validation
+machinery.  It should return a two-element list of the form ($ok, $msg).
+If validation succeeds, $ok must be 1 and $msg will be ignored.  If
+validation fails, $ok must be 0 and $msg should be a descriptive
+error message.
+
+The arguments passed to the validation hooks are as follows:
+
+For 'update', 'RT::Ticket', the validation code is passed two
+arguments: the first is the RT::Ticket object that is being
+updated and the second is the $data hash that was passed in
+to the update API call.
+
+For 'create', 'RT::Ticket', the validation code is passed
+two arguments: the first is the RT::Queue object in which
+the ticket is being created and the second is the $data
+hash that was passed in to the create API call.
+
+If multiple validation hooks are defined for the same
+$type and $objtype, then they are called in the order
+they were added.  If any hook returns failure, then
+validation is considered to have failed.
+
 =cut
 
+
 # XXX TODO: API doc
 
 sub to_psgi_app {

-----------------------------------------------------------------------


More information about the Bps-public-commit mailing list