View Issue Details

IDProjectCategoryView StatusLast Update
0003904mantisbtsqlpublic2004-08-29 01:53
Reportergrangeway Assigned Tothraxisp  
PrioritynormalSeverityfeatureReproducibilityalways
Status closedResolutionfixed 
Product Versiongit trunk 
Fixed in Version0.19.0rc1 
Summary0003904: cache missing 'user pref' rows
Description

if a user does not have an entry in user pref table, the failure should be cached.

Adding to the if statement($g_cache_user_pref[$c_user_id][$c_project_id] = false; ), line 107-114 of user_pref_api, appears to resolve the duplicat queries :

if ( 0 == db_num_rows( $result ) ) {
if ( $p_trigger_errors ) {
trigger_error( ERROR_USER_PREFS_NOT_FOUND, ERROR );
} else {
$g_cache_user_pref[$c_user_id][$c_project_id] = false;
return false;
}
}

TagsNo tags attached.
Attached Files
prefs.diff (15,029 bytes)   
? mantisbt
Index: account_prefs_inc.php
===================================================================
RCS file: /cvsroot/mantisbt/mantisbt/account_prefs_inc.php,v
retrieving revision 1.28
diff -u -r1.28 account_prefs_inc.php
--- account_prefs_inc.php	8 Aug 2004 11:39:00 -0000	1.28
+++ account_prefs_inc.php	11 Aug 2004 17:18:21 -0000
@@ -37,8 +37,7 @@
 	    }
 
 	    # prefix data with u_
-		$row = user_pref_get_row( $p_user_id );
-		extract( $row, EXTR_PREFIX_ALL, 'u' );
+		$t_pref = user_pref_get( $p_user_id );
 ?>
 <?php # Account Preferences Form BEGIN ?>
 <br />
@@ -65,7 +64,7 @@
 	</td>
 	<td width="50%">
 		<select name="default_project">
-			<?php print_project_option_list( $u_default_project ) ?>
+			<?php print_project_option_list( $t_pref->default_project ) ?>
 		</select>
 	</td>
 </tr>
