Browse Source

SONAR-17416 Improve Source code viewer perfomance and allow to load more line each time

tags/9.7.0.61563
Mathieu Suen 1 year ago
parent
commit
27a5998843

+ 22
- 84
server/sonar-web/src/main/js/components/SourceViewer/SourceViewer.tsx View File

@@ -17,7 +17,7 @@
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
import { intersection, uniqBy } from 'lodash';
import { intersection } from 'lodash';
import * as React from 'react';
import {
getComponentData,
@@ -56,7 +56,7 @@ import {
symbolsByLine
} from './helpers/indexing';
import { LINES_TO_LOAD } from './helpers/lines';
import defaultLoadIssues from './helpers/loadIssues';
import loadIssues from './helpers/loadIssues';
import SourceViewerCode from './SourceViewerCode';
import { SourceViewerContext } from './SourceViewerContext';
import SourceViewerHeader from './SourceViewerHeader';
@@ -76,12 +76,6 @@ export interface Props {
// but kept to maintaint the location indexes
highlightedLocations?: (FlowLocation | undefined)[];
highlightedLocationMessage?: { index: number; text: string | undefined };
loadIssues?: (
component: string,
from: number,
to: number,
branchLike: BranchLike | undefined
) => Promise<Issue[]>;
onLoaded?: (component: SourceViewerFile, sources: SourceLine[], issues: Issue[]) => void;
onLocationSelect?: (index: number) => void;
onIssueChange?: (issue: Issue) => void;
@@ -116,7 +110,6 @@ interface State {
}

export default class SourceViewer extends React.PureComponent<Props, State> {
node?: HTMLElement | null;
mounted = false;

static defaultProps = {
@@ -184,8 +177,6 @@ export default class SourceViewer extends React.PureComponent<Props, State> {
}
);
}
} else {
this.checkSelectedIssueChange();
}
}

@@ -203,18 +194,6 @@ export default class SourceViewer extends React.PureComponent<Props, State> {
}));
}

checkSelectedIssueChange() {
const { selectedIssue } = this.props;
const { issues } = this.state;
if (
selectedIssue !== undefined &&
issues !== undefined &&
issues.find(issue => issue.key === selectedIssue) === undefined
) {
this.reloadIssues();
}
}

