0

I have the following code where I need to get a value from two separate observables (they are actually promises but have been converted to observables using "from") and then use the values returned from each to make a final API call where both the values are used.

The method should return the result from the API call which is an object called "AppUser".

My code also has some logic in it to retry from a backup API if the primary API fails and then if the backup fails it should return the value from local storage.

This code does work but having read up about nested observables not being good practice I wondered if there is a better way to write this? Any advice would be appreciated.

    // Get app user
    getAppUser(getFromLocal: boolean = false, isRetry: boolean = false): Observable<AppUser> {

      // If "get from local" is true
      if (getFromLocal) {
        return this.getAppUserFromLocal();
      } else {

        // Create observables to get values from local storage
        const customerObs = from(this.authLocal.getCustomerId());
        const authTokenObs = from(this.authLocal.getAuthToken());

        return customerObs
        .pipe(mergeMap((customerId) => {

          return authTokenObs
          .pipe(mergeMap((authToken) => {

            // API Header
            const headers = new HttpHeaders({
              'Content-Type': 'application/json',
              Accept: 'application/json',
              Authorization: `Bearer ${authToken}`
            });
            const url = (!isRetry ? this.baseUrl : this.backupBaseUrl) + '/GetAppUser?customerid=' + customerId;

            return this.http.get(url, { headers })
              .pipe(map((appUser: AppUser) => {
   
                // Set AppUser in local storage
                this.storage.set('appUser', appUser);
   
                return appUser;
              }),
              catchError(e => {

                if (!isRetry) {
                  // Try again using backup API
                  return this.getAppUser(getFromLocal, true);
                } else {
                  // Try again from local storage
                  return this.getAppUser(true, true);
                }
              }));
          }));
        }));
      }
    }

UPDATED with potential solution

I have updated as per the answers with the following potential solution:

    // Get app user
    getAppUser(getFromLocal: boolean = false, isRetry: boolean = false): Observable<AppUser> {

      // If "get from local" is true
      if (getFromLocal) {
        return this.getAppUserFromLocal();
      } else {

        // Create observables to get values from local storage
        const customerId$ = from(this.authLocal.getCustomerId());
        const authToken$ = from(this.authLocal.getAuthToken());

        return forkJoin([
          customerId$,
          authToken$
        ]).pipe(
          switchMap(([customerId, authToken]) => {
            const headers = new HttpHeaders({
              'Content-Type': 'application/json',
              Accept: 'application/json',
              Authorization: `Bearer ${authToken}`
            });
            const url = (!isRetry ? this.baseUrl : this.backupBaseUrl) + '/GetAppUser?customerId=' + customerId;
            return this.http.get(url, { headers })
            .pipe(
              map((appUser: AppUser) => {
                appUser.AuthToken = authToken;
                return appUser;
              }),
              catchError(e => {
                if (!isRetry) {
                  // Try again using backup API
                  return this.getAppUser(getFromLocal, true);
                } else {
                  // Try again from local storage
                  return this.getAppUser(true, true);
                }
              }));
          }),
          tap((appUser: AppUser)  => {

            // Set AppUser in local storage
            this.storage.set('appUser', appUser);
          }),
        );
WebFletch
  • 172
  • 1
  • 16

2 Answers2

1

You can use forkJoin to emit both values, especially because you know they will complete as they are from Promises (I've also renamed your Observables to better follow norms) :

return forkJoin({
  customerId: customerId$,
  authToken: authToken$
}).pipe(
  switchMap(({ customerId: string, authToken: string }) => {
    const headers = new HttpHeaders({
      'Content-Type': 'application/json',
      Accept: 'application/json',
      Authorization: `Bearer ${authToken}`
    });
    const url = (!isRetry ? this.baseUrl : this.backupBaseUrl) + '/GetAppUser?customerid=' + customerId;
    return this.http.get(url, { headers }).pipe(retry(1));
  }),
  tap(user => this.storage.set('appUser', appUser)),
);

The only error handling here is the retry operator (you can choose to retry more times), and I've used tap for calling setUser as it is the recommended method for side effects.

Will Alexander
  • 3,425
  • 1
  • 10
  • 17
  • I assume the "retry" is only for retrying exactly the same API? In my original code, I am actually retrying a completely different API that runs on a separate server (for example if the first API is completely down so a retry to the same API will not work), and then if neither API is accessible (maybe they have no internet connection) it gets it from local storage. Also are there any advantages/disadvantages to using the forkJoin method over the combineLatest method suggested by @SnorreDan – WebFletch Jun 28 '20 at 12:34
  • 1
    Here I just put the retry in on the HTTP call so it tries it a second time before giving up. You could use a `catchError` to simply retrieve from localStorage if the second call doesn't work. In this specific case, `combineLatest` will work like `forkJoin`, because each Observable only emits once and then completes. I'd argue `forkJoin` is the cleaner choice, but I don't see any real trade off in this particular situation. – Will Alexander Jun 28 '20 at 12:49
  • Thanks, just to clarify, I assume before the code you have written I would have the following (variable names changed to match yours): const customerId$ = from(this.authLocal.getCustomerId()); const authToken$ = from(this.authLocal.getAuthToken()); – WebFletch Jun 28 '20 at 18:54
  • Also, there seems to be a problem with this part: switchMap(({ customerId: string, authToken: string }), it is giving me an error on "string" as follows: Duplicate identifier 'string'. This then causes errors within the switchMap where it says it cannot find name authtoken/masterCustomerId. – WebFletch Jun 28 '20 at 20:17
  • I have updated my original question with a combination of your code and the code from @SnorreDan because of the issues with your code mentioned above. I have also added in a "map" function because I need to modify the "AppUser" to add the "AuthToken" to it before it gets set in local storage using the "tap" function. Does the new version look okay or are there any potential improvements or issues that you can see? – WebFletch Jun 28 '20 at 21:02
1

So you want two observables to get their values first, and then you want to jump over to your third observable (api observable). A general way of solving this problem could be with combineLatest and switchMap:

combineLatest([ customerObs, authTokenObs ])
  .pipe(
    switchMap(([customerId, authToken]) => {
      // Do the code that creates your api observable
      return apiObs;
    })
  )
  .subscribe(resultFromApiCall => console.log(resultFromApiCall));
SnorreDan
  • 2,740
  • 1
  • 14
  • 17
  • Is there any advantages/disadvantages from using combineLatest instead of forkJoin as suggested by @Will Alexander – WebFletch Jun 28 '20 at 12:47
  • 1
    As @Will Alexander is saying, forkJoin is a good fit here if you know that your source Observables will only emit once. But if your source Observables were to emit more than once, then combineLatest would emit for each new value, while forkJoin would only emit the first one. – SnorreDan Jun 28 '20 at 13:41
  • Thanks, yes the observables should only emit once when the user calls the method, the whole method will be called again throughout the App but it will need to run again completely (i.e. getting updated values from API, returning them and saving them to local storage). As per my comment on @WillAlexander's answer I have updated my question with a potential solution if you wouldn't mind giving it a look over. – WebFletch Jun 28 '20 at 21:06