Skip to content

Remove circular dependency between entity and api schema #1990

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,9 @@
import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
import org.apache.polaris.core.admin.model.AzureStorageConfigInfo;
import org.apache.polaris.core.admin.model.Catalog;
import org.apache.polaris.core.admin.model.CatalogProperties;
import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
import org.apache.polaris.core.admin.model.ExternalCatalog;
import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
import org.apache.polaris.core.admin.model.GcpStorageConfigInfo;
import org.apache.polaris.core.admin.model.PolarisCatalog;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
import org.apache.polaris.core.config.BehaviorChangeConfiguration;
import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
Expand Down Expand Up @@ -95,39 +92,6 @@ public static CatalogEntity fromCatalog(CallContext callContext, Catalog catalog
return builder.build();
}

public Catalog asCatalog() {
Map<String, String> internalProperties = getInternalPropertiesAsMap();
Catalog.TypeEnum catalogType =
Optional.ofNullable(internalProperties.get(CATALOG_TYPE_PROPERTY))
.map(Catalog.TypeEnum::valueOf)
.orElseGet(() -> getName().equalsIgnoreCase("ROOT") ? Catalog.TypeEnum.INTERNAL : null);
Map<String, String> propertiesMap = getPropertiesAsMap();
CatalogProperties catalogProps =
CatalogProperties.builder(propertiesMap.get(DEFAULT_BASE_LOCATION_KEY))
.putAll(propertiesMap)
.build();
return catalogType == Catalog.TypeEnum.INTERNAL
? PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(getName())
.setProperties(catalogProps)
.setCreateTimestamp(getCreateTimestamp())
.setLastUpdateTimestamp(getLastUpdateTimestamp())
.setEntityVersion(getEntityVersion())
.setStorageConfigInfo(getStorageInfo(internalProperties))
.build()
: ExternalCatalog.builder()
.setType(Catalog.TypeEnum.EXTERNAL)
.setName(getName())
.setProperties(catalogProps)
.setCreateTimestamp(getCreateTimestamp())
.setLastUpdateTimestamp(getLastUpdateTimestamp())
.setEntityVersion(getEntityVersion())
.setStorageConfigInfo(getStorageInfo(internalProperties))
.setConnectionConfigInfo(getConnectionInfo(internalProperties))
.build();
}

