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

Reply via email to