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 <gmc@equinoxinitiative.org>

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 32ad107..0a96183 100644 (file)
--- 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<void> {
-        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;
@@ -220,11 +204,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;
@@ -238,65 +222,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 (file)
index 0000000..4a040da
--- /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<void> {
+        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 (file)
index 0000000..906f012
--- /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<Promise<any[]>> {
+
+    savedId: number = null;
+    
+    constructor(
+        private router: Router,
+        private attrDefs: AttrDefsService,
+    ) {}
+
+    resolve(
+        route: ActivatedRouteSnapshot,
+        state: RouterStateSnapshot): Promise<any[]> {
+
+        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 337621c..e05e58f 100644 (file)
--- 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 {}