Add test migrating between local and synced

Author: simolus3

crates/core/src/schema/management.rs

@@ -38,36 +38,55 @@ fn update_tables(db: *mut sqlite::sqlite3, schema: &Schema) -> Result<(), PowerS
     {
         // In a block so that all statements are finalized before dropping tables.
         for table in &schema.tables {
-            if let Some(_) = existing_tables.remove(&*table.name) {
-                // This table exists already, nothing to do.
-                // TODO: Handle switch between local only <-> regular tables?
-            } else {
-                // New table.
-                let quoted_internal_name = quote_identifier(&table.internal_name());
-
-                db.exec_safe(&format!(
-                    "CREATE TABLE {:}(id TEXT PRIMARY KEY NOT NULL, data TEXT)",
-                    quoted_internal_name
-                ))
-                .into_db_result(db)?;
+            if let Some(existing) = existing_tables.remove(&*table.name) {
+                if existing.local_only && !table.local_only() {
+                    // Migrate from a local-only to a synced table. Because none of the local writes
+                    // would have created CRUD entries, we'll have to re-create the table from
+                    // scratch.
+
+                    // To delete the old existing table in the end.
+                    existing_tables.insert(&existing.name, existing);
+                } else if !existing.local_only && table.local_only() {
+                    // Migrate from a synced table to a local-only table. We can keep existing rows
+                    // and will also keep existing CRUD data to be uploaded before the switch.
+                    db.exec_safe(&format!(
+                        "ALTER TABLE {} RENAME TO {}",
+                        quote_identifier(&existing.internal_name),
+                        quote_identifier(&table.internal_name()),
+                    ))
+                    .into_db_result(db)?;
+                    continue;
+                } else {
+                    // Identical table exists already, nothing to do.
+                    continue;
+                }
+            }
+
+            // New table.
+            let quoted_internal_name = quote_identifier(&table.internal_name());
+
+            db.exec_safe(&format!(
+                "CREATE TABLE {:}(id TEXT PRIMARY KEY NOT NULL, data TEXT)",
+                quoted_internal_name
+            ))
+            .into_db_result(db)?;
 
-                if !table.local_only() {
-                    // MOVE data if any
-                    db.exec_text(
-                        &format!(
-                            "INSERT INTO {:}(id, data)
+            if !table.local_only() {
+                // MOVE data if any
+                db.exec_text(
+                    &format!(
+                        "INSERT INTO {:}(id, data)
     SELECT id, data
     FROM ps_untyped
     WHERE type = ?",
-                            quoted_internal_name
-                        ),
-                        &table.name,
-                    )
-                    .into_db_result(db)?;
+                        quoted_internal_name
+                    ),
+                    &table.name,
+                )
+                .into_db_result(db)?;
 
-                    // language=SQLite
-                    db.exec_text("DELETE FROM ps_untyped WHERE type = ?", &table.name)?;
-                }
+                // language=SQLite
+                db.exec_text("DELETE FROM ps_untyped WHERE type = ?", &table.name)?;
             }
         }
 

dart/test/schema_test.dart

@@ -45,6 +45,79 @@ void main() {
           greaterThan(versionAfter2['schema_version'] as int));
     });
 
+    group('migrate tables', () {
+      final local = {
+        "tables": [
+          {
+            "name": "users",
+            "local_only": true,
+            "insert_only": false,
+            "columns": [
+              {"name": "name", "type": "TEXT"},
+            ],
+          },
+        ]
+      };
+
+      final synced = {
+        "tables": [
+          {
+            "name": "users",
+            "local_only": false,
+            "insert_only": false,
+            "columns": [
+              {"name": "name", "type": "TEXT"},
+            ],
+          },
+        ]
+      };
+
+      test('from synced to local', () {
+        // Start with synced table, and sync row
+        db.execute('SELECT powersync_replace_schema(?)', [json.encode(synced)]);
+        db.execute(
+          'INSERT INTO ps_data__users (id, data) VALUES (?, ?)',
+          [
+            'synced-id',
+            json.encode({'name': 'name'})
+          ],
+        );
+
+        // Migrate to local table.
+        db.execute('SELECT powersync_replace_schema(?)', [json.encode(local)]);
+
+        // The synced table should not exist anymore.
+        expect(() => db.select('SELECT * FROM ps_data__users'),
+            throwsA(isA<SqliteException>()));
+
+        // Data should still be there.
+        expect(db.select('SELECT * FROM users'), [
+          {'id': 'synced-id', 'name': 'name'}
+        ]);
+
+        // Inserting into local-only table should not record CRUD item.
+        db.execute(
+            'INSERT INTO users (id, name) VALUES (uuid(), ?)', ['local']);
+        expect(db.select('SELECT * FROM ps_crud'), isEmpty);
+      });
+
+      test('from local to synced', () {
+        // Start with local table, and local row
+        db.execute('SELECT powersync_replace_schema(?)', [json.encode(local)]);
+        db.execute(
+            'INSERT INTO users (id, name) VALUES (uuid(), ?)', ['local']);
+
+        // Migrate to synced table. Because the previous local write would never
+        // get uploaded, this clears local data.
+        db.execute('SELECT powersync_replace_schema(?)', [json.encode(synced)]);
+        expect(db.select('SELECT * FROM users'), isEmpty);
+
+        // The local table should not exist anymore.
+        expect(() => db.select('SELECT * FROM ps_data_local__users'),
+            throwsA(isA<SqliteException>()));
+      });
+    });
+
     group('metadata', () {
       // This is a special because we have two delete triggers when
       // include_metadata is true (one for actual `DELETE` statements and one