2

I am using a default client side auth service from a visual studio template.

There is a typescript AuthorizeService which has a function called isAuthenticated which calls the below function and checks that it is null or not.

The getUser function:

public getUser(): Observable<IUser> {
    return concat(
      this.userSubject.pipe(take(1), filter(u => !!u)),
      this.getUserFromStorage().pipe(filter(u => !!u), tap(u => this.userSubject.next(u))),
      this.userSubject.asObservable());
  }

when .subscribe is called on the above function. Subscribe is called three times. Presumably for each observable in the concat function. Using getUser above, I would expect subscribe to be called once. How would I achieve that?

I tried converting the above to nested promises that returns one value but with no success as for some reason, after the result is returned, it returns resolve(null) even though the user exists in session storage

John Mcdowl
  • 35
  • 4
  • Should getUser() really only emit once, or should the returned observable emit the current user whenever the user changes? – frido Aug 31 '20 at 17:41

2 Answers2

1

I'm assuming you want getUser() to only return one user. Right now, your logic says "Get the current value of userSubject and get a user from storage and also get the current and all future values of userSubject."

This means that if userSubject's value is truthy, you'll get at least 3 users back. This is what your logic is saying.

I'm not sure what you mean by 'only subscribe once,' but I'll assume you mean 'only return 1 user.' One simple way to do that is to only take one value from your concat call:

return concat(...).pipe(take(1));

This can lead to unpredictable behavior. Whichever of the three streams emits a value first will be the value you take. If getUserFromStorage() takes some time to complete, you'll always get null back. I'm guessing this is what happens to you when you're nesting promises (though I'd have to see your code to be sure).

The better way to accomplish this is with switchMap or mergeMap (either will work in this case) I'm also guessing that you only want to grab a user from the backend if there isn't one in the userSubject. This approach would be effectively caching the currently authenticated user.

public getUser(): Observable<IUser> {
  return this.userSubject.pipe(
    take(1),
    mergeMap(u => {
      if(u) return of(u);
      return this.getUserFromStorage().pipe(
        tap(u => u && this.userSubject.next(u))
      );
    }), 
    take(1)
  );
}

What does this do? It tries to get a user from storage only if the user from userSubject isn't truthy. this.getUserFromStorage() is never called (or subscribed to) if there's a user in the userSubject. Of note is that the second call to take(1) isn't necessary if getUserFromStorage() only ever returns one value. This also assumes that getUserFromStorage() returns null if there's no user in storage.

Finally, I've removed all the filters since it seems (from your description) that you want this stream to return a null if there is no user in the Subject and no user in storage. If we filter out a null return then we'll never return null. What I've done instead is that we only return null if getUserFromStorage() returns null.

Mrk Sef
  • 2,913
  • 3
  • 13
  • 1
    I ended up Initializing the user at app startup and just returning this.userSubject.asObservable() for subsequent requests – John Mcdowl Sep 02 '20 at 23:47
0

Consider using forkJoin instead (When all observables complete, emit the last emitted value from each as an Array) as demonstrated below:

Note: I've added take operator to the last observable so that it get completed

public getUser(): Observable <[IUser, IUser, IUser]> {
  return forkJoin(
    this.userSubject.pipe(take(1), filter(u => !!u)),
    this.getUserFromStorage().pipe(filter(u => !!u), tap(u => this.userSubject.next(u))),
    this.userSubject.asObservable().pipe(take(1))
  );
}
Rafi Henig
  • 4,362
  • 1
  • 10
  • 21