Bug 36367: Remove _new_userenv

TODO - better review C4::Auth's changes. Are all the removal of
_new_userenv correct/enough?

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

C4/Auth.pm

@@ -933,7 +933,7 @@ sub checkauth {
                 $session->delete();
                 $session->flush;
                 $cookie = $cookie_mgr->clear_unless( $query->cookie, @$cookie );
-                C4::Context::_unset_userenv($sessionID);
+                C4::Context::unset_userenv();
                 $sessionID = undef;
                 undef $userid; # IMPORTANT: this assures us a new session in code below
                 $auth_state = 'failed';
@@ -982,7 +982,7 @@ sub checkauth {
             $session->delete();
             $session->flush;
         }
-        C4::Context::_unset_userenv($sessionID);
+        C4::Context::unset_userenv();
         $cookie = $cookie_mgr->clear_unless( $query->cookie, @$cookie );
 
         if ($cas and $caslogout) {
@@ -1012,7 +1012,6 @@ sub checkauth {
         }
 
         $sessionID = $session->id;
-        C4::Context->_new_userenv($sessionID);
         $cookie = $cookie_mgr->replace_in_list( $cookie, $query->cookie(
             -name     => 'CGISESSID',
             -value    => $sessionID,
@@ -1158,7 +1157,7 @@ sub checkauth {
                     # although we do present an authorization failure. (Yes, the
                     # authentication was actually correct.)
                     $info{'nopermission'} = 1;
-                    C4::Context::_unset_userenv($sessionID);
+                    C4::Context::unset_userenv();
                 }
                 my ( $borrowernumber, $firstname, $surname, $userflags,
                     $branchcode, $branchname, $emailaddress, $desk_id,
@@ -1306,7 +1305,7 @@ sub checkauth {
             else {
                 if ($userid) {
                     $info{'invalid_username_or_password'} = 1;
-                    C4::Context::_unset_userenv($sessionID);
+                    C4::Context::unset_userenv();
                 }
                 $session->param( 'lasttime', time() );
                 $session->param( 'ip',       $session->remote_addr() );
@@ -1667,7 +1666,6 @@ sub check_api_auth {
             return ( "failed", undef, undef ) unless $session;
 
             my $sessionID = $session->id;
-            C4::Context->_new_userenv($sessionID);
             my $cookie = $query->cookie(
                 -name     => 'CGISESSID',
                 -value    => $sessionID,
@@ -1831,7 +1829,7 @@ sub check_cookie_auth {
     unless ( $sessionID ) {
         return ( "failed", undef );
     }
-    C4::Context::_unset_userenv($sessionID); # remove old userenv first
+    C4::Context::unset_userenv();
     my $session   = get_session($sessionID);
     if ($session) {
         my $userid   = $session->param('id');
@@ -1868,7 +1866,6 @@ sub check_cookie_auth {
             return ("password_expired", undef ) if $patron->password_expired;
             my $flags = defined($flagsrequired) ? haspermission( $userid, $flagsrequired ) : 1;
             if ($flags) {
-                C4::Context->_new_userenv($sessionID);
                 if ( !C4::Context->interface ) {
                     # No need to override the interface, most often set by get_template_and_user
                     C4::Context->interface( $session->param('interface') );
@@ -1898,7 +1895,6 @@ sub check_cookie_auth {
             }
 
         } else {
-            C4::Context->_new_userenv($sessionID);
             C4::Context->interface($session->param('interface'));
             C4::Context->set_userenv( undef, q{} );
             return ( "anon", $session );

C4/Context.pm

@@ -182,7 +182,6 @@ sub new {
 
     $self->{"Zconn"} = undef;    # Zebra Connections
     $self->{"userenv"} = undef;        # User env
-    $self->{"activeuser"} = undef;        # current active user
     $self->{"shelves"} = undef;
     $self->{tz} = undef; # local timezone object
 
@@ -758,19 +757,10 @@ sub restore_dbh
 
 Retrieves a hash for user environment variables.
 
-This hash shall be cached for future use: if you call
-C<C4::Context-E<gt>userenv> twice, you will get the same hash without real DB access
-
 =cut
 
-#'
 sub userenv {
-    my $var = $context->{"activeuser"};
-    if (defined $var and defined $context->{"userenv"}->{$var}) {
-        return $context->{"userenv"}->{$var};
-    } else {
-        return;
-    }
+    return $context->{userenv};
 }
 
 =head2 set_userenv
@@ -798,7 +788,6 @@ sub set_userenv {
         $register_id,  $register_name
     ) = @_;
 
-    my $var=$context->{"activeuser"} || '';
     my $cell = {
         "number"     => $usernum,
         "id"         => $userid,
@@ -817,44 +806,21 @@ sub set_userenv {
         "register_id"   => $register_id,
         "register_name" => $register_name
     };
-    $context->{userenv}->{$var} = $cell;
+    $context->{userenv} = $cell;
     return $cell;
 }
 
-=head2 _new_userenv
+=head2 unset_userenv
 
-  C4::Context->_new_userenv($session);  # FIXME: This calling style is wrong for what looks like an _internal function
+  C4::Context->unset_userenv;
 
-Builds a hash for user environment variables.
-
-This hash shall be cached for future use: if you call
-C<C4::Context-E<gt>userenv> twice, you will get the same hash without real DB access
-
-_new_userenv is called in Auth.pm
+Destroys user environment variables.
 
 =cut
 
-#'
-sub _new_userenv
+sub unset_userenv
 {
-    shift;  # Useless except it compensates for bad calling style
-    my ($sessionID)= @_;
-     $context->{"activeuser"}=$sessionID;
-}
-
-=head2 _unset_userenv
-
-  C4::Context->_unset_userenv;
-
-Destroys the hash for activeuser user environment variables.
-
-=cut
-
-#'
-
-sub _unset_userenv
-{
-    delete $context->{activeuser};
+    $context->{userenv} = {};
 }
 
 

C4/InstallAuth.pm

@@ -252,7 +252,6 @@ sub checkauth {
     my $session = Koha::Session->get_session( { sessionID => $sessionID, storage_method => 'file' } );
 
     if ( $session ) {
-        C4::Context->_new_userenv($sessionID);
         if ( $session->param('cardnumber') ) {
             C4::Context->set_userenv(
                 $session->param('number'),
@@ -279,7 +278,7 @@ sub checkauth {
 
     if ($logout || !$session) {
         # voluntary logout the user
-        C4::Context->_unset_userenv($sessionID);
+        C4::Context->unset_userenv();
         $session = Koha::Session->get_session( { storage_method => 'file' } );
     }
 
@@ -288,7 +287,6 @@ sub checkauth {
     unless ($userid) {
         $userid = $query->param('login_userid');
         my $password = $query->param('login_password');
-        C4::Context->_new_userenv($sessionID);
         my ( $return, $cardnumber ) = checkpw( $userid, $password );
         if ($return) {
             $loggedin = 1;
@@ -334,7 +332,7 @@ sub checkauth {
         else {
             if ($userid) {
                 $info{'invalid_username_or_password'} = 1;
-                C4::Context->_unset_userenv($sessionID);
+                C4::Context->unset_userenv();
             }
         }
     }

Koha/BackgroundJob.pm

@@ -170,7 +170,6 @@ sub process {
 
     if ( $self->context ) {
         my $context = $self->json->decode($self->context);
-        C4::Context->_new_userenv(-1);
         C4::Context->interface( $context->{interface} );
         C4::Context->set_userenv(
             $context->{number},       $context->{id},

Koha/Middleware/UserEnv.pm

@@ -22,7 +22,7 @@ sub call {
 
     my $req = Plack::Request->new($env);
 
-    C4::Context->_unset_userenv;
+    C4::Context->unset_userenv;
 
     return $self->app->($env);
 }

Koha/REST/V1/Auth.pm

@@ -568,7 +568,7 @@ sub _set_userenv {
         $THE_library = $patron->library;
     }
 
-    C4::Context->_new_userenv( $patron->borrowernumber );
+    C4::Context->new_userenv();
     C4::Context->set_userenv(
         $patron->borrowernumber,  # number,
         $patron->userid,          # userid,

Koha/Script.pm

@@ -46,11 +46,9 @@ sub import {
     my $class = shift;
     my @flags = @_;
 
-    C4::Context->_new_userenv(1);
     if ( ( $flags[0] || '' ) eq '-cron' ) {
 
         # Set userenv
-        C4::Context->_new_userenv(1);
         C4::Context->set_userenv(
             undef, undef, undef, 'CRON', 'CRON',
             undef, undef, undef, undef,  undef

opac/ilsdi.pl

@@ -128,7 +128,6 @@ unless ( $cgi->param('service') ) {
 }
 
 # Set the userenv
-C4::Context->_new_userenv( 'ILSDI_'.time() );
 C4::Context->set_userenv(
     undef, undef, undef, 'ILSDI', 'ILSDI',
     undef, undef, undef, undef, undef,

t/Token.t

@@ -27,7 +27,6 @@ use Time::HiRes qw|usleep|;
 use C4::Context;
 use Koha::Token;
 
-C4::Context->_new_userenv('DUMMY SESSION');
 C4::Context->set_userenv(0,42,0,'firstname','surname', 'CPL', 'Library 1', 0, '');
 
 my $tokenizer = Koha::Token->new;
@@ -114,14 +113,12 @@ subtest 'testing _add_default_csrf_params with/without userenv (bug 27849)' => s
     is( $result->{id}, 'anonymous_567', 'Check userid' );
 
     # Clear userenv
-    C4::Context::_unset_userenv('DUMMY SESSION');
     is( C4::Context::userenv, undef, 'No userenv anymore' );
     $result = Koha::Token::_add_default_csrf_params({}); # pass no session_id
     is( $result->{session_id}, Koha::Token::DEFA_SESSION_ID, 'Check session id' );
     is( $result->{id}, Koha::Token::DEFA_SESSION_USERID. '_'. $result->{session_id}, 'Check userid' );
 
     # Empty anonymous userenv (see C4::Auth::check_cookie_auth)
-    C4::Context->_new_userenv('ANON SESSION');
     C4::Context->set_userenv( undef, q{} );
     ok( C4::Context->userenv, "Userenv exists" );
     $result = Koha::Token::_add_default_csrf_params( {} );    # pass no session_id

t/db_dependent/Auth.t

@@ -199,7 +199,6 @@ subtest 'checkauth() tests' => sub {
         $session->param( 'interface', 'intranet' );
         $session->flush;
         my $sessionID = $session->id;
-        C4::Context->_new_userenv($sessionID);
 
         my ($return) =
             C4::Auth::check_cookie_auth( $sessionID, undef, { skip_version_check => 1, remote_addr => '1.2.3.4' } );
@@ -233,7 +232,6 @@ subtest 'checkauth() tests' => sub {
         $session->param( 'interface',    'opac' );
         $session->flush;
         my $previous_sessionID = $session->id;
-        C4::Context->_new_userenv($previous_sessionID);
 
         my ( $return ) = C4::Auth::check_cookie_auth( $previous_sessionID, undef, { skip_version_check => 1, remote_addr => '1.2.3.4' } );
         is( $return, 'ok', 'Former session in shape now' );
@@ -262,7 +260,6 @@ subtest 'checkauth() tests' => sub {
         $session->param( 'interface',    'intranet' );
         $session->flush;
         $previous_sessionID = $session->id;
-        C4::Context->_new_userenv($previous_sessionID);
         $cgi->param( -name => 'login_userid',             -value => $patron2->userid );
         $cgi->param( -name => 'login_password',           -value => $password );
         $cgi->param( -name => 'koha_login_context', -value => 1 );
@@ -417,7 +414,6 @@ subtest 'checkauth() tests' => sub {
         is( $sesh->param('branch'), $branch->branchcode, "If user is superlibrarian, they should be able to choose a branch" );
 
     };
-    C4::Context->_new_userenv; # For next tests
 };
 
 subtest 'no_set_userenv parameter tests' => sub {
@@ -430,8 +426,8 @@ subtest 'no_set_userenv parameter tests' => sub {
     my $password = set_weak_password($patron);
 
     ok( checkpw( $patron->userid, $password, undef, undef, 1 ), 'checkpw returns true' );
+    C4::Context->unset_userenv;
     is( C4::Context->userenv, undef, 'Userenv should be undef as required' );
-    C4::Context->_new_userenv('DUMMY SESSION');
     C4::Context->set_userenv(0,0,0,'firstname','surname', $library->branchcode, 'Library 1', 0, '', '');
     is( C4::Context->userenv->{branch}, $library->branchcode, 'Userenv gives correct branch' );
     ok( checkpw( $patron->userid, $password, undef, undef, 1 ), 'checkpw returns true' );
@@ -940,7 +936,6 @@ subtest 'Userenv clearing in check_cookie_auth' => sub {
     is( C4::Context->userenv, undef, 'Environment should be cleared too' );
 
     # Show that we clear the userenv again: set up env and check deleted session
-    C4::Context->_new_userenv( $sessionID );
     C4::Context->set_userenv; # empty
     is( defined C4::Context->userenv, 1, 'There should be an empty userenv again' );
     ( $auth_status, $session) = C4::Auth::check_cookie_auth( $sessionID );

t/db_dependent/Koha/BackgroundJob.t

@@ -229,7 +229,6 @@ subtest 'process tests' => sub {
         { size => 10, a => 'aaa', b => 'bbb' } );
     my $job    = Koha::BackgroundJobs->find($job_id);
 
-    C4::Context->_new_userenv(-1);
     C4::Context->interface('opac');
     is( C4::Context->userenv, undef, "Userenv unset prior to calling process");
     is( C4::Context->interface, 'opac', "Interface set to opac prior to calling process");

t/db_dependent/Koha/Items.t

@@ -1317,7 +1317,7 @@ subtest 'store' => sub {
             C4::Circulation::LostItem( $item->itemnumber, 1 );
 
             # Unset the userenv
-            C4::Context->_new_userenv(undef);
+            C4::Context->unset_userenv();
 
             # Simluate item marked as found
             $item->itemlost(0)->store;

t/db_dependent/Koha/Reviews.t

@@ -70,7 +70,6 @@ is( $retrieved_review_1_1->review, $new_review_1_1->review, 'Find a review by id
 
 subtest 'search_limited' => sub {
     plan tests => 2;
-    C4::Context->_new_userenv('xxx');
     my $group_1 = Koha::Library::Group->new( { title => 'TEST Group 1' } )->store;
     my $group_2 = Koha::Library::Group->new( { title => 'TEST Group 2' } )->store;
     Koha::Library::Group->new({ parent_id => $group_1->id,  branchcode => $patron_1->branchcode })->store();

t/db_dependent/Koha/Template/Plugin/Registers.t

@@ -45,7 +45,7 @@ subtest 'session_register_id' => sub {
         '1', "Returns the register id when set in the userenv" );
 
     # Unset the userenv
-    C4::Context->_new_userenv(undef);
+    C4::Context->unset_userenv();
 };
 
 subtest 'session_register_name' => sub {
@@ -61,7 +61,7 @@ subtest 'session_register_name' => sub {
         'Register One', "Returns the register name when set in the userenv" );
 
     # Unset the userenv
-    C4::Context->_new_userenv(undef);
+    C4::Context->unset_userenv();
 };
 
 subtest 'all() tests' => sub {

t/db_dependent/Patron/Borrower_Discharge.t

@@ -47,7 +47,6 @@ my $library         = $builder->build({ source => 'Branch' });
 my $another_library = $builder->build({ source => 'Branch' });
 my $itemtype        = $builder->build({ source => 'Itemtype' })->{itemtype};
 
-C4::Context->_new_userenv('xxx');
 my $patron = $builder->build_object({
     class => 'Koha::Patrons',
     value => {

t/lib/Mocks.pm

@@ -119,8 +119,6 @@ Also, some sane defaults are set if no parameters are passed.
 sub mock_userenv {
     my ( $params ) = @_;
 
-    C4::Context->_new_userenv(42);
-
     my $userenv;
     if ( $params and my $patron = $params->{patron} ) {
         $userenv = $patron->unblessed;