Skip to content

Commit

Permalink
fix(compiler): cannot lift a class that extends a cloud resource (#6490)
Browse files Browse the repository at this point in the history
This PR aims to fix an issue where unexpected bugs happen when you try extending a class from the Wing SDK, like `cloud.Bucket`. The root cause is related to how Wings' inheritance is implemented in JavaScript.

Each cloud resource has a JavaScript class modeling its inflight API. The class needs to be constructed with target-specific arguments. For example, for the implementation of `cloud.Bucket`, there's a class definition `BucketClient` for each of the supported cloud targets (`tf-aws`, `tf-gcp`, and `tf-azure`). What's important to note is that each class needs to be instantiated at runtime with value that are specific to that cloud target. For example, `new aws.BucketClient(...)` expects an S3 Bucket name, while `new azure.BucketClient(...)` expects an Azure container name _and_ an Azure storage account name.

How does Wing ensure the right values are passed when the class is constructed? Currently, each preflight class has an `_toInflight()` method which returns a string of JavaScript code. That code instantiates the class and hardcodes in the necessary arguments.
This method is code-generated by the compiler for most Wing classes.

But suppose we have a class `MyBucket` that extends `Bucket`. To instantiate such a class, we'd need to instantiate `MyBucket` with both the arguments needed by `MyBucket` AND the arguments needed by `Bucket`, and ensure that the `super()` call in `MyBucket`'s constructor passes the appropriate arguments to `Bucket`'s constructor. 

To support this, we're now exposing a new method in the TypeScript source code named `_liftedState()` which can be overridden by classes inside our TypeScript SDK implementation to specify what arguments they need when they're instantiated. When `_toInflight()` is called, it will automatically call `_liftedState()` to obtain the constructor arguments _and_ instantiate the class. But if you just call `_toInflightType()`, the class will be returned without instantiating an instance of it. These internal changes should not have any user-facing effects.

Fixes #4203
Fixes #6093
Fixes #6851

## Checklist

- [ ] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [ ] Description explains motivation and solution
- [ ] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
Chriscbr authored Jul 24, 2024
1 parent 562f7c9 commit c901bf6
Show file tree
Hide file tree
Showing 365 changed files with 3,433 additions and 8,478 deletions.
2 changes: 1 addition & 1 deletion apps/wing/src/commands/test/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ async function testTf(synthDir: string, options: TestOptions): Promise<std.TestR

const testArns = await terraformOutput(synthDir, ENV_WING_TEST_RUNNER_FUNCTION_IDENTIFIERS);
const { TestRunnerClient } = await import(testRunnerPath);
const runner = new TestRunnerClient(testArns);
const runner = new TestRunnerClient({ $tests: testArns });

const allTests = await runner.listTests();
const filteredTests = filterTests(allTests, testFilter);
Expand Down
2 changes: 1 addition & 1 deletion docs/api/04-standard-library/cloud/topic.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ AWS implementations of `cloud.Topic` use [AWS SNS](https://docs.aws.amazon.com/s

### Topic <a name="Topic" id="@winglang/sdk.cloud.Topic"></a>

A topic.
A topic for pub/sub notifications.

#### Initializers <a name="Initializers" id="@winglang/sdk.cloud.Topic.Initializer"></a>

Expand Down
49 changes: 49 additions & 0 deletions examples/tests/valid/extend_counter.test.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
bring cloud;
bring util;
bring expect;

let global_value = "yo";

class MyCounter extends cloud.Counter {
pub field1: num;
new() {
this.field1 = 5;
}

pub inflight inc() {
expect.equal(this.field1, 5);
expect.equal(global_value, "yo");
super.inc();
}

pub inflight extra1(): str {
return "extra1";
}
}

class MySuperCounter extends MyCounter {
pub field2: num;
new() {
this.field2 = 10;
}

pub inflight inc() {
expect.equal(this.field2, 10);
expect.equal(this.field1, 5);
expect.equal(global_value, "yo");
super.inc();
}

pub inflight extra2(): str {
return "extra2";
}
}

let c = new MySuperCounter();

test "counter works" {
c.inc();
expect.equal(c.peek(), 1);
expect.equal(c.extra1(), "extra1");
expect.equal(c.extra2(), "extra2");
}
24 changes: 16 additions & 8 deletions libs/awscdk/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ import { isAwsCdkFunction } from "./function";
* AWS Implementation of `cloud.Api`.
*/
export class Api extends cloud.Api implements IAwsApi {
/** @internal */
public static _toInflightType(): string {
return core.InflightClient.forType(
__filename.replace("api", "api.inflight"),
"ApiClient"
);
}

private readonly api: WingRestApi;
private readonly handlers: Record<string, cloud.Function> = {};
private readonly endpoint: cloud.Endpoint;
Expand Down Expand Up @@ -53,7 +61,7 @@ export class Api extends cloud.Api implements IAwsApi {
method: string,
path: string,
inflight: cloud.IApiEndpointHandler,
props?: cloud.ApiEndpointOptions,
props?: cloud.ApiEndpointOptions
): void {
const lowerMethod = method.toLowerCase();
const upperMethod = method.toUpperCase();
Expand Down Expand Up @@ -186,7 +194,7 @@ export class Api extends cloud.Api implements IAwsApi {
inflight: cloud.IApiEndpointHandler,
method: string,
path: string,
props?: cloud.ApiEndpointOptions,
props?: cloud.ApiEndpointOptions
): cloud.Function {
return this.addInflightHandler(inflight, method, path, props);
}
Expand All @@ -201,7 +209,7 @@ export class Api extends cloud.Api implements IAwsApi {
inflight: cloud.IApiEndpointHandler,
method: string,
path: string,
props?: cloud.ApiEndpointOptions,
props?: cloud.ApiEndpointOptions
): cloud.Function {
let handler = this.handlers[inflight._id];
if (!handler) {
Expand All @@ -214,7 +222,7 @@ export class Api extends cloud.Api implements IAwsApi {
this,
App.of(this).makeId(this, prefix),
newInflight,
props,
props
);
this.handlers[inflight._id] = handler;
}
Expand All @@ -229,10 +237,10 @@ export class Api extends cloud.Api implements IAwsApi {
}

/** @internal */
public _toInflight(): string {
return core.InflightClient.for(__dirname, __filename, "ApiClient", [
`process.env["${this.urlEnvName()}"]`,
]);
public _liftedState(): Record<string, string> {
return {
$url: `process.env["${this.urlEnvName()}"]`,
};
}

private urlEnvName(): string {
Expand Down
48 changes: 33 additions & 15 deletions libs/awscdk/src/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ import { BucketDeployment, Source } from "aws-cdk-lib/aws-s3-deployment";
import { LambdaDestination } from "aws-cdk-lib/aws-s3-notifications";
import { Construct } from "constructs";
import { App } from "./app";
import { cloud, core, std } from "@winglang/sdk";
import { cloud, std } from "@winglang/sdk";
import { calculateBucketPermissions } from "@winglang/sdk/lib/shared-aws/permissions";
import { IAwsBucket } from "@winglang/sdk/lib/shared-aws/bucket";
import {
IAwsCdkFunction,
addPolicyStatements,
isAwsCdkFunction,
} from "./function";
import { LiftMap, lift } from "@winglang/sdk/lib/core";
import { LiftMap, lift, InflightClient } from "@winglang/sdk/lib/core";

const EVENTS = {
[cloud.BucketEventType.DELETE]: EventType.OBJECT_REMOVED,
Expand All @@ -31,6 +31,14 @@ const EVENTS = {
* @inflight `@winglang/sdk.cloud.IBucketClient`
*/
export class Bucket extends cloud.Bucket implements IAwsBucket {
/** @internal */
public static _toInflightType(): string {
return InflightClient.forType(
__filename.replace("bucket", "bucket.inflight"),
"BucketClient"
);
}

private readonly bucket: S3Bucket;
private readonly public: boolean;
private bucketDeployment?: BucketDeployment;
Expand Down Expand Up @@ -59,7 +67,10 @@ export class Bucket extends cloud.Bucket implements IAwsBucket {
inflight: cloud.IBucketEventHandler,
opts?: cloud.BucketOnCreateOptions
): IAwsCdkFunction {
const functionHandler = lift({ handler: inflight, eventType: event }).inflight(async (ctx, event) => {
const functionHandler = lift({
handler: inflight,
eventType: event,
}).inflight(async (ctx, event) => {
const record = event.Records[0];
if (!record) {
throw new Error("No record found in the S3 event");
Expand Down Expand Up @@ -112,7 +123,11 @@ export class Bucket extends cloud.Bucket implements IAwsBucket {
inflight: cloud.IBucketEventHandler,
opts?: cloud.BucketOnCreateOptions
): void {
const fn = this.onEventFunction(cloud.BucketEventType.CREATE, inflight, opts);
const fn = this.onEventFunction(
cloud.BucketEventType.CREATE,
inflight,
opts
);

std.Node.of(this).addConnection({
source: this,
Expand All @@ -130,7 +145,11 @@ export class Bucket extends cloud.Bucket implements IAwsBucket {
inflight: cloud.IBucketEventHandler,
opts?: cloud.BucketOnDeleteOptions
): void {
const fn = this.onEventFunction(cloud.BucketEventType.DELETE, inflight, opts);
const fn = this.onEventFunction(
cloud.BucketEventType.DELETE,
inflight,
opts
);

std.Node.of(this).addConnection({
source: this,
Expand All @@ -148,7 +167,11 @@ export class Bucket extends cloud.Bucket implements IAwsBucket {
inflight: cloud.IBucketEventHandler,
opts?: cloud.BucketOnUpdateOptions
): void {
const fn = this.onEventFunction(cloud.BucketEventType.UPDATE, inflight, opts);
const fn = this.onEventFunction(
cloud.BucketEventType.UPDATE,
inflight,
opts
);

std.Node.of(this).addConnection({
source: this,
Expand Down Expand Up @@ -189,15 +212,10 @@ export class Bucket extends cloud.Bucket implements IAwsBucket {
}

/** @internal */
public _toInflight(): string {
return core.InflightClient.for(__dirname, __filename, "BucketClient", [
`process.env["${this.envName()}"]`,
`process.env["${this.isPublicEnvName()}"]`,
]);
}

private isPublicEnvName(): string {
return `${this.envName()}_IS_PUBLIC`;
public _liftedState(): Record<string, string> {
return {
$url: `process.env["${this.envName()}"]`,
};
}

private envName(): string {
Expand Down
22 changes: 15 additions & 7 deletions libs/awscdk/src/counter.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
import { RemovalPolicy } from "aws-cdk-lib";
import { AttributeType, BillingMode, Table } from "aws-cdk-lib/aws-dynamodb";
import { Construct } from "constructs";
import { cloud, core, std } from "@winglang/sdk";
import { cloud, std } from "@winglang/sdk";
import { COUNTER_HASH_KEY } from "@winglang/sdk/lib/shared-aws/commons";
import { calculateCounterPermissions } from "@winglang/sdk/lib/shared-aws/permissions";
import { IAwsCounter } from "@winglang/sdk/lib/shared-aws/counter";
import { addPolicyStatements, isAwsCdkFunction } from "./function";
import { LiftMap } from "@winglang/sdk/lib/core";
import { InflightClient, LiftMap } from "@winglang/sdk/lib/core";

/**
* AWS implementation of `cloud.Counter`.
*
* @inflight `@winglang/sdk.cloud.ICounterClient`
*/
export class Counter extends cloud.Counter implements IAwsCounter {
/** @internal */
public static _toInflightType(): string {
return InflightClient.forType(
__filename.replace("counter", "counter.inflight"),
"CounterClient"
);
}

private readonly table: Table;

constructor(scope: Construct, id: string, props: cloud.CounterProps = {}) {
Expand Down Expand Up @@ -52,11 +60,11 @@ export class Counter extends cloud.Counter implements IAwsCounter {
}

/** @internal */
public _toInflight(): string {
return core.InflightClient.for(__dirname, __filename, "CounterClient", [
`process.env["${this.envName()}"]`,
`${this.initial}`,
]);
public _liftedState(): Record<string, string> {
return {
$tableName: `process.env["${this.envName()}"]`,
$initial: `${this.initial}`,
};
}

private envName(): string {
Expand Down
29 changes: 20 additions & 9 deletions libs/awscdk/src/endpoint.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
import { CfnOutput } from "aws-cdk-lib";
import { Construct } from "constructs";
import { core, cloud, std } from "@winglang/sdk";
import { cloud, std } from "@winglang/sdk";
import { InflightClient } from "@winglang/sdk/lib/core";

/**
* AWS implementation of `cloud.Endpoint`.
*/
export class Endpoint extends cloud.Endpoint {
constructor(scope: Construct, id: string, url: string, props: cloud.EndpointProps = {}) {
/** @internal */
public static _toInflightType(): string {
return InflightClient.forType(
__filename.replace("endpoint", "endpoint.inflight"),
"EndpointClient"
);
}

constructor(
scope: Construct,
id: string,
url: string,
props: cloud.EndpointProps = {}
) {
super(scope, id, url, props);

new CfnOutput(this, "Url", {
Expand All @@ -21,13 +35,10 @@ export class Endpoint extends cloud.Endpoint {
}

/** @internal */
public _toInflight(): string {
return core.InflightClient.for(
__dirname,
__filename,
"EndpointClient",
[`process.env["${this.urlEnvName()}"]`]
);
public _liftedState(): Record<string, string> {
return {
$url: `process.env["${this.urlEnvName()}"]`,
};
}

private urlEnvName(): string {
Expand Down
21 changes: 15 additions & 6 deletions libs/awscdk/src/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import { LogGroup, RetentionDays } from "aws-cdk-lib/aws-logs";
import { Asset } from "aws-cdk-lib/aws-s3-assets";
import { Construct, IConstruct } from "constructs";
import { cloud, std, core } from "@winglang/sdk";
import { cloud, std } from "@winglang/sdk";
import { NotImplementedError } from "@winglang/sdk/lib/core/errors";
import { createBundle } from "@winglang/sdk/lib/shared/bundling";
import {
Expand All @@ -24,7 +24,7 @@ import { makeAwsLambdaHandler } from "@winglang/sdk/lib/shared-aws/function-util
import { resolve } from "path";
import { renameSync, rmSync, writeFileSync } from "fs";
import { App } from "./app";
import { LiftMap } from "@winglang/sdk/lib/core";
import { InflightClient, LiftMap } from "@winglang/sdk/lib/core";

/**
* Implementation of `awscdk.Function` are expected to implement this
Expand Down Expand Up @@ -58,6 +58,14 @@ export class Function
extends cloud.Function
implements IAwsCdkFunction, IAwsFunction
{
/** @internal */
public static _toInflightType(): string {
return InflightClient.forType(
__filename.replace("function", "function.inflight"),
"FunctionClient"
);
}

private readonly function: CdkFunction;
private readonly assetPath: string;

Expand Down Expand Up @@ -151,10 +159,11 @@ export class Function
}

/** @internal */
public _toInflight(): string {
return core.InflightClient.for(__dirname, __filename, "FunctionClient", [
`process.env["${this.envName()}"], "${this.node.path}"`,
]);
public _liftedState(): Record<string, string> {
return {
$functionArn: `process.env["${this.envName()}"]`,
$constructPath: `"${this.node.path}"`,
};
}

/**
Expand Down
Loading

0 comments on commit c901bf6

Please sign in to comment.