Permalink
Browse files

Fixed a massive problem about colliding plugins. Made a duplicate check.

  • Loading branch information...
1 parent effc745 commit a73ce18c2dcf251d39d4c5415280307d58cb722f @Getty Getty committed Mar 5, 2013
View
65 lib/DDG/Block.pm
@@ -4,6 +4,7 @@ package DDG::Block;
use Moo::Role;
use Carp;
use Class::Load ':all';
+use POSIX qw(strftime);
requires qw(
request
@@ -79,10 +80,12 @@ has plugins => (
sub _build_plugins { die (ref shift)." requires plugins" }
-=attr return_one
+=attr allow_missing_plugins
-This attribute defines if the block should stop if there is a hit which gives
-a result. By default this is on.
+This attribute defines if the block should die on missing plugins, or if he
+should just go on. If given a CODEREF, then this CODEREF will get executed
+with the block as first parameter, and the missing class name as second. By
+default this is disabled.
=cut
@@ -92,12 +95,10 @@ has allow_missing_plugins => (
default => sub { 0 },
);
-=attr allow_missing_plugins
+=attr return_one
-This attribute defines if the block should die on missing plugins, or if he
-should just go on. If given a CODEREF, then this CODEREF will get executed
-with the block as first parameter, and the missing class name as second. By
-default this is disabled.
+This attribute defines if the block should stop if there is a hit which gives
+a result. By default this is on.
=cut
@@ -107,6 +108,35 @@ has return_one => (
default => sub { 1 },
);
+=attr allow_duplicate
+
+This attribute defines if the block is only allowing one instance of every
+plugin be called once, even if they hit cause of several cases. By default
+this is off, and should stay off.
+
+=cut
+
+has allow_duplicate => (
+ #isa => 'Bool',
+ is => 'ro',
+ default => sub { 0 },
+);
+
+=attr debug_trace
+
+=cut
+
+has debug_trace => (
+ #isa => 'Bool',
+ is => 'ro',
+ default => sub { defined $ENV{DDG_BLOCK_TRACE} && $ENV{DDG_BLOCK_TRACE} ? 1 : 0 },
+);
+
+sub trace {
+ return unless shift->debug_trace;
+ print STDERR ('[DDG_BLOCK_TRACE] ',join(" ",@_),"\n");
+}
+
=attr before_build
A coderef that is executed before the build of the plugins. It gets the block
@@ -251,12 +281,29 @@ sub parse_trigger { shift; shift; }
=method empty_trigger
-Ggets called, if the plugin doesnt deliver any trigger, here you can wrap this to your own specific
+Gets called, if the plugin doesnt deliver any trigger, here you can wrap this to your own specific
definition. Its so far only used in the L<DDG::Block::Words>, to disallow empty triggers totally. By default
it returns B<undef>.
=cut
sub empty_trigger { return undef }
+=method handle_request_matches
+
+This function is used for calling I<handle_request_matches> on the plugin,
+which is implemented there via L<DDG::Meta::RequestHandler>.
+
+=cut
+
+sub handle_request_matches {
+ my ( $self, $plugin, $request, @args ) = @_;
+ my $plugin_class = ref $plugin;
+ unless ($self->allow_duplicate) {
+ return () if grep { $_ eq $plugin_class } @{$request->seen_plugins};
+ }
+ push @{$request->seen_plugins}, $plugin_class;
+ return $plugin->handle_request_matches($request, @args);
+}
+
1;
View
82 lib/DDG/Block/Words.pm
@@ -7,7 +7,7 @@ with qw( DDG::Block );
sub BUILD {
my ( $self ) = @_;
- for (reverse @{$self->plugin_objs}) {
+ for (@{$self->plugin_objs}) {
my $triggers = $_->[0];
my $plugin = $_->[1];
for (@{$triggers}) {
@@ -63,10 +63,12 @@ sub _set_word_plugin {
my $word_count = scalar @split_word;
$self->_words_plugins->{$type}->{$key} = {} unless defined $self->_words_plugins->{$type}->{$key};
if ($word_count eq 1) {
- $self->_words_plugins->{$type}->{$key}->{1} = $plugin;
+ $self->_words_plugins->{$type}->{$key}->{1} = [] unless defined $self->_words_plugins->{$type}->{$key}->{$word_count};
+ push @{$self->_words_plugins->{$type}->{$key}->{1}}, $plugin;
} else {
$self->_words_plugins->{$type}->{$key}->{$word_count} = {} unless defined $self->_words_plugins->{$type}->{$key}->{$word_count};
- $self->_words_plugins->{$type}->{$key}->{$word_count}->{$word} = $plugin;
+ $self->_words_plugins->{$type}->{$key}->{$word_count}->{$word} = [] unless defined $self->_words_plugins->{$type}->{$key}->{$word_count}->{$word};
+ push @{$self->_words_plugins->{$type}->{$key}->{$word_count}->{$word}}, $plugin;
}
}
@@ -98,13 +100,46 @@ sub _build__words_plugins {{
sub request {
my ( $self, $request ) = @_;
my @results;
+ $self->trace( "Query raw: ", "'".$request->query_raw."'" );
+ #
+ # Mapping positions of keywords in the request
+ # to a flat array which we can access stepwise.
+ #
+ # So @poses is an array of the positions inside
+ # the triggers hash.
+ #
+ ################################################
my %triggers = %{$request->triggers};
my $max = scalar keys %triggers;
my @poses = sort { $a <=> $b } keys %triggers;
+ $self->trace( "Trigger word positions: ", @poses );
for my $cnt (0..$max-1) {
+ #
+ # We do split up this into a flat array to have it
+ # easier to determine if the query is starting, ending
+ # or still in the beginning, this is very essential
+ # for the following steps.
+ #
my $start = $cnt == 0 ? 1 : 0;
my $end = $cnt == $max-1 ? 1 : 0;
for my $word (@{$request->triggers->{$poses[$cnt]}}) {
+ $self->trace( "Testing word:", "'".$word."'" );
+ #
+ # Checking if any of the plugins have this specific word
+ # in the start end or any trigger. start and end of course
+ # only if its first or last word in the query.
+ #
+ # It gives back a touple of 2 elements, a bool which defines
+ # if there COULD BE more words after it (so this fits for
+ # any and start triggers), the second is the part of the
+ # prepared trigger set of the blocks which is responsible
+ # for this word.
+ #
+ # The keys inside the hitstruct define the words count it
+ # additional carries. This allows to kick out the ones which
+ # are not fitting anymore into the length of the query (by
+ # wordcount)
+ #
if (my ( $begin, $hitstruct ) =
$start && defined $self->_words_plugins->{start}->{$word}
? ( 1 => $self->_words_plugins->{start}->{$word} )
@@ -113,12 +148,31 @@ sub request {
: defined $self->_words_plugins->{any}->{$word}
? ( 1 => $self->_words_plugins->{any}->{$word} )
: undef) {
+ ######################################################
+ $self->trace("Got a hit with","'".$word."'","!", $begin ? "And it's just the beginning..." : "");
+ #
+ # $cnt is the specific position inside our flat array of
+ # positions inside the query.
+ #
my $pos = $poses[$cnt];
+ #
+ # This for loop is only executed if for the specific word
+ # that is triggered is having "more then one word" triggers
+ # that are attached to it. In this case it iterates through
+ # all those different combination and tries to match it
+ # with the request of the query.
+ #
for my $word_count (sort { $b <=> $a } grep { $_ > 1 } keys %{$hitstruct}) {
+ ############################################################
+ $self->trace( "Checking additional multiword triggers with length of", $word_count);
my @sofar_words = @{$triggers{$pos}};
- for (@sofar_words) {
- push @results, $hitstruct->{$word_count}->{$_}->handle_request_matches($request,$pos) if defined $hitstruct->{$word_count}->{$_};
- return @results if $self->return_one && @results;
+ for my $sofar_word (@sofar_words) {
+ if (defined $hitstruct->{$word_count}->{$sofar_word}) {
+ for (@{$hitstruct->{$word_count}->{$sofar_word}}) {
+ push @results, $self->handle_request_matches($_,$request,$pos);
+ return @results if $self->return_one && @results;
+ }
+ }
}
my @next_poses_key = grep { $_ >= 0 } $begin ? ($cnt+1)..($cnt+$word_count-1) : ($cnt-$word_count-1)..($cnt-1);
my @next_poses = grep { defined $_ && defined $triggers{$_} } @poses[@next_poses_key];
@@ -131,16 +185,24 @@ sub request {
my $new_next_word = $begin
? join(" ",$current_sofar_word,$next_trigger)
: join(" ",$next_trigger,$current_sofar_word);
- push @results, $hitstruct->{$word_count}->{$new_next_word}->handle_request_matches($request,( $pos < $next_pos ) ? ( $pos,$next_pos ) : ( $next_pos,$pos ) ) if defined $hitstruct->{$word_count}->{$new_next_word};
- return @results if $self->return_one && @results;
+ if (defined $hitstruct->{$word_count}->{$new_next_word}) {
+ for (@{$hitstruct->{$word_count}->{$new_next_word}}) {
+ push @results, $self->handle_request_matches($_,$request,( $pos < $next_pos ) ? ( $pos,$next_pos ) : ( $next_pos,$pos ));
+ return @results if $self->return_one && @results;
+ }
+ }
push @new_next_words, $new_next_word;
}
}
push @sofar_words, @new_next_words;
}
}
- push @results, $hitstruct->{1}->handle_request_matches($request,$poses[$cnt]) if defined $hitstruct->{1};
- return @results if $self->return_one && @results;
+ if (defined $hitstruct->{1}) {
+ push @results, $self->handle_request_matches($_,$request,$poses[$cnt]) for @{$hitstruct->{1}};
+ return @results if $self->return_one && @results;
+ }
+ } else {
+ $self->trace("No hit with","'".$word."'");
}
}
}
View
7 lib/DDG/Manual/Translation.pod
@@ -1,8 +1,7 @@
-=encoding utf-8
-
-=head1 NAME
+# PODNAME: DDG::Manual::Translation
+# ABSTRACT: Overview of the translation system of DuckDuckGo
-DDG::Manual::Translation - Overview of the translation system of DuckDuckGo
+=encoding utf-8
=head1 THE MISSION
View
15 lib/DDG/Request.pm
@@ -350,6 +350,21 @@ has wordcount => (
);
sub _build_wordcount { scalar @{shift->words} }
+=attr seen_plugins
+
+This array contains all the plugins which already worked with this request.
+This means all the plugins which are triggered. If they gave back a result or
+not, doesn't matter here. This list is used by L<DDG::Block/allow_duplicate>.
+
+=cut
+
+has seen_plugins => (
+ is => 'rw',
+ lazy => 1,
+ builder => '_build_seen_plugins',
+);
+sub _build_seen_plugins {[]}
+
#
# LANGUAGE / LOCATION / IP
#
View
13 t/35-block.t
@@ -57,6 +57,8 @@ BEGIN {
DDGTest::Goodie::WoBlockTwo
DDGTest::Goodie::WoBlockThree
DDGTest::Goodie::WoBlockArr
+ DDGTest::Goodie::CollideOne
+ DDGTest::Goodie::CollideTwo
)];
my $before_wp = 0;
my $after_wp = 0;
@@ -104,7 +106,7 @@ BEGIN {
my @queries = (
'aROUNd two' => {
- wo => [zci('two','woblockone')],
+ wo => [zci('two','woblockone'),zci('aROUNd','woblocktwo')],
re => [],
},
'wikipedia blub' => {
@@ -171,11 +173,14 @@ BEGIN {
wo => [zci('a|or|b|or|c','woblockarr')],
re => [],
},
-
+ 'collide' => {
+ wo => [zci('collide','collideone'),zci('collide','collidetwo')],
+ re => [],
+ },
# Triggers multiple plugins.
'or two' => {
- wo => [zci('or|two','woblockarr'), zci('or','woblocktwo')],
- re => [],
+ wo => [zci('or|two','woblockarr'), zci('or','woblocktwo')],
+ re => [],
},
);
View
9 t/lib/DDGTest/Goodie/CollideOne.pm
@@ -0,0 +1,9 @@
+package DDGTest::Goodie::CollideOne;
+
+use DDG::Goodie;
+
+triggers any => 'collide';
+
+handle query_parts => sub { join('|',@_) };
+
+1;
View
9 t/lib/DDGTest/Goodie/CollideTwo.pm
@@ -0,0 +1,9 @@
+package DDGTest::Goodie::CollideTwo;
+
+use DDG::Goodie;
+
+triggers any => 'collide';
+
+handle query_parts => sub { join('|',@_) };
+
+1;

0 comments on commit a73ce18

Please sign in to comment.