Merge pull request #1072 from qtomlinson/qt/fix-pypi

Use dependency injection in PypiCoordinatesMapper
clearlydefined · Mar 13, 2024 · fe8343f · fe8343f
2 parents 91cdee3 + 87ffe18
commit fe8343f
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 35 deletions.
diff --git a/lib/pypiCoordinatesMapper.js b/lib/pypiCoordinatesMapper.js
@@ -5,8 +5,9 @@ const requestPromise = require('request-promise-native')
 const EntityCoordinates = require('./entityCoordinates')
 
 class PypiCoordinatesMapper {
-  constructor() {
+  constructor(fetch = requestPromise) {
     this.baseUrl = 'https://pypi.python.org'
+    this._fetch = fetch
   }
 
   async map(coordinates) {
@@ -21,19 +22,16 @@ class PypiCoordinatesMapper {
   }
 
   async _resolve(coordinates) {
+    if (coordinates.name === '..') return null
     const url = new URL(`/pypi/${coordinates.name}/json`, this.baseUrl).toString()
     try {
-      const answer = await this._handleRequest(url)
+      const answer = await this._fetch({ url, method: 'GET', json: true })
       return answer?.info?.name && { name: answer.info.name }
     } catch (error) {
       if (error.statusCode === 404) return null
       throw error
     }
   }
-
-  _handleRequest(url) {
-    return requestPromise({ url, method: 'GET', json: true })
-  }
 }
 
 module.exports = PypiCoordinatesMapper
diff --git a/test/lib/pypiCoordinatesMapper.js b/test/lib/pypiCoordinatesMapper.js
@@ -7,7 +7,8 @@ const EntityCoordinates = require('../../lib/entityCoordinates')
 const PypiCoordinatesMapper = require('../../lib/pypiCoordinatesMapper')
 
 function mockPypiCoordinates(name) {
-  return EntityCoordinates.fromString(`pypi/pypi/-/${name}/1.1.0a4`)
+  const spec = { type: 'pypi', provider: 'pypi', revision: '1.0.0', name }
+  return EntityCoordinates.fromObject(spec)
 }
 
 function mockPypiAnswer(name) {
@@ -16,30 +17,32 @@ function mockPypiAnswer(name) {
 
 describe('PypiCoordinatesMapper', () => {
   let coordinatesMapper
+  let fetchStub
   beforeEach(() => {
-    coordinatesMapper = new PypiCoordinatesMapper()
+    fetchStub = sinon.stub()
+    coordinatesMapper = new PypiCoordinatesMapper(fetchStub)
   })
 
   it('should map name containing "_" mapped to "-"', async () => {
-    sinon.stub(coordinatesMapper, '_handleRequest').resolves(mockPypiAnswer('0-core-client'))
+    fetchStub.resolves(mockPypiAnswer('0-core-client'))
     const mapped = await coordinatesMapper.map(mockPypiCoordinates('0_core_client'))
     expect(mapped.name).to.be.eq('0-core-client')
   })
 
   it('should map name containing "." mapped to "-"', async () => {
-    sinon.stub(coordinatesMapper, '_handleRequest').resolves(mockPypiAnswer('0-core-client'))
+    fetchStub.resolves(mockPypiAnswer('0-core-client'))
     const mapped = await coordinatesMapper.map(mockPypiCoordinates('0.core_client'))
     expect(mapped.name).to.be.eq('0-core-client')
   })
 
   it('should map name containing "-" mapped to "_"', async () => {
-    sinon.stub(coordinatesMapper, '_handleRequest').resolves(mockPypiAnswer('backports.ssl_match_hostname'))
+    fetchStub.resolves(mockPypiAnswer('backports.ssl_match_hostname'))
     const mapped = await coordinatesMapper.map(mockPypiCoordinates('Backports.ssl-match-hostname'))
     expect(mapped.name).to.be.eq('backports.ssl_match_hostname')
   })
 
   it('should return null when pypi api returns 404', async () => {
-    sinon.stub(coordinatesMapper, '_handleRequest').throws({ statusCode: 404 })
+    fetchStub.throws({ statusCode: 404 })
     const mapped = await coordinatesMapper.map(mockPypiCoordinates('blivet-gui'))
     expect(mapped).to.be.null
   })
@@ -55,30 +58,19 @@ describe('PypiCoordinatesMapper', () => {
     const mapped = await coordinatesMapper.map(mockPypiCoordinates('backports'))
     expect(mapped).to.be.null
   })
-
-  it('should return null when pypi name to be mapped is invalid', async () => {
-    sinon.stub(coordinatesMapper, '_handleRequest').rejects('Should not be called')
-    const spec = {
-      type: 'pypi',
-      provider: 'pypi',
-      name: 'back.ports/test',
-      revision: '1.0.0'
-    }
-    const coordinates = EntityCoordinates.fromObject(spec)
-    const mapped = await coordinatesMapper.map(coordinates)
-    expect(mapped).to.be.null
-  })
 
-  it('should return null given no name', async () => {
-    sinon.stub(coordinatesMapper, '_handleRequest').rejects('Should not be called')
-    const spec = {
-      type: 'pypi',
-      provider: 'pypi'
-    }
-    const coordinates = EntityCoordinates.fromObject(spec)
-    const mapped = await coordinatesMapper.map(coordinates)
-    expect(mapped).to.be.null
+  describe('should handle invalid package names and skip network calls', () => {
+    beforeEach(() => fetchStub.rejects('Should not be called'))
+
+    it('should return null for an invalid name', async () => handleInvalidName(coordinatesMapper, 'backports./test'))
+
+    it('should return null when the name is ..', async () => handleInvalidName(coordinatesMapper, '..'))
+
+    it('should return null for no name', async () => handleInvalidName(coordinatesMapper))
   })
 })
 
-
+async function handleInvalidName(coordinatesMapper, name) {
+  const mapped = await coordinatesMapper.map(mockPypiCoordinates(name))
+  expect(mapped).to.be.null
+}