-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
FTN-58: Search by indicator id #29
base: main
Are you sure you want to change the base?
Changes from 1 commit
f2a1f6b
0fafeb1
ad1b4c5
1e6f0ca
51eb436
ef0a43a
466e45d
79bebaf
7533064
dbf291a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
import { SearchService } from './searchService'; | ||
|
||
export interface IndicatorSearchResult { | ||
id: string; | ||
indicatorName: string; | ||
latestDataPeriod?: string; | ||
dataSource?: string; | ||
lastUpdated?: string; | ||
} | ||
|
||
export interface IndicatorSearch { | ||
searchByIndicator(indicator: string): Promise<IndicatorSearchResult[]>; | ||
} | ||
|
||
export const MOCK_DATA: IndicatorSearchResult[] = [ | ||
{ | ||
id: '1', | ||
indicatorName: 'NHS', | ||
latestDataPeriod: '2023', | ||
dataSource: 'NHS website', | ||
lastUpdated: formatDate(new Date('December 6, 2024')), | ||
}, | ||
{ | ||
id: '2', | ||
indicatorName: 'DHSC', | ||
latestDataPeriod: '2022', | ||
dataSource: 'Student article', | ||
lastUpdated: formatDate(new Date('November 5, 2023')), | ||
}, | ||
]; | ||
|
||
function formatDate(date: Date): string { | ||
const day = String(date.getDate()).padStart(2, '0'); | ||
const month = date.toLocaleString('en-GB', { month: 'long' }); | ||
const year = date.getFullYear(); | ||
|
||
return `${day} ${month} ${year}`; | ||
} | ||
|
||
let searchService: SearchService; | ||
try { | ||
searchService = new SearchService(); | ||
} catch { | ||
// Handle error | ||
} | ||
gareth-allan-bjss marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export const getSearchService = (): IndicatorSearch => { | ||
return searchService | ||
? searchService | ||
: { | ||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
searchByIndicator(indicator: string): Promise<IndicatorSearchResult[]> { | ||
return Promise.resolve(MOCK_DATA); | ||
}, | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be cleaner to implement this as a separate implementation of the |
||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import { IndicatorSearch, IndicatorSearchResult } from './searchResultData'; | ||
import { AzureKeyCredential, SearchClient } from '@azure/search-documents'; | ||
|
||
type Indicator = { | ||
IID: string; | ||
Descriptive: { | ||
Name: string; | ||
Definition: string; | ||
DataSource: string; | ||
}; | ||
DataChange: { | ||
LastUploadedAt: string; | ||
}; | ||
}; | ||
|
||
const getEnvironmentVariable = (name: string): string => { | ||
return ( | ||
process.env[name] ?? | ||
(() => { | ||
throw new Error(`Missing environment variable ${name}`); | ||
})() | ||
); | ||
}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might make more sense in its own file, as I expect we'll want to use it from other places. |
||
|
||
export class SearchService implements IndicatorSearch { | ||
readonly searchClient: SearchClient<Indicator>; | ||
|
||
constructor() { | ||
const serviceName = getEnvironmentVariable('DHSC_AI_SEARCH_SERVICE_NAME'); | ||
const indexName = getEnvironmentVariable('DHSC_AI_SEARCH_INDEX_NAME'); | ||
const apiKey = getEnvironmentVariable('DHSC_AI_SEARCH_API_KEY'); | ||
|
||
this.searchClient = new SearchClient<Indicator>( | ||
`https://${serviceName}.search.windows.net`, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I appreciate this was how I did it in the PoC, but I think we're probably better taking the full URL to the search service as an environment variable (e.g. |
||
indexName, | ||
new AzureKeyCredential(apiKey) | ||
); | ||
} | ||
|
||
async searchByIndicator(indicator: string): Promise<IndicatorSearchResult[]> { | ||
const query = `/.*${indicator}.*/`; | ||
|
||
const searchResponse = await this.searchClient.search(query, { | ||
queryType: 'full', | ||
gareth-allan-bjss marked this conversation as resolved.
Show resolved
Hide resolved
|
||
searchFields: ['IID'], | ||
includeTotalCount: true, | ||
}); | ||
|
||
const results: IndicatorSearchResult[] = []; | ||
for await (const result of searchResponse.results) { | ||
results.push({ | ||
id: result?.document?.IID, | ||
indicatorName: result?.document?.Descriptive?.Name, | ||
latestDataPeriod: undefined, | ||
dataSource: result.document?.Descriptive?.DataSource, | ||
lastUpdated: result.document?.DataChange?.LastUploadedAt, | ||
}); | ||
} | ||
|
||
return results; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm understanding this correctly. This will be a singleton of the SearchService instance on the NextJS server to handle all requests. Rather than create a new instance per request. Is this the intended idea here and does this have any draw backs? Just thinking performance, load or concurrency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, although I did think that performing the initial set-up of the search client on each call would be a repetition of effort. My thinking was if Node is single-threaded (think it might be) then creating a new instance each time doesn't help as it's just repeated work, & if it's multi-threaded then there's nothing to stop different threads running the same function.