Skip to content

Commit c241f30

Browse files
CarsonFCarsonF
authored
Refactor & polish search service code (#3360)
1 parent b3d919e commit c241f30

File tree

3 files changed

+101
-54
lines changed

3 files changed

+101
-54
lines changed

‎src/components/search/dto/search-results.dto.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createUnionType } from '@nestjs/graphql';
2-
import { entries, simpleSwitch } from '@seedcompany/common';
2+
import { entries, setOf, simpleSwitch } from '@seedcompany/common';
33
import { uniq } from 'lodash';
44
import { EnumType, makeEnum } from '~/common';
55
import { ResourceMap } from '~/core';
@@ -88,7 +88,9 @@ export type SearchableMap = {
8888
>;
8989
};
9090

91-
export const SearchResultTypes = entries(publicSearchable).map(([k]) => k);
91+
export const SearchResultTypes = setOf(
92+
entries(publicSearchable).map(([k]) => k),
93+
);
9294

9395
// __typename is a GQL thing to identify type at runtime
9496
// It makes sense to use this key to not conflict with actual properties and

‎src/components/search/search.repository.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
import { Injectable } from '@nestjs/common';
22
import { node, relation } from 'cypher-query-builder';
3+
import { Merge } from 'type-fest';
34
import { CommonRepository, OnIndex, OnIndexParams } from '~/core/database';
45
import {
56
ACTIVE,
67
escapeLuceneSyntax,
78
FullTextIndex,
89
} from '~/core/database/query';
910
import { BaseNode } from '~/core/database/results';
11+
import type { ResourceMap } from '~/core/resources';
1012
import { SearchInput } from './dto';
1113

