Skip to content

Commit

Permalink
DB Errors were not printing for schema updates.
Browse files Browse the repository at this point in the history
Schema updates would always return as true because the object was
being returned instead of a boolean as being checked. This would make
it ever more difficult to track down bugs in the sql statements
especially considering there are over 3k lines.
In older versions, even those that did display errors, in some cases
could still blast through and update the schema version. This left
potential unset database items that could be referenced elsewhere
in the system. Due to this the schema version will no longer be updated
past and should stop trying to execute beyond that point.

Address this by:
1. Store the error message into the current property.
2. Run through all the schema updates and collect any errors. Break if error
  is found.
3. Return an http response code that will make updating the database
  from the installer fail to execute.
4. The schema version will not be updated past that point so a bug can be
  reported and hopefully fixed relatively quickly.

Among this schema rethought, cleanup the myisam conversion as it really
shouldn't be needed. It was for some time, but we really shouldn't test
this every time you click a link (it creates more IO to read the tables,
 check types, and iterate over the types to change them to myisam).

Cleanup the report image and snapin log date presentation. Instead of calling
a large sql to group an array to filter, just get the min start date and display
all dates in between (even if nothing happened on those dates).
  • Loading branch information
mastacontrola committed Jan 14, 2017
1 parent 64849f8 commit 477c201
Show file tree
Hide file tree
Showing 13 changed files with 2,111 additions and 2,143 deletions.
8 changes: 8 additions & 0 deletions packages/web/lib/db/pdodb.class.php
Expand Up @@ -25,6 +25,12 @@
*/
class PDODB extends DatabaseManager
{
/**
* Stores last error for query.
*
* @var bool
*/
public $error;
/**
* Stores the current connection
*
Expand Down Expand Up @@ -257,6 +263,7 @@ public function query(
_('No database to work off')
);
}
$this->error = false;
} catch (PDOException $e) {
$msg = sprintf(
'%s %s: %s: %s',
Expand All @@ -272,6 +279,7 @@ public function query(
self::_debugDumpParams()
);
}
$this->error = $msg;
}
return $this;
}
Expand Down
17 changes: 0 additions & 17 deletions packages/web/lib/fog/fogcore.class.php
Expand Up @@ -254,23 +254,6 @@ public static function setSessionEnv()
$_SESSION['HostCount']
);
self::$DB->query($incQuery);
$setEngine = 'SELECT TABLE_NAME from INFORMATION_SCHEMA.TABLES '
. "TABLE_SCHEMA = '"
. DATABASE_NAME
. "' AND ENGINE != 'MyISAM'";
$vals = self::$DB
->query($setEngine)
->fetch(MYSQLI_NUM, 'fetch_all')
->get('TABLE_NAME');
$alterStr = sprintf(
'ALTER TABLE `%s`.`%s` ENGINE=MyISAM',
DATABASE_NAME,
'%s'
);
foreach ((array)$vals as &$table) {
self::$DB->query(sprintf($alterStr, $table));
unset($table);
}
$getSettings = array(
'FOG_DATA_RETURNED',
'FOG_FORMAT_FLAG_IN_GUI',
Expand Down
2 changes: 1 addition & 1 deletion packages/web/lib/fog/system.class.php
Expand Up @@ -53,7 +53,7 @@ private static function _versionCompare()
public function __construct()
{
self::_versionCompare();
define('FOG_VERSION', '1.3.1-RC-7');
define('FOG_VERSION', '8');
define('FOG_SCHEMA', 245);
define('FOG_BCACHE_VER', 111);
define('FOG_SVN_REVISION', 6052);
Expand Down
101 changes: 38 additions & 63 deletions packages/web/lib/pages/reportmanagementpage.class.php
Expand Up @@ -223,37 +223,26 @@ public function imaginglog()
'${field}',
'${input}',
);
$AllDates = self::fastmerge(
self::$DB->query(
"SELECT DATE_FORMAT(`ilStartTime`,'%Y-%m-%d') start FROM "
. "`imagingLog` WHERE DATE_FORMAT(`ilStartTime`,'%Y-%m-%d') "
. "!= '0000-00-00' GROUP BY start ORDER BY start DESC"
)->fetch(
MYSQLI_NUM,
'fetch_all'
)->get('start'),
self::$DB->query(
"SELECT DATE_FORMAT(`ilFinishTime`,'%Y-%m-%d') finish FROM "
. "`imagingLog` WHERE DATE_FORMAT(`ilFinishTime`,'%Y-%m-%d') "
. "!= '0000-00-00' GROUP BY finish ORDER BY finish DESC"
)->fetch(
MYSQLI_NUM,
'fetch_all'
)->get('start')
);
foreach ((array)$AllDates as &$Date) {
if (is_string($Date)) {
$Date = array($Date);
}
$tmp = array_shift($Date);
if (!self::validDate($tmp)) {
continue;
}
$Dates[] = $tmp;
unset($Date, $tmp);
$start = self::$DB->query(
"SELECT DATE_FORMAT(MIN(`ilStartTime`),'%Y-%m-%d') start FROM "
. "`imagingLog`"
)->fetch(
MYSQLI_NUM,
'fetch'
)->get('start');
$start = self::niceDate($start);
$end = self::niceDate();
if (!self::validDate($start)) {
return $this->render();
}
$interval = new DateInterval('P1D');
$daterange = new DatePeriod($start, $interval, $end);
$dates = iterator_to_array($daterange);
foreach ((array)$dates as &$date) {
$Dates[] = $date->format('Y-m-d');
unset($Date);
}
unset($AllDates);
$Dates = array_unique($Dates);
unset($dates);
rsort($Dates);
if (count($Dates) > 0) {
ob_start();
Expand Down Expand Up @@ -1418,40 +1407,26 @@ public function snapinlog()
'${field}',
'${input}',
);
$AllDates = self::fastmerge(
self::$DB->query(
"SELECT DATE_FORMAT(`stCheckinDate`,'%Y-%m-%d') "
. "start FROM `snapinTasks` WHERE DATE_FORMAT("
. "`stCheckinDate`,'%Y-%m-%d') != '0000-00-00' "
. "GROUP BY start ORDER BY start DESC"
)->fetch(
MYSQLI_NUM,
'fetch_all'
)->get('start'),
self::$DB->query(
"SELECT DATE_FORMAT(`stCompleteDate`,'%Y-%m-%d') "
. "finish FROM `snapinTasks` WHERE DATE_FORMAT("
. "`stCompleteDate`,'%Y-%m-%d') != '0000-00-00' "
. "GROUP BY finish ORDER BY finish DESC"
)->fetch(
MYSQLI_NUM,
'fetch_all'
)->get('start')
);
foreach ((array)$AllDates as &$Date) {
$tmp = (
!is_array($Date) ?
$Date :
array_shift($Date)
);
if (!self::validDate($tmp)) {
continue;
}
$Dates[] = $tmp;
unset($Date, $tmp);
$start = self::$DB->query(
"SELECT DATE_FORMAT(MIN(`stCheckinDate`),'%Y-%m-%d') start FROM "
. "`snapinTasks`"
)->fetch(
MYSQLI_NUM,
'fetch'
)->get('start');
$start = self::niceDate($start);
$end = self::niceDate();
if (!self::validDate($start)) {
return $this->render();
}
$interval = new DateInterval('P1D');
$daterange = new DatePeriod($start, $interval, $end);
$dates = iterator_to_array($daterange);
foreach ((array)$dates as &$date) {
$Dates[] = $date->format('Y-m-d');
unset($Date);
}
unset($AllDates);
$Dates = array_unique($Dates);
unset($dates);
rsort($Dates);
if (count($Dates) > 0) {
ob_start();
Expand Down
14 changes: 8 additions & 6 deletions packages/web/lib/pages/schemaupdaterpage.class.php
Expand Up @@ -159,7 +159,7 @@ public function indexPost()
if (is_string($result)) {
$errors[] = sprintf(
'<p><b>%s %s:</b>'
. ' %s</p><p><b>%s %s:</b>'
. ' %s<br/><br/><b>%s %s:</b>'
. ' <pre>%s</pre></p>'
. '<p><b>%s:</b>'
. ' <pre>%s</pre></p>',
Expand All @@ -173,10 +173,10 @@ public function indexPost()
print_r($update, 1)
);
}
} elseif (false === self::$DB->query($update)) {
} elseif (false !== self::$DB->query($update)->error) {
$errors[] = sprintf(
'<p><b>%s %s:</b>'
. ' %s</p><p><b>%s %s:</b>'
. ' %s<br/><br/><b>%s %s:</b>'
. ' <pre>%s</pre></p>'
. '<p><b>%s:</b>'
. ' <pre>%s</pre></p>',
Expand All @@ -185,7 +185,7 @@ public function indexPost()
$version,
_('Database'),
_('Error'),
self::$DB->sqlerror(),
self::$DB->error,
_('Database SQL'),
$update
);
Expand All @@ -194,7 +194,9 @@ public function indexPost()
}
$newSchema = self::getClass('Schema', 1)
->set('version', ++$version);
if (!$newSchema->save()) {
if (count($errors) > 0
|| !$newSchema->save()
) {
$fatalerrmsg = '';
$fatalerrmsg = sprintf(
'<p>%s</p>',
Expand Down Expand Up @@ -231,7 +233,7 @@ public function indexPost()
echo $text;
} catch (Exception $e) {
printf('<p>%s</p>', $e->getMessage());
exit(1);
http_response_code(404);
}
}
}

0 comments on commit 477c201

Please sign in to comment.