[web] convert GeoIpLookup to async/await (#28802)

GitOrigin-RevId: 38ad8af970a0674a514bf5ed0dacb8becd7c1f72

services/web/app/src/infrastructure/GeoIpLookup.js

@@ -1,9 +1,6 @@
-const request = require('request')
 const settings = require('@overleaf/settings')
-const _ = require('lodash')
 const logger = require('@overleaf/logger')
-const { URL } = require('url')
-const { promisify, promisifyMultiResult } = require('@overleaf/promise-utils')
+const { fetchJson } = require('@overleaf/fetch-utils')
 
 const DEFAULT_CURRENCY_CODE = 'USD'
 
@@ -65,7 +62,9 @@ const EuroCountries = [
   'ES',
 ]
 
-_.forEach(EuroCountries, country => (currencyMappings[country] = 'EUR'))
+for (const country of EuroCountries) {
+  currencyMappings[country] = 'EUR'
+}
 
 function isValidCurrencyParam(currency) {
   if (!currency) {
@@ -74,54 +73,42 @@ function isValidCurrencyParam(currency) {
   return validCurrencyParams.includes(currency)
 }
 
-function getDetails(ip, callback) {
+async function getDetails(ip, callback) {
   if (!ip) {
     return callback(new Error('no ip passed'))
   }
   ip = ip.trim().split(' ')[0]
-  const opts = {
-    url: new URL(ip, settings.apis.geoIpLookup.url).href,
-    timeout: 1000,
-    json: true,
-  }
-  logger.debug({ ip, opts }, 'getting geo ip details')
-  request.get(opts, function (err, res, ipDetails) {
-    if (err) {
-      logger.warn({ err, ip }, 'error getting ip details')
-    }
-    callback(err, ipDetails)
-  })
+  const url = new URL(settings.apis.geoIpLookup.url)
+  url.pathname += ip
+  logger.debug({ ip, url }, 'getting geo ip details')
+  return await fetchJson(url, { signal: AbortSignal.timeout(1_000) })
 }
 
-function getCurrencyCode(ip, callback) {
-  getDetails(ip, function (err, ipDetails) {
-    if (err || !ipDetails) {
-      logger.err(
-        { err, ip },
-        `problem getting currencyCode for ip, defaulting to ${DEFAULT_CURRENCY_CODE}`
-      )
-      return callback(null, DEFAULT_CURRENCY_CODE)
-    }
-    const countryCode =
-      ipDetails && ipDetails.country_code
-        ? ipDetails.country_code.toUpperCase()
-        : undefined
-    const currencyCode = currencyMappings[countryCode] || DEFAULT_CURRENCY_CODE
-    logger.debug({ ip, currencyCode, ipDetails }, 'got currencyCode for ip')
-    callback(err, currencyCode, countryCode)
-  })
+async function getCurrencyCode(ip) {
+  let ipDetails
+  try {
+    ipDetails = await getDetails(ip)
+  } catch (err) {
+    logger.err(
+      { err, ip },
+      `problem getting currencyCode for ip, defaulting to ${DEFAULT_CURRENCY_CODE}`
+    )
+    return { currencyCode: DEFAULT_CURRENCY_CODE }
+  }
+  const countryCode =
+    ipDetails && ipDetails.country_code
+      ? ipDetails.country_code.toUpperCase()
+      : undefined
+  const currencyCode = currencyMappings[countryCode] || DEFAULT_CURRENCY_CODE
+  logger.debug({ ip, currencyCode, ipDetails }, 'got currencyCode for ip')
+  return { currencyCode, countryCode }
 }
 
 module.exports = {
-  getDetails,
-  getCurrencyCode,
   isValidCurrencyParam,
   promises: {
-    getDetails: promisify(getDetails),
-    getCurrencyCode: promisifyMultiResult(getCurrencyCode, [
-      'currencyCode',
-      'countryCode',
-    ]),
+    getDetails,
+    getCurrencyCode,
   },
   DEFAULT_CURRENCY_CODE,
 }

services/web/test/unit/src/infrastructure/GeoIpLookupTest.js

@@ -10,20 +10,6 @@ const modulePath = path.join(
 
 describe('GeoIpLookup', function () {
   beforeEach(function () {
-    this.request = { get: sinon.stub() }
-    this.settings = {
-      apis: {
-        geoIpLookup: {
-          url: 'http://lookup.com',
-        },
-      },
-    }
-    this.GeoIpLookup = SandboxedModule.require(modulePath, {
-      requires: {
-        request: this.request,
-        '@overleaf/settings': this.settings,
-      },
-    })
     this.ipAddress = '12.34.56.78'
 
     this.stubbedResponse = {
@@ -39,6 +25,22 @@ describe('GeoIpLookup', function () {
       metro_code: '',
       area_code: '',
     }
+    this.fetchUtils = {
+      fetchJson: sinon.stub().resolves(this.stubbedResponse),
+    }
+    this.settings = {
+      apis: {
+        geoIpLookup: {
+          url: 'http://lookup.com/',
+        },
+      },
+    }
+    this.GeoIpLookup = SandboxedModule.require(modulePath, {
+      requires: {
+        '@overleaf/fetch-utils': this.fetchUtils,
+        '@overleaf/settings': this.settings,
+      },
+    })
   })
 
   describe('isValidCurrencyParam', function () {
@@ -56,60 +58,15 @@ describe('GeoIpLookup', function () {
 
   describe('getDetails', function () {
     beforeEach(function () {
-      this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-    })
-
-    describe('callback', function () {
-      it('should request the details using the ip', function (done) {
-        this.GeoIpLookup.getDetails(this.ipAddress, err => {
-          assert.equal(err, null)
-          this.request.get
-            .calledWith({
-              url: this.settings.apis.geoIpLookup.url + '/' + this.ipAddress,
-              timeout: 1000,
-              json: true,
-            })
-            .should.equal(true)
-          done()
-        })
-      })
-
-      it('should return the ip details', function (done) {
-        this.GeoIpLookup.getDetails(this.ipAddress, (err, returnedDetails) => {
-          assert.equal(err, null)
-          assert.deepEqual(returnedDetails, this.stubbedResponse)
-          done()
-        })
-      })
-
-      it('should take the first ip in the string', function (done) {
-        this.GeoIpLookup.getDetails(
-          ` ${this.ipAddress} 123.123.123.123 234.234.234.234`,
-          err => {
-            assert.equal(err, null)
-            this.request.get
-              .calledWith({
-                url: this.settings.apis.geoIpLookup.url + '/' + this.ipAddress,
-                timeout: 1000,
-                json: true,
-              })
-              .should.equal(true)
-            done()
-          }
-        )
-      })
+      this.fetchUtils.fetchJson.resolves(this.stubbedResponse)
     })
 
     describe('async', function () {
       it('should request the details using the ip', async function () {
         await this.GeoIpLookup.promises.getDetails(this.ipAddress)
-        this.request.get
-          .calledWith({
-            url: this.settings.apis.geoIpLookup.url + '/' + this.ipAddress,
-            timeout: 1000,
-            json: true,
-          })
-          .should.equal(true)
+        this.fetchUtils.fetchJson.should.have.been.calledWith(
+          new URL(this.settings.apis.geoIpLookup.url + this.ipAddress)
+        )
       })
 
       it('should return the ip details', async function () {
@@ -123,111 +80,16 @@ describe('GeoIpLookup', function () {
         await this.GeoIpLookup.promises.getDetails(
           ` ${this.ipAddress} 123.123.123.123 234.234.234.234`
         )
-        this.request.get
-          .calledWith({
-            url: this.settings.apis.geoIpLookup.url + '/' + this.ipAddress,
-            timeout: 1000,
-            json: true,
-          })
-          .should.equal(true)
+        this.fetchUtils.fetchJson.should.have.been.calledWith(
+          new URL(this.settings.apis.geoIpLookup.url + this.ipAddress)
+        )
       })
     })
   })
 
   describe('getCurrencyCode', function () {
-    describe('callback', function () {
-      it('should return GBP for GB country', function (done) {
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        this.GeoIpLookup.getCurrencyCode(
-          this.ipAddress,
-          (err, currencyCode) => {
-            assert.equal(err, null)
-            currencyCode.should.equal('GBP')
-            done()
-          }
-        )
-      })
-
-      it('should return GBP for gb country', function (done) {
-        this.stubbedResponse.country_code = 'gb'
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        this.GeoIpLookup.getCurrencyCode(
-          this.ipAddress,
-          (err, currencyCode) => {
-            assert.equal(err, null)
-            currencyCode.should.equal('GBP')
-            done()
-          }
-        )
-      })
-
-      it('should return USD for US', function (done) {
-        this.stubbedResponse.country_code = 'US'
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        this.GeoIpLookup.getCurrencyCode(
-          this.ipAddress,
-          (err, currencyCode) => {
-            assert.equal(err, null)
-            currencyCode.should.equal('USD')
-            done()
-          }
-        )
-      })
-
-      it('should return EUR for DE', function (done) {
-        this.stubbedResponse.country_code = 'DE'
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        this.GeoIpLookup.getCurrencyCode(
-          this.ipAddress,
-          (err, currencyCode) => {
-            assert.equal(err, null)
-            currencyCode.should.equal('EUR')
-            done()
-          }
-        )
-      })
-
-      it('should default to USD if there is an error', function (done) {
-        this.request.get.callsArgWith(1, null, null, { error: true })
-        this.GeoIpLookup.getCurrencyCode(
-          this.ipAddress,
-          (err, currencyCode) => {
-            assert.equal(err, null)
-            currencyCode.should.equal('USD')
-            done()
-          }
-        )
-      })
-
-      it('should default to USD if there are no details', function (done) {
-        this.request.get.callsArgWith(1, null, null, {})
-        this.GeoIpLookup.getCurrencyCode(
-          this.ipAddress,
-          (err, currencyCode) => {
-            assert.equal(err, null)
-            currencyCode.should.equal('USD')
-            done()
-          }
-        )
-      })
-
-      it('should default to USD if there is no match for their country', function (done) {
-        this.stubbedResponse.country_code = 'Non existant'
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
-        this.GeoIpLookup.getCurrencyCode(
-          this.ipAddress,
-          (err, currencyCode) => {
-            assert.equal(err, null)
-            currencyCode.should.equal('USD')
-            done()
-          }
-        )
-      })
-    })
-
     describe('async', function () {
       it('should return GBP for GB country', async function () {
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
         const { currencyCode, countryCode } =
           await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('GBP')
@@ -236,7 +98,6 @@ describe('GeoIpLookup', function () {
 
       it('should return GBP for gb country', async function () {
         this.stubbedResponse.country_code = 'gb'
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
         const { currencyCode, countryCode } =
           await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('GBP')
@@ -245,7 +106,6 @@ describe('GeoIpLookup', function () {
 
       it('should return USD for US', async function () {
         this.stubbedResponse.country_code = 'US'
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
         const { currencyCode, countryCode } =
           await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('USD')
@@ -254,7 +114,6 @@ describe('GeoIpLookup', function () {
 
       it('should return EUR for DE', async function () {
         this.stubbedResponse.country_code = 'DE'
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
         const { currencyCode, countryCode } =
           await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('EUR')
@@ -262,7 +121,7 @@ describe('GeoIpLookup', function () {
       })
 
       it('should default to USD if there is an error', async function () {
-        this.request.get.callsArgWith(1, null, null, { error: true })
+        this.fetchUtils.fetchJson.rejects(new Error('foo'))
         const { currencyCode, countryCode } =
           await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('USD')
@@ -270,7 +129,7 @@ describe('GeoIpLookup', function () {
       })
 
       it('should default to USD if there are no details', async function () {
-        this.request.get.callsArgWith(1, null, null, {})
+        this.fetchUtils.fetchJson.resolves({})
         const { currencyCode, countryCode } =
           await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('USD')
@@ -279,7 +138,6 @@ describe('GeoIpLookup', function () {
 
       it('should default to USD if there is no match for their country', async function () {
         this.stubbedResponse.country_code = 'Non existant'
-        this.request.get.callsArgWith(1, null, null, this.stubbedResponse)
         const { currencyCode, countryCode } =
           await this.GeoIpLookup.promises.getCurrencyCode(this.ipAddress)
         currencyCode.should.equal('USD')