Skip to content

G
gitlab-ce
Commit d9260cc3 authored by Phil Hughes's avatar Phil Hughes
Browse files
Options

Merge branch... 

Merge branch '7737-ci-pipeline-view-slowed-down-massivly-if-security-tabs-has-many-entries-ee' into 'master'

Improve performance of rendering large reports

Closes #7737

See merge request gitlab-org/gitlab-ee!8027
parents 958d9d91 a7e61568
Hide whitespace changes
Inline Side-by-side
Showing with 353 additions and 127 deletions
app/assets/javascripts/reports/components/issues_list.vue
View file @ d9260cc3
<script>
import IssuesBlock from '~/reports/components/report_issues.vue';
import { STATUS_SUCCESS, STATUS_FAILED, STATUS_NEUTRAL } from '~/reports/constants';
import ReportItem from '~/reports/components/report_item.vue';
import { STATUS_FAILED, STATUS_NEUTRAL, STATUS_SUCCESS } from '~/reports/constants';
import SmartVirtualList from '~/vue_shared/components/smart_virtual_list.vue';
const wrapIssueWithState = (status, isNew = false) => issue => ({
status: issue.status || status,
isNew,
issue,
});
/**
* Renders block of issues
*/
export default {
components: {
IssuesBlock,
SmartVirtualList,
ReportItem,
},
success: STATUS_SUCCESS,
failed: STATUS_FAILED,
neutral: STATUS_NEUTRAL,
// Typical height of a report item in px
typicalReportItemHeight: 32,
/*
The maximum amount of shown issues. This is calculated by
( max-height of report-block-list / typicalReportItemHeight ) + some safety margin
We will use VirtualList if we have more items than this number.
For entries lower than this number, the virtual scroll list calculates the total height of the element wrongly.
*/
maxShownReportItems: 20,
props: {
newIssues: {
type: Array,
......@@ -40,42 +53,34 @@ export default {
default: '',
},
},
computed: {
issuesWithState() {
return [
...this.newIssues.map(wrapIssueWithState(STATUS_FAILED, true)),
...this.unresolvedIssues.map(wrapIssueWithState(STATUS_FAILED)),
...this.neutralIssues.map(wrapIssueWithState(STATUS_NEUTRAL)),
...this.resolvedIssues.map(wrapIssueWithState(STATUS_SUCCESS)),
];
},
},
};
</script>
<template>
<div class="report-block-container">
<issues-block
v-if="newIssues.length"
:component="component"
:issues="newIssues"
class="js-mr-code-new-issues"
status="failed"
is-new
/>
<issues-block
v-if="unresolvedIssues.length"
:component="component"
:issues="unresolvedIssues"
:status="$options.failed"
class="js-mr-code-new-issues"
/>
<issues-block
v-if="neutralIssues.length"
:component="component"
:issues="neutralIssues"
:status="$options.neutral"
class="js-mr-code-non-issues"
/>
<issues-block
v-if="resolvedIssues.length"
<smart-virtual-list
:length="issuesWithState.length"
:remain="$options.maxShownReportItems"
:size="$options.typicalReportItemHeight"
class="report-block-container"
wtag="ul"
wclass="report-block-list"
>
<report-item
v-for="(wrapped, index) in issuesWithState"
:key="index"
:issue="wrapped.issue"
:status="wrapped.status"
:component="component"
:issues="resolvedIssues"
:status="$options.success"
class="js-mr-code-resolved-issues"
:is-new="wrapped.isNew"
/>
</div>
</smart-virtual-list>
</template>
app/assets/javascripts/reports/components/report_issues.vue → app/assets/javascripts/reports/components/report_item.vue
View file @ d9260cc3
......@@ -3,14 +3,14 @@ import IssueStatusIcon from '~/reports/components/issue_status_icon.vue';
import { components, componentNames } from 'ee/vue_shared/components/reports/issue_body';
export default {
name: 'ReportIssues',
name: 'ReportItem',
components: {
IssueStatusIcon,
...components,
},
props: {
issues: {
type: Array,
issue: {
type: Object,
required: true,
},
component: {
......@@ -33,27 +33,21 @@ export default {
};
</script>
<template>
<div>
<ul class="report-block-list">
<li
v-for="(issue, index) in issues"
:key="index"
:class="{ 'is-dismissed': issue.isDismissed }"
class="report-block-list-issue"
>
<issue-status-icon
:status="issue.status || status"
class="append-right-5"
/>
<li
:class="{ 'is-dismissed': issue.isDismissed }"
class="report-block-list-issue"
>
<issue-status-icon
:status="status"
class="append-right-5"
/>
<component
:is="component"
v-if="component"
:issue="issue"
:status="issue.status || status"
:is-new="isNew"
/>
</li>
</ul>
</div>
<component
:is="component"
v-if="component"
:issue="issue"
:status="status"
:is-new="isNew"
/>
</li>
</template>
app/assets/javascripts/vue_shared/components/smart_virtual_list.vue 0 → 100644
View file @ d9260cc3
<script>
import VirtualList from 'vue-virtual-scroll-list';
export default {
name: 'SmartVirtualList',
components: { VirtualList },
props: {
size: { type: Number, required: true },
length: { type: Number, required: true },
remain: { type: Number, required: true },
rtag: { type: String, default: 'div' },
wtag: { type: String, default: 'div' },
wclass: { type: String, default: null },
},
};
</script>
<template>
<virtual-list
v-if="length > remain"
v-bind="$attrs"
:size="remain"
:remain="remain"
:rtag="rtag"
:wtag="wtag"
:wclass="wclass"
class="js-virtual-list"
>
<slot></slot>
</virtual-list>
<component
:is="rtag"
v-else
class="js-plain-element"
>
<component
:is="wtag"
:class="wclass"
>
<slot></slot>
</component>
</component>
</template>
changelogs/unreleased/7737-ci-pipeline-view-slowed-down-massivly-if-security-tabs-has-many-entries-ee.yml 0 → 100644
View file @ d9260cc3
---
title: Improve performance of rendering large reports
merge_request: 22835
author:
type: performance
ee/spec/javascripts/vue_shared/components/reports/report_issues_spec.js
View file @ d9260cc3
import Vue from 'vue';
import reportIssues from '~/reports/components/report_issues.vue';
import reportIssues from '~/reports/components/report_item.vue';
import { STATUS_FAILED, STATUS_SUCCESS } from '~/reports/constants';
import { componentNames } from 'ee/vue_shared/components/reports/issue_body';
import store from 'ee/vue_shared/security_reports/store';
......@@ -27,76 +27,45 @@ describe('Report issues', () => {
describe('resolved issues', () => {
beforeEach(() => {
vm = mountComponent(ReportIssues, {
issues: codequalityParsedIssues,
issue: codequalityParsedIssues[0],
component: componentNames.CodequalityIssueBody,
status: STATUS_SUCCESS,
});
});
it('should render a list of resolved issues', () => {
expect(vm.$el.querySelectorAll('.report-block-list li').length).toEqual(
codequalityParsedIssues.length,
);
});
it('should render "Fixed" keyword', () => {
expect(vm.$el.querySelector('.report-block-list li').textContent).toContain('Fixed');
expect(
vm.$el
.querySelector('.report-block-list li')
.textContent.replace(/\s+/g, ' ')
.trim(),
).toEqual('Fixed: Insecure Dependency in Gemfile.lock:12');
expect(vm.$el.textContent).toContain('Fixed');
expect(vm.$el.textContent.replace(/\s+/g, ' ').trim()).toEqual(
'Fixed: Insecure Dependency in Gemfile.lock:12',
);
});
});
describe('unresolved issues', () => {
beforeEach(() => {
vm = mountComponent(ReportIssues, {
issues: codequalityParsedIssues,
issue: codequalityParsedIssues[0],
component: componentNames.CodequalityIssueBody,
status: STATUS_FAILED,
});
});
it('should render a list of unresolved issues', () => {
expect(vm.$el.querySelectorAll('.report-block-list li').length).toEqual(
codequalityParsedIssues.length,
);
});
it('should not render "Fixed" keyword', () => {
expect(vm.$el.querySelector('.report-block-list li').textContent).not.toContain('Fixed');
expect(vm.$el.textContent).not.toContain('Fixed');
});
});
});
describe('for security issues', () => {
beforeEach(() => {
vm = mountComponent(ReportIssues, {
issues: sastParsedIssues,
component: componentNames.SastIssueBody,
status: STATUS_FAILED,
});
});
it('should render a list of unresolved issues', () => {
expect(vm.$el.querySelectorAll('.report-block-list li').length).toEqual(
sastParsedIssues.length,
);
});
});
describe('with location', () => {
it('should render location', () => {
vm = mountComponent(ReportIssues, {
issues: sastParsedIssues,
issue: sastParsedIssues[0],
component: componentNames.SastIssueBody,
status: STATUS_FAILED,
});
expect(vm.$el.querySelector('.report-block-list li').textContent).toContain('in');
expect(vm.$el.querySelector('.report-block-list li a').getAttribute('href')).toEqual(
expect(vm.$el.textContent).toContain('in');
expect(vm.$el.querySelector('.report-block-list-issue a').getAttribute('href')).toEqual(
sastParsedIssues[0].urlPath,
);
});
......@@ -105,47 +74,41 @@ describe('Report issues', () => {
describe('without location', () => {
it('should not render location', () => {
vm = mountComponent(ReportIssues, {
issues: [
{
title: 'foo',
},
],
issue: {
title: 'foo',
},
component: componentNames.SastIssueBody,
status: STATUS_SUCCESS,
});
expect(vm.$el.querySelector('.report-block-list li').textContent).not.toContain('in');
expect(vm.$el.querySelector('.report-block-list li a')).toEqual(null);
expect(vm.$el.textContent).not.toContain('in');
expect(vm.$el.querySelector('.report-block-list-issue a')).toEqual(null);
});
});
describe('for container scanning issues', () => {
beforeEach(() => {
vm = mountComponent(ReportIssues, {
issues: dockerReportParsed.unapproved,
issue: dockerReportParsed.unapproved[0],
component: componentNames.SastContainerIssueBody,
status: STATUS_FAILED,
});
});
it('renders severity', () => {
expect(vm.$el.querySelector('.report-block-list li').textContent.trim()).toContain(
dockerReportParsed.unapproved[0].severity,
);
expect(vm.$el.textContent.trim()).toContain(dockerReportParsed.unapproved[0].severity);
});
it('renders CVE name', () => {
expect(vm.$el.querySelector('.report-block-list button').textContent.trim()).toEqual(
expect(vm.$el.querySelector('.report-block-list-issue button').textContent.trim()).toEqual(
dockerReportParsed.unapproved[0].title,
);
});
it('renders namespace', () => {
expect(vm.$el.querySelector('.report-block-list li').textContent.trim()).toContain(
dockerReportParsed.unapproved[0].path,
);
expect(vm.$el.textContent.trim()).toContain(dockerReportParsed.unapproved[0].path);
expect(vm.$el.querySelector('.report-block-list li').textContent.trim()).toContain('in');
expect(vm.$el.textContent.trim()).toContain('in');
});
});
......@@ -154,7 +117,7 @@ describe('Report issues', () => {
vm = mountComponentWithStore(ReportIssues, {
store,
props: {
issues: parsedDast,
issue: parsedDast[0],
component: componentNames.DastIssueBody,
status: STATUS_FAILED,
},
......
ee/spec/javascripts/vue_shared/components/reports/report_item_spec.js 0 → 100644
View file @ d9260cc3
import Vue from 'vue';
import reportIssue from '~/reports/components/report_item.vue';
import { STATUS_FAILED, STATUS_SUCCESS } from '~/reports/constants';
import { componentNames } from 'ee/vue_shared/components/reports/issue_body';
import store from 'ee/vue_shared/security_reports/store';
import mountComponent, { mountComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import { codequalityParsedIssues } from 'spec/vue_mr_widget/mock_data';
import {
sastParsedIssues,
dockerReportParsed,
parsedDast,
} from 'ee_spec/vue_shared/security_reports/mock_data';
describe('Report issue', () => {
let vm;
let ReportIssue;
beforeEach(() => {
ReportIssue = Vue.extend(reportIssue);
});
afterEach(() => {
vm.$destroy();
});
describe('for codequality issue', () => {
describe('resolved issue', () => {
beforeEach(() => {
vm = mountComponent(ReportIssue, {
issue: codequalityParsedIssues[0],
component: componentNames.CodequalityIssueBody,
status: STATUS_SUCCESS,
});
});
it('should render "Fixed" keyword', () => {
expect(vm.$el.textContent).toContain('Fixed');
expect(vm.$el.textContent.replace(/\s+/g, ' ').trim()).toEqual(
'Fixed: Insecure Dependency in Gemfile.lock:12',
);
});
});
describe('unresolved issue', () => {
beforeEach(() => {
vm = mountComponent(ReportIssue, {
issue: codequalityParsedIssues[0],
component: componentNames.CodequalityIssueBody,
status: STATUS_FAILED,
});
});
it('should not render "Fixed" keyword', () => {
expect(vm.$el.textContent).not.toContain('Fixed');
});
});
});
describe('with location', () => {
it('should render location', () => {
vm = mountComponent(ReportIssue, {
issue: sastParsedIssues[0],
component: componentNames.SastIssueBody,
status: STATUS_FAILED,
});
expect(vm.$el.textContent).toContain('in');
expect(vm.$el.querySelector('li a').getAttribute('href')).toEqual(
sastParsedIssues[0].urlPath,
);
});
});
describe('without location', () => {
it('should not render location', () => {
vm = mountComponent(ReportIssue, {
issue: {
title: 'foo',
},
component: componentNames.SastIssueBody,
status: STATUS_SUCCESS,
});
expect(vm.$el.textContent).not.toContain('in');
expect(vm.$el.querySelector('a')).toEqual(null);
});
});
describe('for container scanning issue', () => {
beforeEach(() => {
vm = mountComponent(ReportIssue, {
issue: dockerReportParsed.unapproved[0],
component: componentNames.SastContainerIssueBody,
status: STATUS_FAILED,
});
});
it('renders severity', () => {
expect(vm.$el.textContent.trim()).toContain(dockerReportParsed.unapproved[0].severity);
});
it('renders CVE name', () => {
expect(vm.$el.querySelector('button').textContent.trim()).toEqual(
dockerReportParsed.unapproved[0].title,
);
});
it('renders namespace', () => {
expect(vm.$el.textContent.trim()).toContain(dockerReportParsed.unapproved[0].path);
expect(vm.$el.textContent.trim()).toContain('in');
});
});
describe('for dast issue', () => {
beforeEach(() => {
vm = mountComponentWithStore(ReportIssue, {
store,
props: {
issue: parsedDast[0],
component: componentNames.DastIssueBody,
status: STATUS_FAILED,
},
});
});
it('renders severity (confidence) and title', () => {
expect(vm.$el.textContent).toContain(parsedDast[0].title);
expect(vm.$el.textContent).toContain(
`${parsedDast[0].severity} (${parsedDast[0].confidence})`,
);
});
});
});
spec/javascripts/reports/components/grouped_test_reports_app_spec.js
View file @ d9260cc3
......@@ -151,11 +151,11 @@ describe('Grouped Test Reports App', () => {
it('renders resolved failures', done => {
setTimeout(() => {
expect(vm.$el.querySelector('.js-mr-code-resolved-issues').textContent).toContain(
expect(vm.$el.querySelector('.report-block-container').textContent).toContain(
resolvedFailures.suites[0].resolved_failures[0].name,
);
expect(vm.$el.querySelector('.js-mr-code-resolved-issues').textContent).toContain(
expect(vm.$el.querySelector('.report-block-container').textContent).toContain(
resolvedFailures.suites[0].resolved_failures[1].name,
);
done();
......
spec/javascripts/reports/components/report_section_spec.js
View file @ d9260cc3
......@@ -120,7 +120,7 @@ describe('Report section', () => {
'Code quality improved on 1 point and degraded on 1 point',
);
expect(vm.$el.querySelectorAll('.js-mr-code-resolved-issues li').length).toEqual(
expect(vm.$el.querySelectorAll('.report-block-container li').length).toEqual(
resolvedIssues.length,
);
});
......
spec/javascripts/vue_shared/components/smart_virtual_list_spec.js 0 → 100644
View file @ d9260cc3
import Vue from 'vue';
import SmartVirtualScrollList from '~/vue_shared/components/smart_virtual_list.vue';
import mountComponent from 'spec/helpers/vue_mount_component_helper';
describe('Toggle Button', () => {
let vm;
const createComponent = ({ length, remain }) => {
const smartListProperties = {
rtag: 'section',
wtag: 'ul',
wclass: 'test-class',
// Size in pixels does not matter for our tests here
size: 35,
length,
remain,
};
const Component = Vue.extend({
components: {
SmartVirtualScrollList,
},
smartListProperties,
items: Array(length).fill(1),
template: `
<smart-virtual-scroll-list v-bind="$options.smartListProperties">
<li v-for="(val, key) in $options.items" :key="key">{{ key + 1 }}</li>
</smart-virtual-scroll-list>`,
});
return mountComponent(Component);
};
afterEach(() => {
vm.$destroy();
});
describe('if the list is shorter than the maximum shown elements', () => {
const listLength = 10;
beforeEach(() => {
vm = createComponent({ length: listLength, remain: 20 });
});
it('renders without the vue-virtual-scroll-list component', () => {
expect(vm.$el.classList).not.toContain('js-virtual-list');
expect(vm.$el.classList).toContain('js-plain-element');
});
it('renders list with provided tags and classes for the wrapper elements', () => {
expect(vm.$el.tagName).toEqual('SECTION');
expect(vm.$el.firstChild.tagName).toEqual('UL');
expect(vm.$el.firstChild.classList).toContain('test-class');
});
it('renders all children list elements', () => {
expect(vm.$el.querySelectorAll('li').length).toEqual(listLength);
});
});
describe('if the list is longer than the maximum shown elements', () => {
const maxItemsShown = 20;
beforeEach(() => {
vm = createComponent({ length: 1000, remain: maxItemsShown });
});
it('uses the vue-virtual-scroll-list component', () => {
expect(vm.$el.classList).toContain('js-virtual-list');
expect(vm.$el.classList).not.toContain('js-plain-element');
});
it('renders list with provided tags and classes for the wrapper elements', () => {
expect(vm.$el.tagName).toEqual('SECTION');
expect(vm.$el.firstChild.tagName).toEqual('UL');
expect(vm.$el.firstChild.classList).toContain('test-class');
});
it('renders at max twice the maximum shown elements', () => {
expect(vm.$el.querySelectorAll('li').length).toBeLessThanOrEqual(2 * maxItemsShown);
});
});
});
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7