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

Fix filter performance by building filters during integration #147

Merged
merged 1 commit into from
Jul 20, 2016
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 12 additions & 0 deletions conf/option-fields.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
"subCamp",
"village",
"campGroup",
"localGroup",
"ageGroup",
"childNaps",
"accommodation",
"country",
"willOfTheWisp",
"willOfTheWispWave"
]
45 changes: 25 additions & 20 deletions src/client/actions/SearchFilterActions.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import _ from 'lodash';

export function getSearchFilterActions(alt, searchFilterResource, participantResource, participantDateResource, errorActions) {
export function getSearchFilterActions(alt, searchFilterResource, participantResource, participantDateResource, optionResource, errorActions) {
class SearchFilterActions {
saveSearchFilter(name, filter) {
return dispatch => {
Expand Down Expand Up @@ -43,23 +43,27 @@ export function getSearchFilterActions(alt, searchFilterResource, participantRes
return searchFilters;
}

loadOptions(property) {
loadOptions() {
return dispatch => {
dispatch();
if (!property) {
this.optionsLoaded('', null); // dispatch empty list
} else {
participantResource.findAll(`filter[fields][${property}]=true`)
.then(response => this.optionsLoaded(property, processResults(response)),
err => errorActions.error(err, `Hakusuodatinta ${property} ei voitu ladata`));
}
optionResource.findAll()
.then(response => this.optionsLoaded(processResults(response)),
err => errorActions.error(err, `Hakusuodattimia ei voitu ladata`));
};

function processResults(result) {
const optionStrings = result.map(obj => obj[property]);
const uniqueStrings = _.uniq(optionStrings);
uniqueStrings.sort();
return _.concat([''], uniqueStrings);
function processResults(response) {
//TODO Clean up after a good night's sleep
const x = {};
for (let i = 0; i < response.length; i++) {
const property = response[i].property;
if (x[property]) {
x[property].push(response[i].value);
} else {
x[property] = [];
x[property].push(response[i].value);
}
}
return x;
}
}

Expand All @@ -69,16 +73,17 @@ export function getSearchFilterActions(alt, searchFilterResource, participantRes
return dispatch => {
dispatch();
participantDateResource.findAll(`filter[fields][date]=true`)
.then(response => this.optionsLoaded('dates', processResults(response)),
.then(response => this.dateOptionsLoaded(processResults(response)),
err => this.optionsLoadingFailed(err));
};
}

optionsLoaded(property, options) {
return {
property: property,
options: options,
};
optionsLoaded(options) {
return options;
}

dateOptionsLoaded(options) {
return options;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ export function getParticipantListPage(participantStore, participantActions, sea
);
}

componentWillMount() {
searchFilterActions.loadOptions.defer();
}

componentDidMount() {
participantStore.listen(this.checkNoneOnParticipantsChanged);
searchFilterStore.listen(this.onSearchFilterStoreChanged);
Expand All @@ -216,7 +220,7 @@ export function getParticipantListPage(participantStore, participantActions, sea

extractDatesFromSearchFilters() {
const state = this.state;
state.availableDates = searchFilterStore.getState().options.dates || [];
state.availableDates = searchFilterStore.getState().dates || [];
return state;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export function getDateFilterContainer(searchFilterStore, searchFilterActions) {
}

extractState() {
return { options: searchFilterStore.getState().options[this.props.property] || [] };
return { options: searchFilterStore.getState().dates || [] };
}

render() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ export function getPropertyFilterContainer(searchFilterStore, searchFilterAction
this.extractState = this.extractState.bind(this);
}

componentWillMount() {
searchFilterActions.loadOptions.defer(this.props.property);
}

componentDidMount() {
searchFilterStore.listen(this.onStoreChanged);
}
Expand Down
3 changes: 2 additions & 1 deletion src/client/main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@ const participantResource = new RestfulResource('/api/participants', accessToken
const participantDateResource = new RestfulResource('/api/participantDates', accessToken);
const registryUserResource = new RestfulResource('/api/registryusers', accessToken);
const searchFilterResource = new RestfulResource('/api/searchfilters', accessToken);
const optionResource = new RestfulResource('/api/options', accessToken);

const alt = new Alt();

const errorActions = actions.getErrorActions(alt);
const participantActions = actions.getParticipantActions(alt, participantResource, errorActions);
const searchFilterActions = actions.getSearchFilterActions(alt, searchFilterResource, participantResource, participantDateResource, errorActions);
const searchFilterActions = actions.getSearchFilterActions(alt, searchFilterResource, participantResource, participantDateResource, optionResource, errorActions);
const registryUserActions = actions.getRegistryUserActions(alt, registryUserResource, errorActions);

const errorStore = stores.getErrorStore(alt, errorActions);
Expand Down
9 changes: 7 additions & 2 deletions src/client/stores/SearchFilterStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,20 @@ export function getSearchFilterStore(alt, SearchFilterActions) {
this.bindListeners({
handleSearchFilterListUpdated: SearchFilterActions.SEARCH_FILTER_LIST_UPDATED,
handleOptionsLoaded: SearchFilterActions.OPTIONS_LOADED,
handleDateOptionsLoaded: SearchFilterActions.DATE_OPTIONS_LOADED,
});
}

handleSearchFilterListUpdated(searchFilters) {
this.searchFilters = searchFilters;
}

handleOptionsLoaded({ property, options }) {
this.options[property] = options;
handleOptionsLoaded(options) {
this.options = options;
}

handleDateOptionsLoaded(dates) {
this.dates = dates;
}
}

Expand Down
1 change: 1 addition & 0 deletions src/common/models-list.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const modelList = [
['ParticipantAllergy', false],
['ParticipantDate', false],
['PresenceHistory', false],
['Option', false],
['AuditEvent', false],
['SearchFilter', false],
['Selection', false],
Expand Down
66 changes: 66 additions & 0 deletions src/common/models/option.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
{
"name": "Option",
"base": "PersistedModel",
"idInjection": false,
"options": {
"validateUpsert": true,
"postgresql": {
"schema": "public",
"table": "option"
}
},
"properties": {
"id": {
"type": "Number",
"id": true,
"generated": true,
"postgresql": {
"columnName": "id",
"dataType": "integer",
"dataLength": null,
"dataPrecision": 32,
"dataScale": 0,
"nullable": "NO"
}
},
"property": {
"type": "string",
"required": true,
"postgresql": {
"columnName": "property",
"dataType": "character varying",
"dataLength": 255,
"dataPrecision": null,
"dataScale": null,
"nullable": "NO"
}
},
"value": {
"type": "string",
"required": true,
"postgresql": {
"columnName": "value",
"dataType": "character varying",
"dataLength": 255,
"dataPrecision": null,
"dataScale": null,
"nullable": "NO"
}
}
},
"validations": [],
"acls": [
{
"principalType": "ROLE",
"principalId": "$everyone",
"permission": "DENY"
},
{
"accessType": "READ",
"principalType": "ROLE",
"principalId": "registryUser",
"permission": "ALLOW"
}
],
"methods": []
}
26 changes: 25 additions & 1 deletion src/kuksa-integration/rebuild-tables.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import Promise from 'bluebird';
import { _ } from 'lodash';
import moment from 'moment';
import paymentToDateMappings from '../../conf/payment-date-mappings.json';
import optionFields from '../../conf/option-fields.json';

const KuksaParticipant = app.models.KuksaParticipant;
const findKuksaParticipants = Promise.promisify(KuksaParticipant.find, { context: KuksaParticipant });
Expand All @@ -12,6 +13,10 @@ const upsertParticipant = Promise.promisify(Participant.upsert, { context: Parti
const findParticipants = Promise.promisify(Participant.find, { context: Participant });
const destroyAllParticipants = Promise.promisify(Participant.destroyAll, { context: Participant });

const Option = app.models.Option;
const destroyAllOptions = Promise.promisify(Option.destroyAll, { context: Option });
const upsertOption = Promise.promisify(Option.upsert, { context: Option });

if (require.main === module) {
main().then(
() => { console.log('Finished successfully.'); process.exit(0); },
Expand All @@ -25,7 +30,8 @@ function main() {
.then(addAllergiesToParticipants)
.then(addDatesToParticipants)
.then(buildSelectionTable)
.then(deleteCancelledParticipants);
.then(deleteCancelledParticipants)
.then(buildOptionTable);
}

function buildAllergyTable() {
Expand Down Expand Up @@ -217,3 +223,21 @@ function deleteCancelledParticipants() {
.then(idsToDelete => destroyAllParticipants({ participantId: { inq: idsToDelete } }))
.then(info => console.log(`Deleted ${info.count} cancelled participants.`));
}

function buildOptionTable() {
const addFieldValues = ({ field, values }) => Promise.each(values, value => upsertOption({ property: field, value: value }));
return destroyAllOptions()
.then(() => Promise.mapSeries(optionFields, getFieldValues))
.then(items => Promise.each(items, addFieldValues));

function getFieldValues(field) {
const filter = { fields: { } };
filter['fields'][field] = true;
return findParticipants(filter)
.then(values => ({
field: field,
values: _(values).map(obj => obj[field]).uniq().reject(_.isNull).value().sort(),
}));
}
}

4 changes: 4 additions & 0 deletions src/server/model-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@
"dataSource": "db",
"public": true
},
"Option": {
"dataSource": "db",
"public": true
},
"AuditEvent": {
"dataSource": "db",
"public": false
Expand Down
15 changes: 15 additions & 0 deletions test/kuksa-integration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('Kuksa integration', () => {
const findAllergyById = Promise.promisify(app.models.Allergy.findById, { context: app.models.Allergy });
const findSelections = Promise.promisify(app.models.Selection.find, { context: app.models.Selection });
const countSelections = Promise.promisify(app.models.Selection.count, { context: app.models.Selection });
const countOptions = Promise.promisify(app.models.Option.count, { context: app.models.Option });

before(function(done) {
this.timeout(80000);
Expand Down Expand Up @@ -167,6 +168,20 @@ describe('Kuksa integration', () => {
})
);

it('builds options in advance',
() => expect(countOptions()).to.eventually.be.above(0)
);

it('creates each option only once',
() => expect(countOptions({ property: 'subCamp', value: 'Unity' })).to.eventually.equal(1)
);

it('builds correct amount of options',
() => expect(countOptions({ property: 'village' })).to.eventually.equal(4)
);

//TODO Check it saves correct optinos for each field

after(() => {
mockKuksa.stop();
});
Expand Down