fix: paginating test cases with testSuiteId filter (#13886)

open-metadata · Nov 7, 2023 · 04f6bc7 · 04f6bc7
1 parent 354c381
commit 04f6bc7
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 69 deletions.
diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java
@@ -3316,16 +3316,16 @@ default int countOfTestCases(List<UUID> testCaseIds) {
     @ConnectionAwareSqlQuery(
         value =
             "SELECT * FROM (SELECT json, ranked FROM "
-                + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json ->> '$.testCaseResult.timestamp') DESC) AS ranked FROM <table>) executionTimeSorted "
-                + "<cond> AND ranked < :before "
+                + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json ->> '$.testCaseResult.timestamp') DESC) AS ranked FROM <table> <cond>) executionTimeSorted "
+                + "WHERE ranked < :before "
                 + "ORDER BY ranked DESC "
                 + "LIMIT :limit) rankedBefore ORDER BY ranked",
         connectionType = MYSQL)
     @ConnectionAwareSqlQuery(
         value =
             "SELECT * FROM (SELECT json, ranked FROM "
-                + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json -> 'testCaseResult'->>'timestamp') DESC NULLS LAST) AS ranked FROM <table>) executionTimeSorted "
-                + "<cond> AND ranked < :before "
+                + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json -> 'testCaseResult'->>'timestamp') DESC NULLS LAST) AS ranked FROM <table> <cond>) executionTimeSorted "
+                + "WHERE ranked < :before "
                 + "ORDER BY ranked DESC "
                 + "LIMIT :limit) rankedBefore ORDER BY ranked",
         connectionType = POSTGRES)
@@ -3341,16 +3341,16 @@ List<TestCaseRecord> listBeforeTsOrdered(
         value =
             "SELECT json, ranked FROM "
                 + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json ->> '$.testCaseResult.timestamp') DESC ) AS ranked FROM <table> "
-                + ") executionTimeSorted "
-                + "<cond> AND ranked > :after "
+                + "<cond>) executionTimeSorted "
+                + "WHERE ranked > :after "
                 + "LIMIT :limit",
         connectionType = MYSQL)
     @ConnectionAwareSqlQuery(
         value =
             "SELECT json, ranked FROM "
                 + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json->'testCaseResult'->>'timestamp') DESC NULLS LAST) AS ranked FROM <table> "
-                + ") executionTimeSorted "
-                + "<cond> AND ranked > :after "
+                + "<cond>) executionTimeSorted "
+                + "WHERE ranked > :after "
                 + "LIMIT :limit",
         connectionType = POSTGRES)
     @RegisterRowMapper(TestCaseRecordMapper.class)

diff --git a/...ervice/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java b/...ervice/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java
@@ -1057,45 +1057,16 @@ void test_listTestCaseByExecutionTimePagination_200(TestInfo test) throws IOExce
     ResultList<TestCase> allEntities = getTestCases(1000000, null, null, "*", true, ADMIN_AUTH_HEADERS);
     int totalRecords = allEntities.getData().size();
 
