Add feedback from Luca

aws · Mar 25, 2024 · e427903 · e427903
1 parent e594fab
commit e427903
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 17 deletions.
diff --git a/packages/aws-cdk-lib/aws-sns/lib/topic.ts b/packages/aws-cdk-lib/aws-sns/lib/topic.ts
@@ -3,7 +3,7 @@ import { CfnTopic } from './sns.generated';
 import { ITopic, TopicBase } from './topic-base';
 import { IRole } from '../../aws-iam';
 import { IKey } from '../../aws-kms';
-import { ArnFormat, Lazy, Names, Stack, Token } from '../../core';
+import { ArnFormat, Fn, Lazy, Names, Stack, Token } from '../../core';
 
 /**
  * Properties for a new SNS topic
@@ -143,6 +143,22 @@ export enum LoggingProtocol {
   APPLICATION = 'application',
 }
 
+/**
+ * Represents an SNS topic defined outside of this stack.
+ */
+export interface TopicAttributes {
+  /**
+   * The ARN of the SNS topic.
+   */
+  readonly topicArn: string;
+
+  /**
+   * Whether content-based deduplication is enabled.
+   * Only applicable for FIFO topics.
+   */
+  readonly contentBasedDeduplication?: boolean;
+}
+
 /**
  * A new SNS topic
  */
@@ -154,14 +170,13 @@ export class Topic extends TopicBase {
    * @param scope The parent creating construct
    * @param id The construct's name
    * @param topicArn topic ARN (i.e. arn:aws:sns:us-east-2:444455556666:MyTopic)
-   * @param contentBasedDeduplication If content-based deduplication is enabled
    */
-  public static fromTopicArn(scope: Construct, id: string, topicArn: string, contentBasedDeduplication?: boolean): ITopic {
+  public static fromTopicArn(scope: Construct, id: string, topicArn: string): ITopic {
     class Import extends TopicBase {
       public readonly topicArn = topicArn;
       public readonly topicName = Stack.of(scope).splitArn(topicArn, ArnFormat.NO_RESOURCE_NAME).resource;
       public readonly fifo = this.topicName.endsWith('.fifo');
-      public readonly contentBasedDeduplication = contentBasedDeduplication ?? false;
+      public readonly contentBasedDeduplication = false;
       protected autoCreatePolicy: boolean = false;
     }
 
@@ -170,6 +185,25 @@ export class Topic extends TopicBase {
     });
   }
 
+  /**
+   * Import an existing SNS topic provided a topic attributes
+   *
+   * @param scope The parent creating construct
+   * @param id The construct's name
+   * @param attrs the attributes of the topic to import
+   */
+  public static fromTopicAttributes(scope: Construct, id: string, attrs: TopicAttributes): ITopic {
+    class Import extends TopicBase {
+      public readonly topicArn = attrs.topicArn;
+      public readonly topicName = extractNameFromArn(attrs.topicArn);
+      public readonly fifo = this.topicName.endsWith('.fifo');
+      public readonly contentBasedDeduplication = attrs.contentBasedDeduplication || false;
+      protected autoCreatePolicy: boolean = false;
+    }
+
+    return new Import(scope, id);
+  }
+
   public readonly topicArn: string;
   public readonly topicName: string;
   public readonly contentBasedDeduplication: boolean;
@@ -265,3 +299,20 @@ export class Topic extends TopicBase {
     this.loggingConfigs.push(config);
   }
 }
+
+/**
+ * Given an opaque (token) ARN, returns a CloudFormation expression that extracts the topic
+ * name from the ARN.
+ *
+ * Function ARNs look like this:
+ *
+ *   arn:aws:sns:region:account-id:topic-name
+ *
+ * ..which means that in order to extract the `topic-name` component from the ARN, we can
+ * split the ARN using ":" and select the component in index 5.
+ *
+ * @returns `FnSelect(5, FnSplit(':', arn))`
+ */
+function extractNameFromArn(arn: string) {
+  return Fn.select(5, Fn.split(':', arn));
+}
diff --git a/packages/aws-cdk-lib/aws-sns/test/sns.test.ts b/packages/aws-cdk-lib/aws-sns/test/sns.test.ts
@@ -435,35 +435,35 @@ describe('Topic', () => {
     expect(imported.fifo).toEqual(true);
   });
 
-  test('fromTopicArn contentBasedDeduplication true', () => {
+  test('fromTopicAttributes contentBasedDeduplication false', () => {
     // GIVEN
     const stack = new cdk.Stack();
 
     // WHEN
-    const topic = new sns.Topic(stack, 'MyTopic', {
-      topicName: 'MyTopic',
-      fifo: true,
-      contentBasedDeduplication: true,
+    const imported = sns.Topic.fromTopicAttributes(stack, 'Imported', {
+      topicArn: 'arn:aws:sns:*:123456789012:mytopic',
     });
-    const imported = sns.Topic.fromTopicArn(stack, 'Imported', topic.topicArn, true);
 
     // THEN
-    expect(imported.contentBasedDeduplication).toEqual(true);
+    expect(imported.topicName).toEqual('mytopic');
+    expect(imported.topicArn).toEqual('arn:aws:sns:*:123456789012:mytopic');
+    expect(imported.contentBasedDeduplication).toEqual(false);
   });
 
-  test('fromTopicArn contentBasedDeduplication not provided (false)', () => {
+  test('fromTopicAttributes contentBasedDeduplication true', () => {
     // GIVEN
     const stack = new cdk.Stack();
 
     // WHEN
-    const topic = new sns.Topic(stack, 'MyTopic', {
-      topicName: 'MyTopic',
-      fifo: true,
+    const imported = sns.Topic.fromTopicAttributes(stack, 'Imported', {
+      topicArn: 'arn:aws:sns:*:123456789012:mytopic.fifo',
+      contentBasedDeduplication: true,
     });
-    const imported = sns.Topic.fromTopicArn(stack, 'Imported', topic.topicArn);
 
     // THEN
-    expect(imported.contentBasedDeduplication).toEqual(false);
+    expect(imported.topicName).toEqual('mytopic.fifo');
+    expect(imported.topicArn).toEqual('arn:aws:sns:*:123456789012:mytopic.fifo');
+    expect(imported.contentBasedDeduplication).toEqual(true);
   });
 
   test('sets account for imported topic env', () => {