fu: Close-up of Fu, bringing a scoop of water to her mouth (Default)
fu ([personal profile] fu) wrote in [site community profile] changelog2011-12-08 10:03 am

[dw-free] Site search retains deleted content

[commit: http://hg.dwscoalition.org/dw-free/rev/0363ba0c8e9a]

http://bugs.dwscoalition.org/show_bug.cgi?id=4073

Adds a filter by is_deleted, to indicate when the journal is not visible.
This bit is changed when the user deletes/undeletes or is
suspended/unsuspended. It's only recognized after the indexer has been
rerun, though, so also add an additional check to the output in /search.bml
(which is uglier, but reveals less info). Then, also make sure we delete
from posts_raw when the user is purged -- the entries don't show up anyway
because the user was previously deleted, but this cleans up.

Patch by [personal profile] fu.

Files modified:
  • bin/worker/sphinx-search-gm
  • cgi-bin/DW/Hooks/SiteSearch.pm
  • cgi-bin/LJ/User.pm
--------------------------------------------------------------------------------
diff -r 3ea6b65d559c -r 0363ba0c8e9a bin/worker/sphinx-search-gm
--- a/bin/worker/sphinx-search-gm	Thu Dec 08 01:02:37 2011 -0600
+++ b/bin/worker/sphinx-search-gm	Thu Dec 08 18:02:35 2011 +0800
@@ -56,6 +56,10 @@
     $sx->SetFilter( 'allow_global_search', [ 1 ] )
         unless $args->{userid};
 
+    # we don't want items marked deleted (user is not visible)
+    $sx->SetFilter( 'is_deleted', [ 0 ] );
+
+
     # security filtering is a dangerous game.  basically, the caller tells us whether to
     # ignore security or gives us a mask of bits to work with.  from that we intuit what
     # security options to enable on our filters to Sphinx.
@@ -84,9 +88,10 @@
         # convenience only... they're the same hashrefs you know and love
         my @out;
 
+        my $remote = LJ::get_remote();
         foreach my $match ( @{ $res->{matches} } ) {
             my $entry = LJ::Entry->new( $match->{journal_id}, jitemid => $match->{jitemid} );
-            if ( $entry && $entry->valid ) {
+            if ( $entry && $entry->visible_to( $remote ) ) {
                 # use text only version of event for excerpt purposes.  best effort.
                 $match->{entry} = $entry->event_text;
                 $match->{entry} =~ s#<(?:br|p)\s*/?># #gi;
diff -r 3ea6b65d559c -r 0363ba0c8e9a cgi-bin/DW/Hooks/SiteSearch.pm
--- a/cgi-bin/DW/Hooks/SiteSearch.pm	Thu Dec 08 01:02:37 2011 -0600
+++ b/cgi-bin/DW/Hooks/SiteSearch.pm	Thu Dec 08 18:02:35 2011 +0800
@@ -19,13 +19,17 @@
 use strict;
 use LJ::Hooks;
 
+sub _sphinx_db {
+    # ensure we can talk to our system
+    return LJ::get_dbh( 'sphinx_search' )
+        or die "Unable to get sphinx_search database handle.\n";
+}
+
 LJ::Hooks::register_hook( 'setprop', sub {
     my %opts = @_;
     return unless $opts{prop} eq 'opt_blockglobalsearch';
 
-    # ensure we can talk to our system
-    my $dbh = LJ::get_dbh( 'sphinx_search' )
-        or die "Unable to get sphinx_search database handle.\n";
+    my $dbh = _sphinx_db();
     $dbh->do( 'UPDATE posts_raw SET allow_global_search = ? WHERE journal_id = ?',
               undef, $opts{value} eq 'Y' ? 0 : 1, $opts{u}->id );
     die $dbh->errstr if $dbh->err;
@@ -34,4 +38,40 @@
     return 1;
 } );
 
+
+# set when the user's status(vis) changes
+# the user may still undelete or be unsuspended
+# so we don't want to remove from indexing just yet
+sub _mark_deleted {
+    my ( $u, $is_deleted ) = @_;
+
+    my $dbh = _sphinx_db();
+    $dbh->do( 'UPDATE posts_raw SET is_deleted = ? where journal_id = ?',
+              undef, $is_deleted, $u->id );
+    die $dbh->errstr if $dbh->err;
+
+    return 1;
+}
+
+LJ::Hooks::register_hook( 'account_delete', sub { _mark_deleted( $_[0], 1 ) } );
+LJ::Hooks::register_hook( 'account_cancel', sub { _mark_deleted( $_[0], 1 ) } );
+LJ::Hooks::register_hook( 'account_makevisible', sub {
+    my ( $u, %opts ) = @_;
+
+    my $old = $opts{old_statusvis};
+    _mark_deleted( $u, 0 ) if $old eq "D" || $old eq "S";
+} );
+
+
+LJ::Hooks::register_hook( 'purged_user', sub {
+    my ( $u ) = @_;
+
+    my $sclient = LJ::theschwartz() or die;
+
+    # queue up a copier job, which will notice that the entries by this user have been deleted...
+    $sclient->insert_jobs( TheSchwartz::Job->new_from_array( 'DW::Worker::Sphinx::Copier',
+                                { userid => $u->id } ) );
+
+});
+
 1;
diff -r 3ea6b65d559c -r 0363ba0c8e9a cgi-bin/LJ/User.pm
--- a/cgi-bin/LJ/User.pm	Thu Dec 08 01:02:37 2011 -0600
+++ b/cgi-bin/LJ/User.pm	Thu Dec 08 18:02:35 2011 +0800
@@ -566,7 +566,7 @@
         return 0;
     }
 
-    my $res = $u->set_statusvis('V');
+    my $res = $u->set_visible;
     unless ($res) {
         $$errref = "DB error while setting statusvis to 'V'" if ref $errref;
         return $res;
@@ -580,7 +580,13 @@
 
 sub set_visible {
     my $u = shift;
-    return $u->set_statusvis('V');
+
+    my $old_statusvis = $u->statusvis;
+    my $ret = $u->set_statusvis('V');
+
+    LJ::Hooks::run_hooks( "account_makevisible", $u, old_statusvis => $old_statusvis );
+
+    return $ret;
 }
 
 
--------------------------------------------------------------------------------