Skip to content
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

RN-603 de dg multi mapping #4158

Merged
merged 20 commits into from
Sep 19, 2022
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/data-broker/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"case": "^1.5.5",
"lodash.flatten": "^4.4.0",
"lodash.groupby": "^4.6.0",
"lodash.isequal": "^4.5.0",
"lodash.keyby": "^4.6.0",
"lodash.pickby": "^4.6.0",
"moment": "^2.24.0",
Expand Down
164 changes: 139 additions & 25 deletions packages/data-broker/src/DataBroker.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
*/

import { lower } from 'case';
import groupBy from 'lodash.groupby';
import isequal from 'lodash.isequal';

import { ModelRegistry, TupaiaDatabase } from '@tupaia/database';
import { countDistinct, toArray } from '@tupaia/utils';
import { createService } from './services';
import { DATA_SOURCE_TYPES } from './utils';
import { DataServiceResolver } from './services/DataServiceResolver';

export const BES_ADMIN_PERMISSION_GROUP = 'BES Admin';

Expand All @@ -35,6 +36,7 @@ export class DataBroker {
constructor(context = {}) {
this.context = context;
this.models = getModelRegistry();
this.dataServiceResolver = new DataServiceResolver(this.models);
this.resultMergers = {
[this.getDataSourceTypes().DATA_ELEMENT]: this.mergeAnalytics,
[this.getDataSourceTypes().DATA_GROUP]: this.mergeEvents,
Expand Down Expand Up @@ -153,32 +155,51 @@ export class DataBroker {
return createService(this.models, serviceType, this);
}

async push(dataSourceSpec, data) {
async push(dataSourceSpec, data, options = {}) {
const dataSources = await this.fetchDataSources(dataSourceSpec);
if (countDistinct(dataSources, 'service_type') > 1) {
throw new Error('Cannot push data belonging to different services');
}
const service = this.createService(dataSources[0].service_type);
return service.push(dataSources, data, { type: dataSourceSpec.type });
const { type: dataSourceType } = dataSourceSpec;
const { serviceType, dataServiceMapping } = await this.getSingleServiceAndMapping(
dataSources,
options,
);

const service = this.createService(serviceType);
return service.push(dataSources, data, { type: dataSourceType, dataServiceMapping });
}

async delete(dataSourceSpec, data, options) {
async delete(dataSourceSpec, data, options = {}) {
const dataSources = await this.fetchDataSources(dataSourceSpec);
const [dataSource] = dataSources;
const service = this.createService(dataSource.service_type);
return service.delete(dataSource, data, { type: dataSourceSpec.type, ...options });
const { serviceType, dataServiceMapping } = await this.getSingleServiceAndMapping(
dataSources,
options,
);

const service = this.createService(serviceType);
return service.delete(dataSource, data, {
type: dataSourceSpec.type,
dataServiceMapping,
...options,
});
}

async pull(dataSourceSpec, options) {
async pull(dataSourceSpec, options = {}) {
const dataSources = await this.fetchDataSources(dataSourceSpec);
const { type } = dataSourceSpec;
const { organisationUnitCode, organisationUnitCodes } = options;
const orgUnitCodes = organisationUnitCodes || [organisationUnitCode];

const dataSourcesByService = groupBy(dataSources, 'service_type');
const dataSourceFetches = Object.values(dataSourcesByService);
const pulls = await this.getPulls(dataSources, orgUnitCodes);
const nestedResults = await Promise.all(
dataSourceFetches.map(dataSourceForFetch =>
this.pullForServiceAndType(dataSourceForFetch, options, type),
),
pulls.map(({ dataSources: dataSourcesForThisPull, serviceType, dataServiceMapping }) => {
return this.pullForServiceAndType(
dataSourcesForThisPull,
options,
type,
serviceType,
dataServiceMapping,
);
}),
);
const mergeResults = this.resultMergers[type];

Expand All @@ -188,13 +209,12 @@ export class DataBroker {
);
}

pullForServiceAndType = async (dataSources, options, type) => {
const { service_type: serviceType } = dataSources[0];
pullForServiceAndType = async (dataSources, options, type, serviceType, dataServiceMapping) => {
const permissionChecker = this.permissionCheckers[type];
// Permission checkers will throw if they fail
await permissionChecker(dataSources);
const service = this.createService(serviceType);
return service.pull(dataSources, type, options);
return service.pull(dataSources, type, { dataServiceMapping, ...options });
};

mergeAnalytics = (target = { results: [], metadata: {} }, source) => {
Expand Down Expand Up @@ -241,14 +261,108 @@ export class DataBroker {

mergeSyncGroups = (target = [], source) => target.concat(source);

async pullMetadata(dataSourceSpec, options) {
async pullMetadata(dataSourceSpec, options = {}) {
const dataSources = await this.fetchDataSources(dataSourceSpec);
if (countDistinct(dataSources, 'service_type') > 1) {
throw new Error('Cannot pull metadata for data sources belonging to different services');
}
const service = this.createService(dataSources[0].service_type);
const { serviceType, dataServiceMapping } = await this.getSingleServiceAndMapping(
dataSources,
options,
);

const service = this.createService(serviceType);
// `dataSourceSpec` is defined for a single `type`
const { type } = dataSourceSpec;
return service.pullMetadata(dataSources, type, options);
return service.pullMetadata(dataSources, type, { dataServiceMapping, ...options });
}

/**
* Given some DataSources, returns a single serviceType or throws an error if multiple found
* @private
* @param {DataSource[]} dataSources
* @param {{organisationUnitCode?: string}} options
* @return {Promise<{ serviceType: string, mapping: DataServiceMapping }>}
*/
async getSingleServiceAndMapping(dataSources, options = {}) {
const { organisationUnitCode } = options;

const dataServiceMapping = await this.dataServiceResolver.getMappingByOrgUnitCode(
dataSources,
organisationUnitCode,
);
if (dataServiceMapping.uniqueServiceTypes().length > 1) {
throw new Error('Multiple data service types found, only a single service type expected');
}

const [serviceType] = dataServiceMapping.uniqueServiceTypes();
return {
serviceType,
dataServiceMapping,
};
}

async getPulls(dataSources, orgUnitCodes) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We had discussed breaking this out into a separate class right? I'm not too fussed though, feel it's okay being here as well 👍

const orgUnits = await this.models.entity.find({ code: orgUnitCodes });

// Note: each service will pull for ALL org units and ALL data sources.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason we can't fetch for just the countries/data elements in each mapping? We work out which countries and dataElements/groups belong to each along the way, so we'd just need to combine them when building the uniqueMapping?

It feels a little odd to pull for dataElements and countries that we know aren't in the data service (and also could create issues if they actually are in the data service but Tupaia isn't configured to fetch them from there...)

Copy link
Contributor Author

@IgorNadj IgorNadj Sep 16, 2022

Choose a reason for hiding this comment

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

Discussed.

	ORIGINAL FETCH:
		orgUnits: FJ, TO
		de: DrugA, DrugB

	MAPPING:
		DrugA & FJ => superset
		DrugB & FJ => dhis (dhis-regional)
		DrugA & TO => dhis (dhis-tonga)
		DrugB & TO => dhis (dhis-tonga)


	UNIQUE FETCHES:
		superset:
			orgUnits: FJ
			de: DrugA
			mapping:
				DrugA: superset
		dhis:
			orgUnits: TO, FJ
			de: DrugA, DrugB
			mapping:
				DrugA: dhis-tonga
				DrugB: dhis-regional
				DrugB: dhis-tonga

The issue is that our DataServiceMapping class and mapping resolution doesn't play nice with double ups like DrugB in dhis there.

Options are to rework the mapping, or to leave it until we do the full implementation which would drop the need for a DataServiceMapping class at all, make all this trivial. I say we leave it.

// This will likely lead to problems in the future, for now this is ok because
// our services happily ignore extra org units, and our vizes do not ask for
// data elements that don't exist in dhis (dhis throws if it cant find it).

// First we get the mapping for each country, then if any two countries have the
// exact same mapping we simply combine them
const orgUnitCountryCodes = orgUnits
.map(orgUnit => orgUnit.country_code)
.filter(countryCode => countryCode !== null && countryCode !== undefined);
const countryCodes = [...new Set(orgUnitCountryCodes)];

if (countryCodes.length === 1) {
// No special logic needed, exit early
const [countryCode] = countryCodes;
const dataServiceMapping = await this.dataServiceResolver.getMappingByCountryCode(
dataSources,
countryCode,
);
return Object.entries(dataServiceMapping.dataSourcesByServiceType()).map(
([serviceType, dataSourcesForThisServiceType]) => ({
dataSources: dataSourcesForThisServiceType,
serviceType,
dataServiceMapping,
}),
);
}

const mappingsByCountryCode = {};
for (const countryCode of countryCodes) {
mappingsByCountryCode[countryCode] = await this.dataServiceResolver.getMappingByCountryCode(
dataSources,
countryCode,
);
}

const uniqueMappings = [];
for (const mappingA of Object.values(mappingsByCountryCode)) {
let alreadyAdded = false;
for (const mappingB of uniqueMappings) {
if (mappingA === mappingB) continue;
if (mappingA.equals(mappingB)) {
alreadyAdded = true;
break;
}
}
if (!alreadyAdded) uniqueMappings.push(mappingA);
}

// And finally split each by service type
const pulls = [];
for (const mapping of uniqueMappings) {
for (const serviceType of mapping.uniqueServiceTypes()) {
pulls.push({
dataSources,
serviceType,
dataServiceMapping: mapping,
});
}
}

return pulls;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,51 @@
* Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd
*/

import { TYPES } from '@tupaia/database';

// Data elements and groups share the same codes on purpose, to assert that
// `DataBroker` can still distinguish them using their type
export const DATA_ELEMENTS = {
TEST_01: { code: 'TEST_01', service_type: 'test', config: {}, permission_groups: ['*'] },
TEST_02: { code: 'TEST_02', service_type: 'test', config: {}, permission_groups: ['*'] },
OTHER_01: { code: 'OTHER_01', service_type: 'other', config: {}, permission_groups: ['*'] },
TEST_01: {
code: 'TEST_01',
service_type: 'test',
config: {},
permission_groups: ['*'],
databaseType: TYPES.DATA_ELEMENT,
},
TEST_02: {
code: 'TEST_02',
service_type: 'test',
config: {},
permission_groups: ['*'],
databaseType: TYPES.DATA_ELEMENT,
},
OTHER_01: {
code: 'OTHER_01',
service_type: 'other',
config: {},
permission_groups: ['*'],
databaseType: TYPES.DATA_ELEMENT,
},
MAPPED_01: {
code: 'MAPPED_01',
service_type: 'test',
config: {},
permission_groups: ['*'],
databaseType: TYPES.DATA_ELEMENT,
},
MAPPED_02: {
code: 'MAPPED_02',
service_type: 'test',
config: {},
permission_groups: ['*'],
databaseType: TYPES.DATA_ELEMENT,
},
};
export const DATA_GROUPS = {
TEST_01: { code: 'TEST_01', service_type: 'test', config: {} },
TEST_02: { code: 'TEST_02', service_type: 'test', config: {} },
OTHER_01: { code: 'OTHER_01', service_type: 'other', config: {} },
TEST_01: { code: 'TEST_01', service_type: 'test', config: {}, databaseType: TYPES.DATA_GROUP },
TEST_02: { code: 'TEST_02', service_type: 'test', config: {}, databaseType: TYPES.DATA_GROUP },
OTHER_01: { code: 'OTHER_01', service_type: 'other', config: {}, databaseType: TYPES.DATA_GROUP },
};

export const DATA_BY_SERVICE = {
Expand All @@ -28,3 +62,25 @@ export const DATA_BY_SERVICE = {
{ code: 'OTHER_01', type: 'dataGroup', name: 'Other group 1', value: 30 },
],
};

export const ENTITIES = {
TO_FACILITY_01: { code: 'TO_FACILITY_01', country_code: 'TO' },
FJ_FACILITY_01: { code: 'FJ_FACILITY_01', country_code: 'FJ' },
TO: { code: 'TO', country_code: 'TO', type: 'country' },
FJ: { code: 'FJ', country_code: 'FJ', type: 'country' },
};

export const DATA_ELEMENT_DATA_SERVICES = [
{
data_element_code: 'MAPPED_01',
country_code: 'FJ',
service_type: 'other',
service_config: { cow: 'moo' },
},
{
data_element_code: 'MAPPED_02',
country_code: 'FJ',
service_type: 'other',
service_config: { sheep: 'baaaa' },
},
];
18 changes: 15 additions & 3 deletions packages/data-broker/src/__tests__/DataBroker/DataBroker.stubs.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
import { createJestMockInstance } from '@tupaia/utils';
import { createModelsStub as baseCreateModelsStub } from '@tupaia/database';
import * as CreateService from '../../services/createService';
import {
DATA_ELEMENT_DATA_SERVICES,
DATA_ELEMENTS,
DATA_GROUPS,
ENTITIES,
} from './DataBroker.fixtures';

export const stubCreateService = services =>
jest.spyOn(CreateService, 'createService').mockImplementation((_, type) => {
Expand Down Expand Up @@ -43,10 +49,10 @@ export const createServiceStub = serviceData => {
return createJestMockInstance('@tupaia/data-broker/src/services/Service', 'Service', { pull });
};

export const createModelsStub = (dataElements, dataGroups) => {
export const createModelsStub = () => {
return baseCreateModelsStub({
dataElement: {
records: dataElements,
records: Object.values(DATA_ELEMENTS),
extraMethods: {
getTypes: () => ({
DATA_ELEMENT: 'dataElement',
Expand All @@ -56,10 +62,16 @@ export const createModelsStub = (dataElements, dataGroups) => {
},
},
dataGroup: {
records: dataGroups,
records: Object.values(DATA_GROUPS),
extraMethods: {
getDataElementsInDataGroup: () => [],
},
},
entity: {
records: Object.values(ENTITIES),
},
dataElementDataService: {
records: DATA_ELEMENT_DATA_SERVICES,
},
});
};
Loading