Commit 88e63ada authored by Thomas Randolph's avatar Thomas Randolph

Separate metadata processing from normal diff processing

There are basically two changes here:

- Make the argument to prepareDiffData an object so that
    the arguments list isn't unwieldy (majority of changes)
- Skip identifying diff files when the preparation is for metadata,
    since those diff "files" don't have uniquely identifying data
parent fea95f88
...@@ -191,7 +191,13 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { ...@@ -191,7 +191,13 @@ export const fetchDiffFilesMeta = ({ commit, state }) => {
commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []); commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []);
commit(types.SET_DIFF_METADATA, strippedData); commit(types.SET_DIFF_METADATA, strippedData);
worker.postMessage(prepareDiffData(data, state.diffFiles)); worker.postMessage(
prepareDiffData({
diff: data,
priorFiles: state.diffFiles,
meta: true,
}),
);
return data; return data;
}) })
......
...@@ -73,7 +73,10 @@ export default { ...@@ -73,7 +73,10 @@ export default {
}, },
[types.SET_DIFF_DATA_BATCH](state, data) { [types.SET_DIFF_DATA_BATCH](state, data) {
const files = prepareDiffData(data, state.diffFiles); const files = prepareDiffData({
diff: data,
priorFiles: state.diffFiles,
});
Object.assign(state, { Object.assign(state, {
...convertObjectPropsToCamelCase(data), ...convertObjectPropsToCamelCase(data),
...@@ -143,7 +146,7 @@ export default { ...@@ -143,7 +146,7 @@ export default {
}, },
[types.ADD_COLLAPSED_DIFFS](state, { file, data }) { [types.ADD_COLLAPSED_DIFFS](state, { file, data }) {
const files = prepareDiffData(data); const files = prepareDiffData({ diff: data });
const [newFileData] = files.filter(f => f.file_hash === file.file_hash); const [newFileData] = files.filter(f => f.file_hash === file.file_hash);
const selectedFile = state.diffFiles.find(f => f.file_hash === file.file_hash); const selectedFile = state.diffFiles.find(f => f.file_hash === file.file_hash);
Object.assign(selectedFile, { ...newFileData }); Object.assign(selectedFile, { ...newFileData });
......
...@@ -386,9 +386,9 @@ function deduplicateFilesList(files) { ...@@ -386,9 +386,9 @@ function deduplicateFilesList(files) {
return Object.values(dedupedFiles); return Object.values(dedupedFiles);
} }
export function prepareDiffData(diff, priorFiles = []) { export function prepareDiffData({ diff, priorFiles = [], meta = false }) {
const cleanedFiles = (diff.diff_files || []) const cleanedFiles = (diff.diff_files || [])
.map((file, index, allFiles) => prepareRawDiffFile({ file, allFiles })) .map((file, index, allFiles) => prepareRawDiffFile({ file, allFiles, meta }))
.map(ensureBasicDiffFileLines) .map(ensureBasicDiffFileLines)
.map(prepareDiffFileLines) .map(prepareDiffFileLines)
.map(finalizeDiffFile); .map(finalizeDiffFile);
......
...@@ -33,23 +33,26 @@ function collapsed(file) { ...@@ -33,23 +33,26 @@ function collapsed(file) {
}; };
} }
export function identifier({ file }) { function identifier(file) {
return uuids({ return uuids({
seeds: [file.file_identifier_hash, file.content_sha], seeds: [file.file_identifier_hash, file.content_sha],
})[0]; })[0];
} }
export function prepareRawDiffFile({ file, allFiles }) { export function prepareRawDiffFile({ file, allFiles, meta = false }) {
Object.assign(file, { const additionalProperties = {
id: identifier({ file }),
brokenSymlink: fileSymlinkInformation(file, allFiles), brokenSymlink: fileSymlinkInformation(file, allFiles),
viewer: { viewer: {
...file.viewer, ...file.viewer,
...collapsed(file), ...collapsed(file),
}, },
}); };
if (!meta) {
additionalProperties.id = identifier(file);
}
return file; return Object.assign(file, additionalProperties);
} }
export function collapsedType(file) { export function collapsedType(file) {
......
...@@ -409,10 +409,13 @@ describe('DiffsStoreUtils', () => { ...@@ -409,10 +409,13 @@ describe('DiffsStoreUtils', () => {
diff_files: [{ ...mock, [INLINE_DIFF_LINES_KEY]: undefined }], diff_files: [{ ...mock, [INLINE_DIFF_LINES_KEY]: undefined }],
}; };
preparedDiff.diff_files = utils.prepareDiffData(preparedDiff); preparedDiff.diff_files = utils.prepareDiffData({ diff: preparedDiff });
splitInlineDiff.diff_files = utils.prepareDiffData(splitInlineDiff); splitInlineDiff.diff_files = utils.prepareDiffData({ diff: splitInlineDiff });
splitParallelDiff.diff_files = utils.prepareDiffData(splitParallelDiff); splitParallelDiff.diff_files = utils.prepareDiffData({ diff: splitParallelDiff });
completedDiff.diff_files = utils.prepareDiffData(completedDiff, [mock]); completedDiff.diff_files = utils.prepareDiffData({
diff: completedDiff,
priorFiles: [mock],
});
}); });
it('sets the renderIt and collapsed attribute on files', () => { it('sets the renderIt and collapsed attribute on files', () => {
...@@ -447,7 +450,10 @@ describe('DiffsStoreUtils', () => { ...@@ -447,7 +450,10 @@ describe('DiffsStoreUtils', () => {
content_sha: 'ABC', content_sha: 'ABC',
file_hash: 'DEF', file_hash: 'DEF',
}; };
const updatedFilesList = utils.prepareDiffData({ diff_files: [fakeNewFile] }, priorFiles); const updatedFilesList = utils.prepareDiffData({
diff: { diff_files: [fakeNewFile] },
priorFiles,
});
expect(updatedFilesList).toEqual([mock, fakeNewFile]); expect(updatedFilesList).toEqual([mock, fakeNewFile]);
}); });
...@@ -460,7 +466,10 @@ describe('DiffsStoreUtils', () => { ...@@ -460,7 +466,10 @@ describe('DiffsStoreUtils', () => {
{ ...mock, [INLINE_DIFF_LINES_KEY]: undefined }, { ...mock, [INLINE_DIFF_LINES_KEY]: undefined },
{ ...mock, [INLINE_DIFF_LINES_KEY]: undefined, content_sha: 'ABC', file_hash: 'DEF' }, { ...mock, [INLINE_DIFF_LINES_KEY]: undefined, content_sha: 'ABC', file_hash: 'DEF' },
]; ];
const updatedFilesList = utils.prepareDiffData({ diff_files: fakeBatch }, priorFiles); const updatedFilesList = utils.prepareDiffData({
diff: { diff_files: fakeBatch },
priorFiles,
});
expect(updatedFilesList).toEqual([ expect(updatedFilesList).toEqual([
mock, mock,
...@@ -498,7 +507,7 @@ describe('DiffsStoreUtils', () => { ...@@ -498,7 +507,7 @@ describe('DiffsStoreUtils', () => {
beforeEach(() => { beforeEach(() => {
mock = getDiffMetadataMock(); mock = getDiffMetadataMock();
preparedDiffFiles = utils.prepareDiffData(mock); preparedDiffFiles = utils.prepareDiffData({ diff: mock, meta: true });
}); });
it('sets the renderIt and collapsed attribute on files', () => { it('sets the renderIt and collapsed attribute on files', () => {
...@@ -514,7 +523,7 @@ describe('DiffsStoreUtils', () => { ...@@ -514,7 +523,7 @@ describe('DiffsStoreUtils', () => {
const fileMock = getDiffFileMock(); const fileMock = getDiffFileMock();
const metaData = getDiffMetadataMock(); const metaData = getDiffMetadataMock();
const priorFiles = [fileMock]; const priorFiles = [fileMock];
const updatedFilesList = utils.prepareDiffData(metaData, priorFiles); const updatedFilesList = utils.prepareDiffData({ diff: metaData, priorFiles, meta: true });
expect(updatedFilesList.length).toEqual(2); expect(updatedFilesList.length).toEqual(2);
expect(updatedFilesList[0]).toEqual(fileMock); expect(updatedFilesList[0]).toEqual(fileMock);
...@@ -539,7 +548,7 @@ describe('DiffsStoreUtils', () => { ...@@ -539,7 +548,7 @@ describe('DiffsStoreUtils', () => {
const fileMock = getDiffFileMock(); const fileMock = getDiffFileMock();
const metaMock = getDiffMetadataMock(); const metaMock = getDiffMetadataMock();
const priorFiles = [{ ...fileMock }]; const priorFiles = [{ ...fileMock }];
const updatedFilesList = utils.prepareDiffData(metaMock, priorFiles); const updatedFilesList = utils.prepareDiffData({ diff: metaMock, priorFiles, meta: true });
expect(updatedFilesList).toEqual([ expect(updatedFilesList).toEqual([
fileMock, fileMock,
......
...@@ -2,24 +2,44 @@ import { prepareRawDiffFile } from '~/diffs/utils/diff_file'; ...@@ -2,24 +2,44 @@ import { prepareRawDiffFile } from '~/diffs/utils/diff_file';
const DIFF_FILES = [ const DIFF_FILES = [
{ {
blob: {
id: 'abc',
mode: '100644',
},
file_hash: 'ABC', // This file is just a normal file file_hash: 'ABC', // This file is just a normal file
}, },
{ {
blob: {
id: 'bcd',
mode: '100644',
},
file_hash: 'DEF', // This file replaces a symlink file_hash: 'DEF', // This file replaces a symlink
a_mode: '0', a_mode: '0',
b_mode: '0755', b_mode: '0755',
}, },
{ {
blob: {
id: 'cde',
mode: '100644',
},
file_hash: 'DEF', // This symlink is replaced by a file file_hash: 'DEF', // This symlink is replaced by a file
a_mode: '120000', a_mode: '120000',
b_mode: '0', b_mode: '0',
}, },
{ {
blob: {
id: 'def',
mode: '100644',
},
file_hash: 'GHI', // This symlink replaces a file file_hash: 'GHI', // This symlink replaces a file
a_mode: '0', a_mode: '0',
b_mode: '120000', b_mode: '120000',
}, },
{ {
blob: {
id: 'efg',
mode: '100644',
},
file_hash: 'GHI', // This file is replaced by a symlink file_hash: 'GHI', // This file is replaced by a symlink
a_mode: '0755', a_mode: '0755',
b_mode: '0', b_mode: '0',
......
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