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 <[email protected]>
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 <[email protected]>
---
.../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);
