Skip to content

Commit

Permalink
Set the owner in table dto on create call for Thrift API (#530)
Browse files Browse the repository at this point in the history 
* Set table owner in dto when available in thrift create table call

* Add metacat-thrift-interface to list of invalid owners

* Also set owner when converting from metacat to hive format

* Add assertion of owner from utility API in TableDto
  • Loading branch information
@swaranga-netflix
swaranga-netflix authored Feb 7, 2023
1 parent 8203a08 commit 0efc6f6
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 13 deletions.
5 changes: 4 additions & 1 deletion metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,10 @@ void updateTableOwner(final TableDto tableDto, final String userId) {
}

private boolean isOwnerValid(@Nullable final String userId) {
return StringUtils.isNotBlank(userId) && !"metacat".equals(userId) && !"root".equals(userId);
return StringUtils.isNotBlank(userId)
&& !"metacat".equals(userId)
&& !"root".equals(userId)
&& !"metacat-thrift-interface".equals(userId);
}

@SuppressFBWarnings
Expand Down
20 changes: 11 additions & 9 deletions ...t-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,17 +342,19 @@ class TableServiceImplSpec extends Specification {
then:
tableDto.getDefinitionMetadata() == toObjectNode(expectedDefMetadata)
tableDto.getSerde() == expectedSerde
tableDto.getTableOwner() == Optional.ofNullable(expetedFinalOwner)

where:
initialDefinitionMetadata | sessionUser | initialSerde || expectedDefMetadata | expectedSerde
null | null | null || "{}" | new StorageDto()
null | "ssarma" | null || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto()
null | "root" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"swaranga\"}}" | new StorageDto(owner: "swaranga")
"{\"owner\":{\"userId\":\"ssarma\"}}" | "asdf" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto(owner: "swaranga")
"{\"owner\":{\"userId\":\"metacat\"}}" | "ssarma" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto(owner: "swaranga")
"{\"owner\":{\"userId\":\"root\"}}" | "ssarma" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto(owner: "swaranga")
"{\"owner\":{\"userId\":\"root\"}}" | "metacat" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"swaranga\"}}" | new StorageDto(owner: "swaranga")
"{\"owner\":{\"userId\":\"root\"}}" | "metacat" | new StorageDto(owner: "metacat") || "{\"owner\":{\"userId\":\"root\"}}" | new StorageDto(owner: "metacat")
initialDefinitionMetadata | sessionUser | initialSerde || expectedDefMetadata | expectedSerde | expetedFinalOwner
null | null | null || "{}" | new StorageDto() | null
null | "ssarma" | null || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto() | "ssarma"
null | "root" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"swaranga\"}}" | new StorageDto(owner: "swaranga") | "swaranga"
"{\"owner\":{\"userId\":\"ssarma\"}}" | "asdf" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto(owner: "swaranga") | "ssarma"
"{\"owner\":{\"userId\":\"metacat\"}}" | "ssarma" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto(owner: "swaranga") | "ssarma"
"{\"owner\":{\"userId\":\"root\"}}" | "ssarma" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto(owner: "swaranga") | "ssarma"
"{\"owner\":{\"userId\":\"root\"}}" | "metacat" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"swaranga\"}}" | new StorageDto(owner: "swaranga") | "swaranga"
"{\"owner\":{\"userId\":\"root\"}}" | "metacat" | new StorageDto(owner: "metacat") || "{\"owner\":{\"userId\":\"root\"}}" | new StorageDto(owner: "metacat") | "root"
"{\"owner\":{\"userId\":\"metacat-thrift-interface\"}}" | "ssarma" | new StorageDto(owner: "swaranga") || "{\"owner\":{\"userId\":\"ssarma\"}}" | new StorageDto(owner: "swaranga") | "ssarma"
}

ObjectNode toObjectNode(jsonString) {
Expand Down
16 changes: 16 additions & 0 deletions metacat-thrift/src/main/java/com/netflix/metacat/thrift/HiveConvertersImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
*/
package com.netflix.metacat.thrift;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
Expand All @@ -28,6 +30,7 @@
import com.netflix.metacat.common.dto.StorageDto;
import com.netflix.metacat.common.dto.TableDto;
import com.netflix.metacat.common.dto.ViewDto;
import org.apache.commons.lang.StringUtils;
import org.apache.hadoop.hive.common.FileUtils;
import org.apache.hadoop.hive.metastore.TableType;
import org.apache.hadoop.hive.metastore.Warehouse;
Expand All @@ -54,6 +57,7 @@
* Hive converter.
*/
public class HiveConvertersImpl implements HiveConverters {
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

@VisibleForTesting
Integer dateToEpochSeconds(@Nullable final Date date) {
Expand Down Expand Up @@ -121,6 +125,15 @@ public TableDto hiveToMetacatTable(final QualifiedName name, final Table table)
dto.setFields(allFields);
dto.setView(new ViewDto(table.getViewOriginalText(),
table.getViewExpandedText()));

if (StringUtils.isNotBlank(table.getOwner())) {
final ObjectNode definitionMetadata = OBJECT_MAPPER.createObjectNode();
final ObjectNode ownerNode = definitionMetadata.with("owner");
ownerNode.put("userId", table.getOwner());

dto.setDefinitionMetadata(definitionMetadata);
}

return dto;
}

Expand Down Expand Up @@ -208,6 +221,9 @@ public Table metacatToHiveTable(final TableDto dto) {
table.setPartitionKeys(partitionFields);
table.getSd().setCols(nonPartitionFields);
}

dto.getTableOwner().ifPresent(table::setOwner);

return table;
}

Expand Down
17 changes: 14 additions & 3 deletions metacat-thrift/src/test/groovy/com/netflix/metacat/thrift/HiveConvertersSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@

package com.netflix.metacat.thrift

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.metacat.common.QualifiedName
import com.netflix.metacat.common.dto.*
import com.netflix.metacat.common.server.connectors.ConnectorTypeConverter
import com.netflix.metacat.common.server.properties.Config
import com.netflix.metacat.common.type.VarcharType
import org.apache.hadoop.hive.conf.HiveConf
import org.apache.hadoop.hive.metastore.api.*
import org.apache.hadoop.hive.ql.metadata.Hive
import org.apache.hadoop.hive.ql.session.SessionState
import org.joda.time.Instant
import spock.lang.Specification

Expand All @@ -32,6 +30,8 @@ import java.util.Date

class HiveConvertersSpec extends Specification {
private static final ZoneOffset PACIFIC = LocalDateTime.now().atZone(ZoneId.of('America/Los_Angeles')).offset
private static final ObjectMapper MAPPER = new ObjectMapper()

Config config = Mock(Config)
// TypeManager typeManager = Mock(TypeManager)
HiveConvertersImpl converter
Expand Down Expand Up @@ -134,6 +134,15 @@ class HiveConvertersSpec extends Specification {
table.partitionKeys != null
table.parameters != null
table.tableType != null

when:
def definitionMetadata = MAPPER.createObjectNode()
def ownerNode = definitionMetadata.with("owner")
ownerNode.put("userId", "asdf")
dto.setDefinitionMetadata(definitionMetadata)

then:
converter.metacatToHiveTable(dto).owner == "asdf"
}

def 'test metacatToHiveTable'() {
Expand Down Expand Up @@ -224,6 +233,8 @@ class HiveConvertersSpec extends Specification {
}
dto.fields.findAll { it.partition_key }.size() == 2
dto.metadata == tableParams
dto.definitionMetadata.get("owner").get("userId").asText() == owner
dto.getTableOwner().get() == owner

where:
databaseName = 'database'
Expand Down

0 comments on commit 0efc6f6

Please sign in to comment.