From 7609e3a80d9d1a836c8c9f905276e35ef155357c Mon Sep 17 00:00:00 2001 From: Gabriel Igliozzi Date: Thu, 20 Aug 2026 18:32:02 +0200 Subject: [PATCH 1/2] implement aliasing feature --- .gitignore | 3 + .../server/usermetadata/AliasService.java | 10 + .../src/functionalTest/resources/schema.sql | 13 + .../schema.sql | 13 + .../schema.sql | 13 + .../polaris/PolarisAliasService.java | 64 +++++ .../configs/PolarisConnectorConfig.java | 21 ++ .../store/entities/PolarisAliasEntity.java | 55 ++++ .../store/repos/PolarisAliasRepository.java | 36 +++ .../polaris/PolarisAliasServiceTest.java | 81 ++++++ .../src/test/resources/h2db/schema.sql | 15 ++ metacat-functional-tests/build.gradle | 12 +- .../schema.sql | 13 + .../schema.sql | 13 + .../docker-compose-aurora.yml | 3 +- .../metacat/main/api/RequestWrapper.java | 23 +- .../main/api/v1/MetacatController.java | 27 +- .../main/api/v1/PartitionController.java | 26 +- .../metacat/main/configs/ServicesConfig.java | 17 +- .../services/impl/ConnectorAliasService.java | 78 ++++++ .../main/services/impl/TableServiceImpl.java | 106 +++++++- .../main/api/RequestWrapperSpec.groovy | 68 +++++ .../api/v1/MetacatControllerAliasSpec.groovy | 151 +++++++++++ .../impl/ConnectorAliasServiceSpec.groovy | 96 +++++++ .../services/impl/TableServiceImplSpec.groovy | 244 +++++++++++++++++- 25 files changed, 1149 insertions(+), 52 deletions(-) create mode 100644 metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisAliasService.java create mode 100644 metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/store/entities/PolarisAliasEntity.java create mode 100644 metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/store/repos/PolarisAliasRepository.java create mode 100644 metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisAliasServiceTest.java create mode 100644 metacat-main/src/main/java/com/netflix/metacat/main/services/impl/ConnectorAliasService.java create mode 100644 metacat-main/src/test/groovy/com/netflix/metacat/main/api/v1/MetacatControllerAliasSpec.groovy create mode 100644 metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/ConnectorAliasServiceSpec.groovy diff --git a/.gitignore b/.gitignore index b4106d2b2..1cb6ff106 100644 --- a/.gitignore +++ b/.gitignore @@ -35,6 +35,7 @@ Thumbs.db ################ *~ *.swp +.factorypath # Gradle Files # ################ @@ -47,6 +48,8 @@ Thumbs.db */target /build */build +/bin +*/bin # IntelliJ specific files/directories out diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/AliasService.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/AliasService.java index d31347fc3..3598e7ea2 100644 --- a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/AliasService.java +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/AliasService.java @@ -36,4 +36,14 @@ public interface AliasService { default QualifiedName getTableName(@NonNull final QualifiedName tableAlias) { return tableAlias; } + + /** + * Returns true if the given name refers to a table alias rather than to a table itself. + * + * @param name the table name. + * @return true if the name is an alias, false otherwise. + */ + default boolean isAlias(@NonNull final QualifiedName name) { + return !name.equals(getTableName(name)); + } } diff --git a/metacat-connector-polaris/src/functionalTest/resources/schema.sql b/metacat-connector-polaris/src/functionalTest/resources/schema.sql index f8effbc2d..1d8f90e8c 100644 --- a/metacat-connector-polaris/src/functionalTest/resources/schema.sql +++ b/metacat-connector-polaris/src/functionalTest/resources/schema.sql @@ -1,3 +1,4 @@ +DROP TABLE IF EXISTS ALIASES; DROP TABLE IF EXISTS TBLS; DROP TABLE IF EXISTS DBS; @@ -31,3 +32,15 @@ CREATE TABLE TBLS ( CONSTRAINT unique_catalog_db_tbl UNIQUE (catalog_name, db_name, tbl_name), CONSTRAINT fk_tbls_db FOREIGN KEY (catalog_name, db_name) REFERENCES DBS(catalog_name, name) ON DELETE CASCADE ON UPDATE CASCADE ); + +CREATE TABLE ALIASES ( + id UUID DEFAULT gen_random_uuid() NOT NULL PRIMARY KEY, + db_id UUID NOT NULL REFERENCES DBS(id), + name VARCHAR(255) NOT NULL, + source_tbl_id UUID NOT NULL REFERENCES TBLS(id), + created_by VARCHAR(255), + created_date TIMESTAMP NOT NULL, + last_updated_by VARCHAR(255), + last_updated_date TIMESTAMP NOT NULL, + CONSTRAINT unique_alias_name_db UNIQUE (name, db_id) +); diff --git a/metacat-connector-polaris/src/functionalTest/scripts/datastores/aurora/docker-entrypoint-initdb-primary.d/schema.sql b/metacat-connector-polaris/src/functionalTest/scripts/datastores/aurora/docker-entrypoint-initdb-primary.d/schema.sql index 97da2d6ca..727c35bf7 100644 --- a/metacat-connector-polaris/src/functionalTest/scripts/datastores/aurora/docker-entrypoint-initdb-primary.d/schema.sql +++ b/metacat-connector-polaris/src/functionalTest/scripts/datastores/aurora/docker-entrypoint-initdb-primary.d/schema.sql @@ -1,6 +1,7 @@ -- Enable the uuid-ossp extension for UUID generation CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; +DROP TABLE IF EXISTS ALIASES; DROP TABLE IF EXISTS TBLS; DROP TABLE IF EXISTS DBS; @@ -37,3 +38,15 @@ CREATE TABLE TBLS ( CONSTRAINT unique_catalog_db_tbl UNIQUE (catalog_name, db_name, tbl_name), CONSTRAINT fk_tbls_db FOREIGN KEY (catalog_name, db_name) REFERENCES DBS(catalog_name, name) ON DELETE RESTRICT ON UPDATE CASCADE ); + +CREATE TABLE ALIASES ( + id UUID DEFAULT uuid_generate_v4() NOT NULL PRIMARY KEY, + db_id UUID NOT NULL REFERENCES DBS(id), + name VARCHAR(255) NOT NULL, + source_tbl_id UUID NOT NULL REFERENCES TBLS(id), + created_by VARCHAR(255), + created_date TIMESTAMP NOT NULL, + last_updated_by VARCHAR(255), + last_updated_date TIMESTAMP NOT NULL, + CONSTRAINT unique_alias_name_db UNIQUE (name, db_id) +); diff --git a/metacat-connector-polaris/src/functionalTest/scripts/datastores/aurora/docker-entrypoint-initdb-reader.d/schema.sql b/metacat-connector-polaris/src/functionalTest/scripts/datastores/aurora/docker-entrypoint-initdb-reader.d/schema.sql index 97da2d6ca..727c35bf7 100644 --- a/metacat-connector-polaris/src/functionalTest/scripts/datastores/aurora/docker-entrypoint-initdb-reader.d/schema.sql +++ b/metacat-connector-polaris/src/functionalTest/scripts/datastores/aurora/docker-entrypoint-initdb-reader.d/schema.sql @@ -1,6 +1,7 @@ -- Enable the uuid-ossp extension for UUID generation CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; +DROP TABLE IF EXISTS ALIASES; DROP TABLE IF EXISTS TBLS; DROP TABLE IF EXISTS DBS; @@ -37,3 +38,15 @@ CREATE TABLE TBLS ( CONSTRAINT unique_catalog_db_tbl UNIQUE (catalog_name, db_name, tbl_name), CONSTRAINT fk_tbls_db FOREIGN KEY (catalog_name, db_name) REFERENCES DBS(catalog_name, name) ON DELETE RESTRICT ON UPDATE CASCADE ); + +CREATE TABLE ALIASES ( + id UUID DEFAULT uuid_generate_v4() NOT NULL PRIMARY KEY, + db_id UUID NOT NULL REFERENCES DBS(id), + name VARCHAR(255) NOT NULL, + source_tbl_id UUID NOT NULL REFERENCES TBLS(id), + created_by VARCHAR(255), + created_date TIMESTAMP NOT NULL, + last_updated_by VARCHAR(255), + last_updated_date TIMESTAMP NOT NULL, + CONSTRAINT unique_alias_name_db UNIQUE (name, db_id) +); diff --git a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisAliasService.java b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisAliasService.java new file mode 100644 index 000000000..6b9b60443 --- /dev/null +++ b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisAliasService.java @@ -0,0 +1,64 @@ +package com.netflix.metacat.connector.polaris; + +import com.netflix.metacat.common.QualifiedName; +import com.netflix.metacat.common.server.properties.Config; +import com.netflix.metacat.common.server.usermetadata.AliasService; +import com.netflix.metacat.connector.polaris.store.repos.PolarisAliasRepository; +import lombok.NonNull; +import lombok.extern.slf4j.Slf4j; + +import java.util.Optional; + +/** + * Resolves table aliases against the polaris store. + */ +@Slf4j +public class PolarisAliasService implements AliasService { + private final PolarisAliasRepository aliasRepository; + private final Config config; + + /** + * Constructor. + * + * @param aliasRepository the alias repository + * @param config the runtime config + */ + public PolarisAliasService(final PolarisAliasRepository aliasRepository, final Config config) { + this.aliasRepository = aliasRepository; + this.config = config; + } + + /** + * {@inheritDoc} + */ + @Override + public QualifiedName getTableName(@NonNull final QualifiedName tableAlias) { + return resolve(tableAlias).orElse(tableAlias); + } + + /** + * {@inheritDoc} + * + *

