Fix publish type backwards compatibility (#1910)

* Progress fixing type backwards compatibility

* Progress to class extension error

* Move reset table site def back to callsites

* Fix class inheritence error

* Fix various type conflict issues

* Fix type parameter conversion

* Fix lint

* Bump version

Author: Elias Kassell

core/actions/incremental_table.ts

@@ -1,11 +1,15 @@
 import { verifyObjectMatchesProto, VerifyProtoErrorBehaviour } from "df/common/protos";
 import {
   ActionBuilder,
-  ILegacyTableBigqueryConfig,
+  ILegacyBigQueryOptions,
+  ILegacyTableConfig,
   ITableContext,
-  LegacyConfigConverter
+  LegacyConfigConverter,
+  TableType
 } from "df/core/actions";
 import { Assertion } from "df/core/actions/assertion";
+import { Table } from "df/core/actions/table";
+import { View } from "df/core/actions/view";
 import { ColumnDescriptors } from "df/core/column_descriptors";
 import { Contextable, Resolvable } from "df/core/common";
 import * as Path from "df/core/path";
@@ -25,25 +29,6 @@ import {
 } from "df/core/utils";
 import { dataform } from "df/protos/ts";
 
-/**
- * @hidden
- * This maintains backwards compatability with older versions.
- * TODO(ekrekr): consider breaking backwards compatability of these in v4.
- */
-export interface ILegacyIncrementalTableConfig
-  extends dataform.ActionConfig.IncrementalTableConfig {
-  dependencies: Resolvable[];
-  database: string;
-  schema: string;
-  fileName: string;
-  type: string;
-  bigquery?: ILegacyTableBigqueryConfig;
-  // Legacy incremental table config's table assertions cannot directly extend the protobuf
-  // incremental table config definition because of legacy incremental table config's flexible
-  // types.
-  assertions: any;
-}
-
 /**
  * @hidden
  */
@@ -71,9 +56,15 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
   private uniqueKeyAssertions: Assertion[] = [];
   private rowConditionsAssertion: Assertion;
 
+  private unverifiedConfig: any;
+  private configPath: string | undefined;
+
   constructor(session?: Session, unverifiedConfig?: any, configPath?: string) {
     super(session);
     this.session = session;
+    this.configPath = configPath;
+    // A copy is used here to prevent manipulation of the original.
+    this.unverifiedConfig = Object.assign({}, unverifiedConfig);
 
     if (!unverifiedConfig) {
       return;
@@ -158,11 +149,48 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
     if (config.filename) {
       this.proto.fileName = config.filename;
     }
-    this.proto.onSchemaChange = this.mapOnSchemaChange(config.onSchemaChange)
+    this.proto.onSchemaChange = this.mapOnSchemaChange(config.onSchemaChange);
 
     return this;
   }
 
+  /**
+   * @hidden
+   * @deprecated
+   * Deprecated in favor of action type can being set in the configs passed to action constructor
+   * functions.
+   */
+  public type(type: TableType) {
+    let newAction: View | Table;
+    switch (type) {
+      case "table":
+        newAction = new Table(
+          this.session,
+          { ...this.unverifiedConfig, type: "table" },
+          this.configPath
+        );
+        break;
+      case "incremental":
+        return this;
+      case "view":
+        newAction = new View(
+          this.session,
+          { ...this.unverifiedConfig, type: "view" },
+          this.configPath
+        );
+        break;
+      default:
+        throw new Error(`Unexpected table type: ${type}`);
+    }
+    const existingAction = this.session.actions.indexOf(this);
+    if (existingAction === -1) {
+      throw Error(
+        "Expected pre-existing action, but none found. Please report this to the Dataform team."
+      );
+    }
+    this.session.actions[existingAction] = newAction;
+  }
+
   public query(query: Contextable<ITableContext, string>) {
     this.contextableQuery = query;
     return this;
@@ -398,8 +426,15 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
     return protoOps;
   }
 
+  /**
+   * Verify config checks that the constructor provided config matches the expected proto structure,
+   * or the previously accepted legacy structure. If the legacy structure is used, it is converted
+   * to the new structure.
+   */
   private verifyConfig(
-    unverifiedConfig: ILegacyIncrementalTableConfig
+    // `any` is used here to facilitate the type merging of the legacy table config, which is very
+    // different to the new structure.
+    unverifiedConfig: dataform.ActionConfig.IncrementalTableConfig | ILegacyTableConfig | any
   ): dataform.ActionConfig.IncrementalTableConfig {
     // The "type" field only exists on legacy incremental table configs. Here we convert them to the
     // new format.
@@ -441,7 +476,7 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
             throw e;
           },
           unverifiedConfig.bigquery,
-          strictKeysOf<ILegacyTableBigqueryConfig>()([
+          strictKeysOf<ILegacyBigQueryOptions>()([
             "partitionBy",
             "clusterBy",
             "updatePartitionFilter",
@@ -471,27 +506,37 @@ export class IncrementalTable extends ActionBuilder<dataform.Table> {
   // - for sqlx it will have type "string"
   // - for action.yaml it will be converted to enum which is represented
   // in TypeScript as a "number".
-  private mapOnSchemaChange(onSchemaChange?: string|number) : dataform.OnSchemaChange {
+  private mapOnSchemaChange(onSchemaChange?: string | number): dataform.OnSchemaChange {
     if (!onSchemaChange) {
       return dataform.OnSchemaChange.IGNORE;
     }
 
     if (typeof onSchemaChange === "number") {
       switch (onSchemaChange) {
-        case dataform.ActionConfig.OnSchemaChange.IGNORE: return dataform.OnSchemaChange.IGNORE;
-        case dataform.ActionConfig.OnSchemaChange.FAIL: return dataform.OnSchemaChange.FAIL;
-        case dataform.ActionConfig.OnSchemaChange.EXTEND: return dataform.OnSchemaChange.EXTEND;
-        case dataform.ActionConfig.OnSchemaChange.SYNCHRONIZE: return dataform.OnSchemaChange.SYNCHRONIZE;
-        default: throw new Error(`OnSchemaChange value "${onSchemaChange}" is not supported`);
+        case dataform.ActionConfig.OnSchemaChange.IGNORE:
+          return dataform.OnSchemaChange.IGNORE;
+        case dataform.ActionConfig.OnSchemaChange.FAIL:
+          return dataform.OnSchemaChange.FAIL;
+        case dataform.ActionConfig.OnSchemaChange.EXTEND:
+          return dataform.OnSchemaChange.EXTEND;
+        case dataform.ActionConfig.OnSchemaChange.SYNCHRONIZE:
+          return dataform.OnSchemaChange.SYNCHRONIZE;
+        default:
+          throw new Error(`OnSchemaChange value "${onSchemaChange}" is not supported`);
       }
     }
 
     switch (onSchemaChange.toString().toUpperCase()) {
-      case "IGNORE": return dataform.OnSchemaChange.IGNORE;
-      case "FAIL": return dataform.OnSchemaChange.FAIL;
-      case "EXTEND": return dataform.OnSchemaChange.EXTEND;
-      case "SYNCHRONIZE": return dataform.OnSchemaChange.SYNCHRONIZE;
-      default: throw new Error(`OnSchemaChange value "${onSchemaChange}" is not supported`);
+      case "IGNORE":
+        return dataform.OnSchemaChange.IGNORE;
+      case "FAIL":
+        return dataform.OnSchemaChange.FAIL;
+      case "EXTEND":
+        return dataform.OnSchemaChange.EXTEND;
+      case "SYNCHRONIZE":
+        return dataform.OnSchemaChange.SYNCHRONIZE;
+      default:
+        throw new Error(`OnSchemaChange value "${onSchemaChange}" is not supported`);
     }
   }
 }

core/actions/index.ts

@@ -1,12 +1,19 @@
 import { Assertion } from "df/core/actions/assertion";
 import { DataPreparation } from "df/core/actions/data_preparation";
 import { Declaration } from "df/core/actions/declaration";
-import { ILegacyIncrementalTableConfig, IncrementalTable } from "df/core/actions/incremental_table";
+import { IncrementalTable } from "df/core/actions/incremental_table";
 import { Notebook } from "df/core/actions/notebook";
 import { Operation } from "df/core/actions/operation";
-import { ILegacyTableConfig, Table } from "df/core/actions/table";
-import { ILegacyViewConfig, View } from "df/core/actions/view";
-import { ICommonContext } from "df/core/common";
+import { Table } from "df/core/actions/table";
+import { View } from "df/core/actions/view";
+import {
+  IActionConfig,
+  ICommonContext,
+  IDependenciesConfig,
+  IDocumentableConfig,
+  INamedConfig,
+  ITargetableConfig
+} from "df/core/common";
 import { Session } from "df/core/session";
 import { dataform } from "df/protos/ts";
 
@@ -114,6 +121,71 @@ export interface ITableContext extends ICommonContext {
   incremental: () => boolean;
 }
 
+/**
+ * @hidden
+ * @deprecated
+ * This is no longer needed other than for legacy backwards compatibility purposes, as tables are
+ * now configured in separate actions.
+ */
+export type TableType = typeof TableType[number];
+
+/**
+ * @hidden
+ * @deprecated
+ * This is no longer needed other than for legacy backwards compatibility purposes, as tables are
+ * now configured in separate actions.
+ */
+export const TableType = ["table", "view", "incremental"] as const;
+
+/**
+ * @hidden
+ * @deprecated
+ * These options are only here to preserve backwards compatibility of legacy config options.
+ * consider breaking backwards compatability of this in v4.
+ */
+export interface ILegacyTableConfig
+  extends IActionConfig,
+    IDependenciesConfig,
+    IDocumentableConfig,
+    INamedConfig,
+    ITargetableConfig {
+  type?: TableType;
+  protected?: boolean;
+  bigquery?: ILegacyBigQueryOptions;
+  assertions?: ILegacyTableAssertions;
+  uniqueKey?: string[];
+  materialized?: boolean;
+}
+
+/**
+ * @hidden
+ * @deprecated
+ * These options are only here to preserve backwards compatibility of legacy config options.
+ * consider breaking backwards compatability of this in v4.
+ */
+export interface ILegacyBigQueryOptions {
+  partitionBy?: string;
+  clusterBy?: string[];
+  updatePartitionFilter?: string;
+  labels?: { [name: string]: string };
+  partitionExpirationDays?: number;
+  requirePartitionFilter?: boolean;
+  additionalOptions?: { [name: string]: string };
+}
+
+/**
+ * @hidden
+ * @deprecated
+ * These options are only here to preserve backwards compatibility of legacy config options.
+ * consider breaking backwards compatability of this in v4.
+ */
+export interface ILegacyTableAssertions {
+  uniqueKey?: string | string[];
+  uniqueKeys?: string[][];
+  nonNull?: string | string[];
+  rowConditions?: string[];
+}
+
 export class LegacyConfigConverter {
   // This is a workaround to make bigquery options output empty fields with the same behaviour as
   // they did previously.
@@ -137,9 +209,11 @@ export class LegacyConfigConverter {
     return bigqueryFiltered;
   }
 
-  public static insertLegacyInlineAssertionsToConfigProto<
-    T extends ILegacyTableConfig | ILegacyIncrementalTableConfig | ILegacyViewConfig
-  >(legacyConfig: T): T {
+  public static insertLegacyInlineAssertionsToConfigProto<T extends ILegacyTableConfig>(
+    unverifiedConfig: T
+  ): T {
+    // Type `any` is used here to facilitate the type hacking for legacy compatibility.
+    const legacyConfig: any = unverifiedConfig;
     if (legacyConfig?.assertions) {
       if (typeof legacyConfig.assertions?.uniqueKey === "string") {
         legacyConfig.assertions.uniqueKey = [legacyConfig.assertions.uniqueKey];
@@ -158,9 +232,11 @@ export class LegacyConfigConverter {
     return legacyConfig;
   }
 
-  public static insertLegacyBigQueryOptionsToConfigProto<
-    T extends ILegacyTableConfig | ILegacyIncrementalTableConfig
-  >(legacyConfig: T): T {
+  public static insertLegacyBigQueryOptionsToConfigProto<T extends ILegacyTableConfig>(
+    unverifiedConfig: T
+  ): T {
+    // Type `any` is used here to facilitate the type hacking for legacy compatibility.
+    const legacyConfig: any = unverifiedConfig;
     if (!legacyConfig?.bigquery) {
       return legacyConfig;
     }
@@ -173,8 +249,7 @@ export class LegacyConfigConverter {
       delete legacyConfig.bigquery.clusterBy;
     }
     if (!!legacyConfig.bigquery.updatePartitionFilter) {
-      (legacyConfig as ILegacyIncrementalTableConfig).updatePartitionFilter =
-        legacyConfig.bigquery.updatePartitionFilter;
+      legacyConfig.updatePartitionFilter = legacyConfig.bigquery.updatePartitionFilter;
       delete legacyConfig.bigquery.updatePartitionFilter;
     }
     if (!!legacyConfig.bigquery.labels) {
@@ -201,13 +276,3 @@ export class LegacyConfigConverter {
     return legacyConfig;
   }
 }
-
-export interface ILegacyTableBigqueryConfig {
-  partitionBy?: string;
-  clusterBy?: string[];
-  updatePartitionFilter?: string;
-  labels?: { [key: string]: string };
-  partitionExpirationDays?: number;
-  requirePartitionFilter?: boolean;
-  additionalOptions?: { [key: string]: string };
-}