From 8bb52bc39ad48c2b0158990b60c2818702cbffa6 Mon Sep 17 00:00:00 2001 From: Ashok Date: Mon, 30 Oct 2023 18:40:40 +0530 Subject: [PATCH] AWS: Remove AssertHelpers usage (#8937) --- .../aws/TestDefaultAwsClientFactory.java | 51 ++++---- .../aws/dynamodb/TestDynamoDbCatalog.java | 97 ++++++-------- .../aws/dynamodb/TestDynamoDbLockManager.java | 11 +- .../glue/TestGlueCatalogCommitFailure.java | 99 ++++++-------- .../aws/glue/TestGlueCatalogNamespace.java | 58 ++++----- .../aws/glue/TestGlueCatalogTable.java | 95 +++++++------- .../TestLakeFormationDataOperations.java | 34 +++-- .../TestLakeFormationMetadataOperations.java | 122 +++++++++--------- 8 files changed, 257 insertions(+), 310 deletions(-) diff --git a/aws/src/integration/java/org/apache/iceberg/aws/TestDefaultAwsClientFactory.java b/aws/src/integration/java/org/apache/iceberg/aws/TestDefaultAwsClientFactory.java index 60c06b1f08d7..8e750a0280a4 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/TestDefaultAwsClientFactory.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/TestDefaultAwsClientFactory.java @@ -19,9 +19,9 @@ package org.apache.iceberg.aws; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.aws.s3.S3FileIOProperties; import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.assertj.core.api.Assertions; import org.junit.Test; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.services.dynamodb.DynamoDbClient; @@ -39,11 +39,11 @@ public void testGlueEndpointOverride() { properties.put(AwsProperties.GLUE_CATALOG_ENDPOINT, "https://unknown:1234"); AwsClientFactory factory = AwsClientFactories.from(properties); GlueClient glueClient = factory.glue(); - AssertHelpers.assertThrowsCause( - "Should refuse connection to unknown endpoint", - SdkClientException.class, - "Unable to execute HTTP request: unknown", - () -> glueClient.getDatabase(GetDatabaseRequest.builder().name("TEST").build())); + Assertions.assertThatThrownBy( + () -> glueClient.getDatabase(GetDatabaseRequest.builder().name("TEST").build())) + .cause() + .isInstanceOf(SdkClientException.class) + .hasMessageContaining("Unable to execute HTTP request: unknown"); } @Test @@ -52,11 +52,12 @@ public void testS3FileIoEndpointOverride() { properties.put(S3FileIOProperties.ENDPOINT, "https://unknown:1234"); AwsClientFactory factory = AwsClientFactories.from(properties); S3Client s3Client = factory.s3(); - AssertHelpers.assertThrowsCause( - "Should refuse connection to unknown endpoint", - SdkClientException.class, - "Unable to execute HTTP request: bucket.unknown", - () -> s3Client.getObject(GetObjectRequest.builder().bucket("bucket").key("key").build())); + Assertions.assertThatThrownBy( + () -> + s3Client.getObject(GetObjectRequest.builder().bucket("bucket").key("key").build())) + .cause() + .isInstanceOf(SdkClientException.class) + .hasMessageContaining("Unable to execute HTTP request: bucket.unknown"); } @Test @@ -66,16 +67,15 @@ public void testS3FileIoCredentialsOverride() { properties.put(S3FileIOProperties.SECRET_ACCESS_KEY, "unknown"); AwsClientFactory factory = AwsClientFactories.from(properties); S3Client s3Client = factory.s3(); - AssertHelpers.assertThrows( - "Should fail request because of bad access key", - S3Exception.class, - "The AWS Access Key Id you provided does not exist in our records", - () -> - s3Client.getObject( - GetObjectRequest.builder() - .bucket(AwsIntegTestUtil.testBucketName()) - .key("key") - .build())); + Assertions.assertThatThrownBy( + () -> + s3Client.getObject( + GetObjectRequest.builder() + .bucket(AwsIntegTestUtil.testBucketName()) + .key("key") + .build())) + .isInstanceOf(S3Exception.class) + .hasMessageContaining("The AWS Access Key Id you provided does not exist in our records"); } @Test @@ -84,10 +84,9 @@ public void testDynamoDbEndpointOverride() { properties.put(AwsProperties.DYNAMODB_ENDPOINT, "https://unknown:1234"); AwsClientFactory factory = AwsClientFactories.from(properties); DynamoDbClient dynamoDbClient = factory.dynamo(); - AssertHelpers.assertThrowsCause( - "Should refuse connection to unknown endpoint", - SdkClientException.class, - "Unable to execute HTTP request: unknown", - dynamoDbClient::listTables); + Assertions.assertThatThrownBy(dynamoDbClient::listTables) + .cause() + .isInstanceOf(SdkClientException.class) + .hasMessageContaining("Unable to execute HTTP request: unknown"); } } diff --git a/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java b/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java index 9fae307fbc69..49ba0d6ee260 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java @@ -24,7 +24,6 @@ import java.util.concurrent.ForkJoinPool; import java.util.stream.Collectors; import java.util.stream.IntStream; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.HasTableOperations; import org.apache.iceberg.Schema; @@ -105,27 +104,19 @@ public void testCreateNamespace() { "namespace must be stored in DynamoDB", namespace.toString(), response.item().get("namespace").s()); - - AssertHelpers.assertThrows( - "should not create duplicated namespace", - AlreadyExistsException.class, - "already exists", - () -> catalog.createNamespace(namespace)); + Assertions.assertThatThrownBy(() -> catalog.createNamespace(namespace)) + .isInstanceOf(AlreadyExistsException.class) + .hasMessageContaining("already exists"); } @Test public void testCreateNamespaceBadName() { - AssertHelpers.assertThrows( - "should not create namespace with empty level", - ValidationException.class, - "must not be empty", - () -> catalog.createNamespace(Namespace.of("a", "", "b"))); - - AssertHelpers.assertThrows( - "should not create namespace with dot in level", - ValidationException.class, - "must not contain dot", - () -> catalog.createNamespace(Namespace.of("a", "b.c"))); + Assertions.assertThatThrownBy(() -> catalog.createNamespace(Namespace.of("a", "", "b"))) + .isInstanceOf(ValidationException.class) + .hasMessageContaining("must not be empty"); + Assertions.assertThatThrownBy(() -> catalog.createNamespace(Namespace.of("a", "b.c"))) + .isInstanceOf(ValidationException.class) + .hasMessageContaining("must not contain dot"); } @Test @@ -180,29 +171,23 @@ public void testCreateTable() { "table must be stored in DynamoDB with namespace as sort key", namespace.toString(), response.item().get("namespace").s()); - - AssertHelpers.assertThrows( - "should not create duplicated table", - AlreadyExistsException.class, - "already exists", - () -> catalog.createTable(tableIdentifier, SCHEMA)); + Assertions.assertThatThrownBy(() -> catalog.createTable(tableIdentifier, SCHEMA)) + .isInstanceOf(AlreadyExistsException.class) + .hasMessageContaining("already exists"); } @Test public void testCreateTableBadName() { Namespace namespace = Namespace.of(genRandomName()); catalog.createNamespace(namespace); - AssertHelpers.assertThrows( - "should not create table name with empty namespace", - ValidationException.class, - "Table namespace must not be empty", - () -> catalog.createTable(TableIdentifier.of(Namespace.empty(), "a"), SCHEMA)); - - AssertHelpers.assertThrows( - "should not create table name with dot", - ValidationException.class, - "must not contain dot", - () -> catalog.createTable(TableIdentifier.of(namespace, "a.b"), SCHEMA)); + Assertions.assertThatThrownBy( + () -> catalog.createTable(TableIdentifier.of(Namespace.empty(), "a"), SCHEMA)) + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Table namespace must not be empty"); + Assertions.assertThatThrownBy( + () -> catalog.createTable(TableIdentifier.of(namespace, "a.b"), SCHEMA)) + .isInstanceOf(ValidationException.class) + .hasMessageContaining("must not contain dot"); } @Test @@ -243,15 +228,18 @@ public void testDropTable() { .key(DynamoDbCatalog.tablePrimaryKey(tableIdentifier)) .build()) .hasItem()); - AssertHelpers.assertThrows( - "metadata location should be deleted", - NoSuchKeyException.class, - () -> - s3.headObject( - HeadObjectRequest.builder() - .bucket(testBucket) - .key(metadataLocation.substring(testBucket.length() + 6)) // s3:// + end slash - .build())); + Assertions.assertThatThrownBy( + () -> + s3.headObject( + HeadObjectRequest.builder() + .bucket(testBucket) + .key( + metadataLocation.substring( + testBucket.length() + 6)) // s3:// + end slash + .build())) + .as("metadata location should be deleted") + .isInstanceOf(NoSuchKeyException.class) + .hasMessageContaining("not found"); } @Test @@ -263,18 +251,14 @@ public void testRenameTable() { TableIdentifier tableIdentifier = TableIdentifier.of(namespace, genRandomName()); catalog.createTable(tableIdentifier, SCHEMA); TableIdentifier tableIdentifier2 = TableIdentifier.of(namespace2, genRandomName()); + Assertions.assertThatThrownBy( + () -> catalog.renameTable(TableIdentifier.of(namespace, "a"), tableIdentifier2)) + .isInstanceOf(NoSuchTableException.class) + .hasMessageContaining("does not exist"); - AssertHelpers.assertThrows( - "should not be able to rename a table not exist", - NoSuchTableException.class, - "does not exist", - () -> catalog.renameTable(TableIdentifier.of(namespace, "a"), tableIdentifier2)); - - AssertHelpers.assertThrows( - "should not be able to rename an existing table", - AlreadyExistsException.class, - "already exists", - () -> catalog.renameTable(tableIdentifier, tableIdentifier)); + Assertions.assertThatThrownBy(() -> catalog.renameTable(tableIdentifier, tableIdentifier)) + .isInstanceOf(AlreadyExistsException.class) + .hasMessageContaining("already exists"); String metadataLocation = dynamo @@ -404,7 +388,8 @@ public void testRegisterExistingTable() { TableOperations ops = ((HasTableOperations) registeringTable).operations(); String metadataLocation = ((DynamoDbTableOperations) ops).currentMetadataLocation(); Assertions.assertThatThrownBy(() -> catalog.registerTable(identifier, metadataLocation)) - .isInstanceOf(AlreadyExistsException.class); + .isInstanceOf(AlreadyExistsException.class) + .hasMessageContaining("already exists"); Assertions.assertThat(catalog.dropTable(identifier, true)).isTrue(); Assertions.assertThat(catalog.dropNamespace(namespace)).isTrue(); } diff --git a/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbLockManager.java b/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbLockManager.java index 90b8a8347395..eade5713bc7b 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbLockManager.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbLockManager.java @@ -25,11 +25,11 @@ import java.util.concurrent.ForkJoinPool; import java.util.stream.Collectors; import java.util.stream.IntStream; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.CatalogProperties; import org.apache.iceberg.aws.AwsClientFactories; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.assertj.core.api.Assertions; import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; @@ -227,11 +227,10 @@ public void testTableCreationFailure() { Mockito.doThrow(ResourceNotFoundException.class) .when(dynamo2) .describeTable(Mockito.any(DescribeTableRequest.class)); - AssertHelpers.assertThrows( - "should fail to initialize the lock manager", - IllegalStateException.class, - "Cannot find Dynamo table", - () -> new DynamoDbLockManager(dynamo2, lockTableName)); + Assertions.assertThatThrownBy(() -> new DynamoDbLockManager(dynamo2, lockTableName)) + .as("should fail to initialize the lock manager") + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot find Dynamo table"); } private static String genTableName() { diff --git a/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogCommitFailure.java b/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogCommitFailure.java index 2ce8408ec458..079423cd1245 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogCommitFailure.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogCommitFailure.java @@ -20,7 +20,6 @@ import java.io.File; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.BaseMetastoreTableOperations; import org.apache.iceberg.HasTableOperations; import org.apache.iceberg.Table; @@ -61,12 +60,9 @@ public void testFailedCommit() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps, new CommitFailedException("Datacenter on fire")); - - AssertHelpers.assertThrows( - "Commit failed exception should directly throw", - CommitFailedException.class, - "Datacenter on fire", - () -> spyOps.commit(metadataV2, metadataV1)); + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(CommitFailedException.class) + .hasMessageContaining("Datacenter on fire"); ops.refresh(); Assert.assertEquals("Current metadata should not have changed", metadataV2, ops.current()); @@ -84,12 +80,9 @@ public void testFailedCommitThrowsUnknownException() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps); - - AssertHelpers.assertThrows( - "Should throw CommitStateUnknownException since exception is unexpected", - CommitStateUnknownException.class, - "Datacenter on fire", - () -> spyOps.commit(metadataV2, metadataV1)); + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(CommitStateUnknownException.class) + .hasMessageContaining("Datacenter on fire"); Mockito.verify(spyOps, Mockito.times(1)).refresh(); ops.refresh(); @@ -111,14 +104,11 @@ public void testConcurrentModificationExceptionDoesNotCheckCommitStatus() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps, ConcurrentModificationException.builder().build()); - - AssertHelpers.assertThrowsWithCause( - "GlueCatalog should fail on concurrent modifications", - CommitFailedException.class, - "Glue detected concurrent update", - ConcurrentModificationException.class, - null, - () -> spyOps.commit(metadataV2, metadataV1)); + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(CommitFailedException.class) + .hasMessageContaining("Glue detected concurrent update") + .cause() + .isInstanceOf(ConcurrentModificationException.class); Mockito.verify(spyOps, Mockito.times(0)).refresh(); ops.refresh(); @@ -236,12 +226,9 @@ public void testFailedCommitThrowsUnknownExceptionWhenStatusCheckFails() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps); breakFallbackCatalogCommitCheck(spyOps); - - AssertHelpers.assertThrows( - "Should throw CommitStateUnknownException since the catalog check was blocked", - CommitStateUnknownException.class, - "Datacenter on fire", - () -> spyOps.commit(metadataV2, metadataV1)); + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(CommitStateUnknownException.class) + .hasMessageContaining("Datacenter on fire"); ops.refresh(); @@ -265,13 +252,9 @@ public void testSucceededCommitThrowsUnknownException() { GlueTableOperations spyOps = Mockito.spy(ops); commitAndThrowException(ops, spyOps); breakFallbackCatalogCommitCheck(spyOps); - - AssertHelpers.assertThrows( - "Should throw CommitStateUnknownException since the catalog check was blocked", - CommitStateUnknownException.class, - "Datacenter on fire", - () -> spyOps.commit(metadataV2, metadataV1)); - + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(CommitStateUnknownException.class) + .hasMessageContaining("Datacenter on fire"); ops.refresh(); Assert.assertNotEquals("Current metadata should have changed", ops.current(), metadataV2); @@ -357,12 +340,9 @@ public void testCreateTableWithInvalidDB() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps, EntityNotFoundException.builder().build()); - - AssertHelpers.assertThrows( - "Should throw not found exception", - NotFoundException.class, - "because Glue cannot find the requested entity", - () -> spyOps.commit(metadataV2, metadataV1)); + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(NotFoundException.class) + .hasMessageContaining("because Glue cannot find the requested entity"); ops.refresh(); Assert.assertEquals("Current metadata should not have changed", metadataV2, ops.current()); @@ -380,13 +360,9 @@ public void testGlueAccessDeniedException() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps, AccessDeniedException.builder().build()); - - AssertHelpers.assertThrows( - "Should throw forbidden exception", - ForbiddenException.class, - "because Glue cannot access the requested resources", - () -> spyOps.commit(metadataV2, metadataV1)); - + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(ForbiddenException.class) + .hasMessageContaining("because Glue cannot access the requested resources"); ops.refresh(); Assert.assertEquals("Current metadata should not have changed", metadataV2, ops.current()); Assert.assertTrue("Current metadata should still exist", metadataFileExists(metadataV2)); @@ -403,12 +379,10 @@ public void testGlueValidationException() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps, ValidationException.builder().build()); - - AssertHelpers.assertThrows( - "Should throw validation exception", - org.apache.iceberg.exceptions.ValidationException.class, - "because Glue encountered a validation exception while accessing requested resources", - () -> spyOps.commit(metadataV2, metadataV1)); + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(org.apache.iceberg.exceptions.ValidationException.class) + .hasMessageContaining( + "because Glue encountered a validation exception while accessing requested resources"); ops.refresh(); Assert.assertEquals("Current metadata should not have changed", metadataV2, ops.current()); @@ -426,10 +400,9 @@ public void testS3Exception() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps, S3Exception.builder().statusCode(300).build()); - - AssertHelpers.assertThrows( - null, S3Exception.class, () -> spyOps.commit(metadataV2, metadataV1)); - + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(S3Exception.class) + .hasMessage(null); ops.refresh(); Assert.assertEquals("Current metadata should not have changed", metadataV2, ops.current()); Assert.assertTrue("Current metadata should still exist", metadataFileExists(metadataV2)); @@ -446,9 +419,9 @@ public void testOtherGlueException() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps, GlueException.builder().statusCode(300).build()); - - AssertHelpers.assertThrows( - null, GlueException.class, () -> spyOps.commit(metadataV2, metadataV1)); + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(GlueException.class) + .hasMessage(null); ops.refresh(); Assert.assertEquals("Current metadata should not have changed", metadataV2, ops.current()); @@ -466,9 +439,9 @@ public void testInternalServerErrorRetryCommit() { GlueTableOperations spyOps = Mockito.spy(ops); failCommitAndThrowException(spyOps, GlueException.builder().statusCode(500).build()); - - AssertHelpers.assertThrows( - null, CommitFailedException.class, () -> spyOps.commit(metadataV2, metadataV1)); + Assertions.assertThatThrownBy(() -> spyOps.commit(metadataV2, metadataV1)) + .isInstanceOf(CommitFailedException.class) + .hasMessage(null); ops.refresh(); Assert.assertEquals("Current metadata should not have changed", metadataV2, ops.current()); diff --git a/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogNamespace.java b/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogNamespace.java index bbecb098d992..756666037a06 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogNamespace.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogNamespace.java @@ -21,7 +21,6 @@ import java.util.List; import java.util.Map; import java.util.UUID; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.exceptions.AlreadyExistsException; import org.apache.iceberg.exceptions.NamespaceNotEmptyException; @@ -30,6 +29,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.relocated.com.google.common.collect.Sets; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Test; import software.amazon.awssdk.services.glue.model.CreateTableRequest; @@ -44,11 +44,11 @@ public class TestGlueCatalogNamespace extends GlueTestBase { public void testCreateNamespace() { String namespace = getRandomName(); namespaces.add(namespace); - AssertHelpers.assertThrows( - "namespace does not exist before create", - EntityNotFoundException.class, - "not found", - () -> glue.getDatabase(GetDatabaseRequest.builder().name(namespace).build())); + Assertions.assertThatThrownBy( + () -> glue.getDatabase(GetDatabaseRequest.builder().name(namespace).build())) + .as("namespace does not exist before create") + .isInstanceOf(EntityNotFoundException.class) + .hasMessageContaining("not found"); Map properties = ImmutableMap.of( IcebergToGlueConverter.GLUE_DB_DESCRIPTION_KEY, @@ -74,11 +74,10 @@ public void testCreateNamespace() { @Test public void testCreateDuplicate() { String namespace = createNamespace(); - AssertHelpers.assertThrows( - "should not create namespace with the same name", - AlreadyExistsException.class, - "it already exists in Glue", - () -> glueCatalog.createNamespace(Namespace.of(namespace))); + Assertions.assertThatThrownBy(() -> glueCatalog.createNamespace(Namespace.of(namespace))) + .as("should not create namespace with the same name") + .isInstanceOf(AlreadyExistsException.class) + .hasMessageContaining("it already exists in Glue"); } @Test @@ -87,11 +86,10 @@ public void testCreateBadName() { Lists.newArrayList(Namespace.of("db-1"), Namespace.of("db", "db2")); for (Namespace namespace : invalidNamespaces) { - AssertHelpers.assertThrows( - "should not create namespace with invalid or nested names", - ValidationException.class, - "Cannot convert namespace", - () -> glueCatalog.createNamespace(namespace)); + Assertions.assertThatThrownBy(() -> glueCatalog.createNamespace(namespace)) + .as("should not create namespace with invalid or nested names") + .isInstanceOf(ValidationException.class) + .hasMessageContaining("Cannot convert namespace"); } } @@ -161,22 +159,21 @@ public void testNamespaceProperties() { public void testDropNamespace() { String namespace = createNamespace(); glueCatalog.dropNamespace(Namespace.of(namespace)); - AssertHelpers.assertThrows( - "namespace should not exist after deletion", - EntityNotFoundException.class, - "not found", - () -> glue.getDatabase(GetDatabaseRequest.builder().name(namespace).build())); + Assertions.assertThatThrownBy( + () -> glue.getDatabase(GetDatabaseRequest.builder().name(namespace).build())) + .as("namespace should not exist after deletion") + .isInstanceOf(EntityNotFoundException.class) + .hasMessageContaining("not found"); } @Test public void testDropNamespaceThatContainsOnlyIcebergTable() { String namespace = createNamespace(); createTable(namespace); - AssertHelpers.assertThrows( - "namespace should not be dropped when still has Iceberg table", - NamespaceNotEmptyException.class, - "still contains Iceberg tables", - () -> glueCatalog.dropNamespace(Namespace.of(namespace))); + Assertions.assertThatThrownBy(() -> glueCatalog.dropNamespace(Namespace.of(namespace))) + .as("namespace should not be dropped when still has Iceberg table") + .isInstanceOf(NamespaceNotEmptyException.class) + .hasMessageContaining("still contains Iceberg tables"); } @Test @@ -187,10 +184,9 @@ public void testDropNamespaceThatContainsNonIcebergTable() { .databaseName(namespace) .tableInput(TableInput.builder().name(UUID.randomUUID().toString()).build()) .build()); - AssertHelpers.assertThrows( - "namespace should not be dropped when still has non-Iceberg table", - NamespaceNotEmptyException.class, - "still contains non-Iceberg tables", - () -> glueCatalog.dropNamespace(Namespace.of(namespace))); + Assertions.assertThatThrownBy(() -> glueCatalog.dropNamespace(Namespace.of(namespace))) + .as("namespace should not be dropped when still has non-Iceberg table") + .isInstanceOf(NamespaceNotEmptyException.class) + .hasMessageContaining("still contains non-Iceberg tables"); } } diff --git a/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java b/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java index c51bc2a86509..bb598d184d7f 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/glue/TestGlueCatalogTable.java @@ -26,7 +26,6 @@ import java.util.Optional; import java.util.stream.Collectors; import org.apache.iceberg.AppendFiles; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.BaseMetastoreTableOperations; import org.apache.iceberg.BaseTable; import org.apache.iceberg.DataFile; @@ -112,25 +111,25 @@ public void testCreateTable() { public void testCreateTableDuplicate() { String namespace = createNamespace(); String tableName = createTable(namespace); - AssertHelpers.assertThrows( - "should not create table with the same name", - AlreadyExistsException.class, - "Table already exists", - () -> - glueCatalog.createTable( - TableIdentifier.of(namespace, tableName), schema, partitionSpec)); + Assertions.assertThatThrownBy( + () -> + glueCatalog.createTable( + TableIdentifier.of(namespace, tableName), schema, partitionSpec)) + .isInstanceOf(AlreadyExistsException.class) + .as("should not create table with the same name") + .hasMessageContaining("Table already exists"); } @Test public void testCreateTableBadName() { String namespace = createNamespace(); - AssertHelpers.assertThrows( - "should not create table with bad name", - IllegalArgumentException.class, - "Invalid table identifier", - () -> - glueCatalog.createTable( - TableIdentifier.of(namespace, "table-1"), schema, partitionSpec)); + Assertions.assertThatThrownBy( + () -> + glueCatalog.createTable( + TableIdentifier.of(namespace, "table-1"), schema, partitionSpec)) + .isInstanceOf(IllegalArgumentException.class) + .as("should not create table with bad name") + .hasMessageContaining("Invalid table identifier"); } @Test @@ -240,14 +239,14 @@ public void testRenameTableFailsToCreateNewTable() { .databaseName(namespace) .tableInput(TableInput.builder().name(newTableName).build()) .build()); - AssertHelpers.assertThrows( - "should fail to rename to an existing table", - software.amazon.awssdk.services.glue.model.AlreadyExistsException.class, - "Table already exists", - () -> - glueCatalog.renameTable( - TableIdentifier.of(namespace, tableName), - TableIdentifier.of(namespace, newTableName))); + Assertions.assertThatThrownBy( + () -> + glueCatalog.renameTable( + TableIdentifier.of(namespace, tableName), + TableIdentifier.of(namespace, newTableName))) + .isInstanceOf(software.amazon.awssdk.services.glue.model.AlreadyExistsException.class) + .as("should fail to rename to an existing table") + .hasMessageContaining("Table already exists"); // old table can still be read with same metadata Table oldTable = glueCatalog.loadTable(id); Assert.assertEquals(table.location(), oldTable.location()); @@ -269,21 +268,21 @@ public void testRenameTableFailsToDeleteOldTable() { .databaseName(namespace) .tableInput(TableInput.builder().name(tableName).parameters(Maps.newHashMap()).build()) .build()); - AssertHelpers.assertThrows( - "should fail to rename", - ValidationException.class, - "Input Glue table is not an iceberg table", - () -> - glueCatalog.renameTable( - TableIdentifier.of(namespace, tableName), - TableIdentifier.of(namespace, newTableName))); - AssertHelpers.assertThrows( - "renamed table should be deleted", - EntityNotFoundException.class, - "not found", - () -> - glue.getTable( - GetTableRequest.builder().databaseName(namespace).name(newTableName).build())); + Assertions.assertThatThrownBy( + () -> + glueCatalog.renameTable( + TableIdentifier.of(namespace, tableName), + TableIdentifier.of(namespace, newTableName))) + .isInstanceOf(ValidationException.class) + .as("should fail to rename") + .hasMessageContaining("Input Glue table is not an iceberg table"); + Assertions.assertThatThrownBy( + () -> + glue.getTable( + GetTableRequest.builder().databaseName(namespace).name(newTableName).build())) + .isInstanceOf(EntityNotFoundException.class) + .as("renamed table should be deleted") + .hasMessageContaining("not found"); } @Test @@ -291,11 +290,11 @@ public void testDeleteTableWithoutPurge() { String namespace = createNamespace(); String tableName = createTable(namespace); glueCatalog.dropTable(TableIdentifier.of(namespace, tableName), false); - AssertHelpers.assertThrows( - "should not have table", - NoSuchTableException.class, - "Table does not exist", - () -> glueCatalog.loadTable(TableIdentifier.of(namespace, tableName))); + Assertions.assertThatThrownBy( + () -> glueCatalog.loadTable(TableIdentifier.of(namespace, tableName))) + .isInstanceOf(NoSuchTableException.class) + .as("should not have table") + .hasMessageContaining("Table does not exist"); String warehouseLocation = glueCatalog.defaultWarehouseLocation(TableIdentifier.of(namespace, tableName)); String prefix = warehouseLocation.split(testBucketName + "/", -1)[1]; @@ -345,11 +344,11 @@ public void testDeleteTableWithPurge() { txn.commitTransaction(); glueCatalog.dropTable(TableIdentifier.of(namespace, tableName)); - AssertHelpers.assertThrows( - "should not have table", - NoSuchTableException.class, - "Table does not exist", - () -> glueCatalog.loadTable(TableIdentifier.of(namespace, tableName))); + Assertions.assertThatThrownBy( + () -> glueCatalog.loadTable(TableIdentifier.of(namespace, tableName))) + .isInstanceOf(NoSuchTableException.class) + .as("should not have table") + .hasMessageContaining("Table does not exist"); String warehouseLocation = glueCatalog.defaultWarehouseLocation(TableIdentifier.of(namespace, tableName)); String prefix = warehouseLocation.split(testBucketName + "/", -1)[1]; diff --git a/aws/src/integration/java/org/apache/iceberg/aws/lakeformation/TestLakeFormationDataOperations.java b/aws/src/integration/java/org/apache/iceberg/aws/lakeformation/TestLakeFormationDataOperations.java index 10ab9083a879..9b7db24324b9 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/lakeformation/TestLakeFormationDataOperations.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/lakeformation/TestLakeFormationDataOperations.java @@ -18,13 +18,13 @@ */ package org.apache.iceberg.aws.lakeformation; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.DataFile; import org.apache.iceberg.DataFiles; import org.apache.iceberg.Table; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.ForbiddenException; +import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -53,13 +53,13 @@ public void after() { @Test public void testLoadTableWithNoTableAccess() { - AssertHelpers.assertThrows( - "attempt to load a table without SELECT permission should fail", - AccessDeniedException.class, - "Insufficient Lake Formation permission(s)", - () -> - glueCatalogPrivilegedRole.loadTable( - TableIdentifier.of(Namespace.of(testDbName), testTableName))); + Assertions.assertThatThrownBy( + () -> + glueCatalogPrivilegedRole.loadTable( + TableIdentifier.of(Namespace.of(testDbName), testTableName))) + .as("attempt to load a table without SELECT permission should fail") + .isInstanceOf(AccessDeniedException.class) + .hasMessageContaining("Insufficient Lake Formation permission(s)"); } @Test @@ -81,11 +81,10 @@ public void testUpdateTableWithNoInsertAccess() { .withFileSizeInBytes(10) .withRecordCount(1) .build(); - AssertHelpers.assertThrows( - "attempt to insert to a table without INSERT permission should fail", - S3Exception.class, - "Access Denied", - () -> table.newAppend().appendFile(dataFile).commit()); + Assertions.assertThatThrownBy(() -> table.newAppend().appendFile(dataFile).commit()) + .as("attempt to insert to a table without INSERT permission should fail") + .isInstanceOf(S3Exception.class) + .hasMessageContaining("Access Denied"); } @Test @@ -118,11 +117,10 @@ public void testDeleteWithNoDataPathAccess() { .withFileSizeInBytes(10) .withRecordCount(1) .build(); - AssertHelpers.assertThrows( - "attempt to delete without DATA_LOCATION_ACCESS permission should fail", - ForbiddenException.class, - "Glue cannot access the requested resources", - () -> table.newDelete().deleteFile(dataFile).commit()); + Assertions.assertThatThrownBy(() -> table.newDelete().deleteFile(dataFile).commit()) + .as("attempt to delete without DATA_LOCATION_ACCESS permission should fail") + .isInstanceOf(ForbiddenException.class) + .hasMessageContaining("Glue cannot access the requested resources"); } @Test diff --git a/aws/src/integration/java/org/apache/iceberg/aws/lakeformation/TestLakeFormationMetadataOperations.java b/aws/src/integration/java/org/apache/iceberg/aws/lakeformation/TestLakeFormationMetadataOperations.java index 1f3386b59b86..4f247755cda6 100644 --- a/aws/src/integration/java/org/apache/iceberg/aws/lakeformation/TestLakeFormationMetadataOperations.java +++ b/aws/src/integration/java/org/apache/iceberg/aws/lakeformation/TestLakeFormationMetadataOperations.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.Map; -import org.apache.iceberg.AssertHelpers; import org.apache.iceberg.Table; import org.apache.iceberg.TableProperties; import org.apache.iceberg.UpdateProperties; @@ -28,6 +27,7 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.ForbiddenException; import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Test; import software.amazon.awssdk.services.glue.model.AccessDeniedException; @@ -48,11 +48,11 @@ public void testCreateAndDropDatabaseSuccessful() { @Test public void testCreateDatabaseNoPrivileges() { String testDbName = getRandomDbName(); - AssertHelpers.assertThrows( - "attempt to create a database without CREATE_DATABASE permission should fail", - AccessDeniedException.class, - "Insufficient Lake Formation permission(s)", - () -> glueCatalogPrivilegedRole.createNamespace(Namespace.of(testDbName))); + Assertions.assertThatThrownBy( + () -> glueCatalogPrivilegedRole.createNamespace(Namespace.of(testDbName))) + .as("attempt to create a database without CREATE_DATABASE permission should fail") + .isInstanceOf(AccessDeniedException.class) + .hasMessageContaining("Insufficient Lake Formation permission(s)"); } @Test @@ -60,11 +60,11 @@ public void testDropDatabaseNoPrivileges() { String testDbName = getRandomDbName(); lfRegisterPathRoleCreateDb(testDbName); try { - AssertHelpers.assertThrows( - "attempt to drop a database without DROP permission should fail", - AccessDeniedException.class, - "Insufficient Lake Formation permission(s)", - () -> glueCatalogPrivilegedRole.dropNamespace(Namespace.of(testDbName))); + Assertions.assertThatThrownBy( + () -> glueCatalogPrivilegedRole.dropNamespace(Namespace.of(testDbName))) + .as("attempt to drop a database without DROP permission should fail") + .isInstanceOf(AccessDeniedException.class) + .hasMessageContaining("Insufficient Lake Formation permission(s)"); } finally { lfRegisterPathRoleDeleteDb(testDbName); } @@ -92,17 +92,17 @@ public void testCreateTableNoCreateTablePermission() { String tableLocation = getTableLocation(testTableName); grantDataPathPrivileges(tableLocation); try { - AssertHelpers.assertThrows( - "attempt to create a table without CREATE_TABLE permission should fail", - AccessDeniedException.class, - "Insufficient Lake Formation permission(s)", - () -> - glueCatalogPrivilegedRole.createTable( - TableIdentifier.of(testDbName, testTableName), - schema, - partitionSpec, - tableLocation, - null)); + Assertions.assertThatThrownBy( + () -> + glueCatalogPrivilegedRole.createTable( + TableIdentifier.of(testDbName, testTableName), + schema, + partitionSpec, + tableLocation, + null)) + .as("attempt to create a table without CREATE_TABLE permission should fail") + .isInstanceOf(AccessDeniedException.class) + .hasMessageContaining("Insufficient Lake Formation permission(s)"); } finally { lfRegisterPathRoleDeleteDb(testDbName); } @@ -132,11 +132,11 @@ public void testShowTablesNoPrivileges() { lfRegisterPathRoleCreateDb(testDbName); lfRegisterPathRoleCreateTable(testDbName, testTableName); try { - AssertHelpers.assertThrows( - "attempt to show tables without any permissions should fail", - AccessDeniedException.class, - "Insufficient Lake Formation permission(s)", - () -> glueCatalogPrivilegedRole.listTables(Namespace.of(testDbName))); + Assertions.assertThatThrownBy( + () -> glueCatalogPrivilegedRole.listTables(Namespace.of(testDbName))) + .as("attempt to show tables without any permissions should fail") + .isInstanceOf(AccessDeniedException.class) + .hasMessageContaining("Insufficient Lake Formation permission(s)"); } finally { lfRegisterPathRoleDeleteTable(testDbName, testTableName); lfRegisterPathRoleDeleteDb(testDbName); @@ -150,17 +150,17 @@ public void testCreateTableNoDataPathPermission() { lfRegisterPathRoleCreateDb(testDbName); grantDatabasePrivileges(testDbName, Permission.CREATE_TABLE); try { - AssertHelpers.assertThrows( - "attempt to create a table without DATA_LOCATION_ACCESS permission should fail", - ForbiddenException.class, - "Glue cannot access the requested resources", - () -> - glueCatalogPrivilegedRole.createTable( - TableIdentifier.of(testDbName, testTableName), - schema, - partitionSpec, - getTableLocation(testTableName), - null)); + Assertions.assertThatThrownBy( + () -> + glueCatalogPrivilegedRole.createTable( + TableIdentifier.of(testDbName, testTableName), + schema, + partitionSpec, + getTableLocation(testTableName), + null)) + .as("attempt to create a table without DATA_LOCATION_ACCESS permission should fail") + .isInstanceOf(ForbiddenException.class) + .hasMessageContaining("Glue cannot access the requested resources"); } finally { lfRegisterPathRoleDeleteDb(testDbName); } @@ -210,13 +210,13 @@ public void testDropTableNoDropPermission() { lfRegisterPathRoleCreateTable(testDbName, testTableName); grantTablePrivileges(testDbName, testTableName, Permission.SELECT); try { - AssertHelpers.assertThrows( - "attempt to drop a table without DROP permission should fail", - AccessDeniedException.class, - "Insufficient Lake Formation permission(s)", - () -> - glueCatalogPrivilegedRole.dropTable( - TableIdentifier.of(testDbName, testTableName), false)); + Assertions.assertThatThrownBy( + () -> + glueCatalogPrivilegedRole.dropTable( + TableIdentifier.of(testDbName, testTableName), false)) + .as("attempt to drop a table without DROP permission should fail") + .isInstanceOf(AccessDeniedException.class) + .hasMessageContaining("Insufficient Lake Formation permission(s)"); } finally { lfRegisterPathRoleDeleteTable(testDbName, testTableName); lfRegisterPathRoleDeleteDb(testDbName); @@ -265,11 +265,10 @@ public void testAlterTableSetPropertiesNoDataPathAccess() { TableProperties.DEFAULT_FILE_FORMAT, TableProperties.DEFAULT_FILE_FORMAT_DEFAULT); UpdateProperties updateProperties = table.updateProperties(); properties.forEach(updateProperties::set); - AssertHelpers.assertThrows( - "attempt to alter a table without ALTER permission should fail", - ForbiddenException.class, - "Glue cannot access the requested resources", - updateProperties::commit); + Assertions.assertThatThrownBy(updateProperties::commit) + .as("attempt to alter a table without ALTER permission should fail") + .isInstanceOf(ForbiddenException.class) + .hasMessageContaining("Glue cannot access the requested resources"); } finally { lfRegisterPathRoleDeleteTable(testDbName, testTableName); lfRegisterPathRoleDeleteDb(testDbName); @@ -284,13 +283,13 @@ public void testAlterTableSetPropertiesNoPrivileges() { lfRegisterPathRoleCreateTable(testDbName, testTableName); grantDataPathPrivileges(getTableLocation(testTableName)); try { - AssertHelpers.assertThrows( - "attempt to alter a table without ALTER permission should fail", - AccessDeniedException.class, - "Insufficient Lake Formation permission(s)", - () -> - glueCatalogPrivilegedRole.loadTable( - TableIdentifier.of(Namespace.of(testDbName), testTableName))); + Assertions.assertThatThrownBy( + () -> + glueCatalogPrivilegedRole.loadTable( + TableIdentifier.of(Namespace.of(testDbName), testTableName))) + .as("attempt to alter a table without ALTER permission should fail") + .isInstanceOf(AccessDeniedException.class) + .hasMessageContaining("Insufficient Lake Formation permission(s)"); } finally { lfRegisterPathRoleDeleteTable(testDbName, testTableName); lfRegisterPathRoleDeleteDb(testDbName); @@ -314,11 +313,10 @@ public void testAlterTableSetPropertiesNoAlterPermission() { TableProperties.DEFAULT_FILE_FORMAT, TableProperties.DEFAULT_FILE_FORMAT_DEFAULT); UpdateProperties updateProperties = table.updateProperties(); properties.forEach(updateProperties::set); - AssertHelpers.assertThrows( - "attempt to alter a table without ALTER privileges should fail", - ForbiddenException.class, - "Glue cannot access the requested resources", - updateProperties::commit); + Assertions.assertThatThrownBy(updateProperties::commit) + .as("attempt to alter a table without ALTER privileges should fail") + .isInstanceOf(ForbiddenException.class) + .hasMessageContaining("Glue cannot access the requested resources"); } finally { lfRegisterPathRoleDeleteTable(testDbName, testTableName); lfRegisterPathRoleDeleteDb(testDbName);