[LON-CAPA-cvs] cvs: rat / lonuserstate.pm

foxr foxr at source.lon-capa.org
Tue Jul 26 06:40:23 EDT 2011


foxr		Tue Jul 26 10:40:23 2011 EDT

  Modified files:              
    /rat	lonuserstate.pm 
  Log:
  1. Centralize tempdir location in unlink_tmpfiles.
  2. Get rid of >dirty< next in parse_resource ... returning
    undef instead for zombie resources and checking in the
    loop that calls it.
  
  
-------------- next part --------------
Index: rat/lonuserstate.pm
diff -u rat/lonuserstate.pm:1.137 rat/lonuserstate.pm:1.138
--- rat/lonuserstate.pm:1.137	Thu Apr 21 13:28:50 2011
+++ rat/lonuserstate.pm	Tue Jul 26 10:40:23 2011
@@ -1,7 +1,7 @@
 # The LearningOnline Network with CAPA
 # Construct and maintain state and binary representation of course for user
 #
-# $Id: lonuserstate.pm,v 1.137 2011/04/21 13:28:50 raeburn Exp $
+# $Id: lonuserstate.pm,v 1.138 2011/07/26 10:40:23 foxr Exp $
 #
 # Copyright Michigan State University Board of Trustees
 #
@@ -43,11 +43,13 @@
 use Apache::lonenc;
 use Fcntl qw(:flock);
 use LONCAPA;
+use File::Basename;
+
  
 
 # ---------------------------------------------------- Globals for this package
 
-my $pc;      # Package counter
+my $pc;      # Package counter is this what 'Guts' calls the map counter?
 my %hash;    # The big tied hash
 my %parmhash;# The hash with the parameters
 my @cond;    # Array with all of the conditions
@@ -112,20 +114,30 @@
     }
 }
 
