When testing effects they are fired twice, which breaks HttpTestingController

[martinroob]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request

What is the current behavior?

I am trying to test an effect following the way described in the documentation.
The effect calls a backend system via HttpClient and I want to test this by using angulars HttpTestingController. But it seams that the effect is called 2 times and the (mock) backend is called 2 times too.

The effect looks like this:

  @Effect({dispatch: false})
  readData$: Observable = this.actions$
    .ofType(READ)
    .switchMap((action: ReadAction) => {
      return this.httpClient.get('/example/data/' + action.payload)
        .map((response: any) => new ReadSucceededAction(response))
        .catch(error => Observable.of(new ReadFailedAction(error)));
    });

The testcase looks like this:

  it('readData$ effect should be initiated by READ action and then call the backend', () => {
    const initiator: ReadAction = new ReadAction('test1');
    actions = new ReplaySubject();
    actions.next(initiator);

    const expectedAction: ReadSucceededAction = new ReadSucceededAction(null);
    effects.readData$.subscribe((action: ReadSucceededAction) => {
      expect(action.type).toBe(expectedAction.type);
      expect(action.payload.message).toBe('a message from backend');
    });
    const req = httpMock.expectOne('/example/data/test1');
    expect(req.request.method).toEqual('GET');
    const responseData = {message: 'a message from backend'};
    req.flush(responseData);
    httpMock.verify();
  });

.. and the test is green.

Now when I duplicated the testcase, the duplicated test case is red with the follwoing error:
Error: Expected no open requests, found 1: GET /example/data/test1
In fact it receives data send by the first testcase and it fails due to "open requests".

Expected behavior:

The effect should only fire once per test case.

Minimal reproduction of the problem with instructions:

There is a demo repo ngrxEffectsFiredTwice containing the code, generated with angular cli. Just run "ng test" to see the failure,

Version of affected browser(s),operating system(s), npm, node and ngrx:

Chrome, Karma, Angular 5.0 like generated by angular cli, ngrc version is 4.1

Other information:

[Arnaud73]

I seam to have the same problem after I migrated from 4.0.5 to 4.1.0.

My Effects seams to be subscribed twice, although they are loaded only once by a EffectsModule.forRoot().

I added some .share() at the end of my effects, but it's a dirty fix that does not fix all side-effects (actions generated by Effects are still duplicated).

[Arnaud73]

I can confirm that reverting @ngrx/store to 4.0.3 and @ngrx/effects to 4.0.5 fixed the problem for me.
I can provide any information of the way Effects are implemented in my app if required.

[martinroob]

share() did not work for me, back to 4.0.5 is not simply possible because demo is angular 5 based.

[brandonroberts]

We'll look into the issue of why this occurs when using EffectsModule.forRoot in your test, but you don't need to use it. Just add the effect class to the providers array and it functions as intended.

[martinroob]

@brandonroberts You are right-When I do not use EffectsModule.forRoot it works as expected.
By the way, In my "real" app I do not see any HTTP requests done twice, everything looks like working fine, only difference seams to be that I do not subscribe to the effect there.

[Arnaud73]

In my situation, the problem arises in the "real" app.

I declare the Store and load Effects only in my app module, with this calls:
StoreModule.forRoot({ plant: plantReducer, division: divisionReducer, workshop: workshopReducer, segment: segmentReducer, workstation: workstationReducer, team: teamReducer, actor: actorReducer, profile: profileReducer, user: userReducer, kpi: kpiReducer, kpiSource: kpiSourceReducer, defect: defectReducer, defectSource: defectSourceReducer, risk: riskReducer, riskSource: riskSourceReducer, synoptic: synopticReducer, note: noteReducer, task: taskReducer, proApwWorkstation: proApwWorkstationReducer, proApwVariant: proApwVariantReducer, proApwImage: proApwImageReducer, proApwActivity: proApwActivityReducer, message: messageReducer }), !environment.production ? StoreDevtoolsModule.instrument({ maxAge: 25 }) : [], EffectsModule.forRoot([ PlantEffects, DivisionEffects, WorkshopEffects, SegmentEffects, WorkstationEffects, TeamEffects, ActorEffects, ProfileEffects, UserEffects, KpiEffects, KpiSourceEffects, DefectEffects, DefectSourceEffects, RiskEffects, RiskSourceEffects, SynopticEffects, NoteEffects, TaskEffects, ProApwWorkstationEffects, ProApwVariantEffects, ProApwImageEffects, ProApwActivityEffects, MessageEffects ]),

