This is an automated email from the ASF dual-hosted git repository. pinal pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/atlas.git
The following commit(s) were added to refs/heads/master by this push: new 92174bbd9 ATLAS-4995: When user doesn't have permission on one glossary , /glossary call throws exception, it doesn't list the remaining glossaries as well (#304) 92174bbd9 is described below commit 92174bbd96bf61228a39b22a3c2b7a16b58cf95c Author: sheetalshah1007 <sheetal.s...@freestoneinfotech.com> AuthorDate: Tue Mar 18 16:32:01 2025 +0530 ATLAS-4995: When user doesn't have permission on one glossary , /glossary call throws exception, it doesn't list the remaining glossaries as well (#304) Co-authored-by: Sheetal Shah <sheetal.s...@cloudera.com> --- .../org/apache/atlas/glossary/GlossaryService.java | 77 ++++++++++--- .../apache/atlas/glossary/GlossaryServiceTest.java | 127 +++++++++++++++++++-- 2 files changed, 182 insertions(+), 22 deletions(-) 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 5533d9292..c48d2490d 100644 --- a/repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java +++ b/repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java @@ -18,6 +18,7 @@ package org.apache.atlas.glossary; import org.apache.atlas.AtlasErrorCode; +import org.apache.atlas.RequestContext; import org.apache.atlas.SortOrder; import org.apache.atlas.annotation.GraphTransaction; import org.apache.atlas.bulkimport.BulkImportResponse; @@ -71,8 +72,7 @@ import static org.apache.atlas.glossary.GlossaryUtils.getGlossarySkeleton; @Service public class GlossaryService { - private static final Logger LOG = LoggerFactory.getLogger(GlossaryService.class); - + private static final Logger LOG = LoggerFactory.getLogger(GlossaryService.class); private static final String ATLAS_GLOSSARY_TERM = "AtlasGlossaryTerm"; private static final String NAME_ATTR = "name"; private static final String QUALIFIED_NAME_ATTR = "qualifiedName"; @@ -121,24 +121,56 @@ public class GlossaryService { public List<AtlasGlossary> getGlossaries(int limit, int offset, SortOrder sortOrder) throws AtlasBaseException { LOG.debug("==> GlossaryService.getGlossaries({}, {}, {})", limit, offset, sortOrder); - List<String> glossaryGuids = AtlasGraphUtilsV2.findEntityGUIDsByType(GlossaryUtils.ATLAS_GLOSSARY_TYPENAME, sortOrder); - PaginationHelper<String> paginationHelper = new PaginationHelper<>(glossaryGuids, offset, limit); + List<String> glossaryGuids = AtlasGraphUtilsV2.findEntityGUIDsByType(GlossaryUtils.ATLAS_GLOSSARY_TYPENAME, sortOrder); + + if (CollectionUtils.isEmpty(glossaryGuids)) { + return Collections.emptyList(); + } + + List<AtlasGlossary> ret = new ArrayList<>(); + int currentOffset = offset; + int maxSize = glossaryGuids.size(); + + // If limit is negative, use maxSize; otherwise, take the minimum of limit and maxSize + int adjustedLimit = (limit < 0) ? maxSize : Integer.min(maxSize, limit); + + try { + // Enable skipping failed entities during processing + RequestContext.get().setSkipFailedEntities(true); + + PaginationHelper<String> paginationHelper = new PaginationHelper<>(glossaryGuids); + + while (ret.size() < adjustedLimit && currentOffset < glossaryGuids.size()) { + // Fetch next batch of GUIDs + List<String> guidsToLoad = paginationHelper.getPaginatedList(currentOffset, adjustedLimit - ret.size()); - List<AtlasGlossary> ret; - List<String> guidsToLoad = paginationHelper.getPaginatedList(); - AtlasEntity.AtlasEntitiesWithExtInfo glossaryEntities; + // If no GUIDs are found, stop fetching + if (CollectionUtils.isEmpty(guidsToLoad)) { + break; + } + + // Fetch glossary entities by GUIDs + AtlasEntity.AtlasEntitiesWithExtInfo glossaryEntities = dataAccess.getAtlasEntityStore().getByIds(guidsToLoad, true, false); + List<AtlasEntity> entityList = glossaryEntities.getEntities(); - if (CollectionUtils.isNotEmpty(guidsToLoad)) { - glossaryEntities = dataAccess.getAtlasEntityStore().getByIds(guidsToLoad, true, false); - ret = new ArrayList<>(); + if (CollectionUtils.isNotEmpty(entityList)) { + for (AtlasEntity glossaryEntity : entityList) { + AtlasGlossary glossary = glossaryDTO.from(glossaryEntity); + ret.add(glossary); - for (AtlasEntity glossaryEntity : glossaryEntities.getEntities()) { - AtlasGlossary glossary = glossaryDTO.from(glossaryEntity); + // Stop adding if the page limit is reached + if (ret.size() >= adjustedLimit) { + break; + } + } + } - ret.add(glossary); + // Move offset forward + currentOffset += guidsToLoad.size(); } - } else { - ret = Collections.emptyList(); + } finally { + // Disable skipping failed entities after processing + RequestContext.get().setSkipFailedEntities(false); } LOG.debug("<== GlossaryService.getGlossaries() : {}", ret); @@ -1221,6 +1253,12 @@ public class GlossaryService { pageEnd = Integer.min(adjustedLimit + pageStart, maxSize); } + PaginationHelper(Collection<T> items) { + this.items = new ArrayList<>(items); + this.maxSize = items.size(); + pageStart = pageEnd = 0; + } + List<T> getPaginatedList() { List<T> ret; @@ -1237,6 +1275,15 @@ public class GlossaryService { return ret; } + List<T> getPaginatedList(int offset, int limit) { + if (offset >= maxSize) { + return Collections.emptyList(); + } + + int localPageEnd = Math.min(offset + limit, maxSize); + return items.subList(offset, localPageEnd); + } + private boolean isPagingNeeded() { return !(pageStart == 0 && pageEnd == maxSize) && pageStart <= pageEnd; } 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 02a92305e..bee4e44ae 100644 --- a/repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java +++ b/repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java @@ -40,8 +40,11 @@ import org.apache.atlas.model.instance.AtlasRelatedObjectId; import org.apache.atlas.model.instance.EntityMutationResponse; import org.apache.atlas.model.typedef.AtlasClassificationDef; import org.apache.atlas.model.typedef.AtlasTypesDef; +import org.apache.atlas.repository.ogm.DataAccess; +import org.apache.atlas.repository.ogm.glossary.AtlasGlossaryDTO; import org.apache.atlas.repository.store.graph.AtlasEntityStore; import org.apache.atlas.repository.store.graph.v2.AtlasEntityStream; +import org.apache.atlas.repository.store.graph.v2.AtlasGraphUtilsV2; import org.apache.atlas.store.AtlasTypeDefStore; import org.apache.atlas.type.AtlasType; import org.apache.atlas.type.AtlasTypeRegistry; @@ -50,6 +53,10 @@ import org.apache.atlas.utils.AtlasJson; import org.apache.atlas.utils.TestLoadModelUtils; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.SkipException; @@ -71,6 +78,14 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertNull; @@ -79,13 +94,11 @@ import static org.testng.Assert.fail; @Guice(modules = TestModules.TestOnlyModule.class) public class GlossaryServiceTest { - private static final Logger LOG = LoggerFactory.getLogger(GlossaryServiceTest.class); - - public static final String CSV_FILES = "/csvFiles/"; - public static final String EXCEL_FILES = "/excelFiles/"; - + public static final String CSV_FILES = "/csvFiles/"; + public static final String EXCEL_FILES = "/excelFiles/"; + private static final Logger LOG = LoggerFactory.getLogger(GlossaryServiceTest.class); @Inject - private GlossaryService glossaryService; + private GlossaryService glossaryService; @Inject private AtlasTypeDefStore typeDefStore; @@ -94,11 +107,23 @@ public class GlossaryServiceTest { private AtlasTypeRegistry typeRegistry; @Inject - private AtlasEntityStore entityStore; + private AtlasEntityStore entityStore; @Inject private AtlasDiscoveryService discoveryService; + @Mock + private GlossaryService mockedGlossaryService; // Mocked GlossaryService + + @Mock + private DataAccess mockedDataAccess; // Mocked DataAccess layer + + @Mock + private AtlasEntityStore mockedEntityStore; // Mocked Entity Store + + @Mock + private AtlasGlossaryDTO mockedGlossaryDTO; // Mocked DTO for entity conversion + private AtlasGlossary bankGlossary; private AtlasGlossary creditUnionGlossary; private AtlasGlossary debitUnionGlossary; @@ -242,6 +267,11 @@ public class GlossaryServiceTest { currentAccount.setAbbreviation("CURR"); currentAccount.setExamples(Arrays.asList("Personal", "Joint")); currentAccount.setUsage("N/A"); + + // Create a real instance of GlossaryService with mocked dependencies + mockedDataAccess = mock(DataAccess.class); + mockedGlossaryDTO = mock(AtlasGlossaryDTO.class); + mockedGlossaryService = new GlossaryService(mockedDataAccess, null, null, null, mockedGlossaryDTO); } @Test(groups = "Glossary.CREATE") @@ -1270,6 +1300,89 @@ public class GlossaryServiceTest { } } + @DataProvider(name = "getAllGlossaryForPaginationDataProvider") + public Object[][] getAllGlossaryForPaginationDataProvider() { + + List<String> listAllGuids = Arrays.asList("guid-1", "guid-2", "guid-3", "guid-4", "guid-5", + "guid-6", "guid-7", "guid-8", "guid-9", "guid-10"); + + return new Object[][] { + // limit, offset, sortOrder, expected glossaries guids, skipped glossaries guids, expected page-count + {-1, 0, SortOrder.ASCENDING, 10, listAllGuids, null, 1}, + {15, 0, SortOrder.ASCENDING, 10, listAllGuids, null, 1}, + {10, 0, SortOrder.ASCENDING, 10, listAllGuids, null, 1}, + {10, 2, SortOrder.ASCENDING, 8, listAllGuids, null, 1}, + {10, 6, SortOrder.ASCENDING, 4, listAllGuids, null, 1}, + {5, 0, SortOrder.ASCENDING, 5, listAllGuids, null, 1}, + {5, 5, SortOrder.ASCENDING, 5, listAllGuids, null, 1}, + {5, 2, SortOrder.ASCENDING, 5, listAllGuids, null, 1}, + {10, 0, SortOrder.ASCENDING, 9, listAllGuids, Collections.singletonList("guid-3"), 1}, + {5, 2, SortOrder.ASCENDING, 5, listAllGuids, Arrays.asList("guid-3", "guid-7"), 2} + }; + } + + @Test(dataProvider = "getAllGlossaryForPaginationDataProvider") + void testGetGlossaries_WithPaginationHandlingSkippedGlossaries(int limit, int offset, SortOrder sortOrder, int expectedSize, + List<String> allGlossaryGuids, List<String> guidsToSkip, int expectedPageCount) throws Exception { + + mockedEntityStore = mock(AtlasEntityStore.class); + when(mockedDataAccess.getAtlasEntityStore()).thenReturn(mockedEntityStore); + + List<String> finalGuidsToSkip = (guidsToSkip == null) ? Collections.emptyList() : guidsToSkip; + + try (MockedStatic<AtlasGraphUtilsV2> mockedGraphUtilsClass = Mockito.mockStatic(AtlasGraphUtilsV2.class)) { + + // Mocking the retrieval of glossary GUIDs from the system + mockedGraphUtilsClass.when(() -> AtlasGraphUtilsV2.findEntityGUIDsByType(anyString(), any())) + .thenReturn(allGlossaryGuids); + + //Mocking getByIds() so it removes skipped glossaries before returning results. + when(mockedEntityStore.getByIds(anyList(), anyBoolean(), anyBoolean())) + .thenAnswer(invocation -> { + List<String> requestedGuids = invocation.getArgument(0); // Get first argument (List<String>) + List<AtlasEntity> filteredEntities = new ArrayList<>(); + + // Filter out guids to be skipped + for (String guid : requestedGuids) { + if (!finalGuidsToSkip.contains(guid)) { + AtlasEntity entity = new AtlasEntity(); + entity.setGuid(guid); + filteredEntities.add(entity); + + AtlasGlossary glossaryToReturn = new AtlasGlossary(); + glossaryToReturn.setGuid(guid); + when(mockedGlossaryDTO.from(entity)).thenReturn(glossaryToReturn); + } + } + return new AtlasEntity.AtlasEntitiesWithExtInfo(filteredEntities); + }); + + List<AtlasGlossary> fetchedGlossaries = mockedGlossaryService.getGlossaries(limit, offset, sortOrder); + + assertNotNull(fetchedGlossaries); + + int actualFetchedResults = fetchedGlossaries.size(); + assertEquals(actualFetchedResults, expectedSize, "Expected to fetch " + expectedSize + " glossaries but got " + actualFetchedResults); + + for (AtlasGlossary fetchedGlossary : fetchedGlossaries) { + assertNotNull(fetchedGlossary); + } + + // Capture method calls to count pages + ArgumentCaptor<List<String>> guidsCaptor = ArgumentCaptor.forClass(List.class); + verify(mockedEntityStore, atLeast(1)).getByIds(guidsCaptor.capture(), anyBoolean(), anyBoolean()); + + // Number of pages that were needed is the number of times `getByIds()` was called + int actualPageCount = guidsCaptor.getAllValues().size(); + + //Verify that the number of pages fetched matches expectation + assertEquals(actualPageCount, expectedPageCount, + "Expected " + expectedPageCount + " pages but got " + actualPageCount); + } catch (Exception e) { + fail("Test failed due to exception: " + e.getMessage()); + } + } + private static InputStream getFile(String subDir, String fileName) { final String userDir = System.getProperty("user.dir"); String filePath = getTestFilePath(userDir, subDir, fileName);