Skip to content

Commit

Permalink
fix: Fixing the previous page pagination for Firestore (appsmithorg#7374
Browse files Browse the repository at this point in the history 
)

The problem was that the pagination was designed & tested with only 2 pages. Firestore has a different function `limitToLast` which solves for previous page pagination commands.
  • Loading branch information
@mohanarpit
mohanarpit committed Sep 13, 2021
1 parent 20d2eae commit c3acf9e
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 82 deletions.
7 changes: 6 additions & 1 deletion .../appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,12 @@ private Mono<ActionExecutionResult> methodGetCollection(CollectionReference quer
return Mono.just(query1);
})
// Apply limit, always provided, since without it we can inadvertently end up processing too much data.
.map(query1 -> query1.limit(limit))
.map(query1 -> {
if (PaginationField.PREV.equals(paginationField) && !CollectionUtils.isEmpty(endBefore)) {
return query1.limitToLast(limit);
}
return query1.limit(limit);
})
// Run the Firestore query to get a Future of the results.
.map(Query::get)
// Consume the future to get the actual results.
Expand Down
150 changes: 69 additions & 81 deletions ...smith-plugins/firestorePlugin/src/test/java/com/external/plugins/FirestorePluginTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import org.testcontainers.utility.DockerImageName;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import reactor.util.function.Tuple3;
import reactor.util.function.Tuple4;

import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
Expand Down Expand Up @@ -445,92 +445,73 @@ public void testAddToCollection() {
.verifyComplete();
}

@Test
public void testPagination() {
final ActionConfiguration actionConfiguration = new ActionConfiguration();
actionConfiguration.setPath("pagination");
actionConfiguration.setPluginSpecifiedTemplates(List.of(
new Property("method", "GET_COLLECTION"),
new Property("order", "[\"n\"]"),
new Property("limit", "5")
));

private ActionConfiguration constructActionConfiguration(Map<String, Object> first, Map<String, Object> last) {
final ObjectMapper objectMapper = new ObjectMapper();
ActionConfiguration actionConfiguration = new ActionConfiguration();
actionConfiguration.setPath("pagination");
List<Property> propertyList = new ArrayList<>();
propertyList.add(new Property("method", "GET_COLLECTION"));
propertyList.add(new Property("order", "[\"n\"]"));
propertyList.add(new Property("limit", "5"));

if (first != null && last != null) {
try {
propertyList.add(new Property());
propertyList.add(new Property());
propertyList.add(new Property());
propertyList.add(new Property("startAfter", objectMapper.writeValueAsString(last)));
propertyList.add(new Property("endBefore", objectMapper.writeValueAsString(first)));
} catch (JsonProcessingException e) {
e.printStackTrace();
}
}
actionConfiguration.setPluginSpecifiedTemplates(propertyList);
return actionConfiguration;
}

Mono<Tuple3<ActionExecutionResult, ActionExecutionResult, ActionExecutionResult>> pagesMono = pluginExecutor
.executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration)
.flatMap(result -> {
List<Map<String, Object>> results = (List) result.getBody();
final Map<String, Object> first = results.get(0);
final Map<String, Object> last = results.get(results.size() - 1);
private Mono<ActionExecutionResult> getNextOrPrevPage(ActionExecutionResult currentPage, PaginationField paginationField) {
List<Map<String, Object>> results = (List) currentPage.getBody();
final Map<String, Object> first = results.get(0);
final Map<String, Object> last = results.get(results.size() - 1);

final ExecuteActionDTO execDetails = new ExecuteActionDTO();
execDetails.setPaginationField(PaginationField.NEXT);
final ExecuteActionDTO execDetails = new ExecuteActionDTO();
execDetails.setPaginationField(paginationField);

final ActionConfiguration actionConfiguration1 = new ActionConfiguration();
actionConfiguration1.setPath(actionConfiguration.getPath());
try {
actionConfiguration1.setPluginSpecifiedTemplates(List.of(
new Property("method", "GET_COLLECTION"),
new Property("order", "[\"n\"]"),
new Property("limit", "5"),
new Property(),
new Property(),
new Property(),
new Property("startAfter", objectMapper.writeValueAsString(last)),
new Property("endBefore", objectMapper.writeValueAsString(first))
));
} catch (JsonProcessingException e) {
e.printStackTrace();
return Mono.error(e);
}
final ActionConfiguration actionConfiguration1 = constructActionConfiguration(first, last);
return pluginExecutor.executeParameterized(firestoreConnection, execDetails, dsConfig, actionConfiguration1);
}

return Mono.zip(
Mono.just(result),
pluginExecutor.executeParameterized(firestoreConnection, execDetails, dsConfig, actionConfiguration1)
);
})
.flatMap(twoPagesMono -> {
final ActionExecutionResult page1 = twoPagesMono.getT1();
final ActionExecutionResult page2 = twoPagesMono.getT2();
@Test
public void testPagination() {
final ActionConfiguration actionConfiguration = constructActionConfiguration(null, null);
// Fetch data for page 1
Mono<ActionExecutionResult> page1Mono = pluginExecutor
.executeParameterized(firestoreConnection, null, dsConfig, actionConfiguration)
.cache();

List<Map<String, Object>> results = (List) page2.getBody();
final Map<String, Object> first = results.get(0);
final Map<String, Object> last = results.get(results.size() - 1);
// Fetch data for page 2 by clicking on the next button
Mono<ActionExecutionResult> page2Mono = page1Mono
.flatMap(page1 -> getNextOrPrevPage(page1, PaginationField.NEXT))
.cache();

final ExecuteActionDTO execDetails = new ExecuteActionDTO();
execDetails.setPaginationField(PaginationField.PREV);
// Fetch data for page 3 by clicking on the next button
Mono<ActionExecutionResult> page3Mono = page2Mono
.flatMap(page2 -> getNextOrPrevPage(page2, PaginationField.NEXT))
.cache();

final ActionConfiguration actionConfiguration1 = new ActionConfiguration();
actionConfiguration1.setPath(actionConfiguration.getPath());
try {
actionConfiguration1.setPluginSpecifiedTemplates(List.of(
new Property("method", "GET_COLLECTION"),
new Property("order", "[\"n\"]"),
new Property("limit", "5"),
new Property(),
new Property(),
new Property(),
new Property("startAfter", objectMapper.writeValueAsString(last)),
new Property("endBefore", objectMapper.writeValueAsString(first))
));
} catch (JsonProcessingException e) {
e.printStackTrace();
return Mono.error(e);
}
// Fetch data for page 2 by clicking on the previous button
Mono<ActionExecutionResult> prevPage2Mono = page3Mono
.flatMap(page3 -> getNextOrPrevPage(page3, PaginationField.PREV))
.cache();

return Mono.zip(
Mono.just(page1),
Mono.just(page2),
pluginExecutor.executeParameterized(firestoreConnection, execDetails, dsConfig, actionConfiguration1)
);
});
var pagesMono = Mono.zip(page1Mono, page2Mono, page3Mono, prevPage2Mono);

StepVerifier.create(pagesMono)
.assertNext(resultPages -> {
final ActionExecutionResult firstPageResult = resultPages.getT1();
final ActionExecutionResult secondPageResult = resultPages.getT2();
final ActionExecutionResult firstPageResultAgain = resultPages.getT3();
final ActionExecutionResult thirdPageResult = resultPages.getT3();
final ActionExecutionResult secondPageResultAgain = resultPages.getT4();

assertTrue(firstPageResult.getIsExecutionSuccess());

Expand All @@ -548,13 +529,20 @@ public void testPagination() {
secondResults.stream().map(m -> m.get("n").toString()).collect(Collectors.toList()).toString()
);

List<Map<String, Object>> firstResultsAgain = (List) firstPageResultAgain.getBody();
List<Map<String, Object>> firstResultsAgain = (List) thirdPageResult.getBody();
assertEquals(5, firstResultsAgain.size());
assertEquals(
"[1, 2, 3, 4, 5]",
"[11, 12, 13, 14, 15]",
firstResultsAgain.stream().map(m -> m.get("n").toString()).collect(Collectors.toList()).toString()
);

List<Map<String, Object>> secondResultsAgain = (List) secondPageResultAgain.getBody();
assertEquals(5, secondResultsAgain.size());
assertEquals(
"[6, 7, 8, 9, 10]",
secondResultsAgain.stream().map(m -> m.get("n").toString()).collect(Collectors.toList()).toString()
);

})
.verifyComplete();
}
Expand All @@ -580,7 +568,7 @@ public void testDatasourceCreateErrorOnBadServiceAccountCredentials() {
error.getMessage());

