Skip to content

ConnectWise PSA - efficient pagination #18623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 2, 2025
Merged

ConnectWise PSA - efficient pagination #18623

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc186bf
efficient pagination
andrewjschuang Oct 2, 2025
d98247b
bump versions
andrewjschuang Oct 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions components/connectwise_psa/connectwise_psa.app.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ export default {
resourceFn,
params,
max,
lastId,
}) {
params = {
...params,
Expand All @@ -350,12 +351,19 @@ export default {
params,
});
for (const item of items) {
yield item;
count++;
if (max && count >= max) {
return;
}

// orderby id desc
if (item.id <= lastId) {
return;
}

yield item;
count++;
}

hasMore = items.length;
params.page++;
} while (hasMore);
Expand Down
34 changes: 21 additions & 13 deletions components/connectwise_psa/sources/common/base.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ export default {
this.db.set("lastId", lastId);
},
getParams() {
return {};
return {
pageSize: 500,
orderBy: "id desc",
};
},
generateMeta(item) {
return {
Expand All @@ -38,25 +41,30 @@ export default {
const lastId = this._getLastId();
const resourceFn = this.getResourceFn();
const params = this.getParams();
const results = this.connectwise.paginate({
const iterator = this.connectwise.paginate({
resourceFn,
params,
max,
lastId,
});
let items = [];
for await (const item of results) {
if (item.id > lastId) {
items.push(item);

const items = [];
let firstNewId;

for await (const item of iterator) {
if (!firstNewId) {
firstNewId = item.id;
}
items.push(item);
}
Comment on lines +51 to 59
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Consider memory usage when max is undefined.

The code collects all items in memory before emitting them in reverse order. When max is not provided (e.g., in the run method), this could consume significant memory for large result sets. The paginate function has a lastId early-termination mechanism, but for sources with many new items, the in-memory collection could grow large.

Consider one of the following:

  1. Add a reasonable default max value in the run method to cap memory usage.
  2. Document the memory implications for integrators who extend this base class.
  3. Stream items without collecting them if reverse order is not strictly necessary (though this would require design changes).

If the current behavior is intentional and memory usage is not a concern for typical use cases, consider adding a comment explaining the trade-off.

🤖 Prompt for AI Agents 
In components/connectwise_psa/sources/common/base.mjs around lines 51-59 the
code collects all iterator items into memory before reversing and emitting them,
which can cause unbounded memory growth when max is undefined; update the
run/paginate usage to enforce a sensible default cap (e.g., default max = 1000)
when callers don't provide max, or if changing callers is undesirable, add an
explicit comment in this file explaining the memory trade-off and that callers
must supply max for large result sets; implement the default by checking for
undefined max at the entry point and limiting iteration or early-terminating
when the cap is reached, and document the behavior in the method docstring.

if (!items.length) {
return;
}
if (max) {
items = items.slice(-1 * max);

if (firstNewId) {
this._setLastId(firstNewId);
}
this._setLastId(items[items.length - 1].id);
items.forEach((item) => this.$emit(item, this.generateMeta(item)));

items
.reverse()
.forEach((item) => this.$emit(item, this.generateMeta(item)));
},
getResourceFn() {
throw new Error("getResourceFn is not implemented");
Expand Down
Loading