@@ -74,7 +73,7 @@
 		<?php echo lang_get( 'advanced_report' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="advanced_report" <?php check_checked( $u_advanced_report, ON ); ?> />
+		<input type="checkbox" name="advanced_report" <?php check_checked( $t_pref->advanced_report, ON ); ?> />
 	</td>
 </tr>
 <tr class="row-1">
@@ -82,7 +81,7 @@
 		<?php echo lang_get( 'advanced_view' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="advanced_view" <?php check_checked( $u_advanced_view, ON ); ?> />
+		<input type="checkbox" name="advanced_view" <?php check_checked( $t_pref->advanced_view, ON ); ?> />
 	</td>
 </tr>
 <tr class="row-2">
@@ -90,7 +89,7 @@
 		<?php echo lang_get( 'advanced_update' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="advanced_update" <?php check_checked( $u_advanced_update, ON ); ?> />
+		<input type="checkbox" name="advanced_update" <?php check_checked( $t_pref->advanced_update, ON ); ?> />
 	</td>
 </tr>
 <tr class="row-1">
@@ -98,7 +97,7 @@
 		<?php echo lang_get( 'refresh_delay' ) ?>
 	</td>
 	<td>
-		<input type="text" name="refresh_delay" size="4" maxlength="4" value="<?php echo $u_refresh_delay ?>" />
+		<input type="text" name="refresh_delay" size="4" maxlength="4" value="<?php echo $t_pref->refresh_delay ?>" />
 	</td>
 </tr>
 <tr class="row-2">
@@ -106,7 +105,7 @@
 		<?php echo lang_get( 'redirect_delay' ) ?>
 	</td>
 	<td>
-		<input type="text" name="redirect_delay" size="1" maxlength="1" value="<?php echo $u_redirect_delay ?>" />
+		<input type="text" name="redirect_delay" size="1" maxlength="1" value="<?php echo $t_pref->redirect_delay ?>" />
 	</td>
 </tr>
 <tr class="row-1">
@@ -114,8 +113,8 @@
 		<?php echo lang_get( 'bugnote_order' ) ?>
 	</td>
 	<td>
-		<input type="radio" name="bugnote_order" value="ASC" <?php check_checked( $u_bugnote_order, 'ASC' ); ?> /><?php echo lang_get( 'bugnote_order_asc' ) ?>
-		<input type="radio" name="bugnote_order" value="DESC" <?php check_checked( $u_bugnote_order, 'DESC' ); ?> /><?php echo lang_get( 'bugnote_order_desc' ) ?>
+		<input type="radio" name="bugnote_order" value="ASC" <?php check_checked( $t_pref->bugnote_order, 'ASC' ); ?> /><?php echo lang_get( 'bugnote_order_asc' ) ?>
+		<input type="radio" name="bugnote_order" value="DESC" <?php check_checked( $t_pref->bugnote_order, 'DESC' ); ?> /><?php echo lang_get( 'bugnote_order_desc' ) ?>
 	</td>
 </tr>
 <?php
@@ -126,12 +125,12 @@
 		<?php echo lang_get( 'email_on_new' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_new" <?php check_checked( $u_email_on_new, ON ); ?> />
+		<input type="checkbox" name="email_on_new" <?php check_checked( $t_pref->email_on_new, ON ); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_new_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_new_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_new_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -140,12 +139,12 @@
 		<?php echo lang_get( 'email_on_assigned' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_assigned" <?php check_checked( $u_email_on_assigned, ON ); ?> />
+		<input type="checkbox" name="email_on_assigned" <?php check_checked( $t_pref->email_on_assigned, ON ); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_assigned_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_assigned_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_assigned_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -154,12 +153,12 @@
 		<?php echo lang_get( 'email_on_feedback' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_feedback" <?php check_checked( $u_email_on_feedback, ON ); ?> />
+		<input type="checkbox" name="email_on_feedback" <?php check_checked( $t_pref->email_on_feedback, ON ); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_feedback_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_feedback_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_feedback_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -168,12 +167,12 @@
 		<?php echo lang_get( 'email_on_resolved' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_resolved" <?php check_checked( $u_email_on_resolved, ON ); ?> />
+		<input type="checkbox" name="email_on_resolved" <?php check_checked( $t_pref->email_on_resolved, ON ); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_resolved_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_resolved_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_resolved_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -182,12 +181,12 @@
 		<?php echo lang_get( 'email_on_closed' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_closed" <?php check_checked( $u_email_on_closed, ON ); ?> />
+		<input type="checkbox" name="email_on_closed" <?php check_checked( $t_pref->email_on_closed, ON ); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_closed_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_closed_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_closed_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -196,12 +195,12 @@
 		<?php echo lang_get( 'email_on_reopened' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_reopened" <?php check_checked( $u_email_on_reopened, ON ); ?> />
+		<input type="checkbox" name="email_on_reopened" <?php check_checked( $t_pref->email_on_reopened, ON ); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_reopened_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_reopened_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_reopened_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -210,12 +209,12 @@
 		<?php echo lang_get( 'email_on_bugnote_added' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_bugnote" <?php check_checked( $u_email_on_bugnote, ON ); ?> />
+		<input type="checkbox" name="email_on_bugnote" <?php check_checked( $t_pref->email_on_bugnote, ON ); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_bugnote_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_bugnote_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_bugnote_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -224,12 +223,12 @@
 		<?php echo lang_get( 'email_on_status_change' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_status" <?php check_checked( $u_email_on_status, ON ); ?> />
+		<input type="checkbox" name="email_on_status" <?php check_checked( $t_pref->email_on_status, ON ); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_status_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_status_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_status_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -238,12 +237,12 @@
 		<?php echo lang_get( 'email_on_priority_change' ) ?>
 	</td>
 	<td>
-		<input type="checkbox" name="email_on_priority" <?php check_checked( $u_email_on_priority , ON); ?> />
+		<input type="checkbox" name="email_on_priority" <?php check_checked( $t_pref->email_on_priority , ON); ?> />
 		<?php echo lang_get( 'with_minimum_severity' ) ?>
 		<select name="email_on_priority_minimum_severity">
 			<option value="any"><?php echo lang_get( 'any' ) ?></option>
 			<option value="any"></option>
-			<?php print_enum_string_option_list( 'severity', $u_email_on_priority_minimum_severity ) ?>
+			<?php print_enum_string_option_list( 'severity', $t_pref->email_on_priority_minimum_severity ) ?>
 		</select>
 	</td>
 </tr>
@@ -252,28 +251,28 @@
 		<?php echo lang_get( 'email_bugnote_limit' ) ?>
 	</td>
 	<td>
-		<input type="text" name="email_bugnote_limit" maxlength="2" size="2" value="<?php echo $u_email_bugnote_limit ?>">
+		<input type="text" name="email_bugnote_limit" maxlength="2" size="2" value="<?php echo $t_pref->email_bugnote_limit ?>">
 	</td>
 </tr>
 <?php } else { ?>
-		<input type="hidden" name="email_on_new"      value="<?php echo $u_email_on_new ?>" />
-		<input type="hidden" name="email_on_assigned" value="<?php echo $u_email_on_assigned ?>" />
-		<input type="hidden" name="email_on_feedback" value="<?php echo $u_email_on_feedback ?>" />
-		<input type="hidden" name="email_on_resolved" value="<?php echo $u_email_on_resolved ?>" />
-		<input type="hidden" name="email_on_closed"   value="<?php echo $u_email_on_closed ?>" />
-		<input type="hidden" name="email_on_reopened" value="<?php echo $u_email_on_reopened ?>" />
-		<input type="hidden" name="email_on_bugnote"  value="<?php echo $u_email_on_bugnote ?>" />
-		<input type="hidden" name="email_on_status"   value="<?php echo $u_email_on_status ?>" />
-		<input type="hidden" name="email_on_priority" value="<?php echo $u_email_on_priority ?>" />
-		<input type="hidden" name="email_on_new_minimum_severity"      value="<?php echo $u_email_on_new_minimum_severity ?>" />
-		<input type="hidden" name="email_on_assigned_minimum_severity" value="<?php echo $u_email_on_assigned_minimum_severity ?>" />
-		<input type="hidden" name="email_on_feedback_minimum_severity" value="<?php echo $u_email_on_feedback_minimum_severity ?>" />
-		<input type="hidden" name="email_on_resolved_minimum_severity" value="<?php echo $u_email_on_resolved_minimum_severity ?>" />
-		<input type="hidden" name="email_on_closed_minimum_severity"   value="<?php echo $u_email_on_closed_minimum_severity ?>" />
-		<input type="hidden" name="email_on_reopened_minimum_severity" value="<?php echo $u_email_on_reopened_minimum_severity ?>" />
-		<input type="hidden" name="email_on_bugnote_minimum_severity"  value="<?php echo $u_email_on_bugnote_minimum_severity ?>" />
-		<input type="hidden" name="email_on_status_minimum_severity"   value="<?php echo $u_email_on_status_minimum_severity ?>" />
-		<input type="hidden" name="email_on_priority_minimum_severity" value="<?php echo $u_email_on_priority_minimum_severity ?>" />
+		<input type="hidden" name="email_on_new"      value="<?php echo $t_pref->email_on_new ?>" />
+		<input type="hidden" name="email_on_assigned" value="<?php echo $t_pref->email_on_assigned ?>" />
+		<input type="hidden" name="email_on_feedback" value="<?php echo $t_pref->email_on_feedback ?>" />
+		<input type="hidden" name="email_on_resolved" value="<?php echo $t_pref->email_on_resolved ?>" />
+		<input type="hidden" name="email_on_closed"   value="<?php echo $t_pref->email_on_closed ?>" />
+		<input type="hidden" name="email_on_reopened" value="<?php echo $t_pref->email_on_reopened ?>" />
+		<input type="hidden" name="email_on_bugnote"  value="<?php echo $t_pref->email_on_bugnote ?>" />
+		<input type="hidden" name="email_on_status"   value="<?php echo $t_pref->email_on_status ?>" />
+		<input type="hidden" name="email_on_priority" value="<?php echo $t_pref->email_on_priority ?>" />
+		<input type="hidden" name="email_on_new_minimum_severity"      value="<?php echo $t_pref->email_on_new_minimum_severity ?>" />
+		<input type="hidden" name="email_on_assigned_minimum_severity" value="<?php echo $t_pref->email_on_assigned_minimum_severity ?>" />
+		<input type="hidden" name="email_on_feedback_minimum_severity" value="<?php echo $t_pref->email_on_feedback_minimum_severity ?>" />
+		<input type="hidden" name="email_on_resolved_minimum_severity" value="<?php echo $t_pref->email_on_resolved_minimum_severity ?>" />
+		<input type="hidden" name="email_on_closed_minimum_severity"   value="<?php echo $t_pref->email_on_closed_minimum_severity ?>" />
+		<input type="hidden" name="email_on_reopened_minimum_severity" value="<?php echo $t_pref->email_on_reopened_minimum_severity ?>" />
+		<input type="hidden" name="email_on_bugnote_minimum_severity"  value="<?php echo $t_pref->email_on_bugnote_minimum_severity ?>" />
+		<input type="hidden" name="email_on_status_minimum_severity"   value="<?php echo $t_pref->email_on_status_minimum_severity ?>" />
+		<input type="hidden" name="email_on_priority_minimum_severity" value="<?php echo $t_pref->email_on_priority_minimum_severity ?>" />
 <?php } ?>
 <tr class="row-2">
 	<td class="category">
@@ -281,7 +280,7 @@
 	</td>
 	<td>
 		<select name="language">
-			<?php print_language_option_list( $u_language ) ?>
+			<?php print_language_option_list( $t_pref->language ) ?>
 		</select>
 	</td>
 </tr>
Index: core/user_pref_api.php
===================================================================
RCS file: /cvsroot/mantisbt/mantisbt/core/user_pref_api.php,v
retrieving revision 1.18
diff -u -r1.18 user_pref_api.php
--- core/user_pref_api.php	8 Aug 2004 11:39:00 -0000	1.18
+++ core/user_pref_api.php	11 Aug 2004 17:18:21 -0000
@@ -112,6 +112,7 @@
 			if ( $p_trigger_errors ) {
 				trigger_error( ERROR_USER_PREFS_NOT_FOUND, ERROR );
 			} else {
+				$g_cache_user_pref[$c_user_id][$c_project_id] = false;
 				return false;
 			}
 		}
prefs.diff (15,029 bytes)   
prefs.tar.gz (4,721 bytes)

Relationships

child of 0003987 closedvboctor Mantis 0.19.0 Release 

Activities

grangeway

grangeway

2004-08-01 18:59

reporter   ~0006545

Victor/thraxisp: do you run xdebug?

jlatour

jlatour

2004-08-06 11:29

reporter   ~0006714

Paul, can you commit this to CVS?

grangeway

grangeway

2004-08-06 17:32

reporter   ~0006733

jlatour- do you run xdebug ?

Victor suggested instead of 'false' we should return default preferences or null - can't remember which.

Either way, i'm hitting loops/other issues when trying to modify this routine.

And when trying to see what's going on with xdebug, having php quit out on me half way through html_api's print top header routines at a config_get statement for some random reason.

thraxisp

thraxisp

2004-08-07 16:15

reporter   ~0006777

I took a look into this (e.g., what happens when there is no pref's entry). There seem to be many cases where an invalid or non-existent pref is not handled properly. You will get notices and bizarre behaviour.

My suggestion would be to return the defaults if user_pref_cache_row is called with project=ALL PROJECTS, and an entry is not found. This would typically only happen with the built-in administrator, as prefs are set up when a user is created and not removed until the user is.

I can implement and test this in an hour or so.

thraxisp

thraxisp

2004-08-07 16:16

reporter   ~0006778

ps. No, I don't run xdebug. I might look at compiling it for MacOS in future.

jlatour

jlatour

2004-08-07 17:26

reporter   ~0006783

thraxisp, are you taking this issue (including the caching)?

grangeway

grangeway

2004-08-07 19:18

reporter   ~0006785

It's a single issue - which basically boils down to we need to do 'something' when there are no preferences. Atm, various bits of mantis/pref api seem to handle the different possible failure cases in different ways.

What i'd like to be able to do is replace the line of:
$g_cache_user_pref[$c_user_id][$c_project_id] = false;
with something returning 'new UserPreferences' with default values, however i seem to recall user_pref api checks for a cached value when creating the defaults (from memory).

I'm not going to have a look at this again until next friday/sat at the earliest. Both thraxisp and myself have analysed this issue, and drawn similar/same conclusions.

Just a matter of getting a solution into CVS now.

thraxisp

thraxisp

2004-08-07 20:39

reporter   ~0006790

I spent some more time looking at this. It appears that all checks of preferences (except one in account_prefs_inc) go through user_pref_get. This overlays the defaults with the settings from the database. Even if there are no prefs, the user will get the defaults. The database is updated when the user goes to the My Account page.

I can't see a problem here.

account_prefs_inc should be rewritted to the user_pref_get.

thraxisp

thraxisp

2004-08-11 12:19

reporter   ~0006943

The changes are attached for review. The major change is a re-write of account_prefs_inc.php to eliminate the use of the raw prefs settings. You get failures on this page, if there is no user prefs entry.