Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 32 additions & 1 deletion src/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,36 @@
const parseLinkHeader = require('parse-link-header');
const { RequestContext, ResponseContext } = require('./generated/http/http');

function splitUri(uri) {
const idx = uri.indexOf('?');
if (idx < 0) {
return { base: uri, query: '' };
}
return { base: uri.slice(0, idx), query: uri.slice(idx + 1) };
}

// Merge query params from the original request URI into the paginated `next`
// URI, giving the `next` URI precedence. The Okta API does not always echo
// request-shaping params (e.g. `expand`) in the Link header's `next` URL (#439),
// which would otherwise drop them across pages.
function mergePreservedQueryParams(originalQuery, nextUri) {
if (!originalQuery) {
return nextUri;
}
const { base, query: nextQuery } = splitUri(nextUri);
const originalParams = new URLSearchParams(originalQuery);
const nextParams = new URLSearchParams(nextQuery);
for (const key of new Set(originalParams.keys())) {
if (!nextParams.has(key)) {
for (const value of originalParams.getAll(key)) {
nextParams.append(key, value);
}
}
}
const merged = nextParams.toString();
return merged ? `${base}?${merged}` : base;
}

/**
* Provides an interface to iterate over all objects in a collection that has pagination via Link headers
*/
Expand All @@ -28,6 +58,7 @@ class Collection {
*/
constructor(httpApi, uri, factory, request) {
this.nextUri = uri;
this.initialQuery = splitUri(uri).query;
this.httpApi = httpApi;
this.factory = factory;
this.currentItems = [];
Expand Down Expand Up @@ -93,7 +124,7 @@ class Collection {
if (link) {
const parsed = parseLinkHeader(link);
if (parsed.next) {
this.nextUri = parsed.next.url;
this.nextUri = mergePreservedQueryParams(this.initialQuery, parsed.next.url);
return res instanceof ResponseContext ? this.factory.parseResponse(res) : res.json();
}
}
Expand Down
77 changes: 77 additions & 0 deletions test/unit/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,83 @@ describe('Collection', () => {
const collection = new Collection(mockClient, '/', mockFactory, mockRequest);
await collection.each(noop);
});

it('should preserve original query params (e.g. expand) across paginated requests when the next Link header omits them', async () => {
// Regression test for #439: Okta's Link header does not echo `expand`
// in the next URL, so without preservation the second page drops it.
const requestedUris = [];
const mockClient = {
http: (uri) => {
requestedUris.push(uri);
return Promise.resolve({
headers: {
get: () => {
if (uri.startsWith('/api/v1/apps/app1/users?') && uri.includes('expand=user') && !uri.includes('after=')) {
// First-page Link header — note: no expand param in next URL
return '</api/v1/apps/app1/users?limit=5&after=cursor1>; rel="next"';
}
},
},
json: () => {
if (!uri.includes('after=')) {
return Promise.resolve([{ id: 'u1' }]);
}
if (uri.includes('after=cursor1')) {
return Promise.resolve([{ id: 'u2' }]);
}
}
});
}
};
const mockFactory = {
createInstance: (item) => item
};
const collection = new Collection(
mockClient,
'/api/v1/apps/app1/users?limit=5&expand=user',
mockFactory
);
const collected = [];
await collection.each((item) => {
collected.push(item);
});
expect(collected).to.deep.equal([{ id: 'u1' }, { id: 'u2' }]);
expect(requestedUris).to.have.lengthOf(2);
expect(requestedUris[0]).to.equal('/api/v1/apps/app1/users?limit=5&expand=user');
expect(requestedUris[1]).to.include('after=cursor1');
expect(requestedUris[1]).to.include('expand=user');
});

it('should let the next Link header override overlapping query params (e.g. after/cursor)', async () => {
const requestedUris = [];
const mockClient = {
http: (uri) => {
requestedUris.push(uri);
return Promise.resolve({
headers: {
get: () => {
if (!uri.includes('after=cursor2')) {
return '</api/v1/users?limit=5&after=cursor2>; rel="next"';
}
},
},
json: () => {
return Promise.resolve([{}]);
}
});
}
};
const mockFactory = { createInstance: (item) => item };
const collection = new Collection(
mockClient,
'/api/v1/users?limit=5&after=cursor1&expand=user',
mockFactory
);
await collection.each(() => {});
expect(requestedUris[1]).to.include('after=cursor2');
expect(requestedUris[1]).to.not.include('after=cursor1');
expect(requestedUris[1]).to.include('expand=user');
});
});

describe('.subscribe()', () => {
Expand Down