-# --------------------------------------------------------- Loads map from disk
+# --------------------------------------------------------- Loads from disk
 
 sub loadmap { 
     my ($uri,$parent_rid)=@_;
+
+    # Is the map already included?
+
     if ($hash{'map_pc_'.$uri}) { 
 	$errtext.='<p class="LC_error">'.
 	    &mt('Multiple use of sequence/page [_1]! The course will not function properly.','<tt>'.$uri.'</tt>').
 	    '</p>';
 	return; 
     }
+    # Register the resource in it's map_pc_ [for the URL]
+    # map_id.nnn is the nesting level -> to the URI.
+
     $pc++;
     my $lpc=$pc;
     $hash{'map_pc_'.$uri}=$lpc;
     $hash{'map_id_'.$lpc}=$uri;
+
+    # If the parent is of the form n.m hang this map underneath it in the
+    # map hierarchy.
+
     if ($parent_rid =~ /^(\d+)\.\d+$/) {
         my $parent_pc = $1;
         if (defined($hash{'map_hierarchy_'.$parent_pc})) {
@@ -136,17 +148,21 @@
         }
     }
 
-# Determine and check filename
+# Determine and check filename of the sequence we need to read:
+
     my $fn=&Apache::lonnet::filelocation('',&putinversion($uri));
 
     my $ispage=($fn=~/\.page$/);
 
-    unless (($fn=~/\.sequence$/) ||
-            ($fn=~/\.page$/)) { 
+    # We can only nest sequences or pages.  Anything else is an illegal nest.
+
+    unless (($fn=~/\.sequence$/) || $ispage) { 
 	$errtext.=&mt("<br />Invalid map: <tt>[_1]</tt>",$fn);
 	return; 
     }
 
+    # Read the XML that constitutes the file.
+
     my $instr=&Apache::lonnet::getfile($fn);
 
     if ($instr eq -1) {
@@ -154,18 +170,29 @@
 	return;
     }
 
-# Successfully got file, parse it
+    # Successfully got file, parse it
+
+    # parse for parameter processing.
+    # Note that these are <param... / > tags
+    # so we only care about 'S' (tag start) nodes.
+
 
     my $parser = HTML::TokeParser->new(\$instr);
     $parser->attr_encoded(1);
+
     # first get all parameters
+
+
     while (my $token = $parser->get_token) {
 	next if ($token->[0] ne 'S');
 	if ($token->[1] eq 'param') {
 	    &parse_param($token,$lpc);
 	} 
     }
-    #reset parser
+
+    # Get set to take another pass through the XML:
+    # for resources and links.
+
     $parser = HTML::TokeParser->new(\$instr);
     $parser->attr_encoded(1);
 
@@ -177,21 +204,40 @@
 
     my $randomize = ($randomorder{$parent_rid} =~ /^yes$/i);
 
+    # Parse the resources, link and condition tags.
+    # Note that if randomorder or random select is chosen the links and
+    # conditions are meaningless but are determined by the randomization.
+    # This is handled in the next chunk of code.
+
     my @map_ids;
     while (my $token = $parser->get_token) {
 	next if ($token->[0] ne 'S');
+
+	# Resource
+
 	if ($token->[1] eq 'resource') {
-	    push(@map_ids,&parse_resource($token,$lpc,$ispage,$uri));
+	    my $resource_id = &parse_resource($token,$lpc,$ispage,$uri);
+	    if (defined $resource_id) {
+		push(@map_ids, $resource_id);
+	    }
+
+       # Link
+
 	} elsif ($token->[1] eq 'link' && !$randomize) {
-# ----------------------------------------------------------------------- Links
 	    &make_link(++$linkpc,$lpc,$token->[2]->{'to'},
 		       $token->[2]->{'from'},
 		       $token->[2]->{'condition'});
+
+	# condition
+
 	} elsif ($token->[1] eq 'condition' && !$randomize) {
 	    &parse_condition($token,$lpc);
 	}
     }
 
+
+    # Handle randomization and random selection
+
     if ($randomize) {
 	if (!$env{'request.role.adv'}) {
 	    my $seed;
@@ -205,7 +251,10 @@
 		
 		$seed = $symb;
 	    }
-	
+
+	    # Here for sure we need to pass along the username/domain
+	    # so that we can impersonate users in lonprintout e.g.
+
 	    my $rndseed=&Apache::lonnet::rndseed($seed);
 	    &Apache::lonnet::setup_random_from_rndseed($rndseed);
 	    @map_ids=&Math::Random::random_permutation(@map_ids);
@@ -241,9 +290,52 @@
 
 
 # -------------------------------------------------------------------- Resource
+#
+#  Parses a resource tag to produce the value to push into the
+#  map_ids array.
+# 
+#
+#  Information about the actual type of resource is provided by the file extension
+#  of the uri (e.g. .problem, .sequence etc. etc.).
+#
+#  Parameters:
+#    $token   - A token from HTML::TokeParser
+#               This is an array that describes the most recently parsed HTML item.
+#    $lpc     - Map nesting level (?)
+#    $ispage  - True if this resource is encapsulated in a .page (assembled resourcde).
+#    $uri     - URI of the enclosing resource.
+# Returns:
+#
+# Note:
+#   The token is an array that contains the following elements:
+#   [0]   => 'S' indicating this is a start token
+#   [1]   => 'resource'  indicating this tag is a <resource> tag.
+#   [2]   => Hash of attribute =>value pairs.
+#   [3]   => @(keys [2]).
+#   [4]   => unused.
+#
+#   The attributes of the resourcde tag include:
+#
+#   id     - The resource id.
+#   src    - The URI of the resource.
+#   type   - The resource type (e.g. start and finish).
+#   title  - The resource title.
+
+
 sub parse_resource {
     my ($token,$lpc,$ispage,$uri) = @_;
-    if ($token->[2]->{'type'} eq 'zombie') { next; }
+    
+    # I refuse to coutenance code like this that has 
+    # such a dirty side effect (and forcing this sub to be called within a loop).
+    #
+    #  if ($token->[2]->{'type'} eq 'zombie') { next; }
+
+    # Zombie resources don't produce anything useful.
+
+    if ($token->[2]->{'type'} eq 'zombie') {
+	return undef;
+    }
+
     my $rid=$lpc.'.'.$token->[2]->{'id'};
 	    
     $hash{'kind_'.$rid}='res';
@@ -373,36 +465,77 @@
 }
 
 # ------------------------------------------------------------------- Parameter
+# Parse a <parameter> tag in the map.
+# Parmameters:
+#    $token Token array for a start tag from HTML::TokeParser
+#           [0] = 'S'
+#           [1] = tagname ("param")
+#           [2] = Hash of {attribute} = values.
+#           [3] = Array of the keys in [2].
+#           [4] = unused.
+#    $lpc   Current map nesting level.a
+#
+#  Typical attributes:
+#     to=n      - Number of the resource the parameter applies to.
+#     type=xx   - Type of parameter value (e.g. string_yesno or int_pos).
+#     name=xxx  - Name ofr parameter (e.g. parameter_randompick or parameter_randomorder).
+#     value=xxx - value of the parameter.
 
 sub parse_param {
     my ($token,$lpc) = @_;
-    my $referid=$lpc.'.'.$token->[2]->{'to'};
-    my $name=$token->[2]->{'name'};
+    my $referid=$lpc.'.'.$token->[2]->{'to'}; # Resource param applies to.
+    my $name=$token->[2]->{'name'};	      # Name of parameter
     my $part;
-    if ($name=~/^parameter_(.*)_/) {
+
+
+    if ($name=~/^parameter_(.*)_/) { 
 	$part=$1;
     } else {
 	$part=0;
     }
+
+    # Peel the parameter_ off the parameter name.
+
     $name=~s/^.*_([^_]*)$/$1/;
+
+    # The value is:
+    #   type.part.name.value
+
     my $newparam=
 	&escape($token->[2]->{'type'}).':'.
 	&escape($part.'.'.$name).'='.
 	&escape($token->[2]->{'value'});
+
+    # The hash key is param_resourceid.
+    # Multiple parameters for a single resource are & separated in the hash.
+
+
     if (defined($hash{'param_'.$referid})) {
 	$hash{'param_'.$referid}.='&'.$newparam;
     } else {
 	$hash{'param_'.$referid}=''.$newparam;
     }
-    if ($token->[2]->{'name'}=~/^parameter_(0_)*randompick$/) {
+    #
+    #  These parameters have to do with randomly selecting
+    # resources, therefore a separate hash is also created to 
+    # make it easy to locate them when actually computing the resource set later on
+    # See the code conditionalized by ($randomize) in loadmap().
+
+    if ($token->[2]->{'name'}=~/^parameter_(0_)*randompick$/) { # Random selection turned on
 	$randompick{$referid}=$token->[2]->{'value'};
     }
-    if ($token->[2]->{'name'}=~/^parameter_(0_)*randompickseed$/) {
+    if ($token->[2]->{'name'}=~/^parameter_(0_)*randompickseed$/) { # Randomseed provided.
 	$randompickseed{$referid}=$token->[2]->{'value'};
     }
-    if ($token->[2]->{'name'}=~/^parameter_(0_)*randomorder$/) {
+    if ($token->[2]->{'name'}=~/^parameter_(0_)*randomorder$/) { # Random order turned on.
 	$randomorder{$referid}=$token->[2]->{'value'};
     }
+
+    # These parameters have to do with how the URLs of resources are presented to
+    # course members(?).  encrypturl presents encypted url's while
+    # hiddenresource hides the URL.
+    #
+
     if ($token->[2]->{'name'}=~/^parameter_(0_)*encrypturl$/) {
 	if ($token->[2]->{'value'}=~/^yes$/i) {
 	    $encurl{$referid}=1;
@@ -657,7 +790,11 @@
 sub readmap {
     my $short=shift;
     $short=~s/^\///;
-    my %cenv=&Apache::lonnet::coursedescription($short,{'freshen_cache'=>1});
+
+    # TODO:  Hidden dependency on current user:
+
+    my %cenv=&Apache::lonnet::coursedescription($short,{'freshen_cache'=>1}); 
+
     my $fn=$cenv{'fn'};
     my $uri;
     $short=~s/\//\_/g;
@@ -669,27 +806,40 @@
     @cond=('true:normal');
 
     unless (open(LOCKFILE,">$fn.db.lock")) {
+	# 
+	# Most likely a permissions problem on the lockfile or its directory.
+	#
         $errtext.='<br />'.&mt('Map not loaded - Lock file could not be opened when reading map:').' <tt>'.$fn.'</tt>.';
         $retfurl = '';
         return ($retfurl,$errtext);
     }
     my $lock=0;
     my $gotstate=0;
-    if (flock(LOCKFILE,LOCK_EX|LOCK_NB)) {
-	$lock=1;
+    
+    # If we can get the lock without delay any files there are idle
+    # and from some prior request.  We'll kill them off and regenerate them:
+
+    if (flock(LOCKFILE,LOCK_EX|LOCK_NB)) {	
+	$lock=1;		# Remember that we hold the lock.
         &unlink_tmpfiles($fn);
     }
     undef %randompick;
     undef %hiddenurl;
     undef %encurl;
     $retfrid='';
-    my ($untiedhash,$untiedparmhash,$tiedhash,$tiedparmhash);
+    my ($untiedhash,$untiedparmhash,$tiedhash,$tiedparmhash); # More state flags.
+
+    # if we got the lock, regenerate course regnerate empty files and tie them.
+
     if ($lock) {
         if (tie(%hash,'GDBM_File',"$fn.db",&GDBM_WRCREAT(),0640)) {
             $tiedhash = 1;
             if (tie(%parmhash,'GDBM_File',$fn.'_parms.db',&GDBM_WRCREAT(),0640)) {
                 $tiedparmhash = 1;
-                $gotstate = &build_tmp_hashes($uri,$fn,$short,\%cenv);
+                $gotstate = &build_tmp_hashes($uri,
+					      $fn,
+					      $short,
+					      \%cenv); # TODO: Need to provide requested user at dom
                 unless ($gotstate) {
                     &Apache::lonnet::logthis('Failed to write statemap at first attempt '.$fn.' for '.$uri.'.</font>');
                 }
@@ -705,7 +855,10 @@
                     'Could not untie coursemap hash '.$fn.' for '.$uri.'.</font>');
             }
         }
-	flock(LOCKFILE,LOCK_UN);
+	flock(LOCKFILE,LOCK_UN); # RF: this is what I don't get unless there are other
+	                         # unlocked places the remainder happens..seems like if we
+                                 # just kept the lock here the rest of the code would have
+                                 # been much easier? 
     }
     unless ($lock && $tiedhash && $tiedparmhash) { 
 	# if we are here it is likely because we are already trying to 
@@ -713,6 +866,13 @@
 	# tie the hashes for the next 90 seconds, if we succeed forward 
 	# them on to navmaps, if we fail, throw up the Could not init 
 	# course screen
+	#
+	# RF: I'm not seeing the case where the ties/unties can fail in a way
+	#     that can be remedied by this.  Since we owned the lock seems
+	#     Tie/untie failures are a result of something like a permissions problem instead?
+	#
+
+	#  In any vent, undo what we did manage to do above first:
 	if ($lock) {
 	    # Got the lock but not the DB files
 	    flock(LOCKFILE,LOCK_UN);
@@ -728,15 +888,25 @@
                 untie(%parmhash);
             }
         }
+	# Log our failure:
+
 	&Apache::lonnet::logthis('<font color="blue">WARNING: '.
 				 "Could not tie coursemap $fn for $uri.</font>");
         $tiedhash = '';
         $tiedparmhash = '';
 	my $i=0;
+
+	# Keep on retrying the lock for 90 sec until we succeed.
+
 	while($i<90) {
 	    $i++;
 	    sleep(1);
 	    if (flock(LOCKFILE,LOCK_EX|LOCK_NB)) {
+
+		# Got the lock, tie the hashes...the assumption in this code is
+		# that some other worker thread has created the db files quite recently
+		# so no load is needed:
+
                 $lock = 1;
 		if (tie(%hash,'GDBM_File',"$fn.db",&GDBM_READER(),0640)) {
                     $tiedhash = 1;
@@ -744,6 +914,11 @@
                         $tiedparmhash = 1;
                         if (-e "$fn.state") {
 		            $retfurl='/adm/navmaps';
+
+			    # BUG BUG: Side effect!
+			    # Should conditionalize on something so that we can use this
+			    # to load maps for courses that are not current?
+			    #
 		            &Apache::lonnet::appenv({"request.course.id"  => $short,
 		   			             "request.course.fn"  => $fn,
 					             "request.course.uri" => $uri});
@@ -775,6 +950,11 @@
             }
         }
     }
+    # I think this branch of code is all about what happens if we just did the stuff above, 
+    # but found that the  state file did not exist...again if we'd just held the lock
+    # would that have made this logic simpler..as generating all the files would be
+    # an atomic operation with respect to the lock.
+    #
     unless ($gotstate) {
         $lock = 0;
         &Apache::lonnet::logthis('<font color="blue">WARNING: '.
@@ -787,10 +967,13 @@
         undef %hiddenurl;
         undef %encurl;
         $retfrid='';
+	#
+	# Once more through the routine of tying and loading and so on.
+	#
         if ($lock) {
             if (tie(%hash,'GDBM_File',"$fn.db",&GDBM_WRCREAT(),0640)) {
                 if (tie(%parmhash,'GDBM_File',$fn.'_parms.db',&GDBM_WRCREAT(),0640)) {
-                    $gotstate = &build_tmp_hashes($uri,$fn,$short,\%cenv);
+                    $gotstate = &build_tmp_hashes($uri,$fn,$short,\%cenv); # TODO: User dependent?
                     unless ($gotstate) {
                         &Apache::lonnet::logthis('<font color="blue">WARNING: '.
                             'Failed to write statemap at second attempt '.$fn.' for '.$uri.'.</font>');
@@ -814,6 +997,8 @@
             flock(LOCKFILE,LOCK_UN);
             $lock = 0;
         } else {
+	    # Failed to get the immediate lock.
+
             &Apache::lonnet::logthis('<font color="blue">WARNING: '.
             'Could not obtain lock to tie coursemap hash '.$fn.'.db for '.$uri.'.</font>');
         }
@@ -821,10 +1006,13 @@
     close(LOCKFILE);
     unless (($errtext eq '') || ($env{'request.course.uri'} =~ m{^/uploaded/})) {
         &Apache::lonmsg::author_res_msg($env{'request.course.uri'},
-                                        $errtext);
+                                        $errtext); # TODO: User dependent?
     }
 # ------------------------------------------------- Check for critical messages
 
+#  Depends on user must parameterize this as well..or separate as this is:
+#  more part of determining what someone sees on entering a course?
+
     my @what=&Apache::lonnet::dump('critical',$env{'user.domain'},
 				   $env{'user.name'});
     if ($what[0]) {
@@ -835,20 +1023,39 @@
     return ($retfurl,$errtext);
 }
 
+#
+#  This sub is called when the course hash and the param hash have been tied and
+#  their lock file is held.
+#  Parameters:
+#     $uri      -  URI that identifies the course.
+#     $fn       -  The base path/filename of the files that make up the context
+#                  being built.
+#     $short    -  Short course name.
+#     $cenvref  -  Reference to the course environment hash returned by 
+#                  Apache::lonnet::coursedescription
+#
+#  Assumptions:
+#    The globals
+#    %hash, %paramhash are tied to their gdbm files and we hold the lock on them.
+#
 sub build_tmp_hashes {
     my ($uri,$fn,$short,$cenvref) = @_;
+    
     unless(ref($cenvref) eq 'HASH') {
         return;
     }
     my %cenv = %{$cenvref};
     my $gotstate = 0;
-    %hash=();
+    %hash=();			# empty the global course and  parameter hashes.
     %parmhash=();
-    $errtext='';
+    $errtext='';		# No error messages yet.
     $pc=0;
     &clear_mapalias_count();
     &processversionfile(%cenv);
     my $furi=&Apache::lonnet::clutter($uri);
+    #
+    #  the map staring points.
+    #
     $hash{'src_0.0'}=&versiontrack($furi);
     $hash{'title_0.0'}=&Apache::lonnet::metadata($uri,'title');
     $hash{'ids_'.$furi}='0.0';
@@ -906,7 +1113,9 @@
 
 sub unlink_tmpfiles {
     my ($fn) = @_;
-    if ($fn =~ m{^\Q$Apache::lonnet::perlvar{'lonDaemons'}\E/tmp/}) {
+    my $file_dir = dirname($fn);
+
+    if ($fn eq LONCAPA::tempdir()) {
         my @files = qw (.db _symb.db .state _parms.db);
         foreach my $file (@files) {
             if (-e $fn.$file) {
@@ -960,6 +1169,9 @@
     return $state;
 }
 
+#  This block seems to have code to manage/detect doubly defined
+#  aliases in maps.
+
 {
     my %mapalias_cache;
     sub count_mapalias {


More information about the LON-CAPA-cvs mailing list