1214
@Injectable()
@@ -20,7 +22,7 @@ export class SearchRepository extends CommonRepository {
2022
* Search for nodes based on input, only returning their id and "type"
2123
* which is based on their first valid search label.
2224
*/
23-
async search(input: SearchInput) {
25+
async search(input: Merge<SearchInput, { type: Array<keyof ResourceMap> }>) {
2426
const escaped = escapeLuceneSyntax(input.query);
2527
// Emphasize exact matches but allow fuzzy as well
2628
const lucene = `"${escaped}"^2 ${escaped}*`;

‎src/components/search/search.service.ts

+94-51
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
import { Injectable } from '@nestjs/common';
2-
import { isNotFalsy } from '@seedcompany/common';
2+
import { isNotNil, setHas, setOf } from '@seedcompany/common';
3+
import { uniqBy } from 'lodash';
4+
import { ValueOf } from 'type-fest';
35
import { ID, NotFoundException, ServerException, Session } from '~/common';
4-
import { ResourceResolver, ResourcesHost } from '~/core';
6+
import { ResourceMap, ResourceResolver, ResourcesHost } from '~/core/resources';
57
import { Privileges } from '../authorization';
68
import { PartnerService } from '../partner';
79
import {
810
SearchableMap,
911
SearchInput,
1012
SearchOutput,
1113
SearchResult,
12-
SearchResultMap,
1314
SearchResultTypes,
1415
} from './dto';
1516
import { SearchRepository } from './search.repository';
@@ -19,12 +20,20 @@ type HydratorMap = {
1920
};
2021
type Hydrator<R> = (id: ID, session: Session) => Promise<R>;
2122

23+
type Match<Types extends string> = ValueOf<{
24+
[Type in Types]: {
25+
type: Type;
26+
id: ID;
27+
matchedProps: readonly string[];
28+
};
29+
}>;
30+
2231
@Injectable()
2332
export class SearchService {
2433
// mapping of base nodes to functions that,
2534
// given id & session, will return the object.
2635
/* eslint-disable @typescript-eslint/naming-convention */
27-
private readonly hydrators: HydratorMap = {
36+
private readonly customHydrators: HydratorMap = {
2837
PartnerByOrg: async (...args) => ({
2938
...(await this.partners.readOnePartnerByOrgId(...args)),
3039
__typename: 'Partner',
@@ -33,68 +42,93 @@ export class SearchService {
3342
/* eslint-enable @typescript-eslint/naming-convention */
3443

3544
constructor(
36-
private readonly resourceHost: ResourcesHost,
37-
private readonly resources: ResourceResolver,
45+
private readonly resources: ResourcesHost,
46+
private readonly resourceResolver: ResourceResolver,
3847
private readonly privileges: Privileges,
3948
private readonly partners: PartnerService,
4049
private readonly repo: SearchRepository,
4150
) {}
4251

4352
async search(input: SearchInput, session: Session): Promise<SearchOutput> {
44-
// if type isn't specified default to all types
45-
const inputTypes = input.type || SearchResultTypes;
53+
const types = input.type
54+
? setOf(
55+
// Expand interfaces to their concretes
56+
// This is needed here now, because below we confirm the match results
57+
// are within this `types` filter
58+
// and those results are the concretes not the interfaces.
59+
input.type
60+
.flatMap((type) =>
61+
this.resources.getImplementations(this.resources.enhance(type)),
62+
)
63+
.flatMap((type) =>
64+
setHas(SearchResultTypes, type.name) ? type.name : [],
65+
),
66+
)
67+
: // if a type filter isn't specified default to all types
68+
SearchResultTypes;
4669

47-
const types = [
48-
...inputTypes,
49-
// Add Organization label when searching for Partners we can search for
50-
// Partner by organization name
51-
...(inputTypes.includes('Partner') ? (['Organization'] as const) : []),
52-
];
70+
const resourceTypes = new Set<keyof ResourceMap>(types);
71+
// Include dependency types for types that have identifiers in sub-resources.
72+
types.has('Partner') && resourceTypes.add('Organization');
5373

5474
// Search for nodes based on input, only returning their id and "type"
5575
// which is based on their first valid search label.
56-
const results = await this.repo.search({ ...input, type: types });
76+
const results = await this.repo.search({
77+
...input,
78+
type: [...resourceTypes],
79+
});
5780

58-
const ResourceMap = this.resourceHost.getMap();
59-
60-
// Individually convert each result (id & type) to its search result
61-
// based on this.hydrators
62-
const hydrated = await Promise.all(
81+
const maybeHydrated = await Promise.all(
6382
results
64-
.map(({ node, matchedProps }) => ({
65-
type: this.resources.resolveTypeByBaseNode(
66-
node,
67-
) as keyof SearchableMap,
68-
id: node.properties.id,
69-
matchedProps,
70-
}))
71-
// Map Org results to Org & Partner results based on types asked for
72-
.flatMap((result) =>
73-
result.type !== 'Organization'
74-
? result
75-
: [
76-
...(inputTypes.includes('Organization') ? [result] : []),
77-
// If matched Organization, include Partner implicitly
78-
...(inputTypes.includes('Partner')
79-
? [
80-
{
81-
id: result.id, // hydrator knows this is an org id not partner
82-
type: 'PartnerByOrg' as const,
83-
matchedProps: ['organization' as const],
84-
},
85-
]
86-
: []),
87-
],
88-
)
83+
// Normalize result & resolve type from neo4j data
84+
.map(({ node, matchedProps }) => {
85+
const result = {
86+
type: this.resourceResolver.resolveTypeByBaseNode(node),
87+
id: node.properties.id,
88+
matchedProps,
89+
};
90+
return result as Match<keyof ResourceMap>;
91+
})
92+
// Ensure resource types matched are within the search type filters
93+
// and handle special cases.
94+
.flatMap<Match<keyof SearchableMap>>((result) => {
95+
if (result.type === 'Organization') {
96+
return [
97+
...(types.has('Organization') ? [result] : []),
98+
...(types.has('Partner')
99+
? [
100+
{
101+
type: 'PartnerByOrg',
102+
id: result.id,
103+
matchedProps: ['organization'],
104+
} as const,
105+
]
106+
: []),
107+
];
108+
}
109+
110+
// This is a sanity/type check.
111+
// Functionally, we shouldn't have any results at this point that
112+
// aren't within the `types` filter.
113+
// However, this does require that the logic above is in sync with
114+
// the resources & type filters.
115+
return setHas(types, result.type)
116+
? (result as Extract<typeof result, { type: keyof SearchableMap }>)
117+
: [];
118+
})
119+
// Do hydration data loading for each identified resource.
89120
.map(
90-
async ({ id, matchedProps, type }): Promise<SearchResult | null> => {
121+
async ({ type, id, matchedProps }): Promise<SearchResult | null> => {
91122
const hydrator = this.hydrate(type);
92123
const hydrated = await hydrator(id, session);
93-
if (!hydrated || !(hydrated.__typename in ResourceMap)) {
124+
if (
125+
!hydrated ||
126+
!(hydrated.__typename in this.resources.getEnhancedMap())
127+
) {
94128
return null;
95129
}
96130

97-
const resource = this.resourceHost.getByName(hydrated.__typename);
131+
const resource = this.resources.getByName(hydrated.__typename);
98132
const perms = this.privileges.for(session, resource, hydrated).all;
99133
return matchedProps.some((key) =>
100134
// @ts-expect-error strict typing is hard for this dynamic use case.
@@ -105,21 +139,30 @@ export class SearchService {
105139
},
106140
),
107141
);
142+
const hydrated = maybeHydrated.filter(isNotNil);
143+
144+
// It is possible that to have two different matches that end up resolving
145+
// to the same resource, so they need to be de-duped.
146+
// For example, a language name and ethnologue code are both matched,
147+
// but in hydrating we convert the ethnologue language to a regular language.
148+
// Only at this point can we check for this convergence.
149+
const items = uniqBy(hydrated, (result) => result.id);
108150

109151
return {
110-
items: hydrated.filter(isNotFalsy).slice(0, input.count),
152+
items: items.slice(0, input.count),
111153
};
112154
}
113155

114156
private hydrate<K extends keyof SearchableMap>(type: K) {
115157
return async (
116158
...args: Parameters<Hydrator<any>>
117159
): Promise<SearchResult | null> => {
118-
const hydrator = this.hydrators[type] as Hydrator<SearchResultMap[K]>;
160+
const hydrator =
161+
type in this.customHydrators ? this.customHydrators[type] : undefined;
119162
try {
120163
const obj = hydrator
121164
? await hydrator(...args)
122-
: await this.resources.lookup(type, ...args);
165+
: await this.resourceResolver.lookup(type, ...args);
123166
return obj as SearchResult;
124167
} catch (err) {
125168
if (err instanceof NotFoundException) return null;

0 commit comments

Comments
 (0)