skip to Main Content

I have a third party Angular library as a dependency, which is doing http requests (superagent), so I can’t intercept it. I want to cache the values I get, because I have a list with sometimes similar information in the UI and I don’t need to always make the same request.

So here is my code to receive a user from the API:

getUser(userId: number): Promise<any> {
    return new Promise(async (resolve) => {
      const cached = this.cacheService.get(userId);
        if (cached) {
          resolve(cached);
        } else {
        let user = await this.THIRDPARTYAPI.getUser(userId);
        this.cacheService.put(user.id, user);
        resolve(user);
      }
  });}

(The async/await is just my latest try).

This gets used in the following part

     data.forEach((row: any) => {
       this.getUser(row.userId).then((user: any)=>{
      //do something
      });

However it will always trigger the THIRDPARTYAPI.getUser(userId) method, so it will put the value inside my cache service, after it’s already to late and the values have already been check. It doesn’t really run the forEach entirely one after the other, and I am confused, how I could realise this.

2

Answers


  1. It seems that the problem isn’t really in the getUser() method but how you’re consuming it inside the forEach().

    I assume that the reason why you’re trying to cache the responses for each userId is because they show up multiple times inside data.

    If you’re calling getUser() inside the forEach() you should keep in mind that each iteration will not wait until they’re finished. This means that for each item in data you would have immediately called getUser(). Since this method is called way too quickly, your cacheService will always miss.


    One way to solve this could be to use await inside a for...of block as such:

    for (const row of data) {
      const result = await this.getUser(row.userId)
      // do something with result
    }
    

    This would execute each request sequentially.


    Another way could be to filter unique rows based on the row.userId property, call getUser() on them and then do your forEach() loop. At that point, all the user objects should have been cached an your forEach() would always hit the cache.

    Login or Signup to reply.
  2. Here’s an example of how you might cache the promise:

    getUser(userId: number): Promise<any> | any {
      const cached = this.cacheService.get(userId);
      if (cached) return cached;
      
      const userPromise = this.THIRDPARTYAPI.getUser(userId);
      this.cacheService.put(user.id, userPromise);
    
      // Optional if you want the cache to eventually contain users and not just promises.
      userPromise.then(user => cacheService.put(user.id, user));
      
      return userPromise;
    }
    

    As an aside, your original code is a very bad use of new Promise() and it will ignore errors. This is how your original code should have looked like for your future reference:

    async getUser(userId: number): Promise<any> {
    
       const cached = this.cacheService.get(userId);
       if (cached) return cached;
    
       const user = await this.THIRDPARTYAPI.getUser(userId);
       this.cacheService.put(user.id, user);
       return user;
    }
    

    In most cases new Promise() is a red flag that something bad is about to happen.

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search