Fix issue where user profile was fetched repeatidly (#4285)

- remove calls to fetchUserProfile - ensure entity service is used instead of monitor (will fetch profile) - update FetchUserProfileAction action to a EntityRequestAction - there's no response to update profile, so ensure we fetch profile again afterwards - also fixed issue where metrics endpoint reporting a k8s endpoint would cause a console.error
cloudfoundry · May 13, 2020 · 7354a8f · 7354a8f
1 parent 09a4458
commit 7354a8f
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 105 deletions.
diff --git a/src/frontend/packages/core/src/core/core.module.ts b/src/frontend/packages/core/src/core/core.module.ts
@@ -1,22 +1,24 @@
 import { PortalModule } from '@angular/cdk/portal';
 import { NgModule } from '@angular/core';
 import { FormsModule, ReactiveFormsModule } from '@angular/forms';
-import { RouterModule } from '@angular/router';
 import { Title } from '@angular/platform-browser';
+import { RouterModule } from '@angular/router';
 import { MomentModule } from 'ngx-moment';
 
+import { EntityServiceFactory } from '../../../store/src/entity-service-factory.service';
 import { NoContentMessageComponent } from '../shared/components/no-content-message/no-content-message.component';
 import { RecentEntitiesComponent } from '../shared/components/recent-entities/recent-entities.component';
+import { UserAvatarComponent } from './../shared/components/user-avatar/user-avatar.component';
 import { AuthGuardService } from './auth-guard.service';
 import { ButtonBlurOnClickDirective } from './button-blur-on-click.directive';
 import { BytesToHumanSize, MegaBytesToHumanSize } from './byte-formatters.pipe';
 import { ClickStopPropagationDirective } from './click-stop-propagation.directive';
+import { APP_TITLE, appTitleFactory } from './core.types';
 import { CurrentUserPermissionsService } from './current-user-permissions.service';
 import { DisableRouterLinkDirective } from './disable-router-link.directive';
 import { DotContentComponent } from './dot-content/dot-content.component';
 import { EndpointsService } from './endpoints.service';
 import { EntityFavoriteStarComponent } from './entity-favorite-star/entity-favorite-star.component';
-import { EntityServiceFactory } from '../../../store/src/entity-service-factory.service';
 import { EventWatcherService } from './event-watcher/event-watcher.service';
 import { InfinityPipe } from './infinity.pipe';
 import { LogOutDialogComponent } from './log-out-dialog/log-out-dialog.component';
@@ -28,12 +30,10 @@ import { PageNotFoundComponentComponent } from './page-not-found-component/page-
 import { SafeImgPipe } from './safe-img.pipe';
 import { StatefulIconComponent } from './stateful-icon/stateful-icon.component';
 import { TruncatePipe } from './truncate.pipe';
+import { UserProfileService } from './user-profile.service';
 import { UserService } from './user.service';
 import { UtilsService } from './utils.service';
 import { WindowRef } from './window-ref/window-ref.service';
-import { APP_TITLE, appTitleFactory } from './core.types';
-import { UserProfileService } from './user-profile.service';
-import { UserAvatarComponent } from './../shared/components/user-avatar/user-avatar.component';
 
 @NgModule({
   imports: [
@@ -76,8 +76,8 @@ import { UserAvatarComponent } from './../shared/components/user-avatar/user-ava
     LoggerService,
     EndpointsService,
     UserService,
-    EntityServiceFactory,
     UserProfileService,
+    EntityServiceFactory,
     CurrentUserPermissionsService,
     {
       provide: APP_TITLE,

diff --git a/src/frontend/packages/core/src/core/user-profile.service.ts b/src/frontend/packages/core/src/core/user-profile.service.ts
@@ -1,30 +1,23 @@
 import { Injectable } from '@angular/core';
 import { Store } from '@ngrx/store';
 import { combineLatest, Observable, of as observableOf, of } from 'rxjs';
-import { filter, first, map } from 'rxjs/operators';
+import { filter, first, map, publishReplay, refCount, switchMap } from 'rxjs/operators';
 
 import {
   FetchUserProfileAction,
   UpdateUserPasswordAction,
   UpdateUserProfileAction,
 } from '../../../store/src/actions/user-profile.actions';
 import { AppState } from '../../../store/src/app-state';
-import { UserProfileEffect, userProfilePasswordUpdatingKey } from '../../../store/src/effects/user-profile.effects';
-import {
-  ActionState,
-  getDefaultActionState,
-  rootUpdatingKey,
-} from '../../../store/src/reducers/api-request-reducer/types';
+import { userProfilePasswordUpdatingKey } from '../../../store/src/effects/user-profile.effects';
+import { entityCatalog } from '../../../store/src/entity-catalog/entity-catalog.service';
+import { EntityService } from '../../../store/src/entity-service';
+import { EntityServiceFactory } from '../../../store/src/entity-service-factory.service';
+import { ActionState, getDefaultActionState, rootUpdatingKey } from '../../../store/src/reducers/api-request-reducer/types';
 import { AuthState } from '../../../store/src/reducers/auth.reducer';
 import { selectRequestInfo, selectUpdateInfo } from '../../../store/src/selectors/api.selectors';
-import {
-  UserProfileInfo,
-  UserProfileInfoEmail,
-  UserProfileInfoUpdates,
-} from '../../../store/src/types/user-profile.types';
+import { UserProfileInfo, UserProfileInfoEmail, UserProfileInfoUpdates } from '../../../store/src/types/user-profile.types';
 import { userProfileEntitySchema } from '../base-entity-schemas';
-import { entityCatalog } from '../../../store/src/entity-catalog/entity-catalog.service';
-import { EntityMonitor } from '../../../store/src/monitors/entity-monitor';
 
 
 @Injectable()
@@ -34,40 +27,57 @@ export class UserProfileService {
 
   isFetching$: Observable<boolean>;
 
-  entityMonitor: EntityMonitor<UserProfileInfo>;
+  entityService: Observable<EntityService<UserProfileInfo>>;
 
   userProfile$: Observable<UserProfileInfo>;
 
   private stratosUserConfig = entityCatalog.getEntity(userProfileEntitySchema.endpointType, userProfileEntitySchema.entityType);
 
-  constructor(private store: Store<AppState>) {
+  constructor(
+    private store: Store<AppState>,
+    esf: EntityServiceFactory
+  ) {
     if (!this.stratosUserConfig) {
       console.error('Can not get user profile entity');
       this.userProfile$ = of({} as UserProfileInfo);
       return;
     }
 
-    this.entityMonitor = this.stratosUserConfig.getEntityMonitor(this.store, UserProfileEffect.guid);
+    this.entityService = this.createFetchUserAction().pipe(
+      first(),
+      map(action => esf.create<UserProfileInfo>(action.guid, action)),
+      publishReplay(1),
+      refCount()
+    );
 
-    this.userProfile$ = this.entityMonitor.entity$.pipe(
+    this.userProfile$ = this.entityService.pipe(
+      switchMap(service => service.waitForEntity$),
+      map(({ entity }) => entity),
       filter(data => data && !!data.id)
     );
-    this.isFetching$ = this.entityMonitor.isFetchingEntity$;
+    this.isFetching$ = this.entityService.pipe(
+      switchMap(service => service.isFetchingEntity$)
+    );
 
-    this.isError$ = this.store.select(selectRequestInfo(this.stratosUserConfig.entityKey, UserProfileEffect.guid)).pipe(
+    this.isError$ = this.store.select(selectRequestInfo(this.stratosUserConfig.entityKey, FetchUserProfileAction.guid)).pipe(
       filter(requestInfo => !!requestInfo && !requestInfo.fetching),
       map(requestInfo => requestInfo.error)
     );
   }
 
-  fetchUserProfile() {
-    // Once we have the user's guid, fetch their profile
-    this.store.select(s => s.auth).pipe(
+  private createFetchUserAction(): Observable<FetchUserProfileAction> {
+    return this.store.select(s => s.auth).pipe(
       filter((auth: AuthState) => !!(auth && auth.sessionData)),
       map((auth: AuthState) => auth.sessionData),
-      first()
-    ).subscribe(data => {
-      this.store.dispatch(new FetchUserProfileAction(data.user.guid));
+      first(),
+      map(data => new FetchUserProfileAction(data.user.guid))
+    );
+  }
+
+  fetchUserProfile() {
+    // Once we have the user's guid, fetch their profile
+    this.createFetchUserAction().pipe(first()).subscribe(action => {
+      this.store.dispatch(action);
     });
   }
 
@@ -99,7 +109,7 @@ export class UserProfileService {
   updateProfile(profile: UserProfileInfo, profileChanges: UserProfileInfoUpdates): Observable<[ActionState, ActionState]> {
     const didChangeProfile = (profileChanges.givenName !== undefined ||
       profileChanges.familyName !== undefined ||
-      profileChanges.emailAddress !== undefined );
+      profileChanges.emailAddress !== undefined);
     const didChangePassword = !!(profileChanges.newPassword && profileChanges.currentPassword);
     const profileObs$ = didChangeProfile ? this.updateProfileInfo(profile, profileChanges) : observableOf(getDefaultActionState());
     const passwordObs$ = didChangePassword ? this.updatePassword(profile, profileChanges) : observableOf(getDefaultActionState());
@@ -125,7 +135,7 @@ export class UserProfileService {
     }
     this.store.dispatch(new UpdateUserProfileAction(updatedProfile, profileChanges.currentPassword));
     const actionState = selectUpdateInfo(this.stratosUserConfig.entityKey,
-      UserProfileEffect.guid,
+      FetchUserProfileAction.guid,
       rootUpdatingKey);
     return this.store.select(actionState).pipe(
       filter(item => item && !item.busy)
@@ -139,7 +149,7 @@ export class UserProfileService {
     };
     this.store.dispatch(new UpdateUserPasswordAction(profile.id, passwordUpdates));
     const actionState = selectUpdateInfo(this.stratosUserConfig.entityKey,
-      UserProfileEffect.guid,
+      FetchUserProfileAction.guid,
       userProfilePasswordUpdatingKey);
     return this.store.select(actionState).pipe(
       filter(item => item && !item.busy)

diff --git a/src/frontend/packages/core/src/features/metrics/metrics.helpers.ts b/src/frontend/packages/core/src/features/metrics/metrics.helpers.ts
@@ -22,37 +22,38 @@ export interface MetricsEndpointInfo {
 // Process the endpoint and Stratos marker file data to give a single list of endpoitns
 // linked to this metrics endpoint, comprising those that are known in Stratos and those that are not
 export function mapMetricsData(ep: MetricsEndpointProvider): MetricsEndpointInfo[] {
-    const data: MetricsEndpointInfo[] = [];
+  const data: MetricsEndpointInfo[] = [];
 
-    // Add all of the known endpoints first
-    ep.endpoints.forEach(endpoint => {
-      const catalogEndpoint = entityCatalog.getEndpoint(endpoint.cnsi_type, endpoint.sub_type);
+  // Add all of the known endpoints first
+  ep.endpoints.forEach(endpoint => {
+    const catalogEndpoint = entityCatalog.getEndpoint(endpoint.cnsi_type, endpoint.sub_type);
 
-      data.push({
-        known: true,
-        name: endpoint.name,
-        url: getFullEndpointApiUrl(endpoint),
-        type: catalogEndpoint.definition.label,
-        icon: {
-          name: catalogEndpoint.definition.icon,
-          font: 'stratos-icons'
-        },
-        metadata: {
-          metrics_job: endpoint.metadata ? endpoint.metadata.metrics_job : null,
-          metrics_environment: endpoint.metadata ? endpoint.metadata.metrics_environment : null
-        },
-        status: observableOf(StratosStatus.OK)
-      });
+    data.push({
+      known: true,
+      name: endpoint.name,
+      url: getFullEndpointApiUrl(endpoint),
+      type: catalogEndpoint.definition.label,
+      icon: {
+        name: catalogEndpoint.definition.icon,
+        font: 'stratos-icons'
+      },
+      metadata: {
+        metrics_job: endpoint.metadata ? endpoint.metadata.metrics_job : null,
+        metrics_environment: endpoint.metadata ? endpoint.metadata.metrics_environment : null
+      },
+      status: observableOf(StratosStatus.OK)
     });
+  });
 
-    // Add all of the potentially unknown endpoints
-    if (ep.provider && ep.provider.metadata && ep.provider.metadata && ep.provider.metadata.metrics_stratos
+  // Add all of the potentially unknown endpoints
+  if (ep.provider && ep.provider.metadata && ep.provider.metadata && ep.provider.metadata.metrics_stratos
     && Array.isArray(ep.provider.metadata.metrics_stratos)) {
-      ep.provider.metadata.metrics_stratos.forEach(endp => {
-        // See if we already know about this endpoint
-        const hasEndpoint = data.findIndex(i => i.url === endp.url || i.url === endp.cfEndpoint) !== -1;
-        if (!hasEndpoint) {
-          const catalogEndpoint = entityCatalog.getEndpoint(endp.type, '');
+    ep.provider.metadata.metrics_stratos.forEach(endp => {
+      // See if we already know about this endpoint
+      const hasEndpoint = data.findIndex(i => i.url === endp.url || i.url === endp.cfEndpoint) !== -1;
+      if (!hasEndpoint) {
+        const catalogEndpoint = entityCatalog.getEndpoint(endp.type, '');
+        if (catalogEndpoint) { // Provider metadata could give k8 endpoint
           data.push({
             known: false,
             name: '<Unregistered Endpoint>',
@@ -68,7 +69,9 @@ export function mapMetricsData(ep: MetricsEndpointProvider): MetricsEndpointInfo
             status: observableOf(StratosStatus.WARNING)
           });
         }
-      });
-    }
-    return data;
+
+      }
+    });
   }
+  return data;
+}
diff --git a/.../packages/core/src/features/user-profile/edit-profile-info/edit-profile-info.component.ts b/.../packages/core/src/features/user-profile/edit-profile-info/edit-profile-info.component.ts
@@ -2,13 +2,13 @@ import { Component, OnDestroy, OnInit } from '@angular/core';
 import { AbstractControl, FormBuilder, FormGroup, ValidatorFn, Validators } from '@angular/forms';
 import { ErrorStateMatcher, ShowOnDirtyErrorStateMatcher } from '@angular/material/core';
 import { Subscription } from 'rxjs';
-import { first, map, take } from 'rxjs/operators';
+import { first, map, take, tap } from 'rxjs/operators';
 
 import { UserProfileInfo, UserProfileInfoUpdates } from '../../../../../store/src/types/user-profile.types';
 import { CurrentUserPermissions } from '../../../core/current-user-permissions.config';
 import { CurrentUserPermissionsService } from '../../../core/current-user-permissions.service';
-import { StepOnNextFunction } from '../../../shared/components/stepper/step/step.component';
 import { UserProfileService } from '../../../core/user-profile.service';
+import { StepOnNextFunction } from '../../../shared/components/stepper/step/step.component';
 
 
 @Component({
@@ -58,7 +58,6 @@ export class EditProfileInfoComponent implements OnInit, OnDestroy {
   public passwordRequired = false;
 
   ngOnInit() {
-    this.userProfileService.fetchUserProfile();
     this.userProfileService.userProfile$.pipe(first()).subscribe(profile => {
       // UAA needs the user's password for email changes. Local user does not
       // Both need it for password change
@@ -131,6 +130,8 @@ export class EditProfileInfoComponent implements OnInit, OnDestroy {
           redirect: okay,
           message: okay ? '' : `An error occurred whilst updating your profile: ${message}`
         };
-      }));
+      }),
+      tap(() => this.userProfileService.fetchUserProfile())
+    );
   }
 }
diff --git a/src/frontend/packages/core/src/features/user-profile/profile-info/profile-info.component.ts b/src/frontend/packages/core/src/features/user-profile/profile-info/profile-info.component.ts
@@ -1,5 +1,4 @@
-import { SetGravatarEnabledAction } from './../../../../../store/src/actions/dashboard-actions';
-import { Component, OnInit } from '@angular/core';
+import { Component } from '@angular/core';
 import { Store } from '@ngrx/store';
 import { Observable } from 'rxjs';
 import { map } from 'rxjs/operators';
@@ -9,15 +8,16 @@ import { DashboardOnlyAppState } from '../../../../../store/src/app-state';
 import { selectDashboardState } from '../../../../../store/src/selectors/dashboard.selectors';
 import { UserProfileInfo } from '../../../../../store/src/types/user-profile.types';
 import { ThemeService } from '../../../core/theme.service';
-import { UserService } from '../../../core/user.service';
 import { UserProfileService } from '../../../core/user-profile.service';
+import { UserService } from '../../../core/user.service';
+import { SetGravatarEnabledAction } from './../../../../../store/src/actions/dashboard-actions';
 
 @Component({
   selector: 'app-profile-info',
   templateUrl: './profile-info.component.html',
   styleUrls: ['./profile-info.component.scss']
 })
-export class ProfileInfoComponent implements OnInit {
+export class ProfileInfoComponent {
 
   public timeoutSession$ = this.store.select(selectDashboardState).pipe(
     map(dashboardState => dashboardState.timeoutSession ? 'true' : 'false')
@@ -31,7 +31,7 @@ export class ProfileInfoComponent implements OnInit {
     map(dashboardState => dashboardState.gravatarEnabled ? 'true' : 'false')
   );
 
-  public allowGravatar$ =  this.store.select(selectDashboardState).pipe(
+  public allowGravatar$ = this.store.select(selectDashboardState).pipe(
     map(dashboardState => dashboardState.gravatarEnabled)
   );
 
@@ -85,8 +85,4 @@ export class ProfileInfoComponent implements OnInit {
     this.hasMultipleThemes = themeService.getThemes().length > 1;
   }
 
-  ngOnInit() {
-    this.userProfileService.fetchUserProfile();
-  }
-
 }
diff --git a/src/frontend/packages/core/src/shared/components/page-header/page-header.component.ts b/src/frontend/packages/core/src/shared/components/page-header/page-header.component.ts
@@ -1,4 +1,3 @@
-import { selectDashboardState } from './../../../../../store/src/selectors/dashboard.selectors';
 import { TemplatePortal } from '@angular/cdk/portal';
 import { AfterViewInit, Component, Input, OnDestroy, TemplateRef, ViewChild } from '@angular/core';
 import { ActivatedRoute, Router } from '@angular/router';
@@ -11,18 +10,18 @@ import { Logout } from '../../../../../store/src/actions/auth.actions';
 import { ToggleSideNav } from '../../../../../store/src/actions/dashboard-actions';
 import { AddRecentlyVisitedEntityAction } from '../../../../../store/src/actions/recently-visited.actions';
 import { EntityCatalogHelpers } from '../../../../../store/src/entity-catalog/entity-catalog.helper';
-import { AuthState } from '../../../../../store/src/reducers/auth.reducer';
 import { selectIsMobile } from '../../../../../store/src/selectors/dashboard.selectors';
 import { InternalEventSeverity } from '../../../../../store/src/types/internal-events.types';
 import { IFavoriteMetadata, UserFavorite } from '../../../../../store/src/types/user-favorites.types';
 import { TabNavService } from '../../../../tab-nav.service';
+import { UserProfileService } from '../../../core/user-profile.service';
 import { IPageSideNavTab } from '../../../features/dashboard/page-side-nav/page-side-nav.component';
 import { GlobalEventService, IGlobalEvent } from '../../global-events.service';
 import { StratosStatus } from '../../shared.types';
 import { FavoritesConfigMapper } from '../favorites-meta-card/favorite-config-mapper';
-import { BREADCRUMB_URL_PARAM, IHeaderBreadcrumb, IHeaderBreadcrumbLink } from './page-header.types';
-import { UserProfileService } from '../../../core/user-profile.service';
+import { selectDashboardState } from './../../../../../store/src/selectors/dashboard.selectors';
 import { UserProfileInfo } from './../../../../../store/src/types/user-profile.types';
+import { BREADCRUMB_URL_PARAM, IHeaderBreadcrumb, IHeaderBreadcrumbLink } from './page-header.types';
 
 @Component({
   selector: 'app-page-header',
@@ -170,7 +169,6 @@ export class PageHeaderComponent implements OnDestroy, AfterViewInit {
     this.breadcrumbKey = route.snapshot.queryParams[BREADCRUMB_URL_PARAM] || null;
 
     this.user$ = this.userProfileService.userProfile$;
-    this.userProfileService.fetchUserProfile();
 
     this.username$ = this.user$.pipe(
       map(profile => {