Overridden so that the answer comes from the same cached lookup as + * {@link #getTableName(QualifiedName)} rather than costing a second resolution. + */ + @Override + public boolean isAlias(@NonNull final QualifiedName name) { + return config.isTableAliasEnabled() && resolve(name).isPresent(); + } + + private Optional resolve(final QualifiedName name) { + if (name.getType() != QualifiedName.Type.TABLE) { + return Optional.empty(); + } + try { + return aliasRepository + .findSourceTable(name.getCatalogName(), name.getDatabaseName(), name.getTableName()) + .map(table -> QualifiedName.ofTable( + table.getCatalogName(), table.getDbName(), table.getTblName())); + } catch (Exception e) { + log.error("Failed to resolve alias for {}", name, e); + throw new IllegalStateException("Failed to resolve alias for " + name, e); + } + } +} diff --git a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/configs/PolarisConnectorConfig.java b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/configs/PolarisConnectorConfig.java index c34a80ae4..403ec9963 100644 --- a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/configs/PolarisConnectorConfig.java +++ b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/configs/PolarisConnectorConfig.java @@ -1,6 +1,7 @@ package com.netflix.metacat.connector.polaris.configs; import com.netflix.metacat.common.server.connectors.ConnectorContext; +import com.netflix.metacat.common.server.usermetadata.AliasService; import com.netflix.metacat.common.server.util.ThreadServiceManager; import com.netflix.metacat.connector.hive.commonview.CommonViewHandler; import com.netflix.metacat.connector.hive.converters.HiveConnectorInfoConverter; @@ -9,18 +10,22 @@ import com.netflix.metacat.connector.hive.iceberg.IcebergTableHandler; import com.netflix.metacat.connector.hive.iceberg.IcebergTableOpWrapper; import com.netflix.metacat.connector.hive.iceberg.IcebergTableOpsProxy; +import com.netflix.metacat.connector.polaris.PolarisAliasService; import com.netflix.metacat.connector.polaris.PolarisConnectorDatabaseService; import com.netflix.metacat.connector.polaris.PolarisConnectorPartitionService; import com.netflix.metacat.connector.polaris.PolarisConnectorTableService; import com.netflix.metacat.connector.polaris.mappers.PolarisTableMapper; import com.netflix.metacat.connector.polaris.store.PolarisStoreService; +import com.netflix.metacat.connector.polaris.store.repos.PolarisAliasRepository; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.context.annotation.Bean; + /** * Config for polaris connector. */ public class PolarisConnectorConfig { + /** * Creates a new instance of a polaris connector partition service. * @@ -87,6 +92,22 @@ public PolarisConnectorTableService polarisTableService( ); } + /** + * Create the alias service backing table alias resolution for this catalog. + * + * @param aliasRepository alias repository + * @param connectorContext server context + * @return AliasService + */ + @Bean + @ConditionalOnMissingBean(AliasService.class) + public AliasService polarisAliasService( + final PolarisAliasRepository aliasRepository, + final ConnectorContext connectorContext + ) { + return new PolarisAliasService(aliasRepository, connectorContext.getConfig()); + } + /** * Create PolarisTableMapper. * @param connectorContext server context diff --git a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/store/entities/PolarisAliasEntity.java b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/store/entities/PolarisAliasEntity.java new file mode 100644 index 000000000..52099efe4 --- /dev/null +++ b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/store/entities/PolarisAliasEntity.java @@ -0,0 +1,55 @@ +package com.netflix.metacat.connector.polaris.store.entities; + +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.ToString; +import org.hibernate.annotations.GenericGenerator; +import org.springframework.data.jpa.domain.support.AuditingEntityListener; + +import jakarta.persistence.Basic; +import jakarta.persistence.Column; +import jakarta.persistence.Embedded; +import jakarta.persistence.Entity; +import jakarta.persistence.EntityListeners; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + +/** + * Entity class for a table alias. + */ +@Getter +@AllArgsConstructor +@NoArgsConstructor +@Builder(toBuilder = true) +@EqualsAndHashCode +@Entity +@ToString(callSuper = true) +@Table(name = "ALIASES") +@EntityListeners(AuditingEntityListener.class) +public class PolarisAliasEntity { + @Basic + @Id + @GeneratedValue(generator = "uuid") + @GenericGenerator(name = "uuid", strategy = "uuid2") + @Column(name = "id", nullable = false, unique = true, updatable = false) + private String aliasId; + + @Basic + @Column(name = "db_id", nullable = false, updatable = false) + private String dbId; + + @Basic + @Column(name = "name", nullable = false, updatable = false) + private String name; + + @Basic + @Column(name = "source_tbl_id", nullable = false) + private String sourceTblId; + + @Embedded + private AuditEntity audit; +} diff --git a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/store/repos/PolarisAliasRepository.java b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/store/repos/PolarisAliasRepository.java new file mode 100644 index 000000000..10160460e --- /dev/null +++ b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/store/repos/PolarisAliasRepository.java @@ -0,0 +1,36 @@ +package com.netflix.metacat.connector.polaris.store.repos; + +import com.netflix.metacat.connector.polaris.store.entities.PolarisAliasEntity; +import com.netflix.metacat.connector.polaris.store.entities.PolarisTableEntity; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; +import org.springframework.stereotype.Repository; + +import java.util.Optional; + +/** + * JPA repository implementation for storing PolarisAliasEntity. + */ +@Repository +public interface PolarisAliasRepository extends JpaRepository { + + /** + * Resolves an alias to the table it points at. + * + * @param catalogName catalog the alias is defined in + * @param dbName database the alias is defined in + * @param aliasName the alias + * @return the source table, or empty if the name is not an alias + */ + @Query("SELECT t FROM PolarisAliasEntity a " + + "JOIN PolarisDatabaseEntity d ON d.dbId = a.dbId " + + "JOIN PolarisTableEntity t ON t.tblId = a.sourceTblId " + + "WHERE d.catalogName = :catalogName " + + "AND d.dbName = :dbName " + + "AND a.name = :aliasName") + Optional findSourceTable( + @Param("catalogName") String catalogName, + @Param("dbName") String dbName, + @Param("aliasName") String aliasName); +} diff --git a/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisAliasServiceTest.java b/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisAliasServiceTest.java new file mode 100644 index 000000000..f8059d311 --- /dev/null +++ b/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisAliasServiceTest.java @@ -0,0 +1,81 @@ +package com.netflix.metacat.connector.polaris; + +import com.netflix.metacat.common.QualifiedName; +import com.netflix.metacat.common.server.properties.Config; +import com.netflix.metacat.connector.polaris.store.entities.PolarisTableEntity; +import com.netflix.metacat.connector.polaris.store.repos.PolarisAliasRepository; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests for alias resolution against the polaris store. + */ +public class PolarisAliasServiceTest { + + private PolarisAliasRepository aliasRepository; + private Config config; + + private PolarisAliasService service; + private QualifiedName alias; + private QualifiedName source; + + @BeforeEach + public void setup() { + aliasRepository = Mockito.mock(PolarisAliasRepository.class); + config = Mockito.mock(Config.class); + when(config.isTableAliasEnabled()).thenReturn(true); + service = new PolarisAliasService(aliasRepository, config); + alias = QualifiedName.ofTable("prodhive", "db", "the_alias"); + source = QualifiedName.ofTable("prodhive", "db", "the_source"); + } + + private void givenAlias(final String catalog, final String db, final String table) { + when(aliasRepository.findSourceTable("prodhive", "db", "the_alias")) + .thenReturn(Optional.of(PolarisTableEntity.builder() + .catalogName(catalog).dbName(db).tblName(table).build())); + } + + @Test + public void testResolvesAliasToSourceTable() { + givenAlias("prodhive", "db", "the_source"); + assertThat(service.getTableName(alias)).isEqualTo(source); + assertThat(service.isAlias(alias)).isTrue(); + } + + @Test + public void testReturnsInputWhenNotAnAlias() { + when(aliasRepository.findSourceTable("prodhive", "db", "the_alias")).thenReturn(Optional.empty()); + assertThat(service.getTableName(alias)).isEqualTo(alias); + assertThat(service.isAlias(alias)).isFalse(); + } + + @Test + public void testResolvesAliasPointingAtAnotherDatabase() { + givenAlias("prodhive", "other_db", "the_source"); + assertThat(service.getTableName(alias)) + .isEqualTo(QualifiedName.ofTable("prodhive", "other_db", "the_source")); + } + + @Test + public void testNonTableNamesAreNotLookedUp() { + final QualifiedName database = QualifiedName.ofDatabase("prodhive", "db"); + assertThat(service.getTableName(database)).isEqualTo(database); + assertThat(service.isAlias(database)).isFalse(); + verify(aliasRepository, never()).findSourceTable("prodhive", "db", null); + } + + @Test + public void testDoesNotResolveAliasWhenAliasNotEnabled() { + givenAlias("prodhive", "other_db", "the_source"); + when(config.isTableAliasEnabled()).thenReturn(false); + assertThat(service.isAlias(QualifiedName.ofTable("prodhive", "other_db", "the_source"))).isFalse(); + } +} diff --git a/metacat-connector-polaris/src/test/resources/h2db/schema.sql b/metacat-connector-polaris/src/test/resources/h2db/schema.sql index d36c00dd8..08abd56ad 100644 --- a/metacat-connector-polaris/src/test/resources/h2db/schema.sql +++ b/metacat-connector-polaris/src/test/resources/h2db/schema.sql @@ -1,3 +1,4 @@ +drop table if exists ALIASES; drop table if exists TBLS; drop table if exists DBS; @@ -30,3 +31,17 @@ create table TBLS ( ); CREATE INDEX DB_NAME_IDX ON TBLS(db_name); + +create table ALIASES ( + id varchar(255) default random_uuid() not null primary key, + db_id varchar(255) not null, + name varchar(255) not null, + source_tbl_id varchar(255) not null, + created_by varchar(255), + created_date TIMESTAMP not null, + last_updated_by varchar(255), + last_updated_date TIMESTAMP not null, + constraint uniq_alias unique(name, db_id), + foreign key (db_id) references DBS(id), + foreign key (source_tbl_id) references TBLS(id) +); diff --git a/metacat-functional-tests/build.gradle b/metacat-functional-tests/build.gradle index 5a1cedf56..25c2bc2d1 100644 --- a/metacat-functional-tests/build.gradle +++ b/metacat-functional-tests/build.gradle @@ -90,9 +90,19 @@ task expandWar(type: Copy) { into file("${buildDir}/metacat-war-expanded/ROOT") } +// Place the test log4j config into the expanded war rather than bind-mounting it over the +// war directory mount. Nested bind mounts require the daemon to create the mountpoint inside +// the host directory, which some container runtimes (e.g. OrbStack) refuse to do. +task copyWarLog4jConfig(type: Copy) { + dependsOn 'expandWar' + + from file('metacat-test-cluster/resources/log4j.properties') + into file("${buildDir}/metacat-war-expanded/ROOT/WEB-INF/classes") +} + // Aurora configuration tasks task startMetacatClusterAurora(type: Exec) { - dependsOn 'expandWar' + dependsOn 'expandWar', 'copyWarLog4jConfig' if (project.ext.startCluster) { commandLine 'sh', rootProject.file('scripts/start_metacat_test_cluster.sh'), project.file('metacat-test-cluster/docker-compose-aurora.yml') } else { diff --git a/metacat-functional-tests/metacat-test-cluster/datastores/aurora/docker-entrypoint-initdb-primary.d/schema.sql b/metacat-functional-tests/metacat-test-cluster/datastores/aurora/docker-entrypoint-initdb-primary.d/schema.sql index 97da2d6ca..727c35bf7 100644 --- a/metacat-functional-tests/metacat-test-cluster/datastores/aurora/docker-entrypoint-initdb-primary.d/schema.sql +++ b/metacat-functional-tests/metacat-test-cluster/datastores/aurora/docker-entrypoint-initdb-primary.d/schema.sql @@ -1,6 +1,7 @@ -- Enable the uuid-ossp extension for UUID generation CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; +DROP TABLE IF EXISTS ALIASES; DROP TABLE IF EXISTS TBLS; DROP TABLE IF EXISTS DBS; @@ -37,3 +38,15 @@ CREATE TABLE TBLS ( CONSTRAINT unique_catalog_db_tbl UNIQUE (catalog_name, db_name, tbl_name), CONSTRAINT fk_tbls_db FOREIGN KEY (catalog_name, db_name) REFERENCES DBS(catalog_name, name) ON DELETE RESTRICT ON UPDATE CASCADE ); + +CREATE TABLE ALIASES ( + id UUID DEFAULT uuid_generate_v4() NOT NULL PRIMARY KEY, + db_id UUID NOT NULL REFERENCES DBS(id), + name VARCHAR(255) NOT NULL, + source_tbl_id UUID NOT NULL REFERENCES TBLS(id), + created_by VARCHAR(255), + created_date TIMESTAMP NOT NULL, + last_updated_by VARCHAR(255), + last_updated_date TIMESTAMP NOT NULL, + CONSTRAINT unique_alias_name_db UNIQUE (name, db_id) +); diff --git a/metacat-functional-tests/metacat-test-cluster/datastores/aurora/docker-entrypoint-initdb-reader.d/schema.sql b/metacat-functional-tests/metacat-test-cluster/datastores/aurora/docker-entrypoint-initdb-reader.d/schema.sql index 97da2d6ca..727c35bf7 100644 --- a/metacat-functional-tests/metacat-test-cluster/datastores/aurora/docker-entrypoint-initdb-reader.d/schema.sql +++ b/metacat-functional-tests/metacat-test-cluster/datastores/aurora/docker-entrypoint-initdb-reader.d/schema.sql @@ -1,6 +1,7 @@ -- Enable the uuid-ossp extension for UUID generation CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; +DROP TABLE IF EXISTS ALIASES; DROP TABLE IF EXISTS TBLS; DROP TABLE IF EXISTS DBS; @@ -37,3 +38,15 @@ CREATE TABLE TBLS ( CONSTRAINT unique_catalog_db_tbl UNIQUE (catalog_name, db_name, tbl_name), CONSTRAINT fk_tbls_db FOREIGN KEY (catalog_name, db_name) REFERENCES DBS(catalog_name, name) ON DELETE RESTRICT ON UPDATE CASCADE ); + +CREATE TABLE ALIASES ( + id UUID DEFAULT uuid_generate_v4() NOT NULL PRIMARY KEY, + db_id UUID NOT NULL REFERENCES DBS(id), + name VARCHAR(255) NOT NULL, + source_tbl_id UUID NOT NULL REFERENCES TBLS(id), + created_by VARCHAR(255), + created_date TIMESTAMP NOT NULL, + last_updated_by VARCHAR(255), + last_updated_date TIMESTAMP NOT NULL, + CONSTRAINT unique_alias_name_db UNIQUE (name, db_id) +); diff --git a/metacat-functional-tests/metacat-test-cluster/docker-compose-aurora.yml b/metacat-functional-tests/metacat-test-cluster/docker-compose-aurora.yml index 928ca93a7..3f8c22c6b 100644 --- a/metacat-functional-tests/metacat-test-cluster/docker-compose-aurora.yml +++ b/metacat-functional-tests/metacat-test-cluster/docker-compose-aurora.yml @@ -40,8 +40,8 @@ services: - '12003' - '12004' volumes: + # log4j.properties is copied into the expanded war by the copyWarLog4jConfig gradle task - ../build/metacat-war-expanded/ROOT:/usr/local/tomcat/webapps/ROOT - - ./resources/log4j.properties:/usr/local/tomcat/webapps/ROOT/WEB-INF/classes/log4j.properties - ./resources/logging.properties:/usr/local/tomcat/conf/logging.properties - ../build/logs/metacat:/usr/local/tomcat/logs - ../build/logs/metacat:/var/log/metacat @@ -70,6 +70,7 @@ services: -Dmetacat.definition.metadata.delete.enableDeleteForQualifiedNames=hive-metastore/hsmoke_ddb,hive-metastore/hsmoke_ddb1/test_create_table1,embedded-hive-metastore,embedded-fast-hive-metastore/fsmoke_db1,embedded-fast-hive-metastore/fsmoke_ddb1,embedded-fast-hive-metastore/shard,embedded-fast-hive-metastore/fsmoke_db4,s3-mysql-db,mysql-56-db -Dmetacat.hive.metastore.batchSize=10 -Dmetacat.hive.iceberg.enabled=true + -Dmetacat.alias-service-properties.enabled=true -Dmetacat.usermetadata.config.location=/etc/metacat/usermetadata.properties -Dmetacat.cache.enabled=true -Dmetacat.authorization.enabled=true diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/api/RequestWrapper.java b/metacat-main/src/main/java/com/netflix/metacat/main/api/RequestWrapper.java index f5642c2b9..55cee49ad 100644 --- a/metacat-main/src/main/java/com/netflix/metacat/main/api/RequestWrapper.java +++ b/metacat-main/src/main/java/com/netflix/metacat/main/api/RequestWrapper.java @@ -103,18 +103,29 @@ public RequestWrapper(@NotNull @NonNull final Registry registry, } /** - * Creates the qualified name. + * Creates the qualified name, resolving a table alias to the table it points at. * * @param nameSupplier supplier * @return name */ public QualifiedName qualifyName(final Supplier nameSupplier) { + final QualifiedName name = unresolvedQualifiedName(nameSupplier); + if (config.isTableAliasEnabled() && name.getType() == QualifiedName.Type.TABLE) { + return aliasService.getTableName(name); + } + return name; + } + + /** + * Creates the qualified name without resolving a table alias. Table endpoints use this so that the + * table service sees the name as it was requested, and can resolve or reject the alias itself. + * + * @param nameSupplier supplier + * @return name + */ + public QualifiedName unresolvedQualifiedName(final Supplier nameSupplier) { try { - final QualifiedName name = nameSupplier.get(); - if (config.isTableAliasEnabled() && name.getType() == QualifiedName.Type.TABLE) { - return aliasService.getTableName(name); - } - return name; + return nameSupplier.get(); } catch (Exception e) { log.error("Invalid qualified name", e); throw new MetacatBadRequestException(e.getMessage()); diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/MetacatController.java b/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/MetacatController.java index 80809bb8a..3340f4c91 100644 --- a/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/MetacatController.java +++ b/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/MetacatController.java @@ -219,7 +219,7 @@ public TableDto createTable( @Parameter(description = "The table information", required = true) @Valid @RequestBody final TableDto table ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); if (MetacatServiceHelper.isIcebergTable(table)) { @@ -387,7 +387,7 @@ public TableDto deleteTable( @Parameter(description = "The name of the table", required = true) @PathVariable("table-name") final String tableName ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); return this.requestWrapper.processRequest( @@ -642,9 +642,9 @@ public TableDto getTable( name = "includeMetadataLocationOnly", defaultValue = "false") final boolean includeMetadataLocationOnly ) { - final Supplier qualifiedNameSupplier = - () -> QualifiedName.ofTable(catalogName, databaseName, tableName); - final QualifiedName name = this.requestWrapper.qualifyName(qualifiedNameSupplier); + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( + () -> QualifiedName.ofTable(catalogName, databaseName, tableName) + ); return this.requestWrapper.processRequest( name, "getTable", @@ -669,11 +669,7 @@ public TableDto getTable( .build() ); - final TableDto tableDto = table.orElseThrow(() -> new TableNotFoundException(name)); - // Set the name to whatever the request was for because - // for aliases, this could've been set to the original name - tableDto.setName(qualifiedNameSupplier.get()); - return tableDto; + return table.orElseThrow(() -> new TableNotFoundException(name)); } ); } @@ -1001,10 +997,10 @@ public void renameTable( @Parameter(description = "The name of the table", required = true) @RequestParam("newTableName") final String newTableName ) { - final QualifiedName oldName = this.requestWrapper.qualifyName( + final QualifiedName oldName = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); - final QualifiedName newName = this.requestWrapper.qualifyName( + final QualifiedName newName = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, newTableName) ); this.requestWrapper.processRequest( @@ -1212,18 +1208,13 @@ public TableDto updateTable( name = "shouldThrowExceptionOnMetadataSaveFailure", defaultValue = "false") final boolean shouldThrowExceptionOnMetadataSaveFailure ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); return this.requestWrapper.processRequest( name, "updateTable", () -> { - Preconditions.checkArgument(table.getName() != null - && tableName.equalsIgnoreCase(table.getName().getTableName() - ), - "Table name does not match the name in the table" - ); return this.tableService.updateAndReturn(name, table, shouldThrowExceptionOnMetadataSaveFailure); } ); diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/PartitionController.java b/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/PartitionController.java index fd9040f28..c0d2efde3 100644 --- a/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/PartitionController.java +++ b/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/PartitionController.java @@ -117,7 +117,7 @@ public void deletePartitions( @Parameter(description = "partitionId of the partitions to be deleted from this table", required = true) @RequestBody final List partitionIds ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); this.requestWrapper.processRequest( @@ -179,7 +179,7 @@ public void deletePartitions( @Parameter(description = "partitionId of the partitions to be deleted from this table", required = true) @RequestBody final List partitionIds ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName) ); this.requestWrapper.processRequest( @@ -251,7 +251,7 @@ public List getPartitions( @Parameter(description = "Whether to include user metadata information to the response") @RequestParam(name = "includeUserMetadata", defaultValue = "false") final boolean includeUserMetadata ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); return this.requestWrapper.processRequest( @@ -327,7 +327,7 @@ public List getPartitions( @Parameter(description = "Whether to include user metadata information to the response") @RequestParam(name = "includeUserMetadata", defaultValue = "false") final boolean includeUserMetadata ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName) ); return this.requestWrapper.processRequest( @@ -1123,7 +1123,7 @@ public PartitionsSaveResponseDto savePartitions( @Parameter(description = "Request containing the list of partitions", required = true) @RequestBody final PartitionsSaveRequestDto partitionsSaveRequestDto ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName) ); return this.requestWrapper.processRequest( @@ -1188,7 +1188,7 @@ public Integer getPartitionCount( @Parameter(description = "The name of the table", required = true) @PathVariable("table-name") final String tableName ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); return this.requestWrapper.processRequest( @@ -1237,7 +1237,7 @@ public Integer getPartitionCount( @Parameter(description = "The name of the view", required = true) @PathVariable("view-name") final String viewName ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName) ); return this.requestWrapper.processRequest( @@ -1258,7 +1258,7 @@ private List getPartitions( final boolean includeUserMetadata, @Nullable final GetPartitionsRequestDto getPartitionsRequestDto ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); @@ -1291,7 +1291,7 @@ private List getPartitions( final boolean includeUserMetadata, @Nullable final GetPartitionsRequestDto getPartitionsRequestDto ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName) ); return this.requestWrapper.processRequest( @@ -1321,7 +1321,7 @@ private List _getPartitionUris( @Nullable final Integer limit, @Nullable final GetPartitionsRequestDto getPartitionsRequestDto ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); return this.requestWrapper.processRequest( @@ -1352,7 +1352,7 @@ private List _getMViewPartitionKeys( @Nullable final GetPartitionsRequestDto getPartitionsRequestDto ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName) ); return this.requestWrapper.processRequest( @@ -1382,7 +1382,7 @@ private List _getMViewPartitionUris( @Nullable final Integer limit, @Nullable final GetPartitionsRequestDto getPartitionsRequestDto ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofView(catalogName, databaseName, tableName, viewName) ); return this.requestWrapper.processRequest( @@ -1412,7 +1412,7 @@ private List _getPartitionKeys( @Nullable final GetPartitionsRequestDto getPartitionsRequestDto ) { - final QualifiedName name = this.requestWrapper.qualifyName( + final QualifiedName name = this.requestWrapper.unresolvedQualifiedName( () -> QualifiedName.ofTable(catalogName, databaseName, tableName) ); return this.requestWrapper.processRequest( diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/configs/ServicesConfig.java b/metacat-main/src/main/java/com/netflix/metacat/main/configs/ServicesConfig.java index b15f751db..98c6eb84c 100644 --- a/metacat-main/src/main/java/com/netflix/metacat/main/configs/ServicesConfig.java +++ b/metacat-main/src/main/java/com/netflix/metacat/main/configs/ServicesConfig.java @@ -27,7 +27,6 @@ import com.netflix.metacat.common.server.properties.Config; import com.netflix.metacat.common.server.usermetadata.AliasService; import com.netflix.metacat.common.server.usermetadata.AuthorizationService; -import com.netflix.metacat.common.server.usermetadata.DefaultAliasService; import com.netflix.metacat.common.server.usermetadata.DefaultAuthorizationService; import com.netflix.metacat.common.server.usermetadata.DefaultLookupService; import com.netflix.metacat.common.server.usermetadata.DefaultTagService; @@ -53,6 +52,7 @@ import com.netflix.metacat.main.services.TableService; import com.netflix.metacat.main.services.health.MetacatHealthIndicator; import com.netflix.metacat.main.services.impl.CatalogServiceImpl; +import com.netflix.metacat.main.services.impl.ConnectorAliasService; import com.netflix.metacat.main.services.impl.ConnectorTableServiceProxy; import com.netflix.metacat.main.services.impl.DatabaseServiceImpl; import com.netflix.metacat.main.services.impl.DefaultOwnerValidationService; @@ -125,14 +125,16 @@ public OwnerValidationService ownerValidationService(final Registry registry) { } /** - * Alias service. + * Alias service. Delegates to the connector that owns the catalog, since that is where alias + * data lives; catalogs whose connector provides no alias service resolve to themselves. * + * @param connectorManager connector manager * @return an instance of the Alias service. */ @Bean @ConditionalOnMissingBean(AliasService.class) - public AliasService aliasService() { - return new DefaultAliasService(); + public AliasService aliasService(final ConnectorManager connectorManager) { + return new ConnectorAliasService(connectorManager); } /** @@ -230,6 +232,7 @@ public DatabaseService databaseService( * @param authorizationService authorization Service * @param ownerValidationService owner validation service * @param parentChildRelMetadataService parentChildRelMetadataService + * @param aliasService alias service * * @return The table service bean */ @@ -247,7 +250,8 @@ public TableService tableService( final ConverterUtil converterUtil, final AuthorizationService authorizationService, final OwnerValidationService ownerValidationService, - final ParentChildRelMetadataService parentChildRelMetadataService) { + final ParentChildRelMetadataService parentChildRelMetadataService, + final AliasService aliasService) { return new TableServiceImpl( connectorManager, connectorTableServiceProxy, @@ -261,7 +265,8 @@ public TableService tableService( converterUtil, authorizationService, ownerValidationService, - parentChildRelMetadataService + parentChildRelMetadataService, + aliasService ); } diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/ConnectorAliasService.java b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/ConnectorAliasService.java new file mode 100644 index 000000000..1444da443 --- /dev/null +++ b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/ConnectorAliasService.java @@ -0,0 +1,78 @@ +/* + * Copyright 2026 Netflix, Inc. + * + * Licensed 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 com.netflix.metacat.main.services.impl; + +import com.netflix.metacat.common.QualifiedName; +import com.netflix.metacat.common.server.connectors.ConnectorFactory; +import com.netflix.metacat.common.server.connectors.ConnectorFactoryDecorator; +import com.netflix.metacat.common.server.connectors.SpringConnectorFactory; +import com.netflix.metacat.common.server.connectors.exception.CatalogNotFoundException; +import com.netflix.metacat.common.server.usermetadata.AliasService; +import com.netflix.metacat.main.manager.ConnectorManager; +import lombok.NonNull; +import lombok.RequiredArgsConstructor; + +import java.util.Optional; + +/** + * Alias service that delegates to the connector backing the catalog in the name. + * + * Catalogs whose connector registers no {@link AliasService} have no + * aliases and their names pass through untouched. + */ +@RequiredArgsConstructor +public class ConnectorAliasService implements AliasService { + private final ConnectorManager connectorManager; + + /** + * {@inheritDoc} + */ + @Override + public QualifiedName getTableName(@NonNull final QualifiedName tableAlias) { + return forCatalog(tableAlias) + .map(aliasService -> aliasService.getTableName(tableAlias)) + .orElse(tableAlias); + } + + /** + * {@inheritDoc} + */ + @Override + public boolean isAlias(@NonNull final QualifiedName name) { + return forCatalog(name) + .map(aliasService -> aliasService.isAlias(name)) + .orElse(false); + } + + private Optional forCatalog(final QualifiedName name) { + final ConnectorFactory factory; + try { + factory = connectorManager.getConnectorFactory(name); + } catch (CatalogNotFoundException e) { + // An unknown catalog is not this service's error to raise. Pass the name through so the + // request fails where it normally would, with its usual message. + return Optional.empty(); + } + final ConnectorFactory delegate = factory instanceof ConnectorFactoryDecorator + ? ((ConnectorFactoryDecorator) factory).getDelegate() + : factory; + return delegate instanceof SpringConnectorFactory + ? Optional.ofNullable(((SpringConnectorFactory) delegate).getBeanIfAvailable(AliasService.class)) + : Optional.empty(); + } +} diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java index 6c55baa3c..dbb5df72b 100644 --- a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java +++ b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java @@ -27,6 +27,7 @@ import com.netflix.metacat.common.dto.DatabaseDto; import com.netflix.metacat.common.dto.StorageDto; import com.netflix.metacat.common.dto.TableDto; +import com.netflix.metacat.common.exception.MetacatAlreadyExistsException; import com.netflix.metacat.common.exception.MetacatBadRequestException; import com.netflix.metacat.common.exception.MetacatNotSupportedException; import com.netflix.metacat.common.json.MetacatJson; @@ -51,6 +52,7 @@ import com.netflix.metacat.common.server.monitoring.Metrics; import com.netflix.metacat.common.server.properties.Config; import com.netflix.metacat.common.server.spi.MetacatCatalogConfig; +import com.netflix.metacat.common.server.usermetadata.AliasService; import com.netflix.metacat.common.server.usermetadata.AuthorizationService; import com.netflix.metacat.common.server.usermetadata.GetMetadataInterceptorParameters; import com.netflix.metacat.common.server.usermetadata.ParentChildRelMetadataConstants; @@ -103,6 +105,7 @@ public class TableServiceImpl implements TableService { private final AuthorizationService authorizationService; private final OwnerValidationService ownerValidationService; private final ParentChildRelMetadataService parentChildRelMetadataService; + private final AliasService aliasService; /** * {@inheritDoc} @@ -111,6 +114,11 @@ public class TableServiceImpl implements TableService { public TableDto create(final QualifiedName name, final TableDto tableDto) { final MetacatRequestContext metacatRequestContext = MetacatContextManager.getContext(); validate(name); + + if (isAlias(name)) { + throw new MetacatAlreadyExistsException("Alias with name: " + name + ", already exists."); + } + this.authorizationService.checkPermission(metacatRequestContext.getUserName(), tableDto.getName(), MetacatOperation.CREATE); @@ -348,6 +356,11 @@ private void tag(final QualifiedName name, final ObjectNode definitionMetadata) public TableDto deleteAndReturn(final QualifiedName name, final boolean isMView) { final MetacatRequestContext metacatRequestContext = MetacatContextManager.getContext(); validate(name); + + if (isAlias(name)) { + throw new MetacatNotSupportedException("Deleting alias: " + name + " is not allowed"); + } + this.authorizationService.checkPermission(metacatRequestContext.getUserName(), name, MetacatOperation.DELETE); @@ -510,14 +523,20 @@ private boolean isEnabledForTableDefinitionMetadataDelete(final QualifiedName ta * {@inheritDoc} */ @Override - public Optional get(final QualifiedName name, final GetTableServiceParameters getTableServiceParameters) { + public Optional get(final QualifiedName requestedName, + final GetTableServiceParameters getTableServiceParameters) { + // A read that wants table info must fail on an alias rather than quietly serve the table it + // points at, so its name is left unresolved for the connector to reject. + final QualifiedName name = resolveAlias(requestedName, getTableServiceParameters); + final boolean isAliasRead = isAlias(requestedName); + validate(name); TableDto tableInternal = null; final TableDto table; final MetacatCatalogConfig catalogConfig = connectorManager.getCatalogConfig(name); - if (getTableServiceParameters.isIncludeInfo() + if (!isAliasRead && (getTableServiceParameters.isIncludeInfo() || (getTableServiceParameters.isIncludeDefinitionMetadata() && catalogConfig.isInterceptorEnabled() - && !getTableServiceParameters.isDisableOnReadMetadataIntercetor())) { + && !getTableServiceParameters.isDisableOnReadMetadataIntercetor()))) { try { final boolean useCache = getTableServiceParameters.isUseCache() && config.isCacheEnabled() && catalogConfig.isCacheEnabled(); @@ -558,7 +577,7 @@ public Optional get(final QualifiedName name, final GetTableServicePar if (getTableServiceParameters.isIncludeDataMetadata() && catalogConfig.isHasDataExternal()) { TableDto dto = table; - if (tableInternal == null && !getTableServiceParameters.isIncludeInfo()) { + if (!isAliasRead && tableInternal == null && !getTableServiceParameters.isIncludeInfo()) { try { final boolean useCache = getTableServiceParameters.isUseCache() && config.isCacheEnabled(); dto = converterUtil.toTableDto( @@ -586,6 +605,15 @@ public void rename( final boolean isMView ) { validate(oldName); + + if (isAlias(oldName)) { + throw new MetacatBadRequestException("Renaming of aliases is not supported: " + oldName); + } + + if (isAlias(newName)) { + throw new MetacatAlreadyExistsException("Table " + newName + " already exists"); + } + final MetacatRequestContext metacatRequestContext = MetacatContextManager.getContext(); this.authorizationService.checkPermission(metacatRequestContext.getUserName(), oldName, MetacatOperation.RENAME); @@ -875,9 +903,25 @@ public List getQualifiedNames(final QualifiedName name, } @Override - public TableDto updateAndReturn(final QualifiedName name, final TableDto tableDto, + public TableDto updateAndReturn(final QualifiedName requestedName, final TableDto tableDto, final boolean shouldThrowExceptionOnMetadataSaveFailure) { + final QualifiedName name = resolveAlias(requestedName); validate(name); + + if (isAlias(requestedName) && carriesTableInfo(tableDto)) { + throw new MetacatNotSupportedException( + "Updating table info through alias " + requestedName + " is not supported"); + } + + final QualifiedName dtoName = tableDto.getName(); + Preconditions.checkArgument( + Objects.isNull(dtoName) || name.getTableName().equalsIgnoreCase(dtoName.getTableName()), + "Table name does not match the name in the table" + ); + // Past this point the alias does not exist: everything, user metadata included, is keyed off + // the table it resolved to. + tableDto.setName(name); + final MetacatRequestContext metacatRequestContext = MetacatContextManager.getContext(); final TableDto oldTable = get(name, GetTableServiceParameters.builder() .disableOnReadMetadataIntercetor(false) @@ -996,5 +1040,57 @@ private void validate(final QualifiedName name) { Preconditions.checkArgument(name.isTableDefinition(), "Definition {} does not refer to a table", name); } + /** + * Whether the dto carries anything that would reach the connector, as opposed to user metadata. + * + * @param tableDto the dto + * @return true if the dto describes the table itself + */ + private static boolean carriesTableInfo(final TableDto tableDto) { + return (tableDto.getFields() != null && !tableDto.getFields().isEmpty()) + || (tableDto.getMetadata() != null && !tableDto.getMetadata().isEmpty()) + || tableDto.getSerde() != null; + } + + private QualifiedName resolveAlias(final QualifiedName name) { + return config.isTableAliasEnabled() ? aliasService.getTableName(name) : name; + } + + /** + * Resolves a table alias to the table it points at. Names that are not aliases, and every name at + * all when table aliasing is disabled, are returned untouched. + * + * @param name the requested name + * @param getTableServiceParameters params from request + * @return the table the alias points at, or the given name + */ + private QualifiedName resolveAlias(final QualifiedName name, + final GetTableServiceParameters getTableServiceParameters) { + return readsTableInfo(getTableServiceParameters) ? name : resolveAlias(name); + } + /** + * Whether the read asks for anything the connector owns, as opposed to user metadata. Three + * separate parameters select a connector read, see {@link #getFromTableServiceProxy}. + * + * @param getTableServiceParameters params from request + * @return true if the read wants the table itself + */ + private static boolean readsTableInfo(final GetTableServiceParameters getTableServiceParameters) { + return getTableServiceParameters.isIncludeInfo() + || getTableServiceParameters.isIncludeMetadataLocationOnly() + || getTableServiceParameters.isIncludeMetadataFromConnector(); + } + + /** + * Whether the name is a table alias. False whenever table aliasing is disabled, so that this + * agrees with {@link #resolveAlias(QualifiedName)} without relying on every {@link AliasService} + * implementation to check the flag itself. + * + * @param name the requested name + * @return true if the name is an alias + */ + private boolean isAlias(final QualifiedName name) { + return config.isTableAliasEnabled() && aliasService.isAlias(name); + } } diff --git a/metacat-main/src/test/groovy/com/netflix/metacat/main/api/RequestWrapperSpec.groovy b/metacat-main/src/test/groovy/com/netflix/metacat/main/api/RequestWrapperSpec.groovy index 458004d47..df4efa1b2 100644 --- a/metacat-main/src/test/groovy/com/netflix/metacat/main/api/RequestWrapperSpec.groovy +++ b/metacat-main/src/test/groovy/com/netflix/metacat/main/api/RequestWrapperSpec.groovy @@ -71,4 +71,72 @@ class RequestWrapperSpec extends Specification { then: MetacatContextManager.getContext().getRequestName() == "getTable" } + + def "qualifyName resolves a table alias to its source table"() { + given: + def alias = QualifiedName.ofTable("c", "d", "the_alias") + def source = QualifiedName.ofTable("c", "d", "the_source") + + when: + def result = requestWrapper.qualifyName({ alias }) + + then: + 1 * config.isTableAliasEnabled() >> true + 1 * aliasService.getTableName(alias) >> source + result == source + } + + def "qualifyName does not consult the alias service when aliasing is disabled"() { + given: + def alias = QualifiedName.ofTable("c", "d", "the_alias") + + when: + def result = requestWrapper.qualifyName({ alias }) + + then: + 1 * config.isTableAliasEnabled() >> false + 0 * aliasService.getTableName(_) + result == alias + } + + def "qualifyName only resolves aliases for table names"() { + given: + def dbName = QualifiedName.ofDatabase("c", "d") + + when: + def result = requestWrapper.qualifyName({ dbName }) + + then: + 0 * aliasService.getTableName(_) + result == dbName + } + + def "qualifyName converts a failing supplier into a bad request"() { + when: + requestWrapper.qualifyName({ throw new IllegalArgumentException("bad name") }) + + then: + thrown(MetacatBadRequestException) + } + + def "unresolvedQualifiedName leaves an alias alone, so the table service can decide what to do with it"() { + given: + def alias = QualifiedName.ofTable("c", "d", "the_alias") + + when: + def result = requestWrapper.unresolvedQualifiedName({ alias }) + + then: + 0 * aliasService.getTableName(_) + 0 * aliasService.isAlias(_) + result == alias + } + + def "unresolvedQualifiedName converts a failing supplier into a bad request"() { + when: + requestWrapper.unresolvedQualifiedName({ throw new IllegalArgumentException("bad name") }) + + then: + thrown(MetacatBadRequestException) + } } diff --git a/metacat-main/src/test/groovy/com/netflix/metacat/main/api/v1/MetacatControllerAliasSpec.groovy b/metacat-main/src/test/groovy/com/netflix/metacat/main/api/v1/MetacatControllerAliasSpec.groovy new file mode 100644 index 000000000..a78b0da39 --- /dev/null +++ b/metacat-main/src/test/groovy/com/netflix/metacat/main/api/v1/MetacatControllerAliasSpec.groovy @@ -0,0 +1,151 @@ +/* + * Copyright 2026 Netflix, Inc. + * + * Licensed 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 com.netflix.metacat.main.api.v1 + +import com.netflix.metacat.common.QualifiedName +import com.netflix.metacat.common.dto.TableDto +import com.netflix.metacat.common.exception.MetacatBadRequestException +import com.netflix.metacat.common.server.api.traffic_control.RequestGateway +import com.netflix.metacat.common.exception.MetacatNotFoundException +import com.netflix.metacat.common.server.properties.Config +import com.netflix.metacat.common.server.usermetadata.AliasService +import com.netflix.metacat.main.api.RequestWrapper +import com.netflix.metacat.main.services.CatalogService +import com.netflix.metacat.main.services.DatabaseService +import com.netflix.metacat.main.services.MViewService +import com.netflix.metacat.main.services.TableService +import com.netflix.spectator.api.NoopRegistry +import spock.lang.Specification + +/** + * Covers how table endpoints behave when the requested name is an alias. + */ +class MetacatControllerAliasSpec extends Specification { + + def config = Mock(Config) + def aliasService = Mock(AliasService) + def requestGateway = Mock(RequestGateway) + def tableService = Mock(TableService) + def requestWrapper = new RequestWrapper(new NoopRegistry(), config, aliasService, requestGateway) + def controller = new MetacatController( + Mock(CatalogService), Mock(DatabaseService), Mock(MViewService), + tableService, requestWrapper, config) + + def alias = QualifiedName.ofTable("prodhive", "db", "the_alias") + def source = QualifiedName.ofTable("prodhive", "db", "the_source") + + private static TableDto tableDto(QualifiedName name) { + def dto = new TableDto() + dto.setName(name) + return dto + } + + def "updateTable hands the alias down untouched, leaving the table service to resolve it"() { + given: "a request against an alias, with the body named for the alias the caller asked for" + def body = tableDto(alias) + + when: + controller.updateTable("prodhive", "db", "the_alias", body, false) + + then: "the controller does not resolve the alias itself" + 0 * aliasService.getTableName(_) + 1 * tableService.updateAndReturn(alias, body, false) >> tableDto(source) + } + + def "getTable passes the requested name through unresolved, since TableServiceImpl decides whether to resolve it"() { + when: + def result = controller.getTable("prodhive", "db", "the_alias", true, true, true, false, false) + + then: "the controller no longer resolves aliases itself" + 0 * aliasService.getTableName(_) + 1 * tableService.get(alias, _) >> Optional.of(tableDto(source)) + result.getName() == source + } + + def "getTable against an alias 404s when TableServiceImpl leaves it unresolved (includeInfo reads)"() { + when: + controller.getTable("prodhive", "db", "the_alias", true, true, true, false, false) + + then: "the connector has no concept of aliases, so an unresolved alias name naturally 404s" + 1 * tableService.get(alias, _) >> Optional.empty() + thrown(MetacatNotFoundException) + } + + def "updateTable responds with the source table the service resolved to, never the alias"() { + given: + def body = tableDto(alias) + + when: + def result = controller.updateTable("prodhive", "db", "the_alias", body, false) + + then: + 1 * tableService.updateAndReturn(alias, _, false) >> tableDto(source) + and: "unlike the old behaviour, the requested alias is not stamped back on" + result.getName() == source + } + + def "updateTable dosen't require the body to be named for the table that was requested"() { + when: "the body names a different table than the path" + controller.updateTable("prodhive", "db", "the_alias", tableDto(source), false) + + then: + 1 * tableService.updateAndReturn(_, _, _) >> tableDto(source) + } + + def "updateTable leaves a non-alias name untouched"() { + given: + def real = QualifiedName.ofTable("prodhive", "db", "a_real_table") + def body = tableDto(real) + + when: + controller.updateTable("prodhive", "db", "a_real_table", body, false) + + then: + 0 * aliasService.getTableName(_) + 1 * tableService.updateAndReturn(real, body, false) >> body + } + + def "deleteTable hands the alias down untouched, so the table service can refuse it"() { + when: + controller.deleteTable("prodhive", "db", "the_alias") + + then: "the controller no longer resolves or rejects aliases itself" + 0 * aliasService.getTableName(_) + 1 * tableService.deleteAndReturn(alias, false) >> tableDto(alias) + } + + def "createTable hands the alias down untouched, so the table service can refuse it"() { + when: + controller.createTable("prodhive", "db", "the_alias", tableDto(alias)) + + then: "the controller no longer resolves or rejects aliases itself" + 0 * aliasService.getTableName(_) + 1 * tableService.create(alias, _) >> tableDto(alias) + } + + def "renameTable hands both names down untouched, so the table service can refuse them"() { + given: + def newName = QualifiedName.ofTable("prodhive", "db", "the_new_name") + + when: + controller.renameTable("prodhive", "db", "the_alias", "the_new_name") + + then: "the controller no longer resolves or rejects aliases itself" + 0 * aliasService.getTableName(_) + 1 * tableService.rename(alias, newName, false) + } +} diff --git a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/ConnectorAliasServiceSpec.groovy b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/ConnectorAliasServiceSpec.groovy new file mode 100644 index 000000000..5738e62ef --- /dev/null +++ b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/ConnectorAliasServiceSpec.groovy @@ -0,0 +1,96 @@ +/* + * Copyright 2026 Netflix, Inc. + * + * Licensed 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 com.netflix.metacat.main.services.impl + +import com.netflix.metacat.common.QualifiedName +import com.netflix.metacat.common.server.connectors.ConnectorFactory +import com.netflix.metacat.common.server.connectors.ConnectorFactoryDecorator +import com.netflix.metacat.common.server.connectors.SpringConnectorFactory +import com.netflix.metacat.common.server.connectors.exception.CatalogNotFoundException +import com.netflix.metacat.common.server.usermetadata.AliasService +import com.netflix.metacat.main.manager.ConnectorManager +import spock.lang.Specification + +class ConnectorAliasServiceSpec extends Specification { + + def connectorManager = Mock(ConnectorManager) + def connectorAliasService = new ConnectorAliasService(connectorManager) + + def alias = QualifiedName.ofTable("prodhive", "db", "the_alias") + def source = QualifiedName.ofTable("prodhive", "db", "the_source") + + private def springFactoryProviding(AliasService aliasService) { + def springFactory = Mock(SpringConnectorFactory) + springFactory.getBeanIfAvailable(AliasService) >> aliasService + def decorator = Mock(ConnectorFactoryDecorator) + decorator.getDelegate() >> springFactory + return decorator + } + + def "delegates resolution to the catalog's connector"() { + given: + def polarisAliasService = Mock(AliasService) + connectorManager.getConnectorFactory(alias) >> springFactoryProviding(polarisAliasService) + + when: + def result = connectorAliasService.getTableName(alias) + + then: + 1 * polarisAliasService.getTableName(alias) >> source + result == source + } + + def "delegates isAlias to the catalog's connector"() { + given: + def polarisAliasService = Mock(AliasService) + connectorManager.getConnectorFactory(alias) >> springFactoryProviding(polarisAliasService) + + when: + def result = connectorAliasService.isAlias(alias) + + then: + 1 * polarisAliasService.isAlias(alias) >> true + result + } + + def "passes the name through when the connector provides no alias service"() { + given: + connectorManager.getConnectorFactory(alias) >> springFactoryProviding(null) + + expect: + connectorAliasService.getTableName(alias) == alias + !connectorAliasService.isAlias(alias) + } + + def "passes the name through for a non-spring connector"() { + given: + connectorManager.getConnectorFactory(alias) >> Mock(ConnectorFactory) + + expect: + connectorAliasService.getTableName(alias) == alias + !connectorAliasService.isAlias(alias) + } + + def "passes the name through for an unknown catalog rather than raising"() { + given: + connectorManager.getConnectorFactory(alias) >> { throw new CatalogNotFoundException("prodhive") } + + expect: + connectorAliasService.getTableName(alias) == alias + !connectorAliasService.isAlias(alias) + } +} diff --git a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy index 9d4c11342..50b997002 100644 --- a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy +++ b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy @@ -24,8 +24,12 @@ import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.node.ObjectNode import com.netflix.metacat.common.QualifiedName import com.netflix.metacat.common.dto.AuditDto +import com.netflix.metacat.common.dto.FieldDto import com.netflix.metacat.common.dto.StorageDto import com.netflix.metacat.common.dto.TableDto +import com.netflix.metacat.common.exception.MetacatAlreadyExistsException +import com.netflix.metacat.common.exception.MetacatBadRequestException +import com.netflix.metacat.common.exception.MetacatNotSupportedException import com.netflix.metacat.common.json.MetacatJsonLocator import com.netflix.metacat.common.server.connectors.ConnectorRequestContext import com.netflix.metacat.common.server.connectors.ConnectorTableService @@ -40,6 +44,7 @@ import com.netflix.metacat.common.server.model.ChildInfo import com.netflix.metacat.common.server.properties.Config import com.netflix.metacat.common.server.properties.ParentChildRelationshipProperties import com.netflix.metacat.common.server.spi.MetacatCatalogConfig +import com.netflix.metacat.common.server.usermetadata.AliasService import com.netflix.metacat.common.server.usermetadata.DefaultAuthorizationService import com.netflix.metacat.common.server.usermetadata.ParentChildRelMetadataConstants import com.netflix.metacat.common.server.usermetadata.TagService @@ -74,6 +79,7 @@ class TableServiceImplSpec extends Specification { def usermetadataService = Mock(UserMetadataService) def eventBus = Mock(MetacatEventBus) def converterUtil = Mock(ConverterUtil) + def aliasService = Mock(AliasService) def catalogConfig = MetacatCatalogConfig.createFromMapAndRemoveProperties('migration', 'a', ['metacat.interceptor.enabled': 'true', 'metacat.has-data-external':'true', 'metacat.type': 'hive']) def catalogConfigFalse = MetacatCatalogConfig.createFromMapAndRemoveProperties('migration', 'a', ['metacat.interceptor.enabled': 'false', 'metacat.has-data-external':'false', 'metacat.type': 'hive']) def registry = new NoopRegistry() @@ -104,7 +110,8 @@ class TableServiceImplSpec extends Specification { service = new TableServiceImpl(connectorManager, connectorTableServiceProxy, databaseService, tagService, usermetadataService, new MetacatJsonLocator(), - eventBus, registry, config, converterUtil, authorizationService, ownerValidationService, parentChildRelSvc) + eventBus, registry, config, converterUtil, authorizationService, ownerValidationService, parentChildRelSvc, + aliasService) } ObjectNode createParentChildRelMetadata(String rootTableName, String rootTableUuid, String childTableUuid) { @@ -164,6 +171,238 @@ class TableServiceImplSpec extends Specification { 0 * usermetadataService.getDefinitionMetadataWithInterceptor(_,_) >> Optional.empty() } + def "get resolves an alias for a definition-metadata-only read, but not for a read that touches the connector"() { + given: + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + config.isTableAliasEnabled() >> true + aliasService.getTableName(aliasName) >> name + + when: "the read only touches definition metadata" + service.get(aliasName, GetTableServiceParameters.builder() + .includeInfo(false) + .includeDefinitionMetadata(true) + .build()) + + then: "the alias is resolved to the source table before anything else happens" + 1 * connectorManager.getCatalogConfig(name) >> catalogConfigFalse + + when: "the read touches the connector (e.g. Iceberg location/serde/columns)" + service.get(aliasName, GetTableServiceParameters.builder() + .includeInfo(true) + .build()) + + then: "the alias name is left unresolved, since the connector has no concept of aliases" + 1 * connectorManager.getCatalogConfig(aliasName) >> catalogConfigFalse + 1 * connectorTableService.get(_, aliasName) >> { throw new TableNotFoundException(aliasName) } + } + + def "get through an alias serves definition metadata only, never the table info"() { + given: "a catalog whose interceptor makes the definition-metadata read touch the connector" + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + config.isTableAliasEnabled() >> true + aliasService.getTableName(aliasName) >> name + aliasService.isAlias(aliasName) >> true + def definitionMetadata = objectMapper.createObjectNode().put('k', 'v') + + when: + def result = service.get(aliasName, GetTableServiceParameters.builder() + .includeInfo(false) + .includeDataMetadata(true) + .includeDefinitionMetadata(true) + .build()) + + then: "the connector is read to feed the interceptor, but its table info is not handed back" + 1 * connectorManager.getCatalogConfig(name) >> catalogConfig + 0 * connectorTableService.get(_, name) + 1 * usermetadataService.getDefinitionMetadataWithInterceptor(name, _) >> Optional.of(definitionMetadata) + 0 * usermetadataService.getDataMetadata(_) + result.get().getDefinitionMetadata() == definitionMetadata + result.get().getName() == name + result.get().getSerde() == null + result.get().getMetadata() == null + result.get().getFields() == null + } + + def "create refuses to create a table over an existing alias name"() { + given: + config.isTableAliasEnabled() >> true + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + def createTableDto = new TableDto(name: aliasName, serde: new StorageDto(uri: 's3:/a/b/c')) + + when: + service.create(aliasName, createTableDto) + + then: "the alias check happens before anything is written" + 1 * aliasService.isAlias(aliasName) >> true + 0 * connectorTableServiceProxy.create(_, _) + def e = thrown(MetacatAlreadyExistsException) + e.message.contains('the_alias') + } + + def "create proceeds normally when the name is not an alias"() { + given: + config.isTableAliasEnabled() >> true + ownerValidationService.extractPotentialOwners(_) >> [] + ownerValidationService.extractPotentialOwnerGroups(_) >> [] + + when: + service.create(name, tableDto) + + then: + (1.._) * aliasService.isAlias(name) >> false + 1 * connectorTableServiceProxy.create(name, _) + } + + def "deleteAndReturn refuses to delete through an alias"() { + given: + config.isTableAliasEnabled() >> true + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + + when: + service.deleteAndReturn(aliasName, false) + + then: "the alias check happens before the table is read or dropped" + 1 * aliasService.isAlias(aliasName) >> true + 0 * connectorTableServiceProxy.delete(_) + def e = thrown(MetacatNotSupportedException) + e.message.contains('the_alias') + } + + def "deleteAndReturn proceeds normally when the name is not an alias"() { + given: + config.isTableAliasEnabled() >> true + config.getNoTableDeleteOnTags() >> [] + config.canDeleteTableDefinitionMetadata() >> true + + when: + service.deleteAndReturn(name, false) + + then: + (1.._) * aliasService.isAlias(name) >> false + 1 * connectorTableServiceProxy.delete(name) + } + + def "rename refuses to rename an alias"() { + given: + config.isTableAliasEnabled() >> true + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + def newName = QualifiedName.ofTable('a', 'b', 'the_new_name') + + when: + service.rename(aliasName, newName, false) + + then: "the alias check happens before the table is read or renamed" + 1 * aliasService.isAlias(aliasName) >> true + 0 * connectorTableServiceProxy.rename(_, _, _) + def e = thrown(MetacatBadRequestException) + e.message.contains('the_alias') + } + + def "rename refuses to rename a table onto an existing alias name"() { + given: + config.isTableAliasEnabled() >> true + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + + when: + service.rename(name, aliasName, false) + + then: "the target name collides with an alias, so it is rejected like a create would be" + 1 * aliasService.isAlias(name) >> false + 1 * aliasService.isAlias(aliasName) >> true + 0 * connectorTableServiceProxy.rename(_, _, _) + def e = thrown(MetacatAlreadyExistsException) + e.message.contains('the_alias') + } + + def "rename proceeds normally when neither name is an alias"() { + given: + config.isTableAliasEnabled() >> true + def newName = QualifiedName.ofTable('a', 'b', 'the_new_name') + config.getNoTableRenameOnTags() >> [] + config.isParentChildRenameEnabled() >> true + + when: + service.rename(name, newName, false) + + then: + (1.._) * aliasService.isAlias(name) >> false + (1.._) * aliasService.isAlias(newName) >> false + 1 * connectorTableServiceProxy.rename(name, newName, _) + } + + def "updateAndReturn renames the dto onto the source, so metadata is not written under the alias"() { + given: + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + def updateTableDto = new TableDto(name: name, + definitionMetadata: objectMapper.createObjectNode().put('k', 'v')) + config.isTableAliasEnabled() >> true + aliasService.getTableName(aliasName) >> name + aliasService.getTableName(name) >> name + + when: + service.updateAndReturn(aliasName, updateTableDto, false) + + then: "definition metadata is keyed off the dto name, which points at the source table" + 1 * usermetadataService.saveMetadata(_, { TableDto dto -> dto.getDefinitionName() == name }, true) + } + + @Unroll + def "updateAndReturn refuses to push #info through an alias"() { + given: + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + config.isTableAliasEnabled() >> true + aliasService.getTableName(aliasName) >> name + aliasService.isAlias(aliasName) >> true + + when: + service.updateAndReturn(aliasName, updateTableDto, false) + + then: "nothing reaches the connector, and no metadata is written either" + 0 * connectorTableServiceProxy.update(_, _) + 0 * usermetadataService.saveMetadata(_, _, _) + def e = thrown(MetacatNotSupportedException) + e.message.contains('the_alias') + + where: + info | updateTableDto + 'an iceberg commit' | new TableDto(metadata: [metadata_location: '/tmp/00001.metadata.json']) + 'a schema change' | new TableDto(fields: [new FieldDto(name: 'c', type: 'string', pos: 0)]) + 'a serde change' | new TableDto(serde: new StorageDto(uri: 's3:/a/b/c')) + } + + def "updateAndReturn accepts a body with no name, updating the table the alias points at"() { + given: + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + def updateTableDto = new TableDto(definitionMetadata: objectMapper.createObjectNode().put('k', 'v')) + config.isTableAliasEnabled() >> true + aliasService.getTableName(aliasName) >> name + + when: "the caller asked for the alias and named no table in the body" + service.updateAndReturn(aliasName, updateTableDto, false) + + then: "the body is taken as an update of the source table the url resolved to" + noExceptionThrown() + 1 * usermetadataService.saveMetadata(_, { TableDto dto -> dto.getDefinitionName() == name }, true) + } + + def "updateAndReturn rejects a body that names a different table"() { + given: + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + def otherName = QualifiedName.ofTable('a', 'b', 'some_other_table') + def updateTableDto = new TableDto(name: otherName) + config.isTableAliasEnabled() >> true + aliasService.getTableName(aliasName) >> name + aliasService.getTableName(otherName) >> otherName + + when: + service.updateAndReturn(aliasName, updateTableDto, false) + + then: "neither name resolves to the other, so the update is refused before anything is written" + 0 * connectorTableServiceProxy.update(_, _) + def e = thrown(IllegalArgumentException) + e.message.contains('does not match') + } + def testIsTableInfoProvided() { def name = QualifiedName.ofTable('a','b','c') def oldTableDto = new TableDto(name: name, serde: new StorageDto(uri: 's3:/a/b/c')) @@ -672,7 +911,8 @@ class TableServiceImplSpec extends Specification { connectorManager, connectorTableServiceProxy, databaseService, tagService, usermetadataService, new MetacatJsonLocator(), eventBus, new DefaultRegistry(), config, converterUtil, authorizationService, - new DefaultOwnerValidationService(new NoopRegistry()), parentChildRelSvc) + new DefaultOwnerValidationService(new NoopRegistry()), parentChildRelSvc, + aliasService) def initialDefinitionMetadataJson = toObjectNode(initialDefinitionMetadata) tableDto = new TableDto( From 276498ce0c015e612e282d31c1c06e84b75f71ec Mon Sep 17 00:00:00 2001 From: Gabriel Igliozzi Date: Fri, 21 Aug 2026 13:52:18 +0200 Subject: [PATCH 2/2] resolve to 404 for alias with iceberg metadata request --- .../main/services/impl/TableServiceImpl.java | 23 +++----- .../services/impl/TableServiceImplSpec.groovy | 57 ++++++++++++++++--- 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java index dbb5df72b..e23e65d74 100644 --- a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java +++ b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java @@ -525,10 +525,14 @@ private boolean isEnabledForTableDefinitionMetadataDelete(final QualifiedName ta @Override public Optional get(final QualifiedName requestedName, final GetTableServiceParameters getTableServiceParameters) { - // A read that wants table info must fail on an alias rather than quietly serve the table it - // points at, so its name is left unresolved for the connector to reject. - final QualifiedName name = resolveAlias(requestedName, getTableServiceParameters); final boolean isAliasRead = isAlias(requestedName); + // The connector knows nothing about aliases, and an alias serves definition metadata only, + // so a read that wants table info finds nothing under one. + if (isAliasRead && readsTableInfo(getTableServiceParameters)) { + return Optional.empty(); + } + // Only an alias needs resolving; any other name already names the table it reads. + final QualifiedName name = isAliasRead ? resolveAlias(requestedName) : requestedName; validate(name); TableDto tableInternal = null; @@ -1056,19 +1060,6 @@ private QualifiedName resolveAlias(final QualifiedName name) { return config.isTableAliasEnabled() ? aliasService.getTableName(name) : name; } - /** - * Resolves a table alias to the table it points at. Names that are not aliases, and every name at - * all when table aliasing is disabled, are returned untouched. - * - * @param name the requested name - * @param getTableServiceParameters params from request - * @return the table the alias points at, or the given name - */ - private QualifiedName resolveAlias(final QualifiedName name, - final GetTableServiceParameters getTableServiceParameters) { - return readsTableInfo(getTableServiceParameters) ? name : resolveAlias(name); - } - /** * Whether the read asks for anything the connector owns, as opposed to user metadata. Three * separate parameters select a connector read, see {@link #getFromTableServiceProxy}. diff --git a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy index 50b997002..29e20a2ab 100644 --- a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy +++ b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy @@ -176,6 +176,7 @@ class TableServiceImplSpec extends Specification { def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') config.isTableAliasEnabled() >> true aliasService.getTableName(aliasName) >> name + aliasService.isAlias(aliasName) >> true when: "the read only touches definition metadata" service.get(aliasName, GetTableServiceParameters.builder() @@ -187,17 +188,59 @@ class TableServiceImplSpec extends Specification { 1 * connectorManager.getCatalogConfig(name) >> catalogConfigFalse when: "the read touches the connector (e.g. Iceberg location/serde/columns)" - service.get(aliasName, GetTableServiceParameters.builder() + def result = service.get(aliasName, GetTableServiceParameters.builder() .includeInfo(true) .build()) - then: "the alias name is left unresolved, since the connector has no concept of aliases" - 1 * connectorManager.getCatalogConfig(aliasName) >> catalogConfigFalse - 1 * connectorTableService.get(_, aliasName) >> { throw new TableNotFoundException(aliasName) } + then: "there is nothing to serve, since the connector has no concept of aliases" + 0 * connectorTableService.get(_, _) + !result.isPresent() + } + + @Unroll + def "get through an alias finds nothing when the read asks for #info"() { + given: + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + config.isTableAliasEnabled() >> true + aliasService.getTableName(aliasName) >> name + aliasService.isAlias(aliasName) >> true + + when: + def result = service.get(aliasName, params) + + then: "the connector is never asked, and the caller gets a not-found" + 0 * connectorTableService.get(_, _) + !result.isPresent() + + where: + info | params + 'the table info' | GetTableServiceParameters.builder().includeInfo(true).build() + 'a metadata location' | GetTableServiceParameters.builder().includeMetadataLocationOnly(true).build() + 'connector details' | GetTableServiceParameters.builder().includeMetadataFromConnector(true).build() + } + + def "get through an alias serves no data metadata, and does not read the connector for it"() { + given: + def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') + config.isTableAliasEnabled() >> true + aliasService.getTableName(aliasName) >> name + aliasService.isAlias(aliasName) >> true + + when: "data metadata is asked for on a catalog that keeps data externally" + def result = service.get(aliasName, GetTableServiceParameters.builder() + .includeInfo(false) + .includeDataMetadata(true) + .build()) + + then: "the serde is never fetched to key the lookup off, so there is no data metadata" + 1 * connectorManager.getCatalogConfig(name) >> catalogConfig + 0 * connectorTableService.get(_, _) + 0 * usermetadataService.getDataMetadata(_) + result.get().getDataMetadata() == null } def "get through an alias serves definition metadata only, never the table info"() { - given: "a catalog whose interceptor makes the definition-metadata read touch the connector" + given: "a catalog whose interceptor would otherwise make the read touch the connector" def aliasName = QualifiedName.ofTable('a', 'b', 'the_alias') config.isTableAliasEnabled() >> true aliasService.getTableName(aliasName) >> name @@ -211,9 +254,9 @@ class TableServiceImplSpec extends Specification { .includeDefinitionMetadata(true) .build()) - then: "the connector is read to feed the interceptor, but its table info is not handed back" + then: "the connector is not read at all, and only definition metadata comes back" 1 * connectorManager.getCatalogConfig(name) >> catalogConfig - 0 * connectorTableService.get(_, name) + 0 * connectorTableService.get(_, _) 1 * usermetadataService.getDefinitionMetadataWithInterceptor(name, _) >> Optional.of(definitionMetadata) 0 * usermetadataService.getDataMetadata(_) result.get().getDefinitionMetadata() == definitionMetadata