// Check that the error does not get logged externally.
assertFalse(AppsmithErrorAction.LOG_EXTERNALLY.equals(((AppsmithPluginException)error).getError().getErrorAction()));
assertFalse(AppsmithErrorAction.LOG_EXTERNALLY.equals(((AppsmithPluginException) error).getError().getErrorAction()));
})
.verify();
}
Expand Down Expand Up @@ -710,7 +698,7 @@ public void testWhereConditional() {
* - get all documents where category == test.
* - this returns 2 documents.
*/
((List)whereProperty.getValue()).add(new HashMap<String, Object>() {{
((List) whereProperty.getValue()).add(new HashMap<String, Object>() {{
put("path", "{{Input1.text}}");
put("operator", "EQ");
put("value", "{{Input2.text}}");
Expand All @@ -720,7 +708,7 @@ public void testWhereConditional() {
* - get all documents where name == two.
* - Of the two documents returned by above condition, this will narrow it down to one.
*/
((List)whereProperty.getValue()).add(new HashMap<String, Object>() {{
((List) whereProperty.getValue()).add(new HashMap<String, Object>() {{
put("path", "{{Input3.text}}");
put("operator", "EQ");
put("value", "{{Input4.text}}");
Expand Down Expand Up @@ -1048,7 +1036,7 @@ public void testDynamicBindingSubstitutionInActionConfiguration() {
* - get all documents where category == test.
* - this returns 2 documents.
*/
((List)whereProperty.getValue()).add(new HashMap<String, Object>() {{
((List) whereProperty.getValue()).add(new HashMap<String, Object>() {{
put("path", "{{Input2.text}}");
put("operator", "EQ");
put("value", "{{Input3.text}}");
Expand Down Expand Up @@ -1080,10 +1068,10 @@ public void testDynamicBindingSubstitutionInActionConfiguration() {
// check if dynamic binding values have been substituted correctly
assertEquals("initial", actionConfiguration.getPath());
assertEquals("category",
((Map)((List)actionConfiguration.getPluginSpecifiedTemplates().get(3).getValue()).get(0)).get(
((Map) ((List) actionConfiguration.getPluginSpecifiedTemplates().get(3).getValue()).get(0)).get(
"path"));
assertEquals("test",
((Map)((List)actionConfiguration.getPluginSpecifiedTemplates().get(3).getValue()).get(0)).get(
((Map) ((List) actionConfiguration.getPluginSpecifiedTemplates().get(3).getValue()).get(0)).get(
"value"));
}
}

0 comments on commit c3acf9e

Please sign in to comment.