Then, effects are derived from this class:
`
import 'rxjs/Rx';

import { Actions, Effect, toPayload } from '@ngrx/effects';
import { BaseActions, TypedAction } from './base.actions';

import { AppState } from '../app.state';
import { CrudEntity } from '../model/crud-entity.model';
import { CrudService } from '../service/crud.service';
import { Message } from '../model/message.model';
import { MessageActions } from './message.actions';
import { MessageFormat } from '../model/message-format.enum';
import { MessageType } from '../model/message-type.enum';
import { Observable } from 'rxjs/Rx';
import { Store } from '@ngrx/store';

export abstract class BaseEffects {

/**
 * Get all entities by criteria from the backend and then ask for fetched entities
 * to be added or replaced in the store.
 */
@Effect() fetchByCriteria$ = this.getFetchByCriteriaEffect();

/**
 * Get entity by id from the backend.
 */
@Effect() fetchById$ = this.getFetchByIdEffect();

/**
 * Create an entity in the backend
 */
@Effect() create$ = this.actions$
    .ofType(this.baseActions.actionTypeTemplate(BaseActions.CREATE))
    .map((action: TypedAction<E>) => action.payload)
    .mergeMap(entity => this.crudService
        .create(entity)
        .mergeMap(createdEntity => {
            return Observable.from([
                this.baseActions.load([createdEntity]),
                this.messageActions.add(new Message({
                    i18n: 'SHARED.CRUD.MESSAGE.CREATE.ENTITY',
                    i18nParams: {},
                    buttonI18n: 'SHARED.CRUD.MESSAGE.BUTTON',
                    type: MessageType.SUCCESS,
                    format: MessageFormat.SNACK
                }))
            ]);
        })
        .catch(BaseActions.errorHandlerBuilder())
    );

/**
 * Update an entity in the backend
 */
@Effect() update$ = this.actions$
    .ofType(this.baseActions.actionTypeTemplate(BaseActions.UPDATE))
    .map((action: TypedAction<{ entityId: string, entity: E, currentEntity: E }>) => action.payload)
    .mergeMap(payload => this.crudService
        .update(payload.entityId, payload.entity)
        .mergeMap(entity => {
            return Observable.from([
                this.baseActions.load([entity]),
                this.messageActions.add(new Message({
                    i18n: 'SHARED.CRUD.MESSAGE.UPDATE.ENTITY',
                    i18nParams: {},
                    buttonI18n: 'SHARED.CRUD.MESSAGE.BUTTON',
                    type: MessageType.SUCCESS,
                    format: MessageFormat.SNACK
                }))
            ]);
        })
        .catch(BaseActions.errorHandlerBuilder())
    );

/**
 * Delete an entity
 */
@Effect() delete$ = this.actions$
    .ofType(this.baseActions.actionTypeTemplate(BaseActions.DELETE))
    .map((action: TypedAction<string>) => action.payload)
    .mergeMap(payload => this.crudService
        .delete(payload)
        .mergeMap(response => Observable.from([
            this.baseActions.unload(payload),
            this.messageActions.add(new Message({
                i18n: 'SHARED.CRUD.MESSAGE.DELETE.ENTITY',
                i18nParams: {},
                buttonI18n: 'SHARED.CRUD.MESSAGE.BUTTON',
                type: MessageType.SUCCESS,
                format: MessageFormat.SNACK
            }))
        ])
        )
        .catch(BaseActions.errorHandlerBuilder())
    );

/**
 * Allow for overriding the default fetchByCriteria effect
 */
protected getFetchByCriteriaEffect() {
    return this.actions$
        .ofType(this.baseActions.actionTypeTemplate(BaseActions.FETCH_BY_CRITERIA))
        .map((action: TypedAction<{ criteria: { [key: string]: string | boolean }, replace: boolean }>) => action.payload)
        .mergeMap(payload => this.crudService
            .fetchByCriteria(payload.criteria)
            .map((entities: E[]) => payload.replace ?
                this.baseActions.replace(entities) :
                this.baseActions.load(entities))
            .catch(BaseActions.errorHandlerBuilder())
        );
}

/**
 * Allow for overriding the default fetchById effect
 */
protected getFetchByIdEffect() {
    return this.actions$
        .ofType(this.baseActions.actionTypeTemplate(BaseActions.FETCH_BY_ID))
        .map((action: TypedAction<string>) => action.payload)
        .flatMap(entityId => this.crudService
            .fetchById(entityId)
            .map(entity => this.baseActions.load([entity]))
            .catch(BaseActions.errorHandlerBuilder())
        );
}

/**
 * Creates an instance of BaseEffects.
 *
 * @param {Store<AppState>} store
 * @param {Actions} actions$
 * @param {BaseActions<E, any>} baseActions
 * @param {MessageActions} messageActions
 * @param {CrudService<E>} crudService
 * @memberof BaseEffects
 */
constructor(
    protected store: Store<AppState>,
    protected actions$: Actions,
    protected baseActions: BaseActions<E, any>,
    protected messageActions: MessageActions,
    protected crudService: CrudService<E>
) {
    this.fetchByCriteria$.subscribe(store);
    this.fetchById$.subscribe(store);
    this.create$.subscribe(store);
    this.update$.subscribe(store);
    this.delete$.subscribe(store);
}

}
`