-    // List entity with "limit" set from 1 to maxEntities size with random jumps (to reduce the test time)
-    // Each time compare the returned list with allTables list to make sure right results are returned
-    for (int limit = 1; limit < maxEntities; limit += random.nextInt(5) + 1) {
-      String after = null;
-      String before;
-      int pageCount = 0;
-      int indexInAllTables = 0;
-      ResultList<TestCase> forwardPage;
-      ResultList<TestCase> backwardPage;
-      boolean foundDeleted = false;
-      do { // For each limit (or page size) - forward scroll till the end
-        forwardPage = getTestCases(limit, null, after, "*", true, ADMIN_AUTH_HEADERS);
-        after = forwardPage.getPaging().getAfter();
-        before = forwardPage.getPaging().getBefore();
-        assertEntityPagination(allEntities.getData(), forwardPage, limit, indexInAllTables);
-
-        if (pageCount == 0) { // CASE 0 - First page is being returned. There is no before-cursor
-          assertNull(before);
-        } else {
-          // Make sure scrolling back based on before cursor returns the correct result
-          backwardPage = getTestCases(limit, before, null, "*", true, ADMIN_AUTH_HEADERS);
-          assertEntityPagination(allEntities.getData(), backwardPage, limit, (indexInAllTables - limit));
-        }
-
-        indexInAllTables += forwardPage.getData().size();
-        pageCount++;
-      } while (after != null);
+    paginate(maxEntities, allEntities, null);
 
-      // We have now reached the last page - test backward scroll till the beginning
-      pageCount = 0;
-      indexInAllTables = totalRecords - limit - forwardPage.getData().size();
-      do {
-        forwardPage = getTestCases(limit, before, null, "*", true, ADMIN_AUTH_HEADERS);
-        before = forwardPage.getPaging().getBefore();
-        assertEntityPagination(allEntities.getData(), forwardPage, limit, indexInAllTables);
-        pageCount++;
-        indexInAllTables -= forwardPage.getData().size();
-      } while (before != null);
-    }
+    // Validate Pagination when filtering by testSuiteId
+    TestSuiteResourceTest testSuiteResourceTest = new TestSuiteResourceTest();
+    CreateTestSuite createLogicalTestSuite = testSuiteResourceTest.createRequest(test);
+    TestSuite logicalTestSuite = testSuiteResourceTest.createEntity(createLogicalTestSuite, ADMIN_AUTH_HEADERS);
+    List<UUID> testCaseIds = createdTestCase.stream().map(TestCase::getId).collect(Collectors.toList());
+    testSuiteResourceTest.addTestCasesToLogicalTestSuite(logicalTestSuite, testCaseIds);
+    allEntities = getTestCases(1000000, null, null, "*", null, logicalTestSuite, false, true, ADMIN_AUTH_HEADERS);
+    paginate(maxEntities, allEntities, logicalTestSuite);
   }
 
   public void deleteTestCaseResult(String fqn, Long timestamp, Map<String, String> authHeaders)
@@ -1128,33 +1099,41 @@ private TestSummary getTestSummary(Map<String, String> authHeaders, String testS
   }
 
   public ResultList<TestCase> getTestCases(
-      Integer limit, String fields, String link, Boolean includeAll, Map<String, String> authHeaders)
+      Integer limit,
+      String before,
+      String after,
+      String fields,
+      String link,
+      TestSuite testSuite,
+      Boolean includeAll,
+      Boolean orderByLastExecutionDate,
+      Map<String, String> authHeaders)
       throws HttpResponseException {
     WebTarget target = getCollection();
-    target = limit != null ? target.queryParam("limit", limit) : target;
     target = target.queryParam("fields", fields);
-    if (link != null) {
-      target = target.queryParam("entityLink", link);
-    }
+    target = limit != null ? target.queryParam("limit", limit) : target;
+    target = before != null ? target.queryParam("before", before) : target;
+    target = after != null ? target.queryParam("after", after) : target;
+    target = link != null ? target.queryParam("entityLink", link) : target;
+    target = testSuite != null ? target.queryParam("testSuiteId", testSuite.getId()) : target;
+    target = orderByLastExecutionDate ? target.queryParam("orderByLastExecutionDate", true) : target;
     if (includeAll) {
       target = target.queryParam("includeAllTests", true);
       target = target.queryParam("include", "all");
     }
     return TestUtils.get(target, TestCaseResource.TestCaseList.class, authHeaders);
   }
 
