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;
 }
 
 
--------------------------------------------------------------------------------

Post a comment in response:

This account has disabled anonymous posting.
If you don't have an account you can create one now.
HTML doesn't work in the subject.
More info about formatting

If you are unable to use this captcha for any reason, please contact us by email at support@dreamwidth.org