Implement schemaValidation mode handling in firebase dataconnect:sql:diff (#7633)

* Compat mode support.

* Support handling schema validation options in `diff`.

* Fix build.

* Format.

* Don't output second set of diffs if they're equal to the first.

* Address review comments.

* Tweak output logs.

Author: rosalyntan

src/commands/dataconnect-sql-diff.ts

@@ -22,6 +22,9 @@ export const command = new Command("dataconnect:sql:diff [serviceId]")
     await ensureApis(projectId);
     const serviceInfo = await pickService(projectId, options.config, serviceId);
 
-    const diffs = await diffSchema(serviceInfo.schema);
+    const diffs = await diffSchema(
+      serviceInfo.schema,
+      serviceInfo.dataConnectYaml.schema.datasource.postgresql?.schemaValidation,
+    );
     return { projectId, serviceId, diffs };
   });

src/dataconnect/schemaMigration.ts

@@ -1,7 +1,7 @@
 import * as clc from "colorette";
 import { format } from "sql-formatter";
 
-import { IncompatibleSqlSchemaError, Diff, SCHEMA_ID } from "./types";
+import { IncompatibleSqlSchemaError, Diff, SCHEMA_ID, SchemaValidation } from "./types";
 import { getSchema, upsertSchema, deleteConnector } from "./client";
 import {
   setupIAMUsers,
@@ -23,21 +23,42 @@ import { logLabeledBullet, logLabeledWarning, logLabeledSuccess } from "../utils
 import * as experiments from "../experiments";
 import * as errors from "./errors";
 
-export async function diffSchema(schema: Schema): Promise<Diff[]> {
+export async function diffSchema(
+  schema: Schema,
+  schemaValidation?: SchemaValidation,
+): Promise<Diff[]> {
   const { serviceName, instanceName, databaseId } = getIdentifiers(schema);
   await ensureServiceIsConnectedToCloudSql(
     serviceName,
     instanceName,
     databaseId,
     /* linkIfNotConnected=*/ false,
   );
+  let diffs: Diff[] = [];
+
+  let validationMode: SchemaValidation = "STRICT";
+  if (experiments.isEnabled("fdccompatiblemode")) {
+    if (!schemaValidation) {
+      // If the schema validation mode is unset, we surface both STRICT and COMPATIBLE mode diffs, starting with COMPATIBLE.
+      validationMode = "COMPATIBLE";
+    } else {
+      validationMode = schemaValidation;
+    }
+  }
+  setSchemaValidationMode(schema, validationMode);
 
-  setCompatibleMode(schema, databaseId, instanceName);
   try {
+    if (!schemaValidation && experiments.isEnabled("fdccompatiblemode")) {
+      logLabeledBullet("dataconnect", `generating required schema changes...`);
+    }
     await upsertSchema(schema, /** validateOnly=*/ true);
-    logLabeledSuccess("dataconnect", `Database schema is up to date.`);
+    if (validationMode === "STRICT") {
+      logLabeledSuccess("dataconnect", `Database schema is up to date.`);
+    } else {
+      logLabeledSuccess("dataconnect", `Database schema is compatible.`);
+    }
   } catch (err: any) {
-    if (err.status !== 400) {
+    if (err?.status !== 400) {
       throw err;
     }
     const invalidConnectors = errors.getInvalidConnectors(err);
@@ -52,11 +73,47 @@ export async function diffSchema(schema: Schema): Promise<Diff[]> {
       displayInvalidConnectors(invalidConnectors);
     }
     if (incompatible) {
-      displaySchemaChanges(incompatible);
-      return incompatible.diffs;
+      displaySchemaChanges(incompatible, validationMode, instanceName, databaseId);
+      diffs = incompatible.diffs;
     }
   }
-  return [];
+
+  if (experiments.isEnabled("fdccompatiblemode")) {
+    // If the validation mode is unset, then we also surface any additional optional STRICT diffs.
+    if (!schemaValidation) {
+      validationMode = "STRICT";
+      setSchemaValidationMode(schema, validationMode);
+      try {
+        logLabeledBullet("dataconnect", `generating schema changes, including optional changes...`);
+        await upsertSchema(schema, /** validateOnly=*/ true);
+        logLabeledSuccess("dataconnect", `no additional optional changes`);
+      } catch (err: any) {
+        if (err?.status !== 400) {
+          throw err;
+        }
+        const incompatible = errors.getIncompatibleSchemaError(err);
+        if (incompatible) {
+          if (!diffsEqual(diffs, incompatible.diffs)) {
+            if (diffs.length === 0) {
+              displaySchemaChanges(
+                incompatible,
+                "STRICT_AFTER_COMPATIBLE",
+                instanceName,
+                databaseId,
+              );
+            } else {
+              displaySchemaChanges(incompatible, validationMode, instanceName, databaseId);
+            }
+            // Return STRICT diffs if the --json flag is passed and schemaValidation is unset.
+            diffs = incompatible.diffs;
+          } else {
+            logLabeledSuccess("dataconnect", `no additional optional changes`);
+          }
+        }
+      }
+    }
+  }
+  return diffs;
 }
 
 export async function migrateSchema(args: {
@@ -75,13 +132,14 @@ export async function migrateSchema(args: {
     /* linkIfNotConnected=*/ true,
   );
 
-  setCompatibleMode(schema, databaseId, instanceName);
+  const validationMode = experiments.isEnabled("fdccompatiblemode") ? "COMPATIBLE" : "STRICT";
+  setSchemaValidationMode(schema, validationMode);
 
   try {
     await upsertSchema(schema, validateOnly);
     logger.debug(`Database schema was up to date for ${instanceId}:${databaseId}`);
   } catch (err: any) {
-    if (err.status !== 400) {
+    if (err?.status !== 400) {
       throw err;
     }
     // Parse and handle failed precondition errors, then retry.
@@ -94,9 +152,11 @@ export async function migrateSchema(args: {
 
     const migrationMode = await promptForSchemaMigration(
       options,
+      instanceName,
       databaseId,
       incompatible,
       validateOnly,
+      validationMode,
     );
 
     const shouldDeleteInvalidConnectors = await promptForInvalidConnectorError(
@@ -129,22 +189,26 @@ export async function migrateSchema(args: {
   return [];
 }
 
-function setCompatibleMode(schema: Schema, databaseId: string, instanceName: string) {
-  if (experiments.isEnabled("fdccompatiblemode")) {
-    if (schema.primaryDatasource.postgresql?.schemaValidation) {
-      schema.primaryDatasource.postgresql.schemaValidation = "COMPATIBLE";
-    } else {
-      schema.primaryDatasource = {
-        postgresql: {
-          database: databaseId,
-          cloudSql: {
-            instance: instanceName,
-          },
-          schemaValidation: "COMPATIBLE",
-        },
-      };
+function diffsEqual(x: Diff[], y: Diff[]): boolean {
+  if (x.length !== y.length) {
+    return false;
+  }
+  for (let i = 0; i < x.length; i++) {
+    if (
+      x[i].description !== y[i].description ||
+      x[i].destructive !== y[i].destructive ||
+      x[i].sql !== y[i].sql
+    ) {
+      return false;
     }
   }
+  return true;
+}
+
+function setSchemaValidationMode(schema: Schema, schemaValidation: SchemaValidation) {
+  if (experiments.isEnabled("fdccompatiblemode") && schema.primaryDatasource.postgresql) {
+    schema.primaryDatasource.postgresql.schemaValidation = schemaValidation;
+  }
 }
 
 function getIdentifiers(schema: Schema): {
@@ -274,14 +338,16 @@ async function handleIncompatibleSchemaError(args: {
 
 async function promptForSchemaMigration(
   options: Options,
-  databaseName: string,
+  instanceName: string,
+  databaseId: string,
   err: IncompatibleSqlSchemaError | undefined,
   validateOnly: boolean,
+  schemaValidation: SchemaValidation,
 ): Promise<"none" | "all"> {
   if (!err) {
     return "none";
   }
-  displaySchemaChanges(err);
+  displaySchemaChanges(err, schemaValidation, instanceName, databaseId);
   if (!options.nonInteractive) {
     if (validateOnly && options.force) {
       // `firebase dataconnect:sql:migrate --force` performs all migrations
@@ -299,7 +365,7 @@ async function promptForSchemaMigration(
           { name: "Abort changes", value: "none" },
         ];
     return await promptOnce({
-      message: `Would you like to execute these changes against ${databaseName}?`,
+      message: `Would you like to execute these changes against ${databaseId}?`,
       type: "list",
       choices,
     });
@@ -434,21 +500,51 @@ async function ensureServiceIsConnectedToCloudSql(
   try {
     await upsertSchema(currentSchema, /** validateOnly=*/ false);
   } catch (err: any) {
-    if (err.status >= 500) {
+    if (err?.status >= 500) {
       throw err;
     }
     logger.debug(err);
   }
 }
 
-function displaySchemaChanges(error: IncompatibleSqlSchemaError) {
+function displaySchemaChanges(
+  error: IncompatibleSqlSchemaError,
+  schemaValidation: SchemaValidation | "STRICT_AFTER_COMPATIBLE",
+  instanceName: string,
+  databaseId: string,
+) {
   switch (error.violationType) {
     case "INCOMPATIBLE_SCHEMA":
       {
-        const message =
-          "Your new schema is incompatible with the schema of your CloudSQL database. " +
-          "The following SQL statements will migrate your database schema to match your new Data Connect schema.\n" +
-          error.diffs.map(toString).join("\n");
+        let message;
+        if (schemaValidation === "COMPATIBLE") {
+          message =
+            "Your new application schema is incompatible with the schema of your PostgreSQL database " +
+            databaseId +
+            " in your CloudSQL instance " +
+            instanceName +
+            ". " +
+            "The following SQL statements will migrate your database schema to be compatible with your new Data Connect schema.\n" +
+            error.diffs.map(toString).join("\n");
+        } else if (schemaValidation === "STRICT_AFTER_COMPATIBLE") {
+          message =
+            "Your new application schema is compatible with the schema of your PostgreSQL database " +
+            databaseId +
+            " in your CloudSQL instance " +
+            instanceName +
+            ", but contains unused tables or columns. " +
+            "The following optional SQL statements will migrate your database schema to match your new Data Connect schema.\n" +
+            error.diffs.map(toString).join("\n");
+        } else {
+          message =
+            "Your new application schema does not match the schema of your PostgreSQL database " +
+            databaseId +
+            " in your CloudSQL instance " +
+            instanceName +
+            ". " +
+            "The following SQL statements will migrate your database schema to match your new Data Connect schema.\n" +
+            error.diffs.map(toString).join("\n");
+        }
         logLabeledWarning("dataconnect", message);
       }
       break;

src/dataconnect/types.ts

@@ -29,10 +29,12 @@ export interface Datasource {
   postgresql?: PostgreSql;
 }
 
+export type SchemaValidation = "STRICT" | "COMPATIBLE";
+
 export interface PostgreSql {
   database: string;
   cloudSql: CloudSqlInstance;
-  schemaValidation?: "STRICT" | "COMPATIBLE" | "NONE" | "SQL_SCHEMA_VALIDATION_UNSPECIFIED";
+  schemaValidation?: SchemaValidation | "NONE" | "SQL_SCHEMA_VALIDATION_UNSPECIFIED";
 }
 
 export interface CloudSqlInstance {
@@ -117,6 +119,7 @@ export interface DatasourceYaml {
     cloudSql: {
       instanceId: string;
     };
+    schemaValidation?: SchemaValidation;
   };
 }
 
@@ -182,6 +185,7 @@ export function toDatasource(
         cloudSql: {
           instance: `projects/${projectId}/locations/${locationId}/instances/${ds.postgresql.cloudSql.instanceId}`,
         },
+        schemaValidation: ds.postgresql.schemaValidation,
       },
     };
   }