From a652b2ff23bab34e7d83286ea60338ec87d001d7 Mon Sep 17 00:00:00 2001 From: Jamie McCarthy Date: Sat, 26 Apr 2003 13:38:03 +0000 Subject: [PATCH] Pull the code that makes subscriber-related boolean decisions about each page up into prepareUser(). Now instead of calling a $subscribe method, the code just checks flags like $user->{state}{page_adless}, which are valid anytime after prepareUser() returns. This is the first step toward reworking the logic of prepAds(). --- Slash/Apache/Log/Log.pm | 14 ++---- Slash/DB/MySQL/MySQL.pm | 22 ++++----- Slash/Utility/Anchor/Anchor.pm | 29 ++++++++---- Slash/Utility/Environment/Environment.pm | 39 ++++++++++++---- plugins/Subscribe/Subscribe.pm | 59 ++++++++++++------------ themes/slashcode/htdocs/article.pl | 11 ++--- themes/slashcode/htdocs/comments.pl | 11 ++--- 7 files changed, 101 insertions(+), 84 deletions(-) diff --git a/Slash/Apache/Log/Log.pm b/Slash/Apache/Log/Log.pm index 95a1680ae..b1e3884ee 100644 --- a/Slash/Apache/Log/Log.pm +++ b/Slash/Apache/Log/Log.pm @@ -60,12 +60,7 @@ sub UserLog { && ($user->{is_subscriber} || !$constants->{subscribe_hits_only}) ) { $user_update = { -hits => 'hits+1' }; - my $subscribe = getObject('Slash::Subscribe'); - my $buying = 0; - if ($subscribe && $subscribe->buyingThisPage($r)) { - $user_update->{-hits_bought} = 'hits_bought+1'; - $buying = 1; - } + $user_update->{-hits_bought} = 'hits_bought+1' if $user->{state}{page_buying}; my @gmt = gmtime; my $today = sprintf("%04d%02d%02d", $gmt[5]+1900, $gmt[4]+1, $gmt[3]); @@ -74,7 +69,7 @@ sub UserLog { # be buying this page. The day has not rolled over. # Increment hits_bought_today iff they are buying this page. $user_update->{-hits_bought_today} = 'hits_bought_today+1' - if $buying; + if $user->{state}{page_buying}; } else { # User may or may not be a subscriber, and may or may not # be buying this page. The day has rolled over since their @@ -82,7 +77,7 @@ sub UserLog { # buying this page, or 1 if they are. Note that we do not # want to set it to the empty string, as that would delete # the param row. - $user_update->{hits_bought_today} = $buying ? 1 : 0; + $user_update->{hits_bought_today} = $user->{state}{page_buying} ? 1 : 0; if ($user->{hits_bought_today} && !$user->{is_admin}) { my $day = join("-", $user->{lastclick} =~ /^(\d{4})(\d{2})(\d{2})/); @@ -97,7 +92,8 @@ sub UserLog { } } } - if ($buying && $user->{hits_bought} == $user->{hits_paidfor}-1 + if ($user->{state}{page_buying} + && $user->{hits_bought} == $user->{hits_paidfor}-1 and my $statsSave = getObject('Slash::Stats::Writer')) { $statsSave->addStatDaily("subscribe_runout", 1); } diff --git a/Slash/DB/MySQL/MySQL.pm b/Slash/DB/MySQL/MySQL.pm index 655cae637..d056bf220 100644 --- a/Slash/DB/MySQL/MySQL.pm +++ b/Slash/DB/MySQL/MySQL.pm @@ -4773,22 +4773,16 @@ sub _stories_time_clauses { my $secs = $constants->{subscribe_future_secs}; # Tweak $secs here somewhat, based on something...? Nah. - # First decide whether we're looking into the future or not. - # Do the quick tests first, then if necessary do the more - # expensive check to see if this page is plummy. + # First decide whether we're looking into the future or not. If we + # are going to try for this sort of thing, then either we must NOT + # be limiting it to subscribers only, OR the user must be a subscriber + # and this page must be plummy (able to have plums). my $future = 0; $future = 1 if $try_future && $constants->{subscribe} - && $secs; - $future = 0 if $must_be_subscriber && !$user->{is_subscriber}; - if ($future && $must_be_subscriber) { - if ($user->{is_subscriber}) { - my $subscribe = getObject("Slash::Subscribe"); - $future = 0 unless $subscribe->plummyPage(); - } else { - $future = 0; - } - } + && $secs + && (!$must_be_subscriber + || ($user->{is_subscriber} && $user->{state}{page_plummy})); if ($future) { $is_future_column = "IF($column_name < NOW(), 0, 1) AS is_future"; @@ -6306,7 +6300,7 @@ sub getTopicImageBySection { my ($self, $topic, $section, $values, $cache) = @_; my $image_sections = $self->getDescriptions("topic_images_section"); my $image_id = $image_sections->{"$topic->{tid}|$section"} || $topic->{default_image}; - print STDERR "TOPIC $topic->{tid}|$section:$topic->{default_image}:$image_id\n"; +# print STDERR "TOPIC $topic->{tid}|$section:$topic->{default_image}:$image_id\n"; my $answer = _genericGetCache({ table => 'topic_images', table_prime => 'id', diff --git a/Slash/Utility/Anchor/Anchor.pm b/Slash/Utility/Anchor/Anchor.pm index 2b20ab536..47777f6df 100755 --- a/Slash/Utility/Anchor/Anchor.pm +++ b/Slash/Utility/Anchor/Anchor.pm @@ -348,11 +348,7 @@ sub prepAds { my $ad_max = $constants->{ad_max} || $ad_messaging_num; my $ad_messaging_prob = $constants->{ad_messaging_prob} || 0.5; - my $adless = 0; - if ($constants->{subscribe}) { - my $subscribe = getObject('Slash::Subscribe'); - $adless = 1 if $subscribe && $subscribe->adlessPage(); - } + my $adless = $user->{state}{page_adless} ? 1 : 0; # Let's lay out some representative possibilities so we # get the logic right: @@ -471,13 +467,28 @@ sub prepAds { # If we're not showing a messaging ad, it doesn't get shown. $use_this_ad = 0 if !$use_messaging && $num == $ad_messaging_num; # If there's no ad here, it doesn't get shown obviously. - $use_this_ad = 0 if !$ENV{"AD_BANNER_$num"}; + # But if we're testing, assume there could be ads everywhere. + if (!$constants->{debug_adtext}) { + $use_this_ad = 0 if !$ENV{"AD_BANNER_$num"}; + } if ($use_this_ad) { - $user->{state}{ad}{$num} = $ENV{"AD_BANNER_$num"}; + if ($constants->{debug_adtext}) { + $user->{state}{ad}{$num} = "\nAD $num HERE\n"; + } else { + $user->{state}{ad}{$num} = $ENV{"AD_BANNER_$num"}; + } } elsif ($num == 1 && $ENV{AD_PAGECOUNTER}) { - $user->{state}{ad}{$num} = $ENV{AD_PAGECOUNTER}; + if ($constants->{debug_adtext}) { + $user->{state}{ad}{$num} = "\nPAGECOUNTER\n"; + } else { + $user->{state}{ad}{$num} = $ENV{AD_PAGECOUNTER}; + } } else { - $user->{state}{ad}{$num} = "\n\n"; + if ($constants->{debug_adtext}) { + $user->{state}{ad}{$num} = "\nno ad $num here\n"; + } else { + $user->{state}{ad}{$num} = "\n\n"; + } } } } diff --git a/Slash/Utility/Environment/Environment.pm b/Slash/Utility/Environment/Environment.pm index b656da2ff..29b6bd487 100755 --- a/Slash/Utility/Environment/Environment.pm +++ b/Slash/Utility/Environment/Environment.pm @@ -1094,7 +1094,25 @@ sub setCookie { -path => $cookiepath ); + # This old code may be wrong, says Pudge. + my $secure_old = 0; + if ($constants->{cookiesecure}) { + my $subr = $r->lookup_uri($r->uri); + if ($subr && $subr->subprocess_env('HTTPS') eq 'on') { + $secure_old = 1; +# $cookiehash{-secure} = 1; + } + } + # And this new (old) code is right, says Pudge. + my $secure_new = 0; if ($constants->{cookiesecure} && Slash::Apache::ConnectionIsSSL()) { + $secure_new = 1; +# $cookiehash{-secure} = 1; + } + if ($secure_old || $secure_new) { + my $uid = getCurrentUser('uid'); + print STDERR scalar(gmtime) . " uid '$uid' secure_old '$secure_old' secure_new '$secure_new'\n" + if $secure_old xor $secure_new; $cookiehash{-secure} = 1; } @@ -1202,7 +1220,7 @@ sub prepareUser { $uid = $constants->{anonymous_coward_uid} unless defined($uid) && $uid ne ''; - my $reader = getObject('Slash::DB', { virtual_user => $user_types{'reader'} }); + my $reader = getObject('Slash::DB', { virtual_user => $user_types{reader} }); if (isAnon($uid)) { if ($ENV{GATEWAY_INTERFACE}) { @@ -1284,15 +1302,20 @@ sub prepareUser { $user->{currentPage} = 'misc'; } - if ($constants->{subscribe} - && $user->{hits_paidfor} - && $user->{hits_bought} < $user->{hits_paidfor} - ) { - $user->{is_subscriber} = 1; - if (my $subscribe = getObject('Slash::Subscribe')) { - $user->{state}{plummy_page} = $subscribe->plummyPage($r); + if ($constants->{subscribe}) { + # Decide whether the user is a subscriber. + $user->{is_subscriber} = 1 if $user->{hits_paidfor} + && $user->{hits_bought} < $user->{hits_paidfor}; + # Make other decisions about subscriber-related attributes + # of this page. Note that we still have $r lying around, + # so we can save Subscribe.pm a bit of work. + if (my $subscribe = getObject('Slash::Subscribe', { db_type => 'reader' })) { + $user->{state}{page_plummy} = $subscribe->plummyPage($r, $user); + $user->{state}{page_buying} = $subscribe->buyingThisPage($r, $user); + $user->{state}{page_adless} = $subscribe->adlessPage($r, $user); } } + print STDERR scalar(localtime) . " user->currentPage '$user->{currentPage}' user->state->page_ plummy='$user->{state}{page_plummy}' buying='$user->{state}{page_buying}' adless='$user->{state}{page_adless}'\n"; if ($user->{seclev} >= 100) { $user->{is_admin} = 1; diff --git a/plugins/Subscribe/Subscribe.pm b/plugins/Subscribe/Subscribe.pm index cb5046157..c7d0631ec 100644 --- a/plugins/Subscribe/Subscribe.pm +++ b/plugins/Subscribe/Subscribe.pm @@ -18,12 +18,12 @@ use base 'Slash::DB::MySQL'; ($VERSION) = ' $Revision$ ' =~ /\$Revision:\s+([^\s]+)/; sub new { - my($class) = @_; - my $self = { }; + my($class) = @_; + my $self = { }; my $slashdb = getCurrentDB(); - my $plugins = $slashdb->getDescriptions('plugins'); - return unless $plugins->{Subscribe}; + my $plugins = $slashdb->getDescriptions('plugins'); + return unless $plugins->{Subscribe}; $self->{defpage} = { map { ( $_, 1 ) } @@ -40,9 +40,9 @@ sub new { || $self->{defpage}{article} || $self->{defpage}{comments}; - bless($self, $class); + bless($self, $class); - return $self; + return $self; } ######################################################## @@ -88,22 +88,17 @@ sub new { # [E] User's max pages per day to buy is set >= the default (10) # sub _subscribeDecisionPage { - my($self, $trueOnOther, $useMaxNotToday, $r) = @_; + my($self, $trueOnOther, $useMaxNotToday, $r, $user) = @_; - my $user = getCurrentUser(); + $user ||= getCurrentUser(); my $uid = $user->{uid} || 0; - return 0 if !$user - || !$uid - || $user->{is_anon}; + return 0 if !$user + || !$uid + || $user->{is_anon}; # At this point, if we're asking about buying a page, we may know # the answer already. if (!$trueOnOther && !$useMaxNotToday) { - # Any part of the code can set this user state variable at any time. - # At the moment, this is totally unused, but it might be in future, - # if we ever decide that a user action can force a page to be bought - # when it otherwise might not be. - return 1 if $user->{state}{buyingpage}; # If the user hasn't paid for any pages, or has already bought # (used up) all the pages they've paid for, then they are not # buying this one. @@ -116,7 +111,7 @@ sub _subscribeDecisionPage { # short-circuit here. my $constants = getCurrentStatic(); if ($trueOnOther && !$useMaxNotToday) { - # If ads aren't on, the user isn't buying this one. + # If ads aren't on, we're not going to show this one. return 0 if !$constants->{run_ads}; # Otherwise, if the user is not a subscriber, this page # is not adless (though it may not actually have an ad, @@ -159,14 +154,18 @@ sub _subscribeDecisionPage { # We should use $user->{currentPage} instead of parsing $r->uri # separately here. But first we'll need to audit the code and # make sure this method is never called before that field is set. + # Update 2003/04/25: Now things have been rearranged a bit and + # _subscribeDecisionPage is always called at the same point, + # within prepareUser(), after currentPage has been set. So we + # can almost certainly switch to using currentPage. my $decision = 0; - $r ||= Apache->request; - my $uri = $r->uri; - if ($uri eq '/') { - $uri = 'index'; - } else { - $uri =~ s{^.*/([^/]+)\.pl$}{$1}; - } + $r ||= Apache->request; + my $uri = $r->uri; + if ($uri eq '/') { + $uri = 'index'; + } else { + $uri =~ s{^.*/([^/]+)\.pl$}{$1}; + } # We check to see if the user has saved preferences for # which page types they want to buy. This assumes the # data like $user->{buypage_index} is stored in @@ -203,18 +202,18 @@ sub _subscribeDecisionPage { } sub adlessPage { - my($self, $r) = @_; - return $self->_subscribeDecisionPage(1, 0, $r); + my($self, $r, $user) = @_; + return $self->_subscribeDecisionPage(1, 0, $r, $user); } sub buyingThisPage { - my($self, $r) = @_; - return $self->_subscribeDecisionPage(0, 0, $r); + my($self, $r, $user) = @_; + return $self->_subscribeDecisionPage(0, 0, $r, $user); } sub plummyPage { - my($self, $r) = @_; - return $self->_subscribeDecisionPage(1, 1, $r); + my($self, $r, $user) = @_; + return $self->_subscribeDecisionPage(1, 1, $r, $user); } # By default, allow readers to buy x pages for $y, 2x pages for $2y, diff --git a/themes/slashcode/htdocs/article.pl b/themes/slashcode/htdocs/article.pl index 05ba64817..d010e2bcf 100755 --- a/themes/slashcode/htdocs/article.pl +++ b/themes/slashcode/htdocs/article.pl @@ -23,14 +23,9 @@ sub main { my $future_err = 0; if ($story && $story->{is_future} && !($user->{is_admin} || $user->{author})) { - if (!$constants->{subscribe} || !$user->{is_subscriber}) { - $future_err = 1; - } else { - my $subscribe = getObject("Slash::Subscribe"); - if (!$subscribe || !$subscribe->plummyPage()) { - $future_err = 1; - } - } + $future_err = 1 if !$constants->{subscribe} + || !$user->{is_subscriber} + || !$user->{state}{page_plummy}; if ($future_err) { $story = ''; } diff --git a/themes/slashcode/htdocs/comments.pl b/themes/slashcode/htdocs/comments.pl index 9549a5b19..c4981fcc9 100755 --- a/themes/slashcode/htdocs/comments.pl +++ b/themes/slashcode/htdocs/comments.pl @@ -159,12 +159,11 @@ sub main { # etc.) and if that story is viewable. $future_err = 1; $null_it_out = 1; - } else { - my $subscribe = getObject("Slash::Subscribe"); - if (!$subscribe || !$subscribe->plummyPage()) { - $future_err = 1; - $null_it_out = 1; - } + } elsif (!$user->{is_subscriber} || !$user->{state}{page_plummy}) { + # If the user is not a subscriber or the page is + # not able to have plums, sorry! + $future_err = 1; + $null_it_out = 1; } } elsif ($discussion->{sid} && !$slashdb->checkStoryViewable($discussion->{sid})) { # Probably a Never Display'd story.