fix(3433): repeated coverage info requests in jobs table

app/components/pipeline/jobs/table/cell/coverage/component.js

@@ -17,7 +17,7 @@ export default class PipelineJobsTableCellCoverageComponent extends Component {
       const pipeline = this.pipelinePageState.getPipeline();
 
       this.shuttle
-        .fetchFromApi('get', '/coverage/info', {
+        .fetchCoverage({
           jobId: job.id,
           buildId: this.args.record.build.id,
           startTime: this.args.record.build.startTime,

app/shuttle/service.js

@@ -15,6 +15,12 @@ export default Service.extend({
   store: service(),
   session: service(),
 
+  init() {
+    this._super(...arguments);
+    this.coverageCache = new Map();
+    this.coverageRequests = new Map();
+  },
+
   storeHost: `${ENV.APP.SDSTORE_HOSTNAME}/${ENV.APP.SDSTORE_NAMESPACE}`,
 
   apiHost: `${ENV.APP.SDAPI_HOSTNAME}/${ENV.APP.SDAPI_NAMESPACE}`,
@@ -39,16 +45,19 @@ export default Service.extend({
     };
   },
 
-  fetchFrom(host = 'store', method = 'get', url, data = {}, raw = false) {
+  fetchFrom(host, method, url, data = {}, raw = false) {
+    const requestHost = host || 'store';
+    const requestMethod = method || 'get';
+
     let baseHost = this.apiHost;
 
-    if (host === 'store') {
+    if (requestHost === 'store') {
       baseHost = this.storeHost;
     }
 
-    const optionsType = method.toUpperCase();
+    const optionsType = requestMethod.toUpperCase();
 
-    let requestType = method.toLowerCase();
+    let requestType = requestMethod.toLowerCase();
 
     if (raw) {
       requestType = 'raw';
@@ -65,11 +74,11 @@ export default Service.extend({
     return this.ajax[requestType](uri, options);
   },
 
-  fetchFromApi(method = 'get', url, data, raw = false) {
+  fetchFromApi(method, url, data = {}, raw = false) {
     return this.fetchFrom('api', method, url, data, raw);
   },
 
-  fetchFromStore(method = 'get', url, data, raw = false) {
+  fetchFromStore(method, url, data = {}, raw = false) {
     return this.fetchFrom('store', method, url, data, raw);
   },
 
@@ -96,6 +105,13 @@ export default Service.extend({
     return this.fetchFromApi(method, url, data);
   },
 
+  getCoverageCacheKey(data = {}) {
+    return Object.keys(data)
+      .sort()
+      .map(key => `${key}:${data[key]}`)
+      .join('|');
+  },
+
   /**
    * Fetch coverage info from coverage plugin
    * @param  {Object}  data
@@ -113,8 +129,29 @@ export default Service.extend({
   async fetchCoverage(data) {
     const method = 'get';
     const url = `/coverage/info`;
+    const cacheKey = this.getCoverageCacheKey(data);
 
-    return this.fetchFromApi(method, url, data);
+    if (this.coverageCache.has(cacheKey)) {
+      return this.coverageCache.get(cacheKey);
+    }
+
+    if (this.coverageRequests.has(cacheKey)) {
+      return this.coverageRequests.get(cacheKey);
+    }
+
+    const request = this.fetchFromApi(method, url, data)
+      .then(response => {
+        this.coverageCache.set(cacheKey, response);
+
+        return response;
+      })
+      .finally(() => {
+        this.coverageRequests.delete(cacheKey);
+      });
+
+    this.coverageRequests.set(cacheKey, request);
+
+    return request;
   },
 
   async openPr(checkoutUrl, yaml = '', pipelineId = 1) {

tests/unit/shuttle/service-test.js

@@ -77,15 +77,15 @@ module('Unit | Service | shuttle', function (hooks) {
       })
     ]);
 
-    const buildId = 243421;
-    const jobId = 21;
-    const startTime = '2020-05-06T23:36:46.779Z';
-    const endTime = '2020-05-06T23:50:18.590Z';
-    const pipelineId = 123456;
-    const prNum = null;
-
     service
-      .fetchCoverage(buildId, jobId, startTime, endTime, pipelineId, prNum)
+      .fetchCoverage({
+        buildId: 243421,
+        jobId: 21,
+        startTime: '2020-05-06T23:36:46.779Z',
+        endTime: '2020-05-06T23:50:18.590Z',
+        pipelineId: 123456,
+        prNum: null
+      })
       .then(result => {
         const { coverage, projectUrl } = result;
 
@@ -98,6 +98,90 @@ module('Unit | Service | shuttle', function (hooks) {
       });
   });
 
+  test('fetchCoverage reuses cached response', async function (assert) {
+    assert.expect(2);
+
+    const service = this.owner.lookup('service:shuttle');
+    const config = {
+      buildId: 243421,
+      jobId: 21,
+      startTime: '2020-05-06T23:36:46.779Z',
+      endTime: '2020-05-06T23:50:18.590Z',
+      pipelineName: 'example'
+    };
+
+    server.get(`${ENV.APP.SDAPI_HOSTNAME}/v4/coverage/info`, () => [
+      200,
+      {
+        'Content-Type': 'application/json'
+      },
+      JSON.stringify({ coverage: '71.4' })
+    ]);
+
+    const firstResponse = await service.fetchCoverage(config);
+    const secondResponse = await service.fetchCoverage(config);
+
+    assert.strictEqual(
+      firstResponse,
+      secondResponse,
+      'uses cached response object'
+    );
+    assert.equal(
+      server.handledRequests.length,
+      1,
+      'only one coverage request is sent'
+    );
+  });
+
+  test('fetchCoverage allows retry after a failed request', async function (assert) {
+    assert.expect(2);
+
+    const service = this.owner.lookup('service:shuttle');
+
+    let requestCount = 0;
+    const config = {
+      buildId: 243421,
+      jobId: 21,
+      startTime: '2020-05-06T23:36:46.779Z',
+      endTime: '2020-05-06T23:50:18.590Z',
+      pipelineName: 'example'
+    };
+
+    server.get(`${ENV.APP.SDAPI_HOSTNAME}/v4/coverage/info`, () => {
+      requestCount += 1;
+
+      if (requestCount === 1) {
+        return [
+          500,
+          { 'Content-Type': 'application/json' },
+          JSON.stringify({})
+        ];
+      }
+
+      return [
+        200,
+        {
+          'Content-Type': 'application/json'
+        },
+        JSON.stringify({ coverage: '71.4' })
+      ];
+    });
+
+    try {
+      await service.fetchCoverage(config);
+    } catch (error) {
+      assert.ok(true, 'first request fails');
+    }
+
+    const response = await service.fetchCoverage(config);
+
+    assert.equal(
+      response.coverage,
+      '71.4',
+      'second request succeeds after retry'
+    );
+  });
+
   test('getLatestCommitEvent payload', function (assert) {
     assert.expect(2);
     const service = this.owner.lookup('service:shuttle');