diff --git a/lib/MongoDB/Role/_CommandMonitoring.pm b/lib/MongoDB/Role/_CommandMonitoring.pm index 55d43527..b1e63050 100644 --- a/lib/MongoDB/Role/_CommandMonitoring.pm +++ b/lib/MongoDB/Role/_CommandMonitoring.pm @@ -37,12 +37,17 @@ sub publish_command_started { return unless $self->monitoring_callback; $command = _to_tied_ixhash($command); + my $command_name = tied(%$command)->Keys(0); my $event = { type => 'command_started', databaseName => $self->db_name, - commandName => tied(%$command)->Keys(0), - command => $command, + commandName => $command_name, + command => ( + _needs_redaction($command_name) + ? _to_tied_ixhash([]) + : $command, + ), requestId => $request_id, connectionId => $link->address, }; @@ -79,7 +84,11 @@ sub publish_command_reply { requestId => $start_event->{requestId}, connectionId => $start_event->{connectionId}, durationSecs => $duration, - reply => $reply, + reply => ( + _needs_redaction($start_event->{commandName}) + ? {} + : $reply, + ), }; if ( $reply->{ok} ) { @@ -158,6 +167,22 @@ sub publish_legacy_query_error { return $self->publish_command_reply($reply); } +sub _needs_redaction { + my ($name) = @_; + return 1 if grep { $name eq $_ } qw( + authenticate + saslStart + saslContinue + getnonce + createUser + updateUser + copydbgetnonce + copydbsaslstart + copydb + ); + return 0; +} + sub _convert_legacy_insert { my ( $self, $op_doc ) = @_; $op_doc = [$op_doc] unless ref $op_doc eq 'ARRAY'; diff --git a/t/monitoring.t b/t/monitoring.t index a0f0235c..277e5241 100644 --- a/t/monitoring.t +++ b/t/monitoring.t @@ -253,6 +253,25 @@ subtest "exceptions are command_failed" => sub { }; }; +subtest 'redactions' => sub { + clear_events(); + my $mc = build_client( monitoring_callback => \&event_cb ); + my $testdb = get_test_db($mc); + + $testdb->run_command([getnonce => 1]); + my ($started, $succeeded) = + grep { $_->{commandName} eq 'getnonce' } + @events; + + is $started->{type}, 'command_started', 'start event'; + is $succeeded->{type}, 'command_succeeded', 'success event'; + + ok defined($started->{command}), 'command not empty'; + ok defined($succeeded->{reply}), 'reply not empty'; + is scalar(keys %{ $started->{command} }), 0, 'no command fields'; + is scalar(keys %{ $succeeded->{reply} }), 0, 'no reply fields'; +}; + sub _coll_with_monitor { my $mc = build_client( monitoring_callback => \&event_cb ); my $testdb = get_test_db($mc); diff --git a/t/monitoring_spec.t b/t/monitoring_spec.t index 12b53569..8d4243c6 100644 --- a/t/monitoring_spec.t +++ b/t/monitoring_spec.t @@ -17,7 +17,7 @@ use strict; use warnings; use Test::More 0.96; -use JSON::MaybeXS; +use JSON::MaybeXS qw( is_bool decode_json ); use Test::Deep; use Path::Tiny; use Try::Tiny; @@ -56,6 +56,19 @@ my $server_type = server_type($conn); my $feat_compat_ver = get_feature_compat_version($conn); my $coll = $testdb->get_collection('test_collection'); +# defines which argument hash fields become positional arguments +my %method_args = ( + insert_one => [qw( document )], + insert_many => [qw( documents )], + delete_one => [qw( filter )], + delete_many => [qw( filter )], + update_one => [qw( filter update )], + update_many => [qw( filter update )], + find => [qw( filter )], + count => [qw( filter )], + bulk_write => [qw( requests )], +); + my $dir = path("t/data/command-monitoring"); my $iterator = $dir->iterator( { recurse => 1 } ); while ( my $path = $iterator->() ) { @@ -69,17 +82,41 @@ while ( my $path = $iterator->() ) { subtest $name => sub { for my $test ( @{ $plan->{tests} } ) { - $coll->drop; - $coll->insert_many( $plan->{data} ); - clear_events(); - my $op = $test->{operation}; - my $meth = $op->{name}; - $meth =~ s{([A-Z])}{_\L$1}g; - my $test_meth = "test_$meth"; - plan skip_all => "not implemented" - unless main->can("$test_meth"); - my $res = main->$test_meth( $test->{description}, $meth, $op->{arguments}, - $test->{expectations} ); + subtest $test->{description} => sub { + + my $max_ver = $test->{ignore_if_server_version_greater_than}; + my $min_ver = $test->{ignore_if_server_version_less_than}; + my $ignore_topologies = $test->{ignore_if_topology_type}; + + plan skip_all => "Ignored for versions above $max_ver" + if defined $max_ver + and $server_version > version->parse("v$max_ver"); + plan skip_all => "Ignored for versions below $min_ver" + if defined $min_ver + and $server_version < version->parse("v$min_ver"); + + for my $topology (@{ $ignore_topologies || [] }) { + my %to_server_type = (sharded => 'Mongos'); + my $ignore_server_type = $to_server_type{$topology} + or die "Unknown topology type '$topology'"; + plan skip_all => "Ignored for '$topology' topology" + if $ignore_server_type eq $server_type; + } + + $coll->drop; + $coll->insert_many( $plan->{data} ); + clear_events(); + + my $op = $test->{operation}; + my $meth = $op->{name}; + $meth =~ s{([A-Z])}{_\L$1}g; + my $test_meth = "test_$meth"; + my $res = test_dispatch( + $meth, + $op->{arguments}, + $test->{expectations}, + ); + }; } }; } @@ -88,37 +125,281 @@ while ( my $path = $iterator->() ) { # generic tests #--------------------------------------------------------------------------# -sub test_find { - my ( $class, $label, $method, $args, $events ) = @_; - my $filter = delete $args->{filter}; - my $res = $coll->$method( grep { defined } $filter, $args ); - check_event_expectations( $label, $method, $events ); +# runs the collection method and dispatches event tests +sub test_dispatch { + my ($method, $args, $events) = @_; + + my @call_args = _adjust_arguments($method, $args); + my $res = eval { + my $res = $coll->$method(@call_args); + + # special case 'find' so commands are actually emitted + $res->all + if $method eq 'find'; + + $res; + }; + + my $err = $@; + note "error from '$method': $err" + if $err; + + check_event_expectations($method, _adjust_types($events)); +} + +# prepare collection method arguments +# adjusts data structures and extracts leading positional arguments +sub _adjust_arguments { + my ($method, $args) = @_; + + $args = _adjust_types($args); + my @fields = @{ $method_args{$method} }; + my @field_values = map { + my $val = delete $args->{$_}; + # bulk write is special cased to reuse argument extraction + ($method eq 'bulk_write' and $_ eq 'requests') + ? _adjust_bulk_write_requests($val) + : $val; + } @fields; + + return( + (grep { defined } @field_values), + scalar(keys %$args) ? $args : (), + ); +} + +# some type transformations +# currenetly only turns { '$numberLong' => $n } into 0+$n +sub _adjust_types { + my ($value) = @_; + if (ref $value eq 'HASH') { + if (scalar(keys %$value) == 1) { + my ($name, $value) = %$value; + if ($name eq '$numberLong') { + return 0+$value; + } + } + return +{map { + my $key = $_; + ($key, _adjust_types($value->{$key})); + } keys %$value}; + } + elsif (ref $value eq 'ARRAY') { + return [map { _adjust_types($_) } @$value]; + } + else { + return $value; + } +} + +# prepare bulk write requests for use as argument to ->bulk_write +sub _adjust_bulk_write_requests { + my ($requests) = @_; + return [map { + my ($name, $args) = %$_; + $name =~ s{([A-Z])}{_\L$1}g; + +{ $name => [_adjust_arguments($name, $args)] }; + } @$requests]; +} + +# common overrides for event data expectations +sub prepare_data_spec { + my ($spec) = @_; + if (not ref $spec) { + if ($spec eq 'test') { + return any(qw( test test_collection )); + } + if ($spec eq 'test-unacknowledged-bulk-write') { + return code(\&_verify_is_nonempty_str); + } + if ($spec eq 'command-monitoring-tests.test') { + return code(\&_verify_is_nonempty_str); + } + return $spec; + } + elsif (is_bool $spec) { + my $specced = $spec ? 1 : 0; + return code(sub { + my $value = shift; + return(0, 'expected a true boolean value') + if $specced and not $value; + return(0, 'expected a false boolean value') + if $value and not $specced; + return 1; + }); + } + elsif (ref $spec eq 'ARRAY') { + return [map { + prepare_data_spec($_) + } @$spec]; + } + elsif (ref $spec eq 'HASH') { + return +{map { + ($_, prepare_data_spec($spec->{$_})) + } keys %$spec}; + } + else { + return $spec; + } } sub check_event_expectations { - my ($label, $method, $expected) = @_; + my ($method, $expected) = @_; my @got = @events; - my $ok = 1; + for my $exp ( @$expected ) { - if (!@got ) { - $ok = 0; - last; + my ($exp_type, $exp_spec) = %$exp; + subtest $exp_type => sub { + ok(scalar(@got), 'event available') + or return; + my $event = shift @got; + is($event->{type}.'_event', $exp_type, "is a $exp_type") + or return; + my $event_tester = "check_$exp_type"; + main->can($event_tester)->($exp_spec, $event); + }; + } + + is scalar(@got), 0, 'no outstanding events'; +} + +sub check_event { + my ($exp, $event) = @_; + for my $key (sort keys %$exp) { + my $check = "check_${key}_field"; + main->can($check)->($exp->{$key}, $event); + } +} + +# +# per-event type test handlers +# + +sub check_command_started_event { + my ($exp, $event) = @_; + check_event($exp, $event); +} + +sub check_command_succeeded_event { + my ($exp, $event) = @_; + check_event($exp, $event); +} + +sub check_command_failed_event { + my ($exp, $event) = @_; + check_event($exp, $event); +} + +# +# verificationi subs for use with Test::Deep::code +# + +sub _verify_is_positive_num { + my $value = shift; + return(0, "error code is not defined") + unless defined $value; + return(0, "error code is not positive") + unless $value > 1; + return 1; +} + +sub _verify_is_nonempty_str { + my $value = shift; + return(0, "error message is not defined") + unless defined $value; + return(0, "error message is empty") + unless length $value; + return 1; +} + +# +# event field test handlers +# + +# $event.database_name +sub check_database_name_field { + my ($exp_name, $event) = @_; + ok defined($event->{databaseName}), "database_name defined"; + ok length($event->{databaseName}), "database_name non-empty"; +} + +# $event.command_name +sub check_command_name_field { + my ($exp_name, $event) = @_; + is $event->{commandName}, $exp_name, "command name"; +} + +# $event.reply +sub check_reply_field { + my ($exp_reply, $event) = @_; + my $event_reply = $event->{reply}; + + # special case for $event.reply.cursor.id + if (exists $exp_reply->{cursor}) { + if (exists $exp_reply->{cursor}{id}) { + $exp_reply->{cursor}{id} = code(\&_verify_is_positive_num) + if $exp_reply->{cursor}{id} eq '42'; } - if ( $got[0]->{type} ne $exp->{type} ) { - shift @got; - redo; + } + + # special case for $event.reply.writeErrors + if (exists $exp_reply->{writeErrors}) { + for my $error (@{ $exp_reply->{writeErrors} }) { + if (exists $error->{code} and $error->{code} eq 42) { + $error->{code} = code(\&_verify_is_positive_num); + } + if (exists $error->{errmsg} and $error->{errmsg} eq '') { + $error->{errmsg} = code(\&_verify_is_nonempty_str); + } } - ...; + } + + for my $exp_key (sort keys %$exp_reply) { + cmp_deeply + $event_reply->{$exp_key}, + prepare_data_spec($exp_reply->{$exp_key}), + "reply field $exp_key"; } } -sub _prep_to_ignore_special_data { - my ($hr) = @_; - if (exists $hr->{command} && exists $hr->{command}{cursor} ) { - $hr->{command}{cursor}{id} = ignore(); +# $event.command +sub check_command_field { + my ($exp_command, $event) = @_; + my $event_command = $event->{command}; + + # ordered defaults to true + delete $exp_command->{ordered}; + + # special case for $event.command.getMore + if (exists $exp_command->{getMore}) { + $exp_command->{getMore} = code(\&_verify_is_positive_num) + if $exp_command->{getMore} eq '42'; + } + + # special case for $event.command.writeConcern.wtimeout + if (exists $exp_command->{writeConcern}) { + $exp_command->{writeConcern}{wtimeout} = ignore(); } - if (exists $hr->{reply} && exists $hr->{reply}{cursor} ) { - $hr->{reply}{cursor}{id} = ignore(); + + for my $exp_key (sort keys %$exp_command) { + my $event_value = $event_command->{$exp_key}; + my $exp_value = prepare_data_spec($exp_command->{$exp_key}); + my $label = "command field '$exp_key'"; + + if ( + (grep { $exp_key eq $_ } qw( comment maxTimeMS writeConcern )) + or + ($event->{commandName} eq 'getMore' and $exp_key eq 'batchSize') + ) { + TODO: { + local $TODO = + "Command field '$exp_key' requires other fixes"; + cmp_deeply $event_value, $exp_value, $label; + } + } + else { + cmp_deeply $event_value, $exp_value, $label; + } } }