+  public ResultList<TestCase> getTestCases(
+      Integer limit, String fields, String link, Boolean includeAll, Map<String, String> authHeaders)
+      throws HttpResponseException {
+    return getTestCases(limit, null, null, fields, link, null, includeAll, false, authHeaders);
+  }
+
   public ResultList<TestCase> getTestCases(
       Integer limit, String fields, TestSuite testSuite, Boolean includeAll, Map<String, String> authHeaders)
       throws HttpResponseException {
-    WebTarget target = getCollection();
-    target = limit != null ? target.queryParam("limit", limit) : target;
-    target = target.queryParam("fields", fields);
-    target = target.queryParam("testSuiteId", testSuite.getId());
-    if (includeAll) {
-      target = target.queryParam("includeAllTests", true);
-      target = target.queryParam("include", "all");
-    }
-    return TestUtils.get(target, TestCaseResource.TestCaseList.class, authHeaders);
+    return getTestCases(limit, null, null, fields, null, testSuite, includeAll, false, authHeaders);
   }
 
   public ResultList<TestCase> getTestCases(
@@ -1165,15 +1144,7 @@ public ResultList<TestCase> getTestCases(
       Boolean orderByLastExecutionDate,
       Map<String, String> authHeaders)
       throws HttpResponseException {
-    WebTarget target = getCollection();
-    target = limit != null ? target.queryParam("limit", limit) : target;
-    target = before != null ? target.queryParam("before", before) : target;
-    target = after != null ? target.queryParam("after", after) : target;
-    target = target.queryParam("fields", fields);
-    if (orderByLastExecutionDate) {
-      target = target.queryParam("orderByLastExecutionDate", true);
-    }
-    return TestUtils.get(target, TestCaseResource.TestCaseList.class, authHeaders);
+    return getTestCases(limit, before, after, fields, null, null, false, orderByLastExecutionDate, authHeaders);
   }
 
   private TestCaseResult patchTestCaseResult(
@@ -1217,6 +1188,51 @@ private void verifyTestCaseResult(TestCaseResult expected, TestCaseResult actual
     assertEquals(expected, actual);
   }
 
+  private void paginate(Integer maxEntities, ResultList<TestCase> allEntities, TestSuite testSuite)
+      throws HttpResponseException {
+    Random random = new Random();
+    int totalRecords = allEntities.getData().size();
+
+    for (int limit = 1; limit < maxEntities; limit += random.nextInt(5) + 1) {
+      String after = null;
+      String before;
+      int pageCount = 0;
+      int indexInAllTables = 0;
+      ResultList<TestCase> forwardPage;
+      ResultList<TestCase> backwardPage;
+      boolean foundDeleted = false;
+      do { // For each limit (or page size) - forward scroll till the end
+        forwardPage = getTestCases(limit, null, after, "*", null, testSuite, false, true, ADMIN_AUTH_HEADERS);
+        after = forwardPage.getPaging().getAfter();
+        before = forwardPage.getPaging().getBefore();
+        assertEntityPagination(allEntities.getData(), forwardPage, limit, indexInAllTables);
+
+        if (pageCount == 0) { // CASE 0 - First page is being returned. There is no before-cursor
+          assertNull(before);
+        } else {
+          // Make sure scrolling back based on before cursor returns the correct result
+          backwardPage = getTestCases(limit, before, null, "*", null, testSuite, false, true, ADMIN_AUTH_HEADERS);
+          getTestCases(limit, before, null, "*", true, ADMIN_AUTH_HEADERS);
+          assertEntityPagination(allEntities.getData(), backwardPage, limit, (indexInAllTables - limit));
+        }
+
+        indexInAllTables += forwardPage.getData().size();
+        pageCount++;
+      } while (after != null);
+
+      // We have now reached the last page - test backward scroll till the beginning
+      pageCount = 0;
+      indexInAllTables = totalRecords - limit - forwardPage.getData().size();
+      do {
+        forwardPage = getTestCases(limit, before, null, "*", null, testSuite, false, true, ADMIN_AUTH_HEADERS);
+        before = forwardPage.getPaging().getBefore();
+        assertEntityPagination(allEntities.getData(), forwardPage, limit, indexInAllTables);
+        pageCount++;
+        indexInAllTables -= forwardPage.getData().size();
+      } while (before != null);
+    }
+  }
+
   @Override
   public CreateTestCase createRequest(String name) {
     return new CreateTestCase()