Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,81 @@ describe('createPointInTimeFinder()', () => {
);
});

test('does not yield empty first page', async () => {
repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'abc123',
});
repository.find.mockResolvedValueOnce({
total: 2,
saved_objects: [],
pit_id: 'abc123',
per_page: 2,
page: 0,
});

const findOptions: SavedObjectsCreatePointInTimeFinderOptions = {
type: ['visualization'],
search: 'foo*',
};

const internalOptions = {};
const finder = new PointInTimeFinder(findOptions, {
logger,
client: repository,
internalOptions,
});

const hits: SavedObjectsFindResult[] = [];
let pageCount = 0;
for await (const result of finder.find()) {
hits.push(...result.saved_objects);
pageCount++;
}

expect(pageCount).toEqual(0);
expect(hits.length).toEqual(0);
});

test('yields empty first page if aggregations are used', async () => {
repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'abc123',
});
repository.find.mockResolvedValueOnce({
total: 2,
saved_objects: [],
pit_id: 'abc123',
per_page: 2,
page: 0,
});

const findOptions: SavedObjectsCreatePointInTimeFinderOptions = {
type: ['visualization'],
search: 'foo*',
aggs: {
some: {
avg: { field: 'fo' },
},
},
};

const internalOptions = {};
const finder = new PointInTimeFinder(findOptions, {
logger,
client: repository,
internalOptions,
});

const hits: SavedObjectsFindResult[] = [];
let pageCount = 0;
for await (const result of finder.find()) {
hits.push(...result.saved_objects);
pageCount++;
}

expect(pageCount).toEqual(1);
expect(hits.length).toEqual(0);
});

test('still applies the defaults in the mandatory fields even when `undefined` is explicitly provided', async () => {
repository.openPointInTimeForType.mockResolvedValueOnce({
id: 'abc123',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,12 @@ export class PointInTimeFinder<T = unknown, A = unknown>
await this.close();
}

yield results;
// do not yield first page if empty, unless there are aggregations
// (in which case we always want to return at least one page)
if (lastResultsCount > 0 || this.#findOptions.aggs) {
yield results;
}
Comment on lines +96 to +100
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Even if this is an edge case, it is possible to specify aggregations with a PIT search, and this is effectively done at least once in our code:

let aggResults: UninstallTokenSOAggregationBucket[] = [];
for await (const result of idFinder.find()) {
if (
!result?.aggregations?.by_policy_id.buckets ||
!Array.isArray(result?.aggregations?.by_policy_id.buckets)
) {
break;
}
aggResults = result.aggregations.by_policy_id.buckets;
break;
}

Note that this specific usage could be adapted (given there isn't any reason to use a PIT search here), but we scenario remains valid more globally, so we still have to check for aggregations and yield, otherwise there's no way for the API consumers to retrieve their aggregation results in case of empty hits.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

cc @elastic/fleet as you may want to adapt this snippet to use a standard SOR.find call instead of a PIT given you break on first page.


// We've reached the end when there are fewer hits than our perPage size,
// or when `close()` has been called.
} while (this.#open && lastResultsCount >= this.#findOptions.perPage!);
Expand Down