private StorageConfigInfo getStorageInfo(Map<String, String> internalProperties) {
if (internalProperties.containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName())) {
PolarisStorageConfigurationInfo configInfo = getStorageConfigurationInfo();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,6 @@ public static CatalogRoleEntity fromCatalogRole(CatalogRole catalogRole) {
.build();
}

public CatalogRole asCatalogRole() {
CatalogRole catalogRole =
new CatalogRole(
getName(),
getPropertiesAsMap(),
getCreateTimestamp(),
getLastUpdateTimestamp(),
getEntityVersion());
return catalogRole;
}

public static class Builder extends PolarisEntity.BaseBuilder<CatalogRoleEntity, Builder> {
public Builder() {
super();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.polaris.core.entity;

import static org.apache.polaris.core.admin.model.StorageConfigInfo.StorageTypeEnum.AZURE;
import static org.apache.polaris.core.entity.CatalogEntity.CATALOG_TYPE_PROPERTY;
import static org.apache.polaris.core.entity.CatalogEntity.DEFAULT_BASE_LOCATION_KEY;

import java.util.Map;
import java.util.Optional;
import org.apache.iceberg.catalog.Namespace;
import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
import org.apache.polaris.core.admin.model.AzureStorageConfigInfo;
import org.apache.polaris.core.admin.model.Catalog;
import org.apache.polaris.core.admin.model.CatalogProperties;
import org.apache.polaris.core.admin.model.CatalogRole;
import org.apache.polaris.core.admin.model.ConnectionConfigInfo;
import org.apache.polaris.core.admin.model.ExternalCatalog;
import org.apache.polaris.core.admin.model.FileStorageConfigInfo;
import org.apache.polaris.core.admin.model.GcpStorageConfigInfo;
import org.apache.polaris.core.admin.model.PolarisCatalog;
import org.apache.polaris.core.admin.model.Principal;
import org.apache.polaris.core.admin.model.PrincipalRole;
import org.apache.polaris.core.admin.model.StorageConfigInfo;
import org.apache.polaris.core.connection.ConnectionConfigInfoDpo;
import org.apache.polaris.core.entity.table.federated.FederatedEntities;
import org.apache.polaris.core.storage.FileStorageConfigurationInfo;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo;
import org.apache.polaris.core.storage.azure.AzureStorageConfigurationInfo;
import org.apache.polaris.core.storage.gcp.GcpStorageConfigurationInfo;

public final class EntityConverter {

public static Catalog toCatalog(CatalogEntity entity) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually remove the circular dependency? The old CatalogEntity.asCatalog() method and this method do the same thing and exist in the case package 🤔

Map<String, String> internalProperties = entity.getInternalPropertiesAsMap();
Catalog.TypeEnum catalogType =
Optional.ofNullable(internalProperties.get(CATALOG_TYPE_PROPERTY))
.map(Catalog.TypeEnum::valueOf)
.orElseGet(
() -> entity.getName().equalsIgnoreCase("ROOT") ? Catalog.TypeEnum.INTERNAL : null);
Map<String, String> propertiesMap = entity.getPropertiesAsMap();
CatalogProperties catalogProps =
CatalogProperties.builder(propertiesMap.get(DEFAULT_BASE_LOCATION_KEY))
.putAll(propertiesMap)
.build();
return catalogType == Catalog.TypeEnum.INTERNAL
? PolarisCatalog.builder()
.setType(Catalog.TypeEnum.INTERNAL)
.setName(entity.getName())
.setProperties(catalogProps)
.setCreateTimestamp(entity.getCreateTimestamp())
.setLastUpdateTimestamp(entity.getLastUpdateTimestamp())
.setEntityVersion(entity.getEntityVersion())
.setStorageConfigInfo(getEntityStorageInfo(internalProperties, entity))
.build()
: ExternalCatalog.builder()
.setType(Catalog.TypeEnum.EXTERNAL)
.setName(entity.getName())
.setProperties(catalogProps)
.setCreateTimestamp(entity.getCreateTimestamp())
.setLastUpdateTimestamp(entity.getLastUpdateTimestamp())
.setEntityVersion(entity.getEntityVersion())
.setStorageConfigInfo(getEntityStorageInfo(internalProperties, entity))
.setConnectionConfigInfo(getEntityConnectionInfo(internalProperties, entity))
.build();
}

public static CatalogRole toCatalogRole(CatalogRoleEntity entity) {
return new CatalogRole(
entity.getName(),
entity.getPropertiesAsMap(),
entity.getCreateTimestamp(),
entity.getLastUpdateTimestamp(),
entity.getEntityVersion());
}

public static Namespace toNamespace(NamespaceEntity entity) {
Namespace parent = entity.getParentNamespace();
String[] levels = new String[parent.length() + 1];
for (int i = 0; i < parent.length(); ++i) {
levels[i] = parent.level(i);
}
levels[levels.length - 1] = entity.getName();
return Namespace.of(levels);
}

public static Principal toPrincipal(PrincipalEntity entity) {
return new Principal(
entity.getName(),
entity.getClientId(),
entity.getPropertiesAsMap(),
entity.getCreateTimestamp(),
entity.getLastUpdateTimestamp(),
entity.getEntityVersion());
}

public static PrincipalRole toPrincipalRole(PrincipalRoleEntity entity) {
return new PrincipalRole(
entity.getName(),
FederatedEntities.isFederated(entity),
entity.getPropertiesAsMap(),
entity.getCreateTimestamp(),
entity.getLastUpdateTimestamp(),
entity.getEntityVersion());
}

private static StorageConfigInfo getEntityStorageInfo(
Map<String, String> internalProperties, CatalogEntity entity) {
if (internalProperties.containsKey(PolarisEntityConstants.getStorageConfigInfoPropertyName())) {
PolarisStorageConfigurationInfo configInfo = entity.getStorageConfigurationInfo();
if (configInfo instanceof AwsStorageConfigurationInfo) {
AwsStorageConfigurationInfo awsConfig = (AwsStorageConfigurationInfo) configInfo;
return AwsStorageConfigInfo.builder()
.setRoleArn(awsConfig.getRoleARN())
.setExternalId(awsConfig.getExternalId())
.setUserArn(awsConfig.getUserARN())
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
.setAllowedLocations(awsConfig.getAllowedLocations())
.setRegion(awsConfig.getRegion())
.build();
}
if (configInfo instanceof AzureStorageConfigurationInfo) {
AzureStorageConfigurationInfo azureConfig = (AzureStorageConfigurationInfo) configInfo;
return AzureStorageConfigInfo.builder()
.setTenantId(azureConfig.getTenantId())
.setMultiTenantAppName(azureConfig.getMultiTenantAppName())
.setConsentUrl(azureConfig.getConsentUrl())
.setStorageType(AZURE)
.setAllowedLocations(azureConfig.getAllowedLocations())
.build();
}
if (configInfo instanceof GcpStorageConfigurationInfo) {
GcpStorageConfigurationInfo gcpConfigModel = (GcpStorageConfigurationInfo) configInfo;
return GcpStorageConfigInfo.builder()
.setGcsServiceAccount(gcpConfigModel.getGcpServiceAccount())
.setStorageType(StorageConfigInfo.StorageTypeEnum.GCS)
.setAllowedLocations(gcpConfigModel.getAllowedLocations())
.build();
}
if (configInfo instanceof FileStorageConfigurationInfo) {
FileStorageConfigurationInfo fileConfigModel = (FileStorageConfigurationInfo) configInfo;
return new FileStorageConfigInfo(
StorageConfigInfo.StorageTypeEnum.FILE, fileConfigModel.getAllowedLocations());
}
return null;
}
return null;
}

private static ConnectionConfigInfo getEntityConnectionInfo(
Map<String, String> internalProperties, CatalogEntity entity) {
if (internalProperties.containsKey(
PolarisEntityConstants.getConnectionConfigInfoPropertyName())) {
ConnectionConfigInfoDpo configInfo = entity.getConnectionConfigInfoDpo();
return configInfo.asConnectionConfigInfoModel();
}
return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,6 @@ public Namespace getParentNamespace() {
return RESTUtil.decodeNamespace(encodedNamespace);
}

public Namespace asNamespace() {
Namespace parent = getParentNamespace();
String[] levels = new String[parent.length() + 1];
for (int i = 0; i < parent.length(); ++i) {
levels[i] = parent.level(i);
}
levels[levels.length - 1] = getName();
return Namespace.of(levels);
}

@Override
@JsonIgnore
public String getBaseLocation() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ public static PrincipalEntity fromPrincipal(Principal principal) {
.build();
}

public Principal asPrincipal() {
return new Principal(
getName(),
getClientId(),
getPropertiesAsMap(),
getCreateTimestamp(),
getLastUpdateTimestamp(),
getEntityVersion());
}

public String getClientId() {
return getInternalPropertiesAsMap().get(PolarisEntityConstants.getClientIdPropertyName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,6 @@ public static PrincipalRoleEntity fromPrincipalRole(PrincipalRole principalRole)
.build();
}

public PrincipalRole asPrincipalRole() {
return new PrincipalRole(
getName(),
FederatedEntities.isFederated(this),
getPropertiesAsMap(),
getCreateTimestamp(),
getLastUpdateTimestamp(),
getEntityVersion());
}

public static class Builder extends PolarisEntity.BaseBuilder<PrincipalRoleEntity, Builder> {
public Builder() {
super();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
*/
package org.apache.polaris.service.quarkus.admin;

import static org.apache.polaris.core.entity.EntityConverter.toCatalog;

import io.quarkus.test.junit.QuarkusTest;
import io.quarkus.test.junit.TestProfile;
import java.util.List;
Expand Down Expand Up @@ -133,7 +135,7 @@ public void testCreateCatalogSufficientPrivileges() {
PRINCIPAL_ROLE2, PolarisPrivilege.CATALOG_DROP))
.isTrue();
final CatalogEntity newCatalog = new CatalogEntity.Builder().setName("new_catalog").build();
final CreateCatalogRequest createRequest = new CreateCatalogRequest(newCatalog.asCatalog());
final CreateCatalogRequest createRequest = new CreateCatalogRequest(toCatalog(newCatalog));

doTestSufficientPrivileges(
List.of(
Expand All @@ -152,7 +154,7 @@ public void testCreateCatalogSufficientPrivileges() {
@Test
public void testCreateCatalogInsufficientPrivileges() {
final CatalogEntity newCatalog = new CatalogEntity.Builder().setName("new_catalog").build();
final CreateCatalogRequest createRequest = new CreateCatalogRequest(newCatalog.asCatalog());
final CreateCatalogRequest createRequest = new CreateCatalogRequest(toCatalog(newCatalog));

doTestInsufficientPrivileges(
List.of(
Expand Down Expand Up @@ -288,7 +290,7 @@ public void testDeleteCatalogSufficientPrivileges() {
PRINCIPAL_ROLE2, PolarisPrivilege.CATALOG_CREATE))
.isTrue();
final CatalogEntity newCatalog = new CatalogEntity.Builder().setName("new_catalog").build();
final CreateCatalogRequest createRequest = new CreateCatalogRequest(newCatalog.asCatalog());
final CreateCatalogRequest createRequest = new CreateCatalogRequest(toCatalog(newCatalog));
adminService.createCatalog(createRequest);

doTestSufficientPrivileges(
Expand All @@ -308,7 +310,7 @@ public void testDeleteCatalogSufficientPrivileges() {
@Test
public void testDeleteCatalogInsufficientPrivileges() {
final CatalogEntity newCatalog = new CatalogEntity.Builder().setName("new_catalog").build();
final CreateCatalogRequest createRequest = new CreateCatalogRequest(newCatalog.asCatalog());
final CreateCatalogRequest createRequest = new CreateCatalogRequest(toCatalog(newCatalog));
adminService.createCatalog(createRequest);

doTestInsufficientPrivileges(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package org.apache.polaris.service.quarkus.admin;

import static org.apache.iceberg.types.Types.NestedField.required;
import static org.apache.polaris.core.entity.EntityConverter.toCatalog;

import com.google.auth.oauth2.AccessToken;
import com.google.auth.oauth2.GoogleCredentials;
Expand Down Expand Up @@ -280,13 +281,14 @@ public void before(TestInfo testInfo) {
catalogEntity =
adminService.createCatalog(
new CreateCatalogRequest(
new CatalogEntity.Builder()
.setName(CATALOG_NAME)
.setCatalogType("INTERNAL")
.setDefaultBaseLocation(storageLocation)
.setStorageConfigurationInfo(callContext, storageConfigModel, storageLocation)
.build()
.asCatalog()));
toCatalog(
new CatalogEntity.Builder()
.setName(CATALOG_NAME)
.setCatalogType("INTERNAL")
.setDefaultBaseLocation(storageLocation)
.setStorageConfigurationInfo(
callContext, storageConfigModel, storageLocation)
.build())));

initBaseCatalog();

Expand Down
Loading
Loading