Skip to content

Commit

Permalink
Foundations for Warnings System (#161)
Browse files Browse the repository at this point in the history
Currently, a staged place stores only the "original values" which come from the user. When code wants the corresponding "formatted value" for that propertly, it uses the validation library to format it on demand. When importing large CSV lists of CHUs/CHPs (like the Narok dataset), this results in >300,000 calls of Validation.formatSingle. This isn't that slow; but it slowed down a lot when we started autogenerating properties #47.

The work happening in #20 requires more formatted of property data. Although the performance gains from this change aren't huge (below), I view these changes as a required foundation for that work lest things get even slower.

This change therefore validates and formats the inputs once. The result is persisted as an IPropertyValue which contains .formatted and .original values. This adds complexity in the place datamodel, but simplifies upstream classes like RemotePlaceResolver.

The interface to the Validation library has changed significantly. New interface is to create a new ValidatedPropertyValue().

Remote Place Caching
Currently, the ChtApi.getPlacesWithType downloads all documents of a particular type and caches a subset of the data (id, name, lineage) as a RemotePlace. The warning system is going to need more data about these places so we can check for uniqueness (eg. unique facility codes). The doc's name information should also now be stored as "Property Value" and so this resulted in a reduced role of ChtApi (just fetches docs now), an increased role for RemotePlaceCache (it maps the doc down to a reduced set for caching), and a bit more complexity in Config.

Performance Measures
For Narok upload performance set (1640 CHPs)
Before: 1170ms
After: 601ms
  • Loading branch information
kennsippell authored Dec 13, 2024
1 parent 846b8bf commit 90ef80d
Show file tree
Hide file tree
Showing 54 changed files with 1,480 additions and 1,022 deletions.
2 changes: 1 addition & 1 deletion docker-local-setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ to build missing images";echo;
fi

echo;echo "Starting Docker Compose...";echo
CHT_USER_MANAGEMENT_IMAGE=cht-user-management:local CHT_USER_MANAGEMENT_WORKER_IMAGE=cht-user-management-worker:local docker compose up -d
CHT_USER_MANAGEMENT_IMAGE=cht-user-management:local CHT_USER_MANAGEMENT_WORKER_IMAGE=cht-user-management-worker:local docker compose up

echo;echo "Server is now running at http://127.0.0.1:$EXTERNAL_PORT/login";echo
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "cht-user-management",
"version": "1.4.2",
"version": "1.5.0",
"main": "dist/index.js",
"dependencies": {
"@bull-board/api": "^5.17.0",
Expand Down
12 changes: 7 additions & 5 deletions scripts/create-user-managers/create-user-managers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import { Command } from 'commander';
import { AuthenticationInfo, ContactType } from '../../src/config';
import { createUserWithRetries } from '../../src/lib/retry-logic';
import Place from '../../src/services/place';
import { UserPayload } from '../../src/services/user-payload';
import RemotePlaceCache, { RemotePlace } from '../../src/lib/remote-place-cache';
import { PropertyValues, UnvalidatedPropertyValue } from '../../src/property-value';
import UserManager from './ke_user_manager.json';
import { UserPayload } from '../../src/services/user-payload';

const { ChtApi } = require('../../src/lib/cht-api'); // require is needed for rewire
const ChtSession = require('../../src/lib/cht-session').default; // require is needed for rewire
Expand Down Expand Up @@ -53,8 +55,8 @@ export default async function createUserManagers(argv: string[]) {

async function createUserManager(username: string, placeDocId: string, chtApi: typeof ChtApi, adminUsername: string, passwordOverride?: string) {
const place = new Place(UserManagerContactType);
place.contact.properties.name = `${username} (User Manager)`;
place.userRoleProperties.role = UserManagerContactType.user_role.join(' ');
place.contact.properties.name = new UnvalidatedPropertyValue(`${username} (User Manager)`, 'name');
place.userRoleProperties.role = new UnvalidatedPropertyValue(UserManagerContactType.user_role.join(' '), 'role');

const chtPayload = place.asChtPayload(adminUsername);
chtPayload.contact.role = 'user_manager';
Expand Down Expand Up @@ -96,8 +98,8 @@ function parseCommandlineArguments(argv: string[]): CommandLineArgs {
}

async function getPlaceDocId(county: string | undefined, chtApi: typeof ChtApi) {
const counties = await chtApi.getPlacesWithType('a_county');
const countyMatches = counties.filter((c: any) => !county || c.name === county.toLowerCase());
const counties = await RemotePlaceCache.getPlacesWithType(chtApi, UserManagerContactType, UserManagerContactType.hierarchy[0]);
const countyMatches = counties.filter((c: RemotePlace) => !county || PropertyValues.isMatch(county, c.name));
if (countyMatches.length < 1) {
throw Error(`Could not find county "${county}"`);
}
Expand Down
10 changes: 9 additions & 1 deletion scripts/create-user-managers/ke_user_manager.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,15 @@
"contact_type": "person",
"user_role": ["user_manager", "mm-online"],
"username_from_place": false,
"hierarchy": [],
"hierarchy": [{
"type": "name",
"friendly_name": "County",
"property_name": "name",
"required": false,
"parameter": ["\\sCounty"],
"contact_type": "a_county",
"level": 0
}],
"deactivate_users_on_replace": false,
"replacement_property": {
"friendly_name": "Unused",
Expand Down
2 changes: 1 addition & 1 deletion src/config/config-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import kenyaConfig from './chis-ke';
import togoConfig from './chis-tg';
import civConfig from './chis-civ';

const CONFIG_MAP: { [key: string]: PartnerConfig } = {
export const CONFIG_MAP: { [key: string]: PartnerConfig } = {
'CHIS-KE': kenyaConfig,
'CHIS-UG': ugandaConfig,
'CHIS-TG': togoConfig,
Expand Down
37 changes: 35 additions & 2 deletions src/config/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import _ from 'lodash';
import { ChtApi, PlacePayload } from '../lib/cht-api';
import getConfigByKey from './config-factory';
import Validation from '../validation';

export type ConfigSystem = {
domains: AuthenticationInfo[];
Expand Down Expand Up @@ -28,10 +29,13 @@ export type ContactType = {
hint?: string;
};

const KnownContactPropertyTypes = [...Validation.getKnownContactPropertyTypes()] as const;
export type ContactPropertyType = typeof KnownContactPropertyTypes[number];

export type HierarchyConstraint = {
friendly_name: string;
property_name: string;
type: string;
type: ContactPropertyType;
required: boolean;
parameter? : string | string[] | object;
errorDescription? : string;
Expand All @@ -43,7 +47,7 @@ export type HierarchyConstraint = {
export type ContactProperty = {
friendly_name: string;
property_name: string;
type: string;
type: ContactPropertyType;
required: boolean;
parameter? : string | string[] | object;
errorDescription? : string;
Expand All @@ -55,6 +59,7 @@ export type AuthenticationInfo = {
useHttp?: boolean;
};


const {
CONFIG_NAME,
NODE_ENV,
Expand Down Expand Up @@ -187,6 +192,33 @@ export class Config {
return _.sortBy(domains, 'friendly');
}

// TODO: Joi? Chai?
public static assertValid({ config }: PartnerConfig = partnerConfig) {
for (const contactType of config.contact_types) {
const allHierarchyProperties = [...contactType.hierarchy, contactType.replacement_property];
const allProperties = [
...contactType.place_properties,
...contactType.contact_properties,
...allHierarchyProperties,
Config.getUserRoleConfig(contactType),
];

Config.getPropertyWithName(contactType.place_properties, 'name');
Config.getPropertyWithName(contactType.contact_properties, 'name');

allProperties.forEach(property => {
if (!KnownContactPropertyTypes.includes(property.type)) {
throw Error(`Unknown property type "${property.type}"`);
}
});

const generatedHierarchyProperties = allHierarchyProperties.filter(hierarchy => hierarchy.type === 'generated');
if (generatedHierarchyProperties.length) {
throw Error('Hierarchy properties cannot be of type "generated"');
}
}
}

public static getCsvTemplateColumns(placeType: string) {
const placeTypeConfig = Config.getContactType(placeType);
const hierarchy = Config.getHierarchyWithReplacement(placeTypeConfig);
Expand All @@ -205,3 +237,4 @@ export class Config {
return columns;
}
}

9 changes: 3 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
require('dotenv').config();
import { Config } from './config';
import build from './server';
import { env } from 'process';
const {
INTERFACE
} = process.env;

const port: number = env.PORT ? parseInt(env.PORT) : 3500;
Config.assertValid();

(async () => {
const loggerConfig = {
transport: {
target: 'pino-pretty',
},
};
const server = build({
logger: loggerConfig,
logger: true,
});

// in 1.1.0 we allowed INTERFACE to be declared in .env, but let's be
Expand Down
39 changes: 5 additions & 34 deletions src/lib/cht-api.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import _ from 'lodash';
import { AxiosInstance } from 'axios';
import ChtSession from './cht-session';
import { Config, ContactType } from '../config';
import { UserPayload } from '../services/user-payload';
import { AxiosInstance } from 'axios';

export type PlacePayload = {
name: string;
Expand All @@ -18,17 +18,6 @@ export type PlacePayload = {
[key: string]: any;
};

export type RemotePlace = {
id: string;
name: string;
lineage: string[];
ambiguities?: RemotePlace[];

// sadly, sometimes invalid or uncreated objects "pretend" to be remote
// should reconsider this naming
type: 'remote' | 'local' | 'invalid';
};

export class ChtApi {
public readonly chtSession: ChtSession;
private axiosInstance: AxiosInstance;
Expand Down Expand Up @@ -174,26 +163,15 @@ export class ChtApi {
};

getPlacesWithType = async (placeType: string)
: Promise<RemotePlace[]> => {
const url = `medic/_design/medic-client/_view/contacts_by_type_freetext`;
: Promise<any[]> => {
const url = `medic/_design/medic-client/_view/contacts_by_type`;
const params = {
startkey: JSON.stringify([ placeType, 'name:']),
endkey: JSON.stringify([ placeType, 'name:\ufff0']),
key: JSON.stringify([placeType]),
include_docs: true,
};
console.log('axios.get', url, params);
const resp = await this.axiosInstance.get(url, { params });

return resp.data.rows
.map((row: any): RemotePlace => {
const nameData = row.key[1];
return {
id: row.id,
name: nameData.substring('name:'.length),
lineage: extractLineage(row.doc),
type: 'remote',
};
});
return resp.data.rows.map((row: any) => row.doc);
};

getDoc = async (id: string): Promise<any> => {
Expand Down Expand Up @@ -228,10 +206,3 @@ function minify(doc: any): any {
};
}

function extractLineage(doc: any): string[] {
if (doc?.parent?._id) {
return [doc.parent._id, ...extractLineage(doc.parent)];
}

return [];
}
2 changes: 1 addition & 1 deletion src/lib/cht-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { AuthenticationInfo } from '../config';
import { AxiosHeaders, AxiosInstance } from 'axios';
import axiosRetry from 'axios-retry';
import { axiosRetryConfig } from './retry-logic';
import { RemotePlace } from './cht-api';
import { RemotePlace } from './remote-place-cache';

const COUCH_AUTH_COOKIE_NAME = 'AuthSession=';
const ADMIN_FACILITY_ID = '*';
Expand Down
52 changes: 52 additions & 0 deletions src/lib/credentials-file.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { Config, ContactType } from '../config';
import SessionCache from '../services/session-cache';
import { stringify } from 'csv/sync';

type File = {
filename: string;
content: string;
};

export default function getCredentialsFiles(sessionCache: SessionCache, contactTypes: ContactType[]): File[] {
const files: File[] = [];
for (const contactType of contactTypes) {
const places = sessionCache.getPlaces({ type: contactType.name });
if (!places.length) {
continue;
}

const rows = places.map((place) => [
...Object.values(place.hierarchyProperties).map(prop => prop.formatted),
place.name,
place.contact.properties.name?.formatted,
place.contact.properties.phone?.formatted,
place.userRoles.join(' '),
place.creationDetails.username,
place.creationDetails.password,
]);

const constraints = Config.getHierarchyWithReplacement(contactType);
const props = Object.keys(places[0].hierarchyProperties)
.map(prop => constraints.find(c => c.property_name === prop)!.friendly_name);
const columns = [
...props,
contactType.friendly,
'name',
'phone',
'role',
'username',
'password',
];

const content = stringify(rows, {
columns,
header: true,
});
files.push({
filename: `${contactType.name}.csv`,
content,
});
}

return files;
}
4 changes: 2 additions & 2 deletions src/lib/move.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export default class MoveLib {
}

if (toId === fromLineage[1]?.id) {
throw Error(`Place "${fromLineage[0]?.name}" already has "${toLineage[1]?.name}" as parent`);
throw Error(`Place "${fromLineage[0]?.name.original}" already has "${toLineage[1]?.name.original}" as parent`);
}

const jobName = this.getJobName(fromLineage, toLineage);
Expand Down Expand Up @@ -63,7 +63,7 @@ async function resolve(prefix: string, formData: any, contactType: ContactType,
await RemotePlaceResolver.resolveOne(place, sessionCache, chtApi, { fuzz: true });
place.validate();

const validationError = place.validationErrors && Object.keys(place.validationErrors).find(err => err.startsWith('hierarchy_'));
const validationError = place.validationErrors && Object.keys(place.validationErrors).find(err => err.startsWith(prefix));
if (validationError) {
throw Error(place.validationErrors?.[validationError]);
}
Expand Down
Loading

0 comments on commit 90ef80d

Please sign in to comment.