Skip to content

Commit 05db610

Browse files
authored
Fix NPE in CreateCatalog (#2435)
1 parent a29f800 commit 05db610

File tree

3 files changed

+51
-1
lines changed

3 files changed

+51
-1
lines changed

runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,13 @@ public PolarisEntity createCatalog(CreateCatalogRequest catalogRequest) {
781781
throw new AlreadyExistsException(
782782
"Cannot create Catalog %s. Catalog already exists or resolution failed",
783783
entity.getName());
784+
} else if (!catalogResult.isSuccess()) {
785+
throw new IllegalStateException(
786+
String.format(
787+
"Cannot create Catalog %s: %s with extraInfo %s",
788+
entity.getName(),
789+
catalogResult.getReturnStatus(),
790+
catalogResult.getExtraInformation()));
784791
}
785792
return PolarisEntity.of(catalogResult.getCatalog());
786793
}

runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public Response createCatalog(
164164
Catalog catalog = request.getCatalog();
165165
validateStorageConfig(catalog.getStorageConfigInfo());
166166
validateExternalCatalog(catalog);
167-
Catalog newCatalog = new CatalogEntity(adminService.createCatalog(request)).asCatalog();
167+
Catalog newCatalog = CatalogEntity.of(adminService.createCatalog(request)).asCatalog();
168168
LOGGER.info("Created new catalog {}", newCatalog);
169169
return Response.status(Response.Status.CREATED).build();
170170
}

runtime/service/src/test/java/org/apache/polaris/service/admin/ManagementServiceTest.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,16 @@
4949
import org.apache.polaris.core.entity.PrincipalRoleEntity;
5050
import org.apache.polaris.core.persistence.MetaStoreManagerFactory;
5151
import org.apache.polaris.core.persistence.PolarisMetaStoreManager;
52+
import org.apache.polaris.core.persistence.dao.entity.BaseResult;
5253
import org.apache.polaris.core.persistence.dao.entity.CreateCatalogResult;
5354
import org.apache.polaris.core.persistence.dao.entity.EntityResult;
5455
import org.apache.polaris.core.secrets.UnsafeInMemorySecretsManager;
5556
import org.apache.polaris.service.TestServices;
5657
import org.apache.polaris.service.config.ReservedProperties;
58+
import org.assertj.core.api.Assertions;
5759
import org.junit.jupiter.api.BeforeEach;
5860
import org.junit.jupiter.api.Test;
61+
import org.mockito.Mockito;
5962

6063
public class ManagementServiceTest {
6164
private TestServices services;
@@ -285,4 +288,44 @@ public void testCanListCatalogs() {
285288
.extracting(Catalog::getName)
286289
.containsExactlyInAnyOrder("my-catalog-1", "my-catalog-2");
287290
}
291+
292+
@Test
293+
public void testCreateCatalogReturnErrorOnFailure() {
294+
PolarisMetaStoreManager metaStoreManager = Mockito.spy(setupMetaStoreManager());
295+
PolarisCallContext callContext = services.newCallContext();
296+
PolarisAdminService polarisAdminService =
297+
setupPolarisAdminService(metaStoreManager, callContext);
298+
299+
AwsStorageConfigInfo awsConfigModel =
300+
AwsStorageConfigInfo.builder()
301+
.setRoleArn("arn:aws:iam::123456789012:role/my-role")
302+
.setExternalId("externalId")
303+
.setUserArn("userArn")
304+
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
305+
.setAllowedLocations(List.of("s3://my-old-bucket/path/to/data"))
306+
.build();
307+
String catalogName = "mycatalog";
308+
Catalog catalog =
309+
PolarisCatalog.builder()
310+
.setType(Catalog.TypeEnum.INTERNAL)
311+
.setName(catalogName)
312+
.setProperties(new CatalogProperties("s3://bucket/path/to/data"))
313+
.setStorageConfigInfo(awsConfigModel)
314+
.build();
315+
CreateCatalogResult resultWithError =
316+
new CreateCatalogResult(
317+
BaseResult.ReturnStatus.UNEXPECTED_ERROR_SIGNALED, "Unexpected Error Occurred");
318+
Mockito.doAnswer(invocation -> resultWithError)
319+
.when(metaStoreManager)
320+
.createCatalog(Mockito.any(), Mockito.any(), Mockito.any());
321+
Assertions.assertThatThrownBy(
322+
() -> polarisAdminService.createCatalog(new CreateCatalogRequest(catalog)))
323+
.isInstanceOf(IllegalStateException.class)
324+
.hasMessage(
325+
String.format(
326+
"Cannot create Catalog %s: %s with extraInfo %s",
327+
catalogName,
328+
resultWithError.getReturnStatus(),
329+
resultWithError.getExtraInformation()));
330+
}
288331
}

0 commit comments

Comments
 (0)