Skip to content

Commit

Permalink
Fixes #13803 : Reduce the number of call in case of columns (#13819)
Browse files Browse the repository at this point in the history
* Fixes #13803 : Reduce the number of call in case of columns

* Add Generic Method and Interface for Fields

* Removed unnecessary comments

* Make FieldInterface return List<? extends FieldInterface> for getChildren

* Fix Failing Tests

* - Remove Pipeline or Kill on app Disable
- Add AppMarketPlaceResourceTest
- Reschedule app automatically on restore

* Add Debug Log in validate Change Event

* Fix BotResourceTest

* Api Refactor on UI
  • Loading branch information
mohityadav766 authored Nov 7, 2023
1 parent ec6184d commit 4a73978
Show file tree
Hide file tree
Showing 25 changed files with 535 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import static org.openmetadata.common.utils.CommonUtil.listOf;
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
import static org.openmetadata.service.util.EntityUtil.getFlattenedEntityField;

import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import io.github.classgraph.ClassGraph;
Expand All @@ -41,6 +42,7 @@
import org.jdbi.v3.core.Jdbi;
import org.openmetadata.schema.EntityInterface;
import org.openmetadata.schema.EntityTimeSeriesInterface;
import org.openmetadata.schema.FieldInterface;
import org.openmetadata.schema.entity.services.ServiceType;
import org.openmetadata.schema.type.EntityReference;
import org.openmetadata.schema.type.Include;
Expand All @@ -61,6 +63,7 @@
import org.openmetadata.service.resources.feeds.MessageParser.EntityLink;
import org.openmetadata.service.search.SearchRepository;
import org.openmetadata.service.util.EntityUtil.Fields;
import org.openmetadata.service.util.FullyQualifiedName;

@Slf4j
public final class Entity {
Expand Down Expand Up @@ -529,4 +532,22 @@ private static List<Class<?>> getRepositories() {
return classList.loadClasses();
}
}

