Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Commit

Permalink
fix: Show true size for ES results (#76)
Browse files Browse the repository at this point in the history
* fix: Show true size for ES results

* chore: update the error case
  • Loading branch information
rsmayda authored Jun 4, 2021
1 parent d06bb43 commit 09300b3
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 0 deletions.
40 changes: 40 additions & 0 deletions src/__snapshots__/elasticSearchService.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -161,6 +162,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -189,6 +191,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -272,6 +275,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -332,6 +336,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -360,6 +365,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -449,6 +455,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -624,6 +631,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -684,6 +692,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -712,6 +721,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -772,6 +782,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -832,6 +843,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -915,6 +927,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1004,6 +1017,7 @@ Array [
"from": 0,
"index": "medicationrequest",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1065,6 +1079,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1102,6 +1117,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand All @@ -1128,6 +1144,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1156,6 +1173,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1184,6 +1202,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1212,6 +1231,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1240,6 +1260,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1272,6 +1293,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand All @@ -1298,6 +1320,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1340,6 +1363,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1398,6 +1422,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1459,6 +1484,7 @@ Array [
"from": 2,
"index": "patient",
"size": 10,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1499,6 +1525,7 @@ Array [
"from": 2,
"index": "patient",
"size": 10,
"track_total_hits": true,
},
],
]
Expand All @@ -1525,6 +1552,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1564,6 +1592,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1639,6 +1668,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1706,6 +1736,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1755,6 +1786,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1814,6 +1846,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand All @@ -1840,6 +1873,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1895,6 +1929,7 @@ Array [
"from": 2,
"index": "patient",
"size": 10,
"track_total_hits": true,
},
],
]
Expand All @@ -1915,6 +1950,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -1967,6 +2003,7 @@ Array [
"from": 0,
"index": "library",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -2022,6 +2059,7 @@ Array [
"from": 0,
"index": "patient",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -2074,6 +2112,7 @@ Array [
"from": 0,
"index": "person",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down Expand Up @@ -2227,6 +2266,7 @@ Array [
"from": 0,
"index": "observation",
"size": 20,
"track_total_hits": true,
},
],
]
Expand Down
2 changes: 2 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@ export const NON_SEARCHABLE_PARAMETERS = [
'_revinclude',
...ITERATIVE_INCLUSION_PARAMETERS,
];

export const MAX_ES_WINDOW_SIZE: number = 10000;
12 changes: 12 additions & 0 deletions src/elasticSearchService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,18 @@ describe('typeSearch', () => {
).rejects.toThrowError(InvalidSearchParameterError);
});

test('Invalid search range (> 10k)', () => {
const es = new ElasticSearchService(FILTER_RULES_FOR_ACTIVE_RESOURCES);
expect(
es.typeSearch({
resourceType: 'Patient',
baseUrl: 'https://base-url.com',
queryParams: { _count: '20', _getpagesoffset: '10000' },
allowedResourceTypes: ALLOWED_RESOURCE_TYPES,
}),
).rejects.toThrowError(InvalidSearchParameterError);
});

test('Response format', async () => {
const patientSearchResult = {
body: {
Expand Down
14 changes: 14 additions & 0 deletions src/elasticSearchService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
SearchEntry,
SearchFilter,
FhirVersion,
InvalidSearchParameterError,
} from 'fhir-works-on-aws-interface';
import { Client } from '@elastic/elasticsearch';
import { ElasticSearch } from './elasticSearch';
Expand All @@ -25,6 +26,7 @@ import {
SEARCH_PAGINATION_PARAMS,
ITERATIVE_INCLUSION_PARAMETERS,
SORT_PARAMETER,
MAX_ES_WINDOW_SIZE,
} from './constants';
import { buildIncludeQueries, buildRevIncludeQueries } from './searchInclusions';
import { FHIRSearchParametersRegistry } from './FHIRSearchParametersRegistry';
Expand Down Expand Up @@ -92,6 +94,16 @@ export class ElasticSearchService implements Search {
? Number(queryParams[SEARCH_PAGINATION_PARAMS.COUNT])
: DEFAULT_SEARCH_RESULTS_PER_PAGE;

if (from + size > MAX_ES_WINDOW_SIZE) {
logger.info(
`Search request is out of bound. Trying to access ${from} to ${from +
size} which is outside of the max: ${MAX_ES_WINDOW_SIZE}`,
);
throw new InvalidSearchParameterError(
`Search parameters: ${SEARCH_PAGINATION_PARAMS.PAGES_OFFSET} and ${SEARCH_PAGINATION_PARAMS.COUNT} are accessing items outside the max range (${MAX_ES_WINDOW_SIZE}). Please narrow your search to access the remaining items`,
);
}

const filter: any[] = ElasticSearchService.buildElasticSearchFilter([
...this.searchFiltersForAllQueries,
...(searchFilters ?? []),
Expand All @@ -102,6 +114,7 @@ export class ElasticSearchService implements Search {
index: resourceType.toLowerCase(),
from,
size,
track_total_hits: true,
body: {
query,
},
Expand Down Expand Up @@ -175,6 +188,7 @@ export class ElasticSearchService implements Search {
hits: [],
};
}

throw error;
}
}
Expand Down

0 comments on commit 09300b3

Please sign in to comment.