From 4d9858ebc26e639d91fd45fe592fcaae60818155 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Mon, 13 Jul 2020 15:08:38 -0400 Subject: [PATCH] DO NOT CHERRY-PICK [taken from AA3; will already been in place when AA1 is cherry-picked on to a modern branch] fix race condition that could result in retrieval failure ... I wish there were a clear way to ensure that a service could wait on initialization from an async source Noting that the separate AttrDefsService now exists because it can be a singleton by AcqSearchService (currently?) cannot Signed-off-by: Galen Charlton --- .../src/app/staff/acq/search/acq-search.service.ts | 132 +++++++++------------ .../src/app/staff/acq/search/attr-defs.service.ts | 34 ++++++ .../src/app/staff/acq/search/resolver.service.ts | 25 ++++ .../eg2/src/app/staff/acq/search/routing.module.ts | 6 +- 4 files changed, 121 insertions(+), 76 deletions(-) create mode 100644 Open-ILS/src/eg2/src/app/staff/acq/search/attr-defs.service.ts create mode 100644 Open-ILS/src/eg2/src/app/staff/acq/search/resolver.service.ts diff --git a/Open-ILS/src/eg2/src/app/staff/acq/search/acq-search.service.ts b/Open-ILS/src/eg2/src/app/staff/acq/search/acq-search.service.ts index 88ac7caca4..3024720144 100644 --- a/Open-ILS/src/eg2/src/app/staff/acq/search/acq-search.service.ts +++ b/Open-ILS/src/eg2/src/app/staff/acq/search/acq-search.service.ts @@ -8,6 +8,7 @@ import {PcrudService} from '@eg/core/pcrud.service'; import {Pager} from '@eg/share/util/pager'; import {IdlObject} from '@eg/core/idl.service'; import {EventService} from '@eg/core/event.service'; +import {AttrDefsService} from './attr-defs.service'; const baseIdlClass = { lineitem: 'jub', @@ -105,35 +106,18 @@ export class AcqSearchService { _terms: AcqSearchTerm[] = []; _conjunction = 'all'; - attrDefs: {[code: string]: IdlObject}; firstRun = true; constructor( private net: NetService, private evt: EventService, private auth: AuthService, - private pcrud: PcrudService + private pcrud: PcrudService, + private attrDefs: AttrDefsService ) { - this.attrDefs = {}; this.firstRun = true; } - fetchAttrDefs(): Promise { - if (Object.keys(this.attrDefs).length) { - return Promise.resolve(); - } - return new Promise((resolve, reject) => { - this.pcrud.retrieveAll('acqliad', {}, - {atomic: true} - ).subscribe(list => { - list.forEach(acqliad => { - this.attrDefs[acqliad.code()] = acqliad; - }); - resolve(); - }); - }); - } - setSearch(search: AcqSearch) { this._terms = search.terms; this._conjunction = search.conjunction; @@ -211,11 +195,11 @@ export class AcqSearchService { searchTerm[operatorMap[filterOp]] = true; } if ((['title', 'author'].indexOf(filterField) > -1) && - (filterField in this.attrDefs)) { + (filterField in this.attrDefs.attrDefs)) { if (!('acqlia' in andTerms)) { andTerms['acqlia'] = []; } - searchTerm[this.attrDefs[filterField].id()] = filterVal; + searchTerm[this.attrDefs.attrDefs[filterField].id()] = filterVal; andTerms['acqlia'].push(searchTerm); } else { searchTerm[filterField] = filterVal; @@ -229,65 +213,63 @@ export class AcqSearchService { getAcqSearchDataSource(searchType: string): GridDataSource { const gridSource = new GridDataSource(); - this.fetchAttrDefs().then(() => { - gridSource.getRows = (pager: Pager, sort: any[]) => { + gridSource.getRows = (pager: Pager, sort: any[]) => { - // don't do a search the very first time we - // get invoked, which is during initialization; we'll - // let components higher up the change decide whether - // to submit a search - if (this.firstRun) { - this.firstRun = false; - return empty(); - } + // don't do a search the very first time we + // get invoked, which is during initialization; we'll + // let components higher up the change decide whether + // to submit a search + if (this.firstRun) { + this.firstRun = false; + return empty(); + } - const currentSearch = this.generateAcqSearch(searchType, gridSource.filters); + const currentSearch = this.generateAcqSearch(searchType, gridSource.filters); - const opts = { ...searchOptions[searchType] }; - opts['offset'] = pager.offset; - opts['limit'] = pager.limit; - opts['au_by_id'] = true; + const opts = { ...searchOptions[searchType] }; + opts['offset'] = pager.offset; + opts['limit'] = pager.limit; + opts['au_by_id'] = true; - if (sort.length > 0) { - opts['order_by'] = []; - sort.forEach(sort_clause => { - if (searchType === 'lineitem' && - ['title', 'author'].indexOf(sort_clause.name) > -1) { - opts['order_by'].push({ - class: 'acqlia', - field: 'attr_value', - direction: sort_clause.dir - }); - opts['order_by_attr'] = sort_clause.name; - } else { - opts['order_by'].push({ - class: baseIdlClass[searchType], - field: sort_clause.name, - direction: sort_clause.dir - }); - } - }); - } + if (sort.length > 0) { + opts['order_by'] = []; + sort.forEach(sort_clause => { + if (searchType === 'lineitem' && + ['title', 'author'].indexOf(sort_clause.name) > -1) { + opts['order_by'].push({ + class: 'acqlia', + field: 'attr_value', + direction: sort_clause.dir + }); + opts['order_by_attr'] = sort_clause.name; + } else { + opts['order_by'].push({ + class: baseIdlClass[searchType], + field: sort_clause.name, + direction: sort_clause.dir + }); + } + }); + } - return this.net.request( - 'open-ils.acq', - 'open-ils.acq.' + searchType + '.unified_search', - this.auth.token(), - currentSearch.andTerms, - currentSearch.orTerms, - null, - opts - ).pipe( - map(res => { - if (this.evt.parse(res)) { - throw throwError(res); - } else { - return res; - } - }), - ); - }; - }); + return this.net.request( + 'open-ils.acq', + 'open-ils.acq.' + searchType + '.unified_search', + this.auth.token(), + currentSearch.andTerms, + currentSearch.orTerms, + null, + opts + ).pipe( + map(res => { + if (this.evt.parse(res)) { + throw throwError(res); + } else { + return res; + } + }), + ); + }; return gridSource; } } diff --git a/Open-ILS/src/eg2/src/app/staff/acq/search/attr-defs.service.ts b/Open-ILS/src/eg2/src/app/staff/acq/search/attr-defs.service.ts new file mode 100644 index 0000000000..4a040dae38 --- /dev/null +++ b/Open-ILS/src/eg2/src/app/staff/acq/search/attr-defs.service.ts @@ -0,0 +1,34 @@ +import {Injectable} from '@angular/core'; +import {empty, throwError} from 'rxjs'; +import {map} from 'rxjs/operators'; +import {PcrudService} from '@eg/core/pcrud.service'; +import {IdlObject} from '@eg/core/idl.service'; + +@Injectable() +export class AttrDefsService { + + attrDefs: {[code: string]: IdlObject}; + + constructor( + private pcrud: PcrudService + ) { + this.attrDefs = {}; + } + + fetchAttrDefs(): Promise { + if (Object.keys(this.attrDefs).length) { + return Promise.resolve(); + } + return new Promise((resolve, reject) => { + this.pcrud.retrieveAll('acqliad', {}, + {atomic: true} + ).subscribe(list => { + list.forEach(acqliad => { + this.attrDefs[acqliad.code()] = acqliad; + }); + resolve(); + }); + }); + } + +} diff --git a/Open-ILS/src/eg2/src/app/staff/acq/search/resolver.service.ts b/Open-ILS/src/eg2/src/app/staff/acq/search/resolver.service.ts new file mode 100644 index 0000000000..906f012e2e --- /dev/null +++ b/Open-ILS/src/eg2/src/app/staff/acq/search/resolver.service.ts @@ -0,0 +1,25 @@ +import {Injectable} from '@angular/core'; +import {Router, Resolve, RouterStateSnapshot, + ActivatedRouteSnapshot} from '@angular/router'; +import {AttrDefsService} from './attr-defs.service'; + +@Injectable() +export class AttrDefsResolver implements Resolve> { + + savedId: number = null; + + constructor( + private router: Router, + private attrDefs: AttrDefsService, + ) {} + + resolve( + route: ActivatedRouteSnapshot, + state: RouterStateSnapshot): Promise { + + return Promise.all([ + this.attrDefs.fetchAttrDefs() + ]); + } + +} diff --git a/Open-ILS/src/eg2/src/app/staff/acq/search/routing.module.ts b/Open-ILS/src/eg2/src/app/staff/acq/search/routing.module.ts index 337621cb92..e05e58f3ce 100644 --- a/Open-ILS/src/eg2/src/app/staff/acq/search/routing.module.ts +++ b/Open-ILS/src/eg2/src/app/staff/acq/search/routing.module.ts @@ -1,14 +1,18 @@ import {NgModule} from '@angular/core'; import {RouterModule, Routes} from '@angular/router'; import {AcqSearchComponent} from './acq-search.component'; +import {AttrDefsResolver} from './resolver.service'; +import {AttrDefsService} from './attr-defs.service'; const routes: Routes = [ { path: '', component: AcqSearchComponent, + resolve: { attrDefsResolver : AttrDefsResolver }, runGuardsAndResolvers: 'always' }, { path: ':searchtype', component: AcqSearchComponent, + resolve: { attrDefsResolver : AttrDefsResolver }, runGuardsAndResolvers: 'always' } ]; @@ -16,7 +20,7 @@ const routes: Routes = [ @NgModule({ imports: [RouterModule.forChild(routes)], exports: [RouterModule], - providers: [] + providers: [AttrDefsResolver, AttrDefsService] }) export class AcqSearchRoutingModule {} -- 2.11.0