public static <T extends FieldInterface> void populateEntityFieldTags(
String entityType, List<T> fields, String fqnPrefix, boolean setTags) {
EntityRepository<?> repository = Entity.getEntityRepository(entityType);
// Get Flattened Fields
List<T> flattenedFields = getFlattenedEntityField(fields);

// Fetch All tags belonging to Prefix
Map<String, List<TagLabel>> allTags = repository.getTagsByPrefix(fqnPrefix);
for (T c : listOrEmpty(flattenedFields)) {
if (setTags) {
List<TagLabel> columnTag = allTags.get(FullyQualifiedName.buildHash(c.getFullyQualifiedName()));
c.setTags(columnTag == null ? new ArrayList<>() : columnTag);
} else {
c.setTags(c.getTags());
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.openmetadata.service.apps.bundles.test;

import lombok.extern.slf4j.Slf4j;
import org.openmetadata.schema.entity.app.App;
import org.openmetadata.service.apps.AbstractNativeApplication;
import org.openmetadata.service.jdbi3.CollectionDAO;
import org.openmetadata.service.search.SearchRepository;

@Slf4j
public class NoOpTestApplication extends AbstractNativeApplication {

@Override
public void init(App app, CollectionDAO dao, SearchRepository searchRepository) {
super.init(app, dao, searchRepository);
LOG.info("NoOpTestApplication is initialized");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public AppMarketPlaceRepository() {
"",
"");
supportsSearch = false;
quoteFqn = true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import javax.ws.rs.InternalServerErrorException;
import lombok.extern.slf4j.Slf4j;
import org.openmetadata.schema.api.teams.CreateUser;
import org.openmetadata.schema.auth.JWTAuthMechanism;
Expand All @@ -21,14 +20,12 @@
import org.openmetadata.schema.type.ProviderType;
import org.openmetadata.schema.type.Relationship;
import org.openmetadata.service.Entity;
import org.openmetadata.service.apps.scheduler.AppScheduler;
import org.openmetadata.service.exception.EntityNotFoundException;
import org.openmetadata.service.resources.apps.AppResource;
import org.openmetadata.service.security.jwt.JWTTokenGenerator;
import org.openmetadata.service.util.EntityUtil;
import org.openmetadata.service.util.JsonUtils;
import org.openmetadata.service.util.ResultList;
import org.quartz.SchedulerException;

@Slf4j
public class AppRepository extends EntityRepository<App> {
Expand All @@ -45,6 +42,7 @@ public AppRepository() {
UPDATE_FIELDS,
UPDATE_FIELDS);
supportsSearch = false;
quoteFqn = true;
}

@Override
Expand Down Expand Up @@ -153,16 +151,6 @@ public void storeEntity(App entity, boolean update) {
entity.withBot(botUserRef).withOwner(ownerRef);
}

@Override
public void postDelete(App entity) {
try {
AppScheduler.getInstance().deleteScheduledApplication(entity);
} catch (SchedulerException ex) {
LOG.error("Failed in delete Application from Scheduler.", ex);
throw new InternalServerErrorException("Failed in Delete App from Scheduler.");
}
}

public EntityReference getBotUser(App application) {
return application.getBot() != null
? application.getBot()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@
import com.fasterxml.jackson.core.type.TypeReference;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
Expand Down Expand Up @@ -2201,10 +2203,66 @@ default List<TagLabel> getTags(String targetFQN) {
return tags;
}

default Map<String, List<TagLabel>> getTagsByPrefix(String targetFQNPrefix) {
Map<String, List<TagLabel>> resultSet = new LinkedHashMap<>();
List<Pair<String, TagLabel>> tags = getTagsInternalByPrefix(targetFQNPrefix);
tags.forEach(
pair -> {
String targetHash = pair.getLeft();
TagLabel tagLabel = pair.getRight();
List<TagLabel> listOfTarget = new ArrayList<>();
if (resultSet.containsKey(targetHash)) {
listOfTarget = resultSet.get(targetHash);
listOfTarget.add(tagLabel);
} else {
listOfTarget.add(tagLabel);
}
resultSet.put(targetHash, listOfTarget);
});
return resultSet;
}

@SqlQuery(
"SELECT source, tagFQN, labelType, state FROM tag_usage WHERE targetFQNHash = :targetFQNHash ORDER BY tagFQN")
List<TagLabel> getTagsInternal(@BindFQN("targetFQNHash") String targetFQNHash);

@ConnectionAwareSqlQuery(
value =
"SELECT source, tagFQN, labelType, targetFQNHash, state, json "
+ "FROM ("
+ " SELECT gterm.* , tu.* "
+ " FROM glossary_term_entity AS gterm "
+ " JOIN tag_usage AS tu "
+ " ON gterm.fqnHash = tu.tagFQNHash "
+ " WHERE tu.source = 1 "
+ " UNION ALL "
+ " SELECT ta.*, tu.* "
+ " FROM tag AS ta "
+ " JOIN tag_usage AS tu "
+ " ON ta.fqnHash = tu.tagFQNHash "
+ " WHERE tu.source = 0 "
+ ") AS combined_data "
+ "WHERE combined_data.targetFQNHash LIKE CONCAT(:targetFQNHashPrefix, '.%')",
connectionType = MYSQL)
@ConnectionAwareSqlQuery(
value =
"SELECT source, tagFQN, labelType, targetFQNHash, state, json "
+ "FROM ("
+ " SELECT gterm.*, tu.* "
+ " FROM glossary_term_entity AS gterm "
+ " JOIN tag_usage AS tu ON gterm.fqnHash = tu.tagFQNHash "
+ " WHERE tu.source = 1 "
+ " UNION ALL "
+ " SELECT ta.*, tu.* "
+ " FROM tag AS ta "
+ " JOIN tag_usage AS tu ON ta.fqnHash = tu.tagFQNHash "
+ " WHERE tu.source = 0 "
+ ") AS combined_data "
+ "WHERE combined_data.targetFQNHash LIKE CONCAT(:targetFQNHashPrefix, '.%')",
connectionType = POSTGRES)
@RegisterRowMapper(TagLabelRowMapperWithTargetFqnHash.class)
List<Pair<String, TagLabel>> getTagsInternalByPrefix(@BindFQN("targetFQNHashPrefix") String targetFQNHashPrefix);

@SqlQuery("SELECT * FROM tag_usage")
@Deprecated(since = "Release 1.1")
@RegisterRowMapper(TagLabelMapperMigration.class)
Expand Down Expand Up @@ -2294,6 +2352,35 @@ public TagLabel map(ResultSet r, StatementContext ctx) throws SQLException {
}
}

class TagLabelRowMapperWithTargetFqnHash implements RowMapper<Pair<String, TagLabel>> {
@Override
public Pair<String, TagLabel> map(ResultSet r, StatementContext ctx) throws SQLException {
TagLabel label =
new TagLabel()
.withSource(TagLabel.TagSource.values()[r.getInt("source")])
.withLabelType(TagLabel.LabelType.values()[r.getInt("labelType")])
.withState(TagLabel.State.values()[r.getInt("state")])
.withTagFQN(r.getString("tagFQN"));
TagLabel.TagSource source = TagLabel.TagSource.values()[r.getInt("source")];
if (source == TagLabel.TagSource.CLASSIFICATION) {
Tag tag = JsonUtils.readValue(r.getString("json"), Tag.class);
label.setName(tag.getName());
label.setDisplayName(tag.getDisplayName());
label.setDescription(tag.getDescription());
label.setStyle(tag.getStyle());
} else if (source == TagLabel.TagSource.GLOSSARY) {
GlossaryTerm glossaryTerm = JsonUtils.readValue(r.getString("json"), GlossaryTerm.class);
label.setName(glossaryTerm.getName());
label.setDisplayName(glossaryTerm.getDisplayName());
label.setDescription(glossaryTerm.getDescription());
label.setStyle(glossaryTerm.getStyle());
} else {
throw new IllegalArgumentException("Invalid source type " + source);
}
return Pair.of(r.getString("targetFQNHash"), label);
}
}

@Getter
@Setter
@Deprecated(since = "Release 1.1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static org.openmetadata.service.Entity.FIELD_PARENT;
import static org.openmetadata.service.Entity.FIELD_TAGS;
import static org.openmetadata.service.Entity.STORAGE_SERVICE;
import static org.openmetadata.service.Entity.populateEntityFieldTags;

import com.google.common.collect.Lists;
import java.util.ArrayList;
Expand Down Expand Up @@ -54,7 +55,8 @@ public Container setFields(Container container, EntityUtil.Fields fields) {
setDefaultFields(container);
container.setParent(fields.contains(FIELD_PARENT) ? getParent(container) : container.getParent());
if (container.getDataModel() != null) {
populateDataModelColumnTags(fields.contains(FIELD_TAGS), container.getDataModel().getColumns());
populateDataModelColumnTags(
fields.contains(FIELD_TAGS), container.getFullyQualifiedName(), container.getDataModel().getColumns());
}
return container;
}
Expand All @@ -65,11 +67,8 @@ public Container clearFields(Container container, EntityUtil.Fields fields) {
return container.withDataModel(fields.contains("dataModel") ? container.getDataModel() : null);
}

private void populateDataModelColumnTags(boolean setTags, List<Column> columns) {
for (Column c : listOrEmpty(columns)) {
c.setTags(setTags ? getTags(c.getFullyQualifiedName()) : null);
populateDataModelColumnTags(setTags, c.getChildren());
}
private void populateDataModelColumnTags(boolean setTags, String fqnPrefix, List<Column> columns) {
populateEntityFieldTags(entityType, columns, fqnPrefix, setTags);
}

private void setDefaultFields(Container container) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@

package org.openmetadata.service.jdbi3;

import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
import static org.openmetadata.schema.type.Include.ALL;
import static org.openmetadata.service.Entity.DASHBOARD_DATA_MODEL;
import static org.openmetadata.service.Entity.FIELD_TAGS;
import static org.openmetadata.service.Entity.populateEntityFieldTags;

import java.util.List;
import lombok.SneakyThrows;
Expand Down Expand Up @@ -161,7 +161,11 @@ public DashboardDataModel setInheritedFields(DashboardDataModel dataModel, Field

@Override
public DashboardDataModel setFields(DashboardDataModel dashboardDataModel, Fields fields) {
getColumnTags(fields.contains(FIELD_TAGS), dashboardDataModel.getColumns());
populateEntityFieldTags(
entityType,
dashboardDataModel.getColumns(),
dashboardDataModel.getFullyQualifiedName(),
fields.contains(FIELD_TAGS));
if (dashboardDataModel.getService() == null) {
dashboardDataModel.withService(getContainer(dashboardDataModel.getId()));
}
Expand All @@ -183,14 +187,6 @@ public void restorePatchAttributes(DashboardDataModel original, DashboardDataMod
.withId(original.getId());
}

// TODO move this to base class?
private void getColumnTags(boolean setTags, List<Column> columns) {
for (Column c : listOrEmpty(columns)) {
c.setTags(setTags ? getTags(c.getFullyQualifiedName()) : c.getTags());
getColumnTags(setTags, c.getChildren());
}
}

private void applyTags(List<Column> columns) {
// Add column level tags by adding tag to column relationship
for (Column column : columns) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1296,6 +1296,10 @@ protected List<TagLabel> getTags(String fqn) {
return !supportsTags ? null : daoCollection.tagUsageDAO().getTags(fqn);
}

public Map<String, List<TagLabel>> getTagsByPrefix(String prefix) {
return !supportsTags ? null : daoCollection.tagUsageDAO().getTagsByPrefix(prefix);
}

protected List<EntityReference> getFollowers(T entity) {
return !supportsFollower || entity == null
? Collections.emptyList()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import static org.openmetadata.service.Entity.FIELD_TAGS;
import static org.openmetadata.service.Entity.TABLE;
import static org.openmetadata.service.Entity.getEntity;
import static org.openmetadata.service.Entity.populateEntityFieldTags;
import static org.openmetadata.service.util.LambdaExceptionUtil.ignoringComparator;
import static org.openmetadata.service.util.LambdaExceptionUtil.rethrowFunction;

Expand Down Expand Up @@ -128,7 +129,8 @@ public Table setFields(Table table, Fields fields) {
}
if (fields.contains(COLUMN_FIELD)) {
// We'll get column tags only if we are getting the column fields
getColumnTags(fields.contains(FIELD_TAGS), table.getColumns());
populateEntityFieldTags(
entityType, table.getColumns(), table.getFullyQualifiedName(), fields.contains(FIELD_TAGS));
}
table.setJoins(fields.contains("joins") ? getJoins(table) : table.getJoins());
table.setTableProfilerConfig(
Expand Down Expand Up @@ -253,7 +255,7 @@ public Table getSampleData(UUID tableId, boolean authorizePII) {

// Set the column tags. Will be used to mask the sample data
if (!authorizePII) {
getColumnTags(true, table.getColumns());
populateEntityFieldTags(entityType, table.getColumns(), table.getFullyQualifiedName(), true);
table.setTags(getTags(table));
return PIIMasker.getSampleData(table);
}
Expand Down Expand Up @@ -486,7 +488,7 @@ public Table getLatestTableProfile(String fqn, boolean authorizePII) {

// Set the column tags. Will be used to hide the data
if (!authorizePII) {
getColumnTags(true, table.getColumns());
populateEntityFieldTags(entityType, table.getColumns(), table.getFullyQualifiedName(), true);
return PIIMasker.getTableProfile(table);
}

Expand Down Expand Up @@ -803,14 +805,6 @@ private static Column getChildColumn(List<Column> column, String childrenName) {
return childrenColumn;
}

// TODO duplicated code
private void getColumnTags(boolean setTags, List<Column> columns) {
for (Column c : listOrEmpty(columns)) {
c.setTags(setTags ? getTags(c.getFullyQualifiedName()) : c.getTags());
getColumnTags(setTags, c.getChildren());
}
}

private void validateTableFQN(String fqn) {
try {
dao.existsByName(fqn);
Expand Down
Loading

0 comments on commit 4a73978

Please sign in to comment.