yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2025-12-24 22:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixes #10854 - Create/Update Chart Not working as Expected (#10991)

Browse Source
This commit is contained in:
Deepa Rao 2023-04-11 06:28:01 -07:00 committed by GitHub
parent 75aa42fe43
commit 147b7c551e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 123 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
bootstrap/sql/com.mysql.cj.jdbc.Driver/v009__create_db_connection_info.sql
View File

@ -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');

3
bootstrap/sql/org.postgresql.Driver/v009__create_db_connection_info.sql
View File

@ -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}';

2
ingestion/tests/unit/topology/dashboard/test_domodashboard.py
View File

@ -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"),

3
ingestion/tests/unit/topology/dashboard/test_quicksight.py
View File

@ -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",

3
ingestion/tests/unit/topology/dashboard/test_tableau.py
View File

@ -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"),

17
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ChartRepository.java
View File

@ -90,4 +90,21 @@ public class ChartRepository extends EntityRepository<Chart> {
.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());
}
}
}

1
openmetadata-service/src/main/java/org/openmetadata/service/resources/charts/ChartResource.java
View File

@ -418,7 +418,6 @@ public class ChartResource extends EntityResource<Chart, ChartRepository> {
.withService(EntityUtil.getEntityReference(Entity.DASHBOARD_SERVICE, create.getService()))
.withChartType(create.getChartType())
.withChartUrl(create.getChartUrl())
.withTables(getEntityReferences(Entity.TABLE, create.getTables()))
.withTags(create.getTags());
}
}

101
openmetadata-service/src/test/java/org/openmetadata/service/resources/charts/ChartResourceTest.java
View File

@ -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<Chart, CreateChart> {
@ -75,6 +81,86 @@ public class ChartResourceTest extends EntityResourceTest<Chart, CreateChart> {
}
}
@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<Chart, CreateChart> {
public void validateCreatedEntity(Chart chart, CreateChart createRequest, Map<String, String> 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<String, String> 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);
}
}
}

7
openmetadata-spec/src/main/resources/json/schema/api/data/createChart.json
View File

@ -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",

4
openmetadata-spec/src/main/resources/json/schema/entity/data/chart.json
View File

@ -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"