Bug 253588: Change Bugzilla->user to be usable even for a logged-out user

Bugzilla.pm

@@ -51,6 +51,11 @@ sub cgi {
 my $_user;
 sub user {
     my $class = shift;
+
+    if (not defined $_user) {
+        $_user = new Bugzilla::User;
+    }
+
     return $_user;
 }
 
@@ -61,12 +66,11 @@ sub login {
 
 sub logout {
     my ($class, $option) = @_;
-    if (! $_user) {
-        # If we're not logged in, go away
-        return;
-    }
-    $option = LOGOUT_CURRENT unless defined $option;
 
+    # If we're not logged in, go away
+    return unless user->id;
+
+    $option = LOGOUT_CURRENT unless defined $option;
     Bugzilla::Auth::Login::WWW->logout($_user, $option);
 }
 

Bugzilla/Auth/Login/WWW.pm

@@ -43,9 +43,7 @@ sub login {
 
     # Avoid double-logins, which may confuse the auth code
     # (double cookies, odd compat code settings, etc)
-    if (defined $user) {
-        return $user;
-    }
+    return $user if $user->id;
 
     $type = LOGIN_NORMAL unless defined $type;
 

Bugzilla/Bug.pm

@@ -390,7 +390,7 @@ sub user {
     use Bugzilla;
 
     my @movers = map { trim $_ } split(",", Param("movers"));
-    my $canmove = Param("move-enabled") && Bugzilla->user && 
+    my $canmove = Param("move-enabled") && Bugzilla->user->id && 
                   (lsearch(\@movers, Bugzilla->user->login) != -1);
 
     # In the below, if the person hasn't logged in, then we treat them
@@ -399,12 +399,12 @@ sub user {
     # Display everything as if they have all the permissions in the
     # world; their permissions will get checked when they log in and
     # actually try to make the change.
-    my $privileged = (!Bugzilla->user)
+    my $privileged = (!Bugzilla->user->id)
                      || Bugzilla->user->in_group("editbugs")
                      || Bugzilla->user->id == $self->{'assigned_to'}{'id'}
                      || (Param('useqacontact') && $self->{'qa_contact'} &&
                          Bugzilla->user->id == $self->{'qa_contact'}{'id'});
-    my $isreporter = Bugzilla->user && 
+    my $isreporter = Bugzilla->user->id && 
                      Bugzilla->user->id == $self->{'reporter'}{'id'};
 
     my $canedit = $privileged || $isreporter;

Bugzilla/Error.pm

@@ -47,7 +47,7 @@ sub _throw_error {
         $mesg .= "\n[$$] " . time2str("%D %H:%M:%S ", time());
         $mesg .= "$name $error ";
         $mesg .= "$ENV{REMOTE_ADDR} " if $ENV{REMOTE_ADDR};
-        $mesg .= Bugzilla->user->login if Bugzilla->user;
+        $mesg .= Bugzilla->user->login;
         $mesg .= "\n";
         my %params = Bugzilla->cgi->Vars;
         $Data::Dumper::Useqq = 1;

Bugzilla/Search.pm

@@ -1172,7 +1172,7 @@ sub init {
                 " LEFT JOIN bug_group_map " .
                 " ON bug_group_map.bug_id = bugs.bug_id ";
 
-    if ($user) {
+    if ($user->id) {
         if (%{$user->groups}) {
             $query .= " AND bug_group_map.group_id NOT IN (" . join(',', values(%{$user->groups})) . ") ";
         }
@@ -1183,7 +1183,7 @@ sub init {
     $query .= " WHERE " . join(' AND ', (@wherepart, @andlist)) .
               " AND ((bug_group_map.group_id IS NULL)";
 
-    if ($user) {
+    if ($user->id) {
         my $userid = $user->id;
         $query .= "    OR (bugs.reporter_accessible = 1 AND bugs.reporter = $userid) " .
               "    OR (bugs.cclist_accessible = 1 AND cc.who IS NOT NULL) " .
@@ -1338,7 +1338,7 @@ sub getSQL {
 sub pronoun {
     my ($noun, $user) = (@_);
     if ($noun eq "%user%") {
-        if ($user) {
+        if ($user->id) {
             return $user->id;
         } else {
             ThrowUserError('login_required_for_pronoun');

Bugzilla/User.pm

@@ -18,7 +18,7 @@
 # Rights Reserved.
 #
 # Contributor(s): Myk Melez <myk@mozilla.org>
-#                 Erik Stambaugh <not_erik@dasbistro.com>
+#                 Erik Stambaugh <erik@dasbistro.com>
 #                 Bradley Baetz <bbaetz@acm.org>
 #                 Joel Peshkin <bugreport@peshkin.net> 
 
@@ -43,6 +43,9 @@ use Bugzilla::Constants;
 
 sub new {
     my $invocant = shift;
+    if (scalar @_ == 0) {
+        return $invocant->_create;
+    }
     return $invocant->_create("userid=?", @_);
 }
 
@@ -69,6 +72,16 @@ sub _create {
     my $cond = shift;
     my $val = shift;
 
+    # Allow invocation with no parameters to create a blank object
+    my $self = {
+        'id'             => 0,
+        'name'           => '',
+        'login'          => '',
+        'showmybugslink' => 0,
+    };
+    bless ($self, $class);
+    return $self unless $cond;
+
     # We're checking for validity here, so any value is OK
     trick_taint($val);
 
@@ -90,13 +103,10 @@ sub _create {
 
     return undef unless defined $id;
 
-    my $self = { id => $id,
-                 name => $name,
-                 login => $login,
-                 showmybugslink => $mybugslink,
-               };
-
-    bless ($self, $class);
+    $self->{'id'}             = $id;
+    $self->{'name'}           = $name;
+    $self->{'login'}          = $login;
+    $self->{'showmybugslink'} = $mybugslink;
 
     # Now update any old group information if needed
     my $result = $dbh->selectrow_array(q{SELECT 1
@@ -133,6 +143,8 @@ sub showmybugslink { $_[0]->{showmybugslink}; }
 sub identity {
     my $self = shift;
 
+    return "" unless $self->id;
+
     if (!defined $self->{identity}) {
         $self->{identity} = 
           $self->{name} ? "$self->{name} <$self->{login}>" : $self->{login};
@@ -144,6 +156,8 @@ sub identity {
 sub nick {
     my $self = shift;
 
+    return "" unless $self->id;
+
     if (!defined $self->{nick}) {
         $self->{nick} = (split(/@/, $self->{login}, 2))[0];
     }
@@ -155,6 +169,7 @@ sub queries {
     my $self = shift;
 
     return $self->{queries} if defined $self->{queries};
+    return [] unless $self->id;
 
     my $dbh = Bugzilla->dbh;
     my $sth = $dbh->prepare(q{  SELECT name, query, linkinfooter
@@ -186,6 +201,7 @@ sub groups {
     my $self = shift;
 
     return $self->{groups} if defined $self->{groups};
+    return {} unless $self->id;
 
     my $dbh = Bugzilla->dbh;
     my $groups = $dbh->selectcol_arrayref(q{SELECT DISTINCT groups.name, group_id
@@ -209,6 +225,7 @@ sub in_group {
 
     # If we already have the info, just return it.
     return defined($self->{groups}->{$group}) if defined $self->{groups};
+    return 0 unless $self->id;
 
     # Otherwise, go check for it
 
@@ -232,6 +249,7 @@ sub in_group {
 sub visible_groups_inherited {
     my $self = shift;
     return $self->{visible_groups_inherited} if defined $self->{visible_groups_inherited};
+    return [] unless $self->id;
     my @visgroups = @{$self->visible_groups_direct};
     @visgroups = flatten_group_membership(@visgroups);
     $self->{visible_groups_inherited} = \@visgroups;
@@ -244,6 +262,7 @@ sub visible_groups_direct {
     my $self = shift;
     my @visgroups = ();
     return $self->{visible_groups_direct} if defined $self->{visible_groups_direct};
+    return [] unless $self->id;
 
     my $dbh = Bugzilla->dbh;
     my $glist = join(',',(-1,values(%{$self->groups})));
@@ -265,6 +284,7 @@ sub derive_groups {
     my ($self, $already_locked) = @_;
 
     my $id = $self->id;
+    return unless $id;
 
     my $dbh = Bugzilla->dbh;
 
@@ -352,6 +372,7 @@ sub can_bless {
     my $self = shift;
 
     return $self->{can_bless} if defined $self->{can_bless};
+    return 0 unless $self->id;
 
     my $dbh = Bugzilla->dbh;
     # First check if the user can explicitly bless a group
@@ -729,6 +750,7 @@ sub email_prefs {
     # Get or set (not implemented) the user's email notification preferences.
     
     my $self = shift;
+    return {} unless $self->id;
     
     # If the calling code is setting the email preferences, update the object
     # but don't do anything else.  This needs to write email preferences back
@@ -820,8 +842,10 @@ L<Bugzilla-E<gt>user|Bugzilla/"user">.
 
 =item C<new($userid)>
 
-Creates a new C<Bugzilla::User> object for the given user id. Returns
-C<undef> if no matching user is found.
+Creates a new C{Bugzilla::User> object for the given user id.  If no user
+id was given, a blank object is created with no user attributes.
+
+If an id was given but there was no matching user found, undef is returned.
 
 =begin undocumented
 

buglist.cgi

@@ -195,7 +195,7 @@ sub iCalendarDateTime {
 sub LookupNamedQuery {
     my ($name) = @_;
     Bugzilla->login(LOGIN_REQUIRED);
-    my $userid = DBNameToIdAndCheck(Bugzilla->user->login);
+    my $userid = Bugzilla->user->id;
     my $qname = SqlQuote($name);
     SendSQL("SELECT query FROM namedqueries WHERE userid = $userid AND name = $qname");
     my $result = FetchOneColumn();
@@ -318,7 +318,7 @@ if ($::FORM{'cmdtype'} eq "dorem") {
     }
     elsif ($::FORM{'remaction'} eq "forget") {
         Bugzilla->login(LOGIN_REQUIRED);
-        my $userid = DBNameToIdAndCheck(Bugzilla->user->login);
+        my $userid = Bugzilla->user->id;
         my $qname = SqlQuote($::FORM{'namedcmd'});
         SendSQL("DELETE FROM namedqueries WHERE userid = $userid AND name = $qname");
 
@@ -338,7 +338,7 @@ if ($::FORM{'cmdtype'} eq "dorem") {
 elsif (($::FORM{'cmdtype'} eq "doit") && $::FORM{'remtype'}) {
     if ($::FORM{'remtype'} eq "asdefault") {
         Bugzilla->login(LOGIN_REQUIRED);
-        my $userid = DBNameToIdAndCheck(Bugzilla->user->login);
+        my $userid = Bugzilla->user->id;
         my $qname = SqlQuote($::defaultqueryname);
         my $qbuffer = SqlQuote($::buffer);
 
@@ -361,7 +361,7 @@ elsif (($::FORM{'cmdtype'} eq "doit") && $::FORM{'remtype'}) {
     }
     elsif ($::FORM{'remtype'} eq "asnamed") {
         Bugzilla->login(LOGIN_REQUIRED);
-        my $userid = DBNameToIdAndCheck(Bugzilla->user->login);
+        my $userid = Bugzilla->user->id;
 
         my $name = trim($::FORM{'newqueryname'});
         $name || ThrowUserError("query_name_missing");
@@ -850,7 +850,7 @@ $vars->{'urlquerypart'} =~ s/(order|cmdtype)=[^&]*&?//g;
 $vars->{'order'} = $order;
 
 # The user's login account name (i.e. email address).
-my $login = Bugzilla->user ? Bugzilla->user->login : "";
+my $login = Bugzilla->user->login;
 
 $vars->{'caneditbugs'} = UserInGroup('editbugs');
 

describecomponents.cgi

@@ -47,7 +47,7 @@ if (!defined $product) {
 
     if (AnyEntryGroups()) {
         # OK, now only add products the user can see
-        Bugzilla->login(LOGIN_REQUIRED) unless Bugzilla->user;
+        Bugzilla->login(LOGIN_REQUIRED);
         foreach my $p (@::legal_product) {
             if (CanEnterProduct($p)) {
                 $products{$p} = $::proddesc{$p};

docs/rel_notes.txt

@@ -287,7 +287,7 @@ Code Changes Which May Affect Customizations
 - Use Bugzilla->user->login in place of $::COOKIE{Bugzilla_login}
 
 - You can tell if there's a user logged in or not by checking if
-  Bugzilla->user exists rather than looking for $::userid==0
+  Bugzilla->user->id != 0  rather than looking for $::userid != 0
 
 
 Recommended Practice for the Upgrade

globals.pl

@@ -482,7 +482,7 @@ sub CanEditProductId {
     my $query = "SELECT group_id FROM group_control_map " .
                 "WHERE product_id = $productid " .
                 "AND canedit != 0 "; 
-    if (defined Bugzilla->user && %{Bugzilla->user->groups}) {
+    if (%{Bugzilla->user->groups}) {
         $query .= "AND group_id NOT IN(" . 
                    join(',', values(%{Bugzilla->user->groups})) . ") ";
     }
@@ -504,7 +504,7 @@ sub CanEnterProduct {
                 "LEFT JOIN group_control_map " .
                 "ON group_control_map.product_id = products.id " .
                 "AND group_control_map.entry != 0 ";
-    if (defined Bugzilla->user && %{Bugzilla->user->groups}) {
+    if (%{Bugzilla->user->groups}) {
         $query .= "AND group_id NOT IN(" . 
                    join(',', values(%{Bugzilla->user->groups})) . ") ";
     }
@@ -547,7 +547,7 @@ sub GetSelectableProducts {
         $query .= "AND group_control_map.membercontrol = " .
                   CONTROLMAPMANDATORY . " ";
     }
-    if (defined Bugzilla->user && %{Bugzilla->user->groups}) {
+    if (%{Bugzilla->user->groups}) {
         $query .= "AND group_id NOT IN(" . 
                    join(',', values(%{Bugzilla->user->groups})) . ") ";
     }
@@ -1192,7 +1192,7 @@ sub UserInGroup {
         die "UserInGroup no longer takes a second parameter.";
     }
     
-    return defined Bugzilla->user && defined Bugzilla->user->groups->{$_[0]};
+    return defined Bugzilla->user->groups->{$_[0]};
 }
 
 sub UserCanBlessGroup {

query.cgi

@@ -62,13 +62,12 @@ if (defined $::FORM{"GoAheadAndLogIn"}) {
     Bugzilla->login();
 }
 
-my $user = Bugzilla->user;
-my $userid = $user ? $user->id : 0;
+my $userid = Bugzilla->user->id;
 
 # Backwards compatibility hack -- if there are any of the old QUERY_*
 # cookies around, and we are logged in, then move them into the database
 # and nuke the cookie. This is required for Bugzilla 2.8 and earlier.
-if ($user) {
+if ($userid) {
     my @oldquerycookies;
     foreach my $i ($cgi->cookie()) {
         if ($i =~ /^QUERY_(.*)$/) {
@@ -102,7 +101,7 @@ if ($user) {
 }
 
 if ($::FORM{'nukedefaultquery'}) {
-    if ($user) {
+    if ($userid) {
         SendSQL("DELETE FROM namedqueries " .
                 "WHERE userid = $userid AND name = '$::defaultqueryname'");
     }
@@ -110,7 +109,7 @@ if ($::FORM{'nukedefaultquery'}) {
 }
 
 my $userdefaultquery;
-if ($user) {
+if ($userid) {
     SendSQL("SELECT query FROM namedqueries " .
             "WHERE userid = $userid AND name = '$::defaultqueryname'");
     $userdefaultquery = FetchOneColumn();
@@ -363,7 +362,7 @@ for (my $chart = 0; $::FORM{"field$chart-0-0"}; $chart++) {
 $default{'charts'} = \@charts;
 
 # Named queries
-if ($user) {
+if ($userid) {
     my @namedqueries;
     SendSQL("SELECT name FROM namedqueries " .
             "WHERE userid = $userid AND name != '$::defaultqueryname' " .

template/en/default/bug/show.xml.tmpl

@@ -25,7 +25,7 @@
 <bugzilla version="[% VERSION %]"
           urlbase="[% Param('urlbase') %]"
           maintainer="[% Param('maintainer') FILTER xml %]"
-[% IF user %]
+[% IF user.id %]
           exporter="[% user.login FILTER xml %]"
 [% END %]
 >

template/en/default/global/useful-links.html.tmpl

@@ -38,14 +38,14 @@
         
         <a href="report.cgi">Reports</a> 
         
-        [% IF user %]
+        [% IF user.id %]
           [% email = user.login FILTER url_quote %]
           | <a href="request.cgi?requester=[% email %]&amp;requestee=[% email %]&amp;do_union=1&amp;group=type">My Requests</a>
         [% ELSE %]
           | <a href="request.cgi">Requests</a>
         [% END %]
         
-        [% IF user && Param('usevotes') %]
+        [% IF user.id && Param('usevotes') %]
           | <a href="votes.cgi?action=show_user">My&nbsp;Votes</a>
         [% END %]      
         

template/en/default/sidebar.xul.tmpl

@@ -75,7 +75,7 @@ function normal_keypress_handler( aEvent ) {
       <text class="text-link" onclick="load_relative_url('enter_bug.cgi')" value="new [% terms.bug %]"/>
       <separator class="thin"/>
 
-[% IF user %]
+[% IF user.id %]
       <text class="text-link" onclick="load_relative_url('userprefs.cgi')" value="edit prefs"/>
   [%- IF user.groups.tweakparams %]
       <text class="text-link" onclick="load_relative_url('editparams.cgi')" value="edit params"/>

votes.cgi

@@ -126,10 +126,9 @@ sub show_user {
     
     my $name = $cgi->param('user') || Bugzilla->user->login;
     my $who = DBNameToIdAndCheck($name);
-    my $userid = Bugzilla->user ? Bugzilla->user->id : 0;
+    my $userid = Bugzilla->user->id;
     
-    my $canedit = 1 if (Bugzilla->user &&
-                        $name eq Bugzilla->user->login);
+    my $canedit = 1 if ($userid && $name eq Bugzilla->user->login);
     
     SendSQL("LOCK TABLES bugs READ, products READ, votes WRITE,
              cc READ, bug_group_map READ, user_group_map READ,