So for example:
`
import { Actions } from '@ngrx/effects';
import { AppState } from 'app/shared/app.state';
import { BaseEffects } from 'app/shared/store/base.effects';
import { Injectable } from '@angular/core';
import { MessageActions } from 'app/shared/store/message.actions';
import { Store } from '@ngrx/store';
import { Team } from '../model/team.model';
import { TeamActions } from './team.actions';
import { TeamService } from '../service/team.service';

@Injectable()
export class TeamEffects extends BaseEffects {

constructor(
    protected store: Store<AppState>,
    protected actions$: Actions,
    protected messageActions: MessageActions,
    protected teamActions: TeamActions,
    protected teamService: TeamService
) {
    super(store, actions$, teamActions, messageActions, teamService);
}

}
`

Previously, with ngrx Store v1 and up to 4.0.3 / 4.0.5, everything was fine. Now, if I just update to version 4.1.0, all my effects are subscribed twice and so executed twice, which causes a lot of side effects.

[brandonroberts]

@Arnaud73 you don't need to subscribe to the effects to the store that are using the @Effect decorator. That would explain why your effects are subscribed twice.

[Arnaud73]

Thanks @brandonroberts for the feedback.

In my model, the class BaseEffect is abstract and not included in the call to {EffectsModule.forRoot} so Decorators are not automatically evaluated, that being the reason of the subscription in the constructor. Without these subscriptions, Effects are not evaluated at runtime.

I'm pretty confident the problem is related to having a superclass with all effects shared with all subclasses because I started deactivated much of the code to test the problem and I reached a minimum in the application with just a few effects (and the BaseEffect class) and the problem kept happening.

If required, I can try to make a small project to reproduce the issue (although being in a hurry for the moment).

[brandonroberts]

Yes, a small reproduction would help

[brandonroberts]

@martinroob closing this as its working as designed. Using EffectsModule.forRoot subscribes to your effects for you, so you're effectively creating 2 subscriptions by using it in the imports, which explains why its making double HttpClient requests.

@Arnaud73 if you can make a reproduction, open a new issue.

[martinroob]

Ok, thanks for your help.