fix(glue): Empty string is not undefined. (aws#5763)

When the Glue Table s3Prefix is an empty string, it should not be replaced with a default value.
melgenek · Jan 13, 2020 · 5b11289 · 5b11289
1 parent 5292bd5
commit 5b11289
Show file tree

Hide file tree

Showing 4 changed files with 111 additions and 7 deletions.
diff --git a/packages/@aws-cdk/aws-glue/lib/database.ts b/packages/@aws-cdk/aws-glue/lib/database.ts
@@ -90,20 +90,29 @@ export class Database extends Resource implements IDatabase {
       physicalName: props.databaseName,
     });
 
-    if (props.locationUri) {
+    let databaseInput: CfnDatabase.DatabaseInputProperty = {
+      name: props.databaseName,
+    };
+
+    if (props.locationUri && props.locationUri.length > 0) {
       this.locationUri = props.locationUri;
+      databaseInput = {
+        locationUri: this.locationUri,
+        ...databaseInput
+      };
     } else {
       const bucket = new s3.Bucket(this, 'Bucket');
       this.locationUri = `s3://${bucket.bucketName}/${props.databaseName}`;
+      databaseInput = {
+        locationUri: this.locationUri,
+        ...databaseInput
+      };
     }
 
     this.catalogId = Stack.of(this).account;
     const resource = new CfnDatabase(this, 'Resource', {
       catalogId: this.catalogId,
-      databaseInput: {
-        name: this.physicalName,
-        locationUri: this.locationUri
-      }
+      databaseInput: databaseInput
     });
 
     // see https://docs.aws.amazon.com/glue/latest/dg/glue-specifying-resource-arns.html#data-catalog-resource-arns

diff --git a/packages/@aws-cdk/aws-glue/lib/table.ts b/packages/@aws-cdk/aws-glue/lib/table.ts
@@ -236,7 +236,7 @@ export class Table extends Resource implements ITable {
 
     this.database = props.database;
     this.dataFormat = props.dataFormat;
-    this.s3Prefix = props.s3Prefix || 'data/';
+    this.s3Prefix = (props.s3Prefix !== undefined && props.s3Prefix !== null) ? props.s3Prefix : 'data/';
 
     validateSchema(props.columns, props.partitionKeys);
     this.columns = props.columns;

diff --git a/packages/@aws-cdk/aws-glue/test/test.database.ts b/packages/@aws-cdk/aws-glue/test/test.database.ts
@@ -4,7 +4,7 @@ import { Test } from 'nodeunit';
 import * as glue from '../lib';
 
 export = {
-  'default database creates a bucket to store the datbase'(test: Test) {
+  'default database creates a bucket to store the database'(test: Test) {
     const stack = new Stack();
 
     new glue.Database(stack, 'Database', {
@@ -75,6 +75,33 @@ export = {
     test.done();
   },
 
+  'empty locationURI'(test: Test) {
+    const stack = new Stack();
+
+    new glue.Database(stack, 'Database', {
+      databaseName: 'test_database',
+      locationUri: ''
+    });
+
+    expect(stack).toMatch({
+      Resources: {
+        DatabaseB269D8BB: {
+          Type: 'AWS::Glue::Database',
+          Properties: {
+            CatalogId: {
+              Ref: "AWS::AccountId"
+            },
+            DatabaseInput: {
+              Name: "test_database"
+            }
+          }
+        }
+      }
+    });
+
+    test.done();
+  },
+
   'fromDatabase'(test: Test) {
     // GIVEN
     const stack = new Stack();

diff --git a/packages/@aws-cdk/aws-glue/test/test.table.ts b/packages/@aws-cdk/aws-glue/test/test.table.ts
@@ -1079,6 +1079,74 @@ export = {
     test.done();
   },
 
+  'explicit s3 bucket and with empty prefix'(test: Test) {
+    const app = new cdk.App();
+    const dbStack = new cdk.Stack(app, 'db');
+    const stack = new cdk.Stack(app, 'app');
+    const bucket = new s3.Bucket(stack, 'ExplicitBucket');
+    const database = new glue.Database(dbStack, 'Database', {
+      databaseName: 'database',
+    });
+
+    new glue.Table(stack, 'Table', {
+      database,
+      bucket,
+      s3Prefix: '',
+      tableName: 'table',
+      columns: [{
+        name: 'col',
+        type: glue.Schema.STRING
+      }],
+      dataFormat: glue.DataFormat.Json,
+    });
+
+    expect(stack).to(haveResource('AWS::Glue::Table', {
+      CatalogId: {
+        Ref: "AWS::AccountId"
+      },
+      DatabaseName: {
+        "Fn::ImportValue": "db:ExportsOutputRefDatabaseB269D8BB88F4B1C4"
+      },
+      TableInput: {
+        Description: "table generated by CDK",
+        Name: "table",
+        Parameters: {
+          has_encrypted_data: false
+        },
+        StorageDescriptor: {
+          Columns: [
+            {
+              Name: "col",
+              Type: "string"
+            }
+          ],
+          Compressed: false,
+          InputFormat: "org.apache.hadoop.mapred.TextInputFormat",
+          Location: {
+            "Fn::Join": [
+              "",
+              [
+                "s3://",
+                {
+                  Ref: "ExplicitBucket0AA51A3F"
+                },
+                "/"
+              ]
+            ]
+          },
+          OutputFormat: "org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat",
+          SerdeInfo: {
+            SerializationLibrary: "org.openx.data.jsonserde.JsonSerDe"
+          },
+          StoredAsSubDirectories: false
+        },
+        TableType: "EXTERNAL_TABLE"
+      }
+    }));
+
+    test.done();
+  },
+
   'grants': {
     'read only'(test: Test) {
       const stack = new cdk.Stack();