From 04f6bc7b6258cf3b5a62a7e4c22aa2d0086a9783 Mon Sep 17 00:00:00 2001 From: Teddy Date: Tue, 7 Nov 2023 18:27:09 +0100 Subject: [PATCH] fix: paginating test cases with testSuiteId filter (#13886) --- .../service/jdbi3/CollectionDAO.java | 16 +- .../dqtests/TestCaseResourceTest.java | 138 ++++++++++-------- 2 files changed, 85 insertions(+), 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 index 5f96d1678ced..902d6943e0de 100644 --- 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 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 ) executionTimeSorted " - + " AND ranked < :before " + + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json ->> '$.testCaseResult.timestamp') DESC) AS ranked FROM
) 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
) executionTimeSorted " - + " AND ranked < :before " + + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json -> 'testCaseResult'->>'timestamp') DESC NULLS LAST) AS ranked FROM
) executionTimeSorted " + + "WHERE ranked < :before " + "ORDER BY ranked DESC " + "LIMIT :limit) rankedBefore ORDER BY ranked", connectionType = POSTGRES) @@ -3341,16 +3341,16 @@ List listBeforeTsOrdered( value = "SELECT json, ranked FROM " + "(SELECT id, json, deleted, ROW_NUMBER() OVER(ORDER BY (json ->> '$.testCaseResult.timestamp') DESC ) AS ranked FROM
" - + ") executionTimeSorted " - + " AND ranked > :after " + + ") 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
" - + ") executionTimeSorted " - + " AND ranked > :after " + + ") executionTimeSorted " + + "WHERE ranked > :after " + "LIMIT :limit", connectionType = POSTGRES) @RegisterRowMapper(TestCaseRecordMapper.class) diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java index 4bc7907033f9..7328166da949 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java @@ -1057,45 +1057,16 @@ void test_listTestCaseByExecutionTimePagination_200(TestInfo test) throws IOExce ResultList 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 forwardPage; - ResultList 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 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 authHeaders) @@ -1128,14 +1099,24 @@ private TestSummary getTestSummary(Map authHeaders, String testS } public ResultList getTestCases( - Integer limit, String fields, String link, Boolean includeAll, Map authHeaders) + Integer limit, + String before, + String after, + String fields, + String link, + TestSuite testSuite, + Boolean includeAll, + Boolean orderByLastExecutionDate, + Map 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"); @@ -1143,18 +1124,16 @@ public ResultList getTestCases( return TestUtils.get(target, TestCaseResource.TestCaseList.class, authHeaders); } + public ResultList getTestCases( + Integer limit, String fields, String link, Boolean includeAll, Map authHeaders) + throws HttpResponseException { + return getTestCases(limit, null, null, fields, link, null, includeAll, false, authHeaders); + } + public ResultList getTestCases( Integer limit, String fields, TestSuite testSuite, Boolean includeAll, Map 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 getTestCases( @@ -1165,15 +1144,7 @@ public ResultList getTestCases( Boolean orderByLastExecutionDate, Map 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 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 forwardPage; + ResultList 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()