loadSources(
key: string,
from: number | undefined,
@@ -224,10 +203,6 @@ export default class SourceViewer extends React.PureComponent<Props, State> {
return getSources({ key, from, to, ...getBranchLikeQuery(branchLike) });
}

get loadIssues() {
return this.props.loadIssues || defaultLoadIssues;
}

computeCoverageStatus(lines: SourceLine[]) {
return lines.map(line => ({ ...line, coverageStatus: getCoverageStatus(line) }));
}
@@ -246,9 +221,8 @@ export default class SourceViewer extends React.PureComponent<Props, State> {
fetchComponent() {
this.setState({ loading: true });

const to = (this.props.aroundLine || 0) + LINES_TO_LOAD;
const loadIssues = (component: SourceViewerFile, sources: SourceLine[]) => {
this.loadIssues(this.props.component, 1, to, this.props.branchLike).then(
const loadIssuesCallback = (component: SourceViewerFile, sources: SourceLine[]) => {
loadIssues(this.props.component, this.props.branchLike).then(
issues => {
if (this.mounted) {
const finalSources = sources.slice(0, LINES_TO_LOAD);
@@ -310,7 +284,7 @@ export default class SourceViewer extends React.PureComponent<Props, State> {
const sourcesRequest =
component.q === 'FIL' || component.q === 'UTS' ? this.fetchSources() : Promise.resolve([]);
sourcesRequest.then(
sources => loadIssues(component, sources),
sources => loadIssuesCallback(component, sources),
response => onFailLoadSources(response, component)
);
};
@@ -321,33 +295,6 @@ export default class SourceViewer extends React.PureComponent<Props, State> {
);
}

reloadIssues() {
if (!this.state.sources) {
return;
}
const firstSourceLine = this.state.sources[0];
const lastSourceLine = this.state.sources[this.state.sources.length - 1];
this.loadIssues(
this.props.component,
firstSourceLine && firstSourceLine.line,
lastSourceLine && lastSourceLine.line,
this.props.branchLike
).then(
issues => {
if (this.mounted) {
this.setState({
issues,
issuesByLine: issuesByLine(issues),
issueLocationsByLine: locationsByLine(issues)
});
}
},
() => {
/* no op */
}
);
}

fetchSources = (): Promise<SourceLine[]> => {
return new Promise((resolve, reject) => {
const onFailLoadSources = (response: Response) => {
@@ -387,18 +334,16 @@ export default class SourceViewer extends React.PureComponent<Props, State> {
const firstSourceLine = this.state.sources[0];
this.setState({ loadingSourcesBefore: true });
const from = Math.max(1, firstSourceLine.line - LINES_TO_LOAD);
Promise.all([
this.loadSources(this.props.component, from, firstSourceLine.line - 1, this.props.branchLike),
this.loadIssues(this.props.component, from, firstSourceLine.line - 1, this.props.branchLike)
]).then(
([sources, issues]) => {
this.loadSources(
this.props.component,
from,
firstSourceLine.line - 1,
this.props.branchLike
).then(
sources => {
if (this.mounted) {
this.setState(prevState => {
const nextIssues = uniqBy([...issues, ...(prevState.issues || [])], issue => issue.key);
return {
issues: nextIssues,
issuesByLine: issuesByLine(nextIssues),
issueLocationsByLine: locationsByLine(nextIssues),
loadingSourcesBefore: false,
sources: [...this.computeCoverageStatus(sources), ...(prevState.sources || [])],
symbolsByLine: { ...prevState.symbolsByLine, ...symbolsByLine(sources) }
@@ -419,29 +364,22 @@ export default class SourceViewer extends React.PureComponent<Props, State> {
const lastSourceLine = this.state.sources[this.state.sources.length - 1];
this.setState({ loadingSourcesAfter: true });
const fromLine = lastSourceLine.line + 1;
// request one additional line to define `hasSourcesAfter`
const toLine = lastSourceLine.line + LINES_TO_LOAD + 1;
Promise.all([
this.loadSources(this.props.component, fromLine, toLine, this.props.branchLike),
this.loadIssues(this.props.component, fromLine, toLine, this.props.branchLike)
]).then(
([sources, issues]) => {
this.loadSources(this.props.component, fromLine, toLine, this.props.branchLike).then(
sources => {
if (this.mounted) {
const hasSourcesAfter = LINES_TO_LOAD < sources.length;
if (hasSourcesAfter) {
sources.pop();
}
this.setState(prevState => {
const nextIssues = uniqBy([...(prevState.issues || []), ...issues], issue => issue.key);
return {
issues: nextIssues,
issuesByLine: issuesByLine(nextIssues),
issueLocationsByLine: locationsByLine(nextIssues),
hasSourcesAfter: sources.length > LINES_TO_LOAD,
hasSourcesAfter,
loadingSourcesAfter: false,
sources: [
...(prevState.sources || []),
...this.computeCoverageStatus(sources.slice(0, LINES_TO_LOAD))
],
sources: [...(prevState.sources || []), ...this.computeCoverageStatus(sources)],
symbolsByLine: {
...prevState.symbolsByLine,
...symbolsByLine(sources.slice(0, LINES_TO_LOAD))
...symbolsByLine(sources)
}
};
});
@@ -646,7 +584,7 @@ export default class SourceViewer extends React.PureComponent<Props, State> {

return (
<SourceViewerContext.Provider value={{ branchLike: this.props.branchLike, file: component }}>
<div className="source-viewer" ref={node => (this.node = node)}>
<div className="source-viewer">
{this.renderHeader(component)}
{sourceRemoved && (
<Alert className="spacer-top" variant="warning">

+ 39
- 36
server/sonar-web/src/main/js/components/SourceViewer/__tests__/SourceViewer-it.tsx View File

@@ -24,10 +24,15 @@ import { SourceViewerServiceMock } from '../../../api/mocks/SourceViewerServiceM
import { HttpStatus } from '../../../helpers/request';
import { mockIssue } from '../../../helpers/testMocks';
import { renderComponent } from '../../../helpers/testReactTestingUtils';
import loadIssues from '../helpers/loadIssues';
import SourceViewer from '../SourceViewer';

jest.mock('../../../api/components');
jest.mock('../../../api/issues');
jest.mock('../helpers/loadIssues', () => ({
__esModule: true,
default: jest.fn().mockResolvedValue([])
}));
jest.mock('../helpers/lines', () => {
const lines = jest.requireActual('../helpers/lines');
return {
@@ -98,34 +103,33 @@ it('should show a permalink on line number', async () => {
});

it('should show issue on empty file', async () => {
(loadIssues as jest.Mock).mockResolvedValueOnce([
mockIssue(false, {
key: 'first-issue',
message: 'First Issue',
line: undefined,
textRange: undefined
})
]);
renderSourceViewer({
component: handler.getEmptyFile(),
loadIssues: jest.fn().mockResolvedValue([
mockIssue(false, {
key: 'first-issue',
message: 'First Issue',
line: undefined,
textRange: undefined
})
])
component: handler.getEmptyFile()
});
expect(await screen.findByRole('table')).toBeInTheDocument();
expect(await screen.findByRole('row', { name: 'First Issue' })).toBeInTheDocument();
});

it('should be able to interact with issue action', async () => {
(loadIssues as jest.Mock).mockResolvedValueOnce([
mockIssue(false, {
actions: ['set_type', 'set_tags', 'comment', 'set_severity', 'assign'],
key: 'first-issue',
message: 'First Issue',
line: 1,
textRange: { startLine: 1, endLine: 1, startOffset: 0, endOffset: 1 }
})
]);
const user = userEvent.setup();
renderSourceViewer({
loadIssues: jest.fn().mockResolvedValue([
mockIssue(false, {
actions: ['set_type', 'set_tags', 'comment', 'set_severity', 'assign'],
key: 'first-issue',
message: 'First Issue',
line: 1,
textRange: { startLine: 1, endLine: 1, startOffset: 0, endOffset: 1 }
})
])
});
renderSourceViewer();

//Open Issue type
await user.click(
@@ -259,25 +263,25 @@ it('should show SCM information', async () => {
});

it('should show issue indicator', async () => {
(loadIssues as jest.Mock).mockResolvedValueOnce([
mockIssue(false, {
key: 'first-issue',
message: 'First Issue',
line: 1,
textRange: { startLine: 1, endLine: 1, startOffset: 0, endOffset: 1 }
}),
mockIssue(false, {
key: 'second-issue',
message: 'Second Issue',
line: 1,
textRange: { startLine: 1, endLine: 1, startOffset: 1, endOffset: 2 }
})
]);
const user = userEvent.setup();
const onIssueSelect = jest.fn();
renderSourceViewer({
onIssueSelect,
displayAllIssues: false,
loadIssues: jest.fn().mockResolvedValue([
mockIssue(false, {
key: 'first-issue',
message: 'First Issue',
line: 1,
textRange: { startLine: 1, endLine: 1, startOffset: 0, endOffset: 1 }
}),
mockIssue(false, {
key: 'second-issue',
message: 'Second Issue',
line: 1,
textRange: { startLine: 1, endLine: 1, startOffset: 1, endOffset: 2 }
})
])
displayAllIssues: false
});
const row = await screen.findByRole('row', { name: /.*\/ \*$/ });
const issueRow = within(row);
@@ -391,7 +395,6 @@ function getSourceViewerUi(override?: Partial<SourceViewer['props']>) {
displayIssueLocationsCount={true}
displayIssueLocationsLink={false}
displayLocationMarkers={true}
loadIssues={jest.fn().mockResolvedValue([])}
onIssueChange={jest.fn()}
onIssueSelect={jest.fn()}
onLoaded={jest.fn()}

+ 10
- 39
server/sonar-web/src/main/js/components/SourceViewer/__tests__/SourceViewer-test.tsx View File

@@ -57,41 +57,11 @@ it('should render correctly', async () => {
expect(wrapper).toMatchSnapshot();
});

it('should use load props if provided', () => {
const loadIssues = jest.fn().mockResolvedValue([]);
const wrapper = shallowRender({
loadIssues
});

expect(wrapper.instance().loadIssues).toBe(loadIssues);
});

it('should reload', async () => {
(defaultLoadIssues as jest.Mock)
.mockResolvedValueOnce([mockIssue()])
.mockResolvedValueOnce([mockIssue()]);
(getComponentForSourceViewer as jest.Mock).mockResolvedValueOnce(mockSourceViewerFile());
(getComponentData as jest.Mock).mockResolvedValueOnce({
component: { leakPeriodDate: '2018-06-20T17:12:19+0200' }
});
(getSources as jest.Mock).mockResolvedValueOnce([mockSourceLine()]);

const wrapper = shallowRender();
await waitAndUpdate(wrapper);

wrapper.instance().reloadIssues();

expect(defaultLoadIssues).toBeCalledTimes(2);

await waitAndUpdate(wrapper);

expect(wrapper.state().issues).toHaveLength(1);
});

it('should load sources before', async () => {
(defaultLoadIssues as jest.Mock)
.mockResolvedValueOnce([mockIssue(false, { key: 'issue1' })])
.mockResolvedValueOnce([mockIssue(false, { key: 'issue2' })]);
(defaultLoadIssues as jest.Mock).mockResolvedValueOnce([
mockIssue(false, { key: 'issue1' }),
mockIssue(false, { key: 'issue2' })
]);
(getComponentForSourceViewer as jest.Mock).mockResolvedValueOnce(mockSourceViewerFile());
(getComponentData as jest.Mock).mockResolvedValueOnce({
component: { leakPeriodDate: '2018-06-20T17:12:19+0200' }
@@ -106,7 +76,7 @@ it('should load sources before', async () => {
wrapper.instance().loadSourcesBefore();
expect(wrapper.state().loadingSourcesBefore).toBe(true);

expect(defaultLoadIssues).toBeCalledTimes(2);
expect(defaultLoadIssues).toBeCalledTimes(1);
expect(getSources).toBeCalledTimes(2);

await waitAndUpdate(wrapper);
@@ -115,9 +85,10 @@ it('should load sources before', async () => {
});

it('should load sources after', async () => {
(defaultLoadIssues as jest.Mock)
.mockResolvedValueOnce([mockIssue(false, { key: 'issue1' })])
.mockResolvedValueOnce([mockIssue(false, { key: 'issue2' })]);
(defaultLoadIssues as jest.Mock).mockResolvedValueOnce([
mockIssue(false, { key: 'issue1' }),
mockIssue(false, { key: 'issue2' })
]);
(getComponentForSourceViewer as jest.Mock).mockResolvedValueOnce(mockSourceViewerFile());
(getComponentData as jest.Mock).mockResolvedValueOnce({
component: { leakPeriodDate: '2018-06-20T17:12:19+0200' }
@@ -132,7 +103,7 @@ it('should load sources after', async () => {
wrapper.instance().loadSourcesAfter();
expect(wrapper.state().loadingSourcesAfter).toBe(true);

expect(defaultLoadIssues).toBeCalledTimes(2);
expect(defaultLoadIssues).toBeCalledTimes(1);
expect(getSources).toBeCalledTimes(2);

await waitAndUpdate(wrapper);

+ 42
- 27
server/sonar-web/src/main/js/components/SourceViewer/components/LineCode.tsx View File

@@ -23,7 +23,12 @@ import { IssueSourceViewerScrollContext } from '../../../apps/issues/components/
import { LinearIssueLocation, SourceLine } from '../../../types/types';
import LocationIndex from '../../common/LocationIndex';
import Tooltip from '../../controls/Tooltip';
import { highlightIssueLocations, highlightSymbol, splitByTokens } from '../helpers/highlight';
import {
highlightIssueLocations,
highlightSymbol,
splitByTokens,
Token
} from '../helpers/highlight';

interface Props {
className?: string;
@@ -76,6 +81,37 @@ export default class LineCode extends React.PureComponent<React.PropsWithChildre
}
};

renderToken(tokens: Token[]) {
const { highlightedLocationMessage, secondaryIssueLocations } = this.props;
const renderedTokens: React.ReactNode[] = [];

// track if the first marker is displayed before the source code
// set `false` for the first token in a row
let leadingMarker = false;

tokens.forEach((token, index) => {
if (this.props.displayLocationMarkers && token.markers.length > 0) {
token.markers.forEach(marker => {
const selected =
highlightedLocationMessage !== undefined && highlightedLocationMessage.index === marker;
const loc = secondaryIssueLocations.find(loc => loc.index === marker);
const message = loc && loc.text;
renderedTokens.push(this.renderMarker(marker, message, selected, leadingMarker));
});
}
renderedTokens.push(
// eslint-disable-next-line react/no-array-index-key
<span className={token.className} key={index}>
{token.text}
</span>
);

// keep leadingMarker truthy if previous token has only whitespaces
leadingMarker = (index === 0 ? true : leadingMarker) && !token.text.trim().length;
});
return renderedTokens;
}

renderMarker(index: number, message: string | undefined, selected: boolean, leading: boolean) {
const { onLocationSelect } = this.props;
const onClick = onLocationSelect ? () => onLocationSelect(index) : undefined;
@@ -112,7 +148,10 @@ export default class LineCode extends React.PureComponent<React.PropsWithChildre
secondaryIssueLocations
} = this.props;

let tokens = splitByTokens(this.props.line.code || '');
const container = document.createElement('div');
container.innerHTML = this.props.line.code || '';

let tokens = splitByTokens(container.childNodes);

if (highlightedSymbols) {
highlightedSymbols.forEach(symbol => {
@@ -137,31 +176,7 @@ export default class LineCode extends React.PureComponent<React.PropsWithChildre
}
}

const renderedTokens: React.ReactNode[] = [];

// track if the first marker is displayed before the source code
// set `false` for the first token in a row
let leadingMarker = false;

tokens.forEach((token, index) => {
if (this.props.displayLocationMarkers && token.markers.length > 0) {
token.markers.forEach(marker => {
const selected =
highlightedLocationMessage !== undefined && highlightedLocationMessage.index === marker;
const loc = secondaryIssueLocations.find(loc => loc.index === marker);
const message = loc && loc.text;
renderedTokens.push(this.renderMarker(marker, message, selected, leadingMarker));
});
}
renderedTokens.push(
<span className={token.className} key={index}>
{token.text}
</span>
);

// keep leadingMarker truthy if previous token has only whitespaces
leadingMarker = (index === 0 ? true : leadingMarker) && !token.text.trim().length;
});
const renderedTokens = this.renderToken(tokens);

const style = padding ? { paddingBottom: `${padding}px` } : undefined;


+ 1
- 1
server/sonar-web/src/main/js/components/SourceViewer/helpers/__tests__/loadIssues-test.ts View File

@@ -82,7 +82,7 @@ jest.mock('../../../../api/issues', () => ({

describe('loadIssues', () => {
it('should load issues', async () => {
const result = await loadIssues('foo.java', 1, 500, mockMainBranch());
const result = await loadIssues('foo.java', mockMainBranch());
expect(result).toMatchSnapshot();
});
});

+ 3
- 5
server/sonar-web/src/main/js/components/SourceViewer/helpers/highlight.ts View File

@@ -28,15 +28,13 @@ export interface Token {

const ISSUE_LOCATION_CLASS = 'source-line-code-issue';

export function splitByTokens(code: string, rootClassName = ''): Token[] {
const container = document.createElement('div');
export function splitByTokens(code: NodeListOf<ChildNode>, rootClassName = ''): Token[] {
let tokens: Token[] = [];
container.innerHTML = code;
[].forEach.call(container.childNodes, (node: Element) => {
Array.prototype.forEach.call(code, (node: Element) => {
if (node.nodeType === 1) {
// ELEMENT NODE
const fullClassName = rootClassName ? rootClassName + ' ' + node.className : node.className;
const innerTokens = splitByTokens(node.innerHTML, fullClassName);
const innerTokens = splitByTokens(node.childNodes, fullClassName);
tokens = tokens.concat(innerTokens);
}
if (node.nodeType === 3 && node.nodeValue) {

+ 3
- 1
server/sonar-web/src/main/js/components/SourceViewer/helpers/indexing.ts View File

@@ -86,7 +86,9 @@ export function duplicationsByLine(duplications: Duplication[] | undefined) {
export function symbolsByLine(sources: SourceLine[]) {
const index: { [line: number]: string[] } = {};
sources.forEach(line => {
const tokens = splitByTokens(line.code || '');
const container = document.createElement('div');
container.innerHTML = line.code || '';
const tokens = splitByTokens(container.childNodes);
const symbols = flatten(
tokens.map(token => {
const keys = token.className.match(/sym-\d+/g);

+ 1
- 1
server/sonar-web/src/main/js/components/SourceViewer/helpers/lines.ts View File

@@ -20,7 +20,7 @@
import { intersection } from 'lodash';
import { LinearIssueLocation } from '../../../types/types';

export const LINES_TO_LOAD = 500;
export const LINES_TO_LOAD = 1000;

export function optimizeHighlightedSymbols(
symbolsForLine: string[] = [],

+ 14
- 29
server/sonar-web/src/main/js/components/SourceViewer/helpers/loadIssues.ts View File

@@ -25,6 +25,8 @@ import { Issue, RawQuery } from '../../../types/types';

// maximum possible value
const PAGE_SIZE = 500;
// Maximum issues return 20*500 for the API.
const PAGE_MAX = 20;

function buildQuery(component: string, branchLike: BranchLike | undefined) {
return {
@@ -36,7 +38,7 @@ function buildQuery(component: string, branchLike: BranchLike | undefined) {
};
}

export function loadPage(query: RawQuery, page: number, pageSize = PAGE_SIZE): Promise<Issue[]> {
function loadPage(query: RawQuery, page: number, pageSize = PAGE_SIZE): Promise<Issue[]> {
return searchIssues({
...query,
p: page,
@@ -46,42 +48,25 @@ export function loadPage(query: RawQuery, page: number, pageSize = PAGE_SIZE): P
);
}

export function loadPageAndNext(
query: RawQuery,
toLine: number,
page: number,
pageSize = PAGE_SIZE
): Promise<Issue[]> {
return loadPage(query, page).then(issues => {
if (issues.length === 0) {
return [];
}
async function loadPageAndNext(query: RawQuery, page = 1, pageSize = PAGE_SIZE): Promise<Issue[]> {
const issues = await loadPage(query, page);

const lastIssue = issues[issues.length - 1];
if (issues.length === 0) {
return [];
}

if (
(lastIssue.textRange != null && lastIssue.textRange.endLine > toLine) ||
issues.length < pageSize
) {
return issues;
}
if (issues.length < pageSize || page >= PAGE_MAX) {
return issues;
}

return loadPageAndNext(query, toLine, page + 1, pageSize).then(nextIssues => {
return [...issues, ...nextIssues];
});
});
const nextIssues = await loadPageAndNext(query, page + 1, pageSize);
return [...issues, ...nextIssues];
}

export default function loadIssues(
component: string,
_fromLine: number,
toLine: number,
branchLike: BranchLike | undefined
): Promise<Issue[]> {
const query = buildQuery(component, branchLike);
return new Promise(resolve => {
loadPageAndNext(query, toLine, 1).then(issues => {
resolve(issues);
});
});
return loadPageAndNext(query);
}

Loading…
Cancel
Save