Skip to content

Commit

Permalink
refactor: Move app aware logic into a single location
Browse files Browse the repository at this point in the history
When a construct is app aware we:
  - Suffix it's ID with the app string
  - Add the `App` tag

Currently we have to remember to do this and remember it during review.

This change creates a mixin to simplify this.

Before
```typescript
interface MyBucketProps extends BucketProps, AppIdentity {}

export class MyBucket extends Bucket {
  constructor(scope: GuStack, id: string, props: MyBucketProps) {
    const idWithApp = AppIdentity.suffixText(props, id) # <-- this is getting replaced

    super(scope, idWithApp, props);

    AppIdentity.taggedConstruct(props, this); # <-- this is getting replaced
  }
}
```

After
```typescript
interface MyBucketProps extends BucketProps, AppIdentity {}

class MyBucket extends GuAppAwareConstruct(Bucket) {
  constructor(scope: GuStack, id: string, props: MyBucketProps) {
    super(scope, id, props);
  }
}
```

This is similar to #418.
  • Loading branch information
akash1810 committed Oct 22, 2021
1 parent 7f6ba14 commit e021ca4
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 0 deletions.
50 changes: 50 additions & 0 deletions src/utils/mixin/__snapshots__/app-aware-contruct.test.ts.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`The GuAppAwareConstruct mixin should add the app tag 1`] = `
Object {
"Parameters": Object {
"Stage": Object {
"AllowedValues": Array [
"CODE",
"PROD",
],
"Default": "CODE",
"Description": "Stage name",
"Type": "String",
},
},
"Resources": Object {
"MyBucketTest262E966E": Object {
"DeletionPolicy": "Retain",
"Properties": Object {
"Tags": Array [
Object {
"Key": "App",
"Value": "Test",
},
Object {
"Key": "gu:cdk:version",
"Value": "TEST",
},
Object {
"Key": "gu:repo",
"Value": "guardian/cdk",
},
Object {
"Key": "Stack",
"Value": "test-stack",
},
Object {
"Key": "Stage",
"Value": Object {
"Ref": "Stage",
},
},
],
},
"Type": "AWS::S3::Bucket",
"UpdateReplacePolicy": "Retain",
},
},
}
`;
48 changes: 48 additions & 0 deletions src/utils/mixin/app-aware-construct.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { Construct } from "@aws-cdk/core";
import { AppIdentity } from "../../constructs/core/identity";
import type { AnyConstructor } from "./types";

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types -- mixin
export function GuAppAwareConstruct<TBase extends AnyConstructor>(BaseClass: TBase) {
class Mixin extends BaseClass {
/**
* The ID of the construct with the App suffix.
* This should be used in place of `id` when trying to reference the construct.
*/
readonly idWithApp: string;

// eslint-disable-next-line custom-rules/valid-constructors, @typescript-eslint/no-explicit-any -- mixin
protected constructor(...args: any[]) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- mixin
const [scope, id, props, ...rest] = args;

if (!AppIdentity.isAppIdentity(props)) {
// `props` does not include the `app` property so nothing to do
super(...args);

// This isn't of much use downstream, but it produces a nicer API with `idWithApp` of type `string` rather than `string | undefined`
this.idWithApp = id as string;
} else {
const app: string = props.app;
const idWithApp = AppIdentity.suffixText({ app }, id as string);

// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- mixin
const newArgs = [scope, idWithApp, props, ...rest];
super(...newArgs);

this.idWithApp = idWithApp;

/*
Add the `App` tag to the construct.
Although not every resource can be tagged, it's still safe to make the call.
If AWS support tags on a new resource one day, our test suite will fail and we can celebrate!
See https://docs.aws.amazon.com/ARG/latest/userguide/supported-resources.html
*/
if (Construct.isConstruct(this)) {
AppIdentity.taggedConstruct({ app }, this);
}
}
}
}
return Mixin;
}
46 changes: 46 additions & 0 deletions src/utils/mixin/app-aware-contruct.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { SynthUtils } from "@aws-cdk/assert";
import type { BucketProps } from "@aws-cdk/aws-s3";
import { Bucket } from "@aws-cdk/aws-s3";
import type { GuStack } from "../../constructs/core";
import type { AppIdentity } from "../../constructs/core/identity";
import { simpleGuStackForTesting } from "../test";
import { GuAppAwareConstruct } from "./app-aware-construct";

// `GuAppAwareConstruct` should only operate if `props` has an `app` property,
// so usage here should be a no-op.
class TestConstruct extends GuAppAwareConstruct(Bucket) {
constructor(scope: GuStack, id: string, props: BucketProps) {
super(scope, id, props);
}
}

interface TestAppAwareConstructProps extends BucketProps, AppIdentity {}

class TestAppAwareConstruct extends GuAppAwareConstruct(Bucket) {
constructor(scope: GuStack, id: string, props: TestAppAwareConstructProps) {
super(scope, id, props);
}
}

describe("The GuAppAwareConstruct mixin", () => {
// demonstrates usage of `GuAppAwareConstruct`
it("should leave the id alone if no app identifier is provided", () => {
const stack = simpleGuStackForTesting();
const bucket = new TestConstruct(stack, "MyBucket", {});

expect(bucket.idWithApp).toBe("MyBucket");
});

it("should suffix the id with the app identifier", () => {
const stack = simpleGuStackForTesting();
const bucket = new TestAppAwareConstruct(stack, "MyBucket", { app: "Test" });

expect(bucket.idWithApp).toBe("MyBucketTest");
});

it("should add the app tag", () => {
const stack = simpleGuStackForTesting();
new TestAppAwareConstruct(stack, "MyBucket", { app: "Test" });
expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot();
});
});

0 comments on commit e021ca4

Please sign in to comment.