From 54f0152aa61dc42ddd2358fa5b88e87dc028f8e4 Mon Sep 17 00:00:00 2001 From: Umesh Patil Date: Fri, 31 Jul 2026 14:21:08 +0530 Subject: [PATCH 1/2] ATLAS-5322: Fix glossary bulk import relation parsing and error reporting. --- dashboard/src/components/ImportDialog.tsx | 23 ++-- .../__tests__/glossaryImportUtils.test.ts | 50 ++++++++ dashboard/src/utils/glossaryImportUtils.ts | 49 ++++++++ .../views/Glossary/AddUpdateGlossaryForm.tsx | 23 ++-- .../js/views/import/ImportLayoutView.js | 27 ++-- .../atlas/glossary/GlossaryService.java | 88 +++++++++++++ .../atlas/glossary/GlossaryTermUtils.java | 119 ++++++++++++++---- .../atlas/glossary/GlossaryServiceTest.java | 11 +- 8 files changed, 337 insertions(+), 53 deletions(-) create mode 100644 dashboard/src/utils/__tests__/glossaryImportUtils.test.ts create mode 100644 dashboard/src/utils/glossaryImportUtils.ts diff --git a/dashboard/src/components/ImportDialog.tsx b/dashboard/src/components/ImportDialog.tsx index 10d82aa1d08..1adfcaece5a 100644 --- a/dashboard/src/components/ImportDialog.tsx +++ b/dashboard/src/components/ImportDialog.tsx @@ -39,6 +39,11 @@ import ListItemText from "@mui/material/ListItemText"; import ArrowBackIosNewIcon from "@mui/icons-material/ArrowBackIosNew"; import { postGlossaryImportFormData } from "@utils/glossaryImportFlow"; import { getApiErrorToastMessage } from "@utils/apiErrorToastMessage"; +import { + buildGlossaryImportFailureSummary, + formatGlossaryImportFailure, + GlossaryImportFailure +} from "@utils/glossaryImportUtils"; const BootstrapDialog = styled(Dialog)(({ theme }) => ({ "& .MuiDialogContent-root": { @@ -113,7 +118,7 @@ export const ImportDialog: React.FC = ({ if (importResp.data.failedImportInfoList != undefined) { toast.dismiss(toastId.current); toastId.current = toast.error( - importResp.data.failedImportInfoList[0].remarks + buildGlossaryImportFailureSummary(importResp.data) ); setErrorDetails(true); @@ -200,17 +205,15 @@ export const ImportDialog: React.FC = ({ > {importData.failedImportInfoList.map( - ( - value: { - index: number; - remarks: string; - }, - index: number - ) => ( - + (value: GlossaryImportFailure, index: number) => ( + ) diff --git a/dashboard/src/utils/__tests__/glossaryImportUtils.test.ts b/dashboard/src/utils/__tests__/glossaryImportUtils.test.ts new file mode 100644 index 00000000000..520c45d3d57 --- /dev/null +++ b/dashboard/src/utils/__tests__/glossaryImportUtils.test.ts @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { + buildGlossaryImportFailureSummary, + formatGlossaryImportFailure +} from "../glossaryImportUtils"; + +describe("glossaryImportUtils", () => { + it("formats failure with glossary term label", () => { + expect( + formatGlossaryImportFailure({ + childObjectName: "Patient", + parentObjectName: "Healthcare Glossary", + remarks: "Reference not found" + }) + ).toBe("Patient@Healthcare Glossary: Reference not found"); + }); + + it("builds import summary with success and failure counts", () => { + expect( + buildGlossaryImportFailureSummary({ + successImportInfoList: [{ childObjectName: "A" }], + failedImportInfoList: [ + { + childObjectName: "Patient", + parentObjectName: "Healthcare Glossary", + remarks: "Invalid relation" + } + ] + }) + ).toBe( + "Glossary import completed with 1 failure(s) out of 2 term(s). See error details." + ); + }); +}); diff --git a/dashboard/src/utils/glossaryImportUtils.ts b/dashboard/src/utils/glossaryImportUtils.ts new file mode 100644 index 00000000000..56845297684 --- /dev/null +++ b/dashboard/src/utils/glossaryImportUtils.ts @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export interface GlossaryImportFailure { + childObjectName?: string; + parentObjectName?: string; + remarks?: string; + rowNumber?: number; +} + +export interface GlossaryImportResponse { + failedImportInfoList?: GlossaryImportFailure[]; + successImportInfoList?: GlossaryImportFailure[]; +} + +export const formatGlossaryImportFailure = ( + failure: GlossaryImportFailure +): string => { + const termLabel = + failure.childObjectName && failure.parentObjectName + ? `${failure.childObjectName}@${failure.parentObjectName}` + : failure.childObjectName || "Unknown term"; + + return `${termLabel}: ${failure.remarks || "Import failed"}`; +}; + +export const buildGlossaryImportFailureSummary = ( + response: GlossaryImportResponse +): string => { + const failedCount = response.failedImportInfoList?.length || 0; + const successCount = response.successImportInfoList?.length || 0; + const totalCount = failedCount + successCount; + + return `Glossary import completed with ${failedCount} failure(s) out of ${totalCount} term(s). See error details.`; +}; diff --git a/dashboard/src/views/Glossary/AddUpdateGlossaryForm.tsx b/dashboard/src/views/Glossary/AddUpdateGlossaryForm.tsx index 5f4a2285406..43833603990 100644 --- a/dashboard/src/views/Glossary/AddUpdateGlossaryForm.tsx +++ b/dashboard/src/views/Glossary/AddUpdateGlossaryForm.tsx @@ -28,6 +28,11 @@ import { postGlossaryImportFormData } from "@utils/glossaryImportFlow"; import { getApiErrorToastMessage } from "@utils/apiErrorToastMessage"; +import { + buildGlossaryImportFailureSummary, + formatGlossaryImportFailure, + GlossaryImportFailure +} from "@utils/glossaryImportUtils"; import { toast } from "react-toastify"; import { useCallback, useEffect, useRef, useState } from "react"; import type { MouseEvent } from "react"; @@ -163,7 +168,7 @@ const AddUpdateGlossaryForm = (props: { if (importResp.data.failedImportInfoList != undefined) { toast.dismiss(toastId.current); toastId.current = toast.error( - importResp.data.failedImportInfoList[0].remarks + buildGlossaryImportFailureSummary(importResp.data) ); setImportErrorDetails(true); } @@ -361,17 +366,15 @@ const AddUpdateGlossaryForm = (props: { > {importData.failedImportInfoList.map( - ( - value: { - index: number; - remarks: string; - }, - index: number - ) => ( - + (value: GlossaryImportFailure, index: number) => ( + ) diff --git a/dashboardv2/public/js/views/import/ImportLayoutView.js b/dashboardv2/public/js/views/import/ImportLayoutView.js index 27440bb9a4a..78bc2b5522d 100644 --- a/dashboardv2/public/js/views/import/ImportLayoutView.js +++ b/dashboardv2/public/js/views/import/ImportLayoutView.js @@ -117,18 +117,25 @@ define([ var success = true; if (response.failedImportInfoList && response.failedImportInfoList.length) { var errorStr = '', - notificationMsg = ''; + failedCount = response.failedImportInfoList.length, + successCount = (response.successImportInfoList && response.successImportInfoList.length) || 0, + totalCount = failedCount + successCount, + notificationMsg = 'Glossary import completed with ' + failedCount + ' failure(s) out of ' + totalCount + ' term(s). See error details.'; success = false; that.ui.errorDetails.empty(); - Utils.defaultErrorHandler(null, file.xhr, { defaultErrorMessage: response.failedImportInfoList[0].remarks }); - if (response.failedImportInfoList.length > 1) { - var modalTitle = '
Error Details
'; - _.each(response.failedImportInfoList, function(err_obj) { - errorStr += '
  • ' + _.escape(err_obj.remarks) + '
    • '; - }); - that.ui.errorDetails.append(errorStr); - that.toggleErrorAndDropZoneView({ title: modalTitle, isErrorView: true }); - } + Utils.notifyError({ + content: notificationMsg + }); + var modalTitle = '
    Error Details
    '; + _.each(response.failedImportInfoList, function(err_obj, index) { + var termLabel = err_obj.childObjectName || 'Unknown term'; + if (err_obj.parentObjectName) { + termLabel = err_obj.childObjectName + '@' + err_obj.parentObjectName; + } + errorStr += '
  • ' + (index + 1) + '. ' + _.escape(termLabel) + ': ' + _.escape(err_obj.remarks || '') + '
    • '; + }); + that.ui.errorDetails.append(errorStr); + that.toggleErrorAndDropZoneView({ title: modalTitle, isErrorView: true }); } if (success) { that.modal.trigger("cancel"); diff --git a/repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java b/repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java index f28b9cb625a..e254589f078 100644 --- a/repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java +++ b/repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java @@ -27,6 +27,7 @@ import org.apache.atlas.model.glossary.AtlasGlossary; import org.apache.atlas.model.glossary.AtlasGlossaryCategory; import org.apache.atlas.model.glossary.AtlasGlossaryTerm; +import org.apache.atlas.model.glossary.AtlasGlossaryTermHeader; import org.apache.atlas.model.glossary.relations.AtlasRelatedCategoryHeader; import org.apache.atlas.model.glossary.relations.AtlasRelatedTermHeader; import org.apache.atlas.model.glossary.relations.AtlasTermCategorizationHeader; @@ -53,9 +54,12 @@ import java.io.InputStream; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -945,6 +949,8 @@ public BulkImportResponse importGlossaryData(InputStream inputStream, String fil List glossaryTermsWithRelations = glossaryTermUtils.getGlossaryTermDataWithRelations(fileData, ret); updateGlossaryTermsRelation(glossaryTermsWithRelations, ret); + + reconcileBulkImportResponse(ret); } finally { glossaryTermUtils.clearImportCache(); } @@ -1181,8 +1187,21 @@ private void createGlossaryTerms(List glossaryTerms, BulkImpo String glossaryName = getGlossaryName(glossaryTerm); try { + if (termExists2(glossaryTerm)) { + String existingTermGuid = getExistingTermGuid(glossaryTerm); + + glossaryTermUtils.cacheImportedTermGuid(glossaryTerm.getQualifiedName(), existingTermGuid); + + bulkImportResponse.addToSuccessImportInfoList(new ImportInfo(glossaryName, glossaryTermName, SUCCESS, + AtlasJson.toJson(getGlossaryTermHeader(existingTermGuid, glossaryTerm.getQualifiedName())))); + + continue; + } + AtlasGlossaryTerm createdTerm = createTerm(glossaryTerm); + glossaryTermUtils.cacheImportedTermGuid(createdTerm.getQualifiedName(), createdTerm.getGuid()); + bulkImportResponse.addToSuccessImportInfoList(new ImportInfo(glossaryName, glossaryTermName, SUCCESS, AtlasJson.toJson(createdTerm.getGlossaryTermHeader()))); } catch (AtlasBaseException e) { LOG.error(AtlasErrorCode.FAILED_TO_CREATE_GLOSSARY_TERM.toString(), glossaryTermName, e); @@ -1194,6 +1213,20 @@ private void createGlossaryTerms(List glossaryTerms, BulkImpo checkForSuccessImports(bulkImportResponse); } + private String getExistingTermGuid(AtlasGlossaryTerm glossaryTerm) { + Map uniqAttr = new HashMap<>(); + + uniqAttr.put(QUALIFIED_NAME_ATTR, glossaryTerm.getQualifiedName()); + + AtlasVertex vertex = AtlasGraphUtilsV2.findByUniqueAttributes(atlasTypeRegistry.getEntityTypeByName(GlossaryUtils.ATLAS_GLOSSARY_TERM_TYPENAME), uniqAttr); + + return vertex != null ? AtlasGraphUtilsV2.getIdFromVertex(vertex) : null; + } + + private AtlasGlossaryTermHeader getGlossaryTermHeader(String termGuid, String qualifiedName) { + return new AtlasGlossaryTermHeader(termGuid, qualifiedName); + } + private void updateGlossaryTermsRelation(List glossaryTerms, BulkImportResponse bulkImportResponse) { for (AtlasGlossaryTerm glossaryTerm : glossaryTerms) { glossaryTermUtils.updateGlossaryTermRelations(glossaryTerm); @@ -1232,6 +1265,61 @@ private void checkForSuccessImports(BulkImportResponse bulkImportResponse) throw } } + private void reconcileBulkImportResponse(BulkImportResponse bulkImportResponse) { + if (CollectionUtils.isEmpty(bulkImportResponse.getFailedImportInfoList())) { + return; + } + + Map mergedFailures = new LinkedHashMap<>(); + + for (ImportInfo failedInfo : bulkImportResponse.getFailedImportInfoList()) { + String termKey = getImportTermKey(failedInfo); + + if (StringUtils.isBlank(termKey)) { + mergedFailures.put("row-" + failedInfo.getRemarks(), failedInfo); + continue; + } + + ImportInfo existingFailure = mergedFailures.get(termKey); + + if (existingFailure == null) { + mergedFailures.put(termKey, failedInfo); + } else { + existingFailure.setRemarks(mergeImportRemarks(existingFailure.getRemarks(), failedInfo.getRemarks())); + } + } + + bulkImportResponse.setFailedImportInfoList(new ArrayList<>(mergedFailures.values())); + + Set failedTermKeys = mergedFailures.keySet(); + + bulkImportResponse.getSuccessImportInfoList().removeIf(successInfo -> failedTermKeys.contains(getImportTermKey(successInfo))); + } + + private String getImportTermKey(ImportInfo importInfo) { + if (importInfo == null || StringUtils.isBlank(importInfo.getChildObjectName())) { + return StringUtils.EMPTY; + } + + return importInfo.getParentObjectName() + "|" + importInfo.getChildObjectName(); + } + + private String mergeImportRemarks(String existingRemarks, String newRemarks) { + if (StringUtils.isBlank(existingRemarks)) { + return newRemarks; + } + + if (StringUtils.isBlank(newRemarks) || StringUtils.equals(existingRemarks, newRemarks)) { + return existingRemarks; + } + + Set uniqueRemarks = new HashSet<>(Arrays.asList(existingRemarks.split(System.lineSeparator()))); + + uniqueRemarks.add(newRemarks); + + return String.join(System.lineSeparator(), uniqueRemarks); + } + static class PaginationHelper { private final int pageStart; private final int pageEnd; diff --git a/repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java b/repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java index 82f4a276033..94b035afa86 100644 --- a/repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java +++ b/repository/src/main/java/org/apache/atlas/glossary/GlossaryTermUtils.java @@ -182,6 +182,12 @@ public void clearImportCache() { GLOSSARY_TERM_Q_NAME_GUID_CACHE.get().clear(); } + public void cacheImportedTermGuid(String qualifiedName, String termGuid) { + if (StringUtils.isNotBlank(qualifiedName) && StringUtils.isNotBlank(termGuid)) { + GLOSSARY_TERM_Q_NAME_GUID_CACHE.get().put(qualifiedName, termGuid); + } + } + public void updateGlossaryTermRelations(AtlasGlossaryTerm updatedGlossaryTerm) { if (GLOSSARY_TERM_Q_NAME_GUID_CACHE.get().containsKey(updatedGlossaryTerm.getQualifiedName())) { try { @@ -320,36 +326,38 @@ protected Set getAtlasRelatedTermHeaderSet(String csvRec String[] csvRecordArray = csvRecord.split(FileUtils.ESCAPE_CHARACTER + FileUtils.PIPE_CHARACTER); - AtlasRelatedTermHeader relatedTermHeader; - for (String data : csvRecordArray) { - String[] dataArray = data.split(FileUtils.ESCAPE_CHARACTER + FileUtils.COLON_CHARACTER); + if (StringUtils.isBlank(data)) { + continue; + } - if (dataArray.length == 2) { - String relatedTermQualifiedName = dataArray[1] + INVALID_NAME_CHARS[0] + dataArray[0]; - String currTermQualifiedName = termName + INVALID_NAME_CHARS[0] + glossaryName; + RelatedTermReference relatedTermReference = resolveRelatedTermReference(data.trim(), glossaryName, failedTermMsgs, termName); - if (relatedTermQualifiedName.equalsIgnoreCase(currTermQualifiedName)) { - failedTermMsgs.add("Invalid relationship specified for Term. Term cannot have a relationship with self"); - } else { - AtlasVertex vertex = AtlasGraphUtilsV2.findByTypeAndUniquePropertyName(GlossaryUtils.ATLAS_GLOSSARY_TERM_TYPENAME, GlossaryUtils.ATLAS_GLOSSARY_TERM_TYPENAME + INVALID_NAME_CHARS[1] + QUALIFIED_NAME_ATTR, relatedTermQualifiedName); + if (relatedTermReference == null || StringUtils.isBlank(relatedTermReference.getQualifiedName())) { + continue; + } - if (vertex != null) { - String glossaryTermGuid = AtlasGraphUtilsV2.getIdFromVertex(vertex); + String relatedTermQualifiedName = relatedTermReference.getQualifiedName(); + String currTermQualifiedName = termName + INVALID_NAME_CHARS[0] + glossaryName; + + if (relatedTermQualifiedName.equalsIgnoreCase(currTermQualifiedName)) { + failedTermMsgs.add("Invalid relationship specified for Term. Term cannot have a relationship with self"); + } else { + String glossaryTermGuid = lookupGlossaryTermGuid(relatedTermQualifiedName); - relatedTermHeader = new AtlasRelatedTermHeader(); + if (StringUtils.isNotEmpty(glossaryTermGuid)) { + AtlasRelatedTermHeader relatedTermHeader = new AtlasRelatedTermHeader(); - relatedTermHeader.setTermGuid(glossaryTermGuid); + relatedTermHeader.setTermGuid(glossaryTermGuid); - cacheRelatedTermQNameGuid(currTermQualifiedName, relatedTermQualifiedName, glossaryTermGuid); + cacheRelatedTermQNameGuid(currTermQualifiedName, relatedTermQualifiedName, glossaryTermGuid); - ret.add(relatedTermHeader); - } else { - failedTermMsgs.add("The provided Reference " + dataArray[1] + "@" + dataArray[0] + " does not exist at Atlas referred at record with TermName : " + termName + " and GlossaryName : " + glossaryName); - } + ret.add(relatedTermHeader); + } else if (relatedTermReference.isExplicitReference()) { + failedTermMsgs.add("The provided Reference " + relatedTermQualifiedName + " does not exist in Atlas for TermName : " + termName + " and GlossaryName : " + glossaryName); + } else { + LOG.debug("Skipping unresolved same-glossary reference '{}' for term {}", data.trim(), termName); } - } else { - failedTermMsgs.add("Incorrect relation data specified for the term : " + termName + "@" + glossaryName); } } } @@ -357,6 +365,57 @@ protected Set getAtlasRelatedTermHeaderSet(String csvRec return ret; } + protected RelatedTermReference resolveRelatedTermReference(String relationReference, String glossaryName, List failedTermMsgs, String termName) { + if (StringUtils.isBlank(relationReference)) { + return null; + } + + String trimmedReference = relationReference.trim(); + + // Format: TermName@GlossaryName (Atlas qualified name) + if (StringUtils.contains(trimmedReference, String.valueOf(INVALID_NAME_CHARS[0]))) { + String[] atParts = trimmedReference.split(FileUtils.ESCAPE_CHARACTER + INVALID_NAME_CHARS[0], 2); + + if (atParts.length == 2 && StringUtils.isNotBlank(atParts[0]) && StringUtils.isNotBlank(atParts[1])) { + return new RelatedTermReference(atParts[0] + INVALID_NAME_CHARS[0] + atParts[1], true); + } + } + + // Format: GlossaryName:TermName + String[] colonParts = trimmedReference.split(FileUtils.ESCAPE_CHARACTER + FileUtils.COLON_CHARACTER, 2); + + if (colonParts.length == 2 && StringUtils.isNotBlank(colonParts[0]) && StringUtils.isNotBlank(colonParts[1])) { + return new RelatedTermReference(colonParts[1] + INVALID_NAME_CHARS[0] + colonParts[0], true); + } + + // Format: TermName (same glossary shorthand — best-effort, not a hard failure if unresolved) + if (!StringUtils.contains(trimmedReference, FileUtils.COLON_CHARACTER) && !StringUtils.contains(trimmedReference, String.valueOf(INVALID_NAME_CHARS[0]))) { + return new RelatedTermReference(trimmedReference + INVALID_NAME_CHARS[0] + glossaryName, false); + } + + failedTermMsgs.add("Incorrect relation data '" + trimmedReference + "' for term " + termName + INVALID_NAME_CHARS[0] + glossaryName + + ". Expected GlossaryName:TermName, TermName@GlossaryName, or TermName"); + + return null; + } + + protected String resolveRelatedTermQualifiedName(String relationReference, String glossaryName, List failedTermMsgs, String termName) { + RelatedTermReference relatedTermReference = resolveRelatedTermReference(relationReference, glossaryName, failedTermMsgs, termName); + + return relatedTermReference != null ? relatedTermReference.getQualifiedName() : null; + } + + protected String lookupGlossaryTermGuid(String relatedTermQualifiedName) { + AtlasVertex vertex = AtlasGraphUtilsV2.findByTypeAndUniquePropertyName(GlossaryUtils.ATLAS_GLOSSARY_TERM_TYPENAME, + GlossaryUtils.ATLAS_GLOSSARY_TERM_TYPENAME + INVALID_NAME_CHARS[1] + QUALIFIED_NAME_ATTR, relatedTermQualifiedName); + + if (vertex != null) { + return AtlasGraphUtilsV2.getIdFromVertex(vertex); + } + + return GLOSSARY_TERM_Q_NAME_GUID_CACHE.get().get(relatedTermQualifiedName); + } + protected AtlasGlossaryTerm populateGlossaryTermObject(List failedTermMsgList, String[] record, String glossaryGuid, boolean populateRelations) { int length = record.length; int i = INDEX_FOR_TERM_AT_RECORD; @@ -975,4 +1034,22 @@ private void copyRelations(AtlasGlossaryTerm toGlossaryTerm, AtlasGlossaryTerm f } } } + + protected static class RelatedTermReference { + private final String qualifiedName; + private final boolean explicitReference; + + protected RelatedTermReference(String qualifiedName, boolean explicitReference) { + this.qualifiedName = qualifiedName; + this.explicitReference = explicitReference; + } + + protected String getQualifiedName() { + return qualifiedName; + } + + protected boolean isExplicitReference() { + return explicitReference; + } + } } diff --git a/repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java b/repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java index f8cac1c3cdf..f38c1a27121 100644 --- a/repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java +++ b/repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java @@ -1271,6 +1271,13 @@ public void testImportGlossaryData() { assertNotNull(bulkImportResponse2); assertEquals(bulkImportResponse2.getSuccessImportInfoList().size(), 3); assertEquals(bulkImportResponse2.getFailedImportInfoList().size(), 0); + + InputStream inputStream3 = getFile(CSV_FILES, "glossary_healthcare_filled.csv"); + BulkImportResponse bulkImportResponse3 = glossaryService.importGlossaryData(inputStream3, "glossary_healthcare_filled.csv"); + + assertNotNull(bulkImportResponse3); + assertEquals(bulkImportResponse3.getSuccessImportInfoList().size(), 20); + assertEquals(bulkImportResponse3.getFailedImportInfoList().size(), 0); } catch (AtlasBaseException e) { fail("The GlossaryTerm should have been created " + e); } @@ -1322,9 +1329,9 @@ public void testIncorrectFileException() { try { BulkImportResponse bulkImportResponse = glossaryService.importGlossaryData(inputStream, "incorrectFile.csv"); - assertEquals(bulkImportResponse.getSuccessImportInfoList().size(), 1); + // Term is created in pass 1 but relation fails in pass 2; reconcile removes it from success + assertEquals(bulkImportResponse.getSuccessImportInfoList().size(), 0); - //Due to invalid Relation we get Failed message even the import succeeded for the term assertEquals(bulkImportResponse.getFailedImportInfoList().size(), 1); } catch (AtlasBaseException e) { fail("The incorrect file exception should have handled " + e); From 51a89eb5c7c3f61e970fda29b6f599b83b553b4e Mon Sep 17 00:00:00 2001 From: Umesh Patil Date: Thu, 6 Aug 2026 20:54:48 +0530 Subject: [PATCH 2/2] ATLAS-5322: Fix BM import regression, expand import failure tests and resolved review comments. --- dashboard/src/components/ImportDialog.tsx | 9 +- .../__tests__/ImportDialog.test.tsx | 64 +++++++++-- .../__tests__/glossaryImportUtils.test.ts | 88 +++++++++++++- dashboard/src/utils/glossaryImportUtils.ts | 68 ++++++++++- .../__tests__/AddUpdateGlossaryForm.test.tsx | 108 +++++++++++++++++- .../js/views/import/ImportLayoutView.js | 20 +++- 6 files changed, 327 insertions(+), 30 deletions(-) diff --git a/dashboard/src/components/ImportDialog.tsx b/dashboard/src/components/ImportDialog.tsx index 1adfcaece5a..e4ed19fd208 100644 --- a/dashboard/src/components/ImportDialog.tsx +++ b/dashboard/src/components/ImportDialog.tsx @@ -40,8 +40,8 @@ import ArrowBackIosNewIcon from "@mui/icons-material/ArrowBackIosNew"; import { postGlossaryImportFormData } from "@utils/glossaryImportFlow"; import { getApiErrorToastMessage } from "@utils/apiErrorToastMessage"; import { - buildGlossaryImportFailureSummary, - formatGlossaryImportFailure, + formatImportFailureForDisplay, + getImportFailureToastMessage, GlossaryImportFailure } from "@utils/glossaryImportUtils"; @@ -79,6 +79,7 @@ export const ImportDialog: React.FC = ({ const [errorDetails, setErrorDetails] = useState(false); const [importData, setImportData] = useState(null); const toastId: any = useRef(null); + const isGlossaryImport = title !== "Import Business Metadata"; const onUpload = async () => { if (fileData) { @@ -118,7 +119,7 @@ export const ImportDialog: React.FC = ({ if (importResp.data.failedImportInfoList != undefined) { toast.dismiss(toastId.current); toastId.current = toast.error( - buildGlossaryImportFailureSummary(importResp.data) + getImportFailureToastMessage(isGlossaryImport, importResp.data) ); setErrorDetails(true); @@ -213,7 +214,7 @@ export const ImportDialog: React.FC = ({ >     ) diff --git a/dashboard/src/components/__tests__/ImportDialog.test.tsx b/dashboard/src/components/__tests__/ImportDialog.test.tsx index d504ce78523..a3d9ab6f456 100644 --- a/dashboard/src/components/__tests__/ImportDialog.test.tsx +++ b/dashboard/src/components/__tests__/ImportDialog.test.tsx @@ -53,14 +53,14 @@ jest.mock('../muiComponents', () => ({ })) const uploadMock = jest.fn() -const glossaryMock = jest.fn() +const glossaryImportMock = jest.fn() jest.mock('../../api/apiMethods/entitiesApiMethods', () => ({ getBusinessMetadataImport: (...args: any[]) => uploadMock(...args) })) -jest.mock('../../api/apiMethods/glossaryApiMethod', () => ({ - getGlossaryImport: (...args: any[]) => glossaryMock(...args) +jest.mock('@utils/glossaryImportFlow', () => ({ + postGlossaryImportFormData: (...args: any[]) => glossaryImportMock(...args) })) jest.mock('../../views/SideBar/Import/ImportLayout', () => ({ @@ -100,10 +100,16 @@ describe('ImportDialog', () => { }) }) - it('shows error details when import returns failed info', async () => { - glossaryMock.mockResolvedValue({ + it('shows glossary-specific error details when glossary import returns failed info', async () => { + glossaryImportMock.mockResolvedValue({ data: { - failedImportInfoList: [{ index: 1, remarks: 'Bad row' }] + failedImportInfoList: [ + { + childObjectName: 'Patient', + parentObjectName: 'Healthcare Glossary', + remarks: 'Bad row' + } + ] } }) @@ -113,12 +119,48 @@ describe('ImportDialog', () => { fireEvent.click(screen.getByText('Upload')) await waitFor(() => { - expect(glossaryMock).toHaveBeenCalled() - expect(toastError).toHaveBeenCalledWith('Bad row') + expect(glossaryImportMock).toHaveBeenCalled() + expect(toastError).toHaveBeenCalledWith( + 'Glossary import completed with 1 failure(s) out of 1 term(s). See error details.' + ) }) expect(screen.getByText('Error Details')).toBeTruthy() - expect(screen.getByText('1. Bad row')).toBeTruthy() + expect( + screen.getByText('1. Patient@Healthcare Glossary: Bad row') + ).toBeTruthy() + }) + + it('shows business metadata errors without glossary formatting', async () => { + uploadMock.mockResolvedValue({ + data: { + failedImportInfoList: [ + { + parentObjectName: 'guid-123', + childObjectName: 'attr1', + remarks: 'Invalid attribute' + } + ] + } + }) + + render( + + ) + + fireEvent.click(screen.getByText('Select File')) + fireEvent.click(screen.getByText('Upload')) + + await waitFor(() => { + expect(uploadMock).toHaveBeenCalled() + expect(toastError).toHaveBeenCalledWith('Invalid attribute') + }) + + expect(screen.getByText('Error Details')).toBeTruthy() + expect(screen.getByText('1. Invalid attribute')).toBeTruthy() + expect( + screen.queryByText('1. attr1@guid-123: Invalid attribute') + ).toBeNull() }) it('handles upload errors', async () => { @@ -147,9 +189,9 @@ describe('ImportDialog', () => { }) it('returns to upload view when back is clicked', async () => { - glossaryMock.mockResolvedValue({ + glossaryImportMock.mockResolvedValue({ data: { - failedImportInfoList: [{ index: 1, remarks: 'Bad row' }] + failedImportInfoList: [{ remarks: 'Bad row' }] } }) diff --git a/dashboard/src/utils/__tests__/glossaryImportUtils.test.ts b/dashboard/src/utils/__tests__/glossaryImportUtils.test.ts index 520c45d3d57..78bad538166 100644 --- a/dashboard/src/utils/__tests__/glossaryImportUtils.test.ts +++ b/dashboard/src/utils/__tests__/glossaryImportUtils.test.ts @@ -16,12 +16,17 @@ */ import { + buildGenericImportFailureSummary, buildGlossaryImportFailureSummary, - formatGlossaryImportFailure + formatGenericImportFailure, + formatGlossaryImportFailure, + formatImportFailureForDisplay, + getGlossaryImportTermLabel, + getImportFailureToastMessage } from "../glossaryImportUtils"; describe("glossaryImportUtils", () => { - it("formats failure with glossary term label", () => { + it("formats failure with glossary term label when both names exist", () => { expect( formatGlossaryImportFailure({ childObjectName: "Patient", @@ -31,6 +36,33 @@ describe("glossaryImportUtils", () => { ).toBe("Patient@Healthcare Glossary: Reference not found"); }); + it("formats failure with childObjectName only (same-glossary shorthand)", () => { + expect( + formatGlossaryImportFailure({ + childObjectName: "Patient", + remarks: "Invalid relation" + }) + ).toBe("Patient: Invalid relation"); + expect(getGlossaryImportTermLabel({ childObjectName: "Patient" })).toBe( + "Patient" + ); + }); + + it("formats failure with remarks only (no term names)", () => { + expect( + formatGlossaryImportFailure({ + remarks: "Bad row" + }) + ).toBe("Unknown term: Bad row"); + }); + + it("falls back to Import failed when remarks are empty", () => { + expect(formatGlossaryImportFailure({ childObjectName: "Patient" })).toBe( + "Patient: Import failed" + ); + expect(formatGenericImportFailure({})).toBe("Import failed"); + }); + it("builds import summary with success and failure counts", () => { expect( buildGlossaryImportFailureSummary({ @@ -47,4 +79,56 @@ describe("glossaryImportUtils", () => { "Glossary import completed with 1 failure(s) out of 2 term(s). See error details." ); }); + + it("builds summary with failures only (no successes)", () => { + expect( + buildGlossaryImportFailureSummary({ + failedImportInfoList: [{ remarks: "Bad row" }] + }) + ).toBe( + "Glossary import completed with 1 failure(s) out of 1 term(s). See error details." + ); + }); + + it("builds summary with empty success and failure lists", () => { + expect(buildGlossaryImportFailureSummary({})).toBe( + "Glossary import completed with 0 failure(s) out of 0 term(s). See error details." + ); + }); + + it("uses generic import helpers for business metadata", () => { + const response = { + successImportInfoList: [{ remarks: "ok" }], + failedImportInfoList: [ + { + parentObjectName: "guid-123", + childObjectName: "attr1", + remarks: "Invalid attribute" + } + ] + }; + + expect(getImportFailureToastMessage(false, response)).toBe( + "Invalid attribute" + ); + expect(formatImportFailureForDisplay(false, response.failedImportInfoList[0])).toBe( + "Invalid attribute" + ); + expect(buildGenericImportFailureSummary(response)).toBe( + "Import completed with 1 failure(s) out of 2 item(s). See error details." + ); + }); + + it("uses generic summary toast for multiple business metadata failures", () => { + expect( + getImportFailureToastMessage(false, { + failedImportInfoList: [ + { remarks: "First error" }, + { remarks: "Second error" } + ] + }) + ).toBe( + "Import completed with 2 failure(s) out of 2 item(s). See error details." + ); + }); }); diff --git a/dashboard/src/utils/glossaryImportUtils.ts b/dashboard/src/utils/glossaryImportUtils.ts index 56845297684..51c43636ec8 100644 --- a/dashboard/src/utils/glossaryImportUtils.ts +++ b/dashboard/src/utils/glossaryImportUtils.ts @@ -27,17 +27,27 @@ export interface GlossaryImportResponse { successImportInfoList?: GlossaryImportFailure[]; } -export const formatGlossaryImportFailure = ( +/** Glossary-only: term label for TermName@GlossaryName when both names exist. */ +export const getGlossaryImportTermLabel = ( failure: GlossaryImportFailure ): string => { - const termLabel = - failure.childObjectName && failure.parentObjectName - ? `${failure.childObjectName}@${failure.parentObjectName}` - : failure.childObjectName || "Unknown term"; + if (failure.childObjectName && failure.parentObjectName) { + return `${failure.childObjectName}@${failure.parentObjectName}`; + } + if (failure.childObjectName) { + return failure.childObjectName; + } + return "Unknown term"; +}; - return `${termLabel}: ${failure.remarks || "Import failed"}`; +/** Glossary-only: one formatted failure line (term label + reason). */ +export const formatGlossaryImportFailure = ( + failure: GlossaryImportFailure +): string => { + return `${getGlossaryImportTermLabel(failure)}: ${failure.remarks || "Import failed"}`; }; +/** Glossary-only: summary toast for bulk glossary term import failures. */ export const buildGlossaryImportFailureSummary = ( response: GlossaryImportResponse ): string => { @@ -47,3 +57,49 @@ export const buildGlossaryImportFailureSummary = ( return `Glossary import completed with ${failedCount} failure(s) out of ${totalCount} term(s). See error details.`; }; + +/** Generic import (e.g. Business Metadata): use raw remarks, not glossary @ labels. */ +export const formatGenericImportFailure = ( + failure: GlossaryImportFailure +): string => { + return failure.remarks || "Import failed"; +}; + +/** Generic import summary when multiple failures exist (non-glossary imports). */ +export const buildGenericImportFailureSummary = ( + response: GlossaryImportResponse +): string => { + const failedCount = response.failedImportInfoList?.length || 0; + const successCount = response.successImportInfoList?.length || 0; + const totalCount = failedCount + successCount; + + return `Import completed with ${failedCount} failure(s) out of ${totalCount} item(s). See error details.`; +}; + +/** Pick toast message based on import type — glossary vs shared/BM dialog. */ +export const getImportFailureToastMessage = ( + isGlossaryImport: boolean, + response: GlossaryImportResponse +): string => { + if (isGlossaryImport) { + return buildGlossaryImportFailureSummary(response); + } + + const failedList = response.failedImportInfoList; + if (failedList && failedList.length === 1) { + return failedList[0]?.remarks ?? "Import failed"; + } + + return buildGenericImportFailureSummary(response); +}; + +/** Pick error-detail line based on import type. */ +export const formatImportFailureForDisplay = ( + isGlossaryImport: boolean, + failure: GlossaryImportFailure +): string => { + if (isGlossaryImport) { + return formatGlossaryImportFailure(failure); + } + return formatGenericImportFailure(failure); +}; diff --git a/dashboard/src/views/Glossary/__tests__/AddUpdateGlossaryForm.test.tsx b/dashboard/src/views/Glossary/__tests__/AddUpdateGlossaryForm.test.tsx index fbb8e57b5cb..ec89306f251 100644 --- a/dashboard/src/views/Glossary/__tests__/AddUpdateGlossaryForm.test.tsx +++ b/dashboard/src/views/Glossary/__tests__/AddUpdateGlossaryForm.test.tsx @@ -32,8 +32,11 @@ const mockEditGlossary = jest.fn(); const mockFetchGlossaryData = jest.fn(); const mockOnClose = jest.fn(); const mockToastSuccess = jest.fn(); +const mockToastError = jest.fn(); const mockToastDismiss = jest.fn(); const mockServerError = jest.fn(); +const mockPostGlossaryImportFormData = jest.fn(); +const mockDownloadGlossaryImportTemplate = jest.fn(); // Mock glossary data const mockGlossaryData = [ @@ -57,10 +60,33 @@ const mockGlossaryData = [ jest.mock('react-toastify', () => ({ toast: { success: (...args: any[]) => mockToastSuccess(...args), + error: (...args: any[]) => mockToastError(...args), dismiss: (...args: any[]) => mockToastDismiss(...args) } })); +jest.mock('@utils/glossaryImportFlow', () => ({ + postGlossaryImportFormData: (...args: any[]) => + mockPostGlossaryImportFormData(...args), + downloadGlossaryImportTemplate: (...args: any[]) => + mockDownloadGlossaryImportTemplate(...args) +})); + +jest.mock('@views/SideBar/Import/ImportLayout', () => ({ + __esModule: true, + default: ({ setFileData, setProgress }: any) => ( + + ) +})); + // Mock API methods jest.mock('@api/apiMethods/glossaryApiMethod', () => ({ createGlossary: (...args: any[]) => mockCreateGlossary(...args), @@ -156,10 +182,10 @@ jest.mock('@utils/Utils', () => { // Mock Modal component jest.mock('@components/Modal', () => ({ __esModule: true, - default: ({ open, onClose, children, title, button1Label, button1Handler, button2Label, button2Handler, disableButton2 }: any) => + default: ({ open, onClose, children, title, titleIcon, button1Label, button1Handler, button2Label, button2Handler, disableButton2 }: any) => open ? (
    -
    {title}
    +
    {titleIcon}{title}
    {children}