Make API call on Node server and assign result to a class property

Question

I have a blogging web app. Whenever you create a post, a new instance of Post is created by the post controller. I want to make an API call to OpenWeatherMap and assign the temperature returned by the API call to the Post property this.data.weather.

This is the class Post

let Post = function (data, userid, requestedPostId) {
  this.data = data
  this.errors = []
  this.userid = userid
  this.requestedPostId = requestedPostId

}

The function that makes the API call is as follows:

Post.prototype.getWeather = function () {
  return new Promise(async (resolve, reject) => {
    let fetch_weather = await fetch(
      "https://api.openweathermap.org/data/2.5/weather?id=6359002&appid=MY_API_KEY"
    )

    let weather_json = await fetch_weather.json()
    console.log(weather_json.main.temp)
    resolve(weather_json.main.temp)
  })
}

I have tried to include this in the class declaration:

this.data.weather = this.getWeather()

The function runs successfully, and I can successfully access all the fields of the json returned and log them into the console, but this.data.weather is assigned a value of Object. I want this.data.weather to have a value of the temperature (which is what the getWeather() method returns).

PS: I'm using the npm package node-fetch that allows me to use fetch on the server the same way as with the native fetch API on the browser.

Edit after Roamer-1888's answer.

Below is the Post model with the create and cleanUp methods. Before storing the post in the databse, the data passed to the Post model is cleaned up and I am calling this.consumeWeather() (I've renamed the letsConsumeAsynchronouslyDerivedWeatherData() method). Still, the model sends a value of undefined to the database and I am at a loss here.

let Post = function (data, userid, requestedPostId) {
  this.data = data
  this.errors = []
  this.userid = userid
  this.requestedPostId = requestedPostId
}

Post.prototype.cleanUp = function () {
  if (typeof this.data.title != "string") {
    this.data.title = ""
  }
  if (typeof this.data.body != "string") {
    this.data.body = ""
  }

  this.consumeWeather()

  //Get rid of any bogus properties
  this.data = {
    title: sanitizeHTML(this.data.title.trim(), {
      allowedTags: [],
      allowedAttributes: [],
    }),
    body: sanitizeHTML(this.data.body.trim(), {
      allowedTags: [],
      allowedAttributes: [],
    }),
    createdDate: new Date(),
    author: ObjectID(this.userid),
    location: sanitizeHTML(this.data.location.trim()),
    temp: this.data.temp,
  }
}

Post.prototype.validate = function () {
  if (this.data.title === "") {
    this.errors.push("You must provide a title.")
  }
  if (this.data.body === "") {
    this.errors.push("You must post content.")
  }
}

Post.prototype.create = function () {
  return new Promise((resolve, reject) => {
    this.cleanUp()
    this.validate()
    if (!this.errors.length) {
      //Save post into database if there are no errors
      postsCollection
        .insertOne(this.data)
        .then((info) => {
          resolve(info.ops[0]._id) //This promise resolves with the id of the post created so that it can be used by the controller to redirect to the newly created post upon saving it
        })
        .catch(() => {
          this.errors.push("Please try again later")
          reject(this.errors)
        })
    } else {
      reject(this.errors)
    }
  })
}

Answer 1

I want to make an API call to OpenWeatherMap and assign the temperature returned by the API call to the Post property this.data.weather.

No, don't try to cache asynchronously derived data. It can't be reliably accessed as it always remains uncertain as to whether or not it has been delivered. Instead, cache the Promise in which the data is wrapped, and always access the data via .then() or await. Consequently, it is inescapable that consumers of asynchronously derived data are themselves asynchronous, and so on up the call stack.

Also, avoid the explicit promise construction antipattern.

For example:

Post.prototype.getWeather = async function() {
    if(!this.weather_data_promise) {
        let fetch_weather = await fetch("https://api.openweathermap.org/data/2.5/weather?id=6359002&appid=MY_API_KEY");
        this.weather_data_promise = fetch_weather.json(); // cache the Promise
    }
    let weather_data = await this.weather_data_promise;
    return weather_data.main;
}

Now, every consumer of weather data must access it asynchronously:

For example:

async function letsConsumeAsynchronouslyDerivedWeatherData() {
    try {
        let weather_data = await Post.getWeather();
        let temperature = weather_data.temp;
        // do something with `temperature`
    }
    catch(error) {
        // something went wrong
    }
}

EDIT

So, having seen more of the Post.prototype methods, this is what I would write ..... explanations in commemts and below.

Post.prototype.getWeather = async function() {
    if(!this.weather_data_promise) {
        let fetch_weather = await fetch("https://api.openweathermap.org/data/2.5/weather?id=6359002&appid=MY_API_KEY");
        this.weather_data_promise = fetch_weather.json(); // cache the Promise
    }
    let weather_data = await this.weather_data_promise;
    return weather_data.main;
}
Post.prototype.getCleanData = async function () { // renamed from .cleanUp()
    // Sanitize this.data and add in the asynchronously derived data
    // but don't persist either in the Post instance.
    let title = (typeof this.data.title === "string") ? this.data.title : '';
    let body = (typeof this.data.body === "string") ? this.data.body : '';
    return this.getWeather()
    .then((weather_data) => {
        return { 
            'title': sanitizeHTML(title.trim(), {
                'allowedTags': [],
                'allowedAttributes': []
            }),
            'body': sanitizeHTML(body.trim(), {
                'allowedTags': [],
                'allowedAttributes': []
            }),
            'createdDate': new Date(),
            'author': ObjectID(this.userid),
            'location': sanitizeHTML(this.data.location.trim())
            'temp': weather_data.temp // asynchronously derived.
        };
    });
};
Post.prototype.validate = function (data) { // now accepts `data` as a parameter.
    // Like the data, don't persist errors in the Post instance.
    // Instead, the `errors` array is created here and returned.
    let errors = [];
    if (data.title === '') {
        errors.push("You must provide a title.");
    }
    if (data.body === '') {
        errors.push("You must post content.");
    }
    return errors;
};
Post.prototype.create = function () {
    return this.getCleanData() // asynchronous because getCleanData() is a consumer of asynchronous data.
    .then((data) => { // getCleanData()'s promise delivers sanitized data
        let errors = this.validate(data); // pass sanitized data.
        if(errors.length === 0) { 
            // Save post into database if there are no errors
            return postsCollection
            .insertOne(data)
            .then(info => info.ops[0]._id); // the returned promise will be fulfilled with the id of the post created so that it can be used by the controller to redirect to the newly created post upon saving it.
        } else {
            throw new Error(errors.join(" | ")); // Consolidate validation errors into a single Error object, and throw.
        }
    })
    .catch(error => {
        // handle error as necessary
        // rethrow error if necesary
    });
};

The main differences are that cleaned up data, asynchronously derived data and validation errors are not persisted in the Post instance.

As before, the data is only safe to use once it has been sanitized and validated, therefore Post.prototype.getCleanData() and Post.prototype.validate(data) have to be run each time the data is accessed.

Persisting the sanitized data in the Post instance would mean that, on the 2nd and subsequent calls, the sanitization code would need to be capable of operating on already (partially?) sanitized data. And that would mean a whole lot more testing.