Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: firestore pagination issue (#32912)

Browse Source 
## Description

As per the documentation the where clause must always come before the
order by clause, and the startAfter clause must come after the order by
clause i.e the recommended order : `where clause --> order by --> start
after`. However, the way code was written it applies the where clause
after the startAfter i.e as per the code : `order by --> start after -->
where clause` .

In this PR, we change the logic to make sure we follow the order `where
clause -> order by and startAfter / endBefore` when generating query.

Fixes https://github.com/appsmithorg/appsmith/issues/32604

documentation link. -
https://firebase.google.com/docs/firestore/query-data/query-cursors

Changes were taken from the PR created by @sumitsum here
https://github.com/appsmithorg/appsmith-ee/pull/4001




## Automation

/ok-to-test tags="@tag.Datasource"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/8814737910>
> Commit: 4c7ed97cc1808a537d40682fc0e930cd0e30be70
> Cypress dashboard url: <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=8814737910&attempt=1"
target="_blank">Click here!</a>

<!-- end of auto-generated comment: Cypress test results  -->




## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Refactor**
- Improved the order of operations in Firestore queries to enhance query
performance and reliability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rishabh Rathod 2024-04-29 11:23:34 +05:30 committed by GitHub
parent 1330485f4f
commit 45172ac8ae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 26 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
app/server/appsmith-plugins/firestorePlugin/src/main/java/com/external/plugins/FirestorePlugin.java vendored
View File

@ -683,32 +683,9 @@ public class FirestorePlugin extends BasePlugin {
}
return Mono.just(query)
// Apply ordering, if provided.
.map(query1 -> {
Query q = query1;
final List<Object> startAfterValues = new ArrayList<>();
final List<Object> endBeforeValues = new ArrayList<>();
for (final String field : orderings) {
q = q.orderBy(
field.replaceAll("^-", ""),
field.startsWith("-") ? Query.Direction.DESCENDING : Query.Direction.ASCENDING);
if (startAfter != null) {
startAfterValues.add(startAfter.get(field));
}
if (endBefore != null) {
endBeforeValues.add(endBefore.get(field));
}
}
if (PaginationField.NEXT.equals(paginationField) && !CollectionUtils.isEmpty(startAfter)) {
q = q.startAfter(startAfterValues.toArray());
} else if (PaginationField.PREV.equals(paginationField)
&& !CollectionUtils.isEmpty(endBefore)) {
q = q.endBefore(endBeforeValues.toArray());
}
return q;
})
// Apply where condition, if provided.
.flatMap(query1 -> {
.flatMap(q -> {
Query query1 = q;
if (!isWhereMethodUsed(formData)) {
return Mono.just(query1);
}
@ -742,6 +719,30 @@ public class FirestorePlugin extends BasePlugin {
return Mono.just(query1);
})
// Apply ordering, if provided.
.map(query1 -> {
Query q = query1;
final List<Object> startAfterValues = new ArrayList<>();
final List<Object> endBeforeValues = new ArrayList<>();
for (final String field : orderings) {
q = q.orderBy(
field.replaceAll("^-", ""),
field.startsWith("-") ? Query.Direction.DESCENDING : Query.Direction.ASCENDING);
if (startAfter != null) {
startAfterValues.add(startAfter.get(field));
}
if (endBefore != null) {
endBeforeValues.add(endBefore.get(field));
}
}
if (PaginationField.NEXT.equals(paginationField) && !CollectionUtils.isEmpty(startAfter)) {
q = q.startAfter(startAfterValues.toArray());
} else if (PaginationField.PREV.equals(paginationField)
&& !CollectionUtils.isEmpty(endBefore)) {
q = q.endBefore(endBeforeValues.toArray());
}
return q;
})
// Apply limit, always provided, since without it, we can inadvertently end up processing too much
// data.
.map(query1 -> {