wolbroadcast: adopt the non-destructive schema() contract

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 -- <PluginName>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(

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;
     }
 }