From 9ec937114eeefe4313e3ea673070405c582de28d Mon Sep 17 00:00:00 2001 From: salman Date: Tue, 5 Dec 2023 19:38:47 +0530 Subject: [PATCH 01/11] feat: Allow specifying Data Product URN via UI --- .../src/main/resources/entity.graphql | 4 ++ .../CreateDataProductModal.tsx | 1 + .../DataProductBuilderForm.tsx | 59 ++++++++++++++++++- .../entity/domain/DataProductsTab/types.ts | 1 + 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index 4f3769d908815..87a7decd9c9ec 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -11054,6 +11054,10 @@ input CreateDataProductInput { The primary key of the Domain """ domainUrn: String! + """ + An id for the DataProduct + """ + id : String } """ diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx index 2d82521a90df5..a1c9f4aa9b5c3 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx @@ -32,6 +32,7 @@ export default function CreateDataProductModal({ domain, onCreateDataProduct, on variables: { input: { domainUrn: domain.urn, + id: builderState.id || '', properties: { name: builderState.name, description: builderState.description || undefined, diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx index b5a27a6e1b876..4c59036925f79 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx @@ -1,14 +1,33 @@ -import { Form, Input, Typography } from 'antd'; +import { Collapse, Form, Input, Typography } from 'antd'; import React from 'react'; import styled from 'styled-components'; import { Editor as MarkdownEditor } from '../../shared/tabs/Documentation/components/editor/Editor'; import { ANTD_GRAY } from '../../shared/constants'; import { DataProductBuilderState } from './types'; +import { validateCustomUrnId } from '../../../shared/textUtil'; const StyledEditor = styled(MarkdownEditor)` border: 1px solid ${ANTD_GRAY[4]}; `; +const FormItem = styled(Form.Item)` + .ant-form-item-label { + padding-bottom: 2px; + } +`; + +const FormItemWithMargin = styled(FormItem)` + margin-bottom: 16px; +`; + +const FormItemNoMargin = styled(FormItem)` + margin-bottom: 0; +`; + +const AdvancedLabel = styled(Typography.Text)` + color: #373d44; +`; + type Props = { builderState: DataProductBuilderState; updateBuilderState: (newState: DataProductBuilderState) => void; @@ -29,6 +48,13 @@ export default function DataProductBuilderForm({ builderState, updateBuilderStat }); } + function updateDomainId(id: string) { + updateBuilderState({ + ...builderState, + id, + }); + } + return (
Description}> + + Advanced Options} key="1"> + Domain Id} + help="By default, a random UUID will be generated to uniquely identify this domain. If + you'd like to provide a custom id instead to more easily keep track of this domain, + you may provide it here. Be careful, you cannot easily change the domain id after + creation." + > + ({ + validator(_, value) { + if (value && validateCustomUrnId(value)) { + return Promise.resolve(); + } + return Promise.reject(new Error('Please enter a valid Domain id')); + }, + }), + ]} + > + updateDomainId(e.target.value)} + /> + + + +
); } diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/types.ts b/datahub-web-react/src/app/entity/domain/DataProductsTab/types.ts index 1ed3ede39cfbe..4227c98cafbd4 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/types.ts +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/types.ts @@ -1,4 +1,5 @@ export type DataProductBuilderState = { name: string; + id?: string; description?: string; }; From 3c0c228e9d0499ef0767b3e347336f4a3b8455a6 Mon Sep 17 00:00:00 2001 From: salman Date: Tue, 5 Dec 2023 20:01:41 +0530 Subject: [PATCH 02/11] feat: fix text changes --- .../DataProductsTab/DataProductBuilderForm.tsx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx index 4c59036925f79..0c55a3c9b5cf8 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx @@ -48,7 +48,7 @@ export default function DataProductBuilderForm({ builderState, updateBuilderStat }); } - function updateDomainId(id: string) { + function updateDataProductId(id: string) { updateBuilderState({ ...builderState, id, @@ -76,10 +76,10 @@ export default function DataProductBuilderForm({ builderState, updateBuilderStat Advanced Options} key="1"> Domain Id} - help="By default, a random UUID will be generated to uniquely identify this domain. If - you'd like to provide a custom id instead to more easily keep track of this domain, - you may provide it here. Be careful, you cannot easily change the domain id after + label={Data Product Id} + help="By default, a random UUID will be generated to uniquely identify this data product. If + you'd like to provide a custom id instead to more easily keep track of this data product, + you may provide it here. Be careful, you cannot easily change the data product id after creation." > updateDomainId(e.target.value)} + onChange={(e) => updateDataProductId(e.target.value)} /> From 86df28c3e4cd615e1d4d10325665281eb61ba9f7 Mon Sep 17 00:00:00 2001 From: Aseem Bansal Date: Thu, 7 Dec 2023 15:39:08 +0530 Subject: [PATCH 03/11] add changes for the backend --- .../resolvers/dataproduct/CreateDataProductResolver.java | 1 + .../linkedin/metadata/service/DataProductService.java | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/dataproduct/CreateDataProductResolver.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/dataproduct/CreateDataProductResolver.java index 10c487a839f35..8ac7b2c3ce375 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/dataproduct/CreateDataProductResolver.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/resolvers/dataproduct/CreateDataProductResolver.java @@ -47,6 +47,7 @@ public CompletableFuture get(final DataFetchingEnvironment environm try { final Urn dataProductUrn = _dataProductService.createDataProduct( + input.getId(), input.getProperties().getName(), input.getProperties().getDescription(), authentication); diff --git a/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java b/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java index 10016ee89605b..34a41a2766d1e 100644 --- a/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java +++ b/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java @@ -58,11 +58,16 @@ public DataProductService(@Nonnull EntityClient entityClient, @Nonnull GraphClie * @return the urn of the newly created DataProduct */ public Urn createDataProduct( - @Nullable String name, @Nullable String description, @Nonnull Authentication authentication) { + @Nullable String id, @Nullable String name, @Nullable String description, + @Nonnull Authentication authentication) { // 1. Generate a unique id for the new DataProduct. final DataProductKey key = new DataProductKey(); - key.setId(UUID.randomUUID().toString()); + if (id != null && !id.isBlank()) { + key.setId(id); + } else { + key.setId(UUID.randomUUID().toString()); + } // 2. Create a new instance of DataProductProperties final DataProductProperties properties = new DataProductProperties(); From 22254c593ff442e3034dcfc95bd6a531e4c792f7 Mon Sep 17 00:00:00 2001 From: Aseem Bansal Date: Thu, 7 Dec 2023 20:11:29 +0530 Subject: [PATCH 04/11] fix lint --- .../com/linkedin/metadata/service/DataProductService.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java b/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java index 34a41a2766d1e..5385b1329a088 100644 --- a/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java +++ b/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java @@ -58,8 +58,10 @@ public DataProductService(@Nonnull EntityClient entityClient, @Nonnull GraphClie * @return the urn of the newly created DataProduct */ public Urn createDataProduct( - @Nullable String id, @Nullable String name, @Nullable String description, - @Nonnull Authentication authentication) { + @Nullable String id, + @Nullable String name, + @Nullable String description, + @Nonnull Authentication authentication) { // 1. Generate a unique id for the new DataProduct. final DataProductKey key = new DataProductKey(); From 3f2fdb2c49d6863a16721e618923e2fff9d18762 Mon Sep 17 00:00:00 2001 From: Aseem Bansal Date: Thu, 7 Dec 2023 21:22:23 +0530 Subject: [PATCH 05/11] improve error handling --- .../DataHubDataFetcherExceptionHandler.java | 32 ++++++++++++------- .../CreateDataProductModal.tsx | 4 +-- .../metadata/service/DataProductService.java | 13 ++++++++ 3 files changed, 36 insertions(+), 13 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java index 7c3ea1d581b6e..9b555afeefa2c 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java @@ -24,23 +24,33 @@ public DataFetcherExceptionHandlerResult onException( DataHubGraphQLErrorCode errorCode = DataHubGraphQLErrorCode.SERVER_ERROR; String message = "An unknown error occurred."; - // note: make sure to access the true error message via `getCause()` - if (exception.getCause() instanceof IllegalArgumentException) { + IllegalArgumentException illException = + findFirstThrowableCauseOfClass(exception, IllegalArgumentException.class); + if (illException != null) { + log.error("Illegal Argument Exception"); errorCode = DataHubGraphQLErrorCode.BAD_REQUEST; - message = exception.getCause().getMessage(); + message = illException.getMessage(); } - if (exception instanceof DataHubGraphQLException) { - errorCode = ((DataHubGraphQLException) exception).errorCode(); - message = exception.getMessage(); - } - - if (exception.getCause() instanceof DataHubGraphQLException) { - errorCode = ((DataHubGraphQLException) exception.getCause()).errorCode(); - message = exception.getCause().getMessage(); + DataHubGraphQLException graphQLException = + findFirstThrowableCauseOfClass(exception, DataHubGraphQLException.class); + if (graphQLException != null) { + errorCode = graphQLException.errorCode(); + message = graphQLException.getMessage(); } DataHubGraphQLError error = new DataHubGraphQLError(message, path, sourceLocation, errorCode); return DataFetcherExceptionHandlerResult.newResult().error(error).build(); } + + T findFirstThrowableCauseOfClass(Throwable throwable, Class clazz) { + while (throwable != null && throwable.getCause() != null) { + if (throwable.getCause().getClass().equals(clazz)) { + return (T) throwable.getCause(); + } else { + throwable = throwable.getCause(); + } + } + return null; + } } diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx index a1c9f4aa9b5c3..b9fbdbec36f22 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx @@ -50,10 +50,10 @@ export default function CreateDataProductModal({ domain, onCreateDataProduct, on onClose(); } }) - .catch(() => { + .catch(( error ) => { onClose(); message.destroy(); - message.error({ content: 'Failed to create Data Product. An unexpected error occurred' }); + message.error({ content: `Failed to create Data Product: ${error.message}.` }); }); } diff --git a/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java b/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java index 5385b1329a088..6c939fa713e86 100644 --- a/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java +++ b/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java @@ -29,8 +29,12 @@ import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; + +import com.linkedin.r2.RemoteInvocationException; import lombok.extern.slf4j.Slf4j; +import static com.linkedin.metadata.Constants.DATA_PRODUCT_ENTITY_NAME; + /** * This class is used to permit easy CRUD operations on a DataProduct * @@ -70,6 +74,15 @@ public Urn createDataProduct( } else { key.setId(UUID.randomUUID().toString()); } + try { + if (_entityClient.exists( + EntityKeyUtils.convertEntityKeyToUrn(key, DATA_PRODUCT_ENTITY_NAME), + authentication)) { + throw new IllegalArgumentException("This Data product already exists!"); + } + } catch (RemoteInvocationException e) { + throw new RuntimeException("Unable to check for existence of Data Product!"); + } // 2. Create a new instance of DataProductProperties final DataProductProperties properties = new DataProductProperties(); From 99ff161267cc369677f47f668625743134a49114 Mon Sep 17 00:00:00 2001 From: Aseem Bansal Date: Thu, 7 Dec 2023 21:42:27 +0530 Subject: [PATCH 06/11] fix lint --- .../linkedin/metadata/service/DataProductService.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java b/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java index 6c939fa713e86..d60427a27a5c5 100644 --- a/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java +++ b/metadata-service/services/src/main/java/com/linkedin/metadata/service/DataProductService.java @@ -1,5 +1,7 @@ package com.linkedin.metadata.service; +import static com.linkedin.metadata.Constants.DATA_PRODUCT_ENTITY_NAME; + import com.datahub.authentication.Authentication; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -22,6 +24,7 @@ import com.linkedin.metadata.graph.GraphClient; import com.linkedin.metadata.query.filter.RelationshipDirection; import com.linkedin.metadata.utils.EntityKeyUtils; +import com.linkedin.r2.RemoteInvocationException; import java.util.List; import java.util.Objects; import java.util.UUID; @@ -29,12 +32,8 @@ import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; - -import com.linkedin.r2.RemoteInvocationException; import lombok.extern.slf4j.Slf4j; -import static com.linkedin.metadata.Constants.DATA_PRODUCT_ENTITY_NAME; - /** * This class is used to permit easy CRUD operations on a DataProduct * @@ -76,8 +75,7 @@ public Urn createDataProduct( } try { if (_entityClient.exists( - EntityKeyUtils.convertEntityKeyToUrn(key, DATA_PRODUCT_ENTITY_NAME), - authentication)) { + EntityKeyUtils.convertEntityKeyToUrn(key, DATA_PRODUCT_ENTITY_NAME), authentication)) { throw new IllegalArgumentException("This Data product already exists!"); } } catch (RemoteInvocationException e) { From 6e8ab9b8127fa502b55b50a6603bcd326b8e54fc Mon Sep 17 00:00:00 2001 From: salman Date: Fri, 8 Dec 2023 17:06:22 +0530 Subject: [PATCH 07/11] feat: create separate Component for data product id --- .../DataProductAdvanceOption.tsx | 68 ++++++++++++++++++ .../DataProductBuilderForm.tsx | 70 ++----------------- .../entity/domain/DataProductsTab/types.ts | 5 ++ 3 files changed, 78 insertions(+), 65 deletions(-) create mode 100644 datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvanceOption.tsx diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvanceOption.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvanceOption.tsx new file mode 100644 index 0000000000000..8a56b758667eb --- /dev/null +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvanceOption.tsx @@ -0,0 +1,68 @@ +import React from "react"; +import { Collapse, Form, Input, Typography } from "antd"; +import styled from "styled-components"; +import { validateCustomUrnId } from '../../../shared/textUtil'; +import { DataProductBuilderFormProps } from "./types"; + + +const FormItem = styled(Form.Item)` + .ant-form-item-label { + padding-bottom: 2px; + } +`; + +const FormItemWithMargin = styled(FormItem)` + margin-bottom: 16px; +`; + +const FormItemNoMargin = styled(FormItem)` + margin-bottom: 0; +`; + +const AdvancedLabel = styled(Typography.Text)` + color: #373d44; +`; + +export function DataProductAdvanceOption({builderState, updateBuilderState }:DataProductBuilderFormProps){ + + function updateDataProductId(id: string) { + updateBuilderState({ + ...builderState, + id, + }); + } + + return ( + + Advanced Options} key="1"> + Data Product Id} + help="By default, a random UUID will be generated to uniquely identify this data product. If + you'd like to provide a custom id instead to more easily keep track of this data product, + you may provide it here. Be careful, you cannot easily change the data product id after + creation." + > + ({ + validator(_, value) { + if (value && validateCustomUrnId(value)) { + return Promise.resolve(); + } + return Promise.reject(new Error('Please enter a valid Data product id')); + }, + }), + ]} + > + updateDataProductId(e.target.value)} + /> + + + + + ) +} \ No newline at end of file diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx index 0c55a3c9b5cf8..ff322f67656d7 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx @@ -1,39 +1,16 @@ -import { Collapse, Form, Input, Typography } from 'antd'; +import { Form, Input, Typography } from 'antd'; import React from 'react'; import styled from 'styled-components'; import { Editor as MarkdownEditor } from '../../shared/tabs/Documentation/components/editor/Editor'; import { ANTD_GRAY } from '../../shared/constants'; -import { DataProductBuilderState } from './types'; -import { validateCustomUrnId } from '../../../shared/textUtil'; +import { DataProductBuilderFormProps } from './types'; +import { DataProductAdvanceOption } from './DataProductAdvanceOption'; const StyledEditor = styled(MarkdownEditor)` border: 1px solid ${ANTD_GRAY[4]}; `; -const FormItem = styled(Form.Item)` - .ant-form-item-label { - padding-bottom: 2px; - } -`; - -const FormItemWithMargin = styled(FormItem)` - margin-bottom: 16px; -`; - -const FormItemNoMargin = styled(FormItem)` - margin-bottom: 0; -`; - -const AdvancedLabel = styled(Typography.Text)` - color: #373d44; -`; - -type Props = { - builderState: DataProductBuilderState; - updateBuilderState: (newState: DataProductBuilderState) => void; -}; - -export default function DataProductBuilderForm({ builderState, updateBuilderState }: Props) { +export default function DataProductBuilderForm({ builderState, updateBuilderState }: DataProductBuilderFormProps) { function updateName(name: string) { updateBuilderState({ ...builderState, @@ -48,13 +25,6 @@ export default function DataProductBuilderForm({ builderState, updateBuilderStat }); } - function updateDataProductId(id: string) { - updateBuilderState({ - ...builderState, - id, - }); - } - return (
Description}> - - Advanced Options} key="1"> - Data Product Id} - help="By default, a random UUID will be generated to uniquely identify this data product. If - you'd like to provide a custom id instead to more easily keep track of this data product, - you may provide it here. Be careful, you cannot easily change the data product id after - creation." - > - ({ - validator(_, value) { - if (value && validateCustomUrnId(value)) { - return Promise.resolve(); - } - return Promise.reject(new Error('Please enter a valid Data product id')); - }, - }), - ]} - > - updateDataProductId(e.target.value)} - /> - - - - + ); } diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/types.ts b/datahub-web-react/src/app/entity/domain/DataProductsTab/types.ts index 4227c98cafbd4..fe22e3ed9a2a4 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/types.ts +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/types.ts @@ -3,3 +3,8 @@ export type DataProductBuilderState = { id?: string; description?: string; }; + +export type DataProductBuilderFormProps = { + builderState: DataProductBuilderState; + updateBuilderState: (newState: DataProductBuilderState) => void; +}; \ No newline at end of file From 9d428f315f36873060c54b41c548b9d0db91f28f Mon Sep 17 00:00:00 2001 From: salman Date: Mon, 11 Dec 2023 23:30:25 +0530 Subject: [PATCH 08/11] feat:resolve comment --- datahub-graphql-core/src/main/resources/entity.graphql | 4 ++-- .../entity/domain/DataProductsTab/CreateDataProductModal.tsx | 2 +- ...ProductAdvanceOption.tsx => DataProductAdvancedOption.tsx} | 2 +- .../entity/domain/DataProductsTab/DataProductBuilderForm.tsx | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) rename datahub-web-react/src/app/entity/domain/DataProductsTab/{DataProductAdvanceOption.tsx => DataProductAdvancedOption.tsx} (95%) diff --git a/datahub-graphql-core/src/main/resources/entity.graphql b/datahub-graphql-core/src/main/resources/entity.graphql index 0a00d837ef978..307c7f7b383e3 100644 --- a/datahub-graphql-core/src/main/resources/entity.graphql +++ b/datahub-graphql-core/src/main/resources/entity.graphql @@ -11056,9 +11056,9 @@ input CreateDataProductInput { """ domainUrn: String! """ - An id for the DataProduct + An optional id for the new data product """ - id : String + id: String } """ diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx index b9fbdbec36f22..0610fbfa7a770 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/CreateDataProductModal.tsx @@ -32,7 +32,7 @@ export default function CreateDataProductModal({ domain, onCreateDataProduct, on variables: { input: { domainUrn: domain.urn, - id: builderState.id || '', + id: builderState.id, properties: { name: builderState.name, description: builderState.description || undefined, diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvanceOption.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvancedOption.tsx similarity index 95% rename from datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvanceOption.tsx rename to datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvancedOption.tsx index 8a56b758667eb..a077a0308af1f 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvanceOption.tsx +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductAdvancedOption.tsx @@ -23,7 +23,7 @@ const AdvancedLabel = styled(Typography.Text)` color: #373d44; `; -export function DataProductAdvanceOption({builderState, updateBuilderState }:DataProductBuilderFormProps){ +export function DataProductAdvancedOption({builderState, updateBuilderState }: DataProductBuilderFormProps){ function updateDataProductId(id: string) { updateBuilderState({ diff --git a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx index ff322f67656d7..98bb09098a36e 100644 --- a/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx +++ b/datahub-web-react/src/app/entity/domain/DataProductsTab/DataProductBuilderForm.tsx @@ -4,7 +4,7 @@ import styled from 'styled-components'; import { Editor as MarkdownEditor } from '../../shared/tabs/Documentation/components/editor/Editor'; import { ANTD_GRAY } from '../../shared/constants'; import { DataProductBuilderFormProps } from './types'; -import { DataProductAdvanceOption } from './DataProductAdvanceOption'; +import { DataProductAdvancedOption } from './DataProductAdvancedOption'; const StyledEditor = styled(MarkdownEditor)` border: 1px solid ${ANTD_GRAY[4]}; @@ -43,7 +43,7 @@ export default function DataProductBuilderForm({ builderState, updateBuilderStat Description}> - + ); } From ec0254e349b59694df142689cf99c9e2f6c26d81 Mon Sep 17 00:00:00 2001 From: Aseem Bansal Date: Tue, 12 Dec 2023 19:22:29 +0530 Subject: [PATCH 09/11] ensure authorisation error messages do not break --- .../DataHubDataFetcherExceptionHandler.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java index 9b555afeefa2c..898f263d58761 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java @@ -12,6 +12,8 @@ @Slf4j public class DataHubDataFetcherExceptionHandler implements DataFetcherExceptionHandler { + private static final String DEFAULT_ERROR_MESSAGE = "An unknown error occurred."; + @Override public DataFetcherExceptionHandlerResult onException( DataFetcherExceptionHandlerParameters handlerParameters) { @@ -22,12 +24,11 @@ public DataFetcherExceptionHandlerResult onException( log.error("Failed to execute DataFetcher", exception); DataHubGraphQLErrorCode errorCode = DataHubGraphQLErrorCode.SERVER_ERROR; - String message = "An unknown error occurred."; + String message = DEFAULT_ERROR_MESSAGE; IllegalArgumentException illException = findFirstThrowableCauseOfClass(exception, IllegalArgumentException.class); if (illException != null) { - log.error("Illegal Argument Exception"); errorCode = DataHubGraphQLErrorCode.BAD_REQUEST; message = illException.getMessage(); } @@ -39,6 +40,13 @@ public DataFetcherExceptionHandlerResult onException( message = graphQLException.getMessage(); } + if (DEFAULT_ERROR_MESSAGE.equals(message) + && exception.getCause() != null + && exception.getCause() instanceof DataHubGraphQLException) { + errorCode = ((DataHubGraphQLException) exception.getCause()).errorCode(); + message = exception.getCause().getMessage(); + } + DataHubGraphQLError error = new DataHubGraphQLError(message, path, sourceLocation, errorCode); return DataFetcherExceptionHandlerResult.newResult().error(error).build(); } From 3e3f50dc6e80da0c68ef0a1227a0c361fb9aaeaf Mon Sep 17 00:00:00 2001 From: Aseem Bansal Date: Tue, 12 Dec 2023 22:06:04 +0530 Subject: [PATCH 10/11] ensure authorisation exception happens correctly --- .../DataHubDataFetcherExceptionHandler.java | 13 +++---------- smoke-test/tests/privileges/test_privileges.py | 7 ++++--- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java index 898f263d58761..87f001e487157 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java @@ -40,21 +40,14 @@ public DataFetcherExceptionHandlerResult onException( message = graphQLException.getMessage(); } - if (DEFAULT_ERROR_MESSAGE.equals(message) - && exception.getCause() != null - && exception.getCause() instanceof DataHubGraphQLException) { - errorCode = ((DataHubGraphQLException) exception.getCause()).errorCode(); - message = exception.getCause().getMessage(); - } - DataHubGraphQLError error = new DataHubGraphQLError(message, path, sourceLocation, errorCode); return DataFetcherExceptionHandlerResult.newResult().error(error).build(); } T findFirstThrowableCauseOfClass(Throwable throwable, Class clazz) { - while (throwable != null && throwable.getCause() != null) { - if (throwable.getCause().getClass().equals(clazz)) { - return (T) throwable.getCause(); + while (throwable != null) { + if (clazz.isInstance(throwable)) { + return (T) throwable; } else { throwable = throwable.getCause(); } diff --git a/smoke-test/tests/privileges/test_privileges.py b/smoke-test/tests/privileges/test_privileges.py index aa54a50b04e7f..75e2265f1f555 100644 --- a/smoke-test/tests/privileges/test_privileges.py +++ b/smoke-test/tests/privileges/test_privileges.py @@ -63,7 +63,7 @@ def _ensure_cant_perform_action(session, json,assertion_key): action_response.raise_for_status() action_data = action_response.json() - assert action_data["errors"][0]["extensions"]["code"] == 403 + assert action_data["errors"][0]["extensions"]["code"] == 403, action_data["errors"][0] assert action_data["errors"][0]["extensions"]["type"] == "UNAUTHORIZED" assert action_data["data"][assertion_key] == None @@ -367,8 +367,9 @@ def test_privilege_to_create_and_manage_policies(): # Verify new user can't create a policy create_policy = { - "query": """mutation createPolicy($input: PolicyUpdateInput!) {\n - createPolicy(input: $input) }""", + "query": """mutation createPolicy($input: PolicyUpdateInput!) { + createPolicy(input: $input) + }""", "variables": { "input": { "type": "PLATFORM", From ab47920b8a2a9b120e4ea6dfc9e32eb5dc4f055d Mon Sep 17 00:00:00 2001 From: Aseem Bansal Date: Tue, 12 Dec 2023 22:07:53 +0530 Subject: [PATCH 11/11] better stacktraces --- .../exception/DataHubDataFetcherExceptionHandler.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java index 87f001e487157..746ce0cdc10fe 100644 --- a/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java +++ b/datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/exception/DataHubDataFetcherExceptionHandler.java @@ -21,14 +21,13 @@ public DataFetcherExceptionHandlerResult onException( SourceLocation sourceLocation = handlerParameters.getSourceLocation(); ResultPath path = handlerParameters.getPath(); - log.error("Failed to execute DataFetcher", exception); - DataHubGraphQLErrorCode errorCode = DataHubGraphQLErrorCode.SERVER_ERROR; String message = DEFAULT_ERROR_MESSAGE; IllegalArgumentException illException = findFirstThrowableCauseOfClass(exception, IllegalArgumentException.class); if (illException != null) { + log.error("Failed to execute", illException); errorCode = DataHubGraphQLErrorCode.BAD_REQUEST; message = illException.getMessage(); } @@ -36,10 +35,14 @@ public DataFetcherExceptionHandlerResult onException( DataHubGraphQLException graphQLException = findFirstThrowableCauseOfClass(exception, DataHubGraphQLException.class); if (graphQLException != null) { + log.error("Failed to execute", graphQLException); errorCode = graphQLException.errorCode(); message = graphQLException.getMessage(); } + if (illException == null && graphQLException == null) { + log.error("Failed to execute", exception); + } DataHubGraphQLError error = new DataHubGraphQLError(message, path, sourceLocation, errorCode); return DataFetcherExceptionHandlerResult.newResult().error(error).build(); }