Skip to content

Commit 4271ab8

Browse files
ciaranschutteCiaran Schutte
andcommitted
Fed search cleanup typings (#906)
* remove unsued type defs * add type watch script * fix import * fix import * type an ANY type * fix agg resolver typing * ts no-check for test file * fix Agg Accumulator types * type response formatter * fix test file type * address feedback --------- Co-authored-by: Ciaran Schutte <[email protected]>
1 parent c0cf685 commit 4271ab8

File tree

10 files changed

+83
-61
lines changed

10 files changed

+83
-61
lines changed

modules/server/src/network/aggregations/AggregationAccumulator.ts

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { ALL_NETWORK_AGGREGATION_TYPES_MAP } from '..';
2-
import { SUPPORTED_AGGREGATIONS } from '../common';
2+
import { SupportedAggregation, SUPPORTED_AGGREGATIONS } from '../common';
33
import { Aggregations, Bucket, NumericAggregations } from '../types/aggregations';
44
import { Hits } from '../types/hits';
55
import { AllAggregations } from '../types/types';
@@ -14,36 +14,57 @@ type ResolveAggregationInput = {
1414
type AggregationsTuple = [Aggregations, Aggregations];
1515
type NumericAggregationsTuple = [NumericAggregations, NumericAggregations];
1616

17-
const emptyAggregation = (hits: number) => ({
17+
const emptyAggregation = (hits: number): Aggregations => ({
18+
__typename: 'Aggregations',
1819
bucket_count: 1,
1920
buckets: [{ key: '___aggregation_not_available___', doc_count: hits }],
2021
});
2122

2223
// mutation - update a single aggregations field in the accumulator
23-
const addToAccumulator = ({ existingAggregation, aggregation, type }) => {
24+
const addToAccumulator = <T extends Aggregations | NumericAggregations>({
25+
existingAggregation,
26+
aggregation,
27+
type,
28+
}: {
29+
existingAggregation: T | undefined;
30+
aggregation: T;
31+
type: SupportedAggregation;
32+
}) => {
2433
// if first aggregation, nothing to resolve with yet
2534
return !existingAggregation
2635
? aggregation
27-
: resolveToNetworkAggregation(type, [aggregation, existingAggregation]);
36+
: resolveToNetworkAggregation<T>(type, [aggregation, existingAggregation]);
2837
};
2938

3039
/**
3140
* Resolves returned aggregations from network queries into single accumulated aggregation
41+
* ALL_NETWORK_AGGREGATION_TYPES_MAP should be initialised before using this function
3242
*
3343
* @param
3444
*/
3545
const resolveAggregations = ({ data, accumulator, requestedFields }: ResolveAggregationInput) => {
3646
requestedFields.forEach((requestedField) => {
3747
const { aggregations, hits } = data;
3848

39-
const isFieldAvailable = !!aggregations[requestedField];
49+
/*
50+
* requested field will always be in ALL_NETWORK_AGGREGATION_TYPES_MAP
51+
* GQL schema validation will throw an error earlier if a requested field isn't in the schema
52+
*/
4053
const type = ALL_NETWORK_AGGREGATION_TYPES_MAP.get(requestedField);
54+
if (type === undefined) {
55+
console.log(
56+
'Could not find aggregation type.\nPlease ensure ALL_NETWORK_AGGREGATION_TYPES_MAP is initialised.',
57+
);
58+
return;
59+
}
60+
61+
const aggregation = aggregations[requestedField];
4162
const existingAggregation = accumulator[requestedField];
4263

43-
if (isFieldAvailable) {
64+
if (aggregation !== undefined) {
4465
accumulator[requestedField] = addToAccumulator({
4566
existingAggregation,
46-
aggregation: aggregations[requestedField],
67+
aggregation,
4768
type,
4869
});
4970
} else {
@@ -66,9 +87,9 @@ const resolveAggregations = ({ data, accumulator, requestedFields }: ResolveAggr
6687
* @param type
6788
* @param aggregations
6889
*/
69-
const resolveToNetworkAggregation = (
90+
const resolveToNetworkAggregation = <T>(
7091
type: string,
71-
aggregations: AggregationsTuple | NumericAggregationsTuple,
92+
aggregations: [T, T],
7293
): Aggregations | NumericAggregations => {
7394
if (type === SUPPORTED_AGGREGATIONS.Aggregations) {
7495
return resolveAggregation(aggregations as AggregationsTuple);
@@ -179,7 +200,11 @@ export const resolveAggregation = (aggregations: AggregationsTuple): Aggregation
179200
const resolvedAggregation = aggregations.reduce((resolvedAggregation, agg) => {
180201
const computedBuckets = resolvedAggregation.buckets;
181202
agg.buckets.forEach((bucket) => updateComputedBuckets(bucket, computedBuckets));
182-
return { bucket_count: computedBuckets.length, buckets: computedBuckets };
203+
return {
204+
bucket_count: computedBuckets.length,
205+
buckets: computedBuckets,
206+
__typename: resolvedAggregation.__typename,
207+
};
183208
});
184209

185210
return resolvedAggregation;

modules/server/src/network/aggregations/tests/aggregation.test.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
/*
2+
* TODO: needs work to resolve ALL_NETWORK_AGGREGATION_TYPES_MAP correctly mocked
3+
* only works at this top level, once. Jest will hoist
4+
* jest.doMock requires extra configuration with "require" instead of "import"
5+
* doesn't mock correctly right now between tests
6+
*/
7+
8+
// @ts-nocheck
9+
110
import { ALL_NETWORK_AGGREGATION_TYPES_MAP } from '@/network';
211
import { AggregationAccumulator } from '../AggregationAccumulator';
312
import { aggregation as fixture } from './fixture';
@@ -13,12 +22,6 @@ jest.mock('../../index', () => ({
1322
ALL_NETWORK_AGGREGATION_TYPES_MAP: new Map<string, string>([['donors_gender', 'Aggregations']]),
1423
}));
1524

16-
/*
17-
* TODO: needs work to resolve ALL_NETWORK_AGGREGATION_TYPES_MAP correctly mocked
18-
* only works at this top level, once. Jest will hoist
19-
* jest.doMock requires extra configuration with "require" instead of "import"
20-
* doesn't mock correctly right now between tests
21-
*/
2225
xdescribe('Network aggregation resolution', () => {
2326
describe('resolves multiple aggregations into a single aggregation:', () => {
2427
beforeEach(() => {

modules/server/src/network/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { makeExecutableSchema } from '@graphql-tools/schema';
2-
import { SUPPORTED_AGGREGATIONS_LIST } from './common';
2+
import { SupportedAggregation, SUPPORTED_AGGREGATIONS_LIST } from './common';
33
import { createResolvers } from './resolvers';
44
import { getAllFieldTypes } from './setup/fields';
55
import { fetchAllNodeAggregations } from './setup/query';
66
import { createTypeDefs } from './typeDefs';
77
import { NetworkConfig } from './types/setup';
88

9-
export let ALL_NETWORK_AGGREGATION_TYPES_MAP: Map<string, string> = new Map();
9+
export let ALL_NETWORK_AGGREGATION_TYPES_MAP: Map<string, SupportedAggregation> = new Map();
1010

1111
/**
1212
* GQL Federated Search schema setup

modules/server/src/network/resolvers/aggregations.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ type QueryVariables = {
3030
*/
3131
const fetchData = async <SuccessType>(
3232
query: NetworkQuery,
33-
): Promise<Result<SuccessType, typeof CONNECTION_STATUS.error>> => {
33+
): Promise<Result<SuccessType, typeof CONNECTION_STATUS.ERROR>> => {
3434
const { url, gqlQuery, queryVariables } = query;
3535

3636
console.log(`Fetch data starting for ${url}`);
@@ -45,10 +45,12 @@ const fetchData = async <SuccessType>(
4545
// axios response "data" field, graphql response "data" field
4646
const responseData = response.data?.data;
4747
if (response.status === 200 && response.statusText === 'OK') {
48+
console.log(`Fetch data completing for ${query.url}`);
4849
return success(responseData);
4950
}
5051
} catch (error) {
5152
if (axios.isCancel(error)) {
53+
console.log(`Fetch data cancelled for ${query.url}`);
5254
return failure(CONNECTION_STATUS.ERROR, `Request cancelled: ${url}`);
5355
}
5456

@@ -61,9 +63,9 @@ const fetchData = async <SuccessType>(
6163
}
6264
}
6365
return failure(CONNECTION_STATUS.ERROR, `Unknown error`);
64-
} finally {
65-
console.log(`Fetch data completing for ${query.url}`);
6666
}
67+
// TS would like a return value outside of try/catch handling
68+
return failure(CONNECTION_STATUS.ERROR, `Unknown error`);
6769
};
6870

6971
/**
@@ -129,12 +131,12 @@ export const createNodeQueryString = (
129131
*
130132
* @param config
131133
* @param requestedFields
132-
* @returns
134+
* @returns a GQL document node or undefined if a valid GQL document node cannot be created
133135
*/
134136
export const createNetworkQuery = (
135137
config: NodeConfig,
136138
requestedFields: RequestedFieldsMap,
137-
): DocumentNode => {
139+
): DocumentNode | undefined => {
138140
const availableFields = config.aggregations;
139141
const documentName = config.documentName;
140142

@@ -161,6 +163,7 @@ export const createNetworkQuery = (
161163
return gqlQuery;
162164
} catch (err) {
163165
console.error('invalid gql', err);
166+
return undefined;
164167
}
165168
};
166169

@@ -184,11 +187,13 @@ export const aggregationPipeline = async (
184187

185188
const aggregationResultPromises = configs.map(async (config) => {
186189
const gqlQuery = createNetworkQuery(config, requestedAggregationFields);
187-
const response = await fetchData<SuccessResponse>({
188-
url: config.graphqlUrl,
189-
gqlQuery,
190-
queryVariables,
191-
});
190+
const response = gqlQuery
191+
? await fetchData<SuccessResponse>({
192+
url: config.graphqlUrl,
193+
gqlQuery,
194+
queryVariables,
195+
})
196+
: failure(CONNECTION_STATUS.ERROR, 'Invalid GQL query');
192197

193198
const nodeName = config.displayName;
194199

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1+
import { AllAggregations } from '../types/types';
2+
import { NetworkNode } from './networkNode';
3+
14
/**
2-
* Format response object to match gql type defs
5+
* Format response object to match GQL type defs
36
*/
4-
export const createResponse = ({ aggregationResults, nodeInfo }) => {
7+
export const createResponse = ({
8+
aggregationResults,
9+
nodeInfo,
10+
}: {
11+
aggregationResults: AllAggregations;
12+
nodeInfo: NetworkNode[];
13+
}) => {
514
return { remoteConnections: nodeInfo, aggregations: aggregationResults };
615
};

modules/server/src/network/setup/fields.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ export const getFieldTypes = (
5959
export const getAllFieldTypes = (
6060
nodeConfigs: NodeConfig[],
6161
supportedTypes: SupportedAggregation[],
62-
) => {
62+
): SupportedNetworkFieldType[] => {
6363
const nodeFieldTypes = nodeConfigs.map((config) => {
6464
const { supportedAggregations, unsupportedAggregations } = getFieldTypes(
6565
config.aggregations,
@@ -75,7 +75,7 @@ export const getAllFieldTypes = (
7575
/*
7676
* Returns unique fields
7777
* eg. if NodeA and NodeB both have `analysis__analysis__id`, only include it once
78-
* This during server startup for creating the Apollo server.
78+
* This is during server startup for creating the Apollo server.
7979
* Please do not use expensive stringify and parsing for server queries.
8080
*/
8181
const uniqueSupportedAggregations = Array.from(

modules/server/src/network/tests/helpers.test.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,21 @@
1-
import { getFieldTypes } from '..';
1+
import { SupportedAggregation } from '../common';
2+
import { getFieldTypes } from '../setup/fields';
23

34
describe('helpers', () => {
45
test('getField returns both supported and unsupported types', () => {
5-
const supportedAggregations = ['Aggregations'];
6+
const supportedAggregations: SupportedAggregation[] = ['Aggregations'];
67
const fields = [
78
{
89
name: 'analysis__analysis_id',
9-
type: {
10-
name: 'Aggregations',
11-
},
10+
type: 'Aggregations',
1211
},
1312
{
1413
name: 'analysis__analysis_state',
15-
type: {
16-
name: 'Aggregations',
17-
},
14+
type: 'Aggregations',
1815
},
1916
{
2017
name: 'clinical__donor__number_of_children',
21-
type: {
22-
name: 'HumanAggregate',
23-
},
18+
type: 'HumanAggregate',
2419
},
2520
];
2621

modules/server/src/network/typeDefs/aggregations.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import {
77
GraphQLString,
88
} from 'graphql';
99
import GraphQLJSON from 'graphql-type-json';
10-
import { SupportedNetworkFieldType } from '../types';
10+
import { SupportedNetworkFieldType } from '../types/types';
1111
import { singleToNetworkAggregationMap } from './networkAggregations';
1212

1313
/**
@@ -16,7 +16,7 @@ import { singleToNetworkAggregationMap } from './networkAggregations';
1616
* @example
1717
* { name: "donor_age", type: "NumericAggregations" } => { donor_age: { type: "NetworkNumericAggregations" } }
1818
*/
19-
const convertToGQLObjectType = (networkFieldTypes) => {
19+
const convertToGQLObjectType = (networkFieldTypes: SupportedNetworkFieldType[]) => {
2020
return networkFieldTypes.reduce((allFields, currentField) => {
2121
const field = {
2222
[currentField.name]: { type: singleToNetworkAggregationMap.get(currentField.type) },
Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
import { mergeTypeDefs } from '@graphql-tools/merge';
21
import { SupportedNetworkFieldType } from '../types/types';
32
import { createNetworkAggregationTypeDefs } from './aggregations';
4-
import { remoteConnectionTypes } from './remoteConnections';
53

64
export const createTypeDefs = (networkFieldTypes: SupportedNetworkFieldType[]) => {
7-
const aggregationTypes = createNetworkAggregationTypeDefs(networkFieldTypes);
8-
const typeDefs = mergeTypeDefs([remoteConnectionTypes, aggregationTypes]);
5+
const typeDefs = createNetworkAggregationTypeDefs(networkFieldTypes);
96
return typeDefs;
107
};

modules/server/src/network/typeDefs/remoteConnections.ts

Lines changed: 0 additions & 12 deletions
This file was deleted.

0 commit comments

Comments
 (0)