Commit c8df337a authored by Miranda Fluharty's avatar Miranda Fluharty Committed by Clement Ho

Move code quality comparison into a worker thread

Prevent blocking the main thread while codequality comparison is running
Move filterByKey function to a file so it can be used by the worker
parent 2709ff73
......@@ -159,13 +159,15 @@ export default {
this.isLoadingCodequality = true;
Promise.all([this.service.fetchReport(head_path), this.service.fetchReport(base_path)])
.then(values => {
.then(values =>
this.mr.compareCodeclimateMetrics(
values[0],
values[1],
this.mr.headBlobPath,
this.mr.baseBlobPath,
);
),
)
.then(() => {
this.isLoadingCodequality = false;
})
.catch(() => {
......
import CEMergeRequestStore from '~/vue_merge_request_widget/stores/mr_widget_store';
import { filterByKey } from 'ee/vue_shared/security_reports/store/utils';
import { mapApprovalsResponse, mapApprovalRulesResponse } from '../mappers';
import CodeQualityComparisonWorker from '../workers/code_quality_comparison_worker';
export default class MergeRequestStore extends CEMergeRequestStore {
constructor(data) {
......@@ -94,19 +94,27 @@ export default class MergeRequestStore extends CEMergeRequestStore {
};
}
static doCodeClimateComparison(headIssues, baseIssues) {
// Do these comparisons in worker threads to avoid blocking the main thread
return new Promise(resolve => {
const worker = new CodeQualityComparisonWorker();
worker.addEventListener('message', ({ data }) => resolve(data));
worker.postMessage({
headIssues,
baseIssues,
});
});
}
compareCodeclimateMetrics(headIssues, baseIssues, headBlobPath, baseBlobPath) {
const parsedHeadIssues = MergeRequestStore.parseCodeclimateMetrics(headIssues, headBlobPath);
const parsedBaseIssues = MergeRequestStore.parseCodeclimateMetrics(baseIssues, baseBlobPath);
this.codeclimateMetrics.newIssues = filterByKey(
parsedHeadIssues,
parsedBaseIssues,
'fingerprint',
);
this.codeclimateMetrics.resolvedIssues = filterByKey(
parsedBaseIssues,
parsedHeadIssues,
'fingerprint',
return MergeRequestStore.doCodeClimateComparison(parsedHeadIssues, parsedBaseIssues).then(
response => {
this.codeclimateMetrics.newIssues = response.newIssues;
this.codeclimateMetrics.resolvedIssues = response.resolvedIssues;
},
);
}
......
import filterByKey from 'ee/vue_shared/security_reports/store/utils/filter_by_key';
const keyToFilterBy = 'fingerprint';
// eslint-disable-next-line no-restricted-globals
self.addEventListener('message', e => {
const { data } = e;
if (data === undefined) {
return;
}
// eslint-disable-next-line no-restricted-globals
self.postMessage({
newIssues: filterByKey(data.parsedHeadIssues, data.parsedBaseIssues, keyToFilterBy),
resolvedIssues: filterByKey(data.parsedBaseIssues, data.parsedHeadIssues, keyToFilterBy),
});
// eslint-disable-next-line no-restricted-globals
self.close();
});
......@@ -3,12 +3,12 @@ import * as types from './mutation_types';
import {
parseSastIssues,
parseDependencyScanningIssues,
filterByKey,
getDastSite,
parseDastIssues,
getUnapprovedVulnerabilities,
findIssueIndex,
} from './utils';
import filterByKey from './utils/filter_by_key';
import { parseSastContainer } from './utils/container_scanning';
import { visitUrl } from '~/lib/utils/url_utility';
......
......@@ -254,17 +254,6 @@ export const parseDastIssues = (issues = [], feedback = []) =>
};
});
/**
* Compares two arrays by the given key and returns the difference
*
* @param {Array} firstArray
* @param {Array} secondArray
* @param {String} key
* @returns {Array}
*/
export const filterByKey = (firstArray = [], secondArray = [], key = '') =>
firstArray.filter(item => !secondArray.find(el => el[key] === item[key]));
export const getUnapprovedVulnerabilities = (issues = [], unapproved = []) =>
issues.filter(item => unapproved.find(el => el === item.vulnerability));
......
/**
* Compares two arrays by the given key and returns the difference
*
* @param {Array} firstArray
* @param {Array} secondArray
* @param {String} key
* @returns {Array}
*/
const filterByKey = (firstArray = [], secondArray = [], key = '') =>
firstArray.filter(item => !secondArray.find(el => el[key] === item[key]));
export default filterByKey;
---
title: Un-block UI interactions while Code Quality MR widget is loading
merge_request: 14323
author:
type: fixed
......@@ -6,12 +6,12 @@ import {
parseDependencyScanningIssues,
getDastSite,
parseDastIssues,
filterByKey,
getUnapprovedVulnerabilities,
groupedTextBuilder,
statusIcon,
countIssues,
} from 'ee/vue_shared/security_reports/store/utils';
import filterByKey from 'ee/vue_shared/security_reports/store/utils/filter_by_key';
import {
formatContainerScanningDescription,
formatContainerScanningMessage,
......
......@@ -4,6 +4,7 @@ import axios from '~/lib/utils/axios_utils';
import mrWidgetOptions from 'ee/vue_merge_request_widget/mr_widget_options.vue';
import MRWidgetService from 'ee/vue_merge_request_widget/services/mr_widget_service';
import MRWidgetStore from 'ee/vue_merge_request_widget/stores/mr_widget_store';
import filterByKey from 'ee/vue_shared/security_reports/store/utils/filter_by_key';
import mountComponent from 'spec/helpers/vue_mount_component_helper';
import { TEST_HOST } from 'spec/test_constants';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
......@@ -14,6 +15,8 @@ import mockData, {
headIssues,
basePerformance,
headPerformance,
parsedBaseIssues,
parsedHeadIssues,
} from 'ee_spec/vue_mr_widget/mock_data';
import {
......@@ -322,6 +325,14 @@ describe('ee merge request widget options', () => {
mock.onGet('head.json').reply(200, headIssues);
mock.onGet('base.json').reply(200, baseIssues);
vm = mountComponent(Component);
// mock worker response
spyOn(MRWidgetStore, 'doCodeClimateComparison').and.callFake(() =>
Promise.resolve({
newIssues: filterByKey(parsedHeadIssues, parsedBaseIssues, 'fingerprint'),
resolvedIssues: filterByKey(parsedBaseIssues, parsedHeadIssues, 'fingerprint'),
}),
);
});
it('should render provided data', done => {
......@@ -372,6 +383,14 @@ describe('ee merge request widget options', () => {
mock.onGet('head.json').reply(200, []);
mock.onGet('base.json').reply(200, []);
vm = mountComponent(Component);
// mock worker response
spyOn(MRWidgetStore, 'doCodeClimateComparison').and.callFake(() =>
Promise.resolve({
newIssues: filterByKey([], [], 'fingerprint'),
resolvedIssues: filterByKey([], [], 'fingerprint'),
}),
);
});
afterEach(() => {
......@@ -390,6 +409,28 @@ describe('ee merge request widget options', () => {
});
});
describe('with codeclimate comparison worker rejection', () => {
beforeEach(() => {
mock.onGet('head.json').reply(200, headIssues);
mock.onGet('base.json').reply(200, baseIssues);
vm = mountComponent(Component);
// mock worker rejection
spyOn(MRWidgetStore, 'doCodeClimateComparison').and.callFake(() => Promise.reject());
});
it('should render error indicator', done => {
setTimeout(() => {
expect(
removeBreakLine(
vm.$el.querySelector('.js-codequality-widget .js-code-text').textContent,
),
).toEqual('Failed to load codeclimate report');
done();
}, 0);
});
});
describe('with failed request', () => {
beforeEach(() => {
mock.onGet('head.json').reply(500, []);
......
import MergeRequestStore from 'ee/vue_merge_request_widget/stores/mr_widget_store';
import filterByKey from 'ee/vue_shared/security_reports/store/utils/filter_by_key';
import { stateKey } from '~/vue_merge_request_widget/stores/state_maps';
import mockData, {
headIssues,
......@@ -16,7 +17,15 @@ describe('MergeRequestStore', () => {
describe('compareCodeclimateMetrics', () => {
beforeEach(() => {
store.compareCodeclimateMetrics(headIssues, baseIssues, 'headPath', 'basePath');
// mock worker response
spyOn(MergeRequestStore, 'doCodeClimateComparison').and.callFake(() =>
Promise.resolve({
newIssues: filterByKey(parsedHeadIssues, parsedBaseIssues, 'fingerprint'),
resolvedIssues: filterByKey(parsedBaseIssues, parsedHeadIssues, 'fingerprint'),
}),
);
return store.compareCodeclimateMetrics(headIssues, baseIssues, 'headPath', 'basePath');
});
it('should return the new issues', () => {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment