diff --git a/tests/tables-carry-column-defaults.test.php b/tests/tables-carry-column-defaults.test.php index 23575f1..e49590c 100644 --- a/tests/tables-carry-column-defaults.test.php +++ b/tests/tables-carry-column-defaults.test.php @@ -63,6 +63,45 @@ function tcStrip($file) return $clean; } +/** + * Lifts one method's body out of a class file by matching braces. + * + * A fixed-length slice does not work: install() is often two lines, so any + * window wide enough to hold a real body runs past the closing brace into the + * uninstall() that usually follows -- and then every plugin looks like it + * drops its own table. Twenty-one false positives on the first run. + * + * @param string $src the source, comments already stripped + * @param string $name the method name + * + * @return string + */ +function tcMethodBody($src, $name) +{ + $at = strpos($src, 'function ' . $name . '('); + if (false === $at) { + return ''; + } + $open = strpos($src, '{', $at); + if (false === $open) { + return ''; + } + $depth = 0; + for ($i = $open, $n = strlen($src); $i < $n; $i++) { + if ($src[$i] === '{') { + $depth++; + } + if ($src[$i] === '}') { + $depth--; + if ($depth === 0) { + return substr($src, $open, $i - $open + 1); + } + } + } + + return ''; +} + /** * Records a check. * @@ -124,6 +163,50 @@ function tcCheck($ok, $message) $short ) ); + // The schema() contract. Plugin::installdb() uses it when it is there and + // falls back to calling install() when it is not -- and that fallback is + // where the destructive pattern lived: install() dropping the table and + // rebuilding it, throwing away the user's rows on every reinstall. + // wolbroadcast was the last plugin on that path. + tcCheck( + false !== strpos($src, 'function createSql('), + sprintf( + '%s builds its table outside a createSql() method, so there is ' + . 'no step 0 for schema() to hand Schema::applyUpdates()', + $short + ) + ); + // Only the plugin's OWN manager needs schema(). Plugin::installdb() + // resolves exactly one class -- Manager -- and a secondary + // manager (an association table, a sub-table) is reached as a STEP inside + // that one's schema(), by design. Requiring schema() of every manager + // flags eleven files that are correct. + $plugin = basename(dirname(dirname($path))); + $isOwnManager = strtolower(basename($path)) + === strtolower($plugin) . 'manager.class.php'; + if ($isOwnManager) { + tcCheck( + false !== strpos($src, 'function schema('), + sprintf( + '%s is its plugin\'s own manager and has no schema(), so ' + . 'Plugin::installdb() falls back to calling install() and ' + . 'the plugin gets no migration tracking at all -- pSchema ' + . 'stays at 0 and an added step never lands.', + $short + ) + ); + } + $install = tcMethodBody($src, 'install'); + tcCheck( + false === strpos($install, 'uninstall('), + sprintf( + '%s drops its own table on install(). uninstall() is a DROP, so ' + . 'reinstalling the plugin -- or repairing it after a failed ' + . 'install -- throws away every row the user entered. Use the ' + . 'non-destructive schema() contract instead.', + $short + ) + ); } tcCheck( diff --git a/wolbroadcast/class/wolbroadcastmanager.class.php b/wolbroadcast/class/wolbroadcastmanager.class.php index 54a55c0..ae6608a 100644 --- a/wolbroadcast/class/wolbroadcastmanager.class.php +++ b/wolbroadcast/class/wolbroadcastmanager.class.php @@ -28,14 +28,15 @@ class WolbroadcastManager extends FOGManagerController */ public $tablename = 'wolbroadcast'; /** - * Perform the database and plugin installation + * Returns the CREATE TABLE (IF NOT EXISTS) statement for this table. * - * @return bool + * Non-destructive and safe to re-run. Used as a step in schema(). + * + * @return string */ - public function install() + public function createSql() { - $this->uninstall(); - $sql = $this->createTableSql( + return $this->createTableSql( $this->tablename, true, [ @@ -62,12 +63,43 @@ public function install() false, false ], - ['wbID'], + [ + 'wbID' + ], 'InnoDB', 'utf8', 'wbID', 'wbID' ); - return self::$DB->query($sql); + } + /** + * The plugin's ordered, append-only schema migration list. Append new + * steps (e.g. "ALTER TABLE `wolbroadcast` ADD COLUMN ...") to the END. + * + * @return array + */ + public function schema() + { + return [ + // 0 + $this->createSql(), + ]; + } + /** + * Installs the database non-destructively (create-if-absent + apply any + * pending additive steps). Does not drop existing data. + * + * This used to call uninstall() first, which DROPS the table -- so + * reinstalling the plugin, or repairing it after a failed install, threw + * away every broadcast address the user had entered. It was the last + * plugin here still doing that; the rest moved to the schema() contract, + * and this brings it in line. + * + * @return bool + */ + public function install() + { + $res = Schema::applyUpdates($this->schema(), 0); + return $res['error'] === null; } }