jayendrap commented on code in PR #304:
URL: https://github.com/apache/atlas/pull/304#discussion_r1997615632
##########
repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java:
##########
@@ -1270,6 +1300,90 @@ public void testIncorrectFileException() {
}
}
+ @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, Number of pages required to fetch expected glossaries
+ {-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;
+
+ List<String> guidsToReturn = new ArrayList<>(allGlossaryGuids);
+ guidsToReturn.removeAll(finalGuidsToSkip);
+
+ 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);
+
+ // Create a list of AtlasEntity and AtlasGlossary objects
+ for (String guid : guidsToReturn) {
Review Comment:
Since on line 1363, we are creating already creating the list of AtlasEntity
objects to be returned, then do we need this block?
##########
repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java:
##########
@@ -121,26 +121,56 @@ public static boolean isNameInvalid(String name) {
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);
- List<AtlasGlossary> ret;
- List<String> guidsToLoad =
paginationHelper.getPaginatedList();
- AtlasEntity.AtlasEntitiesWithExtInfo glossaryEntities;
+ // Enable skipping failed entities during processing
+ RequestContext.get().setSkipFailedEntities(true);
Review Comment:
Put the code till line 172 in try block and in finally block unset
skipFailedEntities, else just in case there is an exception the
skipFailedEntities will remain true.
##########
repository/src/test/java/org/apache/atlas/glossary/GlossaryServiceTest.java:
##########
@@ -1270,6 +1300,90 @@ public void testIncorrectFileException() {
}
}
+ @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, Number of pages required to fetch expected glossaries
+ {-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;
+
+ List<String> guidsToReturn = new ArrayList<>(allGlossaryGuids);
+ guidsToReturn.removeAll(finalGuidsToSkip);
+
+ try (MockedStatic<AtlasGraphUtilsV2> mockedGraphUtilsClass =
Mockito.mockStatic(AtlasGraphUtilsV2.class)) {
Review Comment:
If an exception is thrown then we should fail the test.
##########
repository/src/main/java/org/apache/atlas/glossary/GlossaryService.java:
##########
@@ -1237,6 +1273,16 @@ List<T> getPaginatedList() {
return ret;
}
+ List<T> getPaginatedList(int offset, int limit) {
+ if (offset >= maxSize) {
+ return Collections.emptyList();
+ }
+
+ int pageStart = Math.max(0, offset);
Review Comment:
pageStart isn't used, so we can remove this. Also pageStart and pageEnd
variables will conflict with class level variable names
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@atlas.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
