r53358 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r53357‎ | r53358 | r53359 >
Date:15:18, 16 July 2009
Author:mrzman
Status:reverted (Comments)
Tags:
Comment:
(bug 19590) Database error messages are no longer hardcoded to use "MySQL".
Added a new function DatabaseBase::getDBtype() to get the DB type for messages, updated all subclasses.
Message change needs propagating.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/db/Database.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseIbm_db2.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseMssql.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseMysql.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseOracle.php (modified) (history)
  • /trunk/phase3/includes/db/DatabasePostgres.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseSqlite.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/db/DatabaseMysql.php
——@@ -289,6 +289,13 @@
290290 return '[http://www.mysql.com/ MySQL]';
291291 }
292292
 293+ /**
 294+ * @return String: Database name for messages
 295+ */
 296+ function getDBtype() {
 297+ return 'MySQL';
 298+ }
 299+
293300 public function setTimeout( $timeout ) {
294301 $this->query( "SET net_read_timeout=$timeout" );
295302 $this->query( "SET net_write_timeout=$timeout" );
Index: trunk/phase3/includes/db/DatabaseOracle.php
——@@ -743,6 +743,13 @@
744744 }
745745
746746 /**
 747+ * @return String: Database name for messages
 748+ */
 749+ function getDBtype() {
 750+ return 'Oracle';
 751+ }
 752+
 753+ /**
747754 * Query whether a given table exists (in the given schema, or the default mw one if not given)
748755 */
749756 function tableExists($table) {
Index: trunk/phase3/includes/db/DatabasePostgres.php
——@@ -1048,6 +1048,13 @@
10491049 }
10501050
10511051 /**
 1052+ * @return String: Database name for messages
 1053+ */
 1054+ function getDBtype() {
 1055+ return 'PostgreSQL';
 1056+ }
 1057+
 1058+ /**
10521059 * @return string Version information from the database
10531060 */
10541061 function getServerVersion() {
Index: trunk/phase3/includes/db/DatabaseIbm_db2.php
——@@ -1453,6 +1453,13 @@
14541454 return "[http://www.ibm.com/software/data/db2/express/?s_cmp=ECDDWW01&s_tact=MediaWiki IBM DB2]";
14551455 }
14561456
 1457+ /**
 1458+ * @return String: Database name for messages
 1459+ */
 1460+ function getDBtype() {
 1461+ return 'IBM DB2';
 1462+ }
 1463+
14571464 ###
14581465 # Fix search crash
14591466 ###
Index: trunk/phase3/includes/db/Database.php
——@@ -1953,6 +1953,17 @@
19541954 }
19551955
19561956 /**
 1957+ * Returns the database type for user-visible purposes
 1958+ * e.g. DB error messages
 1959+ * Other uses should just use $wgDBtype
 1960+ *
 1961+ * @return String: Database name for messages
 1962+ */
 1963+ function getDBtype() {
 1964+ return 'Database';
 1965+ }
 1966+
 1967+ /**
19571968 * A string describing the current software version, like from
19581969 * mysql_get_server_info(). Will be listed on Special:Version, etc.
19591970 *
——@@ -2548,7 +2559,8 @@
25492560 function getText() {
25502561 if ( $this->useMessageCache() ) {
25512562 return wfMsg( 'dberrortextcl', htmlspecialchars( $this->getSQL() ),
2552 - htmlspecialchars( $this->fname ), $this->errno, htmlspecialchars( $this->error ) ) . "\n";
 2563+ htmlspecialchars( $this->fname ), $this->errno, htmlspecialchars( $this->error ),
 2564+ htmlspecialchars( $this->db->getDBtype() ) ) . "\n";
25532565 } else {
25542566 return $this->getMessage();
25552567 }
——@@ -2575,7 +2587,8 @@
25762588 function getHTML() {
25772589 if ( $this->useMessageCache() ) {
25782590 return wfMsgNoDB( 'dberrortext', htmlspecialchars( $this->getSQL() ),
2579 - htmlspecialchars( $this->fname ), $this->errno, htmlspecialchars( $this->error ) );
 2591+ htmlspecialchars( $this->fname ), $this->errno, htmlspecialchars( $this->error ),
 2592+ htmlspecialchars( $this->db->getDBtype() ) );
25802593 } else {
25812594 return nl2br( htmlspecialchars( $this->getMessage() ) );
25822595 }
Index: trunk/phase3/includes/db/DatabaseMssql.php
——@@ -900,6 +900,13 @@
901901 }
902902
903903 /**
 904+ * @return String: Database name for messages
 905+ */
 906+ function getDBtype() {
 907+ return 'Microsoft SQL Server';
 908+ }
 909+
 910+ /**
904911 * @return string Version information from the database
905912 */
906913 function getServerVersion() {
Index: trunk/phase3/includes/db/DatabaseSqlite.php
——@@ -288,6 +288,13 @@
289289 }
290290
291291 /**
 292+ * @return String: Database name for messages
 293+ */
 294+ function getDBtype() {
 295+ return 'SQLite';
 296+ }
 297+
 298+ /**
292299 * @return string Version information from the database
293300 */
294301 function getServerVersion() {
Index: trunk/phase3/languages/messages/MessagesEn.php
——@@ -947,12 +947,12 @@
948948 The last attempted database query was:
949949 <blockquote><tt>$1</tt></blockquote>
950950 from within function "<tt>$2</tt>".
951 -MySQL returned error "<tt>$3: $4</tt>".',
 951+$5 returned error "<tt>$3: $4</tt>".',
952952 'dberrortextcl' => 'A database query syntax error has occurred.
953953 The last attempted database query was:
954954 "$1"
955955 from within function "$2".
956 -MySQL returned error "$3: $4"',
 956+$5 returned error "$3: $4"',
957957 'laggedslavemode' => "'''Warning:''' Page may not contain recent updates.",
958958 'readonly' => 'Database locked',
959959 'enterlockreason' => 'Enter a reason for the lock, including an estimate of when the lock will be released',
Index: trunk/phase3/RELEASE-NOTES
——@@ -276,6 +276,8 @@
277277 * (bug 19513) RTL fixes for new Search UI
278278 * (bug 16497) Special:Allmessages is paginated
279279 * (bug 18708) CSS plainlinks class now available to all skins
 280+* (bug 19590) Database error messages no longer have "MySQL" hardcoded as the
 281+ database type
280282
281283 == API changes in 1.16 ==
282284

Follow-up revisions

RevisionCommit summaryAuthorDate
r53365Followup to r53358 - Tweak the function name to be a little clearer as to wha...mrzman16:49, 16 July 2009
r55501Mostly revert r53358 and r53365 per comments on code review. Change message t...mrzman20:17, 22 August 2009

Comments

#Comment by Brion VIBBER (talk | contribs)   00:46, 22 August 2009

These really should just say "database"; no need to pop the specific server software name around. I'd recommend naming the function getSoftwareName() for consistency with existing getSoftwareLink() if it is necessary (but I don't think it is for error messages!)

Status & tagging log