From e80d818c79afb5a682cf0d7e0322ee04973206f9 Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Mon, 23 Sep 2019 16:54:27 +0100 Subject: [PATCH 1/2] Ensure multi endpoint requests only go out to connected/unconnectable endpoints - Fixes #3887 --- .../base-single-entity-request.pipeline.ts | 3 +- .../entity-pagination-request-pipeline.ts | 3 +- .../make-request-entity-request.pipe.ts | 12 ++++--- .../entity-request-pipeline.ts | 6 ++-- .../entity-request-pipeline.types.ts | 7 ++-- .../pagination-iterator.pipe.ts | 2 +- .../pipline-http-client.service.ts | 31 +++++++++++++----- .../store/src/selectors/endpoint.selectors.ts | 32 ++++++++++++++----- 8 files changed, 67 insertions(+), 29 deletions(-) diff --git a/src/frontend/packages/store/src/entity-request-pipeline/base-single-entity-request.pipeline.ts b/src/frontend/packages/store/src/entity-request-pipeline/base-single-entity-request.pipeline.ts index 0cbff41dfe..b708b1deaa 100644 --- a/src/frontend/packages/store/src/entity-request-pipeline/base-single-entity-request.pipeline.ts +++ b/src/frontend/packages/store/src/entity-request-pipeline/base-single-entity-request.pipeline.ts @@ -3,6 +3,7 @@ import { Observable } from 'rxjs'; import { map } from 'rxjs/operators'; import { StratosBaseCatalogueEntity } from '../../../core/src/core/entity-catalogue/entity-catalogue-entity'; +import { entityCatalogue } from '../../../core/src/core/entity-catalogue/entity-catalogue.service'; import { IStratosEntityDefinition } from '../../../core/src/core/entity-catalogue/entity-catalogue.types'; import { AppState, InternalAppState } from '../app-state'; import { EntityRequestAction } from '../types/request.types'; @@ -36,7 +37,7 @@ export const baseRequestPipelineFactory: EntityRequestPipeline = ( return makeRequestEntityPipe( httpClient, request, - action.endpointType, + entityCatalogue.getEndpoint(action.endpointType, action.subType), action.endpointGuid, action.externalRequest ).pipe( diff --git a/src/frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts b/src/frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts index 387fa70ae0..5bd62130e0 100644 --- a/src/frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts +++ b/src/frontend/packages/store/src/entity-request-pipeline/entity-pagination-request-pipeline.ts @@ -7,6 +7,7 @@ import { StratosBaseCatalogueEntity, StratosCatalogueEntity, } from '../../../core/src/core/entity-catalogue/entity-catalogue-entity'; +import { entityCatalogue } from '../../../core/src/core/entity-catalogue/entity-catalogue.service'; import { IStratosEntityDefinition } from '../../../core/src/core/entity-catalogue/entity-catalogue.types'; import { AppState, InternalAppState } from '../app-state'; import { PaginationFlattenerConfig } from '../helpers/paginated-request-helpers'; @@ -44,7 +45,7 @@ function getRequestObservable( const initialRequest = makeRequestEntityPipe( httpClient, request, - action.endpointType, + entityCatalogue.getEndpoint(action.endpointType, action.subType), action.endpointGuid, action.externalRequest ); diff --git a/src/frontend/packages/store/src/entity-request-pipeline/entity-request-base-handlers/make-request-entity-request.pipe.ts b/src/frontend/packages/store/src/entity-request-pipeline/entity-request-base-handlers/make-request-entity-request.pipe.ts index 122b842493..657afac20c 100644 --- a/src/frontend/packages/store/src/entity-request-pipeline/entity-request-base-handlers/make-request-entity-request.pipe.ts +++ b/src/frontend/packages/store/src/entity-request-pipeline/entity-request-base-handlers/make-request-entity-request.pipe.ts @@ -1,18 +1,20 @@ -import { MakeEntityRequestPipe } from '../entity-request-pipeline.types'; import { HttpRequest } from '@angular/common/http'; import { switchMap } from 'rxjs/operators'; +import { StratosCatalogueEndpointEntity } from '../../../../core/src/core/entity-catalogue/entity-catalogue-entity'; +import { MakeEntityRequestPipe } from '../entity-request-pipeline.types'; + export const makeRequestEntityPipe: MakeEntityRequestPipe = ( httpClient, requestOrObservable, - endpointType, - endpointGuids, + endpointConfig: StratosCatalogueEndpointEntity, + endpointGuids: string[], externalRequest: boolean = false ) => { if (requestOrObservable instanceof HttpRequest) { return httpClient.pipelineRequest( requestOrObservable, - endpointType, + endpointConfig, endpointGuids, externalRequest ); @@ -20,7 +22,7 @@ export const makeRequestEntityPipe: MakeEntityRequestPipe = ( return requestOrObservable.pipe( switchMap(request => httpClient.pipelineRequest( request, - endpointType, + endpointConfig, endpointGuids, externalRequest )) diff --git a/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.ts b/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.ts index f1aa7a0d96..097adb2977 100644 --- a/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.ts +++ b/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.ts @@ -7,15 +7,15 @@ import { entityCatalogue } from '../../../core/src/core/entity-catalogue/entity- import { AppState, InternalAppState } from '../app-state'; import { RecursiveDelete } from '../effects/recursive-entity-delete.effect'; import { ApiRequestTypes, getRequestTypeFromMethod } from '../reducers/api-request-reducer/request-helpers'; +import { PaginatedAction } from '../types/pagination.types'; import { EntityRequestAction } from '../types/request.types'; import { failedEntityHandler } from './entity-request-base-handlers/fail-entity-request.handler'; +import { patchActionWithForcedConfig } from './entity-request-base-handlers/forced-action-type.helpers'; import { jetstreamErrorHandler } from './entity-request-base-handlers/jetstream-error.handler'; import { startEntityHandler } from './entity-request-base-handlers/start-entity-request.handler'; import { successEntityHandler } from './entity-request-base-handlers/success-entity-request.handler'; import { EntityRequestPipeline, PreApiRequest, SuccessfulApiResponseDataMapper } from './entity-request-pipeline.types'; import { PipelineHttpClient } from './pipline-http-client.service'; -import { patchActionWithForcedConfig } from './entity-request-base-handlers/forced-action-type.helpers'; -import { PaginatedAction } from '../types/pagination.types'; export interface PipelineFactoryConfig { store: Store; @@ -46,7 +46,7 @@ export const apiRequestPipelineFactory = ( const actionDispatcher = (actionToDispatch: Action) => store.dispatch(actionToDispatch); const requestType = getRequestTypeFromMethod(patchedAction); - const catalogueEntity = entityCatalogue.getEntity(patchedAction.endpointType, patchedAction.entityType); + const catalogueEntity = entityCatalogue.getEntity(patchedAction); const recursivelyDelete = shouldRecursivelyDelete(requestType, patchedAction); if (recursivelyDelete) { diff --git a/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.types.ts b/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.types.ts index e5af76edad..b1238b410f 100644 --- a/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.types.ts +++ b/src/frontend/packages/store/src/entity-request-pipeline/entity-request-pipeline.types.ts @@ -3,7 +3,10 @@ import { RequestOptions } from '@angular/http'; import { Action, Store } from '@ngrx/store'; import { Observable } from 'rxjs'; -import { StratosBaseCatalogueEntity } from '../../../core/src/core/entity-catalogue/entity-catalogue-entity'; +import { + StratosBaseCatalogueEntity, + StratosCatalogueEndpointEntity, +} from '../../../core/src/core/entity-catalogue/entity-catalogue-entity'; import { JetStreamErrorResponse } from '../../../core/src/jetstream.helpers'; import { AppState, InternalAppState } from '../app-state'; import { ApiRequestTypes } from '../reducers/api-request-reducer/request-helpers'; @@ -47,7 +50,7 @@ export type MakeEntityRequestPipe< > = ( httpClient: PipelineHttpClient, request: HttpRequest | Observable>, - endpointType: string, + endpointConfig: StratosCatalogueEndpointEntity, endpointGuids: string | string[], externalRequest?: boolean ) => Observable>; diff --git a/src/frontend/packages/store/src/entity-request-pipeline/pagination-request-base-handlers/pagination-iterator.pipe.ts b/src/frontend/packages/store/src/entity-request-pipeline/pagination-request-base-handlers/pagination-iterator.pipe.ts index b410111966..9165d3bbbe 100644 --- a/src/frontend/packages/store/src/entity-request-pipeline/pagination-request-base-handlers/pagination-iterator.pipe.ts +++ b/src/frontend/packages/store/src/entity-request-pipeline/pagination-request-base-handlers/pagination-iterator.pipe.ts @@ -35,7 +35,7 @@ export class PaginationPageIterator { private makeRequest(httpRequest: HttpRequest>) { return this.httpClient.pipelineRequest>( httpRequest, - this.action.endpointType, + entityCatalogue.getEndpoint(this.action.endpointType, this.action.subType), this.action.endpointGuid, this.action.externalRequest ); diff --git a/src/frontend/packages/store/src/entity-request-pipeline/pipline-http-client.service.ts b/src/frontend/packages/store/src/entity-request-pipeline/pipline-http-client.service.ts index 21ddc2fb6a..272fa82b43 100644 --- a/src/frontend/packages/store/src/entity-request-pipeline/pipline-http-client.service.ts +++ b/src/frontend/packages/store/src/entity-request-pipeline/pipline-http-client.service.ts @@ -3,9 +3,12 @@ import { Injectable } from '@angular/core'; import { Store } from '@ngrx/store'; import { Observable } from 'rxjs'; import { filter, first, map, mergeMap } from 'rxjs/operators'; -import { InternalAppState } from '../app-state'; -import { registeredEndpointsOfTypesSelector } from '../selectors/endpoint.selectors'; + +import { StratosCatalogueEndpointEntity } from '../../../core/src/core/entity-catalogue/entity-catalogue-entity'; +import { IStratosEndpointDefinition } from '../../../core/src/core/entity-catalogue/entity-catalogue.types'; import { environment } from '../../../core/src/environments/environment'; +import { InternalAppState } from '../app-state'; +import { connectedEndpointsOfTypesSelector, registeredEndpointsOfTypesSelector } from '../selectors/endpoint.selectors'; const { proxyAPIVersion, cfAPIVersion } = environment; @@ -19,20 +22,32 @@ export class PipelineHttpClient { private store: Store, ) { } - private makeRequest(hr: HttpRequest, endpointType: string, endpointGuids: string | string[] = null, externalRequest = false) { + private makeRequest( + hr: HttpRequest, + endpointConfig: IStratosEndpointDefinition, + endpointGuids: string | string[] = null, + externalRequest = false + ) { if (externalRequest) { return this.externalRequest(hr); } - return this.jetstreamRequest(hr, endpointType, endpointGuids); + return this.jetstreamRequest(hr, endpointConfig, endpointGuids); } - private jetstreamRequest(hr: HttpRequest, endpointType: string, endpointGuids: string | string[]) { + private jetstreamRequest( + hr: HttpRequest, + endpointConfig: IStratosEndpointDefinition, + endpointGuids: string | string[]) { const url = `/pp/${proxyAPIVersion}/proxy/${cfAPIVersion}/${hr.url}`; if (endpointGuids && endpointGuids.length) { const headers = hr.headers.set(PipelineHttpClient.EndpointHeader, endpointGuids); return this.httpClient.request(hr.clone({ headers, url })); } else { - return this.store.select(registeredEndpointsOfTypesSelector(endpointType)).pipe( + const selector = endpointConfig.unConnectable ? + registeredEndpointsOfTypesSelector(endpointConfig.type) : + connectedEndpointsOfTypesSelector(endpointConfig.type); + + return this.store.select(selector).pipe( first(), mergeMap(endpoints => { const headers = hr.headers.set(PipelineHttpClient.EndpointHeader, Object.keys(endpoints)); @@ -48,11 +63,11 @@ export class PipelineHttpClient { public pipelineRequest( hr: HttpRequest, - endpointType: string, + endpointConfig: StratosCatalogueEndpointEntity, endpointGuids: string | string[] = null, externalRequest = false ): Observable { - return this.makeRequest(hr, endpointType, endpointGuids, externalRequest).pipe( + return this.makeRequest(hr, endpointConfig.definition, endpointGuids, externalRequest).pipe( filter(event => event instanceof HttpResponse), map((response: HttpResponse) => response.body) ); diff --git a/src/frontend/packages/store/src/selectors/endpoint.selectors.ts b/src/frontend/packages/store/src/selectors/endpoint.selectors.ts index b00b2233b3..e6f43d2ea5 100644 --- a/src/frontend/packages/store/src/selectors/endpoint.selectors.ts +++ b/src/frontend/packages/store/src/selectors/endpoint.selectors.ts @@ -1,4 +1,4 @@ -import { createSelector } from '@ngrx/store'; +import { compose, createSelector } from '@ngrx/store'; import { STRATOS_ENDPOINT_TYPE } from '../../../core/src/base-entity-schemas'; import { EntityCatalogueHelpers } from '../../../core/src/core/entity-catalogue/entity-catalogue.helper'; @@ -24,36 +24,52 @@ export const endpointOfTypeSelector = (type: string) => return endpointsOfType; }, {}); }; -// TODO More this #3769 + +// TODO: Move this #3769 export const cfEndpointEntitiesSelector = endpointOfTypeSelector('cf'); -export const getRegisteredEndpoints = (endpoints: IRequestEntityTypeState) => { - const registered = {} as IRequestEntityTypeState; - Object.values(endpoints).map(endpoint => { +export const getRegisteredEndpoints = (endpoints: IRequestEntityTypeState) => + Object.values(endpoints).reduce((registered, endpoint) => { if (endpoint.registered) { registered[endpoint.guid] = endpoint; } return registered; - }); - return registered; -}; + }, {} as IRequestEntityTypeState); + +export const getConnectedEndpoints = (endpoints: IRequestEntityTypeState) => + Object.values(endpoints).reduce((connected, endpoint) => { + if (endpoint.connectionStatus === 'connected') { + connected[endpoint.guid] = endpoint; + } + return connected; + }, {} as IRequestEntityTypeState); + // All Registered endpoint request data export const endpointsRegisteredEntitiesSelector = createSelector( endpointEntitiesSelector, getRegisteredEndpoints ); +export const connectedEndpointsOfTypesSelector = (endpointType: string) => compose( + getConnectedEndpoints, + endpointOfTypeSelector(endpointType), + getRegisteredEndpoints, + endpointEntitiesSelector, +); + export const registeredEndpointsOfTypesSelector = (endpointType: string) => createSelector( endpointEntitiesSelector, endpointOfTypeSelector(endpointType), getRegisteredEndpoints ); +// TODO: Move this #3769 export const endpointsCFEntitiesSelector = createSelector( endpointEntitiesSelector, cfEndpointEntitiesSelector ); +// TODO: Move this #3769 export const endpointsRegisteredCFEntitiesSelector = createSelector( endpointsCFEntitiesSelector, getRegisteredEndpoints From 0659b5a5be9c2d55992fa47fdca3251b5df5e7c0 Mon Sep 17 00:00:00 2001 From: Richard Cox Date: Mon, 23 Sep 2019 17:45:56 +0100 Subject: [PATCH 2/2] Fix two actionBuilder params issues - Fix three places where `includeRelations` were passed incorrectly to actionBuilder - Fix bind service --- .../add-service-instance/csi-mode.service.ts | 2 +- .../app-service-binding-data-source.ts | 28 ++++++++-------- ...cf-spaces-service-instances-data-source.ts | 32 ++++++++++--------- .../cf-org-space-service.service.ts | 9 ++++-- 4 files changed, 39 insertions(+), 32 deletions(-) diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/csi-mode.service.ts b/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/csi-mode.service.ts index a7b230244a..634c4b54df 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/csi-mode.service.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/add-service-instance/csi-mode.service.ts @@ -130,8 +130,8 @@ export class CsiModeService { const servceBindingEntity = entityCatalogue.getEntity(CF_ENDPOINT_TYPE, serviceBindingEntityType); const actionBuilder = servceBindingEntity.actionOrchestrator.getActionBuilder('create'); const createServiceBindingAction = actionBuilder( - cfGuid, guid, + cfGuid, { applicationGuid: appGuid, serviceInstanceGuid, params } ); this.store.dispatch(createServiceBindingAction); diff --git a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-data-source.ts b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-data-source.ts index 8f3479bc96..ed612953d8 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-data-source.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/components/list/list-types/app-sevice-bindings/app-service-binding-data-source.ts @@ -1,6 +1,5 @@ import { Store } from '@ngrx/store'; -import { GetAppServiceBindings } from '../../../../../../../cloud-foundry/src/actions/application-service-routes.actions'; import { CFAppState } from '../../../../../../../cloud-foundry/src/cf-app-state'; import { applicationEntityType, @@ -10,21 +9,21 @@ import { serviceInstancesEntityType, servicePlanEntityType, } from '../../../../../../../cloud-foundry/src/cf-entity-factory'; +import { + createEntityRelationKey, + createEntityRelationPaginationKey, +} from '../../../../../../../cloud-foundry/src/entity-relations/entity-relations.types'; import { ApplicationService } from '../../../../../../../cloud-foundry/src/features/applications/application.service'; import { getRowMetadata } from '../../../../../../../cloud-foundry/src/features/cloud-foundry/cf.helpers'; import { IServiceBinding } from '../../../../../../../core/src/core/cf-api-svc.types'; +import { entityCatalogue } from '../../../../../../../core/src/core/entity-catalogue/entity-catalogue.service'; import { ListDataSource, } from '../../../../../../../core/src/shared/components/list/data-sources-controllers/list-data-source'; import { IListConfig } from '../../../../../../../core/src/shared/components/list/list.component.types'; -import { - createEntityRelationKey, - createEntityRelationPaginationKey, -} from '../../../../../../../cloud-foundry/src/entity-relations/entity-relations.types'; import { APIResource } from '../../../../../../../store/src/types/api.types'; -import { entityCatalogue } from '../../../../../../../core/src/core/entity-catalogue/entity-catalogue.service'; -import { CF_ENDPOINT_TYPE } from '../../../../../../cf-types'; import { PaginatedAction } from '../../../../../../../store/src/types/pagination.types'; +import { CF_ENDPOINT_TYPE } from '../../../../../../cf-types'; export class AppServiceBindingDataSource extends ListDataSource> { static createGetAllServiceBindings(appGuid: string, cfGuid: string) { @@ -32,12 +31,15 @@ export class AppServiceBindingDataSource extends ListDataSource { constructor(cfGuid: string, spaceGuid: string, store: Store, listConfig?: IListConfig) { const paginationKey = createEntityRelationPaginationKey(spaceEntityType, spaceGuid); const serviceInstanceEntity = entityCatalogue.getEntity(CF_ENDPOINT_TYPE, serviceInstancesEntityType); const actionBuilder = serviceInstanceEntity.actionOrchestrator.getActionBuilder('getAllInSpace'); - const action = actionBuilder(spaceGuid, cfGuid, paginationKey, null, [ - createEntityRelationKey(serviceInstancesEntityType, serviceBindingEntityType), - createEntityRelationKey(serviceInstancesEntityType, serviceEntityType), - createEntityRelationKey(serviceInstancesEntityType, servicePlanEntityType), - createEntityRelationKey(serviceInstancesEntityType, spaceEntityType), - createEntityRelationKey(serviceBindingEntityType, applicationEntityType), - ], true, false) as PaginatedAction; + const action = actionBuilder(spaceGuid, cfGuid, paginationKey, null, { + includeRelations: [ + createEntityRelationKey(serviceInstancesEntityType, serviceBindingEntityType), + createEntityRelationKey(serviceInstancesEntityType, serviceEntityType), + createEntityRelationKey(serviceInstancesEntityType, servicePlanEntityType), + createEntityRelationKey(serviceInstancesEntityType, spaceEntityType), + createEntityRelationKey(serviceBindingEntityType, applicationEntityType), + ], + populateMissing: true + }) as PaginatedAction; super({ store, action, diff --git a/src/frontend/packages/cloud-foundry/src/shared/data-services/cf-org-space-service.service.ts b/src/frontend/packages/cloud-foundry/src/shared/data-services/cf-org-space-service.service.ts index 5480ef6ae9..413643fd67 100644 --- a/src/frontend/packages/cloud-foundry/src/shared/data-services/cf-org-space-service.service.ts +++ b/src/frontend/packages/cloud-foundry/src/shared/data-services/cf-org-space-service.service.ts @@ -299,9 +299,12 @@ export class CfOrgSpaceDataService implements OnDestroy { private createPaginationAction() { const organizationEntity = entityCatalogue.getEntity(CF_ENDPOINT_TYPE, organizationEntityType); const actionBuilder = organizationEntity.actionOrchestrator.getActionBuilder('getMultiple'); - const getAllOrganizationsAction = actionBuilder(null, CfOrgSpaceDataService.CfOrgSpaceServicePaginationKey, [ - createEntityRelationKey(organizationEntityType, spaceEntityType), - ]); + const getAllOrganizationsAction = actionBuilder(null, CfOrgSpaceDataService.CfOrgSpaceServicePaginationKey, { + includeRelations: [ + createEntityRelationKey(organizationEntityType, spaceEntityType), + ], + populateMissing: true + }); return getAllOrganizationsAction; }