diff --git a/bootstrap/sql/com.mysql.cj.jdbc.Driver/v009__create_db_connection_info.sql b/bootstrap/sql/com.mysql.cj.jdbc.Driver/v009__create_db_connection_info.sql index 51a08e908d7..51c1252f161 100644 --- a/bootstrap/sql/com.mysql.cj.jdbc.Driver/v009__create_db_connection_info.sql +++ b/bootstrap/sql/com.mysql.cj.jdbc.Driver/v009__create_db_connection_info.sql @@ -148,3 +148,6 @@ where serviceType = 'Druid' UPDATE entity_extension_time_series SET jsonSchema = 'ingestionPipelineStatus', extension = 'ingestionPipeline.pipelineStatus' WHERE jsonSchema = 'pipelineStatus' AND extension <> 'pipeline.PipelineStatus'; + +UPDATE chart_entity +SET json = JSON_REMOVE(json, '$.tables'); diff --git a/bootstrap/sql/org.postgresql.Driver/v009__create_db_connection_info.sql b/bootstrap/sql/org.postgresql.Driver/v009__create_db_connection_info.sql index 5425f16522e..46e48826a32 100644 --- a/bootstrap/sql/org.postgresql.Driver/v009__create_db_connection_info.sql +++ b/bootstrap/sql/org.postgresql.Driver/v009__create_db_connection_info.sql @@ -148,3 +148,6 @@ WHERE servicetype = 'Druid' and json #>'{connection,config,database}' is not nul UPDATE entity_extension_time_series SET jsonSchema = 'ingestionPipelineStatus', extension = 'ingestionPipeline.pipelineStatus' WHERE jsonSchema = 'pipelineStatus' AND extension <> 'pipeline.PipelineStatus'; + +UPDATE chart_entity +SET json = json::jsonb #- '{tables}'; diff --git a/ingestion/tests/unit/topology/dashboard/test_domodashboard.py b/ingestion/tests/unit/topology/dashboard/test_domodashboard.py index 60d4cd71e5c..51cf4c96b69 100644 --- a/ingestion/tests/unit/topology/dashboard/test_domodashboard.py +++ b/ingestion/tests/unit/topology/dashboard/test_domodashboard.py @@ -104,7 +104,6 @@ EXPECTED_CHARTS = [ ), chartType="Other", chartUrl="https://domain.domo.com/page/552315335/kpis/details/1982511286", - tables=None, tags=None, owner=None, service=FullyQualifiedEntityName(__root__="domodashboard_source_test"), @@ -118,7 +117,6 @@ EXPECTED_CHARTS = [ ), chartType="Other", chartUrl="https://domain.domo.com/page/552315335/kpis/details/781210736", - tables=None, tags=None, owner=None, service=FullyQualifiedEntityName(__root__="domodashboard_source_test"), diff --git a/ingestion/tests/unit/topology/dashboard/test_quicksight.py b/ingestion/tests/unit/topology/dashboard/test_quicksight.py index 480dea02f19..fa10e2f81e9 100644 --- a/ingestion/tests/unit/topology/dashboard/test_quicksight.py +++ b/ingestion/tests/unit/topology/dashboard/test_quicksight.py @@ -107,7 +107,6 @@ EXPECTED_DASHBOARDS = [ description="", chartType="Other", chartUrl="https://us-east-2.quicksight.aws.amazon.com/sn/dashboards/552315335", - tables=None, tags=None, owner=None, service="quicksight_source_test", @@ -118,7 +117,6 @@ EXPECTED_DASHBOARDS = [ description="", chartType="Other", chartUrl="https://us-east-2.quicksight.aws.amazon.com/sn/dashboards/552315335", - tables=None, tags=None, owner=None, service="quicksight_source_test", @@ -129,7 +127,6 @@ EXPECTED_DASHBOARDS = [ description="", chartType="Other", chartUrl="https://us-east-2.quicksight.aws.amazon.com/sn/dashboards/552315335", - tables=None, tags=None, owner=None, service="quicksight_source_test", diff --git a/ingestion/tests/unit/topology/dashboard/test_tableau.py b/ingestion/tests/unit/topology/dashboard/test_tableau.py index abfca10766e..69837053896 100644 --- a/ingestion/tests/unit/topology/dashboard/test_tableau.py +++ b/ingestion/tests/unit/topology/dashboard/test_tableau.py @@ -134,7 +134,6 @@ EXPECTED_CHARTS = [ description=None, chartType="Other", chartUrl="#site/tableauSiteUrl/views/Regional/Obesity", - tables=None, tags=None, owner=None, service=FullyQualifiedEntityName(__root__="tableau_source_test"), @@ -145,7 +144,6 @@ EXPECTED_CHARTS = [ description=None, chartType="Other", chartUrl="#site/tableauSiteUrl/views/Regional/College", - tables=None, tags=None, owner=None, service=FullyQualifiedEntityName(__root__="tableau_source_test"), @@ -156,7 +154,6 @@ EXPECTED_CHARTS = [ description=None, chartType="Other", chartUrl="#site/tableauSiteUrl/views/Regional/GlobalTemperatures", - tables=None, tags=None, owner=None, service=FullyQualifiedEntityName(__root__="tableau_source_test"), diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChartRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChartRepository.java index 8c7b42438c6..f0082270139 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChartRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChartRepository.java @@ -90,4 +90,21 @@ public class ChartRepository extends EntityRepository { .withService(original.getService()) .withId(original.getId()); } + + @Override + public EntityUpdater getUpdater(Chart original, Chart updated, Operation operation) { + return new ChartUpdater(original, updated, operation); + } + + public class ChartUpdater extends ColumnEntityUpdater { + public ChartUpdater(Chart chart, Chart updated, Operation operation) { + super(chart, updated, operation); + } + + @Override + public void entitySpecificUpdate() throws IOException { + recordChange("chartType", original.getChartType(), updated.getChartType()); + recordChange("chartUrl", original.getChartUrl(), updated.getChartUrl()); + } + } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/charts/ChartResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/charts/ChartResource.java index ffd76400469..c3a3f58a413 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/charts/ChartResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/charts/ChartResource.java @@ -418,7 +418,6 @@ public class ChartResource extends EntityResource { .withService(EntityUtil.getEntityReference(Entity.DASHBOARD_SERVICE, create.getService())) .withChartType(create.getChartType()) .withChartUrl(create.getChartUrl()) - .withTables(getEntityReferences(Entity.TABLE, create.getTables())) .withTags(create.getTags()); } } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/charts/ChartResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/charts/ChartResourceTest.java index 9ec86f9364c..51f8a6c7a7b 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/charts/ChartResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/charts/ChartResourceTest.java @@ -14,8 +14,11 @@ package org.openmetadata.service.resources.charts; import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static javax.ws.rs.core.Response.Status.OK; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.openmetadata.service.util.EntityUtil.fieldAdded; +import static org.openmetadata.service.util.EntityUtil.fieldUpdated; import static org.openmetadata.service.util.TestUtils.ADMIN_AUTH_HEADERS; import static org.openmetadata.service.util.TestUtils.assertListNotNull; import static org.openmetadata.service.util.TestUtils.assertListNull; @@ -32,12 +35,15 @@ import org.junit.jupiter.api.parallel.Execution; import org.junit.jupiter.api.parallel.ExecutionMode; import org.openmetadata.schema.api.data.CreateChart; import org.openmetadata.schema.entity.data.Chart; +import org.openmetadata.schema.type.ChangeDescription; import org.openmetadata.schema.type.ChartType; import org.openmetadata.schema.type.EntityReference; import org.openmetadata.service.Entity; import org.openmetadata.service.resources.EntityResourceTest; import org.openmetadata.service.resources.charts.ChartResource.ChartList; +import org.openmetadata.service.util.JsonUtils; import org.openmetadata.service.util.ResultList; +import org.openmetadata.service.util.TestUtils.UpdateType; @Slf4j public class ChartResourceTest extends EntityResourceTest { @@ -75,6 +81,86 @@ public class ChartResourceTest extends EntityResourceTest { } } + @Test + void update_chart_200(TestInfo test) throws IOException { + ChartType type1 = ChartType.Bar; + ChartType type2 = ChartType.Line; + + // Create with no url, description and chart type. + CreateChart request = + createRequest(test) + .withService(METABASE_REFERENCE.getName()) + .withChartUrl(null) + .withDescription(null) + .withChartType(null); + Chart chart = createAndCheckEntity(request, ADMIN_AUTH_HEADERS); + + // Set url, description and chart type. + ChangeDescription change = getChangeDescription(chart.getVersion()); + chart.withChartType(type1).withChartUrl("url1").withDescription("desc1"); + fieldAdded(change, "description", "desc1"); + fieldAdded(change, "chartType", type1); + fieldAdded(change, "chartUrl", "url1"); + + chart = + updateAndCheckEntity( + request.withDescription("desc1").withChartType(type1).withChartUrl("url1"), + OK, + ADMIN_AUTH_HEADERS, + UpdateType.MINOR_UPDATE, + change); + + // Update description, chartType and chart url and verify update + change = getChangeDescription(chart.getVersion()); + chart.withChartType(type2).withChartUrl("url2").withDescription("desc2"); + + fieldUpdated(change, "description", "desc1", "desc2"); + fieldUpdated(change, "chartType", type1, type2); + fieldUpdated(change, "chartUrl", "url1", "url2"); + + updateAndCheckEntity( + request.withDescription("desc2").withChartType(type2).withChartUrl("url2"), + OK, + ADMIN_AUTH_HEADERS, + UpdateType.MINOR_UPDATE, + change); + } + + @Test + void patch_chart_200(TestInfo test) throws IOException { + ChartType type1 = ChartType.Bar; + ChartType type2 = ChartType.Line; + + // Create with no url, description and chart type. + CreateChart request = + createRequest(test) + .withService(METABASE_REFERENCE.getName()) + .withChartUrl(null) + .withDescription(null) + .withChartType(null); + Chart chart = createAndCheckEntity(request, ADMIN_AUTH_HEADERS); + String originalJson = JsonUtils.pojoToJson(chart); + + // Set url, description and chart type. + ChangeDescription change = getChangeDescription(chart.getVersion()); + chart.withChartType(type1).withChartUrl("url1").withDescription("desc1"); + fieldAdded(change, "description", "desc1"); + fieldAdded(change, "chartType", type1); + fieldAdded(change, "chartUrl", "url1"); + + chart = patchEntityAndCheck(chart, originalJson, ADMIN_AUTH_HEADERS, UpdateType.MINOR_UPDATE, change); + + // Update description, chartType and chart url and verify patch + originalJson = JsonUtils.pojoToJson(chart); + change = getChangeDescription(chart.getVersion()); + fieldUpdated(change, "description", "desc1", "desc2"); + fieldUpdated(change, "chartType", type1, type2); + fieldUpdated(change, "chartUrl", "url1", "url2"); + + chart.withChartType(type2).withChartUrl("url2").withDescription("desc2"); + patchEntityAndCheck(chart, originalJson, ADMIN_AUTH_HEADERS, UpdateType.MINOR_UPDATE, change); + } + @Override @Execution(ExecutionMode.CONCURRENT) public Chart validateGetWithDifferentFields(Chart chart, boolean byName) throws HttpResponseException { @@ -116,15 +202,28 @@ public class ChartResourceTest extends EntityResourceTest { public void validateCreatedEntity(Chart chart, CreateChart createRequest, Map authHeaders) { assertNotNull(chart.getServiceType()); assertReference(createRequest.getService(), chart.getService()); + assertEquals(createRequest.getChartType(), chart.getChartType()); + assertEquals(createRequest.getChartUrl(), chart.getChartUrl()); } @Override public void compareEntities(Chart expected, Chart patched, Map authHeaders) { assertReference(expected.getService(), patched.getService()); + assertEquals(expected.getChartType(), patched.getChartType()); + assertEquals(expected.getChartUrl(), patched.getChartUrl()); } @Override public void assertFieldChange(String fieldName, Object expected, Object actual) throws IOException { - assertCommonFieldChange(fieldName, expected, actual); + if (expected == actual) { + return; + } + if (fieldName.startsWith("chartType")) { + ChartType expectedChartType = (ChartType) expected; + ChartType actualChartType = ChartType.fromValue(actual.toString()); + assertEquals(expectedChartType, actualChartType); + } else { + assertCommonFieldChange(fieldName, expected, actual); + } } } diff --git a/openmetadata-spec/src/main/resources/json/schema/api/data/createChart.json b/openmetadata-spec/src/main/resources/json/schema/api/data/createChart.json index 9a46b95ca48..7f90a81085e 100644 --- a/openmetadata-spec/src/main/resources/json/schema/api/data/createChart.json +++ b/openmetadata-spec/src/main/resources/json/schema/api/data/createChart.json @@ -26,13 +26,6 @@ "description": "Chart URL suffix from its service.", "type": "string" }, - "tables": { - "description": "Link to list of table fully qualified names used in this chart.", - "type" : "array", - "items": { - "$ref" : "../../type/basic.json#/definitions/fullyQualifiedEntityName" - } - }, "tags": { "description": "Tags for this chart", "type": "array", diff --git a/openmetadata-spec/src/main/resources/json/schema/entity/data/chart.json b/openmetadata-spec/src/main/resources/json/schema/entity/data/chart.json index b4deb197e4e..f0a9a364cab 100644 --- a/openmetadata-spec/src/main/resources/json/schema/entity/data/chart.json +++ b/openmetadata-spec/src/main/resources/json/schema/entity/data/chart.json @@ -105,10 +105,6 @@ "description": "Owner of this dashboard.", "$ref": "../../type/entityReference.json" }, - "tables": { - "description": "Link to table used in this chart.", - "$ref": "../../type/entityReference.json#/definitions/entityReferenceList" - }, "followers": { "description": "Followers of this chart.", "$ref": "../../type/entityReference.json#/definitions/entityReferenceList"